[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-09-18 Thread Tim Gu via cfe-commits 

https://github.com/tcgu-amd updated 
https://github.com/llvm/llvm-project/pull/158145

>From 2790a7a05ab9453176a29ce7f5c3def7e4c9a1ae Mon Sep 17 00:00:00 2001
From: Tim Gu 
Date: Thu, 18 Sep 2025 13:03:11 -0400
Subject: [PATCH] Removed custom type checking & changed type string from
 __fp16 to _Float16

---
 clang/include/clang/Basic/BuiltinsAMDGPU.def | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/BuiltinsAMDGPU.def 
b/clang/include/clang/Basic/BuiltinsAMDGPU.def
index fda16e42d2c6b..4271bd2d55618 100644
--- a/clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ b/clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -268,7 +268,7 @@ TARGET_BUILTIN(__builtin_amdgcn_fmed3h, "", "nc", 
"gfx9-insts")
 
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f64, "dd*1d", "t", 
"gfx90a-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_f32, "ff*1f", "t", 
"atomic-fadd-rtn-insts")
-TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2hV2h*1V2h", "t", 
"atomic-buffer-global-pk-add-f16-insts")
+TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2f16, "V2xV2x*1V2x", "n", 
"atomic-buffer-global-pk-add-f16-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmin_f64, "dd*1d", "t", 
"gfx90a-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fmax_f64, "dd*1d", "t", 
"gfx90a-insts")
 
@@ -280,11 +280,11 @@ TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f64, 
"dd*3d", "t", "gfx90a-insts"
 TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_f32, "ff*3f", "t", "gfx8-insts")
 
 TARGET_BUILTIN(__builtin_amdgcn_flat_atomic_fadd_f32, "ff*0f", "t", 
"gfx940-insts")
-TARGET_BUILTIN(__builtin_amdgcn_flat_atomic_fadd_v2f16, "V2hV2h*0V2h", "t", 
"atomic-flat-pk-add-16-insts")
+TARGET_BUILTIN(__builtin_amdgcn_flat_atomic_fadd_v2f16, "V2xV2x*0V2x", "n", 
"atomic-flat-pk-add-16-insts")
 TARGET_BUILTIN(__builtin_amdgcn_flat_atomic_fadd_v2bf16, "V2sV2s*0V2s", "t", 
"atomic-flat-pk-add-16-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_atomic_fadd_v2bf16, "V2sV2s*1V2s", "t", 
"atomic-global-pk-add-bf16-inst")
 TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_v2bf16, "V2sV2s*3V2s", "t", 
"atomic-ds-pk-add-16-insts")
-TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_v2f16, "V2hV2h*3V2h", "t", 
"atomic-ds-pk-add-16-insts")
+TARGET_BUILTIN(__builtin_amdgcn_ds_atomic_fadd_v2f16, "V2xV2x*3V2x", "n", 
"atomic-ds-pk-add-16-insts")
 TARGET_BUILTIN(__builtin_amdgcn_load_to_lds, "vv*v*3IUiIiIUi", "", 
"vmem-to-lds-load-insts")
 TARGET_BUILTIN(__builtin_amdgcn_global_load_lds, "vv*1v*3IUiIiIUi", "t", 
"vmem-to-lds-load-insts")
 

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-10-18 Thread Tim Gu via cfe-commits 


@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -verify %s
+
+// Test semantic analysis for AMDGCN atomic fadd v2f16 builtins
+// These tests ensure proper type checking for the builtin arguments
+
+typedef _Float16 v2f16 __attribute__((ext_vector_type(2)));
+typedef float v2f32 __attribute__((ext_vector_type(2)));
+typedef _Float16 v4f16 __attribute__((ext_vector_type(4)));
+

tcgu-amd wrote:

I think the type was from hip though. How do I add a type from hip_fp16 to a 
clang test? Thanks! 

https://github.com/llvm/llvm-project/pull/158145
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-09-12 Thread Tim Gu via cfe-commits 


@@ -436,4 +440,32 @@ void SemaAMDGPU::handleAMDGPUMaxNumWorkGroupsAttr(Decl *D,
   addAMDGPUMaxNumWorkGroupsAttr(D, AL, AL.getArgAsExpr(0), YExpr, ZExpr);
 }
 
