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

Tim Northover t.p.northover at gmail.com
Fri Oct 10 08:30:05 PDT 2014


================
Comment at: lib/Target/AArch64/AArch64FixCortexA53_835769.cpp:143
@@ +142,3 @@
+  for (MachineBasicBlock *S : MBB.predecessors())
+    if (S == PrevBB)
+      return S;
----------------
bsmith wrote:
> 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?
Ah, I see. OK, I think it's probably correct now, but the name is a bit misleading because the block may not fall through at all.

I'd probably change to using AnalyzeBranch anyway, simply because it indicates the type of checks you're performing and can hopefully be relied on to get the fiddly details right:

    if(!AnalyzeBranch(..., TBB, FBB) && !TBB & !FBB)
      return std::prev(MBB);

    return nullptr;

Alternatively, perhaps rename the function to indicate you don't actually care if it's not a real fallthrough.

http://reviews.llvm.org/D5721






More information about the llvm-commits mailing list