AsmPrinter: Produce PC-relative cstexprs when possible.

[jckarter]

Summary

When a constant expr is relative to the constant foo currently being emitted, transform it from the form bar-foo+n to bar-.+m. MC will naively assemble the former version into a subtractor relocation, which isn't always supported, for instance in most ELF platforms. PC-relative relocations with offsets are more widely supported and better tested in most linkers.

[lhames]

CC'ing Nick.

[lhames]

Hi Joe,

This looks fine on the MachO side. I also tested it with ELF on x86_64 and AArch64, where it fixed the test case you included.

Everything looks good to me, but Rafael and Rui should probably sign off on this, since it touches ELF and COFF too. Rafael, Rui - do you see any problems with this?

• Lang.

[jckarter]

Thanks Lang! Rafael and Rui, do you think you'll be able to take a look soon?

[ruiu]

This change looks good to me, but I'm not really an owner of this part of the code, so I guess you want to have Rafael take a look.

[jckarter]

Thanks Rui. Rafael, have you had a chance to look over this yet?

[• rafael]

looking, sorry for the delay.

[• rafael]

It seems odd to do this at the asm *printer*

Currently the ELF testcase prints

.long .Lbar-foo
.long .Lbar-foo

And with gas that produces R_X86_64_PC32. In contrast MC produces an error:

Cannot represent a subtraction with a weak symbol.

One of the many horrible things about the .s format is that there is no way to differentiate "this symbol foo" versus "whatever symbol foo resolves to". MC Is trying the second one and failing. Gas is using the first one and finding a relocation that works.

I will take a quick look to see what the change would be like for MC to behave like gas on this one.

Is it possible that a similar issue is impacting MachO?

[jckarter]

No worries, thanks for the feedback, Rafael. I was following the example of how we handle rewriting relative references to GOT-equivalent globals into GOTPCREL relocations, which also happens in the AsmPrinter. It'd be great if MC could be made smarter about choosing relocations. The issue also impacts Mach-O, though unlike ELF, Mach-O does support generalized subtractor relocations, so it isn't a showstopper there. These subtractor relocations have however been giving me heartache, exposing unused corners of Darwin's ld and MCJIT, so I think it's better to favor PCREL relocation if we can get them on Mach-O too, since those are much better exercised.

[jckarter]

Can confirm that r258329 addresses my use case. Thanks again Rafael. I'll file a separate PR for Mach-O; like I said, it's not a showstopper there.

[lhames]

I will take a quick look to see what the change would be like for MC to behave like gas on this one.

That's a much nicer fix. Thanks Rafael!

Joe - should we close this revision?

[jckarter]

Yeah, we can close this. Thanks Lang, Rui, and Rafael for taking a look.

[jckarter]

Closing.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

     AP.GlobalGOTEquivs[GOTEquivSym] = std::make_pair(GV, NumUses);
 }
 
+/// \brief Transform a not absolute MCExpr `something - foo + N` to
+/// PC-relative `something - . + M` if we're based on `foo`.
+static void handleRelativeSymViaPCRel(AsmPrinter &AP, const MCExpr **ME,
+                                      const Constant *BaseCst,
+                                      uint64_t Offset) {
+  MCValue MV;
+  if (!(*ME)->evaluateAsRelocatable(MV, nullptr, nullptr) || MV.isAbsolute())
+    return;
+  const MCSymbolRefExpr *SymA = MV.getSymA();
+  if (!SymA)
+    return;
+
+  const GlobalValue *BaseGV = dyn_cast_or_null<GlobalValue>(BaseCst);
+  if (!BaseGV)
+    return;
+  
+  const MCSymbol *BaseSym = AP.getSymbol(BaseGV);
+  const MCSymbolRefExpr *SymB = MV.getSymB();
+  
+  if (!SymB || BaseSym != &SymB->getSymbol())
+    return;
+
+  int64_t PCRelCst = Offset + MV.getConstant();
+  
+  // Emit a local symbol for the current position to base the rewritten
+  // reference on.
+  MCSymbol *PCSym = AP.OutContext.createTempSymbol();
+  AP.OutStreamer->EmitLabel(PCSym);
+  
+  const MCExpr *PCSymRef = MCSymbolRefExpr::create(PCSym, AP.OutContext);
+  const MCExpr *PCRelRef
+    = MCBinaryExpr::createSub(SymA, PCSymRef, AP.OutContext);
+  if (PCRelCst != 0) {
+    const MCExpr *Cst = MCConstantExpr::create(PCRelCst, AP.OutContext);
+    PCRelRef = MCBinaryExpr::createAdd(PCRelRef, Cst, AP.OutContext);
+  }
+  *ME = PCRelRef;
+}
+
 static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV,
                                    AsmPrinter &AP, const Constant *BaseCV,
                                    uint64_t Offset) {

   if (AP.getObjFileLowering().supportIndirectSymViaGOTPCRel())
     handleIndirectSymViaGOTPCRel(AP, &ME, BaseCV, Offset);
 
+  if (BaseCV)
+    handleRelativeSymViaPCRel(AP, &ME, BaseCV, Offset);
+  
   AP.OutStreamer->EmitValue(ME, Size);
 }
 