+
+bool SemaAMDGPU::checkAMDGCNAtomicFaddV2F16Type(CallExpr *TheCall) {
+  // Check that the pointer argument is a pointer to v2f16
+
+  Expr *Arg = TheCall->getArg(1);
+  QualType ArgType = Arg->getType();
+
+  // Check if it's a vector type
+  if (!ArgType->isVectorType()) {
+Diag(Arg->getBeginLoc(), diag::err_typecheck_call_different_arg_types)
+<< "expected _Float16 vector of length 2" << ArgType
+<< Arg->getSourceRange();
+return true;
+  }
+
+  const VectorType *VT = ArgType->getAs();
+
+  // Check element type (should be _Float16) and vector length (should be 2)
+  QualType ElementType = VT->getElementType();
+  if (!ElementType->isFloat16Type() || VT->getNumElements() != 2) {

tcgu-amd wrote:

Thanks for the comment! That's what I thought as well, but for some reason 
without explicit checking it here clang does not seem to catch the type 
mismatch. I am new to clang development so I don't know if that's expected. 

https://github.com/llvm/llvm-project/pull/158145
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-09-13 Thread Tim Gu via cfe-commits 


@@ -436,4 +440,32 @@ void SemaAMDGPU::handleAMDGPUMaxNumWorkGroupsAttr(Decl *D,
   addAMDGPUMaxNumWorkGroupsAttr(D, AL, AL.getArgAsExpr(0), YExpr, ZExpr);
 }
 
+
+bool SemaAMDGPU::checkAMDGCNAtomicFaddV2F16Type(CallExpr *TheCall) {
+  // Check that the pointer argument is a pointer to v2f16
+
+  Expr *Arg = TheCall->getArg(1);
+  QualType ArgType = Arg->getType();
+
+  // Check if it's a vector type
+  if (!ArgType->isVectorType()) {
+Diag(Arg->getBeginLoc(), diag::err_typecheck_call_different_arg_types)
+<< "expected _Float16 vector of length 2" << ArgType

tcgu-amd wrote:

There doesn't seem to be a "expect vector" message so I resorted to this. What 
do you suggest I use instead? Thanks!

https://github.com/llvm/llvm-project/pull/158145
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-09-13 Thread Tim Gu via cfe-commits 

https://github.com/tcgu-amd created 
https://github.com/llvm/llvm-project/pull/158145

Addresses https://github.com/ROCm/ROCm/issues/5253. 

The builtin expects a vector _Float16 of length 2, but clang does not emit 
errors when the wrong argument types is supplied (e.g. half2). This causes the 
compilation to pass but error during LTO. 

This fix the issue by adding sema checks to the builtins. 

>From 2192052dff2df318a35b8e2b28c7cf4f75070734 Mon Sep 17 00:00:00 2001
From: Tim Gu 
Date: Thu, 11 Sep 2025 15:26:31 -0400
Subject: [PATCH] Added clang sema check to ensure atomic_fadd_v2f16 builtin
 uses v2f16 arg.

---
 clang/include/clang/Sema/SemaAMDGPU.h |  2 ++
 clang/lib/Sema/SemaAMDGPU.cpp | 32 +++
 2 files changed, 34 insertions(+)

diff --git a/clang/include/clang/Sema/SemaAMDGPU.h 
b/clang/include/clang/Sema/SemaAMDGPU.h
index bac812a9d4fcf..9ca4418349fff 100644
--- a/clang/include/clang/Sema/SemaAMDGPU.h
+++ b/clang/include/clang/Sema/SemaAMDGPU.h
@@ -31,6 +31,8 @@ class SemaAMDGPU : public SemaBase {
   bool checkMovDPPFunctionCall(CallExpr *TheCall, unsigned NumArgs,
unsigned NumDataArgs);
 
+  bool checkAMDGCNAtomicFaddV2F16Type(CallExpr *TheCall);
+
   /// Create an AMDGPUWavesPerEUAttr attribute.
   AMDGPUFlatWorkGroupSizeAttr *
   CreateAMDGPUFlatWorkGroupSizeAttr(const AttributeCommonInfo &CI, Expr *Min,
diff --git a/clang/lib/Sema/SemaAMDGPU.cpp b/clang/lib/Sema/SemaAMDGPU.cpp
index baba503239e9f..c0f17f185982e 100644
--- a/clang/lib/Sema/SemaAMDGPU.cpp
+++ b/clang/lib/Sema/SemaAMDGPU.cpp
@@ -109,6 +109,10 @@ bool SemaAMDGPU::CheckAMDGCNBuiltinFunctionCall(unsigned 
BuiltinID,
   case AMDGPU::BI__builtin_amdgcn_cooperative_atomic_store_16x8B:
   case AMDGPU::BI__builtin_amdgcn_cooperative_atomic_store_8x16B:
 return checkCoopAtomicFunctionCall(TheCall, /*IsStore=*/true);
+  case AMDGPU::BI__builtin_amdgcn_global_atomic_fadd_v2f16:
+  case AMDGPU::BI__builtin_amdgcn_flat_atomic_fadd_v2f16:
+  case AMDGPU::BI__builtin_amdgcn_ds_atomic_fadd_v2f16:
+return checkAMDGCNAtomicFaddV2F16Type(TheCall);
   default:
 return false;
   }
@@ -436,4 +440,32 @@ void SemaAMDGPU::handleAMDGPUMaxNumWorkGroupsAttr(Decl *D,
   addAMDGPUMaxNumWorkGroupsAttr(D, AL, AL.getArgAsExpr(0), YExpr, ZExpr);
 }
 
+
+bool SemaAMDGPU::checkAMDGCNAtomicFaddV2F16Type(CallExpr *TheCall) {
+  // Check that the pointer argument is a pointer to v2f16
+
+  Expr *Arg = TheCall->getArg(1);
+  QualType ArgType = Arg->getType();
+
+  // Check if it's a vector type
+  if (!ArgType->isVectorType()) {
+Diag(Arg->getBeginLoc(), diag::err_typecheck_call_different_arg_types)
+<< "expected _Float16 vector of length 2" << ArgType
+<< Arg->getSourceRange();
+return true;
+  }
+
+  const VectorType *VT = ArgType->getAs();
+
+  // Check element type (should be _Float16) and vector length (should be 2)
+  QualType ElementType = VT->getElementType();
+  if (!ElementType->isFloat16Type() || VT->getNumElements() != 2) {
+Diag(Arg->getBeginLoc(), diag::err_typecheck_call_different_arg_types)
+<< "expected _Float16 vector of length 2" << ArgType
+<< Arg->getSourceRange();
+return true;
+  }
+
+  return false;
+}
 } // namespace clang

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Add sema check for global_atomic_fadd_v2f16 builtin (PR #158145)

2025-10-17 Thread Tim Gu via cfe-commits 

https://github.com/tcgu-amd edited 
https://github.com/llvm/llvm-project/pull/158145
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits