[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 :-)



More information about the llvm-commits mailing list