[PATCH] D18869: AArch64: Use a callee save registers for swiftself parameters

[Matthias Braun via llvm-commits]

MatzeB added inline comments.

================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:704
@@ -707,1 +703,3 @@
+  bool IsLiveIn = MF.getRegInfo().isLiveIn(Reg);
+  return getKillRegState(!IsLiveIn);
 }
----------------
manmanren wrote:
> So if isReturnAddressTaken is true, LRLiveIn will be true as well?
> The original check of !(LRLiveIn && MF.getFrameInfo()->isReturnAddressTaken()) is equivalent to !LRLiveIn?
If LR is used to access the return address, then isLiveIn(LR) will be true, which is fine. The new code is slightly more allowing since it does not explicitly check for isReturnAddressTaken() anymore. However I don't see how this could ever be marked in otherwise. (Also missing kill flags are not a correctness problem but can just result in worse register scavenging results, though with the reserved LR register this is not an issue anyway).

================
Comment at: test/CodeGen/AArch64/swiftself.ll:31
@@ +30,3 @@
+; CHECK: bl {{_?}}take_swiftself
+; CHECK: {{ldp|ldr}} {{.*}}x19{{.*}}sp
+; CHECK: ret
----------------
manmanren wrote:
> Are these the spills and reloads you mentioned in the summary?
> 
> Currently the generated code is correct but unnecessarily spills and
> reloads arguments passed in callee save registers, I will address this
> in upcoming patches.
Yep that is what I mentioned, I should probably remove them from the test, as that is not what we want to test here.
And indeed we should find a way to avoid them longer term (I've seen that you worked on SplitCSR before which can be adapted to this case I assume...)


Repository:
  rL LLVM

http://reviews.llvm.org/D18869