[llvm] r235079 - TRUNCATE constant folding - minor fix for rL233224
James Molloy
james at jamesmolloy.co.uk
Thu Apr 16 13:03:54 PDT 2015
Right, sorry my answer was rather ARM specific as that was what the
testcase that caught this bug was based on.
(On ARM) , i16 is illegal but v4i16 is illegal. There are many other
examples in different architectures where the set of scalar legal types
isn't an exact match with the set of vector legal types. So BUILD_VECTOR
really does need to deal with implicit truncations like this.
In fact, the size of a type may be smaller than the size of its container.
Consider an i16 in an i32 register- the upper 16 bits can be undefined in
DAG form. We can use an SIGN_EXTEND_INREG for example to widen it to i32,
but the input and output type of the sign extend nose will both be i32!
I hope this helps,
James
On Thu, 16 Apr 2015 at 20:58, Mehdi Amini <mehdi.amini at apple.com> wrote:
> Hi James,
>
> I’m not sure what you mean about i16 being illegal? Isn’t it something
> target specific while this commit change a target-independent behavior?
> What if I have a target where both i32 and i16 are legal?
>
> My impression was that if you feed two i32 to BUILD_VECTOR you should get
> a v2i32, and you can’t get a v2i16 if you don’t feed i16 inputs. But if I
> understood correctly Ahmed’s answer, there are implicit conversions
> happening with BUILD_VECTOR.
>
> —
> Mehdi
>
>
> On Apr 16, 2015, at 12:41 PM, James Molloy <james at jamesmolloy.co.uk>
> wrote:
>
> Hi Mehdi,
>
> It's not altogether surprising- after type legalisation BUILD_VECTOR can't
> have any illegal types feeding it, and i16 is illegal. I32 is the smallest
> legal type.
>
> Therefore it must be able deal with truncation!
>
> Cheers,
>
> James
> On Thu, 16 Apr 2015 at 20:24, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>>
>> > On Apr 16, 2015, at 10:52 AM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
>> wrote:
>> >
>> > On Thu, Apr 16, 2015 at 10:44 AM, Mehdi Amini <mehdi.amini at apple.com>
>> wrote:
>> >> Hi Simon,
>> >>
>> >> I have an out-of-tree target broken by this commits, and I have a
>> feeling that the problem is in this change.
>> >>
>> >> I have this node: v2i32 BUILD_VECTOR { i32 = Constant<0>, i32 =
>> undef }
>> >> That I try to truncate:
>> >> getNode(ISD::TRUNCATE, dl, v2i16, N)
>> >>
>> >> After your change, getNode() returns this node:
>> >>
>> >> N = v2i16 BUILD_VECTOR { i32 = Constant<0>, i32 = undef }
>> >>
>> >> Note that the operands are not truncated, which breaks the DAG type
>> system.
>> >
>> > It does? BUILD_VECTOR can truncate integers, and that's the point of
>> > this fix, no?
>>
>> I didn’t know BUILD_VECTOR had an implicit TRUNCATE. Does it carry only
>> implicit truncate or other conversion as well (EXTEND for instance)?
>>
>> So we are not allowed to fold naively an EXTRACT_VECTOR_ELEMENT that is
>> fed with a BUILD_VECTOR?
>> This does not seem in-line with usual behavior in the DAG and it makes
>> combining more complex.
>>
>>
>> Thanks,
>>
>> Mehdi
>>
>> >
>> > -Ahmed
>> >
>> >> I assume you are relying on FoldConstantArithmetic() to magically
>> convert the constant to the truncated type, however it does not work with
>> undef.
>> >>
>> >> I have the feeling that the proper way requires to wrap every operand
>> with a TRUNCATE before calling getNode, but you may see an alternative?
>> >>
>> >> Thanks,
>> >>
>> >> Mehdi
>> >>
>> >>
>> >>
>> >>> On Apr 16, 2015, at 1:21 AM, Simon Pilgrim <llvm-dev at redking.me.uk>
>> wrote:
>> >>>
>> >>> Author: rksimon
>> >>> Date: Thu Apr 16 03:21:09 2015
>> >>> New Revision: 235079
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=235079&view=rev
>> >>> Log:
>> >>> TRUNCATE constant folding - minor fix for rL233224
>> >>>
>> >>> Fix for test case found by James Molloy - TRUNCATE of constant build
>> vectors can be more simply achieved by simply replacing with a new build
>> vector node with the truncated value type - no need to touch the scalar
>> operands at all.
>> >>>
>> >>> Added:
>> >>> llvm/trunk/test/CodeGen/AArch64/fold-constants.ll
>> >>> Modified:
>> >>> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> >>>
>> >>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> >>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=235079&r1=235078&r2=235079&view=diff
>> >>>
>> ==============================================================================
>> >>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
>> >>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Thu Apr 16
>> 03:21:09 2015
>> >>> @@ -2851,13 +2851,16 @@ SDValue SelectionDAG::getNode(unsigned O
>> >>> // FIXME: Entirely reasonable to perform folding of other unary
>> >>> // operations here as the need arises.
>> >>> break;
>> >>> + case ISD::TRUNCATE:
>> >>> + // Constant build vector truncation can be done with the
>> original scalar
>> >>> + // operands but with a new build vector with the truncated
>> value type.
>> >>> + return getNode(ISD::BUILD_VECTOR, DL, VT, BV->ops());
>> >>> case ISD::FNEG:
>> >>> case ISD::FABS:
>> >>> case ISD::FCEIL:
>> >>> case ISD::FTRUNC:
>> >>> case ISD::FFLOOR:
>> >>> case ISD::FP_EXTEND:
>> >>> - case ISD::TRUNCATE:
>> >>> case ISD::UINT_TO_FP:
>> >>> case ISD::SINT_TO_FP: {
>> >>> // Let the above scalar folding handle the folding of each
>> element.
>> >>>
>> >>> Added: llvm/trunk/test/CodeGen/AArch64/fold-constants.ll
>> >>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/fold-constants.ll?rev=235079&view=auto
>> >>>
>> ==============================================================================
>> >>> --- llvm/trunk/test/CodeGen/AArch64/fold-constants.ll (added)
>> >>> +++ llvm/trunk/test/CodeGen/AArch64/fold-constants.ll Thu Apr 16
>> 03:21:09 2015
>> >>> @@ -0,0 +1,21 @@
>> >>> +; RUN: llc -mtriple=aarch64-linux-gnu -o - %s | FileCheck %s
>> >>> +
>> >>> +define i64 @dotests_616() {
>> >>> +; CHECK-LABEL: dotests_616
>> >>> +; CHECK: movi d0, #0000000000000000
>> >>> +; CHECK-NEXT: umov w8, v0.b[2]
>> >>> +; CHECK-NEXT: sbfx w8, w8, #0, #1
>> >>> +; CHECK-NEXT: fmov s0, w8
>> >>> +; CHECK-NEXT: fmov x0, d0
>> >>> +; CHECK-NEXT: ret
>> >>> +entry:
>> >>> + %0 = bitcast <2 x i64> zeroinitializer to <8 x i16>
>> >>> + %1 = and <8 x i16> zeroinitializer, %0
>> >>> + %2 = icmp ne <8 x i16> %1, zeroinitializer
>> >>> + %3 = extractelement <8 x i1> %2, i32 2
>> >>> + %vgetq_lane285 = sext i1 %3 to i16
>> >>> + %vset_lane = insertelement <4 x i16> undef, i16 %vgetq_lane285,
>> i32 0
>> >>> + %4 = bitcast <4 x i16> %vset_lane to <1 x i64>
>> >>> + %vget_lane = extractelement <1 x i64> %4, i32 0
>> >>> + ret i64 %vget_lane
>> >>> +}
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> llvm-commits mailing list
>> >>> llvm-commits at cs.uiuc.edu
>> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >>
>> >>
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at cs.uiuc.edu
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150416/a1051c10/attachment.html>
More information about the llvm-commits
mailing list