[PATCH] [AArch64] Add workaround for Cortex-A53 erratum (835769)
bradley.smith at arm.com
Fri Oct 10 04:10:43 PDT 2014
Comment at: lib/Target/AArch64/AArch64.h:43
@@ -42,2 +42,3 @@
/// \brief Creates an ARM-specific Target Transformation Info pass.
> I still maintain what I said internally that the name of the pass should fit convention - there's a convention in two lines above of putting "A5*" after AArch64. Can you please change to fit that convention unless you have a compelling reason otherwise?
The rationale behind this name is that this phase isn't an A53 specific phase, it's a phase that addresses an A53 erratum. In a big-little system you may well want this phase enabled for A57 code.
Comment at: lib/Target/AArch64/AArch64FixCortexA53_835769.cpp:205
@@ +204,3 @@
+ if (MachineBasicBlock *PMBB = getBBFallenThrough(MBB))
+ PrevInstr = getLastNonPseudo(PMBB);
> What if PrevInstr is nullptr here? Will you have to jump back to the previous fallthrough if one exists? or assert?
> It doesn't seem right to continue with PrevInstr being nullptr.
I've added an assert to getLastNonPseudo since if we have a block we've fallen through we expect to find an instruction. If getBBFallenThrough returns nullptr it means there was no fallen through block, hence PrevInstr being nullptr is valid in this case, meaning we have no previous instruction yet (the loop will then just go to the next interation where it does have one).
Comment at: test/CodeGen/AArch64/aarch64-fix-cortex-a53-835769.ll:39
@@ +38,3 @@
+; CHECK-BASIC-PASS-ENABLED-NEXT: nop
+; CHECK-BASIC-PASS-ENABLED-NEXT: madd
+; CHECK-BASIC-PASS-DISABLED-LABEL: f_load_madd_64:
> There is no RUN checks for these...
These are a hangup from some previous code I had, I'll remove them. (They test the samething as CHECK-NOWORKAROUND).
More information about the llvm-commits