[PATCH] D83468: [Debuginfo] Fix for PR46653

Wed Aug 12 16:45:42 PDT 2020

aprantl added a comment.

In D83468#2207667 <https://reviews.llvm.org/D83468#2207667>, @Jac1494 wrote:

> In D83468#2177491 <https://reviews.llvm.org/D83468#2177491>, @aprantl wrote:
>
>> Thanks for posting the line table. It appears to confirm my suspicion that this patch merely hides the issue, but it isn't correct. To be sure it may be possible to preserve the debug location in this function, but implicitly setting it to the location of the instruction before it, is not the right thing to do. It may have the desired effect in your example, but it't not generally correct.
>
> @aprantl Thank you for comment .  Line llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2187
>
>   EntryBuilder->setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt())); 
>
> is setting Debug location for Line number zero , because of that AsmPrinter is getting debug location for line number zero and it is adding this entry into debug_line table .

Yes, and that seems to be the correct behavior.

I think I haven't quit understood what your goal is. Are you trying to avoid having the prologue end at line 0?

================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:107

-  MIRBuilder.setInstrAndDebugLoc(MI);
+  MIRBuilder.setInstr(MI);

----------------
This also looks wrong. We would be inheriting a random previous debug location for all instructions created by MIRBuilder if we did this?

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

https://reviews.llvm.org/D83468