[PATCH] D126861: [RISCV] Fix missing stack pointer recover

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:648
+  if (RI->hasStackRealignment(MF) || MFI.hasVarSizedObjects() ||
+      !hasReservedCallFrame(MF)) {
     assert(hasFP(MF) && "frame pointer should not have been eliminated");
----------------
rogfer01 wrote:
> rogfer01 wrote:
> > I assume that the exceptional path doesn't execute the `add sp, sp, 32` after `call _Z3fooiiiiiiiiiiPi at plt`, instead we directly land to `.LBB0_2: # %lpad`. If this is th case, it does leaves the stack unaligned.
> > 
> > However, the expansion of `!hasReservedCallFrame(MF)` results in checking `hasFP(MF) && hasRVVFrameObject(MF)`.
> > 
> > Isn't this an issue that happens irrespective of vectors and it is caused by having a `fp`? E.g. when using something like `-fno-omit-frame-pointer`.
> Answering myself here: when not using `v` then we account the callee arguments as part of the caller stack frame.
Yeah, your understating is right.

BTW, same issue could be happen without `v` in theory (without this patch), but that situation will //**fixed**// by clang, it will emit stack pointer store/restore intrinsic...use this case to demonstrate:

`simple.cpp`
`$ clang simple.cpp -o - -S -O -emit-llvm -march=rv64gc -target riscv64-unknown-linux-gnu`
```
void foo(int, int, int, int, int, int, int, int, int, int, int *);

bool check_frame_variant(int x) {
  int exception_value = 1;
  try {
      int yy[x];
      foo(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, &yy[0]);
  } catch (int i) {
    exception_value = i;
  }
  return exception_value;
}
```

Generated LLVM IR:
```
define dso_local noundef zeroext i1 @_Z19check_frame_varianti(i32 noundef signext %x) local_unnamed_addr #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
entry:
  %0 = zext i32 %x to i64
  %1 = call i8* @llvm.stacksave()   <--- clang emit a sp back up here
  %vla = alloca i32, i64 %0, align 8
  invoke void @_Z3fooiiiiiiiiiiPi(i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef signext 0, i32 noundef 0, i32 noundef 0, i32* noundef nonnull %vla)
          to label %invoke.cont unwind label %lpad

invoke.cont:
  call void @llvm.stackrestore(i8* %1) <-- and a sp restore here!!!!!
  br label %try.cont

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126861