InstructionSimplify: Simplify a shuffle with a undef mask to undef

[zvi]

Summary

Following the discussion in pr32486, adding the simplification:
shuffle %x, %y, undef -> undef

[davide]

lib/Analysis/InstructionSimplify.cpp

4147–4149 ↗

(On Diff #95955)

microopt: you can move this up to the beginning of the function.

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

We have a similar case here, maybe it makes sense to handle them together?

[jroelofs]

We are not allowed to fold (select undef x y) -> undef, but we can fold (select undef x y) to x or to y. Likewise, I think this transformation needs to be (shufflevector x y undef) -> x.

[jroelofs]

In D32293#732162, @jroelofs wrote:

We are not allowed to fold (select undef x y) -> undef, but we can fold (select undef x y) to x or to y. Likewise, I think this transformation needs to be (shufflevector x y undef) -> x.

Wait, never mind, LangRef says: "If the shuffle mask is undef, the result vector is undef"

[zvi]

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

Sorry i don't follow you. What do you mean by handling them together?
This case you are pointing out is handled by a more generic combine starting from DAGCombine.cpp:4154

[davide]

Uhm, I don't follow you. Maybe DAGCombiner has a similar transformation, but instsimplify is powerful enough to handle the case I pointed out.

[davide@cupiditate bin]$ ./opt -instsimplify tinkywinky.ll -S
; ModuleID = 'tinkywinky.ll'
source_filename = "tinkywinky.ll"

define <4 x i32> @undef_mask(<4 x i32> %x) {
  ret <4 x i32> undef
}
[davide@cupiditate bin]$ cat tinkywinky.ll
define <4 x i32> @undef_mask(<4 x i32> %x) {
  %shuf = shufflevector <4 x i32> %x, <4 x i32> undef, <4 x i32> undef
  ret <4 x i32> %shuf
}

[zvi]

In D32293#732239, @davide wrote:

Uhm, I don't follow you. Maybe DAGCombiner has a similar transformation, but instsimplify is powerful enough to handle the case I pointed out.

Sorry, what i really meant was InstructionSimplify.cpp:4154

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

I actually meant InstructionSimplify.cpp:4154 :)

[zvi]

Moving added code to top of function

[davide]

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

I was wondering how hard it would be to extend that combine to handle also this case.

[zvi]

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

I plan in a future patch to do a refactoring/NFC-ish canonicalization of the shuffle input vectors and the mask. I believe that will make the special handling of the "single constant vector" case unnecessary, because the unreferenced variable vector will be canonicalized to an undef. Similarly, it is possible that canonicalization will cover the case of an undef mask, because it will replace both vector operands to undefs and then trivial constant-folding will handle that case.

[davide]

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

I'd appreciate if you can propose a refactoring before committing this patch (if possible)

[zvi]

test/Transforms/InstSimplify/shufflevector.ll

113–119 ↗

(On Diff #95955)

D32338 adds a couple of canonicalization rules which are sufficient for what we cover so far.

[zvi]

ping

[spatel]

This patch is trivially LGTM based on the LangRef and discussion in PR32486:
https://bugs.llvm.org//show_bug.cgi?id=32486

[zvi]

@davide, do you have any further comments?

[davide]

No further comments from me.

[davide]

Note, I don't qualify to LGTM as I'm not particularly familiar with vector code in LLVM.

[zvi]

Rebase on top of trunk

llvm/trunk/lib/Analysis/InstructionSimplify.cpp

if (RootElt != DestElt)
     return nullptr;
 
   return RootVec;
 }
 
 static Value *SimplifyShuffleVectorInst(Value *Op0, Value *Op1, Constant *Mask,
                                         Type *RetTy, const SimplifyQuery &Q,
                                         unsigned MaxRecurse) {
+  if (isa<UndefValue>(Mask))
+    return UndefValue::get(RetTy);
+
   Type *InVecTy = Op0->getType();
   unsigned MaskNumElts = Mask->getType()->getVectorNumElements();
   unsigned InVecNumElts = InVecTy->getVectorNumElements();
 
   auto *Op0Const = dyn_cast<Constant>(Op0);
   auto *Op1Const = dyn_cast<Constant>(Op1);
 
   // If all operands are constant, constant fold the shuffle.

llvm/trunk/test/Transforms/InstSimplify/shufflevector.ll

 define <4 x i32> @undef_mask(<4 x i32> %x) {
 ; CHECK-LABEL: @undef_mask(
 ; CHECK-NEXT:    ret <4 x i32> undef
 ;
   %shuf = shufflevector <4 x i32> %x, <4 x i32> undef, <4 x i32> undef
   ret <4 x i32> %shuf
 }
 
+define <4 x i32> @undef_mask_1(<4 x i32> %x, <4 x i32> %y) {
+; CHECK-LABEL: @undef_mask_1(
+; CHECK-NEXT:    ret <4 x i32> undef
+;
+  %shuf = shufflevector <4 x i32> %x, <4 x i32> %y, <4 x i32> undef
+  ret <4 x i32> %shuf
+}
+
 define <4 x i32> @identity_mask_0(<4 x i32> %x) {
 ; CHECK-LABEL: @identity_mask_0(
 ; CHECK-NEXT:    ret <4 x i32> [[X:%.*]]
 ;
   %shuf = shufflevector <4 x i32> %x, <4 x i32> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   ret <4 x i32> %shuf
 }