[LLVMdev] Possible DAGCombiner or TargetData Bug

[David Greene]

On Wednesday 18 February 2009 21:43, Dan Gohman wrote:
> I agree, that doesn't look right.  It looks like this
> is what was intended:
>
> Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(revision 65000)
> +++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(working copy)
> @@ -4903,9 +4903,9 @@
>    // resultant store does not need a higher alignment than the original.
>    if (Value.getOpcode() == ISD::BIT_CONVERT && !ST->isTruncatingStore() &&
>        ST->isUnindexed()) {
> -    unsigned Align = ST->getAlignment();
> +    unsigned OrigAlign = ST->getAlignment();
>      MVT SVT = Value.getOperand(0).getValueType();
> -    unsigned OrigAlign = TLI.getTargetData()->
> +    unsigned Align = TLI.getTargetData()->
>        getABITypeAlignment(SVT.getTypeForMVT());
>      if (Align <= OrigAlign &&
>          ((!LegalOperations && !ST->isVolatile()) ||
>
> Does that look right to you?

Yes, and it fixes the problem.

What's your opinion about how TargetData and X86Subtarget define ABI alignment 
for SSE registers?  I think that's suspect too.  It's too bad we can't specify 
separate ABI alignments for v16i/f8, v8i/f16, v4i/f32 and v2i/f64 as we 
should probably set the ABI alignment to the element alignment.  But I guess 
to be conservative we should set it to 8 bits.  Unless I'm misunderstanding 
the purpose of the ABI alignment.

                                                  -Dave