Re: [PATCH v4] RISC-V: Add support for inlining subword atomic operations

[Kito Cheng <kito.cheng@gmail.com>]

LGTM with minor comments, it's time to move forward, thanks Patrick and Palmer.

> +
> +void
> +riscv_subword_address (rtx mem, rtx *aligned_mem, rtx *shift, rtx *mask,
> +                      rtx *not_mask)
> +{
> +  /* Align the memory addess to a word.  */
> +  rtx addr = force_reg (Pmode, XEXP (mem, 0));
> +
> +  rtx aligned_addr = gen_reg_rtx (Pmode);
> +  emit_move_insn (aligned_addr,  gen_rtx_AND (Pmode, addr,
> +                                             gen_int_mode (-4, Pmode)));
> +
> +  *aligned_mem = change_address (mem, SImode, aligned_addr);
> +
> +  /* Calculate the shift amount.  */
> +  *shift = gen_reg_rtx (SImode);

Already allocated reg_rtx outside, this line could be removed.

> +  emit_move_insn (*shift, gen_rtx_AND (SImode, gen_lowpart (SImode, addr),
> +                                     gen_int_mode (3, SImode)));
> +  emit_move_insn (*shift, gen_rtx_ASHIFT (SImode, *shift,
> +                                        gen_int_mode(3, SImode)));
> +
> +  /* Calculate the mask.  */
> +  int unshifted_mask;
> +  if (GET_MODE (mem) == QImode)
> +    unshifted_mask = 0xFF;
> +  else
> +    unshifted_mask = 0xFFFF;
> +
> +  rtx mask_reg = gen_reg_rtx (SImode);

Ditto.

> @@ -152,6 +348,128 @@
>    DONE;
>  })
>
> +(define_expand "atomic_compare_and_swap<mode>"
> +  [(match_operand:SI 0 "register_operand" "")    ;; bool output
> +   (match_operand:SHORT 1 "register_operand" "") ;; val output
> +   (match_operand:SHORT 2 "memory_operand" "")   ;; memory
> +   (match_operand:SHORT 3 "reg_or_0_operand" "") ;; expected value
> +   (match_operand:SHORT 4 "reg_or_0_operand" "") ;; desired value
> +   (match_operand:SI 5 "const_int_operand" "")   ;; is_weak
> +   (match_operand:SI 6 "const_int_operand" "")   ;; mod_s
> +   (match_operand:SI 7 "const_int_operand" "")]  ;; mod_f
> +  "TARGET_ATOMIC && TARGET_INLINE_SUBWORD_ATOMIC"
> +{
> +  emit_insn (gen_atomic_cas_value_strong<mode> (operands[1], operands[2],
> +                                               operands[3], operands[4],
> +                                               operands[6], operands[7]));
> +
> +  rtx val = gen_reg_rtx (SImode);
> +  if (operands[1] != const0_rtx)
> +    emit_insn (gen_rtx_SET (val, gen_rtx_SIGN_EXTEND (SImode, operands[1])));
> +  else
> +    emit_insn (gen_rtx_SET (val, const0_rtx));

nit: emit_move_insn rather than emit_insn + gen_rtx_SET

> +
> +  rtx exp = gen_reg_rtx (SImode);
> +  if (operands[3] != const0_rtx)
> +    emit_insn (gen_rtx_SET (exp, gen_rtx_SIGN_EXTEND (SImode, operands[3])));
> +  else
> +    emit_insn (gen_rtx_SET (exp, const0_rtx));

nit: emit_move_insn rather than emit_insn + gen_rtx_SET

> +
> +  rtx compare = val;
> +  if (exp != const0_rtx)
> +    {
> +      rtx difference = gen_rtx_MINUS (SImode, val, exp);
> +      compare = gen_reg_rtx (SImode);
> +      emit_insn (gen_rtx_SET (compare, difference));

nit: emit_move_insn rather than emit_insn + gen_rtx_SET

> +    }
> +
> +  if (word_mode != SImode)
> +    {
> +      rtx reg = gen_reg_rtx (word_mode);
> +      emit_insn (gen_rtx_SET (reg, gen_rtx_SIGN_EXTEND (word_mode, compare)));

nit: emit_move_insn rather than emit_insn + gen_rtx_SET


> +      compare = reg;
> +    }
> +
> +  emit_insn (gen_rtx_SET (operands[0], gen_rtx_EQ (SImode, compare, const0_rtx)));

nit: emit_move_insn rather than emit_insn + gen_rtx_SET