Skip to content

Conversation

@jeanPerier
Copy link
Contributor

See https://reviews.llvm.org/D157626 for the rational of declare having side effects.

The write effect is to scary for passes that look for read/write effects without caring about the resource affected. I know Slava asked for it, but I think the creation of the DebuggingResource was enough and that a write is too much. The alloca effect is sufficient to prevent DCE to remove it, which is all we care about currently.

This currently is flag as a reason for creating LHS temporary in assignment to vector subscripted entity with array constructor.
There is a lot of read/write side effect analysis in the "lower-hlfir-ordered-assignments" pass, and I feel like we will just keep adding weird "debug ressource" bypassing here and there with these side effects.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Oct 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: None (jeanPerier)

Changes

See https://reviews.llvm.org/D157626 for the rational of declare having side effects.

The write effect is to scary for passes that look for read/write effects without caring about the resource affected. I know Slava asked for it, but I think the creation of the DebuggingResource was enough and that a write is too much. The alloca effect is sufficient to prevent DCE to remove it, which is all we care about currently.

This currently is flag as a reason for creating LHS temporary in assignment to vector subscripted entity with array constructor.
There is a lot of read/write side effect analysis in the "lower-hlfir-ordered-assignments" pass, and I feel like we will just keep adding weird "debug ressource" bypassing here and there with these side effects.


Full diff: https://github.com/llvm/llvm-project/pull/113321.diff

2 Files Affected:

  • (modified) flang/include/flang/Optimizer/Dialect/FIROps.td (+1-1)
  • (modified) flang/include/flang/Optimizer/HLFIR/HLFIROps.td (+1-1)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td index d467f0ab4e1c1e..eda8f26e936fb6 100644 --- a/flang/include/flang/Optimizer/Dialect/FIROps.td +++ b/flang/include/flang/Optimizer/Dialect/FIROps.td @@ -3078,7 +3078,7 @@ def fir_IsPresentOp : fir_SimpleOp<"is_present", [NoMemoryEffect]> { // debug information so we would like to keep this around even if the value // is not used. def fir_DeclareOp : fir_Op<"declare", [AttrSizedOperandSegments, - MemoryEffects<[MemWrite<DebuggingResource>]>, + MemoryEffects<[MemAlloc<DebuggingResource>]>, DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>]> { let summary = "declare a variable"; diff --git a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td index fdf0db9d3c75de..354d3baf8c2067 100644 --- a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td +++ b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td @@ -36,7 +36,7 @@ class hlfir_Op<string mnemonic, list<Trait> traits> // from the declare operation can be used to generate debug information so we // don't want to remove it as dead code def hlfir_DeclareOp : hlfir_Op<"declare", [AttrSizedOperandSegments, - MemoryEffects<[MemWrite<DebuggingResource>]>, + MemoryEffects<[MemAlloc<DebuggingResource>]>, DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>]> { let summary = "declare a variable and produce an SSA value that can be used as a variable in HLFIR operations"; 
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jeanPerier jeanPerier merged commit 03cef62 into llvm:main Oct 23, 2024
12 checks passed
@jeanPerier jeanPerier deleted the optim-vec-sub-2 branch October 23, 2024 10:33
@frobtech frobtech mentioned this pull request Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang Flang issues not falling into any other category

5 participants