[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-07 Thread Eli Friedman via cfe-commits 

https://github.com/efriedma-quic created 
https://github.com/llvm/llvm-project/pull/74790

The flag -fms-volatile has existed as a -cc1 flag for a while.  It also 
technically existed as a driver flag, but didn't do anything because it wasn't 
wired up to anything in the driver.

This patch adds -fno-ms-volatile, and makes both -fms-volatile and 
-fno-ms-volatile work the same way as the cl-mode flags. The defaults are 
unchanged (default on for x86 in cl mode, default off otherwise).

>From bd8582278a76bffa4a43be5ba00f0e915b57bbb0 Mon Sep 17 00:00:00 2001
From: Eli Friedman 
Date: Thu, 7 Dec 2023 16:21:53 -0800
Subject: [PATCH] [clang][Driver] Support -fms-volatile as equivalent to
 /volatile:ms

The flag "-fms-volatile" has existed as a -cc1 flag for a while.  It
also technically existed as a driver flag, but didn't do anything
because it wasn't wired up to anything in the driver.

This patch adds -fno-ms-volatile, and makes both -fms-volatile and
-fno-ms-volatile work the same way as the cl-mode flags. The defaults
are unchanged (default on for x86 in cl mode, default off otherwise).
---
 clang/include/clang/Driver/Options.td | 12 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 16 
 clang/test/Driver/cl-options.c|  6 --
 clang/test/Driver/clang_f_opts.c  |  6 ++
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 0eec2b3526376..6274ff76950d9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2869,9 +2869,11 @@ defm asm_blocks : BoolFOption<"asm-blocks",
   LangOpts<"AsmBlocks">, Default,
   PosFlag,
   NegFlag>;
-def fms_volatile : Flag<["-"], "fms-volatile">, Group,
-  Visibility<[ClangOption, CC1Option]>,
-  MarshallingInfoFlag>;
+defm ms_volatile : BoolFOption<"ms-volatile",
+  LangOpts<"MSVolatile">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def fmsc_version : Joined<["-"], "fmsc-version=">, Group,
   Visibility<[ClangOption, CLOption]>,
   HelpText<"Microsoft compiler version number to report in _MSC_VER (0 = don't 
define it (default))">;
@@ -8184,7 +8186,7 @@ def _SLASH_winsysroot : CLJoinedOrSeparate<"winsysroot">,
   HelpText<"Same as \"/diasdkdir /DIA SDK\" /vctoolsdir 
/VC/Tools/MSVC/ \"/winsdkdir /Windows Kits/10\"">,
   MetaVarName<"">;
 def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have standard semantics">;
 def _SLASH_vmb : CLFlag<"vmb">,
   HelpText<"Use a best-case representation method for member pointers">;
@@ -8199,7 +8201,7 @@ def _SLASH_vmv : CLFlag<"vmv">,
   HelpText<"Set the default most-general representation to "
"virtual inheritance">;
 def _SLASH_volatile_ms  : Option<["/", "-"], "volatile:ms", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have acquire and release semantics">;
 def _SLASH_clang : CLJoined<"clang:">,
   HelpText<"Pass  to the clang driver">, MetaVarName<"">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f..c6be0750e1c33 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5545,6 +5545,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_fno_auto_import);
   }
 
+  if (Args.hasFlag(options::OPT_fms_volatile, options::OPT_fno_ms_volatile,
+   Triple.isX86() && D.IsCLMode()))
+CmdArgs.push_back("-fms-volatile");
+
   // Non-PIC code defaults to -fdirect-access-external-data while PIC code
   // defaults to -fno-direct-access-external-data. Pass the option if different
   // from the default.
@@ -7877,18 +7881,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, 
types::ID InputType,
 CmdArgs.push_back("-P");
   }
 
-  unsigned VolatileOptionID;
-  if (getToolChain().getTriple().isX86())
-VolatileOptionID = options::OPT__SLASH_volatile_ms;
-  else
-VolatileOptionID = options::OPT__SLASH_volatile_iso;
-
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
-VolatileOptionID = A->getOption().getID();
-
-  if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
-CmdArgs.push_back("-fms-volatile");
-
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
   false)) {
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6d929b19e7e2e..6bc315b09477a 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -272,10 +272,12 @@
 // RUN: not %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck 
-check-prefix=VMX %s
 // VMX: '/vms'

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-07 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)


