[NewGVN] Apply SimplifySelectInst if any of the options is undef.

[fhahn]

Summary

The Simplify* functions expect undefs to be replaced as we go along, to
avoid the same undef use being used with different values.

See PR33165 for an extensive discussion of the issue.

[fhahn]

ping

[mkazantsev]

Looks good. Is it still not merged? :)

[mkazantsev]

test/Transforms/NewGVN/pr33165.ll

2

Please add run commant with -passes=newgvn

[fhahn]

In D51922#2189925, @mkazantsev wrote:

Looks good. Is it still not merged? :)

unfortunately I did not really have much time to focus on some of the outstanding NewGVN patches, but finally found some time :)

While this patch on it's own makes sense, it also hides a slightly different problem, but I think we finally have a reasonable approach to fix this other problem more generally (D84792), so I think it should be safe to also land this patch (which is an additional optimization). Thanks for taking a look!

[fhahn]

After d236e1c7b606c461b5cb8a8a87d50ead5d1bcbb9, the original optimizations do not trigger any more, as uses of undef are restricted. I don't think this is needed any longer.

lib/Transforms/Scalar/NewGVN.cpp

if (auto *CI = dyn_cast<CmpInst>(I)) {
     assert((E->getOperand(0)->getType() == I->getOperand(0)->getType() &&
             E->getOperand(1)->getType() == I->getOperand(1)->getType()));
     Value *V =
         SimplifyCmpInst(Predicate, E->getOperand(0), E->getOperand(1), SQ);
     if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
       return SimplifiedE;
   } else if (isa<SelectInst>(I)) {
     if (isa<Constant>(E->getOperand(0)) ||
-        E->getOperand(1) == E->getOperand(2)) {
+        E->getOperand(1) == E->getOperand(2) ||
+        isa<UndefValue>(E->getOperand(1)) ||
+        isa<UndefValue>(E->getOperand(2))) {
       assert(E->getOperand(1)->getType() == I->getOperand(1)->getType() &&
              E->getOperand(2)->getType() == I->getOperand(2)->getType());
       Value *V = SimplifySelectInst(E->getOperand(0), E->getOperand(1),
                                     E->getOperand(2), SQ);
       if (const Expression *SimplifiedE = checkSimplificationResults(E, I, V))
         return SimplifiedE;
     }
   } else if (I->isBinaryOp()) {

test/Transforms/NewGVN/pr33165.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -newgvn -S < %s | FileCheck %s
+
+define i2 @t1(i2, i1) {
+; CHECK-LABEL: @t1(
+; CHECK-NEXT:    [[A:%.*]] = xor i2 [[TMP0:%.*]], -1
+; CHECK-NEXT:    ret i2 [[A]]
+;
+  %a = xor i2 %0, -1
+  %b = select i1 %1, i2 %a, i2 undef
+  %c = and i2 %a, %b
+  ret i2 %c
+}
+
+define i2 @t2(i2, i1) {
+; CHECK-LABEL: @t2(
+; CHECK-NEXT:    [[A:%.*]] = xor i2 [[TMP0:%.*]], -1
+; CHECK-NEXT:    ret i2 [[A]]
+;
+  %a = xor i2 %0, -1
+  %b = select i1 %1, i2 undef, i2 %a
+  %c = and i2 %a, %b
+  ret i2 %c
+}