From 2e937620bb58666b663654b470c5d88ef188bce1 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 12 Sep 2025 17:22:19 -0700 Subject: [PATCH 1/2] [LegalizeTypes][X86] Use getShiftAmountConstant in ExpandIntRes_SIGN_EXTEND. This ensures we don't need to fixup the shift amount later. Unfortunately, this enabled the (SRA (SHL X, ShlConst), SraConst) -> (SRA (sext_in_reg X), SraConst - ShlConst) combine in combineShiftRightArithmetic for some cases in is_fpclass-fp80.ll. So we need to also update checkSignTestSetCCCombine to look through sign_extend_inreg to prevent a regression. --- .../SelectionDAG/LegalizeIntegerTypes.cpp | 18 ++++++++---------- llvm/lib/Target/X86/X86ISelLowering.cpp | 8 ++++++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp index 87570e6f44a6f..4671124a37f5d 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp @@ -5088,9 +5088,8 @@ void DAGTypeLegalizer::ExpandIntRes_SIGN_EXTEND(SDNode *N, Lo = DAG.getNode(ISD::SIGN_EXTEND, dl, NVT, N->getOperand(0)); // The high part is obtained by SRA'ing all but one of the bits of low part. unsigned LoSize = NVT.getSizeInBits(); - Hi = DAG.getNode( - ISD::SRA, dl, NVT, Lo, - DAG.getConstant(LoSize - 1, dl, TLI.getPointerTy(DAG.getDataLayout()))); + Hi = DAG.getNode(ISD::SRA, dl, NVT, Lo, + DAG.getShiftAmountConstant(LoSize - 1, NVT, dl)); } else { // For example, extension of an i48 to an i64. The operand type necessarily // promotes to the result type, so will end up being expanded too. @@ -5123,8 +5122,8 @@ ExpandIntRes_SIGN_EXTEND_INREG(SDNode *N, SDValue &Lo, SDValue &Hi) { // The high part gets the sign extension from the lo-part. This handles // things like sextinreg V:i64 from i8. Hi = DAG.getNode(ISD::SRA, dl, Hi.getValueType(), Lo, - DAG.getConstant(Hi.getValueSizeInBits() - 1, dl, - TLI.getPointerTy(DAG.getDataLayout()))); + DAG.getShiftAmountConstant(Hi.getValueSizeInBits() - 1, + Hi.getValueType(), dl)); } else { // For example, extension of an i48 to an i64. Leave the low part alone, // sext_inreg the high part. @@ -5929,14 +5928,13 @@ SDValue DAGTypeLegalizer::ExpandIntOp_STORE(StoreSDNode *N, unsigned OpNo) { if (ExcessBits < NVT.getSizeInBits()) { // Transfer high bits from the top of Lo to the bottom of Hi. - Hi = DAG.getNode(ISD::SHL, dl, NVT, Hi, - DAG.getConstant(NVT.getSizeInBits() - ExcessBits, dl, - TLI.getPointerTy(DAG.getDataLayout()))); + Hi = DAG.getNode( + ISD::SHL, dl, NVT, Hi, + DAG.getShiftAmountConstant(NVT.getSizeInBits() - ExcessBits, NVT, dl)); Hi = DAG.getNode( ISD::OR, dl, NVT, Hi, DAG.getNode(ISD::SRL, dl, NVT, Lo, - DAG.getConstant(ExcessBits, dl, - TLI.getPointerTy(DAG.getDataLayout())))); + DAG.getShiftAmountConstant(ExcessBits, NVT, dl))); } // Store both the high bits and maybe some of the low bits. diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index 3631016b0f5c7..eeb5eb8a262de 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -48396,13 +48396,17 @@ static SDValue checkSignTestSetCCCombine(SDValue Cmp, X86::CondCode &CC, MVT SrcVT = Src.getSimpleValueType(); APInt BitMask = APInt::getSignMask(SrcVT.getScalarSizeInBits()); - // If Src came from a SHL (probably from an expanded SIGN_EXTEND_INREG), then - // peek through and adjust the TEST bit. + // If Src came from a SIGN_EXTEND_INREG or SHL (probably from an expanded + // SIGN_EXTEND_INREG), then peek through and adjust the TEST bit. if (Src.getOpcode() == ISD::SHL) { if (std::optional ShiftAmt = DAG.getValidShiftAmount(Src)) { Src = Src.getOperand(0); BitMask.lshrInPlace(*ShiftAmt); } + } else if (Src.getOpcode() == ISD::SIGN_EXTEND_INREG) { + EVT ExtVT = cast(Src.getOperand(1))->getVT(); + Src = Src.getOperand(0); + BitMask.lshrInPlace(BitMask.getBitWidth() - ExtVT.getScalarSizeInBits()); } SDValue Mask = DAG.getNode(ISD::AND, DL, SrcVT, Src, From 7aaa08b48a9da3086934b9bc613ee2c3a3c6edb4 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 12 Sep 2025 18:54:12 -0700 Subject: [PATCH 2/2] fixup! remove accidental changes. --- llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp index 4671124a37f5d..5967b4eb3769a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp @@ -5928,13 +5928,14 @@ SDValue DAGTypeLegalizer::ExpandIntOp_STORE(StoreSDNode *N, unsigned OpNo) { if (ExcessBits < NVT.getSizeInBits()) { // Transfer high bits from the top of Lo to the bottom of Hi. - Hi = DAG.getNode( - ISD::SHL, dl, NVT, Hi, - DAG.getShiftAmountConstant(NVT.getSizeInBits() - ExcessBits, NVT, dl)); + Hi = DAG.getNode(ISD::SHL, dl, NVT, Hi, + DAG.getConstant(NVT.getSizeInBits() - ExcessBits, dl, + TLI.getPointerTy(DAG.getDataLayout()))); Hi = DAG.getNode( ISD::OR, dl, NVT, Hi, DAG.getNode(ISD::SRL, dl, NVT, Lo, - DAG.getShiftAmountConstant(ExcessBits, NVT, dl))); + DAG.getConstant(ExcessBits, dl, + TLI.getPointerTy(DAG.getDataLayout())))); } // Store both the high bits and maybe some of the low bits.