[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG400d3261a0da: [HLSL] Cleanup support for `this` as an 
l-value (authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3876,7 +3876,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -754,15 +754,16 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
-/*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
+Context, /*This=*/nullptr, ThisType,
+/*IsArrow=*/!Context.getLangOpts().HLSL,
+/*Op=*/SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
 
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction 

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 555380.
beanz added a comment.

Updating based on PR feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -753,15 +753,16 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
-/*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
+Context, /*This=*/nullptr, ThisType,
+/*IsArrow=*/!Context.getLangOpts().HLSL,
+/*Op=*/SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
 
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction = Constructor;
 User.FoundConversionFunction = Best->FoundDecl;
 User.After.setAsIdentityConversion();
-

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 555118.
beanz added a comment.

Updating based on review feedback from @core3ntin. Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -753,14 +753,15 @@
 
   if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
   isa(DC) && cast(DC)->isInstance()) {
-QualType ThisType = cast(DC)->getThisType();
+QualType ThisType = cast(DC)->getThisType().getNonReferenceType();
 
 // Since the 'this' expression is synthesized, we don't need to
 // perform the double-lookup check.
 NamedDecl *FirstQualifierInScope = nullptr;
 
 return CXXDependentScopeMemberExpr::Create(
-Context, /*This*/ nullptr, ThisType, /*IsArrow*/ true,
+Context, /*This*/ nullptr, ThisType,
+/*IsArrow*/ !Context.getLangOpts().HLSL,
 /*Op*/ SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
 FirstQualifierInScope, NameInfo, TemplateArgs);
   }
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -689,7 +689,7 @@
 if (CMD->isStatic())
   Type.MemberType = FuncType::ft_static_member;
 else {
-  Type.This = CMD->getThisType()->getPointeeType();
+  Type.This = CMD->getThisObjectType();
   Type.MemberType = FuncType::ft_non_static_member;
 }
 Type.Func = CMD->getType()->castAs();
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3532,14 +3532,14 @@
   case OR_Success: {
 // Record the standard conversion we used and the conversion function.
 CXXConstructorDecl *Constructor = cast(Best->Function);
-QualType ThisType = Constructor->getThisType();
+QualType ThisType = Constructor->getThisObjectType();
 // Initializer lists don't have conversions as such.
 User.Before.setAsIdentityConversion();
 User.HadMultipleCandidates = HadMultipleCandidates;
 User.ConversionFunction = Constructor;
 User.FoundConversionFunction = Best->FoundDecl;
 User.After.setAsIdentityConversion();
-

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Yea, we could do that approach. It will mean filtering about a few 
`getNonReferenceType()` calls, which is what I was trying to avoid. That said, 
it might be the better solution since it can be unconditional and will work for 
both deducing `this` and HLSL.

I'll test it out and post an update shortly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Full disclosure here, I need to write some more tests for this. It should be 
NFC for all languages other than HLSL, but I want to spend some time making 
sure that the tests adequately exercise the HLSL paths (which should also be 
mostly NFC).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159247/new/

https://reviews.llvm.org/D159247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: cor3ntin, aaron.ballman, bogner.
Herald added subscribers: Anastasia, ChuanqiXu, martong.
Herald added a reviewer: shafik.
Herald added a reviewer: NoQ.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

The goal of this change is to clean up some of the code surrounding
HLSL using CXXThisExpr as a non-pointer l-value. This change cleans up
a bunch of assumptions and inconsistencies around how the type of
`this` is handled through the AST and code generation.

This change is be mostly NFC for HLSL, and completely NFC for other
language modes.

This change introduces a new member to query for the this object's type
and seeks to clarify the normal usages of the this type.

With the introudction of HLSL to clang, CXXThisExpr may now be an
l-value and behave like a reference type rather than C++'s normal
method of it being an r-value of pointer type.

With this change there are now three ways in which a caller might need
to query the type of `this`:

- The type of the `CXXThisExpr`
- The type of the object `this` referrs to
- The type of the implicit (or explicit) `this` argument

This change codifies those three ways you may need to query
respectively as:

- CXXMethodDecl::getThisType()
- CXXMethodDecl::getThisObjectType()
- CXXMethodDecl::getThisArgType()

This change then revisits all uses of `getThisType()`, and in cases
where the only use was to resolve the pointee type, it replaces the
call with `getThisObjectType()`. In other cases it evaluates whether
the desired returned type is the type of the `this` expr, or the type
of the `this` function argument. The `this` expr type is used for
creating additional expr AST nodes and for member lookup, while the
argument type is used mostly for code generation.

Additionally some cases that used `getThisType` in simple queries could
be substituted for `getThisObjectType`. Since `getThisType` is
implemented in terms of `getThisObjectType` calling the later should be
more efficient if the former isn't needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159247

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Analysis/Consumed.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/test/CodeGenHLSL/this-reference.hlsl

Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- clang/test/CodeGenHLSL/this-reference.hlsl
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s -debug-info-kind=standalone -dwarf-version=4 | FileCheck %s
 
 struct Pair {
   int First;
@@ -26,3 +26,9 @@
   // CHECK-NEXT:  store i32 %call, ptr %First, align 4
   // CHECK-NEXT:  %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
   // CHECK-NEXT:  %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
+
+// CHECK: [[Pair:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair"
+// CHECK: [[getFirst:![0-9]+]] = distinct !DISubprogram(name: "getFirst"
+// CHECK-SAME: scope: [[Pair]]
+// CHECK: [[FirstThis:![0-9]+]] = !DILocalVariable(name: "this", arg: 1, scope: [[getFirst]], type: [[thisType:![0-9]+]]
+// CHECK: [[thisType]] = !DIDerivedType(tag: DW_TAG_reference_type, baseType: [[Pair]], size: 32)
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3880,7 +3880,7 @@
   break;
 
 case EXPR_CXX_THIS:
-  S = new (Context) CXXThisExpr(Empty);
+  S = CXXThisExpr::CreateEmpty(Context);
   break;
 
 case EXPR_CXX_THROW:
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ 

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Yea, the gist of it is that in HLSL `this` is a reference not a pointer, which 
means the `CXXThisExpr` is always an LValue.

I think the right fix for this is to cleanup the `CXXThisExpr` creation code 
and create a `CXXThisExpr::Create` method like the other AST nodes. Then I can 
have a simpler way to force the `CXXThisExpr` to always be an LValue for HLSL.

@aaron.ballman, does that sound reasonable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159126/new/

https://reviews.llvm.org/D159126

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

@cor3ntin, I know what the problem is and I think I can put up a review for a 
fix tonight or (more likely) tomorrow. Is that okay?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159126/new/

https://reviews.llvm.org/D159126

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I'll apply this patch and debug the issue today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159126/new/

https://reviews.llvm.org/D159126

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:30
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous 
declaration}}
+[shader("pixel")]

bob80905 wrote:
> bogner wrote:
> > bob80905 wrote:
> > > I don't think we should expect this error, given that there is no 
> > > previous declaration for doubledUp(). Maybe something else like %0 and %1 
> > > are incompatible attributes? 
> > I don't necessarily disagree, but if we're going to change that I think it 
> > should be in a different change than this one.
> I'm also curious, why is the change from compute to pixel necessary here?
I suspect this change is just to prevent the presence of the `compute` 
annotation from triggering the `missing numthreads` error.



Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:61
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int entry() {

Oh interesting! DXC doesn't actually support this syntax, but I think it is 
worth supporting in Clang.
Should we also check for the numthreads attribute?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.

LGTM. This is definitely an improvement over the awfulness we were doing. 
Thanks @bogner!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156178: [HLSL] add pow library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/CodeGenHLSL/builtins/pow.hlsl:6
+// RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
+// RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
+

Does this need to set `-D__HLSL_ENABLE_16_BIT`? If 16-bit types are disabled, 
this should result in `half` being 32-bit right?



Comment at: clang/test/CodeGenHLSL/builtins/pow.hlsl:12
+// NO_HALF: call float @llvm.pow.f32(
+half test_pow_half(half p0, half p1)
+{

nit: can you clang-format this source?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156178/new/

https://reviews.llvm.org/D156178

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

philnik wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > Should this also get an extension warning/should attributes be disabled 
> > > > for HLSL?
> > > CC @beanz 
> > > 
> > > I was wondering the same thing. :-)
> > By bug rather than design DXC allows C++ attribute syntax in some places 
> > for HLSL.
> > 
> > I'm totally fine with (and prefer) following the rest of the languages here 
> > and having HLSL in Clang always allow C++ attributes regardless of language 
> > version.
> Would you like some sort of warning?
Since DXC accepts the syntax without a warning/error I think we're fine without 
one here. There won't be any portability issues with code that use C++ 
attributes in HLSL unless they go back to the _extremely_ old compiler that we 
don't really support anyways.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151683/new/

https://reviews.llvm.org/D151683

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

aaron.ballman wrote:
> philnik wrote:
> > Should this also get an extension warning/should attributes be disabled for 
> > HLSL?
> CC @beanz 
> 
> I was wondering the same thing. :-)
By bug rather than design DXC allows C++ attribute syntax in some places for 
HLSL.

I'm totally fine with (and prefer) following the rest of the languages here and 
having HLSL in Clang always allow C++ attributes regardless of language version.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151683/new/

https://reviews.llvm.org/D151683

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143553: [Clang][CMake] Use perf-training for Clang-BOLT

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM.

Sorry for the delays reviewing!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143553/new/

https://reviews.llvm.org/D143553

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149119: [CMake] Use LLVM own tools in extract_symbols.py

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

One potential area of concern here: If `llvm-driver` is ever extended to work 
as a plugin loader (thus exporting its symbols), removing support for the 
pre-installed host tools could cause a cyclic dependency.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149119/new/

https://reviews.llvm.org/D149119

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:642
  T __builtin_elementwise_log10(T x)  return the base 10 logarithm of x 
   floating point types
+ T __builtin_elementwise_exp(T x)returns the base-e exponential, 
or e^x, of the specified value   floating point types
+ T __builtin_elementwise_exp2(T x)   returns the base-2 exponential, 
or 2^x, of the specified value   floating point types

erichkeane wrote:
> The naming difference between these is a little clunky, but I dont use these 
> enough to know if this is a common pattern.  Its weird to me that _exp means 
> "e^x", but _exp2 means "2^x".  
Agreed that it seems odd, but in C and C++ math libraries `exp` is base-e, and 
`exp2` is base-2:

https://cplusplus.com/reference/cmath/exp/
https://cplusplus.com/reference/cmath/exp2/

It is probably best to be consistent with C here even if it is unintuitive at 
first glance.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145270/new/

https://reviews.llvm.org/D145270

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143208: Repair sphinx doc generation

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143208/new/

https://reviews.llvm.org/D143208

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140489: Add builtin_elementwise_log

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM. This looks pretty straightforward and similar to other changes you've 
been making.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140489/new/

https://reviews.llvm.org/D140489

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Driver/Types.def:110
 TYPE("api-information",  API_INFO, INVALID, "json",   
phases::Precompile)
+TYPE("dx-container", DX_CONTAINER, INVALID, "dxc",
phases::Compile, phases::Backend)
 TYPE("none", Nothing,  INVALID, nullptr,  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)

The normal dx-container extension is `dxbc` right? not `dxc`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141705/new/

https://reviews.llvm.org/D141705

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:687
+def warn_drv_dxc_missing_dxv : Warning<"dxv not found."
+" Resulting DXIL will not be signed for use in release environments.">;
 

Not all `dxv` binaries can sign… some just validate. Only the “official” 
releases support signing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141705/new/

https://reviews.llvm.org/D141705

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

@python3kgae, this change introduced a bunch of warning spew because it is 
using an API that was deprecated shortly before the change merged. Can you 
please address this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136031/new/

https://reviews.llvm.org/D136031

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131052/new/

https://reviews.llvm.org/D131052

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141705: [HLSL] [Dirver] add dxv as a VerifyDebug Job

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4215
+  // Call validator for dxc.
+  if (IsDXCMode()) {
+Action *LastAction = Actions.back();

Shouldn't the validator only run if we are targeting DXIL? Also we should 
probably add the `-Vd` flag to opt out.



Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:169
+: ToolChain(D, Triple, Args) {
+  if (Args.hasArg(options::OPT_dxc_validator_path_EQ))
+getProgramPaths().push_back(

If this option isn't specified we should probably search for `dxv` relative to 
`clang` and on the `PATH`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141705/new/

https://reviews.llvm.org/D141705

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141705: [HLSL] [Dirver] add dxv as a VerifyDebug Job

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Re-using the `VerifyDebug` action really doesn't make sense. That's not what 
the DXIL validator does, and it will be a source of confusion forever.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141705/new/

https://reviews.llvm.org/D141705

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

The convention that `find_program` uses is to cache the variables, which causes 
them to be defined at global scope. That also avoids needing to recompute 
filesystem lookups in incremental builds, which is desirable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131052/new/

https://reviews.llvm.org/D131052

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:2401
+
+macro(setup_host_tool tool_name setting_name exe_var_name target_var_name)
+  cmake_parse_arguments(ARG "" "SCOPE" "" ${ARGN})

Please make this a `function` instead of a `macro`. In general CMake `macros` 
expand in ways that are unintuitive which can be a maintenance burden for 
people coming in and modifying the code.

Also a bit nity, but maybe a name like `find_host_program` would be more 
consistent naming. I suspect you could even simplify this implementation by 
using CMake's `find_program` 
(https://cmake.org/cmake/help/latest/command/find_program.html)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131052/new/

https://reviews.llvm.org/D131052

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136031/new/

https://reviews.llvm.org/D136031

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

It is probably worth adding some unit tests to test the `CBufferDataLayout` 
class.

I think the meat of this change is fine. This code mixes `unsigned` and 
`uint32_t` interchangeably. They aren't required by the language to be the same.

I have a general preference toward `uint32_t` which is explicitly sized, but we 
should match existing interfaces where appropriate.

For example, we should also probably be using `llvm::TypeSize` where 
appropriate to match `llvm::DataLayout`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136031/new/

https://reviews.llvm.org/D136031

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139137: add floor library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM. @bob80905 if you haven't already, you should follow the steps here 
(https://www.llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access) to get 
commit access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139137/new/

https://reviews.llvm.org/D139137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135721: [HLSL] Added HLSL this as a reference

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/AST/HLSL/RWBuffer-AST.hlsl:49-50
 // CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 
'element_type' lvalue
-// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' 
lvalue ->h 0x{{[0-9A-Fa-f]+}}
-// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const 
RWBuffer *' implicit this
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' 
lvalue .h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const 
RWBuffer' lvalue implicit this
 // CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' 
ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'

aaron.ballman wrote:
> `// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type 
> *' lvalue .h 0x{{[0-9A-Fa-f]+}}`
> 
> I'm confused by this -- it says the type of the expression is `element_type 
> *` but that it uses `.` as an operator instead of `->`. One of these seems 
> like it is wrong, but perhaps I'm missing something.
Isn't that the type of the member not the type of the `this`? This example 
seems to result in a pointer `MemberExpr` with a `.` access: 
https://godbolt.org/z/j9f5nP4s6

This is a little awkward because we have a pointer member inside this struct 
even though pointers are illegal in HLSL source :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135721/new/

https://reviews.llvm.org/D135721

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135721: [HLSL] Added HLSL this as a reference

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);

aaron.ballman wrote:
> gracejennings wrote:
> > aaron.ballman wrote:
> > > python3kgae wrote:
> > > > gracejennings wrote:
> > > > > python3kgae wrote:
> > > > > > gracejennings wrote:
> > > > > > > python3kgae wrote:
> > > > > > > > gracejennings wrote:
> > > > > > > > > python3kgae wrote:
> > > > > > > > > > Should this be a reference type?
> > > > > > > > > Could you expand on the question? I'm not sure I understand 
> > > > > > > > > what you're asking. The two changes in this file were made to 
> > > > > > > > > update the previous RWBuffer implementation
> > > > > > > > The current code will create CXXThisExpr with the pointeeType.
> > > > > > > > I thought it should be a reference type of the pointeeType.
> > > > > > > > 
> > > > > > > > Like in the test,
> > > > > > > > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 
> > > > > > > > 'RWBuffer *' implicit this
> > > > > > > > 
> > > > > > > > The type is RWBuffer * before,
> > > > > > > > I expected this patch will change it to
> > > > > > > > RWBuffer &.
> > > > > > > The change that makes it more reference like than c++ from:
> > > > > > > 
> > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}}  'int' lvalue 
> > > > > > > ->First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}}  'Pair *' this`
> > > > > > > 
> > > > > > > to hlsl with this change
> > > > > > > 
> > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}}  'int' lvalue 
> > > > > > > .First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}}  'Pair' lvalue this`
> > > > > > I guess we have to change clang codeGen for this anyway.
> > > > > > 
> > > > > > Not sure which has less impact for codeGen side,  lvalue like what 
> > > > > > is in the current patch or make it a lvalue reference? My feeling 
> > > > > > is lvalue reference might be eaiser.
> > > > > > 
> > > > > > Did you test what needs to change for clang codeGen on top of the 
> > > > > > current patch?
> > > > > > 
> > > > > With just the lvalue change in the current patch there should be no 
> > > > > additional changes needed in clang CodeGen on top of the current patch
> > > > Since we already get the codeGen working with this patch,
> > > > it would be nice to have a codeGen test.
> > > I think it's an interesting question to consider, but I have some 
> > > concerns. Consider code like this:
> > > ```
> > > struct S {
> > >   int Val = 0;
> > >   void foo() {
> > > Val = 10;
> > > S Another;
> > > this = Another; // If this is a non-const reference, we can assign 
> > > into it...
> > > print(); // ... so do we print 0 or 10?
> > > // When this function ends, is `this` destroyed because `Another` 
> > > goes out of scope?
> > >   }
> > >   void print() {
> > > std::cout << Val;
> > >   }
> > > };
> > > ```
> > > I think we need to prevent code like that from working. But it's not just 
> > > direct assignments that are a concern. Consider:
> > > ```
> > > struct S;
> > > 
> > > void func(S , S ) {
> > >   Out = In;
> > > }
> > > 
> > > struct S {
> > >   int Val = 0;
> > >   void foo() {
> > > Val = 10;
> > > S Another;
> > > func(this, Another); // Same problem here!
> > > print();
> > >   }
> > >   void print() {
> > > std::cout << Val;
> > >   }
> > > };
> > > ```
> > > Another situation that I'm not certain of is whether HLSL supports 
> > > tail-allocation where the code is doing something like `this + 1` to get 
> > > to the start of the trailing objects.
> > For the first example we would expect this to work for HLSL because `this` 
> > is reference like (with modifications to make it supported by HLSL). We 
> > would expect `Val` to be `0`, printing `0`, and `Another` to be destroyed, 
> > but not `this`. I went ahead and added similar CodeGen test coverage.
> > 
> > For the second example, there is not reference support in HLSL. Changing to 
> > a similar example with copy-in and copy-out to make it HLSL supported would 
> > take care of that case, but copy-in/out is not supported upstream yet. 
> > For the first example we would expect this to work for HLSL because this is 
> > reference like (with modifications to make it supported by HLSL). We would 
> > expect Val to be 0, printing 0, and Another to be destroyed, but not this. 
> > I went ahead and added similar CodeGen test coverage.
> 
> I was surprised about the dangers of that design, so I spoke with @beanz over 
> IRC about it and he was saying that the goal for HLSL was for that code to 
> call the copy assignment operator and not do a reference replacement, so it'd 
> behave more like `*this` in C++, as in: https://godbolt.org/z/qrEav6sjq. That 
> design makes a lot more sense to me, but I'm also not at all an expert on 
> HLSL, 

[PATCH] D136271: [HLSL] Remove unused frontend-generated ID

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c7218e77026: [HLSL] Remove unused frontend-generated ID 
(authored by beanz).

Changed prior to commit:
  https://reviews.llvm.org/D136271?vs=468961=469681#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136271/new/

https://reviews.llvm.org/D136271

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/test/CodeGenHLSL/builtins/RWBuffer-annotations.hlsl
  clang/test/CodeGenHLSL/cbuf.hlsl
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/test/CodeGen/DirectX/UAVMetadata.ll

Index: llvm/test/CodeGen/DirectX/UAVMetadata.ll
===
--- llvm/test/CodeGen/DirectX/UAVMetadata.ll
+++ llvm/test/CodeGen/DirectX/UAVMetadata.ll
@@ -37,16 +37,16 @@
 
 !hlsl.uavs = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9}
 
-!0 = !{ptr @Zero, !"RWBuffer", i32 0, i32 10, i32 0, i32 0}
-!1 = !{ptr @One, !"Buffer>", i32 1, i32 10, i32 1, i32 0}
-!2 = !{ptr @Two, !"Buffer", i32 2, i32 10, i32 2, i32 0}
-!3 = !{ptr @Three, !"Buffer", i32 3, i32 10, i32 3, i32 0}
-!4 = !{ptr @Four, !"ByteAddressBuffer", i32 4, i32 11, i32 5, i32 0}
-!5 = !{ptr @Five, !"StructuredBuffer", i32 5, i32 12, i32 6, i32 0}
-!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 6, i32 10, i32 7, i32 0}
-!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 7, i32 12, i32 8, i32 0}
-!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 8, i32 11, i32 9, i32 0}
-!9 = !{ptr @Nine, !"RWBuffer", i32 9, i32 10, i32 10, i32 2}
+!0 = !{ptr @Zero, !"RWBuffer", i32 10, i32 0, i32 0}
+!1 = !{ptr @One, !"Buffer>", i32 10, i32 1, i32 0}
+!2 = !{ptr @Two, !"Buffer", i32 10, i32 2, i32 0}
+!3 = !{ptr @Three, !"Buffer", i32 10, i32 3, i32 0}
+!4 = !{ptr @Four, !"ByteAddressBuffer", i32 11, i32 5, i32 0}
+!5 = !{ptr @Five, !"StructuredBuffer", i32 12, i32 6, i32 0}
+!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 10, i32 7, i32 0}
+!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 12, i32 8, i32 0}
+!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 11, i32 9, i32 0}
+!9 = !{ptr @Nine, !"RWBuffer", i32 10, i32 10, i32 2}
 
 ; CHECK: !dx.resources = !{[[ResList:[!][0-9]+]]}
 
Index: llvm/lib/Frontend/HLSL/HLSLResource.cpp
===
--- llvm/lib/Frontend/HLSL/HLSLResource.cpp
+++ llvm/lib/Frontend/HLSL/HLSLResource.cpp
@@ -27,34 +27,29 @@
   return cast(Entry->getOperand(1))->getString();
 }
 
-Constant *FrontendResource::getID() {
-  return cast(Entry->getOperand(2))->getValue();
-}
-
 uint32_t FrontendResource::FrontendResource::getResourceKind() {
   return cast(
- cast(Entry->getOperand(3))->getValue())
+ cast(Entry->getOperand(2))->getValue())
   ->getLimitedValue();
 }
 uint32_t FrontendResource::getResourceIndex() {
   return cast(
- cast(Entry->getOperand(4))->getValue())
+ cast(Entry->getOperand(3))->getValue())
   ->getLimitedValue();
 }
 uint32_t FrontendResource::getSpace() {
   return cast(
- cast(Entry->getOperand(5))->getValue())
+ cast(Entry->getOperand(4))->getValue())
   ->getLimitedValue();
 }
 
 FrontendResource::FrontendResource(GlobalVariable *GV, StringRef TypeStr,
-   uint32_t Counter, ResourceKind RK,
-   uint32_t ResIndex, uint32_t Space) {
+   ResourceKind RK, uint32_t ResIndex,
+   uint32_t Space) {
   auto  = GV->getContext();
   IRBuilder<> B(Ctx);
   Entry = MDNode::get(
   Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TypeStr),
-ConstantAsMetadata::get(B.getInt32(Counter)),
 ConstantAsMetadata::get(B.getInt32(static_cast(RK))),
 ConstantAsMetadata::get(B.getInt32(ResIndex)),
 ConstantAsMetadata::get(B.getInt32(Space))});
Index: llvm/include/llvm/Frontend/HLSL/HLSLResource.h
===
--- llvm/include/llvm/Frontend/HLSL/HLSLResource.h
+++ llvm/include/llvm/Frontend/HLSL/HLSLResource.h
@@ -60,15 +60,14 @@
 
 public:
   FrontendResource(MDNode *E) : Entry(E) {
-assert(Entry->getNumOperands() == 6 && "Unexpected metadata shape");
+assert(Entry->getNumOperands() == 5 && "Unexpected metadata shape");
   }
 
-  FrontendResource(GlobalVariable *GV, StringRef TypeStr, uint32_t Counter,
-   ResourceKind RK, uint32_t ResIndex, uint32_t Space);
+  FrontendResource(GlobalVariable *GV, StringRef TypeStr, ResourceKind RK,
+   uint32_t ResIndex, uint32_t Space);
 
   GlobalVariable *getGlobalVariable();
   StringRef getSourceType();
-  Constant *getID();
   uint32_t getResourceKind();
   uint32_t getResourceIndex();
   uint32_t 

[PATCH] D136134: [NFC] [DirectX backend] move ResourceClass into llvm.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:20
 #include "clang/Sema/Sema.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 

python3kgae wrote:
> python3kgae wrote:
> > beanz wrote:
> > > You need to add FrontendHLSL to the Sema/CMakeLists.txt file too.
> > Good catch.
> > Not sure why both local build and the pre-commit check cannot hit it. :(
> I think the reason it works is that it only used the enum decl in the header, 
> not anything which needs to link.
> Do we need to add FrontendHLSL to CMakeLists in this case?
The first header include added to a component should add the linkage dependency 
(even if it isn't strictly needed).

It is too easy to add a linkage dependency later without realizing it isn't 
already specified. Because of how static archive linking works you don't 
necessarily notice the missing dependency because many of our tools link both 
Sema and CodeGen where there is already a dependency.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136134/new/

https://reviews.llvm.org/D136134

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136134: [NFC] [DirectX backend] move ResourceClass into llvm.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:20
 #include "clang/Sema/Sema.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 

You need to add FrontendHLSL to the Sema/CMakeLists.txt file too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136134/new/

https://reviews.llvm.org/D136134

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136271: [HLSL] Remove unused frontend-generated ID

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added a reviewer: python3kgae.
Herald added subscribers: Anastasia, hiraditya.
Herald added a project: All.
beanz requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.

As @python3kgae pointed out we're going to want to assign these IDs
after optimization so that we can remove unused resrouces. This patch
just removes the unused ID value from the frontend metadata, clang code
generation, and updates associated test cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136271

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/test/CodeGenHLSL/builtins/RWBuffer-annotations.hlsl
  clang/test/CodeGenHLSL/cbuf.hlsl
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/test/CodeGen/DirectX/UAVMetadata.ll

Index: llvm/test/CodeGen/DirectX/UAVMetadata.ll
===
--- llvm/test/CodeGen/DirectX/UAVMetadata.ll
+++ llvm/test/CodeGen/DirectX/UAVMetadata.ll
@@ -37,16 +37,16 @@
 
 !hlsl.uavs = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9}
 
-!0 = !{ptr @Zero, !"RWBuffer", i32 0, i32 10, i32 0, i32 0}
-!1 = !{ptr @One, !"Buffer>", i32 1, i32 10, i32 1, i32 0}
-!2 = !{ptr @Two, !"Buffer", i32 2, i32 10, i32 2, i32 0}
-!3 = !{ptr @Three, !"Buffer", i32 3, i32 10, i32 3, i32 0}
-!4 = !{ptr @Four, !"ByteAddressBuffer", i32 4, i32 11, i32 5, i32 0}
-!5 = !{ptr @Five, !"StructuredBuffer", i32 5, i32 12, i32 6, i32 0}
-!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 6, i32 10, i32 7, i32 0}
-!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 7, i32 12, i32 8, i32 0}
-!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 8, i32 11, i32 9, i32 0}
-!9 = !{ptr @Nine, !"RWBuffer", i32 9, i32 10, i32 10, i32 2}
+!0 = !{ptr @Zero, !"RWBuffer", i32 10, i32 0, i32 0}
+!1 = !{ptr @One, !"Buffer>", i32 10, i32 1, i32 0}
+!2 = !{ptr @Two, !"Buffer", i32 10, i32 2, i32 0}
+!3 = !{ptr @Three, !"Buffer", i32 10, i32 3, i32 0}
+!4 = !{ptr @Four, !"ByteAddressBuffer", i32 11, i32 5, i32 0}
+!5 = !{ptr @Five, !"StructuredBuffer", i32 12, i32 6, i32 0}
+!6 = !{ptr @Six, !"RasterizerOrderedBuffer", i32 10, i32 7, i32 0}
+!7 = !{ptr @Seven, !"RasterizerOrderedStructuredBuffer", i32 12, i32 8, i32 0}
+!8 = !{ptr @Eight, !"RasterizerOrderedByteAddressBuffer", i32 11, i32 9, i32 0}
+!9 = !{ptr @Nine, !"RWBuffer", i32 10, i32 10, i32 2}
 
 ; CHECK: !dx.resources = !{[[ResList:[!][0-9]+]]}
 
Index: llvm/lib/Frontend/HLSL/HLSLResource.cpp
===
--- llvm/lib/Frontend/HLSL/HLSLResource.cpp
+++ llvm/lib/Frontend/HLSL/HLSLResource.cpp
@@ -27,34 +27,29 @@
   return cast(Entry->getOperand(1))->getString();
 }
 
-Constant *FrontendResource::getID() {
-  return cast(Entry->getOperand(2))->getValue();
-}
-
 uint32_t FrontendResource::FrontendResource::getResourceKind() {
   return cast(
- cast(Entry->getOperand(3))->getValue())
+ cast(Entry->getOperand(2))->getValue())
   ->getLimitedValue();
 }
 uint32_t FrontendResource::getResourceIndex() {
   return cast(
- cast(Entry->getOperand(4))->getValue())
+ cast(Entry->getOperand(3))->getValue())
   ->getLimitedValue();
 }
 uint32_t FrontendResource::getSpace() {
   return cast(
- cast(Entry->getOperand(5))->getValue())
+ cast(Entry->getOperand(4))->getValue())
   ->getLimitedValue();
 }
 
 FrontendResource::FrontendResource(GlobalVariable *GV, StringRef TypeStr,
-   uint32_t Counter, ResourceKind RK,
-   uint32_t ResIndex, uint32_t Space) {
+   ResourceKind RK, uint32_t ResIndex,
+   uint32_t Space) {
   auto  = GV->getContext();
   IRBuilder<> B(Ctx);
   Entry = MDNode::get(
   Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TypeStr),
-ConstantAsMetadata::get(B.getInt32(Counter)),
 ConstantAsMetadata::get(B.getInt32(static_cast(RK))),
 ConstantAsMetadata::get(B.getInt32(ResIndex)),
 ConstantAsMetadata::get(B.getInt32(Space))});
Index: llvm/include/llvm/Frontend/HLSL/HLSLResource.h
===
--- llvm/include/llvm/Frontend/HLSL/HLSLResource.h
+++ llvm/include/llvm/Frontend/HLSL/HLSLResource.h
@@ -51,15 +51,14 @@
 
 public:
   FrontendResource(MDNode *E) : Entry(E) {
-assert(Entry->getNumOperands() == 6 && "Unexpected metadata shape");
+assert(Entry->getNumOperands() == 5 && "Unexpected metadata shape");
   }
 
-  FrontendResource(GlobalVariable *GV, StringRef TypeStr, uint32_t Counter,
-   ResourceKind RK, uint32_t ResIndex, uint32_t Space);
+  FrontendResource(GlobalVariable *GV, StringRef TypeStr, ResourceKind RK,
+   uint32_t 

[PATCH] D135973: Move HLSL builtins into hlsl namespace

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd146a5241c50: Move HLSL builtins into hlsl namespace 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135973/new/

https://reviews.llvm.org/D135973

Files:
  clang/lib/Headers/hlsl/hlsl_basic_types.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/basic_types.hlsl
  clang/test/CodeGenHLSL/builtins/abs.hlsl
  clang/test/CodeGenHLSL/builtins/ceil.hlsl
  clang/test/CodeGenHLSL/builtins/sqrt.hlsl
  clang/test/SemaHLSL/Wave.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -5,5 +5,5 @@
 // expected-warning@#site {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
 // expected-note@hlsl/hlsl_intrinsics.h:* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
 // expected-note@#site {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
-return WaveActiveCountBits(b); // #site
+return hlsl::WaveActiveCountBits(b); // #site
 }
Index: clang/test/SemaHLSL/Wave.hlsl
===
--- clang/test/SemaHLSL/Wave.hlsl
+++ clang/test/SemaHLSL/Wave.hlsl
@@ -4,5 +4,5 @@
 
 // expected-no-diagnostics
 unsigned foo(bool b) {
-return WaveActiveCountBits(b);
+return hlsl::WaveActiveCountBits(b);
 }
Index: clang/test/CodeGenHLSL/builtins/sqrt.hlsl
===
--- clang/test/CodeGenHLSL/builtins/sqrt.hlsl
+++ clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::sqrt;
+
 double sqrt_d(double x)
 {
   return sqrt(x);
Index: clang/test/CodeGenHLSL/builtins/ceil.hlsl
===
--- clang/test/CodeGenHLSL/builtins/ceil.hlsl
+++ clang/test/CodeGenHLSL/builtins/ceil.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::ceil;
+
 // CHECK: define noundef half @
 // CHECK: call half @llvm.ceil.f16(
 // NO_HALF: define noundef float @"?test_ceil_half@@YA$halff@$halff@@Z"(
Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- clang/test/CodeGenHLSL/builtins/abs.hlsl
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -5,6 +5,7 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::abs;
 
 // CHECK: define noundef signext i16 @
 // FIXME: int16_t is promoted to i32 now. Change to abs.i16 once it is fixed.
Index: clang/test/CodeGenHLSL/basic_types.hlsl
===
--- clang/test/CodeGenHLSL/basic_types.hlsl
+++ clang/test/CodeGenHLSL/basic_types.hlsl
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
 // RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
 // RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - -DNAMESPACED| FileCheck %s
 
 
 // CHECK:"?uint16_t_Val@@3GA" = global i16 0, align 2
@@ -36,7 +39,11 @@
 // CHECK:"?double3_Val@@3T?$__vector@N$02@__clang@@A" = global <3 x double> zeroinitializer, align 32
 // CHECK:"?double4_Val@@3T?$__vector@N$03@__clang@@A" = global <4 x double> zeroinitializer, align 32
 
+#ifdef NAMESPACED
+#define TYPE_DECL(T)  hlsl::T T##_Val
+#else
 #define TYPE_DECL(T)  T T##_Val
+#endif
 
 #ifdef __HLSL_ENABLE_16_BIT
 TYPE_DECL(uint16_t);
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,8 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+namespace hlsl {
+
 __attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
@@ -99,4 +101,6 @@
 __attribute__((clang_builtin_alias(__builtin_elementwise_ceil)))
 double4 ceil(double4);
 
+} // namespace hlsl
+
 #endif //_HLSL_HLSL_INTRINSICS_H_
Index: 

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:193
-
-void CGHLSLRuntime::generateGlobalCtorDtorCalls() {
-  llvm::Module  = CGM.getModule();

I think you've been a bit too aggressive about what code you're moving. The 
code that generates the global constructor calls should not be removed.

We should generate correct IR from clang, and part of correct IR for HLSL is 
that the global constructors are called from the entries.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135429/new/

https://reviews.llvm.org/D135429

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In addition to restoring the test cases that you deleted, I think you should 
also reduce and simplify the test cases you're adding. These new tests are very 
large blobs of IR generated by clang. They include a lot of extra instructions 
that aren't needed to exercise the code you wrote in this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135429/new/

https://reviews.llvm.org/D135429

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135429: [HLSL] [DirectX backend] Move generateGlobalCtorDtorCalls into DirectX backend.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I don’t think you should be deleting all those frontend tests. Those tests 
existed before your change to remove the global constructor global variable, 
they provide valuable coverage of the frontend constructor generation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135429/new/

https://reviews.llvm.org/D135429

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133983: [HLSL] Add SV_DispatchThreadID

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133983/new/

https://reviews.llvm.org/D133983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

The llvm-config test issue should be resolved in rGa4b010034f57 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I think I have a fix for this and I'll get it up today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:73
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
   hlsl::ResourceClass::NumClasses)] = {0};

beanz wrote:
> The `ResourceCounters` here was a stand-in for allocating resource indices.
> 
> If you have a different path for allocating these, we shouldn't need the 
> counter anymore. I'm a little concerned that it looks like you're not 
> allocating these in code generation. Can you explain how you intend to 
> allocate indices? Is there a reason not to do this in code gen?
Wait... I guess I actually fed the `ResourceCounter` to the ID, which is fine.

I don't like that we're not allocating the indices in codegen, but we can 
address that in a subsequent patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130951/new/

https://reviews.llvm.org/D130951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:73
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
   hlsl::ResourceClass::NumClasses)] = {0};

The `ResourceCounters` here was a stand-in for allocating resource indices.

If you have a different path for allocating these, we shouldn't need the 
counter anymore. I'm a little concerned that it looks like you're not 
allocating these in code generation. Can you explain how you intend to allocate 
indices? Is there a reason not to do this in code gen?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130951/new/

https://reviews.llvm.org/D130951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Basic/HLSLRuntime.h:31
+// NOTE: keep sync with ResourceBase::Kinds in DirectX backend.
+enum class ResourceKind : uint32_t {
+  Invalid = 0,

If this is only used in the clangCodeGen library, we can move it into 
libLLVMFrontendHLSL to share instead of keeping them in sync.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:222
+  IRBuilder<> B(Ctx);
+  ResourceMD->addOperand(MDNode::get(
+  Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TyName),

Shouldn't this be using `FrontendResource`?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:253
+  addBufferResourceAnnotation(GV, QT.getAsString(),
+  static_cast(RC), Binding);
+}

since we're relying on these two enums basically being identical, can you add 
`static_assert`s up near the top of this file to ensure that they are the same? 
It would be unfortunate if in the future one got updated and the other didn't.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130951/new/

https://reviews.llvm.org/D130951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG911d2dc23035: [NFC] [HLSL] Move common metadata to 
LLVMFrontend (authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CMakeLists.txt
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/HLSL/CMakeLists.txt
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/lib/Target/DirectX/CMakeLists.txt
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h

Index: llvm/lib/Target/DirectX/DXILResource.h
===
--- llvm/lib/Target/DirectX/DXILResource.h
+++ llvm/lib/Target/DirectX/DXILResource.h
@@ -16,6 +16,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/Compiler.h"
 #include 
@@ -26,22 +27,6 @@
 
 namespace dxil {
 
-// FIXME: Ultimately this class and some of these utilities should be moved into
-// a new LLVMFrontendHLSL library so that they can be reused in Clang.
-// See issue https://github.com/llvm/llvm-project/issues/58000.
-class FrontendResource {
-  MDNode *Entry;
-
-public:
-  FrontendResource(MDNode *E) : Entry(E) {
-assert(Entry->getNumOperands() == 3 && "Unexpected metadata shape");
-  }
-
-  GlobalVariable *getGlobalVariable();
-  StringRef getSourceType();
-  Constant *getID();
-};
-
 class ResourceBase {
 protected:
   uint32_t ID;
@@ -50,7 +35,7 @@
   uint32_t Space;
   uint32_t LowerBound;
   uint32_t RangeSize;
-  ResourceBase(uint32_t I, FrontendResource R);
+  ResourceBase(uint32_t I, hlsl::FrontendResource R);
 
   void write(LLVMContext , MutableArrayRef Entries) const;
 
@@ -142,7 +127,7 @@
   void parseSourceType(StringRef S);
 
 public:
-  UAVResource(uint32_t I, FrontendResource R);
+  UAVResource(uint32_t I, hlsl::FrontendResource R);
 
   MDNode *write() const;
   void print(raw_ostream ) const;
Index: llvm/lib/Target/DirectX/DXILResource.cpp
===
--- llvm/lib/Target/DirectX/DXILResource.cpp
+++ llvm/lib/Target/DirectX/DXILResource.cpp
@@ -20,19 +20,7 @@
 
 using namespace llvm;
 using namespace llvm::dxil;
-
-GlobalVariable *FrontendResource::getGlobalVariable() {
-  return cast(
-  cast(Entry->getOperand(0))->getValue());
-}
-
-StringRef FrontendResource::getSourceType() {
-  return cast(Entry->getOperand(1))->getString();
-}
-
-Constant *FrontendResource::getID() {
-  return cast(Entry->getOperand(2))->getValue();
-}
+using namespace llvm::hlsl;
 
 void Resources::collectUAVs(Module ) {
   NamedMDNode *Entry = M.getNamedMetadata("hlsl.uavs");
Index: llvm/lib/Target/DirectX/CMakeLists.txt
===
--- llvm/lib/Target/DirectX/CMakeLists.txt
+++ llvm/lib/Target/DirectX/CMakeLists.txt
@@ -35,6 +35,7 @@
   Support
   DirectXInfo
   DXILBitWriter
+  FrontendHLSL
 
   ADD_TO_COMPONENT
   DirectX
Index: llvm/lib/Frontend/HLSL/HLSLResource.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/HLSL/HLSLResource.cpp
@@ -0,0 +1,41 @@
+//===- HLSLResource.cpp - HLSL Resource helper objects ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file This file contains helper objects for working with HLSL Resources.
+///
+//===--===//
+
+#include "llvm/Frontend/HLSL/HLSLResource.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
+
+using namespace llvm;
+using namespace llvm::hlsl;
+
+GlobalVariable *FrontendResource::getGlobalVariable() {
+  return cast(
+  cast(Entry->getOperand(0))->getValue());
+}
+
+StringRef FrontendResource::getSourceType() {
+  return cast(Entry->getOperand(1))->getString();
+}
+
+Constant *FrontendResource::getID() {
+  return cast(Entry->getOperand(2))->getValue();
+}
+
+FrontendResource::FrontendResource(GlobalVariable *GV, StringRef TypeStr,
+   uint32_t Counter) {
+  auto  = GV->getContext();
+  IRBuilder<> B(Ctx);
+  Entry =
+  MDNode::get(Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TypeStr),
+ConstantAsMetadata::get(B.getInt32(Counter))});
+}
Index: llvm/lib/Frontend/HLSL/CMakeLists.txt

[PATCH] D135973: Move HLSL builtins into hlsl namespace

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: python3kgae, pow2clk, bob80905.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

Should have done this from the start. Since all the injected AST types
are in the hlsl namespace we should also put the header-defined types
and functions in there too.

This updates the basic_types test to run once with the namespaced types
and once without, and adds using declarations or namespaces calls in
other tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135973

Files:
  clang/lib/Headers/hlsl/hlsl_basic_types.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/basic_types.hlsl
  clang/test/CodeGenHLSL/builtins/abs.hlsl
  clang/test/CodeGenHLSL/builtins/ceil.hlsl
  clang/test/CodeGenHLSL/builtins/sqrt.hlsl
  clang/test/SemaHLSL/Wave.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -5,5 +5,5 @@
 // expected-warning@#site {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
 // expected-note@hlsl/hlsl_intrinsics.h:* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
 // expected-note@#site {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
-return WaveActiveCountBits(b); // #site
+return hlsl::WaveActiveCountBits(b); // #site
 }
Index: clang/test/SemaHLSL/Wave.hlsl
===
--- clang/test/SemaHLSL/Wave.hlsl
+++ clang/test/SemaHLSL/Wave.hlsl
@@ -4,5 +4,5 @@
 
 // expected-no-diagnostics
 unsigned foo(bool b) {
-return WaveActiveCountBits(b);
+return hlsl::WaveActiveCountBits(b);
 }
Index: clang/test/CodeGenHLSL/builtins/sqrt.hlsl
===
--- clang/test/CodeGenHLSL/builtins/sqrt.hlsl
+++ clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::sqrt;
+
 double sqrt_d(double x)
 {
   return sqrt(x);
Index: clang/test/CodeGenHLSL/builtins/ceil.hlsl
===
--- clang/test/CodeGenHLSL/builtins/ceil.hlsl
+++ clang/test/CodeGenHLSL/builtins/ceil.hlsl
@@ -5,6 +5,8 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::ceil;
+
 // CHECK: define noundef half @
 // CHECK: call half @llvm.ceil.f16(
 // NO_HALF: define noundef float @"?test_ceil_half@@YA$halff@$halff@@Z"(
Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- clang/test/CodeGenHLSL/builtins/abs.hlsl
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -5,6 +5,7 @@
 // RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
 // RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
 
+using hlsl::abs;
 
 // CHECK: define noundef signext i16 @
 // FIXME: int16_t is promoted to i32 now. Change to abs.i16 once it is fixed.
Index: clang/test/CodeGenHLSL/basic_types.hlsl
===
--- clang/test/CodeGenHLSL/basic_types.hlsl
+++ clang/test/CodeGenHLSL/basic_types.hlsl
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
 // RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
 // RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - -DNAMESPACED| FileCheck %s
 
 
 // CHECK:"?uint16_t_Val@@3GA" = global i16 0, align 2
@@ -36,7 +39,11 @@
 // CHECK:"?double3_Val@@3T?$__vector@N$02@__clang@@A" = global <3 x double> zeroinitializer, align 32
 // CHECK:"?double4_Val@@3T?$__vector@N$03@__clang@@A" = global <4 x double> zeroinitializer, align 32
 
+#ifdef NAMESPACED
+#define TYPE_DECL(T)  hlsl::T T##_Val
+#else
 #define TYPE_DECL(T)  T T##_Val
+#endif
 
 #ifdef __HLSL_ENABLE_16_BIT
 TYPE_DECL(uint16_t);
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,8 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+namespace hlsl {
+
 __attribute__((availability(shadermodel, 

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: llvm/lib/Target/DirectX/DXILResource.h:46
   // can only be added to the end, and not removed.
   enum class Kinds : uint32_t {
 Invalid = 0,

python3kgae wrote:
> Could we move ResourceBase::Kinds to Frontend/HLSL/HLSLResource.h so we can 
> share it between clang and llvm?
Within reason, we can move any code that makes sense to share. We should do 
that in changes where the moved code will be used rather than making this 
change larger.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 467606.
beanz added a comment.

Rebasing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CMakeLists.txt
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/HLSL/CMakeLists.txt
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/lib/Target/DirectX/CMakeLists.txt
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h

Index: llvm/lib/Target/DirectX/DXILResource.h
===
--- llvm/lib/Target/DirectX/DXILResource.h
+++ llvm/lib/Target/DirectX/DXILResource.h
@@ -16,6 +16,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/Compiler.h"
 #include 
@@ -26,22 +27,6 @@
 
 namespace dxil {
 
-// FIXME: Ultimately this class and some of these utilities should be moved into
-// a new LLVMFrontendHLSL library so that they can be reused in Clang.
-// See issue https://github.com/llvm/llvm-project/issues/58000.
-class FrontendResource {
-  MDNode *Entry;
-
-public:
-  FrontendResource(MDNode *E) : Entry(E) {
-assert(Entry->getNumOperands() == 3 && "Unexpected metadata shape");
-  }
-
-  GlobalVariable *getGlobalVariable();
-  StringRef getSourceType();
-  Constant *getID();
-};
-
 class ResourceBase {
 protected:
   uint32_t ID;
@@ -50,7 +35,7 @@
   uint32_t Space;
   uint32_t LowerBound;
   uint32_t RangeSize;
-  ResourceBase(uint32_t I, FrontendResource R);
+  ResourceBase(uint32_t I, hlsl::FrontendResource R);
 
   void write(LLVMContext , MutableArrayRef Entries) const;
 
@@ -142,7 +127,7 @@
   void parseSourceType(StringRef S);
 
 public:
-  UAVResource(uint32_t I, FrontendResource R);
+  UAVResource(uint32_t I, hlsl::FrontendResource R);
 
   MDNode *write() const;
   void print(raw_ostream ) const;
Index: llvm/lib/Target/DirectX/DXILResource.cpp
===
--- llvm/lib/Target/DirectX/DXILResource.cpp
+++ llvm/lib/Target/DirectX/DXILResource.cpp
@@ -20,19 +20,7 @@
 
 using namespace llvm;
 using namespace llvm::dxil;
-
-GlobalVariable *FrontendResource::getGlobalVariable() {
-  return cast(
-  cast(Entry->getOperand(0))->getValue());
-}
-
-StringRef FrontendResource::getSourceType() {
-  return cast(Entry->getOperand(1))->getString();
-}
-
-Constant *FrontendResource::getID() {
-  return cast(Entry->getOperand(2))->getValue();
-}
+using namespace llvm::hlsl;
 
 void Resources::collectUAVs(Module ) {
   NamedMDNode *Entry = M.getNamedMetadata("hlsl.uavs");
Index: llvm/lib/Target/DirectX/CMakeLists.txt
===
--- llvm/lib/Target/DirectX/CMakeLists.txt
+++ llvm/lib/Target/DirectX/CMakeLists.txt
@@ -34,6 +34,7 @@
   Support
   DirectXInfo
   DXILBitWriter
+  FrontendHLSL
 
   ADD_TO_COMPONENT
   DirectX
Index: llvm/lib/Frontend/HLSL/HLSLResource.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/HLSL/HLSLResource.cpp
@@ -0,0 +1,41 @@
+//===- HLSLResource.cpp - HLSL Resource helper objects ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file This file contains helper objects for working with HLSL Resources.
+///
+//===--===//
+
+#include "llvm/Frontend/HLSL/HLSLResource.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
+
+using namespace llvm;
+using namespace llvm::hlsl;
+
+GlobalVariable *FrontendResource::getGlobalVariable() {
+  return cast(
+  cast(Entry->getOperand(0))->getValue());
+}
+
+StringRef FrontendResource::getSourceType() {
+  return cast(Entry->getOperand(1))->getString();
+}
+
+Constant *FrontendResource::getID() {
+  return cast(Entry->getOperand(2))->getValue();
+}
+
+FrontendResource::FrontendResource(GlobalVariable *GV, StringRef TypeStr,
+   uint32_t Counter) {
+  auto  = GV->getContext();
+  IRBuilder<> B(Ctx);
+  Entry =
+  MDNode::get(Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TypeStr),
+ConstantAsMetadata::get(B.getInt32(Counter))});
+}
Index: llvm/lib/Frontend/HLSL/CMakeLists.txt
===
--- /dev/null
+++ llvm/lib/Frontend/HLSL/CMakeLists.txt
@@ -0,0 +1,14 @@
+add_llvm_component_library(LLVMFrontendHLSL
+  

[PATCH] D135060: [HLSL] Add groupshare address space.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:11
+// expected-warning@+1 {{'auto' type specifier is a C++11 extension}}
+auto l = []() groupshared  {};
+

What happens to this if you set the language standard to hlsl 202x? I setup 
202x in Clang to be C++11-based since I think that is the direction 202x will 
take (still a lot unsure about the exact features).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135060/new/

https://reviews.llvm.org/D135060

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135060: [HLSL] Add groupshare address space.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:4063
+  let Spellings = [Keyword<"groupshared">];
+  let Documentation = [HLSLGroupSharedAddressSpaceDocs];
+}

Shouldn't this have `let Subjects = SubjectList<[Var]>;`?

I don't think we support `groupshared` on anything except variable declarations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135060/new/

https://reviews.llvm.org/D135060

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG19a0a5674911: [HLSL] Add utility to convert environment to 
stage (authored by beanz).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D135595?vs=466519=467261#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

Files:
  clang/include/clang/Basic/HLSLRuntime.h
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -249,6 +249,9 @@
 MacABI, // Mac Catalyst variant of Apple's iOS deployment target.
 
 // Shader Stages
+// The order of these values matters, and must be kept in sync with the
+// language options enum in Clang. The ordering is enforced in
+// static_asserts in Triple.cpp and in Clang.
 Pixel,
 Vertex,
 Geometry,
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/DarwinSDKInfo.h"
+#include "clang/Basic/HLSLRuntime.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -6819,12 +6820,12 @@
 static void handleHLSLNumThreadsAttr(Sema , Decl *D, const ParsedAttr ) {
   using llvm::Triple;
   Triple Target = S.Context.getTargetInfo().getTriple();
+  auto Env = S.Context.getTargetInfo().getTriple().getEnvironment();
   if (!llvm::is_contained({Triple::Compute, Triple::Mesh, Triple::Amplification,
Triple::Library},
-  Target.getEnvironment())) {
+  Env)) {
 uint32_t Pipeline =
-(uint32_t)S.Context.getTargetInfo().getTriple().getEnvironment() -
-(uint32_t)llvm::Triple::Pixel;
+static_cast(hlsl::getStageFromEnvironment(Env));
 S.Diag(AL.getLoc(), diag::err_hlsl_attr_unsupported_in_stage)
 << AL << Pipeline << "Compute, Amplification, Mesh or Library";
 return;
@@ -6891,16 +6892,13 @@
 
 static void handleHLSLSVGroupIndexAttr(Sema , Decl *D, const ParsedAttr ) {
   using llvm::Triple;
-  Triple Target = S.Context.getTargetInfo().getTriple();
-  if (Target.getEnvironment() != Triple::Compute &&
-  Target.getEnvironment() != Triple::Library) {
+  auto Env = S.Context.getTargetInfo().getTriple().getEnvironment();
+  if (Env != Triple::Compute && Env != Triple::Library) {
 // FIXME: it is OK for a compute shader entry and pixel shader entry live in
 // same HLSL file. Issue https://github.com/llvm/llvm-project/issues/57880.
-uint32_t Pipeline =
-(uint32_t)S.Context.getTargetInfo().getTriple().getEnvironment() -
-(uint32_t)llvm::Triple::Pixel;
+ShaderStage Pipeline = hlsl::getStageFromEnvironment(Env);
 S.Diag(AL.getLoc(), diag::err_hlsl_attr_unsupported_in_stage)
-<< AL << Pipeline << "Compute";
+<< AL << (uint32_t)Pipeline << "Compute";
 return;
   }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Randstruct.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/HLSLRuntime.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
@@ -10117,10 +10118,11 @@
 S->getDepth() == 0) {
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
-auto TripleShaderType = TargetInfo.getTriple().getEnvironment();
+auto Env = TargetInfo.getTriple().getEnvironment();
 AttributeCommonInfo AL(NewFD->getBeginLoc());
-HLSLShaderAttr::ShaderType ShaderType = (HLSLShaderAttr::ShaderType)(
-TripleShaderType - (uint32_t)llvm::Triple::Pixel);
+HLSLShaderAttr::ShaderType ShaderType =
+static_cast(
+hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
 if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -11,6 +11,7 @@
 //===--===//
 

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Basic/HLSLRuntime.h:39
+  return static_cast(Pipeline);
+}
+

bogner wrote:
> You're not actually introducing the dependency here (it was already there), 
> but neither `ShaderStage` in LangOptions.h nor the shader stage 
> `EnvironmentType` in Triple.h mention that they need to be kept in sync with 
> the other. Can you add comments to both that note the relationship?
Will do!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135721: [HLSL] Added HLSL this as a reference

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a reviewer: aaron.ballman.
beanz added a subscriber: aaron.ballman.
beanz added a comment.

Looping in @aaron.ballman here too because this is a bit of a fun one... I know 
Aaron loves when we show him the best of HLSL 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135721/new/

https://reviews.llvm.org/D135721

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

We should absolutely add utility functions when we have uses for them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D133668#3847871 , @rjmccall wrote:

> But that's purely on the implementation level, right?  Everything is 
> implicitly vectorized and you're just specifying the computation of a single 
> lane, but as far as that lane-wise computation is concerned, you just have 
> expressions which produce scalar values.

Yes, with the caveat that the language doesn't dictate the maximum SIMD width, 
but some features have minimum widths. The language source (and IR) operate on 
one lane of scalar and vector values, but we do have cross-SIMD lane 
operations, and true scalar (uniform) values, so we have to model the full 
breadth of parallel fun...

> If you don't otherwise have 16-bit (or 8-bit?) types, and it's the type 
> behavior you want, I'm fine with you just using `_BitInt`.  I just want to 
> make sure I understand the situation well enough to feel confident that 
> you've considered the alternatives.

We don't currently have 8-bit types (although I fully expect someone will want 
them because ML seems to love small data types). I suspect that the promoting 
integer behaviors for types smaller than int will likely never make sense for 
HLSL (or really any SPMD/implicit-SIMD) programming model).

My inclination is that we should define all our smaller than `int` fixed-size 
integer types as `_BitInt` to opt-out of promotion. Along with that I expect 
that we'll disable `short` under HLSL. We will still have `char`, but the 
intent for `char` is really for the _extremely_ limited cases where strings get 
used (i.e. `printf` for debugging).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133668/new/

https://reviews.llvm.org/D133668

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D133668#3847163 , @rjmccall wrote:

> Sure, but it's extremely easy to unpromote that arithmetic for most 
> operators, and I'm sure LLVM already has a pass that will do that.

Okay... but HLSL explicitly doesn't promote. Having the compiler rely on an 
optimization to generate correct code is undesirable. Especially since we want 
debug builds to behave correctly.

> Alternatively, if you're worried about your ability to unpromote, and since 
> you're breaking strict conformance anyway, have you considered just removing 
> the initial promotion to `int` from the usual arithmetic conversions?  I'm 
> pretty sure the rest of the rules hang together just fine if you do.  Then 
> you have a uniform rule that permits easier vectorization for all small 
> integer types rather than being sensitive specifically to using the `int16_t` 
> typedef.

HLSL isn't conformant to C or C++. One of the big areas that things get wonky 
is that every `int` is actually an implicit SIMD vector of a 
hardware-determined size.

We could disable promotion for HLSL. That is what we do in DXC. Part of what 
made me think `BitInt` was a better solution is that HLSL doesn't have the 
`short` keyword at all. The only 16-bit int types we support are the 
`[u]int16_t` explicit-sized typedefs. If you think disabling promotion under 
the HLSL language mode is a better approach, we can do that instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133668/new/

https://reviews.llvm.org/D133668

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Worth noting, we do have a similar set of static_asserts in Triple.cpp to 
validate the ordering of enum cases and that the subtraction results in the 
appropriate values:

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Triple.cpp#L1942


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D135595#3847056 , @python3kgae 
wrote:

> Backend needs the same thing.
> Is it possible to move this to llvm and share it between frontend and backend?

This translates the triple to a clang-defined enum... so strictly speaking no, 
that isn't possible.

Can you elaborate on the context where the backend needs this? I suspect in the 
backend we need a similar but not quite the same utility. Specifically, I think 
we need to translate the environment value to an unsigned integer. For all 
cases where an enum makes sense we should be using the Environment value in the 
backend.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: aaron.ballman, MaskRay, klimek, python3kgae, pow2clk.
Herald added subscribers: Anastasia, StephenFan.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

We had a bunch of places in the code where we were translating triple
environment enum cases to shader stage enum cases. The order of these
enums needs to be kept in sync for the translation to be simple, but we
were not properly handling out-of-bounds cases.

In normal compilation out-of-bounds cases shouldn't be possible because
the driver errors if you don't have a valid shader environment set, but
in clang tooling that error doesn't get treated as fatal and parsing
continues. This can result in crashes in clang tooling for out-of-range
shader stages.

To address this, this patch adds a constexpr method to handle the
conversion which handles out-of-range values by converting them to
`Invalid`.

Since this new method is a constexpr, the tests for this are a group of
static_asserts in the implementation file that verifies the correct
conversion for each valid enum case and validates that other cases are
converted to `Invalid`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135595

Files:
  clang/include/clang/Basic/HLSLRuntime.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/DarwinSDKInfo.h"
+#include "clang/Basic/HLSLRuntime.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -6819,12 +6820,12 @@
 static void handleHLSLNumThreadsAttr(Sema , Decl *D, const ParsedAttr ) {
   using llvm::Triple;
   Triple Target = S.Context.getTargetInfo().getTriple();
+  auto Env = S.Context.getTargetInfo().getTriple().getEnvironment();
   if (!llvm::is_contained({Triple::Compute, Triple::Mesh, Triple::Amplification,
Triple::Library},
-  Target.getEnvironment())) {
+  Env)) {
 uint32_t Pipeline =
-(uint32_t)S.Context.getTargetInfo().getTriple().getEnvironment() -
-(uint32_t)llvm::Triple::Pixel;
+static_cast(hlsl::getStageFromEnvironment(Env));
 S.Diag(AL.getLoc(), diag::err_hlsl_attr_unsupported_in_stage)
 << AL << Pipeline << "Compute, Amplification, Mesh or Library";
 return;
@@ -6891,16 +6892,13 @@
 
 static void handleHLSLSVGroupIndexAttr(Sema , Decl *D, const ParsedAttr ) {
   using llvm::Triple;
-  Triple Target = S.Context.getTargetInfo().getTriple();
-  if (Target.getEnvironment() != Triple::Compute &&
-  Target.getEnvironment() != Triple::Library) {
+  auto Env = S.Context.getTargetInfo().getTriple().getEnvironment();
+  if (Env != Triple::Compute && Env != Triple::Library) {
 // FIXME: it is OK for a compute shader entry and pixel shader entry live in
 // same HLSL file. Issue https://github.com/llvm/llvm-project/issues/57880.
-uint32_t Pipeline =
-(uint32_t)S.Context.getTargetInfo().getTriple().getEnvironment() -
-(uint32_t)llvm::Triple::Pixel;
+ShaderStage Pipeline = hlsl::getStageFromEnvironment(Env);
 S.Diag(AL.getLoc(), diag::err_hlsl_attr_unsupported_in_stage)
-<< AL << Pipeline << "Compute";
+<< AL << (uint32_t)Pipeline << "Compute";
 return;
   }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/Randstruct.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/HLSLRuntime.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
@@ -10071,10 +10072,11 @@
 S->getDepth() == 0) {
   CheckHLSLEntryPoint(NewFD);
   if (!NewFD->isInvalidDecl()) {
-auto TripleShaderType = TargetInfo.getTriple().getEnvironment();
+auto Env = TargetInfo.getTriple().getEnvironment();
 AttributeCommonInfo AL(NewFD->getBeginLoc());
-HLSLShaderAttr::ShaderType ShaderType = (HLSLShaderAttr::ShaderType)(
-TripleShaderType - (uint32_t)llvm::Triple::Pixel);
+HLSLShaderAttr::ShaderType ShaderType =
+static_cast(
+hlsl::getStageFromEnvironment(Env));
 // To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
 // function.
 if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
Index: clang/lib/Frontend/InitPreprocessor.cpp

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Avoiding argument promotion is one part of what we need, but not all of it. For 
example if you take this trial code:

  const RWBuffer In;
  RWBuffer Out;
  
  [numthreads(1,1,1)]
  void main(uint GI : SV_GroupIndex) {
Out[GI] = In[GI].x + In[GI].y;
  }

Following C rules, clang promotes the `short` math to `int` math, so the IR for 
`main` looks like:

  ; Function Attrs: noinline norecurse nounwind optnone
  define internal void @"?main@@YAXI@Z"(i32 noundef %GI) #2 {
  entry:
%GI.addr = alloca i32, align 4
store i32 %GI, ptr %GI.addr, align 4
%0 = load i32, ptr %GI.addr, align 4
%call = call noundef nonnull align 4 dereferenceable(4) ptr 
@"??A?$RWBuffer@T?$__vector@F$01@__clang@@@hlsl@@QBAAAT?$__vector@F$01@__clang@@I@Z"(ptr
 noundef nonnull align 4 dereferenceable(4) @In, i32 noundef %0)
%1 = load <2 x i16>, ptr %call, align 4
%2 = extractelement <2 x i16> %1, i32 0
%conv = sext i16 %2 to i32
%3 = load i32, ptr %GI.addr, align 4
%call1 = call noundef nonnull align 4 dereferenceable(4) ptr 
@"??A?$RWBuffer@T?$__vector@F$01@__clang@@@hlsl@@QBAAAT?$__vector@F$01@__clang@@I@Z"(ptr
 noundef nonnull align 4 dereferenceable(4) @In, i32 noundef %3)
%4 = load <2 x i16>, ptr %call1, align 4
%5 = extractelement <2 x i16> %4, i32 1
%conv2 = sext i16 %5 to i32
%add = add nsw i32 %conv, %conv2
%conv3 = trunc i32 %add to i16
%6 = load i32, ptr %GI.addr, align 4
%call4 = call noundef nonnull align 2 dereferenceable(2) ptr 
@"??A?$RWBuffer@F@hlsl@@QFI@Z"(ptr noundef nonnull align 4 
dereferenceable(4) @"?Out@@3V?$RWBuffer@F@hlsl@@A", i32 noundef %6)
store i16 %conv3, ptr %call4, align 2
ret void
  }

Because of the implicit vector nature of HLSL, these promotions and truncations 
would be extremely expensive. Using `_BitInt` allows us a language-header only 
solution that opts HLSL's `[u]int16_t` out of `Sema::UsualUnaryConversions`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133668/new/

https://reviews.llvm.org/D133668

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133983: [HLSL] Add SV_DispatchThreadID

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:135
+llvm::Function *DxThreadID = CGM.getIntrinsic(Intrinsic::dx_thread_id);
+// dx_thread_id
+return buildVectorInput(B, DxThreadID, Ty);

nit: this comment doesn't add value



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:166
+  unsigned SRetOffset = 0;
+  for (auto  : Fn->args()) {
+if (Param.hasStructRetAttr()) {

Was dropping the `const` here intentional? Getting the parameter's type 
shouldn't modify it.



Comment at: clang/test/CodeGenHLSL/sret_output.hlsl:5
+
+// FIXME: add semantic to a
+// See https://github.com/llvm/llvm-project/issues/57874





Comment at: clang/test/CodeGenHLSL/sret_output.hlsl:18
+};
\ No newline at end of file


add newline to the end of the file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133983/new/

https://reviews.llvm.org/D133983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133993: [HLSL] Remove global ctor/dtor variable for non-lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

@efriedma, I think that's a great suggestion. @python3kgae, if you don't adjust 
this patch, can you file an issue for that so that we don't lose that feedback?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133993/new/

https://reviews.llvm.org/D133993

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133993: [HLSL] Remove global ctor/dtor variable for non-lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D133993#3816526 , @efriedma wrote:

> Why are we using different mechanisms for global constructors in "libraries" 
> vs. other code?  If we need a mechanism in LLVM already, we might as well use 
> it all the time?

To elaborate here in a different wording... You can kinda think of HLSL 
"library" builds a lot like static archive linking in C/C++, so for the most 
part library shaders just work like C++.

The non-library mode is _very_ different. For non-library builds there is no 
linker, we emit fully contained executable code from the backend. In 
non-library code we need to inline global constructors into the entry function, 
but we don't need to keep the global variables around because we have no 
dynamic linker or loader, shader execution is much more similar to baremetal 
targets in that regard.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133993/new/

https://reviews.llvm.org/D133993

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a reviewer: asl.
beanz added a subscriber: asl.
beanz added a comment.

+@asl for codegen owner perspective.

This LGTM too. The changes here are well isolated to HLSL, so they should have 
no adverse impact on other language support.

@efriedma, @asl & @rjmccall any feedback?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134330: [Docs] [HLSL] Add note about PCH support

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/docs/HLSL/HLSLSupport.rst:98
+``HLSLExternalSemaSource`` will create new decls and use the old decls as
+argument for setPreviousDecl.
+

We can probably generalize this to something like:
```
When precompiled headers are used compiling HLSL, the ``ExternalSemaSource``
will be a ``MultiplexExternalSemaSource`` which include both the ``ASTReader``
and ``HLSLExternalSemaSource``. For Built-in declarations that are  already
completed in the serialized AST, the ``HLSLExternalSemaSource`` will reuse the
existing declarations and not introduce new declarations. If the built-in types
are not completed in the serialized AST, the ``HLSLExternalSemaSource`` will
create new declarations and connect the de-serialized decls as the previous
declaration.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134330/new/

https://reviews.llvm.org/D134330

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D135110#3832728 , @tschuett wrote:

> The OpenMP frontend is mainly an IRBuilder. It is a different layering than 
> for HLSL. Are there plans for an HLSL(IR)Builder?

HLSL and OpenMP are different in a lot of ways. HLSL's code generation is 
fairly consistent with C/C++. There will be some areas where we have special IR 
generation mechanics (like around copy-in/copy-out function parameters), which 
I expect we'll create some sort of reusable API for. I'm unsure if we have 
enough unique cases to make a full builder worth it.

Similar to OpenCL we also have a lot of metadata that needs to get passed from 
the frontend to the backend. That's really where this patch is starting. Having 
shared utilities for reading and writing metadata.




Comment at: llvm/include/llvm/Frontend/HLSL/HLSLResource.h:35
+  GlobalVariable *getGlobalVariable();
+  StringRef getSourceType();
+  Constant *getID();

python3kgae wrote:
> Could we save things like shape and component type instead of a StringRef 
> which needs to be parsed later?
This change is an NFC refactoring to move code around and start sharing between 
the front-end and backend. We have a separate issue tracking extending the 
frontend attribute support to capture more metadata 
(https://github.com/llvm/llvm-project/issues/57991). That will be a separate 
change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135110/new/

https://reviews.llvm.org/D135110

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135110: [NFC] [HLSL] Move common metadata to LLVMFrontend

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: python3kgae, tschuett, jdoerfert, bogner, pow2clk, 
tex3d, Anastasia, efriedma.
Herald added a subscriber: hiraditya.
Herald added a project: All.
beanz requested review of this revision.
Herald added projects: clang, LLVM.

This change pulls some code from the DirectX backend into a new
LLVMFrontendHLSL library to share utility data structures between the
HLSL code generation in Clang and the backend in LLVM.

This is a small refactoring as a first start to get code into the
right structure and get the library built and dependencies correct.

Fixes #58000 (https://github.com/llvm/llvm-project/issues/58000)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135110

Files:
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CMakeLists.txt
  llvm/include/llvm/Frontend/HLSL/HLSLResource.h
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/HLSL/CMakeLists.txt
  llvm/lib/Frontend/HLSL/HLSLResource.cpp
  llvm/lib/Target/DirectX/CMakeLists.txt
  llvm/lib/Target/DirectX/DXILResource.cpp
  llvm/lib/Target/DirectX/DXILResource.h

Index: llvm/lib/Target/DirectX/DXILResource.h
===
--- llvm/lib/Target/DirectX/DXILResource.h
+++ llvm/lib/Target/DirectX/DXILResource.h
@@ -16,6 +16,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Frontend/HLSL/HLSLResource.h"
 #include "llvm/IR/Metadata.h"
 #include 
 
@@ -25,22 +26,6 @@
 
 namespace dxil {
 
-// FIXME: Ultimately this class and some of these utilities should be moved into
-// a new LLVMFrontendHLSL library so that they can be reused in Clang.
-// See issue https://github.com/llvm/llvm-project/issues/58000.
-class FrontendResource {
-  MDNode *Entry;
-
-public:
-  FrontendResource(MDNode *E) : Entry(E) {
-assert(Entry->getNumOperands() == 3 && "Unexpected metadata shape");
-  }
-
-  GlobalVariable *getGlobalVariable();
-  StringRef getSourceType();
-  Constant *getID();
-};
-
 class ResourceBase {
 protected:
   uint32_t ID;
@@ -49,7 +34,7 @@
   uint32_t Space;
   uint32_t LowerBound;
   uint32_t RangeSize;
-  ResourceBase(uint32_t I, FrontendResource R);
+  ResourceBase(uint32_t I, hlsl::FrontendResource R);
 
   void write(LLVMContext , MutableArrayRef Entries);
 
@@ -130,7 +115,7 @@
   void parseSourceType(StringRef S);
 
 public:
-  UAVResource(uint32_t I, FrontendResource R);
+  UAVResource(uint32_t I, hlsl::FrontendResource R);
 
   MDNode *write();
 };
Index: llvm/lib/Target/DirectX/DXILResource.cpp
===
--- llvm/lib/Target/DirectX/DXILResource.cpp
+++ llvm/lib/Target/DirectX/DXILResource.cpp
@@ -18,19 +18,7 @@
 
 using namespace llvm;
 using namespace llvm::dxil;
-
-GlobalVariable *FrontendResource::getGlobalVariable() {
-  return cast(
-  cast(Entry->getOperand(0))->getValue());
-}
-
-StringRef FrontendResource::getSourceType() {
-  return cast(Entry->getOperand(1))->getString();
-}
-
-Constant *FrontendResource::getID() {
-  return cast(Entry->getOperand(2))->getValue();
-}
+using namespace llvm::hlsl;
 
 void Resources::collectUAVs() {
   NamedMDNode *Entry = Mod.getNamedMetadata("hlsl.uavs");
Index: llvm/lib/Target/DirectX/CMakeLists.txt
===
--- llvm/lib/Target/DirectX/CMakeLists.txt
+++ llvm/lib/Target/DirectX/CMakeLists.txt
@@ -32,6 +32,7 @@
   Support
   DirectXInfo
   DXILBitWriter
+  FrontendHLSL
 
   ADD_TO_COMPONENT
   DirectX
Index: llvm/lib/Frontend/HLSL/HLSLResource.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/HLSL/HLSLResource.cpp
@@ -0,0 +1,41 @@
+//===- HLSLResource.cpp - HLSL Resource helper objects ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file This file contains helper objects for working with HLSL Resources.
+///
+//===--===//
+
+#include "llvm/Frontend/HLSL/HLSLResource.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
+
+using namespace llvm;
+using namespace llvm::hlsl;
+
+GlobalVariable *FrontendResource::getGlobalVariable() {
+  return cast(
+  cast(Entry->getOperand(0))->getValue());
+}
+
+StringRef FrontendResource::getSourceType() {
+  return cast(Entry->getOperand(1))->getString();
+}
+
+Constant *FrontendResource::getID() {
+  return cast(Entry->getOperand(2))->getValue();
+}
+
+FrontendResource::FrontendResource(GlobalVariable *GV, StringRef TypeStr,
+   uint32_t Counter) {
+  auto  = 

[PATCH] D133983: [HLSL] Add SV_DispatchThreadID

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:167
+  for (const auto  : Fn->args()) {
+if (Param.hasStructRetAttr()) {
+  // FIXME: support output.

python3kgae wrote:
> beanz wrote:
> > I might be missing something, but I'm not seeing a test that exercises the 
> > `sret` case.
> Issue https://github.com/llvm/llvm-project/issues/57874 is to track this.
> Cannot create a legal test on compute shader which only has input.
My point is that if this code can't be reached and tested, we shouldn't add it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133983/new/

https://reviews.llvm.org/D133983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133983: [HLSL] Add SV_DispatchThreadID

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:167
+  for (const auto  : Fn->args()) {
+if (Param.hasStructRetAttr()) {
+  // FIXME: support output.

I might be missing something, but I'm not seeing a test that exercises the 
`sret` case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133983/new/

https://reviews.llvm.org/D133983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I think this approach mostly looks sane to me. @phosek, and @Ericson2314 may 
have different feedback.




Comment at: clang/lib/Support/CMakeLists.txt:23
+  # libLLVM-*.so).
+  llvm_add_library(clangSupport_tablegen
+STATIC

Unless there is a reason not to you should probably use `add_llvm_library` here 
probably with the `BUILDTREE_ONLY` option.



Comment at: clang/lib/Support/CMakeLists.txt:27
+${clangSupport_sources})
+endif()
+

We could add an `else` here that creates the clangSupport_tablegen target as an 
alias of clangSupport
```
add_library(clangSupport_tablegen ALIAS clangSupport)
```

See: https://cmake.org/cmake/help/v3.13/command/add_library.html#alias-libraries

This would allow clang-tablegen to always depend on clangSupport_tablegen 
simplifying the code on that end.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134637/new/

https://reviews.llvm.org/D134637

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134693: [CMake] Add `CLANG_ENABLE_HLSL` CMake option

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe432108bf254: [CMake] Add `CLANG_ENABLE_HLSL` CMake option 
(authored by beanz).

Changed prior to commit:
  https://reviews.llvm.org/D134693?vs=463079=463275#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134693/new/

https://reviews.llvm.org/D134693

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/HLSL.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt


Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -61,7 +61,11 @@
   set(CLANG_LINKS_TO_CREATE clang++ clang-cl clang-cpp)
 endif()
 
-foreach(link ${CLANG_LINKS_TO_CREATE})
+if (CLANG_ENABLE_HLSL)
+  set(HLSL_LINK clang-dxc)
+endif()
+
+foreach(link ${CLANG_LINKS_TO_CREATE} ${HLSL_LINK})
   add_clang_symlink(${link} clang)
 endforeach()
 
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -62,11 +62,15 @@
   __clang_hip_runtime_wrapper.h
   )
 
-set(hlsl_files
-  hlsl.h
+set(hlsl_h hlsl.h)
+set(hlsl_subdir_files
   hlsl/hlsl_basic_types.h
   hlsl/hlsl_intrinsics.h
   )
+set(hlsl_files
+  ${hlsl_h}
+  ${hlsl_subdir_files}
+  )
 
 set(mips_msa_files
   msa.h
@@ -548,10 +552,20 @@
   EXCLUDE_FROM_ALL
   COMPONENT x86-resource-headers)
 
+if(NOT CLANG_ENABLE_HLSL)
+  set(EXCLUDE_HLSL EXCLUDE_FROM_ALL)
+endif()
+
 install(
-  FILES ${hlsl_files}
+  FILES ${hlsl_h}
   DESTINATION ${header_install_dir}
-  EXCLUDE_FROM_ALL
+  ${EXCLUDE_HLSL}
+  COMPONENT hlsl-resource-headers)
+
+install(
+  FILES ${hlsl_subdir_files}
+  DESTINATION ${header_install_dir}/hlsl
+  ${EXCLUDE_HLSL}
   COMPONENT hlsl-resource-headers)
 
 install(
Index: clang/cmake/caches/HLSL.cmake
===
--- clang/cmake/caches/HLSL.cmake
+++ clang/cmake/caches/HLSL.cmake
@@ -9,3 +9,5 @@
 # HLSL support is currently limted to clang, eventually it will expand to
 # clang-tools-extra too.
 set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "")
+
+set(CLANG_ENABLE_HLSL On CACHE BOOL "")
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -482,6 +482,10 @@
"Generate build targets for the Clang unit tests."
${LLVM_INCLUDE_TESTS})
 
+option(CLANG_ENABLE_HLSL "Include HLSL build products" Off)
+# While HLSL support is experimental this should stay hidden.
+mark_as_advanced(CLANG_ENABLE_HLSL)
+
 add_subdirectory(utils/TableGen)
 
 # Export CLANG_TABLEGEN_EXE for use by flang docs.


Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -61,7 +61,11 @@
   set(CLANG_LINKS_TO_CREATE clang++ clang-cl clang-cpp)
 endif()
 
-foreach(link ${CLANG_LINKS_TO_CREATE})
+if (CLANG_ENABLE_HLSL)
+  set(HLSL_LINK clang-dxc)
+endif()
+
+foreach(link ${CLANG_LINKS_TO_CREATE} ${HLSL_LINK})
   add_clang_symlink(${link} clang)
 endforeach()
 
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -62,11 +62,15 @@
   __clang_hip_runtime_wrapper.h
   )
 
-set(hlsl_files
-  hlsl.h
+set(hlsl_h hlsl.h)
+set(hlsl_subdir_files
   hlsl/hlsl_basic_types.h
   hlsl/hlsl_intrinsics.h
   )
+set(hlsl_files
+  ${hlsl_h}
+  ${hlsl_subdir_files}
+  )
 
 set(mips_msa_files
   msa.h
@@ -548,10 +552,20 @@
   EXCLUDE_FROM_ALL
   COMPONENT x86-resource-headers)
 
+if(NOT CLANG_ENABLE_HLSL)
+  set(EXCLUDE_HLSL EXCLUDE_FROM_ALL)
+endif()
+
 install(
-  FILES ${hlsl_files}
+  FILES ${hlsl_h}
   DESTINATION ${header_install_dir}
-  EXCLUDE_FROM_ALL
+  ${EXCLUDE_HLSL}
+  COMPONENT hlsl-resource-headers)
+
+install(
+  FILES ${hlsl_subdir_files}
+  DESTINATION ${header_install_dir}/hlsl
+  ${EXCLUDE_HLSL}
   COMPONENT hlsl-resource-headers)
 
 install(
Index: clang/cmake/caches/HLSL.cmake
===
--- clang/cmake/caches/HLSL.cmake
+++ clang/cmake/caches/HLSL.cmake
@@ -9,3 +9,5 @@
 # HLSL support is currently limted to clang, eventually it will expand to
 # clang-tools-extra too.
 set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "")
+
+set(CLANG_ENABLE_HLSL On CACHE BOOL "")
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -482,6 +482,10 @@
"Generate build targets for the Clang unit tests."
${LLVM_INCLUDE_TESTS})
 
+option(CLANG_ENABLE_HLSL "Include HLSL build products" Off)
+# While HLSL support is experimental this should 

[PATCH] D134693: [CMake] Add `CLANG_ENABLE_HLSL` CMake option

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: phosek, smeenai, compnerd.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

The HLSL support in clang is in proress and not fully functioning. As
such we don't want to install the related optional build components by
default (yet), but we do need an option to build and install them
locally for testing and for some key users.

This adds the `CLANG_ENABLE_HLSL` option which is off by default and can
be enabled to install the HLSL clang headers and the clang-dxc symlink.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134693

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/HLSL.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt


Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -61,7 +61,11 @@
   set(CLANG_LINKS_TO_CREATE clang++ clang-cl clang-cpp)
 endif()
 
-foreach(link ${CLANG_LINKS_TO_CREATE})
+if (CLANG_ENABLE_HLSL)
+  set(HLSL_LINK clang-dxc)
+endif()
+
+foreach(link ${CLANG_LINKS_TO_CREATE} ${HLSL_LINK})
   add_clang_symlink(${link} clang)
 endforeach()
 
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -548,10 +548,14 @@
   EXCLUDE_FROM_ALL
   COMPONENT x86-resource-headers)
 
+if(NOT CLANG_ENABLE_HLSL)
+  set(EXCLUDE_HLSL EXCLUDE_FROM_ALL)
+endif()
+
 install(
   FILES ${hlsl_files}
   DESTINATION ${header_install_dir}
-  EXCLUDE_FROM_ALL
+  ${EXCLUDE_HLSL}
   COMPONENT hlsl-resource-headers)
 
 install(
Index: clang/cmake/caches/HLSL.cmake
===
--- clang/cmake/caches/HLSL.cmake
+++ clang/cmake/caches/HLSL.cmake
@@ -9,3 +9,5 @@
 # HLSL support is currently limted to clang, eventually it will expand to
 # clang-tools-extra too.
 set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "")
+
+set(CLANG_ENABLE_HLSL On CACHE BOOL "")
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -482,6 +482,10 @@
"Generate build targets for the Clang unit tests."
${LLVM_INCLUDE_TESTS})
 
+option(CLANG_ENABLE_HLSL "Include HLSL build products" Off)
+# While HLSL support is experimental this should stay hidden.
+mark_as_advanced(CLANG_ENABLE_HLSL)
+
 add_subdirectory(utils/TableGen)
 
 # Export CLANG_TABLEGEN_EXE for use by flang docs.


Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -61,7 +61,11 @@
   set(CLANG_LINKS_TO_CREATE clang++ clang-cl clang-cpp)
 endif()
 
-foreach(link ${CLANG_LINKS_TO_CREATE})
+if (CLANG_ENABLE_HLSL)
+  set(HLSL_LINK clang-dxc)
+endif()
+
+foreach(link ${CLANG_LINKS_TO_CREATE} ${HLSL_LINK})
   add_clang_symlink(${link} clang)
 endforeach()
 
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -548,10 +548,14 @@
   EXCLUDE_FROM_ALL
   COMPONENT x86-resource-headers)
 
+if(NOT CLANG_ENABLE_HLSL)
+  set(EXCLUDE_HLSL EXCLUDE_FROM_ALL)
+endif()
+
 install(
   FILES ${hlsl_files}
   DESTINATION ${header_install_dir}
-  EXCLUDE_FROM_ALL
+  ${EXCLUDE_HLSL}
   COMPONENT hlsl-resource-headers)
 
 install(
Index: clang/cmake/caches/HLSL.cmake
===
--- clang/cmake/caches/HLSL.cmake
+++ clang/cmake/caches/HLSL.cmake
@@ -9,3 +9,5 @@
 # HLSL support is currently limted to clang, eventually it will expand to
 # clang-tools-extra too.
 set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "")
+
+set(CLANG_ENABLE_HLSL On CACHE BOOL "")
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -482,6 +482,10 @@
"Generate build targets for the Clang unit tests."
${LLVM_INCLUDE_TESTS})
 
+option(CLANG_ENABLE_HLSL "Include HLSL build products" Off)
+# While HLSL support is experimental this should stay hidden.
+mark_as_advanced(CLANG_ENABLE_HLSL)
+
 add_subdirectory(utils/TableGen)
 
 # Export CLANG_TABLEGEN_EXE for use by flang docs.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134304: [Docs] [HLSL] Add IR reference for HLSL

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd20f9f8d2177: [Docs] [HLSL] Add IR reference for HLSL 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134304/new/

https://reviews.llvm.org/D134304

Files:
  clang/docs/HLSL/HLSLDocs.rst
  clang/docs/HLSL/HLSLIRReference.rst


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the external global variables that
+represent UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the external global variables that
+represent UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134304: [Docs] [HLSL] Add IR reference for HLSL

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 462299.
beanz added a comment.

Updating documentation for `hlsl.uavs`, thanks @python3kgae!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134304/new/

https://reviews.llvm.org/D134304

Files:
  clang/docs/HLSL/HLSLDocs.rst
  clang/docs/HLSL/HLSLIRReference.rst


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the external global variables that
+represent UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the external global variables that
+represent UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131799: [HLSL] clang codeGen for HLSLNumThreadsAttr

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131799/new/

https://reviews.llvm.org/D131799

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

This is looking great. Thank you for all your work iterating on this.

Please update the description to reflect the changes in implementation.




Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:120
+
+void addResourceBinding(GlobalVariable *GV, CGHLSLRuntime::Buffer ) {
+  // FIXME: add resource binding to GV.

Please remove the empty function. We can add it when it does something. Also 
make sure we have an issue for the FIXME.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:141
+
+  // FIXME: support packoffset.
+  uint32_t Offset = 0;

Please file an issue for this or link to the issue if it exists.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134326: [HLSL] Allow SV_GroupIndex for lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134326/new/

https://reviews.llvm.org/D134326

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134326: [HLSL] Allow SV_GroupIndex for lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6901
+// FIXME: it is OK for a compute shader entry and pixel shader entry live 
in
+// same HLSL file.
 uint32_t Pipeline =

I think the underlying issue here is that some attributes should only be 
validated on entry points that are being compiled as entry points in the 
current compilation action. Please file a GitHub issue for this and put the 
issue URL in the comment here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134326/new/

https://reviews.llvm.org/D134326

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134319: [HLSL] add ceil library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

For this builtin you should be able to use `__builtin_elementwise_ceil` for all 
the overload cases, and you should be able to add the vector cases too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134319/new/

https://reviews.llvm.org/D134319

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133993: [HLSL] Remove global ctor/dtor variable for non-lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:208
+  // ctors/dtors added for entry.
+  Triple T(M.getTargetTriple());
+  if (T.getEnvironment() != Triple::EnvironmentType::Library) {

python3kgae wrote:
> beanz wrote:
> > I question whether we should do this early or late. It feels wrong to me to 
> > have clang modifying IR. There are places where clang does this sort of 
> > thing so it isn't unprecedented, but it feels wrong.
> The reason I want to do it is that with the global variable for ctors/dtors, 
> the ctor/dtor functions will not be removed in optimization passes.
> As a result, the global variable will have 2 stores ( 1 for the ctor, 1 for 
> the inlined ctor ). That might cause optimizations to go a different path.
> 
> Also, we insert the call of dtor/ctors to entry which already did what the 
> global variable for ctor/dtor asked. If the global variable for ctor/dtor is 
> still kept, other backends might call the ctor/dtor again.
That makes sense, and is a fair reasoning. This makes me wonder if we would 
benefit from having a target-specific hook to add early IR optimization passes, 
but that is a whole different discussion...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133993/new/

https://reviews.llvm.org/D133993

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133993: [HLSL] Remove global ctor/dtor variable for non-lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:208
+  // ctors/dtors added for entry.
+  Triple T(M.getTargetTriple());
+  if (T.getEnvironment() != Triple::EnvironmentType::Library) {

I question whether we should do this early or late. It feels wrong to me to 
have clang modifying IR. There are places where clang does this sort of thing 
so it isn't unprecedented, but it feels wrong.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6898
   Triple Target = S.Context.getTargetInfo().getTriple();
-  if (Target.getEnvironment() != Triple::Compute) {
+  if (Target.getEnvironment() != Triple::Compute &&
+  Target.getEnvironment() != Triple::Library) {

This change should be separate. It doesn't directly relate to the change you've 
described in the description. I understand you need it to use the same test 
source, but don't group unrelated functional changes.



Comment at: clang/test/CodeGenHLSL/GlobalDestructorsLib.hlsl:1
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -std=hlsl202x -S 
-emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -std=hlsl202x -S 
-emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+

The source of this test and the test it was copied from are almost identical. 
It feels very much like they should be the same file with different check 
prefixes. You can even have them share a check prefix for the things that are 
common and only have a differing prefix for the things that are different.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133993/new/

https://reviews.llvm.org/D133993

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134312: [HLSL] remove unnecessary abs attributes

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd76c654d02b6: [HLSL] remove unnecessary abs attributes 
(authored by bob80905, committed by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134312/new/

https://reviews.llvm.org/D134312

Files:
  clang/lib/Headers/hlsl/hlsl_intrinsics.h


Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -15,11 +15,6 @@
 
 
 // abs builtins
-__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
-__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
-__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
-__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
-
 #ifdef __HLSL_ENABLE_16_BIT
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs)))
 int16_t abs(int16_t);


Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -15,11 +15,6 @@
 
 
 // abs builtins
-__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
-__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
-__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
-__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
-
 #ifdef __HLSL_ENABLE_16_BIT
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs)))
 int16_t abs(int16_t);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134312: [HLSL] remove unnecessary abs attributes

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

Doh... I should have caught this before I pushed your last change. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134312/new/

https://reviews.llvm.org/D134312

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134304: [Docs] [HLSL] Add IR reference for HLSL

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/docs/HLSL/HLSLIRReference.rst:20
+
+The ``hlsl.uavs`` metadata is a list of all the global variables that represent
+to UAV resources.

python3kgae wrote:
> Maybe only external global variables?
> 
Do they have to be external? `static RWBuffer Buf` is valid right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134304/new/

https://reviews.llvm.org/D134304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132711: [HLSL] add sqrt library function

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb95c57444a8a: [HLSL] add sqrt library function (authored by 
bob80905, committed by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132711/new/

https://reviews.llvm.org/D132711

Files:
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/builtins/sqrt.hlsl


Index: clang/test/CodeGenHLSL/builtins/sqrt.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.2-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
+// RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
+
+double sqrt_d(double x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef double @"?sqrt_d@@YANN@Z"(
+// CHECK: call double @llvm.sqrt.f64(double %0)
+
+float sqrt_f(float x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef float @"?sqrt_f@@YAMM@Z"(
+// CHECK: call float @llvm.sqrt.f32(float %0)
+
+half sqrt_h(half x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef half @"?sqrt_h@@YA$f16@$f16@@Z"(
+// CHECK: call half @llvm.sqrt.f16(half %0)
+// NO_HALF: define noundef float @"?sqrt_h@@YA$halff@$halff@@Z"(
+// NO_HALF: call float @llvm.sqrt.f32(float %0)
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -13,6 +13,13 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) 
uint
 WaveActiveCountBits(bool bBit);
 
+
+// abs builtins
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
+__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
+
 #ifdef __HLSL_ENABLE_16_BIT
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs)))
 int16_t abs(int16_t);
@@ -31,6 +38,7 @@
 half4 abs(half4);
 #endif
 
+
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs))) int abs(int);
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs))) int2 abs(int2);
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs))) int3 abs(int3);
@@ -60,4 +68,13 @@
 __attribute__((clang_builtin_alias(__builtin_elementwise_abs)))
 double4 abs(double4);
 
+// sqrt builtins
+__attribute__((clang_builtin_alias(__builtin_sqrt))) double sqrt(double In);
+__attribute__((clang_builtin_alias(__builtin_sqrtf))) float sqrt(float In);
+
+#ifdef __HLSL_ENABLE_16_BIT
+__attribute__((clang_builtin_alias(__builtin_sqrtf16))) half sqrt(half In);
+#endif
+
+
 #endif //_HLSL_HLSL_INTRINSICS_H_


Index: clang/test/CodeGenHLSL/builtins/sqrt.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.2-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
+// RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
+
+double sqrt_d(double x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef double @"?sqrt_d@@YANN@Z"(
+// CHECK: call double @llvm.sqrt.f64(double %0)
+
+float sqrt_f(float x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef float @"?sqrt_f@@YAMM@Z"(
+// CHECK: call float @llvm.sqrt.f32(float %0)
+
+half sqrt_h(half x)
+{
+  return sqrt(x);
+}
+
+// CHECK: define noundef half @"?sqrt_h@@YA$f16@$f16@@Z"(
+// CHECK: call half @llvm.sqrt.f16(half %0)
+// NO_HALF: define noundef float @"?sqrt_h@@YA$halff@$halff@@Z"(
+// NO_HALF: call float @llvm.sqrt.f32(float %0)
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -13,6 +13,13 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
+
+// abs builtins
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);

[PATCH] D134304: [Docs] [HLSL] Add IR reference for HLSL

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/docs/HLSL/HLSLIRReference.rst:12
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+

python3kgae wrote:
> Once we document a metadata attribute, is it OK to update the format later?
Absolutely, none of this is a binding ABI, this document is just intended to be 
a reference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134304/new/

https://reviews.llvm.org/D134304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132421: [HLSL] Support PCH for cc1 mode

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

One small nit, otherwise looks good.




Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:18
 #include "clang/Sema/ExternalSemaSource.h"
+#include "clang/Sema/MultiplexExternalSemaSource.h"
 

nit: This seems to be unused


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132421/new/

https://reviews.llvm.org/D132421

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134304: [Docs] [HLSL] Add IR reference for HLSL

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: pow2clk, python3kgae.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

HLSL uses a variety of named IR metadata and attributes to convey
additional information from the frontend to the backend. This document
tries to capture and document the named annotations to provide a
reference for future contributors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134304

Files:
  clang/docs/HLSL/HLSLDocs.rst
  clang/docs/HLSL/HLSLIRReference.rst


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the global variables that represent
+to UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions


Index: clang/docs/HLSL/HLSLIRReference.rst
===
--- /dev/null
+++ clang/docs/HLSL/HLSLIRReference.rst
@@ -0,0 +1,31 @@
+=
+HLSL IR Reference
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+The goal of this document is to provide a reference for all the special purpose
+IR metadata and attributes used by the HLSL code generation path.
+
+IR Metadata
+===
+
+``hlsl.uavs``
+-
+
+The ``hlsl.uavs`` metadata is a list of all the global variables that represent
+to UAV resources.
+
+Function Attributes
+===
+
+``hlsl.shader``
+---
+
+The ``hlsl.shader`` function attribute is a string attribute applied to entry
+functions. The value is the string representation of the shader stage (i.e.
+``compute``, ``pixel``, etc).
Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -11,5 +11,6 @@
 .. toctree::
:maxdepth: 1
 
+   HLSLIRReference
ResourceTypes
EntryFunctions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132711: [HLSL] add sqrt library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132711/new/

https://reviews.llvm.org/D132711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133958: [HLSL] Pass flags to cc1 based on language

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0c89b343371f: [HLSL] Pass flags to cc1 based on language 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133958/new/

https://reviews.llvm.org/D133958

Files:
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/Types.cpp
  clang/test/Driver/hlsl_no_stdinc.hlsl


Index: clang/test/Driver/hlsl_no_stdinc.hlsl
===
--- clang/test/Driver/hlsl_no_stdinc.hlsl
+++ clang/test/Driver/hlsl_no_stdinc.hlsl
@@ -1,5 +1,7 @@
 // RUN: %clang_dxc  -Tlib_6_7 -fcgl -Fo - %s -### 2>&1 | FileCheck %s 
--check-prefix=STDINC
+// RUN: %clang -target dxil-pc-shadermodel6.3-library -o - %s -### 2>&1 | 
FileCheck %s --check-prefix=STDINC
 // RUN: %clang_dxc  -Tlib_6_7 -hlsl-no-stdinc -fcgl -Fo - %s -### 2>&1 | 
FileCheck %s --check-prefix=NOSTDINC
+// RUN: %clang -target dxil-pc-shadermodel6.3-library -nostdinc -o - %s -### 
2>&1 | FileCheck %s --check-prefix=NOSTDINC
 
 // RUN: %clang -cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump 
-o - %s -verify
 
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -286,6 +286,8 @@
   }
 }
 
+bool types::isHLSL(ID Id) { return Id == TY_HLSL; }
+
 bool types::isSrcFile(ID Id) {
   return Id != TY_Object && getPreprocessedType(Id) != TY_INVALID;
 }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3529,12 +3529,14 @@
  options::OPT_disable_llvm_passes,
  options::OPT_fnative_half_type,
  options::OPT_hlsl_entrypoint};
