[Patch] The comparison in a compare exchange should not take place in VOIDmode

[Patch] The comparison in a compare exchange should not take place in VOIDmode

[James Greenhalgh]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]


Hi,

I was playing with some changes to costs for some immediate values in
AArch64, and ended up tripping an ICE in some builtin expansion code.

The ICE looked like this (some pruning of the boring bits...)

format.c: In function '_gfortran_caf_atomic_cas':
format.c:13:3: internal compiler error: in int_mode_for_mode, at stor-layout.c:443
   (void)__atomic_compare_exchange_n(atom, (uint32_t *)old, *(uint32_t *)new_val,
   ^
0xa5e2b0 int_mode_for_mode(machine_mode)
	/work/gcc-dev/src/gcc/gcc/stor-layout.c:443
0x74cd78 emit_move_via_integer
	/work/gcc-dev/src/gcc/gcc/expr.c:3174
   ...  <snip>
0x73e5cf force_reg(machine_mode, rtx_def*)
	/work/gcc-dev/src/gcc/gcc/explow.c:643
0x997458 prepare_cmp_insn
	/work/gcc-dev/src/gcc/gcc/optabs.c:4076
   ...  <snip>

0x63e4cf expand_builtin_atomic_compare_exchange
	/work/gcc-dev/src/gcc/gcc/builtins.c:5479
   ...  <snip>

The final symptom can be seen in force_reg, where it is asked to provide

  (set:VOID
    (reg:SI) (const_int 0))

And gives up. We end up here, as prepare_cmp_insn tries to force "expensive"
constants to a register, using the mode provided to it from further up
the call stack...

Which takes us to expand_builtin_atomic_compare_exchange, which
explicitly uses a VOIDmode for the comparison. This looks wrong to
me, is a VOIDmode comparison ever valid?

This patch fixes the issue by performing the comparison in the mode
of the cmp-exchange target. It seems sensible to me, but may be
nonsense, so I defer to others' opinions.

Bootstrapped on AArch64/x86_64.

OK?

Thanks,
James

---
2015-06-04  James Greenhalgh  <james.greenhalgh@arm.com>

	* builtins.c (expand_builtin_atomic_compare_exchange): Call
	emit_cmp_and_jump_insns with the mode of target.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Patch-The-comparison-in-a-compare-exchange-should-no.patch --]
[-- Type: text/x-patch;  name=0001-Patch-The-comparison-in-a-compare-exchange-should-no.patch, Size: 616 bytes --]

diff --git a/gcc/builtins.c b/gcc/builtins.c
index ec31e0e..9f9b735 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5476,7 +5476,8 @@ expand_builtin_atomic_compare_exchange (machine_mode mode, tree exp,
      the normal case where EXPECT is totally private, i.e. a register.  At
      which point the store can be unconditional.  */
   label = gen_label_rtx ();
-  emit_cmp_and_jump_insns (target, const0_rtx, NE, NULL, VOIDmode, 1, label);
+  emit_cmp_and_jump_insns (target, const0_rtx, NE, NULL,
+			   GET_MODE (target), 1, label);
   emit_move_insn (expect, oldval);
   emit_label (label);
 

Re: [Patch] The comparison in a compare exchange should not take place in VOIDmode

[Richard Henderson]

On 06/04/2015 07:19 AM, James Greenhalgh wrote:
> 2015-06-04  James Greenhalgh  <james.greenhalgh@arm.com>
> 
> 	* builtins.c (expand_builtin_atomic_compare_exchange): Call
> 	emit_cmp_and_jump_insns with the mode of target.

Ok.


r~