Skip to content

Commit 8117c58

Browse files
committed
[DAGCombiner] Freeze maybe poison operands when folding select to logic
Just like for regular IR we need to treat SELECT as conditionally blocking poison. So (unless the condition itself is poison) the result is only poison if the selected true/false value is poison. Thus, when doing DAG combines that turn SELECT into arithmetic/logical operations (e.g. AND/OR) we need to make sure that the new operations aren't more poisonous. One way to do that is to use FREEZE to make sure the operands aren't posion. This patch aims at fixing the kind of miscompiles reported in #84653 and #85190 Solution is to make sure that we insert FREEZE, if needed to make the fold sound, when using the foldBoolSelectToLogic and foldVSelectToSignBitSplatMask DAG combines.
1 parent 9b02b75 commit 8117c58

20 files changed

+794
-866
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11501,28 +11501,28 @@ static SDValue foldBoolSelectToLogic(SDNode *N, const SDLoc &DL,
1150111501
if (VT != Cond.getValueType() || VT.getScalarSizeInBits() != 1)
1150211502
return SDValue();
1150311503

11504-
// select Cond, Cond, F --> or Cond, F
11505-
// select Cond, 1, F --> or Cond, F
11504+
// select Cond, Cond, F --> or Cond, freeze(F)
11505+
// select Cond, 1, F --> or Cond, freeze(F)
1150611506
if (Cond == T || isOneOrOneSplat(T, /* AllowUndefs */ true))
11507-
return matcher.getNode(ISD::OR, DL, VT, Cond, F);
11507+
return matcher.getNode(ISD::OR, DL, VT, Cond, DAG.getFreeze(F));
1150811508

11509-
// select Cond, T, Cond --> and Cond, T
11510-
// select Cond, T, 0 --> and Cond, T
11509+
// select Cond, T, Cond --> and Cond, freeze(T)
11510+
// select Cond, T, 0 --> and Cond, freeze(T)
1151111511
if (Cond == F || isNullOrNullSplat(F, /* AllowUndefs */ true))
11512-
return matcher.getNode(ISD::AND, DL, VT, Cond, T);
11512+
return matcher.getNode(ISD::AND, DL, VT, Cond, DAG.getFreeze(T));
1151311513

11514-
// select Cond, T, 1 --> or (not Cond), T
11514+
// select Cond, T, 1 --> or (not Cond), freeze(T)
1151511515
if (isOneOrOneSplat(F, /* AllowUndefs */ true)) {
1151611516
SDValue NotCond =
1151711517
matcher.getNode(ISD::XOR, DL, VT, Cond, DAG.getAllOnesConstant(DL, VT));
11518-
return matcher.getNode(ISD::OR, DL, VT, NotCond, T);
11518+
return matcher.getNode(ISD::OR, DL, VT, NotCond, DAG.getFreeze(T));
1151911519
}
1152011520

11521-
// select Cond, 0, F --> and (not Cond), F
11521+
// select Cond, 0, F --> and (not Cond), freeze(F)
1152211522
if (isNullOrNullSplat(T, /* AllowUndefs */ true)) {
1152311523
SDValue NotCond =
1152411524
matcher.getNode(ISD::XOR, DL, VT, Cond, DAG.getAllOnesConstant(DL, VT));
11525-
return matcher.getNode(ISD::AND, DL, VT, NotCond, F);
11525+
return matcher.getNode(ISD::AND, DL, VT, NotCond, DAG.getFreeze(F));
1152611526
}
1152711527

1152811528
return SDValue();
@@ -11550,37 +11550,37 @@ static SDValue foldVSelectToSignBitSplatMask(SDNode *N, SelectionDAG &DAG) {
1155011550
else
1155111551
return SDValue();
1155211552

11553-
// (Cond0 s< 0) ? N1 : 0 --> (Cond0 s>> BW-1) & N1
11553+
// (Cond0 s< 0) ? N1 : 0 --> (Cond0 s>> BW-1) & freeze(N1)
1155411554
if (isNullOrNullSplat(N2)) {
1155511555
SDLoc DL(N);
1155611556
SDValue ShiftAmt = DAG.getConstant(VT.getScalarSizeInBits() - 1, DL, VT);
1155711557
SDValue Sra = DAG.getNode(ISD::SRA, DL, VT, Cond0, ShiftAmt);
11558-
return DAG.getNode(ISD::AND, DL, VT, Sra, N1);
11558+
return DAG.getNode(ISD::AND, DL, VT, Sra, DAG.getFreeze(N1));
1155911559
}
1156011560

11561-
// (Cond0 s< 0) ? -1 : N2 --> (Cond0 s>> BW-1) | N2
11561+
// (Cond0 s< 0) ? -1 : N2 --> (Cond0 s>> BW-1) | freeze(N2)
1156211562
if (isAllOnesOrAllOnesSplat(N1)) {
1156311563
SDLoc DL(N);
1156411564
SDValue ShiftAmt = DAG.getConstant(VT.getScalarSizeInBits() - 1, DL, VT);
1156511565
SDValue Sra = DAG.getNode(ISD::SRA, DL, VT, Cond0, ShiftAmt);
11566-
return DAG.getNode(ISD::OR, DL, VT, Sra, N2);
11566+
return DAG.getNode(ISD::OR, DL, VT, Sra, DAG.getFreeze(N2));
1156711567
}
1156811568

1156911569
// If we have to invert the sign bit mask, only do that transform if the
1157011570
// target has a bitwise 'and not' instruction (the invert is free).
11571-
// (Cond0 s< -0) ? 0 : N2 --> ~(Cond0 s>> BW-1) & N2
11571+
// (Cond0 s< -0) ? 0 : N2 --> ~(Cond0 s>> BW-1) & freeze(N2)
1157211572
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
1157311573
if (isNullOrNullSplat(N1) && TLI.hasAndNot(N1)) {
1157411574
SDLoc DL(N);
1157511575
SDValue ShiftAmt = DAG.getConstant(VT.getScalarSizeInBits() - 1, DL, VT);
1157611576
SDValue Sra = DAG.getNode(ISD::SRA, DL, VT, Cond0, ShiftAmt);
1157711577
SDValue Not = DAG.getNOT(DL, Sra, VT);
11578-
return DAG.getNode(ISD::AND, DL, VT, Not, N2);
11578+
return DAG.getNode(ISD::AND, DL, VT, Not, DAG.getFreeze(N2));
1157911579
}
1158011580

1158111581
// TODO: There's another pattern in this family, but it may require
1158211582
// implementing hasOrNot() to check for profitability:
11583-
// (Cond0 s> -1) ? -1 : N2 --> ~(Cond0 s>> BW-1) | N2
11583+
// (Cond0 s> -1) ? -1 : N2 --> ~(Cond0 s>> BW-1) | freeze(N2)
1158411584

1158511585
return SDValue();
1158611586
}

llvm/test/CodeGen/AArch64/complex-deinterleaving-reductions-predicated-scalable.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ define %"class.std::complex" @complex_mul_predicated_x2_v2f64(ptr %a, ptr %b, pt
229229
; CHECK-NEXT: mov z6.d, z1.d
230230
; CHECK-NEXT: mov z7.d, z0.d
231231
; CHECK-NEXT: add x2, x2, x11
232-
; CHECK-NEXT: cmpne p1.d, p1/z, z2.d, #0
232+
; CHECK-NEXT: cmpne p2.d, p0/z, z2.d, #0
233+
; CHECK-NEXT: and p1.b, p1/z, p1.b, p2.b
233234
; CHECK-NEXT: zip2 p2.d, p1.d, p1.d
234235
; CHECK-NEXT: zip1 p1.d, p1.d, p1.d
235236
; CHECK-NEXT: ld1d { z2.d }, p2/z, [x0, #1, mul vl]

0 commit comments

Comments
 (0)