Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-02 Thread Richard Guenther 
On Tue, Oct 2, 2012 at 1:11 AM, Xinliang David Li  wrote:
> On Mon, Oct 1, 2012 at 4:05 PM, Sharad Singhai  wrote:
>> Thanks for tracking down and fixing the powerpc port.
>>
>> The "dump_kind_p ()" check is redundant but canonical form here. I
>> think blocks of dump code guarded by "if dump_kind_p (...)" might be
>> easier to read/maintain.
>>
>
> I find it confusing to be honest. The redundant check serves no purpose.

The check should be inlined and avoid the call to the diagnostic routine,
thus speed up compile-time.  We should use this pattern, especially
if it guards multiple calls.

Richard.

> David
>
>> Sharad
>> Sharad
>>
>>
>> On Mon, Oct 1, 2012 at 3:45 PM, Xinliang David Li  wrote:
>>> On Mon, Oct 1, 2012 at 2:37 PM, Michael Meissner
>>>  wrote:
 I tracked down some of the other code that previously used REPORT_DETAILS, 
 and
 MSG_NOTE is the new way to do the same thing.  This bootstraps and no
 unexpected errors occur during make check.  Is it ok to install?

 2012-10-01  Michael Meissner  

 * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
 (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
 Singhai.

 * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.

 Index: gcc/config/rs6000/rs6000.c
 ===
 --- gcc/config/rs6000/rs6000.c  (revision 191932)
 +++ gcc/config/rs6000/rs6000.c  (working copy)
 @@ -58,6 +58,7 @@
  #include "tm-constrs.h"
  #include "opts.h"
  #include "tree-vectorizer.h"
 +#include "dumpfile.h"
  #if TARGET_XCOFF
  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
  #endif
 @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
  {
data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
 -  if (vect_print_dump_info (REPORT_DETAILS))
 -   fprintf (vect_dump,
 -"density %d%%, cost %d exceeds threshold, penalizing "
 -"loop body cost by %d%%", density_pct,
 -vec_cost + not_vec_cost, DENSITY_PENALTY);
 +  if (dump_kind_p (MSG_NOTE))
>>>
>>> Is this check needed? Seems redundant.
>>>
>>> David
>>>
>>>
 +   dump_printf_loc (MSG_NOTE, vect_location,
 +"density %d%%, cost %d exceeds threshold, 
 penalizing "
 +"loop body cost by %d%%", density_pct,
 +vec_cost + not_vec_cost, DENSITY_PENALTY);
  }
  }

 Index: gcc/config/rs6000/t-rs6000
 ===
 --- gcc/config/rs6000/t-rs6000  (revision 191932)
 +++ gcc/config/rs6000/t-rs6000  (working copy)
 @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
 -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
 +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h

  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
  $(srcdir)/config/rs6000/rs6000-protos.h \

 --
 Michael Meissner, IBM
 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
 meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899

Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Xinliang David Li 
On Mon, Oct 1, 2012 at 4:05 PM, Sharad Singhai  wrote:
> Thanks for tracking down and fixing the powerpc port.
>
> The "dump_kind_p ()" check is redundant but canonical form here. I
> think blocks of dump code guarded by "if dump_kind_p (...)" might be
> easier to read/maintain.
>

I find it confusing to be honest. The redundant check serves no purpose.

David

> Sharad
> Sharad
>
>
> On Mon, Oct 1, 2012 at 3:45 PM, Xinliang David Li  wrote:
>> On Mon, Oct 1, 2012 at 2:37 PM, Michael Meissner
>>  wrote:
>>> I tracked down some of the other code that previously used REPORT_DETAILS, 
>>> and
>>> MSG_NOTE is the new way to do the same thing.  This bootstraps and no
>>> unexpected errors occur during make check.  Is it ok to install?
>>>
>>> 2012-10-01  Michael Meissner  
>>>
>>> * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
>>> (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
>>> Singhai.
>>>
>>> * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.
>>>
>>> Index: gcc/config/rs6000/rs6000.c
>>> ===
>>> --- gcc/config/rs6000/rs6000.c  (revision 191932)
>>> +++ gcc/config/rs6000/rs6000.c  (working copy)
>>> @@ -58,6 +58,7 @@
>>>  #include "tm-constrs.h"
>>>  #include "opts.h"
>>>  #include "tree-vectorizer.h"
>>> +#include "dumpfile.h"
>>>  #if TARGET_XCOFF
>>>  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
>>>  #endif
>>> @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
>>>&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>>>  {
>>>data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
>>> -  if (vect_print_dump_info (REPORT_DETAILS))
>>> -   fprintf (vect_dump,
>>> -"density %d%%, cost %d exceeds threshold, penalizing "
>>> -"loop body cost by %d%%", density_pct,
>>> -vec_cost + not_vec_cost, DENSITY_PENALTY);
>>> +  if (dump_kind_p (MSG_NOTE))
>>
>> Is this check needed? Seems redundant.
>>
>> David
>>
>>
>>> +   dump_printf_loc (MSG_NOTE, vect_location,
>>> +"density %d%%, cost %d exceeds threshold, 
>>> penalizing "
>>> +"loop body cost by %d%%", density_pct,
>>> +vec_cost + not_vec_cost, DENSITY_PENALTY);
>>>  }
>>>  }
>>>
>>> Index: gcc/config/rs6000/t-rs6000
>>> ===
>>> --- gcc/config/rs6000/t-rs6000  (revision 191932)
>>> +++ gcc/config/rs6000/t-rs6000  (working copy)
>>> @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
>>>$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
>>>output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
>>>$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
>>> -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
>>> +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
>>>
>>>  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
>>>  $(srcdir)/config/rs6000/rs6000-protos.h \
>>>
>>> --
>>> Michael Meissner, IBM
>>> 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
>>> meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
>>>

Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Sharad Singhai 
Thanks for tracking down and fixing the powerpc port.

