Move the check of EnablePostRAScheduler to avoid side effect of disabling antidependency breaker

[mbodart]

Summary

With the current code structure, specifying -post-RA-scheduler=true has different behavior than
enabling it via the scheduling model. The issue is that the mere presence of this option on the
command line suppresses the post scheduler code that would otherwise select an antidependency breaker.

I discovered this when chasing http://llvm.org/PR27681, which should have been reproducible
with -post-RA-scheduler=true. With this change, that will be the case, and should help avoid
such surprises down the road.

I added the "private:" specification simply to make it a little more clear that PostRAScheduler::enablePostRAScheduler
is only referenced from within its class.

[mgrang]

lib/CodeGen/PostRASchedulerList.cpp

296 ↗

(On Diff #56975)

You can get rid of the extra newline.

[spatel]

LGTM.

[mbodart]

lib/CodeGen/PostRASchedulerList.cpp

296 ↗

(On Diff #56975)

Can you clarify which line is "extra"?

There's one line of white space before, separating the code from some local declarations,
and one after. Both lead to better readability, IMO.

llvm/trunk/lib/CodeGen/PostRASchedulerList.cpp

public:
 
     MachineFunctionProperties getRequiredProperties() const override {
       return MachineFunctionProperties().set(
           MachineFunctionProperties::Property::AllVRegsAllocated);
     }
 
     bool runOnMachineFunction(MachineFunction &Fn) override;
 
+  private:
     bool enablePostRAScheduler(
         const TargetSubtargetInfo &ST, CodeGenOpt::Level OptLevel,
         TargetSubtargetInfo::AntiDepBreakMode &Mode,
         TargetSubtargetInfo::RegClassVector &CriticalPathRCs) const;
   };
   char PostRAScheduler::ID = 0;
 
   class SchedulePostRATDList : public ScheduleDAGInstrs {

 
 bool PostRAScheduler::enablePostRAScheduler(
     const TargetSubtargetInfo &ST,
     CodeGenOpt::Level OptLevel,
     TargetSubtargetInfo::AntiDepBreakMode &Mode,
     TargetSubtargetInfo::RegClassVector &CriticalPathRCs) const {
   Mode = ST.getAntiDepBreakMode();
   ST.getCriticalPathRCs(CriticalPathRCs);
+
+  // Check for explicit enable/disable of post-ra scheduling.
+  if (EnablePostRAScheduler.getPosition() > 0)
+    return EnablePostRAScheduler;
+
   return ST.enablePostRAScheduler() &&
          OptLevel >= ST.getOptLevelToEnablePostRAScheduler();
 }
 
 bool PostRAScheduler::runOnMachineFunction(MachineFunction &Fn) {
   if (skipFunction(*Fn.getFunction()))
     return false;
 
   TII = Fn.getSubtarget().getInstrInfo();
   MachineLoopInfo &MLI = getAnalysis<MachineLoopInfo>();
   AliasAnalysis *AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
   TargetPassConfig *PassConfig = &getAnalysis<TargetPassConfig>();
 
   RegClassInfo.runOnMachineFunction(Fn);
 
-  // Check for explicit enable/disable of post-ra scheduling.
   TargetSubtargetInfo::AntiDepBreakMode AntiDepMode =
     TargetSubtargetInfo::ANTIDEP_NONE;
   SmallVector<const TargetRegisterClass*, 4> CriticalPathRCs;
-  if (EnablePostRAScheduler.getPosition() > 0) {
-    if (!EnablePostRAScheduler)
-      return false;
-  } else {
-    // Check that post-RA scheduling is enabled for this target.
-    // This may upgrade the AntiDepMode.
-    if (!enablePostRAScheduler(Fn.getSubtarget(), PassConfig->getOptLevel(),
-                               AntiDepMode, CriticalPathRCs))
-      return false;
-  }
+
 
   // Check for antidep breaking override...
   if (EnableAntiDepBreaking.getPosition() > 0) {
     AntiDepMode = (EnableAntiDepBreaking == "all")
       ? TargetSubtargetInfo::ANTIDEP_ALL
       : ((EnableAntiDepBreaking == "critical")
          ? TargetSubtargetInfo::ANTIDEP_CRITICAL
          : TargetSubtargetInfo::ANTIDEP_NONE);