[PATCH] D30254: Remove sometimes faulty rewrite of memcpy in instcombine.

Mikael Holmén via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 23 01:35:28 PST 2017


uabelho added inline comments.


================
Comment at: test/Transforms/InstCombine/memcpy-to-load.ll:15
+; CHECK: [[TMP:%[0-9]+]] = load i64
+; CHECK: store i64 [[TMP]]
+
----------------
efriedma wrote:
> We already produce this result 64-bit targets (combineLoadToOperationType transforms double load/store to i64 load/store); the interesting piece is what happens on targets where i64 isn't legal.
> 
> In general, it shouldn't matter what we produce here; we should always eventually rewrite loads/stores to the appropriate register type for the target.  And other optimization passes don't really care about the types of loads and stores anyway (SROA is a lot more flexible than it used to be).  So if this does in fact expose a performance regression, we should fix it elsewhere.
> 
> That said, it would be nice if you could give testsuite performance numbers on some 32-bit target to make sure we aren't missing some important optimization.
Ok, for the performance numbers you'd have to give me some guidance then.

Is it these tests you want me to run?
 http://llvm.org/docs/TestingGuide.html#test-suite-quickstart

I have a 64b linux ubuntu 14.04 machine that I think I managed to run the test-suite the LNT way on according to:
 http://llvm.org/docs/lnt/quickstart.html

I ran them two times without and then two times with the patch and imported all the results to a database and started looking through the web UI but to be honest I don't know what I'm looking for and I'm quite confused.

So, please some guidance to what numbers you want me to dig up.


https://reviews.llvm.org/D30254





More information about the llvm-commits mailing list