[PATCH] D26641: [InstCombine] change bitwise logic type to eliminate bitcasts

[Sanjay Patel via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D26641#602029, @efriedma wrote:

> I'm a little skeptical this is a good idea in all cases... it would be terrible to transform a vector xor into an i128 xor (and therefore force the operation into integer registers) just because SROA happened to produce an i128, or some calling convention has weird requirements.  The transform looks good for your testcase in PR27925 in particular because "xor <2 x i64>" and "xor <4 x i32>" lower to the same instruction on x86.


I see 3 possible ways to solve/work-around this:

1. Don't do this transform in InstCombine (but then we're missing a potential canonicalization).
2. Limit this to vector-vector transforms in InstCombine (but then we're still missing a potential canonicalization for some types).
3. Fix it up in DAGCombine by using TLI to transform away from illegal operations.

I think #3 is the answer, but please let me know if I've missed any possibilities.



================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1796
+  if (match(BO->getOperand(0), m_OneUse(m_BitCast(m_Value(X)))) &&
+      X->getType() == DestTy && !isa<Constant>(X)) {
+    // bitcast(logic(bitcast(X), Y)) --> logic'(X, bitcast(Y))
----------------
efriedma wrote:
> Don't you need some sort of check that DestTy is an integer type?
Oops - yes, that was an oversight.


https://reviews.llvm.org/D26641