[PATCH] D133788: [InstCombine] try multi-use demanded bits folds for 'add'

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 04:59:02 PDT 2022


spatel added a comment.

In D133788#3787430 <https://reviews.llvm.org/D133788#3787430>, @nikic wrote:

>> The 2nd part avoids computing known bits for every other multi-use add (giving the small improvement in compile-time). That also results in the test diffs that show large unsigned constants becoming small negative numbers. That should make analysis easier and codegen better in most cases.
>
> I think it might make sense to separate these two changes if we can do so reasonably cheaply. Rather then calling computeKnownBits on the add, which will unnecessarily repeat the two recursive calls, can we call KnownBits::computeForAddSub() on the computed results? I expect that would be about compile-time neutral, while retaining existing behavior.

Yes - good suggestion and your guess about timing looks correct. I drafted the changes as independent patches and pushed them up to llvm-compile-time-tracker as perf/add-demand2:
https://llvm-compile-time-tracker.com/?config=NewPM-O3&stat=instructions&remote=rotateright

The timing diffs are nearly invisible in this set, so we can mostly judge the patches on the test diffs, and if it saves a bit of time too, that's a nice bonus. The sibling change in the codegen patch shows that the overall results may be unpredictable, but we can probably deal with any fallout with backend fixes.


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

https://reviews.llvm.org/D133788



More information about the llvm-commits mailing list