[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-16 Thread Chris B via cfe-commits

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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Chris B via cfe-commits


@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing
+  // address space annotations isn't really useful. The warnings aren't because
+  // you're converting a `private int` to `unsigned int`, it is because you're
+  // conerting `int` to `unsigned int`.
+  if (SourceType.hasAddressSpace())
+SourceType = S.getASTContext().removeAddrSpaceQualType(SourceType);
+  if (T.hasAddressSpace())
+T = S.getASTContext().removeAddrSpaceQualType(T);

llvm-beanz wrote:

If we added an assert it would really be something like:
```
assert(!S.getASTContext().hasSameUnqualifiedType(SourceType, T) && "");
```

My feeling is that doesn't really add a lot of value here because so many 
things in Sema would have to go wrong for us to generate implicit cast AST 
nodes other than qualification conversions where the source and destination 
types have the same unqualified types.

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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Justin Bogner via cfe-commits

https://github.com/bogner approved this pull request.


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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Justin Bogner via cfe-commits


@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing
+  // address space annotations isn't really useful. The warnings aren't because
+  // you're converting a `private int` to `unsigned int`, it is because you're
+  // conerting `int` to `unsigned int`.
+  if (SourceType.hasAddressSpace())
+SourceType = S.getASTContext().removeAddrSpaceQualType(SourceType);
+  if (T.hasAddressSpace())
+T = S.getASTContext().removeAddrSpaceQualType(T);

bogner wrote:

Should we assert here that `SourceType` and `T` aren't identical after 
stripping address spaces to protect against misuse of the function?

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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Joshua Batista via cfe-commits

https://github.com/bob80905 approved this pull request.


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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Chris B via cfe-commits


@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing

llvm-beanz wrote:

No, this should apply everywhere (although it almost certainly doesn't impact 
C/C++ because they don't really have address spaces).

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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Joshua Batista via cfe-commits


@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing

bob80905 wrote:

This was a helpful comment, but should these 2 if statements be nested in an if 
statement that checks if the language is HLSL / OpenCL?

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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)


Changes

The address space of a source value for an implicit cast isn't really relevant 
when emitting conversion warnings. Since the lvalue->rvalue cast effectively 
removes the address space they don't factor in, but they do create visual noise 
in the diagnostics.

This is a small quality-of-life fixup to get in as HLSL adopts more address 
space annotations.

---
Full diff: https://github.com/llvm/llvm-project/pull/135830.diff


3 Files Affected:

- (modified) clang/lib/Sema/SemaChecking.cpp (+8) 
- (added) clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl (+12) 
- (modified) clang/test/SemaOpenCL/cl20-device-side-enqueue.cl (+1-1) 


``diff
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d0143d29a4bcc..90293093fadd2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing
+  // address space annotations isn't really useful. The warnings aren't because
+  // you're converting a `private int` to `unsigned int`, it is because you're
+  // conerting `int` to `unsigned int`.
+  if (SourceType.hasAddressSpace())
+SourceType = S.getASTContext().removeAddrSpaceQualType(SourceType);
+  if (T.hasAddressSpace())
+T = S.getASTContext().removeAddrSpaceQualType(T);
   if (pruneControlFlow) {
 S.DiagRuntimeBehavior(E->getExprLoc(), E,
   S.PDiag(diag)
diff --git a/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl 
b/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl
new file mode 100644
index 0..61e71b219b721
--- /dev/null
+++ b/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library 
-finclude-default-header -Wconversion -fnative-half-type %s -verify
+
+static double D = 2.0;
+static int I = D; // expected-warning{{implicit conversion turns 
floating-point number into integer: 'double' to 'int'}}
+groupshared float F = I; // expected-warning{{implicit conversion from 'int' 
to 'float' may lose precision}}
+
+export void fn() {
+  half d = I; // expected-warning{{implicit conversion from 'int' to 'half' 
may lose precision}}
+  int i = D; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  int j = F; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
+  int k = d; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'half' to 'int'}}
+}
diff --git a/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl 
b/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
index 36b901fc5f29e..524de8ce2f7dc 100644
--- a/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ b/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -97,7 +97,7 @@ kernel void enqueue_kernel_tests(void) {
  },
  c, 1024L);
 #ifdef WCONV
