Skip to content

[HLSL][OpenCL] Strip addrspace from implicit cast diags #135830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2025
Merged

[HLSL][OpenCL] Strip addrspace from implicit cast diags #135830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// 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);
Comment on lines +11367 to +11370
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (pruneControlFlow) {
S.DiagRuntimeBehavior(E->getExprLoc(), E,
S.PDiag(diag)
Expand Down
12 changes: 12 additions & 0 deletions clang/test/SemaHLSL/Language/ImpCastAddrSpace.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'}}
}
2 changes: 1 addition & 1 deletion clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading