[PATCH ARM] Inline expansion of __sync_* builtins.

[Richard Earnshaw]

On Tue, 2010-08-10 at 09:46 +0100, Marcus Shawcroft wrote:
> This patch inlines builtin synchronization primitives for ARM targets that
> provide load/store exclusive.
> 
> All __sync_* builtins applied to a 32bit object will be inlined on ARM
> targets which provide the ldrex/strex instructions, primitives applied to 8
> or 16 bit objects will be inlined on targets which provide
> ldrex{b,h}/strex{b,h} in all other cases we fall back to the existing
> library functions. The __sync_synchronize() builtin will emit an inline DMB
> or MCR instruction on targets where this is appropriate.
>
> The ldrex/strex synchronization loops are represented as opaque instruction
> blobs. This provides a guarantee that no memory reference can be inserted
> between the ldrex / strex pair.
> 
> Memory references in the ldrex*/strex* instructions are always indirect, we
> do not (yet) exploit offset addressing on targets where this is available.
> 
> 2010-08-10  Marcus Shawcroft  <marcus.shawcroft@arm.com>
> 
>         * config/arm/arm-protos.h (arm_expand_sync): New.
>         (arm_output_memory_barrier, arm_output_sync_insn): New.
>         (arm_sync_loop_insns): New.
>         * config/arm/arm.c (FL_ARCH7): New.
>         (FL_FOR_ARCH7): Include FL_ARCH7.
>         (arm_arch7): New.
>         (arm_print_operand): Support %C markup.
>         (arm_legitimize_sync_memory): New.
>         (arm_emit, arm_insn_count, arm_count, arm_output_asm_insn): New.
>         (arm_process_output_memory_barrier, arm_output_memory_barrier): New.
>         (arm_ldrex_suffix, arm_output_ldrex, arm_output_strex): New.
>         (arm_output_op2, arm_output_op3, arm_output_sync_loop): New.
>         (arm_get_sync_operand, FETCH_SYNC_OPERAND): New.
>         (arm_process_output_sync_insn, arm_output_sync_insn): New.
>         (arm_sync_loop_insns,arm_call_generator, arm_expand_sync): New.
>         * config/arm/arm.h (struct arm_sync_generator): New.
>         (TARGET_HAVE_DMB, TARGET_HAVE_DMB_MCR): New.
>         (TARGET_HAVE_LDREX, TARGET_HAVE_LDREXBHD): New.
>         * config/arm/arm.md: Include sync.md.
>         (UNSPEC_MEMORY_BARRIER): New.
>         (VUNSPEC_SYNC_COMPARE_AND_SWAP, VUNSPEC_SYNC_LOCK, VUNSPEC_SYNC_OP):
> New.
>         (VUNSPEC_SYNC_NEW_OP, VUNSPEC_SYNC_OLD_OP): New.
>         (sync_result, sync_memory, sync_required_value): New attributes.
>         (sync_new_value, sync_t1, sync_t2): Likewise.
>         (sync_release_barrier, sync_op): Likewise.
>         (length): Add logic to length attribute defintion to call
>         arm_sync_loop_insns when appropriate.
>         * config/arm/sync.md: New file.
> 

+/* Emit the memory barrier instruction, if any, provided by this
+   target to a specified emitter.  */
+static void
+arm_process_output_memory_barrier (emit_f emit, rtx *operands)
+{
+  if (TARGET_HAVE_DMB)
+    {
+      /* Note we issue a system level barrier. We should consider
+         issuing a inner shareabilty zone barrier here instead, ie.
+         "DMB ISH".  */
+      emit (0, "dmb\tsy", operands);
+    }
+  else if (TARGET_HAVE_DMB_MCR)
+    emit (0, "mcr\tp15, 0, r0, c7, c10, 5", operands);
+}

I'm uncomfortable with the fact that this can fall through and not emit
a barrier in some circumstances.  Shouldn't it fall back to a library
call?  Otherwise code built for a uni-processor may fail to run on a
later multi-processor core.

Apart from this, I can't see any other problems.

R.