This patch overrides TargetFrameLowering::getFrameIndexReference() in order to
specify the correct register when the function needs dynamic stack realignment.
The values returned from this function are used in order to create DW_AT_locations
for DWARF info. These locations would use the wrong registers as it's been
reported in PR25028.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Hi, thank you for looking into this issue. I've just checked the patch with the tip of llvm but doesn't solve the reported bug. The output is the same as the one reported in the original bug. Is it expected that it requires some additional changes?
I accidentally submitted my reply without finishing it.
Don't you see any difference between the old and the new DWARF info files?
Here's what I get for the test2 subprogram and the char_local variable entries:
<1><98>: Abbrev Number: 3 (DW_TAG_subprogram) <99> DW_AT_low_pc : 0x80 <a1> DW_AT_high_pc : 0x47c <a5> Unknown AT value: 3fe7: 1 <a5> DW_AT_frame_base : 1 byte block: 6e (DW_OP_reg30 (r30)) <a7> DW_AT_name : (indirect string, offset: 0xd4): test2 <ab> DW_AT_decl_file : 1 <ac> DW_AT_decl_line : 15 <ad> DW_AT_prototyped : 1 <ad> DW_AT_type : <0x378> <b1> DW_AT_external : 1 ... ... ... <2><c0>: Abbrev Number: 5 (DW_TAG_variable) <c1> DW_AT_location : 3 byte block: 8d b8 4 (DW_OP_breg29 (r29): 568) <c5> DW_AT_name : (indirect string, offset: 0x111): char_local <c9> DW_AT_decl_file : 1 <ca> DW_AT_decl_line : 17 <cb> DW_AT_type : <0x2a>
Hi, sorry I was off yesterday. You've got me, I was assuming you altered the frame base for the function to the $sp. Though I recall I tried with the debugger to print the values and still I've got some of them incorrect. I'll check today what is going wrong.
Hi, I actually confirm the patch solves the issue. Apologies for my previous wrong comment, the failure was due to my test. Though a different question, in the dwarf info the matches between the line numbers in the sources and the addresses in the assembly are not much contiguous, with the statements in the source interspersed at different places in the assembly. In general, the assembly looks somewhat optimised. This makes a bit harder to debug the code. Is there something that can be mitigated to obtain a more direct and contiguous correspondence between the original source and the assembly?
Finally, just as curiosity, is there any particular reason you are creating the expression for all the locals as $sp + offset, instead of setting the frame base to the $sp?
Thanks,
Dean
I can't think of anything other than trying to use lower optimization levels.
Finally, just as curiosity, is there any particular reason you are creating the expression for all the locals as $sp + offset, instead of setting the frame base to the $sp?
When we use dynamic stack realignment we have to use $sp for only locals and $fp for incoming params, ie. conceptually the $fp is really the frame base register. This matches the behaviour of the other targets that I checked too.
In general, the assembly looks somewhat optimised. This makes a bit harder to debug the code. Is there something that can be mitigated to obtain a more direct and contiguous correspondence between the original source and the assembly?
I can't think of anything other than trying to use lower optimization levels.
ouch, I'm already invoking llc with -O0. Does this depend on the front-end?
I'm aware that SelectionDAG will fold identical nodes together, effectively doing CSE.
You could try '-mllvm -pre-RA-sched=source' which tries to respect source order when linearizing the SelectionDAG nodes.
lib/Target/Mips/MipsSEFrameLowering.cpp | ||
---|---|---|
588–593 ↗ | (On Diff #36738) | I'm surprised that the frame pointer isn't a possible choice here. Shouldn't MipsRegisterInfo::getFrameRegister() be called somewhere? |
596 ↗ | (On Diff #36738) | Indentation? I'm not sure on this one but I'd expect clang-format to align to the MFI. |
test/CodeGen/Mips/dynamic-stack-realignment-dwarf.ll | ||
1 ↗ | (On Diff #36738) | (filename): This test should be in test/DebugInfo/Mips |
lib/Target/Mips/MipsSEFrameLowering.cpp | ||
---|---|---|
588–593 ↗ | (On Diff #36738) | MipsRegisterInfo::getFrameRegister() computes the frame register ($sp or $fp). Here, we want to calculate the register + offset used for a specific frame index location. This can also include the base register $s7 when using dynamic stack realignment + VLAs. |