-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[flang][AliasAnalysis] Cray pointers/pointees might alias with anything #170900
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][AliasAnalysis] Cray pointers/pointees might alias with anything #170900
Conversation
The LOC intrinsic allows a cray pointer to alias with ordinary variables.
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Tom Eccles (tblah) ChangesThe LOC intrinsic allows a cray pointer to alias with ordinary variables with no other attribute. See the new test for an example. First part of the un-revert of #169544. That will handle TBAA. Patch is 40.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170900.diff 10 Files Affected:
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 30dd5f754d0b5..455100ff3c003 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -45,7 +45,7 @@ struct AliasAnalysis {
Unknown);
/// Attributes of the memory source object.
- ENUM_CLASS(Attribute, Target, Pointer, IntentIn);
+ ENUM_CLASS(Attribute, Target, Pointer, IntentIn, CrayPointer, CrayPointee);
// See
// https://discourse.llvm.org/t/rfc-distinguish-between-data-and-non-data-in-fir-alias-analysis/78759/1
@@ -161,6 +161,15 @@ struct AliasAnalysis {
/// Return true, if Pointer attribute is set.
bool isPointer() const;
+ /// Return true, if CrayPointer attribute is set.
+ bool isCrayPointer() const;
+
+ /// Return true, if CrayPointee attribute is set.
+ bool isCrayPointee() const;
+
+ /// Return true, if CrayPointer or CrayPointee attribute is set.
+ bool isCrayPointerOrPointee() const;
+
bool isDummyArgument() const;
bool isData() const;
bool isBoxData() const;
diff --git a/flang/include/flang/Optimizer/Dialect/FIRAttr.td b/flang/include/flang/Optimizer/Dialect/FIRAttr.td
index 8a8c60ff0722b..5e3185480a7eb 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRAttr.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRAttr.td
@@ -36,13 +36,17 @@ def FIRvolatile : I32BitEnumAttrCaseBit<"fortran_volatile", 12, "volatile">;
def FIRHostAssoc : I32BitEnumAttrCaseBit<"host_assoc", 13>;
// Used inside parent procedure to flag variables host associated in internal procedure.
def FIRInternalAssoc : I32BitEnumAttrCaseBit<"internal_assoc", 14>;
+// Used by alias analysis
+def FIRcray_pointer : I32BitEnumAttrCaseBit<"cray_pointer", 15>;
+def FIRcray_pointee : I32BitEnumAttrCaseBit<"cray_pointee", 16>;
def fir_FortranVariableFlagsEnum : I32BitEnumAttr<
"FortranVariableFlagsEnum",
"Fortran variable attributes",
[FIRnoAttributes, FIRallocatable, FIRasynchronous, FIRbind_c, FIRcontiguous,
FIRintent_in, FIRintent_inout, FIRintent_out, FIRoptional, FIRparameter,
- FIRpointer, FIRtarget, FIRvalue, FIRvolatile, FIRHostAssoc, FIRInternalAssoc]> {
+ FIRpointer, FIRtarget, FIRvalue, FIRvolatile, FIRHostAssoc, FIRInternalAssoc,
+ FIRcray_pointer, FIRcray_pointee]> {
let separator = ", ";
let cppNamespace = "::fir";
let printBitEnumPrimaryGroups = 1;
diff --git a/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td b/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
index f89be52cf835d..4bdb13fb54708 100644
--- a/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
+++ b/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
@@ -114,6 +114,20 @@ def fir_FortranVariableOpInterface : OpInterface<"FortranVariableOpInterface"> {
fir::FortranVariableFlagsEnum::pointer);
}
+ /// Is this variable a Cray pointer?
+ bool isCrayPointer() {
+ auto attrs = getFortranAttrs();
+ return attrs && bitEnumContainsAny(*attrs,
+ fir::FortranVariableFlagsEnum::cray_pointer);
+ }
+
+ /// Is this variable a Cray pointee?
+ bool isCrayPointee() {
+ auto attrs = getFortranAttrs();
+ return attrs && bitEnumContainsAny(*attrs,
+ fir::FortranVariableFlagsEnum::cray_pointee);
+ }
+
/// Is this variable a Fortran allocatable?
bool isAllocatable() {
auto attrs = getFortranAttrs();
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 53d4d7566acfa..0ededb364bfea 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1806,6 +1806,9 @@ fir::FortranVariableFlagsAttr Fortran::lower::translateSymbolAttributes(
if (sym.test(Fortran::semantics::Symbol::Flag::CrayPointee)) {
// CrayPointee are represented as pointers.
flags = flags | fir::FortranVariableFlagsEnum::pointer;
+ // Still use the CrayPointee flag so that AliasAnalysis can handle these
+ // separately.
+ flags = flags | fir::FortranVariableFlagsEnum::cray_pointee;
return fir::FortranVariableFlagsAttr::get(mlirContext, flags);
}
const auto &attrs = sym.attrs();
@@ -1835,6 +1838,8 @@ fir::FortranVariableFlagsAttr Fortran::lower::translateSymbolAttributes(
flags = flags | fir::FortranVariableFlagsEnum::value;
if (attrs.test(Fortran::semantics::Attr::VOLATILE))
flags = flags | fir::FortranVariableFlagsEnum::fortran_volatile;
+ if (sym.test(Fortran::semantics::Symbol::Flag::CrayPointer))
+ flags = flags | fir::FortranVariableFlagsEnum::cray_pointer;
if (flags == fir::FortranVariableFlagsEnum::None)
return {};
return fir::FortranVariableFlagsAttr::get(mlirContext, flags);
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 0e956d89fd0ee..14138554d163b 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -60,6 +60,10 @@ getAttrsFromVariable(fir::FortranVariableOpInterface var) {
attrs.set(fir::AliasAnalysis::Attribute::Pointer);
if (var.isIntentIn())
attrs.set(fir::AliasAnalysis::Attribute::IntentIn);
+ if (var.isCrayPointer())
+ attrs.set(fir::AliasAnalysis::Attribute::CrayPointer);
+ if (var.isCrayPointee())
+ attrs.set(fir::AliasAnalysis::Attribute::CrayPointee);
return attrs;
}
@@ -138,6 +142,18 @@ bool AliasAnalysis::Source::isPointer() const {
return attributes.test(Attribute::Pointer);
}
+bool AliasAnalysis::Source::isCrayPointee() const {
+ return attributes.test(Attribute::CrayPointee);
+}
+
+bool AliasAnalysis::Source::isCrayPointer() const {
+ return attributes.test(Attribute::CrayPointer);
+}
+
+bool AliasAnalysis::Source::isCrayPointerOrPointee() const {
+ return isCrayPointer() || isCrayPointee();
+}
+
bool AliasAnalysis::Source::isDummyArgument() const {
if (auto v = origin.u.dyn_cast<mlir::Value>()) {
return fir::isDummyArgument(v);
@@ -224,6 +240,12 @@ AliasResult AliasAnalysis::alias(Source lhsSrc, Source rhsSrc, mlir::Value lhs,
return AliasResult::MayAlias;
}
+ // Cray pointers/pointees can alias with anything via LOC.
+ if (lhsSrc.isCrayPointerOrPointee() || rhsSrc.isCrayPointerOrPointee()) {
+ LLVM_DEBUG(llvm::dbgs() << " aliasing because of Cray pointer/pointee\n");
+ return AliasResult::MayAlias;
+ }
+
if (lhsSrc.kind == rhsSrc.kind) {
// If the kinds and origins are the same, then lhs and rhs must alias unless
// either source is approximate. Approximate sources are for parts of the
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-cray-pointers.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-cray-pointers.fir
new file mode 100644
index 0000000000000..ef2263eb45d59
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-cray-pointers.fir
@@ -0,0 +1,43 @@
+// Check that cray pointers might alias with everything.
+
+// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' -mlir-disable-threading 2>&1 | FileCheck %s
+
+// Fortran source:
+// subroutine test()
+// real :: a, b
+// pointer(p, a)
+// p = loc(b)
+// end subroutine
+
+// CHECK-LABEL: Testing : "_QPtest"
+// CHECK-DAG: p#0 <-> b#0: MayAlias
+// CHECK-DAG: p#1 <-> b#0: MayAlias
+// CHECK-DAG: p#0 <-> b#1: MayAlias
+// CHECK-DAG: p#1 <-> b#1: MayAlias
+// CHECK-DAG: p#0 <-> a#0: MayAlias
+// CHECK-DAG: p#1 <-> a#0: MayAlias
+// CHECK-DAG: b#0 <-> a#0: MayAlias
+// CHECK-DAG: b#1 <-> a#0: MayAlias
+// CHECK-DAG: p#0 <-> a#1: MayAlias
+// CHECK-DAG: p#1 <-> a#1: MayAlias
+// CHECK-DAG: b#0 <-> a#1: MayAlias
+// CHECK-DAG: b#1 <-> a#1: MayAlias
+
+func.func @_QPtest() {
+ %0 = fir.alloca !fir.box<!fir.ptr<f32>>
+ %1 = fir.dummy_scope : !fir.dscope
+ %2 = fir.alloca i64 {bindc_name = "p", uniq_name = "_QFtestEp"}
+ %3:2 = hlfir.declare %2 {test.ptr = "p", fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtestEp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+ %4 = fir.alloca f32 {bindc_name = "b", uniq_name = "_QFtestEb"}
+ %5:2 = hlfir.declare %4 {test.ptr = "b", uniq_name = "_QFtestEb"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ %6:2 = hlfir.declare %0 {test.ptr = "a", fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtestEa"} : (!fir.ref<!fir.box<!fir.ptr<f32>>>) -> (!fir.ref<!fir.box<!fir.ptr<f32>>>, !fir.ref<!fir.box<!fir.ptr<f32>>>)
+ %7 = fir.zero_bits !fir.ptr<f32>
+ %8 = fir.embox %7 : (!fir.ptr<f32>) -> !fir.box<!fir.ptr<f32>>
+ fir.store %8 to %6#0 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %9 = fir.embox %5#0 : (!fir.ref<f32>) -> !fir.box<f32>
+ %10 = fir.box_addr %9 : (!fir.box<f32>) -> !fir.ref<f32>
+ %11 = fir.convert %10 : (!fir.ref<f32>) -> i64
+ hlfir.assign %11 to %3#0 : i64, !fir.ref<i64>
+ return
+}
+
diff --git a/flang/test/Lower/HLFIR/cray-pointers.f90 b/flang/test/Lower/HLFIR/cray-pointers.f90
index 082aa1ef8c3f2..9c67b960951df 100644
--- a/flang/test/Lower/HLFIR/cray-pointers.f90
+++ b/flang/test/Lower/HLFIR/cray-pointers.f90
@@ -10,10 +10,10 @@ end subroutine test1
! CHECK-LABEL: func.func @_QPtest1() {
! CHECK: %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.array<?xf32>>>
! CHECK: %[[VAL_1:.*]] = fir.alloca i64 {bindc_name = "cp", uniq_name = "_QFtest1Ecp"}
-! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = "_QFtest1Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest1Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_5:.*]] = arith.constant 10 : index
! CHECK: %[[VAL_7:.*]] = fir.shape %[[VAL_5]] : (index) -> !fir.shape<1>
-! CHECK: %[[VAL_12:.*]]:2 = hlfir.declare %[[VAL_0]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest1Ex"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>)
+! CHECK: %[[VAL_12:.*]]:2 = hlfir.declare %[[VAL_0]] {fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtest1Ex"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>)
! CHECK: %[[VAL_13:.*]] = fir.zero_bits !fir.ptr<!fir.array<?xf32>>
! CHECK: %[[VAL_14:.*]] = fir.embox %[[VAL_13]](%[[VAL_7]]) : (!fir.ptr<!fir.array<?xf32>>, !fir.shape<1>) -> !fir.box<!fir.ptr<!fir.array<?xf32>>>
! CHECK: fir.store %[[VAL_14]] to %[[VAL_12]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
@@ -37,9 +37,9 @@ end subroutine test2
! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<i32> {fir.bindc_name = "n"}) {
! CHECK: %[[VAL_1:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.array<?xf32>>>
! CHECK: %[[VAL_2:.*]] = fir.alloca i64 {bindc_name = "cp", uniq_name = "_QFtest2Ecp"}
-! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_2]] {uniq_name = "_QFtest2Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_2]] {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest2Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_19:.*]] = fir.shape_shift %{{.*}}, %{{.*}} : (index, index) -> !fir.shapeshift<1>
-! CHECK: %[[VAL_24:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest2Ex"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>)
+! CHECK: %[[VAL_24:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtest2Ex"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>)
! CHECK: %[[VAL_25:.*]] = fir.zero_bits !fir.ptr<!fir.array<?xf32>>
! CHECK: %[[VAL_26:.*]] = fir.embox %[[VAL_25]](%[[VAL_19]]) : (!fir.ptr<!fir.array<?xf32>>, !fir.shapeshift<1>) -> !fir.box<!fir.ptr<!fir.array<?xf32>>>
! CHECK: fir.store %[[VAL_26]] to %[[VAL_24]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
@@ -63,11 +63,11 @@ end subroutine test3
! CHECK-SAME: %[[VAL_1:.*]]: !fir.ref<i32> {fir.bindc_name = "n"}) {
! CHECK: %[[VAL_2:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_1]] dummy_scope %{{[0-9]+}} arg {{[0-9]+}} {uniq_name = "_QFtest3En"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %{{[0-9]+}} arg {{[0-9]+}} {uniq_name = "_QFtest3Ecp"} : (!fir.ref<i64>, !fir.dscope) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %{{[0-9]+}} arg {{[0-9]+}} {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest3Ecp"} : (!fir.ref<i64>, !fir.dscope) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_5:.*]] = arith.constant 11 : index
! CHECK: %[[VAL_8:.*]] = arith.constant 11 : index
! CHECK: %[[VAL_24:.*]] = fir.shape_shift %{{.*}}, %{{.*}} : (index, index) -> !fir.shapeshift<1>
-! CHECK: %[[VAL_29:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_8]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest3Ec"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>, index) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>)
+! CHECK: %[[VAL_29:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_8]] {fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtest3Ec"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>, index) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>)
! CHECK: %[[VAL_30:.*]] = fir.zero_bits !fir.ptr<!fir.array<?x!fir.char<1,11>>>
! CHECK: %[[VAL_31:.*]] = fir.embox %[[VAL_30]](%[[VAL_24]]) : (!fir.ptr<!fir.array<?x!fir.char<1,11>>>, !fir.shapeshift<1>) -> !fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>
! CHECK: fir.store %[[VAL_31]] to %[[VAL_29]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,11>>>>>
@@ -90,12 +90,12 @@ end subroutine test4
! CHECK: %[[VAL_1:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.char<1,?>>>
! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %{{[0-9]+}} arg {{[0-9]+}} {uniq_name = "_QFtest4En"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[VAL_3:.*]] = fir.alloca i64 {bindc_name = "cp", uniq_name = "_QFtest4Ecp"}
-! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]] {uniq_name = "_QFtest4Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]] {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest4Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_5:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref<i32>
! CHECK: %[[VAL_6:.*]] = arith.constant 0 : i32
! CHECK: %[[VAL_7:.*]] = arith.cmpi sgt, %[[VAL_5]], %[[VAL_6]] : i32
! CHECK: %[[VAL_8:.*]] = arith.select %[[VAL_7]], %[[VAL_5]], %[[VAL_6]] : i32
-! CHECK: %[[VAL_13:.*]]:2 = hlfir.declare %[[VAL_1]] typeparams %[[VAL_8]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest4Ec"} : (!fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>, i32) -> (!fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>)
+! CHECK: %[[VAL_13:.*]]:2 = hlfir.declare %[[VAL_1]] typeparams %[[VAL_8]] {fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtest4Ec"} : (!fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>, i32) -> (!fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>)
! CHECK: %[[VAL_14:.*]] = fir.zero_bits !fir.ptr<!fir.char<1,?>>
! CHECK: %[[VAL_15:.*]] = fir.embox %[[VAL_14]] typeparams %[[VAL_8]] : (!fir.ptr<!fir.char<1,?>>, i32) -> !fir.box<!fir.ptr<!fir.char<1,?>>>
! CHECK: fir.store %[[VAL_15]] to %[[VAL_13]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.char<1,?>>>>
@@ -122,11 +122,11 @@ end subroutine test5
! CHECK-LABEL: func.func @_QPtest5() {
! CHECK: %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>
! CHECK: %[[VAL_1:.*]] = fir.alloca i64 {bindc_name = "cp", uniq_name = "_QFtest5Ecp"}
-! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = "_QFtest5Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest5Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_5:.*]] = arith.constant 3 : index
! CHECK: %[[VAL_6:.*]] = arith.constant 9 : index
! CHECK: %[[VAL_8:.*]] = fir.shape_shift %[[VAL_5]], %[[VAL_6]] : (index, index) -> !fir.shapeshift<1>
-! CHECK: %[[VAL_13:.*]]:2 = hlfir.declare %[[VAL_0]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest5Ev"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>)
+! CHECK: %[[VAL_13:.*]]:2 = hlfir.declare %[[VAL_0]] {fortran_attrs = #fir.var_attrs<pointer, cray_pointee>, uniq_name = "_QFtest5Ev"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>)
! CHECK: %[[VAL_14:.*]] = fir.zero_bits !fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>
! CHECK: %[[VAL_15:.*]] = fir.embox %[[VAL_14]](%[[VAL_8]]) : (!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>, !fir.shapeshift<1>) -> !fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>
! CHECK: fir.store %[[VAL_15]] to %[[VAL_13]]#0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.type<_QFtest5Tt{r:f32,i:i32}>>>>>
@@ -155,7 +155,7 @@ end subroutine test6
! CHECK: %[[VAL_2:.*]] = fir.alloca !fir.box<!fir.ptr<!fir.array<?x!fir.char<1,?>>>>
! CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %{{[0-9]+}} arg {{[0-9]+}} {uniq_name = "_QFtest6En"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[VAL_4:.*]] = fir.alloca i64 {bindc_name = "cp", uniq_name = "_QFtest6Ecp"}
-! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] {uniq_name = "_QFtest6Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
+! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] {fortran_attrs = #fir.var_attrs<cray_pointer>, uniq_name = "_QFtest6Ecp"} : (!fir.ref<i64>) -> (!fir.ref<i64>, !fir.ref<i64>)
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
! CHECK: %[[VAL_9:.*]] = arith.constant 0 : i32
! CHECK: %[[VAL_10:.*]] = arith.cmpi sgt, %[[VAL_8]], %[[VAL_9]] : i32
@@ -163,13 +163,13 @@ end subroutine test6
! CHECK: %[[VAL_12:.*]] = arith.constant 20 : index
! CHECK: %[[VAL_14:.*]] = fir.shape %[[VAL_12]] : (index) -> !fir.shape<1>
-! CHECK: %[[VAL_20:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_11]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFtest6Ec"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,?>>>>>, i32) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?x!fir.char<1,?>>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<...
[truncated]
|
|
Strictly the pointer itself is an allocation of an integer. That may not need special handling. I'm being conservative here. The important thing here is the pointee: which is modelled as a fir.ptr but is not restricted to only point at TARGETs. |
|
Thank you, Tom. This looks conservatively correct to me, though I am worried if this may cause performance regressions in some apps. Do you think we can enable this only under an options, e.g. This implementation may lose to what gfortran does performance-wise (https://gcc.gnu.org/onlinedocs/gfortran/Cray-pointers.html). They say: |
+1 for that. The flang documentation already says that if one uses Cray Pointer in scenarios where they alias, they should add TARGETs to the cray pointer target for safe behavior. |
+1 for that. The flang documentation already says that if one uses Cray Pointer in scenarios where they alias, they should add TARGETs to the cray pointer target for safe behavior. I do know if we should add more complexity in the alias analysis for a legacy feature not used in the correct way (unless there is a compelling use case in an application that cannot be modified). |
|
This is not from an application. When I landed #169544 the buildbots alerted me to a failing gfortran test: https://github.com/llvm/llvm-test-suite/blob/main/Fortran/gfortran/regression/cray_pointers_10.f90. The problem was that the alias analysis information incorrectly said that the cray pointer could not alias with z and so the if was thought to be always true. Before #169544, all pointers (including the cray pointee) could alias with almost everything. The intention of my patch was to refine that such that fortran standard pointers only alias with TARGETs. This change allows us to keep cray pointees aliasing with everything. I would be surprised if this leads to regressions because for most cases this will produce the same result as current HEAD, although perhaps I might need to be less conservative about the cray pointer (not pointee)? Alternatively we could disable the gfortran test, but I think it would be better to handle cray pointers correctly than produce incorrect code for something that semantics allows without a warning. |
|
Thanks for the details, Tom! It looks like this test contradicts with gfortran's documentation that says that such cases might be optimized in a way that the result would be incorrect. As Jean also pointed, Flang is allowed to optimize it in such a way. Your previous TBAA change made an improvement that potentially enabled more optimizations, and this test started failing. To me it is an expected behavior, and I agree with Jean that maybe we do not need to complicate the implementation because of this case. On ther other hand, it does not look like a huge complication right now. So I would be okay with supporting this case with an option that would be off by default. |
jeanPerier
left a comment
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 would be surprised if this leads to regressions because for most cases this will produce the same result as current HEAD.
That is true at the LLVM level, but not the MLIR level where the alias analysis currently returned no alias between a cray pointer and non pointer/targets (your reverted change only touched the TBAA generation, not the AliasAnalysis itself). Although I agree that given cray pointer usages, the LLVM side probably makes a big difference.
I am also OK with having this change under a flag, it is well done, and it could be useful outside of the gfortran test.
When you add the switch, can you mention it in the AliasAnalysis doc next to the section about cray pointers?
|
Thanks for taking a look. I decided to only make an
I didn't think it was worth complicating a lot of existing core code for this feature that nobody actually asked for. If anyone wants a real driver flag I am happy to add it. |
Flang documentation says that we assume cray pointers do not alias with data not annotated with the TARGET attribute: https://github.com/llvm/llvm-project/blob/7b652195d79bf79dab3d40f962e3b8063a39e20f/flang/docs/Aliasing.md?plain=1#L288 This was discussed in llvm/llvm-project#170900
See discussion on llvm#170900
jeanPerier
left a comment
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.
Thanks!
Flang documentation says that we assume cray pointers do not alias with data not annotated with the TARGET attribute: https://github.com/llvm/llvm-project/blob/7b652195d79bf79dab3d40f962e3b8063a39e20f/flang/docs/Aliasing.md?plain=1#L288 This was discussed in llvm/llvm-project#170900
See discussion on llvm#170900
Depends upon #170900 Re-land #169544 Previously we were less specific for POINTER/TARGET: encoding that they could alias with (almost) anything. In the new system, the "target data" tree is now a sibling of the other trees (e.g. "global data"). POITNTER variables go at the root of the "target data" tree, whereas TARGET variables get their own nodes under that tree. For example, ``` integer, pointer :: ip real, pointer :: rp integer, target :: it integer, target :: it2(:) real, target :: rt integer :: i real :: r ``` - `ip` and `rp` may alias with any variable except `i` and `r`. - `it`, `it2`, and `rt` may alias only with `ip` or `rp`. - `i` and `r` cannot alias with any other variable. Fortran 2023 15.5.2.14 gives restrictions on entities associated with dummy arguments. These do not allow non-target globals to be modified through dummy arguments and therefore I don't think we need to make all globals alias with dummy arguments. I haven't implemented it in this patch, but I wonder whether it is ever possible for `ip` to alias with `rt`. While I was updating the tests I fixed up some tests that still assumed that local alloc tbaa wasn't the default. Cray pointers/pointees are (optionally) modelled as aliasing with all non-descriptor data. This is not enabled by default. I found no functional regressions in the gfortran test suite.
Depends upon llvm/llvm-project#170900 Re-land llvm/llvm-project#169544 Previously we were less specific for POINTER/TARGET: encoding that they could alias with (almost) anything. In the new system, the "target data" tree is now a sibling of the other trees (e.g. "global data"). POITNTER variables go at the root of the "target data" tree, whereas TARGET variables get their own nodes under that tree. For example, ``` integer, pointer :: ip real, pointer :: rp integer, target :: it integer, target :: it2(:) real, target :: rt integer :: i real :: r ``` - `ip` and `rp` may alias with any variable except `i` and `r`. - `it`, `it2`, and `rt` may alias only with `ip` or `rp`. - `i` and `r` cannot alias with any other variable. Fortran 2023 15.5.2.14 gives restrictions on entities associated with dummy arguments. These do not allow non-target globals to be modified through dummy arguments and therefore I don't think we need to make all globals alias with dummy arguments. I haven't implemented it in this patch, but I wonder whether it is ever possible for `ip` to alias with `rt`. While I was updating the tests I fixed up some tests that still assumed that local alloc tbaa wasn't the default. Cray pointers/pointees are (optionally) modelled as aliasing with all non-descriptor data. This is not enabled by default. I found no functional regressions in the gfortran test suite.
The LOC intrinsic allows a cray pointer to alias with ordinary variables with no other attribute. See the new test for an example.
This is not enabled by default. The functionality can be used with
-mmlir -funsafe-cray-pointers.First part of the un-revert of #169544. That will handle TBAA.