[PATCH] D52216: [AArch64] Support adding X[8-15, 18] registers as CSRs.

[Tri Vo via Phabricator via llvm-commits]

trong added inline comments.


================
Comment at: lib/Target/AArch64/AArch64.td:109
+    def FeatureCallSavedX#i : SubtargetFeature<"call-saved-x"#i,
+         "CustomCallSavedXRegs["#i#"]", "true", "Make X"#i#" callee saved.">;
+
----------------
nickdesaulniers wrote:
> Since x18 can now be call-saved, any thoughts to how llvm should handle x18 being both call-saved and reserved (above line 102)?  Maybe a test can be added for this case?
I think -fcall-saved-x18 should have no effect in presence of -ffixed-x18. If x18 is reserved, callee shouldn't allocate x18. So there shouldn't be a  reason to save it. Added a test case.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:4054-4058
+  if (MF.getSubtarget<AArch64Subtarget>().hasCustomCallingConv()) {
+    uint32_t *UpdatedMask = MF.allocateRegMask();
+    TRI->getCustomCallPreservedMask(MF, Mask, UpdatedMask);
+    Mask = UpdatedMask;
+  }
----------------
nickdesaulniers wrote:
> This pattern is repeated, a lot.
> 
> Maybe you could make a single function like:
> 
> ```
> void UpdateMaskCustomCall (const MachineFunction& MF, uint32_t** Mask) {
>   ...
> }
> ...
> {
>    uint32_t* Mask = ...;
>    MachineFunction &MF = ...;
>    UpdateMaskCustomCall(MF, &Mask);
> }
> ```
> ?
MachineFunction will have to be passed non-const to make MF.allocateRegMask() call. So the signature would look something like:
```void UpdateMaskCustomCall (MachineFunction &MF, uint32_t** Mask) ```
AArch64RegisterInfo is probably not a good place to change the state of a MachineFunction, so added a function to AArch64ISelLowering to do this. I left AArch64CallLowering as is since it only gets the custom mask once.


Repository:
  rL LLVM

https://reviews.llvm.org/D52216