[PATCH] D45378: [InstCombine] Propagate null values from conditions to other basic blocks

Dávid Bolvanský via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 11 10:25:17 PDT 2018


xbolva00 abandoned this revision.
xbolva00 added a comment.

In https://reviews.llvm.org/D45378#1064397, @spatel wrote:

> In https://reviews.llvm.org/D45378#1060270, @xbolva00 wrote:
>
> > In https://reviews.llvm.org/D45378#1060239, @spatel wrote:
> >
> > > This is pushing instcombine beyond local simplifications. Propagating values across blocks should probably be handled in CVP (-correlated-propagation).
> >
> >
> > Maybe, but here it also fits, every info required for this transformation is available here.
>
>
> I think it's been shown that every other pass could be subsumed by instcombine. That doesn't mean we should do that.
>
> > Implementation of this optimization would probably be quite massive in the CorrelatedValuePropagation.
>
> I was curious why that might be, so I wrote a patch. It's about the same amount of code as this patch, but more general. Please see if https://reviews.llvm.org/D45448 / https://reviews.llvm.org/rL329755 solved your motivating examples. If yes, I think you can abandon this patch.


Thanks, great :) I will close this patch.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2405
+      for (auto &I : *B) {
+        for (unsigned i = 0; i < I.getNumOperands(); ++i) {
+          auto Arg = I.getOperand(i);
----------------
lebedev.ri wrote:
> I'd try to use range-based loop, if there is a function to go from the pointer to an index, or `setOperand()` would work with index.
I think the current code is ok.


https://reviews.llvm.org/D45378





More information about the llvm-commits mailing list