[flang] use TBAA Forest in TBAABuilder #68437
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This interface allows (HL)FIR passes to add TBAA information to fir.load and fir.store. If present, these TBAA tags take precedence over those added during CodeGen. We can't reuse mlir::LLVMIR::AliasAnalysisOpInterface because that uses the mlir::LLVMIR namespace so it tries to define methods for fir operations in the wrong namespace. But I did re-use the tbaa tag type to minimise boilerplate code. The new builders are to preserve the old interface without the tbaa tag.
For TBBAA attribute types.
tblah
requested review from
kiranchandramohan,
jeanPerier,
Renaud-K and
vzakhari
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-codegen ChangesSee previous reviews at #68414 and #68317 This is important to ensure that tags end up in the same trees that were @vzakhari requested that the old (not-per-function) trees are Patch is 102.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68437.diff 25 Files Affected:
diff --git a/flang/include/flang/Optimizer/Analysis/TBAAForest.h b/flang/include/flang/Optimizer/Analysis/TBAAForest.h
new file mode 100644
index 000000000000000..3d11e6112bacce9
--- /dev/null
+++ b/flang/include/flang/Optimizer/Analysis/TBAAForest.h
@@ -0,0 +1,104 @@
+//===-- TBAAForest.h - A TBAA tree for each function -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_OPTIMIZER_ANALYSIS_TBAA_FOREST_H
+#define FORTRAN_OPTIMIZER_ANALYSIS_TBAA_FOREST_H
+
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/MLIRContext.h"
+#include "llvm/ADT/DenseMap.h"
+#include <string>
+
+namespace fir {
+
+//===----------------------------------------------------------------------===//
+// TBAATree
+//===----------------------------------------------------------------------===//
+/// Per-function TBAA tree. Each tree contins branches for data (of various
+/// kinds) and descriptor access
+struct TBAATree {
+ //===----------------------------------------------------------------------===//
+ // TBAAForrest::TBAATree::SubtreeState
+ //===----------------------------------------------------------------------===//
+ /// This contains a TBAA subtree based on some parent. New tags can be added
+ /// under the parent using getTag.
+ class SubtreeState {
+ friend TBAATree; // only allow construction by TBAATree
+ public:
+ SubtreeState() = delete;
+ SubtreeState(const SubtreeState &) = delete;
+ SubtreeState(SubtreeState &&) = default;
+
+ mlir::LLVM::TBAATagAttr getTag(llvm::StringRef uniqueId) const;
+
+ private:
+ SubtreeState(mlir::MLIRContext *ctx, std::string name,
+ mlir::LLVM::TBAANodeAttr grandParent)
+ : parentId{std::move(name)}, context(ctx) {
+ parent = mlir::LLVM::TBAATypeDescriptorAttr::get(
+ context, parentId, mlir::LLVM::TBAAMemberAttr::get(grandParent, 0));
+ }
+
+ const std::string parentId;
+ mlir::MLIRContext *const context;
+ mlir::LLVM::TBAATypeDescriptorAttr parent;
+ llvm::DenseMap<llvm::StringRef, mlir::LLVM::TBAATagAttr> tagDedup;
+ };
+
+ SubtreeState globalDataTree;
+ SubtreeState allocatedDataTree;
+ SubtreeState dummyArgDataTree;
+ mlir::LLVM::TBAATypeDescriptorAttr anyAccessDesc;
+ mlir::LLVM::TBAATypeDescriptorAttr boxMemberTypeDesc;
+ mlir::LLVM::TBAATypeDescriptorAttr anyDataTypeDesc;
+
+ static TBAATree buildTree(mlir::StringAttr functionName);
+
+private:
+ TBAATree(mlir::LLVM::TBAATypeDescriptorAttr anyAccess,
+ mlir::LLVM::TBAATypeDescriptorAttr dataRoot,
+ mlir::LLVM::TBAATypeDescriptorAttr boxMemberTypeDesc);
+};
+
+//===----------------------------------------------------------------------===//
+// TBAAForrest
+//===----------------------------------------------------------------------===//
+/// Colletion of TBAATrees, usually indexed by function (so that each function
+/// has a different TBAATree)
+class TBAAForrest {
+public:
+ explicit TBAAForrest(bool separatePerFunction = true)
+ : separatePerFunction{separatePerFunction} {}
+
+ inline const TBAATree &operator[](mlir::func::FuncOp func) {
+ return getFuncTree(func.getSymNameAttr());
+ }
+ inline const TBAATree &operator[](mlir::LLVM::LLVMFuncOp func) {
+ return getFuncTree(func.getSymNameAttr());
+ }
+
+private:
+ const TBAATree &getFuncTree(mlir::StringAttr symName) {
+ if (!separatePerFunction)
+ symName = mlir::StringAttr::get(symName.getContext(), "");
+ if (!trees.contains(symName))
+ trees.insert({symName, TBAATree::buildTree(symName)});
+ return trees.at(symName);
+ }
+
+ // Should each function use a different tree?
+ const bool separatePerFunction;
+ // TBAA tree per function
+ llvm::DenseMap<mlir::StringAttr, TBAATree> trees;
+};
+
+} // namespace fir
+
+#endif // FORTRAN_OPTIMIZER_ANALYSIS_TBAA_FOREST_H
diff --git a/flang/include/flang/Optimizer/CodeGen/TBAABuilder.h b/flang/include/flang/Optimizer/CodeGen/TBAABuilder.h
index 3aeb124f911867e..2c5795678a91e16 100644
--- a/flang/include/flang/Optimizer/CodeGen/TBAABuilder.h
+++ b/flang/include/flang/Optimizer/CodeGen/TBAABuilder.h
@@ -13,8 +13,8 @@
#ifndef FORTRAN_OPTIMIZER_CODEGEN_TBAABUILDER_H
#define FORTRAN_OPTIMIZER_CODEGEN_TBAABUILDER_H
+#include "flang/Optimizer/Analysis/TBAAForest.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
-#include "mlir/IR/BuiltinAttributes.h"
namespace fir {
@@ -25,9 +25,9 @@ namespace fir {
//
// TBAA type information is represented with LLVM::MetadataOp operation
// with specific symbol name `TBAABuilder::tbaaMetaOpName`. The basic
-// TBAA tree used for Flang consists of the following nodes:
+// TBAA trees used for Flang consists of the following nodes:
// llvm.metadata @__flang_tbaa {
-// llvm.tbaa_root @root_0 {id = "Flang Type TBAA Root"}
+// llvm.tbaa_root @root_0 {id = "Flang Type TBAA Function Root funcName"}
// llvm.tbaa_type_desc @type_desc_1 {id = "any access",
// members = {<@root_0, 0>}}
// llvm.tbaa_type_desc @type_desc_2 {id = "any data access",
@@ -162,6 +162,9 @@ namespace fir {
// Given the storage association, all non-box accesses are represented
// with the conservative data access tag:
// < `<any data access>`, `<any data access>`, 0 >
+
+// additional tags are added in flang/Optimizer/Transforms/AddAliasTags.cpp
+// (before CodeGen)
class TBAABuilder {
public:
TBAABuilder(mlir::MLIRContext *context, bool applyTBAA);
@@ -184,13 +187,13 @@ class TBAABuilder {
// Returns TBAATagAttr representing access tag:
// < <descriptor member>, <descriptor member>, 0 >
- mlir::LLVM::TBAATagAttr getAnyBoxAccessTag();
+ mlir::LLVM::TBAATagAttr getAnyBoxAccessTag(mlir::LLVM::LLVMFuncOp func);
// Returns TBAATagAttr representing access tag:
// < <any data access>, <any data access>, 0 >
- mlir::LLVM::TBAATagAttr getAnyDataAccessTag();
+ mlir::LLVM::TBAATagAttr getAnyDataAccessTag(mlir::LLVM::LLVMFuncOp func);
// Returns TBAATagAttr representing access tag:
// < <any access>, <any access>, 0 >
- mlir::LLVM::TBAATagAttr getAnyAccessTag();
+ mlir::LLVM::TBAATagAttr getAnyAccessTag(mlir::LLVM::LLVMFuncOp func);
// Returns TBAATagAttr representing access tag described by the base and
// access FIR types and the LLVM::GepOp representing the access in terms of
@@ -198,7 +201,8 @@ class TBAABuilder {
// fir::BaseBoxType.
mlir::LLVM::TBAATagAttr getBoxAccessTag(mlir::Type baseFIRType,
mlir::Type accessFIRType,
- mlir::LLVM::GEPOp gep);
+ mlir::LLVM::GEPOp gep,
+ mlir::LLVM::LLVMFuncOp func);
// Returns TBAATagAttr representing access tag described by the base and
// access FIR types and the LLVM::GepOp representing the access in terms of
@@ -206,34 +210,13 @@ class TBAABuilder {
// "data" access, i.e. not an access of any box/descriptor member.
mlir::LLVM::TBAATagAttr getDataAccessTag(mlir::Type baseFIRType,
mlir::Type accessFIRType,
- mlir::LLVM::GEPOp gep);
+ mlir::LLVM::GEPOp gep,
+ mlir::LLVM::LLVMFuncOp func);
// Set to true, if TBAA builder is active, otherwise, all public
// methods are no-ops.
bool enableTBAA;
- // LLVM::TBAARootAttr identifying Flang's TBAA root.
- mlir::LLVM::TBAARootAttr flangTBAARoot;
- // Identity string for Flang's TBAA root.
- static constexpr llvm::StringRef flangTBAARootId = "Flang Type TBAA Root";
-
- // LLVM::TBAATypeDescriptorAttr identifying "any access".
- mlir::LLVM::TBAATypeDescriptorAttr anyAccessTypeDesc;
- // Identity string for "any access" type descriptor.
- static constexpr llvm::StringRef anyAccessTypeDescId = "any access";
-
- // LLVM::TBAATypeDescriptorAttr identifying "any data access" (i.e. non-box
- // memory access).
- mlir::LLVM::TBAATypeDescriptorAttr anyDataAccessTypeDesc;
- // Identity string for "any data access" type descriptor.
- static constexpr llvm::StringRef anyDataAccessTypeDescId = "any data access";
-
- // LLVM::TBAATypeDescriptorAttr identifying "descriptor member" access, i.e.
- // any access within the bounds of a box/descriptor.
- mlir::LLVM::TBAATypeDescriptorAttr boxMemberTypeDesc;
- // Identity string for "descriptor member" type descriptor.
- static constexpr llvm::StringRef boxMemberTypeDescId = "descriptor member";
-
// Number of attached TBAA tags (used for debugging).
unsigned tagAttachmentCounter = 0;
@@ -247,6 +230,8 @@ class TBAABuilder {
std::tuple<mlir::LLVM::TBAANodeAttr, mlir::LLVM::TBAANodeAttr, int64_t>,
mlir::LLVM::TBAATagAttr>
tagsMap;
+
+ TBAAForrest trees;
};
} // namespace fir
diff --git a/flang/include/flang/Optimizer/Dialect/CMakeLists.txt b/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
index d657e3f16690377..15c835aad9bc7d2 100644
--- a/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
+++ b/flang/include/flang/Optimizer/Dialect/CMakeLists.txt
@@ -18,6 +18,10 @@ set(LLVM_TARGET_DEFINITIONS FortranVariableInterface.td)
mlir_tablegen(FortranVariableInterface.h.inc -gen-op-interface-decls)
mlir_tablegen(FortranVariableInterface.cpp.inc -gen-op-interface-defs)
+set(LLVM_TARGET_DEFINITIONS FirAliasAnalysisOpInterface.td)
+mlir_tablegen(FirAliasAnalaysOpInterface.h.inc -gen-op-interface-decls)
+mlir_tablegen(FirAliasAnalysisOpInterface.cpp.inc -gen-op-interface-defs)
+
set(LLVM_TARGET_DEFINITIONS CanonicalizationPatterns.td)
mlir_tablegen(CanonicalizationPatterns.inc -gen-rewriters)
add_public_tablegen_target(CanonicalizationPatternsIncGen)
diff --git a/flang/include/flang/Optimizer/Dialect/FIRDialect.td b/flang/include/flang/Optimizer/Dialect/FIRDialect.td
index d0735bbeb2d3d88..b366b6d40e4e213 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRDialect.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRDialect.td
@@ -30,7 +30,9 @@ def fir_Dialect : Dialect {
let dependentDialects = [
// Arith dialect provides FastMathFlagsAttr
// supported by some FIR operations.
- "arith::ArithDialect"
+ "arith::ArithDialect",
+ // TBAA Tag types
+ "LLVM::LLVMDialect"
];
}
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index 8f03dc5cf795225..bab35bac5c81f4b 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -11,6 +11,7 @@
#include "flang/Optimizer/Dialect/FIRAttr.h"
#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h"
#include "flang/Optimizer/Dialect/FortranVariableInterface.h"
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index a57add9f731979d..fae9b92662f3559 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -16,10 +16,12 @@
include "mlir/Dialect/Arith/IR/ArithBase.td"
include "mlir/Dialect/Arith/IR/ArithOpsInterfaces.td"
+include "mlir/Dialect/LLVMIR/LLVMAttrDefs.td"
include "flang/Optimizer/Dialect/FIRDialect.td"
include "flang/Optimizer/Dialect/FIRTypes.td"
include "flang/Optimizer/Dialect/FIRAttr.td"
include "flang/Optimizer/Dialect/FortranVariableInterface.td"
+include "flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.td"
include "mlir/IR/BuiltinAttributes.td"
// Base class for FIR operations.
@@ -258,7 +260,7 @@ def fir_FreeMemOp : fir_Op<"freemem", [MemoryEffects<[MemFree]>]> {
let assemblyFormat = "$heapref attr-dict `:` qualified(type($heapref))";
}
-def fir_LoadOp : fir_OneResultOp<"load", []> {
+def fir_LoadOp : fir_OneResultOp<"load", [FirAliasAnalysisOpInterface]> {
let summary = "load a value from a memory reference";
let description = [{
Load a value from a memory reference into an ssa-value (virtual register).
@@ -274,9 +276,11 @@ def fir_LoadOp : fir_OneResultOp<"load", []> {
or null.
}];
- let arguments = (ins Arg<AnyReferenceLike, "", [MemRead]>:$memref);
+ let arguments = (ins Arg<AnyReferenceLike, "", [MemRead]>:$memref,
+ OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
- let builders = [OpBuilder<(ins "mlir::Value":$refVal)>];
+ let builders = [OpBuilder<(ins "mlir::Value":$refVal)>,
+ OpBuilder<(ins "mlir::Type":$resTy, "mlir::Value":$refVal)>];
let hasCustomAssemblyFormat = 1;
@@ -285,7 +289,7 @@ def fir_LoadOp : fir_OneResultOp<"load", []> {
}];
}
-def fir_StoreOp : fir_Op<"store", []> {
+def fir_StoreOp : fir_Op<"store", [FirAliasAnalysisOpInterface]> {
let summary = "store an SSA-value to a memory location";
let description = [{
@@ -305,7 +309,10 @@ def fir_StoreOp : fir_Op<"store", []> {
}];
let arguments = (ins AnyType:$value,
- Arg<AnyReferenceLike, "", [MemWrite]>:$memref);
+ Arg<AnyReferenceLike, "", [MemWrite]>:$memref,
+ OptionalAttr<LLVM_TBAATagArrayAttr>:$tbaa);
+
+ let builders = [OpBuilder<(ins "mlir::Value":$value, "mlir::Value":$memref)>];
let hasCustomAssemblyFormat = 1;
let hasVerifier = 1;
diff --git a/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h b/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h
new file mode 100644
index 000000000000000..c07bf648eab454a
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h
@@ -0,0 +1,27 @@
+//===- FirAliasAnalysisInterface.h ------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains an interface for adding alias analysis information to
+// loads and stores
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef FORTRAN_OPTIMIZER_DIALECT_FIR_ALIAS_ANALYSIS_INTERFACE_H
+#define FORTRAN_OPTIMIZER_DIALECT_FIR_ALIAS_ANALYSIS_INTERFACE_H
+
+#include "mlir/IR/OpDefinition.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/Support/LogicalResult.h"
+
+namespace fir::detail {
+mlir::LogicalResult verifyFirAliasAnalysisOpInterface(mlir::Operation *op);
+} // namespace fir::detail
+
+#include "flang/Optimizer/Dialect/FirAliasAnalaysOpInterface.h.inc"
+
+#endif // FORTRAN_OPTIMIZER_DIALECT_FIR_ALIAS_ANALYSIS_INTERFACE_H
diff --git a/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.td b/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.td
new file mode 100644
index 000000000000000..1d3e49d383f63a4
--- /dev/null
+++ b/flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.td
@@ -0,0 +1,59 @@
+//===-- FirAliasAnalysisOpInterface.td ---------------------*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+include "mlir/IR/Interfaces.td"
+
+def FirAliasAnalysisOpInterface : OpInterface<"FirAliasAnalysisOpInterface"> {
+ let description = [{
+ An interface for memory operations that can carry alias analysis metadata.
+ It provides setters and getters for the operation's alias analysis
+ attributes. The default implementations of the interface methods expect
+ the operation to have an attribute of type ArrayAttr named tbaa.
+ Unlike the mlir::LLVM::AliasAnalysisOpInterface, this only supports tbaa.
+ }];
+
+ let cppNamespace = "::fir";
+ let verify = [{ return detail::verifyFirAliasAnalysisOpInterface($_op); }];
+
+ let methods = [
+ InterfaceMethod<
+ /*desc=*/ "Returns the tbaa attribute or nullptr",
+ /*returnType=*/ "mlir::ArrayAttr",
+ /*methodName=*/ "getTBAATagsOrNull",
+ /*args=*/ (ins),
+ /*methodBody=*/ [{}],
+ /*defaultImpl=*/ [{
+ auto op = mlir::cast<ConcreteOp>(this->getOperation());
+ return op.getTbaaAttr();
+ }]
+ >,
+ InterfaceMethod<
+ /*desc=*/ "Sets the tbaa attribute",
+ /*returnType=*/ "void",
+ /*methodName=*/ "setTBAATags",
+ /*args=*/ (ins "const mlir::ArrayAttr":$attr),
+ /*methodBody=*/ [{}],
+ /*defaultImpl=*/ [{
+ auto op = mlir::cast<ConcreteOp>(this->getOperation());
+ op.setTbaaAttr(attr);
+ }]
+ >,
+ InterfaceMethod<
+ /*desc=*/ "Returns a list of all pointer operands accessed by the "
+ "operation",
+ /*returnType=*/ "::llvm::SmallVector<::mlir::Value>",
+ /*methodName=*/ "getAccessedOperands",
+ /*args=*/ (ins),
+ /*methodBody=*/ [{}],
+ /*defaultImpl=*/ [{
+ auto op = mlir::cast<ConcreteOp>(this->getOperation());
+ return {op.getMemref()};
+ }]
+ >
+ ];
+}
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 64882c8ec406b0a..30d97be3800c191 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -61,6 +61,7 @@ std::unique_ptr<mlir::Pass> createMemDataFlowOptPass();
std::unique_ptr<mlir::Pass> createPromoteToAffinePass();
std::unique_ptr<mlir::Pass> createMemoryAllocationPass();
std::unique_ptr<mlir::Pass> createStackArraysPass();
+std::unique_ptr<mlir::Pass> createAliasTagsPass();
std::unique_ptr<mlir::Pass> createSimplifyIntrinsicsPass();
std::unique_ptr<mlir::Pass> createAddDebugFoundationPass();
std::unique_ptr<mlir::Pass> createLoopVersioningPass();
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 80da485392007fa..7aa08b0616de99d 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -252,6 +252,26 @@ def StackArrays : Pass<"stack-arrays", "mlir::ModuleOp"> {
let constructor = "::fir::createStackArraysPass()";
}
+def AddAliasTags : Pass<"fir-alias-analysis", "mlir::ModuleOp"> {
+ let summary = "Add tbaa tags to operations that implement FirAliasAnalysisOpInterface";
+ let description = [{
+ TBAA (type based alias analysis) is one method to pass pointer alias information
+ from language frontends to LLVM. This pass uses fir::AliasAnalysis to add this
+ information to fir.load and fir.store operations.
+ Additional tags are added during codegen. See fir::TBAABuilder.
+ This needs to be a separate pass so that it happens before structured control
+ flow operations are lowered to branches and basic blocks (this makes tracing
+ the source of values much eaiser). The other TBAA tags need to be applied to
+ box loads and stores which are implicit in FIR and so cannot be annotated
+ until codegen.
+ TODO: this is currently a pass on mlir::ModuleOp to avoid parallelism. In
+ theory, each operation could be considered in prallel, so long as there
+ aren't races adding new tags to the mlir context.
+ }];
+ let dependentDialects = [ "fir::FIROpsDialect" ];
+ let constructor = "::fir::createAliasTagsPass()";
+}
+
def SimplifyRegionLite : Pass<"simplify-region-lite", "mlir::ModuleOp"> {
let summary = "Region simplification";
let description = [{
diff --git a/flang/lib/Optimizer/Analysis/CMakeLists.txt b/flang/lib/Optimizer/Analysis/CMakeLists.txt
index 19dadf72cf4ce14..436d4d3f18969c1 100644
--- a/flang/lib/Optimizer/Analysis/CMakeLists.txt
+++ b/flang/lib/Optimizer/Analysis/...
[truncated]
|
vzakhari
reviewed
Oct 9, 2023
| @@ -39,26 +49,10 @@ static llvm::cl::opt<unsigned> | |||
| namespace fir { | |||
|
|
|||
| TBAABuilder::TBAABuilder(MLIRContext *context, bool applyTBAA) | |||
| : enableTBAA(applyTBAA && !disableTBAA) { | |||
| : enableTBAA(applyTBAA && !disableTBAA), | |||
| trees(/*separatePerFunction=*/perFunctionTBAATrees) { | |||
There was a problem hiding this comment.
Do I understand it correctly that even without running the FIR TBAA attachment pass we will use per-function trees by default after this change? I wonder if we can still stick to a single global tree for those functions that have not been affected by the FIR TBAA attachment pass?
There was a problem hiding this comment.
Yes we will. Although, my intention is to enable my FIR pass whenever we are using TBAA so I don't anticipate TBAABuilder will often run without the FIR alias tags pass..
I am yet to see evidence of significant slowdowns due to using per-function TBAA trees. The FIR pass applies tags to every function which has dummy arguments, so I suspect keeping the global tree for unaffected functions which slow down due to per-function trees is a very uncommon case.
There was a problem hiding this comment.
Thanks for explaining. My only concern is a "clean" switch back to the previous state of things (e.g. to triage new failures, if any). With this change, we will have to specify two options to switch to the TBAA that we currently have, i.e. we will have to disable the new FIR TBAA pass, and also disable the per-function behavior for the codegen. It is not a big deal, I just wanted to confirm that this is the case.
There was a problem hiding this comment.
Thanks. Would a single flag in CLOptions.inc specifying to use the old behavior be easier?
There was a problem hiding this comment.
I think so, but I am okay with having the two flags as well.
There was a problem hiding this comment.
Thank you! It looks good to me.
See RFC at https://discourse.llvm.org/t/rfc-propagate-fir-alias-analysis-information-using-tbaa/73755 This pass adds TBAA tags to all accesses to non-pointer/target dummy arguments. These TBAA tags tell LLVM that these accesses cannot alias: allowing better dead code elimination, hoisting out of loops, and vectorization. Each function has its own TBAA tree so that accesses between funtions MayAlias after inlining. I also included code for adding tags for local allocations and for global variables. Enabling all three kinds of tag is known to produce a miscompile and so these are disabled by default. But it isn't much code and I thought it could be interesting to play with these later if one is looking at a benchmark which looks like it would benefit from more alias information. I'm open to removing this code too. TBAA tags are also added separately by TBAABuilder during CodeGen. TBAABuilder has to run during CodeGen because it adds tags to box accesses, many of which are implicit in FIR. This pass cannot (easily) run in CodeGen because fir::AliasAnalysis has difficulty tracing values between blocks, and by the time CodeGen runs, structured control flow has already been lowered. Coming in follow up patches - Change CodeGen/TBAABuilder to use TBAAForest to add tags within the same per-function trees as are used here (delayed to a later patch to make it easier to revert) - Command line argument processing to actually enable the pass
This is important to ensure that tags end up in the same trees that were created in the FIR TBAA pass. If they are in different trees then everything in one tree will be assumed to MayAlias with everything in the other tree. This leads to poor performance. @vzakhari requested that the old (not-per-function) trees are maintained so I left the old test intact.
This is fallout from getting argument names from fir.declare operations instead of fir.bindc attributes.
tblah
force-pushed
the
ecclescake/tbaa-codegen
branch
from
95b35e1
to
5debd9a
Compare
|
Force push to include fixup commits from other PRs. |
vzakhari
approved these changes
Oct 9, 2023
tblah
added a commit
to tblah/llvm-project
that referenced
this pull request
Oct 10, 2023
The ultimate intention is to have this pass enabled by default whenever we are optimizing for speed. But for now, just add the arguments so this can be more easily tested. Previous PR in this series: llvm#68437
tblah
added a commit
that referenced
this pull request
Oct 11, 2023
This is important to ensure that tags end up in the same trees that were created in the FIR TBAA pass. If they are in different trees then everything in one tree will be assumed to MayAlias with everything in the other tree. This leads to poor performance. @vzakhari requested that the old (not-per-function) trees are maintained so I left the old test intact. PR: #68437
|
Merged manually in 6042c2e |
tblah
added a commit
that referenced
this pull request
Oct 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See previous reviews at #68414 and #68317
This is important to ensure that tags end up in the same trees that were
created in the FIR TBAA pass. If they are in different trees then
everything in one tree will be assumed to MayAlias with everything in the
other tree. This leads to poor performance.
After this change, the trees generated by TBAABuilder are separated per-function. This could lead to less precise alias information after in-lining, but no serious performance regressions have been found so far.
@vzakhari requested that the old (not-per-function) trees are
maintained so I left the old test intact.