[PATCH] D52099: [PPC64] Add split-stack support.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: ELF/Arch/PPC64.cpp:665
+                                             uint8_t StOther) const {
+  // If the caller has a global entry point adjust the buffer past it to find
+  // the start of the split-stack prologue.
----------------
ruiu wrote:
> It's not clear to me what this code does -- is this PPC-specific thing?
Yes, its V2 abi specific. A function can have 2 entry points:

1) A global entry point which will setup the TOC pointer if the function needs it. 
2) A local entry point, which will be the entry if the caller shares the same value for the TOC pointer, this skips over the TOC pointer initialization.
The 3 most significant bits of the  functions st_other flags are used to encode the offset from the global entry to the local entry.

This is something we are already doing in getRelocTargetVA for `R_PPC_CALL` RelExpr and I believe it comes up in the position-dependent long branch thunks patch I posted. I'll move the calculation into its own function and try to explain the how and why more clearly there so its not so opaque to people not familiar with PowerPC64 abi.



================
Comment at: ELF/Arch/PPC64.cpp:694
+  } else {
+    return false;
+  }
----------------
ruiu wrote:
> Can this happen?
I believe so, but I'm not 100% sure. I'm extrapolating from the `x86-64-split-stack-prologue-adjust-silent.s` test which implies that having a .note.GNU-no-split-stack section tells the linker there may be unrecognizable prologues. I wouldn't expect a compiler to generate such a prologue but was guessing it should be valid to write a custom prologue that called __morestack_non_split directly and the program would still be valid.


================
Comment at: ELF/Arch/PPC64.cpp:703
+  } else if (SecondInstr != 0x60000000) {
+    return false;
+  }
----------------
ruiu wrote:
> Is this an ABI violation, or legitimate code has such code sequence in which an instruction other than addi or nop follows the first instruction?
Like the above answer I'm not really sure. I believe a compliant compiler is expected to always emit this sequence but I don't think its necessarily disallowed  to emit a different sequence.  The behavior with the `GNU-no-split-stack` note indicates to me that different sequences are allowed but the handshake with the linker is broken and its up to the user to have implemented something that makes sense, but I can't find this documented anywhere, 


================
Comment at: ELF/Driver.cpp:284-285
+  if (Args.hasArg(OPT_split_stack_adjust_size) && Config->EMachine != EM_PPC64)
+    error(
+        "--split-stack-adjust-size is only supported on the PowerPC64 target.");
+
----------------
ruiu wrote:
> This may be a silly question, but why we have `--split-stack-adjust-size` only for PPC64 and not for x86-64? If we can live without that option with x86-64, why do we need it for PPC64?
I'm not very familiar with the x86-64 implementation, but from my understanding on x86-64 you don't have the room to rewrite the instructions to test with the adjusted stack size for all of the supported prologues types (and/or arbitrary stack frame adjustment sizes). So  instead of adjusting the check in the function prologue you  have to always jump to `__morestack_non_split` and rely on it doing the adjusted check. On PowerPC we will always emit 2 instructions for adjusting the stack, which is enough to rewrite the adjustment for any supported stack frame size, so we don't need to call  `__more_stack_non_split`. 



Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D52099