On Wed, Mar 23, 2016 at 2:03 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Mar 23, 2016 at 03:55:01PM +0300, Kirill Yukhin wrote:
>> > No, the change is OK (standard_sse_constant_p does all the checks),
>> > but nowadays we could write this part as:
>> >
>> > --cut here--
>> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > index 1639704..59154c3 100644
>> > --- a/gcc/config/i386/i386.c
>> > +++ b/gcc/config/i386/i386.c
>> > @@ -10859,10 +10859,7 @@ standard_sse_constant_opcode (rtx_insn *insn, rtx
>> > x)
>> > || get_attr_mode (insn) == MODE_V8DF
>> > || get_attr_mode (insn) == MODE_V16SF)
>> > return "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
>> > - if (TARGET_AVX)
>> > - return "vpcmpeqd\t%0, %0, %0";
>> > - else
>> > - return "pcmpeqd\t%0, %0";
>> > + return "%vpcmpeqd\t%0, %d0";
>> >
>> > default:
>> > break;
>> > -- cut here--
>> This looks much better to me.
>> Going to bootstrap/regtest.
>>
>> Is it ok to check into main trunk?
>
> But what is the advantage of doing that? You trade one simple conditional
> for lots more processing at the insn emit time (parse %v, handle if
> (TARGET_AVX) there, parse 'd' modifier, handle it if (TARGET_AVX)
> conditionally). One thing is in pattern, where it increases readability,
> but in a routine which handles several different conditions anyway?
There are several instances of the same construct in this function, so
I think this change follows the established pattern. And I would do it
in the same way in the original patch, if %d was available back
then...
So, Kirill, please go ahead with the patch (after bootstrap/regtest cycle).
Thanks,
Uros.
