[PATCH] D55345: [AArch64] Refactor the scheduling predicates

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 6 09:42:36 PST 2018


andreadb added a comment.

In D55345#1321500 <https://reviews.llvm.org/D55345#1321500>, @evandro wrote:

> Rebase the patch.


Unfortunally, I am still seeing several tablegen errors.

Tablegen complains about missing definitions. It looks like most (if not all) of those missing definitions will be added by D55375 <https://reviews.llvm.org/D55375>.
I reported only a few missing symbols. Is this patch meant to be dependent on D55375 <https://reviews.llvm.org/D55375>? If not, then this patch should be fixed. If it is going to be complicated, then I don't mind if you merge the two patches into a single diff. As long as I can build it, and check that the generated code is still semantically equivalent to what there was before, then I am happy.

Although this patch is supposed to be an NFC, I'd like to see at least one llvm-mca test. Ideally, a test that shows how we can now analyze instructions that llvm-mca was previously unable to analyze.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:940-941
-
-bool AArch64InstrInfo::isExynosShiftFast(const MachineInstr &MI,
-                                         unsigned Extra) {
-  unsigned Imm, Shift;
----------------
For the record: this method never existed in the original code. Not sure how it ended up in this diff.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredExynos.td:25
+                        [MCOpcodeSwitchCase<
+                           IsArithExt32Op.ValidOpcodes,
+                           MCReturnStatement<
----------------
IsArithExt32Op is not defined.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredExynos.td:27
+                           MCReturnStatement<
+                             CheckAny<[CheckExtBy0,
+                                       CheckAll<
----------------
CheckExtBy0 is not defined.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredExynos.td:35
+                         MCOpcodeSwitchCase<
+                           IsArithExt64Op.ValidOpcodes,
+                           MCReturnStatement<
----------------
IsArithExt64Op is not defined.


================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredExynos.td:48
+// Identify FP instructions.
+def ExynosFPPred : MCSchedPredicate<CheckAny<[CheckDForm, CheckQForm]>>;
+
----------------
These two definitions are missing. I saw you added those in D55375.


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

https://reviews.llvm.org/D55345





More information about the llvm-commits mailing list