Avoid assigning esp (or ebp for framepointer-using frames) in Om1.

For ebp, exclude as needed. For esp, don't mark it as an int register. Not sure exactly how to do a targeted test for this Om1 register allocator. The Om1 regalloc seems to start w/ a fresh whitelist after each instruction, so it may assign the same register (e.g., eax), as an earlier instruction. Without pre-colored registers, I'm not sure how to force it to allocate something other than the first few registers. I do have a test case that has a ton of pre-colored registers, (e.g., cmpxchg8b), but that is a different CL: https://codereview.chromium.org/362463002/ Encountered for: BUG= https://code.google.com/p/nativeclient/issues/detail?id=3882 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/369573005

Avoid assigning esp (or ebp for framepointer-using frames) in Om1.
9559899d · Jan Voung · e169e66d · 9559899d · 9559899d · 9559899d
Commit 9559899d authored Jul 09, 2014 by Jan Voung
Hide whitespace changes
Inline Side-by-side

Showing with 7 additions and 3 deletions

IceInstX8632.def src/IceInstX8632.def +2 -1

IceTargetLowering.cpp src/IceTargetLowering.cpp +0 -1

IceTargetLoweringX8632.cpp src/IceTargetLoweringX8632.cpp +5 -1

No files found.
--- a/src/IceInstX8632.def
+++ b/src/IceInstX8632.def
@@ -15,6 +15,7 @@
 #ifndef SUBZERO_SRC_ICEINSTX8632_DEF
 #define SUBZERO_SRC_ICEINSTX8632_DEF
+// NOTE: esp is not considered isInt, to avoid register allocating it.
 #define REGX8632_TABLE                                                  \
  /* val, init, name, name16, name8, scratch, preserved, stackptr,      \
     frameptr, isI8, isInt, isFP */                                     \
@@ -22,7 +23,7 @@
  X(Reg_ecx, = Reg_eax + 1,   "ecx",  "cx", "cl", 1, 0, 0, 0, 1, 1, 0)  \
  X(Reg_edx, = Reg_eax + 2,   "edx",  "dx", "dl", 1, 0, 0, 0, 1, 1, 0)  \
  X(Reg_ebx, = Reg_eax + 3,   "ebx",  "bx", "bl", 0, 1, 0, 0, 1, 1, 0)  \
-  X(Reg_esp, = Reg_eax + 4,   "esp",  "sp", ""  , 0, 0, 1, 0, 0, 1, 0)  \
+  X(Reg_esp, = Reg_eax + 4,   "esp",  "sp", ""  , 0, 0, 1, 0, 0, 0, 0)  \
  X(Reg_ebp, = Reg_eax + 5,   "ebp",  "bp", ""  , 0, 1, 0, 1, 0, 1, 0)  \
  X(Reg_esi, = Reg_eax + 6,   "esi",  "si", ""  , 0, 1, 0, 0, 0, 1, 0)  \
  X(Reg_edi, = Reg_eax + 7,   "edi",  "di", ""  , 0, 1, 0, 0, 0, 1, 0)  \

--- a/src/IceTargetLowering.cpp
+++ b/src/IceTargetLowering.cpp
@@ -167,7 +167,6 @@ void TargetLowering::regAlloc() {
  RegSetMask RegExclude = RegSet_None;
  RegInclude |= RegSet_CallerSave;
  RegInclude |= RegSet_CalleeSave;
-  RegExclude |= RegSet_StackPointer;
  if (hasFramePointer())
    RegExclude |= RegSet_FramePointer;
  llvm::SmallBitVector RegMask = getRegisterSet(RegInclude, RegExclude);

--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -2569,7 +2569,11 @@ void TargetX8632::postLower() {
  if (Ctx->getOptLevel() != Opt_m1)
    return;
  // TODO: Avoid recomputing WhiteList every instruction.
-  llvm::SmallBitVector WhiteList = getRegisterSet(RegSet_All, RegSet_None);
+  RegSetMask RegInclude = RegSet_All;
+  RegSetMask RegExclude = RegSet_None;
+  if (hasFramePointer())
+    RegExclude |= RegSet_FramePointer;
+  llvm::SmallBitVector WhiteList = getRegisterSet(RegInclude, RegExclude);
  // Make one pass to black-list pre-colored registers.  TODO: If
  // there was some prior register allocation pass that made register
  // assignments, those registers need to be black-listed here as