[HLSL] Set main as default entry.

[python3kgae]

Summary

When there's no -E option, use main as entry function.

[Anastasia]

clang/lib/Driver/ToolChains/HLSL.cpp

182 ↗

(On Diff #426342)

-> If no set entry or If entry is not set explicitly

[python3kgae]

Update comments.

[Anastasia]

clang/test/CodeGenHLSL/entry_default.hlsl

2

Would it make sense to test the opposite i.e. if -E is passed in the command line main is not an entry point?

[python3kgae]

Add test for main is not entry.

[MaskRay]

clang/lib/Driver/ToolChains/HLSL.cpp

194 ↗

(On Diff #432429)

Instead of letting Driver pass a default OPT_hlsl_entrypoint to CC1, you can let CC1 have a default OPT_hlsl_entrypoint.

[python3kgae]

Use default value of MarshallingInfoString.

[Anastasia]

From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with -### checking for the options to be set for the frontend invocation...

[python3kgae]

In D124753#3545779, @Anastasia wrote:

From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with -### checking for the options to be set for the frontend invocation...

There's already a driver test with '-###' in https://reviews.llvm.org/D124751#change-af6Z62NjlfGb

[python3kgae]

Add new line at end of file.

[MaskRay]

clang/test/CodeGenHLSL/entry_default.hlsl

7

space between : and the value.

10

[[MAIN_ATTR]] and the value should be on the same line.

[python3kgae]

Cleanup test.

[Anastasia]

In D124753#3546608, @python3kgae wrote:

In D124753#3545779, @Anastasia wrote:

From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with -### checking for the options to be set for the frontend invocation...

There's already a driver test with '-###' in https://reviews.llvm.org/D124751#change-af6Z62NjlfGb

This test doesn't seem to correspond to the change being added as you are changing the command-line flags. You don't actually add/generate any attributes in this patch.

[python3kgae]

In D124753#3550337, @Anastasia wrote:

In D124753#3546608, @python3kgae wrote:

In D124753#3545779, @Anastasia wrote:

From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with -### checking for the options to be set for the frontend invocation...

There's already a driver test with '-###' in https://reviews.llvm.org/D124751#change-af6Z62NjlfGb

This test doesn't seem to correspond to the change being added as you are changing the command-line flags. You don't actually add/generate any attributes in this patch.

Sorry to make things confusing. I should not split default value for -E option as a separate PR :(

There's dxc_E.hlsl (https://reviews.llvm.org/D124751#change-af6Z62NjlfGb) in https://reviews.llvm.org/D124751 where the -E option is added.
dxc_E.hlsl will test -E option translated into -hlsl-entry for cc1.

There was a test for codeGen of -E option in https://reviews.llvm.org/D124752, but I removed it because it is to almost the same as https://reviews.llvm.org/D124752#change-w4NWvaT68Dhk which test codeGen for ShaderAttr.

And the entry_default.hlsl in current PR test codeGen for the default main and -E option.

I can add a separate codeGen test for -E option if that's what you thought is missing.

[Anastasia]

In D124753#3550670, @python3kgae wrote:

In D124753#3550337, @Anastasia wrote:

In D124753#3546608, @python3kgae wrote:

In D124753#3545779, @Anastasia wrote:

From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with -### checking for the options to be set for the frontend invocation...

There's already a driver test with '-###' in https://reviews.llvm.org/D124751#change-af6Z62NjlfGb

This test doesn't seem to correspond to the change being added as you are changing the command-line flags. You don't actually add/generate any attributes in this patch.

Sorry to make things confusing. I should not split default value for -E option as a separate PR :(

There's dxc_E.hlsl (https://reviews.llvm.org/D124751#change-af6Z62NjlfGb) in https://reviews.llvm.org/D124751 where the -E option is added.
dxc_E.hlsl will test -E option translated into -hlsl-entry for cc1.

There was a test for codeGen of -E option in https://reviews.llvm.org/D124752, but I removed it because it is to almost the same as https://reviews.llvm.org/D124752#change-w4NWvaT68Dhk which test codeGen for ShaderAttr.

And the entry_default.hlsl in current PR test codeGen for the default main and -E option.

I can add a separate codeGen test for -E option if that's what you thought is missing.

Yes, in this patch you are just changing the marchaling flag to -cc1 right? It would be better if you test just exactly that and then in dependent patches you can test what is related to other changes. It will make things less confusing then. ;)

[bogner]

clang/test/CodeGenHLSL/entry_default.hlsl

15

typo, should be CHECK-SAME

[python3kgae]

Fix typo in test.

[python3kgae]

clang/test/CodeGenHLSL/entry_default.hlsl

15

Good catch.
It is strange that the test will pass even with the typo.

[python3kgae]

Rebase and add unit-test.

[python3kgae]

Update to work around arc error when land.

clang/include/clang/Driver/Options.td

 def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM passes at all."
            "Same as -S + -emit-llvm + -disable-llvm-passes.">;
 def fcgl : DXCFlag<"fcgl">, Alias<emit_pristine_llvm>;
 
 def hlsl_entrypoint : Option<["--", "/", "-"], "E", KIND_JOINED_OR_SEPARATE>,
                       Group<dxc_Group>,
                       Flags<[DXCOption, CC1Option, NoXarchOption]>,
-                      MarshallingInfoString<TargetOpts<"HLSLEntry">>,
+                      MarshallingInfoString<TargetOpts<"HLSLEntry">, "\"main\"">,
                       HelpText<"Entry point name">;

clang/test/CodeGenHLSL/entry_default.hlsl

+// RUN: %clang --driver-mode=dxc -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s
+// RUN: %clang --driver-mode=dxc -Efoo -Tcs_6_1 -fcgl -Fo - %s | FileCheck %s --check-prefix=NOTMAIN
+
+// Make sure main is default entry.
+// Make sure not mangle entry.
+// CHECK: define void @main() [[MAIN_ATTR:#[0-9]]]
+// CHECK: define void @_Z3foov() [[FOO_ATTR:#[0-9]]]
+// Make sure only main has dx.shader attribute.
+// CHECK: [[MAIN_ATTR]]
+// CHECK-SAME: "dx.shader"="compute"
+// CHECK-SAME: }
+// CHECK: [[FOO_ATTR]]
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAME: }
+[numthreads(1, 1, 1)] void main() {
+
+}
+
+// NOTMAIN: define void @main() [[MAIN_ATTR:#[0-9]]]
+// NOTMAIN: define void @foo() [[FOO_ATTR:#[0-9]]]
+// Make sure only foo has dx.shader attribute.
+// NOTMAIN: [[MAIN_ATTR]]
+// NOTMAIN-NOT:"dx.shader"="compute"
+// NOTMAIN-SAME:}
+// NOTMAIN: [[FOO_ATTR]]
+// NOTMAIN-SAME: "dx.shader"="compute"
+[numthreads(1, 1, 1)] void foo() {
+
+}