| Message ID | 20140207115837.GA25280@bart |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 07, 2014 at 12:58:37PM +0100, Andreas Krebbel wrote: > 2014-02-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> > > * optabs.c (expand_atomic_compare_and_swap): Allow expander to > fail. Ok. > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -7383,12 +7383,13 @@ expand_atomic_compare_and_swap (rtx *ptarget_bool, rtx *ptarget_oval, > create_integer_operand (&ops[5], is_weak); > create_integer_operand (&ops[6], succ_model); > create_integer_operand (&ops[7], fail_model); > - expand_insn (icode, 8, ops); > - > - /* Return success/failure. */ > - target_bool = ops[0].value; > - target_oval = ops[1].value; > - goto success; > + if (maybe_expand_insn (icode, 8, ops)) > + { > + /* Return success/failure. */ > + target_bool = ops[0].value; > + target_oval = ops[1].value; > + goto success; > + } > } > > /* Otherwise fall back to the original __sync_val_compare_and_swap Jakub
On Fri, 7 Feb 2014, Andreas Krebbel wrote: > Hi, > > on S/390 128 bit atomic operations are not allowed for misaligned > operands. The expanders are supposed to FAIL in that case. While it > works for the other routines atomic_load/store it does not work > currently during compare and swap expansion. > > The patch just turns an expand_insn into maybe_expand_insn to allow > other alternatives to be chosen when the expander fails. I expect this is correct for cases when a misaligned operand gets through that the front end thought was aligned - I doubt it's possible to avoid all such cases where misalignment only becomes visible here. But it also suggests there may well be problems in other places. In particular: * c-common.c:resolve_overloaded_atomic_* may need to check if the alignment is sufficient and generate library calls if not. (This would need to be conservative about alignment, like C11 _Alignof, for cases such as x86 long long in structures; see <http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02862.html>.) * It's possible your target should also increase alignment of relevant types when _Atomic-qualified. (But increasing to an alignment greater than is returned by malloc may not be a good idea.)
diff --git a/gcc/optabs.c b/gcc/optabs.c index e36fd13..cec25a4 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -7383,12 +7383,13 @@ expand_atomic_compare_and_swap (rtx *ptarget_bool, rtx *ptarget_oval, create_integer_operand (&ops[5], is_weak); create_integer_operand (&ops[6], succ_model); create_integer_operand (&ops[7], fail_model); - expand_insn (icode, 8, ops); - - /* Return success/failure. */ - target_bool = ops[0].value; - target_oval = ops[1].value; - goto success; + if (maybe_expand_insn (icode, 8, ops)) + { + /* Return success/failure. */ + target_bool = ops[0].value; + target_oval = ops[1].value; + goto success; + } } /* Otherwise fall back to the original __sync_val_compare_and_swap
Hi, on S/390 128 bit atomic operations are not allowed for misaligned operands. The expanders are supposed to FAIL in that case. While it works for the other routines atomic_load/store it does not work currently during compare and swap expansion. The patch just turns an expand_insn into maybe_expand_insn to allow other alternatives to be chosen when the expander fails. The patch is required to fix many c11-atomic* testcases on S/390. gcc.dg/atomic/c11-atomic-exec-5.c still fails since TARGET_ATOMIC_ASSIGN_EXPAND_FENV is not defined yet. Ok for mainline? 2014-02-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> * optabs.c (expand_atomic_compare_and_swap): Allow expander to fail. commit 63b1953fadae985ef8302159dacbfc8bf541ec1f Author: Andreas Krebbel <krebbel@linux.vnet.ibm.com> Date: Fri Feb 7 12:50:50 2014 +0100 optabs: Allow compare and swap expanders to fail