Skip to content

Commit 7932311

Browse files
committed
Do not assume pointers cannot alias arrays of different types
As added in the comment, the assumption that a pointer of a certain type cannot alias an array of a different type is tricky. First, it is wrong in general currently because equivalence uses pointer aliasing for its aliasing, and the any type can alias any other types. Then, in the normal pointer case, it is at least possible to have complex/real overlap starting with F2008 introduction of %RE/%IM, and in general with derived types and there component types. Although one should note that not all compilers are very carefully here. Complex/Real overlap example: ``` module m contains subroutine foo(p, x) real, target :: x(:) real, pointer :: p(:) p => x end subroutine end module use :: m real, pointer :: real_pointer(:) complex, target :: complex_target(4) = [(1., 1.), (2., 2.), (3., 3.), (4., 4.)] call foo(real_pointer, complex_target%re) real_pointer = abs(complex_target(4:1:-1)) print *, real_pointer end ``` - ifort, gfortran (need >= 9.3 to support %re), and Nagfor all compile and assume potential overlap printing `5.6568542 4.2426405 2.8284271 1.4142135`. - nvfortran hits an ICE (probably caused by passing the complex_target%re). - xlf ignore the overlap and prints `5.656854153 4.242640495 4.690415382 5.744562626`. Derived/Part overlap example: ``` type t integer :: i end type integer, pointer :: integer_pointer(:) type(t), target :: derived_target(4) = [t(1), t(2), t(3), t(4)] integer_pointer => derived_target%i derived_target%i = integer_pointer(4:1:-1) print *, integer_pointer end ``` - Nvfortran and Nagfor assumes it can overlap and print `4 3 2 1` - gfortran, xlf and ifort assume no overlap and prints `4 3 3 4` Hence, once the equivalence aliasing is handled another way, we may want to dig more into this and find the correct requirement here. For now, assumes we need to be as strict as the stricter compilers out there.
1 parent f08fa76 commit 7932311

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,23 +434,26 @@ static bool conflictOnLoad(llvm::ArrayRef<mlir::Operation *> reach,
434434
mlir::Value load;
435435
mlir::Value addr = st.memref();
436436
const bool storeHasPointerType = hasPointerType(addr.getType());
437-
mlir::Type stEleTy =
438-
fir::unwrapSequenceType(fir::unwrapPassByRefType(addr.getType()));
439437
for (auto *op : reach)
440438
if (auto ld = mlir::dyn_cast<ArrayLoadOp>(op)) {
441439
mlir::Type ldTy = ld.memref().getType();
442-
mlir::Type ldEleTy =
443-
fir::unwrapSequenceType(fir::unwrapPassByRefType(ldTy));
444440
if (ld.memref() == addr) {
445441
if (ld.getResult() != st.original())
446442
return true;
447443
if (load)
448444
// TODO: only return if the loads may overlap (look at slices if any).
449445
return true;
450446
load = ld;
451-
} else if ((hasPointerType(ldTy) || storeHasPointerType) &&
452-
stEleTy == ldEleTy) {
447+
} else if ((hasPointerType(ldTy) || storeHasPointerType)) {
453448
// TODO: Use target attribute to restrict this case further.
449+
// TODO: Check if types can also allow ruling out some cases. For now,
450+
// the fact that equivalences is using pointer attribute to enforce
451+
// aliasing is preventing any attempt to do so, and in general, it may
452+
// be wrong to use this if any of the types is a complex or a derived
453+
// for which it is possible to create a pointer to a part with a
454+
// different type than the whole, although this deserve some more
455+
// investigation because existing compiler behavior seem to diverge
456+
// here.
454457
return true;
455458
}
456459
}

flang/test/Fir/array-copies-pointers.f90

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,55 @@
8787
fir.array_merge_store %3, %4 to %arg1 : !fir.array<100xf32>, !fir.array<100xf32>, !fir.ptr<!fir.array<100xf32>>
8888
return
8989
}
90+
91+
// Test derived_target(:)%i = integer_pointer(:)
92+
// The integer pointer may be aliasing the derived target component.
93+
// CHECK-LABEL: func @derived_whose_component_may_be_aliased
94+
// CHECK: fir.allocmem !fir.array<4x!fir.type<some_type{i:i32}>>
95+
func @derived_whose_component_may_be_aliased(%arg0: !fir.box<!fir.array<4x!fir.type<some_type{i:i32}>>> {fir.target}, %arg1: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) {
96+
%c4 = arith.constant 4 : index
97+
%0 = fir.field_index i, !fir.type<some_type{i:i32}>
98+
%c1 = arith.constant 1 : index
99+
%1 = fir.slice %c1, %c4, %c1 path %0 : (index, index, index, !fir.field) -> !fir.slice<1>
100+
%2 = fir.array_load %arg0 [%1] : (!fir.box<!fir.array<4x!fir.type<some_type{i:i32}>>>, !fir.slice<1>) -> !fir.array<4xi32>
101+
%3 = fir.load %arg1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>
102+
%c0 = arith.constant 0 : index
103+
%4:3 = fir.box_dims %3, %c0 : (!fir.box<!fir.ptr<!fir.array<?xi32>>>, index) -> (index, index, index)
104+
%5 = fir.shift %4#0 : (index) -> !fir.shift<1>
105+
%6 = fir.array_load %3(%5) : (!fir.box<!fir.ptr<!fir.array<?xi32>>>, !fir.shift<1>) -> !fir.array<?xi32>
106+
%7 = arith.subi %c4, %c1 : index
107+
%8 = fir.do_loop %arg2 = %c0 to %7 step %c1 unordered iter_args(%arg3 = %2) -> (!fir.array<4xi32>) {
108+
%9 = fir.array_fetch %6, %arg2 : (!fir.array<?xi32>, index) -> i32
109+
%10 = fir.array_update %arg3, %9, %arg2 : (!fir.array<4xi32>, i32, index) -> !fir.array<4xi32>
110+
fir.result %10 : !fir.array<4xi32>
111+
}
112+
fir.array_merge_store %2, %8 to %arg0[%1] : !fir.array<4xi32>, !fir.array<4xi32>, !fir.box<!fir.array<4x!fir.type<some_type{i:i32}>>>, !fir.slice<1>
113+
return
114+
}
115+
116+
// Test real_target = complex_target(:)%re
117+
// The real pointer may be aliasing the complex real part.
118+
// CHECK-LABEL: func @complex_real_aliasing
119+
// CHECK: fir.allocmem !fir.array<?xf32>
120+
func @complex_real_aliasing(%arg0: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, %arg1: !fir.ref<!fir.array<4x!fir.complex<4>>> {fir.target}) {
121+
%c4 = arith.constant 4 : index
122+
%0 = fir.load %arg0 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
123+
%c0 = arith.constant 0 : index
124+
%1:3 = fir.box_dims %0, %c0 : (!fir.box<!fir.ptr<!fir.array<?xf32>>>, index) -> (index, index, index)
125+
%2 = fir.shift %1#0 : (index) -> !fir.shift<1>
126+
%3 = fir.array_load %0(%2) : (!fir.box<!fir.ptr<!fir.array<?xf32>>>, !fir.shift<1>) -> !fir.array<?xf32>
127+
%c0_i32 = arith.constant 0 : i32
128+
%4 = fir.shape %c4 : (index) -> !fir.shape<1>
129+
%c1 = arith.constant 1 : index
130+
%5 = fir.slice %c1, %c4, %c1 path %c0_i32 : (index, index, index, i32) -> !fir.slice<1>
131+
%6 = fir.array_load %arg1(%4) [%5] : (!fir.ref<!fir.array<4x!fir.complex<4>>>, !fir.shape<1>, !fir.slice<1>) -> !fir.array<4xf32>
132+
%7 = arith.subi %c4, %c1 : index
133+
%8 = fir.do_loop %arg2 = %c0 to %7 step %c1 unordered iter_args(%arg3 = %3) -> (!fir.array<?xf32>) {
134+
%9 = fir.array_fetch %6, %arg2 : (!fir.array<4xf32>, index) -> f32
135+
%10 = fir.array_update %arg3, %9, %arg2 : (!fir.array<?xf32>, f32, index) -> !fir.array<?xf32>
136+
fir.result %10 : !fir.array<?xf32>
137+
}
138+
fir.array_merge_store %3, %8 to %0 : !fir.array<?xf32>, !fir.array<?xf32>, !fir.box<!fir.ptr<!fir.array<?xf32>>>
139+
return
140+
}
141+

0 commit comments

Comments
 (0)