How to write shift and add pattern?

[Mohamed Shafi]

Hello all,

I am trying to port a 32bit arch in GCC 4.4.0. My target has support
for 1bit, 2bit shift and add operations. I tried to write patterns for
this , but gcc is not generating those. The following are the patterns
that i have written in md file:

(define_insn "shift_add_<mode>"
 [(set (match_operand:SI 0 "register_operand" "")
       (plus:SI (match_operand:SI 3 "register_operand" "")
                 (ashift:SI (match_operand:SI 1 "register_operand" "")
                             (match_operand:SI 2 "immediate_operand" ""))))]
 ""
 "shadd1\\t%1, %0"
)

(define_insn "shift_add1_<mode>"
 [(set (match_operand:SI 0 "register_operand" "")
       (plus:SI (ashift:SI (match_operand:SI 1 "register_operand" "")
                             (match_operand:SI 2 "immediate_operand" ""))
                 (match_operand:SI 3 "register_operand" "")))]
 ""
 "shadd1\\t%1, %0"
)

(define_insn "shift_n_add_<mode>"
 [(set (match_operand:SI 1 "register_operand" "")
       (ashift:SI (match_dup 1)
                   (match_operand:SI 2 "immediate_operand" "")))
  (set (match_operand:SI 0 "register_operand" "")
       (plus:SI (match_dup 0)
                 (match_dup 1)))]
 ""
 "shadd2\\t%1, %0"
)


As you can see i have tried combinations. Since i was looking for
pattern matching i didnt bother to write according to the target.
Thought i will do that after i get a matching pattern. When i debugged
GCC was generating patterns with multiply. But that gets discarded
since md file doesnt have those patterns. How can i make GCC generate
shift and add pattern? Is GCC generating patterns with multiply due to
cost issues? I havent mentioned any cost details.

Regards,
Shafi

[Richard Henderson-2]

On 08/28/2009 06:51 AM, Mohamed Shafi wrote:

> Hello all,
>
> I am trying to port a 32bit arch in GCC 4.4.0. My target has support
> for 1bit, 2bit shift and add operations. I tried to write patterns for
> this , but gcc is not generating those. The following are the patterns
> that i have written in md file:
>
> (define_insn "shift_add_<mode>"
>   [(set (match_operand:SI 0 "register_operand" "")
>         (plus:SI (match_operand:SI 3 "register_operand" "")
>                   (ashift:SI (match_operand:SI 1 "register_operand" "")
>                               (match_operand:SI 2 "immediate_operand" ""))))]
>   ""
>   "shadd1\\t%1, %0"
> )

...
 > Is GCC generating patterns with multiply due to
 > cost issues? I havent mentioned any cost details.

No, it's merely using multiply because someone way back when
decided that should be the canonical way to represent this.
We canonicalize patterns so that the target file doesn't have
to match both shifts and multiplies.

So your insn should look like:

(define_insn "*shadd1_si"
   [(set (match_operand:SI 0 "register_operand" "r")
         (plus:SI (mult:SI (match_operand:SI "register_operand" "r")
                           (const_int 2))
                  (match_operand:SI "register_operand" "0")))]
   ""
   "shadd1 %1,%0")

This should match even if your target doesn't support a
hardware multiply insn.

See also the shift-add patterns on the Alpha port.  There we
have 2 & 3 bit shifts.  Search for const48_operand to find
those patterns easily.


r~

[Mohamed Shafi]

2009/8/28 Richard Henderson <rth@...>:

> On 08/28/2009 06:51 AM, Mohamed Shafi wrote:
>>
>> Hello all,
>>
>> I am trying to port a 32bit arch in GCC 4.4.0. My target has support
>> for 1bit, 2bit shift and add operations. I tried to write patterns for
>> this , but gcc is not generating those. The following are the patterns
>> that i have written in md file:
>>
>> (define_insn "shift_add_<mode>"
>>  [(set (match_operand:SI 0 "register_operand" "")
>>        (plus:SI (match_operand:SI 3 "register_operand" "")
>>                  (ashift:SI (match_operand:SI 1 "register_operand" "")
>>                              (match_operand:SI 2 "immediate_operand"
>> ""))))]
>>  ""
>>  "shadd1\\t%1, %0"
>> )
>
> ...
>> Is GCC generating patterns with multiply due to
>> cost issues? I havent mentioned any cost details.
>
> No, it's merely using multiply because someone way back when
> decided that should be the canonical way to represent this.
> We canonicalize patterns so that the target file doesn't have
> to match both shifts and multiplies.
>
> So your insn should look like:
>
> (define_insn "*shadd1_si"
>  [(set (match_operand:SI 0 "register_operand" "r")
>        (plus:SI (mult:SI (match_operand:SI "register_operand" "r")
>                          (const_int 2))
>                 (match_operand:SI "register_operand" "0")))]
>  ""
>  "shadd1 %1,%0")
>
> This should match even if your target doesn't support a
> hardware multiply insn.
>
> See also the shift-add patterns on the Alpha port.  There we
> have 2 & 3 bit shifts.  Search for const48_operand to find
> those patterns easily.
>

    The target that i am working on has 1 & 2 bit shift-add patterns.
GCC is not generating shift-add patterns when the shift count is 1. It
is currently generating add operations. What should be done to
generate shift-add pattern instead of add-add pattern?

Another issue is that shift-add pattern will work only with address
registers. So if i have constraints for only address register in the
pattern there will be cases when the normal shift and add instructions
are more profitable than reloading into address registers and doing
sift-add instruction. Am i right ? So what i did was to have both
address register and data registers as constraints and then have a
define split after reload to split shift-add to shift and add when
registers are data registers. But the problem is i cant make the
allocator to allocate address registers for simple cases.

For the following program

extern int a, b;

