Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ValueTracking] Support non-constant idx for computeKnownBits of insertelement #87707

Closed

Conversation

goldsteinn
Copy link
Contributor

  • [ValueTracking] Add tests for non-constant idx in computeKnownBits of insertelement; NFC
  • [ValueTracking] Support non-constant idx for computeKnownBits of insertelement

…nsertelement`

Its same logic as before, we just need to intersect what we know about
the new Elt and the entire pre-existing Vec.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: None (goldsteinn)

Changes
  • [ValueTracking] Add tests for non-constant idx in computeKnownBits of insertelement; NFC
  • [ValueTracking] Support non-constant idx for computeKnownBits of insertelement

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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+10-11)
  • (modified) llvm/test/Transforms/InstCombine/insertelement.ll (+47-2)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 5ad4da43bca7db..15949142a8c77b 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1727,26 +1727,25 @@ static void computeKnownBitsFromOperator(const Operator *I,
     const Value *Vec = I->getOperand(0);
     const Value *Elt = I->getOperand(1);
     auto *CIdx = dyn_cast<ConstantInt>(I->getOperand(2));
-    // Early out if the index is non-constant or out-of-range.
     unsigned NumElts = DemandedElts.getBitWidth();
-    if (!CIdx || CIdx->getValue().uge(NumElts)) {
-      Known.resetAll();
-      return;
+    APInt DemandedVecElts = DemandedElts;
+    bool NeedsElt = true;
+    // If we know the index we are inserting too, clear it from Vec check.
+    if (CIdx && CIdx->getValue().ult(NumElts)) {
+      DemandedVecElts.clearBit(CIdx->getZExtValue());
+      NeedsElt = DemandedElts[CIdx->getZExtValue()];
     }
+
     Known.One.setAllBits();
     Known.Zero.setAllBits();
-    unsigned EltIdx = CIdx->getZExtValue();
-    // Do we demand the inserted element?
-    if (DemandedElts[EltIdx]) {
+    if (NeedsElt) {
       computeKnownBits(Elt, Known, Depth + 1, Q);
       // If we don't know any bits, early out.
       if (Known.isUnknown())
         break;
     }
-    // We don't need the base vector element that has been inserted.
-    APInt DemandedVecElts = DemandedElts;
-    DemandedVecElts.clearBit(EltIdx);
-    if (!!DemandedVecElts) {
+
+    if (!DemandedVecElts.isZero()) {
       computeKnownBits(Vec, DemandedVecElts, Known2, Depth + 1, Q);
       Known = Known.intersectWith(Known2);
     }
diff --git a/llvm/test/Transforms/InstCombine/insertelement.ll b/llvm/test/Transforms/InstCombine/insertelement.ll
index 976c495465ce47..c8df2db6e70caa 100644
--- a/llvm/test/Transforms/InstCombine/insertelement.ll
+++ b/llvm/test/Transforms/InstCombine/insertelement.ll
@@ -17,11 +17,56 @@ define <4 x i32> @insert_unknown_idx(<4 x i32> %x, i32 %idx) {
 ; CHECK-LABEL: @insert_unknown_idx(
 ; CHECK-NEXT:    [[V1:%.*]] = and <4 x i32> [[X:%.*]], <i32 7, i32 7, i32 7, i32 7>
 ; CHECK-NEXT:    [[V2:%.*]] = insertelement <4 x i32> [[V1]], i32 6, i32 [[IDX:%.*]]
-; CHECK-NEXT:    [[V3:%.*]] = and <4 x i32> [[V2]], <i32 7, i32 7, i32 7, i32 7>
-; CHECK-NEXT:    ret <4 x i32> [[V3]]
+; CHECK-NEXT:    ret <4 x i32> [[V2]]
 ;
   %v1 = and <4 x i32> %x, <i32 7, i32 7, i32 7, i32 7>
   %v2 = insertelement <4 x i32> %v1, i32 6, i32 %idx
   %v3 = and <4 x i32> %v2, <i32 7, i32 7, i32 7, i32 7>
   ret <4 x i32> %v3
 }
+
+define <2 x i8> @insert_known_any_idx(<2 x i8> %xx, i8 %yy, i32 %idx) {
+; CHECK-LABEL: @insert_known_any_idx(
+; CHECK-NEXT:    ret <2 x i8> <i8 16, i8 16>
+;
+  %x = or <2 x i8> %xx, <i8 16, i8 16>
+  %y = or i8 %yy, 16
+
+  %ins = insertelement <2 x i8> %x, i8 %y, i32 %idx
+  %r = and <2 x i8> %ins, <i8 16, i8 16>
+  ret <2 x i8> %r
+}
+
+define <2 x i8> @insert_known_any_idx_fail1(<2 x i8> %xx, i8 %yy, i32 %idx) {
+; CHECK-LABEL: @insert_known_any_idx_fail1(
+; CHECK-NEXT:    [[X:%.*]] = or <2 x i8> [[XX:%.*]], <i8 17, i8 33>
+; CHECK-NEXT:    [[Y:%.*]] = or i8 [[YY:%.*]], 16
+; CHECK-NEXT:    [[INS:%.*]] = insertelement <2 x i8> [[X]], i8 [[Y]], i32 [[IDX:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and <2 x i8> [[INS]], <i8 16, i8 16>
+; CHECK-NEXT:    ret <2 x i8> [[R]]
+;
+  %x = or <2 x i8> %xx, <i8 17, i8 33>
+  %y = or i8 %yy, 16
+
+  %ins = insertelement <2 x i8> %x, i8 %y, i32 %idx
+  %r = and <2 x i8> %ins, <i8 16, i8 16>
+  ret <2 x i8> %r
+}
+
+
+define <2 x i8> @insert_known_any_idx_fail2(<2 x i8> %xx, i8 %yy, i32 %idx) {
+; CHECK-LABEL: @insert_known_any_idx_fail2(
+; CHECK-NEXT:    [[X:%.*]] = or <2 x i8> [[XX:%.*]], <i8 17, i8 31>
+; CHECK-NEXT:    [[Y:%.*]] = or i8 [[YY:%.*]], 15
+; CHECK-NEXT:    [[INS:%.*]] = insertelement <2 x i8> [[X]], i8 [[Y]], i32 [[IDX:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = and <2 x i8> [[INS]], <i8 16, i8 16>
+; CHECK-NEXT:    ret <2 x i8> [[R]]
+;
+  %x = or <2 x i8> %xx, <i8 17, i8 31>
+  %y = or i8 %yy, 15
+
+  %ins = insertelement <2 x i8> %x, i8 %y, i32 %idx
+  %r = and <2 x i8> %ins, <i8 16, i8 16>
+  ret <2 x i8> %r
+}
+

@goldsteinn goldsteinn requested a review from dtcxzyw April 4, 2024 20:43
@goldsteinn goldsteinn changed the title goldstein/insertelement known [ValueTracking] Support non-constant idx for computeKnownBits of insertelement Apr 4, 2024
@goldsteinn goldsteinn requested a review from arsenm April 6, 2024 19:58
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@goldsteinn goldsteinn closed this in 964df09 Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants