[PATCH] D34005: [CGP / PowerPC] avoid multi-block overhead for simple memcmp expansion

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

LGTM, you might want to wait for other comments as I'm new here :)



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1703
 
-  IRBuilder<> Builder(CI->getContext());
-  BasicBlock *StartBlock = CI->getParent();
-  EndBlock = StartBlock->splitBasicBlock(CI, "endblock");
-  setupEndBlockPHINodes();
+  // A memcmp with zero-comparison with only one block of load and compare does
+  // not need to set up any extra blocks.
----------------
I think the comment should also explain why in this case (only one block) we don't want to abort everything right now and let the SDAG do the lowering (IIUC, something along the lines of "in that case, we still want to do the memcmp expansion here because this code handles vector expansions better").


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1729
+  IRBuilder<> Builder(CI->getContext());
   Builder.SetCurrentDebugLocation(CI->getDebugLoc());
 }
----------------
The debug location is never used; this looks like a remnant of some previous code before refactoring. It looks like the intent was to use that builder in member functions.  Now the builder is recreated every time (see e.g. MemCmpExpansion::emitLoadCompareByteBlock, line 1750). Builder should be made a member (maybe in another revision).


https://reviews.llvm.org/D34005