[PATCH] D104636: [LoopIdiom] [LoopNest] let the pass deal with runtime memset size

[Yueh-Ting Chen via Phabricator via llvm-commits]

eopXD added a comment.

Thank you @lebedev.ri  and @xbolva00 for leaving comments.
I would follow up with benchmarks of the patch.



================
Comment at: llvm/test/Transforms/LoopIdiom/memset-runtime.ll:7
+;	  	for (j=0; j<n; ++j) {
+;	 	  	long index = i*n + j;
+;	 		  ar[index] = 0;
----------------
xbolva00 wrote:
> LoopFlatten pass could convert two loops to one, no? Than you dont need to teach this pass about this pattern.
> 
> Btw, is this common pattern? Can you show us some motivation examples? Some hits in benchmarks?
Yes, LoopFlatten does convert two loops into one. Sorry that my example is misleading. I have adjusted the test case so LoopFlatten is irrelevant.

In the new test case the `memset` is runtime determined and cannot be optimized by current LoopIdiomRecognize, which this patch deals with the scenario.

To deal with the scenario there should be runtime checks to make sure the optimization is safe, so versioning is needed. To avoid exponential versioning growths, we can do 1 versioning for every nested loop. That is where the `LoopNestPass` comes in handy. The pass runs on `LoopNest` which constructs on a whole nested loop (doesn't run on a sub-loop).



================
Comment at: llvm/test/Transforms/LoopIdiom/memset-runtime.ll:22
+; We need to add runtime checks for this optimization, and version the loop.
+; The optimized IR should be similar to the following:
+; void test(int *ar, long n)
----------------
xbolva00 wrote:
> So we have original loops and memset, which could be expanded to series of stores. Nightmare for codesize..
I think this pass does the opposite. It processes series of store of a loop into a single `memset`.

The code size increase scenario by this patch is when versioning happens.
(Original LoopIdiomRecognize doesn't involve versioning)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104636/new/

https://reviews.llvm.org/D104636