[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:38:13 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.

Just a further point of clarification, it would have been flagged when the last one of the int_amdgcn_flat_atomic_fadd or int_amdgcn_flat_atomic_fadd_v2bf16 was added in the first place.

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


More information about the llvm-commits mailing list