From 95d8a50d20ca270e65bb1641b03b6e44b87bcc56 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 12 Jun 2025 13:44:17 -0700 Subject: [PATCH 1/2] [instcombine] Delete dead transform for rev(binop splat, rev(x)) We canonicalize splats to RHS, so this transform should be dead code. --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index c169ab25b2106..d5738520715b0 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3565,12 +3565,6 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) { OldBinOp, OldBinOp->getName(), II->getIterator())); } - // rev(binop BO0Splat, rev(Y)) --> binop BO0Splat, Y - if (match(BO1, m_VecReverse(m_Value(Y))) && isSplatValue(BO0)) - return replaceInstUsesWith(CI, - BinaryOperator::CreateWithCopiedFlags( - OldBinOp->getOpcode(), BO0, Y, OldBinOp, - OldBinOp->getName(), II->getIterator())); } // rev(unop rev(X)) --> unop X if (match(Vec, m_OneUse(m_UnOp(m_VecReverse(m_Value(X)))))) { From f605b39011e117900d72a8009cbb6317e3191c33 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Mon, 16 Jun 2025 11:12:08 -0700 Subject: [PATCH 2/2] Remove entire binop case --- .../InstCombine/InstCombineCalls.cpp | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index e7fba990f544f..03897117861f6 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3555,26 +3555,13 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) { break; } case Intrinsic::vector_reverse: { - Value *BO0, *BO1, *X, *Y; Value *Vec = II->getArgOperand(0); - if (match(Vec, m_OneUse(m_BinOp(m_Value(BO0), m_Value(BO1))))) { - auto *OldBinOp = cast(Vec); - if (match(BO0, m_VecReverse(m_Value(X)))) { - // rev(binop rev(X), rev(Y)) --> binop X, Y - if (match(BO1, m_VecReverse(m_Value(Y)))) - return replaceInstUsesWith(CI, BinaryOperator::CreateWithCopiedFlags( - OldBinOp->getOpcode(), X, Y, - OldBinOp, OldBinOp->getName(), - II->getIterator())); - // rev(binop rev(X), BO1Splat) --> binop X, BO1Splat - if (isSplatValue(BO1)) - return replaceInstUsesWith(CI, BinaryOperator::CreateWithCopiedFlags( - OldBinOp->getOpcode(), X, BO1, - OldBinOp, OldBinOp->getName(), - II->getIterator())); - } - } + // Note: We canonicalize reverse after binops, so we don't need a + // corresponding binop case here. TODO: Consider canonicalizing + // reverse after fneg? + // rev(unop rev(X)) --> unop X + Value *X; if (match(Vec, m_OneUse(m_UnOp(m_VecReverse(m_Value(X)))))) { auto *OldUnOp = cast(Vec); auto *NewUnOp = UnaryOperator::CreateWithCopiedFlags(