[PATCH] D31772: [PowerPC] Add pass to expand extra memcpy calls

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: lib/Target/PowerPC/PPCLowerMemIntrinsics.cpp:117
+// size is a compile time constant or not. Will go away when the old memcpy
+// expansion implementation does.
+static void ppcExpandMemCpyAsLoop(MemCpyInst *MI,
----------------
hfinkel wrote:
> I don't understand this comment. It looks like this function is just llvm::expandMemCpyAsLoop whenever TTI.useWideIRMemcpyLoopLowering() returns false. Why not just use that function?
This is so old it took me a while to figure out what was going on here but I think I got it now. This is here essentially because:

1)  It originally took pgo data into account but that is split out into a subsequent patch making this the same as ' llvm::expandMemCpyAsLoop when TTI.useWideIRMemcpyLoopLowering() returns false  as you pointed out.

2) I didn't use ' llvm::expandMemCpyAsLoop'  directly because  useWideIRMemcpyLoopLowering is actually off by default for every target,  even PPC(!).  

The reason 'useWideIRMemcpyLoopLowering ' got added in the first place was because I was hesitant to change the targets I didn't have a way of running functional testing for (amd and nvptx backends), and it was only there to give people an easy way to test it out before flipping the switch to the new implementation. It was supposed too get removed in a subsequent cleanup patch that I never implemented.

Rather then  changing this to use 'llvm::expandMemCpyAsLoop' I would like to leave it since the follow up patch (D32872) modifies it so it no longer matches, and I will post the cleanup patch that removes the extra TTI hook and the old byte-copy only implementation of memcpy lowering separately since it can go in independent of this.


Repository:
  rL LLVM

https://reviews.llvm.org/D31772