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

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145
+  case ARM::VMVNq:
+    return CondCodeIsAL(3);
+  // VMOV of 64-bit value between D registers (when condition = al)
----------------
Can/should all these use findFirstPredOperandIdx?

And is it worth checking for more instruction? Anything that sets a Q register, or is that too broad?


================
Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:250
+    // Early return if no instructions are the start of an AES Pair.
+    if (!llvm::any_of(MBB.instrs(), isFirstAESPairInstr))
+      continue;
----------------
This needn't scan through checking for the instruction that the loop below is going to check for too.


================
Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:306
+          // fixup at the start of the function.
+          LLVM_DEBUG(dbgs()
+                     << "Fixup Planned: Live-In (with safe defining instrs): "
----------------
Can you explain more about the IsLiveIn && UnsafeCount==0 case. Am I understanding that correctly that it would be:
```
function(q0, ...)
  lotsofcode...
  q0 = load
  aes q0
```
Is there a better way to detect that the live-in doesn't matter in cases like that?


================
Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:585
 
+  addPass(createARMFixCortexA57AES1742098Pass());
+
----------------
Passes can't insert new instructions (or move things further apart) after ConstantIslandPass. The branches/constant pools it has created may go out of range of the instructions that use them. Would it be OK to move it before that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720