Map the new load to the base pointer of the invariant load hoisted load to be able to find the alias information for it.
Details
Diff Detail
Event Timeline
Hi Roman,
as already discussed on the phone call, we certainly have a problem with the alias set annotator where no no-alias metadata is emitted for hoisted base pointers. However, it seems we do not yet fully understand the issue and the commit message and the patch itself are hard to verify. The very first step we should take is to write a minimal test case. I suggest the following one:
define void @metadata(float** %A, float** %B) { entry: %ptrA = load float*, float** %A %ptrB = load float*, float** %B br label %for for: %indvar = phi i64 [0, %entry], [%indvar.next, %for] %indvar.next = add i64 %indvar, 1 store float 42.0, float* %ptrA store float 42.0, float* %ptrB %icmp = icmp sle i64 %indvar, 1024 br i1 %icmp, label %for, label %exit exit: ret void } define void @nometadata(float** %A, float** %B) { entry: br label %for for: %indvar = phi i64 [0, %entry], [%indvar.next, %for] %indvar.next = add i64 %indvar, 1 %ptrA = load float*, float** %A %ptrB = load float*, float** %B store float 42.0, float* %ptrA store float 42.0, float* %ptrB %icmp = icmp sle i64 %indvar, 1024 br i1 %icmp, label %for, label %exit exit: ret void }
This test case took only 10 minute to write for me. Generally it makes reviewing a lot easier if you try to get to such a minimal test case. If this does not work and you don't feel you make progress, that's OK. In this case I am happy to help. (For this patch I had the feeling you had doubts regarding how to write such a test case, which is why I went in to help you).
Looking at this test case, I believe the change itself is almost what is needed. However, plainly copying the metadata is wrong. The base pointers used in the scop should refer to "poly.alias.scope.MemRef_ptrA" and "poly.alias.scope.MemRef_ptrB", but currently refer to "poly.alias.scope.MemRef_A" and "poly.alias.scope.MemRef_B". The right approach is probably to look for the basepointer of the invariant load hoisted load and see if for this base pointer alias information is available.
Hi Tobias,
This test case took only 10 minute to write for me. Generally it makes reviewing a lot easier if you try to get to such a minimal test case. If this does not work and you don't feel you make progress, that's OK. In this case I am happy to help. (For this patch I had the feeling you had doubts regarding how to write such a test case, which is why I went in to help you).
Thanks! I think that we can also remove the array B.
Looking at this test case, I believe the change itself is almost what is needed. However, plainly copying the metadata is wrong. The base pointers used in the scop should refer to "poly.alias.scope.MemRef_ptrA" and "poly.alias.scope.MemRef_ptrB", but currently refer to "poly.alias.scope.MemRef_A" and "poly.alias.scope.MemRef_B". The right approach is probably to look for the basepointer of the invariant load hoisted load and see if for this base pointer alias information is available.
Right. Should we map the new load to the basepointer of the invariant load hoisted load to be able to do it? (I've tried to implement it in the new version of the patch).
Perfect. This looks a lot better! Thanks for the cleanup!
test/Isl/CodeGen/invariant_load_alias_metadata.ll | ||
---|---|---|
8 | Can you include the arguments of !alias.scope and !noalias as well as the metadata referenced to the check lines? This metadata I mean: !0 = distinct !{!0, !1, !"polly.alias.scope.MemRef_A"} |
Since, according to "assert(MA->isArrayKind() && MA->isRead())", MA models an array and it is a read memory access, AccInst, the access instruction of MA, is a base pointer and PreloadVal should be always mapped to it.
Hi Roman,
this indeed looks a lot better. Can you please include the arguments of !alias.scope and !noalias as well as the metadata referenced in the check lines, and then commit?
Best,
Tobias
Can you include the arguments of !alias.scope and !noalias as well as the metadata referenced to the check lines?
This metadata I mean:
!0 = distinct !{!0, !1, !"polly.alias.scope.MemRef_A"}
!1 = distinct !{!1, !"polly.alias.scope.domain"}
!2 = !{!3}
!3 = distinct !{!3, !1, !"polly.alias.scope.MemRef_ptrA"}