[llvm] a7b898b - [InstCombine] Disallow constant expressions in `not` canonicalization

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 20 08:57:31 PST 2022


Author: Roman Lebedev
Date: 2022-12-20T19:56:37+03:00
New Revision: a7b898b49abb90dd289d7c9adf5f9a6350787347

URL: https://github.com/llvm/llvm-project/commit/a7b898b49abb90dd289d7c9adf5f9a6350787347
DIFF: https://github.com/llvm/llvm-project/commit/a7b898b49abb90dd289d7c9adf5f9a6350787347.diff

LOG: [InstCombine] Disallow constant expressions in `not` canonicalization

As per post-commit feedback - we generally do not like Constant Expressions,
and trying to deal with them leads to inconsistent results
that may very well be non-optimal. So just don't.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 784343532c22..93bb120e6ae1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3679,9 +3679,10 @@ bool InstCombinerImpl::sinkNotIntoLogicalOp(Instruction &I) {
   // And can the operands be adapted?
   for (Value *Op : {Op0, Op1})
     if (!(InstCombiner::isFreeToInvert(Op, /*WillInvertAllUses=*/true) &&
-          (isa<Constant>(Op) ||
-           InstCombiner::canFreelyInvertAllUsersOf(cast<Instruction>(Op),
-                                                   /*IgnoredUser=*/&I))))
+          (match(Op, m_ImmConstant()) ||
+           (isa<Instruction>(Op) &&
+            InstCombiner::canFreelyInvertAllUsersOf(cast<Instruction>(Op),
+                                                    /*IgnoredUser=*/&I)))))
       return false;
 
   for (Value **Op : {&Op0, &Op1}) {
@@ -3733,15 +3734,18 @@ bool InstCombinerImpl::sinkNotIntoOtherHandOfLogicalOp(Instruction &I) {
   Value **OpToInvert = nullptr;
   if (match(Op0, m_Not(m_Value(NotOp0))) &&
       InstCombiner::isFreeToInvert(Op1, /*WillInvertAllUses=*/true) &&
-      (isa<Constant>(Op1) || InstCombiner::canFreelyInvertAllUsersOf(
-                                 cast<Instruction>(Op1), /*IgnoredUser=*/&I))) {
+      (match(Op1, m_ImmConstant()) ||
+       (isa<Instruction>(Op1) &&
+        InstCombiner::canFreelyInvertAllUsersOf(cast<Instruction>(Op1),
+                                                /*IgnoredUser=*/&I)))) {
     Op0 = NotOp0;
     OpToInvert = &Op1;
   } else if (match(Op1, m_Not(m_Value(NotOp1))) &&
              InstCombiner::isFreeToInvert(Op0, /*WillInvertAllUses=*/true) &&
-             (isa<Constant>(Op0) ||
-              InstCombiner::canFreelyInvertAllUsersOf(cast<Instruction>(Op0),
-                                                      /*IgnoredUser=*/&I))) {
+             (match(Op0, m_ImmConstant()) ||
+              (isa<Instruction>(Op0) &&
+               InstCombiner::canFreelyInvertAllUsersOf(cast<Instruction>(Op0),
+                                                       /*IgnoredUser=*/&I)))) {
     Op1 = NotOp1;
     OpToInvert = &Op0;
   } else


        


More information about the llvm-commits mailing list