[PATCH] [AArch64] Add workaround for Cortex-A53 erratum (835769)

Bradley Smith bradley.smith at arm.com
Fri Oct 10 08:15:32 PDT 2014


================
Comment at: lib/Target/AArch64/AArch64FixCortexA53_835769.cpp:143
@@ +142,3 @@
+  for (MachineBasicBlock *S : MBB.predecessors())
+    if (S == PrevBB)
+      return S;
----------------
t.p.northover wrote:
> t.p.northover wrote:
> > This doesn't necessarily mean it was a fallthrough. Particularly at -O0 a block may happen to be before another in layout order but contain a real branch anyway.
> Actually, AnalyzeBranch is probably what you want to use here, if you find some way to go ahead with the NOP at end solution.
In a case where we have a real branch, a nop would never get inserted since the last instruction of the block would be the branch, not a load/store/prefetch. Unless it's easy to check for this case, might it be worth leaving like this if it's harmless?

================
Comment at: lib/Target/AArch64/AArch64FixCortexA53_835769.cpp:168
@@ +167,3 @@
+    DebugLoc DL = I->getDebugLoc();
+    BuildMI(PMBB, DL, TII->get(AArch64::HINT)).addImm(0);
+  }
----------------
t.p.northover wrote:
> I think this may damage expected block semantics too. You're putting an instruction without isTerminator after the real terminators (like the conditional branch).
> 
> I'd hope the verifier would complain about that (but be unsurprised if it didn't). Other passes may not cope (in particular, a glance at AnalyzeBranch suggests it'll fail after this).
> 
> Are you using the previous block to win back some efficiency when there's multiple branches here? If so, it may not be worth the effort, though I'll leave you to decide.
Similarly here, if there is a real terminator a nop would never get inserted, since a load/store/prefetch would never be a terminator I think? How does this work normally for fallthrough blocks with no explicit terminator?

http://reviews.llvm.org/D5721






More information about the llvm-commits mailing list