Page MenuHomePhabricator

[InstCombine] replace shuffle's insertelement operand if inserted scalar is not demanded
ClosedPublic

Authored by spatel on Dec 9 2019, 11:40 AM.

Details

Summary

This pattern is noted as a regression from:
D70246
...where we removed an over-aggressive shuffle simplification.

SimplifyDemandedVectorElts fails to catch this case when the insert has multiple uses, so I'm proposing to pattern match the minimal sequence directly. This fold does not conflict with any of our current shuffle undef/poison semantics AFAICT.

Diff Detail

Event Timeline

spatel created this revision.Dec 9 2019, 11:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2019, 11:40 AM
lebedev.ri accepted this revision.Dec 9 2019, 12:39 PM
lebedev.ri added a subscriber: lebedev.ri.

This is legal.

----------------------------------------
define <4 x float> @insert_undemanded_element_op0(<4 x float> %x, <4 x float> %y) {
%0:
  %ins = insertelement <4 x float> %x, float 42.000000, i32 3
  %s = shufflevector <4 x float> %ins, <4 x float> %y, <4 x i32> { 0, 7, 1, 4 }
  ret <4 x float> %s
}
=>
define <4 x float> @insert_undemanded_element_op0(<4 x float> %x, <4 x float> %y) {
%0:
  %ins = insertelement <4 x float> %x, float 42.000000, i32 3
  %s = shufflevector <4 x float> %x, <4 x float> %y, <4 x i32> { 0, 7, 1, 4 }
  ret <4 x float> %s
}
Transformation seems to be correct!

Summary:
  1 correct transformations
  0 incorrect transformations
  0 errors
This revision is now accepted and ready to land.Dec 9 2019, 12:39 PM
foad accepted this revision.Dec 10 2019, 2:01 AM

I can confirm that this fixes the regression I noted. Thanks!

This revision was automatically updated to reflect the committed changes.