This is an archive of the discontinued LLVM Phabricator instance.

[X86] Teach combineVectorShiftImm to constant fold undef elements to 0 not undef.
ClosedPublic

Authored by craig.topper on Jun 4 2020, 7:42 PM.

Details

Summary

Shifts are supposed to always shift in zeros or sign bits regardless of their inputs. It's possible the input value may have been replaced with undef by SimplifyDemandedBits, but the shift in zeros are still demanded.

This issue was reported to me by ispc from 10.0. Unfortunately their failing test does not fail on trunk. Seems to be because the shl is optimized out earlier now and doesn't become VSHLI.

ispc bug https://github.com/ispc/ispc/issues/1771

Diff Detail

Event Timeline

craig.topper created this revision.Jun 4 2020, 7:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2020, 7:42 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
craig.topper edited the summary of this revision. (Show Details)Jun 4 2020, 7:45 PM
craig.topper added subscribers: dbabokin, Treebeard.
craig.topper edited the summary of this revision. (Show Details)Jun 4 2020, 7:47 PM
spatel accepted this revision.Jun 5 2020, 9:37 AM

LGTM - I double-checked that generic constant folding in IR and SDAG gets this right:

// shift undef, Y --> 0 (can always assume that the undef value is 0)
This revision is now accepted and ready to land.Jun 5 2020, 9:37 AM
RKSimon accepted this revision.Jun 5 2020, 10:16 AM

Possibly add a PSRAI test as well? Its more irregular than the logical shifts as it isn't necessarily zeros...

I think there may be a second bug. Noticed my fix only affected the 2i64 32-bit mode cases in vec_shift5.ll. Those are being handled by combineVectorShiftImm. There's still something happening in intrinsic lowering.

This revision was automatically updated to reflect the committed changes.