public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Patch fixing a typo.
@ 2009-04-22 20:31 Vladimir Makarov
  2009-04-22 21:27 ` DJ Delorie
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Vladimir Makarov @ 2009-04-22 20:31 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ian Lance Taylor

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

I've made a mistake when prepared a big patch in Dec (this patch without 
#ifdef was successfully tested on m32c.  #ifdef and #endif were added a 
bit lately and the final patch was bootstrapped and tested on 
x86/x86_64).  Ian recently found this typo

http://gcc.gnu.org/ml/gcc/2009-04/msg00544.html

The typo was not a serious one.  It could have resulted in worse code 
for most targets when -fira-algorithm=priority is used (it is not a 
default for all architectures except for m32c).  Although it might have 
created problems for m32c (a weird architecture which uses priority 
coloring by default).

Here is the fix

2009-04-22  Vladimir Makarov  <vmakarov@redhat.com>

    * genpreds.c (write_enum_constraint_num): Output definition of
    CONSTRAINT_NUM_DEFINED_P macro.
    * ira.c (setup_cover_and_important_classes): Use
    CONSTRAINT_NUM_DEFINED_P instead of CONSTRAINT__LIMIT in #ifdef.
   

The patch was successfully bootstrapped on x86_64.  I hope it will also 
fix a m32c failure reported by DJ recently.

Is it ok to commit?


[-- Attachment #2: constraint-limit.patch --]
[-- Type: text/plain, Size: 896 bytes --]

Index: genpreds.c
===================================================================
--- genpreds.c	(revision 146528)
+++ genpreds.c	(working copy)
@@ -954,6 +954,7 @@ write_enum_constraint_num (void)
 {
   struct constraint_data *c;
 
+  fputs ("#define CONSTRAINT_NUM_DEFINED_P 1\n", stdout);
   fputs ("enum constraint_num\n"
 	 "{\n"
 	 "  CONSTRAINT__UNKNOWN = 0", stdout);
Index: ira.c
===================================================================
--- ira.c	(revision 146528)
+++ ira.c	(working copy)
@@ -754,9 +754,9 @@ setup_cover_and_important_classes (void)
 	{
 	  if (i == NO_REGS)
 	    continue;
-#ifdef CONSTRAINT__LIMIT
+#ifdef CONSTRAINT_NUM_DEFINED_P
 	  for (j = 0; j < CONSTRAINT__LIMIT; j++)
-	    if ((int) regclass_for_constraint (j) == i)
+	    if ((int) regclass_for_constraint ((enum constraint_num) j) == i)
 	      break;
 	  if (j < CONSTRAINT__LIMIT)
 	    {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch fixing a typo.
  2009-04-22 20:31 Patch fixing a typo Vladimir Makarov
@ 2009-04-22 21:27 ` DJ Delorie
  2009-04-25  0:36 ` Ian Lance Taylor
  2009-04-25  0:53 ` H.J. Lu
  2 siblings, 0 replies; 6+ messages in thread
From: DJ Delorie @ 2009-04-22 21:27 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: gcc-patches


> I hope it will also fix a m32c failure reported by DJ recently.

