[PATCH] D64757: [PEI] Don't re-allocate a pre-allocated stack protector slot

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg marked 2 inline comments as done.
thegameg added inline comments.


================
Comment at: llvm/lib/CodeGen/LocalStackSlotAllocation.cpp:202
   SmallSet<int, 16> ProtectedObjs;
-  if (MFI.getStackProtectorIndex() >= 0) {
+  if (MFI.hasStackProtectorIndex()) {
+    int StackProtectorFI = MFI.getStackProtectorIndex();
----------------
efriedma wrote:
> Is this change a no-op?
Yes, it just adds the assert.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:932
+  // LocalStackSlotPass didn't already allocate a slot for it. If it did, we
+  // need to use that one.
+  if (MFI.hasStackProtectorIndex() &&
----------------
efriedma wrote:
> We obviously don't want to re-allocate a slot for the stack protector, but what about the other stack objects?  Do we guarantee that LocalStackSlotPass will allocate all obejcts where getObjectSSPLayout() is not SSPLK_None?  If we do, we can just simplify this check to `if (MFI.getUseLocalStackAllocationBlock())`.  Otherwise, we should still run this code for all the all the non-stack-protector frame indexes.
`LocalStackSlotPass` works in two steps:

1) Assign offsets to all stack objects except objects used for saving callee saved registers in `MFI.LocalFrameObjects` . These offsets will be **potentially** used by PEI by sliding them to an actual offset after allocating slots for other things like CSRs.
2) Replace `FrameIndex` operands with a vreg operand (itself defined by a FI + offset) + offset operand

`1)` always runs, but if `2)` does not rewrite any FI operands, it does not set `MFI.UseLocalStackAllocationBlock`, and PEI will not use anything from `MFI.LocalFrameObjects`, and it will instead re-allocate everything in `MFI.Objects` as if the pass did not even run. In this specific case, `1)` runs and sets `MFI.StackProtectorIndex` (which is "global"), but only assigns the offsets of local variables in `MFI.LocalFrameObjects`.

`AdjustStackOffset` from `LocalStackSlotPass` uses
```
MFI.mapLocalFrameObject(FrameIdx, LocalOffset);
```

`AdjustStackOffset` from `PEI` uses
```
MFI.setObjectOffset(FrameIdx, Offset)
```



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

https://reviews.llvm.org/D64757