<div dir="ltr">Hi Ahmed,<br><br>Applied with your suggested changes in r235191.<div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Thu, 16 Apr 2015 at 18:37 Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks for having a look so quickly!<br>
<br>
LGTM, with a small nit: IsFloat doesn't need to be an argument, but is<br>
already available locally, as OutVT.isFloatingPoint().<br>
<br>
-Ahmed<br>
<br>
<br>
On Thu, Apr 16, 2015 at 1:43 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>> wrote:<br>
> Hi Ahmed,<br>
><br>
> Thanks for your comments. I've re-looked at the code, and I completely<br>
> misunderstood what an FTRUNC node was - I confused it with FP_ROUND.<br>
><br>
> So yes, FTRUNC should just use the generic unary op splitting, and wide FP<br>
> truncations should not have an FTRUNC node planted but an (FP_ROUND $x, 0).<br>
><br>
> I also agree with the HalfVT problem. Thanks for noticing both of these! I<br>
> attach a patch which I think fixes all the issues, could you please review<br>
> it?<br>
><br>
> I've added tests for v4{f,i}64->v4{i,f}16 to exercise this logic.<br>
><br>
> Cheers,<br>
><br>
> James<br>
><br>
> On Thu, 16 Apr 2015 at 02:51 Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Hi James,<br>
>><br>
>> I got around to looking at this, and I have two concerns.<br>
>><br>
>> First, is FTRUNC really a truncating conversion, and isn't it just a<br>
>> unary op? It returns the exact same type, but removes the fractional<br>
>> part.<br>
>> The code should never fire currently, as I don't think we generate<br>
>> FTRUNCs with different in/out types. It might make sense for<br>
>> "(FP_ROUND x, 1)" though, where it's fine to do the double-rounding.<br>
>><br>
>> Second, for INT_TO_FP, the current logic (this is fixable I think)<br>
>> explicitly creates an integer type as HalfElementVT, but it should<br>
>> really be an FP type (if not, you should get stuff like "uitofp int -><br>
>> int").<br>
>> The code should only fire when there's a 4x size difference; the only<br>
>> case I can think of is f64/i64 -> f16, or i128 -> f32, if that's<br>
>> actually legal somewhere.<br>
>><br>
>> -Ahmed<br>
>><br>
>><br>
>> On Tue, Mar 31, 2015 at 3:20 AM, James Molloy <<a href="mailto:james.molloy@arm.com" target="_blank">james.molloy@arm.com</a>><br>
>> wrote:<br>
>> > Author: jamesm<br>
>> > Date: Tue Mar 31 05:20:58 2015<br>
>> > New Revision: 233681<br>
>> ><br>
>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233681&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233681&view=rev</a><br>
>> > Log:<br>
>> > [SDAG] Move TRUNCATE splitting logic into a helper, and use<br>
>> > it more liberally.<br>
>> ><br>
>> > SplitVecOp_TRUNCATE has logic for recursively splitting oversize vectors<br>
>> > that need more than one round of splitting to become legal. There are<br>
>> > many<br>
>> > other ISD nodes that could benefit from this logic, so factor it out and<br>
>> > use it for FP_TO_UINT,FP_TO_SINT,SINT_TO_FP,UINT_TO_FP and FTRUNC.<br>
>> ><br>
>> > Added:<br>
>> > llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll<br>
>> > Modified:<br>
>> > llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h<br>
>> > llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp<br>
>> > llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll<br>
>> > llvm/trunk/test/CodeGen/ARM/vcvt.ll<br>
>> ><br>
>> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h?rev=233681&r1=233680&r2=233681&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h?rev=233681&r1=233680&r2=233681&view=diff</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h (original)<br>
>> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.h Tue Mar 31<br>
>> > 05:20:58 2015<br>
>> > @@ -593,6 +593,7 @@ private:<br>
>> > bool SplitVectorOperand(SDNode *N, unsigned OpNo);<br>
>> > SDValue SplitVecOp_VSELECT(SDNode *N, unsigned OpNo);<br>
>> > SDValue SplitVecOp_UnaryOp(SDNode *N);<br>
>> > + SDValue SplitVecOp_TruncateHelper(SDNode *N, unsigned TruncateOp);<br>
>> ><br>
>> > SDValue SplitVecOp_BITCAST(SDNode *N);<br>
>> > SDValue SplitVecOp_EXTRACT_SUBVECTOR(SDNode *N);<br>
>> > @@ -600,7 +601,6 @@ private:<br>
>> > SDValue SplitVecOp_STORE(StoreSDNode *N, unsigned OpNo);<br>
>> > SDValue SplitVecOp_MSTORE(MaskedStoreSDNode *N, unsigned OpNo);<br>
>> > SDValue SplitVecOp_CONCAT_VECTORS(SDNode *N);<br>
>> > - SDValue SplitVecOp_TRUNCATE(SDNode *N);<br>
>> > SDValue SplitVecOp_VSETCC(SDNode *N);<br>
>> > SDValue SplitVecOp_FP_ROUND(SDNode *N);<br>
>> ><br>
>> ><br>
>> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp?rev=233681&r1=233680&r2=233681&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp?rev=233681&r1=233680&r2=233681&view=diff</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp<br>
>> > (original)<br>
>> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp Tue Mar<br>
>> > 31 05:20:58 2015<br>
>> > @@ -1293,7 +1293,9 @@ bool DAGTypeLegalizer::SplitVectorOperan<br>
>> > case ISD::EXTRACT_SUBVECTOR: Res = SplitVecOp_EXTRACT_SUBVECTOR(N);<br>
>> > break;<br>
>> > case ISD::EXTRACT_VECTOR_ELT:Res =<br>
>> > SplitVecOp_EXTRACT_VECTOR_ELT(N); break;<br>
>> > case ISD::CONCAT_VECTORS: Res = SplitVecOp_CONCAT_VECTORS(N);<br>
>> > break;<br>
>> > - case ISD::TRUNCATE: Res = SplitVecOp_TRUNCATE(N); break;<br>
>> > + case ISD::TRUNCATE:<br>
>> > + Res = SplitVecOp_TruncateHelper(N, ISD::TRUNCATE);<br>
>> > + break;<br>
>> > case ISD::FP_ROUND: Res = SplitVecOp_FP_ROUND(N); break;<br>
>> > case ISD::STORE:<br>
>> > Res = SplitVecOp_STORE(cast<StoreSDNode>(N), OpNo);<br>
>> > @@ -1304,20 +1306,32 @@ bool DAGTypeLegalizer::SplitVectorOperan<br>
>> > case ISD::VSELECT:<br>
>> > Res = SplitVecOp_VSELECT(N, OpNo);<br>
>> > break;<br>
>> > - case ISD::CTTZ:<br>
>> > - case ISD::CTLZ:<br>
>> > - case ISD::CTPOP:<br>
>> > - case ISD::FP_EXTEND:<br>
>> > case ISD::FP_TO_SINT:<br>
>> > case ISD::FP_TO_UINT:<br>
>> > + if (N->getValueType(0).bitsLT(N->getOperand(0)->getValueType(0)))<br>
>> > + Res = SplitVecOp_TruncateHelper(N, ISD::TRUNCATE);<br>
>> > + else<br>
>> > + Res = SplitVecOp_UnaryOp(N);<br>
>> > + break;<br>
>> > case ISD::SINT_TO_FP:<br>
>> > case ISD::UINT_TO_FP:<br>
>> > - case ISD::FTRUNC:<br>
>> > + if (N->getValueType(0).bitsLT(N->getOperand(0)->getValueType(0)))<br>
>> > + Res = SplitVecOp_TruncateHelper(N, ISD::FTRUNC);<br>
>> > + else<br>
>> > + Res = SplitVecOp_UnaryOp(N);<br>
>> > + break;<br>
>> > + case ISD::CTTZ:<br>
>> > + case ISD::CTLZ:<br>
>> > + case ISD::CTPOP:<br>
>> > + case ISD::FP_EXTEND:<br>
>> > case ISD::SIGN_EXTEND:<br>
>> > case ISD::ZERO_EXTEND:<br>
>> > case ISD::ANY_EXTEND:<br>
>> > Res = SplitVecOp_UnaryOp(N);<br>
>> > break;<br>
>> > + case ISD::FTRUNC:<br>
>> > + Res = SplitVecOp_TruncateHelper(N, ISD::FTRUNC);<br>
>> > + break;<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > @@ -1581,7 +1595,8 @@ SDValue DAGTypeLegalizer::SplitVecOp_CON<br>
>> > return DAG.getNode(ISD::BUILD_VECTOR, DL, N->getValueType(0), Elts);<br>
>> > }<br>
>> ><br>
>> > -SDValue DAGTypeLegalizer::SplitVecOp_TRUNCATE(SDNode *N) {<br>
>> > +SDValue DAGTypeLegalizer::SplitVecOp_TruncateHelper(SDNode *N,<br>
>> > + unsigned<br>
>> > TruncateOp) {<br>
>> > // The result type is legal, but the input type is illegal. If<br>
>> > splitting<br>
>> > // ends up with the result type of each half still being legal, just<br>
>> > // do that. If, however, that would result in an illegal result<br>
>> > type,<br>
>> > @@ -1624,8 +1639,8 @@ SDValue DAGTypeLegalizer::SplitVecOp_TRU<br>
>> > EVT HalfElementVT = EVT::getIntegerVT(*DAG.getContext(),<br>
>> > InElementSize/2);<br>
>> > EVT HalfVT = EVT::getVectorVT(*DAG.getContext(), HalfElementVT,<br>
>> > NumElements/2);<br>
>> > - SDValue HalfLo = DAG.getNode(ISD::TRUNCATE, DL, HalfVT, InLoVec);<br>
>> > - SDValue HalfHi = DAG.getNode(ISD::TRUNCATE, DL, HalfVT, InHiVec);<br>
>> > + SDValue HalfLo = DAG.getNode(N->getOpcode(), DL, HalfVT, InLoVec);<br>
>> > + SDValue HalfHi = DAG.getNode(N->getOpcode(), DL, HalfVT, InHiVec);<br>
>> > // Concatenate them to get the full intermediate truncation result.<br>
>> > EVT InterVT = EVT::getVectorVT(*DAG.getContext(), HalfElementVT,<br>
>> > NumElements);<br>
>> > SDValue InterVec = DAG.getNode(ISD::CONCAT_VECTORS, DL, InterVT,<br>
>> > HalfLo,<br>
>> > @@ -1634,7 +1649,7 @@ SDValue DAGTypeLegalizer::SplitVecOp_TRU<br>
>> > // type. This should normally be something that ends up being legal<br>
>> > directly,<br>
>> > // but in theory if a target has very wide vectors and an annoyingly<br>
>> > // restricted set of legal types, this split can chain to build<br>
>> > things up.<br>
>> > - return DAG.getNode(ISD::TRUNCATE, DL, OutVT, InterVec);<br>
>> > + return DAG.getNode(TruncateOp, DL, OutVT, InterVec);<br>
>> > }<br>
>> ><br>
>> > SDValue DAGTypeLegalizer::SplitVecOp_VSETCC(SDNode *N) {<br>
>> ><br>
>> > Modified: llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll?rev=233681&r1=233680&r2=233681&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll?rev=233681&r1=233680&r2=233681&view=diff</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll (original)<br>
>> > +++ llvm/trunk/test/CodeGen/AArch64/arm64-convert-v4f64.ll Tue Mar 31<br>
>> > 05:20:58 2015<br>
>> > @@ -3,11 +3,11 @@<br>
>> ><br>
>> > define <4 x i16> @fptosi_v4f64_to_v4i16(<4 x double>* %ptr) {<br>
>> > ; CHECK: fptosi_v4f64_to_v4i16<br>
>> > -; CHECK-DAG: fcvtzs v[[LHS:[0-9]+]].2d, v1.2d<br>
>> > -; CHECK-DAG: fcvtzs v[[RHS:[0-9]+]].2d, v0.2d<br>
>> > -; CHECK-DAG: xtn v[[LHS_NA:[0-9]+]].2s, v[[LHS]].2d<br>
>> > -; CHECK-DAG: xtn v[[RHS_NA:[0-9]+]].2s, v[[RHS]].2d<br>
>> > -; CHECK: uzp1 v0.4h, v[[RHS_NA]].4h, v[[LHS_NA]].4h<br>
>> > +; CHECK-DAG: fcvtzs v[[LHS:[0-9]+]].2d, v0.2d<br>
>> > +; CHECK-DAG: fcvtzs v[[RHS:[0-9]+]].2d, v1.2d<br>
>> > +; CHECK-DAG: xtn v[[MID:[0-9]+]].2s, v[[LHS]].2d<br>
>> > +; CHECK-DAG: xtn2 v[[MID]].4s, v[[RHS]].2d<br>
>> > +; CHECK: xtn v0.4h, v[[MID]].4s<br>
>> > %tmp1 = load <4 x double>, <4 x double>* %ptr<br>
>> > %tmp2 = fptosi <4 x double> %tmp1 to <4 x i16><br>
>> > ret <4 x i16> %tmp2<br>
>> > @@ -19,13 +19,13 @@ define <8 x i8> @fptosi_v4f64_to_v4i8(<8<br>
>> > ; CHECK-DAG: fcvtzs v[[CONV1:[0-9]+]].2d, v1.2d<br>
>> > ; CHECK-DAG: fcvtzs v[[CONV2:[0-9]+]].2d, v2.2d<br>
>> > ; CHECK-DAG: fcvtzs v[[CONV3:[0-9]+]].2d, v3.2d<br>
>> > -; CHECK-DAG: xtn v[[NA0:[0-9]+]].2s, v[[CONV0]].2d<br>
>> > -; CHECK-DAG: xtn v[[NA1:[0-9]+]].2s, v[[CONV1]].2d<br>
>> > ; CHECK-DAG: xtn v[[NA2:[0-9]+]].2s, v[[CONV2]].2d<br>
>> > -; CHECK-DAG: xtn v[[NA3:[0-9]+]].2s, v[[CONV3]].2d<br>
>> > -; CHECK-DAG: uzp1 v[[TMP1:[0-9]+]].4h, v[[CONV1]].4h, v[[CONV0]].4h<br>
>> > -; CHECK-DAG: uzp1 v[[TMP2:[0-9]+]].4h, v[[CONV3]].4h, v[[CONV2]].4h<br>
>> > -; CHECK: uzp1 v0.8b, v[[TMP2]].8b, v[[TMP1]].8b<br>
>> > +; CHECK-DAG: xtn2 v[[NA2]].4s, v[[CONV3]].2d<br>
>> > +; CHECK-DAG: xtn v[[NA0:[0-9]+]].2s, v[[CONV0]].2d<br>
>> > +; CHECK-DAG: xtn2 v[[NA0]].4s, v[[CONV1]].2d<br>
>> > +; CHECK-DAG: xtn v[[TMP1:[0-9]+]].4h, v[[NA0]].4s<br>
>> > +; CHECK-DAG: xtn2 v[[TMP1]].8h, v[[NA2]].4s<br>
>> > +; CHECK: xtn v0.8b, v[[TMP1]].8h<br>
>> > %tmp1 = load <8 x double>, <8 x double>* %ptr<br>
>> > %tmp2 = fptosi <8 x double> %tmp1 to <8 x i8><br>
>> > ret <8 x i8> %tmp2<br>
>> ><br>
>> > Added: llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll?rev=233681&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll?rev=233681&view=auto</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll (added)<br>
>> > +++ llvm/trunk/test/CodeGen/AArch64/vcvt-oversize.ll Tue Mar 31 05:20:58<br>
>> > 2015<br>
>> > @@ -0,0 +1,16 @@<br>
>> > +; RUN: llc -mtriple=aarch64 < %s | FileCheck %s<br>
>> > +<br>
>> > +define <8 x i8> @float_to_i8(<8 x float>* %in) {<br>
>> > +; CHECK-LABEL: float_to_i8:<br>
>> > +; CHECK-DAG: fadd v[[LSB:[0-9]+]].4s, v0.4s, v0.4s<br>
>> > +; CHECK-DAG: fadd v[[MSB:[0-9]+]].4s, v1.4s, v1.4s<br>
>> > +; CHECK-DAG: fcvtzu v[[LSB2:[0-9]+]].4s, v[[LSB]].4s<br>
>> > +; CHECK-DAG: fcvtzu v[[MSB2:[0-9]+]].4s, v[[MSB]].4s<br>
>> > +; CHECK-DAG: xtn v[[TMP:[0-9]+]].4h, v[[LSB]].4s<br>
>> > +; CHECK-DAG: xtn2 v[[TMP]].8h, v[[MSB]].4s<br>
>> > +; CHECK-DAG: xtn v0.8b, v[[TMP]].8h<br>
>> > + %l = load <8 x float>, <8 x float>* %in<br>
>> > + %scale = fmul <8 x float> %l, <float 2.0, float 2.0, float 2.0, float<br>
>> > 2.0, float 2.0, float 2.0, float 2.0, float 2.0><br>
>> > + %conv = fptoui <8 x float> %scale to <8 x i8><br>
>> > + ret <8 x i8> %conv<br>
>> > +}<br>
>> ><br>
>> > Modified: llvm/trunk/test/CodeGen/ARM/vcvt.ll<br>
>> > URL:<br>
>> > <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vcvt.ll?rev=233681&r1=233680&r2=233681&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vcvt.ll?rev=233681&r1=233680&r2=233681&view=diff</a><br>
>> ><br>
>> > ==============================================================================<br>
>> > --- llvm/trunk/test/CodeGen/ARM/vcvt.ll (original)<br>
>> > +++ llvm/trunk/test/CodeGen/ARM/vcvt.ll Tue Mar 31 05:20:58 2015<br>
>> > @@ -180,8 +180,8 @@ define <2 x i64> @fix_float_to_i64(<2 x<br>
>> ><br>
>> > define <4 x i16> @fix_double_to_i16(<4 x double> %in) {<br>
>> > ; CHECK-LABEL: fix_double_to_i16:<br>
>> > -; CHECK: vcvt.s32.f64<br>
>> > -; CHECK: vcvt.s32.f64<br>
>> > +; CHECK: vcvt.u32.f64<br>
>> > +; CHECK: vcvt.u32.f64<br>
>> ><br>
>> > %scale = fmul <4 x double> %in, <double 2.0, double 2.0, double 2.0,<br>
>> > double 2.0><br>
>> > %conv = fptoui <4 x double> %scale to <4 x i16><br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > llvm-commits mailing list<br>
>> > <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>