[PATCH] D37121: [DivRemHoist] add a pass to move div/rem pairs into the same block (PR31028)

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 26 08:54:33 PDT 2017


spatel added a comment.

In https://reviews.llvm.org/D37121#853148, @dberlin wrote:

> Okay.
>  Then feel free to either add it, or just use the canonical idiom, and i'll add it and clean them all up.
>  Your call.


If there are no other suggestions for this patch, I'd prefer to commit this as-is (with a FIXME comment) to get more testing underway and then come back for the clean-up.

I do have a few other questions (this is my first try at writing a new pass):

1. Are there guidelines/intuition about where to position this in the opt pipeline? It's currently near the end, but I don't have a good reason for that. I just think that it should be ahead of at least one SimplifyCFG to allow flattening.
2. Are there guidelines for when to enable this? It should be cheap in compile-time, so I put in -O1. But it's also rare, so that may be an argument for -O2 or even -O3?
3. Any thoughts about the TODO for PreservedAnalyses near the end of DivRemHoist.cpp?


https://reviews.llvm.org/D37121





More information about the llvm-commits mailing list