[llvm-commits] [llvm] r140134 - in /llvm/trunk: include/llvm/MC/MCInstrDesc.h lib/CodeGen/SelectionDAG/InstrEmitter.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Target/ARM/ARMISelLowering.cpp lib/Target/ARM/ARMInstrInfo.td lib/Target/ARM/ARMInstrThumb2.td test/CodeGen/ARM/2011-09-19-cpsr.ll utils/TableGen/CodeGenInstruction.cpp utils/TableGen/CodeGenInstruction.h utils/TableGen/InstrInfoEmitter.cpp
Andrew Trick
atrick at apple.com
Mon Sep 19 20:17:40 PDT 2011
Author: atrick
Date: Mon Sep 19 22:17:40 2011
New Revision: 140134
URL: http://llvm.org/viewvc/llvm-project?rev=140134&view=rev
Log:
ARM isel bug fix for adds/subs operands.
Modified ARMISelLowering::AdjustInstrPostInstrSelection to handle the
full gamut of CPSR defs/uses including instructins whose "optional"
cc_out operand is not really optional. This allowed removal of the
hasPostISelHook to simplify the .td files and make the implementation
more robust.
Fixes rdar://10137436: sqlite3 miscompile
Added:
llvm/trunk/test/CodeGen/ARM/2011-09-19-cpsr.ll
Modified:
llvm/trunk/include/llvm/MC/MCInstrDesc.h
llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
llvm/trunk/utils/TableGen/CodeGenInstruction.cpp
llvm/trunk/utils/TableGen/CodeGenInstruction.h
llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp
Modified: llvm/trunk/include/llvm/MC/MCInstrDesc.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrDesc.h?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCInstrDesc.h (original)
+++ llvm/trunk/include/llvm/MC/MCInstrDesc.h Mon Sep 19 22:17:40 2011
@@ -477,14 +477,6 @@
return Flags & (1 << MCID::UsesCustomInserter);
}
- /// hasPostISelHook - Return true if this instruction requires *adjustment*
- /// after instruction selection by calling a target hook. For example, this
- /// can be used to fill in ARM 's' optional operand depending on whether
- /// the conditional flag register is used.
- bool hasPostISelHook() const {
- return Flags & (1 << MCID::HasPostISelHook);
- }
-
/// isRematerializable - Returns true if this instruction is a candidate for
/// remat. This flag is deprecated, please don't use it anymore. If this
/// flag is set, the isReallyTriviallyReMaterializable() method is called to
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/InstrEmitter.cpp Mon Sep 19 22:17:40 2011
@@ -763,8 +763,7 @@
}
// Run post-isel target hook to adjust this instruction if needed.
- if (II.hasPostISelHook())
- TLI->AdjustInstrPostInstrSelection(MI, Node);
+ TLI->AdjustInstrPostInstrSelection(MI, Node);
}
/// EmitSpecialNode - Generate machine code for a target-independent node and
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon Sep 19 22:17:40 2011
@@ -179,12 +179,7 @@
void TargetLowering::AdjustInstrPostInstrSelection(MachineInstr *MI,
SDNode *Node) const {
-#ifndef NDEBUG
- dbgs() << "If a target marks an instruction with "
- "'hasPostISelHook', it must implement "
- "TargetLowering::AdjustInstrPostInstrSelection!";
-#endif
- llvm_unreachable(0);
+ // Do nothing unless the target overrides it.
}
//===----------------------------------------------------------------------===//
Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Mon Sep 19 22:17:40 2011
@@ -5752,27 +5752,68 @@
}
}
+/// Generally, ARM instructions may be optionally encoded with a 's'
+/// bit. However, some opcodes have a compact encoding that forces an implicit
+/// 's' bit. List these exceptions here.
+static bool hasForcedCPSRDef(const MCInstrDesc &MCID) {
+ switch (MCID.getOpcode()) {
+ case ARM::t2ADDSri:
+ case ARM::t2ADDSrr:
+ case ARM::t2ADDSrs:
+ case ARM::t2SUBSri:
+ case ARM::t2SUBSrr:
+ case ARM::t2SUBSrs:
+ return true;
+ }
+ return false;
+}
+
void ARMTargetLowering::AdjustInstrPostInstrSelection(MachineInstr *MI,
SDNode *Node) const {
- // Adjust potentially 's' setting instructions after isel, i.e. ADC, SBC,
- // RSB, RSC. Coming out of isel, they have an implicit CPSR def, but the
- // optional operand is not filled in. If the carry bit is used, then change
- // the optional operand to CPSR. Otherwise, remove the CPSR implicit def.
+ // Adjust potentially 's' setting instructions after isel, i.e. ADC, SBC, RSB,
+ // RSC. Coming out of isel, they have an implicit CPSR def, but the optional
+ // operand is still set to noreg. If needed, set the optional operand's
+ // register to CPSR, and remove the redundant implicit def.
+
const MCInstrDesc &MCID = MI->getDesc();
- if (Node->hasAnyUseOfValue(1)) {
- MachineOperand &MO = MI->getOperand(MCID.getNumOperands() - 1);
- MO.setReg(ARM::CPSR);
- MO.setIsDef(true);
- } else {
- for (unsigned i = MCID.getNumOperands(), e = MI->getNumOperands();
- i != e; ++i) {
- const MachineOperand &MO = MI->getOperand(i);
- if (MO.isReg() && MO.isDef() && MO.getReg() == ARM::CPSR) {
- MI->RemoveOperand(i);
- break;
- }
+ unsigned ccOutIdx = MCID.getNumOperands() - 1;
+ bool forcedCPSR = hasForcedCPSRDef(MCID);
+
+ // Any ARM instruction that sets the 's' bit should specify an optional
+ // "cc_out" operand in the last operand position.
+ if (!MCID.hasOptionalDef() || !MCID.OpInfo[ccOutIdx].isOptionalDef()) {
+ assert(!forcedCPSR && "Optional cc_out operand required");
+ return;
+ }
+ // Look for an implicit def of CPSR added by MachineInstr ctor.
+ bool definesCPSR = false;
+ bool deadCPSR = false;
+ for (unsigned i = MCID.getNumOperands(), e = MI->getNumOperands();
+ i != e; ++i) {
+ const MachineOperand &MO = MI->getOperand(i);
+ if (MO.isReg() && MO.isDef() && MO.getReg() == ARM::CPSR) {
+ definesCPSR = true;
+ if (MO.isDead())
+ deadCPSR = true;
+ MI->RemoveOperand(i);
+ break;
}
}
+ if (!definesCPSR) {
+ assert(!forcedCPSR && "Optional cc_out operand required");
+ return;
+ }
+ assert(deadCPSR == !Node->hasAnyUseOfValue(1) && "inconsistent dead flag");
+
+ // If possible, select the encoding that does not set the 's' bit.
+ if (deadCPSR && !forcedCPSR)
+ return;
+
+ MachineOperand &MO = MI->getOperand(ccOutIdx);
+ MO.setReg(ARM::CPSR);
+ MO.setIsDef(true);
+ if (deadCPSR)
+ MO.setIsDead();
}
//===----------------------------------------------------------------------===//
Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMInstrInfo.td (original)
+++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.td Mon Sep 19 22:17:40 2011
@@ -1026,7 +1026,7 @@
}
/// AsI1_rbin_s_is - Same as AsI1_rbin_s_is except it sets 's' bit by default.
-let hasPostISelHook = 1, isCodeGenOnly = 1, Defs = [CPSR] in {
+let isCodeGenOnly = 1, Defs = [CPSR] in {
multiclass AsI1_rbin_s_is<bits<4> opcod, string opc,
InstrItinClass iii, InstrItinClass iir, InstrItinClass iis,
PatFrag opnode, bit Commutable = 0> {
@@ -1090,7 +1090,7 @@
}
/// AsI1_bin_s_irs - Same as AsI1_bin_irs except it sets the 's' bit by default.
-let hasPostISelHook = 1, isCodeGenOnly = 1, Defs = [CPSR] in {
+let isCodeGenOnly = 1, Defs = [CPSR] in {
multiclass AsI1_bin_s_irs<bits<4> opcod, string opc,
InstrItinClass iii, InstrItinClass iir, InstrItinClass iis,
PatFrag opnode, bit Commutable = 0> {
@@ -1278,7 +1278,7 @@
/// AI1_adde_sube_irs - Define instructions and patterns for adde and sube.
multiclass AI1_adde_sube_irs<bits<4> opcod, string opc, PatFrag opnode,
string baseOpc, bit Commutable = 0> {
- let hasPostISelHook = 1, Defs = [CPSR], Uses = [CPSR] in {
+ let Defs = [CPSR], Uses = [CPSR] in {
def ri : AsI1<opcod, (outs GPR:$Rd), (ins GPR:$Rn, so_imm:$imm),
DPFrm, IIC_iALUi, opc, "\t$Rd, $Rn, $imm",
[(set GPR:$Rd, CPSR, (opnode GPR:$Rn, so_imm:$imm, CPSR))]>,
@@ -1366,7 +1366,7 @@
/// AI1_rsc_irs - Define instructions and patterns for rsc
multiclass AI1_rsc_irs<bits<4> opcod, string opc, PatFrag opnode,
string baseOpc> {
- let hasPostISelHook = 1, Defs = [CPSR], Uses = [CPSR] in {
+ let Defs = [CPSR], Uses = [CPSR] in {
def ri : AsI1<opcod, (outs GPR:$Rd), (ins GPR:$Rn, so_imm:$imm),
DPFrm, IIC_iALUi, opc, "\t$Rd, $Rn, $imm",
[(set GPR:$Rd, CPSR, (opnode so_imm:$imm, GPR:$Rn, CPSR))]>,
Modified: llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td (original)
+++ llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td Mon Sep 19 22:17:40 2011
@@ -592,7 +592,7 @@
/// T2I_bin_s_irs - Similar to T2I_bin_irs except it sets the 's' bit so the
/// instruction modifies the CPSR register.
-let hasPostISelHook = 1, isCodeGenOnly = 1, Defs = [CPSR] in {
+let isCodeGenOnly = 1, Defs = [CPSR] in {
multiclass T2I_bin_s_irs<bits<4> opcod, string opc,
InstrItinClass iii, InstrItinClass iir, InstrItinClass iis,
PatFrag opnode, bit Commutable = 0> {
@@ -738,7 +738,7 @@
/// T2I_rbin_s_is - Same as T2I_rbin_irs except sets 's' bit and the register
/// version is not needed since this is only for codegen.
-let hasPostISelHook = 1, isCodeGenOnly = 1, Defs = [CPSR] in {
+let isCodeGenOnly = 1, Defs = [CPSR] in {
multiclass T2I_rbin_s_is<bits<4> opcod, string opc, PatFrag opnode> {
// shifted imm
def ri : T2sTwoRegImm<
@@ -1846,12 +1846,10 @@
IIC_iALUi, IIC_iALUr, IIC_iALUsi,
BinOpFrag<(ARMsubc node:$LHS, node:$RHS)>>;
-let hasPostISelHook = 1 in {
defm t2ADC : T2I_adde_sube_irs<0b1010, "adc",
BinOpWithFlagFrag<(ARMadde node:$LHS, node:$RHS, node:$FLAG)>, 1>;
defm t2SBC : T2I_adde_sube_irs<0b1011, "sbc",
BinOpWithFlagFrag<(ARMsube node:$LHS, node:$RHS, node:$FLAG)>>;
-}
// RSB
defm t2RSB : T2I_rbin_irs <0b1110, "rsb",
Added: llvm/trunk/test/CodeGen/ARM/2011-09-19-cpsr.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2011-09-19-cpsr.ll?rev=140134&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/2011-09-19-cpsr.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/2011-09-19-cpsr.ll Mon Sep 19 22:17:40 2011
@@ -0,0 +1,54 @@
+; RUN: llc -march=thumb -mcpu=cortex-a8 < %s
+; rdar://problem/10137436: sqlite3 miscompile
+;
+; CHECK: subs
+; CHECK: cmp
+; CHECK: it
+
+target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32"
+target triple = "thumbv7-apple-ios4.0.0"
+
+declare i8* @__memset_chk(i8*, i32, i32, i32) nounwind
+
+define hidden fastcc i32 @sqlite3VdbeExec(i32* %p) nounwind {
+entry:
+ br label %sqlite3VarintLen.exit7424
+
+sqlite3VarintLen.exit7424: ; preds = %do.body.i7423
+ br label %do.body.i
+
+do.body.i: ; preds = %do.body.i, %sqlite3VarintLen.exit7424
+ br i1 undef, label %do.body.i, label %sqlite3VarintLen.exit
+
+sqlite3VarintLen.exit: ; preds = %do.body.i
+ %sub2322 = add i64 undef, undef
+ br i1 undef, label %too_big, label %if.end2327
+
+if.end2327: ; preds = %sqlite3VarintLen.exit
+ br i1 undef, label %if.end2341, label %no_mem
+
+if.end2341: ; preds = %if.end2327
+ br label %for.body2355
+
+for.body2355: ; preds = %for.body2355, %if.end2341
+ %add2366 = add nsw i32 undef, undef
+ br i1 undef, label %for.body2377, label %for.body2355
+
+for.body2377: ; preds = %for.body2355
+ %conv23836154 = zext i32 %add2366 to i64
+ %sub2384 = sub i64 %sub2322, %conv23836154
+ %conv2385 = trunc i64 %sub2384 to i32
+ %len.0.i = select i1 undef, i32 %conv2385, i32 undef
+ %sub.i7384 = sub nsw i32 %len.0.i, 0
+ %call.i.i7385 = call i8* @__memset_chk(i8* undef, i32 0, i32 %sub.i7384, i32 undef) nounwind
+ unreachable
+
+too_big: ; preds = %sqlite3VarintLen.exit
+ unreachable
+
+no_mem: ; preds = %if.end2327, %for.body, %entry.no_mem_crit_edge
+ unreachable
+
+sqlite3ErrStr.exit: ; preds = %if.then82
+ unreachable
+}
Modified: llvm/trunk/utils/TableGen/CodeGenInstruction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenInstruction.cpp?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/CodeGenInstruction.cpp (original)
+++ llvm/trunk/utils/TableGen/CodeGenInstruction.cpp Mon Sep 19 22:17:40 2011
@@ -309,7 +309,6 @@
isReMaterializable = R->getValueAsBit("isReMaterializable");
hasDelaySlot = R->getValueAsBit("hasDelaySlot");
usesCustomInserter = R->getValueAsBit("usesCustomInserter");
- hasPostISelHook = R->getValueAsBit("hasPostISelHook");
hasCtrlDep = R->getValueAsBit("hasCtrlDep");
isNotDuplicable = R->getValueAsBit("isNotDuplicable");
hasSideEffects = R->getValueAsBit("hasSideEffects");
Modified: llvm/trunk/utils/TableGen/CodeGenInstruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenInstruction.h?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/CodeGenInstruction.h (original)
+++ llvm/trunk/utils/TableGen/CodeGenInstruction.h Mon Sep 19 22:17:40 2011
@@ -233,7 +233,6 @@
bool isReMaterializable;
bool hasDelaySlot;
bool usesCustomInserter;
- bool hasPostISelHook;
bool hasCtrlDep;
bool isNotDuplicable;
bool hasSideEffects;
Modified: llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp?rev=140134&r1=140133&r2=140134&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp Mon Sep 19 22:17:40 2011
@@ -288,7 +288,6 @@
if (Inst.isNotDuplicable) OS << "|(1<<MCID::NotDuplicable)";
if (Inst.Operands.hasOptionalDef) OS << "|(1<<MCID::HasOptionalDef)";
if (Inst.usesCustomInserter) OS << "|(1<<MCID::UsesCustomInserter)";
- if (Inst.hasPostISelHook) OS << "|(1<<MCID::HasPostISelHook)";
if (Inst.Operands.isVariadic)OS << "|(1<<MCID::Variadic)";
if (Inst.hasSideEffects) OS << "|(1<<MCID::UnmodeledSideEffects)";
if (Inst.isAsCheapAsAMove) OS << "|(1<<MCID::CheapAsAMove)";
@@ -345,7 +344,7 @@
// We must emit the PHI opcode first...
std::string Namespace = Target.getInstNamespace();
-
+
if (Namespace.empty()) {
fprintf(stderr, "No instructions defined!\n");
exit(1);
More information about the llvm-commits
mailing list