Sorry, it doesn't.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch fixing a typo.
  2009-04-22 20:31 Patch fixing a typo Vladimir Makarov
  2009-04-22 21:27 ` DJ Delorie
@ 2009-04-25  0:36 ` Ian Lance Taylor
  2009-04-25  0:53 ` H.J. Lu
  2 siblings, 0 replies; 6+ messages in thread
From: Ian Lance Taylor @ 2009-04-25  0:36 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: gcc-patches

Vladimir Makarov <vmakarov@redhat.com> writes:

> 2009-04-22  Vladimir Makarov  <vmakarov@redhat.com>
>
>    * genpreds.c (write_enum_constraint_num): Output definition of
>    CONSTRAINT_NUM_DEFINED_P macro.
>    * ira.c (setup_cover_and_important_classes): Use
>    CONSTRAINT_NUM_DEFINED_P instead of CONSTRAINT__LIMIT in #ifdef.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch fixing a typo.
  2009-04-22 20:31 Patch fixing a typo Vladimir Makarov
  2009-04-22 21:27 ` DJ Delorie
  2009-04-25  0:36 ` Ian Lance Taylor
@ 2009-04-25  0:53 ` H.J. Lu
  2009-04-25  1:21   ` H.J. Lu
  2 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2009-04-25  0:53 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: gcc-patches, Ian Lance Taylor

On Wed, Apr 22, 2009 at 1:22 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> I've made a mistake when prepared a big patch in Dec (this patch without
> #ifdef was successfully tested on m32c.  #ifdef and #endif were added a bit
> lately and the final patch was bootstrapped and tested on x86/x86_64).  Ian
> recently found this typo
>
> http://gcc.gnu.org/ml/gcc/2009-04/msg00544.html
>
> The typo was not a serious one.  It could have resulted in worse code for
> most targets when -fira-algorithm=priority is used (it is not a default for
> all architectures except for m32c).  Although it might have created problems
> for m32c (a weird architecture which uses priority coloring by default).
>
> Here is the fix
>
> 2009-04-22  Vladimir Makarov  <vmakarov@redhat.com>
>
>   * genpreds.c (write_enum_constraint_num): Output definition of
>   CONSTRAINT_NUM_DEFINED_P macro.
>   * ira.c (setup_cover_and_important_classes): Use
>   CONSTRAINT_NUM_DEFINED_P instead of CONSTRAINT__LIMIT in #ifdef.
>
> The patch was successfully bootstrapped on x86_64.  I hope it will also fix
> a m32c failure reported by DJ recently.
>
> Is it ok to commit?
>
>
> Index: genpreds.c
> ===================================================================
> --- genpreds.c  (revision 146528)
> +++ genpreds.c  (working copy)
> @@ -954,6 +954,7 @@ write_enum_constraint_num (void)
>  {
>   struct constraint_data *c;
>
> +  fputs ("#define CONSTRAINT_NUM_DEFINED_P 1\n", stdout);
>   fputs ("enum constraint_num\n"
>         "{\n"
>         "  CONSTRAINT__UNKNOWN = 0", stdout);
> Index: ira.c
> ===================================================================
> --- ira.c       (revision 146528)
> +++ ira.c       (working copy)
> @@ -754,9 +754,9 @@ setup_cover_and_important_classes (void)
>        {
>          if (i == NO_REGS)
>            continue;
> -#ifdef CONSTRAINT__LIMIT
> +#ifdef CONSTRAINT_NUM_DEFINED_P
>          for (j = 0; j < CONSTRAINT__LIMIT; j++)
> -           if ((int) regclass_for_constraint (j) == i)
> +           if ((int) regclass_for_constraint ((enum constraint_num) j) ==
> i)
>              break;
>          if (j < CONSTRAINT__LIMIT)
>            {
>

It breaks gcc:

cc1: warnings being treated as errors
../../src-trunk/gcc/ira.c: In function 'setup_cover_and_important_classes':
../../src-trunk/gcc/ira.c:763: error: enum conversion in assignment is
invalid in C++
make[6]: *** [ira.o] Error 1

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch fixing a typo.
  2009-04-25  0:53 ` H.J. Lu
@ 2009-04-25  1:21   ` H.J. Lu
  2009-04-25 17:00     ` Vladimir Makarov
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2009-04-25  1:21 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: gcc-patches, Ian Lance Taylor

On Fri, Apr 24, 2009 at 5:36 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Apr 22, 2009 at 1:22 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>> I've made a mistake when prepared a big patch in Dec (this patch without
>> #ifdef was successfully tested on m32c.  #ifdef and #endif were added a bit
>> lately and the final patch was bootstrapped and tested on x86/x86_64).  Ian
>> recently found this typo
>>
>> http://gcc.gnu.org/ml/gcc/2009-04/msg00544.html
>>
>> The typo was not a serious one.  It could have resulted in worse code for
>> most targets when -fira-algorithm=priority is used (it is not a default for
>> all architectures except for m32c).  Although it might have created problems
>> for m32c (a weird architecture which uses priority coloring by default).
>>
>> Here is the fix
>>
>> 2009-04-22  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>   * genpreds.c (write_enum_constraint_num): Output definition of
>>   CONSTRAINT_NUM_DEFINED_P macro.
>>   * ira.c (setup_cover_and_important_classes): Use
>>   CONSTRAINT_NUM_DEFINED_P instead of CONSTRAINT__LIMIT in #ifdef.
>>
>> The patch was successfully bootstrapped on x86_64.  I hope it will also fix
>> a m32c failure reported by DJ recently.
>>
>> Is it ok to commit?
>>
>>
>> Index: genpreds.c
>> ===================================================================
>> --- genpreds.c  (revision 146528)
>> +++ genpreds.c  (working copy)
>> @@ -954,6 +954,7 @@ write_enum_constraint_num (void)
>>  {
>>   struct constraint_data *c;
>>
>> +  fputs ("#define CONSTRAINT_NUM_DEFINED_P 1\n", stdout);
>>   fputs ("enum constraint_num\n"
>>         "{\n"
>>         "  CONSTRAINT__UNKNOWN = 0", stdout);
>> Index: ira.c
>> ===================================================================
>> --- ira.c       (revision 146528)
>> +++ ira.c       (working copy)
>> @@ -754,9 +754,9 @@ setup_cover_and_important_classes (void)
>>        {
>>          if (i == NO_REGS)
>>            continue;
>> -#ifdef CONSTRAINT__LIMIT
>> +#ifdef CONSTRAINT_NUM_DEFINED_P
>>          for (j = 0; j < CONSTRAINT__LIMIT; j++)
>> -           if ((int) regclass_for_constraint (j) == i)
>> +           if ((int) regclass_for_constraint ((enum constraint_num) j) ==
>> i)
>>              break;
>>          if (j < CONSTRAINT__LIMIT)
>>            {
>>
>
> It breaks gcc:
>
> cc1: warnings being treated as errors
> ../../src-trunk/gcc/ira.c: In function 'setup_cover_and_important_classes':
> ../../src-trunk/gcc/ira.c:763: error: enum conversion in assignment is
> invalid in C++
> make[6]: *** [ira.o] Error 1
>

I am checking it in as an obvious fix.


H.J.
Index: ira.c
===================================================================
--- ira.c	(revision 146749)
+++ ira.c	(working copy)
@@ -760,7 +760,7 @@ setup_cover_and_important_classes (void)
 	      break;
 	  if (j < CONSTRAINT__LIMIT)
 	    {
-	      classes[n++] = i;
+	      classes[n++] = (enum reg_class) i;
 	      continue;
 	    }
 #endif

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Patch fixing a typo.
  2009-04-25  1:21   ` H.J. Lu
@ 2009-04-25 17:00     ` Vladimir Makarov
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Makarov @ 2009-04-25 17:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Ian Lance Taylor

H.J. Lu wrote:
> On Fri, Apr 24, 2009 at 5:36 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>   
>>
>> It breaks gcc:
>>
>> cc1: warnings being treated as errors
>> ../../src-trunk/gcc/ira.c: In function 'setup_cover_and_important_classes':
>> ../../src-trunk/gcc/ira.c:763: error: enum conversion in assignment is
>> invalid in C++
>> make[6]: *** [ira.o] Error 1
>>
>>     
>
>   
H.J., thanks for fixing that.  I am really sorry for breaking the bootstrap.
> I am checking it in as an obvious fix.
>
>
> H.J.
> Index: ira.c
> ===================================================================
> --- ira.c	(revision 146749)
> +++ ira.c	(working copy)
> @@ -760,7 +760,7 @@ setup_cover_and_important_classes (void)
>  	      break;
>  	  if (j < CONSTRAINT__LIMIT)
>  	    {
> -	      classes[n++] = i;
> +	      classes[n++] = (enum reg_class) i;
>  	      continue;
>  	    }
>  #endif
>   

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-04-25 15:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-22 20:31 Patch fixing a typo Vladimir Makarov
2009-04-22 21:27 ` DJ Delorie
2009-04-25  0:36 ` Ian Lance Taylor
2009-04-25  0:53 ` H.J. Lu
2009-04-25  1:21   ` H.J. Lu
2009-04-25 17:00     ` Vladimir Makarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).