Skip to content

[HLSL] Change clang Driver Options to not set CXXOperatorNames #126758

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.

Sign up for GitHub

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 3 commits into from
Feb 12, 2025
Merged

[HLSL] Change clang Driver Options to not set CXXOperatorNames #126758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b7e458
[HLSL] Change clang Driver Options to not set CXXOperatorNames
farzonl Feb 11, 2025
68b7891
address pr feedback
farzonl Feb 11, 2025
711cad3
switch from checking name to checking for FunctionDecl
farzonl Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3397,7 +3397,7 @@ def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group<f_Group>,
def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, Group<f_Group>,
Visibility<[ClangOption, FlangOption]>;
defm operator_names : BoolFOption<"operator-names",
LangOpts<"CXXOperatorNames">, Default<cplusplus.KeyPath>,
LangOpts<"CXXOperatorNames">, Default<!strconcat(cplusplus.KeyPath, " && !",hlsl.KeyPath)>,
NegFlag<SetFalse, [], [ClangOption, CC1Option],
"Do not treat C++ operator name keywords as synonyms for operators">,
PosFlag<SetTrue>>;
Expand Down
41 changes: 41 additions & 0 deletions clang/test/CodeGenHLSL/use-cxx-alt-operator-names.hlsl
View file
Open in desktop
Copy link
Collaborator

@llvm-beanz llvm-beanz Feb 11, 2025

Choose a reason for hiding this comment

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

This seems like the wrong place to put this test. We don't need actual codegen here, we just need to make sure it parses.

I'd put this under SemaHLSL and instead use a run line like:

// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library %s -ast-dump | FileCheck %s

Restricting test cases to only running the parts of the compiler that are strictly required to verify correctness of a change reduces the time it takes to run the test. While in isolation for a single test that may only be shaving off fractions of a second, in aggregate across the entire test suite it can have a huge impact.

Copy link
Member Author

@farzonl farzonl Feb 11, 2025

Choose a reason for hiding this comment

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

Is it ok to keep the tests as check-label? Tests are still passing with Label? Or should this change to // CHECK: FunctionDecl {{.*}} NAME void () Which is how it seems other tests are setup.

Copy link
Collaborator

@llvm-beanz llvm-beanz Feb 11, 2025

Choose a reason for hiding this comment

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

CHECK-LABEL doesn't actually change how the matching is performed (see: https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-label-directive). Notably:

It is treated identically to a normal CHECK directive except that FileCheck makes an additional assumption that a line matched by the directive cannot also be matched by any other check present in match-filename;...

Whether you use CHECK or CHECK-LABEL here doesn't really matter, but the strings you are matching on are too simple, you should be verifying that they are FunctionDecls not just that the string and appears on a line.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
// RUN: dxil-pc-shadermodel6.3-library %s \
// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %s

// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
// RUN: spirv-unknown-vulkan-compute %s \
// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %s

// CHECK-LABEL: and
void and() {}

// CHECK-LABEL: and_eq
void and_eq() {}

// CHECK-LABEL: bitand
void bitand() {}

// CHECK-LABEL: bitor
void bitor() {}

// CHECK-LABEL: compl
void compl() {}

// CHECK-LABEL: not
void not() {}

// CHECK-LABEL: not_eq
void not_eq() {}

// CHECK-LABEL: or
void or() {}

// CHECK-LABEL: or_eq
void or_eq() {}

// CHECK-LABEL: xor
void xor() {}

// CHECK-LABEL: xor_eq
void xor_eq() {}