[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Greg Roth via Phabricator via cfe-commits]

pow2clk added a comment.

Looks to me that all the comments were responded to. I see nothing else worth 
commenting on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 428209.
python3kgae added a comment.

Rebase to main for fix merge conflict.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl

Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for Compute shader entry}}
+void foo() {
+
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9867,6 +9867,55 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+if (!NewFD->isInvalidDecl() &&
+// Skip operator overload which not identifier.
+Name.isIdentifier() &&
+NewFD->getName() == TargetInfo.getTargetOpts().HLSLEntry &&
+// Make sure it is in translation-unit scope.
+S->getDepth() == 0) {
+  CheckHLSLEntryPoint(NewFD);
+  if (!NewFD->isInvalidDecl()) {
+auto TripeShaderType = TargetInfo.getTriple().getEnvironment();
+AttributeCommonInfo AL(NewFD->getBeginLoc());
+HLSLShaderAttr::ShaderType ShaderType =
+HLSLShaderAttr::ShaderType::Callable;
+switch (TripeShaderType) {
+default:
+  break;
+case llvm::Triple::EnvironmentType::Compute:
+  ShaderType = HLSLShaderAttr::ShaderType::Compute;
+  break;
+case llvm::Triple::EnvironmentType::Vertex:
+  ShaderType = HLSLShaderAttr::ShaderType::Vertex;
+  break;
+case llvm::Triple::EnvironmentType::Hull:
+  ShaderType = HLSLShaderAttr::ShaderType::Hull;
+  break;
+case llvm::Triple::EnvironmentType::Domain:
+  ShaderType = HLSLShaderAttr::ShaderType::Domain;
+  break;
+case llvm::Triple::EnvironmentType::Geometry:
+  ShaderType = HLSLShaderAttr::ShaderType::Geometry;
+  break;
+case llvm::Triple::EnvironmentType::Pixel:
+  ShaderType = HLSLShaderAttr::ShaderType::Pixel;
+  break;
+case llvm::Triple::EnvironmentType::Mesh:
+  ShaderType = HLSLShaderAttr::ShaderType::Mesh;
+  break;
+case llvm::Triple::EnvironmentType::Amplification:
+  ShaderType = HLSLShaderAttr::ShaderType::Amplification;
+  break;
+}
+// To share code with HLSLShaderAttr, add HLSLShaderAttr to entry function.
+if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
+  NewFD->addAttr(Attr);
+  }
+}
+  }
+
   if (!getLangOpts().CPlusPlus) {
 // Perform semantic checking on the function declaration.
 if (!NewFD->isInvalidDecl() && NewFD->isMain())
@@ -11687,6 +11736,21 @@
   }
 }
 