test/MC/COFF/cross-section-relative.ll

 
 ;;;; cross-section relative relocations
 
-; CHECK: .quad (g3-t1)+4
+; CHECK:      t1:
+; CHECK-NEXT: [[PC:\.Ltmp.*]]:
+; CHECK-NEXT:  .quad (g3-[[PC]])+4
 @t1 = global i64 add(i64 sub(i64 ptrtoint(i32* @g3 to i64), i64 ptrtoint(i64* @t1 to i64)), i64 4), section ".fix"
-; CHECK: .quad g3-t2
+; CHECK:      t2:
+; CHECK-NEXT: [[PC:\.Ltmp.*]]:
+; CHECK-NEXT:  .quad g3-[[PC]]
 @t2 = global i64 sub(i64 ptrtoint(i32* @g3 to i64), i64 ptrtoint(i64* @t2 to i64)), section ".fix"
-; CHECK: .quad (g3-t3)-4
+; CHECK:      t3:
+; CHECK-NEXT: [[PC:\.Ltmp.*]]:
+; CHECK-NEXT:  .quad (g3-[[PC]])-4
 @t3 = global i64 sub(i64 sub(i64 ptrtoint(i32* @g3 to i64), i64 ptrtoint(i64* @t3 to i64)), i64 4), section ".fix"
-; CHECK: .long g3-t4
+; CHECK:      t4:
+; CHECK-NEXT: [[PC:\.Ltmp.*]]:
+; CHECK:       .long g3-[[PC]]
 @t4 = global i32 trunc(i64 sub(i64 ptrtoint(i32* @g3 to i64), i64 ptrtoint(i32* @t4 to i64)) to i32), section ".fix"
 
 ;;;; image base relocation

 
 %struct.EEType = type { [2 x i8], i64, i32}
 
