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
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@140134 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp
index 7a86658..2886091 100644
--- a/lib/Target/ARM/ARMISelLowering.cpp
+++ b/lib/Target/ARM/ARMISelLowering.cpp
@@ -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();
}
//===----------------------------------------------------------------------===//