[PATCH] D129727: [ARM64EC 11/?] Add support for lowering variadic indirect calls.

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:483
+    // But we don't know the function is variadic or not here.
+    // Not sure if we can align to 8 for normal exit thunk.
+    bool Arm64Ec = isArm64EcThunk(MF.getFunction());
----------------
efriedma wrote:
> For the whole "x3 is stored to the stack" thing, if we're going to continue to use an alloca, I'd probably recommend just increasing the size of the alloca by 8 bytes, then explicitly emit a store instruction in the IR.  Messing with the alignment like this seems likely to cause issues.
I'm trying to move these code into AArch64TargetLowering::LowerCall but still have some problems about the stack layout. And I also trying to make the IR version also correct. Thanks for the idead of allocate extra 8bytes.


================
Comment at: llvm/test/CodeGen/AArch64/arm64ec-cfg.ll:363-365
+; CHECK-NEXT:    mov x8, sp
+; CHECK-NEXT:    sub x0, x8, x15, lsl #4
+; CHECK-NEXT:    mov sp, x0
----------------
efriedma wrote:
> bcl5980 wrote:
> > It looks Microsoft generate code：
> > 
> > ```
> > sub         sp,sp,x15,lsl #4
> > ```
> > Does anyone know why we can't accept SP as input/ouput for the instruction?
> > 
> > ```
> > def : Pat<(sub GPR64:$Rn, arith_shifted_reg64:$Rm),
> >           (SUBSXrs GPR64:$Rn, arith_shifted_reg64:$Rm)>;
> > ```
> > 
> > 
> > 
> The instruction used by the Microsoft compiler is SUBXrx64.  SUBSXrs can't refer to sp that way; the "lsl" is actually an alternate spelling of "uxtx".
> 
> We could add a specialized pattern specifically for subtraction operations where the first operand of the subtraction is a copy from sp.
//We could add a specialized pattern specifically for subtraction operations where the first operand of the subtraction is a copy from sp.//
Do you mean add pattern in td file?

```
def : Pat<(sub GPR64sp:$SP, arith_extended_reg32to64_i64:$Rm),
          (SUBSXrx GPR64sp:$SP, arith_extended_reg32to64_i64:$Rm)>;
```
If yes, we may need to add a new select function similar to arith_extended_reg32to64_i64 but remove check extend. Or can we do this on DAGCombine?

And I think we can do this on another patch.


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

https://reviews.llvm.org/D129727