Changes

The flag -fms-volatile has existed as a -cc1 flag for a while.  It also 
technically existed as a driver flag, but didn't do anything because it wasn't 
wired up to anything in the driver.

This patch adds -fno-ms-volatile, and makes both -fms-volatile and 
-fno-ms-volatile work the same way as the cl-mode flags. The defaults are 
unchanged (default on for x86 in cl mode, default off otherwise).

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


4 Files Affected:

- (modified) clang/include/clang/Driver/Options.td (+7-5) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4-12) 
- (modified) clang/test/Driver/cl-options.c (+4-2) 
- (modified) clang/test/Driver/clang_f_opts.c (+6) 


``diff
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 0eec2b3526376..6274ff76950d9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2869,9 +2869,11 @@ defm asm_blocks : BoolFOption<"asm-blocks",
   LangOpts<"AsmBlocks">, Default,
   PosFlag,
   NegFlag>;
-def fms_volatile : Flag<["-"], "fms-volatile">, Group,
-  Visibility<[ClangOption, CC1Option]>,
-  MarshallingInfoFlag>;
+defm ms_volatile : BoolFOption<"ms-volatile",
+  LangOpts<"MSVolatile">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def fmsc_version : Joined<["-"], "fmsc-version=">, Group,
   Visibility<[ClangOption, CLOption]>,
   HelpText<"Microsoft compiler version number to report in _MSC_VER (0 = don't 
define it (default))">;
@@ -8184,7 +8186,7 @@ def _SLASH_winsysroot : CLJoinedOrSeparate<"winsysroot">,
   HelpText<"Same as \"/diasdkdir /DIA SDK\" /vctoolsdir 
/VC/Tools/MSVC/ \"/winsdkdir /Windows Kits/10\"">,
   MetaVarName<"">;
 def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have standard semantics">;
 def _SLASH_vmb : CLFlag<"vmb">,
   HelpText<"Use a best-case representation method for member pointers">;
@@ -8199,7 +8201,7 @@ def _SLASH_vmv : CLFlag<"vmv">,
   HelpText<"Set the default most-general representation to "
"virtual inheritance">;
 def _SLASH_volatile_ms  : Option<["/", "-"], "volatile:ms", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have acquire and release semantics">;
 def _SLASH_clang : CLJoined<"clang:">,
   HelpText<"Pass  to the clang driver">, MetaVarName<"">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f..c6be0750e1c33 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5545,6 +5545,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_fno_auto_import);
   }
 
+  if (Args.hasFlag(options::OPT_fms_volatile, options::OPT_fno_ms_volatile,
+   Triple.isX86() && D.IsCLMode()))
+CmdArgs.push_back("-fms-volatile");
+
   // Non-PIC code defaults to -fdirect-access-external-data while PIC code
   // defaults to -fno-direct-access-external-data. Pass the option if different
   // from the default.
@@ -7877,18 +7881,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, 
types::ID InputType,
 CmdArgs.push_back("-P");
   }
 
-  unsigned VolatileOptionID;
-  if (getToolChain().getTriple().isX86())
-VolatileOptionID = options::OPT__SLASH_volatile_ms;
-  else
-VolatileOptionID = options::OPT__SLASH_volatile_iso;
-
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
-VolatileOptionID = A->getOption().getID();
-
-  if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
-CmdArgs.push_back("-fms-volatile");
-
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
   false)) {
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6d929b19e7e2e..6bc315b09477a 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -272,10 +272,12 @@
 // RUN: not %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck 
-check-prefix=VMX %s
 // VMX: '/vms' not allowed with '/vmm'
 
-// RUN: %clang_cl /volatile:iso -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=i686-pc-win32 /volatile:iso -### -- %s 2>&1 | 
FileCheck -check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=aarch64-pc-win32 -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
 // VOLATILE-ISO-NOT: "-fms-volatile"
 
-// RUN: %clang_cl /volatile:ms -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-MS %s
+// RUN: %clang_cl --target=aarch64-pc-win32 /volatile:ms -### -- %s 2>&1 | 
FileCheck -c

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-07 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-driver

Author: Eli Friedman (efriedma-quic)


Changes

The flag -fms-volatile has existed as a -cc1 flag for a while.  It also 
technically existed as a driver flag, but didn't do anything because it wasn't 
wired up to anything in the driver.

This patch adds -fno-ms-volatile, and makes both -fms-volatile and 
-fno-ms-volatile work the same way as the cl-mode flags. The defaults are 
unchanged (default on for x86 in cl mode, default off otherwise).

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


4 Files Affected:

- (modified) clang/include/clang/Driver/Options.td (+7-5) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4-12) 
- (modified) clang/test/Driver/cl-options.c (+4-2) 
- (modified) clang/test/Driver/clang_f_opts.c (+6) 


