Patchwork [combine] Fix host-specific behavior in simplify_compare_const()

login
register
mail settings
Submitter Chung-Ju Wu
Date March 7, 2013, 5:48 a.m.
Message ID <CADj25HNj9OZvnwsjWogvEbC1ELVNb9Wo4qBgNjRB4wO5YqYiLA@mail.gmail.com>
Download mbox | patch
Permalink /patch/225727/
State New
Headers show

Comments

Chung-Ju Wu - March 7, 2013, 5:48 a.m.
2013/3/5 Eric Botcazou <ebotcazou@adacore.com>:
>> In other words, any 32-bit target with 'need_64bit_hwint=yes' in config.gcc
>> is not able to have benefit from this optimization because it never
>> passes the condition test.
>>
>>
>> My solution is to use GET_MODE_MASK(mode) to filter out all bits not
>> in target mode. The following is my patch:
>
> The patch is OK for 4.9 once stage #1 is open if it passes bootstrap/regtest.

Thanks for the approval. I will wait for 4.9 stage1 opening.
The following is the new patch according to your suggestions:


gcc/ChangeLog:

	* combine.c (simplify_compare_const): Use GET_MODE_MASK to filter out
	unnecessary bits in the constant power of two case.


       const_op = 0;



Best regards,
jasonwucj
Chung-Ju Wu - April 6, 2013, 7:42 a.m.
2013/3/7 Chung-Ju Wu <jasonwucj@gmail.com>:
> 2013/3/5 Eric Botcazou <ebotcazou@adacore.com>:
>>> In other words, any 32-bit target with 'need_64bit_hwint=yes' in config.gcc
>>> is not able to have benefit from this optimization because it never
>>> passes the condition test.
>>>
>>>
>>> My solution is to use GET_MODE_MASK(mode) to filter out all bits not
>>> in target mode. The following is my patch:
>>
>> The patch is OK for 4.9 once stage #1 is open if it passes bootstrap/regtest.
>
> Thanks for the approval. I will wait for 4.9 stage1 opening.
> The following is the new patch according to your suggestions:
>

Hi, Eric,

Since now it is on 4.9 stage1, I would like to contribute this patch.
The followings are previous discussion:
http://gcc.gnu.org/ml/gcc-patches/2013-02/msg01247.html
http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00169.html
http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00280.html

The attached patch has passed bootstrap/regression test
on x86_64-unknown-linux-gnu with current main trunk.
A plaintext gcc/ChangeLog is as below:


2013-04-06  Chung-Ju Wu  <jasonwucj@gmail.com>

        * combine.c (simplify_compare_const): Use GET_MODE_MASK to filter out
        unnecessary bits in the constant power of two case.


On behalf of Andes Technology Co., we have signed FSF agreement.
However, so far I don't have svn write access yet.
Would you please help to commit this patch?

Thanks again for the approval and I really appreciate your help. :)


Best regards,
jasonwucj
Jeff Law - April 7, 2013, 2:31 a.m.
On 04/06/2013 01:42 AM, Chung-Ju Wu wrote:
>>
> The attached patch has passed bootstrap/regression test
> on x86_64-unknown-linux-gnu with current main trunk.
> A plaintext gcc/ChangeLog is as below:
>
>
> 2013-04-06  Chung-Ju Wu  <jasonwucj@gmail.com>
>
>          * combine.c (simplify_compare_const): Use GET_MODE_MASK to filter out
>          unnecessary bits in the constant power of two case.
>
>
> On behalf of Andes Technology Co., we have signed FSF agreement.
> However, so far I don't have svn write access yet.
> Would you please help to commit this patch?
>
> Thanks again for the approval and I really appreciate your help. :)
I'd suggest getting write access to the repository.


See "Authenticated access" on this page:


http://gcc.gnu.org/svnwrite.html

List me as your sponsor.

jeff
Chung-Ju Wu - April 7, 2013, 1:46 p.m.
2013/4/7 Jeff Law <law@redhat.com>:
> On 04/06/2013 01:42 AM, Chung-Ju Wu wrote:
>>
>> On behalf of Andes Technology Co., we have signed FSF agreement.
>> However, so far I don't have svn write access yet.
>> Would you please help to commit this patch?
>>
>> Thanks again for the approval and I really appreciate your help. :)
>
> I'd suggest getting write access to the repository.
>
> See "Authenticated access" on this page:
>
> http://gcc.gnu.org/svnwrite.html
>
> List me as your sponsor.
>
> jeff
>
>

Thanks!!
I will follow the instructions to get my svn write access,
and then contribute this patch.

I am also planning to contribute a new CPU port, 'nds32' target,
along with related binutils, gdb, and newlib.
(We have signed FSF agreement for binutils and gdb as well.)
Currently, we are working hard to improve our implementation
and follow coding style for current main trunk.
I believe we are able to propose complete nds32 contribution,
on June or July, for GCC community to review it.

Thanks again. We will do our best to make contribution
for GCC and GNU toolchain. :-)


Best regards,
jasonwucj
Eric Botcazou - April 9, 2013, 8:45 a.m.
> The attached patch has passed bootstrap/regression test
> on x86_64-unknown-linux-gnu with current main trunk.
> A plaintext gcc/ChangeLog is as below:
> 
> 
> 2013-04-06  Chung-Ju Wu  <jasonwucj@gmail.com>
> 
>         * combine.c (simplify_compare_const): Use GET_MODE_MASK to filter
> out unnecessary bits in the constant power of two case.
> 
> 
> On behalf of Andes Technology Co., we have signed FSF agreement.
> However, so far I don't have svn write access yet.
> Would you please help to commit this patch?

Jeff has kindly offered to sponsor you for write access, so you should be able 
to install it yourself.  Otherwise I'll be happy to do it for you.

Thanks for fixing this problem in the combiner.
Chung-Ju Wu - April 10, 2013, 9:17 a.m.
2013/4/9 Eric Botcazou <ebotcazou@adacore.com>:
>> On behalf of Andes Technology Co., we have signed FSF agreement.
>> However, so far I don't have svn write access yet.
>> Would you please help to commit this patch?
>
> Jeff has kindly offered to sponsor you for write access, so you should be able
> to install it yourself.  Otherwise I'll be happy to do it for you.
>
> Thanks for fixing this problem in the combiner.
>
> --
> Eric Botcazou

With Jeff's help, I got my svn write access yesterday
and added myself to MAINTAINERS in "Write After Approval" section.

With your approval and suggestions, the revised patch
(http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00354.html)
is committed as Rev. 197666.

Appreciate your approval and Jeff's help. :)


Best regards,
jasonwucj

Patch

diff --git a/gcc/combine.c b/gcc/combine.c
index 67bd776..ce2b583 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10917,8 +10917,9 @@  simplify_compare_const (enum rtx_code code,
rtx op0, rtx *pop1)
       && (code == EQ || code == NE || code == GE || code == GEU
           || code == LT || code == LTU)
       && mode_width <= HOST_BITS_PER_WIDE_INT
-      && exact_log2 (const_op) >= 0
-      && nonzero_bits (op0, mode) == (unsigned HOST_WIDE_INT) const_op)
+      && exact_log2 (const_op & GET_MODE_MASK (mode)) >= 0
+      && (nonzero_bits (op0, mode)
+          == (unsigned HOST_WIDE_INT) (const_op & GET_MODE_MASK (mode))))
     {
       code = (code == EQ || code == GE || code == GEU ? NE : EQ);