[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