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

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ArgumentStackSlotRebase.cpp:76
+  case CallingConv::X86_RegCall:
+    return STI.is64Bit() ? X86::RBX : X86::EBX;
+  default:
----------------
Are we replacing RBX with RBX on 64-bits?


================
Comment at: llvm/lib/Target/X86/X86ArgumentStackSlotRebase.cpp:79
+    if (STI.is64Bit()) {
+      return X86::R10;
+    } else {
----------------
Why `R10`? Do you want `R11`, according to ABI
```
%r10 temporary register, used for passing a function’s static chain pointer No
%r11 temporary register                                                     No
```


================
Comment at: llvm/lib/Target/X86/X86ArgumentStackSlotRebase.cpp:1
+//===- LocalStackSlotAllocation.cpp - Pre-allocate locals to stack slots --===//
+//
----------------
Update name


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:540-541
+
+      // BuildCFI(MBB, MBBI, DL,
+      //          MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
+    } else {
----------------
Remove


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.td:436-439
+// FIXME: it includes the intersection of win64 and linux64 registers that are
+// not used to pass/return argument.
+// def GR64_NotArg: RegisterClass<"X86", [i64], 64, (add R10, R11, RBX,
+//                                                       R12, R13, R14, R15)>;
----------------
Should remove it?


================
Comment at: llvm/test/CodeGen/X86/swifttail-realign.ll:9
 ; CHECK-LABEL: caller:
+; CHECK: andq $-32, %rsp
 ; CHECK: subq $16, %rsp
----------------
No sure if it's correct. I assume the old code used `rsp` to access `%ptr` but it is intentionally omitted.
It must be wrong if the new code still use `rsp` because it is not aligned to 32 now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145650