``diff
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 0eec2b3526376..6274ff76950d9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2869,9 +2869,11 @@ defm asm_blocks : BoolFOption<"asm-blocks",
   LangOpts<"AsmBlocks">, Default,
   PosFlag,
   NegFlag>;
-def fms_volatile : Flag<["-"], "fms-volatile">, Group,
-  Visibility<[ClangOption, CC1Option]>,
-  MarshallingInfoFlag>;
+defm ms_volatile : BoolFOption<"ms-volatile",
+  LangOpts<"MSVolatile">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def fmsc_version : Joined<["-"], "fmsc-version=">, Group,
   Visibility<[ClangOption, CLOption]>,
   HelpText<"Microsoft compiler version number to report in _MSC_VER (0 = don't 
define it (default))">;
@@ -8184,7 +8186,7 @@ def _SLASH_winsysroot : CLJoinedOrSeparate<"winsysroot">,
   HelpText<"Same as \"/diasdkdir /DIA SDK\" /vctoolsdir 
/VC/Tools/MSVC/ \"/winsdkdir /Windows Kits/10\"">,
   MetaVarName<"">;
 def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have standard semantics">;
 def _SLASH_vmb : CLFlag<"vmb">,
   HelpText<"Use a best-case representation method for member pointers">;
@@ -8199,7 +8201,7 @@ def _SLASH_vmv : CLFlag<"vmv">,
   HelpText<"Set the default most-general representation to "
"virtual inheritance">;
 def _SLASH_volatile_ms  : Option<["/", "-"], "volatile:ms", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have acquire and release semantics">;
 def _SLASH_clang : CLJoined<"clang:">,
   HelpText<"Pass  to the clang driver">, MetaVarName<"">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f..c6be0750e1c33 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5545,6 +5545,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_fno_auto_import);
   }
 
+  if (Args.hasFlag(options::OPT_fms_volatile, options::OPT_fno_ms_volatile,
+   Triple.isX86() && D.IsCLMode()))
+CmdArgs.push_back("-fms-volatile");
+
   // Non-PIC code defaults to -fdirect-access-external-data while PIC code
   // defaults to -fno-direct-access-external-data. Pass the option if different
   // from the default.
@@ -7877,18 +7881,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, 
types::ID InputType,
 CmdArgs.push_back("-P");
   }
 
