[HLSL] Simplify test

[spall]

Simplify test that fcgl flag is expanded to the right flags.

[llvmbot]

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

@llvm/pr-subscribers-clang

Author: Sarah Spall (spall)

Changes

Simplify test that fcgl flag is expanded to the right flags.

---

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

1 Files Affected:

• (modified) clang/test/Driver/dxc_fcgl.hlsl (+1-2)

diff --git a/clang/test/Driver/dxc_fcgl.hlsl b/clang/test/Driver/dxc_fcgl.hlsl
index fe65124c197bc..d3de2df1882e4 100644
--- a/clang/test/Driver/dxc_fcgl.hlsl
+++ b/clang/test/Driver/dxc_fcgl.hlsl
@@ -1,5 +1,4 @@
-// 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
+// RUN: %clang_dxc -fcgl -T lib_6_7 %s -### %s 2>&1 | FileCheck %s
 
 // Make sure fcgl option flag which translated into "-emit-llvm" "-disable-llvm-passes".
 // CHECK: "-emit-llvm"

[bogner]

This looks reasonable and correct, but the test may also deserve a CHECK-NOT: "-S" to preserve the regression test that #97001 attempted to add here. Admittedly though, that kind of test can be a bit fragile and this particular issue is somewhat unlikely to regress, so I could be convinced we don't really need it.

[spall]

This looks reasonable and correct, but the test may also deserve a CHECK-NOT: "-S" to preserve the regression test that #97001 attempted to add here. Admittedly though, that kind of test can be a bit fragile and this particular issue is somewhat unlikely to regress, so I could be convinced we don't really need it.

oh i thought this change did preserve that since this test should fail if such an error is produced? Or that was my thought.

[bogner]

oh i thought this change did preserve that since this test should fail if such an error is produced? Or that was my thought.

IIRC the error was only happening when the clang -cc1 command with both -S and -emit-llvm present was actually run, so the -### command wouldn't produce the error on its own. That said, testing that something doesn't happen for a bug that was fixed long ago is kind of tricky business - this is probably good enough as is.

[bogner]

On further inspection, we do emit -S from clang-dxc -fcgl, and the previous issue was about us emitting it both before and after the -emit-llvm flag. I can see how checking for this would be pretty awkward, and honestly I can't really see how we would end up regressing on this particular issue. I think the test as is looks correct and we should merge this as is.

Thanks for digging into this.

[inbelic]

LGTM, once the now redundant lines are removed