-
+  if (!types::isHLSL(InputType))
+return;
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
   A->renderAsInput(Args, CmdArgs);
   // Add the default headers if dxc_no_stdinc is not set.
-  if (!Args.hasArg(options::OPT_dxc_no_stdinc))
+  if (!Args.hasArg(options::OPT_dxc_no_stdinc) &&
+  !Args.hasArg(options::OPT_nostdinc))
 CmdArgs.push_back("-finclude-default-header");
 }
 
@@ -6389,8 +6391,7 @@
   RenderOpenCLOptions(Args, CmdArgs, InputType);
 
   // Forward hlsl options to -cc1
-  if (C.getDriver().IsDXCMode())
-RenderHLSLOptions(Args, CmdArgs, InputType);
+  RenderHLSLOptions(Args, CmdArgs, InputType);
 
   if (IsHIP) {
 if (Args.hasFlag(options::OPT_fhip_new_launch_api,
Index: clang/include/clang/Driver/Types.h
===
--- clang/include/clang/Driver/Types.h
+++ clang/include/clang/Driver/Types.h
@@ -95,6 +95,9 @@
   /// isOpenCL - Is this an "OpenCL" input.
   bool isOpenCL(ID Id);
 
+  /// isHLSL - Is this an HLSL input.
+  bool isHLSL(ID Id);
+
   /// isSrcFile - Is this a source file, i.e. something that still has to be
   /// preprocessed. The logic behind this is the same that decides if the first
   /// compilation phase is a preprocessing one.


Index: clang/test/Driver/hlsl_no_stdinc.hlsl
===
--- clang/test/Driver/hlsl_no_stdinc.hlsl
+++ clang/test/Driver/hlsl_no_stdinc.hlsl
@@ -1,5 +1,7 @@
 // RUN: %clang_dxc  -Tlib_6_7 -fcgl -Fo - %s -### 2>&1 | FileCheck %s --check-prefix=STDINC
+// RUN: %clang -target dxil-pc-shadermodel6.3-library -o - %s -### 2>&1 | FileCheck %s --check-prefix=STDINC
 // RUN: %clang_dxc  -Tlib_6_7 -hlsl-no-stdinc -fcgl -Fo - %s -### 2>&1 | FileCheck %s --check-prefix=NOSTDINC
+// RUN: %clang -target dxil-pc-shadermodel6.3-library -nostdinc -o - %s -### 2>&1 | FileCheck %s --check-prefix=NOSTDINC
 
 // RUN: %clang -cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s -verify
 
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -286,6 +286,8 @@
   }
 }
 
+bool types::isHLSL(ID Id) { return Id == TY_HLSL; }
+
 bool types::isSrcFile(ID Id) {
   return Id != TY_Object && getPreprocessedType(Id) != TY_INVALID;
 }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3529,12 +3529,14 @@
  options::OPT_disable_llvm_passes,
  options::OPT_fnative_half_type,
  options::OPT_hlsl_entrypoint};
-
+  if (!types::isHLSL(InputType))
+return;
   for (const auto  : ForwardedArguments)
 if (const auto *A = 

[PATCH] D133993: [HLSL] Remove global ctor/dtor variable for non-lib profile.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

We should also have a library test case that verifies that the `global_dtors` 
variable is kept.




Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:50
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
-  void generateGlobalCtorDtorCalls();
+  void generateGlobalCtorDtorCalls(llvm::Triple );
 

You don't need to pass the triple in here. If you have the module, you have the 
triple.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133993/new/

https://reviews.llvm.org/D133993

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134067: [HLSL] Enable availability attribute

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG10378c45055f: [HLSL] Enable availability attribute (authored 
by beanz).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134067/new/

https://reviews.llvm.org/D134067

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/test/SemaHLSL/AvailabilityMarkup.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel5.0-library -verify %s
+// WaveActiveCountBits is unavailable before ShaderModel 6.0.
+
+unsigned foo(bool b) {
+// expected-warning@#site {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@hlsl/hlsl_intrinsics.h:* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#site {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
+return WaveActiveCountBits(b); // #site
+}
Index: clang/test/SemaHLSL/AvailabilityMarkup.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/AvailabilityMarkup.hlsl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-library -verify %s
+
+__attribute__((availability(shadermodel, introduced = 6.0)))
+unsigned fn6_0(); // #fn6_0
+
+__attribute__((availability(shadermodel, introduced = 5.1)))
+unsigned fn5_1(); // #fn5_1
+
+__attribute__((availability(shadermodel, introduced = 5.0)))
+unsigned fn5_0();
+
+void fn() {
+// expected-warning@#fn6_0_site {{'fn6_0' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@#fn6_0 {{'fn6_0' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#fn6_0_site {{enclose 'fn6_0' in a __builtin_available check to silence this warning}}
+unsigned A = fn6_0(); // #fn6_0_site
+
+// expected-warning@#fn5_1_site {{'fn5_1' is only available on HLSL ShaderModel 5.1 or newer}}
+// expected-note@#fn5_1 {{'fn5_1' has been marked as being introduced in HLSL ShaderModel 5.1 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#fn5_1_site {{enclose 'fn5_1' in a __builtin_available check to silence this warning}}
+unsigned B = fn5_1(); // #fn5_1_site
+
+unsigned C = fn5_0();
+}
+
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -192,6 +192,9 @@
   case llvm::Triple::MacOSX:
 ForceAvailabilityFromVersion = VersionTuple(/*Major=*/10, /*Minor=*/13);
 break;
+  case llvm::Triple::ShaderModel:
+// Always enable availability diagnostics for shader models.
+return true;
   default:
 // New targets should always warn about availability.
 return Triple.getVendor() == llvm::Triple::Apple;
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,7 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+__attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
Index: clang/lib/Basic/Targets/DirectX.h
===
--- clang/lib/Basic/Targets/DirectX.h
+++ clang/lib/Basic/Targets/DirectX.h
@@ -55,6 +55,8 @@
 HasLegalHalfType = true;
 HasFloat16 = true;
 NoAsmVariants = true;
+PlatformMinVersion = Triple.getOSVersion();
+PlatformName = llvm::Triple::getOSTypeName(Triple.getOS());
 resetDataLayout("e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:"
 "32-f64:64-n8:16:32:64");
 TheCXXABI.set(TargetCXXABI::Microsoft);
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -908,6 +908,7 @@
  .Case("maccatalyst", "macCatalyst")
  .Case("maccatalyst_app_extension", "macCatalyst (App Extension)")
  .Case("swift", "Swift")
+ .Case("shadermodel", "HLSL ShaderModel")
  .Default(llvm::StringRef());
 }
 static llvm::StringRef 

