Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIR] Avoid generating llvm.undef for dummy scoping info #128098

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

razvanlupusoru
Copy link
Contributor

Dummy scoping operations are generated to keep track of scopes for purpose of Fortran level analyses like Alias Analysis. For codegen, the scoping info is converted to a fir.undef during pre-codegen rewrite. Then during declare lowering, this info is no longer used - but it is still translated to llvm.undef. I cleaned up so it is simply erased. The generated LLVM should now no longer have a stray undef which looks off when trying to make sense of the IR.

Dummy scoping operations are generated to keep track of scopes for
purpose of Fortran level analyses like Alias Analysis. For codegen,
the scoping info is converted to a fir.undef during pre-codegen rewrite.
Then during declare lowering, this info is no longer used - but it is
still translated to llvm.undef. I cleaned up so it is simply erased.
The generated LLVM should now no longer have a stray undef which
looks off when trying to make sense of the IR.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:codegen labels Feb 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

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

@llvm/pr-subscribers-flang-codegen

Author: Razvan Lupusoru (razvanlupusoru)

Changes

Dummy scoping operations are generated to keep track of scopes for purpose of Fortran level analyses like Alias Analysis. For codegen, the scoping info is converted to a fir.undef during pre-codegen rewrite. Then during declare lowering, this info is no longer used - but it is still translated to llvm.undef. I cleaned up so it is simply erased. The generated LLVM should now no longer have a stray undef which looks off when trying to make sense of the IR.


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

1 Files Affected:

  • (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+8)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 439cc7a856236..bd87215eeb179 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3575,6 +3575,14 @@ struct UndefOpConversion : public fir::FIROpConversion<fir::UndefOp> {
   llvm::LogicalResult
   matchAndRewrite(fir::UndefOp undef, OpAdaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
+    if (mlir::isa<fir::DummyScopeType>(undef.getType())) {
+      // Dummy scoping is used for Fortran analyses like AA. Once it gets to
+      // pre-codegen rewrite it is erased and a fir.undef is created to
+      // feed to the fir declare operation. Thus, during codegen, we can
+      // simply erase is as it is no longer used.
+      rewriter.eraseOp(undef);
+      return mlir::success();
+    }
     rewriter.replaceOpWithNewOp<mlir::LLVM::UndefOp>(
         undef, convertType(undef.getType()));
     return mlir::success();

Copy link

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' df9d3c20fa0a68a6aeb3174d15279e5e6cf41436 b7d347b1f4103c7c7af66d75dd0de37020970c15 flang/lib/Optimizer/CodeGen/CodeGen.cpp

The following files introduce new uses of undef:

  • flang/lib/Optimizer/CodeGen/CodeGen.cpp

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

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

Thank you, Razvan!

@razvanlupusoru razvanlupusoru merged commit f27081b into llvm:main Feb 21, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:codegen flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants