Problem when defining new mips insns

Problem when defining new mips insns

[Pontus Lidman]

Hello,

I'm trying to define some new instructions for handling TImode scalars
on a mips r5900 cpu, and I encountered a problem I can't seem to
understand properly or solve.

The following program:
long fun (double a) {  return -a; }

produces this result:

../../test/fix2.c: In function 'fun':
../../test/fix2.c:4: error: unrecognizable insn:
(insn 14 13 15 0 ../../test/fix2.c:2 (set (reg:TI 186)
        (ashift:TI (reg:TI 187)
            (const_int 63 [0x3f]))) -1 (insn_list:REG_DEP_TRUE 13
(nil))
    (expr_list:REG_DEAD (reg:TI 187)
        (expr_list:REG_EQUAL (const_int -9223372036854775808
[0x8000000000000000])
            (nil))))
../../test/fix2.c:4: internal compiler error: in extract_insn, at
recog.c:2034

The cpu doesn't have shift instructions for TImode scalars, so I have
defined library functions __ashrti3, __ashlti3. I have verified that
these can be generated by the compiler in other test cases.

What I'm wondering is, why and where is the above unrecognizable insn
generated? I had expected a call to one of my library functions to be
generated.

I have been trying some approaches without success: I have been able
to create a define_insn which matches the above insn, and I guess I
could make it output a library call, but I haven't found out how yet,
and it doesn't seem to be the 'correct' solution. I also tried
defining a define_expand with similiar match parameters, but the
expansion doesn't seem to be used in the above case. Most of all I'd
like to have a library call generated immediately, rather than the
insn above.

Thanks in advance,

Pontus

-- 
Pontus Lidman, pontus@lysator.liu.se, Software Engineer
No matter how cynical you get, it's impossible to keep up.
Scene: www.dc-s.com | MUD: tyme.envy.com 6969 | irc: irc.quakenet.eu.org

Re: Problem when defining new mips insns

[Roger Sayle]

On 21 Oct 2004, Pontus Lidman wrote:
> I'm trying to define some new instructions for handling TImode scalars
> on a mips r5900 cpu, and I encountered a problem I can't seem to
> understand properly or solve.
>
> The following program:
> long fun (double a) {  return -a; }
>
> produces this result:
>
> ../../test/fix2.c: In function 'fun':
> ../../test/fix2.c:4: error: unrecognizable insn:
> (insn 14 13 15 0 ../../test/fix2.c:2 (set (reg:TI 186)
>         (ashift:TI (reg:TI 187)
>             (const_int 63 [0x3f]))) -1 (insn_list:REG_DEP_TRUE 13
> (nil))
>     (expr_list:REG_DEAD (reg:TI 187)
>         (expr_list:REG_EQUAL (const_int -9223372036854775808
> [0x8000000000000000])
>             (nil))))
> ../../test/fix2.c:4: internal compiler error: in extract_insn, at
> recog.c:2034
>
> The cpu doesn't have shift instructions for TImode scalars, so I have
> defined library functions __ashrti3, __ashlti3. I have verified that
> these can be generated by the compiler in other test cases.
>
> What I'm wondering is, why and where is the above unrecognizable insn
> generated? I had expected a call to one of my library functions to be
> generated.


My guess is that the problem might be caused by not defining instructions
to load constants into TImode registers.  By introducing TImode, the GCC
middle-end now realizes that it can implement IEEE floating point negation
by just flipping the sign bit, i.e. by doing an XOR in TImode, presumably
because sizeof(TImode) == sizeof(double).  Hence the middle-end, tries to
expand "nega = (TImode)a ^ 0x8000000000000000".  It looks like the RTL
expanders are unable to load this constant into a TImode register
directly, and instead try to generate this constant via "1 << 63".

I hope this provides enough of a clue to proceed from here.
Perhaps LEGITIMATE_CONSTANT_P or force_const_mem?

Roger
--

Re: Problem when defining new mips insns

[Pontus Lidman]

Roger Sayle <roger@eyesopen.com> writes:

[...]

