This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Use SUBXrx64 for dynamic stack to refer to sp
ClosedPublic

Authored by bcl5980 on Jul 16 2022, 5:11 AM.

Details

Summary

When we lower dynamic stack, we need to substract pattern x15 << 4 from sp.
Subtract instruction with arith shifted register(SUBXrs) can't refer to sp. So for now we need two extra mov like:

mov x0, sp
sub x0, x0, x15, lsl #4
mov sp, x0

If we want to refer to sp in subtract instruction like this:

sub	sp, sp, x15, lsl #4

We must use arith extended register version(SUBXrx).
So in this patch when we find sub have sp operand on src0, try to select to SubXrx64.

Diff Detail

Event Timeline

bcl5980 created this revision.Jul 16 2022, 5:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2022, 5:11 AM
bcl5980 requested review of this revision.Jul 16 2022, 5:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2022, 5:11 AM
bcl5980 edited the summary of this revision. (Show Details)Jul 16 2022, 5:42 AM
efriedma added inline comments.Jul 18 2022, 10:41 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12250 ↗(On Diff #445227)

Please don't use getMachineNode outside of actual instruction selection (AArch64ISelDAGToDAG). It sort of works, but it breaks the relationship between the lowering and instruction selection, so it can have confusing effects. Instead, add an opcode to AArch64ISelLowering.h. (I know this hasn't been followed consistently in the past, but I'd prefer not to expand the existing usage.)

bcl5980 updated this revision to Diff 445730.Jul 19 2022, 1:49 AM
bcl5980 edited the summary of this revision. (Show Details)

I'm trying to use different way to do this. Please tell me which way you think is better.

bcl5980 added inline comments.Jul 19 2022, 1:53 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
12250 ↗(On Diff #445227)

Thanks for the mention about getMachineNode. I'm trying to use the other way you said before to do this to learn how the selection dag works. I will revert and add a new AArchISD if you think it is better.

efriedma accepted this revision.Jul 19 2022, 11:56 AM

LGTM with one minor comment

llvm/lib/Target/AArch64/AArch64InstrInfo.td
1718

Please use SUBXrx64 GPR64sp:$R2 for clarity (although it shouldn't have any effect).

This revision is now accepted and ready to land.Jul 19 2022, 11:56 AM
This revision was landed with ongoing or failed builds.Jul 19 2022, 8:46 PM
This revision was automatically updated to reflect the committed changes.