On 11/09/2011 02:15 PM, Andrew MacLeod wrote:
NAND patchup arithmetic was missing the 2 stage AND then NOT operation.
Instead it was falling into the same sequence as every other operation
and
trying to perform a binary operation on a NOT.
I managed to modify and existing testcase to trigger the bug without
requiring
a configuration with RTL checking enabled.
Bootstrapped on x86_64-unknown-linux-gnu with no new regressions (pending
completetion of test run)
OK for mainline?
ok, here's an adjusted patch to use positive tests for the NOT condition
Andrew
PR rtl-optimization/51040
* optabs.c (expand_atomic_fetch_op): Patchup code for NAND should be AND
followed by NOT.
* builtins.c (expand_builtin_atomic_fetch_op): Patchup code for NAND
should be AND followed by NOT.
* testsuite/gcc.dg/atomic-noinline[-aux].c: Test no-inline NAND and
patchup code.
Index: optabs.c
===================================================================
*** optabs.c (revision 181206)
--- optabs.c (working copy)
*************** expand_atomic_fetch_op (rtx target, rtx
*** 7875,7882 ****
Fetch_before == after REVERSE_OP val. */
if (!after)
code = optab.reverse_code;
! result = expand_simple_binop (mode, code, result, val, target, true,
! OPTAB_LIB_WIDEN);
return result;
}
}
--- 7875,7889 ----
Fetch_before == after REVERSE_OP val. */
if (!after)
code = optab.reverse_code;
! if (code == NOT)
! {
! result = expand_simple_binop (mode, AND, result, val, NULL_RTX,
! true, OPTAB_LIB_WIDEN);
! result = expand_simple_unop (mode, NOT, result, target, true);
! }
! else
! result = expand_simple_binop (mode, code, result, val, target,
! true, OPTAB_LIB_WIDEN);
return result;
}
}
Index: builtins.c
===================================================================
*** builtins.c (revision 181206)
--- builtins.c (working copy)
*************** expand_builtin_atomic_fetch_op (enum mac
*** 5460,5467 ****
/* Then issue the arithmetic correction to return the right result. */
if (!ignore)
! ret = expand_simple_binop (mode, code, ret, val, NULL_RTX, true,
! OPTAB_LIB_WIDEN);
return ret;
}
--- 5460,5476 ----
/* Then issue the arithmetic correction to return the right result. */
if (!ignore)
! {
! if (code == NOT)
! {
! ret = expand_simple_binop (mode, AND, ret, val, NULL_RTX, true,
! OPTAB_LIB_WIDEN);
! ret = expand_simple_unop (mode, NOT, ret, target, true);
! }
! else
! ret = expand_simple_binop (mode, code, ret, val, target, true,
! OPTAB_LIB_WIDEN);
! }
return ret;
}
Index: testsuite/gcc.dg/atomic-noinline.c
===================================================================
*** testsuite/gcc.dg/atomic-noinline.c (revision 181206)
--- testsuite/gcc.dg/atomic-noinline.c (working copy)
*************** main ()
*** 49,54 ****
--- 49,61 ----
if (__atomic_is_lock_free (4, 0) != 10)
abort ();
+ /* PR 51040 was caused by arithmetic code not patching up nand_fetch
properly
+ when used an an external function. Look for proper return value here.
*/
+ ac = 0x3C;
+ bc = __atomic_nand_fetch (&ac, 0x0f, __ATOMIC_RELAXED);
+ if (bc != ac)
+ abort ();
+
return 0;
}
Index: testsuite/gcc.dg/atomic-noinline-aux.c
===================================================================
*** testsuite/gcc.dg/atomic-noinline-aux.c (revision 181206)
--- testsuite/gcc.dg/atomic-noinline-aux.c (working copy)
*************** char __atomic_fetch_add_1 (char *p, char
*** 40,50 ****
*p = 1;
}
! short __atomic_fetch_add_2 (short *p, short v, short i)
{
*p = 1;
}
int __atomic_is_lock_free (int i, void *p)
{
return 10;
--- 40,63 ----
*p = 1;
}
! short __atomic_fetch_add_2 (short *p, short v, int i)
{
*p = 1;
}
+ /* Really perform a NAND. PR51040 showed incorrect calculation of a
+ non-inlined fetch_nand. */
+ unsigned char
+ __atomic_fetch_nand_1 (unsigned char *p, unsigned char v, int i)
+ {
+ unsigned char ret;
+
+ ret = *p;
+ *p = ~(*p & v);
+
+ return ret;
+ }
+
int __atomic_is_lock_free (int i, void *p)
{
return 10;
