[PATCH] Implement emitLeading/TrailingFence in the ARM backend

Robin Morisset morisset at google.com
Wed Sep 3 10:38:28 PDT 2014


Thanks for the review !

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11001
@@ -11000,1 +11000,3 @@
 
+static void MakeDMB(IRBuilder<> &Builder, ARM_MB::MemBOpt Domain) {
+  Module *M = Builder.GetInsertBlock()->getParent()->getParent();
----------------
t.p.northover wrote:
> Should probably be "makeDMB", for LLVM style.
Ok, I will do it.

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11052-11053
@@ +11051,4 @@
+    case AcquireRelease:
+      // FIXME: Too conservative, an isb after a dependent branch is enough
+      // See http://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html, confirmed in
+      // http://www0.cs.ucl.ac.uk/staff/j.alglave/papers/toplas14.pdf page 36,
----------------
t.p.northover wrote:
> By my tests, an ISB is even heavier than a DMB, so we probably don't want to encourage this alternative. I'd just skip the comment entirely.
Awesome, I was planning on doing benchmarks to evaluate this possibility; thanks for having done them :-)
I will remove the comment.

http://reviews.llvm.org/D4960






More information about the llvm-commits mailing list