Perhaps this post should be directed toward port maintainers?

Since several global maintainers have now suggested it, I have created a patch that deprecates basic asm when used in a function (attached). It excludes (ie does not deprecate) top level asm, asm in "naked" functions, asm with empty instruction strings, and extended asm.

Building gcc using this patch turns up a few places that use this feature, so I fixed them. Where possible, I used builtins to replace the asm. For ease-of-review, these changes are in their own patch (attached) and obviously this patch should be checked in first.

But before I send these 2 off to gcc-patches, there's a problem. What about platforms other than x86/x64? I don't speak other assembler languages, and have no setup with which to test them.

I could try to provide patches for other platforms, but it would probably be faster for platform experts to just make the changes themselves, rather than trying to review my efforts. Especially if they also want to move to builtins (which I hope they do).

I could just send the patches and let the chips fall where they may, but if there's a less disruptive approach, let me know.

dw

PS I have done a scan for uses of basic asm to get some idea of the scope of the remaining work. My results:

All basic asm in trunk: 1,105 instances.
- Exclude 273 instances with empty strings leaving 832.
- Exclude 271 instances for boehm-gc project leaving 561.
- Exclude 202 instances for testsuite project leaving 359.
- Exclude 282 instances that are (apparently) top-level leaving

~77 instances of basic-asm-in-a-function to be fixed for gcc builds. Most of these are in gcc/config or libgcc/config with just a handful per platform. Lists available upon request.

FWIW...
Index: gcc/ada/init.c
===================================================================
--- gcc/ada/init.c	(revision 237502)
+++ gcc/ada/init.c	(working copy)
@@ -2141,7 +2141,7 @@
 #if defined (__i386__) && !defined (VTHREADS)
   /* This is used to properly initialize the FPU on an x86 for each
      process thread. Is this needed for x86_64 ???  */
-  asm ("finit");
+  asm ("finit":);
 #endif
 
   /* Similarly for SPARC64.  Achieved by masking bits in the Trap Enable Mask
@@ -2652,7 +2652,7 @@
   /* This is used to properly initialize the FPU on an x86 for each
      process thread.  */
 
-  asm ("finit");
+  asm ("finit":);
 
 #endif  /* Defined __i386__ */
 }
Index: libatomic/config/x86/fenv.c
===================================================================
--- libatomic/config/x86/fenv.c	(revision 237502)
+++ libatomic/config/x86/fenv.c	(working copy)
@@ -68,10 +68,10 @@
   if (excepts & FE_DENORM)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FE_DENORM;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FE_DENORM;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (excepts & FE_DIVBYZERO)
     {
@@ -88,18 +88,18 @@
   if (excepts & FE_OVERFLOW)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FE_OVERFLOW;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FE_OVERFLOW;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (excepts & FE_UNDERFLOW)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FE_UNDERFLOW;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FE_UNDERFLOW;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (excepts & FE_INEXACT)
     {
Index: libcilkrts/runtime/config/x86/os-unix-sysdep.c
===================================================================
--- libcilkrts/runtime/config/x86/os-unix-sysdep.c	(revision 237502)
+++ libcilkrts/runtime/config/x86/os-unix-sysdep.c	(working copy)
@@ -85,7 +85,11 @@
     _mm_pause();
 #   endif
 #elif defined __i386__ || defined __x86_64
+#ifdef __GNUC__
+  __builtin_ia32_pause ();
+#else
     __asm__("pause");
+#endif
 #else
 #   warning __cilkrts_short_pause empty
 #endif
Index: libgcc/config/i386/sfp-exceptions.c
===================================================================
--- libgcc/config/i386/sfp-exceptions.c	(revision 237502)
+++ libgcc/config/i386/sfp-exceptions.c	(working copy)
@@ -59,10 +59,10 @@
   if (_fex & FP_EX_DENORM)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FP_EX_DENORM;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FP_EX_DENORM;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (_fex & FP_EX_DIVZERO)
     {
@@ -79,18 +79,18 @@
   if (_fex & FP_EX_OVERFLOW)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FP_EX_OVERFLOW;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FP_EX_OVERFLOW;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (_fex & FP_EX_UNDERFLOW)
     {
       struct fenv temp;
-      asm volatile ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= FP_EX_UNDERFLOW;
-      asm volatile ("fldenv\t%0" : : "m" (temp));
-      asm volatile ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= FP_EX_UNDERFLOW;
+      __builtin_ia32_fldenv (&temp);
+      asm ("fwait" ::"m" (temp):); /* Trigger the fp exception.  */
     }
   if (_fex & FP_EX_INEXACT)
     {
Index: libgfortran/config/fpu-387.h
===================================================================
--- libgfortran/config/fpu-387.h	(revision 237502)
+++ libgfortran/config/fpu-387.h	(working copy)
@@ -114,10 +114,10 @@
   if (excepts & _FPU_MASK_DM)
     {
       my_fenv_t temp;
-      __asm__ __volatile__ ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= _FPU_MASK_DM;
-      __asm__ __volatile__ ("fldenv\t%0" : : "m" (temp));
-      __asm__ __volatile__ ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= _FPU_MASK_DM;
+      __builtin_ia32_fldenv (&temp);
+      __asm__ ("fwait" ::"m" (temp)); /* Trigger the fp exception.  */
     }
   if (excepts & _FPU_MASK_ZM)
     {
@@ -134,18 +134,18 @@
   if (excepts & _FPU_MASK_OM)
     {
       my_fenv_t temp;
-      __asm__ __volatile__ ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= _FPU_MASK_OM;
-      __asm__ __volatile__ ("fldenv\t%0" : : "m" (temp));
-      __asm__ __volatile__ ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= _FPU_MASK_OM;
+      __builtin_ia32_fldenv (&temp);
+      __asm__ ("fwait" ::"m" (temp)); /* Trigger the fp exception.  */
     }
   if (excepts & _FPU_MASK_UM)
     {
       my_fenv_t temp;
-      __asm__ __volatile__ ("fnstenv\t%0" : "=m" (temp));
-      temp.__status_word |= _FPU_MASK_UM;
-      __asm__ __volatile__ ("fldenv\t%0" : : "m" (temp));
-      __asm__ __volatile__ ("fwait");
+      __builtin_ia32_fnstenv (&temp);
+      temp.__status_word |= _FPU_MASK_UM;
+      __builtin_ia32_fldenv (&temp);
+      __asm__ ("fwait" ::"m" (temp)); /* Trigger the fp exception.  */
     }
   if (excepts & _FPU_MASK_PM)
     {
Index: libsanitizer/sanitizer_common/sanitizer_atomic_clang_x86.h
===================================================================
--- libsanitizer/sanitizer_common/sanitizer_atomic_clang_x86.h	(revision 237502)
+++ libsanitizer/sanitizer_common/sanitizer_atomic_clang_x86.h	(working copy)
@@ -18,7 +18,11 @@
 INLINE void proc_yield(int cnt) {
   __asm__ __volatile__("" ::: "memory");
   for (int i = 0; i < cnt; i++)
+#ifdef __GNUC__
+  __builtin_ia32_pause ();
+#else
     __asm__ __volatile__("pause");
+#endif
   __asm__ __volatile__("" ::: "memory");
 }
 
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 237502)
+++ gcc/c-family/c.opt	(working copy)
@@ -597,6 +597,10 @@
 C++ ObjC++ Var(warn_namespaces) Warning
 Warn on namespace definition.
 
+Wbasic-asm
+C ObjC ObjC++ C++ Var(warn_basic_asm) Init(1) Warning
+Warn on deprecated uses of basic asm.
+
 Wsized-deallocation
 C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra)
 Warn about missing sized deallocation functions.
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 237502)
+++ gcc/c/c-parser.c	(working copy)
@@ -6044,8 +6044,20 @@
   labels = NULL_TREE;
 
   if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
-    goto done_asm;
+    {
+      /* Warn on basic asm used inside of functions,
+	 except when in naked functions.  Also allow asm ("").  */
+      if (warn_basic_asm && TREE_STRING_LENGTH (str) != 1)
+	if (lookup_attribute ("naked",
+			      DECL_ATTRIBUTES (current_function_decl))
+	    == NULL_TREE)
+	  warning_at (asm_loc, OPT_Wbasic_asm,
+		      "Deprecated: asm in function without extended"
+		      " syntax");
 
+      goto done_asm;
+    }
+
   /* Parse each colon-delimited section of operands.  */
   nsections = 3 + is_goto;
   for (section = 0; section < nsections; ++section)
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 237502)
+++ gcc/cp/parser.c	(working copy)
@@ -18064,6 +18064,8 @@
   bool goto_p = false;
   required_token missing = RT_NONE;
 
+  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
+
   /* Look for the `asm' keyword.  */
   cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
 
@@ -18222,6 +18224,17 @@
 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
 	  if (!extended_p)
 	    {
+	      /* Warn on basic asm used inside of functions,
+		 except when in naked functions.  Also allow asm ("").  */
+	      if (warn_basic_asm
+		  && TREE_STRING_LENGTH (string) != 1)
+		if (lookup_attribute ("naked",
+				      DECL_ATTRIBUTES (current_function_decl))
+		    == NULL_TREE)
+		  warning_at (asm_loc, OPT_Wbasic_asm,
+			      "Deprecated: asm in function without extended"
+			      " syntax");
+
 	      tree temp = asm_stmt;
 	      if (TREE_CODE (temp) == CLEANUP_POINT_EXPR)
 		temp = TREE_OPERAND (temp, 0);
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 237502)
+++ gcc/doc/extend.texi	(working copy)
@@ -7507,6 +7507,17 @@
 various @option{-std} options, use @code{__asm__} instead of 
 @code{asm} (@pxref{Alternate Keywords}).
 
+@strong{Warning:} The use of basic @code{asm} in functions has been
+deprecated and support for this usage will eventually be removed.  This
+change does not apply to top-level asm, functions that are marked with the
+@option{naked} attribute, @code{asm} statements with an empty instruction
+string, or extended @code{asm}.
+
+Existing code should be converted to C, modified to use GCC's
+builtins, or replaced with extended @code{asm} (see the GCC
+@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, wiki} for
+guidance on performing this conversion).
+
 @subsubheading Qualifiers
 @table @code
 @item volatile
@@ -7572,7 +7583,7 @@
 symbol errors during compilation if your assembly code defines symbols or 
 labels.
 
-@strong{Warning:} The C standards do not specify semantics for @code{asm},
+The C standards do not specify semantics for @code{asm},
 making it a potential source of incompatibilities between compilers.  These
 incompatibilities may not produce compiler warnings/errors.
 
@@ -7582,15 +7593,8 @@
 discard them as unreferenced.  It also does not know about side effects of
 the assembler code, such as modifications to memory or registers.  Unlike
 some compilers, GCC assumes that no changes to general purpose registers
-occur.  This assumption may change in a future release.
+occur.
 
-To avoid complications from future changes to the semantics and the
-compatibility issues between compilers, consider replacing basic @code{asm}
-with extended @code{asm}.  See
-@uref{https://gcc.gnu.org/wiki/ConvertBasicAsmToExtended, How to convert
-from basic asm to extended asm} for information about how to perform this
-conversion.
-
 The compiler copies the assembler instructions in a basic @code{asm} 
 verbatim to the assembly language output file, without 
 processing dialects or any of the @samp{%} operators that are available with
@@ -7605,9 +7609,9 @@
 Basic @code{asm} provides no
 mechanism to provide different assembler strings for different dialects.
 
-For basic @code{asm} with non-empty assembler string GCC assumes
-the assembler block does not change any general purpose registers,
-but it may read or write any globally accessible variable.
+Starting in v7, GCC assumes non-empty assembler blocks do not change
+any general purpose registers, but they may read or write any globally
+accessible variable.
 
 Here is an example of basic @code{asm} for i386:
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 237502)
+++ gcc/doc/invoke.texi	(working copy)
@@ -5851,6 +5851,13 @@
 a structure that has been marked with the @code{designated_init}
 attribute.
 
