[PATCH] D99066: [AutoUpgrade] Take unnamed types into account.

Mon Mar 22 08:49:05 PDT 2021

fhahn added a comment.

In D99066#2641109 <https://reviews.llvm.org/D99066#2641109>, @jeroen.dobbelaere wrote:

> In D99066#2641043 <https://reviews.llvm.org/D99066#2641043>, @fhahn wrote:
>
>> Is there a way to ensure all call sites that may need to handle opaque pointers pass the module? can we add an assertion ensuring the module is passed if the intrinsic takes overloaded pointer types?
>
> The easiest way is to always pass the `Module` when a list of types is provided. But that is not a guarantee that the Module pointer will ever be needed for that specific intrinsic.

How many places would need to be updated? It sounds like there's no nice solution (automatically catch cases that really need the module), but it also seems brittle to have `Intrinsic::getName` crash in various places because of missing `Module` arguments.

> We could add an extra assertion that triggers when a pointer type is provided, but no `Module` is given. But also here, there will be false positives. (Intrinsics that can get a pointer, but not a pointer involving an unnamed type.)
> I tried this in out, and got extra failures because of `prefetch`, `masked_load`, `masked_store`, `masked_gather`, `masked_scatter`. Can these intrinsics use pointers that involve unnamed types ?

`llvm.prefetch` can be used with opaque types (https://godbolt.org/z/xnjx5b99Y), the other cannot be used with opaque pointers I think, because they also take vector arguments and opaque types cannot be used as vector element types.

> Note: There might also be an issue with `LLVMIntrinsicCopyOverloadedName`, which does not have a `Module` parameter. As long as no unnamed types are used, that interface is fine, but it does not support unnamed types.

Ok it sounds like we also need a fix to the C api?

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

https://reviews.llvm.org/D99066