[llvm-commits] [llvm] r52254 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/Target/X86/X86InstrSSE.td test/CodeGen/X86/2008-06-13-NotVolatileLoadStore.ll test/CodeGen/X86/2008-06-13-VolatileLoadStore.ll

[Dan Gohman]

On Jun 13, 2008, at 12:07 PM, Duncan Sands wrote:
> However this exposed an interesting thing - on x86-32
> a store of i64 is considered legal!  That is because
> operations are marked legal by default, regardless of
> whether the type is legal or not.  In some ways this
> is clever: before type legalization this means that
> operations on illegal types are considered legal;
> after type legalization there are no illegal types
> so now operations are only legal if they really are.
> But I consider this to be too cunning for mere mortals.
> Better to do things explicitly by testing AfterLegalize.
> So I have changed things so that operations with illegal
> types are considered illegal - indeed they can never
> map to a machine operation.  However this means that
> the DAG combiner is more conservative because before
> it was "accidentally" performing transforms where the
> type was illegal because the operation was nonetheless
> marked legal.  So in a few such places I added a check
> on AfterLegalize, which I suppose was actually just
> forgotten before.  This causes the DAG combiner to do
> slightly more than it used to, which resulted in the X86
> backend blowing up because it got a slightly surprising
> node it wasn't expecting, so I tweaked it.

Hi Duncan,

Do you know if there are any places left where this change
makes the DAGCombiner more conservative? I'm curious what
kinds of transformations it was getting away with before.

> -  // Must be a legal type.  Expanded an promoted things won't work  
> with rotates.
> +  // Must be a legal type.  Expanded 'n promoted things won't work  
> with rotates.

Heh, still within 80 columns I see ;-).

Dan