The "dump_kind_p ()" check is redundant but canonical form here. I
think blocks of dump code guarded by "if dump_kind_p (...)" might be
easier to read/maintain.

Sharad
Sharad


On Mon, Oct 1, 2012 at 3:45 PM, Xinliang David Li  wrote:
> On Mon, Oct 1, 2012 at 2:37 PM, Michael Meissner
>  wrote:
>> I tracked down some of the other code that previously used REPORT_DETAILS, 
>> and
>> MSG_NOTE is the new way to do the same thing.  This bootstraps and no
>> unexpected errors occur during make check.  Is it ok to install?
>>
>> 2012-10-01  Michael Meissner  
>>
>> * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
>> (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
>> Singhai.
>>
>> * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.
>>
>> Index: gcc/config/rs6000/rs6000.c
>> ===
>> --- gcc/config/rs6000/rs6000.c  (revision 191932)
>> +++ gcc/config/rs6000/rs6000.c  (working copy)
>> @@ -58,6 +58,7 @@
>>  #include "tm-constrs.h"
>>  #include "opts.h"
>>  #include "tree-vectorizer.h"
>> +#include "dumpfile.h"
>>  #if TARGET_XCOFF
>>  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
>>  #endif
>> @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
>>&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>>  {
>>data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
>> -  if (vect_print_dump_info (REPORT_DETAILS))
>> -   fprintf (vect_dump,
>> -"density %d%%, cost %d exceeds threshold, penalizing "
>> -"loop body cost by %d%%", density_pct,
>> -vec_cost + not_vec_cost, DENSITY_PENALTY);
>> +  if (dump_kind_p (MSG_NOTE))
>
> Is this check needed? Seems redundant.
>
> David
>
>
>> +   dump_printf_loc (MSG_NOTE, vect_location,
>> +"density %d%%, cost %d exceeds threshold, 
>> penalizing "
>> +"loop body cost by %d%%", density_pct,
>> +vec_cost + not_vec_cost, DENSITY_PENALTY);
>>  }
>>  }
>>
>> Index: gcc/config/rs6000/t-rs6000
>> ===
>> --- gcc/config/rs6000/t-rs6000  (revision 191932)
>> +++ gcc/config/rs6000/t-rs6000  (working copy)
>> @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
>>$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
>>output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
>>$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
>> -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
>> +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
>>
>>  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
>>  $(srcdir)/config/rs6000/rs6000-protos.h \
>>
>> --
>> Michael Meissner, IBM
>> 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
>> meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
>>

Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Xinliang David Li 
On Mon, Oct 1, 2012 at 2:37 PM, Michael Meissner
 wrote:
> I tracked down some of the other code that previously used REPORT_DETAILS, and
> MSG_NOTE is the new way to do the same thing.  This bootstraps and no
> unexpected errors occur during make check.  Is it ok to install?
>
> 2012-10-01  Michael Meissner  
>
> * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
> (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
> Singhai.
>
> * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.
>
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c  (revision 191932)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -58,6 +58,7 @@
>  #include "tm-constrs.h"
>  #include "opts.h"
>  #include "tree-vectorizer.h"
> +#include "dumpfile.h"
>  #if TARGET_XCOFF
>  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
>  #endif
> @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
>&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>  {
>data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
> -  if (vect_print_dump_info (REPORT_DETAILS))
> -   fprintf (vect_dump,
> -"density %d%%, cost %d exceeds threshold, penalizing "
> -"loop body cost by %d%%", density_pct,
> -vec_cost + not_vec_cost, DENSITY_PENALTY);
> +  if (dump_kind_p (MSG_NOTE))

Is this check needed? Seems redundant.

David


> +   dump_printf_loc (MSG_NOTE, vect_location,
> +"density %d%%, cost %d exceeds threshold, penalizing 
> "
> +"loop body cost by %d%%", density_pct,
> +vec_cost + not_vec_cost, DENSITY_PENALTY);
>  }
>  }
>
> Index: gcc/config/rs6000/t-rs6000
> ===
> --- gcc/config/rs6000/t-rs6000  (revision 191932)
> +++ gcc/config/rs6000/t-rs6000  (working copy)
> @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
>$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
>output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
>$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
> -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
> +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
>
>  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
>  $(srcdir)/config/rs6000/rs6000-protos.h \
>
> --
> Michael Meissner, IBM
> 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
> meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
>