+@item -Wno-basic-asm @r{(C and C++ only)}
+Suppress warning on deprecated uses of basic asm.
+
+Functions that are marked with the @option{naked} attribute, top-level
+@code{asm} statements, @code{asm} statements with an empty instruction
+string and extended @code{asm} statements are excluded from this check.
+
 @item -Whsa
 Issue a warning when HSAIL cannot be emitted for the compiled function or
 OpenMP construct.
Index: gcc/testsuite/c-c++-common/Wbasic-asm-2.c
===================================================================
--- gcc/testsuite/c-c++-common/Wbasic-asm-2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wbasic-asm-2.c	(working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target naked_functions } */
+
+int __attribute__((naked))
+func (int x, int y)
+{
+  /* Basic asm should not warn in naked functions.  */
+   asm (" "); /* No warning.  */
+}
+
+int main (int argc, char *argv[])
+{
+   return func (argc, argc);
+}
Index: gcc/testsuite/c-c++-common/Wbasic-asm.c
===================================================================
--- gcc/testsuite/c-c++-common/Wbasic-asm.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wbasic-asm.c	(working copy)
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+
+#if defined (__i386__) || defined (__x86_64__)
+/* Acceptable.  */
+register int b asm ("esi");
+#else
+int b = 3;
+#endif
+
+/* Acceptable.  */
+int foo asm ("myfoo") = 2;
+
+/* Acceptable.  */
+asm (" ");
+
+/* Acceptable.  */
+int func (int x, int y) asm ("MYFUNC");
+
+int main (int argc, char *argv[])
+{
+#if defined (__i386__) || defined (__x86_64__)
+   /* Acceptable.  */
+   register int a asm ("edi");
+#else
+   int a = 2;
+#endif
+
+   /* Acceptable.  */
+   asm (" "::"r"(a), "r" (b));
+
+   /* Acceptable.  */
+   asm goto (""::::done);
+
+   /* Acceptable.  */
+   asm ("");
+
+   /* Acceptable.  */
+   asm (" "); /* { dg-warning "Deprecated: asm in function without extended syntax" } */
+
+  done:
+   return 0;
+}

Reply via email to