[PATCH] D32762: [Atomics] Let emitTrailingFence take the loaded value. NFC.

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

In https://reviews.llvm.org/D32762#747208, @timshen wrote:

> In https://reviews.llvm.org/D32762#746760, @kbarton wrote:
>
> >   D32763 only handles the trailing fences. Is there going to be another patch that deals with the leading fences as well?
>
>
> No, because the reason trailing fence can depend on the loaded value is that, trailing fence is after the load in program order. IIRC data/control dependencies follows the program order.


Why does this add Instruction* argument to the emitLeadingFence function, then?



================
Comment at: llvm/include/llvm/Target/TargetLowering.h:1434
 
-  virtual Instruction *emitTrailingFence(IRBuilder<> &Builder,
+  virtual Instruction *emitTrailingFence(IRBuilder<> &Builder, Instruction *,
                                          AtomicOrdering Ord, bool IsStore,
----------------
Didn't modify the docs as to what this Instruction* is, and what it should be used for. Nor does it even have a name.


================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:1098
   if (ShouldInsertFencesForAtomic)
-    TLI->emitTrailingFence(Builder, SuccessOrder, /*IsStore=*/true,
+    TLI->emitTrailingFence(Builder, CI, SuccessOrder, /*IsStore=*/true,
                            /*IsLoad=*/true);
----------------
It feels a bit strange to pass the two-valued CI result struct here.


https://reviews.llvm.org/D32762