[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via llvm-commits]

lenary marked 3 inline comments as done.
lenary added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19
+// The intention is this:
+// - Any 128-bit or 64-bit writes to the neon input register of an AES fused
+//   pair are safe (the inputs are to the AESE/AESD instruction).
+// - Any 32-bit writes to the input register are unsafe, but these may happen
+//   in another function, or only on some control flow paths. In these cases,
+//   conservatively insert the VORRq anyway.
+// - So, analyse both inputs to the AESE/AESD instruction, inserting a VORR if
----------------
simon_tatham wrote:
> This description would leave me still confused if I didn't happen to already know roughly what the plan was. It jumps in half way through the explanation that someone would need if they were coming to this pass cold. (E.g. it talks about "the VORRq" before having even mentioned //that// there is one, let alone //why// there is one.)
> 
> How about the suggested text as a rewording?
I have taken this feedback on board, and reworded the explanation based on your suggestion, with some slight ordering changes and a little more detail about the complex cases and some separation between the erratum itself, and the workaround we have chosen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720