[PATCH] D82072: [InstCombine] Combine select & Phi by same condition

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 22 23:04:41 PDT 2020


mkazantsev marked an inline comment as done.
mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2555
   // FIXME: Remove this workaround when freeze related patches are done.
   // For select with undef operand which feeds into an equality comparison,
   // don't simplify it so loop unswitch can know the equality comparison
----------------
aqjune wrote:
> spatel wrote:
> > xbolva00 wrote:
> > > @spatel can you look at this fixme?
> > That was added with D35811.
> > 
> > I don't know exact status, but I think there are still several steps to go before we can start adding freeze instructions to IR early in the optimization pipeline and not cause perf regressions.
> > 
> > cc @aqjune @nlopes
> I'll be able to work on addressing performance regressions happening from freeze insertions from the middle of July.
> There are two things that I have in my mind: (1) we need a function attribute saying that the argument has a frozen value (I also suggested it at https://reviews.llvm.org/D81678 ), (2) we need to update InstCombine patterns to work well on frozen arguments too.
> For the former one, I'm seeing how the patch goes; if it it goes into a different direction, I'll suggest a patch by myself. For the latter one, my plan is to enable insertion of freeze from the later-most instcombine/simplifycfg pass first, and see which optimization needs to be updated.
Hi, just to make sure: this discussion has nothing to do with the patch, right? From this thread, I could not figure out if some action is required from me regarding this patch or not.


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

https://reviews.llvm.org/D82072





More information about the llvm-commits mailing list