Skip to content

[HLSL][clang][Driver] Fix error when using the option -fcgl in --driver-mode=dxc. #97001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

python3kgae
Copy link
Contributor

@python3kgae python3kgae commented Jun 28, 2024

When -fcgl is set in --driver-mode=dxc, both -S and -emit-llvm are currently enabled.

This results in the following error:

  error: '-S' action ignored; '-emit-llvm' action specified previously.

This change fixes the issue by not rendering -S in RenderHLSLOptions.

Additionally, a test has been added to ensure that enabling -fcgl does not trigger any diagnostics

Fixes #97296

…er-mode=dxc.

When -fcgl is set in --driver-mode=dxc, both -S and -emit-llvm are currently enabled.

This results in the following error:

  error: '-S' action ignored; '-emit-llvm' action specified previously.

This change fixes the issue by not rendering -S in RenderHLSLOptions.

Additionally, a test has been added to ensure that enabling -fcgl does not trigger any diagnostics
@python3kgae python3kgae added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' HLSL HLSL Language Support labels Jun 28, 2024
@python3kgae python3kgae requested a review from MaskRay June 28, 2024 04:19
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jun 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang-driver

Author: Xiang Li (python3kgae)

Changes

When -fcgl is set in --driver-mode=dxc, both -S and -emit-llvm are currently enabled.

This results in the following error:

  error: '-S' action ignored; '-emit-llvm' action specified previously.

This change fixes the issue by not rendering -S in RenderHLSLOptions.

Additionally, a test has been added to ensure that enabling -fcgl does not trigger any diagnostics


Full diff: https://github.com/llvm/llvm-project/pull/97001.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (-1)
  • (modified) clang/test/Driver/dxc_fcgl.hlsl (+5-2)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c0f6bc0c2e45a..9c959617fba98 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3666,7 +3666,6 @@ static void RenderHLSLOptions(const ArgList &Args, ArgStringList &CmdArgs,
   const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
                                          options::OPT_D,
                                          options::OPT_I,
-                                         options::OPT_S,
                                          options::OPT_O,
                                          options::OPT_emit_llvm,
                                          options::OPT_emit_obj,
diff --git a/clang/test/Driver/dxc_fcgl.hlsl b/clang/test/Driver/dxc_fcgl.hlsl
index cfbf2503ddaae..fe65124c197bc 100644
--- a/clang/test/Driver/dxc_fcgl.hlsl
+++ b/clang/test/Driver/dxc_fcgl.hlsl
@@ -1,6 +1,9 @@
 // RUN: not %clang_dxc -fcgl -T lib_6_7 foo.hlsl -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -fcgl -T lib_6_7 %s -Xclang -verify
 
 // Make sure fcgl option flag which translated into "-emit-llvm" "-disable-llvm-passes".
-// CHECK:"-S"
-// CHECK-SAME:"-emit-llvm" "-disable-llvm-passes"
+// CHECK: "-emit-llvm"
+// CHECK-SAME: "-disable-llvm-passes"
 
+// Make sure fcgl option not generate any diagnostics.
+// expected-no-diagnostics

@python3kgae python3kgae requested a review from ChuanqiXu9 June 28, 2024 04:20
Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to revisit the whole ForwardedArguments thing in general, as there are a few funny things going on looking at the -cc1 options coming out of dxc - for example, before this change -S actually shows up twice sometimes.

In any case, this at least seems to make it less broken for now, so LGTM.

@python3kgae python3kgae merged commit 75ec24e into llvm:main Jul 1, 2024
11 checks passed
@python3kgae python3kgae deleted the fcgl branch July 1, 2024 17:44
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…er-mode=dxc. (llvm#97001)

When -fcgl is set in --driver-mode=dxc, both -S and -emit-llvm are
currently enabled.

This results in the following error:
```
  error: '-S' action ignored; '-emit-llvm' action specified previously.
```
This change fixes the issue by not rendering -S in RenderHLSLOptions.

Additionally, a test has been added to ensure that enabling -fcgl does
not trigger any diagnostics

Fixes llvm#97296
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
…er-mode=dxc. (llvm#97001)

When -fcgl is set in --driver-mode=dxc, both -S and -emit-llvm are
currently enabled.

This results in the following error:
```
  error: '-S' action ignored; '-emit-llvm' action specified previously.
```
This change fixes the issue by not rendering -S in RenderHLSLOptions.

Additionally, a test has been added to ensure that enabling -fcgl does
not trigger any diagnostics

Fixes llvm#97296
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[HLSL] -fcgl option failed with error.
4 participants