[PATCH] D93065: [InstCombine] Disable optimizations of select instructions that causes propagation of poison values

[Congzhe Cao via Phabricator via llvm-commits]

congzhe created this revision.
congzhe added reviewers: lebedev.ri, aqjune, nlopes, spatel.
congzhe added projects: fixing bugs in llvm, LLVM.
Herald added a subscriber: hiraditya.
Herald added a reviewer: gkistanova.
congzhe requested review of this revision.
Herald added a subscriber: llvm-commits.

This is a work-in-progress.

Relevant discussions on Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=48353

In brief (cited from Roman Lebedev),

----------------------------------------

define i1 @src(i1 %cmp.i, i1 %cmp41) {
%entry:

  %cmp4 = select i1 %cmp.i, i1 1, i1 %cmp41
  ret i1 %cmp4

}

>
=

define i1 @tgt(i1 %cmp.i, i1 %cmp41) {
%entry:

  %cmp4 = or i1 %cmp.i, %cmp41
  ret i1 %cmp4

}
Transformation doesn't verify!
ERROR: Target is more poisonous than source

Example:
i1 %cmp.i = #x1 (1)
i1 %cmp41 = poison

Source:
i1 %cmp4 = #x1 (1)

Target:
i1 %cmp4 = poison
Source value: #x1 (1)

Target value: poison
--------------------

Therefore, this patches disabled all such optimizations of select instructions that result in propagation of poison values.

In addition, minor changes are also made to ensure there is no functional problem after disabling the abovementioned optimizations.

Performance measurement should be taken to measure potential degradations. Currently in progress.


Repository:
  rZORG LLVM Github Zorg

https://reviews.llvm.org/D93065

Files:
  llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp


Index: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2612,42 +2612,6 @@
 
   CmpInst::Predicate Pred;
 
-  if (SelType->isIntOrIntVectorTy(1) &&
-      TrueVal->getType() == CondVal->getType()) {
-    if (match(TrueVal, m_One())) {
-      // Change: A = select B, true, C --> A = or B, C
-      return BinaryOperator::CreateOr(CondVal, FalseVal);
-    }
-    if (match(TrueVal, m_Zero())) {
-      // Change: A = select B, false, C --> A = and !B, C
-      Value *NotCond = Builder.CreateNot(CondVal, "not." + CondVal->getName());
-      return BinaryOperator::CreateAnd(NotCond, FalseVal);
-    }
-    if (match(FalseVal, m_Zero())) {
-      // Change: A = select B, C, false --> A = and B, C
-      return BinaryOperator::CreateAnd(CondVal, TrueVal);
-    }
-    if (match(FalseVal, m_One())) {
-      // Change: A = select B, C, true --> A = or !B, C
-      Value *NotCond = Builder.CreateNot(CondVal, "not." + CondVal->getName());
-      return BinaryOperator::CreateOr(NotCond, TrueVal);
-    }
-
-    // select a, a, b  -> a | b
-    // select a, b, a  -> a & b
-    if (CondVal == TrueVal)
-      return BinaryOperator::CreateOr(CondVal, FalseVal);
-    if (CondVal == FalseVal)
-      return BinaryOperator::CreateAnd(CondVal, TrueVal);
-
-    // select a, ~a, b -> (~a) & b
-    // select a, b, ~a -> (~a) | b
-    if (match(TrueVal, m_Not(m_Specific(CondVal))))
-      return BinaryOperator::CreateAnd(TrueVal, FalseVal);
-    if (match(FalseVal, m_Not(m_Specific(CondVal))))
-      return BinaryOperator::CreateOr(TrueVal, FalseVal);
-  }
-
   // Selecting between two integer or vector splat integer constants?
   //
   // Note that we don't handle a scalar select of vectors:
@@ -2655,7 +2619,11 @@
   // because that may need 3 instructions to splat the condition value:
   // extend, insertelement, shufflevector.
   if (SelType->isIntOrIntVectorTy() &&
-      CondVal->getType()->isVectorTy() == SelType->isVectorTy()) {
+      CondVal->getType()->isVectorTy() == SelType->isVectorTy() &&
+      TrueVal->getType()->isIntegerTy() &&
+      TrueVal->getType()->getIntegerBitWidth() != 1 &&
+      FalseVal->getType()->isIntegerTy() &&
+      FalseVal->getType()->getIntegerBitWidth() != 1) {
     // select C, 1, 0 -> zext C to int
     if (match(TrueVal, m_One()) && match(FalseVal, m_Zero()))
       return new ZExtInst(CondVal, SelType);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93065.311003.patch
Type: text/x-patch
Size: 2552 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201210/8e93ef7f/attachment.bin>