-  unsigned VolatileOptionID;
-  if (getToolChain().getTriple().isX86())
-VolatileOptionID = options::OPT__SLASH_volatile_ms;
-  else
-VolatileOptionID = options::OPT__SLASH_volatile_iso;
-
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
-VolatileOptionID = A->getOption().getID();
-
-  if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
-CmdArgs.push_back("-fms-volatile");
-
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
   false)) {
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6d929b19e7e2e..6bc315b09477a 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -272,10 +272,12 @@
 // RUN: not %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck 
-check-prefix=VMX %s
 // VMX: '/vms' not allowed with '/vmm'
 
-// RUN: %clang_cl /volatile:iso -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=i686-pc-win32 /volatile:iso -### -- %s 2>&1 | 
FileCheck -check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=aarch64-pc-win32 -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
 // VOLATILE-ISO-NOT: "-fms-volatile"
 
-// RUN: %clang_cl /volatile:ms -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-MS %s
+// RUN: %clang_cl --target=aarch64-pc-win32 /volatile:ms -### -- %s 2>&1 | 
FileC

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-07 Thread Fangrui Song via cfe-commits 

MaskRay wrote:

Current behavior:
```
% clang --target=x86_64-linux-musl -fms-volatile -c a.cc
clang: warning: argument unused during compilation: '-fms-volatile' 
[-Wunused-command-line-argument]
% clang --target=x86_64-windows -fms-volatile -c a.cc
clang: warning: argument unused during compilation: '-fms-volatile' 
[-Wunused-command-line-argument]
```

Shall we make -f[no-]ms-volatile an error for non-Windows targets? (mark it as 
`TargetSpecific` and only claim it for `isOSWindows()`).
>From 
>https://learn.microsoft.com/en-us/cpp/build/reference/volatile-volatile-keyword-interpretation?view=msvc-170
> , I assume that /volatile:ms is a legacy behavior that non-Windows OSes 
>likely don't want to adopt.


https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-08 Thread Reid Kleckner via cfe-commits 

https://github.com/rnk commented:

> I assume that /volatile:ms is a legacy behavior that non-Windows OSes likely 
> don't want to adopt.

That is correct, it is legacy behavior. However, I think this flag has 
potential porting applications, similar to `-fshort-wchar`. I also think 
operating systems don't really have a say or control over what command line 
flags developers use. I think the more relevant question is, does the Clang 
project want to encourage folks to use this flag, thereby increasing our long 
term support burden? If this is a legacy compatibility mode, do we think we 
will be able to sunset it at some point, similar to 
`-fdelayed-template-parsing`?

https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-11 Thread Eli Friedman via cfe-commits 

efriedma-quic wrote:

I got a request for this feature from a user that ran into issues porting code 
from cl to clang for a baremetal target.  I expect scenarios like that to 
continue to be relevant for a while.

I don't expect anyone would want to use this for new code because current 
practice has strongly shifted towards using atomic types for synchronization 
since C++11.

Like the commit message notes, this isn't changing any defaults; it's just 
giving users an additional tool for porting code.  It's something we've already 
provided for many years; it just wasn't exposed outside cl-mode.  And it's a 
pretty straightforward extension to support (it just slightly modifies IR 
generation), so I don't think there's much long-term support burden.

https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-11 Thread Reid Kleckner via cfe-commits 

https://github.com/rnk approved this pull request.

> And it's a pretty straightforward extension to support (it just slightly 
> modifies IR generation), so I don't think there's much long-term support 
> burden.

I find that compelling, but make sure others' concerns are addressed.

https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-11 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-11 Thread Fangrui Song via cfe-commits 


@@ -611,3 +611,9 @@
 // CHECK-INT-OBJEMITTER-NOT: unsupported option '-fintegrated-objemitter' for 
target
 // RUN: not %clang -### -fno-integrated-objemitter --target=x86_64 %s 2>&1 | 
FileCheck -check-prefix=CHECK-NOINT-OBJEMITTER %s
 // CHECK-NOINT-OBJEMITTER: unsupported option '-fno-integrated-objemitter' for 
target
+
+// RUN: %clang -### -target aarch64-windows-msvc %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-MS-VOLATILE %s

MaskRay wrote:

`--target=` for new tests.

`-target ` has been deprecated since clang 3.4.

https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-11 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay approved this pull request.


https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-12 Thread Eli Friedman via cfe-commits 

https://github.com/efriedma-quic updated 
https://github.com/llvm/llvm-project/pull/74790

>From 476402a90cc24a5697efbef0b3bcb1276a4bc6f5 Mon Sep 17 00:00:00 2001
From: Eli Friedman 
Date: Thu, 7 Dec 2023 16:21:53 -0800
Subject: [PATCH] [clang][Driver] Support -fms-volatile as equivalent to
 /volatile:ms

The flag "-fms-volatile" has existed as a -cc1 flag for a while.  It
also technically existed as a driver flag, but didn't do anything
because it wasn't wired up to anything in the driver.

