This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] For Dwarf v5, emit indices into .debug_addr for range list entries
Needs ReviewPublic

Authored by RamNalamothu on Dec 21 2022, 6:52 AM.

Details

Summary

Currently the dwarf emission generates offset pair DW_{R,L}LE_offset_pair
relative to the base address for range list entries and these offsets are
resolved during assembing. But this leads to overlapping addresses in the
range lists when the code size changes due to relaxations during linking.

Hence, for dwarf v5, generate indices DW_{R,L}LE_startx_endx into the
.debug_addr for the range list entries which would need relocations and
we would end up with the correct address range when the relocations are
resolved during linking.

For non dwarf v5, we already generate symbol references, which would need
relocations, for range list entries.

Diff Detail

Event Timeline

RamNalamothu created this revision.Dec 21 2022, 6:52 AM
RamNalamothu requested review of this revision.Dec 21 2022, 6:52 AM

Fix the pre-merge checks test failure.

Another attempt to fix the pre-merge check failure.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

And I was thinking that PROPELLER needed/was considering doing some linker relaxation too, so could benefit from such an abstraction/hook/check - but maybe they didn't end up needing/using linker relaxation? (@tmsriram - did PROPELLER ever end up needing/using linker relaxation? is that still something under consideration?)

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

And I was thinking that PROPELLER needed/was considering doing some linker relaxation too, so could benefit from such an abstraction/hook/check - but maybe they didn't end up needing/using linker relaxation? (@tmsriram - did PROPELLER ever end up needing/using linker relaxation? is that still something under consideration?)

Propeller needs linker relaxation for branch instructions. Making it a shorter branch when the offset can fit in one byte and flipping branches, from like je to jne to convert and remove a subsequent fallthrough.

I remember dealing with issues regarding low_pc, high_pc. I think you reviewed these patches, I forget the details now and can take another look to see how this can help.

Introduce an MCAsmInfo API to check if linker relaxation is enabled for generating relocatable symbol references.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

Unless the relaxations are performed during the assembling, this may not be possible before linking because we will not know which set of labels have instruction sequence(s) that the linker actually might be able to relax after finishing the section layout.
So, depending on whether linker relaxations are enabled, I think we can only either emit relocatable symbols for all the range labels or never.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

I have introduced a new MCAsmInfo API to check if we need relocatable symbol references in dwarf entries based on whether the linker relaxations are enabled.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

Unless the relaxations are performed during the assembling, this may not be possible before linking because we will not know which set of labels have instruction sequence(s) that the linker actually might be able to relax after finishing the section layout.
So, depending on whether linker relaxations are enabled, I think we can only either emit relocatable symbols for all the range labels or never.

Yeah, such a query wouldn't have to be precise, only conservatively correct - so if you can't be sure a given range won't be relaxed, then the query would say that it might be & DWARF would be emitted that'd be resilient to such relaxation.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

I have introduced a new MCAsmInfo API to check if we need relocatable symbol references in dwarf entries based on whether the linker relaxations are enabled.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

And I was thinking that PROPELLER needed/was considering doing some linker relaxation too, so could benefit from such an abstraction/hook/check - but maybe they didn't end up needing/using linker relaxation? (@tmsriram - did PROPELLER ever end up needing/using linker relaxation? is that still something under consideration?)

Propeller needs linker relaxation for branch instructions. Making it a shorter branch when the offset can fit in one byte and flipping branches, from like je to jne to convert and remove a subsequent fallthrough.

I remember dealing with issues regarding low_pc, high_pc. I think you reviewed these patches, I forget the details now and can take another look to see how this can help.

Hmm, yeah, I'm confused - because I'd expect the code for ranges to have to handle this but judging by the code being added in this patch, the code isn't already there that I'd expect to be there for linker relaxation in PROPELLER.

Could you point me to the reviews and/or a simple-ish example that would reproduce the relaxation behavior for PROPELLER so I can see how the DWARF is emitted/works uot?

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

Unless the relaxations are performed during the assembling, this may not be possible before linking because we will not know which set of labels have instruction sequence(s) that the linker actually might be able to relax after finishing the section layout.
So, depending on whether linker relaxations are enabled, I think we can only either emit relocatable symbols for all the range labels or never.

Yeah, such a query wouldn't have to be precise, only conservatively correct - so if you can't be sure a given range won't be relaxed, then the query would say that it might be & DWARF would be emitted that'd be resilient to such relaxation.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

I have introduced a new MCAsmInfo API to check if we need relocatable symbol references in dwarf entries based on whether the linker relaxations are enabled.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

