Skip to content

AArch64 GlobalISel bug with byval Arguments #62138

Open
@lenary

Description

@lenary

Related to #62137, the same issue comes up with GlobalISel:

If you run llvm/test/CodeGen/Generic/2010-11-04-BigByval.ll through llc with the default target set to aarch64, and globalisel enabled, then you get an error when EXPENSIVE_CHECKS is enabled.

The difference here is that the AArch64::ADJCALLSTACKDOWN and AArch64::ADJCALLSTACKUP instructions (which also represent moving the sp for the call) also cannot be nested, and the memcpy for a large byval argument is inserted by target-independent code, as far as I can see. In this case, AArch64CallLowering inserts the call stack adjustments, and then a G_MEMCPY is inserted by CallLowering::ValueHandler::copyArgumentMemory (which is in llvm/lib/CodeGen/GlobalISel/CallLowering.cpp and therefore target-independent). Later, the G_MEMCPY will be legalised to become a libcall, which ends up nesting the ADJCALLSTACKDOWN/UP instructions, which is invalid.

I'm less sure of what the fix is here, given the fact that, unlike in ISelLowering (which has a very long target-specific function to handle lowering arguments), globalisel mixes a lot more target-independent code and structure into the process, making it not very easy to work out where the fix could be applied with the current structure. I wonder if the fix might possibly come from a few other approaches:

  • Maybe a pre-globalisel LLVM IR pass to turn byval arguments into copies and pointer arguments, before we build any Machine IR.
  • Maybe for simple function calls, like memcpy, which don't need to move sp, ADJCALLSTACKDOWN/ADJCALLSTACKUP don't need to be emitted at all, which would prevent the inner nesting?
  • Maybe legalisation to a libcall should know when it's in a ADJCALLSTACKDOWN/ADJCALLSTACKUP sequence and try to hoist the inserted call before the region? This is difficult/dangerous because any reads of sp inside the region are not necessarily the same as reads of sp outside the region.
  • Maybe there's a nicer way to handle byval arguments in the target-independent globalisel code?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions