Skip to content

[InstCombine] Swap out range metadata to range attribute for arm_mve_pred_v2i #94847

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

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

andjo403
Copy link
Contributor

@andjo403 andjo403 commented Jun 8, 2024

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-selectiondag

Author: Andreas Jonson (andjo403)

Changes

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

4 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+13-7)
  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h (+1-1)
  • (modified) llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp (+15-10)
  • (modified) llvm/test/Transforms/InstCombine/ARM/mve-v2i2v.ll (+42-9)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index be5e0f6ef058b..5901d6a584bf5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -10227,21 +10227,27 @@ void SelectionDAGBuilder::visitVACopy(const CallInst &I) {
 }
 
 SDValue SelectionDAGBuilder::lowerRangeToAssertZExt(SelectionDAG &DAG,
-                                                    const Instruction &I,
+                                                    const CallBase &I,
                                                     SDValue Op) {
+  std::optional<ConstantRange> CR = I.getRange();
   const MDNode *Range = getRangeMetadata(I);
-  if (!Range)
-    return Op;
+  if (Range) {
+    ConstantRange R = getConstantRangeFromMetadata(*Range);
+    if (CR) {
+      CR = CR->intersectWith(R);
+    } else {
+      CR = R;
+    }
+  }
 
-  ConstantRange CR = getConstantRangeFromMetadata(*Range);
-  if (CR.isFullSet() || CR.isEmptySet() || CR.isUpperWrapped())
+  if (!CR || CR->isFullSet() || CR->isEmptySet() || CR->isUpperWrapped())
     return Op;
 
-  APInt Lo = CR.getUnsignedMin();
+  APInt Lo = CR->getUnsignedMin();
   if (!Lo.isMinValue())
     return Op;
 
-  APInt Hi = CR.getUnsignedMax();
+  APInt Hi = CR->getUnsignedMax();
   unsigned Bits = std::max(Hi.getActiveBits(),
                            static_cast<unsigned>(IntegerType::MIN_INT_BITS));
 
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
index 1a98fbd7589fb..0f7bad27c4bba 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
@@ -411,7 +411,7 @@ class SelectionDAGBuilder {
 
   // Lower range metadata from 0 to N to assert zext to an integer of nearest
   // floor power of two.
-  SDValue lowerRangeToAssertZExt(SelectionDAG &DAG, const Instruction &I,
+  SDValue lowerRangeToAssertZExt(SelectionDAG &DAG, const CallBase &I,
                                  SDValue Op);
 
   void populateCallLoweringInfo(TargetLowering::CallLoweringInfo &CLI,
diff --git a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
index 7db2e8ee7e6f9..ec2bfb98832e4 100644
--- a/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
@@ -199,17 +199,22 @@ ARMTTIImpl::instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II) const {
                        PatternMatch::m_Value(ArgArg)))) {
       return IC.replaceInstUsesWith(II, ArgArg);
     }
-    if (!II.getMetadata(LLVMContext::MD_range)) {
-      Type *IntTy32 = Type::getInt32Ty(II.getContext());
-      Metadata *M[] = {
-          ConstantAsMetadata::get(ConstantInt::get(IntTy32, 0)),
-          ConstantAsMetadata::get(ConstantInt::get(IntTy32, 0x10000))};
-      II.setMetadata(LLVMContext::MD_range, MDNode::get(II.getContext(), M));
-      II.setMetadata(LLVMContext::MD_noundef,
-                     MDNode::get(II.getContext(), std::nullopt));
-      return &II;
+
+    if (II.getMetadata(LLVMContext::MD_range))
+      break;
+
+    ConstantRange Range(APInt(32, 0), APInt(32, 0x10000));
+
+    if (auto CurrentRange = II.getRange()) {
+      Range = Range.intersectWith(*CurrentRange);
+      if (Range == CurrentRange)
+        break;
     }
-    break;
+
+    II.addRangeRetAttr(Range);
+    II.setMetadata(LLVMContext::MD_noundef,
+                   MDNode::get(II.getContext(), std::nullopt));
+    return &II;
   }
   case Intrinsic::arm_mve_vadc:
   case Intrinsic::arm_mve_vadc_predicated: {
diff --git a/llvm/test/Transforms/InstCombine/ARM/mve-v2i2v.ll b/llvm/test/Transforms/InstCombine/ARM/mve-v2i2v.ll
index d0d17efd23050..2d8e8096efcf1 100644
--- a/llvm/test/Transforms/InstCombine/ARM/mve-v2i2v.ll
+++ b/llvm/test/Transforms/InstCombine/ARM/mve-v2i2v.ll
@@ -66,7 +66,7 @@ entry:
 define <16 x i1> @v2i2v_2_16(<2 x i1> %vin) {
 ; CHECK-LABEL: @v2i2v_2_16(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INT:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v2i1(<2 x i1> [[VIN:%.*]]), !range [[RNG0:![0-9]+]]
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v2i1(<2 x i1> [[VIN:%.*]]), !noundef [[META0:![0-9]+]]
 ; CHECK-NEXT:    [[VOUT:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[INT]])
 ; CHECK-NEXT:    ret <16 x i1> [[VOUT]]
 ;
@@ -79,7 +79,7 @@ entry:
 define <16 x i1> @v2i2v_4_16(<4 x i1> %vin) {
 ; CHECK-LABEL: @v2i2v_4_16(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INT:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[VOUT:%.*]] = call <16 x i1> @llvm.arm.mve.pred.i2v.v16i1(i32 [[INT]])
 ; CHECK-NEXT:    ret <16 x i1> [[VOUT]]
 ;
@@ -92,7 +92,7 @@ entry:
 define <4 x i1> @v2i2v_8_4(<8 x i1> %vin) {
 ; CHECK-LABEL: @v2i2v_8_4(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INT:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v8i1(<8 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v8i1(<8 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[VOUT:%.*]] = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 [[INT]])
 ; CHECK-NEXT:    ret <4 x i1> [[VOUT]]
 ;
@@ -105,7 +105,7 @@ entry:
 define <8 x i1> @v2i2v_16_8(<16 x i1> %vin) {
 ; CHECK-LABEL: @v2i2v_16_8(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INT:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[VOUT:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 [[INT]])
 ; CHECK-NEXT:    ret <8 x i1> [[VOUT]]
 ;
@@ -170,7 +170,7 @@ entry:
 define i32 @v2i_truncext_i16(<4 x i1> %vin) {
 ; CHECK-LABEL: @v2i_truncext_i16(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[WIDE1:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[WIDE1:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    ret i32 [[WIDE1]]
 ;
 entry:
@@ -183,7 +183,7 @@ entry:
 define i32 @v2i_truncext_i8(<4 x i1> %vin) {
 ; CHECK-LABEL: @v2i_truncext_i8(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[WIDE1:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[WIDE1:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[WIDE2:%.*]] = and i32 [[WIDE1]], 255
 ; CHECK-NEXT:    ret i32 [[WIDE2]]
 ;
@@ -197,7 +197,7 @@ entry:
 define i32 @v2i_and_16(<4 x i1> %vin) {
 ; CHECK-LABEL: @v2i_and_16(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[WIDE1:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[WIDE1:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    ret i32 [[WIDE1]]
 ;
 entry:
@@ -209,7 +209,7 @@ entry:
 define i32 @v2i_and_15(<4 x i1> %vin) {
 ; CHECK-LABEL: @v2i_and_15(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[WIDE1:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[WIDE1:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v4i1(<4 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[WIDE2:%.*]] = and i32 [[WIDE1]], 32767
 ; CHECK-NEXT:    ret i32 [[WIDE2]]
 ;
@@ -397,7 +397,7 @@ entry:
 define i32 @range_upper_limit(<16 x i1> %vin) {
 ; CHECK-LABEL: @range_upper_limit(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[INT:%.*]] = call i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !noundef [[META0]]
 ; CHECK-NEXT:    [[C:%.*]] = icmp eq i32 [[INT]], 65535
 ; CHECK-NEXT:    [[S:%.*]] = zext i1 [[C]] to i32
 ; CHECK-NEXT:    ret i32 [[S]]
@@ -408,3 +408,36 @@ entry:
   %s = select i1 %c, i32 1, i32 0
   ret i32 %s
 }
+
+define i32 @range_already_added_larger_range(<16 x i1> %vin) {
+; CHECK-LABEL: @range_already_added_larger_range(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !noundef [[META0]]
+; CHECK-NEXT:    ret i32 [[INT]]
+;
+entry:
+  %int = call range(i32 0, 65540) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> %vin), !noundef !{}
+  ret i32 %int
+}
+
+define i32 @range_already_added_smaller_range(<16 x i1> %vin) {
+; CHECK-LABEL: @range_already_added_smaller_range(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 655) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !noundef [[META0]]
+; CHECK-NEXT:    ret i32 [[INT]]
+;
+entry:
+  %int = call range(i32 0, 655) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> %vin), !noundef !{}
+  ret i32 %int
+}
+
+define i32 @range_already_added_same_range(<16 x i1> %vin) {
+; CHECK-LABEL: @range_already_added_same_range(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[INT:%.*]] = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> [[VIN:%.*]]), !noundef [[META0]]
+; CHECK-NEXT:    ret i32 [[INT]]
+;
+entry:
+  %int = call range(i32 0, 65536) i32 @llvm.arm.mve.pred.v2i.v16i1(<16 x i1> %vin), !noundef !{}
+  ret i32 %int
+}

@andjo403
Copy link
Contributor Author

andjo403 commented Jun 8, 2024

Continues to swap out range metadata to range attribute for calls to be able to deprecate range metadata on calls in the future

@andjo403 andjo403 force-pushed the armTargetRangeAttr branch from 23fcf2b to 0a38244 Compare June 9, 2024 15:42
@arsenm
Copy link
Contributor

arsenm commented Jun 9, 2024

to be able to deprecate range metadata on calls in the future

Meaning what exactly? I don't think calls should be special cased to disallow range metadata, it would just be a less canonical form?

@@ -10227,21 +10227,25 @@ void SelectionDAGBuilder::visitVACopy(const CallInst &I) {
}

SDValue SelectionDAGBuilder::lowerRangeToAssertZExt(SelectionDAG &DAG,
const Instruction &I,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not lose the ability to handle range metadata. We should still be able to lower loads etc. with range to include the assert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will fix, only noticed that it was called for calls now and it made the code a bit simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is fixed now

@nikic
Copy link
Contributor

nikic commented Jun 9, 2024

to be able to deprecate range metadata on calls in the future

Meaning what exactly? I don't think calls should be special cased to disallow range metadata, it would just be a less canonical form?

We generally require that calls use return attributes instead of metadata. E.g. the verifier will reject !nonnull or !align on a call, you have to use the nonnull or align attribute instead. We'll do the same for !range metadata vs attribute once it's fully migrated. The "attribute-like" metadata is only allowed on load instructions.

@andjo403
Copy link
Contributor Author

andjo403 commented Jun 9, 2024

about the deprecation it was my interpretation of the comment from @nikic here
#84627 (comment) but deprecate as in the Verifier not allow range metadata on calls and invokes

@andjo403 andjo403 force-pushed the armTargetRangeAttr branch from e6e8335 to 18248f7 Compare June 12, 2024 22:00
@andjo403 andjo403 force-pushed the armTargetRangeAttr branch from 18248f7 to 88af489 Compare June 17, 2024 17:12
@andjo403
Copy link
Contributor Author

This is rebased on the part that was split out now and ready for review

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

@andjo403 andjo403 merged commit 34a2889 into llvm:main Jun 19, 2024
7 checks passed
@andjo403 andjo403 deleted the armTargetRangeAttr branch June 19, 2024 16:28
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 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.

4 participants