[PATCH] D133766: [LLVM][AArch64] Explain why X16 is reserved when Speculative Load Hardening is in use

[David Spickett via Phabricator via llvm-commits]

DavidSpickett added a comment.

> the warning/error implemented in this patch is not correct?

I see what you mean. This is a unfortunate crossover where `AArch64RegisterInfo::getReservedRegs` is not just used for inline assembly but codegen too. So if codegen wants X16 it can't use it but in the codegen for SLH you've actually handled the inline asm case so there's no reason to warn at all.

Running the test you linked we see that it does already warn when it doesn't need to:

  --
  Exit Code: 0
  
  Command Output (stderr):
  --
  warning: inline asm clobber list contains reserved registers: W16
  note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.



  f_clobbered_reg_w16:                    // @f_clobbered_reg_w16
          .cfi_startproc
  // %bb.0:                               // %entry
          dsb     sy
          isb
          cmp     w0, w1
          b.le    .LBB1_3
  // %bb.1:
          dsb     sy
          isb
  // %bb.2:                               // %if.then
          //APP
  warning: inline asm clobber list contains reserved registers: W16
  note: Reserved registers on the clobber list may not be preserved across the asm statement, and clobbering them may lead to undefined behaviour.
          mov     w16, w0
          //NO_APP
          add     w0, w1, w0
          ret

I think I can make the inline asm check more specific to avoid this but keep the register reserved for codegen.



================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:342
+    return std::string(
+        "X16 is used as the taint register for Speculative Load Hardening.");
+
----------------
kristof.beyls wrote:
> I see that I've used the term "taint register" in the source code of AArch64SpeculationHardening.cpp; but I don't see the term in the (X86-centric) design document about Speculative Load Hardening at https://llvm.org/docs/SpeculativeLoadHardening.html. I don't immediately see an alternative term to clearly name the register that Speculative Load Hardening reserves.
> Therefore, I think that using the term "taint register" is indeed the best option available here.
I could just go with "is used as part of Speculative Load Hardening". The point is, you can google something and land on an llvm page.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133766