[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

1 ↗

(On Diff #432023)

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

6 ↗

(On Diff #433006)

space between : and the value.

9 ↗

(On Diff #433006)

[[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

14 ↗

(On Diff #433301)

typo, should be CHECK-SAME

[python3kgae]

Fix typo in test.

[python3kgae]

clang/test/CodeGenHLSL/entry_default.hlsl

14 ↗

(On Diff #433301)

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

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 enable_16bit_types : DXCFlag<"enable-16bit-types">, Alias<fnative_half_type>,
   HelpText<"Enable 16-bit types and disable min precision types."
            "Available in HLSL 2018 and shader model 6.2.">;
 def hlsl_entrypoint : Option<["-"], "hlsl-entry", KIND_SEPARATE>,
                       Group<dxc_Group>,
                       Flags<[CC1Option]>,
-                      MarshallingInfoString<TargetOpts<"HLSLEntry">>,
+                      MarshallingInfoString<TargetOpts<"HLSLEntry">, "\"main\"">,
                       HelpText<"Entry point name for hlsl">;
 def dxc_entrypoint : Option<["--", "/", "-"], "E", KIND_JOINED_OR_SEPARATE>,
                      Group<dxc_Group>,
                      Flags<[DXCOption, NoXarchOption]>,
                      HelpText<"Entry point name">;

clang/unittests/Driver/ToolChainTest.cpp

 //===- unittests/Driver/ToolChainTest.cpp --- ToolChain tests -------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Unit tests for ToolChains.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/ToolChain.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include <memory>
TEST(DxcModeTest, ValidatorVersionValidation) {
   EXPECT_STREQ(
       DiagConsumer->Errors.back().c_str(),
       "invalid validator version : foo\nFormat of validator version is "
       "\"<major>.<minor>\" (ex:\"1.4\").");
   Diags.Clear();
   DiagConsumer->clear();
 }
 
+TEST(DxcModeTest, DefaultEntry) {
+  IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
+      new llvm::vfs::InMemoryFileSystem);
+
+  InMemoryFileSystem->addFile("foo.hlsl", 0,
+                              llvm::MemoryBuffer::getMemBuffer("\n"));
+
+  const char *Args[] = {"clang", "--driver-mode=dxc", "-Tcs_6_7", "foo.hlsl"};
+
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
+      CompilerInstance::createDiagnostics(new DiagnosticOptions());
+
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
+  std::unique_ptr<CompilerInvocation> CInvok =
+      createInvocation(Args, std::move(CIOpts));
+  EXPECT_TRUE(CInvok);
+  // Make sure default entry is "main".
+  EXPECT_STREQ(CInvok->getTargetOpts().HLSLEntry.c_str(), "main");
+
+  const char *EntryArgs[] = {"clang", "--driver-mode=dxc", "-Ebar", "-Tcs_6_7", "foo.hlsl"};
+  CInvok = createInvocation(EntryArgs, std::move(CIOpts));
+  EXPECT_TRUE(CInvok);
+  // Make sure "-E" will set entry.
+  EXPECT_STREQ(CInvok->getTargetOpts().HLSLEntry.c_str(), "bar");
+}
+
 TEST(ToolChainTest, Toolsets) {
   // Ignore this test on Windows hosts.
   llvm::Triple Host(llvm::sys::getProcessTriple());
   if (Host.isOSWindows())
     GTEST_SKIP();
 
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());