This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Vectorizing abs(char/short/int) on x86.
- From: Cong Hou <congh at google dot com>
- To: Uros Bizjak <ubizjak at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Wed, 30 Oct 2013 13:02:57 -0700
- Subject: Re: [PATCH] Vectorizing abs(char/short/int) on x86.
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4Z0XuPm9Js00xp-9p2+0RFZDrs0hEmJNckDvOgOQrrtGA at mail dot gmail dot com> <CAK=A3=3x_BOkyUvRuimD-3r1cpbG82x+NP15OptR-BetBB_mjg at mail dot gmail dot com> <CAFULd4YKqqrZ8nvid2Vy+6nBjDZixF59DdMmcyw0Rx9bORNh-Q at mail dot gmail dot com> <CAK=A3=3YJnZTVnxQPiMv10n=8-CWQe9H+Bh+H_ChKOedZhoFpA at mail dot gmail dot com> <CAFULd4ZRJGsCXabgpjwemNnJ23f7idUMTerS7wspuiBVpKz4Fg at mail dot gmail dot com> <CAK=A3=3-qKTNUaYuvh4iUyufHvQ=kC24CaGEyQBkzJkvgbO_-w at mail dot gmail dot com> <CAFULd4Z0Mx_dpT73AGmZ58gS_HVYPMiUS=Z=gHAMZMhEfYFOdg at mail dot gmail dot com>
I have run check_GNU_style.sh on my patch.
The patch is submitted. Thank you for your comments and help on this patch!
thanks,
Cong
On Wed, Oct 30, 2013 at 11:13 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Oct 30, 2013 at 7:01 PM, Cong Hou <congh@google.com> wrote:
>
>>>> I found my problem: I put DONE outside of if not inside. You are
>>>> right. I have updated my patch.
>>>
>>> OK, great that we put things in order ;)
>>>
>>> Does this patch need some extra middle-end functionality? I was not
>>> able to vectorize char and short part of your patch.
>>
>>
>> In the original patch, I converted abs() on short and char values to
>> their own types by removing type casts. That is, originally char_val1
>> = abs(char_val2) will be converted to char_val1 = (char) abs((int)
>> char_val2) in the frontend, and I would like to convert it back to
>> char_val1 = abs(char_val2). But after several discussions, it seems
>> this conversion has some problems such as overflow converns, and I
>> thereby removed that part.
>>
>> Now you should still be able to vectorize abs(char) and abs(short) but
>> with packing and unpacking. Later I will consider to write pattern
>> recognizer for abs(char) and abs(short) and then the expand on
>> abs(char)/abs(short) in this patch will be used during vectorization.
>
> OK, this seems reasonable. We already have "unused" SSSE3 8/16 bit abs
> pattern, so I think we can commit SSE2 expanders, even if they will be
> unused for now. The proposed recognizer will benefit SSE2 as well as
> existing SSSE3 patterns.
>
>>> Regarding the testcase - please put it to gcc.target/i386/ directory.
>>> There is nothing generic in the test, as confirmed by target-dependent
>>> scan test. You will find plenty of examples in the mentioned
>>> directory. I'd suggest to split the testcase in three files, and to
>>> simplify it to something like the testcase with global variables I
>>> used earlier.
>>
>>
>> I have done it. The test case is split into three for s8/s16/s32 in
>> gcc.target/i386.
>
> OK.
>
> The patch is OK for mainline, but please check formatting and
> whitespace before the patch is committed.
>
> Thanks,
> Uros.