summaryrefslogtreecommitdiffstats
path: root/llvm/lib
diff options
context:
space:
mode:
authorPhilip Reames <listmail@philipreames.com>2019-02-12 18:33:01 +0000
committerPhilip Reames <listmail@philipreames.com>2019-02-12 18:33:01 +0000
commit7403fac3a825dcf16d5dcf26fea3cb74e765229d (patch)
tree74cd21450a5cac418828d3854873bbbad514898a /llvm/lib
parent80a1ee46d87b34f2bfabfc23e7226d615458bbec (diff)
downloadbcm5719-llvm-7403fac3a825dcf16d5dcf26fea3cb74e765229d.tar.gz
bcm5719-llvm-7403fac3a825dcf16d5dcf26fea3cb74e765229d.zip
[InlineSpiller] Fix a crash due to lack of forward progress from remat (try 2)
This is a recommit of r335091 Add more test cases for deopt-operands via regalloc, and r335077 [InlineSpiller] Fix a crash due to lack of forward progress from remat specifically for STATEPOINT. They were reverted due to a crash. This change includes the text of both original changes, but also includes three aditional pieces: 1) A bug fix for the observed crash. I had failed to record the failed remat value as live which resulted in an instruction being deleted which still had uses. With the machine verifier, this is caught quickly. Without it, we fail in StackSlotColoring due to an empty live interval from LiveStack. 2) A test case which demonstrates the fix for (1). See @test11. 3) A control flag which defaults to disabling this for the moment. Once I've run more extensive validaton, I will switch the default and then remove this flag. llvm-svn: 353871
Diffstat (limited to 'llvm/lib')
-rw-r--r--llvm/lib/CodeGen/InlineSpiller.cpp35
1 files changed, 35 insertions, 0 deletions
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 7f89adfab75..647168b1c33 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -75,6 +75,10 @@ STATISTIC(NumRemats, "Number of rematerialized defs for spilling");
static cl::opt<bool> DisableHoisting("disable-spill-hoist", cl::Hidden,
cl::desc("Disable inline spill hoisting"));
+static cl::opt<bool>
+RestrictStatepointRemat("restrict-statepoint-remat",
+ cl::init(false), cl::Hidden,
+ cl::desc("Restrict remat for statepoint operands"));
namespace {
@@ -214,6 +218,7 @@ private:
void eliminateRedundantSpills(LiveInterval &LI, VNInfo *VNI);
void markValueUsed(LiveInterval*, VNInfo*);
+ bool canGuaranteeAssignmentAfterRemat(unsigned VReg, MachineInstr &MI);
bool reMaterializeFor(LiveInterval &, MachineInstr &MI);
void reMaterializeAll();
@@ -513,6 +518,28 @@ void InlineSpiller::markValueUsed(LiveInterval *LI, VNInfo *VNI) {
} while (!WorkList.empty());
}
+bool InlineSpiller::canGuaranteeAssignmentAfterRemat(unsigned VReg,
+ MachineInstr &MI) {
+ if (!RestrictStatepointRemat)
+ return true;
+ // Here's a quick explanation of the problem we're trying to handle here:
+ // * There are some pseudo instructions with more vreg uses than there are
+ // physical registers on the machine.
+ // * This is normally handled by spilling the vreg, and folding the reload
+ // into the user instruction. (Thus decreasing the number of used vregs
+ // until the remainder can be assigned to physregs.)
+ // * However, since we may try to spill vregs in any order, we can end up
+ // trying to spill each operand to the instruction, and then rematting it
+ // instead. When that happens, the new live intervals (for the remats) are
+ // expected to be trivially assignable (i.e. RS_Done). However, since we
+ // may have more remats than physregs, we're guaranteed to fail to assign
+ // one.
+ // At the moment, we only handle this for STATEPOINTs since they're the only
+ // psuedo op where we've seen this. If we start seeing other instructions
+ // with the same problem, we need to revisit this.
+ return (MI.getOpcode() != TargetOpcode::STATEPOINT);
+}
+
/// reMaterializeFor - Attempt to rematerialize before MI instead of reloading.
bool InlineSpiller::reMaterializeFor(LiveInterval &VirtReg, MachineInstr &MI) {
// Analyze instruction
@@ -568,6 +595,14 @@ bool InlineSpiller::reMaterializeFor(LiveInterval &VirtReg, MachineInstr &MI) {
return true;
}
+ // If we can't guarantee that we'll be able to actually assign the new vreg,
+ // we can't remat.
+ if (!canGuaranteeAssignmentAfterRemat(VirtReg.reg, MI)) {
+ markValueUsed(&VirtReg, ParentVNI);
+ LLVM_DEBUG(dbgs() << "\tcannot remat for " << UseIdx << '\t' << MI);
+ return false;
+ }
+
// Allocate a new register for the remat.
unsigned NewVReg = Edit->createFrom(Original);
OpenPOWER on IntegriCloud