InstSimplify: strip bad TODO (NFC)

[artagnon]

foldIdentityShuffles requires two sets of canceling shuffles. If there are any intervening instructions, they are feeding in the result of the first set of shuffles. To eliminate the two sets of shuffles, you'd have to rewrite the head of the intervening instructions to feed in the operand of the first set of shuffles. Since modifying the IR in any way is disallowed by an analysis, strip this bad TODO.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

foldIdentityShuffles() has a TODO comment about looking through bitcasts. Looking through any intervening instructions when folding shuffles is out-of-scope of InstructionSimplify, as the analysis is not allowed to modify the IR in any way. Strip this bad TODO.

---

Full diff: https://github.com/llvm/llvm-project/pull/92754.diff

1 Files Affected:

• (modified) llvm/lib/Analysis/InstructionSimplify.cpp (-3)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 37a7259a5cd02..e88736b5fab8d 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -5346,9 +5346,6 @@ static Value *foldIdentityShuffles(int DestElt, Value *Op0, Value *Op1,
         SourceShuf->getMaskValue(RootElt), RootVec, MaxRecurse);
   }
 
-  // TODO: Look through bitcasts? What if the bitcast changes the vector element
-  // size?
-
   // The source operand is not a shuffle. Initialize the root vector value for
   // this shuffle if that has not been done yet.
   if (!RootVec)

[arsenm]

foldIdentityShuffles() has a TODO comment about looking through bitcasts. Looking through any intervening instructions when folding shuffles is out-of-scope of InstructionSimplify, as the analysis is not allowed to modify the IR in any way. Strip this bad TODO.

You can look through intervening instructions, you just can't introduce new instructions. You can still probably manage a restricted subset of what you were trying to do but I didn't look that closely

[artagnon]

Think about it. foldIdentityShuffles() requires two sets of canceling shuffles. If there are any intervening instructions, they are feeding in the result of the first set of shuffles. To eliminate the two sets of shuffles, you'd have to rewrite the head of the intervening instructions to feed in the operand of the first set of shuffles.

[nikic]

@artagnon I think the point of this TODO is something like shuffle(bitcast(shuffle()) where the whole sequence cancels out, e.g. because the inner shuffle is 3,2,1,0 and the outer is 6,7,4,5,2,3,0,1 on elements half as wide. There is no need for any IR rewrites in that case.

I'm fine with dropping it though, this kind of TODO often does more damage than good -- the worst PRs we get are from inexperienced contributors trying to resolve TODOs.