[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Changpeng Fang via cfe-commits
changpeng wrote: I am going to propose to rename intrinsics and remove f16/bf16 versions of builtins/intrinsics https://github.com/llvm/llvm-project/pull/86202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Changpeng Fang via cfe-commits
https://github.com/changpeng closed https://github.com/llvm/llvm-project/pull/86202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Changpeng Fang via cfe-commits
changpeng wrote: [AMD Official Use Only - General] I am fine to remove f16/bf16 versions. Enumerating all possible types could be very painful. For example we gave up enumerating for B64, and ended up using v2i32 only. What do others think removing f16/bf16 versions? Thanks Get Outlook for

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Matt Arsenault via cfe-commits
@@ -432,13 +432,15 @@ TARGET_BUILTIN(__builtin_amdgcn_s_wakeup_barrier, "vi", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_barrier_leave, "b", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_get_barrier_state, "Uii", "n", "gfx12-insts")

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Matt Arsenault via cfe-commits
https://github.com/arsenm approved this pull request. https://github.com/llvm/llvm-project/pull/86202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Matt Arsenault via cfe-commits
arsenm wrote: > > I don't think intrinsics are meant for users. Builtins are the user-facing > > front. :-) > > Depending on who you consider an user. Are folks writing MLIR generators > users? They're consumers of an unstable API, changing intrinsics is fine

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Piotr Sobczak via cfe-commits
piotrAMD wrote: The change LG - thanks for adding support for bf16. Agreed that the intrinsics should match the builtins for consistency (now or in a follow-up commit). These intrinsics were added for the upcoming generation - it should be fine to rename them at this stage.

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-22 Thread Stanislav Mekhanoshin via cfe-commits
rampitec wrote: > I don't think intrinsics are meant for users. Builtins are the user-facing > front. :-) Depending on who you consider an user. Are folks writing MLIR generators users? https://github.com/llvm/llvm-project/pull/86202 ___ cfe-commits

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Changpeng Fang via cfe-commits
changpeng wrote: > I don't think intrinsics are meant for users. Builtins are the user-facing > front. :-) Then renaing the intrinsics should be relatively at a lower priority. We may do it in a separate patch once we have reached an agreement. https://github.com/llvm/llvm-project/pull/86202

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Shilei Tian via cfe-commits
shiltian wrote: > > > > Do you want to rename intrinsics as well? Because now intrinsic names > > > > do not match builtin names. > > > > > > > > > Do we have to match builtins with intrinsics? Renaming intrinsics here > > > means we will have to duplicate the intrinsics. > > > > > > Is

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Changpeng Fang via cfe-commits
changpeng wrote: > > > Do you want to rename intrinsics as well? Because now intrinsic names do > > > not match builtin names. > > > > > > Do we have to match builtins with intrinsics? Renaming intrinsics here > > means we will have to duplicate the intrinsics. > > Is that because of the

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Stanislav Mekhanoshin via cfe-commits
rampitec wrote: > > Do you want to rename intrinsics as well? Because now intrinsic names do > > not match builtin names. > > Do we have to match builtins with intrinsics? Renaming intrinsics here means > we will have to duplicate the intrinsics. Is that because of the mangling?

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Changpeng Fang via cfe-commits
changpeng wrote: > Do you want to rename intrinsics as well? Because now intrinsic names do not > match builtin names. Do we have to match builtins with intrinsics? Renaming intrinsics here means we will have to duplicate the intrinsics. https://github.com/llvm/llvm-project/pull/86202

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Stanislav Mekhanoshin via cfe-commits
@@ -432,13 +432,15 @@ TARGET_BUILTIN(__builtin_amdgcn_s_wakeup_barrier, "vi", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_barrier_leave, "b", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_get_barrier_state, "Uii", "n", "gfx12-insts")

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Shilei Tian via cfe-commits
@@ -432,13 +432,15 @@ TARGET_BUILTIN(__builtin_amdgcn_s_wakeup_barrier, "vi", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_barrier_leave, "b", "n", "gfx12-insts") TARGET_BUILTIN(__builtin_amdgcn_s_get_barrier_state, "Uii", "n", "gfx12-insts")

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Stanislav Mekhanoshin via cfe-commits
https://github.com/rampitec commented: Do you want to rename intrinsics as well? Because now intrinsic names do not match builtin names. https://github.com/llvm/llvm-project/pull/86202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread Changpeng Fang via cfe-commits
https://github.com/changpeng created https://github.com/llvm/llvm-project/pull/86202 Make the name of a clang builtin as close to the mnemonic instruction name as possible. The data type suffix may not be enough to tell what instruction the builtin is going to produce. This patch also add

[clang] AMDGPU: Rename and add bf16 support for global_load_tr builtins (PR #86202)

2024-03-21 Thread via cfe-commits
llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: Changpeng Fang (changpeng) Changes Make the name of a clang builtin as close to the mnemonic instruction name as possible. The data type suffix may not be enough to tell what instruction the builtin is going to produce. This