[PATCH] D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation

Fri Nov 13 11:58:38 PST 2020

spatel added a comment.

In D90554#2393696 <https://reviews.llvm.org/D90554#2393696>, @mstorsjo wrote:

> In D90554#2393645 <https://reviews.llvm.org/D90554#2393645>, @spatel wrote:
>
>> In D90554#2393351 <https://reviews.llvm.org/D90554#2393351>, @mstorsjo wrote:
>>
>>> I noticed a change in behaviour due to this commit - on ARM and AArch64, for cases involving the log2 intrinsic - previously, llvm was able to evaluate a big pile of code to deduce that a comparison involving log2 always is true, but after this change, it no longer does that. Is that something you want to have a look into, or is that expected? I wouldn't classify it as a bug in itself, as the code does what it's supposed to, it just optimizes differently than before.
>>
>> If you can reduce a test that shows the difference, that would be great. I'm not sure which pass was responsible for the optimization. 
>> My first guess is that we should create a basic class size-cost exception for all of the scalar intrinsics that correspond to libm functions.
>
> I'm not sure how to reduce it meaningfully, but it's at least observable with https://martin.st/temp/celp_math-preproc.c and https://martin.st/temp/celp_math.ll. With the bitcode, `opt -O3 -S celp_math.ll` used to produce a no-op main() function before this change, same goes if compiling the preprocessed C code with `clang -target aarch64-w64-mingw32 -c -O3 celp_math-preproc.c -fno-math-errno`.

Thanks for the test. The difference with `@main` is in the unroller:
COMPLETELY UNROLLING loop %do.body6 with trip count 11!

And with that, it seems to vanish. Interestingly, the optimization was only happening with -O3 (not -O2) even before this patch because we set the unroll threshold higher at -O3.
I will try to reduce the example and adjust the cost model to make that work again.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90554