-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[flang] Record the original name of a function during ExternalNameCoversion #74065
Conversation
…version We pass TBAA alias information with separate TBAA trees per function (to prevent incorrect alias information after inlining). These TBAA trees are identified by a unique string per function. Naturally, we use the mangled name of the function. TBAA tags are added in two places: during a dedicated pass relatively early (structured control flow makes fir::AliasAnalysis more accurate), then again during CodeGen (when implied box loads and stores become visible). In between these two passes, the ExternalNameConversion pass changes the name of some functions. These functions with changed names previously ended up with separate TBAA trees from the TBAA tags pass and from CodeGen - leading LLVM to think that all data accesses alias with all descriptor accesses. This patch solves this by storing the original name of a function in an attribute during the ExternalNameConversion pass, and using the name from that attribute when creating TBAA trees during CodeGen.
@llvm/pr-subscribers-flang-fir-hlfir Author: Tom Eccles (tblah) ChangesWe pass TBAA alias information with separate TBAA trees per function (to prevent incorrect alias information after inlining). These TBAA trees are identified by a unique string per function. Naturally, we use the mangled name of the function. TBAA tags are added in two places: during a dedicated pass relatively early (structured control flow makes fir::AliasAnalysis more accurate), then again during CodeGen (when implied box loads and stores become visible). In between these two passes, the ExternalNameConversion pass changes the name of some functions. These functions with changed names previously ended up with separate TBAA trees from the TBAA tags pass and from CodeGen - leading LLVM to think that all data accesses alias with all descriptor accesses. This patch solves this by storing the original name of a function in an attribute during the ExternalNameConversion pass, and using the name from that attribute when creating TBAA trees during CodeGen. Full diff: https://github.com/llvm/llvm-project/pull/74065.diff 4 Files Affected:
diff --git a/flang/include/flang/Optimizer/Analysis/TBAAForest.h b/flang/include/flang/Optimizer/Analysis/TBAAForest.h
index 86030f0be26aecc..cb3d25bacb2b5d0 100644
--- a/flang/include/flang/Optimizer/Analysis/TBAAForest.h
+++ b/flang/include/flang/Optimizer/Analysis/TBAAForest.h
@@ -9,9 +9,11 @@
#ifndef FORTRAN_OPTIMIZER_ANALYSIS_TBAA_FOREST_H
#define FORTRAN_OPTIMIZER_ANALYSIS_TBAA_FOREST_H
+#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/Attributes.h"
#include "mlir/IR/MLIRContext.h"
#include "llvm/ADT/DenseMap.h"
#include <string>
@@ -82,6 +84,12 @@ class TBAAForrest {
return getFuncTree(func.getSymNameAttr());
}
inline const TBAATree &operator[](mlir::LLVM::LLVMFuncOp func) {
+ // the external name conversion pass may rename some functinos. Their old
+ // name must be used so that we add to the tbaa tree added in the FIR pass
+ mlir::Attribute attr = func->getAttr(getInternalFuncNameAttrName());
+ if (attr) {
+ return getFuncTree(attr.cast<mlir::StringAttr>());
+ }
return getFuncTree(func.getSymNameAttr());
}
diff --git a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
index 00c06d0cbe6192f..977949e399a53bb 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROpsSupport.h
@@ -93,6 +93,12 @@ static constexpr llvm::StringRef getInternalProcedureAttrName() {
return "fir.internal_proc";
}
+/// Attribute containing the original name of a function from before the
+/// ExternalNameConverision pass runs
+static constexpr llvm::StringRef getInternalFuncNameAttrName() {
+ return "fir.internal_name";
+}
+
/// Does the function, \p func, have a host-associations tuple argument?
/// Some internal procedures may have access to host procedure variables.
bool hasHostAssociationArgument(mlir::func::FuncOp func);
diff --git a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
index e967a8f19d53af3..221e93ff85e18e5 100644
--- a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
@@ -9,11 +9,13 @@
#include "flang/Common/Fortran.h"
#include "flang/Optimizer/Dialect/FIRDialect.h"
#include "flang/Optimizer/Dialect/FIROps.h"
+#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Support/InternalNames.h"
#include "flang/Optimizer/Transforms/Passes.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/Attributes.h"
#include "mlir/IR/SymbolTable.h"
#include "mlir/Pass/Pass.h"
#include "mlir/Transforms/DialectConversion.h"
@@ -75,7 +77,8 @@ struct MangleNameOnFuncOp : public mlir::OpRewritePattern<mlir::func::FuncOp> {
mlir::PatternRewriter &rewriter) const override {
mlir::LogicalResult ret = success();
rewriter.startRootUpdate(op);
- auto result = fir::NameUniquer::deconstruct(op.getSymName());
+ llvm::StringRef oldName = op.getSymName();
+ auto result = fir::NameUniquer::deconstruct(oldName);
if (fir::NameUniquer::isExternalFacingUniquedName(result)) {
auto newSymbol =
rewriter.getStringAttr(mangleExternalName(result, appendUnderscore));
@@ -86,6 +89,9 @@ struct MangleNameOnFuncOp : public mlir::OpRewritePattern<mlir::func::FuncOp> {
op.setSymNameAttr(newSymbol);
mlir::SymbolTable::setSymbolName(op, newSymbol);
+
+ op->setAttr(fir::getInternalFuncNameAttrName(),
+ mlir::StringAttr::get(op->getContext(), oldName));
}
updateEarlyOutliningParentName(op, appendUnderscore);
diff --git a/flang/test/Fir/external-mangling.fir b/flang/test/Fir/external-mangling.fir
index 5e7cb978838e4a3..06e1e2e412295c9 100644
--- a/flang/test/Fir/external-mangling.fir
+++ b/flang/test/Fir/external-mangling.fir
@@ -49,8 +49,8 @@ func.func private @_QPbar2(!fir.ref<f32>)
// LLVMIR-UNDER: llvm.mlir.global common @a_(dense<0> : vector<4xi8>) {{.*}} : !llvm.array<4 x i8>
// LLVMIR-UNDER: llvm.mlir.global common @__BLNK__(dense<0> : vector<4xi8>) {{.*}} : !llvm.array<4 x i8>
-// LLVMIR-UNDER: llvm.func @bar_(!llvm.ptr) attributes {sym_visibility = "private"}
-// LLVMIR-UNDER: llvm.func @bar2_(!llvm.ptr) attributes {sym_visibility = "private"}
+// LLVMIR-UNDER: llvm.func @bar_(!llvm.ptr) attributes {fir.internal_name = "_QPbar", sym_visibility = "private"}
+// LLVMIR-UNDER: llvm.func @bar2_(!llvm.ptr) attributes {fir.internal_name = "_QPbar2", sym_visibility = "private"}
// LLVMIR-NOUNDER: %{{.*}} = llvm.mlir.addressof @a : !llvm.ptr
// LLVMIR-NOUNDER: %{{.*}} = llvm.mlir.addressof @__BLNK__ : !llvm.ptr
@@ -59,8 +59,8 @@ func.func private @_QPbar2(!fir.ref<f32>)
// LLVMIR-NOUNDER: llvm.mlir.global common @a(dense<0> : vector<4xi8>) {{.*}} : !llvm.array<4 x i8>
// LLVMIR-NOUNDER: llvm.mlir.global common @__BLNK__(dense<0> : vector<4xi8>) {{.*}} : !llvm.array<4 x i8>
-// LLVMIR-NOUNDER: llvm.func @bar(!llvm.ptr) attributes {sym_visibility = "private"}
-// LLVMIR-NOUNDER: llvm.func @bar2(!llvm.ptr) attributes {sym_visibility = "private"}
+// LLVMIR-NOUNDER: llvm.func @bar(!llvm.ptr) attributes {fir.internal_name = "_QPbar", sym_visibility = "private"}
+// LLVMIR-NOUNDER: llvm.func @bar2(!llvm.ptr) attributes {fir.internal_name = "_QPbar2", sym_visibility = "private"}
// -----
@@ -96,5 +96,5 @@ func.func @_QPwriteindex_omp_outline_0() attributes {omp.outline_parent_name = "
return
}
-// CHECK-UNDER: attributes {omp.outline_parent_name = "writeindex_"}
-// CHECK-NOUNDER: attributes {omp.outline_parent_name = "writeindex"}
+// CHECK-UNDER: attributes {fir.internal_name = "_QPwriteindex_omp_outline_0", omp.outline_parent_name = "writeindex_"}
+// CHECK-NOUNDER: attributes {fir.internal_name = "_QPwriteindex_omp_outline_0", omp.outline_parent_name = "writeindex"}
|
@vzakhari please could you check if this resolves your tbaa performance regressions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will let Slava confirm this solves the performance issue, but this looks good to me otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding and fixing the issue, Tom! The fix restores leslie3d performance.
Enable by default for optimization levels higher than 0 (same behavior as clang). For simplicity, only forward the flag to the frontend driver when it contradicts what is implied by the optimization level. This was first landed in llvm#73111 but was later reverted due to a performance regression. That regression was fixed by llvm#74065.
Enable by default for optimization levels higher than 0 (same behavior as clang). For simplicity, only forward the flag to the frontend driver when it contradicts what is implied by the optimization level. This was first landed in #73111 but was later reverted due to a performance regression. That regression was fixed by #74065.
We pass TBAA alias information with separate TBAA trees per function (to prevent incorrect alias information after inlining). These TBAA trees are identified by a unique string per function. Naturally, we use the mangled name of the function.
TBAA tags are added in two places: during a dedicated pass relatively early (structured control flow makes fir::AliasAnalysis more accurate), then again during CodeGen (when implied box loads and stores become visible). In between these two passes, the ExternalNameConversion pass changes the name of some functions.
These functions with changed names previously ended up with separate TBAA trees from the TBAA tags pass and from CodeGen - leading LLVM to think that all data accesses alias with all descriptor accesses.
This patch solves this by storing the original name of a function in an attribute during the ExternalNameConversion pass, and using the name from that attribute when creating TBAA trees during CodeGen.