Maxim Kuvyrkov <ma...@codesourcery.com> writes:
> diff --git a/gcc/config/mips/sync.md b/gcc/config/mips/sync.md
> index 604aefa..ac953b5 100644
> --- a/gcc/config/mips/sync.md
> +++ b/gcc/config/mips/sync.md
> @@ -607,10 +607,32 @@
>     (match_operand:GPR 1 "memory_operand")
>     (match_operand:GPR 2 "arith_operand")
>     (match_operand:SI 3 "const_int_operand")]
> -  "GENERATE_LL_SC"
> +  "GENERATE_LL_SC || ISA_HAS_SWAP"
>  {
> +  if (!ISA_HAS_SWAP)
>      emit_insn (gen_atomic_exchange<mode>_llsc (operands[0], operands[1],
>                                              operands[2], operands[3]));
> +  else

Please swap this round so that the ISA_HAS_SWAP stuff comes first.
The LLSC code then remains the fallback if new variations are added.

> +    {
> +      rtx addr;
> +
> +      gcc_assert (MEM_P (operands[1]));
> +      addr = XEXP (operands[1], 0);
> +      if (!REG_P (addr) && can_create_pseudo_p ())
> +        /* Workaround a reload bug that hits (lo_sum (reg) (symbol_ref))
> +        addresses.  Spill the address to a register upfront to simplify
> +        reload's job.  */
> +        addr = force_reg (GET_MODE (addr), addr);

If there's a reload bug here, let's fix it.  But...

> @@ -631,15 +653,52 @@
>     (set_attr "sync_insn1_op2" "2")
>     (set_attr "sync_memmodel" "3")])
>  
> +;; Swap/ldadd instruction accepts only register, no offset, for the address.

["SWAP accepts only register addresses."]

> +;; Therefore, we spell out the MEM verbatim and constrain its address to "d".
> +;; XLP issues implicit sync for swap/ldadd, so no need for an explicit one.

...we should instead define a memory predicate/constraint pair that only
allows register addresses.  Then the expand code would be:

    if (!mem_reg_operand (operands[1], <MODE>mode))
      {
        addr = force_reg (Pmode, XEXP (operands[1], 0));
        operands[1] = replace_equiv_address (operands[1], addr);
      }

> +(define_insn "atomic_exchange<GPR:mode>_swap_<P:mode>"
> +  [(set (match_operand:GPR 0 "register_operand" "=d")
> +     (unspec_volatile:GPR
> +      [(mem:GPR (match_operand:P 1 "address_operand" "d"))]
> +      UNSPEC_ATOMIC_EXCHANGE))
> +   (set (mem:GPR (match_dup 1))
> +     (unspec_volatile:GPR [(match_operand:GPR 2 "arith_operand" "0")]

Should be "register_operand", with a force_reg in the expander.
Same kind of comments for LDADD.

Richard

Reply via email to