[PATCH] D78582: [InstCombine] substitute equivalent constant to reduce logic-of-icmps

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D78582#2002223 <https://reviews.llvm.org/D78582#2002223>, @lebedev.ri wrote:

> In D78582#1999956 <https://reviews.llvm.org/D78582#1999956>, @nikic wrote:
>
> > In D78582#1999708 <https://reviews.llvm.org/D78582#1999708>, @spatel wrote:
> >
> > > In D78582#1999502 <https://reviews.llvm.org/D78582#1999502>, @nikic wrote:
> > >
> > > > It looks like this caused a +0.40% text size increase <http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=c79227cabb3059b4f01c07b6f8bfc7986a71d156&stat=size-text> on kimwitu++. There's a corresponding > 10% compile-time increase <http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=c79227cabb3059b4f01c07b6f8bfc7986a71d156&stat=instructions&details=on> on the main.cc file, so that's likely the relevant one.
> > > >
> > > > If you have time, it would be good to double check what's going on there, as text size increase usually means we're losing optimizations.
> > >
> > >
> > > I'm going to expose my lack of git knowledge here, but how do we know this commit is to blame for the difference?
> >
> >
> > Better link: http://llvm-compile-time-tracker.com/compare.php?from=7003a1da37b2aae5b17460922efde9efde2c229d&to=62da6ecea298739ad59c0563ce6d9493804ef1f0&stat=size-text This one only contains this change in the diff range. (The previous one had an additional MLIR change, which can't make a difference.)
> >
> > > How do I determine the build flags that are needed to repro?
> >
> > The flags are determined by test-suite `cmake/caches/O3.cmake` configuration. I get the flags by running `ninja kc -v` and sticking `-emit-llvm` on the right one, which would be for me:
> >
> > > /home/nikic/llvm-project/build/bin/clang++  -DNDEBUG  -O3   -w -Werror=date-time -I/home/nikic/llvm-test-suite/MultiSource/Applications/kimwitu++ -DYYDEBUG=1 -MD -MT MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/util.cc.o -MF MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/main.cc.o.d -o MultiSource/Applications/kimwitu++/CMakeFiles/kc.dir/main.cc.o -c ../MultiSource/Applications/kimwitu++/main.cc -emit-llvm
> >
> >
> >
> > > I tried reverting this patch locally and compiling /testsuite/trunk/MultiSource/Applications/kimwitu++/main.cc, and the IR is identical to pre-revert compiled at -O2 or -O3.
> >
> > Hm, are you sure you reverted the right one? I also didn't see a difference, until I realized I reverted the InstSimplify change instead of the InstCombine change, duh. After fixing that, I get these two files and diff: https://gist.github.com/nikic/ecbe2482367be25b17ab982c2346b661
>
>
> I'm looking into this, and i believe those are improvements, not regressions.


Thanks! I double-checked my setup, and I'm still not able to repro. I wonder if platform makes a difference (I'm testing on macOS)?
Just staring at some of those math/logic+icmp combos made me think we're missing some underlying icmp transforms, so I'm working on a patch for at least 1 of those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78582