Page MenuHomePhabricator

AArch64: Add FuseCryptoEOR fusion rules
ClosedPublic

Authored by MatzeB on Sep 18 2018, 6:29 PM.

Details

Summary

There's some additional macro fusion opportunities on newer apple CPUs.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB created this revision.Sep 18 2018, 6:29 PM
javed.absar added inline comments.Sep 19 2018, 1:44 AM
lib/Target/AArch64/AArch64.td
159

Would it be better to give a more operational name (e.g. FuseAESCrypto?) along with a comment if it applies to only certain configs. Just a thought, I am ok with as it is as well.

evandro added inline comments.Sep 19 2018, 8:51 AM
lib/Target/AArch64/AArch64.td
159

IIUC, this new feature is an extension of FeatureFuseAES. Therefore, I think that it'd be better if it were just about the new fusion, namely of PMULL and EOR.

351

Thereby retaining FeatureFuseAES here and adding the new feature.

lib/Target/AArch64/AArch64MacroFusion.cpp
128

The first two cases repeat isAESPair(), so methinks that it'd be better if it'd check only the new pair, PMULL AND EOR.

MatzeB updated this revision to Diff 166173.Sep 19 2018, 1:07 PM
MatzeB retitled this revision from AArch64: Add FuseCryptoApple fusion rules to AArch64: Add FuseCryptoEOR fusion rules.

Address review feedback.

MatzeB marked 4 inline comments as done.Sep 19 2018, 1:09 PM
MatzeB added inline comments.
lib/Target/AArch64/AArch64.td
159

Yeah we already had FuseAES and I was just implementing another variation of it, hence my original try with "Apple" in the name. But indeed describing it more function like "CryptoEOR" turned out to look better.

351

ok, reused FeatureFuseAES this time and called the new thing FuseCryptoEOR.

evandro accepted this revision.Sep 19 2018, 1:11 PM

Me likes it.

This revision is now accepted and ready to land.Sep 19 2018, 1:11 PM
MatzeB closed this revision.Sep 19 2018, 1:56 PM
MatzeB marked 2 inline comments as done.

Landed in rL342590