[llvm] [SCEV] Add implicit guard conditions when applying loop guards. (PR #84309)

Mel Chen via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 1 07:18:06 PDT 2024


Mel-Chen wrote:

I'm glad to receive your reply. Sorry for the delayed response, I was caught up with other issues previously.

> First, you're modifying code which is not enabled by default. The first step is assessing the compile time impact of enabling it. If that is prohibitive, then the rest of this line of work is moot.

During this period, I observed the original PR at https://reviews.llvm.org/D89381. The reason for default disabling this feature seems to be not only due to the increase in compile time but also because the performance gain it brings is insufficient. I don't have experience with compile time-related matters and I'm unsure about the relevant processes (such as benchmarks used, compile options, etc.). Currently, my plan is to measure the compile time and performance with and without enabling `-scalar-evolution-use-expensive-range-sharpening`, focusing on spec2006 or spec2017 benchmarks separately. If there are common evaluation processes for compile time issues in the community, please let me know. (cc @nikic )

> Second, you're actual change appears to be three independent pieces. Each of these should be their own review with targeted tests for each case. Two of them appear blocked on enabling the disabled piece of code, one you could maybe do in parallel.

This matter is also bothering me. I tried to only test with `applyLoopGuards` modification, but there was no changes on lit tests. Next, I may try to use asserts to find relevant cases in benchmarks. So that when the `applyLoopGuards` change is split out, there are corresponding test cases to demonstrate the functionality of the patch.


https://github.com/llvm/llvm-project/pull/84309


More information about the llvm-commits mailing list