This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: x86 issue: unwelcome mmx instruction


On 8/24/06, Stuart Hastings <stuart@apple.com> wrote:

On Aug 18, 2006, at 6:10 PM, Ian Lance Taylor wrote:


> Stuart Hastings <stuart@apple.com> writes:
>
>> This x86-specific testcase provokes an MMX instruction in the absence
>> of any MMX builtins (grep the assembly for "%mm").  The trivial patch
>> "fixes" the problem, but the original code very deliberately checked
>> TARGET_SSE, and I don't understand why.
>
> It seems to me that we could support V2SImode and V2SFmode in SSE
> registers, but currently we don't.  And even if we did we don't have a
> way to express a preference as to which one we want.  Perhaps Richard
> was thinking of that.  Or perhaps he will simply remember.
>
>> Index: gcc.fsf.nommx/gcc/config/i386/i386.c
>> ===================================================================
>> --- gcc.fsf.nommx/gcc/config/i386/i386.c        (revision 115741)
>> +++ gcc.fsf.nommx/gcc/config/i386/i386.c        (working copy)
>> @@ -17926,7 +17926,7 @@
>>       {
>>       case V2SImode:
>>       case V2SFmode:
>> -      if (!mmx_ok && !TARGET_SSE)
>> +      if (!mmx_ok)
>>          return false;
>>         /* FALLTHRU */
>
> Essentially the same code is in ix86_expand_vector_init_one_nonzero
> and ix86_expand_vector_init_general.  It seems extremely likely that
> all three should be changed.  Could you please test that?

Removing the TARGET_SSE check in ix86_expand_vector_init_general()
broke the testcase.  I've removed the TARGET_SSE test in
ix86_expand_vector_init_one_nonzero() per your instructions.

> You don't
> have to write test cases for the other two functions, although it
> would probably not be too difficult (or else it could never happen and
> the test is redundant anyhow).

I tried the obvious one-nonzero-constant-value and one-variable-
others-zero testcases, but they all worked either way.

>> 2006-08-18  Stuart Hastings  <stuart@apple.com>
>>
>>      *config/i386/i386.c (ix86_expand_vector_init_duplicate):
>> Remove  TARGET_SSE check.
>>      * testsuite/gcc.target/i386/20060725-1.c: New.
>>
>> O.K. for trunk?
>
> Please fix the ChangeLog spacing.

O.K.

>   And of course the testsuite entry
> goes in testsuite/ChangeLog.

Duh. I knew that... or I should have (thank you for reminding me :-).

2006-08-23 Stuart Hastings <stuart@apple.com>

        PR 28825
        * gcc/config/i386/i386.c (ix86_expand_vector_init_duplicate,
        ix86_expand_vector_init_one_nonzero): Remove TARGET_SSE test.

2006-08-23 Stuart Hastings <stuart@apple.com>

        PR 28825
        * gcc.target/i386/20060821-1.c: New.

Bootstrapped, DejaGnu on Linux/x86_64; no regressions.

> Please mention PR 24073 in the ChangeLog entry.

PR 24073 had grown to two bugs; I cloned it into 28825 to track the
spurious MMX instruction issue, and marked my change with that PR.

The original performance issue identified in 24073 is greatly
improved, but it's not complete.  I've attached what we generate
today for the original testcase.

> The patch to change all three locations is preapproved.  Please wait
> at least until Monday to give Richard a chance to reply.

Done and committed.

PR28960 is the same problem on the 4.1 branch. So, backported, bootstrapped and tested on i686-pc-linux-gnu and applied.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]