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]

Heads-up, PR53273: testsuite separation and dilution problem. Fix for PR53272


The problem was spotted while fixing PR53272, a target bug with
crisv32-* involving the error-prone notice_update_cc function.

When wrapping up the test-case to use as a run-test, adding main
and auxiliary functions to the reduced test-case unexpectedly
made the bug go away.  This despite all functions (except main)
being decorated with noinline, noclone and the special marker
asm ("") ad finitum.  See below: putting the two-file test-case
in a single file causes different code for the
rtc_update_irq_enable function in the .expand stage already.
That REALLY shouldn't happen.  I hope this is just a bug and not
as it's supposed to be, but this is not the first time I notice
this general problem, hence this rant and PR53273:

It is, and you don't understand how this can be a problem, or
think I should just add the brand new function attribute
nofrobnicate?  Well, having to add two files for each test-case
is enough of a problem on its own.  The bigger problems is the
integrity of test-cases: there's no way to know that a future
optimization doesn't see through those separated files (like, an
LTO that is enabled always).

There *must* be a future-proof way to write test-cases marking
where cross-function optimizations should not happen.  If it's
implemented as function-attribute-nofrobnicate so be it, but it
must not be limited to only the optimizations in place today.
Otherwise, some new middle-end generic optimization will
optimize away the test-case (and always return success), most
likely eliminating the point of the test.  When the point of the
test-case is to cover code in a port or lower levels, optimizing
away the test-cases opens up for bugs to silently creep in; the
original bug or bugs in the functionality being covered.  This
optimization-limiting mechanism really should, almost-must, work
within a single file.  In a (semi-)perfect world, someone would
interate over the testsuite, adding such attributes to the
existing tests; I fear a lot of those that don't use "noclone"
are silently already just eliminated to "exit (0)".  We're on a
slippery slope here: it started with having to add "noinline"
attributes, then "noclone" attributes, then the asm("") marker.
Now that doesn't work anymore either.  Can we just have a way to
limit those pesky cross-function optimizations and all their kin
once and for all?

Ok, enough ranting.  If anyone knows off-hand why the code would
differ, feel free to add to PR53273, else I'll eventually
analyze it.  It might just be a minor bug after all; like some
static branch prediction not being cleared when seeing a
noinline-marked function.

The test-case below and patch will be committed to trunk and the
4.7 branch as soon as testing for crisv32-elf finishes.

gcc/testsuite:
	PR target/53272
	* gcc.dg/torture/pr53272-1.c, gcc.dg/torture/pr53272-2.c: New test.

gcc:
	PR target/53272
	* config/cris/cris.c (cris_normal_notice_update_cc): For TARGET_V32,
	when a constant source operand matches an "I" constraint, the "no
	CC0 change" applies to a register-destination only, not a
	strict_low_part-destination.


--- /dev/null	Tue Oct 29 15:57:07 2002
+++ gcc/testsuite/gcc.dg/torture/pr53272-1.c	Tue May  8 03:07:52 2012
@@ -0,0 +1,39 @@
+/* { dg-do run } */
+/* { dg-additional-sources "pr53272-2.c" } */
+struct rtc_class_ops {
+ int (*f)(void *, unsigned int enabled);
+};
+
+struct rtc_device
+{
+ void *owner;
+ const struct rtc_class_ops *ops;
+ int ops_lock;
+};
+
+__attribute__ ((__noinline__, __noclone__))
+extern int foo(void *);
+__attribute__ ((__noinline__, __noclone__))
+extern void foobar(void *);
+
+__attribute__ ((__noinline__, __noclone__))
+int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
+{
+ int err;
+ asm volatile ("");
+
+ err = foo(&rtc->ops_lock);
+
+ if (err)
+  return err;
+
+ if (!rtc->ops)
+  err = -19;
+ else if (!rtc->ops->f)
+  err = -22;
+ else
+  err = rtc->ops->f(rtc->owner, enabled);
+
+ foobar(&rtc->ops_lock);
+ return err;
+}
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ gcc/testsuite/gcc.dg/torture/pr53272-2.c	Tue May  8 02:23:21 2012
@@ -0,0 +1,39 @@
+__attribute__ ((__noinline__, __noclone__))
+int foo(void *x)
+{
+  asm ("");
+  return *(int *) x != 42;
+}
+
+__attribute__ ((__noinline__, __noclone__))
+void foobar(void *x)
+{
+  asm ("");
+  if (foo(x))
+    __builtin_abort();
+}
+
+struct rtc_class_ops {
+ int (*f)(void *, unsigned int enabled);
+};
+
+struct rtc_device
+{
+ void *owner;
+ struct rtc_class_ops *ops;
+ int ops_lock;
+};
+
+extern __attribute__ ((__noinline__, __noclone__))
+int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int);
+
+int main(void)
+{
+ struct rtc_class_ops ops = {(void *) 0};
+  struct rtc_device dev1 = {0, &ops, 42};
+
+  if (rtc_update_irq_enable (&dev1, 1) != -22)
+    __builtin_abort ();
+
+  __builtin_exit (0);
+}

Index: gcc/config/cris/cris.c
===================================================================
--- gcc/config/cris/cris.c	(revision 187269)
+++ gcc/config/cris/cris.c	(working copy)
@@ -1695,6 +1695,7 @@ cris_normal_notice_update_cc (rtx exp, r
 		       && (REGNO (SET_SRC (exp))
 			   > CRIS_LAST_GENERAL_REGISTER))
 		   || (TARGET_V32
+		       && REG_P (SET_DEST (exp))
 		       && satisfies_constraint_I (SET_SRC (exp))))
 	    {
 	      /* There's no CC0 change for this case.  Just check

brgds, H-P


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