[PATCH] Make basic asm implicitly clobber memory, pr24414

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]


Hi,


from the recent discussion on gcc@gcc.gnu.org I became aware that the so called "basic asm"
which is an asm without colon, has no way to specify the clobber list.  That makes it rather
useless, because everyone would expect it to be able to use at least global memory.
Furthermore there is a target hook that allows extended asm to implicitly clobber the "cc" register
on certain targets.  This hook should also be called for basic asm, because nobody would expect to
have to preserve that on a basic asm.  But I would say it is naturally clear that if the basic asm
would clobber any general purpose register the previous value must be pushed by the assembler
code and restored at the end.

Furthermore there is a documented use for asm(""): The empty assembler string is used to make a function
volatile, thus calls can not be optimized away.  But I think it is not necessary to make this clobber anything,
nor should it be an instruction scheduling barrier, as it used to be in the past.

The attached patch implements this by introducing a new parallel block for asm_input with clobbers.
I believe it is quite safe, and will not break any existing code.

It was boot-strapped and regression-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?

... or should I wait for the next stage1?


Thanks
Bernd.

[-- Attachment #2: changelog-basic-asm.txt --]
[-- Type: text/plain, Size: 860 bytes --]

gcc:
2015-12-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c/24414
	* cfgexpand.c (expand_asm_loc): Remove handling for ADDR_EXPR.
	Implicitly clobber memory for basic asm with non-empty assembler
	string.  Use targetm.md_asm_adjust also here.
	* gimple.c (gimple_asm_clobbers_memory_p): Handle basic asm with
	non-empty assembler string.
	* final.c (final_scan_insn): Handle basic asm in PARALLEL block.
	* recog.c (asm_noperands): Handle basic asm in PARALLEL block.
	(decode_asm_operands): Handle basic asm in PARALLEL block.
	(extract_insn): Handle basic asm in PARALLEL block.
	* sched-deps.c (sched_analyze_2): Make no barrier for basic asm with
	empty assembler string.
	* doc/extend.texi: Mention new behavior of basic asm.

testsuite:
2015-12-09  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c/24414
	* gcc.target/i386/pr24414.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-basic-asm.diff --]
[-- Type: text/x-patch; name="patch-basic-asm.diff", Size: 6980 bytes --]

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 231412)
+++ gcc/cfgexpand.c	(working copy)
@@ -2655,9 +2655,6 @@ expand_asm_loc (tree string, int vol, location_t l
 {
   rtx body;
 
-  if (TREE_CODE (string) == ADDR_EXPR)
-    string = TREE_OPERAND (string, 0);
-
   body = gen_rtx_ASM_INPUT_loc (VOIDmode,
 				ggc_strdup (TREE_STRING_POINTER (string)),
 				locus);
@@ -2664,6 +2661,34 @@ expand_asm_loc (tree string, int vol, location_t l
 
   MEM_VOLATILE_P (body) = vol;
 
+  /* Non-empty basic ASM implicitly clobbers memory.  */
+  if (TREE_STRING_LENGTH (string) != 0)
+    {
+      rtx asm_op, clob;
+      unsigned i, nclobbers;
+      auto_vec<rtx> input_rvec, output_rvec;
+      auto_vec<const char *> constraints;
+      auto_vec<rtx> clobber_rvec;
+      HARD_REG_SET clobbered_regs;
+      CLEAR_HARD_REG_SET (clobbered_regs);
+
+      clob = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode));
+      clobber_rvec.safe_push (clob);
+
+      if (targetm.md_asm_adjust)
+	targetm.md_asm_adjust (output_rvec, input_rvec,
+			       constraints, clobber_rvec,
+			       clobbered_regs);
+
+      asm_op = body;
+      nclobbers = clobber_rvec.length ();
+      body = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (1 + nclobbers));
+
+      XVECEXP (body, 0, 0) = asm_op;
+      for (i = 0; i < nclobbers; i++)
+	XVECEXP (body, 0, i + 1) = gen_rtx_CLOBBER (VOIDmode, clobber_rvec[i]);
+    }
+
   emit_insn (body);
 }
 
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 231412)
+++ gcc/doc/extend.texi	(working copy)
@@ -7442,6 +7442,10 @@ all basic @code{asm} blocks use the assembler dial
 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.
+
 Here is an example of basic @code{asm} for i386:
 
 @example
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 231412)
+++ gcc/final.c	(working copy)
@@ -2565,6 +2565,10 @@ final_scan_insn (rtx_insn *insn, FILE *file, int o
 	  (*debug_hooks->source_line) (last_linenum, last_filename,
 				       last_discriminator, is_stmt);
 
+	if (GET_CODE (body) == PARALLEL
+	    && GET_CODE (XVECEXP (body, 0, 0)) == ASM_INPUT)
+	  body = XVECEXP (body, 0, 0);
+
 	if (GET_CODE (body) == ASM_INPUT)
 	  {
 	    const char *string = XSTR (body, 0);
Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 231412)
+++ gcc/gimple.c	(working copy)
@@ -2567,6 +2567,10 @@ gimple_asm_clobbers_memory_p (const gasm *stmt)
 	return true;
     }
 