-; CHECK: .long g3-(t6+16)
+; CHECK:      t6:
+; CHECK:      [[PC:\.Ltmp.*]]:
+; CHECK:       .long g3-[[PC]]
 @t6 = global %struct.EEType { 
         [2 x i8] c"\01\02", 
         i64 256,

test/MC/ELF/cstexpr-pcrel.ll

+; RUN: llc -mtriple=x86_64-pc-linux %s -o %t
+; RUN:  FileCheck %s < %t
+
+; CHECK-LABEL: {{^}}foo:
+; CHECK-NEXT:  [[PC1:\.Ltmp.*]]:
+; CHECK-NEXT:    .long .Lbar-[[PC1]]
+; CHECK-NEXT:  [[PC2:\.Ltmp.*]]:
+; CHECK-NEXT:    .long (.Lbar-[[PC2]])+4
+; CHECK-NEXT:  [[PC3:\.Ltmp.*]]:
+; CHECK-NEXT:    .long .Lbar-.Ltmp2
+
+@foo = linkonce_odr constant [3 x i32] [i32 trunc (i64 sub (i64 ptrtoint ([5 x i8]*  @bar to i64), i64 ptrtoint ([3 x i32]* @foo to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint ([5 x i8]*  @bar to i64), i64 ptrtoint ([3 x i32]* @foo to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint ([5 x i8]*  @bar to i64), i64 ptrtoint (i32* getelementptr ([3 x i32], [3 x i32]* @foo, i32 0, i32 2) to i64)) to i32)]
+
+@bar = private unnamed_addr constant [5 x i8] c"abcd\00"
+

test/MC/MachO/AArch64/cstexpr-gotpcrel.ll

 ; supported on x86-64 but not on ARM64
 
 ; CHECK: .long   5
-; CHECK-NEXT: .long (l_extgotequiv-(_table+44))+24
+; CHECK-NEXT: Ltmp3:
+; CHECK-NEXT: .long (l_extgotequiv-Ltmp3)+24
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 add (i32 trunc (i64 sub (i64 ptrtoint (i32** @extgotequiv to i64),
                                  i64 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i64 3, i32 1, i32 1) to i64))

 ; Test multiple uses of GOT equivalents.
 
 ; CHECK-LABEL: _delta
-; CHECK: Ltmp3:
-; CHECK-NEXT:  .long _extfoo@GOT-Ltmp3
+; CHECK: Ltmp4:
+; CHECK-NEXT:  .long _extfoo@GOT-Ltmp4
 @delta = global i32 trunc (i64 sub (i64 ptrtoint (i32** @extgotequiv to i64),
                                     i64 ptrtoint (i32* @delta to i64))
                            to i32)
 
 ; CHECK-LABEL: _deltaplus:
-; CHECK: .long  (l_localgotequiv-_deltaplus)+55
+; CHECK-NEXT:  Ltmp5:
+; CHECK: .long  (l_localgotequiv-Ltmp5)+55
 @deltaplus = global i32 add (i32 trunc (i64 sub (i64 ptrtoint (i32** @localgotequiv to i64),
                                         i64 ptrtoint (i32* @deltaplus to i64))
                                         to i32), i32 55)

test/MC/MachO/ARM/cstexpr-gotpcrel.ll

   %struct.data { i32 1, %struct.anon { i32 2, i32 3 } },
 ; Test GOT equivalent usage inside nested constant arrays.
 ; CHECK: .long   5
-; CHECK-NOT: l_localgotequiv-(_table+20)
-; CHECK-NEXT: L_localfoo$non_lazy_ptr-(_table+20)
+; CHECK-NEXT: Ltmp0:
+; CHECK-NOT:  l_localgotequiv-Ltmp0
+; CHECK-NEXT: L_localfoo$non_lazy_ptr-Ltmp0
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 sub (i32 ptrtoint (i32** @localgotequiv to i32),
              i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 1, i32 1, i32 1) to i32))}
   },
 ; CHECK: .long   5
-; CHECK-NOT: l_extgotequiv-(_table+32)
-; CHECK-NEXT: L_extfoo$non_lazy_ptr-(_table+32)
+; CHECK-NEXT: Ltmp1:
+; CHECK-NOT:  l_extgotequiv-Ltmp1
+; CHECK-NEXT: L_extfoo$non_lazy_ptr-Ltmp1
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
              i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 2, i32 1, i32 1) to i32))}
   },
 ; Test support for arbitrary constants into the GOTPCREL offset
 ; CHECK: .long   5
-; CHECK-NOT: (l_extgotequiv-(_table+44))+24
-; CHECK-NEXT: L_extfoo$non_lazy_ptr-(_table+20)
+; CHECK-NEXT: Ltmp2:
+; CHECK-NOT: (l_extgotequiv-Ltmp2)+24
+; CHECK-NEXT: (L_extfoo$non_lazy_ptr-Ltmp2)+24
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 add (i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
                       i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 3, i32 1, i32 1) to i32)),

 ], align 16
 
 ; Test multiple uses of GOT equivalents.
