[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 9:13 PM, Chris Lattner <clattner at apple.com> wrote:
>
> 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?

Large parts of DAGTypeLegalizer::run and
DAGTypeLegalizer::PerformExpensiveChecks,
DAGTypeLegalizer::AnalyzeNewNode, DAGTypeLegalizer::AnalyzeNewValue,
DAGTypeLegalizer::RemapValue, a bit of DAGTypeLegalizer::ExpungeNode,
and NodeUpdateListener.  Okay, maybe not quite 500 lines, but still a
lot.

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

LegalizeDAG now asserts that all the results and operands of every
node it inspects have legal type, so I think I've caught all the
issues of that sort.  But one example of something I saw is that PPC
was creating BUILD_VECTOR nodes with operands of illegal type, and the
legalizer never looked at them.

-Eli