[PATCH] D28152: Cortex-A57 scheduling model for ARM backend (AArch32)

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 08:56:02 PST 2017


rovka added a comment.

Hi,

This looks like a lot of work :) Could you please split it up into smaller patches, e.g. to separate the mechanical scheduling definitions from the more structural changes?

Thanks,
Diana



================
Comment at: lib/Target/ARM/ARMScheduleA57.td:17
+// Common description and scheduling model parameters taken from AArch64.
+// The Cortex-A57 is a traditional superscaler microprocessor with a
+// conservative 3-wide in-order stage for decode and dispatch. Combined with the
----------------
Typo: superscalar


================
Comment at: lib/Target/ARM/ARMScheduleA57.td:29
+
+// Cortex A57 rev. r1p0 or later
+def IsR1P0AndLaterPred : SchedPredicate<[{!STI->isA57r0px()}]>;
----------------
Could you break this off into a separate patch? I.e. write an initial patch as if everything were r1p0 or later, and another patch with the r0px delta (or the other way around if it's more convenient for you).


================
Comment at: lib/Target/ARM/ARMScheduleA57.td:86
+  // Enable partial & runtime unrolling. The magic number is chosen based on
+  // experiments and benchmarking data.
+  let LoopMicroOpBufferSize = 16;
----------------
Cool, can you share this data?


================
Comment at: lib/Target/ARM/ARMScheduleA57.td:102
+
+// TODO: In AArch64 FDIV/FSQRT is declared to use X-unit (F0 in doc)
+// but cryptography extensions are declared to use W-unit (the same F0 in doc)
----------------
Isn't this fixed now?


================
Comment at: lib/Target/ARM/ARMSubtarget.h:439
   bool isCortexA15() const { return ARMProcFamily == CortexA15; }
+  bool isCortexA57() const { return ARMProcFamily == CortexA57; }
   bool isSwift()    const { return ARMProcFamily == Swift; }
----------------
Please don't add this function if it can be avoided. We're trying to get rid of these in the long run (in favor of using subtarget features for each specific behavior).


https://reviews.llvm.org/D28152





More information about the llvm-commits mailing list