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

Yingwei Zheng via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 27 03:31:32 PST 2024


================
@@ -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,
                                          DoesConsume, Depth))
       return Builder ? Builder->CreateXor(AV, B) : NonNull;
----------------
dtcxzyw wrote:

> Still trying to properly understand why this is the fix, but something to note: below at line:2377 seems buggy.
> 
> ```
>    if (NewIncomingVal == V)
>         return nullptr;
> ```
> 
> `NewIncomingVal` uses `builder` as `nullptr` so if the return is non-null, it will return `NonNull` which will never equal `V`

I believe it never happens since `NewIncomingVal` always evaluates to nullptr if `U.get()` is also a phi node.


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


More information about the llvm-commits mailing list