[PATCH] Canonicalize compares in combine [2/3] Modifications to try_combine()

[Chung-Lin Tang]

Hi Jeff,
I have verified the patch with a native bootstrap + testsuite run on
powerpc-linux (32-bit), results were clean.

Attached is a single patch with the 1+2 combine parts together, with
comments updated. Please check if they feel descriptive enough.

I haven't updated the CANONICALIZE_COMPARISON stuff, as we discussed it
doesn't look like absolutely needed right now. As for the const0_rtx
compare, because the entire case is guarded by a CONST_INT_P, I think it
should be safe.

Is this now okay for trunk?

Thanks,
Chung-Lin

On 2011/4/30 12:01 AM, Jeff Law wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/26/11 05:44, Chung-Lin Tang wrote:
>>
>> Hi Jeff, thanks for reviewing this quite convoluted patch :)
> FWIW, I don't think it's necessarily your patch that is convoluted, but
> instead the original code.  It's also the case that I haven't spent
> nearly as much time in the combiner as I have in other parts of GCC.
> 
> 
>> Yes I'm not doing anything weird here, although the definition of the
>> CANONICALIZE_COMPARISON macro does not seem to forbid (other ports) from
>> doing so, as long as the comparison result is equivalent. It is
>> currently called only at the end of simplify_comparison(), which itself
>> possibly modifies the compare operands.
>>
>> Considering not a lot of targets currently use CANONICALIZE_COMPARISON,
>> maybe it would be feasible to slightly change its interface? Maybe
>> adding a bool parameter to indicate whether the individual operands have
>> to be retained equivalent.
> That can certainly be done if necessary.  I'm just not sure it's needed,
> at least not at this time.  If you want to make that change, feel free
> to submit it.
> 
>>
>>>
>>> I've generally found that avoiding const0_rtx is wise.  Instead I
>>> suggest using CONST0_RTX (mode) to get the constant in the proper mode.
>>>
>>
>> Do you mean the (op1 == const0_rtx) test?
> Yes.  It's a fairly minor issue.
> 
> 
>>
>>> Given the tangled mess this code happens to be, could you please do a
>>> bootstrap test on target with and without SELECT_CC_MODE.  x86 would be
>>> a great example of the former, powerpc the latter (use the build farm if
>>> you need to).   For ppc, just bootstrapping would be fine; I don't think
>>> a full regression test is needed, just some verification that we don't
>>> break ppc build should be sufficient.
>>>
>>
>> Bootstrap and test on x86 is completed already, both 32 and 64 bit.
>> Bootstrap on ARM itself is also completed, using a Pandaboard. I'll try
>> doing a ppc build too.
> Thanks. Given that x86 & ARM are both SELECT_CC_MODE targets, I suspect
> they're OK.  I mostly want a sniff test on a target that doesn't define
> SELECT_CC_MODE (ppc for example).
> 
> Thanks,
> jeff
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iQEcBAEBAgAGBQJNuuDyAAoJEBRtltQi2kC7gMQH/3hANrSFa3MSCTUvNxaiv9sy
> HLoVYgcVXxNCNwYm/uSxZ3titiqyqB7ySrYPEezXCqdaNJeUSk+5v9w23wszyoel
> bITxU6FC1P9wgHCRVNc9NsxpBJeCZleDSPHcrqAdbW8yO6J59qtaaRAlsBjgz11f
> Isg1X+apG0Cy6DZ8knNRDVv9+HyJNLTYCg+90sSQv2Of1obp0b34sq/C2e03+oHz
> gVBkgCvkLazcvYxLrdyCgXfyXvNMbMfSeVclJzpikJZAOAJBCwceYd16wg9ohBZR
> y1g31oZ4teYcLz6c+yWEiWmZpVbeLnZSB9/bkvwP9lrQwkmKyxP33kdYHl/VP1E=
> =ZZwe
> -----END PGP SIGNATURE-----

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: combine-20110506.diff
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110506/13e57bae/attachment.ksh>