> My guess is that the problem might be caused by not defining instructions
> to load constants into TImode registers.  By introducing TImode, the GCC
> middle-end now realizes that it can implement IEEE floating point negation
> by just flipping the sign bit, i.e. by doing an XOR in TImode, presumably
> because sizeof(TImode) == sizeof(double).  Hence the middle-end, tries to
> expand "nega = (TImode)a ^ 0x8000000000000000".  It looks like the RTL
> expanders are unable to load this constant into a TImode register
> directly, and instead try to generate this constant via "1 << 63".
> 
> I hope this provides enough of a clue to proceed from here.
> Perhaps LEGITIMATE_CONSTANT_P or force_const_mem?

Thank you for your reply. Perhaps I should have mentioned that it's on
mips64. TImode integers are 16 bytes, DImode ones are 8, so
sizeof(DImode)==sizeof(double). So I don't understand why it wants to
use TImode values in this case.

I had a look at LEGITIMATE_CONSTANT_P and the related functions
(mips_legitimize_const_move etc), and to extend those functions to
handle TImode values seems to be problematic since TImode is larger
than HOST_WIDE_INT (which is 8 bytes). I'd rather avoid that path for
integer constants if possible. I do support memory and register moves.

I understand now why the instruction is generated, but I don't
understand why it wants to use TImode, and I also don't understand why
it doesn't generate a library call for the shift. I thought
set_optab_libfunc() for ashl, ashr, lshr in TImode would make it
happen in this case also.

Best Regards,

Pontus

-- 
Pontus Lidman, pontus@lysator.liu.se, Software Engineer
No matter how cynical you get, it's impossible to keep up.
Scene: www.dc-s.com | MUD: tyme.envy.com 6969 | irc: irc.quakenet.eu.org

Re: Problem when defining new mips insns

[Roger Sayle]

On 23 Oct 2004, Pontus Lidman wrote:
> Thank you for your reply. Perhaps I should have mentioned that it's on
> mips64. TImode integers are 16 bytes, DImode ones are 8, so
> sizeof(DImode)==sizeof(double). So I don't understand why it wants to
> use TImode values in this case.

I think if you discover that you'll be closer to debugging your patch :>
The relevant code is (perhaps) around line 2285 of optabs.c:

  /* Try negating floating point values by flipping the sign bit.  */
  if (unoptab->code == NEG && class == MODE_FLOAT
      && GET_MODE_BITSIZE (mode) <= 2 * HOST_BITS_PER_WIDE_INT)
    {
      const struct real_format *fmt = REAL_MODE_FORMAT (mode);
      enum machine_mode imode = int_mode_for_mode (mode);
      int bitpos = (fmt != 0) ? fmt->signbit : -1;

You need to discover why int_mode_for_mode (DFmode) is returning
TImode instead of DImode.  Another possibility is that your target
is unable to perform a DImode XOR, so expand_binop(imode, xor_optab, ...
is widening the operation to TImode.

And of course, the above code should never even get executed unless
your MIPS variant doesn't have a negdf2 pattern!?  It looks like
CVS mainline's current mips.md unconditionally defines a negdf2.

Roger
--

Re: Problem when defining new mips insns

[Pontus Lidman]

Roger Sayle <roger@eyesopen.com> writes:

[...]

> The relevant code is (perhaps) around line 2285 of optabs.c:

[...]

That's exactly the place I was looking for.

> Another possibility is that your target is unable to perform a
> DImode XOR, so expand_binop(imode, xor_optab, ...  is widening the
> operation to TImode.

Bingo. Since the target can do xor in SI or TI modes but not DI, the
operation is widened. I can see several solutions now that I
understand how and why it happens.
 
> And of course, the above code should never even get executed unless
> your MIPS variant doesn't have a negdf2 pattern!?  It looks like
> CVS mainline's current mips.md unconditionally defines a negdf2.

The target only has 32-bit floats; presumably there is already some
magic in gcc to avoid double instructions on such targets, maybe based
on TARGET_SINGLE_FLOAT or TARGET_FLOAT64. Anyway the right thing
happens for me now :)

I have a much clearer understanding of the situation now, thanks to
your help!

Best Regards,

Pontus

-- 
Pontus Lidman, pontus@lysator.liu.se, Software Engineer
No matter how cynical you get, it's impossible to keep up.
Scene: www.dc-s.com | MUD: tyme.envy.com 6969 | irc: irc.quakenet.eu.org