[PATCH] D32762: [Atomic] Remove IsStore/IsLoad in the interface, and pass the instruction instead. NFC.

James Y Knight via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 9 07:30:39 PDT 2017


jyknight added a comment.

This seems basically fine modulo renaming and comments.



================
Comment at: llvm/include/llvm/IR/Instruction.h:460
+  /// Return true if this atomic instruction loads from memory.
+  bool isAtomicLoad() const;
+
----------------
sanjoy wrote:
> Would it be better to call this `hasAtomicLoad`?  Since the instructions here do not *solely* atomic-load.
Agree.


================
Comment at: llvm/include/llvm/Target/TargetLowering.h:1399
   ///   if shouldInsertFencesForAtomic returns true.
-  /// RMW and CmpXchg set both IsStore and IsLoad to true.
+  /// Inst is the memory operation the fence proceeds/succeeds.
   /// This function should either return a nullptr, or a pointer to an IR-level
----------------
Maybe say something like:

Inst is the original atomic instruction, prior to other expansions that may be performed.


https://reviews.llvm.org/D32762





More information about the llvm-commits mailing list