[llvm] [InstCombine] Invalidate changes to `DoesConsume` if the first try fails (PR #82973)

[via llvm-commits]

================
@@ -2307,9 +2309,11 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
   // If `V` is of the form `A ^ ~B` then `~(A ^ ~B)` can be folded
   // into `A ^ B` if we are willing to invert all of the uses.
   if (match(V, m_Xor(m_Value(A), m_Value(B)))) {
+    bool DoesConsumeOldValue = DoesConsume;
     if (auto *BV = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
                                          DoesConsume, Depth))
       return Builder ? Builder->CreateXor(A, BV) : NonNull;
+    DoesConsume = DoesConsumeOldValue;
     if (auto *AV = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
----------------
goldsteinn wrote:

I don't think this is the proper fix.
AFAICT the issue emerges from phi/select where one arm/value sets `DoesConsume` but we don't complete the transformation i.e in the testcase below:
when handling `%p0 = phi i32 [ %not, %entry ], [ %conv, %for.body ]` we will set `DoesConsume` when handling `%not`, but then fail on `%conv` without cleaning up `DoesConsume`.

The following diff seems to fix the issue:
```
@@ -2341,6 +2344,7 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
                   !shouldAvoidAbsorbingNotIntoSelect(*cast<SelectInst>(V));
   // Selects/min/max with invertible operands are freely invertible
   if (IsSelect || match(V, m_MaxOrMin(m_Value(A), m_Value(B)))) {
+    bool SaveDoesConsume = DoesConsume;
     if (!getFreelyInvertedImpl(B, B->hasOneUse(), /*Builder*/ nullptr,
                                DoesConsume, Depth))
       return nullptr;
@@ -2358,20 +2362,26 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
       }
       return NonNull;
     }
+    DoesConsume = SaveDoesConsume;
   }
 
   if (PHINode *PN = dyn_cast<PHINode>(V)) {
     SmallVector<std::pair<Value *, BasicBlock *>, 8> IncomingValues;
+    bool SaveDoesConsume = DoesConsume;
     for (Use &U : PN->operands()) {
       BasicBlock *IncomingBlock = PN->getIncomingBlock(U);
       Value *NewIncomingVal = getFreelyInvertedImpl(
           U.get(), /*WillInvertAllUses=*/false,
           /*Builder=*/nullptr, DoesConsume, MaxAnalysisRecursionDepth - 1);
-      if (NewIncomingVal == nullptr)
+      if (NewIncomingVal == nullptr) {
+          DoesConsume = SaveDoesConsume;
         return nullptr;
+      }
       // Make sure that we can safely erase the original PHI node.
-      if (NewIncomingVal == V)
+      if (NewIncomingVal == V) {
+          DoesConsume = SaveDoesConsume;
         return nullptr;
+      }
       if (Builder != nullptr)
         IncomingValues.emplace_back(NewIncomingVal, IncomingBlock);
     }

```

https://github.com/llvm/llvm-project/pull/82973