[PATCH] D96915: [AArch64][GlobalISel] Emit G_ASSERT_SEXT for SExt parameters in CallLowering

Mon Aug 2 13:23:36 PDT 2021

paquette added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-signext.ll:59
+  ; CHECK:   [[LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[FRAME_INDEX1]](p0) :: (invariant load 1 from %fixed-stack.0, align 8)
+  ; CHECK:   [[ASSERT_SEXT:%[0-9]+]]:_(s32) = G_ASSERT_SEXT [[LOAD1]], 1
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[ASSERT_SEXT]](s32)
----------------
kschwarz wrote:
> Sorry for the late comment, but we recently started to migrate our backend to use the `G_ASSERT_SEXT` and `G_ASSERT_ZEXT` hints and stumbled upon some strange behavior.
> 
> Our target has a very similar ABI to that of aarch64 when it comes to i1/i8/i16 arguments passed on the stack: the callee may not assume the values have been extended when passed by the caller, thus needs to explicitly sign/zero extend the argument. When passed in registers, the values are extended already be the caller.
> 
> I couldn't find any code in the AArch64 backend that actually makes sure that the load will be selected to the proper extending load.
> 
> For the following code:
> 
> ```
> define signext i8 @a([8 x i64], i8 returned signext %x) local_unnamed_addr {
> entry:
>   ret i8 %x
> }
> ```
> 
> GlobalISel and SelectionDAG generate different code:
> 
> GlobalISel:
> ```
> _a:
>         ldrb    w0, [sp]
>         ret
> ```
> 
> SelectionDAG:
> 
> ```
> _a:
>         ldrsb   w0, [sp]
>         ret
> ```
> 
> , which looks like an ABI issue to me?
> 
Thanks for the heads up.

D107305 should fix this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96915