[llvm] 2ce2562 - [RISCV][SelectionDAG] Add a hook to sign extend i32 ConstantInt operands of phis on RV64.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 11 14:40:20 PDT 2022
Author: Craig Topper
Date: 2022-04-11T14:38:39-07:00
New Revision: 2ce25628760ab2bbab764fc48c086704b4d6e279
URL: https://github.com/llvm/llvm-project/commit/2ce25628760ab2bbab764fc48c086704b4d6e279
DIFF: https://github.com/llvm/llvm-project/commit/2ce25628760ab2bbab764fc48c086704b4d6e279.diff
LOG: [RISCV][SelectionDAG] Add a hook to sign extend i32 ConstantInt operands of phis on RV64.
Materializing constants on RISCV is simpler if the constant is sign
extended from i32. By default i32 constant operands of phis are
zero extended.
This patch adds a hook to allow RISCV to override this for i32. We
have an existing isSExtCheaperThanZExt, but it operates on EVT which
we don't have at these places in the code.
Reviewed By: efriedma
Differential Revision: https://reviews.llvm.org/D122951
Added:
Modified:
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
llvm/lib/Target/RISCV/RISCVISelLowering.h
llvm/test/CodeGen/RISCV/aext-to-sext.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index df7d9a5311e9f..8b7904b5e60f2 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -2683,6 +2683,10 @@ class TargetLoweringBase {
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
index e39ad73cf20c9..5ed55b034a183 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -464,7 +464,11 @@ void FunctionLoweringInfo::ComputePHILiveOutRegInfo(const PHINode *PN) {
}
if (ConstantInt *CI = dyn_cast<ConstantInt>(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 @@ void FunctionLoweringInfo::ComputePHILiveOutRegInfo(const PHINode *PN) {
}
if (ConstantInt *CI = dyn_cast<ConstantInt>(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
index 3fdb688506ca7..12827a84249ce 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -10652,6 +10652,7 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
/// the end.
void
SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
+ const TargetLowering &TLI = DAG.getTargetLoweringInfo();
const Instruction *TI = LLVMBB->getTerminator();
SmallPtrSet<MachineBasicBlock *, 4> SuccsHandled;
@@ -10689,10 +10690,12 @@ SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
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<ConstantInt>(PHIOp) ? ISD::ZERO_EXTEND : ISD::ANY_EXTEND;
+ ISD::NodeType ExtendType = ISD::ANY_EXTEND;
+ if (auto *CI = dyn_cast<ConstantInt>(C))
+ ExtendType = TLI.signExtendConstant(CI) ? ISD::SIGN_EXTEND
+ : ISD::ZERO_EXTEND;
CopyValueToVirtualRegister(C, RegOut, ExtendType);
}
Reg = RegOut;
@@ -10713,7 +10716,6 @@ SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
// Remember that this register needs to added to the machine PHI node as
// the input for this MBB.
SmallVector<EVT, 4> 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.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index be54958c4934d..e4e2263e09ec0 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -1218,6 +1218,10 @@ bool RISCVTargetLowering::isSExtCheaperThanZExt(EVT SrcVT, EVT DstVT) const {
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/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h
index 9f1200780912a..f805c2ba26475 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -343,6 +343,7 @@ class RISCVTargetLowering : public TargetLowering {
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/test/CodeGen/RISCV/aext-to-sext.ll b/llvm/test/CodeGen/RISCV/aext-to-sext.ll
index b5455973c7840..d866b4b453e58 100644
--- a/llvm/test/CodeGen/RISCV/aext-to-sext.ll
+++ b/llvm/test/CodeGen/RISCV/aext-to-sext.ll
@@ -76,29 +76,18 @@ bar:
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
More information about the llvm-commits
mailing list