This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't emit KTEST instructions unless only the Z flag is being used
ClosedPublic

Authored by craig.topper on Jan 31 2018, 4:37 PM.

Details

Summary

KTEST has weird flag behavior. The Z flag is set for all bits in the AND of the k-registers being 0, and the C flag is set for all bits being 1. All other flags are cleared.

We currently emit this instruction in EmitTEST and don't check the condition code. This can lead to strange things like using the S flag after a KTEST for a signed compare.

The domain reassignment pass can also transform TEST instructions into KTEST and is not protected against the flag usage either. For now I've disabled this part of the domain reassignment pass. I tried to comment out the checks in the mir test so that we could recover them later, but I couldn't figure out how to get that to work.

This patch moves the KTEST handling into LowerSETCC and now creates a ktest+x86setcc. I've chosen this approach because I'd like to add support for the C flag for all ones in a followup patch. To do that requires that I can rewrite the condition code going in the x86setcc to be different than the original SETCC condition code.

This fixes PR36182. I'll file a PR to fix domain reassignment once this goes in. Should this be merged to 6.0?

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Jan 31 2018, 4:37 PM

Simplify some code by checking VT against MVT enum values directly instead of using isVector, getVectorElementType and getSizeInBits.

zvi added inline comments.Feb 6 2018, 11:03 AM
lib/Target/X86/X86ISelLowering.cpp
18133 ↗(On Diff #132729)

Which tests cover this?

craig.topper added inline comments.Feb 6 2018, 2:07 PM
lib/Target/X86/X86ISelLowering.cpp
18133 ↗(On Diff #132729)

The tests that switched from ktest to kortest in D42772

maybe use the update_mir_test_checks.py script for the mir test?

Re-generate test with update_mir_test_checks.py. Leave the TEST instructions in the MIR test but commented out with a FIXME

guyblank accepted this revision.Feb 7 2018, 10:44 PM
This revision is now accepted and ready to land.Feb 7 2018, 10:44 PM
This revision was automatically updated to reflect the committed changes.