[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