-// expected-warning-re@-2{{implicit conversion changes signedness: '__private 
char' to 'unsigned {{int|long}}'}}
+// expected-warning-re@-2{{implicit conversion changes signedness: 'char' to 
'unsigned {{int|long}}'}}
 #endif
 #define UINT_MAX 4294967295
 

``




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


[clang] [HLSL][OpenCL] Strip addrspace from implicit cast diags (PR #135830)

2025-04-15 Thread Chris B via cfe-commits

https://github.com/llvm-beanz created 
https://github.com/llvm/llvm-project/pull/135830

The address space of a source value for an implicit cast isn't really relevant 
when emitting conversion warnings. Since the lvalue->rvalue cast effectively 
removes the address space they don't factor in, but they do create visual noise 
in the diagnostics.

This is a small quality-of-life fixup to get in as HLSL adopts more address 
space annotations.

>From 6022f3bd57f1ebf21f49d12871c9dbae2fc8cf29 Mon Sep 17 00:00:00 2001
From: Chris Bieneman 
Date: Tue, 15 Apr 2025 13:20:41 -0500
Subject: [PATCH] [HLSL][OpenCL] Strip addrspace from implicit cast diags

The address space of a source value for an implicit cast isn't really
relevant when emitting conversion warnings. Since the lvalue->rvalue
cast effectively removes the address space they don't factor in, but
they do create visual noise in the diagnostics.

This is a small quality-of-life fixup to get in as HLSL adopts more
address space annotations.
---
 clang/lib/Sema/SemaChecking.cpp|  8 
 clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl | 12 
 clang/test/SemaOpenCL/cl20-device-side-enqueue.cl  |  2 +-
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d0143d29a4bcc..90293093fadd2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11360,6 +11360,14 @@ static void AnalyzeAssignment(Sema &S, BinaryOperator 
*E) {
 static void DiagnoseImpCast(Sema &S, Expr *E, QualType SourceType, QualType T,
 SourceLocation CContext, unsigned diag,
 bool pruneControlFlow = false) {
+  // For languages like HLSL and OpenCL, implicit conversion diagnostics 
listing
+  // address space annotations isn't really useful. The warnings aren't because
+  // you're converting a `private int` to `unsigned int`, it is because you're
+  // conerting `int` to `unsigned int`.
+  if (SourceType.hasAddressSpace())
+SourceType = S.getASTContext().removeAddrSpaceQualType(SourceType);
+  if (T.hasAddressSpace())
+T = S.getASTContext().removeAddrSpaceQualType(T);
   if (pruneControlFlow) {
 S.DiagRuntimeBehavior(E->getExprLoc(), E,
   S.PDiag(diag)
diff --git a/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl 
b/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl
new file mode 100644
index 0..61e71b219b721
--- /dev/null
+++ b/clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library 
-finclude-default-header -Wconversion -fnative-half-type %s -verify
+
+static double D = 2.0;
+static int I = D; // expected-warning{{implicit conversion turns 
floating-point number into integer: 'double' to 'int'}}
+groupshared float F = I; // expected-warning{{implicit conversion from 'int' 
to 'float' may lose precision}}
+
+export void fn() {
+  half d = I; // expected-warning{{implicit conversion from 'int' to 'half' 
may lose precision}}
+  int i = D; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  int j = F; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
+  int k = d; // expected-warning{{implicit conversion turns floating-point 
number into integer: 'half' to 'int'}}
+}
diff --git a/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl 
b/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
index 36b901fc5f29e..524de8ce2f7dc 100644
--- a/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ b/clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -97,7 +97,7 @@ kernel void enqueue_kernel_tests(void) {
  },
  c, 1024L);
 #ifdef WCONV
-// expected-warning-re@-2{{implicit conversion changes signedness: '__private 
char' to 'unsigned {{int|long}}'}}
+// expected-warning-re@-2{{implicit conversion changes signedness: 'char' to 
'unsigned {{int|long}}'}}
 #endif
 #define UINT_MAX 4294967295
 

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