Skip to content

Commit 9b2fc66

Browse files
authored
[SDAG] Harden assumption in getMemsetStringVal (#126207)
In 5235973, an ICE was fixed in getMemsetStringVal where f128 wasn't handled. It was noted at the time [1] that the code below this also looks suspect, since it assumes the element type of VT is either an f32 or f64. This part of getMemsetStringVal relates to memcpy operations where the source is a copy from a zero constant. The VT in question is determined by TargetLowering::findOptimalMemOpLowering, which in turn calls a further TLI hook getOptimalMemOpType. For AArch64, getOptimalMemOpType returns either a v16i8, f128, i64, i32 or Other. For Other, TargetLowering::findOptimalMemOpLowering will then pick an integer VT. So on AArch64 at least, I don't believe the suspect code can be reached. For other targets, ARM and x86 are the only ones that return a FP vector type from getOptimalMemOpType. For both targets, the only such type is v2f64, but given f64 is already handled it should also be fine. To defend this, I considered adding an assert as mentioned in [1], but given getConstantFP handles vector types, I figured using this to fully handle the FP types makes the code simpler and more robust. For test coverage I added unreachables to both of the branches handling FP types in this code, but found neither fired with check-llvm across all targets. Test coverage was added to llvm/test/CodeGen/AArch64/memcpy-f128.ll in 5235973 to defend ICE on f128, but at some point it stopped hitting this code. AArch64TargetLowering::getOptimalMemOpType was updated in 2920061, so I suspect this is when it happened, although I haven't verified this. Although I did find by updating the test to disable NEON, getOptimalMemOpType returns an f128 and the branch is once again hit. For the final branch noted as suspect in [1], as far as I can tell this has never had any test coverage, so I've added a test to the ARM backend for this. Fixes: #20521 [1]
1 parent df62441 commit 9b2fc66

File tree

1 file changed

+2
-11
lines changed

1 file changed

+2
-11
lines changed

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8015,17 +8015,8 @@ static SDValue getMemsetStringVal(EVT VT, const SDLoc &dl, SelectionDAG &DAG,
80158015
if (Slice.Array == nullptr) {
80168016
if (VT.isInteger())
80178017
return DAG.getConstant(0, dl, VT);
8018-
if (VT == MVT::f32 || VT == MVT::f64 || VT == MVT::f128)
8019-
return DAG.getConstantFP(0.0, dl, VT);
8020-
if (VT.isVector()) {
8021-
unsigned NumElts = VT.getVectorNumElements();
8022-
MVT EltVT = (VT.getVectorElementType() == MVT::f32) ? MVT::i32 : MVT::i64;
8023-
return DAG.getNode(ISD::BITCAST, dl, VT,
8024-
DAG.getConstant(0, dl,
8025-
EVT::getVectorVT(*DAG.getContext(),
8026-
EltVT, NumElts)));
8027-
}
8028-
llvm_unreachable("Expected type!");
8018+
return DAG.getNode(ISD::BITCAST, dl, VT,
8019+
DAG.getConstant(0, dl, VT.changeTypeToInteger()));
80298020
}
80308021

80318022
assert(!VT.isVector() && "Can't handle vector type here!");

0 commit comments

Comments
 (0)