Skip to content

[NFC] Add CmpIntrinsic class to represent calls to UCMP/SCMP intrinsics #98177

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
Jul 9, 2024

Conversation

Poseydon42
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2024

@llvm/pr-subscribers-llvm-ir

Author: None (Poseydon42)

Changes

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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicInst.h (+37)
  • (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+15-17)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 3963a5c8ab8f9..2a37c06dd2c3c 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -834,6 +834,43 @@ class MinMaxIntrinsic : public IntrinsicInst {
   }
 };
 
+/// This class represents a ucmp/scmp intrinsic
+class CmpIntrinsic : public IntrinsicInst {
+public:
+  static bool classof(const IntrinsicInst *I) {
+    switch (I->getIntrinsicID()) {
+    case Intrinsic::scmp:
+    case Intrinsic::ucmp:
+      return true;
+    default:
+      return false;
+    }
+  }
+  static bool classof(const Value *V) {
+    return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+  }
+
+  Value *getLHS() const { return const_cast<Value *>(getArgOperand(0)); }
+  Value *getRHS() const { return const_cast<Value *>(getArgOperand(1)); }
+
+  static bool isSigned(Intrinsic::ID ID) { return ID == Intrinsic::scmp; }
+  bool isSigned() const { return isSigned(getIntrinsicID()); }
+
+  static CmpInst::Predicate getGTPredicate(Intrinsic::ID ID) {
+    return isSigned(ID) ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
+  }
+  CmpInst::Predicate getGTPredicate() const {
+    return getGTPredicate(getIntrinsicID());
+  }
+
+  static CmpInst::Predicate getLTPredicate(Intrinsic::ID ID) {
+    return isSigned(ID) ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT;
+  }
+  CmpInst::Predicate getLTPredicate() const {
+    return getLTPredicate(getIntrinsicID());
+  }
+};
+
 /// This class represents an intrinsic that is based on a binary operation.
 /// This includes op.with.overflow and saturating add/sub intrinsics.
 class BinaryOpIntrinsic : public IntrinsicInst {
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 20f5dba413212..596078407edd1 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -538,29 +538,28 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
   return false;
 }
 
-static bool processCmpIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
-  bool IsSigned = II->getIntrinsicID() == Intrinsic::scmp;
-  ConstantRange LHS_CR = LVI->getConstantRangeAtUse(II->getOperandUse(0),
-                                                    /*UndefAllowed*/ false);
-  ConstantRange RHS_CR = LVI->getConstantRangeAtUse(II->getOperandUse(1),
-                                                    /*UndefAllowed*/ false);
+static bool processCmpIntrinsic(CmpIntrinsic *CI, LazyValueInfo *LVI) {
+  ConstantRange LHS_CR =
+      LVI->getConstantRangeAtUse(CI->getOperandUse(0), /*UndefAllowed*/ false);
+  ConstantRange RHS_CR =
+      LVI->getConstantRangeAtUse(CI->getOperandUse(1), /*UndefAllowed*/ false);
 
-  if (LHS_CR.icmp(IsSigned ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT, RHS_CR)) {
+  if (LHS_CR.icmp(CI->getGTPredicate(), RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::get(II->getType(), 1));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::get(CI->getType(), 1));
+    CI->eraseFromParent();
     return true;
   }
-  if (LHS_CR.icmp(IsSigned ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT, RHS_CR)) {
+  if (LHS_CR.icmp(CI->getLTPredicate(), RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::getSigned(II->getType(), -1));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::getSigned(CI->getType(), -1));
+    CI->eraseFromParent();
     return true;
   }
   if (LHS_CR.icmp(ICmpInst::ICMP_EQ, RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::get(II->getType(), 0));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::get(CI->getType(), 0));
+    CI->eraseFromParent();
     return true;
   }
 
@@ -658,9 +657,8 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
     return processAbsIntrinsic(&cast<IntrinsicInst>(CB), LVI);
   }
 
