From 4763267eeee7ad0013d107b895dec1900b4f315f Mon Sep 17 00:00:00 2001 From: Mikael Holmen Date: Tue, 10 Dec 2019 08:09:09 +0100 Subject: [PATCH] [LegalizeTypes] Bugfixes for big-endian targets when handling BITCASTs Summary: This fixes PR44135. The special case when we promote a bitcast from a vector to an int needs special handling when we are on a big-endian target. Prior to this fix, for the added vec_to_int we see the following in the SelectionDAG printouts Type-legalized selection DAG: %bb.1 'foo:bb.1' SelectionDAG has 9 nodes: t0: ch = EntryToken t2: v8i16,ch = CopyFromReg t0, Register:v8i16 %0 t17: v4i32 = bitcast t2 t23: i32 = extract_vector_elt t17, Constant:i32<3> t8: ch,glue = CopyToReg t0, Register:i32 $r0, t23 t9: ch = ARMISD::RET_FLAG t8, Register:i32 $r0, t8:1 and I think here the extract_vector_elt is wrong and extracts the value from the wrong index. The program program should return the 32 bits made up of the elements at index 4 and 5 in the vec6 array, but with t23: i32 = extract_vector_elt t17, Constant:i32<3> as far as I can tell, we will extract values that originally didn't even exist in the vec6 vectore. If we would instead extract the element at index 2 we would get the wanted values. With this fix we insert a right shift after the bitcast in DAGTypeLegalizer::PromoteIntRes_BITCAST which then gives us Type-legalized selection DAG: %bb.1 'vec_to_int:bb.1' SelectionDAG has 9 nodes: t0: ch = EntryToken t2: v8i16,ch = CopyFromReg t0, Register:v8i16 %0 t23: v4i32 = bitcast t2 t27: i32 = extract_vector_elt t23, Constant:i32<2> t8: ch,glue = CopyToReg t0, Register:i32 $r0, t27 t9: ch = ARMISD::RET_FLAG t8, Register:i32 $r0, t8:1 So now we get t27: i32 = extract_vector_elt t23, Constant:i32<2> which is what we want. Similarly, the new int_to_vec testcase exposes a bug where we cast the other direction. Then we instead need to add a left shift before the bitcast on big-endian targets for the bits in the input integer to end up at the exptected place in the vector. Reviewers: bogner, spatel, craig.topper, t.p.northover, dmgreen, efriedma, SjoerdMeijer, samparker Reviewed By: efriedma Subscribers: eli.friedman, bjope, kristof.beyls, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D70942 --- .../CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | 17 ++++++++++++++-- .../CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | 23 +++++++++++++++++----- llvm/test/CodeGen/ARM/legalize-bitcast.ll | 21 +++++++++++--------- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp index 9e4e5ad..a8e7645 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp @@ -339,8 +339,21 @@ SDValue DAGTypeLegalizer::PromoteIntRes_BITCAST(SDNode *N) { // The input is widened to the same size. Convert to the widened value. // Make sure that the outgoing value is not a vector, because this would // make us bitcast between two vectors which are legalized in different ways. - if (NOutVT.bitsEq(NInVT) && !NOutVT.isVector()) - return DAG.getNode(ISD::BITCAST, dl, NOutVT, GetWidenedVector(InOp)); + if (NOutVT.bitsEq(NInVT) && !NOutVT.isVector()) { + SDValue Res = + DAG.getNode(ISD::BITCAST, dl, NOutVT, GetWidenedVector(InOp)); + + // For big endian targets we need to shift the casted value or the + // interesting bits will end up at the wrong place. + if (DAG.getDataLayout().isBigEndian()) { + unsigned ShiftAmt = NInVT.getSizeInBits() - InVT.getSizeInBits(); + EVT ShiftAmtTy = TLI.getShiftAmountTy(NOutVT, DAG.getDataLayout()); + assert(ShiftAmt < NOutVT.getSizeInBits() && "Too large shift amount!"); + Res = DAG.getNode(ISD::SRL, dl, NOutVT, Res, + DAG.getConstant(ShiftAmt, dl, ShiftAmtTy)); + } + return Res; + } // If the output type is also a vector and widening it to the same size // as the widened input type would be a legal type, we can widen the bitcast // and handle the promotion after. diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp index 4090ee5..dd8ccac 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp @@ -3457,7 +3457,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_BITCAST(SDNode *N) { switch (getTypeAction(InVT)) { case TargetLowering::TypeLegal: break; - case TargetLowering::TypePromoteInteger: + case TargetLowering::TypePromoteInteger: { // If the incoming type is a vector that is being promoted, then // we know that the elements are arranged differently and that we // must perform the conversion using a stack slot. @@ -3466,11 +3466,24 @@ SDValue DAGTypeLegalizer::WidenVecRes_BITCAST(SDNode *N) { // If the InOp is promoted to the same size, convert it. Otherwise, // fall out of the switch and widen the promoted input. - InOp = GetPromotedInteger(InOp); - InVT = InOp.getValueType(); - if (WidenVT.bitsEq(InVT)) - return DAG.getNode(ISD::BITCAST, dl, WidenVT, InOp); + SDValue NInOp = GetPromotedInteger(InOp); + EVT NInVT = NInOp.getValueType(); + if (WidenVT.bitsEq(NInVT)) { + // For big endian targets we need to shift the input integer or the + // interesting bits will end up at the wrong place. + if (DAG.getDataLayout().isBigEndian()) { + unsigned ShiftAmt = NInVT.getSizeInBits() - InVT.getSizeInBits(); + EVT ShiftAmtTy = TLI.getShiftAmountTy(NInVT, DAG.getDataLayout()); + assert(ShiftAmt < WidenVT.getSizeInBits() && "Too large shift amount!"); + NInOp = DAG.getNode(ISD::SHL, dl, NInVT, NInOp, + DAG.getConstant(ShiftAmt, dl, ShiftAmtTy)); + } + return DAG.getNode(ISD::BITCAST, dl, WidenVT, NInOp); + } + InOp = NInOp; + InVT = NInVT; break; + } case TargetLowering::TypeSoftenFloat: case TargetLowering::TypePromoteFloat: case TargetLowering::TypeExpandInteger: diff --git a/llvm/test/CodeGen/ARM/legalize-bitcast.ll b/llvm/test/CodeGen/ARM/legalize-bitcast.ll index a5d72aa..478ff98 100644 --- a/llvm/test/CodeGen/ARM/legalize-bitcast.ll +++ b/llvm/test/CodeGen/ARM/legalize-bitcast.ll @@ -24,7 +24,7 @@ define i32 @vec_to_int() { ; CHECK-NEXT: vldmia sp, {d16, d17} @ 16-byte Reload ; CHECK-NEXT: vrev32.16 q9, q8 ; CHECK-NEXT: @ kill: def $d19 killed $d19 killed $q9 -; CHECK-NEXT: vmov.32 r0, d19[1] +; CHECK-NEXT: vmov.32 r0, d19[0] ; CHECK-NEXT: add sp, sp, #28 ; CHECK-NEXT: pop {r4} ; CHECK-NEXT: bx lr @@ -41,14 +41,17 @@ bb.1: define i16 @int_to_vec(i80 %in) { ; CHECK-LABEL: int_to_vec: ; CHECK: @ %bb.0: -; CHECK-NEXT: sub sp, sp, #4 -; CHECK-NEXT: vmov.i32 q8, #0x0 -; CHECK-NEXT: vrev32.16 q8, q8 -; CHECK-NEXT: @ kill: def $d16 killed $d16 killed $q8 -; CHECK-NEXT: vmov.u16 r3, d16[0] -; CHECK-NEXT: str r0, [sp] @ 4-byte Spill -; CHECK-NEXT: mov r0, r3 -; CHECK-NEXT: add sp, sp, #4 +; CHECK-NEXT: mov r3, r1 +; CHECK-NEXT: mov r12, r0 +; CHECK-NEXT: lsl r0, r0, #16 +; CHECK-NEXT: orr r0, r0, r1, lsr #16 +; CHECK-NEXT: @ implicit-def: $d16 +; CHECK-NEXT: vmov.32 d16[0], r0 +; CHECK-NEXT: @ implicit-def: $q9 +; CHECK-NEXT: vmov.f64 d18, d16 +; CHECK-NEXT: vrev32.16 q9, q9 +; CHECK-NEXT: @ kill: def $d18 killed $d18 killed $q9 +; CHECK-NEXT: vmov.u16 r0, d18[0] ; CHECK-NEXT: bx lr %vec = bitcast i80 %in to <5 x i16> %e0 = extractelement <5 x i16> %vec, i32 0 -- 2.7.4