[LLVMdev] Turning on LegalizeTypes by default

[Chris Lattner]

On Oct 26, 2008, at 1:03 AM, Duncan Sands wrote:

> Hi all, I plan to turn on the new type legalization infrastructure
> "LegalizeTypes" by default tomorrow.  This is a redesign/ 
> reimplementation
> of the logic currently in LegalizeDAG that turns (for example) 64 bit
> arithmetic on a 32 bit machine into a series of 32 bit operations.   
> As well
> as being a cleaner design, it also supports code generation for  
> arbitrary
> precision integers such as i123.

Woo hoo!

> So please: if LegalizeTypes breaks something, send me a bitcode  
> testcase
> rather than turning LegalizeTypes off again.  With a major change  
> like this
> there is bound to be some pain, but I don't expect it to last more  
> than a
> week or two.

Makes sense to me.

> Once LegalizeTypes has stabilized, I plan to progressively delete type
> legalization functionality from LegalizeDAG, checking that  
> LegalizeTypes
> has the same functionality as I go, and adding it if not.

This sounds good.  Note that this is somewhat trickier than it seems  
and may also expose bugs.  The problems that I hit when I was working  
on legalize types ages ago was that custom lowering ops on some  
targets generates nodes with illegal types.  One example (that I think  
was fixed) was that legalizing a uitofp from 32-bit to double turns  
into a 64-bit sitofp on x87.  Just expect a bit of fallout when  
removing chunks from legalizedag.


> Testsuite results are as follows:
>
> (a) "make check" has two failures:
>  ARM/cse-libcalls.ll - this is because of a bug in UpdateNodeOperands
> which does not do CSE on calls correctly.  The question here is how  
> best
> to fix this while minimizing code duplication.  This will doubtless be
> fixed soon.

Ok.

>  PowerPC/vec_spat.ll (@splat_h) - here code got slightly worse.  The
> problem here is that LegalizeDAG "cheats": it constructs a  
> BUILD_VECTOR
> where the type of the operands doesn't match the vector element type  
> (this
> is supposed to be illegal), while LegalizeTypes correctly constructs a
> BUILD_VECTOR where the operand type is equal to the vector element  
> type,
> but then the PPC custom code doesn't handle this optimally.

After looking at this a bit, I don't think this is the real reason.   
The issue (I think) is a phase ordering issue between legalize types  
and custom lowering.  In the old legalize case, legalize just custom  
lowers the build_vector-of-i16 right away.  In the legalize types  
case, legalize types modifies the DAG quite a bit to eliminate the i16  
operations, and the PPC backend doesn't match on what it expects.

>  Two solutions:
> decide that in fact
>  v8i16 = BUILD_VECTOR(i32, i32, ..., i32)
> is legal and modify LegalizeTypes to take advantage of this

Are you saying that the build_vector would have 8 i32 inputs, or only  
4?  If 8, I really don't like it because it breaks a lot of  
invariants.  If 4 then it should be the same as the current v4i32  
build vector that we're getting.

> or improve
> the PPC so it better handles the code coming out of LegalizeTypes.

Another option would be to have legalizetypes invoke the ppc custom  
lowering hook for BUILD_VECTOR when it sees the build_vector of i16.   
This way, the ppc backend would see the input build_vector unmolested.

Thanks for spearheading this Duncan!

-Chris