[PATCH] D140478: [RISCV] For Dwarf v5, emit indices into .debug_addr for range list entries

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu added a reviewer: asb.
RamNalamothu added inline comments.
Herald added a subscriber: luke.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2773
+  const MCSubtargetInfo *STI =
+      Asm->MF ? &(Asm->getSubtargetInfo()) : Asm->TM.getMCSubtargetInfo();
+  const bool NeedRelocSymbols =
----------------
dblaikie wrote:
> Is  this a subtarget feature? (can it vary per function?)
> 
> I guess/hope eventually it's a per address range property - though I don't know which target/subtarget/etc should be making the choice, given an address range, about whether it can be relaxed.
> 
> I guess it is a subtarget feature - based on the implementation of `doDwarfAddrRangesNeedRelocSymbols` - so maybe a test case with a single CU with one function that doesn't relax, and one that does?
> 
> I'm guessing this'll be more complicated to implement that correctly - probably require being able to go from the start symbol to the function, to query its subtarget - and doing that inside the `SectionRanges` search below, so that we use start_end when necessary, even if it isn't the default/current subtarget?
> Is  this a subtarget feature? (can it vary per function?)
> 

Yes, at least for RISC-V and looks like it is the only one using this feature at the moment.

> I guess/hope eventually it's a per address range property - though I don't know which target/subtarget/etc should be making the choice, given an address range, about whether it can be relaxed.
> 
> I guess it is a subtarget feature - based on the implementation of `doDwarfAddrRangesNeedRelocSymbols` - so maybe a test case with a single CU with one function that doesn't relax, and one that does?
> 

Looks like the `"target-features"="-relax"` or `"target-features"="+relax"` Function attributes have no effect which means the relaxation is either enabled or disabled for the entire module. Hence, I may not be able to produce such a test case.

> I'm guessing this'll be more complicated to implement that correctly - probably require being able to go from the start symbol to the function, to query its subtarget - and doing that inside the `SectionRanges` search below, so that we use start_end when necessary, even if it isn't the default/current subtarget?

It appears that there is not enough context available to go from the symbol to the function/subtarget and hence we can't do a per address range check on whether the relaxation is enabled or not.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2775
+  const bool NeedRelocSymbols =
+      Asm->MAI->doDwarfAddrRangesNeedRelocSymbols(STI);
   const MCSymbol *CUBase = CU.getBaseAddress();
----------------
dblaikie wrote:
> Rather than passing the subtarget to the asm info, could this be a virtual function on the subtarget directly? (I don't know enough about all these abstractions, maybe there's a good reason to keep the choice in the AsmInfo)
Yes, something on those lines is possible but may be after https://reviews.llvm.org/D143645.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2780-2784
+    if (NeedRelocSymbols) {
+      Base = nullptr;
+      BaseIsSet = false;
+      ShouldUseBaseAddress = false;
+    }
----------------
dblaikie wrote:
> Could we avoid the need for doing this in each iteration of the loop by doing it once before the loop (if `CUBase` is set to null, and `ShouldUseBaseAddress` is set to false, then everything else should work out OK, I think? The base will never be set and base-relative forms will never be used)
Yes, change is coming in the next update.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140478/new/

https://reviews.llvm.org/D140478