-; CHECK-LABEL: _delta
-; CHECK: .long   L_extfoo$non_lazy_ptr-_delta
+; CHECK-LABEL: _delta:
+; CHECK-NEXT:  Ltmp3:
+; CHECK-NEXT:   .long   L_extfoo$non_lazy_ptr-Ltmp3
 @delta = global i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
                          i32 ptrtoint (i32* @delta to i32))
 
 ; CHECK-LABEL: _deltaplus:
-; CHECK: .long   L_localfoo$non_lazy_ptr-(_deltaplus-55)
+; CHECK-NEXT:  Ltmp4:
+; CHECK-NEXT:   .long   (L_localfoo$non_lazy_ptr-Ltmp4)+55
 @deltaplus = global i32 add (i32 sub (i32 ptrtoint (i32** @localgotequiv to i32),
                              i32 ptrtoint (i32* @deltaplus to i32)),
                              i32 55)

test/MC/MachO/cstexpr-gotpcrel-32.ll

 ; CHECK-LABEL: _table
   %struct.data { i32 1, %struct.anon { i32 2, i32 3 } },
 ; Test GOT equivalent usage inside nested constant arrays.
-; CHECK: .long   5
-; CHECK-NOT: l_localgotequiv-(_table+20)
-; CHECK-NEXT: L_localfoo$non_lazy_ptr-(_table+20)
+; CHECK:      .long   5
+; CHECK-NEXT: Ltmp0:
+; CHECK-NOT:   l_localgotequiv-Ltmp0
+; CHECK-NEXT:  L_localfoo$non_lazy_ptr-Ltmp0
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 sub (i32 ptrtoint (i32** @localgotequiv to i32),
              i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 1, i32 1, i32 1) to i32))}
   },
-; CHECK: .long   5
-; CHECK-NOT: l_extgotequiv-(_table+32)
-; CHECK-NEXT: L_extfoo$non_lazy_ptr-(_table+32)
+; CHECK:      .long   5
+; CHECK-NEXT: Ltmp1:
+; CHECK-NOT:   l_extgotequiv-Ltmp1
+; CHECK-NEXT:  L_extfoo$non_lazy_ptr-Ltmp1
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
              i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 2, i32 1, i32 1) to i32))}
   },
 ; Test support for arbitrary constants into the GOTPCREL offset
-; CHECK: .long   5
-; CHECK-NOT: (l_extgotequiv-(_table+44))+24
-; CHECK-NEXT: L_extfoo$non_lazy_ptr-(_table+20)
+; CHECK:      .long   5
+; CHECK-NEXT: Ltmp2:
+; CHECK-NOT:   (l_extgotequiv-Ltmp2)+24
+; CHECK-NEXT:  (L_extfoo$non_lazy_ptr-Ltmp2)+24
   %struct.data { i32 4, %struct.anon { i32 5,
     i32 add (i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
                       i32 ptrtoint (i32* getelementptr inbounds ([4 x %struct.data], [4 x %struct.data]* @table, i32 0, i32 3, i32 1, i32 1) to i32)),

 ], align 16
 
 ; Test multiple uses of GOT equivalents.
-; CHECK-LABEL: _delta
-; CHECK: .long   L_extfoo$non_lazy_ptr-_delta
+; CHECK-LABEL: _delta:
+; CHECK-NEXT:  Ltmp3:
+; CHECK-NEXT:   .long   L_extfoo$non_lazy_ptr-Ltmp3
 @delta = global i32 sub (i32 ptrtoint (i32** @extgotequiv to i32),
                          i32 ptrtoint (i32* @delta to i32))
 
 ; CHECK-LABEL: _deltaplus:
-; CHECK: .long   L_localfoo$non_lazy_ptr-(_deltaplus-55)
+; CHECK-NEXT:  Ltmp4:
+; CHECK-NEXT:   .long   (L_localfoo$non_lazy_ptr-Ltmp4)+55
 @deltaplus = global i32 add (i32 sub (i32 ptrtoint (i32** @localgotequiv to i32),
                              i32 ptrtoint (i32* @deltaplus to i32)),
                              i32 55)