On Mon, 15 Sept 2025 at 13:02, Richard Biener
<[email protected]> wrote:
>
> On Mon, Sep 15, 2025 at 11:55 AM Christophe Lyon
> <[email protected]> wrote:
> >
> > On Thu, 11 Sept 2025 at 19:44, Jakub Jelinek <[email protected]> wrote:
> > >
> > > On Thu, Sep 11, 2025 at 07:14:51PM +0200, Richard Biener wrote:
> > > > Yes, RTL expansion inserts an AND operation when !SHIFT_COUNT_TRUNCATED.
> > > > What I was saying there's no way to get a negative shift count flip
> > > > shift
> > > > direction to RTL - it would require a target specific intrinsic that's
> > > > a builtin
> > > > call before RTL expansion and an UNSPEC after.
> > >
> > > Of course the target can have patterns like:
> > > (set (match_operand:M 0)
> > > (if_then_else:M
> > > (lt:SI (match_operand:SI 2) (const_int 0))
> > > (ashiftrt:M (match_operand:M 1) (neg:SI (match_dup 2)))
> > > (ashift:M (match_dup 1) (match_dup 2))))
> > > or so, it doesn't need to use UNSPEC for that.
> > >
> >
> > Hi!
> > Thanks for all the feedback, that was very useful.
> > I've implemented the suggestion above, but so far I couldn't make the
> > 'if' condition match (the shift amount is never seen as negative with
> > my tests).
> > Since the constraint is "rPg", where "Pg" means "constant in range 1
> > to 32", out-of-range values are always passed in register: can the
> > above if_then_else work when operand 2 is a register?
>
> I think the above might match during combine when you write C code like
>
> if (n < 0)
> res = x >> -n;
> else
> res = x << n;
>
> which of course would need to be if-converted (possibly trying to match
> exactly that insn pattern). A variant with UB, like
>
> res1 = x >> -n;
> res2 = x << n;
> res = n < 0 ? res1 : res2;
>
> might be an easier match.
>
so, with:
(define_insn "mve_lsll"
[(set (match_operand:DI 0 "arm_general_register_operand" "=r")
(if_then_else:DI
(lt:SI (match_operand:SI 2 "arm_reg_or_long_shift_imm" "rPg")
(const_int 0))
(ashiftrt:DI (match_operand:DI 1 "arm_general_register_operand" "0")
(neg:SI (match_dup 2)))
(ashift:DI (match_dup 1) (match_dup 2))))]
"TARGET_HAVE_MVE"
"lsll%?\\t%Q0, %R1, %2"
[(set_attr "predicable" "yes")])
both foo1 and foo3 generate the same code (the shift amount is known
at compile time, so only the needed part of the condition is kept)
int64_t
foo1 (int64_t value)
{
int32_t shift = -10;
if (shift < 0)
return value << -shift;
else
return value >> shift;
}
int64_t
foo3 (int64_t value)
{
int32_t shift = -10;
int64_t res1 = value << -shift;
int64_t res2 = value >> shift;
return shift < 0 ? res1 : res2;
}
foo1:
mov r2, r0 @ 18 [c=4 l=2] *thumb2_movsi_vfp/0
mov r3, r1 @ 19 [c=4 l=2] *thumb2_movsi_vfp/0
lsll r2, r3, #10 @ 6 [c=112 l=4] mve_lsll
mov r0, r2 @ 24 [c=4 l=2] *thumb2_movsi_vfp/0
mov r1, r3 @ 25 [c=4 l=2] *thumb2_movsi_vfp/0
bx lr @ 28 [c=8 l=4] *thumb2_return
similarly foo2 and foo4 generate the same code:
int64_t
foo2 (int64_t value, int32_t shift)
{
if (shift < 0)
return value << -shift;
else
return value >> shift;
}
int64_t
foo4 (int64_t value, int32_t shift)
{
int64_t res1 = value << -shift;
int64_t res2 = value >> shift;
return shift < 0 ? res1 : res2;
}
foo2:
cmp r2, #0 @ 7 [c=4 l=2] *movsi_compare0/0
itte lt
rsblt r2, r2, #0 @ 10 [c=8 l=6] *p *arm_negsi2/0
lslllt r0, r1, r2 @ 11 [c=8 l=6] *p mve_lsll
asrlge r0, r1, r2 @ 16 [c=8 l=6] *p mve_asrl
bx lr @ 57 [c=8 l=4] *thumb2_return
With foo calling the intrinsic:
int64_t
foo (int64_t value)
{
return asrl (value, -10);
}
the generated code is the expected:
foo:
mvn r3, #9 @ 6 [c=4 l=4] *thumb2_movsi_vfp/3 ;; r3 = -10
asrl r0, r1, r3 @ 11 [c=172 l=4] mve_asrl ;; shift-right by -10
bx lr @ 19 [c=8 l=4] *thumb2_return
with the final RTL being:
(insn:TI 6 7 11 2 (set (reg:SI 3 r3 [96])
(const_int -10 [0xfffffffffffffff6])) "xxx" 543 {*thumb2_movsi_vfp}
(expr_list:REG_EQUIV (const_int -10 [0xfffffffffffffff6])
(nil)))
(insn 11 6 12 2 (set (reg/i:DI 0 r0)
(if_then_else:DI (lt:SI (reg:SI 3 r3 [96])
(const_int 0 [0]))
(ashift:DI (reg:DI 0 r0 [orig:98 value ] [98])
(neg:SI (reg:SI 3 r3 [96])))
(ashiftrt:DI (reg:DI 0 r0 [orig:98 value ] [98])
(reg:SI 3 r3 [96])))) "xxx" 4849 {mve_asrl}
(expr_list:REG_DEAD (reg:SI 3 r3 [96])
(nil)))
(insn 12 11 23 2 (use (reg/i:DI 0 r0)) "xxx" -1
(nil))
(note 23 12 19 2 NOTE_INSN_EPILOGUE_BEG)
(jump_insn:TI 19 23 20 2 (simple_return) "xxx" 826 {*thumb2_return}
(nil)
-> simple_return)
So finally, given the "Pg" constraint, I'm wondering if there's any
interest in adding the if_then_else part?
Thanks,
Christophe
> Richard.
>
> > Christophe
> >
> > > Jakub
> > >
