[llvm-commits] [llvm] r72325 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAG.h lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

[Chris Lattner]

On May 25, 2009, at 5:49 PM, Eli Friedman wrote:

> On Mon, May 25, 2009 at 4:12 PM, Chris Lattner <clattner at apple.com>  
> wrote:
>> As Duncan said, please convert this from recursive to iterative.  One
>> known-problem with the existing legalizedag stuff is that the
>> recursive parts quickly run out of stack space when run on a thread
>> (which often has a smaller stack than the main process), it would be
>> nice to keep improving this: legalizetypes is iterative.
>
> I don't really want to copy 500 lines of LegalizeTypes code into
> LegalizeVectorOps.cpp... do you have some other suggestion?

What 500 lines would you need?

>>> +  /// This returns true if it made any changes; in that case,
>>> LegalizeTypes
>>> +  /// is called again before Legalize.
>>> +  ///
>>> +  /// Note that this is an involved process that may invalidate
>>> pointers into
>>> +  /// the graph.
>>> +  bool LegalizeVectors();
>>
>> Would it make sense to change this to only return true when an  
>> illegal
>> type is created?
>
> Maybe; the downside is that we lose the extra DAGCombiner run.

Yes, that's true.  I'd also like to get the dag combiner to be more  
incremental at some point.  It would be "really nice" to be possible  
to run the dag combiner on a subgraph of the dag after *every* major  
change (e.g. i64 -> i32 legalization).

> This
> significantly improves code generation for
> test/CodeGen/X86/widen_cast-4.ll, for example.

And others.  that looks like a partially reduced testcase, so the  
effects are probably bigger than in other cases, but I agree that it  
can be a significant win.

>>>> 2. The remaining pieces of LegalizeDAG should be rewritten, both to
>> make it iterative, and also to simplify it.  There is a ton of
>> duplicated code that happens for each operation.  Making it iterative
>> would allow us to simplify a bunch of this away I think.
>
> Yeah; there are also issues at the moment with inconsistently
> legalizing the operands of various operations.

Yeah, the old dag combiner used to get away with never legalizing  
certain magic operands.  I think that the use of TargetConstant, the  
changes to shufflevector etc are reducing the need for this though.

-Chris