Page MenuHomePhabricator

[DAGCombiner] Remove extra bitcasts surrounding vector shuffles
ClosedPublic

Authored by RKSimon on Apr 19 2015, 7:37 AM.

Details

Summary

Patch to remove extra bitcasts from shuffles, this is often a legacy of XformToShuffleWithZero being used to combine bitmaskings (of float vectors bitcast to integer vectors) into shuffles:

bitcast(shuffle(bitcast(s0),bitcast(s1))) -> shuffle(s0,s1)

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon updated this revision to Diff 23992.Apr 19 2015, 7:37 AM
RKSimon retitled this revision from to [DAGCombiner] Remove extra bitcasts surrounding vector shuffles .
RKSimon updated this object.
RKSimon edited the test plan for this revision. (Show Details)
RKSimon added reviewers: qcolombet, andreadb, spatel.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: Unknown Object (MLST).
qcolombet edited edge metadata.Apr 21 2015, 11:23 AM

Hi Simon,

This looks mostly good, just one question.
Are the tests you added enough to cover all the code you add?
In particular, it is not obvious to me that we cover the case where the bitcast operand of the shuffle are undef, integer constant, or fp constant.
If this is indeed not covered, please add a test case.

Thanks,
-Quentin

test/CodeGen/X86/combine-or.ll
290 ↗(On Diff #23992)

Please use instnamer to have easier to update tests.

RKSimon updated this revision to Diff 24228.Apr 22 2015, 8:08 AM
RKSimon edited edge metadata.

Thanks Quentin,

I've added an extra test for peeking through the bitcasted masked constant vectors. The UNDEF implementation was never getting hit so I dropped that for simplicity (visitVECTOR_SHUFFLE has support for cases where the second operand is UNDEF already).

qcolombet accepted this revision.Apr 22 2015, 9:35 AM
qcolombet edited edge metadata.

Thanks Simon.

LGTM.

Cheers,
Q.

This revision is now accepted and ready to land.Apr 22 2015, 9:35 AM
This revision was automatically updated to reflect the committed changes.