Definition of an instruction

Definition of an instruction

[Jean Christophe Beyler]

Dear all,

I've been trying to define this instruction but am having
difficulties. This instruction does 3 things:
 - Updates an internal accumulator (depending on two arguments)
 - Returns the value of that update in an output register
 - Updates the internal accumulator after that


I first wrote :

(define_insn "myInst"
 [
 (set (match_operand:DI 0 "register_operand" "=r")
  (unspec:DI [(match_operand:DI 1 "register_operand" "r")
   (match_operand:DI 2 "register_operand" "r")
   (reg:DI 66)] 5))
   (clobber (reg:DI 66))
 ]
   ""
   "myInst\\t%0,%1,%2"
  [(set_attr "type"     "arith")
   (set_attr "mode"     "DI")
   (set_attr "length"   "1")])

Therefore saying that the unspec was dependent on the accumulator and
the two values and that the result was operand 0.

I added a clobber to the accumulator to say: it's been changed.

However, now if I have two myInst, the first gets optimized out...

I therefore need to define this instruction differently, any ideas ?

Thanks a lot,
Jean Christophe Beyler

Re: Definition of an instruction

[Andrew Pinski]

Maybe try this:

(define_insn "myInst"
 [
 (set (match_operand:DI 0 "register_operand" "=r")
 (unspec:DI [(match_operand:DI 1 "register_operand" "r")
  (match_operand:DI 2 "register_operand" "r")
  (reg:DI 66)] 5))
(set (reg:DI 66)
 (unspec:DI [(match_operand:DI 1 "register_operand" "r")
  (match_operand:DI 2 "register_operand" "r")
  (reg:DI 66)] 6))
 ]
  ""
  "myInst\\t%0,%1,%2"
 [(set_attr "type"     "arith")
  (set_attr "mode"     "DI")
  (set_attr "length"   "1")])


Which basically means op[0] = unspec:5(op[1], op[2], reg:66); [reg:66]
= unspec:6(op[1], op[2], reg:66)
So when you have two in a row, the second depends on the first as you
have the set of reg:66.  The clobber in your original code does not do
that, it says reg 66 cannot be depended on the value across the
instruction.

Thanks,
Andrew Pinski

Re: Definition of an instruction

[Jean Christophe Beyler]

Yeah that's what I had tried first but I get :

genextract: Internal error: abort in VEC_safe_set_locstr

And I can't seem to get rid of that error.

Any idea why this happens?
Jc

On Thu, Mar 4, 2010 at 6:28 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> Maybe try this:
>
> (define_insn "myInst"
>  [
>  (set (match_operand:DI 0 "register_operand" "=r")
>  (unspec:DI [(match_operand:DI 1 "register_operand" "r")
>  (match_operand:DI 2 "register_operand" "r")
>  (reg:DI 66)] 5))
> (set (reg:DI 66)
>  (unspec:DI [(match_operand:DI 1 "register_operand" "r")
>  (match_operand:DI 2 "register_operand" "r")
>  (reg:DI 66)] 6))
>  ]
>  ""
>  "myInst\\t%0,%1,%2"
>  [(set_attr "type"     "arith")
>  (set_attr "mode"     "DI")
>  (set_attr "length"   "1")])
>
>
> Which basically means op[0] = unspec:5(op[1], op[2], reg:66); [reg:66]
> = unspec:6(op[1], op[2], reg:66)
> So when you have two in a row, the second depends on the first as you
> have the set of reg:66.  The clobber in your original code does not do
> that, it says reg 66 cannot be depended on the value across the
> instruction.
>
> Thanks,
> Andrew Pinski
>

Re: Definition of an instruction

[Andrew Pinski]

On Thu, Mar 4, 2010 at 3:38 PM, Jean Christophe Beyler
<jean.christophe.beyler@gmail.com> wrote:
> Yeah that's what I had tried first but I get :
>
> genextract: Internal error: abort in VEC_safe_set_locstr
>
> And I can't seem to get rid of that error.
>
> Any idea why this happens?

Oh because the second time for the match_operand, you should be using
match_dup (and yes the error message should be better).  I had forgot
about that :).
So try this:
(define_insn "myInst"
 [
 (set (match_operand:DI 0 "register_operand" "=r")
 (unspec:DI [(match_operand:DI 1 "register_operand" "r")
 (match_operand:DI 2 "register_operand" "r")
 (reg:DI 66)] 5))
(set (reg:DI 66)
 (unspec:DI [(match_dup 1)
 (match_dup 2)
 (reg:DI 66)] 6))
 ]
 ""
 "myInst\\t%0,%1,%2"
 [(set_attr "type"     "arith")
 (set_attr "mode"     "DI")
 (set_attr "length"   "1")])


Thanks,
Andrew Pinski

Re: Definition of an instruction

[Jean Christophe Beyler]

Yes, I should have thought about it too. The error message threw me a
bit off, thanks for clearing that. Thanks for that !

Final question about this all:

I have a cost adjustment to make if the register operand 0 is used
later on. Basically, if that one is used, I have a +6 cost otherwise
if it's simply the 66 register, it's +1 cost.

I added this to the adjust_cost function :

    /* Otherwise, if one is our instruction, we must check the dependencies */
    if (isMyInst (dep_insn))
    {
        // Get statement
        rtx stmt = PATTERN (dep_insn);
        // Get target of the set, it's our instruction, we know where
it is exactly
        int regno = REGNO (SET_DEST (XVECEXP (stmt, 0, 0)));

        /* Depending on if the def of dep_insn is used in insn, we
adjust the cost */
        bitmap tmp_uses = BITMAP_ALLOC (&reg_obstack);

        /* Get the uses */
        df_simulate_uses (insn, tmp_uses);

        /* If ever one of the uses is our Set, add 6 */
        if (bitmap_bit_p (tmp_uses, regno))
        {
            cost += 6;
        }

        /* Free bitmap */
        BITMAP_FREE (tmp_uses), tmp_uses = NULL;
    }

Basically, my question is: I'm using the df_simulate_uses because
that's the simplest I know how to get all the uses. Is this safe
without doing a df_analyse at the beginning of the adjust_cost
function ?

It works but I'd like to get some input on it.

Thanks again,
Jc

On Thu, Mar 4, 2010 at 6:42 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Thu, Mar 4, 2010 at 3:38 PM, Jean Christophe Beyler
> <jean.christophe.beyler@gmail.com> wrote:
>> Yeah that's what I had tried first but I get :
>>
>> genextract: Internal error: abort in VEC_safe_set_locstr
>>
>> And I can't seem to get rid of that error.
>>
>> Any idea why this happens?
>
> Oh because the second time for the match_operand, you should be using
> match_dup (and yes the error message should be better).  I had forgot
> about that :).
> So try this:
> (define_insn "myInst"
>  [
>  (set (match_operand:DI 0 "register_operand" "=r")
>  (unspec:DI [(match_operand:DI 1 "register_operand" "r")
>  (match_operand:DI 2 "register_operand" "r")
>  (reg:DI 66)] 5))
> (set (reg:DI 66)
>  (unspec:DI [(match_dup 1)
>  (match_dup 2)
>  (reg:DI 66)] 6))
>  ]
>  ""
>  "myInst\\t%0,%1,%2"
>  [(set_attr "type"     "arith")
>  (set_attr "mode"     "DI")
>  (set_attr "length"   "1")])
>
>
> Thanks,
> Andrew Pinski
>