[PATCH] D112041: [InferAddressSpaces] Support assumed addrspaces from addrspace predicates.

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D112041#3074418 <https://reviews.llvm.org/D112041#3074418>, @hliao wrote:

> In D112041#3073676 <https://reviews.llvm.org/D112041#3073676>, @rampitec wrote:
>
>> In D112041#3073637 <https://reviews.llvm.org/D112041#3073637>, @hliao wrote:
>>
>>> In D112041#3073560 <https://reviews.llvm.org/D112041#3073560>, @rampitec wrote:
>>>
>>>> Is there anything to remove assume() call after address space is inferred? We do not need it anymore.
>>>
>>> along with a few other intrinsics, assume intrinsic is discarded in SDAG and GISel.
>>
>> We may want to discard these earlier for the sake of Value::hasOneUse(). These are really not needed after casts are inserted.
>
> That sounds reasonable but may face the limit due to the fact that we run addrspace inferring several times (if my  memory is right, 3 times if the backend one is counted.) Among them, we expect pointer values are prompted from memory to register so that we could infer addrspace for them further. If we remove these assume intrinsic earlier, there would be the risk that later addrspace inferring may not be able to leverage those assumptions.
> NVPTX runs that inferring just once. But after that, there would be too much optimizations at the IR level.
>
> I checked the most hasOneUse() usage in IR passes. Most of them are not applied to pointer arithmetic. Only a few cases are applied to pointers but also have quite limited conditions. I would expect we may need to enhance them case by case if we found real cases where the extra use from assume intrinsic makes code quality worse.

Thanks, that sounds reasonable. If we start seeing problems because of this we can always remove these later. Conceptually LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112041