This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't select (cmp (and, imm), 0) to testw
ClosedPublic

Authored by craig.topper on Sep 25 2017, 9:59 PM.

Details

Summary

X86ISelDAGToDAG tries to analyze ANDs compared with 0 to optimize to narrower immediates using subregisters.

I don't think we should be optimizing to 16-bit test instructions. It goes against our normal behavior of promoting i16 operations to i32. It only saves one byte due to the need to add a 0x66 prefix. I think it would also be subject to a length changing prefix penalty in the decoders on Intel CPUs.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Sep 25 2017, 9:59 PM
spatel edited edge metadata.Sep 27 2017, 9:03 AM

I agree that this seems wrong for the general case. What about with -minsize though?

If we're going to remove the block entirely, I'd prefer to leave a comment in its place to explain why we have the hole between 8-bit and 32-bit.

Do we have other examples of prefering i16 instructions in minsize?

Do we have other examples of prefering i16 instructions in minsize?

Found this in X86TargetLowering::EmitCmp():

// Only promote the compare up to I32 if it is a 16 bit operation
// with an immediate.  16 bit immediates are to be avoided.
if ((Op0.getValueType() == MVT::i16 &&
     (isa<ConstantSDNode>(Op0) || isa<ConstantSDNode>(Op1))) &&
    !DAG.getMachineFunction().getFunction()->optForMinSize() &&

Qualified with optforminsize instead.

spatel accepted this revision.Sep 28 2017, 3:40 PM

LGTM.

This revision is now accepted and ready to land.Sep 28 2017, 3:40 PM
This revision was automatically updated to reflect the committed changes.