[llvm] r185794 - Fix PromoteIntRes_BUILD_VECTOR crash with i1 vectors

Hal Finkel hfinkel at anl.gov
Mon Jul 8 07:05:01 PDT 2013


----- Original Message -----
> Hi Hal,
> 
> >> That doesn't sound like a bug, since BUILD_VECTOR explicitly
> >> allows
> >> the
> >> operands to be bigger than the result element type (see above).
> >>   However
> >> your patch seems to be dealing with the opposite problem (if I
> >> read
> >> it
> >> right): a BUILD_VECTOR where the result element type is larger
> >> than
> >> the
> >> input operand type.  So I'm kind of confused by your description
> >> and
> >> the
> >> comment in the code, since it doesn't seem to match what the patch
> >> does.
> >
> > Thanks for looking at this, but I don't understand why you're
> > confused ;) -- In the situation in the test case, a (<v?i1> = BV
> > <i32>, <i32>, ...) node is having its result promoted. The result
> > of this promotion is (v?i16 = BV <i32>, <i32>, ...). The old code
> > would blindly any_extend the operands to the vector element type
> > of the result (i16 in this case), causing an assertion failure
> > (because we can't any_extend <i32> to <i16>). All I've done in the
> > patch is make sure that the any_extend is legal before we do it.
> > Maybe I should remove mentioning booleans in the comment?
> 
> I read your patch wrong, sorry. 

No problem.

 That said, if the operand type is
> (at least)
> as big as the result type, then I guess there is no need to compute
> the Ops
> array and create a new node, you could just mutate the return type of
> the
> existing node in place (I don't recall if this is possible - I
> haven't worked
> on codegen in a while). 

I also don't recall if this is something that can be done (safely). Maybe if the node has only one user?

> Also, probably
> PromoteIntRes_SCALAR_TO_VECTOR has
> exactly the same problem, because it's essentially doing the same
> thing as
> BUILD_VECTOR.

I'll take a look.

Thanks again!

 -Hal

> 
> Ciao, Duncan.
> 
> >
> >   -Hal
> >
> >>
> >> Ciao, Duncan.
> >>
> >>    As a result, we cannot blindly assume that we can ANY_EXTEND
> >>    the
> >>    operands
> >>> to the result type.
> >>>
> >>> Added:
> >>>       llvm/trunk/test/CodeGen/PowerPC/bv-pres-v8i1.ll
> >>> Modified:
> >>>       llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >>>
> >>> Modified:
> >>> llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp?rev=185794&r1=185793&r2=185794&view=diff
> >>> ==============================================================================
> >>> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >>> (original)
> >>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> >>> Mon Jul  8 01:16:58 2013
> >>> @@ -2930,7 +2930,13 @@ SDValue DAGTypeLegalizer::PromoteIntRes_
> >>>      SmallVector<SDValue, 8> Ops;
> >>>      Ops.reserve(NumElems);
> >>>      for (unsigned i = 0; i != NumElems; ++i) {
> >>> -    SDValue Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem,
> >>> N->getOperand(i));
> >>> +    SDValue Op;
> >>> +    // It is possible for the operands to be larger than the
> >>> result, for example,
> >>> +    // when the operands are promoted booleans and the result
> >>> was
> >>> an i1 vector.
> >>> +    if (N->getOperand(i).getValueType().bitsLT(NOutVTElem))
> >>> +      Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem,
> >>> N->getOperand(i));
> >>> +    else
> >>> +      Op = N->getOperand(i);
> >>>        Ops.push_back(Op);
> >>>      }
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list