llvm dev - Jun 2009 - [LLVMdev] proposal to simplify isel/asmprinter interaction with globals

Hi All,

I'm working on various cleanups and simplifications to the  
asmprinters.  One thing that is driving me nuts is that the  
asmprinters currently "reverse engineer" a lot of information when  
printing an operand that isel had when it created it.

I'm specifically talking about all the suffixes generated by isel,  
like $non_lazy_ptr, @TLSGD, @NTPOFF, (%rip) etc.  These are all  
"magic" things that the assembler uses (for example, it generates a  
reference to a global's GOT entry instead of to the global itself).

The thing that is really frustrating to me is that the asmprinters  
need to reverse engineer what isel *meant* in order to output the  
right thing.  For example, to determine whether to emit $non_lazy_ptr,  
it uses a logic tree like this:

     if (shouldPrintStub(TM, Subtarget)) {
       // Link-once, declaration, or Weakly-linked global variables need
       // non-lazily-resolved stubs
       if (GV->isDeclaration() || GV->isWeakForLinker()) {
         // Dynamically-resolved functions need a stub for the function.
         if (GV->hasHiddenVisibility()) {
           if (!GV->isDeclaration() && !GV->hasCommonLinkage())
   ...
           else {
             printSuffixedName(Name, "$non_lazy_ptr");
           }
         } else {
           printSuffixedName(Name, "$non_lazy_ptr");
         }

  where shouldPrintStub is:

static inline bool shouldPrintStub(TargetMachine &TM, const  
X86Subtarget* ST) {
   return ST->isPICStyleStub() && TM.getRelocationModel() !=  
Reloc::Static;
}

This is really redundant with isel, because isel also needs to know  
exactly which GV references go through a stub so that it isels the  
access correctly.

My proposed fix for this is to add an 'unsigned char' slot to  
MachineOperand that holds a target-specific enum value.  The code in  
asmprinter would be reduced to:

switch (theoperandenum) {
case X86::MO_Flag_non_lazy_ptr:
   O << "$non_lazy_ptr";
   break;
case X86::MO_Flag_TLSGD:
   O << "@TLSGD";
   break;
case X86::MO_Flag_NTPOFF:
   O << "@NTPOFF";
   break;


etc.  The possible set of suffixes and modifiers are all target- 
specific, so the main code generator would just pass them through (as  
it does now).

Does anyone have any objections to this?

-Chris

Hi, Chris
> Does anyone have any objections to this?
Sounds good. I also thought about this as a part of the whole program
to refactor asm emission logic. Unfortunately I had no time to
implement this by myself :(

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

On Jun 20, 2009, at 7:30 PM, Chris Lattner wrote:
>
>
> My proposed fix for this is to add an 'unsigned char' slot to
> MachineOperand that holds a target-specific enum value.  The code in
> asmprinter would be reduced to:
>
> switch (theoperandenum) {
> case X86::MO_Flag_non_lazy_ptr:
>   O << "$non_lazy_ptr";
>   break;
> case X86::MO_Flag_TLSGD:
>   O << "@TLSGD";
>   break;
> case X86::MO_Flag_NTPOFF:
>   O << "@NTPOFF";
>   break;
>
>
> etc.  The possible set of suffixes and modifiers are all target-
> specific, so the main code generator would just pass them through (as
> it does now).
>
> Does anyone have any objections to this?
Can you reorg MachineOperand fields while you are at it? :-) Right now  
each MachineOperand uses 8 bits for type, followed by 5 bits for  
various flags, then a whole 8-bit for subreg.  If we use just 3-bit  
for subreg (should be enough?), the target independent part will just  
take up 16-bits and leave plenty of room for target flags.

Evan
>
> -Chris
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Jun 22, 2009, at 11:49 AM, Evan Cheng wrote:
>>
>> etc.  The possible set of suffixes and modifiers are all target-
>> specific, so the main code generator would just pass them through (as
>> it does now).
>>
>> Does anyone have any objections to this?
>
> Can you reorg MachineOperand fields while you are at it? :-) Right now
> each MachineOperand uses 8 bits for type, followed by 5 bits for
> various flags, then a whole 8-bit for subreg.  If we use just 3-bit
> for subreg (should be enough?), the target independent part will just
> take up 16-bits and leave plenty of room for target flags.
Yep, I'll clean this up when I get a chance,

-Chris

Ok, not too much screaming :), I'll see what I can do.

-Chris

On Jun 20, 2009, at 7:30 PM, Chris Lattner wrote:
> Hi All,
>
> I'm working on various cleanups and simplifications to the
> asmprinters.  One thing that is driving me nuts is that the
> asmprinters currently "reverse engineer" a lot of information
when
> printing an operand that isel had when it created it.
>
> I'm specifically talking about all the suffixes generated by isel,
> like $non_lazy_ptr, @TLSGD, @NTPOFF, (%rip) etc.  These are all
> "magic" things that the assembler uses (for example, it generates
a
> reference to a global's GOT entry instead of to the global itself).
>
> The thing that is really frustrating to me is that the asmprinters
> need to reverse engineer what isel *meant* in order to output the
> right thing.  For example, to determine whether to emit $non_lazy_ptr,
> it uses a logic tree like this:
>
>     if (shouldPrintStub(TM, Subtarget)) {
>       // Link-once, declaration, or Weakly-linked global variables  
> need
>       // non-lazily-resolved stubs
>       if (GV->isDeclaration() || GV->isWeakForLinker()) {
>         // Dynamically-resolved functions need a stub for the  
> function.
>         if (GV->hasHiddenVisibility()) {
>           if (!GV->isDeclaration() &&
!GV->hasCommonLinkage())
>   ...
>           else {
>             printSuffixedName(Name, "$non_lazy_ptr");
>           }
>         } else {
>           printSuffixedName(Name, "$non_lazy_ptr");
>         }
>
>  where shouldPrintStub is:
>
> static inline bool shouldPrintStub(TargetMachine &TM, const
> X86Subtarget* ST) {
>   return ST->isPICStyleStub() && TM.getRelocationModel() !>
Reloc::Static;
> }
>
> This is really redundant with isel, because isel also needs to know
> exactly which GV references go through a stub so that it isels the
> access correctly.
>
> My proposed fix for this is to add an 'unsigned char' slot to
> MachineOperand that holds a target-specific enum value.  The code in
> asmprinter would be reduced to:
>
> switch (theoperandenum) {
> case X86::MO_Flag_non_lazy_ptr:
>   O << "$non_lazy_ptr";
>   break;
> case X86::MO_Flag_TLSGD:
>   O << "@TLSGD";
>   break;
> case X86::MO_Flag_NTPOFF:
>   O << "@NTPOFF";
>   break;
>
>
> etc.  The possible set of suffixes and modifiers are all target-
> specific, so the main code generator would just pass them through (as
> it does now).
>
> Does anyone have any objections to this?
>
> -Chris
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev