[Mlir-commits] [mlir] [mlir][LLVMIR] Avoid duplicate retained local variables (PR #209911)

[llvmlistbot at llvm.org]

qyingwu wrote:

> Thanks for the experiment.
> 
> The test LGTM.
> 
> I would keep #206713 open since the ticket also mentions global variables / global variable expressions. A comment on the ticket that the local variable roundtrip currently works as expected makes sense though.
> 
> Did you try to import the example with the self recursive local variable (without your extra change)? If yes, how did the imported IR look in this case?


Thanks for checking! Makes sense. I’ll keep #206713 open and add a comment there clarifying that the local variable roundtrip looks correct, while the global variable / global variable expression part still needs separate handling.

> Did you try to import the example with the self recursive local variable (without your extra change)? If yes, how did the imported IR look in this case?

Yes. With the retainedNodes path kept close to the existing machinery, i.e. translating the retained node directly and without the extra local-variable cycle-break special case, the import still produced two MLIR local variable attrs.

roughly like this:

```mlir
  #di_subprogram = #llvm.di_subprogram<
     recId = distinct[0]<>, isRecSelf = true
  >

 #di_local_variable = #llvm.di_local_variable<
    scope = #di_subprogram, name = "n", ...
  >

  #di_subprogram1 = #llvm.di_subprogram<
    recId = distinct[0]<>, id = distinct[2]<>, ...,
    retainedNodes = [#di_local_variable]
  >

  #di_local_variable1 = #llvm.di_local_variable<
    scope = #di_subprogram1, name = "n", ...
  >
```

So the retainedNodes traversal sees the subprogram cycle and creates a local variable scoped by the self-recursive subprogram placeholder. Later, when traversing from the debug intrinsic, the subprogram has already been finalized, so the local variable is imported with the concrete subprogram scope instead. That is why the second local variable is not recursive in that traversal.


https://github.com/llvm/llvm-project/pull/209911