diff mbox

[MIPS,committed] Add missing COSTS_N_INSNS call.

Message ID 87k3wl31j2.fsf@talisman.home
State New
Headers show

Commit Message

Richard Sandiford Aug. 26, 2012, 7:18 p.m. UTC 
I'm preparing a patch to turn gcc.target/mips into a torture-like
testsuite.  This showed up a fair few problems, the first of which is
fixed below.  The code that calculated the size cost of multiplications
was missing a COSTS_N_INSNS call.

Tested on mipsisa64-elf, mips64-elf and mips64-linux-gnu.  Applied.

Richard


gcc/
	* config/mips/mips.c (mips_rtx_costs): Add missing COSTS_N_INSNS
	to the size cost of multiplication.

Comments

Hans-Peter Nilsson Aug. 27, 2012, 8:21 p.m. UTC | #1 
On Sun, 26 Aug 2012, Richard Sandiford wrote:
> I'm preparing a patch to turn gcc.target/mips into a torture-like
> testsuite.

While on the subject of gcc.target/mips and its extensions, it
also doesn't handle a build configured with --with-synci=yes.
(Well, not on the 4.7 branch at least.)

brgds, H-P
Richard Sandiford Aug. 28, 2012, 7:17 p.m. UTC | #2 
Hans-Peter Nilsson <hp@bitrange.com> writes:
> On Sun, 26 Aug 2012, Richard Sandiford wrote:
>> I'm preparing a patch to turn gcc.target/mips into a torture-like
>> testsuite.
>
> While on the subject of gcc.target/mips and its extensions, it
> also doesn't handle a build configured with --with-synci=yes.
> (Well, not on the 4.7 branch at least.)

What goes wrong?

Richard
Hans-Peter Nilsson Aug. 28, 2012, 8:36 p.m. UTC | #3 
On Tue, 28 Aug 2012, Richard Sandiford wrote:
> Hans-Peter Nilsson <hp@bitrange.com> writes:
> > On Sun, 26 Aug 2012, Richard Sandiford wrote:
> >> I'm preparing a patch to turn gcc.target/mips into a torture-like
> >> testsuite.
> >
> > While on the subject of gcc.target/mips and its extensions, it
> > also doesn't handle a build configured with --with-synci=yes.
> > (Well, not on the 4.7 branch at least.)
>
> What goes wrong?

I don't remember details, but IIRC some synci-related tests go
wrong for mipsisa32r2el-linux-gnu due to -msynci being the
default.  Don't worry, I've fixed it in the local import. :)
I though the above would entice you to try it, but I guess I
need to report better for that to happen.  Maybe later.

brgds, H-P
Richard Sandiford Aug. 28, 2012, 8:43 p.m. UTC | #4 
Hans-Peter Nilsson <hp@bitrange.com> writes:
> On Tue, 28 Aug 2012, Richard Sandiford wrote:
>> Hans-Peter Nilsson <hp@bitrange.com> writes:
>> > On Sun, 26 Aug 2012, Richard Sandiford wrote:
>> >> I'm preparing a patch to turn gcc.target/mips into a torture-like
>> >> testsuite.
>> >
>> > While on the subject of gcc.target/mips and its extensions, it
>> > also doesn't handle a build configured with --with-synci=yes.
>> > (Well, not on the 4.7 branch at least.)
>>
>> What goes wrong?
>
> I don't remember details, but IIRC some synci-related tests go
> wrong for mipsisa32r2el-linux-gnu due to -msynci being the
> default.  Don't worry, I've fixed it in the local import. :)
> I though the above would entice you to try it, but I guess I
> need to report better for that to happen.  Maybe later.

Trying it now.  I suspect it was the problem that Steve hit:
the implicit -msynci is still (deliberately) kept when a lower
architecture is selected.

I'm testing a patch to make the testsuite work out the default
-m{no,}synci, which ought to be enough.  The usual rules should
then kick in and force -mno-synci where necessary.  Hopefully.

Richard
diff mbox

Patch

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2012-08-26 11:34:07.000000000 +0100
+++ gcc/config/mips/mips.c	2012-08-26 11:35:15.076810487 +0100
@@ -3841,7 +3841,7 @@  mips_rtx_costs (rtx x, int code, int out
 		  ? mips_cost->int_mult_si * 3 + 6
 		  : COSTS_N_INSNS (ISA_HAS_MUL3 ? 7 : 9));
       else if (!speed)
-	*total = (ISA_HAS_MUL3 ? 1 : 2);
+	*total = COSTS_N_INSNS (ISA_HAS_MUL3 ? 1 : 2);
       else if (mode == DImode)
 	*total = mips_cost->int_mult_di;
       else