[PATCH] D134067: [HLSL] Enable availability attribute

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 460879.
beanz added a comment.

Updating test cases to use labels and sort the matches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134067/new/

https://reviews.llvm.org/D134067

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/test/SemaHLSL/AvailabilityMarkup.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel5.0-library -verify %s
+// WaveActiveCountBits is unavailable before ShaderModel 6.0.
+
+unsigned foo(bool b) {
+// expected-warning@#site {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@hlsl/hlsl_intrinsics.h:* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#site {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
+return WaveActiveCountBits(b); // #site
+}
Index: clang/test/SemaHLSL/AvailabilityMarkup.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/AvailabilityMarkup.hlsl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-library -verify %s
+
+__attribute__((availability(shadermodel, introduced = 6.0)))
+unsigned fn6_0(); // #fn6_0
+
+__attribute__((availability(shadermodel, introduced = 5.1)))
+unsigned fn5_1(); // #fn5_1
+
+__attribute__((availability(shadermodel, introduced = 5.0)))
+unsigned fn5_0();
+
+void fn() {
+// expected-warning@#fn6_0_site {{'fn6_0' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@#fn6_0 {{'fn6_0' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#fn6_0_site {{enclose 'fn6_0' in a __builtin_available check to silence this warning}}
+unsigned A = fn6_0(); // #fn6_0_site
+
+// expected-warning@#fn5_1_site {{'fn5_1' is only available on HLSL ShaderModel 5.1 or newer}}
+// expected-note@#fn5_1 {{'fn5_1' has been marked as being introduced in HLSL ShaderModel 5.1 here, but the deployment target is HLSL ShaderModel 5.0}}
+// expected-note@#fn5_1_site {{enclose 'fn5_1' in a __builtin_available check to silence this warning}}
+unsigned B = fn5_1(); // #fn5_1_site
+
+unsigned C = fn5_0();
+}
+
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -192,6 +192,9 @@
   case llvm::Triple::MacOSX:
 ForceAvailabilityFromVersion = VersionTuple(/*Major=*/10, /*Minor=*/13);
 break;
