[PATCH] D142502: [AArch64] Add A+B+1 and A-B-1 macro fusion for Ampere1A

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you add some tests. Possibly with a mir test?



================
Comment at: llvm/lib/Target/AArch64/AArch64.td:293
+def FeatureFuseAddSub2RegAndConstOne : SubtargetFeature<
+   "fuse-addsub-2reg-const1", "HasFuseeAddSub2RegAndConstOne", "true",
+   "CPU fuses (a + b + 1) and (a - b - 1)">;
----------------
HasFuseeAddSub2RegAndConstOne -> HasFuseAddSub2RegAndConstOne


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:1313
   list<SubtargetFeature> Ampere1 = [HasV8_6aOps, FeatureNEON, FeaturePerfMon,
-                                    FeatureSSBS, FeatureRandGen, FeatureSB,
-                                    FeatureSHA2, FeatureSHA3, FeatureAES];
+                                    FeatureSSBS, FeatureRandGen, FeatureSHA2,
+                                    FeatureSHA3, FeatureAES];
----------------
These look unrelated to the macro fusion.


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.cpp:382
 
+// "A + B + 1" or "A - B - 1"
+static bool isAddSub2RegAndConstOnePair(const MachineInstr *FirstMI,
----------------
It may be useful to add brackets for clarity, if it works this way: "(A + B) + 1" or "(A - B) - 1"
Do you know if the first is commutative?


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.cpp:385
+                                        const MachineInstr &SecondMI) {
+  int polarity = 1;
+
----------------
Capitalize Polarity


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142502/new/

https://reviews.llvm.org/D142502