Page MenuHomePhabricator

[X86] Add SchedWrites for CMOV and SETCC. Use them to remove InstRWs.

Authored by craig.topper on Apr 6 2018, 11:14 AM.



Cmov and setcc previously used WriteALU, but on Intel processors at least they are more restricted than basic ALU ops.

This patch adds new SchedWrites for them and removes the InstRWs. I had to leave some InstRWs for CMOVA/CMOVBE and SETA/SETBE because those have an extra uop relative to the other condition codes on Intel CPUs.

Diff Detail


Event Timeline

craig.topper created this revision.Apr 6 2018, 11:14 AM
RKSimon added inline comments.Apr 6 2018, 12:50 PM
145 ↗(On Diff #141384)

Move the WriteSETCCStore def into the models so we don't get the regression on btver2 tests?

craig.topper added inline comments.Apr 6 2018, 1:50 PM
145 ↗(On Diff #141384)

You mean make it a regular SchedWrite and not a WriteSequence?

RKSimon added inline comments.Apr 7 2018, 6:25 AM
145 ↗(On Diff #141384)

If you can, otherwise please can you add a InstRW like znver1 has to btver2?

Use SchedWrite instead of SchedWriteSequence. Avoid all test changes by matching previous behavior for SLM and BTVER2. Add a few FIXMEs.

RKSimon accepted this revision.Apr 8 2018, 2:40 AM
RKSimon added a reviewer: GGanesh.
RKSimon added a subscriber: GGanesh.

Thanks - LGTM as an NFC - I guess the ryzen fixme can stay for now if @GGanesh doesn't get back in time.

620 ↗(On Diff #141502)

@craig.topper You can probably fold this into the WriteSETCC declarations above and move the FIXME question there.

@GGanesh shouldn't this have a ZnAGU dependency?

This revision is now accepted and ready to land.Apr 8 2018, 2:40 AM
GGanesh added inline comments.Apr 8 2018, 3:07 AM
620 ↗(On Diff #141502)

Yes all memory instructions have dependency with address generation unit!

This revision was automatically updated to reflect the committed changes.