+  case llvm::Triple::ShaderModel:
+// Always enable availability diagnostics for shader models.
+return true;
   default:
 // New targets should always warn about availability.
 return Triple.getVendor() == llvm::Triple::Apple;
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,7 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+__attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
Index: clang/lib/Basic/Targets/DirectX.h
===
--- clang/lib/Basic/Targets/DirectX.h
+++ clang/lib/Basic/Targets/DirectX.h
@@ -55,6 +55,8 @@
 HasLegalHalfType = true;
 HasFloat16 = true;
 NoAsmVariants = true;
+PlatformMinVersion = Triple.getOSVersion();
+PlatformName = llvm::Triple::getOSTypeName(Triple.getOS());
 resetDataLayout("e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:"
 "32-f64:64-n8:16:32:64");
 TheCXXABI.set(TargetCXXABI::Microsoft);
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -908,6 +908,7 @@
  .Case("maccatalyst", "macCatalyst")
  .Case("maccatalyst_app_extension", "macCatalyst (App Extension)")
  .Case("swift", "Swift")
+ .Case("shadermodel", "HLSL ShaderModel")
  .Default(llvm::StringRef());
 }
 static llvm::StringRef getPlatformNameSourceSpelling(llvm::StringRef Platform) {
@@ -923,6 +924,7 @@
  

[PATCH] D134067: [HLSL] Enable availability attribute

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/test/SemaHLSL/AvailabilityMarkup.hlsl:15
+void fn() {
+// expected-warning@+2 {{'fn6_0' is only available on HLSL ShaderModel 6.0 
or newer}}
+// expected-note@+1 {{enclose 'fn6_0' in a __builtin_available check to 
silence this warning}}

