With these changes, LGTM.
================
Comment at: lib/CodeGen/CGBuilder.h:50
@@ -23,2 +49,3 @@
+ CGBuilderTy;
#endif
----------------
This is now a lot of parameters to be duplicated in both sizes of the NDEBUG
conditional. You can, for example, define a boolean that depends on NDEBUG and
use that instead.
================
Comment at: lib/CodeGen/CGLoopInfo.cpp:89
@@ +88,3 @@
+void LoopInfoStack::pop() {
+ assert(!Active.empty());
+ Active.pop_back();
----------------
This assert is not self explanatory. Please write:
assert(!Active.empty() && "No active loops to pop");
(or something like that).
================
Comment at: lib/CodeGen/CGLoopInfo.cpp:115
@@ +114,3 @@
+ LI->setMetadata("llvm.mem.parallel_loop_access", L.getLoopID());
+ }
+ }
----------------
This is not quite complete. It misses memory intrinsics, for example. The check
in LLVM's lib/Analysis/LoopInfo.cpp requires metadata on all instructions for
which mayReadOrWriteMemory() returns true.
I think this can be:
if (L.getAttributes().IsParallel && I->mayReadOrWriteMemory())
I->setMetadata("llvm.mem.parallel_loop_access", L.getLoopID());
================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1668
@@ +1667,2 @@
+ llvm::BasicBlock::iterator InsertPt) const;
+#endif
----------------
Same comment here (don't duplicate all of this, just use a boolean or some
other mechanism).
http://reviews.llvm.org/D3644
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
