[PATCH] D81634: Remove assert from ShuffleVectorInst

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Was the failure only a fuzzer problem?
Ideally, callers shouldn't be wasting their time on analysis of degenerate code, so that assert might have actually been useful for real code.
Also, if this did happen in real code, we probably don't want to claim that this shuffle uses a source value if the shuffle doesn't actually exist (see inline comment).



================
Comment at: llvm/lib/IR/Instructions.cpp:2056
   }
-  assert((UsesLHS ^ UsesRHS) && "Should have selected from exactly 1 source");
   return true;
----------------
I think it would be better to change to:
  // Allow for degenerate case: completely undef mask means neither source is used.
  return UsesLHS || UsesRHS;


================
Comment at: llvm/test/CodeGen/X86/cgp_shuffle_crash.ll:1
+; RUN: llc %s -o - | FileCheck %s
+
----------------
I prefer a more direct test of CGP using "opt -codegenprepare -S".
That test should live in this directory:
llvm/test/Transforms/CodeGenPrepare/X86

Also, the test can be reduced to a single block (not sure if this is minimal):


```
define <2 x i8> @autogen_SD20565(i32 %x) {
  %Shuf = shufflevector <2 x i8> zeroinitializer, <2 x i8> zeroinitializer, <2 x i32> undef
  %Cmp = icmp slt i32 480483, %x
  %B = mul <2 x i8> %Shuf, %Shuf
  %S = select i1 %Cmp, <2 x i8> %B, <2 x i8> zeroinitializer
  ret <2 x i8> %Shuf
}

```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81634/new/

https://reviews.llvm.org/D81634