[Driver][SYCL] Issue a warning for converting 'c' input to 'c++' in SYCL mode #5598

hchilama · 2022-02-16T19:40:55Z

No description provided.

…ncompatibility Using err_drv_option_conflict instead of err_drv_opt_incompatible_opt Signed-off-by: Harini Chilamantula <harini.chilamantula@intel.com>

…st compiler

… using -fsycl

mdtoguchi

Driver looks OK - but would like to see opinions on the diagnostic option name from the CFE folks.

mdtoguchi · 2022-02-17T00:01:59Z

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1239,6 +1239,7 @@ def Sycl2020Compat : DiagGroup<"sycl-2020-compat">;
 def SyclStrict : DiagGroup<"sycl-strict", [ Sycl2017Compat, Sycl2020Compat]>;
 def SyclTarget : DiagGroup<"sycl-target">;
 def SyclFPGAMismatch : DiagGroup<"sycl-fpga-mismatch">;
+def SyclCompat : DiagGroup<"sycl-compat">;


I'm on the fence on the naming of the -W option here.

Not up to me, but I agree here. I don't think this warning group fits well here. I'd suggest the CFE/driver reviewers see if we can bikeshed this better

I did quick search to see if I can find a better Warning group and I saw 2 other diagnostics which look related -

def err_drv_fsycl_with_c_type : Error< "'%0' must not be used in conjunction with '-fsycl', which expects C++ source">; def warn_drv_treating_input_as_cxx : Warning< "treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">, InGroup<Deprecated>;

I assume the latter is used for non-sycl programs but would you know what the former is used for now?

I'm not sure what the group should be named here. InvalidSourceFile/UnsupportedFileType/LanguageMismatch? I don't really like any of these. @premanandrao do you have a suggestion?

I did quick search to see if I can find a better Warning group and I saw 2 other diagnostics which look related -

def err_drv_fsycl_with_c_type : Error< "'%0' must not be used in conjunction with '-fsycl', which expects C++ source">; def warn_drv_treating_input_as_cxx : Warning< "treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">, InGroup<Deprecated>;

I assume the latter is used for non-sycl programs but would you know what the former is used for now?

Hi @elizabethandrews , In above 2 defs "err_drv_fsycl_with_c_type" can only be used for error message. so I can't use it for a warning. And the other one "warn_drv_treating_input_as_cxx" I tried using it but @mdtoguchi said "The warning is not accurate for -fsycl". So I have created new one.

This error only emits when 'c' compilation is forced in -fsycl mode.

Sorry for not being clear. My question was - when is err_drv_fsycl_with_c_type thrown ? I was curious about warning vs error functionality here.

That error seems to get issued when compiling as a C file is forced. Ex: clang -c -fsycl -x c foo.c

I'm not sure what the group should be named here. InvalidSourceFile/UnsupportedFileType/LanguageMismatch? I don't really like any of these. @premanandrao do you have a suggestion?

I don't have any great suggestions here: does ExpectedFileType work?

@hchilama I'm ok with InvalidSourceFile or ExpectedFileType. If you agree, please change the group name. Thanks!

Sure thanks @elizabethandrews, I will update the group name.

elizabethandrews · 2022-02-17T20:58:42Z

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1239,6 +1239,7 @@ def Sycl2020Compat : DiagGroup<"sycl-2020-compat">;
 def SyclStrict : DiagGroup<"sycl-strict", [ Sycl2017Compat, Sycl2020Compat]>;
 def SyclTarget : DiagGroup<"sycl-target">;
 def SyclFPGAMismatch : DiagGroup<"sycl-fpga-mismatch">;
+def SyclCompat : DiagGroup<"sycl-compat">;


I did quick search to see if I can find a better Warning group and I saw 2 other diagnostics which look related -

def err_drv_fsycl_with_c_type : Error< "'%0' must not be used in conjunction with '-fsycl', which expects C++ source">; def warn_drv_treating_input_as_cxx : Warning< "treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">, InGroup<Deprecated>;

I assume the latter is used for non-sycl programs but would you know what the former is used for now?

elizabethandrews · 2022-02-17T21:13:21Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -669,4 +669,6 @@ def err_drv_cuda_offload_only_emit_bc : Error<
 def warn_drv_jmc_requires_debuginfo : Warning<
  "/JMC requires debug info. Use '/Zi', '/Z7' or other debug options; option ignored">,
  InGroup<OptionIgnored>;
+def warn_drv_fsycl_with_c_type : Warning<
+  "treating '%0' input as '%1' when -fsycl is used">, InGroup<SyclCompat>;


I'm not sure if we need to mention when -fsycl is used here. @premanandrao WDYT?

I think there is some value is saying why the input type was changed. Suggest quotes around -fsycl though:

Suggested change

"treating '%0' input as '%1' when -fsycl is used">, InGroup<SyclCompat>;

"treating '%0' input as '%1' when '-fsycl' is used">, InGroup<SyclCompat>;

I see this existing driver diagnostic as something that has a similar format:

warn_drv_treating_input_as_cxx : Warning< "treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">, InGroup<Deprecated>;

