[PATCH] D11422: Refactor AtomicExpand::expandAtomicRMWToCmpXchg into a standalone function.

JF Bastien jfb at chromium.org
Thu Jul 23 15:23:43 PDT 2015


jfb added a comment.

Could you edit the patch's description to explain why this change is desirable (where the function will be used)?


================
Comment at: include/llvm/CodeGen/AtomicExpandUtils.h:19
@@ +18,3 @@
+typedef std::function<Value *(IRBuilder<> &, Value *, Value *, Value *,
+                              AtomicOrdering)> CreateCmpXchgInstFun;
+Value *defaultCreateCmpXchgInstFun(IRBuilder<> &Builder, Value *Addr,
----------------
A `function_ref` seems more LLVM-ish.

================
Comment at: include/llvm/CodeGen/AtomicExpandUtils.h:22
@@ +21,3 @@
+                                   Value *Loaded, Value *NewVal,
+                                   AtomicOrdering MemOpOrder);
+/// \brief Expand an atomic RMW instruction into a loop utilizing
----------------
Could you not have a default? Right now it's defined and used only in AtomicExpandPass.cpp, so no default is needed.

================
Comment at: include/llvm/CodeGen/AtomicExpandUtils.h:28
@@ +27,3 @@
+///
+/// Use only if you really know what you're doing.
+///
----------------
Why? This isn't very helpful :-)


Repository:
  rL LLVM

http://reviews.llvm.org/D11422







More information about the llvm-commits mailing list