This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Heads-up, PR53273: testsuite separation and dilution problem. Fix for PR53272
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Tue, 8 May 2012 05:39:51 +0200
- Subject: 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