[PATCH] D30226: [BranchFolding] Merge debug locations from common tail instead of removing
Taewook Oh via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 6 14:29:48 PST 2017
twoh added a comment.
@aprantl Thanks for the review! I added inline questions asking for your opinion about consistency vs guideline.
================
Comment at: lib/CodeGen/BranchFolding.cpp:758
+/// MergeCommonTailDebugLocs - Create merged DebugLocs of identical instructions
+/// across SameTails and assign it to the instruction in common tail.
+void BranchFolder::MergeCommonTailDebugLocs(unsigned commonTailIndex) {
----------------
aprantl wrote:
> doxygen comments should be in the header file and not repeat the method name.
All other functions in BranchFolding.cpp file have doxygen comments inside .cpp file with a method name. I thought it is better to keep the consistency, and wonder what is your opinion on this.
================
Comment at: lib/CodeGen/BranchFolding.cpp:763
+ std::vector<MachineBasicBlock::iterator> NextCommonInsts(SameTails.size());
+ for (unsigned int i = 0 ; i != SameTails.size() ; ++i) {
+ if (i != commonTailIndex)
----------------
aprantl wrote:
> I think local variables should be captialized.
I kept 'i' small here for the consistency as well.
https://reviews.llvm.org/D30226
More information about the llvm-commits
mailing list