[llvm-commits] [PATCH] Fix big-endian codegen bug in DAGTypeLegalizer::ExpandRes_BITCAST

[Michael Liao]

On Fri, 2012-10-12 at 15:22 +0200, Duncan Sands wrote:
> Hi Ulrich, I pointed this out to Michael during code review, but he didn't do
> anything about it.  The patch looks good to me, but maybe there should also be
> a testcase for <32 x i8> as well as <16 x i8>.

Sorry for the late reply. The patch looks good to me.

Yours
- Michael

> 
> Ciao, Duncan.
> 
> >
> > Hello,
> >
> > once the four AltiVec test-suite patches I just posted are applied, I'm
> > still seeing one failure in the AltiVec tests:
> > SingleSource/UnitTests/Vector/Altivec/2007-01-07-lvsl-lvsr-Regression
> >
> > It turns out that this is a real code generation bug, triggering not in the
> > lvsl/lvsr builtins that the test intends to verify, but in the code used to
> > output the resulting vector ...  This code overlays a <16 x i8> vector with
> > an i128 using bitcast, and subsequently accesses the subwords of this i128
> > to print them.  This exposed a bug in how this bitcast is expanded in
> > ExpandRes_BITCAST on big-endian systems.
> >
> > Since i128 is not directly legal, that routine attempts to do an
> > intermediate bitcast to a <4 x i32> vector, and then construct the Hi and
> > Lo parts of the resulting i128 by pairing up two of those i32 vector
> > elements each.  The code already recognizes that on a big-endian system,
> > the first two vector elements form the Hi part, and the final two vector
> > elements form the Lo part (vice-versa from the little-endian situation).
> >
> > However, we also need to take endianness into account when forming each of
> > those separate pairs:  on a big-endian system, vector element 0 is the
> > *high* part of the pair making up the Hi part of the result, and vector
> > element 1 is the low part of the pair.  The code currently always uses
> > vector element 0 as the low part and vector element 1 as the high part, as
> > is appropriate for little-endian platforms only.
> >
> > The patch below fixes this by swapping the vector elements as they are
> > paired up as appropriate.  This fixes the 2007-01-07-lvsl-lvsr-Regression
> > test case.  In addition, the patch also adds a .ll test case under tests/
> > that checks directly for the correct behavior of the bitcast.  (This new
> > also likewise fails without the ExpandRes_BITCAST change, and passes once
> > it is applied.)
> >
> > OK to commit?
> >
> > Bye,
> > Ulrich
> > (See attached file: diff-llvm-expandres-bitcast)
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>