Page MenuHomePhabricator

[X86] Add sched class WriteLAHFSAHF and fix values.
ClosedPublic

Authored by courbet on Jun 12 2018, 8:28 AM.

Details

Summary

I ran llvm-exegesis on SKX, SKL, BDW, HSW, SNB.
Atom is from Agner and SLM is a guess.
I've left AMD processors alone.

Diff Detail

Repository
rL LLVM

Event Timeline

courbet created this revision.Jun 12 2018, 8:28 AM

Please can you LAHF/SAHF tests to the resources-x86_64.s llvm-mca test files

@craig.topper Any comments? You added the TODOs to the models IIRC.

courbet updated this revision to Diff 151859.Jun 19 2018, 12:52 AM

Add mca tests.

RKSimon added inline comments.Jun 19 2018, 8:37 AM
test/tools/llvm-mca/X86/BtVer2/partial-reg-update-2.s
1 ↗(On Diff #151859)

revert this? @gbedwell any idea why this got inserted?

gbedwell added inline comments.Jun 19 2018, 9:03 AM
test/tools/llvm-mca/X86/BtVer2/partial-reg-update-2.s
1 ↗(On Diff #151859)

Merely because I regenerated all tests using a wildcard. It's regenerated the checks which happen to be identical to the previous manually inserted checks. I can revert, but we should probably re-add this line in a separate commit just to reduce noise from wildcard regenerating tests.

RKSimon added inline comments.Jun 19 2018, 9:16 AM
test/tools/llvm-mca/X86/BtVer2/partial-reg-update-2.s
1 ↗(On Diff #151859)

Something I didn't realise - Andrea's commit yesterday already added this line - @courbet if you rebase then this diff should just go away.....

gbedwell added inline comments.Jun 19 2018, 9:32 AM
test/tools/llvm-mca/X86/BtVer2/partial-reg-update-2.s
1 ↗(On Diff #151859)

Aha. I thought I was replying to a different review (hence why I wrote "I regenerated") as I have exactly the same diff in D48276 :)

This change seems fine to me.

RKSimon accepted this revision.Jun 19 2018, 10:53 AM

LGTM - after ensuring that the partial-reg-update-2.s diff is gone - thanks everyone.

This revision is now accepted and ready to land.Jun 19 2018, 10:53 AM
courbet marked an inline comment as done.Jun 19 2018, 11:16 PM

Thanks !

This revision was automatically updated to reflect the committed changes.