And I was thinking that PROPELLER needed/was considering doing some linker relaxation too, so could benefit from such an abstraction/hook/check - but maybe they didn't end up needing/using linker relaxation? (@tmsriram - did PROPELLER ever end up needing/using linker relaxation? is that still something under consideration?)

Propeller needs linker relaxation for branch instructions. Making it a shorter branch when the offset can fit in one byte and flipping branches, from like je to jne to convert and remove a subsequent fallthrough.

I remember dealing with issues regarding low_pc, high_pc. I think you reviewed these patches, I forget the details now and can take another look to see how this can help.

Hmm, yeah, I'm confused - because I'd expect the code for ranges to have to handle this but judging by the code being added in this patch, the code isn't already there that I'd expect to be there for linker relaxation in PROPELLER.

Could you point me to the reviews and/or a simple-ish example that would reproduce the relaxation behavior for PROPELLER so I can see how the DWARF is emitted/works uot?

The one I was remembering: https://reviews.llvm.org/D85085

Basically, the relaxation is part of basic block sections. Here is a simple example:

int glob;
int main() {

 if (glob > 5) {
   __builtin_printf("Large");
 }
 if (glob > 3) {
   __builtin_printf("Medium");
 }
return 0;

}

Without relaxation

$ clang -fbasic-block-sections=all foo.cc -O2 -fuse-ld=lld

With relaxation

$ clang -fbasic-block-sections=all foo.cc -O2 -fuse-ld=lld -Wl,--optimize-bb-jumps

My guess is relaxation will expose some debug info issues unaddressed in the original debug info patch. Currently, Propeller isn't using this relaxation as it only does intra-procedural reordering via clusters. We are looking at inter-procedural reordering which will need relaxation. Thanks for taking a look!

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

Unless the relaxations are performed during the assembling, this may not be possible before linking because we will not know which set of labels have instruction sequence(s) that the linker actually might be able to relax after finishing the section layout.
So, depending on whether linker relaxations are enabled, I think we can only either emit relocatable symbols for all the range labels or never.

Yeah, such a query wouldn't have to be precise, only conservatively correct - so if you can't be sure a given range won't be relaxed, then the query would say that it might be & DWARF would be emitted that'd be resilient to such relaxation.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

I have introduced a new MCAsmInfo API to check if we need relocatable symbol references in dwarf entries based on whether the linker relaxations are enabled.

Is there any mechanism that tests whether a given range bounded by two labels may be subject to linker relaxation? It'd be nice to have that abstraction and use it here (& in some other places in DWARF emission) rather than hardcoding one particular architecture.

The other place this would be needed is in the low/high_pc computation, since currently, even in DWARFv4, the high_pc is encoded as a delta from low_pc, not as a separate relocatable label, so that'd need the same checking.

And I was thinking that PROPELLER needed/was considering doing some linker relaxation too, so could benefit from such an abstraction/hook/check - but maybe they didn't end up needing/using linker relaxation? (@tmsriram - did PROPELLER ever end up needing/using linker relaxation? is that still something under consideration?)

Propeller needs linker relaxation for branch instructions. Making it a shorter branch when the offset can fit in one byte and flipping branches, from like je to jne to convert and remove a subsequent fallthrough.

I remember dealing with issues regarding low_pc, high_pc. I think you reviewed these patches, I forget the details now and can take another look to see how this can help.

Hmm, yeah, I'm confused - because I'd expect the code for ranges to have to handle this but judging by the code being added in this patch, the code isn't already there that I'd expect to be there for linker relaxation in PROPELLER.

Could you point me to the reviews and/or a simple-ish example that would reproduce the relaxation behavior for PROPELLER so I can see how the DWARF is emitted/works uot?

The one I was remembering: https://reviews.llvm.org/D85085

