This is an archive of the discontinued LLVM Phabricator instance.

[X86] In lowerVectorShuffleAsBroadcast, make peeking through CONCAT_VECTORS work correctly if we already walked through a bitcast that changed the element size.
ClosedPublic

Authored by craig.topper on Oct 29 2018, 11:27 AM.

Details

Summary

The CONCAT_VECTORS case was using the original mask element count to determine how to adjust the broadcast index. But if we looked through a bitcast the original mask size doesn't tell us anything about the concat_vectors.

This patch switchs to using the concat_vectors input element count directly instead.

This caused a crash while doing experiments with -mprefer-vector-width=256 with skylake-avx512 on some benchmarks. All the types present in the crash were 256 or 128 bits wide so I'm not sure why -mprefer-vector-width=256 was relevant.

I don't currently have a reduced test case, but I'll see what I can do.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 29 2018, 11:27 AM

Here's a test case that crashes with -mattr=avx2

define <8 x float> @foo(<4 x float> %x, <4 x float> %y, float %z) {
entry:
  %tmp = shufflevector <4 x float> %x, <4 x float> %y, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  %bc = bitcast <8 x float> %tmp to <4 x i64>
  %tmp1 = extractelement <4 x i64> %bc, i32 3
  %tmp2 = bitcast i64 %tmp1 to <2 x float>
  %tmp4 = extractelement <2 x float> %tmp2, i32 1
  %tmp5 = insertelement <8 x float> undef, float %tmp4, i32 4
  %tmp6 = insertelement <8 x float> %tmp5, float %z, i32 5
  ret <8 x float> %tmp6
}
RKSimon accepted this revision.Oct 30 2018, 2:09 AM

LGTM - please can you add that test to the vector-shuffle-256-v8.ll file?

This revision is now accepted and ready to land.Oct 30 2018, 2:09 AM
This revision was automatically updated to reflect the committed changes.