erichkeane wrote:
> So minor thing, and one that I am going to start pushing for more, is better 
> organization of 'expected-diagnostics'.  I think they should better reflect 
> the order and 'cause' of the diagnostic, particularly with notes.  So 
> something like:
> 
> ``` 
>   // expected-warning@+3 {{... only available ...}}
>   // expected-note@#FN60_LOC {{ marked as being introduced...}}
>   // expected-note@+1{{enclose...}}
> ```
> 
> Then on line 5, add the comment: `// FN60`
> 
> I have this preference because it makes it more clear to the reader where the 
> notes come from.  I realize this is a new direction, and perhaps pushing my 
> ability to request, but I'd still appreciate it happening here (I WILL be 
> pushing for it on template reviews, where I think it is even MORE beneficial, 
> thanks to 'instantiation of' diagnostics, and I have the authority to demand 
> it :) ).
I was completely unaware of the ability to create labels like that. That's 
awesome. I'll update the tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134067/new/

https://reviews.llvm.org/D134067

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134067: [HLSL] Enable availability attribute

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 460869.
beanz added a comment.

Updating based on feedback from @erichkeane. Thank you for the fast feedback!

- Moved ShaderModel check into a switch case.
- Added additional self-contained test case with more variations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134067/new/

https://reviews.llvm.org/D134067

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/test/SemaHLSL/AvailabilityMarkup.hlsl
  clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl

