[llvm-commits] [llvm] r160796 - in /llvm/trunk: include/llvm/CodeGen/Passes.h include/llvm/InitializePasses.h lib/CodeGen/CMakeLists.txt lib/CodeGen/EarlyIfConversion.cpp lib/CodeGen/MachineTraceMetrics.cpp lib/CodeGen/MachineTraceMetrics.h

[Andrew Trick]

On Jul 26, 2012, at 11:38 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> Author: stoklund
> Date: Thu Jul 26 13:38:11 2012
> New Revision: 160796
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=160796&view=rev
> Log:
> Start scaffolding for a MachineTraceMetrics analysis pass.
> 
> This is still a work in progress.
> 
> Out-of-order CPUs usually execute instructions from multiple basic
> blocks simultaneously, so it is necessary to look at longer traces when
> estimating the performance effects of code transformations.
> 
> The MachineTraceMetrics analysis will pick a typical trace through a
> given basic block and provide performance metrics for the trace. Metrics
> will include:
> 
> - Instruction count through the trace.
> - Issue count per functional unit.
> - Critical path length, and per-instruction 'slack'.
> 
> These metrics can be used to determine the performance limiting factor
> when executing the trace, and how it will be affected by a code
> transformation.
> 
> Initially, this will be used by the early if-conversion pass.
> 
> Added:
>    llvm/trunk/lib/CodeGen/MachineTraceMetrics.cpp
>    llvm/trunk/lib/CodeGen/MachineTraceMetrics.h
> Modified:
>    llvm/trunk/include/llvm/CodeGen/Passes.h
>    llvm/trunk/include/llvm/InitializePasses.h
>    llvm/trunk/lib/CodeGen/CMakeLists.txt
>    llvm/trunk/lib/CodeGen/EarlyIfConversion.cpp

Really, really nice.

+    // Don't leave CurLoop.
+    if (LB.Loops->getLoopFor(To) != LB.CurLoop)
+      return false;

I don't get this. Traces can overlap, so the current trace can include an inner loop iteration, from the perspective of the loop preheader or loop exit.

+  /// Invalidate cached information about MBB. This must be called *before* MBB
+  /// is erased, or the CFG is otherwise changed.

The cautionary comment on invalid() is good! It seems like it would be easy to catch the inconsistent CFG problem with a verifyAnalysis that checked trace pred/succ edges.

This comment is mysterious:

+    // Ignore invalidated predecessors. This never happens on the first scan,
+    // but if we rejected this predecessor earlier, it won't be revalidated.

AFAICT you should assert that the predecessor is valid. You need to reevaluate it before you can pick correctly after something changes.

It might also help to clarify that there are two things being invalidated, trace-level information vs. block-local information. It helps understand what's happening to know that block-local information will only be invalidated for a single block.

There's also no reason we shouldn't be able to add blocks, as long as the traces reaching new edges are invalidated first. The current invalidate API isn't quite right for that, because we don't want to invalidate any block-local info. Just something to think about for now.

static isFree()... ugh.

-Andy