[PATCH] [AArch64] Add workaround for Cortex-A53 erratum (835769)
Renato Golin
renato.golin at linaro.org
Fri Oct 10 03:04:00 PDT 2014
The implementation seems correct and the rationale seems right. I'd rather Tim had a look at it, too, just in case.
Just a few unimportant comments inline, but overall, looks good to me.
Thanks!
================
Comment at: lib/Target/AArch64/AArch64FixCortexA53_835769.cpp:84
@@ +83,3 @@
+ case AArch64::MSUBWrrr:
+ case AArch64::MADDWrrr:
+ default:
----------------
I'm not sure this is really necessary, but keeping a comment about this is probably good.
================
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...
================
Comment at: test/CodeGen/AArch64/aarch64-fix-cortex-a53-835769.ll:42
@@ +41,3 @@
+; CHECK-BASIC-PASS-DISABLED: ldr
+; CHECK-BASIC-PASS-DISABLED-NEXT: madd
+
----------------
You can just have two checks: ON and OFF and control what needs to be on and off onthe RUN line.
================
Comment at: test/CodeGen/AArch64/aarch64-fix-cortex-a53-835769.ll:128
@@ +127,3 @@
+; CHECK-NOWORKAROUND-LABEL: f_load_mneg_64:
+; FIXME: only add further checks here once LLVM actually produces
+; neg instructions
----------------
I think you should also add the expected CHECK line on both cases but not add the CHECK text.
http://reviews.llvm.org/D5721
More information about the llvm-commits
mailing list