Re: PATCH: PR target/42542: Vectorizer produces incorrect results on max of signed intergers

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Jan 5, 2010 at 9:08 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On 01/05/2010 05:17 PM, Paolo Bonzini wrote:
>>>>
>>>> You can always subtract (or XOR) the sign bit to turn an unsigned
>>>> value to the corresponding signed value. The attached patch (on top of
>>>> the code before H.J.'s patch) fixes the C testcase in the PR, but I
>>>> didn't test it further and I don't have time unfortunately.
>>>
>>> It will work until you change the sign bit of 0x80000000, either of an
>>> input operand or an intermediate result of subtraction. For the
>>> reference of interested readers, this problem is described in [1].
>>> I see no other way to fixup the underflow. Some comments below:
>>
>> No, that's a different thing. ?The problems with 0x8000000 arise when you
>> have 0x80000000 as a signed number, while here you have 0x80000000 as an
>> unsigned number.
>>
>> Subtracting 0x80000000 from a number in the range 0 to 0xFFFFFFFF will
>> produce a result in the range -0x80000000 to 0x7FFFFFFF which is exactly
>> two's complement. ?XOR just happens to give the same result.
>>
>> It's a very well known trick IMNSHO. ?Please provide a counterexample if
>> you think there is one.
>
> I don't have one. 0x8000000 was all that worried me.
>
> H.J., can you please revert your patch and fix the PR with Paolo's proposed
> solution?
>
> Thanks,
> Uros.
>
>
>

Here is the patch. OK for trunk?

Thanks.

-- 
H.J.
---
gcc/

2010-01-05  Paolo Bonzini  <bonzinI@gnu.rg>
	    H.J. Lu  <hongjiu.lu@intel.com>

	PR target/42542
	* config/i386/i386.c (ix86_expand_int_vcond): Convert GTU to GT
	for V4SI and V2DI by subtracting (-(INT MAX) - 1) from both
	operands to make them signed.

	* config/i386/sse.md (umaxv4si3): Revert the last change.
	(umin<mode>3): Likewise.
	(uminv8hi3): Removed.
	(uminv4si3): Likewise.

gcc/testsuite/

2010-01-05  H.J. Lu  <hongjiu.lu@intel.com>

	* gcc.target/i386/pr42542-1.c (res): Make it 8 elements.

Attachment: gcc-pr42542-4.patch
Description: Text document