[clang-tools-extra] Fix Default Asset File locator for clang docs (PR #97505)
@@ -167,7 +167,14 @@ llvm::Error getDefaultAssetFiles(const char *Argv0, llvm::SmallString<128> AssetsPath; AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath); - llvm::sys::path::append(AssetsPath, "..", "share", "clang-doc"); + llvm::sys::path::append(AssetsPath, ".."); + llvm::SmallString<128> tempCopyDbg; + llvm::sys::path::native(AssetsPath, tempCopyDbg); + llvm::sys::path::append(tempCopyDbg, "Debug"); llvm-beanz wrote: Could this also be "Release"? It seems like this should probably be built like the compiler resource directory is where the assets get installed into a known binary relative path. https://github.com/llvm/llvm-project/pull/97505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Implement `export` keyword (PR #96823)
llvm-beanz wrote: > The grammer looks exactly the same with modules. May it be compatible with > modules? The grammar is the same (although HLSL only supports the `export` keyword in a subset of the places C++ does), but the behavior isn't the same. We may look to shift HLSL's behavior to match C++ in the future but for compatibility with existing code we need to support the HLSL behavior. https://github.com/llvm/llvm-project/pull/96823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
@@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify + +// previously, this test would result in an error shown below on the line that +// declares variable a in struct Eg9: +// error: use of undeclared identifier +// 'SV_DispatchThreadID' +// This is because the annotation is parsed as if it was a c++ bit field, and an identifier +// that represents an integer is expected, but not found. + +// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls. +// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly +// attached as an attribute to the member in the struct in the AST. However, in this case +// this can't happen presently because there are other issues with annotations on field decls. +// This test just ensures we make progress by moving the validation error from the realm of +// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation. + +struct Eg9{ +// expected-error@+1{{attribute 'SV_DispatchThreadID' only applies to parameter}} + int a : SV_DispatchThreadID; llvm-beanz wrote: I suspect we were erroring there because we don't have CodeGen support or Sema verification for struct inputs. I think we can resolve that in a separate change. https://github.com/llvm/llvm-project/pull/96346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
@@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify + +// previously, this test would result in an error shown below on the line that +// declares variable a in struct Eg9: +// error: use of undeclared identifier +// 'SV_DispatchThreadID' +// This is because the annotation is parsed as if it was a c++ bit field, and an identifier +// that represents an integer is expected, but not found. + +// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls. +// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly +// attached as an attribute to the member in the struct in the AST. However, in this case +// this can't happen presently because there are other issues with annotations on field decls. +// This test just ensures we make progress by moving the validation error from the realm of +// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation. + +struct Eg9{ +// expected-error@+1{{attribute 'SV_DispatchThreadID' only applies to parameter}} + int a : SV_DispatchThreadID; llvm-beanz wrote: No, my point was that the error you're checking for here isn't an error we can rely on being persistent because that attribute _is_ allowed to apply to struct members. https://github.com/llvm/llvm-project/pull/96346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
@@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify + +// expected-no-diagnostics + +struct MyBitFields { +unsigned int field1 : 3; // 3 bits for field1 +unsigned int field2 : 4; // 4 bits for field2 +int field3 : 5; // 5 bits for field3 (signed) llvm-beanz wrote: Can we verify the AST for these to make sure they are parsed as bitfield declarations rather than just making sure it doesn't produce errors? https://github.com/llvm/llvm-project/pull/96346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Move more things out of `SemaChecking.cpp` (PR #96641)
https://github.com/llvm-beanz approved this pull request. HLSL changes look good to me. https://github.com/llvm/llvm-project/pull/96641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Rework implicit conversion sequences (PR #96011)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/96011 >From 0baa7ec1ded4fa093092d491eaa2c803b736742b Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 18 Jun 2024 20:25:57 -0500 Subject: [PATCH 1/3] [HLSL] Rework implicit conversion sequences This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are: * Exact match * Scalar Widening (i.e. splat) * Promotion * Scalar Widening with Promotion * Conversion * Scalar Widening with Conversion * Dimension Reduction (i.e. truncation) * Dimension Reduction with Promotion * Dimension Reduction with Conversion In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in `CompareStandardConversionSequences`. I belive the added conversion rank values provide a simpler approach, but welcome feedback. --- clang/docs/HLSL/ExpectedDifferences.rst | 11 ++ clang/include/clang/Sema/Overload.h | 38 +++- clang/lib/Sema/SemaExprCXX.cpp| 163 + clang/lib/Sema/SemaOverload.cpp | 85 - .../standard_conversion_sequences.hlsl| 18 +- clang/test/CodeGenHLSL/builtins/dot.hlsl | 34 +--- clang/test/CodeGenHLSL/builtins/lerp.hlsl | 24 --- clang/test/CodeGenHLSL/builtins/mad.hlsl | 22 --- .../SemaHLSL/ScalarOverloadResolution.hlsl| 28 ++- .../SemaHLSL/SplatOverloadResolution.hlsl | 166 ++ .../TruncationOverloadResolution.hlsl | 68 +++ .../Types/BuiltinVector/ScalarSwizzles.hlsl | 2 +- .../VectorElementOverloadResolution.hlsl | 36 ++-- .../SemaHLSL/VectorOverloadResolution.hlsl| 10 +- .../standard_conversion_sequences.hlsl| 16 +- 15 files changed, 447 insertions(+), 274 deletions(-) create mode 100644 clang/test/SemaHLSL/SplatOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/TruncationOverloadResolution.hlsl diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index d1b6010f10f43..a29b6348e0b8e 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -67,12 +67,16 @@ behavior between Clang and DXC. Some examples include: void takesDoubles(double, double, double); cbuffer CB { +bool B; uint U; int I; float X, Y, Z; double3 A, B; } + void twoParams(int, int); + void twoParams(float, float); + export void call() { halfOrInt16(U); // DXC: Fails with call ambiguous between int16_t and uint16_t overloads // Clang: Resolves to halfOrInt16(uint16_t). @@ -98,6 +102,13 @@ behavior between Clang and DXC. Some examples include: // FXC: Expands to compute double dot product with fmul/fadd // Clang: Resolves to dot(float3, float3), emits conversion warnings. + #ifndef IGNORE_ERRORS +tan(B); // DXC: resolves to tan(float). +// Clang: Fails to resolve, ambiguous between integer types. + +twoParams(I, X); // DXC: resolves twoParams(int, int). + // Clang: Fails to resolve ambiguous conversions. + #endif } .. note:: diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h index 4a5c9e8ca1229..9d8b797af6663 100644 --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -201,6 +201,9 @@ class Sema; /// HLSL non-decaying array rvalue cast. ICK_HLSL_Array_RValue, +// HLSL vector splat from scalar or boolean type. +ICK_HLSL_Vector_Splat, + /// The number of conversion kinds ICK_Num_Conversion_Kinds, }; @@ -213,15 +216,27 @@ class Sema; /// Exact Match ICR_Exact_Match = 0, +/// HLSL Scalar Widening +ICR_HLSL_Scalar_Widening, + /// Promotion ICR_Promotion, +/// HLSL Scalar Widening with promotion +ICR_HLSL_Scalar_Widening_Promotion, + +/// HLSL Matching Dimension Reduction +ICR_HLSL_Dimension_Reduction, + /// Conversion ICR_Conversion, /// OpenCL Scalar Widening ICR_OCL_Scalar_Widening, +/// HLSL Scalar Widening with conversion +ICR_HLSL_Scalar_Widening_Conversion, + /// Complex <-> Real conversion ICR_Complex_Real_Conversion, @@ -233,11 +248,21 @@ class Sema; /// Conversion not allowed by the C standard, but that we accept as an /// extension anyway. -ICR_C_Conversion_Extension +ICR_C_Conversion_Extension, + +/// HLSL
[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
@@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify + +// previously, this test would result in an error shown below on the line that +// declares variable a in struct Eg9: +// error: use of undeclared identifier +// 'SV_DispatchThreadID' +// This is because the annotation is parsed as if it was a c++ bit field, and an identifier +// that represents an integer is expected, but not found. + +// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls. +// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly +// attached as an attribute to the member in the struct in the AST. However, in this case +// this can't happen presently because there are other issues with annotations on field decls. +// This test just ensures we make progress by moving the validation error from the realm of +// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation. + +struct Eg9{ +// expected-error@+1{{attribute 'SV_DispatchThreadID' only applies to parameter}} + int a : SV_DispatchThreadID; llvm-beanz wrote: Can you also make sure to have a test to verify bitfield parsing still works? https://github.com/llvm/llvm-project/pull/96346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ParserHLSL] Attempt to parse HLSL annotations on Field Decls. (PR #96346)
@@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify + +// previously, this test would result in an error shown below on the line that +// declares variable a in struct Eg9: +// error: use of undeclared identifier +// 'SV_DispatchThreadID' +// This is because the annotation is parsed as if it was a c++ bit field, and an identifier +// that represents an integer is expected, but not found. + +// This test ensures that hlsl annotations are attempted to be parsed when parsing struct decls. +// Ideally, we'd validate this behavior by ensuring the annotation is parsed and properly +// attached as an attribute to the member in the struct in the AST. However, in this case +// this can't happen presently because there are other issues with annotations on field decls. +// This test just ensures we make progress by moving the validation error from the realm of +// C++ and expecting bitfields, to HLSL and a specialized error for the recognized annotation. + +struct Eg9{ +// expected-error@+1{{attribute 'SV_DispatchThreadID' only applies to parameter}} + int a : SV_DispatchThreadID; llvm-beanz wrote: Sadly this code is actually valid: https://godbolt.org/z/a6q3P1h65 https://github.com/llvm/llvm-project/pull/96346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Rework implicit conversion sequences (PR #96011)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/96011 >From 0baa7ec1ded4fa093092d491eaa2c803b736742b Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 18 Jun 2024 20:25:57 -0500 Subject: [PATCH 1/2] [HLSL] Rework implicit conversion sequences This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are: * Exact match * Scalar Widening (i.e. splat) * Promotion * Scalar Widening with Promotion * Conversion * Scalar Widening with Conversion * Dimension Reduction (i.e. truncation) * Dimension Reduction with Promotion * Dimension Reduction with Conversion In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in `CompareStandardConversionSequences`. I belive the added conversion rank values provide a simpler approach, but welcome feedback. --- clang/docs/HLSL/ExpectedDifferences.rst | 11 ++ clang/include/clang/Sema/Overload.h | 38 +++- clang/lib/Sema/SemaExprCXX.cpp| 163 + clang/lib/Sema/SemaOverload.cpp | 85 - .../standard_conversion_sequences.hlsl| 18 +- clang/test/CodeGenHLSL/builtins/dot.hlsl | 34 +--- clang/test/CodeGenHLSL/builtins/lerp.hlsl | 24 --- clang/test/CodeGenHLSL/builtins/mad.hlsl | 22 --- .../SemaHLSL/ScalarOverloadResolution.hlsl| 28 ++- .../SemaHLSL/SplatOverloadResolution.hlsl | 166 ++ .../TruncationOverloadResolution.hlsl | 68 +++ .../Types/BuiltinVector/ScalarSwizzles.hlsl | 2 +- .../VectorElementOverloadResolution.hlsl | 36 ++-- .../SemaHLSL/VectorOverloadResolution.hlsl| 10 +- .../standard_conversion_sequences.hlsl| 16 +- 15 files changed, 447 insertions(+), 274 deletions(-) create mode 100644 clang/test/SemaHLSL/SplatOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/TruncationOverloadResolution.hlsl diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index d1b6010f10f43..a29b6348e0b8e 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -67,12 +67,16 @@ behavior between Clang and DXC. Some examples include: void takesDoubles(double, double, double); cbuffer CB { +bool B; uint U; int I; float X, Y, Z; double3 A, B; } + void twoParams(int, int); + void twoParams(float, float); + export void call() { halfOrInt16(U); // DXC: Fails with call ambiguous between int16_t and uint16_t overloads // Clang: Resolves to halfOrInt16(uint16_t). @@ -98,6 +102,13 @@ behavior between Clang and DXC. Some examples include: // FXC: Expands to compute double dot product with fmul/fadd // Clang: Resolves to dot(float3, float3), emits conversion warnings. + #ifndef IGNORE_ERRORS +tan(B); // DXC: resolves to tan(float). +// Clang: Fails to resolve, ambiguous between integer types. + +twoParams(I, X); // DXC: resolves twoParams(int, int). + // Clang: Fails to resolve ambiguous conversions. + #endif } .. note:: diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h index 4a5c9e8ca1229..9d8b797af6663 100644 --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -201,6 +201,9 @@ class Sema; /// HLSL non-decaying array rvalue cast. ICK_HLSL_Array_RValue, +// HLSL vector splat from scalar or boolean type. +ICK_HLSL_Vector_Splat, + /// The number of conversion kinds ICK_Num_Conversion_Kinds, }; @@ -213,15 +216,27 @@ class Sema; /// Exact Match ICR_Exact_Match = 0, +/// HLSL Scalar Widening +ICR_HLSL_Scalar_Widening, + /// Promotion ICR_Promotion, +/// HLSL Scalar Widening with promotion +ICR_HLSL_Scalar_Widening_Promotion, + +/// HLSL Matching Dimension Reduction +ICR_HLSL_Dimension_Reduction, + /// Conversion ICR_Conversion, /// OpenCL Scalar Widening ICR_OCL_Scalar_Widening, +/// HLSL Scalar Widening with conversion +ICR_HLSL_Scalar_Widening_Conversion, + /// Complex <-> Real conversion ICR_Complex_Real_Conversion, @@ -233,11 +248,21 @@ class Sema; /// Conversion not allowed by the C standard, but that we accept as an /// extension anyway. -ICR_C_Conversion_Extension +ICR_C_Conversion_Extension, + +/// HLSL
[clang] [HLSL] Rework implicit conversion sequences (PR #96011)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/96011 This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. We've decided that we're going to break compatibility with DXC here, and we may port this new behavior over to DXC instead. This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are: * Exact match * Scalar Widening (i.e. splat) * Promotion * Scalar Widening with Promotion * Conversion * Scalar Widening with Conversion * Dimension Reduction (i.e. truncation) * Dimension Reduction with Promotion * Dimension Reduction with Conversion In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in `CompareStandardConversionSequences`. I believe the added conversion rank values provide a simpler approach, but feedback is appreciated. The HLSL language spec updates are in the PR here: https://github.com/microsoft/hlsl-specs/pull/261 >From 0baa7ec1ded4fa093092d491eaa2c803b736742b Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 18 Jun 2024 20:25:57 -0500 Subject: [PATCH] [HLSL] Rework implicit conversion sequences This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are: * Exact match * Scalar Widening (i.e. splat) * Promotion * Scalar Widening with Promotion * Conversion * Scalar Widening with Conversion * Dimension Reduction (i.e. truncation) * Dimension Reduction with Promotion * Dimension Reduction with Conversion In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in `CompareStandardConversionSequences`. I belive the added conversion rank values provide a simpler approach, but welcome feedback. --- clang/docs/HLSL/ExpectedDifferences.rst | 11 ++ clang/include/clang/Sema/Overload.h | 38 +++- clang/lib/Sema/SemaExprCXX.cpp| 163 + clang/lib/Sema/SemaOverload.cpp | 85 - .../standard_conversion_sequences.hlsl| 18 +- clang/test/CodeGenHLSL/builtins/dot.hlsl | 34 +--- clang/test/CodeGenHLSL/builtins/lerp.hlsl | 24 --- clang/test/CodeGenHLSL/builtins/mad.hlsl | 22 --- .../SemaHLSL/ScalarOverloadResolution.hlsl| 28 ++- .../SemaHLSL/SplatOverloadResolution.hlsl | 166 ++ .../TruncationOverloadResolution.hlsl | 68 +++ .../Types/BuiltinVector/ScalarSwizzles.hlsl | 2 +- .../VectorElementOverloadResolution.hlsl | 36 ++-- .../SemaHLSL/VectorOverloadResolution.hlsl| 10 +- .../standard_conversion_sequences.hlsl| 16 +- 15 files changed, 447 insertions(+), 274 deletions(-) create mode 100644 clang/test/SemaHLSL/SplatOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/TruncationOverloadResolution.hlsl diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index d1b6010f10f43..a29b6348e0b8e 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -67,12 +67,16 @@ behavior between Clang and DXC. Some examples include: void takesDoubles(double, double, double); cbuffer CB { +bool B; uint U; int I; float X, Y, Z; double3 A, B; } + void twoParams(int, int); + void twoParams(float, float); + export void call() { halfOrInt16(U); // DXC: Fails with call ambiguous between int16_t and uint16_t overloads // Clang: Resolves to halfOrInt16(uint16_t). @@ -98,6 +102,13 @@ behavior between Clang and DXC. Some examples include: // FXC: Expands to compute double dot product with fmul/fadd // Clang: Resolves to dot(float3, float3), emits conversion warnings. + #ifndef IGNORE_ERRORS +tan(B); // DXC: resolves to tan(float). +// Clang: Fails to resolve, ambiguous between integer types. + +twoParams(I, X); // DXC: resolves twoParams(int, int). + // Clang: Fails to resolve ambiguous conversions. + #endif } .. note:: diff --git a/clang/include/clang/Sema/Overload.h
[clang] [HLSL] Strict Availability Diagnostics (PR #93860)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/93860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Use hlsl vector template in type printer (PR #95489)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/95489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Use hlsl vector template in type printer (PR #95489)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/95489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Use hlsl vector template in type printer (PR #95489)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/95489 In HLSL we really want to be using the HLSL vector template and other built-in sugared spellings for some builtin types. This updates the type printer to take an option to use HLSL type spellings. This changes printing vector type names from: T __attribute__((ext_vector_type(N))) To: vector >From e391736c66563ac7ddf8cffe4f686535158e3b25 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Thu, 13 Jun 2024 18:21:56 -0500 Subject: [PATCH] [HLSL] Use hlsl vector template in type printer In HLSL we really want to be using the HLSL vector template and other built-in sugared spellings for some builtin types. This updates the type printer to take an option to use HLSL type spellings. This changes printing vector type names from: T __attribute__((ext_vector_type(N))) To: vector --- clang/include/clang/AST/PrettyPrinter.h | 7 +- clang/lib/AST/TypePrinter.cpp | 32 +-- clang/test/AST/HLSL/pch.hlsl | 2 +- clang/test/AST/HLSL/pch_with_buf.hlsl | 2 +- clang/test/AST/HLSL/vector-alias.hlsl | 16 ++-- clang/test/AST/HLSL/vector-constructors.hlsl | 22 ++--- clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl | 2 +- .../test/SemaHLSL/BuiltIns/clamp-errors.hlsl | 2 +- clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl | 2 +- clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl | 2 +- clang/test/SemaHLSL/BuiltIns/mad-errors.hlsl | 2 +- .../test/SemaHLSL/BuiltIns/vector-errors.hlsl | 4 +- .../BuiltinVector/ScalarSwizzleErrors.hlsl| 4 +- .../Types/BuiltinVector/ScalarSwizzles.hlsl | 56 ++-- .../SemaHLSL/VectorOverloadResolution.hlsl| 30 +++ .../standard_conversion_sequences.hlsl| 90 +-- 16 files changed, 149 insertions(+), 126 deletions(-) diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h index da276e26049b0..332ac3c6a004a 100644 --- a/clang/include/clang/AST/PrettyPrinter.h +++ b/clang/include/clang/AST/PrettyPrinter.h @@ -77,7 +77,7 @@ struct PrintingPolicy { PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true), UsePreferredNames(true), AlwaysIncludeTypeForTemplateArgument(false), CleanUglifiedParameters(false), EntireContentsOfLargeArray(true), -UseEnumerators(true) {} +UseEnumerators(true), UseHLSLTypes(LO.HLSL) {} /// Adjust this printing policy for cases where it's known that we're /// printing C++ code (for instance, if AST dumping reaches a C++-only @@ -342,6 +342,11 @@ struct PrintingPolicy { LLVM_PREFERRED_TYPE(bool) unsigned UseEnumerators : 1; + /// Whether or not we're printing known HLSL code and should print HLSL + /// sugared types when possible. + LLVM_PREFERRED_TYPE(bool) + unsigned UseHLSLTypes : 1; + /// Callbacks to use to allow the behavior of printing to be customized. const PrintingCallbacks *Callbacks = nullptr; }; diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp index 58d01705d607b..4add4d3af69a3 100644 --- a/clang/lib/AST/TypePrinter.cpp +++ b/clang/lib/AST/TypePrinter.cpp @@ -644,16 +644,25 @@ void TypePrinter::printDependentAddressSpaceAfter( void TypePrinter::printDependentSizedExtVectorBefore( const DependentSizedExtVectorType *T, raw_ostream ) { + if (Policy.UseHLSLTypes) +OS << "vector<"; printBefore(T->getElementType(), OS); } void TypePrinter::printDependentSizedExtVectorAfter( const DependentSizedExtVectorType *T, raw_ostream ) { - OS << " __attribute__((ext_vector_type("; - if (T->getSizeExpr()) -T->getSizeExpr()->printPretty(OS, nullptr, Policy); - OS << ")))"; + if (Policy.UseHLSLTypes) { +OS << ", "; +if (T->getSizeExpr()) + T->getSizeExpr()->printPretty(OS, nullptr, Policy); +OS << ">"; + } else { +OS << " __attribute__((ext_vector_type("; +if (T->getSizeExpr()) + T->getSizeExpr()->printPretty(OS, nullptr, Policy); +OS << ")))"; + } printAfter(T->getElementType(), OS); } @@ -815,14 +824,23 @@ void TypePrinter::printDependentVectorAfter( void TypePrinter::printExtVectorBefore(const ExtVectorType *T, raw_ostream ) { + if (Policy.UseHLSLTypes) +OS << "vector<"; printBefore(T->getElementType(), OS); } void TypePrinter::printExtVectorAfter(const ExtVectorType *T, raw_ostream ) { printAfter(T->getElementType(), OS); - OS << " __attribute__((ext_vector_type("; - OS << T->getNumElements(); - OS << ")))"; + + if (Policy.UseHLSLTypes) { +OS << ", "; +OS << T->getNumElements(); +OS << ">"; + } else { +OS << " __attribute__((ext_vector_type("; +OS << T->getNumElements(); +OS << ")))"; + } } void
[clang] Enable unguarded availability diagnostic on instantiated template functions (PR #91699)
llvm-beanz wrote: @nico, am I correct to assume those aren't false positives, just a bunch of places that didn't warn before? https://github.com/llvm/llvm-project/pull/91699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [cmake] Respect CLANG_LINK_CLANG_DYLIB for objlibs (PR #93454)
@@ -194,18 +194,20 @@ macro(add_clang_symlink name dest) endmacro() function(clang_target_link_libraries target type) - if (TARGET obj.${target}) -target_link_libraries(obj.${target} ${ARGN}) - endif() - get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS) if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${target} IN_LIST LLVM_DRIVER_TOOLS) set(target llvm-driver) endif() if (CLANG_LINK_CLANG_DYLIB) target_link_libraries(${target} ${type} clang-cpp) +if (TARGET obj.${target}) + target_link_libraries(obj.${target} clang-cpp) +endif() else() target_link_libraries(${target} ${type} ${ARGN}) +if (TARGET obj.${target}) + target_link_libraries(obj.${target} ${ARGN}) llvm-beanz wrote: ```suggestion target_link_libraries(obj.${target} INTERFACE ${ARGN}) ``` https://github.com/llvm/llvm-project/pull/93454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [cmake] Respect CLANG_LINK_CLANG_DYLIB for objlibs (PR #93454)
@@ -194,18 +194,20 @@ macro(add_clang_symlink name dest) endmacro() function(clang_target_link_libraries target type) - if (TARGET obj.${target}) -target_link_libraries(obj.${target} ${ARGN}) - endif() - get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS) if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${target} IN_LIST LLVM_DRIVER_TOOLS) set(target llvm-driver) endif() if (CLANG_LINK_CLANG_DYLIB) target_link_libraries(${target} ${type} clang-cpp) +if (TARGET obj.${target}) + target_link_libraries(obj.${target} clang-cpp) llvm-beanz wrote: Should we specify this as an `INTERFACE` link dependency? ```suggestion target_link_libraries(obj.${target} INTERFACE clang-cpp) ``` https://github.com/llvm/llvm-project/pull/93454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [cmake] Respect CLANG_LINK_CLANG_DYLIB for objlibs (PR #93454)
https://github.com/llvm-beanz approved this pull request. I think dependencies for object libraries are implicitly `INTERAFCE`, but it might be nice to make that explicit. I know CMake complains if you mix and match implicit and explicit specification of linkage types. Otherwise LGTM. https://github.com/llvm/llvm-project/pull/93454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [cmake] Respect CLANG_LINK_CLANG_DYLIB for objlibs (PR #93454)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/93454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][cmake] Fixes for PGO builds when invoking ninja twice (PR #92591)
llvm-beanz wrote: LGTM. Sorry for the delay. https://github.com/llvm/llvm-project/pull/92591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][cmake] Fixes for PGO builds when invoking ninja twice (PR #92591)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/92591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] add loop unroll (PR #93879)
@@ -7401,7 +7495,8 @@ b for constant buffer views (CBV). Register space is specified in the format ``space[number]`` and defaults to ``space0`` if omitted. Here're resource binding examples with and without space: -.. code-block:: c++ + +.. code-block:: hlsl llvm-beanz wrote: I think the two code blocks you changed here to HLSL are fine, but just beware that we do sometimes intentionally use the C++ syntax highlighting because the HLSL highlighting isn't always as good. https://github.com/llvm/llvm-project/pull/93879 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] add loop unroll (PR #93879)
llvm-beanz wrote: Can you also add some tests for nested loops? Cases like: ```hlsl [unroll] for( int i = 0; i < 100; ++i) { // unroll this for( int j = 0; j < 100; ++j) { // but not this } } ``` https://github.com/llvm/llvm-project/pull/93879 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Use llvm::Triple::EnvironmentType instead of HLSLShaderAttr::ShaderType (PR #93847)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/93847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
https://github.com/llvm-beanz approved this pull request. I have one suggested code simplification which will also require test updates, but in general I think this is good. https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -1060,18 +1060,25 @@ static llvm::StringRef canonicalizePlatformName(llvm::StringRef Platform) { .Case("ShaderModel", "shadermodel") .Default(Platform); } -static llvm::StringRef getPrettyEnviromentName(llvm::StringRef Environment) { -return llvm::StringSwitch(Environment) - .Case("pixel", "pixel shader") - .Case("vertex", "vertex shader") - .Case("geometry", "geometry shader") - .Case("hull", "hull shader") - .Case("domain", "domain shader") - .Case("compute", "compute shader") - .Case("mesh", "mesh shader") - .Case("amplification", "amplification shader") - .Case("library", "shader library") - .Default(Environment); +static llvm::StringRef getPrettyEnviromentName(llvm::Triple::EnvironmentType EnvironmentType) { + switch (EnvironmentType) { +case llvm::Triple::Pixel: return "pixel shader"; +case llvm::Triple::Vertex: return "vertex shader"; +case llvm::Triple::Geometry: return "geometry shader"; +case llvm::Triple::Hull: return "hull shader"; +case llvm::Triple::Domain: return "domain shader"; +case llvm::Triple::Compute: return "compute shader"; +case llvm::Triple::RayGeneration: return "raygeneration shader"; +case llvm::Triple::Intersection: return "intersection shader"; +case llvm::Triple::AnyHit: return "anyhit shader"; +case llvm::Triple::ClosestHit: return "closesthit shader"; +case llvm::Triple::Miss: return "miss shader"; +case llvm::Triple::Callable: return "callable shader"; +case llvm::Triple::Mesh: return "mesh shader"; +case llvm::Triple::Amplification: return "amplification shader"; +case llvm::Triple::Library: return "shader library"; +default: return ""; + } llvm-beanz wrote: This function is really just doing the same thing as `Triple::getEnvironmentTypeName`, but instead of "pixel" it returns "pixel shader". Is that worth the code duplication? We could instead write this function as: ```suggestion static llvm::StringRef getPrettyEnviromentName(llvm::Triple::EnvironmentType EnvironmentType) { if (EnvironmentType >= Pixel && EnvironmentType <= Amplification) return Triple::getEnvironmentTypeName(EnvironmentType) return ""; } ``` https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -290,3 +294,295 @@ void SemaHLSL::DiagnoseAttrStageMismatch( << A << HLSLShaderAttr::ConvertShaderTypeToStr(Stage) << (AllowedStages.size() != 1) << join(StageStrings, ", "); } + +namespace { + +/// This class implements HLSL availability diagnostics for default +/// and relaxed mode +/// +/// The goal of this diagnostic is to emit an error or warning when an +/// unavailable API is found in a code that is reachable from the shader +/// entry function or from an exported function (when compiling shader +/// library). +/// +/// This is done by traversing the AST of all shader entry point functions +/// and of all exported functions, and any functions that are refrenced +/// from this AST. In other words, any function that are reachable from +/// the entry points. +class DiagnoseHLSLAvailability +: public RecursiveASTVisitor { + // HEKOTAS this is probably not needed + // typedef RecursiveASTVisitor Base; + + Sema + + // Stack of functions to be scaned + llvm::SmallVector DeclsToScan; + + // List of functions that were already scanned and in which environment. + // + // Maps FunctionDecl to a unsigned number that represents a set of shader + // environments the function has been scanned for. + // Since HLSLShaderAttr::ShaderType enum is generated from Attr.td and is + // defined without any assigned values, it is guaranteed to be numbered + // sequentially from 0 up and we can use it to 'index' individual bits + // in the set. + // The N'th bit in the set will be set if the function has been scanned + // in shader environment whose ShaderType integer value equals N. + // For example, if a function has been scanned in compute and pixel stage + // environment, the value will be 0x21 (11 binary) because + // (int)HLSLShaderAttr::ShaderType::Pixel == 1 and + // (int)HLSLShaderAttr::ShaderType::Compute == 5. + llvm::DenseMap ScannedDecls; + + // Do not access these directly, use the get/set methods below to make + // sure the values are in sync + llvm::Triple::EnvironmentType CurrentShaderEnvironment; + unsigned CurrentShaderStageBit; + + // True if scanning a function that was already scanned in a different + // shader stage context, and therefore we should not report issues that + // depend only on shader model version because they would be duplicate. + bool ReportOnlyShaderStageIssues; + + void SetShaderStageContext(HLSLShaderAttr::ShaderType ShaderType) { +assertunsigned)1) << (unsigned)ShaderType) != 0 && + "ShaderType is too big for this bitmap"); +CurrentShaderEnvironment = HLSLShaderAttr::getTypeAsEnvironment(ShaderType); +CurrentShaderStageBit = (1 << ShaderType); + } + void SetUnknownShaderStageContext() { +CurrentShaderEnvironment = +llvm::Triple::EnvironmentType::UnknownEnvironment; +CurrentShaderStageBit = (1 << 31); + } + llvm::Triple::EnvironmentType GetCurrentShaderEnvironment() { +return CurrentShaderEnvironment; + } + bool InUnknownShaderStageContext() { +return CurrentShaderEnvironment == + llvm::Triple::EnvironmentType::UnknownEnvironment; + } + + // Scanning methods + void HandleFunctionOrMethodRef(FunctionDecl *FD, Expr *RefExpr); + void CheckDeclAvailability(NamedDecl *D, const AvailabilityAttr *AA, + SourceRange Range); + const AvailabilityAttr *FindAvailabilityAttr(const Decl *D); + bool HasMatchingEnvironmentOrNone(const AvailabilityAttr *AA); + bool WasAlreadyScannedInCurrentShaderStage(const FunctionDecl *FD, + bool *WasNeverScanned = nullptr); + void AddToScannedFunctions(const FunctionDecl *FD); + +public: + DiagnoseHLSLAvailability(Sema ) : SemaRef(SemaRef) {} + + // AST traversal methods + void RunOnTranslationUnit(const TranslationUnitDecl *TU); + void RunOnFunction(const FunctionDecl *FD); + + bool VisitDeclRefExpr(DeclRefExpr *DRE) { +FunctionDecl *FD = llvm::dyn_cast(DRE->getDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, DRE); +return true; + } + + bool VisitMemberExpr(MemberExpr *ME) { +FunctionDecl *FD = llvm::dyn_cast(ME->getMemberDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, ME); +return true; + } +}; + +// Returns true if the function has already been scanned in the current +// shader environment. WasNeverScanned will be set to true if the function +// has never been scanned before for any shader environment. +bool DiagnoseHLSLAvailability::WasAlreadyScannedInCurrentShaderStage( +const FunctionDecl *FD, bool *WasNeverScanned) { + const unsigned = ScannedDecls.getOrInsertDefault(FD); + if (WasNeverScanned) +*WasNeverScanned = (ScannedStages == 0); + return ScannedStages & CurrentShaderStageBit; +} + +// Marks the function as scanned in the current shader environment +void DiagnoseHLSLAvailability::AddToScannedFunctions(const FunctionDecl *FD) { + unsigned = ScannedDecls.getOrInsertDefault(FD);
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -290,3 +294,295 @@ void SemaHLSL::DiagnoseAttrStageMismatch( << A << HLSLShaderAttr::ConvertShaderTypeToStr(Stage) << (AllowedStages.size() != 1) << join(StageStrings, ", "); } + +namespace { + +/// This class implements HLSL availability diagnostics for default +/// and relaxed mode +/// +/// The goal of this diagnostic is to emit an error or warning when an +/// unavailable API is found in a code that is reachable from the shader +/// entry function or from an exported function (when compiling shader +/// library). +/// +/// This is done by traversing the AST of all shader entry point functions +/// and of all exported functions, and any functions that are refrenced +/// from this AST. In other words, any function that are reachable from +/// the entry points. +class DiagnoseHLSLAvailability +: public RecursiveASTVisitor { + // HEKOTAS this is probably not needed + // typedef RecursiveASTVisitor Base; + + Sema + + // Stack of functions to be scaned + llvm::SmallVector DeclsToScan; + + // List of functions that were already scanned and in which environment. + // + // Maps FunctionDecl to a unsigned number that represents a set of shader + // environments the function has been scanned for. + // Since HLSLShaderAttr::ShaderType enum is generated from Attr.td and is + // defined without any assigned values, it is guaranteed to be numbered + // sequentially from 0 up and we can use it to 'index' individual bits + // in the set. + // The N'th bit in the set will be set if the function has been scanned + // in shader environment whose ShaderType integer value equals N. + // For example, if a function has been scanned in compute and pixel stage + // environment, the value will be 0x21 (11 binary) because + // (int)HLSLShaderAttr::ShaderType::Pixel == 1 and + // (int)HLSLShaderAttr::ShaderType::Compute == 5. + llvm::DenseMap ScannedDecls; + + // Do not access these directly, use the get/set methods below to make + // sure the values are in sync + llvm::Triple::EnvironmentType CurrentShaderEnvironment; + unsigned CurrentShaderStageBit; + + // True if scanning a function that was already scanned in a different + // shader stage context, and therefore we should not report issues that + // depend only on shader model version because they would be duplicate. + bool ReportOnlyShaderStageIssues; + + void SetShaderStageContext(HLSLShaderAttr::ShaderType ShaderType) { +assertunsigned)1) << (unsigned)ShaderType) != 0 && + "ShaderType is too big for this bitmap"); +CurrentShaderEnvironment = HLSLShaderAttr::getTypeAsEnvironment(ShaderType); +CurrentShaderStageBit = (1 << ShaderType); + } + void SetUnknownShaderStageContext() { +CurrentShaderEnvironment = +llvm::Triple::EnvironmentType::UnknownEnvironment; +CurrentShaderStageBit = (1 << 31); + } + llvm::Triple::EnvironmentType GetCurrentShaderEnvironment() { +return CurrentShaderEnvironment; + } + bool InUnknownShaderStageContext() { +return CurrentShaderEnvironment == + llvm::Triple::EnvironmentType::UnknownEnvironment; + } + + // Scanning methods + void HandleFunctionOrMethodRef(FunctionDecl *FD, Expr *RefExpr); + void CheckDeclAvailability(NamedDecl *D, const AvailabilityAttr *AA, + SourceRange Range); + const AvailabilityAttr *FindAvailabilityAttr(const Decl *D); + bool HasMatchingEnvironmentOrNone(const AvailabilityAttr *AA); + bool WasAlreadyScannedInCurrentShaderStage(const FunctionDecl *FD, + bool *WasNeverScanned = nullptr); + void AddToScannedFunctions(const FunctionDecl *FD); + +public: + DiagnoseHLSLAvailability(Sema ) : SemaRef(SemaRef) {} + + // AST traversal methods + void RunOnTranslationUnit(const TranslationUnitDecl *TU); + void RunOnFunction(const FunctionDecl *FD); + + bool VisitDeclRefExpr(DeclRefExpr *DRE) { +FunctionDecl *FD = llvm::dyn_cast(DRE->getDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, DRE); +return true; + } + + bool VisitMemberExpr(MemberExpr *ME) { +FunctionDecl *FD = llvm::dyn_cast(ME->getMemberDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, ME); +return true; + } +}; + +// Returns true if the function has already been scanned in the current +// shader environment. WasNeverScanned will be set to true if the function +// has never been scanned before for any shader environment. +bool DiagnoseHLSLAvailability::WasAlreadyScannedInCurrentShaderStage( +const FunctionDecl *FD, bool *WasNeverScanned) { + const unsigned = ScannedDecls.getOrInsertDefault(FD); + if (WasNeverScanned) +*WasNeverScanned = (ScannedStages == 0); + return ScannedStages & CurrentShaderStageBit; +} + +// Marks the function as scanned in the current shader environment +void DiagnoseHLSLAvailability::AddToScannedFunctions(const FunctionDecl *FD) { + unsigned = ScannedDecls.getOrInsertDefault(FD);
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
https://github.com/llvm-beanz commented: Looks pretty good to me. A few nits that you can take or leave. The one real concern I have is that I actually don't like `getPrettyEnviromentName` being a string->string mapping. Is it possible to instead use the Triple environment enum? https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -1068,11 +1068,37 @@ static llvm::StringRef getPrettyEnviromentName(llvm::StringRef Environment) { .Case("hull", "hull shader") .Case("domain", "domain shader") .Case("compute", "compute shader") + .Case("raygeneration", "ray generation shader") + .Case("intersection", "intersection shader") + .Case("anyhit", "anyhit shader") + .Case("closesthit", "closesthit shader") + .Case("miss", "miss shader") + .Case("callable", "callable shader") .Case("mesh", "mesh shader") .Case("amplification", "amplification shader") .Case("library", "shader library") .Default(Environment); } +static llvm::StringRef getEnvironmentString(llvm::Triple::EnvironmentType EnvironmentType) { llvm-beanz wrote: Do we need this function or should we use `Triple::getEnvironmentTypeName`? I think the only difference in output is "ray generation" vs "raygeneration", and I'm pretty sure that based on how this is used "ray generation" is a bug. https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -290,3 +294,295 @@ void SemaHLSL::DiagnoseAttrStageMismatch( << A << HLSLShaderAttr::ConvertShaderTypeToStr(Stage) << (AllowedStages.size() != 1) << join(StageStrings, ", "); } + +namespace { + +/// This class implements HLSL availability diagnostics for default +/// and relaxed mode +/// +/// The goal of this diagnostic is to emit an error or warning when an +/// unavailable API is found in a code that is reachable from the shader +/// entry function or from an exported function (when compiling shader +/// library). +/// +/// This is done by traversing the AST of all shader entry point functions +/// and of all exported functions, and any functions that are refrenced +/// from this AST. In other words, any function that are reachable from +/// the entry points. +class DiagnoseHLSLAvailability +: public RecursiveASTVisitor { + // HEKOTAS this is probably not needed + // typedef RecursiveASTVisitor Base; llvm-beanz wrote: nit: ```suggestion ``` https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
@@ -290,3 +294,295 @@ void SemaHLSL::DiagnoseAttrStageMismatch( << A << HLSLShaderAttr::ConvertShaderTypeToStr(Stage) << (AllowedStages.size() != 1) << join(StageStrings, ", "); } + +namespace { + +/// This class implements HLSL availability diagnostics for default +/// and relaxed mode +/// +/// The goal of this diagnostic is to emit an error or warning when an +/// unavailable API is found in a code that is reachable from the shader +/// entry function or from an exported function (when compiling shader +/// library). +/// +/// This is done by traversing the AST of all shader entry point functions +/// and of all exported functions, and any functions that are refrenced +/// from this AST. In other words, any function that are reachable from +/// the entry points. +class DiagnoseHLSLAvailability +: public RecursiveASTVisitor { + // HEKOTAS this is probably not needed + // typedef RecursiveASTVisitor Base; + + Sema + + // Stack of functions to be scaned + llvm::SmallVector DeclsToScan; + + // List of functions that were already scanned and in which environment. + // + // Maps FunctionDecl to a unsigned number that represents a set of shader + // environments the function has been scanned for. + // Since HLSLShaderAttr::ShaderType enum is generated from Attr.td and is + // defined without any assigned values, it is guaranteed to be numbered + // sequentially from 0 up and we can use it to 'index' individual bits + // in the set. + // The N'th bit in the set will be set if the function has been scanned + // in shader environment whose ShaderType integer value equals N. + // For example, if a function has been scanned in compute and pixel stage + // environment, the value will be 0x21 (11 binary) because + // (int)HLSLShaderAttr::ShaderType::Pixel == 1 and + // (int)HLSLShaderAttr::ShaderType::Compute == 5. + llvm::DenseMap ScannedDecls; + + // Do not access these directly, use the get/set methods below to make + // sure the values are in sync + llvm::Triple::EnvironmentType CurrentShaderEnvironment; + unsigned CurrentShaderStageBit; + + // True if scanning a function that was already scanned in a different + // shader stage context, and therefore we should not report issues that + // depend only on shader model version because they would be duplicate. + bool ReportOnlyShaderStageIssues; + + void SetShaderStageContext(HLSLShaderAttr::ShaderType ShaderType) { +assertunsigned)1) << (unsigned)ShaderType) != 0 && + "ShaderType is too big for this bitmap"); +CurrentShaderEnvironment = HLSLShaderAttr::getTypeAsEnvironment(ShaderType); +CurrentShaderStageBit = (1 << ShaderType); + } + void SetUnknownShaderStageContext() { +CurrentShaderEnvironment = +llvm::Triple::EnvironmentType::UnknownEnvironment; +CurrentShaderStageBit = (1 << 31); + } + llvm::Triple::EnvironmentType GetCurrentShaderEnvironment() { +return CurrentShaderEnvironment; + } + bool InUnknownShaderStageContext() { +return CurrentShaderEnvironment == + llvm::Triple::EnvironmentType::UnknownEnvironment; + } + + // Scanning methods + void HandleFunctionOrMethodRef(FunctionDecl *FD, Expr *RefExpr); + void CheckDeclAvailability(NamedDecl *D, const AvailabilityAttr *AA, + SourceRange Range); + const AvailabilityAttr *FindAvailabilityAttr(const Decl *D); + bool HasMatchingEnvironmentOrNone(const AvailabilityAttr *AA); + bool WasAlreadyScannedInCurrentShaderStage(const FunctionDecl *FD, + bool *WasNeverScanned = nullptr); + void AddToScannedFunctions(const FunctionDecl *FD); + +public: + DiagnoseHLSLAvailability(Sema ) : SemaRef(SemaRef) {} + + // AST traversal methods + void RunOnTranslationUnit(const TranslationUnitDecl *TU); + void RunOnFunction(const FunctionDecl *FD); + + bool VisitDeclRefExpr(DeclRefExpr *DRE) { +FunctionDecl *FD = llvm::dyn_cast(DRE->getDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, DRE); +return true; + } + + bool VisitMemberExpr(MemberExpr *ME) { +FunctionDecl *FD = llvm::dyn_cast(ME->getMemberDecl()); +if (FD) + HandleFunctionOrMethodRef(FD, ME); +return true; + } +}; + +// Returns true if the function has already been scanned in the current +// shader environment. WasNeverScanned will be set to true if the function +// has never been scanned before for any shader environment. +bool DiagnoseHLSLAvailability::WasAlreadyScannedInCurrentShaderStage( +const FunctionDecl *FD, bool *WasNeverScanned) { llvm-beanz wrote: Super nitpick: I have a bit of an aversion to this multi-return pattern because I think they get a bit confusing. It seems like this function gets used two ways: 1) Has this already been scanned for the current stage? 2) What stage(s) has this function been scanned for? Maybe we can make two functions, one that returns the unsigned integer
[clang] [HLSL] Default and Relaxed Availability Diagnostics (PR #92704)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/92704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Enable unguarded availability diagnostic on instantiated template functions (PR #91699)
https://github.com/llvm-beanz approved this pull request. We've waited a week on this PR to see if anyone from Apple will chime in. On previous PRs we waited weeks and got no response. We've reached out on Discord too. This PR looks sane to me and it seems to have adequate testing. I think we should move forward and if Apple comes out of the woodwork and has an issue we can revert or move swiftly to address it. https://github.com/llvm/llvm-project/pull/91699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Cache files don't have generator vars (PR #92793)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/92793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Cache files don't have generator vars (PR #92793)
llvm-beanz wrote: Merging before the PR bots finish since the PR bots don't actually use this file anyways... https://github.com/llvm/llvm-project/pull/92793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Cache files don't have generator vars (PR #92793)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/92793 Doh! CMake cache scripts don't have generator variables set yet, so the script can't depend on the generator variables. Instead I've added a variable that a user can specify to enable the distribution settings. >From c8ae03238f839723280ca8dd0a8418716dd8faa5 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Mon, 20 May 2024 12:36:24 -0500 Subject: [PATCH] [HLSL][CMake] Cache files don't have generator vars Doh! CMake cache scripts don't have generator variables set yet, so the script can't depend on the generator variables. Instead I've added a variable that a user can specify to enable the distribution settings. --- clang/cmake/caches/HLSL.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/cmake/caches/HLSL.cmake b/clang/cmake/caches/HLSL.cmake index 27f848fdccf0c..ed813f60c9c69 100644 --- a/clang/cmake/caches/HLSL.cmake +++ b/clang/cmake/caches/HLSL.cmake @@ -12,7 +12,7 @@ set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra" CACHE STRING "") set(CLANG_ENABLE_HLSL On CACHE BOOL "") -if (NOT CMAKE_CONFIGURATION_TYPES) +if (HLSL_ENABLE_DISTRIBUTION) set(LLVM_DISTRIBUTION_COMPONENTS "clang;hlsl-resource-headers;clangd" CACHE STRING "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
llvm-beanz wrote: I think the different slice there is platform vs language. SemaHLSL should be where HLSL tests go regardless of platform (DirectX or Vulkan), but we should try to keep HLSL separate from C/C++ tests. https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] HLSL availability diagnostics design doc (PR #92207)
@@ -0,0 +1,139 @@ += +HLSL Availability Diagnostics += + +.. contents:: + :local: + +Introduction + + +HLSL availability diagnostics emits errors or warning when unavailable shader APIs are used. Unavailable shader APIs are APIs that are exposed in HLSL code but are not available in the target shader stage or shader model version. + +There are three modes of HLSL availability diagnostic: + +#. **Default mode** - compiler emits an error when an unavailable API is found in a code that is reachable from the shader entry point function or from an exported library function (when compiling a shader library) + +#. **Relaxed mode** - same as default mode except the compiler emits a warning. This mode is enabled by ``-Wno-error=hlsl-availability``. + +#. **Strict mode** - compiler emits an error when an unavailable API is found in parsed code regardless of whether it can be reached from the shader entry point or exported functions, or not. This mode is enabled by ``-fhlsl-strict-diagnostics``. + +Implementation Details +== + +Environment Parameter +- + +In order to encode API availability based on the shader model version and shader model stage a new ``environment`` parameter was added to the existing Clang ``availability`` attribute. + +The values allowed for this parameter are a subset of values allowed as the ``llvm::Triple`` environment component. If the environment parameters is present, the declared availability attribute applies only to targets with the same platform and environment. + +Default and Relaxed Diagnostic Modes + + +This mode is implemented in ``DiagnoseHLSLAvailability`` class in ``SemaHLSL.cpp`` and it is invoked after the whole translation unit is parsed (from ``Sema::ActOnEndOfTranslationUnit``). The implementation iterates over all shader entry points and exported library functions in the translation unit and performs an AST traversal of each function body. + +When a reference to another function or member method is found (``DeclRefExpr`` or ``MemberExpr``) and it has a body, the AST of the referenced function is also scanned. This chain of AST traversals will reach all of the code that is reachable from the initial shader entry point or exported library function. + +Another approach would be to construct a call graph by traversing the AST and recording caller and callee for each ``CallExpr``. The availability diagnostics would then run on the call graph. Since Clang currently does not build any call graph during compilation, this seems like an unnecessary step. Traversing all function references (``DeclRefExpr`` or ``MemberExpr``) works just as well, and can be easily extended to support availability diagnostic of classes and other AST nodes. llvm-beanz wrote: nit: Maybe we can replace this paragraph with a simple statement in the paragraph above that traversing the decls avoids needing to generate a call graph. https://github.com/llvm/llvm-project/pull/92207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] HLSL availability diagnostics design doc (PR #92207)
@@ -0,0 +1,139 @@ += +HLSL Availability Diagnostics += + +.. contents:: + :local: + +Introduction + + +HLSL availability diagnostics emits errors or warning when unavailable shader APIs are used. Unavailable shader APIs are APIs that are exposed in HLSL code but are not available in the target shader stage or shader model version. + +There are three modes of HLSL availability diagnostic: + +#. **Default mode** - compiler emits an error when an unavailable API is found in a code that is reachable from the shader entry point function or from an exported library function (when compiling a shader library) + +#. **Relaxed mode** - same as default mode except the compiler emits a warning. This mode is enabled by ``-Wno-error=hlsl-availability``. + +#. **Strict mode** - compiler emits an error when an unavailable API is found in parsed code regardless of whether it can be reached from the shader entry point or exported functions, or not. This mode is enabled by ``-fhlsl-strict-diagnostics``. llvm-beanz wrote: nit: maybe we should name this flag `-fhlsl-strict-availability`, which more clearly associates it with availability rather than more general diagnostics. https://github.com/llvm/llvm-project/pull/92207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] HLSL availability diagnostics design doc (PR #92207)
https://github.com/llvm-beanz approved this pull request. A few small suggestions feel free to take them or leave them, otherwise looks good. https://github.com/llvm/llvm-project/pull/92207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] HLSL availability diagnostics design doc (PR #92207)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/92207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
https://github.com/llvm-beanz commented: A couple comments, but mostly this looks good to me. https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
llvm-beanz wrote: nit: we've generally tried to keep the HLSL-specific tests separated from the other language tests (i.e. SemaHLSL). https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Add clangd and distribution settings (PR #92011)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/92011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Add clangd and distribution settings (PR #92011)
@@ -8,6 +8,12 @@ set(LLVM_EXPERIMENTAL_TARGETS_TO_BUILD "DirectX;SPIRV" CACHE STRING "") # HLSL support is currently limted to clang, eventually it will expand to # clang-tools-extra too. -set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "") +set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra" CACHE STRING "") set(CLANG_ENABLE_HLSL On CACHE BOOL "") + +if (NOT CMAKE_CONFIGURATION_TYPES) + set(LLVM_DISTRIBUTION_COMPONENTS + "clang;hlsl-resource-headers;clangd" + CACHE STRING "") llvm-beanz wrote: The clang symlinks are all part of the `clang` component, so they all get installed together. We should probably change that and have some which are part of the default target (clang, clang++), and others that aren't (like clang-dxc). https://github.com/llvm/llvm-project/pull/92011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] support packoffset in clang codeGen (PR #91999)
llvm-beanz wrote: > This PR will not change this. The padding will only be added when required. _That's the problem_ > > ```hlsl > cbuffer { > float F; > float2 V; > } > ``` > > will still got > > ```llvm > type { float, <2 x float>} > ``` And there will implicitly be padding between the two elements based on what data layout does because the vector will be 64-bit aligned, which _is not_ how cbuffers work. But that's actually not the biggest problem here. The biggest problem is that we shouldn't add padding for `packoffset`, but not for things that are implicitly laid out, and we should have a plan for how we lower this before we do any of this. Your design document that you based this off of explicitly excluded all the IR representations. We shouldn't write this code until the design doc is updated with the IR and lowering design. https://github.com/llvm/llvm-project/pull/91999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] support packoffset in clang codeGen (PR #91999)
llvm-beanz wrote: > You can use `type <{ float, <2 x float>}>` if you need the tightly-packed > layout. I think we need to figure out how we're going to lower this too. Loading from this memory space requires loading 128 bytes at a time, and we need to slice it down to just the parts of the structure we need. @bogner is working on the lowering logic, so I'd like to make sure what we capture at the IR level is appropriate for what he needs in the DirectX backend. https://github.com/llvm/llvm-project/pull/91999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] support packoffset in clang codeGen (PR #91999)
llvm-beanz wrote: > Could you explain more about can't put any vectors or matrices into the > cbuffer structure type? Consider this example (regardless of packoffset): ```hlsl cbuffer { float F; float2 V; } ``` The layout for this is basically: ```c struct { float F; float Vx; // v.x float Vy; // v.y }; ``` These elements are all packed. If you translate this to an IR struct you get something like: ```llvm type { float, <2 x float>} ``` The data layout will insert 4-bytes of padding between the float and the vector because the vector needs to be properly aligned. https://github.com/llvm/llvm-project/pull/91999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] support packoffset in clang codeGen (PR #91999)
https://github.com/llvm-beanz commented: I'm a little concerned about the strategy here. LLVM generally doesn't explicitly capture padding it relies on the data layout rules to capture that. If this is the approach we're going with you can't put any vectors or matrices into the cbuffer structure type and we need to account for all the padding bytes explicitly. https://github.com/llvm/llvm-project/pull/91999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] support packoffset in clang codeGen (PR #91999)
@@ -5671,6 +5671,54 @@ HLSLBufferDecl *HLSLBufferDecl::CreateDeserialized(ASTContext , SourceLocation(), SourceLocation()); } +static uint64_t calculateLegacyCbufferAlign(const ASTContext , +QualType T) { + if (T->isAggregateType()) +return 128; + else if (const VectorType *VT = T->getAs()) +return Context.getTypeSize(VT->getElementType()); + else +return Context.getTypeSize(T); +} + +unsigned HLSLBufferDecl::calculateLegacyCbufferSize(const ASTContext , llvm-beanz wrote: This function seems like something we should be able to have some unit tests of. Can we write unit tests that exercise this through a bunch of the interesting variations to ensure it does the right thing? https://github.com/llvm/llvm-project/pull/91999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
https://github.com/llvm-beanz commented: Overall this looks good to me. One set of suggestions inline. https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
@@ -772,40 +816,58 @@ void DiagnoseUnguardedAvailability::DiagnoseDeclAvailability( const AvailabilityAttr *AA = getAttrForPlatform(SemaRef.getASTContext(), OffendingDecl); +bool EnvironmentMatchesOrNone = +hasMatchingEnvironmentOrNone(SemaRef.getASTContext(), AA); VersionTuple Introduced = AA->getIntroduced(); -if (AvailabilityStack.back() >= Introduced) +if (EnvironmentMatchesOrNone && AvailabilityStack.back() >= Introduced) return; // If the context of this function is less available than D, we should not // emit a diagnostic. -if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, Ctx, +if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, + AA->getEnvironment(), Ctx, OffendingDecl)) return; // We would like to emit the diagnostic even if -Wunguarded-availability is // not specified for deployment targets >= to iOS 11 or equivalent or // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or // later. -unsigned DiagKind = -shouldDiagnoseAvailabilityByDefault( -SemaRef.Context, -SemaRef.Context.getTargetInfo().getPlatformMinVersion(), Introduced) -? diag::warn_unguarded_availability_new -: diag::warn_unguarded_availability; - -std::string PlatformName(AvailabilityAttr::getPrettyPlatformName( -SemaRef.getASTContext().getTargetInfo().getPlatformName())); - -SemaRef.Diag(Range.getBegin(), DiagKind) -<< Range << D << PlatformName << Introduced.getAsString(); +bool UseNewDiagKind = shouldDiagnoseAvailabilityByDefault( +SemaRef.Context, +SemaRef.Context.getTargetInfo().getPlatformMinVersion(), Introduced); + +const TargetInfo = SemaRef.getASTContext().getTargetInfo(); +std::string PlatformName( +AvailabilityAttr::getPrettyPlatformName(TI.getPlatformName())); +llvm::StringRef TargetEnvironment(AvailabilityAttr::getPrettyEnviromentName( +TI.getTriple().getEnvironmentName())); +llvm::StringRef AttrEnvironment = +AA->getEnvironment() ? AvailabilityAttr::getPrettyEnviromentName( + AA->getEnvironment()->getName()) + : ""; +bool UseEnvironment = +(!AttrEnvironment.empty() && !TargetEnvironment.empty()); + +if (EnvironmentMatchesOrNone) { + unsigned DiagKind = UseNewDiagKind ? diag::warn_unguarded_availability_new + : diag::warn_unguarded_availability; + SemaRef.Diag(Range.getBegin(), DiagKind) + << Range << D << PlatformName << Introduced.getAsString() + << UseEnvironment << TargetEnvironment; +} else { + unsigned DiagKind = + UseNewDiagKind ? diag::warn_unguarded_availability_unavailable_new + : diag::warn_unguarded_availability_unavailable; + SemaRef.Diag(Range.getBegin(), DiagKind) << Range << D; +} llvm-beanz wrote: We can do a similar thing here too. Something like: ```suggestion unsigned Warning = EnvironmentMatchesOrNone ? (UseNewDiagKind ? diag::warn_unguarded_availability_new : diag::warn_unguarded_availability) : (UseNewDiagKind ? diag::warn_unguarded_availability_unavailable_new : diag::warn_unguarded_availability_unavailable); SemaRef.Diag(Range.getBegin(), DiagKind) << Range << D << PlatformName << Introduced.getAsString() << UseEnvironment << TargetEnvironment; ``` https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
@@ -376,26 +400,46 @@ static void DoEmitAvailabilityWarning(Sema , AvailabilityResult K, // not specified for deployment targets >= to iOS 11 or equivalent or // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or // later. -const AvailabilityAttr *AA = -getAttrForPlatform(S.getASTContext(), OffendingDecl); +assert(AA != nullptr && "expecting valid availability attribute"); VersionTuple Introduced = AA->getIntroduced(); +bool EnvironmentMatchesOrNone = +hasMatchingEnvironmentOrNone(S.getASTContext(), AA); + +const TargetInfo = S.getASTContext().getTargetInfo(); +std::string PlatformName( +AvailabilityAttr::getPrettyPlatformName(TI.getPlatformName())); +llvm::StringRef TargetEnvironment(AvailabilityAttr::getPrettyEnviromentName( +TI.getTriple().getEnvironmentName())); +llvm::StringRef AttrEnvironment = +AA->getEnvironment() ? AvailabilityAttr::getPrettyEnviromentName( + AA->getEnvironment()->getName()) + : ""; +bool UseEnvironment = +(!AttrEnvironment.empty() && !TargetEnvironment.empty()); bool UseNewWarning = shouldDiagnoseAvailabilityByDefault( S.Context, S.Context.getTargetInfo().getPlatformMinVersion(), Introduced); -unsigned Warning = UseNewWarning ? diag::warn_unguarded_availability_new - : diag::warn_unguarded_availability; - -std::string PlatformName(AvailabilityAttr::getPrettyPlatformName( -S.getASTContext().getTargetInfo().getPlatformName())); -S.Diag(Loc, Warning) << OffendingDecl << PlatformName - << Introduced.getAsString(); +if (EnvironmentMatchesOrNone) { + unsigned DiagKind = UseNewWarning ? diag::warn_unguarded_availability_new +: diag::warn_unguarded_availability; + + S.Diag(Loc, DiagKind) + << OffendingDecl << PlatformName << Introduced.getAsString() + << UseEnvironment << TargetEnvironment; +} else { + unsigned DiagKind = + UseNewWarning ? diag::warn_unguarded_availability_unavailable_new +: diag::warn_unguarded_availability_unavailable; + S.Diag(Loc, DiagKind) << Loc << OffendingDecl; +} llvm-beanz wrote: What about something like this? ```suggestion unsigned Warning = EnvironmentMatchesOrNone ? (UseNewWarning ? diag::warn_unguarded_availability_new : diag::warn_unguarded_availability) : (UseNewWarning ? diag::warn_unguarded_availability_unavailable_new : diag::warn_unguarded_availability_unavailable); S.Diag(Loc, DiagKind) << OffendingDecl << PlatformName << Introduced.getAsString() << UseEnvironment << TargetEnvironment; ``` The the new `warn_unguarded_availability_unavailable` has strictly a subset of the arguments for the other diagnostic so we should be able to just substitute them like this. https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Add clangd and distribution settings (PR #92011)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/92011 >From e1b82c5bb1869ac74080e17633bd8ac7931a47b6 Mon Sep 17 00:00:00 2001 From: Chris B Date: Mon, 13 May 2024 13:55:49 -0500 Subject: [PATCH 1/2] [HLSL][CMake] Add clangd and distribution settings This just adds some simple distribution settings and includes clangd in the build for distribution. --- clang/cmake/caches/HLSL.cmake | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/cmake/caches/HLSL.cmake b/clang/cmake/caches/HLSL.cmake index 84850c86f12cd..9aa28625ab81e 100644 --- a/clang/cmake/caches/HLSL.cmake +++ b/clang/cmake/caches/HLSL.cmake @@ -8,6 +8,10 @@ set(LLVM_EXPERIMENTAL_TARGETS_TO_BUILD "DirectX;SPIRV" CACHE STRING "") # HLSL support is currently limted to clang, eventually it will expand to # clang-tools-extra too. -set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "") +set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra" CACHE STRING "") set(CLANG_ENABLE_HLSL On CACHE BOOL "") + +set(LLVM_DISTRIBUTION_COMPONENTS +"clang;hlsl-resource-headers;clangd" +CACHE STRING "") >From b0f54990e3a5b9bb56c573d13af748b51e3c6115 Mon Sep 17 00:00:00 2001 From: Chris B Date: Mon, 13 May 2024 14:00:42 -0500 Subject: [PATCH 2/2] Don't setup distribuiton on multi-config generators --- clang/cmake/caches/HLSL.cmake | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang/cmake/caches/HLSL.cmake b/clang/cmake/caches/HLSL.cmake index 9aa28625ab81e..27f848fdccf0c 100644 --- a/clang/cmake/caches/HLSL.cmake +++ b/clang/cmake/caches/HLSL.cmake @@ -12,6 +12,8 @@ set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra" CACHE STRING "") set(CLANG_ENABLE_HLSL On CACHE BOOL "") -set(LLVM_DISTRIBUTION_COMPONENTS -"clang;hlsl-resource-headers;clangd" -CACHE STRING "") +if (NOT CMAKE_CONFIGURATION_TYPES) + set(LLVM_DISTRIBUTION_COMPONENTS + "clang;hlsl-resource-headers;clangd" + CACHE STRING "") +endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL][CMake] Add clangd and distribution settings (PR #92011)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/92011 This just adds some simple distribution settings and includes clangd in the build for distribution. >From e1b82c5bb1869ac74080e17633bd8ac7931a47b6 Mon Sep 17 00:00:00 2001 From: Chris B Date: Mon, 13 May 2024 13:55:49 -0500 Subject: [PATCH] [HLSL][CMake] Add clangd and distribution settings This just adds some simple distribution settings and includes clangd in the build for distribution. --- clang/cmake/caches/HLSL.cmake | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/cmake/caches/HLSL.cmake b/clang/cmake/caches/HLSL.cmake index 84850c86f12cd..9aa28625ab81e 100644 --- a/clang/cmake/caches/HLSL.cmake +++ b/clang/cmake/caches/HLSL.cmake @@ -8,6 +8,10 @@ set(LLVM_EXPERIMENTAL_TARGETS_TO_BUILD "DirectX;SPIRV" CACHE STRING "") # HLSL support is currently limted to clang, eventually it will expand to # clang-tools-extra too. -set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "") +set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra" CACHE STRING "") set(CLANG_ENABLE_HLSL On CACHE BOOL "") + +set(LLVM_DISTRIBUTION_COMPONENTS +"clang;hlsl-resource-headers;clangd" +CACHE STRING "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -4372,6 +4372,13 @@ def HLSLResourceBinding: InheritableAttr { let Documentation = [HLSLResourceBindingDocs]; } +def HLSLPackOffset: HLSLAnnotationAttr { + let Spellings = [HLSLAnnotation<"packoffset">]; + let LangOpts = [HLSL]; + let Args = [IntArgument<"Offset">]; llvm-beanz wrote: The feedback here still isn't resolved. The argument is an IdentifierArgument with special parsing handling, it isn't an integer. We can store the parsed integer values in the attribute but we shouldn't be calling the argument an IntArgument. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version (PR #90809)
@@ -1206,6 +1233,47 @@ std::string Triple::normalize(StringRef Str) { } } + // Normalize DXIL triple if it does not include DXIL version number. + // Determine DXIL version number using the minor version number of Shader + // Model version specified in target triple, if any. Prior to decoupling DXIL + // version numbering from that of Shader Model DXIL version 1.Y corresponds to + // SM 6.Y. E.g., dxilv1.Y-unknown-shadermodelX.Y-hull + if (Components[0] == "dxil") { +if (Components.size() > 4) { + Components.resize(4); +} +// Add DXIL version only if shadermodel is specified in the triple +if (OS == Triple::ShaderModel) { + VersionTuple Ver = + parseVersionFromName(Components[2].drop_front(strlen("shadermodel"))); + // Default DXIL minor version when Shader Model version is anything other + // than 6.[0...8] or 6.x (which translates to latest current SM version) + // DXIL version corresponding to Shader Model version other than 6.x + // is 1.0 + unsigned DXILMinor = 0; + const unsigned SMMajor = 6; + const unsigned LatestCurrentDXILMinor = 8; + if (!Ver.empty()) { +if (Ver.getMajor() == SMMajor) { + if (std::optional SMMinor = Ver.getMinor()) { +DXILMinor = *SMMinor; +// Ensure specified minor version is supported +if (DXILMinor > LatestCurrentDXILMinor) { + report_fatal_error("Unsupported Shader Model version", false); +} + } +} + } else { +// Special case: DXIL minor version is set to LatestCurrentDXILMinor for +// shadermodel6.x is +if (Components[2] == "shadermodel6.x") { + DXILMinor = LatestCurrentDXILMinor; +} + } + Components[0] = + Components[0].str().append("v1.").append(std::to_string(DXILMinor)); llvm-beanz wrote: @bharadwajy this is your asan failure. You’re setting a stringref to a locally modified string temporary. https://github.com/llvm/llvm-project/pull/90809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Implement 202x conforming literals (PR #91015)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/91015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version (PR #90809)
https://github.com/llvm-beanz approved this pull request. https://github.com/llvm/llvm-project/pull/90809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Implement 202x conforming literals (PR #91015)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/91015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Implement 202x conforming literals (PR #91015)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/91015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Implement 202x conforming literals (PR #91015)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/91015 This implements the HLSL 202x conforming literals feature. The feature proposal is available here: https://github.com/microsoft/hlsl-specs/blob/main/proposals/0017-conform ing-literals.md The language specification for this behavior is available in (poorly rendered) HTML or PDF: https://microsoft.github.io/hlsl-specs/specs/hlsl.html#Lex.Literal.Float https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf The main implementation details are: 1) Unsuffixed floating literals are `float`. 2) The integer `ll` suffix specifies `int64_t (aka long)` which is 64-bit because HLSL has no defined `long` keyword or `long long` type. Resolves #85714 >From 4511cd525a671e8c7ceec9ffc653b351d9b1a4d8 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Wed, 1 May 2024 14:10:58 -0500 Subject: [PATCH] [HLSL] Implement 202x conforming literals This implements the HLSL 202x conforming literals feature. The feature proposal is available here: https://github.com/microsoft/hlsl-specs/blob/main/proposals/0017-conform ing-literals.md The language specification for this behavior is available in (poorly rendered) HTML or PDF: https://microsoft.github.io/hlsl-specs/specs/hlsl.html#Lex.Literal.Float https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf The main implementation details are: 1) Unsuffixed floating literals are `float`. 2) The integer `ll` suffix specifies `int64_t (aka long)` which is 64-bit because HLSL has no defined `long` keyword or `long long` type. Resolves #85714 --- clang/lib/Sema/SemaExpr.cpp | 11 ++ clang/test/AST/HLSL/vector-constructors.hlsl | 135 -- .../CodeGenHLSL/builtins/ScalarSwizzles.hlsl | 17 ++- .../Arithmetic}/literal_suffixes.hlsl | 5 +- .../Arithmetic/literal_suffixes_202x.hlsl | 115 +++ .../literal_suffixes_no_16bit.hlsl| 5 +- .../Types/BuiltinVector/ScalarSwizzles.hlsl | 13 +- 7 files changed, 215 insertions(+), 86 deletions(-) rename clang/test/SemaHLSL/{ => Types/Arithmetic}/literal_suffixes.hlsl (87%) create mode 100644 clang/test/SemaHLSL/Types/Arithmetic/literal_suffixes_202x.hlsl rename clang/test/SemaHLSL/{ => Types/Arithmetic}/literal_suffixes_no_16bit.hlsl (86%) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 7190e50b156f7b..5b42cf65cf80ff 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -4103,6 +4103,8 @@ ExprResult Sema::ActOnNumericConstant(const Token , Scope *UDLScope) { Ty = Context.Float16Ty; else if (Literal.isFloat128) Ty = Context.Float128Ty; +else if (getLangOpts().HLSL) + Ty = Context.FloatTy; else Ty = Context.DoubleTy; @@ -4173,6 +4175,15 @@ ExprResult Sema::ActOnNumericConstant(const Token , Scope *UDLScope) { // be an unsigned int. bool AllowUnsigned = Literal.isUnsigned || Literal.getRadix() != 10; + // HLSL doesn't really have `long` or `long long`. We support the `ll` + // suffix for portability of code with C++, but both `l` and `ll` are + // 64-bit integer types, and we want the type of `1l` and `1ll` to be the + // same. + if (getLangOpts().HLSL && !Literal.isLong && Literal.isLongLong) { +Literal.isLong = true; +Literal.isLongLong = false; + } + // Check from smallest to largest, picking the smallest type we can. unsigned Width = 0; diff --git a/clang/test/AST/HLSL/vector-constructors.hlsl b/clang/test/AST/HLSL/vector-constructors.hlsl index 7861d5209b5d3e..5e0900bb623693 100644 --- a/clang/test/AST/HLSL/vector-constructors.hlsl +++ b/clang/test/AST/HLSL/vector-constructors.hlsl @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s typedef float float2 __attribute__((ext_vector_type(2))); typedef float float3 __attribute__((ext_vector_type(3))); @@ -11,41 +11,36 @@ void entry() { // For the float2 vector, we just expect a conversion from constructor // parameters to an initialization list -// CHECK: VarDecl 0x{{[0-9a-fA-F]+}} col:10 used Vec2 'float2':'float __attribute__((ext_vector_type(2)))' cinit -// CHECK-NEXT: CXXFunctionalCastExpr 0x{{[0-9a-fA-F]+}} 'float2':'float __attribute__((ext_vector_type(2)))' functional cast to float2 -// CHECK-NEXT: InitListExpr 0x{{[0-9a-fA-F]+}} 'float2':'float __attribute__((ext_vector_type(2)))' -// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} 'float' -// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}} 'double' 1.00e+00 -// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-fA-F]+}} 'float' -// CHECK-NEXT: FloatingLiteral 0x{{[0-9a-fA-F]+}} 'double' 2.00e+00 +// CHECK-LABEL: VarDecl 0x{{[0-9a-fA-F]+}} {{.*}} used Vec2 'float2':'float __attribute__((ext_vector_type(2)))'
[clang] [llvm] [DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version (PR #90809)
https://github.com/llvm-beanz commented: Given that @bogner had concerns about the other approach I think we should get him to review this before moving forward. That said, other than some missing unit test coverage I think this approach looks fine. https://github.com/llvm/llvm-project/pull/90809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version (PR #90809)
@@ -1200,6 +1224,27 @@ std::string Triple::normalize(StringRef Str) { } } + // Normalize DXIL triple if it does not include DXIL version number. + // Determine DXIL version number using the minor version number of Shader + // Model version specified in target triple, if any. Prior to decoupling DXIL + // version numbering from that of Shader Model DXIL version 1.Y corresponds to + // SM 6.Y. E.g., dxilv1.Y-unknown-shadermodelX.Y-hull + if (Components[0] == "dxil") { +std::string DXILVerStr{"dxilv1."}; +if (Components.size() > 2) { + // OS component specified + if (Components[2].starts_with("shadermodel6.")) { +Components[0] = DXILVerStr.append( +Components[2].drop_front(strlen("shadermodel6."))); + } else if (Components[2].starts_with("shadermodel")) { +// If shader model specified is other than 6.x, set DXIL Version to 1.0 +Components[0] = DXILVerStr.append("0"); + } +} +// DXIL version is not set for a non-specified OS string or one that does +// not starts with shadermodel. + } + llvm-beanz wrote: We should add unit tests to test the normalization. https://github.com/llvm/llvm-project/pull/90809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > I don't know if the pre-commit testing guarantees that. Configuration > settings will permit the files to be checked out in either Unix (`\n`) or > Windows (`\r\n`) line-endings. Today on Windows you basically have to check out LLVM as unix line endings. There are a bunch of tests that fail if you don't. Hopefully this is a step toward fixing that (which is why I'm in favor of this). > If the builders are all configured to run in Unix line endings we lose that > coverage. I suspect the Windows bots are basically all configured that way today. Our instructions tell people they need to disable `autocrlf` (see: https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm), and not disabling it causes problems for users still (see this message from today: https://discord.com/channels/636084430946959380/745925509971705877/1235913415902629958). >While I understand that this change only changes how the LLVM sources are >stored, the issue is that the LLVM sources include the _tests_ directory. >These need to be tested in various manners to ensure that we test how we >handle the different inputs (in clang). One option might be to exclude the >tests directory from the line ending conversion if we cannot verify that we >have tests in both formats. I see two issues with this: 1) because the repo doesn't work with autocrlf today, we actually can't rely on testing on different platforms to provide this coverage. 2) We shouldn't be relying on git settings to get this coverage anyways. We should have tests that require specific line endings, and we should use gitattributes to ensure that we run those tests regardless of the user's git settings. > While, yes, it is possible to get the wrong behaviour currently, if the user > configures git explicitly for a line-ending, I would expect them to be aware > of that. The use of `.gitattributes` means that their defaults are not > necessarily honoured and thus this can catch them by surprise. I think today not only is it possible to get the wrong behavior, if you're developing on Windows Git's default behavior causes the wrong behavior for LLVM. Which often means that contributors (especially new contributors) have no idea that they're doing something wrong. I don't know that this PR is perfect, but IMO it is a huge step in the right direction to allow Windows contributors to build, test and contribute to LLVM using the default configurations of the OS and tools. https://github.com/llvm/llvm-project/pull/86318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Cleanup TargetInfo handling (PR #90694)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
https://github.com/llvm-beanz closed https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
https://github.com/llvm-beanz requested changes to this pull request. I'm marking this as requesting changes because I don't think we should land this as-is. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -108,3 +108,18 @@ behavior between Clang and DXC. Some examples include: diagnostic notifying the user of the conversion rather than silently altering precision relative to the other overloads (as FXC does) or generating code that will fail validation (as DXC does). + +Mix packoffset and non-packoffset += + +DXC allows mixing packoffset and non-packoffset elements in the same cbuffer, +accompanied by a warning. + +However, both Clang and FXC do not permit this and instead report an error. llvm-beanz wrote: I worry about this. I thought we had discussed this and DXC did something sane and reasonable so we could do the same in Clang. Is there some detail I'm missing as to why we can't match DXC. (cc @tex3d) https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -4372,6 +4372,13 @@ def HLSLResourceBinding: InheritableAttr { let Documentation = [HLSLResourceBindingDocs]; } +def HLSLPackOffset: HLSLAnnotationAttr { + let Spellings = [HLSLAnnotation<"packoffset">]; + let LangOpts = [HLSL]; + let Args = [IntArgument<"Offset">]; llvm-beanz wrote: I think it is fine to keep these as integers. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/90222 >From a173605b6043739e69f89d3a559a4f6a68d5fc0a Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Thu, 25 Apr 2024 15:47:22 -0500 Subject: [PATCH 1/2] [HLSL] Shore up floating point conversions This PR fixes bugs in HLSL floating conversions. HLSL always has `half`, `float` and `double` types, which promote in the order: `half`->`float`->`double` and convert in the order: `double`->`float`->`half` As with other conversions in C++, promotions are preferred over conversions. We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (https://github.com/microsoft/hlsl-specs/pull/206). Resolves #81047 --- clang/lib/Sema/SemaOverload.cpp | 43 +++- .../SemaHLSL/ScalarOverloadResolution.hlsl| 229 ++ .../VectorElementOverloadResolution.hlsl | 228 + 3 files changed, 499 insertions(+), 1 deletion(-) create mode 100644 clang/test/SemaHLSL/ScalarOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 04cd9e78739d20..a416df2e97c439 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -2587,7 +2587,8 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) { // In HLSL an rvalue of integral type can be promoted to an rvalue of a larger // integral type. - if (Context.getLangOpts().HLSL) + if (Context.getLangOpts().HLSL && FromType->isIntegerType() && + ToType->isIntegerType()) return Context.getTypeSize(FromType) < Context.getTypeSize(ToType); return false; @@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) { ToBuiltin->getKind() == BuiltinType::Ibm128)) return true; + // In HLSL, `half` promotes to `float` or `double`, regardless of whether + // or not native half types are enabled. + if (getLangOpts().HLSL && FromBuiltin->getKind() == BuiltinType::Half && + (ToBuiltin->getKind() == BuiltinType::Float || + ToBuiltin->getKind() == BuiltinType::Double)) +return true; + // Half can be promoted to float. if (!getLangOpts().NativeHalfType && FromBuiltin->getKind() == BuiltinType::Half && @@ -4393,6 +4401,24 @@ getFixedEnumPromtion(Sema , const StandardConversionSequence ) { return FixedEnumPromotion::ToPromotedUnderlyingType; } +static ImplicitConversionSequence::CompareKind +HLSLCompareFloatingRank(QualType LHS, QualType RHS) { + assert(LHS->isVectorType() == RHS->isVectorType() && + "Either both elements should be vectors or neither should."); + if (const auto *VT = LHS->getAs()) +LHS = VT->getElementType(); + + if (const auto *VT = RHS->getAs()) +RHS = VT->getElementType(); + + const auto L = LHS->getAs()->getKind(); + const auto R = RHS->getAs()->getKind(); + if (L == R) +return ImplicitConversionSequence::Indistinguishable; + return L < R ? ImplicitConversionSequence::Better + : ImplicitConversionSequence::Worse; +} + /// CompareStandardConversionSequences - Compare two standard /// conversion sequences to determine whether one is better than the /// other or if they are indistinguishable (C++ 13.3.3.2p3). @@ -4634,6 +4660,21 @@ CompareStandardConversionSequences(Sema , SourceLocation Loc, : ImplicitConversionSequence::Worse; } + if (S.getLangOpts().HLSL) { +// On a promotion we prefer the lower rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Promotion && + SCS2.Second == ICK_Floating_Promotion) || +(SCS1.Element == ICK_Floating_Promotion && + SCS2.Element == ICK_Floating_Promotion)) + return HLSLCompareFloatingRank(SCS1.getToType(2), SCS2.getToType(2)); +// On a conversion we prefer the higher rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Conversion && + SCS2.Second == ICK_Floating_Conversion) || +(SCS1.Element == ICK_Floating_Conversion && + SCS2.Element == ICK_Floating_Conversion)) + return HLSLCompareFloatingRank(SCS2.getToType(2), SCS1.getToType(2)); + } + return ImplicitConversionSequence::Indistinguishable; } diff --git a/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl new file mode 100644 index 00..13758995ec6a1e --- /dev/null +++ b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl @@ -0,0 +1,229 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header
[clang] [HLSL] Shore up floating point conversions (PR #90222)
@@ -0,0 +1,229 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -ast-dump %s | FileCheck %s + +// This test verifies floating point type implicit conversion ranks for overload +// resolution. In HLSL the built-in type ranks are half < float < double. This +// applies to both scalar and vector types. + +// HLSL allows implicit truncation fo types, so it differentiates between +// promotions (converting to larger types) and conversions (converting to +// smaller types). Promotions are preferred over conversions. Promotions prefer +// promoting to the next lowest type in the ranking order. Conversions prefer +// converting to the next highest type in the ranking order. + +void HalfFloatDouble(double D); +void HalfFloatDouble(float F); +void HalfFloatDouble(half H); + +// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double)' llvm-beanz wrote: Personal preference. Since clang's AST output prints line numbers I find it useful to add the checks after the line I'm comparing so that as I'm modifying and writing tests the line numbers of the thing that relates to the `CHECK` remain stable. We don't really have a standard for this in LLVM or Clang. https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
@@ -4393,6 +4401,24 @@ getFixedEnumPromtion(Sema , const StandardConversionSequence ) { return FixedEnumPromotion::ToPromotedUnderlyingType; } +static ImplicitConversionSequence::CompareKind +HLSLCompareFloatingRank(QualType LHS, QualType RHS) { + assert(LHS->isVectorType() == RHS->isVectorType() && llvm-beanz wrote: I'm not yet handling scalar -> vector conversions. That won't change this code, but it will add extra handling in the `CompareStandardConversionSequences` function because we'll need to prefer scalar->vector conversions when the element type is the same (and maybe a promotion). We don't yet generate the cast sequences for the scalar->vector conversions, so we can't yet handle them. For context what this is doing: during overload resolution a list of candidate functions is generated by iterating over each possible function and generating possible conversion sequences for each overload. Then each conversion sequence is evaluated against the other to determine if it is better or worse to choose which overload we want. This code just extends that to allow floating point promotions and conversions more freely than C does, and uses the same logic for vector element conversions. https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
@@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) { ToBuiltin->getKind() == BuiltinType::Ibm128)) return true; + // In HLSL, `half` promotes to `float` or `double`, regardless of whether + // or not native half types are enabled. llvm-beanz wrote: There is a difference between the IR and language types. `half` is always `half` in the language, which is always a different type form `float`. This behavior is consistent in earlier language modes and with 202x: https://godbolt.org/z/3bzoocdrG When you don't pass `-enable-16bit-types`, `half` is 32-bit and conforms to the IEEE single-precision fp32 representation (see: https://github.com/llvm/llvm-project/pull/90694). https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
@@ -0,0 +1,229 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -ast-dump %s | FileCheck %s + +// This test verifies floating point type implicit conversion ranks for overload +// resolution. In HLSL the built-in type ranks are half < float < double. This +// applies to both scalar and vector types. + +// HLSL allows implicit truncation fo types, so it differentiates between +// promotions (converting to larger types) and conversions (converting to +// smaller types). Promotions are preferred over conversions. Promotions prefer +// promoting to the next lowest type in the ranking order. Conversions prefer +// converting to the next highest type in the ranking order. + +void HalfFloatDouble(double D); +void HalfFloatDouble(float F); +void HalfFloatDouble(half H); + +// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double)' +// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (float)' +// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (half)' + +void FloatDouble(double D); +void FloatDouble(float F); + +// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (double)' +// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (float)' + +void HalfDouble(double D); +void HalfDouble(half H); + +// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (double)' +// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (half)' + +void HalfFloat(float F); +void HalfFloat(half H); + +// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (float)' +// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (half)' + +void Double(double D); +void Float(float F); +void Half(half H); + +// CHECK: FunctionDecl {{.*}} used Double 'void (double)' +// CHECK: FunctionDecl {{.*}} used Float 'void (float)' +// CHECK: FunctionDecl {{.*}} used Half 'void (half)' + + +// Case 1: A function declared with overloads for half float and double types. +// (a) When called with half, it will resolve to half because half is an exact +// match. +// (b) When called with float it will resolve to float because float is an +// exact match. +// (c) When called with double it will resolve to double because it is an +// exact match. + +// CHECK: FunctionDecl {{.*}} Case1 'void (half, float, double)' llvm-beanz wrote: Yea, I wasn't relying on any match variables, which is what I usually use `CHECK-LABEL` for, but it could result in better output on error. I'll make that update. https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -4372,6 +4372,13 @@ def HLSLResourceBinding: InheritableAttr { let Documentation = [HLSLResourceBindingDocs]; } +def HLSLPackOffset: HLSLAnnotationAttr { + let Spellings = [HLSLAnnotation<"packoffset">]; + let LangOpts = [HLSL]; + let Args = [IntArgument<"Offset">]; llvm-beanz wrote: Looking at this more this seems really weird to me. The argument isn't actually an integer, it's a special string. You're parsing it to an integer, but that actually loses some fidelity. We should probably store index and sub-component values in the attribute rather than collapsing it down into one value. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Cleanup TargetInfo handling (PR #90694)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/90694 >From 2464bcb75b047c49076f0718470f27a561252a62 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 30 Apr 2024 20:49:35 -0500 Subject: [PATCH 1/2] [NFC][HLSL] Cleanup TargetInfo handling We had some odd places where we set target behaviors. We were setting the long size in target-specific code, but it should be language-based. We were not setting the Half float type semantics correctly, and instead were overriding the query in the AST context. This change is NFC, but it moves existing code to the right places in the Target so that as we continue working on target and language feature they are controlled in the right places. --- clang/lib/AST/ASTContext.cpp | 10 +- clang/lib/Basic/TargetInfo.cpp| 10 ++ clang/lib/Basic/Targets/DirectX.h | 1 - 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index cbf4932aff9a6b..565870eba7014f 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1612,15 +1612,7 @@ const llvm::fltSemantics ::getFloatTypeSemantics(QualType T) const { case BuiltinType::Float16: return Target->getHalfFormat(); case BuiltinType::Half: -// For HLSL, when the native half type is disabled, half will be treat as -// float. -if (getLangOpts().HLSL) - if (getLangOpts().NativeHalfType) -return Target->getHalfFormat(); - else -return Target->getFloatFormat(); -else - return Target->getHalfFormat(); +return Target->getHalfFormat(); case BuiltinType::Float: return Target->getFloatFormat(); case BuiltinType::Double: return Target->getDoubleFormat(); case BuiltinType::Ibm128: diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index f96956f31d50dc..29f5cd14e46e11 100644 --- a/clang/lib/Basic/TargetInfo.cpp +++ b/clang/lib/Basic/TargetInfo.cpp @@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine , LangOptions ) { LongDoubleAlign = 64; } + // HLSL explicitly defines the sizes and formats of some data types, and we + // need to conform to those regardless of what architecture you are targeting. + if (Opts.HLSL) { +LongWidth = LongAlign = 64; +if (!Opts.NativeHalfType) { + HalfFormat = ::APFloat::IEEEsingle(); + HalfWidth = HalfAlign = 32; +} + } + if (Opts.OpenCL) { // OpenCL C requires specific widths for types, irrespective of // what these normally are for the target. diff --git a/clang/lib/Basic/Targets/DirectX.h b/clang/lib/Basic/Targets/DirectX.h index acfcc8c47ba950..a084e2823453fc 100644 --- a/clang/lib/Basic/Targets/DirectX.h +++ b/clang/lib/Basic/Targets/DirectX.h @@ -53,7 +53,6 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public TargetInfo { : TargetInfo(Triple) { TLSSupported = false; VLASupported = false; -LongWidth = LongAlign = 64; AddrSpaceMap = UseAddrSpaceMapMangling = true; HasLegalHalfType = true; >From 330151395257f44a8936080f9e994ab61a0c38ae Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Wed, 1 May 2024 12:02:24 -0500 Subject: [PATCH 2/2] Add test for sizeof(half) --- clang/test/SemaHLSL/Types/Arithmetic/half_size.hlsl | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 clang/test/SemaHLSL/Types/Arithmetic/half_size.hlsl diff --git a/clang/test/SemaHLSL/Types/Arithmetic/half_size.hlsl b/clang/test/SemaHLSL/Types/Arithmetic/half_size.hlsl new file mode 100644 index 00..7de46746999304 --- /dev/null +++ b/clang/test/SemaHLSL/Types/Arithmetic/half_size.hlsl @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -verify %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -verify -fnative-half-type %s +// RUN: %clang_cc1 -triple spirv-linux-vulkan-library -verify %s +// RUN: %clang_cc1 -triple spirv-linux-vulkan-library -verify -fnative-half-type %s + +// expected-no-diagnostics +#ifdef __HLSL_ENABLE_16_BIT +_Static_assert(sizeof(half) == 2, "half is 2 bytes"); +#else +_Static_assert(sizeof(half) == 4, "half is 4 bytes"); +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Cleanup TargetInfo handling (PR #90694)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Cleanup TargetInfo handling (PR #90694)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)
@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine , LangOptions ) { LongDoubleAlign = 64; } + // HLSL explicitly defines the sizes and formats of some data types, and we + // need to conform to those regardless of what architecture you are targeting. + if (Opts.HLSL) { +LongWidth = LongAlign = 64; +if (!Opts.NativeHalfType) { + HalfFormat = ::APFloat::IEEEsingle(); + HalfWidth = HalfAlign = 32; llvm-beanz wrote: Yes, and I totally glossed over that this fixes a bug. Without this line `sizeof(half)` returns the wrong value. https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)
@@ -1612,15 +1612,7 @@ const llvm::fltSemantics ::getFloatTypeSemantics(QualType T) const { case BuiltinType::Float16: return Target->getHalfFormat(); case BuiltinType::Half: -// For HLSL, when the native half type is disabled, half will be treat as -// float. -if (getLangOpts().HLSL) - if (getLangOpts().NativeHalfType) -return Target->getHalfFormat(); - else -return Target->getFloatFormat(); -else - return Target->getHalfFormat(); +return Target->getHalfFormat(); case BuiltinType::Float: return Target->getFloatFormat(); llvm-beanz wrote: Yea, this code would completely change with clang-format, but the bot only cares about the lines I changed. I tried not to include larger whitespace changes. https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Pipelines] Do not run CoroSplit and CoroCleanup in LTO pre-link pipeline (PR #90310)
@@ -0,0 +1,77 @@ +// This tests that the coroutine elide optimization could happen succesfully with ThinLTO. +// This test is adapted from coro-elide.cpp and splits functions into two files. +// +// RUN: split-file %s %t +// RUN: %clang --target=x86_64-linux -std=c++20 -O2 -flto=thin -I %S -c %t/coro-elide-callee.cpp -o coro-elide-callee.o +// RUN: %clang --target=x86_64-linux -std=c++20 -O2 -flto=thin -I %S -c %t/coro-elide-caller.cpp -o coro-elide-caller.o llvm-beanz wrote: Yep! Thank you for the quick fix. https://github.com/llvm/llvm-project/pull/90310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)
@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine , LangOptions ) { LongDoubleAlign = 64; } + // HLSL explicitly defines the sizes and formats of some data types, and we + // need to conform to those regardless of what architecture you are targeting. + if (Opts.HLSL) { +LongWidth = LongAlign = 64; +if (!Opts.NativeHalfType) { + HalfFormat = ::APFloat::IEEEsingle(); + HalfWidth = HalfAlign = 32; llvm-beanz wrote: Just as a note, we don't actually use this value anywhere in the HLSL code generation paths (yet), which is why it wasn't causing problems that we overrode the format returned by the AST context, but didn't override these values. https://github.com/llvm/llvm-project/pull/90694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/90694 We had some odd places where we set target behaviors. We were setting the long size in target-specific code, but it should be language-based. We were not setting the Half float type semantics correctly, and instead were overriding the query in the AST context. This change is NFC, but it moves existing code to the right places in the Target so that as we continue working on target and language feature they are controlled in the right places. >From 2464bcb75b047c49076f0718470f27a561252a62 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 30 Apr 2024 20:49:35 -0500 Subject: [PATCH] [NFC][HLSL] Cleanup TargetInfo handling We had some odd places where we set target behaviors. We were setting the long size in target-specific code, but it should be language-based. We were not setting the Half float type semantics correctly, and instead were overriding the query in the AST context. This change is NFC, but it moves existing code to the right places in the Target so that as we continue working on target and language feature they are controlled in the right places. --- clang/lib/AST/ASTContext.cpp | 10 +- clang/lib/Basic/TargetInfo.cpp| 10 ++ clang/lib/Basic/Targets/DirectX.h | 1 - 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index cbf4932aff9a6b..565870eba7014f 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1612,15 +1612,7 @@ const llvm::fltSemantics ::getFloatTypeSemantics(QualType T) const { case BuiltinType::Float16: return Target->getHalfFormat(); case BuiltinType::Half: -// For HLSL, when the native half type is disabled, half will be treat as -// float. -if (getLangOpts().HLSL) - if (getLangOpts().NativeHalfType) -return Target->getHalfFormat(); - else -return Target->getFloatFormat(); -else - return Target->getHalfFormat(); +return Target->getHalfFormat(); case BuiltinType::Float: return Target->getFloatFormat(); case BuiltinType::Double: return Target->getDoubleFormat(); case BuiltinType::Ibm128: diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp index f96956f31d50dc..29f5cd14e46e11 100644 --- a/clang/lib/Basic/TargetInfo.cpp +++ b/clang/lib/Basic/TargetInfo.cpp @@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine , LangOptions ) { LongDoubleAlign = 64; } + // HLSL explicitly defines the sizes and formats of some data types, and we + // need to conform to those regardless of what architecture you are targeting. + if (Opts.HLSL) { +LongWidth = LongAlign = 64; +if (!Opts.NativeHalfType) { + HalfFormat = ::APFloat::IEEEsingle(); + HalfWidth = HalfAlign = 32; +} + } + if (Opts.OpenCL) { // OpenCL C requires specific widths for types, irrespective of // what these normally are for the target. diff --git a/clang/lib/Basic/Targets/DirectX.h b/clang/lib/Basic/Targets/DirectX.h index acfcc8c47ba950..a084e2823453fc 100644 --- a/clang/lib/Basic/Targets/DirectX.h +++ b/clang/lib/Basic/Targets/DirectX.h @@ -53,7 +53,6 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public TargetInfo { : TargetInfo(Triple) { TLSSupported = false; VLASupported = false; -LongWidth = LongAlign = 64; AddrSpaceMap = UseAddrSpaceMapMangling = true; HasLegalHalfType = true; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Pipelines] Do not run CoroSplit and CoroCleanup in LTO pre-link pipeline (PR #90310)
@@ -0,0 +1,77 @@ +// This tests that the coroutine elide optimization could happen succesfully with ThinLTO. +// This test is adapted from coro-elide.cpp and splits functions into two files. +// +// RUN: split-file %s %t +// RUN: %clang --target=x86_64-linux -std=c++20 -O2 -flto=thin -I %S -c %t/coro-elide-callee.cpp -o coro-elide-callee.o +// RUN: %clang --target=x86_64-linux -std=c++20 -O2 -flto=thin -I %S -c %t/coro-elide-caller.cpp -o coro-elide-caller.o llvm-beanz wrote: This test case requires an X86 backend. Those of us who have embraced the AArch64-only future are encountering test errors when running this test. (maybe it's just me) https://github.com/llvm/llvm-project/pull/90310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
https://github.com/llvm-beanz updated https://github.com/llvm/llvm-project/pull/90222 >From a173605b6043739e69f89d3a559a4f6a68d5fc0a Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Thu, 25 Apr 2024 15:47:22 -0500 Subject: [PATCH] [HLSL] Shore up floating point conversions This PR fixes bugs in HLSL floating conversions. HLSL always has `half`, `float` and `double` types, which promote in the order: `half`->`float`->`double` and convert in the order: `double`->`float`->`half` As with other conversions in C++, promotions are preferred over conversions. We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (https://github.com/microsoft/hlsl-specs/pull/206). Resolves #81047 --- clang/lib/Sema/SemaOverload.cpp | 43 +++- .../SemaHLSL/ScalarOverloadResolution.hlsl| 229 ++ .../VectorElementOverloadResolution.hlsl | 228 + 3 files changed, 499 insertions(+), 1 deletion(-) create mode 100644 clang/test/SemaHLSL/ScalarOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 04cd9e78739d20..a416df2e97c439 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -2587,7 +2587,8 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) { // In HLSL an rvalue of integral type can be promoted to an rvalue of a larger // integral type. - if (Context.getLangOpts().HLSL) + if (Context.getLangOpts().HLSL && FromType->isIntegerType() && + ToType->isIntegerType()) return Context.getTypeSize(FromType) < Context.getTypeSize(ToType); return false; @@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) { ToBuiltin->getKind() == BuiltinType::Ibm128)) return true; + // In HLSL, `half` promotes to `float` or `double`, regardless of whether + // or not native half types are enabled. + if (getLangOpts().HLSL && FromBuiltin->getKind() == BuiltinType::Half && + (ToBuiltin->getKind() == BuiltinType::Float || + ToBuiltin->getKind() == BuiltinType::Double)) +return true; + // Half can be promoted to float. if (!getLangOpts().NativeHalfType && FromBuiltin->getKind() == BuiltinType::Half && @@ -4393,6 +4401,24 @@ getFixedEnumPromtion(Sema , const StandardConversionSequence ) { return FixedEnumPromotion::ToPromotedUnderlyingType; } +static ImplicitConversionSequence::CompareKind +HLSLCompareFloatingRank(QualType LHS, QualType RHS) { + assert(LHS->isVectorType() == RHS->isVectorType() && + "Either both elements should be vectors or neither should."); + if (const auto *VT = LHS->getAs()) +LHS = VT->getElementType(); + + if (const auto *VT = RHS->getAs()) +RHS = VT->getElementType(); + + const auto L = LHS->getAs()->getKind(); + const auto R = RHS->getAs()->getKind(); + if (L == R) +return ImplicitConversionSequence::Indistinguishable; + return L < R ? ImplicitConversionSequence::Better + : ImplicitConversionSequence::Worse; +} + /// CompareStandardConversionSequences - Compare two standard /// conversion sequences to determine whether one is better than the /// other or if they are indistinguishable (C++ 13.3.3.2p3). @@ -4634,6 +4660,21 @@ CompareStandardConversionSequences(Sema , SourceLocation Loc, : ImplicitConversionSequence::Worse; } + if (S.getLangOpts().HLSL) { +// On a promotion we prefer the lower rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Promotion && + SCS2.Second == ICK_Floating_Promotion) || +(SCS1.Element == ICK_Floating_Promotion && + SCS2.Element == ICK_Floating_Promotion)) + return HLSLCompareFloatingRank(SCS1.getToType(2), SCS2.getToType(2)); +// On a conversion we prefer the higher rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Conversion && + SCS2.Second == ICK_Floating_Conversion) || +(SCS1.Element == ICK_Floating_Conversion && + SCS2.Element == ICK_Floating_Conversion)) + return HLSLCompareFloatingRank(SCS2.getToType(2), SCS1.getToType(2)); + } + return ImplicitConversionSequence::Indistinguishable; } diff --git a/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl new file mode 100644 index 00..13758995ec6a1e --- /dev/null +++ b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl @@ -0,0 +1,229 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -7314,6 +7314,41 @@ static void handleHLSLSV_DispatchThreadIDAttr(Sema , Decl *D, D->addAttr(::new (S.Context) HLSLSV_DispatchThreadIDAttr(S.Context, AL)); } +static void handleHLSLPackOffsetAttr(Sema , Decl *D, const ParsedAttr ) { + if (!isa(D) || !isa(D->getDeclContext())) { +S.Diag(AL.getLoc(), diag::err_hlsl_attr_invalid_ast_node) +<< AL << "cbuffer constant"; llvm-beanz wrote: Maybe: ```suggestion << AL << "cbuffer member variable"; ``` "cbuffer constant" is less clear to me because the variable declaration isn't actually marked constant in the source, it is just implicitly constant because it is in a cbuffer. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.3-library -S -finclude-default-header -ast-dump -x hlsl %s | FileCheck %s + + +// CHECK: HLSLBufferDecl {{.*}} cbuffer A +cbuffer A +{ +// CHECK-NEXT: VarDecl {{.*}} C1 'float4' llvm-beanz wrote: We could use some tests for `double` and `half` scalars, arrays and vectors. https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -183,6 +183,86 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes , return; } } break; + case ParsedAttr::AT_HLSLPackOffset: { +// Parse 'packoffset( c[Subcomponent][.component] )'. +// Check '('. +if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after)) { + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; +} +// Check c[Subcomponent] as an identifier. +if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; +} +StringRef OffsetStr = Tok.getIdentifierInfo()->getName(); +SourceLocation OffsetLoc = Tok.getLocation(); +if (OffsetStr[0] != 'c') { + Diag(Tok.getLocation(), diag::err_hlsl_packoffset_invalid_reg) + << OffsetStr; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) + return; +} +OffsetStr = OffsetStr.substr(1); +unsigned SubComponent = 0; +if (!OffsetStr.empty()) { + // Make sure SubComponent is a number. + if (OffsetStr.getAsInteger(10, SubComponent)) { +Diag(OffsetLoc.getLocWithOffset(1), + diag::err_hlsl_unsupported_register_number); +SkipUntil(tok::r_paren, StopAtSemi); // skip through ) +return; + } +} +unsigned Component = 0; +ConsumeToken(); // consume identifier. +if (Tok.is(tok::period)) { + ConsumeToken(); // consume period. + if (!Tok.is(tok::identifier)) { +Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; +SkipUntil(tok::r_paren, StopAtSemi); // skip through ) +return; + } + StringRef ComponentStr = Tok.getIdentifierInfo()->getName(); + SourceLocation SpaceLoc = Tok.getLocation(); + ConsumeToken(); // consume identifier. + // Make sure Component is a single character. + if (ComponentStr.size() != 1) { +Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; +SkipUntil(tok::r_paren, StopAtSemi); // skip through ) +return; + } + switch (ComponentStr[0]) { + case 'x': +Component = 0; +break; + case 'y': +Component = 1; +break; + case 'z': +Component = 2; +break; + case 'w': +Component = 3; +break; + default: +Diag(SpaceLoc, diag::err_hlsl_unsupported_component) << ComponentStr; +SkipUntil(tok::r_paren, StopAtSemi); // skip through ) +return; + } +} +unsigned Offset = SubComponent * 4 + Component; +ASTContext = Actions.getASTContext(); +ArgExprs.push_back(IntegerLiteral::Create( +Ctx, llvm::APInt(Ctx.getTypeSize(Ctx.getSizeType()), Offset), +Ctx.getSizeType(), +SourceLocation())); // Dummy location for integer literal. llvm-beanz wrote: Why not give this the location of the parsed identifier? https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.3-library -verify %s + +// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +cbuffer Mix +{ +float4 M1 : packoffset(c0); +float M2; +float M3 : packoffset(c1.y); +} + +// expected-error@+1{{cannot mix packoffset elements with nonpackoffset elements in a cbuffer}} +cbuffer Mix2 +{ +float4 M4; +float M5 : packoffset(c1.y); +float M6 ; +} + +// expected-error@+1{{attribute 'packoffset' only applies to cbuffer constant}} +float4 g : packoffset(c0); + +cbuffer IllegalOffset +{ +// expected-error@+1{{invalid resource class specifier 't2' for packoffset, expected 'c'}} +float4 i1 : packoffset(t2); +// expected-error@+1{{invalid component 'm' used; expected 'x', 'y', 'z', or 'w'}} +float i2 : packoffset(c1.m); +} + +cbuffer Overlap +{ +float4 o1 : packoffset(c0); +// expected-error@+1{{packoffset overlap between 'o2', 'o1'}} +float2 o2 : packoffset(c0.z); +} + +cbuffer CrossReg +{ +// expected-error@+1{{packoffset cannot cross register boundary}} +float4 c1 : packoffset(c0.y); +// expected-error@+1{{packoffset cannot cross register boundary}} +float2 c2 : packoffset(c1.w); +} + +struct ST { + float s; +}; + +cbuffer Aggregate +{ +// expected-error@+1{{packoffset cannot cross register boundary}} +ST A1 : packoffset(c0.y); +// expected-error@+1{{packoffset cannot cross register boundary}} +float A2[2] : packoffset(c1.w); +} llvm-beanz wrote: Some additional error cases to consider: ``` // Some parsing fails packoffset(); packoffset((c0)); packoffset(c0, x); packoffset(c.x); packoffset; packoffset(; packoffset); packoffset c0.x; // Some other odd failures that look more real packoffset(c0.xy); packoffset(c0.rg); packoffset(c0.yes); packoffset(c0.woo); packoffset(c0.xr); ``` https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.3-library -S -finclude-default-header -ast-dump -x hlsl %s | FileCheck %s + + +// CHECK: HLSLBufferDecl {{.*}} cbuffer A +cbuffer A +{ +// CHECK-NEXT: VarDecl {{.*}} C1 'float4' +// CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 0 +float4 C1 : packoffset(c); +// CHECK-NEXT: VarDecl {{.*}} col:11 C2 'float' +// CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 4 +float C2 : packoffset(c1); +// CHECK-NEXT: VarDecl {{.*}} col:11 C3 'float' +// CHECK-NEXT: HLSLPackOffsetAttr {{.*}} 5 +float C3 : packoffset(c1.y); llvm-beanz wrote: How about some valid cases for vector and array cbuffer elements? https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Support packoffset attribute in AST (PR #89836)
@@ -7398,6 +7398,26 @@ The full documentation is available here: https://docs.microsoft.com/en-us/windo }]; } +def HLSLPackOffsetDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +The packoffset attribute is used to change the layout of a cbuffer. +Attribute spelling in HLSL is: ``packoffset( c[Subcomponent][.component] )``. +A subcomponent is a register number, which is an integer. A component is in the form of [.xyzw]. llvm-beanz wrote: What about `.rgba`? https://godbolt.org/z/z814668Pd https://github.com/llvm/llvm-project/pull/89836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][HLSL] Add environment parameter to availability attribute (PR #89809)
llvm-beanz wrote: > While I like the approach of aligning availability parameters closer to > `llvm::Triple`, I am concerned about how this will interact with existing > precedent. There is a lot of code that passes in _platform_ values that > aren't actually `OSType`. For example > `__attribute__((availability(macCatalyst, ...))` aligns to > `EnviornmentType::MacABI` and > `__attribute__((availability(macOSApplicationExtension, ...)))` aligns to cli > args `-mtargetos=macos -fapplication-extension`. `macCatalyst` would work perfect with the new structure because the platform is `iOS` and the environment is `macabi` (or you could change the string to `catalyst`. App extensions are kinda a weird case because on the one hand, the compiler largely doesn't need to know about them (except for availability), but they drive linking differently. > It may become difficult to maintain the distinction between what is accepted > as a `platform` vs a `environment`. And we can't realistically re-write > client code relying on this. I wouldn't expect rewriting existing code. The new argument added in this PR is optional, so you can use it or not. It seemed like it had the possibility to simplify the use cases. For example, if `app_extension` were an environment you wouldn't need to add multiple new platform cases for each new platform. For us this is a big deal because we effectively have 16 environments that are basically all supported across 2 platforms (shader model and Vulkan). > This attribute behavior also aligns very closely with Swift's `@available` > Would it be possible to similarly treat the target shader stages & > combinations as platform values as well? Or define a new attribute that does > not conflict. It would be preferable for us to separate environment and platform, but that doesn't mean Apple needs to. I think you should consider it because it is a more forward looking and maintainable approach, but I don't think there is any reason why we can't add this argument field while Apple continues to only support the legacy platform strings. https://github.com/llvm/llvm-project/pull/89809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
@@ -0,0 +1,229 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -ast-dump %s | FileCheck %s + +// This test verifies floating point type implicit conversion ranks for overload +// resolution. In HLSL the built-in type ranks are half < float < double. This +// applies to both scalar and vector types. + +// HLSL allows implicit truncation fo types, so it differentiates between +// promotions (converting to larger types) and conversions (converting to +// smaller types). Promotions are preferred over conversions. Promotions prefer +// promoting to the next lowest type in the ranking order. Conversions prefer +// converting to the next highest type in the ranking order. + +void HalfFloatDouble(double D); +void HalfFloatDouble(float F); +void HalfFloatDouble(half H); + +// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double)' llvm-beanz wrote: Yea, the ones at the top are just sanity checking and could be removed. The checks for the `Case*` function decls below force FileCheck to scan up to the declaration, which can improve error reporting if anything goes wrong in the test because it can give you a "last match" marker or show if your match skipped ahead. https://github.com/llvm/llvm-project/pull/90222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Shore up floating point conversions (PR #90222)
https://github.com/llvm-beanz created https://github.com/llvm/llvm-project/pull/90222 This PR fixes bugs in HLSL floating conversions. HLSL always has `half`, `float` and `double` types, which promote in the order: `half`->`float`->`double` and convert in the order: `double`->`float`->`half` As with other conversions in C++, promotions are preferred over conversions. We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (https://github.com/microsoft/hlsl-specs/pull/206). Resolves #81047 >From a173605b6043739e69f89d3a559a4f6a68d5fc0a Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Thu, 25 Apr 2024 15:47:22 -0500 Subject: [PATCH] [HLSL] Shore up floating point conversions This PR fixes bugs in HLSL floating conversions. HLSL always has `half`, `float` and `double` types, which promote in the order: `half`->`float`->`double` and convert in the order: `double`->`float`->`half` As with other conversions in C++, promotions are preferred over conversions. We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (https://github.com/microsoft/hlsl-specs/pull/206). Resolves #81047 --- clang/lib/Sema/SemaOverload.cpp | 43 +++- .../SemaHLSL/ScalarOverloadResolution.hlsl| 229 ++ .../VectorElementOverloadResolution.hlsl | 228 + 3 files changed, 499 insertions(+), 1 deletion(-) create mode 100644 clang/test/SemaHLSL/ScalarOverloadResolution.hlsl create mode 100644 clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 04cd9e78739d20..a416df2e97c439 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -2587,7 +2587,8 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) { // In HLSL an rvalue of integral type can be promoted to an rvalue of a larger // integral type. - if (Context.getLangOpts().HLSL) + if (Context.getLangOpts().HLSL && FromType->isIntegerType() && + ToType->isIntegerType()) return Context.getTypeSize(FromType) < Context.getTypeSize(ToType); return false; @@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) { ToBuiltin->getKind() == BuiltinType::Ibm128)) return true; + // In HLSL, `half` promotes to `float` or `double`, regardless of whether + // or not native half types are enabled. + if (getLangOpts().HLSL && FromBuiltin->getKind() == BuiltinType::Half && + (ToBuiltin->getKind() == BuiltinType::Float || + ToBuiltin->getKind() == BuiltinType::Double)) +return true; + // Half can be promoted to float. if (!getLangOpts().NativeHalfType && FromBuiltin->getKind() == BuiltinType::Half && @@ -4393,6 +4401,24 @@ getFixedEnumPromtion(Sema , const StandardConversionSequence ) { return FixedEnumPromotion::ToPromotedUnderlyingType; } +static ImplicitConversionSequence::CompareKind +HLSLCompareFloatingRank(QualType LHS, QualType RHS) { + assert(LHS->isVectorType() == RHS->isVectorType() && + "Either both elements should be vectors or neither should."); + if (const auto *VT = LHS->getAs()) +LHS = VT->getElementType(); + + if (const auto *VT = RHS->getAs()) +RHS = VT->getElementType(); + + const auto L = LHS->getAs()->getKind(); + const auto R = RHS->getAs()->getKind(); + if (L == R) +return ImplicitConversionSequence::Indistinguishable; + return L < R ? ImplicitConversionSequence::Better + : ImplicitConversionSequence::Worse; +} + /// CompareStandardConversionSequences - Compare two standard /// conversion sequences to determine whether one is better than the /// other or if they are indistinguishable (C++ 13.3.3.2p3). @@ -4634,6 +4660,21 @@ CompareStandardConversionSequences(Sema , SourceLocation Loc, : ImplicitConversionSequence::Worse; } + if (S.getLangOpts().HLSL) { +// On a promotion we prefer the lower rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Promotion && + SCS2.Second == ICK_Floating_Promotion) || +(SCS1.Element == ICK_Floating_Promotion && + SCS2.Element == ICK_Floating_Promotion)) + return HLSLCompareFloatingRank(SCS1.getToType(2), SCS2.getToType(2)); +// On a conversion we prefer the higher rank to disambiguate. +if ((SCS1.Second == ICK_Floating_Conversion && + SCS2.Second == ICK_Floating_Conversion) || +(SCS1.Element == ICK_Floating_Conversion && + SCS2.Element == ICK_Floating_Conversion)) + return HLSLCompareFloatingRank(SCS2.getToType(2), SCS1.getToType(2)); + } +