Skip to content

[flang][FIR] add FirAliasAnalysisOpInterface #68317

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

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Oct 5, 2023

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.

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

llvmbot commented Oct 5, 2023

@llvm/pr-subscribers-flang-codegen

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

Changes

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.


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

10 Files Affected:

  • (modified) flang/include/flang/Optimizer/Dialect/CMakeLists.txt (+4)
  • (modified) flang/include/flang/Optimizer/Dialect/FIROps.h (+1)
  • (modified) flang/include/flang/Optimizer/Dialect/FIROps.td (+12-5)
  • (added) flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h (+27)
  • (added) flang/include/flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.td (+59)
  • (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+12-3)
  • (modified) flang/lib/Optimizer/Dialect/CMakeLists.txt (+1)
  • (modified) flang/lib/Optimizer/Dialect/FIROps.cpp (+16-1)
  • (added) flang/lib/Optimizer/Dialect/FirAliasAnalysisOpInterface.cpp (+31)
  • (added) flang/test/Fir/tbaa-codegen.fir (+47)
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/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/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index d1b7f3de93b4647..f2ce123124895e0 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3085,7 +3085,10 @@ struct LoadOpConversion : public FIROpConversion<fir::LoadOp> {
       auto boxValue = rewriter.create<mlir::LLVM::LoadOp>(
           loc, boxPtrTy.cast<mlir::LLVM::LLVMPointerType>().getElementType(),
           inputBoxStorage);
-      attachTBAATag(boxValue, boxTy, boxTy, nullptr);
+      if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
+        boxValue.setTBAATags(*optionalTag);
+      else
+        attachTBAATag(boxValue, boxTy, boxTy, nullptr);
       auto newBoxStorage =
           genAllocaWithType(loc, boxPtrTy, defaultAlign, rewriter);
       auto storeOp =
@@ -3096,7 +3099,10 @@ struct LoadOpConversion : public FIROpConversion<fir::LoadOp> {
       mlir::Type loadTy = convertType(load.getType());
       auto loadOp = rewriter.create<mlir::LLVM::LoadOp>(
           load.getLoc(), loadTy, adaptor.getOperands(), load->getAttrs());
-      attachTBAATag(loadOp, load.getType(), load.getType(), nullptr);
+      if (std::optional<mlir::ArrayAttr> optionalTag = load.getTbaa())
+        loadOp.setTBAATags(*optionalTag);
+      else
+        attachTBAATag(loadOp, load.getType(), load.getType(), nullptr);
       rewriter.replaceOp(load, loadOp.getResult());
     }
     return mlir::success();
@@ -3340,7 +3346,10 @@ struct StoreOpConversion : public FIROpConversion<fir::StoreOp> {
       newStoreOp = rewriter.create<mlir::LLVM::StoreOp>(
           loc, adaptor.getOperands()[0], adaptor.getOperands()[1]);
     }
-    attachTBAATag(newStoreOp, storeTy, storeTy, nullptr);
+    if (std::optional<mlir::ArrayAttr> optionalTag = store.getTbaa())
+      newStoreOp.setTBAATags(*optionalTag);
+    else
+      attachTBAATag(newStoreOp, storeTy, storeTy, nullptr);
     rewriter.eraseOp(store);
     return mlir::success();
   }
diff --git a/flang/lib/Optimizer/Dialect/CMakeLists.txt b/flang/lib/Optimizer/Dialect/CMakeLists.txt
index fe5edb54a78e9e5..6beabcdb4e25d76 100644
--- a/flang/lib/Optimizer/Dialect/CMakeLists.txt
+++ b/flang/lib/Optimizer/Dialect/CMakeLists.txt
@@ -6,6 +6,7 @@ add_flang_library(FIRDialect
   FIROps.cpp
   FIRType.cpp
   FortranVariableInterface.cpp
+  FirAliasAnalysisOpInterface.cpp
   Inliner.cpp
 
   DEPENDS
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 962b87acd5a8050..2f08cd1b8111531 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -2016,8 +2016,18 @@ void fir::LoadOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
     mlir::emitError(result.location, "not a memory reference type");
     return;
   }
+  build(builder, result, eleTy, refVal);
+}
+
+void fir::LoadOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
+                        mlir::Type resTy, mlir::Value refVal) {
+
+  if (!refVal) {
+    mlir::emitError(result.location, "LoadOp has null argument");
+    return;
+  }
   result.addOperands(refVal);
-  result.addTypes(eleTy);
+  result.addTypes(resTy);
 }
 
 mlir::ParseResult fir::LoadOp::getElementOf(mlir::Type &ele, mlir::Type ref) {
@@ -3288,6 +3298,11 @@ mlir::LogicalResult fir::StoreOp::verify() {
   return mlir::success();
 }
 
+void fir::StoreOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
+                         mlir::Value value, mlir::Value memref) {
+  build(builder, result, value, memref, {});
+}
+
 //===----------------------------------------------------------------------===//
 // StringLitOp
 //===----------------------------------------------------------------------===//
diff --git a/flang/lib/Optimizer/Dialect/FirAliasAnalysisOpInterface.cpp b/flang/lib/Optimizer/Dialect/FirAliasAnalysisOpInterface.cpp
new file mode 100644
index 000000000000000..63686b1d0e7ebb6
--- /dev/null
+++ b/flang/lib/Optimizer/Dialect/FirAliasAnalysisOpInterface.cpp
@@ -0,0 +1,31 @@
+//===-- FirAliasAnalysisOpInterface.cpp ----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
+//
+//===----------------------------------------------------------------------===//
+
+#include "flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+
+#include "flang/Optimizer/Dialect/FirAliasAnalysisOpInterface.cpp.inc"
+
+mlir::LogicalResult
+fir::detail::verifyFirAliasAnalysisOpInterface(mlir::Operation *op) {
+  auto iface = mlir::cast<FirAliasAnalysisOpInterface>(op);
+
+  mlir::ArrayAttr tags = iface.getTBAATagsOrNull();
+  if (!tags)
+    return mlir::success();
+
+  for (mlir::Attribute iter : tags)
+    if (!mlir::isa<mlir::LLVM::TBAATagAttr>(iter))
+      return op->emitOpError("expected op to return array of ")
+             << mlir::LLVM::TBAATagAttr::getMnemonic() << " attributes";
+  return mlir::success();
+}
diff --git a/flang/test/Fir/tbaa-codegen.fir b/flang/test/Fir/tbaa-codegen.fir
new file mode 100644
index 000000000000000..386fe42eaaba9a2
--- /dev/null
+++ b/flang/test/Fir/tbaa-codegen.fir
@@ -0,0 +1,47 @@
+// test that tbaa attributes can be added to fir.load and fir.store
+// and that these attributes are propagated to LLVMIR
+
+// RUN: tco %s | FileCheck %s
+
+// subroutine simple(a)
+//   integer, intent(inout) :: a(:)
+//   a(1) = a(2)
+// end subroutine
+#tbaa_root = #llvm.tbaa_root<id = "Flang function root _QPsimple">
+#tbaa_type_desc = #llvm.tbaa_type_desc<id = "any access", members = {<#tbaa_root, 0>}>
+#tbaa_type_desc1 = #llvm.tbaa_type_desc<id = "any data access", members = {<#tbaa_type_desc, 0>}>
+#tbaa_type_desc2 = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#tbaa_type_desc1, 0>}>
+#tbaa_type_desc3 = #llvm.tbaa_type_desc<id = "dummy arg data/a", members = {<#tbaa_type_desc2, 0>}>
+#tbaa_tag = #llvm.tbaa_tag<base_type = #tbaa_type_desc3, access_type = #tbaa_type_desc3, offset = 0>
+module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.target_triple = "aarch64-unknown-linux-gnu"} {
+  func.func @_QPsimple(%arg0: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "a"}) {
+    %c1 = arith.constant 1 : index
+    %c2 = arith.constant 2 : index
+    %0 = fir.declare %arg0 {fortran_attrs = #fir.var_attrs<intent_inout>, uniq_name = "_QFfuncEa"} : (!fir.box<!fir.array<?xi32>>) -> !fir.box<!fir.array<?xi32>>
+    %1 = fir.rebox %0 : (!fir.box<!fir.array<?xi32>>) -> !fir.box<!fir.array<?xi32>>
+    %2 = fir.array_coor %1 %c2 : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+    %3 = fir.load %2 {tbaa = [#tbaa_tag]} : !fir.ref<i32>
+    %4 = fir.array_coor %1 %c1 : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+    fir.store %3 to %4 {tbaa = [#tbaa_tag]} : !fir.ref<i32>
+    return
+  }
+}
+
+// CHECK-LABEL: define void @_QPsimple(
+// CHECK-SAME:      ptr %[[ARG0:.*]]) {
+// [...]
+// load  a(2):
+// CHECK:  %[[VAL20:.*]] = getelementptr i8, ptr %{{.*}}, i64 %{{.*}}
+// CHECK:  %[[A2:.*]] = load i32, ptr %[[VAL20]], align 4, !tbaa ![[A_ACCESS_TAG:.*]]
+// [...]
+// store a(2) to a(1):
+// CHECK:  %[[A1:.*]] = getelementptr i8, ptr %{{.*}}, i64 %{{.*}}
+// CHECK:  store i32 %[[A2]], ptr %[[A1]], align 4, !tbaa ![[A_ACCESS_TAG]]
+// CHECK:  ret void
+// CHECK: }
+// CHECK: ![[A_ACCESS_TAG]] = !{![[A_ACCESS_TYPE:.*]], ![[A_ACCESS_TYPE]], i64 0}
+// CHECK: ![[A_ACCESS_TYPE]] = !{!"dummy arg data/a", ![[DUMMY_ARG_TYPE:.*]], i64 0}
+// CHECK: ![[DUMMY_ARG_TYPE]] = !{!"dummy arg data", ![[DATA_ACCESS_TYPE:.*]], i64 0}
+// CHECK: ![[DATA_ACCESS_TYPE]] = !{!"any data access", ![[ANY_ACCESS_TYPE:.*]], i64 0}
+// CHECK: ![[ANY_ACCESS_TYPE]] = !{!"any access", ![[ROOT:.*]], i64 0}
+// CHECK: ![[ROOT]] = !{!"Flang function root _QPsimple"}
\ No newline at end of file

@Renaud-K
Copy link
Contributor

Renaud-K commented Oct 5, 2023

We were thinking of introducing a FirAliasAnalysisInterface to allow for the memory source to be followed and avoid the big TypeSwitch in AliasAnalyis.cpp as well as maybe gthering other data such as the pointer and target attributes. Would we still be able to move in that direction ?

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

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

Thanks for sharing this for review, the way this is implemented/added on top FIR operations looks good to me!

I agree with Renaud's point that we will likely want some kind of alias interface to use in alias analysis. My guess is that they would anyway be on the same ops and could be the same interface and that is fine with me, but if we want to keep this interface TBAA centric (e.g., if TBAA won't be placed on HLFIR operations, but added in a later pass) I would rename it something like FirTBAAOpInterface.

Regarding the overall vision of how this plugs with TBAA in FIR, I will rely on @vzakhari review because I have little experience with LLVM TBAA.

@tblah
Copy link
Contributor Author

tblah commented Oct 6, 2023

Thanks for taking a look. My interface is just a stripped down version of the one in mlir::LLVMIR: https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td#L124. I named it to mirror the name used there. Presumably once full restrict is merged, it will be added to that interface in LLVMIR and we can copy it here.

But if we would like to implement alias analysis via interfaces, maybe we would be better renaming this something like AliasTagOpInterface so it is clear that this one is for propagating alias information rather than performing the analysis. I would prefer not to put TBAA in the name so that this interface can be evolved over time to represent alias information in other ways (e.g. full restrict).

@Renaud-K I think we can still move in that direction. As I understand it, the implementation of fir::AliasAnalysis should be orthogonal to what I am doing here. Will the interface stay the same? I would like to keep fir::AliasAnalysis::getSource.

@tblah tblah mentioned this pull request Oct 6, 2023
@Renaud-K
Copy link
Contributor

Renaud-K commented Oct 6, 2023

I can see how fir::AliasAnalysis::getSource can be useful on its own. It is public now, so feel free to use it.
We will want the option to implement fir::AliasAnalysis with interfaces. The new name you propose is fine by me. Thank you!

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 for doing this, Tom! It looks good to me. I think we should not name the interface as TBAA specific just yet.

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.
@tblah tblah force-pushed the ecclescake/firaliasanalysisinterface branch from 1f7ea39 to 3f15283 Compare October 11, 2023 14:06
@tblah tblah merged commit 8301e48 into llvm:main Oct 11, 2023
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.

5 participants