[all-commits] [llvm/llvm-project] b2a950: [flang][hlfir] Fixed associate-op codegen for opti...

[Slava Zakharin via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: b2a9501080d05a33c47d518d9e6ab4ba45b38138
      https://github.com/llvm/llvm-project/commit/b2a9501080d05a33c47d518d9e6ab4ba45b38138
  Author: Slava Zakharin <szakharin at nvidia.com>
  Date:   2023-08-23 (Wed, 23 Aug 2023)

  Changed paths:
    M flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
    M flang/test/HLFIR/associate-codegen.fir

  Log Message:
  -----------
  [flang][hlfir] Fixed associate-op codegen for optimized HLFIR.

This effectively reverts D154715.

The issue appears as the dialect conversion error because we try to
erase an op that has already been erased. See the added LIT test case
with HLFIR that may appear as a result of CSE.
The `adaptor.getSource()` is an operation producing a tuple,
which does not have users, so `allOtherUsesAreSafeForAssociate`
just looks at the empty list of users. So we get completely wrong
answers from it. This causes problems with the following
`eraseAllUsesInDestroys` that tries to remove the `DestroyOp` twice
during both `hflir.associate` processing.

But we also cannot use `associate.getSource()` *efficiently*, because
the original users may still hang around: one example is the original body
of hlfir.elemental (see D154715), another example is other already converted
AssociateOp's that are pending removal in the rewriter
(that is why we have a temporary created for each hlfir.associate
in the newly added LIT case).

This patch just fixes the correctness issue. I think we have to separate
the buffer reuse analysis from the conversion itself.

I also tried to address the issues with the cloned bodies of `hlfir.elemental`,
but this should not matter since D155778: if `hlfir.associate` is inside
`hlfir.elemental`, it will end up inside a do-loop body region, so the early
exit added in D155778 will prevent the buffer reuse.

Reviewed By: tblah

Differential Revision: https://reviews.llvm.org/D158471


  Commit: ebfdbdb73ca8731bb60802eaa84b73c1bc6a9243
      https://github.com/llvm/llvm-project/commit/ebfdbdb73ca8731bb60802eaa84b73c1bc6a9243
  Author: Slava Zakharin <szakharin at nvidia.com>
  Date:   2023-08-23 (Wed, 23 Aug 2023)

  Changed paths:
    M flang/include/flang/Optimizer/HLFIR/HLFIROps.td
    A flang/test/HLFIR/elemental-cse.fir

  Log Message:
  -----------
  [flang][hlfir] Apply MemAlloc effect to hlfir.elemental explicitly.

Related to https://github.com/llvm/llvm-project/issues/64866.
This patch effectively disables CSE for identical hlfir.elemental
operations, because it causes hlfir.destroy to be applied twice
to the same temporary. Moreover, I think MemAlloc is correct
for hlfir.elemental, in general.

Reviewed By: tblah

Differential Revision: https://reviews.llvm.org/D158565


Compare: https://github.com/llvm/llvm-project/compare/06d3ee9603b6...ebfdbdb73ca8