[AsmPrinter] Make the encoding of call sites in .gcc_except_table configurable

[edward-jones]

Summary

The original behavior was to always emit the offsets to each call site in the call site table as uleb128 values, however on some architectures (eg RISCV) these uleb128 offsets into the code cannot always be resolved until link time (because relaxation will invalidate any calculated offsets), and there are no appropriate relocations for uleb128 values. As a consequence it needs to be possible to specify an alternative.

This also switches RISCV to use DW_EH_PE_udata4 for call side encodings in .gcc_except_table

[edward-jones]

Rebased against master

[asb]

Hi Ed, I think it does make sense to merge D63416 into this one. Sometimes it makes sense to separate generic changes from the target-dependent code that uses them, but given that:

• This addition is motivated by the needs of RISC-V
• D63416 is the only reasonable way to test the change
• D63416 is a tiny patch anyway

I think it makes sense to combine in this case, so we can review+commit in an atomic unit.

Is this something that should be specified in the RISC-V psabi doc do you think? If so, might you prepare a pull request for it?

[luismarques]

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

189

Regarding the 0x7 magic constant, it's not very clear in this context why that is the right value. Can we replace that with a symbolic constant, or at least add some context (a comment, etc.) to make it clear to readers the logic behind that value?

197

Ditto magic constant.

[edward-jones]

Merged D63416 into this patch, rebased.

I've stuck a comment above the magic 0x7 constant because there's not a constant specified for the mask value already.

[edward-jones]

I'll look into changes that would need to be made to the psabi docs too.

[lenary]

test/CodeGen/RISCV/exception.ll

94 ↗

