[PATCH] D145650: [X86] Create extra prolog/epilog for stack realignment

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ArgumentStackSlotRebase.cpp:145
+  Register ArgBaseReg = getArgBaseReg(MF);
+  if (!ArgBaseReg.isValid())
+    return false;
----------------
pengfei wrote:
> Should assert rather than return?
My original idea is to leave it as it is if we can't handle it.


================
Comment at: llvm/lib/Target/X86/X86ArgumentStackSlotRebase.cpp:97
+  default:
+    break;
+  }
----------------
pengfei wrote:
> Should assert rather than break?
If we use `break` it can return `NoReg`, so that nothing would be done in this pass and the extra prolog/epilog won't be inserted.


================
Comment at: llvm/test/CodeGen/X86/i386-baseptr.ll:50
 ; CHECK-NEXT:    pushl %esi
-; CHECK-NEXT:    andl $-128, %esp
-; CHECK-NEXT:    subl $128, %esp
-; CHECK-NEXT:    movl %esp, %esi
-; CHECK-NEXT:    .cfi_offset %esi, -12
+; CHECK-NEXT:    subl $244, %esp
+; CHECK-NEXT:    movl %ecx, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Spill
----------------
pengfei wrote:
> Nit, it seems a bit waste in space.
Yes, but it is original code logic that is to align the stack address to 128.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145650/new/

https://reviews.llvm.org/D145650