[PATCH] D83468: [Debuginfo] Fix for PR46653

[Jaydeep Chauhan via Phabricator via llvm-commits]

Jac1494 added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:107
 
-  MIRBuilder.setInstrAndDebugLoc(MI);
+  MIRBuilder.setInstr(MI);
 
----------------
arsenm wrote:
> Jac1494 wrote:
> > aprantl wrote:
> > > This also looks wrong. We would be inheriting a random previous debug location for all instructions created by MIRBuilder if we did this?
> > Yes, I have also check the debugging experience and now it is looks correct.
> Pretty sure this is completely broken. The currently legalizing instruction is the only reasonable choice to set here
If we don't set debug location here than also in executable debug location entry is added properly , because IRTransaltor is already setting debug location. I have tried test case given below and others test cases too and debugging experience is improved lot after this patch. 

```
int var1;           // 1
int var2;           // 2
int main() {        // 3
  if (var1 == 1) {  // 4
    var2 = 2;       // 5
  }                 // 6
  return 0;         // 7
}
```

Also if we add debug location here than it is giving incorrect debugging experience because of line number 0 entry added into debugline table which is added from here ,than what is need of setting debug location here (In legalizer)...? 

>Pretty sure this is completely broken.
Can you please explain more what is completely broken here..?


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

https://reviews.llvm.org/D83468