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

[Jason Kim]

-llvmdev
+llvmcommits

On Tue, Nov 16, 2010 at 4:27 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
> 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.

Got ya. Is this more in tune with what you had in mind? (applies cleanly to
-r119149)

Thanks!

> -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 --------------
A non-text attachment was scrubbed...
Name: arm-mc-elf-s09a-movt.patch
Type: text/x-patch
Size: 5581 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101117/2cfea766/attachment.bin>