[PATCH] D140851: [Patch 3/4]: Add cases for assume (X & Y != {0|Y})
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 4 06:14:27 PST 2023
nikic added inline comments.
================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:701
+ if (!isValidAssumeForContext(I, Q.CxtI, Q.DT))
+ continue;
+
----------------
I believe the placement of this call was intentional, because it is more expansive than the icmp operand checks. That said, I don't see a compile-time impact on CTMark, though that is not exactly an assume-heavy workload.
We did address various inefficiencies in isValidAssumeForContext() over time though, so I would be fine with trying this change. It should be split out into a separate NFC patch though.
================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:939
+ // 1. is_pow2(b) && b == a
+ // v.zeros[log2(b)] = 1
+ // 2. is_pow2(b) && 0 == a
----------------
It looks like we are missing this canonicalization: https://alive2.llvm.org/ce/z/MtveLU With that done, this becomes `v & b == 0` and is covered by existing handling.
================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:943
+ KnownBits BKnown =
+ computeKnownBits(B, Depth + 1, QueryNoAC).anyextOrTrunc(BitWidth);
+ if (isKnownToBeAPowerOfTwo(B, false, Depth + 1, QueryNoAC)) {
----------------
Move this computeKnownBits() call into the isKnownToBeAPowerOfTwo() branch.
================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:950
+ KnownBits AKnown = computeKnownBits(A, Depth + 1, QueryNoAC)
+ .anyextOrTrunc(BitWidth);
+ if (AKnown.isZero()) {
----------------
This should just check whether A is zero, no need to compute known bits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140851/new/
https://reviews.llvm.org/D140851
More information about the llvm-commits
mailing list