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

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 26 00:38:37 PDT 2021


clementval added inline comments.


================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:84
+                                          std::vector<ReadOp> &&loadOps) {
+    for (auto &loadOp : loadOps) {
+      if (domInfo->dominates(storeOp, loadOp))
----------------
mehdi_amini wrote:
> clementval wrote:
> > mehdi_amini wrote:
> > > mehdi_amini wrote:
> > > > It is a bit misleading that the variable name is loadOps but contains instances of `ReadOp` (while there is also a `LoadOp` class...)
> > > I'm also interested in what "the team" would argue here: this code is **very** misleading in the naming and this is mostly because of the use of `auto`.
> > > 
> > In this case `ReadOp` is a template parameter so not sure it would help here. This is what I gathered from the team so if we need to discuss this in more details we can arrange something. 
> OK I totally missed that this was a template parameter in my initial comment here!
> 
> By the way there seems to be a single instantiation of the template right now? It this intended to have more?
> 
I can find only a single instantiation of the template in fir-dev right now. 


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