[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