Re: [PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Gabriel Dos Reis 
On Mon, Oct 1, 2012 at 4:37 PM, Michael Meissner
 wrote:
> I tracked down some of the other code that previously used REPORT_DETAILS, and
> MSG_NOTE is the new way to do the same thing.  This bootstraps and no
> unexpected errors occur during make check.  Is it ok to install?

yes -- qualifies as "obvious".  Thanks!


>
> 2012-10-01  Michael Meissner  
>
> * config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
> (rs6000_density_test): Rework to accomidate 09-30 change by Sharad
> Singhai.
>
> * config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.
>
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c  (revision 191932)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -58,6 +58,7 @@
>  #include "tm-constrs.h"
>  #include "opts.h"
>  #include "tree-vectorizer.h"
> +#include "dumpfile.h"
>  #if TARGET_XCOFF
>  #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
>  #endif
> @@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
>&& vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
>  {
>data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
> -  if (vect_print_dump_info (REPORT_DETAILS))
> -   fprintf (vect_dump,
> -"density %d%%, cost %d exceeds threshold, penalizing "
> -"loop body cost by %d%%", density_pct,
> -vec_cost + not_vec_cost, DENSITY_PENALTY);
> +  if (dump_kind_p (MSG_NOTE))
> +   dump_printf_loc (MSG_NOTE, vect_location,
> +"density %d%%, cost %d exceeds threshold, penalizing 
> "
> +"loop body cost by %d%%", density_pct,
> +vec_cost + not_vec_cost, DENSITY_PENALTY);
>  }
>  }
>
> Index: gcc/config/rs6000/t-rs6000
> ===
> --- gcc/config/rs6000/t-rs6000  (revision 191932)
> +++ gcc/config/rs6000/t-rs6000  (working copy)
> @@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
>$(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
>output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
>$(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
> -  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
> +  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
>
>  rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
>  $(srcdir)/config/rs6000/rs6000-protos.h \
>
> --
> Michael Meissner, IBM
> 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
> meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
>

[PATCH] Fix powerpc breakage, was: Add option for dumping to stderr (issue6190057)

2012-10-01 Thread Michael Meissner 
I tracked down some of the other code that previously used REPORT_DETAILS, and
MSG_NOTE is the new way to do the same thing.  This bootstraps and no
unexpected errors occur during make check.  Is it ok to install?

2012-10-01  Michael Meissner  

* config/rs6000/rs6000.c (toplevel): Include dumpfile.h.
(rs6000_density_test): Rework to accomidate 09-30 change by Sharad
Singhai.

* config/rs6000/t-rs6000 (rs6000.o): Add dumpfile.h dependency.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 191932)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -58,6 +58,7 @@
 #include "tm-constrs.h"
 #include "opts.h"
 #include "tree-vectorizer.h"
+#include "dumpfile.h"
 #if TARGET_XCOFF
 #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
 #endif
@@ -3518,11 +3519,11 @@ rs6000_density_test (rs6000_cost_data *d
   && vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
 {
   data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
-  if (vect_print_dump_info (REPORT_DETAILS))
-   fprintf (vect_dump,
-"density %d%%, cost %d exceeds threshold, penalizing "
-"loop body cost by %d%%", density_pct, 
-vec_cost + not_vec_cost, DENSITY_PENALTY);
+  if (dump_kind_p (MSG_NOTE))
+   dump_printf_loc (MSG_NOTE, vect_location,
+"density %d%%, cost %d exceeds threshold, penalizing "
+"loop body cost by %d%%", density_pct,
+vec_cost + not_vec_cost, DENSITY_PENALTY);
 }
 }
 
Index: gcc/config/rs6000/t-rs6000
===
--- gcc/config/rs6000/t-rs6000  (revision 191932)
+++ gcc/config/rs6000/t-rs6000  (working copy)
@@ -26,7 +26,7 @@ rs6000.o: $(CONFIG_H) $(SYSTEM_H) corety
   $(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \
   output.h dbxout.h $(BASIC_BLOCK_H) toplev.h $(GGC_H) $(HASHTAB_H) \
   $(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h gt-rs6000.h \
-  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H)
+  cfgloop.h $(OPTS_H) $(COMMON_TARGET_H) dumpfile.h
 
 rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c \
 $(srcdir)/config/rs6000/rs6000-protos.h \

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899