This is an archive of the discontinued LLVM Phabricator instance.

X86DAGToDAGISel::matchBitExtract(): prepare 'control' in 32 bits
ClosedPublic

Authored by lebedev.ri on Jan 15 2019, 4:53 AM.

Details

Summary

Noticed while looking at D56052.

// The 'control' of BEXTR has the pattern of:
// [15...8 bit][ 7...0 bit] location
// [ bit count][     shift] name
// I.e. 0b000000011'00000001 means  (x >> 0b1) & 0b11

I.e. we do not care about any of the bits aside from the low 16 bits.
So there is no point in doing the slh,or in 64 bits, let's just do everything in 32 bits, and anyext if needed.

We could do that in 16 even, but we intentionally don't zext to i16 (longer encoding IIRC),
so i'm guessing the same applies here.

Diff Detail

Repository
rL LLVM

Event Timeline

Rebased, NFC.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2019, 1:20 AM

How come SimplifyDemandedBits isn't doing this for us?

How come SimplifyDemandedBits isn't doing this for us?

Note that we are in X86ISelDAGToDAG. I don't observe anything SimplifyDemandedBits-related here.
There is SimplifyDemandedBits stuff in X86TargetLowering, but is that not run before we end up here?
Or am i missing some path how SimplifyDemandedBits will be run after X86DAGToDAGISel::matchBitExtract()?
If i am, i'd love to learn about it, and use it to solve this.

craig.topper accepted this revision.Feb 4 2019, 10:36 AM

I don't think SimplifyDemandedBits can do anything for this. We're taking an i8 shift value and turning it into a control. The issue is entirely within how this code promoted that i8 value to match the width of the BEXTR/BZHI.

LGTM

This revision is now accepted and ready to land.Feb 4 2019, 10:36 AM

I don't think SimplifyDemandedBits can do anything for this. We're taking an i8 shift value and turning it into a control. The issue is entirely within how this code promoted that i8 value to match the width of the BEXTR/BZHI.

LGTM

Thank you for the review.

This revision was automatically updated to reflect the committed changes.