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

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 22 00:36:28 PDT 2021


clementval added inline comments.


================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:29
+  std::vector<OpT> ops;
+  for (auto *user : v.getUsers())
+    if (auto op = dyn_cast<OpT>(user))
----------------
clementval wrote:
> mehdi_amini wrote:
> > clementval wrote:
> > > mehdi_amini wrote:
> > > > mehdi_amini wrote:
> > > > > 
> > > > This is marked as done, but it isn't (it isn't the only one I think)
> > > I have discussed this with others and they would like to keep it like that. The guidlines are kind of blurry on this... If this is a blocker we will ave to discuss this further. 
> > Let's discuss then.
> > (and please don't ignore comments or mark them as done when they aren't!)
> Sorry for that. We have discussed in this patch or in maybe another one and in the last call that during the upstreaming work any "blocker" comment should be put as "Change Requested" so that we can identified where to put our effort. 
> In this particular case `auto` can surely be removed but not everywhere like you mentioned in your comment. I think that's more the everywhere that others were against. 
> 
> > `auto` does not help readability so why not spell out the type? (here and everywhere, in all the upstreaming patches please).
On this particular comment, I remember what happened. I did the change and marked it as done then after discussion I revrted the change but I forgot to mention that. Sorry for that, I didn't mean to discard your comment. 


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