elizabethandrews · 2022-02-17T21:14:20Z

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1239,6 +1239,7 @@ def Sycl2020Compat : DiagGroup<"sycl-2020-compat">;
 def SyclStrict : DiagGroup<"sycl-strict", [ Sycl2017Compat, Sycl2020Compat]>;
 def SyclTarget : DiagGroup<"sycl-target">;
 def SyclFPGAMismatch : DiagGroup<"sycl-fpga-mismatch">;
+def SyclCompat : DiagGroup<"sycl-compat">;


I'm not sure what the group should be named here. InvalidSourceFile/UnsupportedFileType/LanguageMismatch? I don't really like any of these. @premanandrao do you have a suggestion?

clang/include/clang/Basic/DiagnosticGroups.td

smanna12 · 2022-02-23T19:24:01Z

@hchilama, you have conflicts. could you please resolve that? Thanks

…ersion_with_fsycl

premanandrao

I am okay with the changes.

mdtoguchi · 2022-02-23T20:32:33Z

clang/include/clang/Basic/DiagnosticGroups.td

@@ -275,6 +275,9 @@ def CPre2xCompat : DiagGroup<"pre-c2x-compat">;
 def CPre2xCompatPedantic : DiagGroup<"pre-c2x-compat-pedantic",
                                     [CPre2xCompat]>;

+// Warning for treating C input as C++ input.
+def ExpectedFileType : DiagGroup<"ExpectedFileType">;


The name of the diagnostic when used as an option probably should be something like: DiagGroup<"expected-file-type">

Yes, I have verified the other group naming. I will update this.

hchilama added 6 commits June 9, 2021 01:31

[Driver][SYCL] Emitting a diagnostics for -fsycl and -ffreestanding i…

12ea6ed

…ncompatibility Using err_drv_option_conflict instead of err_drv_opt_incompatible_opt Signed-off-by: Harini Chilamantula <harini.chilamantula@intel.com>

Merge remote-tracking branch 'my_remote/sycl' into sycl

36429a9

[Driver][SYCL] Disabling the unexpected warning when using the g++ ho…

22d7510

…st compiler

Corrected the comment message

1dfa093

Merge branch 'intel:sycl' into sycl

4e1a781

[Driver][Sycl]Needs warning before converting 'c' input to 'c++' when…

8e56f08

… using -fsycl

hchilama requested review from a team as code owners February 16, 2022 19:40

mdtoguchi previously approved these changes Feb 17, 2022

View reviewed changes

smanna12 requested a review from erichkeane February 17, 2022 00:41

elizabethandrews reviewed Feb 17, 2022

View reviewed changes

Updated the group name to ExpectedFileType

9d759fb

hchilama dismissed mdtoguchi’s stale review via 9d759fb February 23, 2022 00:54

hchilama requested review from elizabethandrews, mdtoguchi and premanandrao February 23, 2022 00:57

smanna12 reviewed Feb 23, 2022

View reviewed changes

clang/include/clang/Basic/DiagnosticGroups.td Outdated Show resolved Hide resolved

Updated the comment info

2da2b1c

hchilama requested a review from smanna12 February 23, 2022 19:14

Merge remote-tracking branch 'my_remote/sycl' into warn_c_to_c++_conv…

53906dd

…ersion_with_fsycl

premanandrao previously approved these changes Feb 23, 2022

View reviewed changes

smanna12 previously approved these changes Feb 23, 2022

View reviewed changes

mdtoguchi reviewed Feb 23, 2022

View reviewed changes

Updated the diagnostic

a5fb10f

hchilama dismissed stale reviews from smanna12 and premanandrao via a5fb10f February 23, 2022 22:27

smanna12 approved these changes Feb 23, 2022

View reviewed changes

mdtoguchi approved these changes Feb 23, 2022

View reviewed changes

premanandrao approved these changes Feb 23, 2022

View reviewed changes

elizabethandrews approved these changes Feb 24, 2022

View reviewed changes

bader changed the title ~~[Driver][Sycl]Needs warning before converting 'c' input to 'c++' when using -fsycl~~ [Driver][SYCL] Issue a warning for converting 'c' input to 'c++' in SYCL mode Feb 24, 2022

bader merged commit 5b62ee0 into intel:sycl Feb 24, 2022

hchilama deleted the warn_c_to_c++_conversion_with_fsycl branch February 24, 2022 23:25

	"treating '%0' input as '%1' when -fsycl is used">, InGroup<SyclCompat>;
	"treating '%0' input as '%1' when '-fsycl' is used">, InGroup<SyclCompat>;

[Driver][SYCL] Issue a warning for converting 'c' input to 'c++' in SYCL mode #5598

[Driver][SYCL] Issue a warning for converting 'c' input to 'c++' in SYCL mode #5598

Uh oh!

Conversation

hchilama commented Feb 16, 2022

Uh oh!

mdtoguchi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hchilama Feb 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smanna12 commented Feb 23, 2022

Uh oh!

premanandrao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hchilama Feb 18, 2022 •

edited

Loading