[PATCH] D75804: [InstCombine] Fix known bits handling in SimplifyDemandedUseBits

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, lebedev.ri, xbolva00.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
nikic marked 4 inline comments as done.
nikic added a comment.

Some comments on the intrinsics code, as it did some weird things before...



================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:719
-
-        // TODO: Could compute known zero/one bits based on the input.
         break;
----------------
Despite what the comment says, this was already computing known bits, because the "break" falls through to the call below.


================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:752
+        KnownBitsComputed = true;
         break;
       }
----------------
While known bits were computed explicitly here, this break would still fall through to the known bits call below, and thus recompute the result from scratch.


================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:781
+        KnownBitsComputed = true;
+        break;
       }
----------------
This `return nullptr` skipped the bit of code for returning a constant integer below.


================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:790
+
+    if (!KnownBitsComputed)
+      computeKnownBits(V, Known, Depth, CxtI);
----------------
To avoid the three issues mentioned above, I'm now explicitly tracking whether known bits were computed or not for the call.


Fixes a regression from D75801 <https://reviews.llvm.org/D75801>. SimplifyDemandedUseBits() is also supposed to compute the known bits (of the demanded subset) of the instruction. For unknown instructions it does so by directly calling computeKnownBits(). For known instructions it will compute known bits itself. However, for instructions where only some cases are handled directly (e.g. a constant shift amount) the known bits invocation for the unhandled case is sometimes missing. This patch adds the missing calls and thus removes the main discrepancy with ExpensiveCombines mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75804

Files:
  lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
  test/Transforms/InstCombine/all-bits-shift.ll
  test/Transforms/InstCombine/known-bits.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75804.248922.patch
Type: text/x-patch
Size: 7569 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200307/a49db553/attachment.bin>