diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h --- a/llvm/include/llvm/CodeGen/TargetLowering.h +++ b/llvm/include/llvm/CodeGen/TargetLowering.h @@ -2677,6 +2677,10 @@ return false; } + /// Return true if this constant should be sign extended when promoting to + /// a larger type. + virtual bool signExtendConstant(const ConstantInt *C) const { return false; } + /// Return true if sinking I's operands to the same basic block as I is /// profitable, e.g. because the operands can be folded into a target /// instruction during instruction selection. After calling the function diff --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp --- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp @@ -464,7 +464,11 @@ } if (ConstantInt *CI = dyn_cast(V)) { - APInt Val = CI->getValue().zextOrSelf(BitWidth); + APInt Val; + if (TLI->signExtendConstant(CI)) + Val = CI->getValue().sextOrSelf(BitWidth); + else + Val = CI->getValue().zextOrSelf(BitWidth); DestLOI.NumSignBits = Val.getNumSignBits(); DestLOI.Known = KnownBits::makeConstant(Val); } else { @@ -496,7 +500,11 @@ } if (ConstantInt *CI = dyn_cast(V)) { - APInt Val = CI->getValue().zextOrSelf(BitWidth); + APInt Val; + if (TLI->signExtendConstant(CI)) + Val = CI->getValue().sextOrSelf(BitWidth); + else + Val = CI->getValue().zextOrSelf(BitWidth); DestLOI.NumSignBits = std::min(DestLOI.NumSignBits, Val.getNumSignBits()); DestLOI.Known.Zero &= ~Val; DestLOI.Known.One &= Val; diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -10613,6 +10613,7 @@ /// the end. void SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) { + const TargetLowering &TLI = DAG.getTargetLoweringInfo(); const Instruction *TI = LLVMBB->getTerminator(); SmallPtrSet SuccsHandled; @@ -10650,10 +10651,12 @@ unsigned &RegOut = ConstantsOut[C]; if (RegOut == 0) { RegOut = FuncInfo.CreateRegs(C); - // We need to zero extend ConstantInt phi operands to match + // We need to zero/sign extend ConstantInt phi operands to match // assumptions in FunctionLoweringInfo::ComputePHILiveOutRegInfo. - ISD::NodeType ExtendType = - isa(PHIOp) ? ISD::ZERO_EXTEND : ISD::ANY_EXTEND; + ISD::NodeType ExtendType = ISD::ANY_EXTEND; + if (auto *CI = dyn_cast(C)) + ExtendType = TLI.signExtendConstant(CI) ? ISD::SIGN_EXTEND + : ISD::ZERO_EXTEND; CopyValueToVirtualRegister(C, RegOut, ExtendType); } Reg = RegOut; @@ -10674,7 +10677,6 @@ // Remember that this register needs to added to the machine PHI node as // the input for this MBB. SmallVector ValueVTs; - const TargetLowering &TLI = DAG.getTargetLoweringInfo(); ComputeValueVTs(TLI, DAG.getDataLayout(), PN.getType(), ValueVTs); for (unsigned vti = 0, vte = ValueVTs.size(); vti != vte; ++vti) { EVT VT = ValueVTs[vti]; diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h --- a/llvm/lib/Target/RISCV/RISCVISelLowering.h +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h @@ -343,6 +343,7 @@ bool isTruncateFree(EVT SrcVT, EVT DstVT) const override; bool isZExtFree(SDValue Val, EVT VT2) const override; bool isSExtCheaperThanZExt(EVT SrcVT, EVT DstVT) const override; + bool signExtendConstant(const ConstantInt *CI) const override; bool isCheapToSpeculateCttz() const override; bool isCheapToSpeculateCtlz() const override; bool hasAndNotCompare(SDValue Y) const override; diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -1210,6 +1210,10 @@ return Subtarget.is64Bit() && SrcVT == MVT::i32 && DstVT == MVT::i64; } +bool RISCVTargetLowering::signExtendConstant(const ConstantInt *CI) const { + return Subtarget.is64Bit() && CI->getType()->isIntegerTy(32); +} + bool RISCVTargetLowering::isCheapToSpeculateCttz() const { return Subtarget.hasStdExtZbb(); } diff --git a/llvm/test/CodeGen/RISCV/aext-to-sext.ll b/llvm/test/CodeGen/RISCV/aext-to-sext.ll --- a/llvm/test/CodeGen/RISCV/aext-to-sext.ll +++ b/llvm/test/CodeGen/RISCV/aext-to-sext.ll @@ -76,29 +76,18 @@ ret i32 %b } -; The code that inserts AssertZExt based on predecessor information assumes -; constants are zero extended for phi incoming values so an AssertZExt is -; created in 'merge' allowing the zext to be removed. -; SelectionDAG::getNode treats any_extend of i32 constants as sext for RISCV. -; This code used to miscompile because the code that creates phi incoming values -; in the predecessors created any_extend for the constants which then gets -; treated as a sext by getNode. This made the removal of the zext incorrect. -; SelectionDAGBuilder now creates a zero_extend instead of an any_extend to -; match the assumption. -; FIXME: RISCV would prefer constant inputs to phis to be sign extended. -define i64 @miscompile(i32 %c) { -; RV64I-LABEL: miscompile: +; We prefer to sign extend i32 constants for phis. The default behavior in +; SelectionDAGBuilder is zero extend we have a target hook to override it. +define i64 @sext_phi_constants(i32 signext %c) { +; RV64I-LABEL: sext_phi_constants: ; RV64I: # %bb.0: -; RV64I-NEXT: sext.w a0, a0 -; RV64I-NEXT: beqz a0, .LBB2_2 -; RV64I-NEXT: # %bb.1: -; RV64I-NEXT: li a0, -1 +; RV64I-NEXT: li a1, -1 +; RV64I-NEXT: bnez a0, .LBB2_2 +; RV64I-NEXT: # %bb.1: # %iffalse +; RV64I-NEXT: li a1, -2 +; RV64I-NEXT: .LBB2_2: # %merge +; RV64I-NEXT: slli a0, a1, 32 ; RV64I-NEXT: srli a0, a0, 32 -; RV64I-NEXT: ret -; RV64I-NEXT: .LBB2_2: # %iffalse -; RV64I-NEXT: li a0, 1 -; RV64I-NEXT: slli a0, a0, 32 -; RV64I-NEXT: addi a0, a0, -2 ; RV64I-NEXT: ret %a = icmp ne i32 %c, 0 br i1 %a, label %iftrue, label %iffalse