[PATCH] D78397: [mlir][Transforms] Add pass to perform sparse conditional constant propagation

[River Riddle via Phabricator via llvm-commits]

rriddle marked 2 inline comments as done.
rriddle added inline comments.


================
Comment at: mlir/lib/Transforms/SCCP.cpp:313
+
+    // Don't try to fold the results as we can't guarantee folds won't be
+    // in-place.
----------------
bondhugula wrote:
> rriddle wrote:
> > bondhugula wrote:
> > > You can with the last output parameter on tryToFold - is this what you were looking for?
> > > 
> > > ```
> > > bool inPlaceUpdate;
> > > FoldUtils::tryToFold(op, nullptr, nullptr, &inPlaceUpdate);
> > > // operation is folded if inPlaceUpdate is false
> > > ```
> > This is a bit of a tricky situation. We don't actually want to fold here, this is essentially just simulated execution with constant parameters. The constants we have here aren't guaranteed to be those at runtime, so it's better to avoid the extra overhead of OperationFolder as we don't want to generated anything at this point.
> Right - while the algorithm is still running, a current 'constant' lattice value could be later lowered to the bottom symbol ('overdefined'). But once the algorithm has converged, all 'Unknown' lattice values will become either 'constant' or 'overdefined' at which point we could fold the relevant ops. Do you want to collect such ops? You'll still ideally need a new pattern rewriting driver that takes in a list of ops to process as the starting point. In any case, the comment above on "Don't try to fold the results ... won't be in-place" needs to be fixed - it's misleading because tryToFold does in-place updates as well. Instead, change to "can't guarantee that ... will be out of place"?
Yeah, this is what I mentioned in the comment in the test file. I'm not sure all of the types of foldings we want to do within SCCP itself at this point. Right now we replace any results that were found to be constant and erase any operations that we can, but that doesn't cover all of the potential simplifications we could do. Right now my main goal is to get the constant propagation working across regions and inter-procedurally. After that I think we can tune the amount of additional simplifications we do here based on how it fits into user pipelines. Also, thank you for the comment suggestion!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78397/new/

https://reviews.llvm.org/D78397