+  /* Non-empty basic ASM implicitly clobbers memory.  */
+  if (gimple_asm_input_p (stmt) && strlen (gimple_asm_string (stmt)) != 0)
+    return true;
+
   return false;
 }
 
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	(revision 231412)
+++ gcc/recog.c	(working copy)
@@ -1470,6 +1470,8 @@ extract_asm_operands (rtx body)
 
 /* If BODY is an insn body that uses ASM_OPERANDS,
    return the number of operands (both input and output) in the insn.
+   If BODY is an insn body that uses ASM_INPUT with CLOBBERS in PARALLEL,
+   return 0.
    Otherwise return -1.  */
 
 int
@@ -1476,16 +1478,26 @@ int
 asm_noperands (const_rtx body)
 {
   rtx asm_op = extract_asm_operands (CONST_CAST_RTX (body));
-  int n_sets = 0;
+  int i, n_sets = 0;
 
   if (asm_op == NULL)
-    return -1;
+    {
+      if (GET_CODE (body) == PARALLEL && XVECLEN (body, 0) >= 2
+	  && GET_CODE (XVECEXP (body, 0, 0)) == ASM_INPUT)
+	{
+	  /* body is [(asm_input ...) (clobber (reg ...))...].  */
+	  for (i = XVECLEN (body, 0) - 1; i > 0; i--)
+	    if (GET_CODE (XVECEXP (body, 0, i)) != CLOBBER)
+	      return -1;
+	  return 0;
+	}
+      return -1;
+    }
 
   if (GET_CODE (body) == SET)
     n_sets = 1;
   else if (GET_CODE (body) == PARALLEL)
     {
-      int i;
       if (GET_CODE (XVECEXP (body, 0, 0)) == SET)
 	{
 	  /* Multiple output operands, or 1 output plus some clobbers:
@@ -1540,9 +1552,12 @@ asm_noperands (const_rtx body)
    the locations of the operands within the insn into the vector OPERAND_LOCS,
    and the constraints for the operands into CONSTRAINTS.
    Write the modes of the operands into MODES.
+   Write the location info into LOC.
    Return the assembler-template.
+   If BODY is an insn body that uses ASM_INPUT with CLOBBERS in PARALLEL,
+   return the basic assembly string.
 
-   If MODES, OPERAND_LOCS, CONSTRAINTS or OPERANDS is 0,
+   If LOC, MODES, OPERAND_LOCS, CONSTRAINTS or OPERANDS is 0,
    we don't store that info.  */
 
 const char *
@@ -1603,6 +1618,12 @@ decode_asm_operands (rtx body, rtx *operands, rtx
 	      }
 	    nbase = i;
 	  }
+	else if (GET_CODE (asmop) == ASM_INPUT)
+	  {
+	    if (loc)
+	      *loc = ASM_INPUT_SOURCE_LOCATION (asmop);
+	    return XSTR (asmop, 0);
+	  }
 	break;
       }
 
@@ -2244,7 +2265,8 @@ extract_insn (rtx_insn *insn)
     case PARALLEL:
       if ((GET_CODE (XVECEXP (body, 0, 0)) == SET
 	   && GET_CODE (SET_SRC (XVECEXP (body, 0, 0))) == ASM_OPERANDS)
-	  || GET_CODE (XVECEXP (body, 0, 0)) == ASM_OPERANDS)
+	  || GET_CODE (XVECEXP (body, 0, 0)) == ASM_OPERANDS
+	  || GET_CODE (XVECEXP (body, 0, 0)) == ASM_INPUT)
 	goto asm_insn;
       else
 	goto normal_insn;
Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c	(revision 231412)
+++ gcc/sched-deps.c	(working copy)
@@ -2748,11 +2748,14 @@ sched_analyze_2 (struct deps_desc *deps, rtx x, rt
 	/* Traditional and volatile asm instructions must be considered to use
 	   and clobber all hard registers, all pseudo-registers and all of
 	   memory.  So must TRAP_IF and UNSPEC_VOLATILE operations.
+	   But basic asm instructions with empty assembly string don't
+	   have to be barriers.
 
 	   Consider for instance a volatile asm that changes the fpu rounding
 	   mode.  An insn should not be moved across this even if it only uses
 	   pseudo-regs because it might give an incorrectly rounded result.  */
 	if ((code != ASM_OPERANDS || MEM_VOLATILE_P (x))
+	    && !(code == ASM_INPUT && strlen (XSTR (x, 0)) == 0)
 	    && !DEBUG_INSN_P (insn))
 	  reg_pending_barrier = TRUE_BARRIER;
 
Index: gcc/testsuite/gcc.target/i386/pr24414.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr24414.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr24414.c	(working copy)
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+int test;
+
+int
+main ()
+{
+  int x = test;
+  asm ("movl $1,test");
+  if (x + test != 1)
+    __builtin_trap ();
+  return 0;
+}