Ah, yeah, that handles a correct source variable location range descirptions when a function is split over multiple sections (so we can't use length offsets relative to the start of the variable's range - because some parts of the variable's range aren't relative to the start, they're independent due to being in another section)

But that doesn't handle the relaxation issue of shrinking code.

Basically, the relaxation is part of basic block sections. Here is a simple example:

int glob;
int main() {
   if (glob > 5) {
     __builtin_printf("Large");
   }
   if (glob > 3) {
     __builtin_printf("Medium");
   }
  return 0;
}
# Without relaxation
$  clang -fbasic-block-sections=all foo.cc  -O2 -fuse-ld=lld

# With relaxation
$ clang -fbasic-block-sections=all foo.cc  -O2 -fuse-ld=lld -Wl,--optimize-bb-jumps

My guess is relaxation will expose some debug info issues unaddressed in the original debug info patch. Currently, Propeller isn't using this relaxation as it only does intra-procedural reordering via clusters. We are looking at inter-procedural reordering which will need relaxation. Thanks for taking a look!

Yeah, looking at the DWARF it doesn't account for potential relaxation:

0x00000014: [DW_RLE_startx_length]:  0x0000000000000003, 0x0000000000000019 => [0x0000000000000000, 0x0000000000000019)
0x00000017: [DW_RLE_startx_length]:  0x0000000000000004, 0x000000000000000e => [0x0000000000000000, 0x000000000000000e)
0x0000001a: [DW_RLE_startx_length]:  0x0000000000000005, 0x0000000000000013 => [0x0000000000000000, 0x0000000000000013)
0x0000001d: [DW_RLE_startx_length]:  0x0000000000000006, 0x0000000000000004 => [0x0000000000000000, 0x0000000000000004)
0x00000020: [DW_RLE_startx_length]:  0x0000000000000007, 0x0000000000000015 => [0x0000000000000000, 0x0000000000000015)
0x00000023: [DW_RLE_end_of_list  ]

This is the range of addresses for the main function (& for the overall compilation unit, since they're identical) - notice that it uses the startx_length encoding which uses a relocatable address for the start of the range, then a fixed length.

Looking at what this translates to - we can see the overlap between sections due to that relaxation when compiling with --optimize-bb-jumps:

DW_AT_ranges [DW_FORM_rnglistx]   (indexed (0x1) rangelist = 0x00000024
   [0x00000000000017d0, 0x00000000000017e9)
   [0x00000000000017e4, 0x00000000000017f2)
   [0x00000000000017ed, 0x0000000000001800)
   [0x00000000000017fb, 0x00000000000017ff)
   [0x00000000000017c0, 0x00000000000017d5))

We can see the ranges have become overlapping.

so, yeah, PROPELLER will need some way to specify that certain ranges may be subject to relaxation, and those would need to use the startx_endx encoding - which is bigger.

So that explains why there's no logic here already to handle this case and so we're forging new ground in this patch.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
2802

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?

2804

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)

2805–2809

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)

RamNalamothu added inline comments.
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
2802

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.

2804

Yes, something on those lines is possible but may be after https://reviews.llvm.org/D143645.

2805–2809

Yes, change is coming in the next update.

Address review comments.

RamNalamothu marked 3 inline comments as done.Feb 24 2023, 3:54 AM

Few lit tests will fail for using MCSubtargetInfo::checkFeatures without D143645.

