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 :frowning:

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? :slight_smile: 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

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