Hi Dave,
The nit-picky bit - There are still a number of formatting issues with
your patch . Could you run your patch through
contrib/check_GNU_style.sh and correct these. These are typically
around problems with the number of spaces between a full stop and the
end of comment, lines with trailing whitespaces and a few lines with
number of characters > 80. Thanks.
>@@ -23590,82 +23637,142 @@ arm_output_sync_loop (emit_f emit,
>
>+ else
>+ {
>+ /* Silence false potentially unused warning */
>+ required_value_lo = NULL;
>+ required_value_hi = NULL;
>+ }
>
s/NULL/NULL_RTX in a number of places in arm.c
>@@ -23516,14 +23530,41 @@ arm_output_strex (emit_f emit,
> rtx value,
> rtx memory)
> {
>- const char *suffix = arm_ldrex_suffix (mode);
>- rtx operands[3];
>+ rtx operands[4];
>
> operands[0] = result;
> operands[1] = value;
>- operands[2] = memory;
>- arm_output_asm_insn (emit, 0, operands, "strex%s%s\t%%0, %%1, %%C2", suffix,
>- cc);
>+ if (mode != DImode)
>+ {
>+ const char *suffix = arm_ldrex_suffix (mode);
>+ operands[2] = memory;
>+ arm_output_asm_insn (emit, 0, operands, "strex%s%s\t%%0, %%1, %%C2",
>+ suffix, cc);
>+ }
>+ else
>+ {
>+ /* The restrictions on target registers in ARM mode are that the two
>+ registers are consecutive and the first one is even; Thumb is
>+ actually more flexible, but DI should give us this anyway.
>+ Note that the 1st register always gets the lowest word in memory. */
>+ gcc_assert ((REGNO (value) & 1) == 0);
>+ operands[2] = gen_rtx_REG (SImode, REGNO (value) + 1);
>+ operands[3] = memory;
>+ arm_output_asm_insn (emit, 0, operands, "strexd%s\t%%0, %%1, %%2, %%C3",
>+ cc);
>+ }
>
The restriction is actually mandatory for ARM state only and thus I'm fine
with this assertion being true only in ARM state.
I don't like duplicating the tests from gcc.dg into gcc.target/arm.
If you wanted to check for assembler output specific to a target you could
add your own conditions to the test in gcc.dg and conditionalize that on
target arm_eabi
Something like :
{ dg-final { scan-assembler "ldrexd\t"} {target arm_eabi}} } .
I would like a testsuite maintainer to comment on the testsuite infrastructure
bits as well but I have a few comments below .
>> +# Return 1 if the target supports atomic operations on "long long" and can
>> actually
>+# execute them
>+# So far only put checks in for ARM, others may want to add their own
>+proc check_effective_target_sync_longlong { } {
>+ return [check_runtime sync_longlong_runtime {
>+ #include <stdlib.h>
>+ int main()
>+ {
>+ long long l1;
>+
>+ if (sizeof(long long)!=8)
Space between ')' and ! as well as '=' and 8
>+ exit(1);
>+
>+ #ifdef __arm__
Why is this checking only for ARM state ? We could have ldrexd in T2 as
well ?
Otherwise the functionality looks good to me. Can you confirm that
this has survived a testrun for v7-a thumb2 and v7-a arm state ?
cheers
Ramana
