isKnownReachable had only interface (always returns true).
Changed it to call isPotentiallyReachable.
This change enables deductions of other Abstract Attributes depending on AAReachability to use reachability information obtained from CFG, and it can make them stronger.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 424 | I love how this all comes together ;) | |
Comment Actions
I'm sorry for my mistake. I uploaded two disjoint patches separately.
I added all of the changes.
Comment Actions
No worries. I still have the feeling something is missing in this diff. Make sure to squash the commits locally or diff against the proper baseline.
Comment Actions
void use(int *);
__attribute__((noinline)) static void f1(int * restrict p, int c1, int c2) {
if (c1) {
use(p);
}
if (!c2) {
use(p);
}
}
void f2(int * restrict p, int c) { f1(p, c, c); }Could you add this example to test?
In this case, c1 and c2 are always same. Because of flow sensitivity, the first callsite is not reachable to the second callsite. So %p is noalias at both callsites. It can't be deduced with isPotentiallyReachable so please add FIXME tag.
Comment Actions
I added a suggested test.
I squashed all commits to one commit and got diff by git diff -U999999 HEAD^ (including commit for a new test)
Comment Actions
I squashed 2 commits and got diff. But that makes no difference from the previous diff. Therefore the latest diff may be still inappropriate. If so, please notify me.
Comment Actions
This makes sense to me, thank you for your suggestion. I added a slightly different test.
If function use does not make any alias of p (such as only_store in test), noalias can be deduced with the current implementation. But if we impose any constraints on use, noalias should not be deduced.
Therefore, I add a test corresponding to the following C program.
__attribute__((noinline)) static void test16_sub(int * restrict p, int c1, int c2) {
if (c1) {
only_store(p);
make_alias(p);
}
if (!c2) {
only_store(p);
}
}
void test16_caller(int * restrict p, int c) {
test16_sub(p, c, c);
}Like as your test, %p is noalias at both calllsites of only_store. But it is currently not deduced for the second callsite.
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 410 | Please add CHECK-LABEL to function definitions just for testing purposes. | |
Comment Actions
There are two issues with a isPotentiallyReachable approach, not to say we cannot make it work. What we need is 1) use liveness information from AAIsDead 2) caching of the results. We can do that in follow up patches though.
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 410 | Do I have to add CHECK-LABEL to all function definitions? | |
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 410 | No, only to functions with tests. | |
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 460 | @okura @jdoerfert This diff added some checks with missing FileCheck colons, which silently hide the fact that these checks don't match the tool output. Mind taking a look at this fixup patch? https://gist.github.com/jroelofs/f03ae725c6b9a5c80f9f187b80642f79 | |
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 460 | That is my terrible mistake. I am sorry and thank you for the patch. | |
| llvm/test/Transforms/Attributor/noalias.ll | ||
|---|---|---|
| 460 | no worries, thanks for having a look! | |
Please add CHECK-LABEL to function definitions just for testing purposes.