This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] Emit G_ASSERT_SEXT for SExt parameters in CallLowering
ClosedPublic

Authored by paquette on Feb 17 2021, 4:07 PM.

Details

Summary

Similar to how we emit G_ASSERT_ZEXT when we have CCValAssign::LocInfo::ZExt.

This will allow us to combine away some redundant sign extends.

Example: https://godbolt.org/z/cTbKvr

Diff Detail

Event Timeline

paquette created this revision.Feb 17 2021, 4:07 PM
paquette requested review of this revision.Feb 17 2021, 4:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2021, 4:07 PM
aemerson accepted this revision.Feb 18 2021, 4:52 PM

LGTM.

This revision is now accepted and ready to land.Feb 18 2021, 4:52 PM
kschwarz added inline comments.
llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-signext.ll
60

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?

paquette added inline comments.Aug 2 2021, 1:23 PM
llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-signext.ll
60

Thanks for the heads up.

D107305 should fix this.