[PATCH] D20186: Move the check of EnablePostRAScheduler to avoid side effect of disabling antidependency breaker

[Mitch Bodart via llvm-commits]

mbodart created this revision.
mbodart added a reviewer: spatel.
mbodart added a subscriber: llvm-commits.


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.

http://reviews.llvm.org/D20186

Files:
  lib/CodeGen/PostRASchedulerList.cpp

Index: lib/CodeGen/PostRASchedulerList.cpp
===================================================================
--- lib/CodeGen/PostRASchedulerList.cpp
+++ lib/CodeGen/PostRASchedulerList.cpp
@@ -103,6 +103,7 @@
 
     bool runOnMachineFunction(MachineFunction &Fn) override;
 
+  private:
     bool enablePostRAScheduler(
         const TargetSubtargetInfo &ST, CodeGenOpt::Level OptLevel,
         TargetSubtargetInfo::AntiDepBreakMode &Mode,
@@ -269,6 +270,11 @@
     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();
 }
@@ -284,20 +290,15 @@
 
   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 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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D20186.56975.patch
Type: text/x-patch
Size: 1906 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160511/8b1ba303/attachment.bin>