+void Sema::CheckHLSLEntryPoint(FunctionDecl *FD) {
+  auto &TargetInfo = getASTContext().getTargetInfo();
+  switch (TargetInfo.getTriple().getEnvironment()) {
+  default:
+// FIXME: check all shader profiles.
+break;
+  case llvm::Triple::EnvironmentType::Compute:
+if (!FD->hasAttr()) {
+  Diag(FD->getLocation(), diag::err_hlsl_missing_numthreads) << "Compute";
+  FD->setInvalidDecl();
+}
+break;
+  }
+}
+
 bool Sema::CheckForConstantInitializer(Expr *Init, QualType DclT) {
   // FIXME: Need strict checking.  In C89, we need to check for
   // any assignment, increment, decrement, function-calls, or
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Is there a specification or reference implementation stating that -E is used?

> Option<["--", "/", "-"], "E",

Do you need the prefix `--`? You may define something like `CLFlag`. I have 
missed previous patches, but if `/` options are not necessary, removing them 
will be the best to avoid collision with filenames starting with `/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D124751#3513283 , @MaskRay wrote:

> Is there a specification or reference implementation stating that -E is used?
>
>> Option<["--", "/", "-"], "E",
>
> Do you need the prefix `--`? You may define something like `CLFlag`. I have 
> missed previous patches, but if `/` options are not necessary, removing them 
> will be the best to avoid collision with filenames starting with `/`.

Unfortunately, '/' is necessary. dxc allow both '-' and '/' like 'CLFlag'.
Remove '/' means existing customers may need to change their command line to 
compile hlsl.
And add '--' feels not hurt anyone, that's why I choose to add '--' to 
work-around the conflict.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D124751#3513513 , @python3kgae 
wrote:

> In D124751#3513283 , @MaskRay wrote:
>
>> Is there a specification or reference implementation stating that -E is used?
>>
>>> Option<["--", "/", "-"], "E",
>>
>> Do you need the prefix `--`? You may define something like `CLFlag`. I have 
>> missed previous patches, but if `/` options are not necessary, removing them 
>> will be the best to avoid collision with filenames starting with `/`.
>
> Unfortunately, '/' is necessary. dxc allow both '-' and '/' like 'CLFlag'.
> Remove '/' means existing customers may need to change their command line to 
> compile hlsl.
> And add '--' feels not hurt anyone, that's why I choose to add '--' to 
> work-around the conflict.

OK. I guess you have a pre-existing implementation using `-E` and the 
upstreaming implementation wants to be compatible.
I wonder why the new tool needs to reuse 
`clang/include/clang/Driver/Options.td`. ISTM it would be cleaner to use a new 
.td file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D124751#3529652 , @MaskRay wrote:

> In D124751#3513513 , @python3kgae 
> wrote:
>
>> In D124751#3513283 , @MaskRay 
>> wrote:
>>
>>> Is there a specification or reference implementation stating that -E is 
>>> used?
>>>
 Option<["--", "/", "-"], "E",
>>>
>>> Do you need the prefix `--`? You may define something like `CLFlag`. I have 
>>> missed previous patches, but if `/` options are not necessary, removing 
>>> them will be the best to avoid collision with filenames starting with `/`.
>>
>> Unfortunately, '/' is necessary. dxc allow both '-' and '/' like 'CLFlag'.
>> Remove '/' means existing customers may need to change their command line to 
>> compile hlsl.
>> And add '--' feels not hurt anyone, that's why I choose to add '--' to 
>> work-around the conflict.
>
> OK. I guess you have a pre-existing implementation using `-E` and the 
> upstreaming implementation wants to be compatible.
> I wonder why the new tool needs to reuse 
> `clang/include/clang/Driver/Options.td`. ISTM it would be cleaner to use a 
> new .td file.

That is a good idea.
Let me try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9887
+  break;
+case llvm::Triple::EnvironmentType::Compute:
+  ShaderType = HLSLShaderAttr::ShaderType::Compute;

If the `HLSLShaderAttr::ShaderType` enum is properly ordered you should be able 
to convert the integer value:

```
HLSLShaderAttr::ShaderType StageInteger = 
(HLSLShaderAttr::ShaderType)TI.getTriple().getEnvironment() -
(uint32_t)llvm::Triple::Pixel;
```

Then this code doesn't need to be updated if a new shader type is added.



Comment at: clang/test/SemaHLSL/entry.hlsl:16
+}
\ No newline at end of file


Please fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D124751#3529652 , @MaskRay wrote:

> In D124751#3513513 , @python3kgae 
> wrote:
>
>> In D124751#3513283 , @MaskRay 
>> wrote:
>>
>>> Is there a specification or reference implementation stating that -E is 
>>> used?
>>>
 Option<["--", "/", "-"], "E",
>>>
>>> Do you need the prefix `--`? You may define something like `CLFlag`. I have 
>>> missed previous patches, but if `/` options are not necessary, removing 
>>> them will be the best to avoid collision with filenames starting with `/`.
>>
>> Unfortunately, '/' is necessary. dxc allow both '-' and '/' like 'CLFlag'.
>> Remove '/' means existing customers may need to change their command line to 
>> compile hlsl.
>> And add '--' feels not hurt anyone, that's why I choose to add '--' to 
>> work-around the conflict.
>
> OK. I guess you have a pre-existing implementation using `-E` and the 
> upstreaming implementation wants to be compatible.
> I wonder why the new tool needs to reuse 
> `clang/include/clang/Driver/Options.td`. ISTM it would be cleaner to use a 
> new .td file.

I tried to create a DxcOptions.td which only have dxc options and core options. 
Then create OptTable from DxcOptions.td and use the Dxc OptTable when ParseArgs 
in Driver::ParseArgStrings. It works fine.

But after ParseArgs, there're lots of code in Driver::BuildCompilation use 
options::OPT_* enum which depends on Options.td.  Cannot find a clean solution 
to switch to DxcOptions while share code about core options in 
Driver::BuildCompilation. Do you have any suggestions about how to update 
options::OPT_* enum after switch to new .td file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 432355.
python3kgae added a comment.
Herald added a reviewer: aaron.ballman.

Add library to HLSLShaderAttr to help convert ShaderType.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -53,10 +53,11 @@
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
-
+// expected-warning@+1 {{'shader' attribute argument not supported: library}}
+[shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 [shader("compute")]
 int entry() {
   return 1;
@@ -64,11 +65,11 @@
 
 // Because these two attributes match, they should both appear in the AST
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for Compute shader entry}}
+void foo() {
+
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6947,7 +6947,11 @@
 return;
 
   HLSLShaderAttr::ShaderType ShaderType;
-  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType)) {
+  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType) ||
+  // Library is added to help convert HLSLShaderAttr::ShaderType to
+  // llvm::Triple::EnviromentType. It is not a legal
+  // HLSLShaderAttr::ShaderType.
+  ShaderType == HLSLShaderAttr::Library) {
 S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
 << AL << Str << ArgLoc;
 return;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9867,6 +9867,28 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+if (!NewFD->isInvalidDecl() &&
+// Skip operator overload which not identifier.
+Name.isIdentifier() &&
+NewFD->getName() == TargetInfo.getTargetOpts().HLSLEntry &&
+// Make sure it is in translation-unit scope.
+S->getDepth() == 0) {
+  CheckHLSLEntryPoint(NewFD);
+  if (!NewFD->isInvalidDecl()) {
+auto TripleShaderType = TargetInfo.getTriple().getEnvironment();
+AttributeCommonInfo AL(NewFD->getBeginLoc());
+HLSLShaderAttr::ShaderType ShaderType = (HLSLShaderAttr::ShaderType)(
+TripleShaderType - (uint32_t)llvm::Triple::Pixel);
+// To share code with HLSLShaderAttr, add HLSLShaderAttr to entry
+// function.
+if (HLSLShaderAttr *Attr = mergeHLSLShaderAttr(NewFD, AL, ShaderType))
+  NewFD->addAttr(Attr);
+  }
+}
+  }
+
   if (!getLangOpts().CPlusPlus) {
 // Perform semantic checking on the function declaration.
 if (!NewFD->isInvalidDecl() && NewFD->isMa

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi, nice to see this getting in. Comments inline!




Comment at: clang/lib/Sema/SemaDecl.cpp:11720
+if (!FD->hasAttr()) {
+  Diag(FD->getLocation(), diag::err_hlsl_missing_numthreads) << "Compute";
+  FD->setInvalidDecl();

You can use `getEnvironmentTypeName` to get the name here.



Comment at: clang/lib/Sema/SemaDecl.cpp:11721
+  Diag(FD->getLocation(), diag::err_hlsl_missing_numthreads) << "Compute";
+  FD->setInvalidDecl();
+}

Since this is both checking and invalidating a decl, perhaps it should return a 
bool (maybe some asserts in the default case?) and be renamed 
`CheckAndInvalidateHLSLEntryPoint`?



Comment at: clang/test/SemaHLSL/entry.hlsl:2
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo 
-DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo  -o - 
%s  -verify
+

Can you also add a check that rejects `-Efoo` with an error message when the 
language is not `hlsl`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/SemaHLSL/entry.hlsl:2
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo 
-DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo  -o - 
%s  -verify
+

bruno wrote:
> Can you also add a check that rejects `-Efoo` with an error message when the 
> language is not `hlsl`?
For most triples, the cc1 -E means stopping after preprocessing. If dxil -E is 
different, better to use a different option to avoid confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 432563.
python3kgae added a comment.

Add hlsl-entry for cc1 option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Driver/dxc_E.hlsl
  clang/test/Driver/hlsl-entry.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -53,10 +53,11 @@
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
-
+// expected-warning@+1 {{'shader' attribute argument not supported: library}}
+[shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 [shader("compute")]
 int entry() {
   return 1;
@@ -64,11 +65,11 @@
 
 // Because these two attributes match, they should both appear in the AST
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for compute shader entry}}
+void foo() {
+
+}
Index: clang/test/Driver/hlsl-entry.cpp
===
--- /dev/null
+++ clang/test/Driver/hlsl-entry.cpp
@@ -0,0 +1,3 @@
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ -hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+
+// NOTHLSL:invalid argument '-hlsl-entry' not allowed with 'C++'
Index: clang/test/Driver/dxc_E.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_E.hlsl
@@ -0,0 +1,4 @@
+// RUN: %clang_dxc -Efoo foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure E option flag which translated into "-hlsl-entry".
+// CHECK:"-hlsl-entry" "foo"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6947,7 +6947,11 @@
 return;
 
   HLSLShaderAttr::ShaderType ShaderType;
-  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType)) {
+  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType) ||
+  // Library is added to help convert HLSLShaderAttr::ShaderType to
+  // llvm::Triple::EnviromentType. It is not a legal
+  // HLSLShaderAttr::ShaderType.
+  ShaderType == HLSLShaderAttr::Library) {
 S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
 << AL << Str << ArgLoc;
 return;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9867,6 +9867,28 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+if (!NewFD->isInvalidDecl() &&
+// Skip operator overload which not identifier.
+Name.isIdentifier() &&
+NewFD->getName() == TargetInfo.getTargetOpts().HLSLEntry &&
+  

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked 3 inline comments as done.
python3kgae added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:11721
+  Diag(FD->getLocation(), diag::err_hlsl_missing_numthreads) << "Compute";
+  FD->setInvalidDecl();
+}

bruno wrote:
> Since this is both checking and invalidating a decl, perhaps it should return 
> a bool (maybe some asserts in the default case?) and be renamed 
> `CheckAndInvalidateHLSLEntryPoint`?
The name is following CheckMain and CheckMSVCRTEntryPoint which also checking 
and invalidating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 434615.
python3kgae added a comment.

Fix test fail after rebased on llvm/main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Driver/dxc_E.hlsl
  clang/test/Driver/hlsl-entry.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -53,10 +53,11 @@
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
-
+// expected-warning@+1 {{'shader' attribute argument not supported: library}}
+[shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 [shader("compute")]
 int entry() {
   return 1;
@@ -64,11 +65,11 @@
 
 // Because these two attributes match, they should both appear in the AST
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for compute shader entry}}
+void foo() {
+
+}
Index: clang/test/Driver/hlsl-entry.cpp
===
--- /dev/null
+++ clang/test/Driver/hlsl-entry.cpp
@@ -0,0 +1,3 @@
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ -hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+
+// NOTHLSL:invalid argument '-hlsl-entry' not allowed with 'C++'
Index: clang/test/Driver/dxc_E.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_E.hlsl
@@ -0,0 +1,4 @@
+// RUN: %clang_dxc -Efoo -Tcs_6_7 -### %s 2>&1 | FileCheck %s
+
+// Make sure E option flag which translated into "-hlsl-entry".
+// CHECK:"-hlsl-entry" "foo"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6951,7 +6951,11 @@
 return;
 
   HLSLShaderAttr::ShaderType ShaderType;
-  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType)) {
+  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType) ||
+  // Library is added to help convert HLSLShaderAttr::ShaderType to
+  // llvm::Triple::EnviromentType. It is not a legal
+  // HLSLShaderAttr::ShaderType.
+  ShaderType == HLSLShaderAttr::Library) {
 S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
 << AL << Str << ArgLoc;
 return;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9835,6 +9835,28 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+if (!NewFD->isInvalidDecl() &&
+// Skip operator overload which not identifier.
+Name.isIdentifier() &&
+NewFD->getName() == TargetInfo.getTargetOpts().HLSLE

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

Gentle Ping.

All the previous issues are fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[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 very nitpick-y comment, otherwise LGTM.




Comment at: clang/lib/Sema/SemaDecl.cpp:9841
+if (!NewFD->isInvalidDecl() &&
+// Skip operator overload which not identifier.
+Name.isIdentifier() &&

nit: Comments in the middle of the if make it a bit hard to read, and is not a 
common style in LLVM code. Please put the comments above the if describing the 
condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 450115.
python3kgae added a comment.

Cleanup comments and fix test after rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Driver/dxc_E.hlsl
  clang/test/Driver/hlsl-entry.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -53,10 +53,11 @@
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
-
+// expected-warning@+1 {{'shader' attribute argument not supported: library}}
+[shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 [shader("compute")]
 int entry() {
   return 1;
@@ -64,11 +65,11 @@
 
 // Because these two attributes match, they should both appear in the AST
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for compute shader entry}}
+void foo() {
+
+}
Index: clang/test/Driver/hlsl-entry.cpp
===
--- /dev/null
+++ clang/test/Driver/hlsl-entry.cpp
@@ -0,0 +1,3 @@
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ -hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+
+// NOTHLSL:invalid argument '-hlsl-entry' not allowed with 'C++'
Index: clang/test/Driver/dxc_E.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_E.hlsl
@@ -0,0 +1,4 @@
+// RUN: %clang_dxc -Efoo -Tlib_6_7 foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure E option flag which translated into "-hlsl-entry".
+// CHECK:"-hlsl-entry" "foo"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6904,7 +6904,11 @@
 return;
 
   HLSLShaderAttr::ShaderType ShaderType;
-  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType)) {
+  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType) ||
+  // Library is added to help convert HLSLShaderAttr::ShaderType to
+  // llvm::Triple::EnviromentType. It is not a legal
+  // HLSLShaderAttr::ShaderType.
+  ShaderType == HLSLShaderAttr::Library) {
 S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
 << AL << Str << ArgLoc;
 return;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10005,6 +10005,27 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+// Skip operator overload which not identifier.
+// Also make sure NewFD is in translation-unit scope.
+if (!NewFD->isInvalidDecl() && Name.isIdentifier() &&
+

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a27a2f89f83: [HLSL] Support -E option for HLSL. (authored 
by python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Driver/dxc_E.hlsl
  clang/test/Driver/hlsl-entry.cpp
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/prohibit_pointer.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -53,10 +53,11 @@
 [shader(1)]
 // expected-warning@+1 {{'shader' attribute argument not supported: cs}}
 [shader("cs")]
-
+// expected-warning@+1 {{'shader' attribute argument not supported: library}}
+[shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 [shader("compute")]
 int entry() {
   return 1;
@@ -64,11 +65,11 @@
 
 // Because these two attributes match, they should both appear in the AST
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
 [shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/prohibit_pointer.hlsl
===
--- clang/test/SemaHLSL/prohibit_pointer.hlsl
+++ clang/test/SemaHLSL/prohibit_pointer.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -o - -fsyntax-only %s -verify
 
 // expected-error@+1 {{pointers are unsupported in HLSL}}
 typedef int (*fn_int)(int);
Index: clang/test/SemaHLSL/entry.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s  -verify
+
+
+// Make sure add HLSLShaderAttr along with HLSLNumThreadsAttr.
+// CHECK:HLSLNumThreadsAttr 0x{{.*}}  1 1 1
+// CHECK:HLSLShaderAttr 0x{{.*}}  Compute
+
+#ifdef WITH_NUM_THREADS
+[numthreads(1,1,1)]
+#endif
+// expected-error@+1 {{missing numthreads attribute for compute shader entry}}
+void foo() {
+
+}
Index: clang/test/Driver/hlsl-entry.cpp
===
--- /dev/null
+++ clang/test/Driver/hlsl-entry.cpp
@@ -0,0 +1,3 @@
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ -hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+
+// NOTHLSL:invalid argument '-hlsl-entry' not allowed with 'C++'
Index: clang/test/Driver/dxc_E.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_E.hlsl
@@ -0,0 +1,4 @@
+// RUN: %clang_dxc -Efoo -Tlib_6_7 foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure E option flag which translated into "-hlsl-entry".
+// CHECK:"-hlsl-entry" "foo"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6904,7 +6904,11 @@
 return;
 
   HLSLShaderAttr::ShaderType ShaderType;
-  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType)) {
+  if (!HLSLShaderAttr::ConvertStrToShaderType(Str, ShaderType) ||
+  // Library is added to help convert HLSLShaderAttr::ShaderType to
+  // llvm::Triple::EnviromentType. It is not a legal
+  // HLSLShaderAttr::ShaderType.
+  ShaderType == HLSLShaderAttr::Library) {
 S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
 << AL << Str << ArgLoc;
 return;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10005,6 +10005,27 @@
 }
   }
 
+  if (getLangOpts().HLSL) {
+auto &TargetInfo = getASTContext().getTargetInfo();
+// Skip operator overload which not identifier.
+// Also make sure NewFD is in translation-unit scope.
+if (!NewFD

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/hlsl-entry.cpp:1
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ 
-hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+

Note, test/Driver deliberately discourages `%clang_cc1` and `%clang -cc1` 
tests. The tests test other components of Clang (not Driver) and should be 
moved to appropriate components.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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

[PATCH] D124751: [HLSL] Support -E option for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added inline comments.



Comment at: clang/test/Driver/hlsl-entry.cpp:1
+// RUN:not %clang -cc1 -triple dxil-pc-shadermodel6.3-compute -x c++ 
-hlsl-entry foo  %s  2>&1 | FileCheck %s --check-prefix=NOTHLSL
+

MaskRay wrote:
> Note, test/Driver deliberately discourages `%clang_cc1` and `%clang -cc1` 
> tests. The tests test other components of Clang (not Driver) and should be 
> moved to appropriate components.
Moved to Frontend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124751

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