[PATCH] D52256: AArch64: Add FuseCryptoApple fusion rules

[Evandro Menezes via Phabricator via llvm-commits]

evandro added inline comments.


================
Comment at: lib/Target/AArch64/AArch64.td:159
 
+def FeatureFuseCryptoApple : SubtargetFeature<
+    "fuse-crypto", "HasFuseCryptoApple", "true",
----------------
javed.absar wrote:
> 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.
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`.


================
Comment at: lib/Target/AArch64/AArch64.td:350
+                                   FeatureFuseCryptoApple,
                                    FeatureNEON,
                                    FeaturePerfMon,
----------------
Thereby retaining `FeatureFuseAES` here and adding the new feature.


================
Comment at: lib/Target/AArch64/AArch64MacroFusion.cpp:128
+static bool isCryptoPair(unsigned FirstOpcode, unsigned SecondOpcode) {
+  switch (SecondOpcode) {
+  case AArch64::AESMCrr:
----------------
The first two cases repeat `isAESPair()`, so methinks that it'd be better if it'd check only the new pair, `PMULL` AND `EOR`.


Repository:
  rL LLVM

https://reviews.llvm.org/D52256