[PATCH] Add AtomicExpandPass::bracketInstWithFences, and use it whenever getInsertFencesForAtomic would trigger in SelectionDAGBuilder
Robin Morisset
morisset at google.com
Mon Sep 22 10:37:32 PDT 2014
Thanks for the review, I will fix the comments as described below.
================
Comment at: include/llvm/Target/TargetLowering.h:963
@@ -962,2 +962,3 @@
/// RMW and CmpXchg set both IsStore and IsLoad to true.
/// Backends with !getInsertFencesForAtomic() should keep a no-op here.
+ virtual Instruction* emitLeadingFence(IRBuilder<> &Builder, AtomicOrdering Ord,
----------------
jfb wrote:
> What should the return value be? Are there cases where a backend emits more than one instruction?
The return value is either nullptr (if the backend did not insert anything) or a pointer to the inserted (IR-level) instruction. There is no reason for a backend to insert several (IR-level) instructions, the only thing that makes sense for a backend to insert is an intrinsic corresponding to its hardware fence(s). Even if it requires several machine instructions for a fence, it can still be just one IR-level instruction.
I will add a comment to that effect.
================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:167
@@ -117,3 +166,3 @@
// The only 64-bit load guaranteed to be single-copy atomic by ARM is
// an ldrexd (A3.5.3).
Value *Val =
----------------
jfb wrote:
> Should this still be here, since the code wants to handle more than just ARM?
Probably not exactly as worded. I will reword it by saying that on some architectures load-linked is atomic for bigger sizes, and that for example on ARM...
http://reviews.llvm.org/D5179
More information about the llvm-commits
mailing list