craig.topper added inline comments.
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1299 ↗(On Diff #500131)

This also feels like it should be a virtual function.

llvm/lib/CodeGen/MachineFunction.cpp
635 ↗(On Diff #500131)

This feels like there should be a virtual function somewhere that a target can override and answer using its own knowledge. Rather than using feature strings that don't exist on most targets.

RamNalamothu added inline comments.Feb 24 2023, 11:57 AM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1299 ↗(On Diff #500131)

That's what I started with in the previous versions of this revision https://reviews.llvm.org/D140478?vs=on&id=484785#toc but @dblaikie mentioned an interesting point that -mrelax is a subtarget feature, which is true at least for RISC-V. Hence I thought having these checks per machine function as in MachineFunction::hasRelaxationsEnabled would be right/better. Please see my previous replies to @dblaikie.

I can introduce a virtual function in MCSubtargetInfo but that will defeat the purpose of MCSubtargetInfo::checkFeatures.

And another challenge is we don't have enough context in DwarfDebug to know if relaxations are enabled or not. We have to go through AsmPrinter.

craig.topper added inline comments.Feb 24 2023, 12:09 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1299 ↗(On Diff #500131)

Even with your changes to MCSubtargetInfo::checkFeatures isn't it still printing a message to errs() on targets that aren't RISC-V? A message that isn't actionable by the compiler.

Created a virtual method in AsmPrinter instead of using MCSubtargetInfo::checkFeatures.

dblaikie added inline comments.Feb 27 2023, 4:05 PM
llvm/include/llvm/CodeGen/AsmPrinter.h
456–461

Does it make sense to forward-compat this by providing the two labels to this function, even if they're ignored for now? Is it plausible that some backend could determine whether a given range is relaxable or not from those labels? Guess we can always add that in later...

Maybe the name could be more about relaxation? "is this address range a compile-time constant" or "is this address range not subject to linker relaxation"?

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
254

Is this possible, even when the MachineFunction's subtarget has the relax feature? (I'd have thought the subtarget would already reflect/account for/be derived from the function's attributes?)

dblaikie added inline comments.Feb 27 2023, 4:07 PM
llvm/include/llvm/CodeGen/AsmPrinter.h
456–461

At least I think this function should take the MachineFunction to operate on, rather than using the one that might be set on the AsmPrinter - I would guess if you test this with a module with some functions with relaxation and some without, the current implementation doesn't get it correct - because we emit all these ranges together, so the AsmPrinter's current MF, if it has one, won't be correct for all of the ranges being emitted, right?

456–461

Which is to say - please include some test coverage showing a mixed module, demonstrating that the appropriate encodings are used for the appropriate functions (relocatable for relaxable functions, and non-relocatable for non-relaxable functions)

RamNalamothu marked 3 inline comments as done.

Remove unnecessary checks for relaxation flags in "target-features".

RamNalamothu marked an inline comment as done.Mar 2 2023, 8:09 AM

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V. If that changes in future, I guess it will need many other changes as well and this will also get looked at then.

@craig.topper @asb

The latest update has a target hook, which is specialized for RISC-V, and the changes are effective for only RISC-V target.
Does this look good to go now?

llvm/include/llvm/CodeGen/AsmPrinter.h
456–461

Does it make sense to forward-compat this by providing the two labels to this function, even if they're ignored for now? Is it plausible that some backend could determine whether a given range is relaxable or not from those labels? Guess we can always add that in later...

Yeah, it's a simple change that can be added when needed.

Maybe the name could be more about relaxation? "is this address range a compile-time constant" or "is this address range not subject to linker relaxation"?

I am not getting ideas here. Feel free to suggest a name.

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
254

I don't know for sure. I think what you said is true.

Will change the checks accordingly.

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

jrtc27 added a comment.Mar 2 2023, 3:43 PM

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

It's a per-function feature that has implications for other functions in the same section (deleting code in one function at link time will shunt around the code in the rest of the section).

Thanks for the clarification @jrtc27.

Does this patch need any further changes for it to be acceptable?

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

It's a per-function feature that has implications for other functions in the same section (deleting code in one function at link time will shunt around the code in the rest of the section).

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

It's a per-function feature that has implications for other functions in the same section (deleting code in one function at link time will shunt around the code in the rest of the section).

What does it mean for it to be per-function? Can it vary at all between functions in the same module? (if not, why is it per-function?)

jrtc27 added a comment.Mar 3 2023, 9:43 PM

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

It's a per-function feature that has implications for other functions in the same section (deleting code in one function at link time will shunt around the code in the rest of the section).

@dblaikie

As I already replied earlier, there is not enough context to reach the MachineFunction from the symbol or determine if relaxation is enabled for the given address range.

Then won't this make the wrong decisions if the value varies between functions/subtargets?

Also, at the moment, the -mrelax/-mno-relax affects the entire compilation unit for the RISC-V.

Then wouldn't these be target features, rather than subtarget features? My understanding is that subtarget features generally can vary by function, whereas target features cannot.

Even if the command line flags don't support it, we could probably make a simple IR example with different relaxation per function? & so we probably should be able to handle that case.

It's a per-function feature that has implications for other functions in the same section (deleting code in one function at link time will shunt around the code in the rest of the section).

What does it mean for it to be per-function? Can it vary at all between functions in the same module? (if not, why is it per-function?)

It governs whether or not that function's text relocations are paired with R_RISCV_RELAX to tell the linker it can relax them.

What does it mean for it to be per-function? Can it vary at all between functions in the same module? (if not, why is it per-function?)

It governs whether or not that function's text relocations are paired with R_RISCV_RELAX to tell the linker it can relax them.

OK, then it seems reasonable that this be handled for DWARF on a per-function basis. We shouldn't use an addr+addr representation for a function that won't change size at link time (equally needRelocSymbolsForAddrRange couldn't provide the right answer if it's not implemented per-function, right? Which Function does it consider? If it's not looking at the right function it might conclude there's no linker relaxation for the whole module, when one function does have relaxation)

If a label pair doesn't have enough info (you can get the relaxation from the MCSection, retrievable from the MCSymbol? Or is it a function of using a different kind of instruction or relocation (so not a property of the section or symbol) - in which case, the dwarf emission code could keep the MachineFunction or similar for each address range, then that could be passed into the callback and queried to determine if relaxation is enabled)