Re: [cxx-mem-model] compare_exchange implementation

[Richard Henderson <rth at redhat dot com>]

> !   weak = expand_expr (CALL_EXPR_ARG (exp, 3), NULL_RTX, ptr_mode,
> ! 		      EXPAND_NORMAL);
> ! 
> !   if (weak != const0_rtx && weak != const1_rtx)
> !     {
> !       error ("strong/weak parameter must be 0 or 1 for %<__atomic_compare_exchange%>");
> !       return NULL_RTX;
> !     }

Really?  Are you even allowed to do that, like unto the variable memory model parameters?

> +       /* Emit the compare_and_swap.  */
>         create_output_operand (&ops[0], target, QImode);
>         create_output_operand (&ops[1], mem, mode);
> !       create_output_operand (&ops[2], current_val, mode);
> !       create_convert_operand_to (&ops[3], expected_val, mode, true);
> !       create_convert_operand_to (&ops[4], desired, mode, true);
> !       create_integer_operand (&ops[5], success);
> !       expand_insn (icode, 6, ops);

The mem operand must use create_fixed_operand, as with all of the other atomic builtins.
I strongly suggest swapping op 1 and op2, so that all of the "real" outputs come first.

I suggested on IRC that you examine the mode of the boolean output operand, and not
hard-code QImode.  Most targets will produce a word-sized boolean output that need not
be zero-extended.

> !       emit_cmp_and_jump_insns (ops[0].value, const0_rtx, NE, const0_rtx,
> ! 			       QImode, 1, true_label);
> ! 
> !       /* if not successful copy expected_val into *expected and issue the 
> ! 	 failure fence.  */
> !       emit_move_insn (gen_rtx_MEM (mode, expected), current_val);
> !       expand_builtin_mem_thread_fence (failure);
> ! 
> !       /* If no success fence is required, we're done. Otherwise jump around the
> !          code for TRUE and emitthe fence.  */
> !       if (true_label != done_label)
> !         {
> ! 	  emit_jump_insn (gen_jump (done_label));
> ! 
> ! 	  emit_label (true_label);
> ! 	  if (success != MEMMODEL_RELAXED && success != MEMMODEL_RELEASE)
> ! 	    expand_builtin_mem_thread_fence (success);
> ! 	}

Really?  We can do better than this.  In particular, by leaving it up to the target.
In the x86 case, cmpxchg is a full barrier.  Always.  No need for any followup barriers.

The ll/sc targets are going to generate code that looks like

	start-barrier
 restart:
	ll	curval, mem
	mov	t, 0
	cmp	curval, oldval
	bne	fail
	mov	t, newval
	sc	t, mem
	beq	t, restart	// strong-version only
 fail:
	end-barrier

  // outputs: t = 1/0 for success/failure
  //          curval = the current value of the memory, to be stored back into expected

where the position of fail: before or after the end-barrier depends on the relative
strength of the memory models.  No one is going to want to insert an extra jump 
around a barrier, especially for a not-expected failure path.

We should encode the two memory model parameters and the weak parameter into a single
CONST_INT operand.  Probably with some macros in optabs.h to extract those parameters
in the targets.


r~