-  if (CB.getIntrinsicID() == Intrinsic::scmp ||
-      CB.getIntrinsicID() == Intrinsic::ucmp) {
-    return processCmpIntrinsic(&cast<IntrinsicInst>(CB), LVI);
+  if (auto* CI = dyn_cast<CmpIntrinsic>(&CB)) {
+    return processCmpIntrinsic(CI, LVI);
   }
 
   if (auto *MM = dyn_cast<MinMaxIntrinsic>(&CB)) {

@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (Poseydon42)

Changes

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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/IntrinsicInst.h (+37)
  • (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+15-17)
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 3963a5c8ab8f9..2a37c06dd2c3c 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -834,6 +834,43 @@ class MinMaxIntrinsic : public IntrinsicInst {
   }
 };
 
+/// This class represents a ucmp/scmp intrinsic
+class CmpIntrinsic : public IntrinsicInst {
+public:
+  static bool classof(const IntrinsicInst *I) {
+    switch (I->getIntrinsicID()) {
+    case Intrinsic::scmp:
+    case Intrinsic::ucmp:
+      return true;
+    default:
+      return false;
+    }
+  }
+  static bool classof(const Value *V) {
+    return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+  }
+
+  Value *getLHS() const { return const_cast<Value *>(getArgOperand(0)); }
+  Value *getRHS() const { return const_cast<Value *>(getArgOperand(1)); }
+
+  static bool isSigned(Intrinsic::ID ID) { return ID == Intrinsic::scmp; }
+  bool isSigned() const { return isSigned(getIntrinsicID()); }
+
+  static CmpInst::Predicate getGTPredicate(Intrinsic::ID ID) {
+    return isSigned(ID) ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
+  }
+  CmpInst::Predicate getGTPredicate() const {
+    return getGTPredicate(getIntrinsicID());
+  }
+
+  static CmpInst::Predicate getLTPredicate(Intrinsic::ID ID) {
+    return isSigned(ID) ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT;
+  }
+  CmpInst::Predicate getLTPredicate() const {
+    return getLTPredicate(getIntrinsicID());
+  }
+};
+
 /// This class represents an intrinsic that is based on a binary operation.
 /// This includes op.with.overflow and saturating add/sub intrinsics.
 class BinaryOpIntrinsic : public IntrinsicInst {
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 20f5dba413212..596078407edd1 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -538,29 +538,28 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
   return false;
 }
 
-static bool processCmpIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
-  bool IsSigned = II->getIntrinsicID() == Intrinsic::scmp;
-  ConstantRange LHS_CR = LVI->getConstantRangeAtUse(II->getOperandUse(0),
-                                                    /*UndefAllowed*/ false);
-  ConstantRange RHS_CR = LVI->getConstantRangeAtUse(II->getOperandUse(1),
-                                                    /*UndefAllowed*/ false);
+static bool processCmpIntrinsic(CmpIntrinsic *CI, LazyValueInfo *LVI) {
+  ConstantRange LHS_CR =
+      LVI->getConstantRangeAtUse(CI->getOperandUse(0), /*UndefAllowed*/ false);
+  ConstantRange RHS_CR =
+      LVI->getConstantRangeAtUse(CI->getOperandUse(1), /*UndefAllowed*/ false);
 
-  if (LHS_CR.icmp(IsSigned ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT, RHS_CR)) {
+  if (LHS_CR.icmp(CI->getGTPredicate(), RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::get(II->getType(), 1));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::get(CI->getType(), 1));
+    CI->eraseFromParent();
     return true;
   }
-  if (LHS_CR.icmp(IsSigned ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT, RHS_CR)) {
+  if (LHS_CR.icmp(CI->getLTPredicate(), RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::getSigned(II->getType(), -1));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::getSigned(CI->getType(), -1));
+    CI->eraseFromParent();
     return true;
   }
   if (LHS_CR.icmp(ICmpInst::ICMP_EQ, RHS_CR)) {
     ++NumCmpIntr;
-    II->replaceAllUsesWith(ConstantInt::get(II->getType(), 0));
-    II->eraseFromParent();
+    CI->replaceAllUsesWith(ConstantInt::get(CI->getType(), 0));
+    CI->eraseFromParent();
     return true;
   }
 
@@ -658,9 +657,8 @@ static bool processCallSite(CallBase &CB, LazyValueInfo *LVI) {
     return processAbsIntrinsic(&cast<IntrinsicInst>(CB), LVI);
   }
 
-  if (CB.getIntrinsicID() == Intrinsic::scmp ||
-      CB.getIntrinsicID() == Intrinsic::ucmp) {
-    return processCmpIntrinsic(&cast<IntrinsicInst>(CB), LVI);
+  if (auto* CI = dyn_cast<CmpIntrinsic>(&CB)) {
+    return processCmpIntrinsic(CI, LVI);
   }
 
   if (auto *MM = dyn_cast<MinMaxIntrinsic>(&CB)) {

Copy link

github-actions bot commented Jul 9, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@Poseydon42 Poseydon42 force-pushed the uscmp-instruction-class branch from 9cc2c27 to 170991a Compare July 9, 2024 15:53
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

@nikic nikic merged commit 0162386 into llvm:main Jul 9, 2024
5 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 9, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/1481

Here is the relevant piece of the build log for the reference:

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/memory_manager.cpp (776 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49779.cpp (777 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/test_libc.cpp (778 of 786)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp (779 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/wtime.c (780 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/bug49021.cpp (781 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/std_complex_arithmetic.cpp (782 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/complex_reduction.cpp (783 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49021.cpp (784 of 786)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (785 of 786)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1227.948696

aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 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.

5 participants