I am trying to learn the rationale for the pointer component logic used in Flang’s AliasAnalysis implementation. It appears that others find that logic confusing as well: the recent PR #91020, despite a thorough review, introduced a test that appears to understand this logic differently than an existing test. The goal of this RFC is to clear up this confusion and decide whether to change the implementation or just improve the documentation and tests.
@szakharin @jeanPerier @Renaud-K @tblah
Conflicting Tests
Introduced more than a year ago, test alias-analysis-3.fir line 106 checks that “Two dummy arguments of composite type with a pointer component may alias each other”. Here is a reduced version, where it expects MayAlias for %arg0 vs. %arg1:
// module m
// type t
// real, pointer :: pointer_component
// end type t
// contains
// subroutine test(a, b)
// type(t) :: a, b
// end subroutine test
// end module m
func.func @_QMmPtest(%arg0: !fir.ref<!fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>> {fir.bindc_name = "a"}, %arg1: !fir.ref<!fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>> {fir.bindc_name = "b"}) {
// ...
}
Introduced by the recent PR #91020, test alias-analysis-9.fir line 21 has nearly the same check as above but with a todo saying, “x and y are non pointer, non target argument and therefore do not alias.” Here is a reduced version, where the todo expects NoAlias for x vs. y, which are labels for hlfir.declare ops on %arg0 and %arg1:
// module m
// type t
// type(t), pointer :: next
// end type
// contains
// subroutine foo(x, y)
// type(t) :: x, y
// end subroutine
// end module
func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {
// ...
%4:2 = hlfir.declare %arg0 {uniq_name = "_QMmFfooEx", test.ptr = "x"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
%5:2 = hlfir.declare %arg1 {uniq_name = "_QMmFfooEy", test.ptr = "y"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
// ...
}
Of course, AliasAnalysis sometimes returns MayAlias for cases where it is not powerful enough yet to return a better result. Is alias-analysis-3.fir just missing associated todo comments? Are alias-analysis-9.fir’s comments incorrect? Or is there some difference between these cases that should produce different results, once AliasAnalysis is powerful enough?
Alternative Logic
In alias-analysis-9.fir, I think I understand why MayAlias makes sense for x vs. xnext1, which is the address of the pointer x%next (not the address in the pointer). To me, that seems like a special case of how the address of a composite may alias the address of any of its components. My understanding of the rationale is that a store to either one can affect the result of a load from the other. But I do not understand why there might be aliasing between the addresses of different composites (e.g., x and y) just because they have pointer components.
To investigate those points further, I wrote a small patch [edit: that’s now part of this draft PR] that rewrites the pointer component logic accordingly. First, it removes the old pointer component logic to avoid cases (e.g., MayAlias for x vs. y) that don’t seem reasonable to me. Second, it assumes the address of a pointer component should be treated just like the address of any other component. Logic for other components is already implemented without this patch, but it misses pointer components because the current representation treats pointers as not “data”, so this patch adds similar handling for them.
This patch adjusts both of the aforementioned tests to have what I think are more reasonable expected results. That includes resolving some fixme/todo comments in those tests. Other tests do not require changes, and check-flang still behaves.
Does that patch mishandle any use cases?