Skip to content

[SDAG] Lower range attribute to AssertZext #95450

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 3 commits into from
Jun 14, 2024

Conversation

andjo403
Copy link
Contributor

split out as requested in #94847 (comment)
CC @nikic

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Jun 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 13, 2024

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: Andreas Jonson (andjo403)

Changes

split out as requested in #94847 (comment)
CC @nikic


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+16-7)
  • (modified) llvm/test/CodeGen/X86/legalize-vec-assertzext.ll (+18)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index be5e0f6ef058b..5d21a905c316c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4487,6 +4487,18 @@ static const MDNode *getRangeMetadata(const Instruction &I) {
   return I.getMetadata(LLVMContext::MD_range);
 }
 
+static std::optional<ConstantRange> getRange(const Instruction &I) {
+  if (const auto *CB = dyn_cast<CallBase>(&I)) {
+    // see comment in getRangeMetadata about this check
+    if (CB->hasRetAttr(Attribute::NoUndef))
+      return CB->getRange();
+  }
+  if (const MDNode *Range = getRangeMetadata(I)) {
+    return getConstantRangeFromMetadata(*Range);
+  }
+  return std::nullopt;
+}
+
 void SelectionDAGBuilder::visitLoad(const LoadInst &I) {
   if (I.isAtomic())
     return visitAtomicLoad(I);
@@ -10229,19 +10241,16 @@ void SelectionDAGBuilder::visitVACopy(const CallInst &I) {
 SDValue SelectionDAGBuilder::lowerRangeToAssertZExt(SelectionDAG &DAG,
                                                     const Instruction &I,
                                                     SDValue Op) {
-  const MDNode *Range = getRangeMetadata(I);
-  if (!Range)
-    return Op;
+  std::optional<ConstantRange> CR = getRange(I);
 
-  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/test/CodeGen/X86/legalize-vec-assertzext.ll b/llvm/test/CodeGen/X86/legalize-vec-assertzext.ll
index 1c595b7fb5e1e..2cf37c68b8b40 100644
--- a/llvm/test/CodeGen/X86/legalize-vec-assertzext.ll
+++ b/llvm/test/CodeGen/X86/legalize-vec-assertzext.ll
@@ -34,6 +34,24 @@ define i64 @widen_assertzext(ptr %x) nounwind {
   ret i64 %d
 }
 
+define i64 @widen_assertzext_range_attr(ptr %x) nounwind {
+; CHECK-LABEL: widen_assertzext_range_attr:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    callq test2@PLT
+; CHECK-NEXT:    movb $127, %al
+; CHECK-NEXT:    kmovw %eax, %k1
+; CHECK-NEXT:    vpexpandq %zmm0, %zmm0 {%k1} {z}
+; CHECK-NEXT:    vextracti32x4 $3, %zmm0, %xmm0
+; CHECK-NEXT:    vmovq %xmm0, %rax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    vzeroupper
+; CHECK-NEXT:    retq
+  %e = call noundef range(i64 0, 2) <7 x i64> @test2()
+  %d = extractelement <7 x i64> %e, i32 6
+  ret i64 %d
+}
+
 declare  <16 x i64> @test()
 declare  <7 x i64> @test2()
 !0 = !{ i64 0, i64 2 }

@andjo403 andjo403 force-pushed the rangeAttrAssertZextLowering branch from f6ffb4a to 65238d6 Compare June 13, 2024 20:44
@andjo403
Copy link
Contributor Author

tests added

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

It looks like MachineMemOperand represents ranges as const MDNode *, so supporting range attributes for other getRangeMetadata() calls in this file may be tricky :(

@andjo403
Copy link
Contributor Author

fixed the braces. have not received merge access (but have asked for it now at least)

yes the MachineMemOperand representation have me a bit stumped have tried to replace it with std::optional<ConstantRange> but have hit problems with the MIParser that I think that I need to update

@nikic nikic merged commit ae71609 into llvm:main Jun 14, 2024
5 of 6 checks passed
@nikic
Copy link
Contributor

nikic commented Jun 14, 2024

@andjo403 The thing I would be concerned about is memory usage. Currently this is one pointer, while ConstantRange is four pointers large (though it only actually holds two and a half pointers worth of data...)

@andjo403 andjo403 deleted the rangeAttrAssertZextLowering branch June 15, 2024 19:38
EthanLuisMcDonough pushed a commit to EthanLuisMcDonough/llvm-project that referenced this pull request Aug 13, 2024
Add support for range attributes on calls, in addition to range metadata.
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