(On Diff #207766)

Do you think that extra patches will be required to emit these relocations correctly?

We're running into errors with overlapping FDEs (even after applying this patch), and fear it may be an interaction of eh_frame data, relocations and relaxation (which also seems to have an effect).

[edward-jones]

test/CodeGen/RISCV/exception.ll

94 ↗

(On Diff #207766)

I've just submitted another patch (D64715) which might help with an "overlapping FDEs" error.

[asb]

LGTM, thanks. I've spent quite a bit of time comparing output vs gcc so I'm pretty confident here now. I committed a slightly different test strategy in the DWARF encoding work I just committed, so will adjust this patch to build on that.

include/llvm/CodeGen/AsmPrinter.h

public:
   /// emitDwarfSymbolReference().
   void emitDwarfStringOffset(DwarfStringPoolEntry S) const;
 
   /// Emit the 4-byte offset of a string from the start of its section.
   void emitDwarfStringOffset(DwarfStringPoolEntryRef S) const {
     emitDwarfStringOffset(S.getEntry());
   }
 
+  /// Emit reference to a call site with a specified encoding
+  void EmitCallSiteOffset(const MCSymbol *Hi, const MCSymbol *Lo,
+                          unsigned Encoding) const;
+  /// Emit an integer value corresponding to the call site encoding
+  void EmitCallSiteValue(uint64_t Value, unsigned Encoding) const;
+
   /// Get the value for DW_AT_APPLE_isa. Zero if no isa encoding specified.
   virtual unsigned getISAEncoding() { return 0; }
 
   /// Emit the directive and value for debug thread local expression
   ///
   /// \p Value - The value to emit.
   /// \p Size - The size of the integer (in bytes) to emit.
   virtual void EmitDebugValue(const MCExpr *Value, unsigned Size) const;

include/llvm/Target/TargetLoweringObjectFile.h

protected:
   bool SupportGOTPCRelWithOffset = true;
   bool SupportDebugThreadLocalLocation = true;
 
   /// PersonalityEncoding, LSDAEncoding, TTypeEncoding - Some encoding values
   /// for EH.
   unsigned PersonalityEncoding = 0;
   unsigned LSDAEncoding = 0;
   unsigned TTypeEncoding = 0;
+  unsigned CallSiteEncoding = 0;
 
   /// This section contains the static constructor pointer list.
   MCSection *StaticCtorSection = nullptr;
 
   /// This section contains the static destructor pointer list.
   MCSection *StaticDtorSection = nullptr;
 
 public:
public:
   // The symbol that gets passed to .cfi_personality.
   virtual MCSymbol *getCFIPersonalitySymbol(const GlobalValue *GV,
                                             const TargetMachine &TM,
                                             MachineModuleInfo *MMI) const;
 
   unsigned getPersonalityEncoding() const { return PersonalityEncoding; }
   unsigned getLSDAEncoding() const { return LSDAEncoding; }
   unsigned getTTypeEncoding() const { return TTypeEncoding; }
+  unsigned getCallSiteEncoding() const { return CallSiteEncoding; }
 
   const MCExpr *getTTypeReference(const MCSymbolRefExpr *Sym, unsigned Encoding,
                                   MCStreamer &Streamer) const;
 
   virtual MCSection *getStaticCtorSection(unsigned Priority,
                                           const MCSymbol *KeySym) const {
     return StaticCtorSection;
   }

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

void AsmPrinter::emitDwarfStringOffset(DwarfStringPoolEntry S) const {
   // Just emit the offset directly; no need for symbol math.
   emitInt32(S.Offset);
 }
 
 void AsmPrinter::EmitDwarfOffset(const MCSymbol *Label, uint64_t Offset) const {
   EmitLabelPlusOffset(Label, Offset, MAI->getCodePointerSize());
 }
 
+void AsmPrinter::EmitCallSiteOffset(const MCSymbol *Hi,
+                                    const MCSymbol *Lo,
+                                    unsigned Encoding) const {
+  if ((Encoding & 0x7) == dwarf::DW_EH_PE_uleb128)
+    EmitLabelDifferenceAsULEB128(Hi, Lo);
+  else
+    EmitLabelDifference(Hi, Lo, GetSizeOfEncodedValue(Encoding));
+}
+
+void AsmPrinter::EmitCallSiteValue(uint64_t Value,
+                                   unsigned Encoding) const {
+  if ((Encoding & 0x7) == dwarf::DW_EH_PE_uleb128)
+    EmitULEB128(Value);
+  else
+    OutStreamer->EmitIntValue(Value, GetSizeOfEncodedValue(Encoding));
+}
+
 //===----------------------------------------------------------------------===//
 // Dwarf Lowering Routines
 //===----------------------------------------------------------------------===//
 
 void AsmPrinter::emitCFIInstruction(const MCCFIInstruction &Inst) const {
   switch (Inst.getOperation()) {
   default:
     llvm_unreachable("Unexpected instruction");

lib/CodeGen/AsmPrinter/EHStreamer.cpp

MCSymbol *EHStreamer::emitExceptionTable() {
 
   // Compute the call-site table.
   SmallVector<CallSiteEntry, 64> CallSites;
   computeCallSiteTable(CallSites, LandingPads, FirstActions);
 
   bool IsSJLJ = Asm->MAI->getExceptionHandlingType() == ExceptionHandling::SjLj;
   bool IsWasm = Asm->MAI->getExceptionHandlingType() == ExceptionHandling::Wasm;
   unsigned CallSiteEncoding =
-      IsSJLJ ? dwarf::DW_EH_PE_udata4 : dwarf::DW_EH_PE_uleb128;
+      IsSJLJ ? static_cast<unsigned>(dwarf::DW_EH_PE_udata4) :
+               Asm->getObjFileLowering().getCallSiteEncoding();
   bool HaveTTData = !TypeInfos.empty() || !FilterIds.empty();
 
   // Type infos.
   MCSection *LSDASection = Asm->getObjFileLowering().getLSDASection();
   unsigned TTypeEncoding;
 
   if (!HaveTTData) {
     // If there is no TypeInfo, then we just explicitly say that we're omitting
for (SmallVectorImpl<CallSiteEntry>::const_iterator
         BeginLabel = EHFuncBeginSym;
       MCSymbol *EndLabel = S.EndLabel;
       if (!EndLabel)
         EndLabel = Asm->getFunctionEnd();
 
       // Offset of the call site relative to the start of the procedure.
       if (VerboseAsm)
         Asm->OutStreamer->AddComment(">> Call Site " + Twine(++Entry) + " <<");
-      Asm->EmitLabelDifferenceAsULEB128(BeginLabel, EHFuncBeginSym);
+      Asm->EmitCallSiteOffset(BeginLabel, EHFuncBeginSym, CallSiteEncoding);
       if (VerboseAsm)
         Asm->OutStreamer->AddComment(Twine("  Call between ") +
                                      BeginLabel->getName() + " and " +
                                      EndLabel->getName());
-      Asm->EmitLabelDifferenceAsULEB128(EndLabel, BeginLabel);
+      Asm->EmitCallSiteOffset(EndLabel, BeginLabel, CallSiteEncoding);
 
       // Offset of the landing pad relative to the start of the procedure.
       if (!S.LPad) {
         if (VerboseAsm)
           Asm->OutStreamer->AddComment("    has no landing pad");
-        Asm->EmitULEB128(0);
+        Asm->EmitCallSiteValue(0, CallSiteEncoding);
       } else {
         if (VerboseAsm)
           Asm->OutStreamer->AddComment(Twine("    jumps to ") +
                                        S.LPad->LandingPadLabel->getName());
-        Asm->EmitLabelDifferenceAsULEB128(S.LPad->LandingPadLabel,
-                                          EHFuncBeginSym);
+        Asm->EmitCallSiteOffset(S.LPad->LandingPadLabel, EHFuncBeginSym,
+                                CallSiteEncoding);
       }
 
       // Offset of the first associated action record, relative to the start of
       // the action table. This value is biased by 1 (1 indicates the start of
       // the action table), and 0 indicates that there are no actions.
       if (VerboseAsm) {
         if (S.Action == 0)
           Asm->OutStreamer->AddComment("  On action: cleanup");

lib/Target/TargetLoweringObjectFile.cpp

void TargetLoweringObjectFile::Initialize(MCContext &ctx,
   // `Initialize` can be called more than once.
   delete Mang;
   Mang = new Mangler();
   InitMCObjectFileInfo(TM.getTargetTriple(), TM.isPositionIndependent(), *Ctx,
                        TM.getCodeModel() == CodeModel::Large);
 
   // Reset various EH DWARF encodings.
   PersonalityEncoding = LSDAEncoding = TTypeEncoding = dwarf::DW_EH_PE_absptr;
+  CallSiteEncoding = dwarf::DW_EH_PE_uleb128;
 }
 
 TargetLoweringObjectFile::~TargetLoweringObjectFile() {
   delete Mang;
 }
 
 static bool isNullOrUndef(const Constant *C) {
   // Check that the constant isn't all zeros or undefs.