int foo ()
{
  a = a + (b << 2) ;
  return a;
}

the complier should generate with address registers so that shift-add
pattern can be used. But i cant make the compiler to generate those.
It is generating with data registers. Here is the pattern that i have
written:


(define_insn "*saddl"
  [(set (match_operand:SI 0 "register_operand" "=r,d")
        (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "r,d")
                          (match_operand:SI 2 "const24_operand" "J,J"))
                 (match_operand:SI 3 "register_operand" "0,0")))]

How can i do this. Will the constraint modifiers '?' or '!' help?
How can make GCC generate shift and add sequence when the shift count is 1?

Regards,
Shafi

[Ian Lance Taylor-3]

Mohamed Shafi <shafitvm@...> writes:

> It is generating with data registers. Here is the pattern that i have
> written:
>
>
> (define_insn "*saddl"
>   [(set (match_operand:SI 0 "register_operand" "=r,d")
> (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "r,d")
>  (match_operand:SI 2 "const24_operand" "J,J"))
> (match_operand:SI 3 "register_operand" "0,0")))]
>
> How can i do this. Will the constraint modifiers '?' or '!' help?
> How can make GCC generate shift and add sequence when the shift count is 1?

Does 'd' represent a data register?  I assume that 'r' is a general
register, as it always is.  What is the constraint character for an
address register?  You don't seem to have an alternative here for
address registers, so I'm not surprised that the compiler isn't
picking it.  No doubt I misunderstand something.

Ian

[Richard Henderson-2]

On 11/06/2009 05:29 AM, Mohamed Shafi wrote:
>      The target that i am working on has 1&  2 bit shift-add patterns.
> GCC is not generating shift-add patterns when the shift count is 1. It
> is currently generating add operations. What should be done to
> generate shift-add pattern instead of add-add pattern?

I'm not sure.  You may have to resort to matching

   (set (match_operand 0 "register_operand" "")
        (plus (plus (match_operand 1 "register_operand" "")
                   (match_dup 1))
              (match_operand 2 "register_operand" ""))))

But you should debug make_compound_operation first to
figure out what's going on for your port, because it's
working for x86_64:

        long foo(long a, long b) { return a*2 + b; }

        leaq (%rsi,%rdi,2), %rax # 8 *lea_2_rex64
        ret # 26 return_internal


r~

[Mohamed Shafi]

2009/11/6 Richard Henderson <rth@...>:

> On 11/06/2009 05:29 AM, Mohamed Shafi wrote:
>>
>>     The target that i am working on has 1&  2 bit shift-add patterns.
>> GCC is not generating shift-add patterns when the shift count is 1. It
>> is currently generating add operations. What should be done to
>> generate shift-add pattern instead of add-add pattern?
>
> I'm not sure.  You may have to resort to matching
>
>  (set (match_operand 0 "register_operand" "")
>       (plus (plus (match_operand 1 "register_operand" "")
>                   (match_dup 1))
>             (match_operand 2 "register_operand" ""))))
>
> But you should debug make_compound_operation first to
> figure out what's going on for your port, because it's
> working for x86_64:
>
>        long foo(long a, long b) { return a*2 + b; }
>
>        leaq    (%rsi,%rdi,2), %rax     # 8     *lea_2_rex64
>        ret                             # 26    return_internal
>
>
> r~
>

   I have fixed this. The culprit was the cost factor. I added the
case in targetm.rtx_costs and now it works properly. But i am having
issues with the reload.

Regards,
Shafi

[Mohamed Shafi]

2009/11/6 Ian Lance Taylor <iant@...>:

> Mohamed Shafi <shafitvm@...> writes:
>
>> It is generating with data registers. Here is the pattern that i have
>> written:
>>
>>
>> (define_insn "*saddl"
>>   [(set (match_operand:SI 0 "register_operand" "=r,d")
>>       (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "r,d")
>>                         (match_operand:SI 2 "const24_operand" "J,J"))
>>                (match_operand:SI 3 "register_operand" "0,0")))]
>>
>> How can i do this. Will the constraint modifiers '?' or '!' help?
>> How can make GCC generate shift and add sequence when the shift count is 1?
>
> Does 'd' represent a data register?  I assume that 'r' is a general
> register, as it always is.  What is the constraint character for an
> address register?  You don't seem to have an alternative here for
> address registers, so I'm not surprised that the compiler isn't
> picking it.  No doubt I misunderstand something.
>

   Ok the constrain for address register is 'a'. Thats typo in the
pattern that i given here. The proper pattern is

 (define_insn "*saddl"
   [(set (match_operand:SI 0 "register_operand" "=a,d")
       (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "a,d")
                         (match_operand:SI 2 "const24_operand" "J,J"))
                (match_operand:SI 3 "register_operand" "0,0")))]

So how can i choose the address registers over data registers if that
is more profitable?

Regards,
Shafi

[Ian Lance Taylor-3]

Mohamed Shafi <shafitvm@...> writes:

>    Ok the constrain for address register is 'a'. Thats typo in the
> pattern that i given here. The proper pattern is
>
>  (define_insn "*saddl"
>    [(set (match_operand:SI 0 "register_operand" "=a,d")
>        (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "a,d")
>                          (match_operand:SI 2 "const24_operand" "J,J"))
>                 (match_operand:SI 3 "register_operand" "0,0")))]
>
> So how can i choose the address registers over data registers if that
> is more profitable?

If it is cheaper to move two values from data registers to address
registers than it is to use two data registers for this instruction,
then you should omit the d,d,J alternative.

If it is cheaper to move one value from a data register to an address
register than it is to use two data registers for this instruction,
then you should use !, as in "=a,!d".

Otherwise you should use ?, as in "=a,?d".  You can additional ?'s for
additional tuning if appropriate.

Ian