Index: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel5.0-library -verify %s
+// WaveActiveCountBits is unavailable before ShaderModel 6.0.
+
+unsigned foo(bool b) {
+// expected-warning@+2 {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@+1 {{enclose 'WaveActiveCountBits' in a __builtin_available check to silence this warning}}
+return WaveActiveCountBits(b);
+}
+
+// expected-note@* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
Index: clang/test/SemaHLSL/AvailabilityMarkup.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/AvailabilityMarkup.hlsl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-library -verify %s
+
+// expected-note@+2 {{'fn6_0' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
+__attribute__((availability(shadermodel, introduced = 6.0)))
+unsigned fn6_0();
+
+// expected-note@+2 {{'fn5_1' has been marked as being introduced in HLSL ShaderModel 5.1 here, but the deployment target is HLSL ShaderModel 5.0}}
+__attribute__((availability(shadermodel, introduced = 5.1)))
+unsigned fn5_1();
+
+__attribute__((availability(shadermodel, introduced = 5.0)))
+unsigned fn5_0();
+
+void fn() {
+// expected-warning@+2 {{'fn6_0' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@+1 {{enclose 'fn6_0' in a __builtin_available check to silence this warning}}
+unsigned A = fn6_0();
+
+// expected-warning@+2 {{'fn5_1' is only available on HLSL ShaderModel 5.1 or newer}}
+// expected-note@+1 {{enclose 'fn5_1' in a __builtin_available check to silence this warning}}
+unsigned B = fn5_1();
+
+unsigned C = fn5_0();
+}
+
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -192,6 +192,9 @@
   case llvm::Triple::MacOSX:
 ForceAvailabilityFromVersion = VersionTuple(/*Major=*/10, /*Minor=*/13);
 break;
+  case llvm::Triple::ShaderModel:
+// Always enable availability diagnostics for shader models.
+return true;
   default:
 // New targets should always warn about availability.
 return Triple.getVendor() == llvm::Triple::Apple;
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,7 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+__attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
Index: clang/lib/Basic/Targets/DirectX.h
===
--- clang/lib/Basic/Targets/DirectX.h
+++ clang/lib/Basic/Targets/DirectX.h
@@ -55,6 +55,8 @@
 HasLegalHalfType = true;
 HasFloat16 = true;
 NoAsmVariants = true;
+PlatformMinVersion = Triple.getOSVersion();
+PlatformName = llvm::Triple::getOSTypeName(Triple.getOS());
 resetDataLayout("e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:"
 "32-f64:64-n8:16:32:64");
 TheCXXABI.set(TargetCXXABI::Microsoft);
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -908,6 +908,7 @@
  .Case("maccatalyst", "macCatalyst")
  .Case("maccatalyst_app_extension", "macCatalyst (App Extension)")
  .Case("swift", "Swift")
+ .Case("shadermodel", "HLSL ShaderModel")
  .Default(llvm::StringRef());
 }
 static llvm::StringRef getPlatformNameSourceSpelling(llvm::StringRef Platform) {
@@ -923,6 +924,7 @@
  

[PATCH] D134067: [HLSL] Enable availability attribute

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: MaskRay, erichkeane, rnk, arphaman, python3kgae, 
pow2clk, tex3d.
Herald added a reviewer: aaron.ballman.
Herald added subscribers: Anastasia, StephenFan.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

Some HLSL functionality is gated on the target shader model version.
Enabling the use of availability markup allows us to diagnose
availability issues easily in the frontend.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134067

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/test/SemaHLSL/AvailabilityMarkup.hlsl


Index: clang/test/SemaHLSL/AvailabilityMarkup.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/AvailabilityMarkup.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -finclude-default-header -triple 
dxil-pc-shadermodel5.0-library -verify %s
+// WaveActiveCountBits is unavailable before ShaderModel 6.0.
+
+unsigned foo(bool b) {
+// expected-warning@+2 {{'WaveActiveCountBits' is only available on HLSL 
ShaderModel 6.0 or newer}}
+// expected-note@+1 {{enclose 'WaveActiveCountBits' in a 
__builtin_available check to silence this warning}}
+return WaveActiveCountBits(b);
+}
+
+// expected-note@* {{'WaveActiveCountBits' has been marked as being introduced 
in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 
5.0}}
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -194,7 +194,8 @@
 break;
   default:
 // New targets should always warn about availability.
-return Triple.getVendor() == llvm::Triple::Apple;
+return Triple.getVendor() == llvm::Triple::Apple ||
+   Triple.getOS() == llvm::Triple::ShaderModel;
   }
   return DeploymentVersion >= ForceAvailabilityFromVersion ||
  DeclVersion >= ForceAvailabilityFromVersion;
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -9,6 +9,7 @@
 #ifndef _HLSL_HLSL_INTRINSICS_H_
 #define _HLSL_HLSL_INTRINSICS_H_
 
+__attribute__((availability(shadermodel, introduced = 6.0)))
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) 
uint
 WaveActiveCountBits(bool bBit);
 
Index: clang/lib/Basic/Targets/DirectX.h
===
--- clang/lib/Basic/Targets/DirectX.h
+++ clang/lib/Basic/Targets/DirectX.h
@@ -55,6 +55,8 @@
 HasLegalHalfType = true;
 HasFloat16 = true;
 NoAsmVariants = true;
+PlatformMinVersion = Triple.getOSVersion();
+PlatformName = llvm::Triple::getOSTypeName(Triple.getOS());
 resetDataLayout("e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:"
 "32-f64:64-n8:16:32:64");
 TheCXXABI.set(TargetCXXABI::Microsoft);
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -908,6 +908,7 @@
  .Case("maccatalyst", "macCatalyst")
  .Case("maccatalyst_app_extension", "macCatalyst (App Extension)")
  .Case("swift", "Swift")
+ .Case("shadermodel", "HLSL ShaderModel")
  .Default(llvm::StringRef());
 }
 static llvm::StringRef getPlatformNameSourceSpelling(llvm::StringRef Platform) 
{
@@ -923,6 +924,7 @@
  .Case("maccatalyst", "macCatalyst")
  .Case("maccatalyst_app_extension", 
"macCatalystApplicationExtension")
  .Case("zos", "z/OS")
+ .Case("shadermodel", "ShaderModel")
  .Default(Platform);
 }
 static llvm::StringRef canonicalizePlatformName(llvm::StringRef Platform) {
@@ -937,6 +939,7 @@
  .Case("watchOSApplicationExtension", "watchos_app_extension")
  .Case("macCatalyst", "maccatalyst")
  .Case("macCatalystApplicationExtension", 
"maccatalyst_app_extension")
+ .Case("ShaderModel", "shadermodel")
  .Default(Platform);
 } }];
   let HasCustomParsing = 1;


Index: clang/test/SemaHLSL/AvailabilityMarkup.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/AvailabilityMarkup.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel5.0-library -verify %s
+// WaveActiveCountBits is unavailable before ShaderModel 6.0.
+
+unsigned foo(bool b) {
+// expected-warning@+2 {{'WaveActiveCountBits' is only available on HLSL ShaderModel 6.0 or newer}}
+// expected-note@+1 {{enclose 

[PATCH] D128462: [HLSL] add -I option for dxc mode.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I understand your frustration with the regression, but let’s try to 
constructive. We all care about quality and we’re all working hard to do the 
best we can.

In D128462#3794868 , @kadircet wrote:

> My main complaint here's around **breaking** functionality for existing 
> driver-modes/toolchains, and I am asking either getting OWNERS approvals 
> going forward or being more through with the changes or changing the design 
> overall to make these less complicated.

Regressions happen. There are things we can do to avoid them (code review being 
one), but the most fool-proof way to avoid regressions is test coverage. Here 
we had a gap in test coverage of existing functionality. A new change went in, 
all the tests passed, but an important usage regression occurred. It is 
unfortunate, but it happened.

Getting owner approval before commit has never been required by our developer 
policy. In fact, the wording of the policy puts the onus on the owner for 
post-commit review. In this case we had a fairly innocuous seeming change by a 
relatively new contributor and I reviewed and approved it because it seemed 
fairly innocuous. This said, hopefully the recent changes in Clang code 
ownership will help here too.

This change was tested, and introduces a new test in accordance with our 
community standards and policy.

I don’t think it is fair to say this change contributed to a complicated design 
other than that llvm’s libOption is pretty complicated and has lots of 
difficult to see edge cases, and the tools that share option parsing logic 
often have disparate and duplicated logic for handing those options which can 
cause a variety of issues.

In terms of your other complaints around bug handling; I also understand 
frustration there. Unfortunately, clangd has a separate bug reporting and 
tracking process from the rest of the LLVM and Clang code it depends on. I’m 
sure it is frustrating to get bug reports that relate to code you didn’t write 
or design, but I think that is a reality of the process and complexity of the 
software that we have today. It is incredibly common in LLVM for bugs to get 
routed to the wrong area and need to be rerouted. Sometimes this is because 
users reporting issues don’t understand the architecture of the code base well 
enough to route to the right place, other times it is because the cultural 
boundaries of responsibility in LLVM aren’t well codified.

In order to support the incremental implementation process that LLVM has always 
used we need a way to be able to add driver and language modes incrementally. 
If there’s a gap in the Tooling testing or architecture we can change to make 
that better, let’s please discuss that, rather than trying to apply additional 
hurdles to a review process because a regression was introduced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128462/new/

https://reviews.llvm.org/D128462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128462: [HLSL] add -I option for dxc mode.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I think we had no expectation that DXC-mode would be supported by the tooling 
APIs at this point.

For context, DXC is the HLSL compiler that is based on clang-3.7. The DXC mode 
provides interface compatibility with it. DXC doesn't work with any of the 
clang tooling infrastructure, and clang's HLSL support isn't complete enough 
that people are going to be relying on tooling. I expect that we'll make larger 
investments in the Tooling library as we get further along in the HLSL 
implementation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128462/new/

https://reviews.llvm.org/D128462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits