[PATCH] D91786: [GVN] Strengthen the updating of dominated users

Jonas Paulsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 15 08:57:08 PST 2021


jonpa abandoned this revision.
jonpa added a comment.

In D91786#2474795 <https://reviews.llvm.org/D91786#2474795>, @lebedev.ri wrote:

> In D91786#2406652 <https://reviews.llvm.org/D91786#2406652>, @lebedev.ri wrote:
>
>> As of D90231 <https://reviews.llvm.org/D90231> i'm personally still unconvinced that it is the LLVM code that needs fixing and not the particular code that is using `__builtin_constant_p()`.
>
> ^ still am.
> Are langref changes needed to justify these optimization changes?

The reason was that this code was somehow the result of some kind of generated code via macros that happened to turn out a certain way, I guess. However, the developers have now decided to change this after all to use builtin calls instead.

So you were right, and I abandon this now given that this doesn't give much as a general improvement... Thanks for review.

I wonder if I perhaps should add a small comment somewhere in GVN.cpp that this has been tried without much benefit?


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

https://reviews.llvm.org/D91786



More information about the llvm-commits mailing list