[PATCH] D111288: [fir] Add data flow optimization pass
Valentin Clement via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 8 01:32:00 PDT 2021
clementval added a comment.
In D111288#3050087 <https://reviews.llvm.org/D111288#3050087>, @jdoerfert wrote:
> Drive by: Just glancing at this I am very doubtful this is correct or well tested:
>
> - All non-load/store users are ignored, which is already a bad thing (I just so the FIXME which is easily lost and not mentioned in the rather sparse commit message).
> - The reasoning about "nearest stores" ignores all non dominating stores, which should also not be correct.
> - The reasoning about "unneeded stores" ignores non-dominated loads, which should also not be correct.
> - Given that the test have no comments I'm not sure if they are positive or negative tests. I would hope both are present and the situations described above are in there.
As for probably 80% of the fir-dev upstreaming patch, I'm not the primary author of this code so maybe @schweitz can comment on that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111288/new/
https://reviews.llvm.org/D111288
More information about the llvm-commits
mailing list