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

[Eli Friedman]

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?

>> +  /// 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.  This
significantly improves code generation for
test/CodeGen/X86/widen_cast-4.ll, for example.

>> +// This file implements the SelectionDAG::LegalizeVectors method.
>> +//
>> +// The vector legalizer looks for vector operations which might
>> need to be
>> +// unrolled and legalizes them.
>
> s/unroll/scalarize/g?  This should never do any "splitting" of
> vectors, right?

Sure, I'll change that.

>>> Possibly; I took the current approach from the DAG legalizer.  Taking
>>> a quick look, the type legalizer's logic to deal with nodes being
>>> inserted during legalization seems complicated enough that I wouldn't
>>> want to duplicate it... perhaps some sort of general legalizer base
>>> class would be appropriate?
>>
>> I think Chris wants to redesign LegalizeDAG.  Hopefully at that point
>> all the different legalizers will be able to share some generic
>> infrastructure.  I suspect the type legalizer could be simplified a
>> lot
>> by using an SDAG equivalent of CallbackVH.
>
> Now that you disabled the type legalization code from LegalizeDAG,
> there are several steps I think we should take:
>
> 1. Finish ripping the dead code out of LegalizeDAG.

Of course :)

> 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.

-Eli