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

[via flang-commits]

qyingwu wrote:

> It seems like you implemented some logic to break the cycle at the local variable instead of at the subprogram. That should not be necessary? Note that self recursive machinery is extremely complex and fragile. We should avoid changing it and stick as close as possible to the PR I linked.
> 
> What was the reason for breaking the cycle at the local variable rather at the subprogram? Did the imported example debug info look unexpected?
> 
> Note I didn't get to the end with the review. I think it makes sense to move the closer to the PR I linked first.

I tried the suggested approach locally, but it does not fix #206713.

With that approach, the issue test imports roughly as:

  ```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 import creates one local variable scoped by the self-recursive subprogram placeholder, and the
later dbg.value import creates another local variable scoped by the finalized subprogram. It is the same LLVM DILocalVariable, but it becomes two different MLIR attrs.

That is why I broke the cycle at the local variable for retainedNodes that are also referenced by debug intrinsics. I’ll keep looking for a way to simplify the patch and stay closer to the existing recursive machinery.

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