Pushed with a small correction: the changelog section was not
correctly formatted.

//Claudiu

On Tue, Nov 4, 2025 at 4:12 PM Luis Silva <[email protected]> wrote:
>
> From: Loeka Rogge <[email protected]>
>
> Address PR target/120375
>
> Devices without a barrel shifter end up using a sequence of
> instructions. These can use the condition codes and/or loop count
> register, so those need to be marked as 'clobbered'. These clobbers were
> previously added only after split1, which is too late. This patch adds
> these clobbers from the beginning, in the define_expand.
>
> Previously, define_insn_and_split *<insn>si3_nobs would match any shift or
> rotate instruction and would generate the necessary patterns to emulate a
> barrel shifter, but it did not have any output assembly for itself.
> In many cases this would create a loop with parallel clobbers. This pattern
> is then matched by the <insn>si3_loop pattern.
>
> In the no-barrel-shifter.c test tree code:
>
> ;; no-barrel-shifter.c:9:     int sign = (x >> 31) & 1;
> _2 = x.0_1 >> 31;
>
> in the expand pass becomes the following pattern that matches *lshrsi3_nobs:
>
> (insn 18 17 19 4 (set (reg:SI 153 [ _2 ])
>         (lshiftrt:SI (reg/v:SI 156 [ x ])
>             (const_int 31 [0x1f]))) "test2.c":9:24 -1
>      (nil))
>
> This pattern misses the necessary clobbers and remains untouched until the
> split1 pass. Together with the later branch it becomes
>
> ;; no-barrel-shifter.c:9:     int sign = (x >> 31) & 1;
>         add.f   0,r0,r0
> ;; no-barrel-shifter.c:14:     if (mag == 0x7f800000)
>         beq.d   .L8
> ;; no-barrel-shifter.c:9:     int sign = (x >> 31) & 1;
>         rlc     r0,0
>
> Leading to an issue: the add.f instructions overwrites CC but beq expects
> CC to contain an earlier value indicating mag == 0x7f800000.
>
> Now, these are combined in define_insn_and_split <insn>si3_loop that is
> explicitly emitted in the define_expand and already contains the clobbers.
> This can then be split into another pattern or remain the loop pattern.
>
> In the expand pass, the same example now becomes:
>
> (insn 18 17 19 4 (parallel [
>             (set (reg:SI 153 [ _2 ])
>                 (lshiftrt:SI (reg/v:SI 156 [ x ])
>                     (const_int 31 [0x1f])))
>             (clobber (reg:SI 60 lp_count))
>             (clobber (reg:CC 61 cc))
>         ]) "test2.c":9:24 -1
>      (nil))
>
> Because the correct clobbers are now taken into account, the branch condition
> is reevaluated by using breq instead of br.
>
> ;; no-barrel-shifter.c:9:     int sign = (x >> 31) & 1;
>         add.f   0,r0,r0
>         rlc     r0,0
> ;; no-barrel-shifter.c:14:     if (mag == 0x7f800000)
>         breq    r2,2139095040,.L8
>
> Regtested for arc.
>
> gcc/ChangeLog:
>
>         * config/arc/arc.md (*<insn>si3_nobs): merged with <insn>si3_loop
>         (<insn>si3_loop): splits to relevant pattern or emits loop assembly
>         (<insn>si3_cnt1_clobber): Removes clobber for shift or rotate by 
> const1
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/arc/no-barrel-shifter.c: New test.
>
> Co-authored-by: Keith Packard <[email protected]>
> Signed-off-by: Loeka Rogge <[email protected]>
> ---
>  gcc/config/arc/arc.md                         | 60 +++++++++++--------
>  .../gcc.target/arc/no-barrel-shifter.c        | 19 ++++++
>  2 files changed, 54 insertions(+), 25 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arc/no-barrel-shifter.c
>
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index dd3d47e2e8..f85d462907 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -3554,7 +3554,14 @@ archs4x, archs4xd"
>    [(set (match_operand:SI 0 "dest_reg_operand" "")
>         (ANY_SHIFT_ROTATE:SI (match_operand:SI 1 "register_operand" "")
>                              (match_operand:SI 2 "nonmemory_operand" "")))]
> -  "")
> +  ""
> +{
> +  if (!TARGET_BARREL_SHIFTER && operands[2] != const1_rtx)
> +    {
> +      emit_insn (gen_<insn>si3_loop (operands[0], operands[1], operands[2]));
> +      DONE;
> +    }
> +})
>
>  ; asl, asr, lsr patterns:
>  ; There is no point in including an 'I' alternative since only the lowest 5
> @@ -3653,35 +3660,23 @@ archs4x, archs4xd"
>    [(set_attr "type" "shift")
>     (set_attr "length" "8")])
>
> -(define_insn_and_split "*<insn>si3_nobs"
> -  [(set (match_operand:SI 0 "dest_reg_operand")
> -       (ANY_SHIFT_ROTATE:SI (match_operand:SI 1 "register_operand")
> -                            (match_operand:SI 2 "nonmemory_operand")))]
> +(define_insn_and_split "<insn>si3_loop"
> +  [(set (match_operand:SI 0 "dest_reg_operand" "=r,r")
> +  (ANY_SHIFT_ROTATE:SI (match_operand:SI 1 "register_operand" "0,0")
> +        (match_operand:SI 2 "nonmemory_operand" "rn,Cal")))
> +  (clobber (reg:SI LP_COUNT))
> +  (clobber (reg:CC CC_REG))]
>    "!TARGET_BARREL_SHIFTER
> -   && operands[2] != const1_rtx
> -   && arc_pre_reload_split ()"
> -  "#"
> -  "&& 1"
> +  && operands[2] != const1_rtx"
> +  "* return output_shift_loop (<CODE>, operands);"
> +  "&& arc_pre_reload_split ()"
>    [(const_int 0)]
>  {
>    arc_split_<insn> (operands);
>    DONE;
> -})
> -
> -;; <ANY_SHIFT_ROTATE>si3_loop appears after <ANY_SHIFT_ROTATE>si3_nobs
> -(define_insn "<insn>si3_loop"
> -  [(set (match_operand:SI 0 "dest_reg_operand" "=r,r")
> -       (ANY_SHIFT_ROTATE:SI
> -         (match_operand:SI 1 "register_operand" "0,0")
> -         (match_operand:SI 2 "nonmemory_operand" "rn,Cal")))
> -   (clobber (reg:SI LP_COUNT))
> -   (clobber (reg:CC CC_REG))
> -  ]
> -  "!TARGET_BARREL_SHIFTER
> -   && operands[2] != const1_rtx"
> -  "* return output_shift_loop (<CODE>, operands);"
> -  [(set_attr "type" "shift")
> -   (set_attr "length" "16,20")])
> +}
> +[(set_attr "type" "shift")
> + (set_attr "length" "16,20")])
>
>  ;; DImode shifts
>
> @@ -6413,6 +6408,21 @@ archs4x, archs4xd"
>     (set_attr "length" "4")
>     (set_attr "predicable" "no")])
>
> +;; Match <insn>si3_loop pattern if operand 2 has become const_int 1 in the 
> meantime
> +(define_insn_and_split "<insn>si3_cnt1_clobber"
> +  [(set (match_operand:SI 0 "dest_reg_operand")
> +  (ANY_SHIFT_ROTATE:SI (match_operand:SI 1 "register_operand")
> +        (const_int 1)))
> +  (clobber (reg:SI LP_COUNT))
> +  (clobber (reg:CC CC_REG))]
> +  "!TARGET_BARREL_SHIFTER"
> +  "#"
> +  "&& arc_pre_reload_split ()"
> +  [(set (match_dup 0) (ANY_SHIFT_ROTATE:SI (match_dup 1) (const_int 1)))]
> +  ""
> +[(set_attr "type" "shift")
> + (set_attr "length" "4")])
> +
>  (define_peephole2
>    [(set (match_operand:SI 0 "register_operand" "")
>         (zero_extract:SI (match_dup 0)
> diff --git a/gcc/testsuite/gcc.target/arc/no-barrel-shifter.c 
> b/gcc/testsuite/gcc.target/arc/no-barrel-shifter.c
> new file mode 100644
> index 0000000000..2506bdfcad
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/no-barrel-shifter.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-skip-if "" { { barrelshifter } } } */
> +int fromfloat(float fx);
> +
> +float foo(float fx)
> +{
> +  int x = fromfloat(fx);
> +  int sign = (x >> 31) & 1;
> +  unsigned int mag = x & 0x7fffffff;
> +
> +  if (mag > 0x7f800000)
> +    return fx;
> +  if (mag == 0x7f800000)
> +    return (sign == 0);
> +  return fx * (27 + sign);
> +}
> +
> +/* { dg-final { scan-assembler-not 
> "add.f\\s\+\[0-9\]\+,r\[0-9\]\+,r\[0-9\]\+\\n\\s\+beq.d" } } */
> --
> 2.43.0
>

Reply via email to