[llvm-commits] [llvm] r53476 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAGISel.h include/llvm/Support/Timer.h lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Support/Timer.cpp

Evan Cheng evan.cheng at apple.com
Fri Jul 11 18:37:32 PDT 2008


Hi Dan,

It makes no sense at all, but this patch appears to cause llc to  
miscompile 179.art. Try compiling the attached bc file with and  
without the patch and you can see some differences.

This probably means a bug in the scheduler. But I haven't had a chance  
to debug it. I'll just back out this and 53480 for now. Please take a  
look.

Thanks,

Evan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bugpoint.test.bc
Type: application/octet-stream
Size: 672 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080711/6d986598/attachment.obj>
-------------- next part --------------



On Jul 11, 2008, at 2:54 PM, Dan Gohman wrote:

> Author: djg
> Date: Fri Jul 11 16:54:34 2008
> New Revision: 53476
>
> URL: http://llvm.org/viewvc/llvm-project?rev=53476&view=rev
> Log:
> Add support for putting NamedRegionTimers in TimerGroups, and
> use a timer group for the timers in SelectionDAGISel. Also,
> Split scheduling out from emitting, to give each their own
> timer.
>
> Modified:
>    llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h
>    llvm/trunk/include/llvm/Support/Timer.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>    llvm/trunk/lib/Support/Timer.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h?rev=53476&r1=53475&r2=53476&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h Fri Jul 11  
> 16:54:34 2008
> @@ -30,6 +30,7 @@
>   class FunctionLoweringInfo;
>   class HazardRecognizer;
>   class CollectorMetadata;
> +  class ScheduleDAG;
>
> /// SelectionDAGISel - This is the common base class used for  
> SelectionDAG-based
> /// pattern-matching instruction selectors.
> @@ -191,9 +192,9 @@
>
>   void ComputeLiveOutVRegInfo(SelectionDAG &DAG);
>
> -  /// Pick a safe ordering and emit instructions for each target  
> node in the
> +  /// Pick a safe ordering for instructions for each target node in  
> the
>   /// graph.
> -  void ScheduleAndEmitDAG(SelectionDAG &DAG);
> +  ScheduleDAG *Schedule(SelectionDAG &DAG);
>
>   /// SwitchCases - Vector of CaseBlock structures used to communicate
>   /// SwitchInst code generation information.
>
> Modified: llvm/trunk/include/llvm/Support/Timer.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Timer.h?rev=53476&r1=53475&r2=53476&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/Support/Timer.h (original)
> +++ llvm/trunk/include/llvm/Support/Timer.h Fri Jul 11 16:54:34 2008
> @@ -132,6 +132,8 @@
> ///
> struct NamedRegionTimer : public TimeRegion {
>   explicit NamedRegionTimer(const std::string &Name);
> +  explicit NamedRegionTimer(const std::string &Name,
> +                            const std::string &GroupName);
> };
>
>
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp?rev=53476&r1=53475&r2=53476&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGList.cpp Fri Jul  
> 11 16:54:34 2008
> @@ -103,9 +103,6 @@
>   DOUT << "*** Final schedule ***\n";
>   DEBUG(dumpSchedule());
>   DOUT << "\n";
> -
> -  // Emit in scheduled order
> -  EmitSchedule();
> }
>
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp?rev=53476&r1=53475&r2=53476&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp  
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp Fri  
> Jul 11 16:54:34 2008
> @@ -208,9 +208,6 @@
>   DOUT << "*** Final schedule ***\n";
>   DEBUG(dumpSchedule());
>   DOUT << "\n";
> -
> -  // Emit in scheduled order
> -  EmitSchedule();
> }
>
> /// CommuteNodesToReducePressure - If a node is two-address and  
> commutable, and
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=53476&r1=53475&r2=53476&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp  
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Fri Jul  
> 11 16:54:34 2008
> @@ -5284,10 +5284,11 @@
> void SelectionDAGISel::CodeGenAndEmitDAG(SelectionDAG &DAG) {
>   DOUT << "Lowered selection DAG:\n";
>   DEBUG(DAG.dump());
> +  std::string GroupName = "Instruction Selection and Scheduling";
>
>   // Run the DAG combiner in pre-legalize mode.
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("DAG Combining 1");
> +    NamedRegionTimer T("DAG Combining 1", GroupName);
>     DAG.Combine(false, *AA);
>   } else {
>     DAG.Combine(false, *AA);
> @@ -5304,7 +5305,7 @@
>   }
>
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("DAG Legalization");
> +    NamedRegionTimer T("DAG Legalization", GroupName);
>     DAG.Legalize();
>   } else {
>     DAG.Legalize();
> @@ -5315,7 +5316,7 @@
>
>   // Run the DAG combiner in post-legalize mode.
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("DAG Combining 2");
> +    NamedRegionTimer T("DAG Combining 2", GroupName);
>     DAG.Combine(true, *AA);
>   } else {
>     DAG.Combine(true, *AA);
> @@ -5332,24 +5333,41 @@
>   // Third, instruction select all of the operations to machine  
> code, adding the
>   // code to the MachineBasicBlock.
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("Instruction Selection");
> +    NamedRegionTimer T("Instruction Selection", GroupName);
>     InstructionSelect(DAG);
>   } else {
>     InstructionSelect(DAG);
>   }
>
> +  // Schedule machine code.
> +  ScheduleDAG *Scheduler;
> +  if (TimePassesIsEnabled) {
> +    NamedRegionTimer T("Instruction Scheduling", GroupName);
> +    Scheduler = Schedule(DAG);
> +  } else {
> +    Scheduler = Schedule(DAG);
> +  }
> +
>   // Emit machine code to BB.  This can change 'BB' to the last  
> block being
>   // inserted into.
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("Instruction Scheduling");
> -    ScheduleAndEmitDAG(DAG);
> +    NamedRegionTimer T("Instruction Creation", GroupName);
> +    Scheduler->EmitSchedule();
> +  } else {
> +    Scheduler->EmitSchedule();
> +  }
> +
> +  // Free the scheduler state.
> +  if (TimePassesIsEnabled) {
> +    NamedRegionTimer T("Instruction Scheduling Cleanup", GroupName);
> +    delete Scheduler;
>   } else {
> -    ScheduleAndEmitDAG(DAG);
> +    delete Scheduler;
>   }
>
>   // Perform target specific isel post processing.
>   if (TimePassesIsEnabled) {
> -    NamedRegionTimer T("Instruction Selection Post Processing");
> +    NamedRegionTimer T("Instruction Selection Post Processing",  
> GroupName);
>     InstructionSelectPostProcessing(DAG);
>   } else {
>     InstructionSelectPostProcessing(DAG);
> @@ -5597,10 +5615,10 @@
> }
>
>
> -// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> -/// ScheduleAndEmitDAG - Pick a safe ordering and emit instructions  
> for each
> +/// Schedule - Pick a safe ordering for instructions for each
> /// target node in the graph.
> -void SelectionDAGISel::ScheduleAndEmitDAG(SelectionDAG &DAG) {
> +///
> +ScheduleDAG *SelectionDAGISel::Schedule(SelectionDAG &DAG) {
>   if (ViewSchedDAGs) DAG.viewGraph();
>
>   RegisterScheduler::FunctionPassCtor Ctor =  
> RegisterScheduler::getDefault();
> @@ -5610,12 +5628,11 @@
>     RegisterScheduler::setDefault(Ctor);
>   }
>
> -  ScheduleDAG *SL = Ctor(this, &DAG, BB, FastISel);
> -  BB = SL->Run();
> -
> -  if (ViewSUnitDAGs) SL->viewGraph();
> +  ScheduleDAG *Scheduler = Ctor(this, &DAG, BB, FastISel);
> +  BB = Scheduler->Run();
>
> -  delete SL;
> +  if (ViewSUnitDAGs) Scheduler->viewGraph();
> +  return Scheduler;
> }
>
>
>
> Modified: llvm/trunk/lib/Support/Timer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Timer.cpp?rev=53476&r1=53475&r2=53476&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Support/Timer.cpp (original)
> +++ llvm/trunk/lib/Support/Timer.cpp Fri Jul 11 16:54:34 2008
> @@ -182,19 +182,51 @@
> //   NamedRegionTimer Implementation
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
>
> -static ManagedStatic<std::map<std::string, Timer> > NamedTimers;
> +namespace {
> +
> +typedef std::map<std::string, Timer> Name2Timer;
> +typedef std::map<std::string, std::pair<TimerGroup, Name2Timer> >  
> Name2Pair;
> +
> +}
> +
> +static ManagedStatic<Name2Timer> NamedTimers;
> +
> +static ManagedStatic<Name2Pair> NamedGroupedTimers;
>
> static Timer &getNamedRegionTimer(const std::string &Name) {
> -  std::map<std::string, Timer>::iterator I = NamedTimers->find(Name);
> +  Name2Timer::iterator I = NamedTimers->find(Name);
>   if (I != NamedTimers->end())
>     return I->second;
>
>   return NamedTimers->insert(I, std::make_pair(Name, Timer(Name)))- 
> >second;
> }
>
> +static Timer &getNamedRegionTimer(const std::string &Name,
> +                                  const std::string &GroupName) {
> +
> +  Name2Pair::iterator I = NamedGroupedTimers->find(GroupName);
> +  if (I == NamedGroupedTimers->end()) {
> +    TimerGroup TG(GroupName);
> +    std::pair<TimerGroup, Name2Timer> Pair(TG, Name2Timer());
> +    I = NamedGroupedTimers->insert(I, std::make_pair(GroupName,  
> Pair));
> +  }
> +
> +  Name2Timer::iterator J = I->second.second.find(Name);
> +  if (J == I->second.second.end())
> +    J = I->second.second.insert(J,
> +                                std::make_pair(Name,
> +                                               Timer(Name,
> +                                                     I- 
> >second.first)));
> +
> +  return J->second;
> +}
> +
> NamedRegionTimer::NamedRegionTimer(const std::string &Name)
>   : TimeRegion(getNamedRegionTimer(Name)) {}
>
> +NamedRegionTimer::NamedRegionTimer(const std::string &Name,
> +                                   const std::string &GroupName)
> +  : TimeRegion(getNamedRegionTimer(Name, GroupName)) {}
>
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> //   TimerGroup Implementation
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list