[PATCH] D21685: [ARM] Do not test for CPUs, use SubtargetFeatures (Part 2). NFCI

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 24 15:08:38 PDT 2016


MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

Looks good from my side.

I share Eric's feelings of some functions looking strange. However we already do have a better thing: New-style scheduling models ("MCSchedule.h") instead of the itineraries ("MCInstrItineraries.h"). A lot of the information computed by those strange functions will be pulled from the scheduling model directly. However this would mean rewriting a bunch of scheduling definitions which is out of scope for this patch.


================
Comment at: lib/Target/ARM/ARMSubtarget.h:61
@@ +60,3 @@
+  /// What kind of timing do load multiple/store multiple instructions have.
+  enum ARMLdStMultipleTiming {
+    /// Can load/store 2 registers/cycle.
----------------
echristo wrote:
> This feels a little awkward, but I guess not terrible. I'm not sure I like it more than just having it all explicit in routines. Nothing binding here, just commentary of "is there something better here?"
I share the sentiment of this looking strange. And there is something better: Using new-style scheduling models ("MCSchedule.h") instead of the itineraries ("MCInstrItineraries.h") will make those functions go away and the information comes directly out of the scheduling model. However this would mean rewriting a bunch of scheduling definitions which is out of scope for this patch.


http://reviews.llvm.org/D21685





More information about the llvm-commits mailing list