llvm dev - Nov 2010 - [LLVMdev] [llvm-commits] [patch] ARM/MC/ELF add new stub for movt/movw in ARMFixupKinds

+llvmdev
-llvmcommits

On Fri, Nov 12, 2010 at 8:03 AM, Jim Grosbach <grosbach at apple.com>
wrote:
> Sorta. getBinaryCodeForInst() is auto-generated by tablegen, so
shouldn't be modified directly. The target can register hooks for
instruction operands for any special encoding needs, including registering
fixups, using the EncoderMethod string. For an example, have a look at the
LDRi12 instruction and how it registers a fixup for the addrmode_imm12 operand
when it needs one.
Hi Jim,. follow up question for ya:

The current movt/movw pair (as defined in ARMInstrInfo.td)  does not
use EncoderMethod string to declare  a special case handler.

At the current time, for the assembly printing,
MCAsmStreamer::EmitInstruction(const MCInst &Inst) calls out to
  MCExpr::print(raw_ostream &OS)
   which then calls out to MCSymbolRefExpr::getVariantKindName() to
print the magic :lower16: and :upper16: asm tags for .s emission
Currently, movt/movw emission works correctly in .s, but not in .o emission

This lead me to believe that the correct place to put the code to handle
MCSymbolRefExpr::VK_ARM_(HI||LO)16 for the .o path was to place a case
in getMachineOpValue() (i.e. not
ARMMCCodeEmitter::getBinaryCodeForInstr like I mistakenly wrote in my
prior email.)

Are you implying that the movt/movw instruction definition in the .td
files need to be fixed up instead to declare a new special case for .o
emission via the EncoderMethod string, for the .o emission of
movt/movw to be considered "correct"?
(If so, I have a few more followup^2 quesitions :-)

Thanks!

-jason
>
> On Nov 11, 2010, at 7:06 PM, Jason Kim wrote:
>
>> Is getBinaryCodeForInst the best place to place the case for
>> supporting movt/movw fixup emission?
>> The call stack seems to be:
>>
>> #0 ARMMCCodeEmitter::getBinaryCodeForInstr
>> #1 ARMMCCodeEmitter::EncodeInstruction
>> #2 MCELFStreamer::EmitInstToData
>> #3 MCObjectStreamer::EmitInstruction
>> #4 ARMAsmPrinter::EmitInstruction
>>
<arm-mc-elf-s08.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>

On Nov 16, 2010, at 4:01 PM, Jason Kim wrote:
> +llvmdev
> -llvmcommits
> 
> On Fri, Nov 12, 2010 at 8:03 AM, Jim Grosbach <grosbach at apple.com>
wrote:
>> Sorta. getBinaryCodeForInst() is auto-generated by tablegen, so
shouldn't be modified directly. The target can register hooks for
instruction operands for any special encoding needs, including registering
fixups, using the EncoderMethod string. For an example, have a look at the
LDRi12 instruction and how it registers a fixup for the addrmode_imm12 operand
when it needs one.
> 
> Hi Jim,. follow up question for ya:
> 
> The current movt/movw pair (as defined in ARMInstrInfo.td)  does not
> use EncoderMethod string to declare  a special case handler.
There's two parts to how this works. First, is the handling of the movt/movw
pair for instruction selection, and then the handling of the upper/lower operand
flags for those instructions.

MOVi32imm is expanded as a pseudo instruction in ARMExpandPseudos pass, and any
other patterns that do similar things should also be expanded there. For a few
special cases where we need the post-RA scheduler to handle the instruction
sequences as an atomic unit (PICLDR, PICADD, the eh.sjlj.setjmp/longjmp
patterns, for example), we expand them even later, during MC lowering in
ARMAsmPrinter.cpp. As a result, the movt and movw instructions should make it
into the code emitter as separate entries, not as a composite pair.

For operands that aren't compile-time constants, the operand flag gets set
in the expansion to the MOVT/MOVW instructions:
      if (MO.isImm()) {
        unsigned Imm = MO.getImm();
        unsigned Lo16 = Imm & 0xffff;
        unsigned Hi16 = (Imm >> 16) & 0xffff;
        LO16 = LO16.addImm(Lo16);
        HI16 = HI16.addImm(Hi16);
      } else {
        const GlobalValue *GV = MO.getGlobal();
        unsigned TF = MO.getTargetFlags();
        LO16 = LO16.addGlobalAddress(GV, MO.getOffset(), TF | ARMII::MO_LO16);
        HI16 = HI16.addGlobalAddress(GV, MO.getOffset(), TF | ARMII::MO_HI16);
      }

That's then marked with VK_ARM_LO16/VK_ARM_HI16 by ARMMCInstLower.cpp when
it's looking at symbol reference operands.

> At the current time, for the assembly printing,
> MCAsmStreamer::EmitInstruction(const MCInst &Inst) calls out to
>  MCExpr::print(raw_ostream &OS)
>   which then calls out to MCSymbolRefExpr::getVariantKindName() to
> print the magic :lower16: and :upper16: asm tags for .s emission
> Currently, movt/movw emission works correctly in .s, but not in .o emission
> 
> This lead me to believe that the correct place to put the code to handle
> MCSymbolRefExpr::VK_ARM_(HI||LO)16 for the .o path was to place a case
> in getMachineOpValue() (i.e. not
> ARMMCCodeEmitter::getBinaryCodeForInstr like I mistakenly wrote in my
> prior email.)
> 
> Are you implying that the movt/movw instruction definition in the .td
> files need to be fixed up instead to declare a new special case for .o
> emission via the EncoderMethod string, for the .o emission of
> movt/movw to be considered "correct"?
A custom encoder may be the best way to differentiate the kinds of fixups
we'll need for these operands (e.g., using LO16 may generate different
fixups depending on which instruction it's used for, which implies different
operand node types, and thus different encoder methods). In any case, the
encoder, be it in getMachineOpValue() or a specialized method, will need to
differentiate between an immediate operand and a symbol reference expression
operand. In the latter case, it'll create a fixup for the lo/hi variantkind
if one is present.

-Jim
>> 
>> On Nov 11, 2010, at 7:06 PM, Jason Kim wrote:
>> 
>>> Is getBinaryCodeForInst the best place to place the case for
>>> supporting movt/movw fixup emission?
>>> The call stack seems to be:
>>> 
>>> #0 ARMMCCodeEmitter::getBinaryCodeForInstr
>>> #1 ARMMCCodeEmitter::EncodeInstruction
>>> #2 MCELFStreamer::EmitInstToData
>>> #3 MCObjectStreamer::EmitInstruction
>>> #4 ARMAsmPrinter::EmitInstruction
>>>
<arm-mc-elf-s08.patch>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20101116/68342a8e/attachment.html>