[llvm-commits] [llvm] r139407 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp test/CodeGen/ARM/2011-09-09-OddVectorDivision.ll

[James Molloy]

So wouldn't a better solution be to make the TypeLegalizer actually legalize the types of BUILD_VECTOR?

>From LegalizeVectorOps.cpp:

// This does not legalize vector manipulations like ISD::BUILD_VECTOR,
// or operations that happen to take a vector which are custom-lowered;
// the legalization for such operations never produces nodes
// with illegal types, so it's okay to put off legalizing them until
// SelectionDAG::Legalize runs.

It seems to me that this statement is false - BUILD_VECTOR can contain illegal types after DAGCombine, so it is *not* safe to put off legalizing them.

Unless the legalization should be done in SelectionDAG::Legalize...?:

  case ISD::BUILD_VECTOR:
    // A weird case: legalization for BUILD_VECTOR never legalizes the
    // operands!
    // FIXME: This really sucks... changing it isn't semantically incorrect,
    // but it massively pessimizes the code for floating-point BUILD_VECTORs
    // because ConstantFP operands get legalized into constant pool loads
    // before the BUILD_VECTOR code can see them.  It doesn't usually bite,
    // though, because BUILD_VECTORS usually get lowered into other nodes
    // which get legalized properly.
    SimpleFinishLegalizing = false;
    break;

It seems to me that there is code elsewhere outside of the DAGCombiner that is not sticking to its pre or postconditions, so fixing the DAGCombiner to produce legal types isn't the best solution. I had originally thought that the type legalization was only done before DAGCombine, which is why I proposed my original solution (which was similar to yours).

Am I missing something here? There could be other places in the DAGCombiner that produces illegal types that aren't legalized that we haven't caught yet...

Cheers,

James


________________________________________
From: Eli Friedman [eli.friedman at gmail.com]
Sent: 10 September 2011 11:46
To: James Molloy
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r139407 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp test/CodeGen/ARM/2011-09-09-OddVectorDivision.ll

On Sat, Sep 10, 2011 at 1:27 AM, James Molloy <James.Molloy at arm.com> wrote:
> Hi Eli,
>
> This makes sense, thanks. A related question though, and please excuse my ignorance because the documentation of SelectionDAG is incomplete at best - under what situations could the DAGCombiner run before the DAGTypeLegalizer? My stepping with GDB found the opposite.

The DAGCombiner always runs before the type legalizer... and after the
type legalizer... and after the type legalizer runs again... see
SelectionDAGISel::CodeGenAndEmitDAG().

-Eli


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.