[PATCH] Erase fence insertion from SelectionDAGBuilder.cpp (NFC)

Robin Morisset morisset at google.com
Fri Oct 3 15:56:37 PDT 2014

Comment at: include/llvm/Target/TargetLowering.h:979
@@ +978,3 @@
+  ///   *unsound* ! Fences cannot, in general, be used to restore sequential
+  ///   consistency. For example, consider the following example:
+  /// atomic<int> x = y = 0;
jfb wrote:
> The FIXME should probably say how to fix this (or maybe point it out in the code below), and then suggest that the asserts be added back.
I tried to give the solution just above "Backends should override this method to produce target-specific intrinsic for their fence". I do not see what more I can say, apart from maybe "look at what the ARM backend does".

Comment at: include/llvm/Target/TargetLowering.h:987
@@ -986,3 +1006,1 @@
-  ///   Instruction*. Even complex fence sequences can be represented by a
-  ///   single Instruction* through an intrinsic to be lowered later.
   virtual Instruction* emitTrailingFence(IRBuilder<> &Builder, AtomicOrdering Ord,
jfb wrote:
> You can create a doxygen group for these functions, start with `@{` in the comment for emitLeading, and after emitTrailing close with `}@`.
Ok, thanks for the trick, I will do that.

Comment at: test/CodeGen/XCore/atomic.ll:26
@@ -28,2 +25,3 @@
 ; CHECK-NEXT: ldaw r[[R1:[0-9]+]], dp[pool]
 ; CHECK-NEXT: ldc r[[R2:[0-9]+]], 0
jfb wrote:
> Why does the barrier move?
I am not entirely sure, but both placements appear correct. So while I agree it is surprising and slightly troubling, I decided to go ahead as I could not find any other case when it happens. I suspect it is just some the instruction scheduler that must pick differently between two valid choices, for some reason (The first lowering happens a bit earlier in the pipeline so maybe this cause some other pass in-between to change subtly its behaviour ?)


More information about the llvm-commits mailing list