This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR opt/5172: Store motion vs. asm_operands



This patches fixes PR optimize/5172.  Analysis of the problem reveals
that the source of the problem is in GCSE's store motion optimization.
Both strcpy(one,"") function calls in Andreas' bug report are correctly
inlined into the equivalent one[0]='\0'.  However, the first is
incorrectly deleted by store_motion as it believes that there are no
uses (loads that alias with the store) between the two.  In reality,
the volatile asm statement between these two stores, accesses that
memory.

My understanding is that both volatile and non-volatile asm statements
may legitimately read from anywhere in memory (and that volatile reflects
the asm's ability to write to, not read from, memory).

This then means that GCC should assume that all stores are killed by
asm_operands, so that it commits all pending changes to memory before
executing the user's assembly code.  The two line fix is to have
find_loads return 1 whenever it encounters ASM_OPERANDS.

I've also created a testcase for the problem for gcc.c-torture/execute.
This fails at -O3 on current mainline, but passes with the attached patch.
I placed the test in gcc.c-torture execute because the problem should
be visible on all platforms (provided that the appropriate __asm__ for
"res = *ptr" is added to the test).


Tested by "make bootstrap" (without ada) and "make -j check" (at the
top level) on i686-pc-linux-gnu with no changes in the regressions.


Ok for 3.1 and to close PR opt/5172 in GNATS?



2002-01-15  Roger Sayle  <roger@eyesopen.com>
	* gcse.c (find_loads): ASM_OPERANDS can potentially read silently
	from anywhere in memory, and should alias with all stores.  This
	prevents store motion across asm()s, and fixes PR opt/5172.
	* testsuite/gcc.c-torture/execute/20020115-1.c: New testcase.


diff -c3pr gcc/gcc/gcse.c patch9/gcc/gcse.c
*** gcc/gcc/gcse.c	Fri Jan 11 18:18:42 2002
--- patch9/gcc/gcse.c	Tue Jan 15 21:18:47 2002
*************** find_loads (x, store_pattern)
*** 6456,6461 ****
--- 6456,6464 ----
    if (GET_CODE (x) == SET)
      x = SET_SRC (x);

+   if (GET_CODE (x) == ASM_OPERANDS)
+     return 1;
+
    if (GET_CODE (x) == MEM)
      {
        if (load_kills_store (x, store_pattern))
*** /dev/null	Mon Dec 31 21:00:00 1979
--- 20020115-1.c	Wed Jan 16 00:27:45 2002
***************
*** 0 ****
--- 1,41 ----
+ /* Copyright (C) 2002  Free Software Foundation.
+
+    Check that store motion doesn't move stores across asm statements.
+    Based upon code by Andreas Jaeger in PR opt/5172.
+
+    Written by Roger Sayle, 01/15/2002.  */
+
+ extern void abort (void);
+ extern char *strcpy (char *, const char *);
+
+ char one[8];
+
+ static int
+ load (const char *ptr)
+ {
+   int res;
+
+ #ifdef __i386__
+   __asm__ __volatile__ ("movzbl (%1), %0" : "=a" (res) : "0" (ptr) );
+ #else
+   res = *ptr;
+ #endif
+   return res;
+ }
+
+ int
+ main ()
+ {
+   int res;
+
+   strcpy (one, "abc");
+   one[0] = '\0';
+
+   res = load (one);
+
+   one[0] = '\0';
+   if (res != '\0')
+     abort ();
+   return 0;
+ }
+

Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-438-3470


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]