[PATCH] D111288: [fir] Add data flow optimization pass

Eric Schweitz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 8 11:32:55 PDT 2021


schweitz added a comment.

In D111288#3050436 <https://reviews.llvm.org/D111288#3050436>, @clementval wrote:

> 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.

I'm not the primary author of this code either.

The background here is that this is some prototype experimental work done to gauge the feasibility of using the affine dialect from Fortran. There are problems here as well as some bit rot. (The experiment yielded some promising results and discovered some interesting issues, as you might expect.)

@clementval  is upstreaming it effectively in an "as-is" state, sharing it with a broader audience, and as a possible starting point for future work. Pros: It puts the prototype out there for anyone interested. Cons: It's just an early prototype. That's just one alternative.

Another alternative is to not bother upstreaming it, since it may be "too much of a prototype" (It's not always super clear how to distinguish "overly prototype-y" from "prototype-y but possibly useful in a new project.), and let someone figure out what to do in the future. Pros: It's not there and no one will mistakenly assume its robust, productized code by mistake. Cons: It's not there, so the reference point, lessons learned, etc. are likely to just be lost over time.


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