[llvm] [LLVM][TableGen] Check overloaded intrinsic mangling suffix conflicts (PR #110324)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 14 08:32:18 PDT 2024


jurahul wrote:

> > > I did actually run into this oddity when removing some of the amdgpu atomic intrinsics
> > 
> > 
> > Could you please elaborate on this? I don't really understand what kind of issue you can hit when _removing_ intrinsics.
> 
> In [9d36428](https://github.com/llvm/llvm-project/commit/9d364286f3b63e99ed3838f179aa2223f930f1ab), I removed the int_amdgcn_flat_atomic_fadd_v2bf16 and int_amdgcn_global_atomic_fadd_v2bf16 intrinsics. These should never have been added. We already had int_amdgcn_global_atomic_fadd and int_amdgcn_flat_atomic_fadd overloaded on the type, so we had a collision between the overloaded int_amdgcn_global_atomic_fadd_v2bf16 and int_amdgcn_global_atomic_fadd with v2bf16.
> 
> I don't remember the details, but something confusing was going on. I'm not sure if the int_amdgcn_global_atomic_fadd_v2bf16 case was ever really parsed. You could create the intrinsic with the suffixed ID, but then it would be recognized as the mangled version, or something like that.

Thanks. Based on my understanding of the current code, in this situation, we would have always matched the int_amdgcn_flat_atomic_fadd_v2bf16 and never matched the int_amdgcn_flat_atomic_fadd overloaded intrinsic with v2bf16 type. Which means it would not be possible to fully test the overloaded intrinsics for these types.

The check I am adding would have flagged that, since .v216 is a valid mangled type suffix, and in this intrinsic, since the first suffix matches a mangled type suffix, it would have flagged that as an error. 

https://github.com/llvm/llvm-project/pull/110324


More information about the llvm-commits mailing list