Skip to content

Conversation

@LU-JOHN
Copy link
Contributor

@LU-JOHN LU-JOHN commented Mar 25, 2025

KnownBits passed to computeKnownBitsFromRangeMetadata must have the same bit width as the range metadata bit width. Otherwise the calculated results will be incorrect.

@LU-JOHN LU-JOHN requested a review from nikic as a code owner March 25, 2025 20:02
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Mar 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2025

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-llvm-analysis

Author: None (LU-JOHN)

Changes

KnownBits passed to computeKnownBitsFromRangeMetadata must have the same bit width as the range metadata bit width. Otherwise the calculated results will be incorrect.


Full diff: https://github.com/llvm/llvm-project/pull/132985.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+2)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 880781742fae0..4e3d5d8f12cbc 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -433,6 +433,8 @@ void llvm::computeKnownBitsFromRangeMetadata(const MDNode &Ranges,
     // The first CommonPrefixBits of all values in Range are equal.
     unsigned CommonPrefixBits =
         (Range.getUnsignedMax() ^ Range.getUnsignedMin()).countl_zero();
+    // BitWidth must equal the Ranges BitWidth for the correct number of high bits to be set.
+    assert(BitWidth == Lower->getBitWidth() );
     APInt Mask = APInt::getHighBitsSet(BitWidth, CommonPrefixBits);
     APInt UnsignedMax = Range.getUnsignedMax().zextOrTrunc(BitWidth);
     Known.One &= UnsignedMax & Mask;

@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Mar 27, 2025
Signed-off-by: John Lu <[email protected]>
Comment on lines +9179 to +9183
assert((!MMO->getRanges() ||
(mdconst::extract<ConstantInt>(MMO->getRanges()->getOperand(0))
->getBitWidth() == MemVT.getScalarSizeInBits() &&
MemVT.isInteger())) &&
"Range metadata and load type must match!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need something like this as it's the closest thing we have to a DAG verifier

@LU-JOHN LU-JOHN requested a review from arsenm March 28, 2025 11:55
@arsenm arsenm merged commit 6a46c6c into llvm:main Apr 3, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants