[PATCH] D52256: AArch64: Add FuseCryptoEOR fusion rules

Wed Sep 19 13:10:36 PDT 2018

MatzeB marked 4 inline comments as done.
MatzeB added inline comments.

================
Comment at: lib/Target/AArch64/AArch64.td:159

+def FeatureFuseCryptoApple : SubtargetFeature<
+    "fuse-crypto", "HasFuseCryptoApple", "true",
----------------
evandro wrote:
> 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`.
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.

================
Comment at: lib/Target/AArch64/AArch64.td:350
+                                   FeatureFuseCryptoApple,
                                    FeatureNEON,
                                    FeaturePerfMon,
----------------
evandro wrote:
> Thereby retaining `FeatureFuseAES` here and adding the new feature.
ok, reused FeatureFuseAES this time and called the new thing FuseCryptoEOR.

Repository:
  rL LLVM

https://reviews.llvm.org/D52256