[PATCH] D37055: [ARM] Reverse PostRASched subtarget feature logic

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 30 02:13:43 PDT 2017


fhahn added a comment.

Thanks Sam! I think this should be in line with Matthias's comments in a previous review, but I think it would be good if he could have another quick look.



================
Comment at: lib/Target/ARM/ARM.td:879
 
-def : ProcessorModel<"cortex-m3", CortexM3Model,        [ARMv7m,
-                                                         ProcM3,
-                                                         FeatureHasNoBranchPredictor,
-                                                         FeaturePostRAScheduler]>;
+def : ProcessorModel<"cortex-m3", CortexM3Model, [ARMv7m,
+                                                  ProcM3,
----------------
Lots of unrelated whitespace changes. I think @javed.absar submitted a change recently to aligned all Processors model fields consistently.


================
Comment at: lib/Target/ARM/ARMSubtarget.cpp:362
 bool ARMSubtarget::enablePostRAScheduler() const {
-  if (usePostRAScheduler())
-    return true;
-  if (SchedModel.PostRAScheduler)
-    return true;
-  // No need for PostRA scheduling on subtargets where we use the
-  // MachineScheduler.
-  if (useMachineScheduler())
+  if (DisablePostRAScheduler)
     return false;
----------------
Convention seems to be using a "getter" functions to access properties.


================
Comment at: lib/Target/ARM/ARMSubtarget.cpp:364
-    return true;
-  if (SchedModel.PostRAScheduler)
-    return true;
----------------
Is PostRAScheduler still defined in some models? If so, I think we should remove those references, as they would be misleading IMO


https://reviews.llvm.org/D37055





More information about the llvm-commits mailing list