[PATCH] D109775: [FuncSpec] Fix isConstant check for globals

Thu Sep 16 01:56:43 PDT 2021

nikic added a comment.

In D109775#3003343 <https://reviews.llvm.org/D109775#3003343>, @ChuanqiXu wrote:

> In D109775#3003332 <https://reviews.llvm.org/D109775#3003332>, @SjoerdMeijer wrote:
>
>> I have precommitted the test in rGa4e437e3c959 <https://reviews.llvm.org/rGa4e437e3c959ac0cb2edb733d081edc95a4fff22>.
>>
>> Specialising on the address confused me, i.e. I had not yet consciously thought about allowing/disallowing it. But now having looked a bit more into this, I don't see the problem, and I don't see why we would disallow it with this patch. So think I will abandon this patch. Any thoughts?
>
> I prefer to disallowing specializing on the address of variable globals. Since it looks like not beneficial in most cases. And if we could be sure that it wouldn't be beneficial, I think we could disallow such cases.

Shouldn't that be a cost model decision though? I agree that it likely won't be beneficial, but it it's not like it can't be (say you have an `icmp eq %arg, @g` in the specialized function).

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

https://reviews.llvm.org/D109775