This patch adds -fno-ms-volatile, and makes both -fms-volatile and
-fno-ms-volatile work the same way as the cl-mode flags. The defaults
are unchanged (default on for x86 in cl mode, default off otherwise).
---
 clang/include/clang/Driver/Options.td | 12 +++-
 clang/lib/Driver/ToolChains/Clang.cpp | 16 
 clang/test/Driver/cl-options.c|  6 --
 clang/test/Driver/clang_f_opts.c  |  6 ++
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 0eec2b35263762..6274ff76950d98 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2869,9 +2869,11 @@ defm asm_blocks : BoolFOption<"asm-blocks",
   LangOpts<"AsmBlocks">, Default,
   PosFlag,
   NegFlag>;
-def fms_volatile : Flag<["-"], "fms-volatile">, Group,
-  Visibility<[ClangOption, CC1Option]>,
-  MarshallingInfoFlag>;
+defm ms_volatile : BoolFOption<"ms-volatile",
+  LangOpts<"MSVolatile">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def fmsc_version : Joined<["-"], "fmsc-version=">, Group,
   Visibility<[ClangOption, CLOption]>,
   HelpText<"Microsoft compiler version number to report in _MSC_VER (0 = don't 
define it (default))">;
@@ -8184,7 +8186,7 @@ def _SLASH_winsysroot : CLJoinedOrSeparate<"winsysroot">,
   HelpText<"Same as \"/diasdkdir /DIA SDK\" /vctoolsdir 
/VC/Tools/MSVC/ \"/winsdkdir /Windows Kits/10\"">,
   MetaVarName<"">;
 def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have standard semantics">;
 def _SLASH_vmb : CLFlag<"vmb">,
   HelpText<"Use a best-case representation method for member pointers">;
@@ -8199,7 +8201,7 @@ def _SLASH_vmv : CLFlag<"vmv">,
   HelpText<"Set the default most-general representation to "
"virtual inheritance">;
 def _SLASH_volatile_ms  : Option<["/", "-"], "volatile:ms", KIND_FLAG>,
-  Group<_SLASH_volatile_Group>, Flags<[NoXarchOption]>, Visibility<[CLOption]>,
+  Visibility<[CLOption]>, Alias,
   HelpText<"Volatile loads and stores have acquire and release semantics">;
 def _SLASH_clang : CLJoined<"clang:">,
   HelpText<"Pass  to the clang driver">, MetaVarName<"">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f0..c6be0750e1c338 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5545,6 +5545,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_fno_auto_import);
   }
 
+  if (Args.hasFlag(options::OPT_fms_volatile, options::OPT_fno_ms_volatile,
+   Triple.isX86() && D.IsCLMode()))
+CmdArgs.push_back("-fms-volatile");
+
   // Non-PIC code defaults to -fdirect-access-external-data while PIC code
   // defaults to -fno-direct-access-external-data. Pass the option if different
   // from the default.
@@ -7877,18 +7881,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, 
types::ID InputType,
 CmdArgs.push_back("-P");
   }
 
-  unsigned VolatileOptionID;
-  if (getToolChain().getTriple().isX86())
-VolatileOptionID = options::OPT__SLASH_volatile_ms;
-  else
-VolatileOptionID = options::OPT__SLASH_volatile_iso;
-
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
-VolatileOptionID = A->getOption().getID();
-
-  if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
-CmdArgs.push_back("-fms-volatile");
-
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
   options::OPT__SLASH_Zc_dllexportInlines,
   false)) {
diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 6d929b19e7e2ef..6bc315b09477a9 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -272,10 +272,12 @@
 // RUN: not %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck 
-check-prefix=VMX %s
 // VMX: '/vms' not allowed with '/vmm'
 
-// RUN: %clang_cl /volatile:iso -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=i686-pc-win32 /volatile:iso -### -- %s 2>&1 | 
FileCheck -check-prefix=VOLATILE-ISO %s
+// RUN: %clang_cl --target=aarch64-pc-win32 -### -- %s 2>&1 | FileCheck 
-check-prefix=VOLATILE-ISO %s
 // VOLATILE-ISO-NOT: "-fms-volatile"
 
-// RUN: %clang_

[clang] [clang][Driver] Support -fms-volatile as equivalent to /volatile:ms (PR #74790)

2023-12-13 Thread Eli Friedman via cfe-commits 

https://github.com/efriedma-quic closed 
https://github.com/llvm/llvm-project/pull/74790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits