jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
LG, minor comments below.
================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2639-2643
+ if (UnrolledCLI) {
+ OMPLoopNestStack.push_back(UnrolledCLI);
+ } else {
+ assert(!NeedsUnrolledCLI);
+ }
----------------
make it an unreachable or sink the assert and combine the conditions. Later
allows to remove all braces.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2077
+ Latch->getTerminator()->setMetadata(LLVMContext::MD_loop, LoopID);
+}
+
----------------
Nit: Add an assert for latch to exist. Just for my sanity ;)
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2119
+ OptLevel));
+}
+
----------------
Should we cache it in the OpenMPIRBuilder?
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2241
+ unsigned Factor = UP.Count;
+ LLVM_DEBUG(dbgs() << "Suggesting unroll factor of " << LoopSize << "\n");
+
----------------
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2275
+ return nullptr;
+ }
+
----------------
This code matches the pattern we have in the 2 members above. Can we make it a
member as well and make it clear in the name of all 3 that we use metadata and
the unroller pass? No strong feelings, more of an idea.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107764/new/
https://reviews.llvm.org/D107764
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
