[mips,testsuite] Fix test to handle optimizations

Submitted by Steve Ellcey on Oct. 8, 2012, 4:16 p.m.

Details

Message ID 102e63ea-6b28-45cf-a75b-10d19561500e@EXCHHUB01.MIPS.com
State New
Headers show

Commit Message

Steve Ellcey Oct. 8, 2012, 4:16 p.m.
The gcc.target/mips/ext_ins.c was failing in little endian mode on MIPS because
the compiler is smart enough now to see that 'c' is uninitialized and it can
insert the field 'a' into 'c' with a shift and a full store instead of an
insert because the store just overwrites unintialized data.  I changed the
code to force the compiler to preserve the other fields of 'c' and that makes
it use the insert instruction in both big and little endian modes.

Tested on mips-mti-elf.

OK to checkin?

Steve Ellcey
sellcey@mips.com



2012-10-08  Steve Ellcey  <sellcey@mips.com>

	* gcc.target/ext_ins.c: Modify f2 to aviod uninitialized data.

Comments

Mike Stump Oct. 8, 2012, 6:15 p.m.
On Oct 8, 2012, at 9:16 AM, Steve Ellcey <sellcey@mips.com> wrote:
> The gcc.target/mips/ext_ins.c was failing in little endian mode on MIPS because
> the compiler is smart enough now to see that 'c' is uninitialized and it can
> insert the field 'a' into 'c' with a shift and a full store instead of an
> insert because the store just overwrites unintialized data.  I changed the
> code to force the compiler to preserve the other fields of 'c' and that makes
> it use the insert instruction in both big and little endian modes.
> 
> Tested on mips-mti-elf.
> 
> OK to checkin?

Ok.
ddaney.cavm@gmail.com Oct. 8, 2012, 6:28 p.m.
On 10/08/2012 11:15 AM, Mike Stump wrote:
> On Oct 8, 2012, at 9:16 AM, Steve Ellcey <sellcey@mips.com> wrote:
>> The gcc.target/mips/ext_ins.c was failing in little endian mode on MIPS because
>> the compiler is smart enough now to see that 'c' is uninitialized and it can
>> insert the field 'a' into 'c' with a shift and a full store instead of an
>> insert because the store just overwrites unintialized data.  I changed the
>> code to force the compiler to preserve the other fields of 'c' and that makes
>> it use the insert instruction in both big and little endian modes.
>>
>> Tested on mips-mti-elf.
>>
>> OK to checkin?
>
> Ok.

I don't think this is the proper fix for this.

Use of BBIT{0,1} instructions will always be smaller than the 
alternative.  So disabling the test for -Os doesn't fix the problem the 
test is designed to find.

The real problem is that some optimizer is broken.  Instead of disabling 
the tests, can we fix the problem instead?

The goal of the testsuite should be to detect problems, not yield clean 
results.

If Richard disagrees with me, then I would defer to him.


David Daney
ddaney.cavm@gmail.com Oct. 8, 2012, 6:52 p.m.
Really I meant this in reply to the  'Fix 
gcc.target/mips/octeon-bbit-2.c for -Os' thread.  Sorry for confusing 
the issue here.

I don't really have an objection to this one.

David Daney

On 10/08/2012 11:28 AM, David Daney wrote:
> On 10/08/2012 11:15 AM, Mike Stump wrote:
>> On Oct 8, 2012, at 9:16 AM, Steve Ellcey <sellcey@mips.com> wrote:
>>> The gcc.target/mips/ext_ins.c was failing in little endian mode on
>>> MIPS because
>>> the compiler is smart enough now to see that 'c' is uninitialized and
>>> it can
>>> insert the field 'a' into 'c' with a shift and a full store instead
>>> of an
>>> insert because the store just overwrites unintialized data.  I
>>> changed the
>>> code to force the compiler to preserve the other fields of 'c' and
>>> that makes
>>> it use the insert instruction in both big and little endian modes.
>>>
>>> Tested on mips-mti-elf.
>>>
>>> OK to checkin?
>>
>> Ok.
>
> I don't think this is the proper fix for this.
>
> Use of BBIT{0,1} instructions will always be smaller than the
> alternative.  So disabling the test for -Os doesn't fix the problem the
> test is designed to find.
>
> The real problem is that some optimizer is broken.  Instead of disabling
> the tests, can we fix the problem instead?
>
> The goal of the testsuite should be to detect problems, not yield clean
> results.
>
> If Richard disagrees with me, then I would defer to him.
>
>
> David Daney
>

Patch hide | download patch | download mbox

diff --git a/gcc/testsuite/gcc.target/mips/ext_ins.c b/gcc/testsuite/gcc.target/mips/ext_ins.c
index f0169bc..36f0f3f 100644
--- a/gcc/testsuite/gcc.target/mips/ext_ins.c
+++ b/gcc/testsuite/gcc.target/mips/ext_ins.c
@@ -18,9 +18,8 @@  NOMIPS16 unsigned int f1 (struct A a)
   return a.j;
 }
 
-NOMIPS16 void f2 (int i)
+NOMIPS16 struct A f2 (struct A a, int i)
 {
-  struct A c;
-  c.j = i;
-  func (c);
+  a.j = i;
+  return a;
 }