Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(19)

Unified Diff: src/IceTargetLoweringX8632.cpp

Issue 969703002: Subzero: Fix a register allocation issue for "advanced phi lowering". (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Improve comments Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tests_lit/llvm2ice_tests/phi.ll » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/IceTargetLoweringX8632.cpp
diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index 861bc0e54ec75915fa3018320de4d452939dfae2..12c2f1c8f8e3f11e31f9cc570210de39cb00d6df 100644
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -1691,10 +1691,19 @@ void TargetX8632::lowerAssign(const InstAssign *Inst) {
_mov(T_Hi, Src0Hi);
_mov(DestHi, T_Hi);
} else {
- // If Dest is in memory, then RI is either a physical register or
- // an immediate, otherwise RI can be anything.
- Operand *RI =
- legalize(Src0, Dest->hasReg() ? Legal_All : Legal_Reg | Legal_Imm);
+ Operand *RI;
jvoung (off chromium) 2015/03/03 00:46:25 Besides Phi, it does seem like this could affect l
Jim Stichnoth 2015/03/03 00:53:50 For what it's worth, the asm code was the same bef
+ if (Dest->hasReg())
+ // If Dest already has a physical register, then legalize the
+ // Src operand into a Variable with the same register
+ // assignment. This is mostly a workaround for advanced phi
+ // lowering's ad-hoc register allocation which assumes no
+ // register allocation is needed when at least one of the
+ // operands is non-memory.
+ RI = legalize(Src0, Legal_Reg, Dest->getRegNum());
+ else
+ // If Dest could be a stack operand, then RI must be a physical
+ // register or a scalar integer immediate.
+ RI = legalize(Src0, Legal_Reg | Legal_Imm);
if (isVectorType(Dest->getType()))
_movp(Dest, RI);
else
@@ -4163,6 +4172,12 @@ namespace {
bool isMemoryOperand(const Operand *Opnd) {
if (const auto Var = llvm::dyn_cast<Variable>(Opnd))
return !Var->hasReg();
+ // We treat vector undef values the same as a memory operand,
+ // because they do in fact need a register to materialize the vector
+ // of zeroes into.
+ if (llvm::isa<ConstantUndef>(Opnd))
+ return isScalarFloatingType(Opnd->getType()) ||
+ isVectorType(Opnd->getType());
if (llvm::isa<Constant>(Opnd))
return isScalarFloatingType(Opnd->getType());
return true;
« no previous file with comments | « no previous file | tests_lit/llvm2ice_tests/phi.ll » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698