[PATCH] D70329: [AArch64] Add isAuthenticating predicate to MCInstDesc

Ahmed Bougacha via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 9 15:24:37 PST 2020


ab accepted this revision.
ab added a comment.
This revision is now accepted and ready to land.

Seems reasonable.  This is an interesting use of MCID flags, in that llvm itself doesn't need it, but I'm all for better disassembler support!

> IIUC, I think "isAuthenticating" would be a more accurate description/name than "isAuthenticated".

FWIW, I actually find "isAuthenticated" more idiomatic, but I'm not enough of a native speaker to justify it ;)

> Hi! Does this cover pre v8.3 pointer authenticating instructions (AUTIASP/AUTIBSP) too?

I don't think it makes sense to include AUT* on v8.3a, but it does on v8.6a, and shouldn't hurt elsewhere, so I'm fine with having them.



================
Comment at: llvm/include/llvm/Target/Target.td:533
   bit isConvergent = 0;     // Is this instruction convergent?
+  bit isAuthenticating = 0;  // Does this instruction authenticate a pointer?
   bit isAsCheapAsAMove = 0; // As cheap (or cheaper) than a move instruction.
----------------
nit: one too many spaces between ';' and '//'


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:822
   def PACIBZ   : SystemNoOperands<0b010, "pacibz">;
-  def AUTIAZ   : SystemNoOperands<0b100, "autiaz">;
-  def AUTIBZ   : SystemNoOperands<0b110, "autibz">;
+  def AUTIAZ   : SystemNoOperandsAuth<0b100, "autiaz">;
+  def AUTIBZ   : SystemNoOperandsAuth<0b110, "autibz">;
----------------
I suggest wrapping the AUT defs with `let isAuthenticating = 1`, this doesn't need a class.


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

https://reviews.llvm.org/D70329





More information about the llvm-commits mailing list