Skip to content

Revert "[PatternMatch] Fix issue of stale reference in new m_{I,F,}Cmp matchers" #99062

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 16, 2024

Conversation

vitalybuka
Copy link
Collaborator

Reverts #98866

It's still use-after-scope.

@vitalybuka vitalybuka requested review from goldsteinn and nikic July 16, 2024 16:54
@vitalybuka vitalybuka merged commit afbdd6f into main Jul 16, 2024
6 of 8 checks passed
@vitalybuka vitalybuka deleted the revert-98866-goldsteinn/fix-cmp-matchers branch July 16, 2024 16:54
@llvmbot
Copy link
Member

llvmbot commented Jul 16, 2024

@llvm/pr-subscribers-llvm-ir

Author: Vitaly Buka (vitalybuka)

Changes

Reverts llvm/llvm-project#98866

It's still use-after-scope.


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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/PatternMatch.h (+10-11)
  • (modified) llvm/unittests/IR/PatternMatch.cpp (+2-2)
diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h
index bea1ad97ea09c..8ae47fb556b25 100644
--- a/llvm/include/llvm/IR/PatternMatch.h
+++ b/llvm/include/llvm/IR/PatternMatch.h
@@ -1550,27 +1550,23 @@ template <typename T> inline Exact_match<T> m_Exact(const T &SubPattern) {
 template <typename LHS_t, typename RHS_t, typename Class, typename PredicateTy,
           bool Commutable = false>
 struct CmpClass_match {
-  PredicateTy *Predicate;
+  PredicateTy &Predicate;
   LHS_t L;
   RHS_t R;
 
   // The evaluation order is always stable, regardless of Commutability.
   // The LHS is always matched first.
   CmpClass_match(PredicateTy &Pred, const LHS_t &LHS, const RHS_t &RHS)
-      : Predicate(&Pred), L(LHS), R(RHS) {}
-  CmpClass_match(const LHS_t &LHS, const RHS_t &RHS)
-      : Predicate(nullptr), L(LHS), R(RHS) {}
+      : Predicate(Pred), L(LHS), R(RHS) {}
 
   template <typename OpTy> bool match(OpTy *V) {
     if (auto *I = dyn_cast<Class>(V)) {
       if (L.match(I->getOperand(0)) && R.match(I->getOperand(1))) {
-        if (Predicate)
-          *Predicate = I->getPredicate();
+        Predicate = I->getPredicate();
         return true;
       } else if (Commutable && L.match(I->getOperand(1)) &&
                  R.match(I->getOperand(0))) {
-        if (Predicate)
-          *Predicate = I->getSwappedPredicate();
+        Predicate = I->getSwappedPredicate();
         return true;
       }
     }
@@ -1599,19 +1595,22 @@ m_FCmp(FCmpInst::Predicate &Pred, const LHS &L, const RHS &R) {
 template <typename LHS, typename RHS>
 inline CmpClass_match<LHS, RHS, CmpInst, CmpInst::Predicate>
 m_Cmp(const LHS &L, const RHS &R) {
-  return CmpClass_match<LHS, RHS, CmpInst, CmpInst::Predicate>(L, R);
+  CmpInst::Predicate Unused;
+  return CmpClass_match<LHS, RHS, CmpInst, CmpInst::Predicate>(Unused, L, R);
 }
 
 template <typename LHS, typename RHS>
 inline CmpClass_match<LHS, RHS, ICmpInst, ICmpInst::Predicate>
 m_ICmp(const LHS &L, const RHS &R) {
-  return CmpClass_match<LHS, RHS, ICmpInst, ICmpInst::Predicate>(L, R);
+  ICmpInst::Predicate Unused;
+  return CmpClass_match<LHS, RHS, ICmpInst, ICmpInst::Predicate>(Unused, L, R);
 }
 
 template <typename LHS, typename RHS>
 inline CmpClass_match<LHS, RHS, FCmpInst, FCmpInst::Predicate>
 m_FCmp(const LHS &L, const RHS &R) {
-  return CmpClass_match<LHS, RHS, FCmpInst, FCmpInst::Predicate>(L, R);
+  FCmpInst::Predicate Unused;
+  return CmpClass_match<LHS, RHS, FCmpInst, FCmpInst::Predicate>(Unused, L, R);
 }
 
 // Same as CmpClass, but instead of saving Pred as out output variable, match a
diff --git a/llvm/unittests/IR/PatternMatch.cpp b/llvm/unittests/IR/PatternMatch.cpp
index 309fcc93996bc..b82711ec244a6 100644
--- a/llvm/unittests/IR/PatternMatch.cpp
+++ b/llvm/unittests/IR/PatternMatch.cpp
@@ -2235,7 +2235,7 @@ typedef ::testing::Types<std::tuple<Value*, Instruction*>,
     MutableConstTestTypes;
 TYPED_TEST_SUITE(MutableConstTest, MutableConstTestTypes, );
 
-TYPED_TEST(MutableConstTest, ICmp) {
+TYPED_TEST(MutableConstTest, /* FIXME: UAR bug */ DISABLED_ICmp) {
   auto &IRB = PatternMatchTest::IRB;
 
   typedef std::tuple_element_t<0, TypeParam> ValueType;
@@ -2319,7 +2319,7 @@ TYPED_TEST(MutableConstTest, ICmp) {
                    .match((InstructionType)IRB.CreateICmp(Pred, L, R)));
 }
 
-TYPED_TEST(MutableConstTest, FCmp) {
+TYPED_TEST(MutableConstTest, /* FIXME: UAR bug */ DISABLED_FCmp) {
   auto &IRB = PatternMatchTest::IRB;
 
   typedef std::tuple_element_t<0, TypeParam> ValueType;

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…mp` matchers" (#99062)

Summary:
Reverts #98866

It's still use-after-scope.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251755
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants