Re: [Patch, Fortran, OOP] PR 57306: ICE on valid with class pointer initialization

[Janus Weil]

Hi,

> Sorry for the belated review.
>
> +  bool ptr = sym->attr.pointer || sym->attr.allocatable
> +|| (sym->ts.type == BT_CLASS
> +&& CLASS_DATA (sym)->attr.class_pointer);
>
>
> That looks quite imbalanced. Why do you not take care of
> CLASS_DATA(sym)->attr.allocatable? Actually, shouldn't that always be true
> for BT_CLASS in this context? A BT_CLASS should either be a
> pointer/allocatable or a dummy argument - but the latter is never
> initialized (while being a dummy).

right. Then it should be ok to just check for BT_CLASS. Updated patch attached.


> Otherwise, it looks OK to me.

Thanks. I will commit the attached version after another regtest.


> (Don't forget the dg-do compile -> run change.)

Done.


> From follow-up emails:
>>>
>>> type t
>>>class(*), pointer :: x
>>> end type t
>>> type(t), target :: y
>>> integer,target :: z
>>> type(t) :: x = t(y)
>>> type(t) :: x = t(z)
>>> class(*), pointer :: a => y
>>
>> Your example yields (with and without the current patch):
>>
>> type(t) :: x = t(y)
>>   1
>> Error: Can't convert TYPE(t) to CLASS(*) at (1)
>>
>> In fact the patch does not really handle unlimited polymorphics. I
>> suspect several fixes might be needed to get your test case running.
>> Is it ok to do this in a follow-up patch?
>
>
> Seems as if there is more work required ... Yes, a follow-up patch is fine,
> but somewhere the omission should be recorded. (As you did in PR49213.)
>
> Talking about the example above, the following is similar and may or may not
> be handled:
>
> integer, target :: tgt
> type t2
> end type t2
> type(t2), target :: tgt2
> type t
>   class(*), pointer :: a => tgt
>   class(*), pointer :: b => tgt2
> end type t
> type(t) :: x
> type(t), SAVE :: y
> end

In addition to the can't-convert error, this one also gives

integer, target :: tgt
  1
Internal Error at (1):

but I don't directly see why. I will also add it to the above PR to
keep track of it.

Cheers,
Janus


pr57306_v5.diff
Description: Binary data


pointer_init_8.f90
Description: Binary data

Re: [Patch, Fortran, OOP] PR 57306: ICE on valid with class pointer initialization

[Janus Weil]

>> Sorry for the belated review.
>>
>> +  bool ptr = sym->attr.pointer || sym->attr.allocatable
>> +|| (sym->ts.type == BT_CLASS
>> +&& CLASS_DATA (sym)->attr.class_pointer);
>>
>>
>> That looks quite imbalanced. Why do you not take care of
>> CLASS_DATA(sym)->attr.allocatable? Actually, shouldn't that always be true
>> for BT_CLASS in this context? A BT_CLASS should either be a
>> pointer/allocatable or a dummy argument - but the latter is never
>> initialized (while being a dummy).
>
> right. Then it should be ok to just check for BT_CLASS. Updated patch 
> attached.
>
>
>> Otherwise, it looks OK to me.
>
> Thanks. I will commit the attached version after another regtest.

Committed as r201521.


Cheers,
Janus

Re: Limit template parameters in hashtable

[François Dumont]

Ok then attached patch applied.

2013-08-06  François Dumont  

* include/bits/hashtable_policy.h (_Hashtable_alloc): New.
(_ReuseOrAllocNode, _AllocNode): Adapt to use latter rather than
_Hashtable.
(_Before_begin<>): Remove.
* include/bits/hashtable.h (_Hashtable): Inherit from
_Hashtable_alloc and adapt. Restore _M_before_begin field.
* python/libstdcxx/v6/printers.py (StdHashtableIterator): Adapt
access to hashtable before begin.
* testsuite/23_containers/unordered_set/
not_default_constructible_hash_neg.cc: Adapt dg-error line number.
* testsuite/23_containers/unordered_set/instantiation_neg.cc:
Likewise.

François


On 08/05/2013 11:39 AM, Jonathan Wakely wrote:

On 28 July 2013 17:44, François Dumont wrote:

Is it ok for this one ?

Yes, sorry for the delay, this is OK to commit.  I think the code is
cleaner now, and I agree with your EBO improvement that puts _M_before
_begin back in the main hashtable.

Thanks very much for doing this.



Index: include/bits/hashtable_policy.h
===
--- include/bits/hashtable_policy.h	(revision 201521)
+++ include/bits/hashtable_policy.h	(working copy)
@@ -102,25 +102,26 @@
   { return std::get<0>(std::forward<_Tp>(__x)); }
   };
 
+  template
+struct _Hashtable_alloc;
+
   // Functor recycling a pool of nodes and using allocation once the pool is
   // empty.
-  template
+  template
 struct _ReuseOrAllocNode
 {
 private:
-  using __hashtable = _Hashtable<_Key, _Value, _Alloc, _ExtractKey,
- _Equal, _H1, _H2, _Hash,
- _RehashPolicy, _Traits>;
-  using __val_alloc_type = typename __hashtable::_Value_alloc_type;
-  using __val_alloc_traits = typename __hashtable::_Value_alloc_traits;
-  using __node_alloc_traits = typename __hashtable::_Node_alloc_traits;
-  using __node_type = typename __hashtable::__node_type;
+  using __node_alloc_type = _NodeAlloc;
+  using __hashtable_alloc = _Hashtable_alloc<__node_alloc_type>;
+  using __value_alloc_type = typename __hashtable_alloc::__value_alloc_type;
+  using __value_alloc_traits =
+	typename __hashtable_alloc::__value_alloc_traits;
+  using __node_alloc_traits =
+	typename __hashtable_alloc::__node_alloc_traits;
+  using __node_type = typename __hashtable_alloc::__node_type;
 
 public:
-  _ReuseOrAllocNode(__node_type* __nodes, __hashtable& __h)
+  _ReuseOrAllocNode(__node_type* __nodes, __hashtable_alloc& __h)
 	: _M_nodes(__nodes), _M_h(__h) { }
   _ReuseOrAllocNode(const _ReuseOrAllocNode&) = delete;
 
@@ -136,12 +137,12 @@
 	  __node_type* __node = _M_nodes;
 	  _M_nodes = _M_nodes->_M_next();
 	  __node->_M_nxt = nullptr;
-	  __val_alloc_type __a(_M_h._M_node_allocator());
-	  __val_alloc_traits::destroy(__a, __node->_M_valptr());
+	  __value_alloc_type __a(_M_h._M_node_allocator());
+	  __value_alloc_traits::destroy(__a, __node->_M_valptr());
 	  __try
 		{
-		  __val_alloc_traits::construct(__a, __node->_M_valptr(),
-		std::forward<_Arg>(__arg));
+		  __value_alloc_traits::construct(__a, __node->_M_valptr(),
+		  std::forward<_Arg>(__arg));
 		}
 	  __catch(...)
 		{
@@ -157,24 +158,19 @@
 
 private:
   mutable __node_type* _M_nodes;
-  __hashtable& _M_h;
+  __hashtable_alloc& _M_h;
 };
 
   // Functor similar to the previous one but without any pool of node to recycle.
-  template
+  template
 struct _AllocNode
 {
 private:
-  using __hashtable = _Hashtable<_Key, _Value, _Alloc, _ExtractKey,
- _Equal, _H1, _H2, _Hash,
- _RehashPolicy, _Traits>;
-  using __node_type = typename __hashtable::__node_type;
+  using __hashtable_alloc = _Hashtable_alloc<_NodeAlloc>;
+  using __node_type = typename __hashtable_alloc::__node_type;
 
 public:
-  _AllocNode(__hashtable& __h)
+  _AllocNode(__hashtable_alloc& __h)
 	: _M_h(__h) { }
 
   template
@@ -183,7 +179,7 @@
 	{ return _M_h._M_allocate_node(std::forward<_Arg>(__arg)); }
 
 private:
-  __hashtable& _M_h;
+  __hashtable_alloc& _M_h;
 };
 
   // Auxiliary types used for all instantiations of _Hashtable nodes
@@ -247,6 +243,8 @@
   template
 struct _Hash_node_value_base : _Hash_node_base
 {
+  typedef _Value value_type;
+
   __gnu_cxx::__aligned_buffer<_Value> _M_storage;
 
   _Value*
@@ -336,9 +334,9 @@
   using __node_type = typename __base_type::__node_type;
 
 public:
-  typedef _Value   value_type;
-  typedef std::ptrdiff_t   difference_type;
-  typedef std::forward_iterator_tagiterator_category;
+  typedef _Value	value_type;
+  typedef std::ptrdiff_tdifference_type;
+  typedef std::forward_iterator_tag			iterator_category;
 
   using pointer = typename std::conditional<__constant_iterators,
 		

Re: New parameters to control stringop expansion libcall strategy

[Michael Zolotukhin]

There are still some formatting issues (like 8 spaces instead of a
tab, wrong indentation of do-loop and some other places) - to reveal
some of them you could use contrib/check_GNU_style.sh script.
But that was a nitpicking again:) Actually I wanted to ask whether
you're going to use this option for some performance experiments
involving memmov/memset - if so, probably you could tune existing
cost-models as well? Is it possible?

Michael

On 5 August 2013 20:44, Xinliang David Li  wrote:
> thanks. Updated patch attached.
>
> David
>
> On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin
>  wrote:
>> Hi,
>> This is a really convenient option, thanks for working on it.
>> I can't approve it as I'm not a maintainer, but it looks ok to me,
>> except fot a small nitpicking: afair, comments should end with
>> dot-space-space.
>>
>> Michael
>>
>> On 04 Aug 20:01, Xinliang David Li wrote:
>>> The attached is a new patch implementing the stringop inline strategy
>>> control using two new -m options:
>>>
>>> -mmemcpy-strategy=
>>> -mmemset-strategy=
>>>
>>> See changes in doc/invoke.texi for description of the new options. Example:
>>>   
>>> -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned
>>>
>>> tells compiler to inline memcpy using rep_8byte when the size is no
>>> larger than 64 byte, using unrolled_loop when size is no larger than
>>> 2048, and for size > 2048, using library call. In all cases,
>>> destination alignment adjustment is not done.
>>>
>>> Tested on x86-64/linux. Ok for trunk?
>>>
>>> thanks,
>>>
>>> David
>>>
>>> 2013-08-02  Xinliang David Li  
>>>
>>> * config/i386/stringop.def: New file.
>>> * config/i386/stringop.opt: New file.
>>> * config/i386/i386-opts.h: Include stringopt.def.
>>> * config/i386/i386.opt: Include stringopt.opt.
>>> * config/i386/i386.c (ix86_option_override_internal):
>>> Override default size based stringop inline strategies
>>> with options.
>>> * config/i386/i386.c (ix86_parse_stringop_strategy_string):
>>> New function.
>>>
>>> 2013-08-04  Xinliang David Li  
>>>
>>> * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test.
>>> * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto.
>>> * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto.
>>> * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto.
>>>
>>>
>>>
>>>
>>> On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li  
>>> wrote:
>>> > On x86_64, when the expected size of memcpy/memset is known (e.g, with
>>> > FDO), libcall strategy is used with the size is > 8192. This value is
>>> > hard coded, which makes it hard to do performance tuning. This patch
>>> > adds two new parameters to do that. Potential usage includes
>>> > per-application libcall strategy min-size tuning based on summary data
>>> > with FDO (e.g, instruction workset size).
>>> >
>>> > Bootstrap and tested on x86_64/linux. Ok for trunk?
>>> >
>>> > thanks,
>>> >
>>> > David
>>> >
>>> >
>>> > 2013-08-02  Xinliang David Li  
>>> >
>>> > * params.def: New parameters.
>>> > * config/i386/i386.c (ix86_option_override_internal):
>>> > Override default libcall size limit with parameters.
>>
>>> Index: config/i386/stringop.def
>>> ===
>>> --- config/i386/stringop.def  (revision 0)
>>> +++ config/i386/stringop.def  (revision 0)
>>> @@ -0,0 +1,42 @@
>>> +/* Definitions for option handling for IA-32.
>>> +   Copyright (C) 2013 Free Software Foundation, Inc.
>>> +
>>> +This file is part of GCC.
>>> +
>>> +GCC is free software; you can redistribute it and/or modify
>>> +it under the terms of the GNU General Public License as published by
>>> +the Free Software Foundation; either version 3, or (at your option)
>>> +any later version.
>>> +
>>> +GCC is distributed in the hope that it will be useful,
>>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +GNU General Public License for more details.
>>> +
>>> +Under Section 7 of GPL version 3, you are granted additional
>>> +permissions described in the GCC Runtime Library Exception, version
>>> +3.1, as published by the Free Software Foundation.
>>> +
>>> +You should have received a copy of the GNU General Public License and
>>> +a copy of the GCC Runtime Library Exception along with this program;
>>> +see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>> +.  */
>>> +
>>> +DEF_ENUM
>>> +DEF_ALG (no_stringop, no_stringop)
>>> +DEF_ENUM
>>> +DEF_ALG (libcall, libcall)
>>> +DEF_ENUM
>>> +DEF_ALG (rep_prefix_1_byte, rep_byte)
>>> +DEF_ENUM
>>> +DEF_ALG (rep_prefix_4_byte, rep_4byte)
>>> +DEF_ENUM
>>> +DEF_ALG (rep_prefix_8_byte, rep_8byte)
>>> +DEF_ENUM
>>> +DEF_ALG (loop_1_byte, byte_loop)
>>> +DEF_ENUM
>>> +DEF_ALG (loop, loop)
>>> +DEF_ENUM
>>> +DEF_ALG 

Re: [AARCH64][Insn classification unification 3/N] ALU/shift types

[James Greenhalgh]

*Ping*

James

On Thu, Aug 01, 2013 at 02:50:07PM +0100, Sofiane Naci wrote:
> Hi,
> 
> This patch is part of the ongoing work to unify instruction classification
> between the ARM and AARCH64 backends.
> 
> This patch fine tunes the ALU/shift type attribute values in the ARM backend
> as detailed in the table below:
> 
> Old  New
> --
> arlo_imm alu_imm, alus_imm, logic_imm, logics_imm
> arlo_reg adc, adcs, adr, alu_ext, alu_reg, alus_ext, alus_reg,
>  bfm, csel, logic_reg, logics_reg, rev.
> arlo_shift   alu_shift_imm, alus_shift_imm, logic_shift_imm,
>  logics_shift_imm.
> arlo_shift_reg   alu_shift_reg, alus_shift_reg, logic_shift_reg,
>  logics_shift_reg.
> clz  clz, rbit.
> shiftshift_imm (rename).
> 
> 
> The changes are propagated through the AARCH64 backend, and all the pipeline
> descriptions in the ARM backend.
> 
> OK for trunk?
> 
> Thanks
> Sofiane
> 
> -
> 
> ChangeLog:
> 
>   * config/arm/types.md (define_attr "type"): Expand "arlo_imm" into
>   "alu_imm", "alus_imm", "logic_imm", "logics_imm".  Expand "arlo_reg"
> into
>   "adc", "adcs", "adr", "alu_ext", "alu_reg", "alus_ext", "alus_reg",
>   "bfm", "csel", "logic_reg", "logics_reg", "rev".  Expand
> "arlo_shift" into
>   "alu_shift_imm", "alus_shift_imm", "logic_shift_imm",
> "logics_shift_imm".
>   Expand "arlo_shift_reg" into "alu_shift_reg", "alus_shift_reg",
>   "logic_shift_reg", "logics_shift_reg".  Expand "clz" into "clz,
> "rbit".
>   Rename "shift" to "shift_imm".
>   * config/arm/arm.md (define_attr "core_cycles"): Update for
> attribute
>   changes.
>   Update for attribute changes all occurrences of arlo_* and shift*
> types.
>   * config/arm/arm-fixed.md: Update for attribute changes all
> occurrences
>   of arlo_* types.
>   * config/arm/thumb2.md: Update for attribute changes all occurrences
>   of arlo_* types.
>   * config/arm/arm.c (xscale_sched_adjust_cost):  (rtx insn, rtx 
>   (cortexa7_older_only): Likewise.
>   (cortexa7_younger):  Likewise.
>   * config/arm/arm1020e.md (1020alu_op): Update for attribute changes.
>   (1020alu_shift_op): Likewise.
>   (1020alu_shift_reg_op): Likewise.
>   * config/arm/arm1026ejs.md (alu_op): Update for attribute changes.
>   (alu_shift_op): Likewise.
>   (alu_shift_reg_op): Likewise.
>   * config/arm/arm1136jfs.md (11_alu_op): Update for attribute
> changes.
>   (11_alu_shift_op): Likewise.
>   (11_alu_shift_reg_op): Likewise.
>   * config/arm/arm926ejs.md (9_alu_op): Update for attribute changes.
>   (9_alu_shift_reg_op): Likewise.
>   * config/arm/cortex-a15.md (cortex_a15_alu): Update for attribute
> changes.
>   (cortex_a15_alu_shift): Likewise.
>   (cortex_a15_alu_shift_reg): Likewise.
>   * config/arm/cortex-a5.md (cortex_a5_alu): Update for attribute
> changes.
>   (cortex_a5_alu_shift): Likewise.
>   * config/arm/cortex-a53.md (cortex_a53_alu): Update for attribute
> changes.
>   (cortex_a53_alu_shift): Likewise.
>   * config/arm/cortex-a7.md (cortex_a7_alu_imm): Update for attribute
>   changes.
>   (cortex_a7_alu_reg): Likewise.
>   (cortex_a7_alu_shift): Likewise.
>   * config/arm/cortex-a8.md (cortex_a8_alu): Update for attribute
> changes.
>   (cortex_a8_alu_shift): Likewise.
>   (cortex_a8_alu_shift_reg): Likewise.
>   * config/arm/cortex-a9.md (cortex_a9_dp): Update for attribute
> changes.
>   (cortex_a9_dp_shift): Likewise.
>   * config/arm/cortex-m4.md (cortex_m4_alu): Update for attribute
> changes.
>   * config/arm/cortex-r4.md (cortex_r4_alu): Update for attribute
> changes.
>   (cortex_r4_mov): Likewise.
>   (cortex_r4_alu_shift_reg): Likewise.
>   * config/arm/fa526.md (526_alu_op): Update for attribute changes.
>   (526_alu_shift_op): Likewise.
>   * config/arm/fa606te.md (606te_alu_op): Update for attribute
> changes.
>   * config/arm/fa626te.md (626te_alu_op): Update for attribute
> changes.
>   (626te_alu_shift_op): Likewise.
>   * config/arm/fa726te.md (726te_alu_op): Update for attribute
> changes.
>   (726te_alu_shift_op): Likewise.
>   (726te_alu_shift_reg_op): Likewise.
>   * config/arm/fmp626.md (mp626_alu_op): Update for attribute changes.
>   (mp626_alu_shift_op): Likewise.
>   * config/arm/marvell-pj4.md (pj4_alu): Update for attribute changes.
>   (pj4_alu_conds): Likewise.
>   (pj4_shift): Likewise.
>   (pj4_shift_conds): Likewise.
>   (pj4_alu_shift): Likewise.
>   (pj4_alu_shift_conds): Likewise.
>   * config/aarch64/aarch64.md: Update for attribute change all
> occurrences
>   of arlo_* and shift* types.

Re: [AARCH64][Insn classification unification 3/N] ALU/shift types

[Richard Earnshaw]

On 06/08/13 09:48, James Greenhalgh wrote:
> *Ping*
> 
> James
> 
> On Thu, Aug 01, 2013 at 02:50:07PM +0100, Sofiane Naci wrote:
>> Hi,
>>
>> This patch is part of the ongoing work to unify instruction classification
>> between the ARM and AARCH64 backends.
>>
>> This patch fine tunes the ALU/shift type attribute values in the ARM backend
>> as detailed in the table below:
>>
>> Old  New
>> --
>> arlo_imm alu_imm, alus_imm, logic_imm, logics_imm
>> arlo_reg adc, adcs, adr, alu_ext, alu_reg, alus_ext, alus_reg,
>>  bfm, csel, logic_reg, logics_reg, rev.

Why arlo_reg into just adc & adcs?  Why not adc_reg and adcs_reg (with
equivalent changes for arlo_imm?

Why is adr factored out of arlo_reg rather than arlo_imm?  adr is an add
immediate using the PC register as a base.

Finally, please, please, please(!) generate your .md file diffs with gnu
diff's '-F ^(define' option.  This makes it *much* easier to identify
which pattern is being modified.

R.

>> arlo_shift   alu_shift_imm, alus_shift_imm, logic_shift_imm,
>>  logics_shift_imm.
>> arlo_shift_reg   alu_shift_reg, alus_shift_reg, logic_shift_reg,
>>  logics_shift_reg.
>> clz  clz, rbit.
>> shiftshift_imm (rename).
>>
>>
>> The changes are propagated through the AARCH64 backend, and all the pipeline
>> descriptions in the ARM backend.
>>
>> OK for trunk?
>>
>> Thanks
>> Sofiane
>>
>> -
>>
>> ChangeLog:
>>
>>  * config/arm/types.md (define_attr "type"): Expand "arlo_imm" into
>>  "alu_imm", "alus_imm", "logic_imm", "logics_imm".  Expand "arlo_reg"
>> into
>>  "adc", "adcs", "adr", "alu_ext", "alu_reg", "alus_ext", "alus_reg",
>>  "bfm", "csel", "logic_reg", "logics_reg", "rev".  Expand
>> "arlo_shift" into
>>  "alu_shift_imm", "alus_shift_imm", "logic_shift_imm",
>> "logics_shift_imm".
>>  Expand "arlo_shift_reg" into "alu_shift_reg", "alus_shift_reg",
>>  "logic_shift_reg", "logics_shift_reg".  Expand "clz" into "clz,
>> "rbit".
>>  Rename "shift" to "shift_imm".
>>  * config/arm/arm.md (define_attr "core_cycles"): Update for
>> attribute
>>  changes.
>>  Update for attribute changes all occurrences of arlo_* and shift*
>> types.
>>  * config/arm/arm-fixed.md: Update for attribute changes all
>> occurrences
>>  of arlo_* types.
>>  * config/arm/thumb2.md: Update for attribute changes all occurrences
>>  of arlo_* types.
>>  * config/arm/arm.c (xscale_sched_adjust_cost):  (rtx insn, rtx 
>>  (cortexa7_older_only): Likewise.
>>  (cortexa7_younger):  Likewise.
>>  * config/arm/arm1020e.md (1020alu_op): Update for attribute changes.
>>  (1020alu_shift_op): Likewise.
>>  (1020alu_shift_reg_op): Likewise.
>>  * config/arm/arm1026ejs.md (alu_op): Update for attribute changes.
>>  (alu_shift_op): Likewise.
>>  (alu_shift_reg_op): Likewise.
>>  * config/arm/arm1136jfs.md (11_alu_op): Update for attribute
>> changes.
>>  (11_alu_shift_op): Likewise.
>>  (11_alu_shift_reg_op): Likewise.
>>  * config/arm/arm926ejs.md (9_alu_op): Update for attribute changes.
>>  (9_alu_shift_reg_op): Likewise.
>>  * config/arm/cortex-a15.md (cortex_a15_alu): Update for attribute
>> changes.
>>  (cortex_a15_alu_shift): Likewise.
>>  (cortex_a15_alu_shift_reg): Likewise.
>>  * config/arm/cortex-a5.md (cortex_a5_alu): Update for attribute
>> changes.
>>  (cortex_a5_alu_shift): Likewise.
>>  * config/arm/cortex-a53.md (cortex_a53_alu): Update for attribute
>> changes.
>>  (cortex_a53_alu_shift): Likewise.
>>  * config/arm/cortex-a7.md (cortex_a7_alu_imm): Update for attribute
>>  changes.
>>  (cortex_a7_alu_reg): Likewise.
>>  (cortex_a7_alu_shift): Likewise.
>>  * config/arm/cortex-a8.md (cortex_a8_alu): Update for attribute
>> changes.
>>  (cortex_a8_alu_shift): Likewise.
>>  (cortex_a8_alu_shift_reg): Likewise.
>>  * config/arm/cortex-a9.md (cortex_a9_dp): Update for attribute
>> changes.
>>  (cortex_a9_dp_shift): Likewise.
>>  * config/arm/cortex-m4.md (cortex_m4_alu): Update for attribute
>> changes.
>>  * config/arm/cortex-r4.md (cortex_r4_alu): Update for attribute
>> changes.
>>  (cortex_r4_mov): Likewise.
>>  (cortex_r4_alu_shift_reg): Likewise.
>>  * config/arm/fa526.md (526_alu_op): Update for attribute changes.
>>  (526_alu_shift_op): Likewise.
>>  * config/arm/fa606te.md (606te_alu_op): Update for attribute
>> changes.
>>  * config/arm/fa626te.md (626te_alu_op): Update for attribute
>> changes.
>>  (626te_alu_shift_op): Likewise.
>>  * config/arm/fa726te.md (726te_alu_op): Update for attribute
>> changes.
>>  (726te_alu_shift_op): Likewise.
>>  (726te_alu_shift_reg_op): Likewise.
>>  * config/arm/fmp626.md (mp626_alu_op): Update for attribute changes.
>>  (mp626_alu_sh

Re: [AARCH64][Insn classification unification 3/N] ALU/shift types

[Richard Earnshaw]

On 06/08/13 10:04, Richard Earnshaw wrote:
> On 06/08/13 09:48, James Greenhalgh wrote:
>> *Ping*
>>
>> James
>>
>> On Thu, Aug 01, 2013 at 02:50:07PM +0100, Sofiane Naci wrote:
>>> Hi,
>>>
>>> This patch is part of the ongoing work to unify instruction classification
>>> between the ARM and AARCH64 backends.
>>>
>>> This patch fine tunes the ALU/shift type attribute values in the ARM backend
>>> as detailed in the table below:
>>>
>>> Old  New
>>> --
>>> arlo_imm alu_imm, alus_imm, logic_imm, logics_imm
>>> arlo_reg adc, adcs, adr, alu_ext, alu_reg, alus_ext, alus_reg,
>>>  bfm, csel, logic_reg, logics_reg, rev.
> 
> Why arlo_reg into just adc & adcs?  Why not adc_reg and adcs_reg (with
> equivalent changes for arlo_imm?
> 
> Why is adr factored out of arlo_reg rather than arlo_imm?  adr is an add
> immediate using the PC register as a base.
> 
> Finally, please, please, please(!) generate your .md file diffs with gnu
> diff's '-F ^(define' option.  This makes it *much* easier to identify
> which pattern is being modified.
> 
> R.
> 

And another point.  Operations such as alu_ext and csel are
architecturally impossible on pre-v8 cores, so there's no need to
consider them in pre-armv8 pipeline descriptions.

R.
>>> arlo_shift   alu_shift_imm, alus_shift_imm, logic_shift_imm,
>>>  logics_shift_imm.
>>> arlo_shift_reg   alu_shift_reg, alus_shift_reg, logic_shift_reg,
>>>  logics_shift_reg.
>>> clz  clz, rbit.
>>> shiftshift_imm (rename).
>>>
>>>
>>> The changes are propagated through the AARCH64 backend, and all the pipeline
>>> descriptions in the ARM backend.
>>>
>>> OK for trunk?
>>>
>>> Thanks
>>> Sofiane
>>>
>>> -
>>>
>>> ChangeLog:
>>>
>>> * config/arm/types.md (define_attr "type"): Expand "arlo_imm" into
>>> "alu_imm", "alus_imm", "logic_imm", "logics_imm".  Expand "arlo_reg"
>>> into
>>> "adc", "adcs", "adr", "alu_ext", "alu_reg", "alus_ext", "alus_reg",
>>> "bfm", "csel", "logic_reg", "logics_reg", "rev".  Expand
>>> "arlo_shift" into
>>> "alu_shift_imm", "alus_shift_imm", "logic_shift_imm",
>>> "logics_shift_imm".
>>> Expand "arlo_shift_reg" into "alu_shift_reg", "alus_shift_reg",
>>> "logic_shift_reg", "logics_shift_reg".  Expand "clz" into "clz,
>>> "rbit".
>>> Rename "shift" to "shift_imm".
>>> * config/arm/arm.md (define_attr "core_cycles"): Update for
>>> attribute
>>> changes.
>>> Update for attribute changes all occurrences of arlo_* and shift*
>>> types.
>>> * config/arm/arm-fixed.md: Update for attribute changes all
>>> occurrences
>>> of arlo_* types.
>>> * config/arm/thumb2.md: Update for attribute changes all occurrences
>>> of arlo_* types.
>>> * config/arm/arm.c (xscale_sched_adjust_cost):  (rtx insn, rtx 
>>> (cortexa7_older_only): Likewise.
>>> (cortexa7_younger):  Likewise.
>>> * config/arm/arm1020e.md (1020alu_op): Update for attribute changes.
>>> (1020alu_shift_op): Likewise.
>>> (1020alu_shift_reg_op): Likewise.
>>> * config/arm/arm1026ejs.md (alu_op): Update for attribute changes.
>>> (alu_shift_op): Likewise.
>>> (alu_shift_reg_op): Likewise.
>>> * config/arm/arm1136jfs.md (11_alu_op): Update for attribute
>>> changes.
>>> (11_alu_shift_op): Likewise.
>>> (11_alu_shift_reg_op): Likewise.
>>> * config/arm/arm926ejs.md (9_alu_op): Update for attribute changes.
>>> (9_alu_shift_reg_op): Likewise.
>>> * config/arm/cortex-a15.md (cortex_a15_alu): Update for attribute
>>> changes.
>>> (cortex_a15_alu_shift): Likewise.
>>> (cortex_a15_alu_shift_reg): Likewise.
>>> * config/arm/cortex-a5.md (cortex_a5_alu): Update for attribute
>>> changes.
>>> (cortex_a5_alu_shift): Likewise.
>>> * config/arm/cortex-a53.md (cortex_a53_alu): Update for attribute
>>> changes.
>>> (cortex_a53_alu_shift): Likewise.
>>> * config/arm/cortex-a7.md (cortex_a7_alu_imm): Update for attribute
>>> changes.
>>> (cortex_a7_alu_reg): Likewise.
>>> (cortex_a7_alu_shift): Likewise.
>>> * config/arm/cortex-a8.md (cortex_a8_alu): Update for attribute
>>> changes.
>>> (cortex_a8_alu_shift): Likewise.
>>> (cortex_a8_alu_shift_reg): Likewise.
>>> * config/arm/cortex-a9.md (cortex_a9_dp): Update for attribute
>>> changes.
>>> (cortex_a9_dp_shift): Likewise.
>>> * config/arm/cortex-m4.md (cortex_m4_alu): Update for attribute
>>> changes.
>>> * config/arm/cortex-r4.md (cortex_r4_alu): Update for attribute
>>> changes.
>>> (cortex_r4_mov): Likewise.
>>> (cortex_r4_alu_shift_reg): Likewise.
>>> * config/arm/fa526.md (526_alu_op): Update for attribute changes.
>>> (526_alu_shift_op): Likewise.
>>> * config/arm/fa606te.md (606te_alu_op): Update for attribute
>>> changes.
>>> * config/arm/fa626te.md (626te_alu_op): Update for attribute
>>> changes

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

Hi,

On 08/06/2013 04:57 AM, Jason Merrill wrote:

On 08/05/2013 06:46 PM, Paolo Carlini wrote:

and after this comment, both pairs of qualify_lookup are called in that
order. Thus I started seriously suspecting that something may be wrong
in the if-else above, that is, that we really want something with
iter->type *before* iter->value there too: the attached patchlet p works
for the testcase and passes bootstrap & test. Does it make sense to you?


Yes.

Ok, good.



Final observation: in many cases, like for example, variants of the
testcase with one less data member, what happens is that iter->type and
iter->value are *both* the same variant of the TYPE_DECL Bar, the one
which is fine, has DECL_IMPLICIT_TYPEDEF_P set.


That's strange.  I would expect that to mean that we don't properly 
give an error for a Bar data member declared after the typedef.

You mean something like this?

class Foo
{
  int u, v, w;//, x;
  typedef struct Bar { } Bar;
  Bar bar;
  virtual void foo(void) {
struct Bar bar;
  }
};

after the patch we accept it, while we rejected it before. Current clang 
and icc also accept it. Note however, that in the above iter->type and 
iter->value are different, usual story. Same with w too commented out or 
neither. I haven't been able to construct a testcase following by and 
large your recipe where iter->type == iter->value. In short alternating 
data members and TYPE_DECLs also makes iter->type != iter->value, thus 
we want to check them in the proper order in such cases too.


Do you have more testcases in mind? (otherwise I would add the above too)

Thanks,
Paolo.

RE: [arm-embedded] Request to backport thumb1 far jump patch to embedded 4.8 branch

[Joey Ye]

OK for embedded 4.8 branch

- Joey

> -Original Message-
> From: Terry Guo
> Sent: Tuesday, August 06, 2013 14:17
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: [arm-embedded] Request to backport thumb1 far jump patch to
> embedded 4.8 branch
> 
> Hello Joey,
> 
> The thumb1 far jump patch is about an optimization to avoid unnecessary lr
> save instruction. It is now in trunk. Is it OK to back port it to embedded
> 4.8 branch?
> 
> BR,
> Terry
> 
> gcc/ChangeLog.arm
> 
>  2013-08-05  Terry Guo  
> 
>   Backport from mainline r197956
>   2013-04-15  Joey Ye  
> 
>   * config/arm/arm.c (thumb1_final_prescan_insn): Assert lr save
>   for real far jump.
>   (thumb_far_jump_used_p): Count instruction size and set
>   far_jump_used.
> 
> gcc/testsuite/ChangeLog.arm
> 
> 2013-08-05  Terry Guo  
> 
>   Backport from mainline r197956
>   2013-04-15  Joey Ye  
> 
>   * gcc.target/arm/thumb1-far-jump-1.c: New test.
>   * gcc.target/arm/thumb1-far-jump-2.c: New test.

Recent libstdc++ build failure in hashtable_policy.h on alpha, r201522

[Uros Bizjak]

Hello!

Recent build failure on alpha-linux-gnu (r201522)

gmake[6]: Entering directory
`/space/homedirs/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/src/c++11'
/bin/sh ../../libtool --tag CXX --tag disable-shared   --mode=compile
/home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc
-B/home/uros/gcc-build-xxx/./gcc -nostdinc++
-L/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/src
-L/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/src/.libs
-B/usr/local/alphaev68-unknown-linux-gnu/bin/
-B/usr/local/alphaev68-unknown-linux-gnu/lib/ -isystem
/usr/local/alphaev68-unknown-linux-gnu/include -isystem
/usr/local/alphaev68-unknown-linux-gnu/sys-include
-I/home/uros/gcc-svn/trunk/libstdc++-v3/../libgcc
-I/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/alphaev68-unknown-linux-gnu
-I/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include
-I/home/uros/gcc-svn/trunk/libstdc++-v3/libsupc++  -std=gnu++11
-prefer-pic -D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra
-Wwrite-strings -Wcast-qual -Wabi  -fdiagnostics-show-location=once
-ffunction-sections -fdata-sections  -frandom-seed=hashtable_c++0x.lo
-g -O2 -D_GNU_SOURCE -mieee  -fimplicit-templates -c
../../../../../gcc-svn/trunk/libstdc++-v3/src/c++11/hashtable_c++0x.cc
libtool: compile:  /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc
-B/home/uros/gcc-build-xxx/./gcc -nostdinc++
-L/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/src
-L/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/src/.libs
-B/usr/local/alphaev68-unknown-linux-gnu/bin/
-B/usr/local/alphaev68-unknown-linux-gnu/lib/ -isystem
/usr/local/alphaev68-unknown-linux-gnu/include -isystem
/usr/local/alphaev68-unknown-linux-gnu/sys-include
-I/home/uros/gcc-svn/trunk/libstdc++-v3/../libgcc
-I/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/alphaev68-unknown-linux-gnu
-I/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include
-I/home/uros/gcc-svn/trunk/libstdc++-v3/libsupc++ -std=gnu++11
-D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra
-Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once
-ffunction-sections -fdata-sections -frandom-seed=hashtable_c++0x.lo
-g -O2 -D_GNU_SOURCE -mieee -fimplicit-templates -c
../../../../../gcc-svn/trunk/libstdc++-v3/src/c++11/hashtable_c++0x.cc
 -fPIC -DPIC -D_GLIBCXX_SHARED -o hashtable_c++0x.o
In file included from
../../../../../gcc-svn/trunk/libstdc++-v3/src/c++11/hashtable_c++0x.cc:32:0:
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:700:11:
error: expected nested-name-specifier before ‘__alloctr_rebind’
  typename __alloctr_rebind<_Alloc, __node_type>::__type;
   ^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:701:42:
error: ‘__node_alloc_type’ was not declared in this scope
   using __node_gen_type = _AllocNode<__node_alloc_type>;
  ^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:701:59:
error: template argument 1 is invalid
   using __node_gen_type = _AllocNode<__node_alloc_type>;
   ^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:
In member function ‘std::__detail::_Insert_base<_Key, _Value, _Alloc,
_ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy,
_Traits>::__ireturn_type std::__detail::_Insert_base<_Key, _Value,
_Alloc, _ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy,
_Traits>::insert(const value_type&)’:
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:717:2:
error: ‘__node_gen_type’ was not declared in this scope
  __node_gen_type __node_gen(__h);
  ^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:718:28:
error: ‘__node_gen’ was not declared in this scope
  return __h._M_insert(__v, __node_gen, __unique_keys());
^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:
In member function ‘std::__detail::_Insert_base<_Key, _Value, _Alloc,
_ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy,
_Traits>::iterator std::__detail::_Insert_base<_Key, _Value, _Alloc,
_ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy,
_Traits>::insert(std::__detail::_Insert_base<_Key, _Value, _Alloc,
_ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy,
_Traits>::const_iterator, const value_type&)’:
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:725:2:
error: ‘__node_gen_type’ was not declared in this scope
  __node_gen_type __node_gen(__h);
  ^
/home/uros/gcc-build-xxx/alphaev68-unknown-linux-gnu/libstdc++-v3/include/bits/hashtable_policy.h:726:36:
error: ‘__node_

Re: Recent libstdc++ build failure in hashtable_policy.h on alpha, r201522

[Paolo Carlini]

On 08/06/2013 11:30 AM, Uros Bizjak wrote:

Hello!

Recent build failure on alpha-linux-gnu (r201522)
Uros, please revert it, thanks, we don't have the time to seriously look 
into it now.


Francois, please be more careful with testing, it's the second time in a 
few days.


Paolo.

Re: Recent libstdc++ build failure in hashtable_policy.h on alpha, r201522

[Uros Bizjak]

On Tue, Aug 6, 2013 at 11:33 AM, Paolo Carlini  wrote:

>> Recent build failure on alpha-linux-gnu (r201522)
>
> Uros, please revert it, thanks, we don't have the time to seriously look
> into it now.
>
> Francois, please be more careful with testing, it's the second time in a few
> days.

It looks I wasn't clear enough: I found the regression when I built
revision r201522. I didn't yet bisect the failure, but if the
offending commit can be found from the data I provided, it will save
me a couple of hours trying to bisect recent commits.

Thanks,
Uros.

Re: New parameters to control stringop expansion libcall strategy

[Jan Hubicka]

> >>> 2013-08-02  Xinliang David Li  
> >>>
> >>> * config/i386/stringop.def: New file.
> >>> * config/i386/stringop.opt: New file.
> >>> * config/i386/i386-opts.h: Include stringopt.def.
> >>> * config/i386/i386.opt: Include stringopt.opt.
> >>> * config/i386/i386.c (ix86_option_override_internal):
> >>> Override default size based stringop inline strategies
> >>> with options.
> >>> * config/i386/i386.c (ix86_parse_stringop_strategy_string):
> >>> New function.
> >>>
> >>> 2013-08-04  Xinliang David Li  
> >>>
> >>> * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test.
> >>> * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto.
> >>> * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto.
> >>> * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto.

The patch looks resonable to me in general.  I wonder why we need to bring
all the cost tables non-const instead of just having writable storage for
the "current strategy" like we do with other flags anyway.

Your strings are definitely more readable than the in-memory representation
I came up with. Perhaps we can even turn the cost tables into strings
for easier maintenance?  I guess they are bit confusing for people
not familiar with a code.

Honza
> >>>
> >>>
> >>>
> >>>
> >>> On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li  
> >>> wrote:
> >>> > On x86_64, when the expected size of memcpy/memset is known (e.g, with
> >>> > FDO), libcall strategy is used with the size is > 8192. This value is
> >>> > hard coded, which makes it hard to do performance tuning. This patch
> >>> > adds two new parameters to do that. Potential usage includes
> >>> > per-application libcall strategy min-size tuning based on summary data
> >>> > with FDO (e.g, instruction workset size).
> >>> >
> >>> > Bootstrap and tested on x86_64/linux. Ok for trunk?
> >>> >
> >>> > thanks,
> >>> >
> >>> > David
> >>> >
> >>> >
> >>> > 2013-08-02  Xinliang David Li  
> >>> >
> >>> > * params.def: New parameters.
> >>> > * config/i386/i386.c (ix86_option_override_internal):
> >>> > Override default libcall size limit with parameters.
> >>
> >>> Index: config/i386/stringop.def
> >>> ===
> >>> --- config/i386/stringop.def  (revision 0)
> >>> +++ config/i386/stringop.def  (revision 0)
> >>> @@ -0,0 +1,42 @@
> >>> +/* Definitions for option handling for IA-32.
> >>> +   Copyright (C) 2013 Free Software Foundation, Inc.
> >>> +
> >>> +This file is part of GCC.
> >>> +
> >>> +GCC is free software; you can redistribute it and/or modify
> >>> +it under the terms of the GNU General Public License as published by
> >>> +the Free Software Foundation; either version 3, or (at your option)
> >>> +any later version.
> >>> +
> >>> +GCC is distributed in the hope that it will be useful,
> >>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> +GNU General Public License for more details.
> >>> +
> >>> +Under Section 7 of GPL version 3, you are granted additional
> >>> +permissions described in the GCC Runtime Library Exception, version
> >>> +3.1, as published by the Free Software Foundation.
> >>> +
> >>> +You should have received a copy of the GNU General Public License and
> >>> +a copy of the GCC Runtime Library Exception along with this program;
> >>> +see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >>> +.  */
> >>> +
> >>> +DEF_ENUM
> >>> +DEF_ALG (no_stringop, no_stringop)
> >>> +DEF_ENUM
> >>> +DEF_ALG (libcall, libcall)
> >>> +DEF_ENUM
> >>> +DEF_ALG (rep_prefix_1_byte, rep_byte)
> >>> +DEF_ENUM
> >>> +DEF_ALG (rep_prefix_4_byte, rep_4byte)
> >>> +DEF_ENUM
> >>> +DEF_ALG (rep_prefix_8_byte, rep_8byte)
> >>> +DEF_ENUM
> >>> +DEF_ALG (loop_1_byte, byte_loop)
> >>> +DEF_ENUM
> >>> +DEF_ALG (loop, loop)
> >>> +DEF_ENUM
> >>> +DEF_ALG (unrolled_loop, unrolled_loop)
> >>> +DEF_ENUM
> >>> +DEF_ALG (vector_loop, vector_loop)
> >>> Index: config/i386/i386.opt
> >>> ===
> >>> --- config/i386/i386.opt  (revision 201458)
> >>> +++ config/i386/i386.opt  (working copy)
> >>> @@ -316,6 +316,14 @@ mstack-arg-probe
> >>>  Target Report Mask(STACK_PROBE) Save
> >>>  Enable stack probing
> >>>
> >>> +mmemcpy-strategy=
> >>> +Target RejectNegative Joined Var(ix86_tune_memcpy_strategy)
> >>> +Specify memcpy expansion strategy when expected size is known
> >>> +
> >>> +mmemset-strategy=
> >>> +Target RejectNegative Joined Var(ix86_tune_memset_strategy)
> >>> +Specify memset expansion strategy when expected size is known
> >>> +
> >>>  mstringop-strategy=
> >>>  Target RejectNegative Joined Enum(stringop_alg) Var(ix86_stringop_alg) 
> >>> Init(no_stringop)
> >>>  Chose strategy to generate stringop using
> >>> Index:

RE: [PATCH] Reassociate X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0

[Zhenqiang Chen]

> -Original Message-
> From: Andrew Pinski [mailto:pins...@gmail.com]
> Sent: Monday, August 05, 2013 4:40 PM
> To: Zhenqiang Chen
> Cc: GCC Patches
> Subject: Re: [PATCH] Reassociate X == CST1 || X == CST2 if popcount (CST2 -
> CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0
> 
> On Mon, Aug 5, 2013 at 1:08 AM, Zhenqiang Chen
>  wrote:
> > Hi
> >
> > The patch reassociates X == CST1 || X == CST2 if popcount (CST2 -
> > CST1) == 1 into ((X - CST1) & ~(CST2 - CST1)) == 0.
> >
> > Bootstrap on x86-64 and ARM chromebook.
> > No make check regression on x86-64 and panda board.
> >
> > For some targets/options, the "(X == CST1 || X == CST2)" might be
> > converted to "if (x == CST1) else if (x == CST2)" at the beginning. In
> > such case, the patch will not be executed. It is hard to write a
> > reliable testcase to detect the transformation. So the testcase does
> > not check the dump logs from
> > reassoc1 pass. It just checks the runtime result.
> >
> > Is it OK for trunk?
> Seems like a better place to put this is inside tree-ssa-ifcombine.c which

The patch is similar with the  code for 
 Optimize X == CST1 || X == CST2
 if popcount (CST1 ^ CST2) == 1 into
 (X & ~(CST1 ^ CST2)) == (CST1 & ~(CST1 ^ CST2))

Your "RFC: Gimple combine/folding interface" is a good design.
But before it, I think reusing code in tree-ssa-reassoc.c is the most easy way 
to implement it.

> handles the case where if(a || b) is split up into if(a) else if(b).
> Moving it into tree-ssa-ifcombine.c allows for code to be optimized which
> was written using the if(a) else if(b) form.

Yes. There might have opportunities to optimized "if(a) else if(b) form" for 
targets which LOGICAL_OP_NON_SHORT_CIRCUIT is FALSE.

If LOGICAL_OP_NON_SHORT_CIRCUIT is TRUE, "a || b" is converted to "a | b" in 
fold-const.c if they are simple operands.

Both reassoc and ifcombine optimize short circuiting. But they handle different 
scenarios. So I think we need both.

As you had mentioned, we need  enhance ifcombine to optimize "if(a) else if(b) 
form" using the similar methods from reassoc pass.
 
> Then it would easier to detect this for all targets and easier to remove from
> fold-const.c the removal of the short circuiting.

A unified interface as your RFC will be helpful for fold-const.c, 
tree-ssa-ifcombine.c and tree-ssa-reassoc.c.

If we can detect the optimization opportunity in fold_truth_andor, we do not 
need to split if(a || b) into if(a) else if(b) at all.

Thanks!
-Zhenqiang

> > ChangeLog
> > 2013-08-05  Zhenqiang Chen  
> >
> > * tree-ssa-reassoc.c (optimize_range_tests): Reasociate
> > X == CST1 || X == CST2 if popcount (CST2 - CST1) == 1 into
> > ((X - CST1) & ~(CST2 - CST1)) == 0.
> >
> > testsuite/ChangeLog
> > 2013-08-05  Zhenqiang Chen  
> >
> > * gcc.dg/reassoc1.c: New testcase.

Re: Recent libstdc++ build failure in hashtable_policy.h on alpha, r201522

[Paolo Carlini]

On 08/06/2013 11:40 AM, Uros Bizjak wrote:

On Tue, Aug 6, 2013 at 11:33 AM, Paolo Carlini  wrote:


Recent build failure on alpha-linux-gnu (r201522)

Uros, please revert it, thanks, we don't have the time to seriously look
into it now.

Francois, please be more careful with testing, it's the second time in a few
days.

It looks I wasn't clear enough: I found the regression when I built
revision r201522.
Well r201522 corresponds to a serious change in exactly that code. I 
thought it was clear. And of course we all bootstrapped on x86_64-linux 
the recent revisions, immediately before that one.


I recommend reverting.

Thanks,
Paolo.

Re: [patch, fortran] RFD: PR 56666 Allow suppression of zero-trip DO loop warning

[Janus Weil]

Hi Thomas,

> the rather self-explanatory patch implements the -Wzerotrip
> option.  The positive form is not really useful, because
> the option is on by default (so the default behavior is
> not changed).
>
> The negative form of the option, -Wno-zerotrip, suppresses the
> warning.  I have also added information of how to suppress the
> warning in the message.
>
> Alternatively, it is also possible to only activate the warning
> if it is set explicitly, or with -Wall.  I can easily change the
> patch to do so, if that turns out to be the consensus (I have no
> strong opinion on the matter either way)
>
> Regression-tested. OK for trunk?

I'm never quite sure whether I like the ongoing inflation of warning
options, but this particular flag is one that I might find useful
myself on occasion. So: Ok for trunk from my side.

However, I would prefer to disable the warning by default, but include
it in -Wall.

Thanks for the patch ...

Cheers,
Janus



> 2013-08-03  Thomas Koenig  
>
> PR fortran/5
> * gfortran.h (gfc_option_t):  Add warn_zerotrip.
> * invoke.texi (-Wzerotrip):  Document option.
> * lang.opt (Wzerotrip):  Add.
> * options.c (gfc_init_options):  Initialize warn_zerotrip.
> (gfc_handle_option):  Handle OPT_Wzerotrip.
> * resolve.c (gfc_resolve_iterator): Honor
> gfc_option.warn_zerotrip; update error message to show
> how to suppress the warning.
>
> 2013-08-03  Thomas Koenig  
>
> PR fortran/5
> * gfortran.dg/do_check_10.f90:  New test.

Re: [PATCH 05/11] Add -fno-rtti when building plugins.

[Dominique Dhumieres]

> Hence plugins that create passes will need to be built with RTTI
> disabled in order to link against gcc, or they will fail to load, with
> an error like: ...

The same holds for darwin, hence the following patch is needed:

--- ../_clean/gcc/testsuite/lib/plugin-support.exp  2013-08-05 
22:51:42.0 +0200
+++ gcc/testsuite/lib/plugin-support.exp2013-08-06 12:36:43.0 
+0200
@@ -101,7 +101,7 @@ proc plugin-test-execute { plugin_src pl
set optstr [concat $optstr " $op"]
}
}
-   set optstr [concat $optstr "-DIN_GCC -fPIC -shared -undefined 
dynamic_lookup"]
+   set optstr [concat $optstr "-DIN_GCC -fPIC -shared -fno-rtti -undefined 
dynamic_lookup"]
 } else {
set plug_cflags $PLUGINCFLAGS 
set optstr "$includes $extra_flags -DIN_GCC -fPIC -shared -fno-rtti"

TIA

Dominique

Re: [wwwdocs] Buildstat update for 4.8

[Gerald Pfeifer]

On Sun, 4 Aug 2013, Tom G. Christensen wrote:
> Latest results for gcc 4.8.x.
> 
> -tgc
> 
> Testresults for 4.8.1
>   arm-unknown-linux-gnueabi
>   hppa2.0w-hp-hpux11.11
>   hppa64-hp-hpux11.11
>   i386-pc-solaris2.9 (2)
>   i686-pc-linux-gnu
>   mips-unknown-linux-gnu
>   mipsel-unknown-linux-gnu
>   powerpc-apple-darwin8.11.0
>   powerpc-unknown-linux-gnu
>   powerpc64-unknown-linux-gnu
>   sparc-sun-solaris2.9
>   sparc-unknown-linux-gnu
>   x86_64-apple-darwin13.0.0
> 
> 
> Testresults for 4.8.0
>   hppa1.1-hp-hpux10.20
>   i386-pc-solaris2.9

Quite some!  Thanks for the update, Tom!

Gerald

[buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org  
wrote:
> New Revision: 201508
> 
> URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> Log:
> Automated conversion of passes to C++ classes
> 
> gcc/
> 
>   Patch autogenerated by refactor_passes.py from
>   https://github.com/davidmalcolm/gcc-refactoring-scripts
>   revision 03fe39476a4c4ea450b49e087cfa817b5f92021e

I see quite some fall-out from this on epiphany-elf:

g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H 
-DGENERATOR_FILE -I. -Ibuild -I../../../../gcc/gcc -I../../../../gcc/gcc/build 
-I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
-I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\
-o build/genflags.o ../../../../gcc/gcc/genflags.c
In file included from ./tm.h:21:0,
 from ../../../../gcc/gcc/genflags.c:26:
../../../../gcc/gcc/config/epiphany/epiphany.h:932:8: error: ‘rtl_opt_pass’ 
does not name a type
 extern rtl_opt_pass *make_pass_mode_switch_use (gcc::context *ctxt);
^
../../../../gcc/gcc/config/epiphany/epiphany.h:933:8: error: ‘rtl_opt_pass’ 
does not name a type
 extern rtl_opt_pass *make_pass_resolve_sw_modes (gcc::context *ctxt);
^

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
  Signature of:  Zensur im Internet? Nein danke!
  the second  :


signature.asc
Description: Digital signature

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Tue, 2013-08-06 13:12:57 +0200, Jan-Benedict Glaw  wrote:
> On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
>  wrote:
> > New Revision: 201508
> > 
> > URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> > Log:
> > Automated conversion of passes to C++ classes
> > 
> > gcc/
> > 
> > Patch autogenerated by refactor_passes.py from
> > https://github.com/davidmalcolm/gcc-refactoring-scripts
> > revision 03fe39476a4c4ea450b49e087cfa817b5f92021e

And probably also for rl78-elf:

g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. 
-I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
-I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
-I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\
../../../../gcc/gcc/config/rl78/rl78.c -o rl78.o
../../../../gcc/gcc/config/rl78/rl78.c:146:1: error: in C++98 
‘rl78_devirt_pass’ must be initialized by constructor, not by ‘{...}’
 };
 ^
../../../../gcc/gcc/config/rl78/rl78.c:146:1: error: could not convert 
‘{RTL_PASS, "devirt", 0, devirt_gate, devirt_pass, 0, 0, 212, TV_MACH_DEP, 0, 
0, 0, 0, 0}’ from ‘’ to ‘opt_pass’

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
  Signature of:  Zensur im Internet? Nein danke!
  the second  :


signature.asc
Description: Digital signature

[buildbot] r201513: Invalid cast

[Jan-Benedict Glaw]

Hi!

On Mon, 2013-08-05 22:09:45 -, olege...@gcc.gnu.org  
wrote:
> Author: olegendo
> Date: Mon Aug  5 22:09:45 2013
> New Revision: 201513
> 
> URL: http://gcc.gnu.org/viewcvs?rev=201513&root=gcc&view=rev
> Log:
>   PR other/12081
>   * recog.h (rtx (*insn_gen_fn) (rtx, ...)): Replace typedef with new
>   class insn_gen_fn.
>   * expr.c (move_by_pieces_1, store_by_pieces_2): Replace argument
>   rtx (*) (rtx, ...) with insn_gen_fn.
>   * genoutput.c (output_insn_data): Cast gen_? function pointers to
>   insn_gen_fn::stored_funcptr.  Add initializer braces.

This probably caused in a powerpc64-linux build:

g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. 
-I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
-I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
-I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\
../../../../gcc/gcc/config/rs6000/rs6000.c -o rs6000.o
In file included from ../../../../gcc/gcc/config/rs6000/rs6000.c:36:0:
../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void 
rs6000_emit_swdiv(rtx, rtx, rtx, bool)’:
../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const 
insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, 
rtx_def*)}’
 #define GEN_FCN(CODE) (insn_data[CODE].genfun)
  ^
../../../../gcc/gcc/config/rs6000/rs6000.c:28145:43: note: in expansion of 
macro ‘GEN_FCN’
   gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
   ^
../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void 
rs6000_emit_swrsqrt(rtx, rtx)’:
../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const 
insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, 
rtx_def*)}’
 #define GEN_FCN(CODE) (insn_data[CODE].genfun)
  ^
../../../../gcc/gcc/config/rs6000/rs6000.c:28223:43: note: in expansion of 
macro ‘GEN_FCN’
   gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
   ^

Other PPC/RS6k targets (ie. rs6000-ibm-aix4.3, ...) seem to be equally
affected.

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
Signature of:http://www.chiark.greenend.org.uk/~sgtatham/bugs.html
the second  :


signature.asc
Description: Digital signature

Re: [testsuite, Android] Add to pr56407.c

[Alexander Ivchenko]

The reason for undefined reference to rand is that it is defined as
"static __inline__" in Bionic stdlib.h:

static __inline__ int rand(void) {
return (int)lrand48();
}

So in fact, if you do "nm libc.so" for Bionic, you won't get the
rand.. which is probably not correct, because it doesn't have the
external linkage. I added Elliott.

Still, in C standart it is said that rand is defined in stdlib.h, and
we don't include it in that testcase.


thanks,
Alexander


2013/8/5 Maxim Kuvyrkov :
> On 5/08/2013, at 10:57 PM, Alexander Ivchenko wrote:
>
>> Hi,
>>
>> The following test case fails to compile on Android: gcc.dg/torture/pr56407.c
>>
>> /tmp/ccA08Isw.o:pr56407.c:function test: error: undefined reference to 'rand'
>> collect2: error: ld returned 1 exit status
>>
>> Which is not surprising at all, since the testcase has only the
>> declarations of abort() and rand()
>> and doesn't have any headers included.
>
> It *is* surprising given that the testcase does have declarations of abort() 
> or rand() -- which are the only two external functions referenced.
>
>>
>> The following patch adds  to the test.
>
> I don't think this a correct fix.  [In most such cases the real problem can 
> be found out by examining why linking a test against glibc works, but against 
> bionic -- doesn't.]
>
> How are you linking this testcase?  Please post both the link line from 
> testsuite log and verbose output of linking (obtainable by adding "-v" to the 
> link line).
>
> Thank you,
>
> --
> Maxim Kuvyrkov
> www.kugelworks.com
>
>

Re: [testsuite, Android] Add to pr56407.c

[Andreas Schwab]

Alexander Ivchenko  writes:

> Still, in C standart it is said that rand is defined in stdlib.h, and
> we don't include it in that testcase.

It also says: "Provided that a library function can be declared without
reference to any type defined in a header, it is also permissible to
declare the function and use it without including its associated
header."

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Tue, 2013-08-06 13:18:16 +0200, Jan-Benedict Glaw  wrote:
> On Tue, 2013-08-06 13:12:57 +0200, Jan-Benedict Glaw  
> wrote:
> > On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
> >  wrote:
> > > New Revision: 201508
> > > 
> > > URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> > > Log:
> > > Automated conversion of passes to C++ classes
> > > 
> > > gcc/
> > > 
> > >   Patch autogenerated by refactor_passes.py from
> > >   https://github.com/davidmalcolm/gcc-refactoring-scripts
> > >   revision 03fe39476a4c4ea450b49e087cfa817b5f92021e

And probably also for sparc{,64}-linux:

g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. 
-I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
-I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
-I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd 
-I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\
../../../../gcc/gcc/config/sparc/sparc.c -o sparc.o
../../../../gcc/gcc/config/sparc/sparc.c:1043:27: error: expected 
primary-expression before ‘.’ token
   &pass_work_around_errata.pass, /* pass */
   ^

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
 Signature of:Arroganz verkürzt fruchtlose Gespräche.
 the second  :   -- Jan-Benedict Glaw


signature.asc
Description: Digital signature

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Tue, 2013-08-06 14:10:11 +0200, Jan-Benedict Glaw  wrote:
> And probably also for sparc{,64}-linux:
> 
> g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> -DHAVE_CONFIG_H -I. -I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
> -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
> -I../../../../gcc/gcc/../libdecnumber 
> -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> -I../../../../gcc/gcc/../libbacktrace\
> ../../../../gcc/gcc/config/sparc/sparc.c -o sparc.o
> ../../../../gcc/gcc/config/sparc/sparc.c:1043:27: error: expected 
> primary-expression before ‘.’ token
>&pass_work_around_errata.pass, /* pass */
>^

This was wrong, it's probably caused by r201511: "Rewrite how
instances of passes are cloned".

MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
Signature of: 23:53 <@jbglaw> So, ich kletter' jetzt mal ins Bett.
the second  : 23:57 <@jever2> .oO( kletter ..., hat er noch Gitter vorm Bett, 
wie früher meine Kinder?)
  00:00 <@jbglaw> jever2: *patsch*
  00:01 <@jever2> *aua*, wofür, Gedanken sind frei!
  00:02 <@jbglaw> Nee, freie Gedanken, die sind seit 1984 doch aus!
  00:03 <@jever2> 1984? ich bin erst seit 1985 verheiratet!


signature.asc
Description: Digital signature

Re: [PATCH] Convert more passes to new dump framework

[Martin Jambor]

Hi,

On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
> This patch ports messages to the new dump framework,

It would be great this new framework was documented somewhere.  I lost
track of what was agreed it would be and from the uses in the
vectorizer I was never quite sure how to utilize it in other passes.

I'd also like to point out two other minor things inline:

[...]

> 2013-08-06  Teresa Johnson  
> Dehao Chen  
> 
> * dumpfile.c (dump_loc): Add column number to output, make newlines
> consistent.
> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
> * ipa-inline-transform.c (clone_inlined_nodes):
> (cgraph_node_opt_info): New function.
> (cgraph_node_call_chain): Ditto.
> (dump_inline_decision): Ditto.
> (inline_call): Invoke dump_inline_decision.
> * doc/invoke.texi: Document optall -fopt-info flag.
> * profile.c (read_profile_edge_counts): Use new dump framework.
> (compute_branch_probabilities): Ditto.
> * passes.c (pass_manager::register_one_dump_file): Use OPTGROUP_OTHER
> when pass not in any opt group.
> * value-prof.c (check_counter): Use new dump framework.
> (find_func_by_funcdef_no): Ditto.
> (check_ic_target): Ditto.
> * coverage.c (get_coverage_counts): Ditto.
> (coverage_init): Setup new dump framework.
> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
> * ipa-inline.h (is_in_ipa_inline): Declare.
> 
> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
> * testsuite/gcc.dg/pr26570.c: Ditto.
> * testsuite/gcc.dg/pr32773.c: Ditto.
> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
> 

[...]

> Index: ipa-inline-transform.c
> ===
> --- ipa-inline-transform.c  (revision 201461)
> +++ ipa-inline-transform.c  (working copy)
> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>  }
> 
> 
> +#define MAX_INT_LENGTH 20
> +
> +/* Return NODE's name and profile count, if available.  */
> +
> +static const char *
> +cgraph_node_opt_info (struct cgraph_node *node)
> +{
> +  char *buf;
> +  size_t buf_size;
> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
> +
> +  if (!bfd_name)
> +bfd_name = "unknown";
> +
> +  buf_size = strlen (bfd_name) + 1;
> +  if (profile_info)
> +buf_size += (MAX_INT_LENGTH + 3);
> +
> +  buf = (char *) xmalloc (buf_size);
> +
> +  strcpy (buf, bfd_name);
> +
> +  if (profile_info)
> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
> +  return buf;
> +}

I'm not sure if output of this function is aimed only at the user or
if it is supposed to be used by gcc developers as well.  If the
latter, an incredibly useful thing is to also dump node->symbol.order
too.  We usually dump it after "/" sign separating it from node name.
It is invaluable when examining decisions in C++ code where you can
have lots of clones of a node (and also because existing dumps print
it, it is easy to combine them).

[...]

> Index: ipa-inline.c
> ===
> --- ipa-inline.c(revision 201461)
> +++ ipa-inline.c(working copy)
> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3.  If not see
>  static int overall_size;
>  static gcov_type max_count;
> 
> +/* Global variable to denote if it is in ipa-inline pass. */
> +bool is_in_ipa_inline = false;
> +
>  /* Return false when inlining edge E would lead to violating
> limits on function unit growth or stack usage growth.
> 

In this age of removing global variables, are you sure you need this?
The only user of this seems to be a function that is only being called
from inline_call... can that ever happen when not inlining?  If you
plan to use this function also elsewhere, perhaps the callers will
know whether we are inlining or not and can provide this in a
parameter?

Thanks,

Martin

Re: [testsuite, Android] Add to pr56407.c

[Alexander Ivchenko]

Thanks Andreas, it seems clear now that Bionic rand function is not
consistent with the standart.


--Alexander

2013/8/6 Andreas Schwab :
> Alexander Ivchenko  writes:
>
>> Still, in C standart it is said that rand is defined in stdlib.h, and
>> we don't include it in that testcase.
>
> It also says: "Provided that a library function can be declared without
> reference to any type defined in a header, it is also permissible to
> declare the function and use it without including its associated
> header."
>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Move function body streaming to passmanager/cgraph control

[Jan Hubicka]

Hi,
this patch breaks out lto.c' code to read given function body into
cgraph_get_body.  Instead of reading all bodies at once, we now read only
bodies that are needed on demand.  This is how I planned to get whole program
compilation working back in 2004, so code is still mostly ready for it. :)
Since we throw away bodies we no longer need during the compilation, this
reduces overall memory use of ltrans stages (and -flto-partition=none).
I tested in on Firefox. -flto-partition=none still gets over 16GB of memory use
but it gets quite a lot of work done before crashing, while w/o patch we crash
right away.  I suppose I need to check what parts of function bodies are getting
stale during compilation (for sure line locators, but probably more).
Slimmer ltrans stages helps to reduce overall footprint in parallel compilation.

Another motivation of this patch is to make IPA pass development easier.
Martin Liska's code unification pass can now read function bodies into WPA
stage when to compare bodies when hash claims they are equivalent.  This is a
lot easier than getting them compared only later at ltrans. Depending on
perofrmance implication of this we can either stay with this or go with not
reading them.

Reading of bodies is mostly done by passmanager just before it executes
local pass on it.  Callgraph materialization and inlining needs bodies of
other functions and therefore it needs its own cgraph_get_body call.
Once dwarf2out is less broken with -g I think it should also gets bodies
on its own to produce abstract origin representations (not includes in this
patch since push_cfun ICEs, I have followup for this. To see some benefits
we however need to stop clearning the origins).

Ipa-pta is only late IPA pass and as such it also needs changes.  These changes
I think should go away next.  I do not see how ipa-pta can reliably work in
presence of clones.  I have separate patch that adds fixup_cfg pass prior
ipa-pta that makes us to apply all IPA transforms.  (of course kiling the
memory benefits of this patch mostly) Currently I think it should be easy to
reproduce a bug where ipa-cp injects some aggregate constants taking address of
an object that ipa-pta don't see.

Bootstrapped/regtested x86_64-linux, will commit it after bit more testing.

Honza

* cgraph.c (cgraph_get_body): New function based on lto.c
implementation.
* cgraph.h (cgraph_get_body): Declare.
* cgraphclones.c (cgraph_create_virtual_clone): Commonize WPA and LTO 
paths.
* cgraphunit.c (expand_function): Get body prior expanding.
* ipa.c (function_and_variable_visibility): Use gimple_has_body_p test.
* lto-cgraph.c (lto_output_node): Do not stream bodies we don't really 
need.
* passes.c (do_per_function_toporder): Get body.
* tree-inline.c (expand_call_inline): Get body prior inlining it.
* tree-ssa-structalias.c (ipa_pta_execute): Get body; skip clones.

* lto.c (lto_materialize_function): Do not read body anymore.
Index: cgraph.c
===
--- cgraph.c(revision 201498)
+++ cgraph.c(working copy)
@@ -2707,4 +2707,44 @@ cgraph_function_node (struct cgraph_node
   return node;
 }
 
+/* When doing LTO, read NODE's body from disk if it is not already present.  */
+
+bool
+cgraph_get_body (struct cgraph_node *node)
+{
+  struct lto_file_decl_data *file_data;
+  const char *data, *name;
+  size_t len;
+  tree decl = node->symbol.decl;
+
+  if (DECL_RESULT (decl))
+return false;
+
+  gcc_assert (in_lto_p);
+
+  file_data = node->symbol.lto_file_data;
+  name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
+
+  /* We may have renamed the declaration, e.g., a static function.  */
+  name = lto_get_decl_name_mapping (file_data, name);
+
+  data = lto_get_section_data (file_data, LTO_section_function_body,
+  name, &len);
+  if (!data)
+{
+   dump_cgraph_node (stderr, node);
+fatal_error ("%s: section %s is missing",
+file_data->file_name,
+name);
+}
+
+  gcc_assert (DECL_STRUCT_FUNCTION (decl) == NULL);
+
+  lto_input_function_body (file_data, decl, data);
+  lto_stats.num_function_bodies++;
+  lto_free_section_data (file_data, LTO_section_function_body, name,
+data, len);
+  return true;
+}
+
 #include "gt-cgraph.h"
Index: cgraph.h
===
--- cgraph.h(revision 201498)
+++ cgraph.h(working copy)
@@ -701,6 +701,7 @@ gimple cgraph_redirect_edge_call_stmt_to
 bool cgraph_propagate_frequency (struct cgraph_node *node);
 struct cgraph_node * cgraph_function_node (struct cgraph_node *,
   enum availability *avail = NULL);
+bool cgraph_get_body (struct cgraph_node *node);
 
 /* In cgraphunit.c  */
 struct asm_node *add_asm_node (tree);
Index: cgraphclones.c
===

Re: [PATCH] Convert more passes to new dump framework

[Teresa Johnson]

On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
> Hi,
>
> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> This patch ports messages to the new dump framework,
>
> It would be great this new framework was documented somewhere.  I lost
> track of what was agreed it would be and from the uses in the
> vectorizer I was never quite sure how to utilize it in other passes.

Cc'ing Sharad who implemented this - Sharad, is this documented on a
wiki or elsewhere?

>
> I'd also like to point out two other minor things inline:
>
> [...]
>
>> 2013-08-06  Teresa Johnson  
>> Dehao Chen  
>>
>> * dumpfile.c (dump_loc): Add column number to output, make newlines
>> consistent.
>> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>> * ipa-inline-transform.c (clone_inlined_nodes):
>> (cgraph_node_opt_info): New function.
>> (cgraph_node_call_chain): Ditto.
>> (dump_inline_decision): Ditto.
>> (inline_call): Invoke dump_inline_decision.
>> * doc/invoke.texi: Document optall -fopt-info flag.
>> * profile.c (read_profile_edge_counts): Use new dump framework.
>> (compute_branch_probabilities): Ditto.
>> * passes.c (pass_manager::register_one_dump_file): Use OPTGROUP_OTHER
>> when pass not in any opt group.
>> * value-prof.c (check_counter): Use new dump framework.
>> (find_func_by_funcdef_no): Ditto.
>> (check_ic_target): Ditto.
>> * coverage.c (get_coverage_counts): Ditto.
>> (coverage_init): Setup new dump framework.
>> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>> * ipa-inline.h (is_in_ipa_inline): Declare.
>>
>> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>> * testsuite/gcc.dg/pr26570.c: Ditto.
>> * testsuite/gcc.dg/pr32773.c: Ditto.
>> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>
>
> [...]
>
>> Index: ipa-inline-transform.c
>> ===
>> --- ipa-inline-transform.c  (revision 201461)
>> +++ ipa-inline-transform.c  (working copy)
>> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>>  }
>>
>>
>> +#define MAX_INT_LENGTH 20
>> +
>> +/* Return NODE's name and profile count, if available.  */
>> +
>> +static const char *
>> +cgraph_node_opt_info (struct cgraph_node *node)
>> +{
>> +  char *buf;
>> +  size_t buf_size;
>> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>> +
>> +  if (!bfd_name)
>> +bfd_name = "unknown";
>> +
>> +  buf_size = strlen (bfd_name) + 1;
>> +  if (profile_info)
>> +buf_size += (MAX_INT_LENGTH + 3);
>> +
>> +  buf = (char *) xmalloc (buf_size);
>> +
>> +  strcpy (buf, bfd_name);
>> +
>> +  if (profile_info)
>> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>> +  return buf;
>> +}
>
> I'm not sure if output of this function is aimed only at the user or
> if it is supposed to be used by gcc developers as well.  If the
> latter, an incredibly useful thing is to also dump node->symbol.order
> too.  We usually dump it after "/" sign separating it from node name.
> It is invaluable when examining decisions in C++ code where you can
> have lots of clones of a node (and also because existing dumps print
> it, it is easy to combine them).

The output is useful for both power users doing performance tuning of
their application, and by gcc developers. Adding the id is not so
useful for the former, but I agree that it is very useful for compiler
developers. In fact, in the google branch version we emit more verbose
information (the lipo module id and the funcdef_no) to help uniquely
identify the routines and to aid in post-processing by humans and
tools. So it is probably useful to add something similar here too. Is
the node->symbol.order more or less unique than the funcdef_no? I see
that you added a patch a few months ago to print the
node->symbol.order in the function header, and it also has the
advantage as you note of matching up with existing ipa dumps.

>
> [...]
>
>> Index: ipa-inline.c
>> ===
>> --- ipa-inline.c(revision 201461)
>> +++ ipa-inline.c(working copy)
>> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3.  If not see
>>  static int overall_size;
>>  static gcov_type max_count;
>>
>> +/* Global variable to denote if it is in ipa-inline pass. */
>> +bool is_in_ipa_inline = false;
>> +
>>  /* Return false when inlining edge E would lead to violating
>> limits on function unit growth or stack usage growth.
>>
>
> In this age of removing global variables, are you sure you need this?
> The only user of this seems to be a function that is only being called
> from inline_call... can that ever happen when not inlining?  If you
> plan to use this function also elsewhere, perhaps the caller

[x86, PATCH] More effecient code for short unsigned conversion to float-point.

[Yuri Rumyantsev]

Hi All,

Here is simple fix which produces more optimal code unsigned
char(short) to float(double) with direct rtl-generation.

Bootstrapping and regression testing were successful for x86-64.

Is it OK for trunk?

ChangeLog:

2013-08-06  Yuri Rumyantsev  

* config/i386/i386.md (floatunssi2 expand): Add support
for QI/HImode operand to produce more effecient code for
unsigned char(short) --> float(double) conversion.


i386-short-uns-conversion-to-fp.patch
Description: Binary data

Re: Request to merge Undefined Behavior Sanitizer in (take 2)

[Marek Polacek]

On Wed, Jul 31, 2013 at 02:52:39PM -0400, Jason Merrill wrote:
> >http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01536.html
> 
> Here, the C++ compiler is wrong to fold away the division by zero,
> but given that bug the folding ought to also eliminate the call to
> the sanitize function.  Seems like you should attach the call to the
> questionable expression itself.

Hm, actually, we can't easily fold the call to the sanitize function
away, I'm afraid, if we want to do it for the 'case '
case.  When we hit the DIV_EXPR in 'case 0 * (1 / 0)',
the ubsan_instrument_division gets 1 as a first argument and 0 as
a second argument, but due to fold_builds in the
ubsan_instrument_division, we replace the case value with just the call
to the __builtin___ubsan_handle_divrem_overflow.  I think, what we
could do, is to tweak verify_constant like this:

--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -6938,6 +6938,14 @@ static bool
 verify_constant (tree t, bool allow_non_constant, bool *non_constant_p,
 bool *overflow_p)
 {
+  /* This is to handle e.g. the goofy 'case 0 * (1 / 0)' case.  */
+  if (flag_sanitize & SANITIZE_UNDEFINED
+  && TREE_CODE (t) == CALL_EXPR
+  && is_ubsan_builtin (t))
+{
+  error ("undefined behavior occured");
+  return *non_constant_p;
+}
   if (!*non_constant_p && !reduced_constant_expression_p (t))
 {
   if (!allow_non_constant)

The logic behind this is that when we can't easily insert the
ubsan builtin call itself (so that it would get called unconditionally
at the runtime), at least error out; at that point the behavior
is certainly undefined thus we're free to do whatever.  I'd say it's better
than to e.g. insert 0 instead of the expression and carry on.
Also, this could be useful in other situations (at places, where
we can't insert the ubsan builtin call, but we *know* that the 
behavior is undefined).  Yes, the behavior
can be undefined only at runtime, but, this would happen only when
sanitizing and in that case is desirable to report even that.
The is_ubsan_builtin fn is not yet implemented, but you get the idea.

Thoughts?  Thanks,

Marek

Re: [x86, PATCH] More effecient code for short unsigned conversion to float-point.

[Andreas Schwab]

Yuri Rumyantsev  writes:

> * config/i386/i386.md (floatunssi2 expand): Add support
> for QI/HImode operand to produce more effecient code for

s/effecient/efficient/

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[Patch, Fortran] gfc_get_code cleanup

[Janus Weil]

Hi all,

attached is a cleanup patch which concerns the gfc_code structure and
gfc_get_code function (in st.c). It basically does two things:

1) It replaces the many occurrences of "XCNEW (gfc_code)" in class.c
by "gfc_get_code ()", which internally sets the locus and saves us
from doing it manually afterward.

2) It adds an argument "op" to gfc_get_code to directly set the .op
component of gfc_code. Every time we set up a new gfc_code structure,
we certainly want to set its op.

2b) There are a few instances where we do not set the op after calling
gfc_get_code, but instead copy the whole structure from someplace
else. For those cases I'm using "XCNEW (gfc_code)" now (which also
avoids setting the locus twice).

2c) In one place I'm using "gfc_get_code (EXEC_NOP)" for technical
reasons, see 'new_level' in parse.c.

Both items (1) and (2) result in more compact code and save a few
extra lines (see diffstat below). Regtested on
x86_64-unknown-linux-gnu. Ok for trunk with a suitable ChangeLog?

Cheers,
Janus



> diffstat gfc_code_cleanup_v2.diff
 class.c  |  180 +++
 gfortran.h   |2
 io.c |   18 +
 match.c  |   58 +++
 parse.c  |4 -
 resolve.c|   30 +++--
 st.c |6 +
 trans-expr.c |3
 trans-stmt.c |3
 9 files changed, 96 insertions(+), 208 deletions(-)


gfc_code_cleanup_v2.diff
Description: Binary data

Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)

[Steve Ellcey]

On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote:

> Given all of the above testing I'm reasonably confident that this works.
> However this is such a large change [1] that there's a non-zero chance
> of at least one glitch - let me know if you see any breakages.

The mips*-*-* targets are not building.  It looks like the mips reorg
pass (pass_mips_machine_reorg2) in config/mips/mips.c was not converted
and/or was not converted correctly.

Steve Ellcey
sell...@mips.com

Re: [PATCH] Convert more passes to new dump framework

[Xinliang David Li]

On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
> Hi,
>
> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> This patch ports messages to the new dump framework,
>
> It would be great this new framework was documented somewhere.  I lost
> track of what was agreed it would be and from the uses in the
> vectorizer I was never quite sure how to utilize it in other passes.

Sharad, can you put the documentation in GCC wiki.

In a nutshell, the new dumping interfaces produces information notes
which have 'dual' outputs -- controlled by different options. When
-fdump-- is on, the dump info will be dumped into the
pass specific dump file, and when -fopt-info=.. is on, the information
will be dumped into stderr.

The dump call should be guarded by dump_enabled_p().

thanks,

David

>
> I'd also like to point out two other minor things inline:
>
> [...]
>
>> 2013-08-06  Teresa Johnson  
>> Dehao Chen  
>>
>> * dumpfile.c (dump_loc): Add column number to output, make newlines
>> consistent.
>> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>> * ipa-inline-transform.c (clone_inlined_nodes):
>> (cgraph_node_opt_info): New function.
>> (cgraph_node_call_chain): Ditto.
>> (dump_inline_decision): Ditto.
>> (inline_call): Invoke dump_inline_decision.
>> * doc/invoke.texi: Document optall -fopt-info flag.
>> * profile.c (read_profile_edge_counts): Use new dump framework.
>> (compute_branch_probabilities): Ditto.
>> * passes.c (pass_manager::register_one_dump_file): Use OPTGROUP_OTHER
>> when pass not in any opt group.
>> * value-prof.c (check_counter): Use new dump framework.
>> (find_func_by_funcdef_no): Ditto.
>> (check_ic_target): Ditto.
>> * coverage.c (get_coverage_counts): Ditto.
>> (coverage_init): Setup new dump framework.
>> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>> * ipa-inline.h (is_in_ipa_inline): Declare.
>>
>> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>> * testsuite/gcc.dg/pr26570.c: Ditto.
>> * testsuite/gcc.dg/pr32773.c: Ditto.
>> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>
>
> [...]
>
>> Index: ipa-inline-transform.c
>> ===
>> --- ipa-inline-transform.c  (revision 201461)
>> +++ ipa-inline-transform.c  (working copy)
>> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>>  }
>>
>>
>> +#define MAX_INT_LENGTH 20
>> +
>> +/* Return NODE's name and profile count, if available.  */
>> +
>> +static const char *
>> +cgraph_node_opt_info (struct cgraph_node *node)
>> +{
>> +  char *buf;
>> +  size_t buf_size;
>> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>> +
>> +  if (!bfd_name)
>> +bfd_name = "unknown";
>> +
>> +  buf_size = strlen (bfd_name) + 1;
>> +  if (profile_info)
>> +buf_size += (MAX_INT_LENGTH + 3);
>> +
>> +  buf = (char *) xmalloc (buf_size);
>> +
>> +  strcpy (buf, bfd_name);
>> +
>> +  if (profile_info)
>> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>> +  return buf;
>> +}
>
> I'm not sure if output of this function is aimed only at the user or
> if it is supposed to be used by gcc developers as well.  If the
> latter, an incredibly useful thing is to also dump node->symbol.order
> too.  We usually dump it after "/" sign separating it from node name.
> It is invaluable when examining decisions in C++ code where you can
> have lots of clones of a node (and also because existing dumps print
> it, it is easy to combine them).
>
> [...]
>
>> Index: ipa-inline.c
>> ===
>> --- ipa-inline.c(revision 201461)
>> +++ ipa-inline.c(working copy)
>> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3.  If not see
>>  static int overall_size;
>>  static gcov_type max_count;
>>
>> +/* Global variable to denote if it is in ipa-inline pass. */
>> +bool is_in_ipa_inline = false;
>> +
>>  /* Return false when inlining edge E would lead to violating
>> limits on function unit growth or stack usage growth.
>>
>
> In this age of removing global variables, are you sure you need this?
> The only user of this seems to be a function that is only being called
> from inline_call... can that ever happen when not inlining?  If you
> plan to use this function also elsewhere, perhaps the callers will
> know whether we are inlining or not and can provide this in a
> parameter?
>
> Thanks,
>
> Martin

Re: [PATCH] Convert more passes to new dump framework

[Martin Jambor]

Hi,

On Tue, Aug 06, 2013 at 07:14:42AM -0700, Teresa Johnson wrote:
> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
> > On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
> >> This patch ports messages to the new dump framework,
> >
> > It would be great this new framework was documented somewhere.  I lost
> > track of what was agreed it would be and from the uses in the
> > vectorizer I was never quite sure how to utilize it in other passes.
> 
> Cc'ing Sharad who implemented this - Sharad, is this documented on a
> wiki or elsewhere?

Thanks

> 
> >
> > I'd also like to point out two other minor things inline:
> >
> > [...]
> >
> >> 2013-08-06  Teresa Johnson  
> >> Dehao Chen  
> >>
> >> * dumpfile.c (dump_loc): Add column number to output, make newlines
> >> consistent.
> >> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
> >> * ipa-inline-transform.c (clone_inlined_nodes):
> >> (cgraph_node_opt_info): New function.
> >> (cgraph_node_call_chain): Ditto.
> >> (dump_inline_decision): Ditto.
> >> (inline_call): Invoke dump_inline_decision.
> >> * doc/invoke.texi: Document optall -fopt-info flag.
> >> * profile.c (read_profile_edge_counts): Use new dump framework.
> >> (compute_branch_probabilities): Ditto.
> >> * passes.c (pass_manager::register_one_dump_file): Use 
> >> OPTGROUP_OTHER
> >> when pass not in any opt group.
> >> * value-prof.c (check_counter): Use new dump framework.
> >> (find_func_by_funcdef_no): Ditto.
> >> (check_ic_target): Ditto.
> >> * coverage.c (get_coverage_counts): Ditto.
> >> (coverage_init): Setup new dump framework.
> >> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
> >> * ipa-inline.h (is_in_ipa_inline): Declare.
> >>
> >> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
> >> * testsuite/gcc.dg/pr26570.c: Ditto.
> >> * testsuite/gcc.dg/pr32773.c: Ditto.
> >> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
> >>
> >
> > [...]
> >
> >> Index: ipa-inline-transform.c
> >> ===
> >> --- ipa-inline-transform.c  (revision 201461)
> >> +++ ipa-inline-transform.c  (working copy)
> >> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
> >>  }
> >>
> >>
> >> +#define MAX_INT_LENGTH 20
> >> +
> >> +/* Return NODE's name and profile count, if available.  */
> >> +
> >> +static const char *
> >> +cgraph_node_opt_info (struct cgraph_node *node)
> >> +{
> >> +  char *buf;
> >> +  size_t buf_size;
> >> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
> >> +
> >> +  if (!bfd_name)
> >> +bfd_name = "unknown";
> >> +
> >> +  buf_size = strlen (bfd_name) + 1;
> >> +  if (profile_info)
> >> +buf_size += (MAX_INT_LENGTH + 3);
> >> +
> >> +  buf = (char *) xmalloc (buf_size);
> >> +
> >> +  strcpy (buf, bfd_name);
> >> +
> >> +  if (profile_info)
> >> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
> >> +  return buf;
> >> +}
> >
> > I'm not sure if output of this function is aimed only at the user or
> > if it is supposed to be used by gcc developers as well.  If the
> > latter, an incredibly useful thing is to also dump node->symbol.order
> > too.  We usually dump it after "/" sign separating it from node name.
> > It is invaluable when examining decisions in C++ code where you can
> > have lots of clones of a node (and also because existing dumps print
> > it, it is easy to combine them).
> 
> The output is useful for both power users doing performance tuning of
> their application, and by gcc developers. Adding the id is not so
> useful for the former, but I agree that it is very useful for compiler
> developers. In fact, in the google branch version we emit more verbose
> information (the lipo module id and the funcdef_no) to help uniquely
> identify the routines and to aid in post-processing by humans and
> tools. So it is probably useful to add something similar here too. Is
> the node->symbol.order more or less unique than the funcdef_no? I see
> that you added a patch a few months ago to print the
> node->symbol.order in the function header, and it also has the
> advantage as you note of matching up with existing ipa dumps.

node->symbol.order is unique and if I remember correctly, it is not
even recycled.  Clones, inline clones, thunks, every symbol table node
gets its own symbol order so it should be more unique than funcdef_no.
On the other hand it may be a bit cryptic for users but at the same
time it is only one number.

> 
> >
> > [...]
> >
> >> Index: ipa-inline.c
> >> ===
> >> --- ipa-inline.c(revision 201461)
> >> +++ ipa-inline.c(working copy)
> >> @@ -118,6 +118,9 @@ along with GCC; see th

Re: [buildbot] r201508: Build failures after pass C++ conversion

[David Malcolm]

On Tue, 2013-08-06 at 13:12 +0200, Jan-Benedict Glaw wrote:
> On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
>  wrote:
> > New Revision: 201508
> > 
> > URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> > Log:
> > Automated conversion of passes to C++ classes
> > 
> > gcc/
> > 
> > Patch autogenerated by refactor_passes.py from
> > https://github.com/davidmalcolm/gcc-refactoring-scripts
> > revision 03fe39476a4c4ea450b49e087cfa817b5f92021e
> 
> I see quite some fall-out from this on epiphany-elf:
> 
> g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../../../gcc/gcc 
> -I../../../../gcc/gcc/build -I../../../../gcc/gcc/../include 
> -I../../../../gcc/gcc/../libcpp/include  
> -I../../../../gcc/gcc/../libdecnumber 
> -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> -I../../../../gcc/gcc/../libbacktrace\
> -o build/genflags.o ../../../../gcc/gcc/genflags.c
> In file included from ./tm.h:21:0,
>  from ../../../../gcc/gcc/genflags.c:26:
> ../../../../gcc/gcc/config/epiphany/epiphany.h:932:8: error: ‘rtl_opt_pass’ 
> does not name a type
>  extern rtl_opt_pass *make_pass_mode_switch_use (gcc::context *ctxt);
> ^
> ../../../../gcc/gcc/config/epiphany/epiphany.h:933:8: error: ‘rtl_opt_pass’ 
> does not name a type
>  extern rtl_opt_pass *make_pass_resolve_sw_modes (gcc::context *ctxt);
> ^
Sorry about this, clearly I failed to test the target-specific passes.

epiphany_init does some interesting manipulation of passes.  I don't
have epiphany hardware, but I've reproduced the build failure on my
x86_64 box using --configure target=epiphany-elf, and the attached patch
ports it to the new API.  With this patch, stage 1 was able to build,
and cc1 seems to generate assembler on a trivial test. I was able to
step through epiphany_init's pass manipulation, and it appears to be
doing the right thing.

This does add two new headers to epiphany.c; I'm not seeing where to add
the deps (gcc/config/epiphany/t-epiphany doesn't list epiphany.o).

I'm kicking off a bootstrap of this (on x86_64) to further verify that
the non-epiphany changes are good.  What other testing can I do to
verify this?

gcc/

* config/epiphany/epiphany.c (pass_mode_switch_use): New.
(epiphany_init): Port to new C++ pass API.
(epiphany_optimize_mode_switching): Likewise.
* config/epiphany/epiphany.h: Likewise.
* pass_manager.h (pass_manager::get_pass_split_all_insns): New.
(pass_manager::get_pass_mode_switching): New.
(pass_manager::get_pass_peephole2): New.
* mode-switching.c (pass_mode_switching): Add clone method.
* recog.c (pass_peephole2): Add clone method.
(pass_split_all_insns): Add clone method.

Sorry again about this.
Index: gcc/mode-switching.c
===
--- gcc/mode-switching.c	(revision 201526)
+++ gcc/mode-switching.c	(working copy)
@@ -809,6 +809,9 @@
   {}
 
   /* opt_pass methods: */
+  /* The epiphany backend creates a second instance of this pass, so we need
+ a clone method.  */
+  opt_pass * clone () { return new pass_mode_switching (ctxt_); }
   bool gate () { return gate_mode_switching (); }
   unsigned int execute () { return rest_of_handle_mode_switching (); }
 
Index: gcc/config/epiphany/epiphany.c
===
--- gcc/config/epiphany/epiphany.c	(revision 201526)
+++ gcc/config/epiphany/epiphany.c	(working copy)
@@ -45,6 +45,8 @@
 #include "ggc.h"
 #include "tm-constrs.h"
 #include "tree-pass.h"	/* for current_pass */
+#include "context.h"
+#include "pass_manager.h"
 
 /* Which cpu we're compiling for.  */
 int epiphany_cpu_type;
@@ -59,6 +61,9 @@
 /* The rounding mode that we generally use for floating point.  */
 int epiphany_normal_fp_rounding;
 
+/* The pass instance, for use in epiphany_optimize_mode_switching. */
+static opt_pass *pass_mode_switch_use;
+
 static void epiphany_init_reg_tables (void);
 static int get_epiphany_condition_code (rtx);
 static tree epiphany_handle_interrupt_attribute (tree *, tree, tree, int, bool *);
@@ -165,20 +170,24 @@
  pass because of the side offect of epiphany_mode_needed on
  MACHINE_FUNCTION(cfun)->unknown_mode_uses.  But it must run before
  pass_resolve_sw_modes.  */
-  static struct register_pass_info insert_use_info
-= { &pass_mode_switch_use.pass, "mode_sw",
+  pass_mode_switch_use = make_pass_mode_switch_use (g);
+  struct register_pass_info insert_use_info
+= { pass_mode_switch_use, "mode_sw",
 	1, PASS_POS_INSERT_AFTER
   };
-  static struct register_pass_info mode_sw2_info
-= { &

Re: New parameters to control stringop expansion libcall strategy

[Xinliang David Li]

On Tue, Aug 6, 2013 at 2:42 AM, Jan Hubicka  wrote:
>> >>> 2013-08-02  Xinliang David Li  
>> >>>
>> >>> * config/i386/stringop.def: New file.
>> >>> * config/i386/stringop.opt: New file.
>> >>> * config/i386/i386-opts.h: Include stringopt.def.
>> >>> * config/i386/i386.opt: Include stringopt.opt.
>> >>> * config/i386/i386.c (ix86_option_override_internal):
>> >>> Override default size based stringop inline strategies
>> >>> with options.
>> >>> * config/i386/i386.c (ix86_parse_stringop_strategy_string):
>> >>> New function.
>> >>>
>> >>> 2013-08-04  Xinliang David Li  
>> >>>
>> >>> * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test.
>> >>> * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto.
>> >>> * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto.
>> >>> * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto.
>
> The patch looks resonable to me in general.  I wonder why we need to bring
> all the cost tables non-const instead of just having writable storage for
> the "current strategy" like we do with other flags anyway.

Having const on those arrays do not bring us anything -- those tables
will be accessed indirectly so const-prop won't happen anyways.
current_strategy is an embedded struct in the cost array so it ends up
in RO data when top level array is const.

>
> Your strings are definitely more readable than the in-memory representation
> I came up with. Perhaps we can even turn the cost tables into strings
> for easier maintenance?  I guess they are bit confusing for people
> not familiar with a code.

I think the in memory representation is fine -- if there is a need for
internal representation cleanup, it should done as another patch.
WDTY?

thanks,

David

>
> Honza
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li  
>> >>> wrote:
>> >>> > On x86_64, when the expected size of memcpy/memset is known (e.g, with
>> >>> > FDO), libcall strategy is used with the size is > 8192. This value is
>> >>> > hard coded, which makes it hard to do performance tuning. This patch
>> >>> > adds two new parameters to do that. Potential usage includes
>> >>> > per-application libcall strategy min-size tuning based on summary data
>> >>> > with FDO (e.g, instruction workset size).
>> >>> >
>> >>> > Bootstrap and tested on x86_64/linux. Ok for trunk?
>> >>> >
>> >>> > thanks,
>> >>> >
>> >>> > David
>> >>> >
>> >>> >
>> >>> > 2013-08-02  Xinliang David Li  
>> >>> >
>> >>> > * params.def: New parameters.
>> >>> > * config/i386/i386.c (ix86_option_override_internal):
>> >>> > Override default libcall size limit with parameters.
>> >>
>> >>> Index: config/i386/stringop.def
>> >>> ===
>> >>> --- config/i386/stringop.def  (revision 0)
>> >>> +++ config/i386/stringop.def  (revision 0)
>> >>> @@ -0,0 +1,42 @@
>> >>> +/* Definitions for option handling for IA-32.
>> >>> +   Copyright (C) 2013 Free Software Foundation, Inc.
>> >>> +
>> >>> +This file is part of GCC.
>> >>> +
>> >>> +GCC is free software; you can redistribute it and/or modify
>> >>> +it under the terms of the GNU General Public License as published by
>> >>> +the Free Software Foundation; either version 3, or (at your option)
>> >>> +any later version.
>> >>> +
>> >>> +GCC is distributed in the hope that it will be useful,
>> >>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >>> +GNU General Public License for more details.
>> >>> +
>> >>> +Under Section 7 of GPL version 3, you are granted additional
>> >>> +permissions described in the GCC Runtime Library Exception, version
>> >>> +3.1, as published by the Free Software Foundation.
>> >>> +
>> >>> +You should have received a copy of the GNU General Public License and
>> >>> +a copy of the GCC Runtime Library Exception along with this program;
>> >>> +see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>> >>> +.  */
>> >>> +
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (no_stringop, no_stringop)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (libcall, libcall)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (rep_prefix_1_byte, rep_byte)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (rep_prefix_4_byte, rep_4byte)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (rep_prefix_8_byte, rep_8byte)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (loop_1_byte, byte_loop)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (loop, loop)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (unrolled_loop, unrolled_loop)
>> >>> +DEF_ENUM
>> >>> +DEF_ALG (vector_loop, vector_loop)
>> >>> Index: config/i386/i386.opt
>> >>> ===
>> >>> --- config/i386/i386.opt  (revision 201458)
>> >>> +++ config/i386/i386.opt  (working copy)
>> >>> @@ -316,6 +316,14 @@ mstack-arg-probe
>> >>>  Target Report Mask(STA

Re: [buildbot] r201508: Build failures after pass C++ conversion

[David Malcolm]

On Tue, 2013-08-06 at 12:06 -0400, David Malcolm wrote:
> On Tue, 2013-08-06 at 13:12 +0200, Jan-Benedict Glaw wrote:
> > On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
> >  wrote:
> > > New Revision: 201508
> > > 
> > > URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> > > Log:
> > > Automated conversion of passes to C++ classes
> > > 
> > > gcc/
> > > 
> > >   Patch autogenerated by refactor_passes.py from
> > >   https://github.com/davidmalcolm/gcc-refactoring-scripts
> > >   revision 03fe39476a4c4ea450b49e087cfa817b5f92021e
> > 
> > I see quite some fall-out from this on epiphany-elf:
> > 
> > g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> > -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> > -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> > -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../../../gcc/gcc 
> > -I../../../../gcc/gcc/build -I../../../../gcc/gcc/../include 
> > -I../../../../gcc/gcc/../libcpp/include  
> > -I../../../../gcc/gcc/../libdecnumber 
> > -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> > -I../../../../gcc/gcc/../libbacktrace\
> > -o build/genflags.o ../../../../gcc/gcc/genflags.c
> > In file included from ./tm.h:21:0,
> >  from ../../../../gcc/gcc/genflags.c:26:
> > ../../../../gcc/gcc/config/epiphany/epiphany.h:932:8: error: ‘rtl_opt_pass’ 
> > does not name a type
> >  extern rtl_opt_pass *make_pass_mode_switch_use (gcc::context *ctxt);
> > ^
> > ../../../../gcc/gcc/config/epiphany/epiphany.h:933:8: error: ‘rtl_opt_pass’ 
> > does not name a type
> >  extern rtl_opt_pass *make_pass_resolve_sw_modes (gcc::context *ctxt);
> > ^
> Sorry about this, clearly I failed to test the target-specific passes.
> 
> epiphany_init does some interesting manipulation of passes.  I don't
> have epiphany hardware, but I've reproduced the build failure on my
> x86_64 box using --configure target=epiphany-elf, and the attached patch
> ports it to the new API.  With this patch, stage 1 was able to build,
> and cc1 seems to generate assembler on a trivial test. I was able to
> step through epiphany_init's pass manipulation, and it appears to be
> doing the right thing.
> 
> This does add two new headers to epiphany.c; I'm not seeing where to add
> the deps (gcc/config/epiphany/t-epiphany doesn't list epiphany.o).
> 
> I'm kicking off a bootstrap of this (on x86_64) to further verify that
> the non-epiphany changes are good.  What other testing can I do to
> verify this?
> 
> gcc/
> 
>   * config/epiphany/epiphany.c (pass_mode_switch_use): New.
>   (epiphany_init): Port to new C++ pass API.
>   (epiphany_optimize_mode_switching): Likewise.
>   * config/epiphany/epiphany.h: Likewise.
>   * pass_manager.h (pass_manager::get_pass_split_all_insns): New.
>   (pass_manager::get_pass_mode_switching): New.
>   (pass_manager::get_pass_peephole2): New.
>   * mode-switching.c (pass_mode_switching): Add clone method.
>   * recog.c (pass_peephole2): Add clone method.
>   (pass_split_all_insns): Add clone method.
> 
This patch also removed "static" qualifiers from the register_pass_info,
which may have been overzealous, though they seemed pointless: the
struct is only used during the lifetime of the register_pass call.

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Jan-Benedict Glaw]

On Tue, 2013-08-06 12:06:24 -0400, David Malcolm  wrote:
> On Tue, 2013-08-06 at 13:12 +0200, Jan-Benedict Glaw wrote:
> > On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
> >  wrote:
[Breakage on epiphany-elf]

> I'm kicking off a bootstrap of this (on x86_64) to further verify that
> the non-epiphany changes are good.  What other testing can I do to
> verify this?

Dunno.  I didn't do more than that with my build robot. Specifically,
I don't have the hardware, so I cannot recommend any further tests.
Maybe the port maintainer (added) could run a gcc built with your
patch[1] to verify it's still working?

MfG, JBG

[1] http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00286.html
-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
Signature of:http://www.chiark.greenend.org.uk/~sgtatham/bugs.html
the second  :


signature.asc
Description: Digital signature

Re: [PATCH] Convert more passes to new dump framework

[Sharad Singhai]

On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  wrote:
> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> Hi,
>>
>> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>>> This patch ports messages to the new dump framework,
>>
>> It would be great this new framework was documented somewhere.  I lost
>> track of what was agreed it would be and from the uses in the
>> vectorizer I was never quite sure how to utilize it in other passes.
>
> Sharad, can you put the documentation in GCC wiki.

Sure. I had user documentation in form of gcc info. But I will add
more developer details to a GCC wiki.

Thanks,
Sharad

> In a nutshell, the new dumping interfaces produces information notes
> which have 'dual' outputs -- controlled by different options. When
> -fdump-- is on, the dump info will be dumped into the
> pass specific dump file, and when -fopt-info=.. is on, the information
> will be dumped into stderr.
>
> The dump call should be guarded by dump_enabled_p().
>
> thanks,
>
> David
>
>>
>> I'd also like to point out two other minor things inline:
>>
>> [...]
>>
>>> 2013-08-06  Teresa Johnson  
>>> Dehao Chen  
>>>
>>> * dumpfile.c (dump_loc): Add column number to output, make newlines
>>> consistent.
>>> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>>> * ipa-inline-transform.c (clone_inlined_nodes):
>>> (cgraph_node_opt_info): New function.
>>> (cgraph_node_call_chain): Ditto.
>>> (dump_inline_decision): Ditto.
>>> (inline_call): Invoke dump_inline_decision.
>>> * doc/invoke.texi: Document optall -fopt-info flag.
>>> * profile.c (read_profile_edge_counts): Use new dump framework.
>>> (compute_branch_probabilities): Ditto.
>>> * passes.c (pass_manager::register_one_dump_file): Use 
>>> OPTGROUP_OTHER
>>> when pass not in any opt group.
>>> * value-prof.c (check_counter): Use new dump framework.
>>> (find_func_by_funcdef_no): Ditto.
>>> (check_ic_target): Ditto.
>>> * coverage.c (get_coverage_counts): Ditto.
>>> (coverage_init): Setup new dump framework.
>>> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>>> * ipa-inline.h (is_in_ipa_inline): Declare.
>>>
>>> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>>> * testsuite/gcc.dg/pr26570.c: Ditto.
>>> * testsuite/gcc.dg/pr32773.c: Ditto.
>>> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>>
>>
>> [...]
>>
>>> Index: ipa-inline-transform.c
>>> ===
>>> --- ipa-inline-transform.c  (revision 201461)
>>> +++ ipa-inline-transform.c  (working copy)
>>> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>>>  }
>>>
>>>
>>> +#define MAX_INT_LENGTH 20
>>> +
>>> +/* Return NODE's name and profile count, if available.  */
>>> +
>>> +static const char *
>>> +cgraph_node_opt_info (struct cgraph_node *node)
>>> +{
>>> +  char *buf;
>>> +  size_t buf_size;
>>> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>>> +
>>> +  if (!bfd_name)
>>> +bfd_name = "unknown";
>>> +
>>> +  buf_size = strlen (bfd_name) + 1;
>>> +  if (profile_info)
>>> +buf_size += (MAX_INT_LENGTH + 3);
>>> +
>>> +  buf = (char *) xmalloc (buf_size);
>>> +
>>> +  strcpy (buf, bfd_name);
>>> +
>>> +  if (profile_info)
>>> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>>> +  return buf;
>>> +}
>>
>> I'm not sure if output of this function is aimed only at the user or
>> if it is supposed to be used by gcc developers as well.  If the
>> latter, an incredibly useful thing is to also dump node->symbol.order
>> too.  We usually dump it after "/" sign separating it from node name.
>> It is invaluable when examining decisions in C++ code where you can
>> have lots of clones of a node (and also because existing dumps print
>> it, it is easy to combine them).
>>
>> [...]
>>
>>> Index: ipa-inline.c
>>> ===
>>> --- ipa-inline.c(revision 201461)
>>> +++ ipa-inline.c(working copy)
>>> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3.  If not see
>>>  static int overall_size;
>>>  static gcov_type max_count;
>>>
>>> +/* Global variable to denote if it is in ipa-inline pass. */
>>> +bool is_in_ipa_inline = false;
>>> +
>>>  /* Return false when inlining edge E would lead to violating
>>> limits on function unit growth or stack usage growth.
>>>
>>
>> In this age of removing global variables, are you sure you need this?
>> The only user of this seems to be a function that is only being called
>> from inline_call... can that ever happen when not inlining?  If you
>> plan to use this function also elsewhere, perhaps the callers will
>> know whether we are inlining or not and can provide this in a
>> parameter?
>>
>>

Re: [PATCH] Convert more passes to new dump framework

[Teresa Johnson]

On Tue, Aug 6, 2013 at 9:01 AM, Martin Jambor  wrote:
> Hi,
>
> On Tue, Aug 06, 2013 at 07:14:42AM -0700, Teresa Johnson wrote:
>> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> > On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> >> This patch ports messages to the new dump framework,
>> >
>> > It would be great this new framework was documented somewhere.  I lost
>> > track of what was agreed it would be and from the uses in the
>> > vectorizer I was never quite sure how to utilize it in other passes.
>>
>> Cc'ing Sharad who implemented this - Sharad, is this documented on a
>> wiki or elsewhere?
>
> Thanks
>
>>
>> >
>> > I'd also like to point out two other minor things inline:
>> >
>> > [...]
>> >
>> >> 2013-08-06  Teresa Johnson  
>> >> Dehao Chen  
>> >>
>> >> * dumpfile.c (dump_loc): Add column number to output, make 
>> >> newlines
>> >> consistent.
>> >> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>> >> * ipa-inline-transform.c (clone_inlined_nodes):
>> >> (cgraph_node_opt_info): New function.
>> >> (cgraph_node_call_chain): Ditto.
>> >> (dump_inline_decision): Ditto.
>> >> (inline_call): Invoke dump_inline_decision.
>> >> * doc/invoke.texi: Document optall -fopt-info flag.
>> >> * profile.c (read_profile_edge_counts): Use new dump framework.
>> >> (compute_branch_probabilities): Ditto.
>> >> * passes.c (pass_manager::register_one_dump_file): Use 
>> >> OPTGROUP_OTHER
>> >> when pass not in any opt group.
>> >> * value-prof.c (check_counter): Use new dump framework.
>> >> (find_func_by_funcdef_no): Ditto.
>> >> (check_ic_target): Ditto.
>> >> * coverage.c (get_coverage_counts): Ditto.
>> >> (coverage_init): Setup new dump framework.
>> >> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>> >> * ipa-inline.h (is_in_ipa_inline): Declare.
>> >>
>> >> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>> >> * testsuite/gcc.dg/pr26570.c: Ditto.
>> >> * testsuite/gcc.dg/pr32773.c: Ditto.
>> >> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>> >>
>> >
>> > [...]
>> >
>> >> Index: ipa-inline-transform.c
>> >> ===
>> >> --- ipa-inline-transform.c  (revision 201461)
>> >> +++ ipa-inline-transform.c  (working copy)
>> >> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>> >>  }
>> >>
>> >>
>> >> +#define MAX_INT_LENGTH 20
>> >> +
>> >> +/* Return NODE's name and profile count, if available.  */
>> >> +
>> >> +static const char *
>> >> +cgraph_node_opt_info (struct cgraph_node *node)
>> >> +{
>> >> +  char *buf;
>> >> +  size_t buf_size;
>> >> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>> >> +
>> >> +  if (!bfd_name)
>> >> +bfd_name = "unknown";
>> >> +
>> >> +  buf_size = strlen (bfd_name) + 1;
>> >> +  if (profile_info)
>> >> +buf_size += (MAX_INT_LENGTH + 3);
>> >> +
>> >> +  buf = (char *) xmalloc (buf_size);
>> >> +
>> >> +  strcpy (buf, bfd_name);
>> >> +
>> >> +  if (profile_info)
>> >> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>> >> +  return buf;
>> >> +}
>> >
>> > I'm not sure if output of this function is aimed only at the user or
>> > if it is supposed to be used by gcc developers as well.  If the
>> > latter, an incredibly useful thing is to also dump node->symbol.order
>> > too.  We usually dump it after "/" sign separating it from node name.
>> > It is invaluable when examining decisions in C++ code where you can
>> > have lots of clones of a node (and also because existing dumps print
>> > it, it is easy to combine them).
>>
>> The output is useful for both power users doing performance tuning of
>> their application, and by gcc developers. Adding the id is not so
>> useful for the former, but I agree that it is very useful for compiler
>> developers. In fact, in the google branch version we emit more verbose
>> information (the lipo module id and the funcdef_no) to help uniquely
>> identify the routines and to aid in post-processing by humans and
>> tools. So it is probably useful to add something similar here too. Is
>> the node->symbol.order more or less unique than the funcdef_no? I see
>> that you added a patch a few months ago to print the
>> node->symbol.order in the function header, and it also has the
>> advantage as you note of matching up with existing ipa dumps.
>
> node->symbol.order is unique and if I remember correctly, it is not
> even recycled.  Clones, inline clones, thunks, every symbol table node
> gets its own symbol order so it should be more unique than funcdef_no.
> On the other hand it may be a bit cryptic for users but at the same
> time it is only one number.

Ok, I am going to go ahead and add this to the output.

>
>>
>> >
>> > [...]

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

... today I did the following: commented out the error at issue 
(decl.c:11828) and ran the testsuite. The attached js.txt is the list of 
fails. Wanted to make sure that we have enough coverage.


I'm also attaching here the complete patch + testcase which passes boot 
& test.


Thanks!
Paolo.

///
FAIL: g++.dg/cpp0x/gen-attrs-27.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/ext/attrib27.C -std=c++98  (test for errors, line 5)
FAIL: g++.dg/ext/attrib27.C -std=c++11  (test for errors, line 5)
FAIL: g++.dg/lookup/struct1.C -std=c++98  (test for errors, line 6)
FAIL: g++.dg/lookup/struct1.C -std=c++98  (test for errors, line 9)
FAIL: g++.dg/lookup/struct1.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/lookup/struct1.C -std=c++11  (test for errors, line 9)
FAIL: g++.dg/parse/elab1.C -std=c++98  (test for errors, line 6)
FAIL: g++.dg/parse/elab1.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/parse/typedef3.C -std=c++98  (test for errors, line 7)
FAIL: g++.dg/parse/typedef3.C -std=c++11  (test for errors, line 7)
FAIL: g++.dg/template/crash26.C -std=c++98  (test for errors, line 8)
FAIL: g++.dg/template/crash26.C -std=c++11  (test for errors, line 8)

FAIL: g++.old-deja/g++.benjamin/typedef01.C -std=c++98  (test for errors, line 
22)
FAIL: g++.old-deja/g++.benjamin/typedef01.C -std=c++11  (test for errors, line 
22)
FAIL: g++.old-deja/g++.brendan/line1.C -std=c++98  (test for errors, line 4)
FAIL: g++.old-deja/g++.brendan/line1.C -std=c++11  (test for errors, line 4)
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 201524)
+++ cp/name-lookup.c(working copy)
@@ -4740,11 +4740,11 @@ lookup_name_real_1 (tree name, int prefer_type, in
  continue;
 
/* If this is the kind of thing we're looking for, we're done.  */
-   if (qualify_lookup (iter->value, flags))
+   if ((flags & LOOKUP_PREFER_TYPES)
+   && qualify_lookup (iter->type, flags))
+ binding = iter->type;
+   else if (qualify_lookup (iter->value, flags))
  binding = iter->value;
-   else if ((flags & LOOKUP_PREFER_TYPES)
-&& qualify_lookup (iter->type, flags))
- binding = iter->type;
else
  binding = NULL_TREE;
 
Index: testsuite/g++.dg/parse/typedef10.C
===
--- testsuite/g++.dg/parse/typedef10.C  (revision 0)
+++ testsuite/g++.dg/parse/typedef10.C  (working copy)
@@ -0,0 +1,20 @@
+// PR c++/46206
+
+class Foo1
+{
+  int u, v, w, x;
+  typedef struct Bar { } Bar;
+  virtual void foo(void) {
+struct Bar bar;
+  }
+};
+
+class Foo2
+{
+  int u, v, w;
+  typedef struct Bar { } Bar;
+  Bar bar;
+  virtual void foo(void) {
+struct Bar bar;
+  }
+};

Re: Go patch committed: Always put immutable structs in unique section

[Ian Lance Taylor]

On Tue, Aug 6, 2013 at 2:20 AM, Uros Bizjak  wrote:
>
> Patch [1] or [2] introduce following warning on alpha:
>
> /space/uros/gcc-build/gcc/testsuite/go2/../../gccgo
> -B/space/uros/gcc-build/gcc/testsuite/go2/../../
> /home/uros/gcc-svn/trunk/gcc/testsuite/go.test/test/cmplxdivide.go
> /home/uros/gcc-svn/trunk/gcc/testsuite/go.test/test/cmplxdivide1.go
> -fno-diagnostics-show-caret -fdiagnostics-color=never
> -I/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo
> -pedantic-errors
> -L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo
> -L/space/uros/gcc-build/alphaev68-unknown-linux-gnu/./libgo/.libs  -lm
>   -o ./cmplxdivide.o
>
> cmplxdivide.s: Assembler messages:
> cmplxdivide.s:357: Warning: setting incorrect section attributes for
> .rodata.__go_tdn_main.Test
> cmplxdivide.o: file not recognized: File truncated
> collect2: error: ld returned 1 exit status
>
> The warning can be fixed If I manually change section attribute of the
> above mentioned section from "aw" to "a" in the attached source.
>
> [1] http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00103.html
> [2] http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00099.html


I committed this patch that I think should fix this problem.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.8 branch.

Ian


2013-08-06  Ian Lance Taylor  

* go-gcc.cc (Gcc_backend::immutable_struct_set_init): Use
compute_reloc_for_constant.


foo.patch
Description: Binary data

Re: New parameters to control stringop expansion libcall strategy

[Xinliang David Li]

Corrected two small problems reported by the style checker (The
warnings about the EnumValue for options  in stringopt.opt are not
valid).

On Tue, Aug 6, 2013 at 1:46 AM, Michael Zolotukhin
 wrote:
> There are still some formatting issues (like 8 spaces instead of a
> tab, wrong indentation of do-loop and some other places) - to reveal
> some of them you could use contrib/check_GNU_style.sh script.
> But that was a nitpicking again:) Actually I wanted to ask whether
> you're going to use this option for some performance experiments
> involving memmov/memset - if so, probably you could tune existing
> cost-models as well? Is it possible?

the option is designed for purpose like this.

thanks,

David

>
> Michael
>
> On 5 August 2013 20:44, Xinliang David Li  wrote:
>> thanks. Updated patch attached.
>>
>> David
>>
>> On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin
>>  wrote:
>>> Hi,
>>> This is a really convenient option, thanks for working on it.
>>> I can't approve it as I'm not a maintainer, but it looks ok to me,
>>> except fot a small nitpicking: afair, comments should end with
>>> dot-space-space.
>>>
>>> Michael
>>>
>>> On 04 Aug 20:01, Xinliang David Li wrote:
 The attached is a new patch implementing the stringop inline strategy
 control using two new -m options:

 -mmemcpy-strategy=
 -mmemset-strategy=

 See changes in doc/invoke.texi for description of the new options. Example:
   
 -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned

 tells compiler to inline memcpy using rep_8byte when the size is no
 larger than 64 byte, using unrolled_loop when size is no larger than
 2048, and for size > 2048, using library call. In all cases,
 destination alignment adjustment is not done.

 Tested on x86-64/linux. Ok for trunk?

 thanks,

 David

 2013-08-02  Xinliang David Li  

 * config/i386/stringop.def: New file.
 * config/i386/stringop.opt: New file.
 * config/i386/i386-opts.h: Include stringopt.def.
 * config/i386/i386.opt: Include stringopt.opt.
 * config/i386/i386.c (ix86_option_override_internal):
 Override default size based stringop inline strategies
 with options.
 * config/i386/i386.c (ix86_parse_stringop_strategy_string):
 New function.

 2013-08-04  Xinliang David Li  

 * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test.
 * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto.
 * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto.
 * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto.




 On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li  
 wrote:
 > On x86_64, when the expected size of memcpy/memset is known (e.g, with
 > FDO), libcall strategy is used with the size is > 8192. This value is
 > hard coded, which makes it hard to do performance tuning. This patch
 > adds two new parameters to do that. Potential usage includes
 > per-application libcall strategy min-size tuning based on summary data
 > with FDO (e.g, instruction workset size).
 >
 > Bootstrap and tested on x86_64/linux. Ok for trunk?
 >
 > thanks,
 >
 > David
 >
 >
 > 2013-08-02  Xinliang David Li  
 >
 > * params.def: New parameters.
 > * config/i386/i386.c (ix86_option_override_internal):
 > Override default libcall size limit with parameters.
>>>
 Index: config/i386/stringop.def
 ===
 --- config/i386/stringop.def  (revision 0)
 +++ config/i386/stringop.def  (revision 0)
 @@ -0,0 +1,42 @@
 +/* Definitions for option handling for IA-32.
 +   Copyright (C) 2013 Free Software Foundation, Inc.
 +
 +This file is part of GCC.
 +
 +GCC is free software; you can redistribute it and/or modify
 +it under the terms of the GNU General Public License as published by
 +the Free Software Foundation; either version 3, or (at your option)
 +any later version.
 +
 +GCC is distributed in the hope that it will be useful,
 +but WITHOUT ANY WARRANTY; without even the implied warranty of
 +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +GNU General Public License for more details.
 +
 +Under Section 7 of GPL version 3, you are granted additional
 +permissions described in the GCC Runtime Library Exception, version
 +3.1, as published by the Free Software Foundation.
 +
 +You should have received a copy of the GNU General Public License and
 +a copy of the GCC Runtime Library Exception along with this program;
 +see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 +.  */
 +

Re: New parameters to control stringop expansion libcall strategy

[Xinliang David Li]

Forgot the patch.

David

On Tue, Aug 6, 2013 at 9:42 AM, Xinliang David Li  wrote:
> Corrected two small problems reported by the style checker (The
> warnings about the EnumValue for options  in stringopt.opt are not
> valid).
>
> On Tue, Aug 6, 2013 at 1:46 AM, Michael Zolotukhin
>  wrote:
>> There are still some formatting issues (like 8 spaces instead of a
>> tab, wrong indentation of do-loop and some other places) - to reveal
>> some of them you could use contrib/check_GNU_style.sh script.
>> But that was a nitpicking again:) Actually I wanted to ask whether
>> you're going to use this option for some performance experiments
>> involving memmov/memset - if so, probably you could tune existing
>> cost-models as well? Is it possible?
>
> the option is designed for purpose like this.
>
> thanks,
>
> David
>
>>
>> Michael
>>
>> On 5 August 2013 20:44, Xinliang David Li  wrote:
>>> thanks. Updated patch attached.
>>>
>>> David
>>>
>>> On Mon, Aug 5, 2013 at 3:57 AM, Michael V. Zolotukhin
>>>  wrote:
 Hi,
 This is a really convenient option, thanks for working on it.
 I can't approve it as I'm not a maintainer, but it looks ok to me,
 except fot a small nitpicking: afair, comments should end with
 dot-space-space.

 Michael

 On 04 Aug 20:01, Xinliang David Li wrote:
> The attached is a new patch implementing the stringop inline strategy
> control using two new -m options:
>
> -mmemcpy-strategy=
> -mmemset-strategy=
>
> See changes in doc/invoke.texi for description of the new options. 
> Example:
>   
> -mmemcpy-strategy=rep_8byte:64:unaligned,unrolled_loop:2048:unaligned,libcall:-1:unaligned
>
> tells compiler to inline memcpy using rep_8byte when the size is no
> larger than 64 byte, using unrolled_loop when size is no larger than
> 2048, and for size > 2048, using library call. In all cases,
> destination alignment adjustment is not done.
>
> Tested on x86-64/linux. Ok for trunk?
>
> thanks,
>
> David
>
> 2013-08-02  Xinliang David Li  
>
> * config/i386/stringop.def: New file.
> * config/i386/stringop.opt: New file.
> * config/i386/i386-opts.h: Include stringopt.def.
> * config/i386/i386.opt: Include stringopt.opt.
> * config/i386/i386.c (ix86_option_override_internal):
> Override default size based stringop inline strategies
> with options.
> * config/i386/i386.c (ix86_parse_stringop_strategy_string):
> New function.
>
> 2013-08-04  Xinliang David Li  
>
> * testsuite/gcc.target/i386/memcpy-strategy-1.c: New test.
> * testsuite/gcc.target/i386/memcpy-strategy-2.c: Ditto.
> * testsuite/gcc.target/i386/memset-strategy-1.c: Ditto.
> * testsuite/gcc.target/i386/memcpy-strategy-3.c: Ditto.
>
>
>
>
> On Fri, Aug 2, 2013 at 9:21 PM, Xinliang David Li  
> wrote:
> > On x86_64, when the expected size of memcpy/memset is known (e.g, with
> > FDO), libcall strategy is used with the size is > 8192. This value is
> > hard coded, which makes it hard to do performance tuning. This patch
> > adds two new parameters to do that. Potential usage includes
> > per-application libcall strategy min-size tuning based on summary data
> > with FDO (e.g, instruction workset size).
> >
> > Bootstrap and tested on x86_64/linux. Ok for trunk?
> >
> > thanks,
> >
> > David
> >
> >
> > 2013-08-02  Xinliang David Li  
> >
> > * params.def: New parameters.
> > * config/i386/i386.c (ix86_option_override_internal):
> > Override default libcall size limit with parameters.

> Index: config/i386/stringop.def
> ===
> --- config/i386/stringop.def  (revision 0)
> +++ config/i386/stringop.def  (revision 0)
> @@ -0,0 +1,42 @@
> +/* Definitions for option handling for IA-32.
> +   Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 3, or (at your option)
> +any later version.
> +
> +GCC is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +GNU General Public License for more details.
> +
> +Under Section 7 of GPL version 3, you are granted additional
> +permissions described in the GCC Runtime Library Exception, version
> +3.1, as published by the Free Software Foundation.
> +
> +You should have received a copy of the GNU

Re: [PATCH] Cilk Keywords (_Cilk_spawn and _Cilk_sync) for C

[Aldy Hernandez]

[Richard, small question for you below].

Not all your changes to Makefile.in have a changelog entry.


+c-family/cilk.o : c-family/cilk.c $(TREE_H) $(SYSTEM_H) $(CONFIG_H)

toplev.h \

+$(TREE_H) coretypes.h tree-iterator.h $(TREE_INLINE_H)

$(CGRAPH_H) \

+   $(DIAGNOSTIC_CORE_H) $(GIMPLE_H) $(CILK_H)


cilk.c includes langhooks.h and c-family/c-common.h, but I don't see 
dependences for them.



+  if (flag_enable_cilkplus)


We really should do a mass renaming of this flag.  It should be 
flag_cilkplus.  Not necessary to get this patch in, but highly 
desirable, or perhaps as a separate patch.



+  /* IF the function has _Cilk_spawn in front of a function call

inside it

Lowercase "IF".  Also, this sentence reads funny.  I don't quite 
understand it.  "function call inside it"?  Can you reword it?




+/* Creates the internal functions for spawn helper and parent.  */
+
+void
+c_install_body_with_frame_cleanup (tree fndecl, tree body)
+{


Since this is in the global namespace, it needs a Cilk specific name. 
Perhaps cilk_install_body_with_frame_cleanup, or something to that effect?



+  tree list;
+  tree frame = make_cilk_frame (fndecl);
+  tree dtor = build_cilk_function_exit (frame, false, false);
+  add_local_decl (cfun, frame);
+
+  DECL_SAVED_TREE (fndecl) = (list = alloc_stmt_list ());


Split this into two statements.


+ error_at (input_location, "consecutive _Cilk_spawn keywords not "
+   "permitted");


"are not permitted"


+
+/* Marks CALL, a CALL_EXPR, as a spawned function call.  */
+
+tree
+c_build_spawns (location_t loc, tree call)
+{
+  cilkplus_set_spawn_marker (loc, call);
+  tree spawn_stmt = build1 (CILK_SPAWN_STMT, TREE_TYPE (call), call);
+  TREE_SIDE_EFFECTS (spawn_stmt) = 1;
+  return spawn_stmt;
+}


First, why the plural?  Second, name should probably be c_build_cilk_spawn


+
+/* Returns a tree of type CILK_SYNC_STMT if Cilk Plus is enabled. Otherwise
+   return error_mark_node.  */


Two spaces after sentence.


+
+tree
+c_build_sync (location_t loc)
+{
+  if (!flag_enable_cilkplus)
+{
+  error_at (loc, "-fcilkplus not enabled");
+  return error_mark_node;
+}


If actually needed, the check for cilkplus should be done in the caller.

Also, rename to c_build_cilk_sync


+  if (flag_enable_cilkplus)
+cpp_define_formatted (pfile, "__cilk=%d", 200);


Why the %d?  Can't you just hard code the 200 into the string?


+@item CILK_SPAWN_STMT
+
+Used to represent a spawning function in the Cilk Plus language extension. This
+tree has one field that holds the name of the spawning function.
+_Cilk_spawn can be written in C in the following way:


First, two spaces after "extension.".

Second, you should provide an accessor macro for the operand.  For 
example for a TRANSACTION_EXPR, we have the following in tree.h:


/* TM directives and accessors.  */
#define TRANSACTION_EXPR_BODY(NODE) \
  TREE_OPERAND (TRANSACTION_EXPR_CHECK (NODE), 0)

After you do this, all uses of TREE_OPERAND (blah, 0) should use this 
new accessor, and the documentation here should match.



+ _Cilk_spawn keyword is parsed and the function that it contains is marked as


The _Cilk_spawn keyword...

s/that it/it


+a spawning function.  The spawning function is called the spawner.  At the end
+of the parsing phase, appropriate internal (builtin) functions are added to


Rename "internal (builtin) functions" to just "built-in functions", and 
by the way, the correct nomenclature in GCC is "built-in".



+the spawner that are defined in Cilk runtime.  The appropriate locations of


s/in Cilk/in the Cilk/


+these functions, and the internal structures are detailed in
+@code{cilk_init_builtins} in file @file{cilk-common.c}.  The pointers to Cilk


s/in file/in the file/



+functions and fields of internal structures are described in @file{cilk.h}.
+The builtin functions are described in @file{cilk-builtins.def}.


built-in


+
+During the gimplification stage, a new "spawn-helper" function is created.


You can probably just say "during gimplification, ..."


+The spawned function is replaced with a spawn helper function in the spawner.
+The spawned function-call is moved into the spawn helper.  The main function
+that does these transformations is @code{gimplify_cilk_spawn} in
+@file{c-family/cilk.c}.  In the spawn-helper, the gimplification function
+@code{gimplify_call_expr}, inserts a function call @code{__cilkrts_detach}.
+This function is expanded by @code{builtin_expand_cilk_detach} located in
+@file{c-family/cilk.c}.
+
+@item _Cilk_sync:
+_Cilk_sync is parsed like any regular keyword.  During gimplification stage,


s/any regular/a
s/During gimplification stage/During gimplification/


+the function @code{gimplify_cilk_sync} in @file{c-family/cilk.c}, will replace
+this keyword with a set of functions that are stored in Cilk Runtime.  One of


"in the Cilk run-time


+the internal functions inserted during gimplification stage,

Re: [PATCH] Cilk Keywords (_Cilk_spawn and _Cilk_sync) for C

[Richard Henderson]

On 08/06/2013 06:49 AM, Aldy Hernandez wrote:
>> --- gcc/ipa-inline-analysis.c
>> +++ gcc/ipa-inline-analysis.c
>> @@ -1433,6 +1433,9 @@ initialize_inline_failed (struct cgraph_edge *e)
>>  e->inline_failed = CIF_REDEFINED_EXTERN_INLINE;
>>else if (e->call_stmt_cannot_inline_p)
>>  e->inline_failed = CIF_MISMATCHED_ARGUMENTS;
>> +  else if (flag_enable_cilkplus && cfun && cfun->calls_spawn)
>> +/* We can't inline if the function is spawing a function.  */
>> +e->inline_failed = CIF_BODY_NOT_AVAILABLE;
> 
> Hmmm, if we don't have a cfun, perhaps we should be sticking this calls_spawn
> bit in the cgraph node.
> 
> Richard?  Anyone?

There will always be a function struct.  Probably not cfun though.
You can get to the callee through the edge.

"BODY_NOT_AVAILABLE"?  Definitely an odd error message to have chosen
for this...


r~

Re: [PATCH] Convert more passes to new dump framework

[Martin Jambor]

On Tue, Aug 06, 2013 at 09:22:02AM -0700, Sharad Singhai wrote:
> On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  wrote:
> > On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
> >> Hi,
> >>
> >> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
> >>> This patch ports messages to the new dump framework,
> >>
> >> It would be great this new framework was documented somewhere.  I lost
> >> track of what was agreed it would be and from the uses in the
> >> vectorizer I was never quite sure how to utilize it in other passes.
> >
> > Sharad, can you put the documentation in GCC wiki.
> 
> Sure. I had user documentation in form of gcc info. But I will add
> more developer details to a GCC wiki.
> 

I have built trunk gccint.info yesterday but could not find any string
dump_enabled_p there, for example.  And when I quickly searched just
for the string "dump," I did not find any thing that looked like
dumping infrastructure either.  OTOH, I agree that fie would be the
best place for the documentation.

Or did I just miss it?  What section is it in then?

Thanks,

Martin

[rl78]: Port to new pass C++ API (was Re: [buildbot] r201508: Build failures after pass C++ conversion)

[David Malcolm]

On Tue, 2013-08-06 at 13:18 +0200, Jan-Benedict Glaw wrote:
> On Tue, 2013-08-06 13:12:57 +0200, Jan-Benedict Glaw  
> wrote:
> > On Mon, 2013-08-05 20:16:05 -, dmalc...@gcc.gnu.org 
> >  wrote:
> > > New Revision: 201508
> > > 
> > > URL: http://gcc.gnu.org/viewcvs?rev=201508&root=gcc&view=rev
> > > Log:
> > > Automated conversion of passes to C++ classes
> > > 
> > > gcc/
> > > 
> > >   Patch autogenerated by refactor_passes.py from
> > >   https://github.com/davidmalcolm/gcc-refactoring-scripts
> > >   revision 03fe39476a4c4ea450b49e087cfa817b5f92021e
> 
> And probably also for rl78-elf:
> 
> g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> -DHAVE_CONFIG_H -I. -I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
> -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
> -I../../../../gcc/gcc/../libdecnumber 
> -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> -I../../../../gcc/gcc/../libbacktrace\
> ../../../../gcc/gcc/config/rl78/rl78.c -o rl78.o
> ../../../../gcc/gcc/config/rl78/rl78.c:146:1: error: in C++98 
> ‘rl78_devirt_pass’ must be initialized by constructor, not by ‘{...}’
>  };
>  ^
> ../../../../gcc/gcc/config/rl78/rl78.c:146:1: error: could not convert 
> ‘{RTL_PASS, "devirt", 0, devirt_gate, devirt_pass, 0, 0, 212, TV_MACH_DEP, 0, 
> 0, 0, 0, 0}’ from ‘’ to ‘opt_pass’
> 
> MfG, JBG

Sorry again.

The attached patch fixes the build for me with --target=rl78-elf.  Only
tested lightly with build&host=x86_64 so far; I was able to build
stage1, verify cc1 generates assembler on a simple .c file, and step
through the changed code.   However it's much more self-contained than
the epiphany fix.  There was a hardcoded value of 212 for the
static_pass_number of the pass, which may have been what stopped by
automated script from fixing this, so I set this in the ctor in case
anything is relying on this.  The make_pass_rl78_devirt function is
rather redundant, but I was deliberately mimicking the changes made by
the automated script.

It adds a dep on context.h to gcc/config/rl78/rl78.c, and I didn't see
how to add this (t-rl78 only lists rl78-c.o).

commit 4eb61ff39f72295f680d4be9d447e7e6bdfe629f
Author: David Malcolm 
Date:   Tue Aug 6 13:08:42 2013 -0400

gcc/
	* config/rl78/rl78.c (rl78_devirt_pass): Convert from a struct to...
	(pass_rl78_devirt): ...new subclass of rtl_opt_pass along with...
	(pass_data_rl78_devirt): ...new pass_data instance and...
	(make_pass_rl78_devirt): ...new function.
	(rl78_asm_file_start): Port pass registration to new C++ API.

diff --git a/gcc/config/rl78/rl78.c b/gcc/config/rl78/rl78.c
index c2ed738..5bfb21f 100644
--- a/gcc/config/rl78/rl78.c
+++ b/gcc/config/rl78/rl78.c
@@ -49,6 +49,7 @@
 #include "rl78-protos.h"
 #include "dumpfile.h"
 #include "tree-pass.h"
+#include "context.h"
 
 static inline bool is_interrupt_func (const_tree decl);
 static inline bool is_brk_interrupt_func (const_tree decl);
@@ -129,30 +130,48 @@ devirt_pass (void)
 /* This pass converts virtual instructions using virtual registers, to
real instructions using real registers.  Rather than run it as
reorg, we reschedule it before vartrack to help with debugging.  */
-static struct opt_pass rl78_devirt_pass =
-{
-  RTL_PASS,
-  "devirt",
-  OPTGROUP_NONE,/* optinfo_flags */
-  devirt_gate,
-  devirt_pass,
-  NULL,
-  NULL,
-  212,
-  TV_MACH_DEP,
-  0, 0, 0,
-  0,
-  0
+namespace {
+
+const pass_data pass_data_rl78_devirt =
+{
+  RTL_PASS, /* type */
+  "devirt", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  true, /* has_gate */
+  true, /* has_execute */
+  TV_MACH_DEP, /* tv_id */
+  0, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
 };
 
-static struct register_pass_info rl78_devirt_info =
+class pass_rl78_devirt : public rtl_opt_pass
 {
-  & rl78_devirt_pass,
-  "vartrack",
-  1,
-  PASS_POS_INSERT_BEFORE
+public:
+  pass_rl78_devirt(gcc::context *ctxt)
+: rtl_opt_pass(pass_data_rl78_devirt, ctxt)
+  {
+/* Prior to porting to C++, the static_pass_number was hardcoded
+   to 212.  Replicate this behavior. */
+static_pass_number = 212;
+  }
+
+  /* opt_pass methods: */
+  bool gate () { return devirt_gate (); }
+  unsigned int execute () { return devirt_pass (); }
 };
 
+} // anon namespace
+
+rtl_opt_pass *
+make_pass_rl78_devirt (gcc::context *ctxt)
+{
+  return new pass_rl78_devirt (ctxt);
+}
+
+
 #undef  TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START rl78_asm_file_start
 
@@ -167,6 +186,15 @@ rl78_asm_file_start (void)
   fprintf (asm_out_file, "r%d\t=\t0x%x\n", 16 + i, 0xffee8 + i);
 }
 
+  opt_pass *rl78_devirt_

Re: [PATCH] Convert more passes to new dump framework

[Sharad Singhai]

On Tue, Aug 6, 2013 at 10:10 AM, Martin Jambor  wrote:
> On Tue, Aug 06, 2013 at 09:22:02AM -0700, Sharad Singhai wrote:
>> On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  wrote:
>> > On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>> >> Hi,
>> >>
>> >> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> >>> This patch ports messages to the new dump framework,
>> >>
>> >> It would be great this new framework was documented somewhere.  I lost
>> >> track of what was agreed it would be and from the uses in the
>> >> vectorizer I was never quite sure how to utilize it in other passes.
>> >
>> > Sharad, can you put the documentation in GCC wiki.
>>
>> Sure. I had user documentation in form of gcc info. But I will add
>> more developer details to a GCC wiki.
>>
>
> I have built trunk gccint.info yesterday but could not find any string
> dump_enabled_p there, for example.  And when I quickly searched just
> for the string "dump," I did not find any thing that looked like
> dumping infrastructure either.  OTOH, I agree that fie would be the
> best place for the documentation.
>
> Or did I just miss it?  What section is it in then?

Actually, the user-facing documentation is in doc/invoke.texi.
However, that doesn't describe dump_enabled_p. Do you think
gccint.info would be a good place? I can add documentation there
instead of creating a GCC wiki.

Thanks,
Sharad

> Thanks,
>
> Martin
>

[PATCH] Fix PR 58041 testcase failuers on i686

[Martin Jambor]

Hi,

Bernd Edlinger reported that my new testcase for PR 58041 generates
warnings about ABI on i686.  The following fixes it so I am going to
commit it now to both trunk and the 4.8 branch so that as few people
as possible see new failures.

Sorry for the fuss and thanks Bernd for reporting and testing,

Martin


2013-08-06  Martin Jambor  
Bernd Edlinger 

testsuite/
* gcc.dg/torture/pr58041.c (foo): Accept z by reference.
(a): Fix constructor.


diff --git a/gcc/testsuite/gcc.dg/torture/pr58041.c 
b/gcc/testsuite/gcc.dg/torture/pr58041.c
index e22ec3c..169a71a 100644
--- a/gcc/testsuite/gcc.dg/torture/pr58041.c
+++ b/gcc/testsuite/gcc.dg/torture/pr58041.c
@@ -3,8 +3,6 @@
 typedef long long V
   __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
 
-typedef struct S { V v; } P __attribute__((aligned (1)));
-
 struct s
 {
   char u;
@@ -12,24 +10,24 @@ struct s
 } __attribute__((packed,aligned(1)));
 
 __attribute__((noinline, noclone))
-long long foo(struct s *x, int y, V z)
+long long foo(struct s *x, int y, V *z)
 {
   V a = x->v[y];
-  x->v[y] = z;
+  x->v[y] = *z;
   return a[1];
 }
 
-struct s a = {0,{0,0}};
+struct s a = {0,{{0,0},{0,0}}};
 int main()
 {
   V v1 = {0,1};
   V v2 = {0,2};
 
-  if (foo(&a,0,v1) != 0)
+  if (foo(&a,0,&v1) != 0)
 __builtin_abort();
-  if (foo(&a,0,v2) != 1)
+  if (foo(&a,0,&v2) != 1)
 __builtin_abort();
-  if (foo(&a,1,v1) != 0)
+  if (foo(&a,1,&v1) != 0)
 __builtin_abort();
   return 0;
 }

[PATCH] Fix broken build on sparc (was Re: [buildbot] r201508: Build failures after pass C++ conversion)

[David Malcolm]

On Tue, 2013-08-06 at 14:12 +0200, Jan-Benedict Glaw wrote:
> On Tue, 2013-08-06 14:10:11 +0200, Jan-Benedict Glaw  
> wrote:
> > And probably also for sparc{,64}-linux:
> > 
> > g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> > -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> > -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> > -DHAVE_CONFIG_H -I. -I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
> > -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
> > -I../../../../gcc/gcc/../libdecnumber 
> > -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> > -I../../../../gcc/gcc/../libbacktrace\
> > ../../../../gcc/gcc/config/sparc/sparc.c -o sparc.o
> > ../../../../gcc/gcc/config/sparc/sparc.c:1043:27: error: expected 
> > primary-expression before ‘.’ token
> >&pass_work_around_errata.pass, /* pass */
> >^
> 
> This was wrong, it's probably caused by r201511: "Rewrite how
> instances of passes are cloned".

I think it's r201508, but in any case, I'm attaching a patch which fixes
this build error.  Only very lightly tested so far, with configure
--target=sparc-linux with build&host x86_64.  Was able to build a cc1
and step through the changed code in the debugger, though am getting
"cc1: error: no include path in which to search for stdc-predef.h"
commit 8abbe9fbce66bdb1b03281fc06bc86707b5b3cf6
Author: David Malcolm 
Date:   Tue Aug 6 13:32:32 2013 -0400

gcc/
	* config/sparc/sparc.c (insert_pass_work_around_errata): Move
	into...
	(sparc_option_override): ...and port to new C++ pass API.
	* config/sparc/t-sparc (sparc.o): Add dep on CONTEXT_H

diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index 66c33f7..7080b33 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "df.h"
 #include "opts.h"
 #include "tree-pass.h"
+#include "context.h"
 
 /* Processor costs */
 
@@ -1038,14 +1039,6 @@ make_pass_work_around_errata (gcc::context *ctxt)
   return new pass_work_around_errata (ctxt);
 }
 
-struct register_pass_info insert_pass_work_around_errata =
-{
-  &pass_work_around_errata.pass,	/* pass */
-  "dbr",/* reference_pass_name */
-  1,	/* ref_pass_instance_number */
-  PASS_POS_INSERT_AFTER			/* po_op */
-};
-
 /* Helpers for TARGET_DEBUG_OPTIONS.  */
 static void
 dump_target_flag_bits (const int flags)
@@ -1495,6 +1488,14 @@ sparc_option_override (void)
  (essentially) final form of the insn stream to work on.
  Registering the pass must be done at start up.  It's convenient to
  do it here.  */
+  opt_pass *errata_pass = make_pass_work_around_errata (g);
+  struct register_pass_info insert_pass_work_around_errata =
+{
+  errata_pass,		/* pass */
+  "dbr",			/* reference_pass_name */
+  1,			/* ref_pass_instance_number */
+  PASS_POS_INSERT_AFTER	/* po_op */
+};
   register_pass (&insert_pass_work_around_errata);
 }
 
diff --git a/gcc/config/sparc/t-sparc b/gcc/config/sparc/t-sparc
index 664f4a4..62ad3f7 100644
--- a/gcc/config/sparc/t-sparc
+++ b/gcc/config/sparc/t-sparc
@@ -24,7 +24,7 @@ sparc.o: $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
   $(FUNCTION_H) $(EXCEPT_H) $(EXPR_H) $(OPTABS_H) $(RECOG_H) \
   $(DIAGNOSTIC_CORE_H) $(GGC_H) $(TM_P_H) debug.h $(TARGET_H) \
   $(TARGET_DEF_H) $(COMMON_TARGET_H) $(GIMPLE_H) $(TREE_PASS_H) \
-  langhooks.h reload.h $(PARAMS_H) $(DF_H) $(OPTS_H) \
+  langhooks.h reload.h $(PARAMS_H) $(DF_H) $(OPTS_H) $(CONTEXT_H) \
   gt-sparc.h
 
 sparc-c.o: $(srcdir)/config/sparc/sparc-c.c \

Re: [PATCH, PR 57539] Testcase produced by multidelta and indent

[Martin Jambor]

Hi,

nobody replied to this, presumably because nobody has any objections
but also nobody really cares, and the patch sits there in my queue.
OTOH, some time ago people on IRC told me to just commit it.  It only
adds a testcase so I'll take the risk of being yelled at and go ahead.
I believe there will be no problems but of course I'll revert it
immediately if any occur (more information why trouble may happen
below in the original mail).

Martin


On Wed, Jun 12, 2013 at 02:13:45PM +0200, Martin Jambor wrote:
> Hi,
> 
> On Wed, Jun 12, 2013 at 01:59:29PM +0200, Martin Jambor wrote:
> > 2013-06-10  Martin Jambor  
> > 
> > PR tree-optimization/57539
> > * cgraphclones.c (cgraph_clone_node): Add parameter new_inlined_to, set
> > global.inlined_to of the new node to it.  All callers changed.
> > * ipa-inline-transform.c (clone_inlined_nodes): New variable
> > inlining_into, pass it to cgraph_clone_node.
> > * ipa-prop.c (ipa_propagate_indirect_call_infos): Do not call
> > ipa_free_edge_args_substructures.
> > (ipa_edge_duplication_hook): Only add edges from inlined nodes to
> > rdesc linked list.  Do not assert rdesc edges have inlined caller.
> > Assert we have found an rdesc in the rdesc list.
> 
> Creating a testcase for this bug from scratch is not particularly easy
> or reliable because it requires inliner to build up a particular data
> structure (and then inline it again), which depends on inlining order
> which can easily change.  So I did not want to spend too much time on
> it.
> 
> However, by running multidelta (several times), indent, multidelta and
> indent again on the testcase supplied by the reporter I quickly came
> up with the following beast.  The downside is that I have very little
> understanding what the testcase does with all potential problems in
> the future.  Thus I'm not sure if we want such a testcase in the
> testsuite at all.  Nevertheless, it currently tests for the bug, does
> not warn (without any -W options) and I'll be fine with reverting it
> should any problems arise.
> 
> So, is it also OK for the trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2013-06-11  Martin Jambor  
> 
> testsuite/
>   PR tree-optimization/57539
>   * gcc.dg/ipa/pr57539.c: New test.
> 
> Index: src/gcc/testsuite/gcc.dg/ipa/pr57539.c
> ===
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/pr57539.c
> @@ -0,0 +1,218 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +typedef long unsigned int size_t;
> +typedef struct
> +{
> +}
> +box;
> +typedef struct
> +{
> +}
> +textpara_t;
> +typedef struct _dtlink_s Dtlink_t;
> +typedef struct _dtdisc_s Dtdisc_t;
> +typedef struct _dtmethod_s Dtmethod_t;
> +typedef struct _dt_s Dt_t;
> +typedef void *(*Dtmemory_f) (Dt_t *, void *, size_t, Dtdisc_t *);
> +typedef void *(*Dtsearch_f) (Dt_t *, void *, int);
> +typedef void *(*Dtmake_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef void (*Dtfree_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef int (*Dtcompar_f) (Dt_t *, void *, void *, Dtdisc_t *);
> +typedef unsigned int (*Dthash_f) (Dt_t *, void *, Dtdisc_t *);
> +typedef int (*Dtevent_f) (Dt_t *, int, void *, Dtdisc_t *);
> +struct _dtlink_s
> +{
> +  Dtlink_t *right;
> +};
> +struct _dtdisc_s
> +{
> +  int key;
> +  int size;
> +  int link;
> +  Dtmake_f makef;
> +  Dtfree_f freef;
> +  Dtcompar_f comparf;
> +  Dthash_f hashf;
> +  Dtmemory_f memoryf;
> +  Dtevent_f eventf;
> +};
> +struct _dt_s
> +{
> +  Dtsearch_f searchf;
> +};
> +extern Dtmethod_t *Dtobag;
> +extern Dt_t *dtopen (Dtdisc_t *, Dtmethod_t *);
> +extern Dtlink_t *dtflatten (Dt_t *);
> +typedef struct Agobj_s Agobj_t;
> +typedef struct Agraph_s Agraph_t;
> +typedef struct Agnode_s Agnode_t;
> +typedef struct Agedge_s Agedge_t;
> +typedef struct Agdesc_s Agdesc_t;
> +typedef struct Agdisc_s Agdisc_t;
> +typedef struct Agrec_s Agrec_t;
> +struct Agobj_s
> +{
> +  Agrec_t *data;
> +};
> +struct Agdesc_s
> +{
> +};
> +extern Agraph_t *agopen (char *name, Agdesc_t desc, Agdisc_t * disc);
> +extern Agnode_t *agfstnode (Agraph_t * g);
> +extern Agnode_t *agnxtnode (Agraph_t * g, Agnode_t * n);
> +extern Agedge_t *agedge (Agraph_t * g, Agnode_t * t, Agnode_t * h, char 
> *name,
> +  int createflag);
> +extern Agedge_t *agfstout (Agraph_t * g, Agnode_t * n);
> +extern Agedge_t *agnxtout (Agraph_t * g, Agedge_t * e);
> +extern Agdesc_t Agdirected, Agstrictdirected, Agundirected,
> +  Agstrictundirected;
> +typedef struct Agraph_s graph_t;
> +typedef struct Agnode_s node_t;
> +typedef struct Agedge_s edge_t;
> +typedef union inside_t
> +{
> +  unsigned short minlen;
> +}
> +Agedgeinfo_t;
> +extern void *gmalloc (size_t);
> +typedef enum
> +{ AM_NONE, AM_VOR, AM_SCALE, AM_NSCALE, AM_SCALEXY, AM_PUSH, AM_PUSHPULL,
> +AM_ORTHO, AM_ORTHO_YX, AM_ORTHOXY, AM_ORTHOYX, AM_PORTHO, AM_PORTHO_YX,
> +AM_PORTHOXY, AM_PORTHOYX, AM_COMPRESS, AM_VPSC, AM_IPSEP, AM_PRISM }
> +adjust_mod

[Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))

[David Malcolm]

On Tue, 2013-08-06 at 08:16 -0700, Steve Ellcey wrote:
> On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote:
> 
> > Given all of the above testing I'm reasonably confident that this works.
> > However this is such a large change [1] that there's a non-zero chance
> > of at least one glitch - let me know if you see any breakages.
> 
> The mips*-*-* targets are not building.  It looks like the mips reorg
> pass (pass_mips_machine_reorg2) in config/mips/mips.c was not converted
> and/or was not converted correctly.

Sorry about this.

I was able to reproduce this build error with configure
--target=mips-linux-elf:
../../src/gcc/config/mips/mips.c:16379:28: error: expected
primary-expression before ‘.’ token

I'm attaching a patch which fixes that issue for me.  Only lightly
tested (build&host=x86_64, target as above) - I'm able to build stage1,
and cc1 appears to generate assembler on a simple test case.  I stepped
through the changed code in the debugger and it appears to do the right
thing.  However I'm not familiar with the internals of the pass in
question.

commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9
Author: David Malcolm 
Date:   Tue Aug 6 13:48:59 2013 -0400

gcc/
	* config/mips/mips.c (insert_pass_mips_machine_reorg2): Move
	into...
	(mips_option_override): ...here, porting to new C++ API for
	passes.

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 05ba003..4da80f4 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "target-globals.h"
 #include "opts.h"
 #include "tree-pass.h"
+#include "context.h"
 
 /* True if X is an UNSPEC wrapper around a SYMBOL_REF or LABEL_REF.  */
 #define UNSPEC_ADDRESS_P(X)	\
@@ -16374,13 +16375,6 @@ make_pass_mips_machine_reorg2 (gcc::context *ctxt)
   return new pass_mips_machine_reorg2 (ctxt);
 }
 
-struct register_pass_info insert_pass_mips_machine_reorg2 =
-{
-  &pass_mips_machine_reorg2.pass,	/* pass */
-  "dbr",/* reference_pass_name */
-  1,	/* ref_pass_instance_number */
-  PASS_POS_INSERT_AFTER			/* po_op */
-};
 
 /* Implement TARGET_ASM_OUTPUT_MI_THUNK.  Generate rtl rather than asm text
in order to avoid duplicating too much logic from elsewhere.  */
@@ -17174,6 +17168,14 @@ mips_option_override (void)
   /* We register a second machine specific reorg pass after delay slot
  filling.  Registering the pass must be done at start up.  It's
  convenient to do it here.  */
+  opt_pass *new_pass = make_pass_mips_machine_reorg2 (g);
+  struct register_pass_info insert_pass_mips_machine_reorg2 =
+{
+  new_pass,		/* pass */
+  "dbr",			/* reference_pass_name */
+  1,			/* ref_pass_instance_number */
+  PASS_POS_INSERT_AFTER	/* po_op */
+};
   register_pass (&insert_pass_mips_machine_reorg2);
 
   if (TARGET_HARD_FLOAT_ABI && TARGET_MIPS5900)

Re: [PATCH 05/11] Add -fno-rtti when building plugins.

[David Malcolm]

On Tue, 2013-08-06 at 12:43 +0200, Dominique Dhumieres wrote:
> > Hence plugins that create passes will need to be built with RTTI
> > disabled in order to link against gcc, or they will fail to load, with
> > an error like: ...
> 
> The same holds for darwin, hence the following patch is needed:
> 
> --- ../_clean/gcc/testsuite/lib/plugin-support.exp2013-08-05 
> 22:51:42.0 +0200
> +++ gcc/testsuite/lib/plugin-support.exp  2013-08-06 12:36:43.0 
> +0200
> @@ -101,7 +101,7 @@ proc plugin-test-execute { plugin_src pl
>   set optstr [concat $optstr " $op"]
>   }
>   }
> - set optstr [concat $optstr "-DIN_GCC -fPIC -shared -undefined 
> dynamic_lookup"]
> + set optstr [concat $optstr "-DIN_GCC -fPIC -shared -fno-rtti -undefined 
> dynamic_lookup"]
>  } else {
>   set plug_cflags $PLUGINCFLAGS 
>   set optstr "$includes $extra_flags -DIN_GCC -fPIC -shared -fno-rtti"

Sorry about that, and thanks for posting the patch.  The patch looks
correct to me, but I'm not a reviewer.

Re: [PATCH] Convert more passes to new dump framework

[Xinliang David Li]

yes -- if this is the place developers look at the most.

David

On Tue, Aug 6, 2013 at 10:18 AM, Sharad Singhai  wrote:
> On Tue, Aug 6, 2013 at 10:10 AM, Martin Jambor  wrote:
>> On Tue, Aug 06, 2013 at 09:22:02AM -0700, Sharad Singhai wrote:
>>> On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li  
>>> wrote:
>>> > On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>>> >> Hi,
>>> >>
>>> >> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>>> >>> This patch ports messages to the new dump framework,
>>> >>
>>> >> It would be great this new framework was documented somewhere.  I lost
>>> >> track of what was agreed it would be and from the uses in the
>>> >> vectorizer I was never quite sure how to utilize it in other passes.
>>> >
>>> > Sharad, can you put the documentation in GCC wiki.
>>>
>>> Sure. I had user documentation in form of gcc info. But I will add
>>> more developer details to a GCC wiki.
>>>
>>
>> I have built trunk gccint.info yesterday but could not find any string
>> dump_enabled_p there, for example.  And when I quickly searched just
>> for the string "dump," I did not find any thing that looked like
>> dumping infrastructure either.  OTOH, I agree that fie would be the
>> best place for the documentation.
>>
>> Or did I just miss it?  What section is it in then?
>
> Actually, the user-facing documentation is in doc/invoke.texi.
> However, that doesn't describe dump_enabled_p. Do you think
> gccint.info would be a good place? I can add documentation there
> instead of creating a GCC wiki.
>
> Thanks,
> Sharad
>
>> Thanks,
>>
>> Martin
>>

Re: [Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))

[Richard Sandiford]

David Malcolm  writes:
> commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9
> Author: David Malcolm 
> Date:   Tue Aug 6 13:48:59 2013 -0400
>
> gcc/
>   * config/mips/mips.c (insert_pass_mips_machine_reorg2): Move
>   into...
>   (mips_option_override): ...here, porting to new C++ API for
>   passes.

OK.  Thanks for the quick fix!

Richard

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ian Lance Taylor]

On Tue, Aug 6, 2013 at 10:35 AM, Caroline Tice  wrote:
>
> choose_tmpdir(), from libiberty is not mentioned in any .h file, so cannot
> (at the moment) be used here.  Is it OK for me to add choose_tmpdir to
> libiberty.h?

Well, maybe.  But the approach looks a bit odd to me.  Also very
underdocumented--the -fvtv-counts option needs expanding.  And I don't
see why a user would ever invoke it anyhow, so perhaps it should be a
--param rather than a -f option.  The output to the file doesn't have
any indication of what file is being compiled, so it will be ambiguous
when run in parallel.  Why shouldn't this data be dumped into a
standard GCC dump file as we do with most data like this?  Your
sum-vtv-counts program could still work, it would just take a set of
dump files and look for the specific strings that it cares about.

Ian

Re: [Patch] Unbreak build on mips (was Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes))

[David Malcolm]

On Tue, 2013-08-06 at 19:11 +0100, Richard Sandiford wrote:
> David Malcolm  writes:
> > commit 11d46884e8bd9802b0f528a16b3970b4076fe8a9
> > Author: David Malcolm 
> > Date:   Tue Aug 6 13:48:59 2013 -0400
> >
> > gcc/
> > * config/mips/mips.c (insert_pass_mips_machine_reorg2): Move
> > into...
> > (mips_option_override): ...here, porting to new C++ API for
> > passes.
> 
> OK.  Thanks for the quick fix!

Thanks; committed to trunk as r201542.

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ian Lance Taylor]

On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice  wrote:
>
> On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor  wrote:
>
>>The output to the file doesn't have
>> any indication of what file is being compiled, so it will be ambiguous
>> when run in parallel.
>
> You are mistaken.  It outputs one line to the log file for each compilation
> unit.  The output line begins with the name of the file that was being
> compiled.  In my use case, I have used this to build a very large system,
> which resulted in something like at 8000 line log file of counts, which I
> then used my sum script on to see how the verifications were going.

I was mistaken in detail but I'm not sure I was mistaken in principle.
What happens if you are building the large system twice in different
directories on the same machine?  Or, for that matter, if two
different people are doing so?  Or if one person did it a while ago,
and now you want to do it, but you can't open the file because it is
owned by the other person?

Maybe you should simply change -fvtv-counts to take a file name, then
we don't have to worry about any of this.

Ian

avx512 mask register spilling

[Richard Henderson]

On 08/06/2013 03:57 AM, Kirill Yukhin wrote:
> On 05 Aug 09:55, Richard Henderson wrote:
>> On 08/05/2013 08:07 AM, Kirill Yukhin wrote:
>>> Hello Richard, Vlad,
>>>
>>> On 31 Jul 06:26, Richard Henderson wrote:
 On 07/31/2013 05:02 AM, Kirill Yukhin wrote:
>
> There's ICE (max. number of generated reload insns per insn is achieved 
> (90)),
> when LRA tried to save mask register before call. This was caused by fact 
> that  split_reg function
> in lra-constraints.c allocates memory for saved reg in 
> SECONDARY_MEMORY_NEEDED_MODE which 

 I've told you before that it's not SECONDARY_MEMORY that you want, but
 a secondary reload.  You should be patching ix86_secondary_reload, right
 below where we handle QImode spills from non-Q registers for x86-32.
>>>
>>> Trying to do that with no success so far.
>>> Could you pls correct me if I am wrong.
>>> What I am trying to do is to introduce 2 new `define_expand' for load and 
>>> store.
>>
>> Huh?  You shouldn't need this.
>>
>> Give me a test case and I can have a look at it.
> 
> Hello,
> 
> I've squashed part 1 and 2 + rebased on recent trunk.
> Testcase is attached.
> To reproduce: build-x86_64-linux/gcc/xgcc -Bbuild-x86_64-linux/gcc -Ofast 
> -mavx512f -march=core-avx2 repro.c  -S -o-  -ffixed-rsi  -ffixed-rdi  
> -ffixed-rbx -ffixed-rbp -m32
> 
> Thanks a lot for help!

You've found what I believe to be a bug in LRA.

Specifically, lra-constraints.c split_reg uses SECONDARY_MEMORY_NEEDED_MODE to
choose what mode to spill a caller-save register.  Given the existing
definition in i386.h, this tries to spill a MASK_CLASS register in SImode.  But
MASK_CLASS does not support SImode, only QI/HImode.  Which leads to substantial
confusion in the allocator trying to satisfy the move.

I believe the use of SECONDARY_MEMORY_NEEDED_MODE in split_reg is wrong.
What's the history behind that, Vlad?  Surely we can spill the value in
its current mode?

Certainly this patch fixes the crash from Kirill's reproducer...


r~

* lra-constraints.c (split_reg): Always spill in the current mode.


diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 728c058..924d588 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -4388,11 +4388,7 @@ split_reg (bool before_p, int original_regno, rtx insn, 
rtx next_usage_insns)
 {
   enum machine_mode sec_mode;
 
-#ifdef SECONDARY_MEMORY_NEEDED_MODE
-  sec_mode = SECONDARY_MEMORY_NEEDED_MODE (GET_MODE (original_reg));
-#else
   sec_mode = GET_MODE (original_reg);
-#endif
   new_reg = lra_create_new_reg (sec_mode, NULL_RTX,
NO_REGS, "save");
 }

Re: Passes are now C++ classes (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)

[Basile Starynkevitch]

On Mon, 2013-08-05 at 17:03 -0400, David Malcolm wrote:
> On Mon, 2013-07-29 at 15:41 -0600, Jeff Law wrote:
> > On 07/26/2013 09:04 AM, David Malcolm wrote:
> > > This patch is the hand-written part of the conversion of passes from
> > > C structs to C++ classes.  It does not work without the subsequent
> > > autogenerated part, which is huge.
> > [ ... ]
> > With the changes from pipeline -> pass_manager this is fine.  As is the 
> > follow-up autogenerated patch.
> 
> I've committed this and the other patches that needed to go with it to
> trunk, with the name fix, having successfully bootstrapped and tested
> them on x86_64-unknown-linux-gnu - so opt_pass is now a C++ class
> hierarchy, allowing for pass instances to hold pass-specific data
> (albeit not GTY refs yet), 

Did you follow my suggestion of putting __FILE__ and __LINE__ in
instances of opt_pass? I really hope that will go into 4.9!

Cheers


-- 
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ian Lance Taylor]

On Tue, Aug 6, 2013 at 12:07 PM, Caroline Tice  wrote:
>
>
>
> On Tue, Aug 6, 2013 at 12:02 PM, Ian Lance Taylor  wrote:
>>
>> On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice  wrote:
>> >
>> > On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor 
>> > wrote:
>> >
>> >>The output to the file doesn't have
>> >> any indication of what file is being compiled, so it will be ambiguous
>> >> when run in parallel.
>> >
>> > You are mistaken.  It outputs one line to the log file for each
>> > compilation
>> > unit.  The output line begins with the name of the file that was being
>> > compiled.  In my use case, I have used this to build a very large
>> > system,
>> > which resulted in something like at 8000 line log file of counts, which
>> > I
>> > then used my sum script on to see how the verifications were going.
>>
>> I was mistaken in detail but I'm not sure I was mistaken in principle.
>> What happens if you are building the large system twice in different
>> directories on the same machine?  Or, for that matter, if two
>> different people are doing so?  Or if one person did it a while ago,
>> and now you want to do it, but you can't open the file because it is
>> owned by the other person?
>>
>> Maybe you should simply change -fvtv-counts to take a file name, then
>> we don't have to worry about any of this.
>>
> It's not quite that simple:  the -fvtv-counts flag actually causes two files
> to be created; also there's another flag, -fvtv-debug that generates a third
> file (i wanted a lot of information when I was working on and debugging this
> feature).  Also if users are arbitrarily allowed to name the counts file
> anything, the summing script program I wrote won't be able to find them.

That doesn't seem like a compelling argument to me, since one could
pass the file names to the summing script as well.

As far as I can see, on a multi-user system, there is no reasonable
alternative to permitting the user to specify the file names to use,
or at least a directory where the files should be placed.  And if
permit that, why not simply require it?

Ian

Merge from gcc 4.8 branch to gccgo branch

[Ian Lance Taylor]

I merged revision 201542 from the 4.8 branch to the gccgo branch.

Ian

Re: [PATCH i386 2/8] [AVX512] Add mask registers.

[Richard Henderson]

BTW, you've a bug in your movqi pattern:

> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index b4c9ac5..d8401b5 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -2298,7 +2298,7 @@
>  ;; partial register stall can be caused there.  Then we use movzx.
>  (define_insn "*movqi_internal"
>[(set (match_operand:QI 0 "nonimmediate_operand" "=q,q ,q ,r,r ,?r,m 
> ,Yk,Yk,r
> -   (match_operand:QI 1 "general_operand"  "q 
> ,qn,qm,q,rn,qm,qn,rm,Yk,Yk
> +   (match_operand:QI 1 "general_operand"  "q 
> ,qn,qm,q,rn,qm,qn,r,Yk,Yk"
>"!(MEM_P (operands[0]) && MEM_P (operands[1]))"
>  {
>switch (get_attr_type (insn))

You can't read from memory into a mask register in QImode.
This will fail if the memory was the last byte of a page,
and the following page is not mapped.

I expected you to need the following patch, to help spill
and fill QImode values, but I havn't found a test case that
actually needs it.  Perhaps LRA is better than old reload
about guessing things like this?

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index fa79441..0e74ec6 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -33926,10 +33926,11 @@ ix86_secondary_reload (bool in_p, rtx x, 
> reg_class_t r
>  
>/* QImode spills from non-QI registers require
>   intermediate register on 32bit targets.  */
> -  if (!TARGET_64BIT
> -  && !in_p && mode == QImode
> -  && INTEGER_CLASS_P (rclass)
> -  && MAYBE_NON_Q_CLASS_P (rclass))
> +  if (mode == QImode
> +  && (MAYBE_MASK_CLASS_P (rclass)
> +  || (!TARGET_64BIT && !in_p
> +  && INTEGER_CLASS_P (rclass)
> +  && MAYBE_NON_Q_CLASS_P (rclass
>  {
>int regno;
>  


r~

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

I was talking with Diego, and he suggested the possibility of putting
the log files in the same directory that the gcc dump files go, i.e.
the one specified by dump_base_name.  Do you think that would be
acceptable?

-- Caroline Tice
cmt...@google.com

On Tue, Aug 6, 2013 at 12:12 PM, Ian Lance Taylor  wrote:
>>> On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice  wrote:
>>> >
>>> > On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor 
>>> > wrote:
>>> >
>>> >>The output to the file doesn't have
>>> >> any indication of what file is being compiled, so it will be ambiguous
>>> >> when run in parallel.
>>> >
>>> > You are mistaken.  It outputs one line to the log file for each
>>> > compilation
>>> > unit.  The output line begins with the name of the file that was being
>>> > compiled.  In my use case, I have used this to build a very large
>>> > system,
>>> > which resulted in something like at 8000 line log file of counts, which
>>> > I
>>> > then used my sum script on to see how the verifications were going.
>>>
>>> I was mistaken in detail but I'm not sure I was mistaken in principle.
>>> What happens if you are building the large system twice in different
>>> directories on the same machine?  Or, for that matter, if two
>>> different people are doing so?  Or if one person did it a while ago,
>>> and now you want to do it, but you can't open the file because it is
>>> owned by the other person?
>>>
>>> Maybe you should simply change -fvtv-counts to take a file name, then
>>> we don't have to worry about any of this.
>>>
>> It's not quite that simple:  the -fvtv-counts flag actually causes two files
>> to be created; also there's another flag, -fvtv-debug that generates a third
>> file (i wanted a lot of information when I was working on and debugging this
>> feature).  Also if users are arbitrarily allowed to name the counts file
>> anything, the summing script program I wrote won't be able to find them.
>
> That doesn't seem like a compelling argument to me, since one could
> pass the file names to the summing script as well.
>
> As far as I can see, on a multi-user system, there is no reasonable
> alternative to permitting the user to specify the file names to use,
> or at least a directory where the files should be placed.  And if
> permit that, why not simply require it?
>
> Ian

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

Actually, I think that was dump_dir_name.

-- Caroline

On Tue, Aug 6, 2013 at 1:12 PM, Caroline Tice  wrote:
> I was talking with Diego, and he suggested the possibility of putting
> the log files in the same directory that the gcc dump files go, i.e.
> the one specified by dump_base_name.  Do you think that would be
> acceptable?
>
> -- Caroline Tice
> cmt...@google.com
>
> On Tue, Aug 6, 2013 at 12:12 PM, Ian Lance Taylor  wrote:
 On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice  wrote:
 >
 > On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor 
 > wrote:
 >
 >>The output to the file doesn't have
 >> any indication of what file is being compiled, so it will be ambiguous
 >> when run in parallel.
 >
 > You are mistaken.  It outputs one line to the log file for each
 > compilation
 > unit.  The output line begins with the name of the file that was being
 > compiled.  In my use case, I have used this to build a very large
 > system,
 > which resulted in something like at 8000 line log file of counts, which
 > I
 > then used my sum script on to see how the verifications were going.

 I was mistaken in detail but I'm not sure I was mistaken in principle.
 What happens if you are building the large system twice in different
 directories on the same machine?  Or, for that matter, if two
 different people are doing so?  Or if one person did it a while ago,
 and now you want to do it, but you can't open the file because it is
 owned by the other person?

 Maybe you should simply change -fvtv-counts to take a file name, then
 we don't have to worry about any of this.

>>> It's not quite that simple:  the -fvtv-counts flag actually causes two files
>>> to be created; also there's another flag, -fvtv-debug that generates a third
>>> file (i wanted a lot of information when I was working on and debugging this
>>> feature).  Also if users are arbitrarily allowed to name the counts file
>>> anything, the summing script program I wrote won't be able to find them.
>>
>> That doesn't seem like a compelling argument to me, since one could
>> pass the file names to the summing script as well.
>>
>> As far as I can see, on a multi-user system, there is no reasonable
>> alternative to permitting the user to specify the file names to use,
>> or at least a directory where the files should be placed.  And if
>> permit that, why not simply require it?
>>
>> Ian

Re: [PATCH/Merge Request] Vtable Verification feature.

[Ian Lance Taylor]

On Tue, Aug 6, 2013 at 1:12 PM, Caroline Tice  wrote:
> I was talking with Diego, and he suggested the possibility of putting
> the log files in the same directory that the gcc dump files go, i.e.
> the one specified by dump_base_name.  Do you think that would be
> acceptable?

Sure.

Ian

> On Tue, Aug 6, 2013 at 12:12 PM, Ian Lance Taylor  wrote:
 On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice  wrote:
 >
 > On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor 
 > wrote:
 >
 >>The output to the file doesn't have
 >> any indication of what file is being compiled, so it will be ambiguous
 >> when run in parallel.
 >
 > You are mistaken.  It outputs one line to the log file for each
 > compilation
 > unit.  The output line begins with the name of the file that was being
 > compiled.  In my use case, I have used this to build a very large
 > system,
 > which resulted in something like at 8000 line log file of counts, which
 > I
 > then used my sum script on to see how the verifications were going.

 I was mistaken in detail but I'm not sure I was mistaken in principle.
 What happens if you are building the large system twice in different
 directories on the same machine?  Or, for that matter, if two
 different people are doing so?  Or if one person did it a while ago,
 and now you want to do it, but you can't open the file because it is
 owned by the other person?

 Maybe you should simply change -fvtv-counts to take a file name, then
 we don't have to worry about any of this.

>>> It's not quite that simple:  the -fvtv-counts flag actually causes two files
>>> to be created; also there's another flag, -fvtv-debug that generates a third
>>> file (i wanted a lot of information when I was working on and debugging this
>>> feature).  Also if users are arbitrarily allowed to name the counts file
>>> anything, the summing script program I wrote won't be able to find them.
>>
>> That doesn't seem like a compelling argument to me, since one could
>> pass the file names to the summing script as well.
>>
>> As far as I can see, on a multi-user system, there is no reasonable
>> alternative to permitting the user to specify the file names to use,
>> or at least a directory where the files should be placed.  And if
>> permit that, why not simply require it?
>>
>> Ian

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Richard Henderson]

On 08/06/2013 06:06 AM, David Malcolm wrote:
> Index: gcc/config/epiphany/epiphany.h
> ===
> --- gcc/config/epiphany/epiphany.h(revision 201526)
> +++ gcc/config/epiphany/epiphany.h(working copy)
> @@ -929,6 +929,9 @@
>  };
>  
>  extern int epiphany_normal_fp_rounding;
> +
> +class rtl_opt_pass;
> +namespace gcc { class context; }
>  extern rtl_opt_pass *make_pass_mode_switch_use (gcc::context *ctxt);
>  extern rtl_opt_pass *make_pass_resolve_sw_modes (gcc::context *ctxt);

Looks like these definitions ought to go into coretypes.h.


r~

Re: [x86, PATCH] More effecient code for short unsigned conversion to float-point.

[Richard Henderson]

On 08/06/2013 04:33 AM, Yuri Rumyantsev wrote:
> -(define_expand "floatunssi2"
> +(define_expand "floatuns2"
>[(parallel
>   [(set (match_operand:X87MODEF 0 "register_operand")
>  (unsigned_float:X87MODEF
> -  (match_operand:SI 1 "nonimmediate_operand")))
> +  (match_operand:SWI124 1 "nonimmediate_operand")))
>(clobber (match_dup 2))
>(clobber (match_scratch:SI 3))])]
>"!TARGET_64BIT
> && ((TARGET_80387 && X87_ENABLE_FLOAT (mode, DImode)
>   && TARGET_SSE)
> -   || (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH))"
> +   || (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH))"
>  {
> -  if (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH)
> +  if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (SImode))
> +{ 
> +  operands[1] = convert_to_mode (SImode, operands[1], 1);
> +  emit_insn (gen_floatsi2 (operands[0], operands[1]));
> +  DONE;
> +}
> +  

The idea is correct, but there's no need to share the same expand,
adding a runtime test against the mode.


r~

Re: [buildbot] r201513: Invalid cast

[Oleg Endo]

On Tue, 2013-08-06 at 13:27 +0200, Jan-Benedict Glaw wrote:
> Hi!
> 
> On Mon, 2013-08-05 22:09:45 -, olege...@gcc.gnu.org 
>  wrote:
> > Author: olegendo
> > Date: Mon Aug  5 22:09:45 2013
> > New Revision: 201513
> > 
> > URL: http://gcc.gnu.org/viewcvs?rev=201513&root=gcc&view=rev
> > Log:
> > PR other/12081
> > * recog.h (rtx (*insn_gen_fn) (rtx, ...)): Replace typedef with new
> > class insn_gen_fn.
> > * expr.c (move_by_pieces_1, store_by_pieces_2): Replace argument
> > rtx (*) (rtx, ...) with insn_gen_fn.
> > * genoutput.c (output_insn_data): Cast gen_? function pointers to
> > insn_gen_fn::stored_funcptr.  Add initializer braces.
> 
> This probably caused in a powerpc64-linux build:
> 
> g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions 
> -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
> -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic 
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
> -DHAVE_CONFIG_H -I. -I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. 
> -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  
> -I../../../../gcc/gcc/../libdecnumber 
> -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
> -I../../../../gcc/gcc/../libbacktrace\
> ../../../../gcc/gcc/config/rs6000/rs6000.c -o rs6000.o
> In file included from ../../../../gcc/gcc/config/rs6000/rs6000.c:36:0:
> ../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void 
> rs6000_emit_swdiv(rtx, rtx, rtx, bool)’:
> ../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const 
> insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, 
> rtx_def*)}’
>  #define GEN_FCN(CODE) (insn_data[CODE].genfun)
>   ^
> ../../../../gcc/gcc/config/rs6000/rs6000.c:28145:43: note: in expansion of 
> macro ‘GEN_FCN’
>gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
>^
> ../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void 
> rs6000_emit_swrsqrt(rtx, rtx)’:
> ../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const 
> insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, 
> rtx_def*)}’
>  #define GEN_FCN(CODE) (insn_data[CODE].genfun)
>   ^
> ../../../../gcc/gcc/config/rs6000/rs6000.c:28223:43: note: in expansion of 
> macro ‘GEN_FCN’
>gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
>^
> 
> Other PPC/RS6k targets (ie. rs6000-ibm-aix4.3, ...) seem to be equally
> affected.

That gen_2arg_fn_t typedef looks strange.  In other places where GEN_FCN
is used in similar ways in rs6000.c, there's no cast to a function
pointer with an explicit number of arguments.  It's not clear to me what
it's trying to achieve there.

I've briefly tested the attached patch by building a powerpc-elf cross
compiler with make all-gcc and checking that it can produce some
assembly code.  Is this OK for trunk?

Speaking of GEN_FCN usage in rs6000.c.  The recently added HTM builtin
code has one interesting piece:

static rtx
htm_expand_builtin (tree exp, rtx target, bool * expandedp)
{
...
switch (nopnds)
  {
  case 0:
pat = GEN_FCN (icode) (NULL_RTX);
break;
  case 1:
pat = GEN_FCN (icode) (op[0]);
break;

The 'case 0' looks suspicious.  If the function behind the pointer
really is a zero-arg function this might or might not work depending on
the ABI.  I'm not sure what the intention here is.  I've compiled
gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c to see if it ever gets
into the 'case 0' but it doesn't.
The function 'htm_init_builtins' doesn't seem to handle a 'case 0'.  I'm
confused, somebody else should have a look at this please.

Cheers,
Oleg

gcc/ChangeLog:
PR other/12081
config/rs6000/rs6000.c (gen_2arg_fn_t): Remove typedef.
(rs6000_emit_swdiv, rs6000_emit_swrsqrt): Don't cast result of 
GEN_FCN to gen_2arg_fn_t.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c	(revision 201512)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -284,9 +284,6 @@
   { "rsqrtd",	 (RECIP_DF_RSQRT | RECIP_V2DF_RSQRT) },
 };
 
-/* 2 argument gen function typedef.  */
-typedef rtx (*gen_2arg_fn_t) (rtx, rtx, rtx);
-
 /* Pointer to function (in rs6000-c.c) that can define or undefine target
macros that have changed.  Languages that don't support the preprocessor
don't link in rs6000-c.c, so we can't call it directly.  */
@@ -28142,7 +28139,7 @@
 passes++;
 
   enum insn_code code = optab_handler (smul_optab, mode);
-  gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
+  insn_gen_fn gen_mul = GEN_FCN (code);
 
   gcc_assert (code != CODE_FOR_nothing);
 
@

Re: [buildbot] r201508: Build failures after pass C++ conversion

[David Malcolm]

On Tue, 2013-08-06 at 10:34 -1000, Richard Henderson wrote:
> On 08/06/2013 06:06 AM, David Malcolm wrote:
> > Index: gcc/config/epiphany/epiphany.h
> > ===
> > --- gcc/config/epiphany/epiphany.h  (revision 201526)
> > +++ gcc/config/epiphany/epiphany.h  (working copy)
> > @@ -929,6 +929,9 @@
> >  };
> >  
> >  extern int epiphany_normal_fp_rounding;
> > +
> > +class rtl_opt_pass;
> > +namespace gcc { class context; }
> >  extern rtl_opt_pass *make_pass_mode_switch_use (gcc::context *ctxt);
> >  extern rtl_opt_pass *make_pass_resolve_sw_modes (gcc::context *ctxt);
> 
> Looks like these definitions ought to go into coretypes.h.

Something like the attached, putting them in the
   #ifndef USED_FOR_TARGET
part?   (this passes the same light testing as on the first patch;
bootstrap of the first patch succeeded on x86_64; am now bootstrapping
the attached patch on x86_64).

commit f5d8960c711540d28be2fd923d65838cb88b34f5
Author: David Malcolm 
Date:   Tue Aug 6 12:41:31 2013 -0400

gcc/
	* coretypes.h (rtl_opt_pass): Add.
	(gcc::context): Add.
	* config/epiphany/epiphany.c (pass_mode_switch_use): New.
	(epiphany_init): Port to new C++ pass API.
	(epiphany_optimize_mode_switching): Likewise.
	* pass_manager.h (pass_manager::get_pass_split_all_insns): New.
	(pass_manager::get_pass_mode_switching): New.
	(pass_manager::get_pass_peephole2): New.
	* mode-switching.c (pass_mode_switching): Add clone method.
	* recog.c (pass_peephole2): Add clone method.
	(pass_split_all_insns): Add clone method.

diff --git a/gcc/config/epiphany/epiphany.c b/gcc/config/epiphany/epiphany.c
index 1dcdc4b..5d020bb 100644
--- a/gcc/config/epiphany/epiphany.c
+++ b/gcc/config/epiphany/epiphany.c
@@ -45,6 +45,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "ggc.h"
 #include "tm-constrs.h"
 #include "tree-pass.h"	/* for current_pass */
+#include "context.h"
+#include "pass_manager.h"
 
 /* Which cpu we're compiling for.  */
 int epiphany_cpu_type;
@@ -59,6 +61,9 @@ char epiphany_punct_chars[256];
 /* The rounding mode that we generally use for floating point.  */
 int epiphany_normal_fp_rounding;
 
+/* The pass instance, for use in epiphany_optimize_mode_switching. */
+static opt_pass *pass_mode_switch_use;
+
 static void epiphany_init_reg_tables (void);
 static int get_epiphany_condition_code (rtx);
 static tree epiphany_handle_interrupt_attribute (tree *, tree, tree, int, bool *);
@@ -165,20 +170,24 @@ epiphany_init (void)
  pass because of the side offect of epiphany_mode_needed on
  MACHINE_FUNCTION(cfun)->unknown_mode_uses.  But it must run before
  pass_resolve_sw_modes.  */
-  static struct register_pass_info insert_use_info
-= { &pass_mode_switch_use.pass, "mode_sw",
+  pass_mode_switch_use = make_pass_mode_switch_use (g);
+  struct register_pass_info insert_use_info
+= { pass_mode_switch_use, "mode_sw",
 	1, PASS_POS_INSERT_AFTER
   };
-  static struct register_pass_info mode_sw2_info
-= { &pass_mode_switching.pass, "mode_sw",
+  opt_pass *mode_sw2 = g->get_passes()->get_pass_mode_switching()->clone ();
+  struct register_pass_info mode_sw2_info
+= { mode_sw2, "mode_sw",
 	1, PASS_POS_INSERT_AFTER
   };
-  static struct register_pass_info mode_sw3_info
-= { &pass_resolve_sw_modes.pass, "mode_sw",
+  opt_pass *mode_sw3 = make_pass_resolve_sw_modes (g);
+  struct register_pass_info mode_sw3_info
+= { mode_sw3, "mode_sw",
 	1, PASS_POS_INSERT_AFTER
   };
-  static struct register_pass_info mode_sw4_info
-= { &pass_split_all_insns.pass, "mode_sw",
+  opt_pass *mode_sw4 = g->get_passes()->get_pass_split_all_insns()->clone ();
+  struct register_pass_info mode_sw4_info
+= { mode_sw4, "mode_sw",
 	1, PASS_POS_INSERT_AFTER
   };
   static const int num_modes[] = NUM_MODES_FOR_MODE_SWITCHING;
@@ -205,8 +214,10 @@ epiphany_init (void)
  (see http://gcc.gnu.org/ml/gcc-patches/2011-10/msg02819.html,)
  we need a second peephole2 pass to get reasonable code.  */
   {
-static struct register_pass_info peep2_2_info
-  = { &pass_peephole2.pass, "peephole2",
+opt_pass *extra_peephole2
+  = g->get_passes ()->get_pass_peephole2()->clone ();
+struct register_pass_info peep2_2_info
+  = { extra_peephole2, "peephole2",
 	  1, PASS_POS_INSERT_AFTER
 	};
 
@@ -2256,7 +2267,7 @@ epiphany_optimize_mode_switching (int entity)
   return (MACHINE_FUNCTION (cfun)->sw_entities_processed
 	  & (1 << EPIPHANY_MSW_ENTITY_ROUND_UNKNOWN)) != 0;
 case EPIPHANY_MSW_ENTITY_FPU_OMNIBUS:
-  return optimize == 0 || current_pass == &pass_mode_switch_use.pass;
+  return optimize == 0 || current_pass == pass_mode_switch_use;
 }
   gcc_unreachable ();
 }
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index edb9c8c..54bfe7f 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -169,6 +169,12 @@ typedef const struct basic_block_d

Re: Request to merge Undefined Behavior Sanitizer in (take 2)

[Jason Merrill]

On 08/05/2013 07:24 AM, Marek Polacek wrote:

On Sat, Aug 03, 2013 at 12:24:32PM -0400, Jason Merrill wrote:

Where are the SAVE_EXPRs coming from?  It doesn't seem to me that x
needs to be wrapped in a SAVE_EXPR at all in this case.  For cases
where the SAVE_EXPR is needed and not used in the test, you could
add the SAVE_EXPR before the condition with a COMPOUND_EXPR.


Those SAVE_EXPRs are coming from c-typeck.c in this case:

   if (flag_sanitize & SANITIZE_UNDEFINED
   && current_function_decl != 0
   && (doing_div_or_mod || doing_shift))
 {
   /* OP0 and/or OP1 might have side-effects.  */
   op0 = c_save_expr (op0);
   op1 = c_save_expr (op1);


Yes, but why do we need to wrap op0 in a save_expr?  That's only 
necessary if we're going to be evaluating it twice on the same path, but 
here it's only evaluated once on each path.


Jason

RFC/A: PR 58079: CONST_INT mode assert in combine.c:do_SUBST

[Richard Sandiford]

PR 58079 is about the do_SUBST assert:

  /* Sanity check that we're replacing oldval with a CONST_INT
 that is a valid sign-extension for the original mode.  */
  gcc_assert (INTVAL (newval)
  == trunc_int_for_mode (INTVAL (newval), GET_MODE (oldval)));

triggering while trying to optimise the temporary result:

  (eq (const_int -73 [0xffb7])
  (reg:QI 234 [ MEM[(const u8 *)buf_7(D) + 4B] ])

combine_simplify_rtx calls simplify_comparison, which first canonicalises
the order so that the constant is second and then promotes the comparison
to SImode (the first supported comparison mode on MIPS).  So we end up with:

  (eq (zero_extend:SI (reg:QI 234 [ MEM[(const u8 *)buf_7(D) + 4B] ]))
  (const_int 183 [0xb7]))

So far so good.  But combine_simplify_rtx then tries to install the
new operands in-place, with the second part of:

  /* If the code changed, return a whole new comparison.  */
  if (new_code != code)
return gen_rtx_fmt_ee (new_code, mode, op0, op1);

  /* Otherwise, keep this operation, but maybe change its operands.
 This also converts (ne (compare FOO BAR) 0) to (ne FOO BAR).  */
  SUBST (XEXP (x, 0), op0);
  SUBST (XEXP (x, 1), op1);

And this triggers the assert because we're replacing a QImode register
with the non-QImode constant 183.

One fix would be to extend the new_code != code condition, as below.
This should avoid the problem cases without generating too much
garbage rtl.  But if the condition's getting this complicated,
perhaps it'd be better just to avoid SUBST here?  I.e.

  if (new_code != code || op0 != XEXP (x, 0) || op1 != XEXP (x, 1))
return gen_rtx_fmt_ee (new_code, mode, op0, op1);

Just asking though. :-)

Tested on mipsisa64-elf.  OK to install?

Thanks,
Richard


gcc/
PR rtl-optimization/58079
* combine.c (combine_simplify_rtx): Avoid using SUBST if
simplify_comparison has widened a comparison with an integer.

gcc/testsuite/
* gcc.dg/torture/pr58079.c: New test.

Index: gcc/combine.c
===
--- gcc/combine.c   2013-08-06 22:03:32.644642305 +0100
+++ gcc/combine.c   2013-08-06 22:08:51.944653901 +0100
@@ -5803,8 +5803,15 @@ combine_simplify_rtx (rtx x, enum machin
return x;
}
 
- /* If the code changed, return a whole new comparison.  */
- if (new_code != code)
+ /* If the code changed, return a whole new comparison.
+We also need to avoid using SUBST in cases where
+simplify_comparison has widened a comparison with a CONST_INT,
+since in that case the wider CONST_INT may fail the sanity
+checks in do_SUBST.  */
+ if (new_code != code
+ || (CONST_INT_P (op1)
+ && GET_MODE (op0) != GET_MODE (XEXP (x, 0))
+ && GET_MODE (op0) != GET_MODE (XEXP (x, 1
return gen_rtx_fmt_ee (new_code, mode, op0, op1);
 
  /* Otherwise, keep this operation, but maybe change its operands.
Index: gcc/testsuite/gcc.dg/torture/pr58079.c
===
--- /dev/null   2013-07-23 18:41:43.074412242 +0100
+++ gcc/testsuite/gcc.dg/torture/pr58079.c  2013-08-06 22:05:06.249523398 
+0100
@@ -0,0 +1,107 @@
+/* { dg-options "-mlong-calls" { target mips*-*-* } } */
+
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned int __kernel_size_t;
+typedef __kernel_size_t size_t;
+struct list_head {
+ struct list_head *next;
+};
+
+struct dmx_ts_feed {
+ int is_filtering;
+};
+struct dmx_section_feed {
+ u16 secbufp;
+ u16 seclen;
+ u16 tsfeedp;
+};
+
+typedef int (*dmx_ts_cb) (
+   const u8 * buffer1,
+  size_t buffer1_length,
+  const u8 * buffer2,
+  size_t buffer2_length
+);
+
+struct dvb_demux_feed {
+ union {
+  struct dmx_ts_feed ts;
+  struct dmx_section_feed sec;
+ } feed;
+ union {
+  dmx_ts_cb ts;
+ } cb;
+ int type;
+ u16 pid;
+ int ts_type;
+ struct list_head list_head;
+};
+
+struct dvb_demux {
+ int (*stop_feed)(struct dvb_demux_feed *feed);
+ struct list_head feed_list;
+};
+
+
+static
+inline
+__attribute__((always_inline))
+u8
+payload(const u8 *tsp)
+{
+ if (tsp[3] & 0x20) {
+   return 184 - 1 - tsp[4];
+ }
+ return 184;
+}
+
+static
+inline
+__attribute__((always_inline))
+int
+dvb_dmx_swfilter_payload(struct dvb_demux_feed *feed, const u8 *buf)
+{
+ int count = payload(buf);
+ int p;
+ if (count == 0)
+  return -1;
+ return feed->cb.ts(&buf[p], count, ((void *)0), 0);
+}
+
+static
+inline
+__attribute__((always_inline))
+void
+dvb_dmx_swfilter_packet_type(struct dvb_demux_feed *feed, const u8 *buf)
+{
+ switch (feed->type) {
+ case 0:
+  if (feed->ts_type & 1) {
+dvb_dmx_swfilter_payload(feed, buf);
+  }
+  if (dvb_dmx_swfilter_section_packet(feed, buf) < 0)
+   feed->feed.sec.seclen = feed->fe

Re: [PATCH/Merge Request] Vtable Verification feature.

[Benjamin De Kosnik]

> > +# Filter out unsupported systems.
> > +case "${target}" in
> > +  x86_64-*-linux* | i?86-*-linux*)
> > + VTV_SUPPORTED=yes
> > + ;;
> > +  powerpc*-*-linux*)
> > + ;;
> > +  sparc*-*-linux*)
> > + ;;
> > +  arm*-*-linux*)
> > + ;;  
> 
> What about powerpc, sparc and arm?  Why are they mentioned here if no
> actual decision is made about support?

This is more a practical consideration: it's the middle of summer
break. Let's error on the side of caution for the moment, and get
this in causing minimal disruption on a convenient platform that I can
verify myself easily.

On a practical note, the libsanitizer acceptance
criteria was/is as above, seems sensible to do the same thing with
libvtv. 

Once this is in trunk, let a million flowers bloom! There is no 
reason specific platform/target maintainers can't enable it at their
leisure on a per-setup manner and when they can verify testresults
easily.

-benjamin

Re: [PATCH/Merge Request] Vtable Verification feature.

[Diego Novillo]

On Tue, Aug 6, 2013 at 2:39 PM, Benjamin De Kosnik  wrote:
>
>> > +# Filter out unsupported systems.
>> > +case "${target}" in
>> > +  x86_64-*-linux* | i?86-*-linux*)
>> > + VTV_SUPPORTED=yes
>> > + ;;
>> > +  powerpc*-*-linux*)
>> > + ;;
>> > +  sparc*-*-linux*)
>> > + ;;
>> > +  arm*-*-linux*)
>> > + ;;
>>
>> What about powerpc, sparc and arm?  Why are they mentioned here if no
>> actual decision is made about support?
>
> This is more a practical consideration: it's the middle of summer
> break. Let's error on the side of caution for the moment, and get
> this in causing minimal disruption on a convenient platform that I can
> verify myself easily.
>
> On a practical note, the libsanitizer acceptance
> criteria was/is as above, seems sensible to do the same thing with
> libvtv.
>
> Once this is in trunk, let a million flowers bloom! There is no
> reason specific platform/target maintainers can't enable it at their
> leisure on a per-setup manner and when they can verify testresults
> easily.

Agreed.  The comments I had have been already addressed by Caroline,
AFAICT.  Once she has that, the patch can go in.


Diego.

Re: [PATCH] Fix illegal cast to rtx (*insn_gen_fn) (rtx, ...)

[Oleg Endo]

On Mon, 2013-08-05 at 13:25 -1000, Richard Henderson wrote:
> On 08/05/2013 12:32 PM, Oleg Endo wrote:
> > Thanks, committed as rev 201513.
> > 4.8 also has the same problem.  The patch applies on 4.8 branch without
> > problems and make all-gcc works.
> > OK for 4.8, too?
> 
> Hum.  I suppose so, since it's relatively self-contained.  I suppose the
> out-of-tree openrisc port will thank us...

Maybe it's better to wait for a while and collect follow up patches such
as the rs6000 one.

Cheers,
Oleg

Re: [PATCH/Merge Request] Vtable Verification feature.

[Caroline Tice]

I have made all the requested changes.  I am in the process of
bootstrapping and running the regressions one last time.  Assuming the
bootstrapping & regression tests pass, is this ok to commit?

-- Caroline
cmt...@google.com


On Tue, Aug 6, 2013 at 2:45 PM, Diego Novillo  wrote:
> On Tue, Aug 6, 2013 at 2:39 PM, Benjamin De Kosnik  wrote:
>>
>>> > +# Filter out unsupported systems.
>>> > +case "${target}" in
>>> > +  x86_64-*-linux* | i?86-*-linux*)
>>> > + VTV_SUPPORTED=yes
>>> > + ;;
>>> > +  powerpc*-*-linux*)
>>> > + ;;
>>> > +  sparc*-*-linux*)
>>> > + ;;
>>> > +  arm*-*-linux*)
>>> > + ;;
>>>
>>> What about powerpc, sparc and arm?  Why are they mentioned here if no
>>> actual decision is made about support?
>>
>> This is more a practical consideration: it's the middle of summer
>> break. Let's error on the side of caution for the moment, and get
>> this in causing minimal disruption on a convenient platform that I can
>> verify myself easily.
>>
>> On a practical note, the libsanitizer acceptance
>> criteria was/is as above, seems sensible to do the same thing with
>> libvtv.
>>
>> Once this is in trunk, let a million flowers bloom! There is no
>> reason specific platform/target maintainers can't enable it at their
>> leisure on a per-setup manner and when they can verify testresults
>> easily.
>
> Agreed.  The comments I had have been already addressed by Caroline,
> AFAICT.  Once she has that, the patch can go in.
>
>
> Diego.

Re: [PATCH/Merge Request] Vtable Verification feature.

[Diego Novillo]

On Tue, Aug 6, 2013 at 2:50 PM, Caroline Tice  wrote:
> I have made all the requested changes.  I am in the process of
> bootstrapping and running the regressions one last time.  Assuming the
> bootstrapping & regression tests pass, is this ok to commit?

Sure.


Diego.

Re: [PATCH 05/11] Add -fno-rtti when building plugins.

[Mike Stump]

Ok.

On Aug 6, 2013, at 3:43 AM, Dominique Dhumieres  wrote:
>> Hence plugins that create passes will need to be built with RTTI
>> disabled in order to link against gcc, or they will fail to load, with
>> an error like: ...
> 
> The same holds for darwin, hence the following patch is needed:
> 
> --- ../_clean/gcc/testsuite/lib/plugin-support.exp2013-08-05 
> 22:51:42.0 +0200
> +++ gcc/testsuite/lib/plugin-support.exp  2013-08-06 12:36:43.0 
> +0200
> @@ -101,7 +101,7 @@ proc plugin-test-execute { plugin_src pl
>   set optstr [concat $optstr " $op"]
>   }
>   }
> - set optstr [concat $optstr "-DIN_GCC -fPIC -shared -undefined 
> dynamic_lookup"]
> + set optstr [concat $optstr "-DIN_GCC -fPIC -shared -fno-rtti -undefined 
> dynamic_lookup"]
> } else {
>   set plug_cflags $PLUGINCFLAGS 
>   set optstr "$includes $extra_flags -DIN_GCC -fPIC -shared -fno-rtti"
> 
> 

Re: [C++ Patch / RFC] PR 46206

[Jason Merrill]

On 08/06/2013 05:26 AM, Paolo Carlini wrote:

That's strange.  I would expect that to mean that we don't properly
give an error for a Bar data member declared after the typedef.



You mean something like this?

class Foo
{
   int u, v, w;//, x;
   typedef struct Bar { } Bar;
   Bar bar;
   virtual void foo(void) {
 struct Bar bar;
   }
};


I mean something like

class Foo
{
  int u, v, w;//, x;
  typedef struct Bar { } Bar;
  int Bar;
  virtual void foo(void) {
struct Bar bar;
  }
};

Re: [C++ Patch / RFC] PR 46206

[Paolo Carlini]

Hi,

On 08/06/2013 11:19 PM, Jason Merrill wrote:

I mean something like

class Foo
{
  int u, v, w;//, x;
  typedef struct Bar { } Bar;
  int Bar;
  virtual void foo(void) {
struct Bar bar;
  }
};
Ah I see, thanks. We reject this before and after the patch. Shall I add 
this variant to the new testcase?


Thanks,
Paolo.

Re: [buildbot] r201508: Build failures after pass C++ conversion

[Richard Henderson]

On 08/06/2013 11:23 AM, David Malcolm wrote:
> gcc/
>   * coretypes.h (rtl_opt_pass): Add.
>   (gcc::context): Add.
>   * config/epiphany/epiphany.c (pass_mode_switch_use): New.
>   (epiphany_init): Port to new C++ pass API.
>   (epiphany_optimize_mode_switching): Likewise.
>   * pass_manager.h (pass_manager::get_pass_split_all_insns): New.
>   (pass_manager::get_pass_mode_switching): New.
>   (pass_manager::get_pass_peephole2): New.
>   * mode-switching.c (pass_mode_switching): Add clone method.
>   * recog.c (pass_peephole2): Add clone method.
>   (pass_split_all_insns): Add clone method.

Ok.


r~

Re: Request to merge Undefined Behavior Sanitizer in (take 2)

[Jason Merrill]

On 08/06/2013 10:42 AM, Marek Polacek wrote:

Hm, actually, we can't easily fold the call to the sanitize function
away, I'm afraid, if we want to do it for the 'case '
case.  When we hit the DIV_EXPR in 'case 0 * (1 / 0)',
the ubsan_instrument_division gets 1 as a first argument and 0 as
a second argument, but due to fold_builds in the
ubsan_instrument_division, we replace the case value with just the call
to the __builtin___ubsan_handle_divrem_overflow.


Ah, and the call isn't folded away because it has side-effects.


I think, what we could do, is to tweak verify_constant like this:

+  /* This is to handle e.g. the goofy 'case 0 * (1 / 0)' case.  */
+  if (flag_sanitize & SANITIZE_UNDEFINED
+  && TREE_CODE (t) == CALL_EXPR
+  && is_ubsan_builtin (t))
+{
+  error ("undefined behavior occured");
+  return *non_constant_p;
+}


I think I'd rather handle ubsan builtins specially in dump_expr.

Jason

Re: [C++ Patch / RFC] PR 46206

[Jason Merrill]

On 08/06/2013 06:14 PM, Paolo Carlini wrote:

Ah I see, thanks. We reject this before and after the patch. Shall I add
this variant to the new testcase?


Sure.

Jason

Re: [buildbot] r201513: Invalid cast

[Richard Henderson]

On 08/06/2013 11:20 AM, Oleg Endo wrote:
>   PR other/12081
>   config/rs6000/rs6000.c (gen_2arg_fn_t): Remove typedef.
>   (rs6000_emit_swdiv, rs6000_emit_swrsqrt): Don't cast result of 
>   GEN_FCN to gen_2arg_fn_t.

Ok.


r~

Re: PPC64 HTM support (was [buildbot] r201513: Invalid cast)

[David Edelsohn]

Peter,

Would you please help answer Oleg questions about case 0 in
htm_expand_builtin in rs6000.c?

Thanks, David

Speaking of GEN_FCN usage in rs6000.c.  The recently added HTM builtin
code has one interesting piece:

static rtx
htm_expand_builtin (tree exp, rtx target, bool * expandedp)
{
...
switch (nopnds)
 {
 case 0:
   pat = GEN_FCN (icode) (NULL_RTX);
   break;
 case 1:
   pat = GEN_FCN (icode) (op[0]);
   break;

The 'case 0' looks suspicious.  If the function behind the pointer
really is a zero-arg function this might or might not work depending on
the ABI.  I'm not sure what the intention here is.  I've compiled
gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c to see if it ever gets
into the 'case 0' but it doesn't.
The function 'htm_init_builtins' doesn't seem to handle a 'case 0'.  I'm
confused, somebody else should have a look at this please.

Re: [rl78]: Port to new pass C++ API (was Re: [buildbot] r201508: Build failures after pass C++ conversion)

[DJ Delorie]

Go for it.  The static "212" is not required as long as the pass shows
up in the right spot in the pass sequence.

Re: [patch, ARM] require 64-bit hw-int for all arm targets

[Joseph S. Myers]

On Tue, 30 Jul 2013, Richard Earnshaw wrote:

> Most arm target configs now require a 64-bit HW-int.  Unfortunately a few of
> the older, less commonly used config targets do not.  The code in arm.c now
> pretty much requires that a 64-bit HW-int is used, especially for
> vectorization to work.
> 
> This patch makes 64-bit HW-int the default for all arm configs and cleans up
> the configure script accordingly.
> 
>   * config.gcc (arm): Require 64-bit host-wide-int for all ARM
>   target configs.

The configuration in libcpp/configure.ac of which targets need 64-bit 
HOST_WIDE_INT always needs to be kept in sync with that in gcc/config.gcc.

-- 
Joseph S. Myers
jos...@codesourcery.com

C++ PATCH for c++/57825 (ref-qualifier lost on template argument)

[Jason Merrill]

Simple oversight.

Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit e468033b49ddb6cadfd3c07d3ac0b81d80bc523a
Author: Jason Merrill 
Date:   Tue Aug 6 19:49:40 2013 -0400

	PR c++/57825
	* tree.c (strip_typedefs) [METHOD_TYPE]: Preserve ref-qualifier.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index f8b4bbc..63ec7fa 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1236,6 +1236,8 @@ strip_typedefs (tree t)
 	result =
 	  build_method_type_directly (class_type, type,
 	  TREE_CHAIN (arg_types));
+	result
+	  = build_ref_qualified_type (result, type_memfn_rqual (t));
 	  }
 	else
 	  {
diff --git a/gcc/testsuite/g++.dg/cpp0x/ref-qual14.C b/gcc/testsuite/g++.dg/cpp0x/ref-qual14.C
new file mode 100644
index 000..8e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/ref-qual14.C
@@ -0,0 +1,18 @@
+// PR c++/57825
+// { dg-do compile { target c++11 } }
+
+template
+struct target_class
+{};
+
+template
+struct target_class
+{};
+
+template
+struct target_class
+{};
+
+template
+struct target_class
+{};

Re: [buildbot] r201508: Build failures after pass C++ conversion

[David Malcolm]

On Tue, 2013-08-06 at 12:49 -1000, Richard Henderson wrote:
> On 08/06/2013 11:23 AM, David Malcolm wrote:
> > gcc/
> > * coretypes.h (rtl_opt_pass): Add.
> > (gcc::context): Add.
> > * config/epiphany/epiphany.c (pass_mode_switch_use): New.
> > (epiphany_init): Port to new C++ pass API.
> > (epiphany_optimize_mode_switching): Likewise.
> > * pass_manager.h (pass_manager::get_pass_split_all_insns): New.
> > (pass_manager::get_pass_mode_switching): New.
> > (pass_manager::get_pass_peephole2): New.
> > * mode-switching.c (pass_mode_switching): Add clone method.
> > * recog.c (pass_peephole2): Add clone method.
> > (pass_split_all_insns): Add clone method.
> 
> Ok.
Thanks.

Committed to trunk as r201549 (I saw some missing spaces between parens
after a successful boostrap/test on x86_64, which I fixed, and then
verified a clean build with --target=epiphany-elf before committing).

Re: [PATCH 05/11] Add -fno-rtti when building plugins.

[David Malcolm]

Thanks; I've committed it to trunk as r201552.

On Tue, 2013-08-06 at 14:53 -0700, Mike Stump wrote:
> Ok.
> 
> On Aug 6, 2013, at 3:43 AM, Dominique Dhumieres  wrote:
> >> Hence plugins that create passes will need to be built with RTTI
> >> disabled in order to link against gcc, or they will fail to load, with
> >> an error like: ...
> > 
> > The same holds for darwin, hence the following patch is needed:
> > 
> > --- ../_clean/gcc/testsuite/lib/plugin-support.exp  2013-08-05 
> > 22:51:42.0 +0200
> > +++ gcc/testsuite/lib/plugin-support.exp2013-08-06 12:36:43.0 
> > +0200
> > @@ -101,7 +101,7 @@ proc plugin-test-execute { plugin_src pl
> > set optstr [concat $optstr " $op"]
> > }
> > }
> > -   set optstr [concat $optstr "-DIN_GCC -fPIC -shared -undefined 
> > dynamic_lookup"]
> > +   set optstr [concat $optstr "-DIN_GCC -fPIC -shared -fno-rtti -undefined 
> > dynamic_lookup"]
> > } else {
> > set plug_cflags $PLUGINCFLAGS 
> > set optstr "$includes $extra_flags -DIN_GCC -fPIC -shared -fno-rtti"
> > 
> > 

Re: [rl78]: Port to new pass C++ API (was Re: [buildbot] r201508: Build failures after pass C++ conversion)

[David Malcolm]

On Tue, 2013-08-06 at 19:53 -0400, DJ Delorie wrote:
> Go for it.  The static "212" is not required as long as the pass shows
> up in the right spot in the pass sequence.
Thanks.  I checked, and both with and without the hardcoded 212, the
dumpfile for the pass for a foo.c is currently:
  foo.c.246r.devirt
i.e. 246, not 212.

I've committed it to trunk (without the 212) as r201553 (having checked
the build of stage1 and a smoketest).

Re: PPC64 HTM support (was [buildbot] r201513: Invalid cast)

[Peter Bergner]

Oleg Endo wrote:
> Speaking of GEN_FCN usage in rs6000.c.  The recently added HTM builtin
> code has one interesting piece:
> 
> static rtx
> htm_expand_builtin (tree exp, rtx target, bool * expandedp)
> {
> ...
> switch (nopnds)
>  {
>  case 0:
>pat = GEN_FCN (icode) (NULL_RTX);
>break;
>  case 1:
>pat = GEN_FCN (icode) (op[0]);
>break;
> 
> The 'case 0' looks suspicious.

Yes, the "case 0:" is not needed.  It was needed at one point
when I originally modeled this code after Andreas's s390 changes.
The code then passed in the target rtx as a separate parameter
from the operands, so nopnds was actually zero in some cases.
I realized I could simplify the code a lot by placing the
target rtx into the op[] array along with the source operands
and at that point, the "case 0:" statements became dead code
as you discovered.  I'll bootstrap and regtest the change
below and commit it as an obvious fix when that is done.

Thanks for spotting this.

Peter


Index: rs6000.c
===
--- rs6000.c(revision 201409)
+++ rs6000.c(working copy)
@@ -11148,9 +11148,6 @@ htm_expand_builtin (tree exp, rtx target
 
switch (nopnds)
  {
- case 0:
-   pat = GEN_FCN (icode) (NULL_RTX);
-   break;
  case 1:
pat = GEN_FCN (icode) (op[0]);
break;

Re: [PATCH] Convert more passes to new dump framework

[Teresa Johnson]

On Tue, Aug 6, 2013 at 9:29 AM, Teresa Johnson  wrote:
> On Tue, Aug 6, 2013 at 9:01 AM, Martin Jambor  wrote:
>> Hi,
>>
>> On Tue, Aug 06, 2013 at 07:14:42AM -0700, Teresa Johnson wrote:
>>> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor  wrote:
>>> > On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>>> >> This patch ports messages to the new dump framework,
>>> >
>>> > It would be great this new framework was documented somewhere.  I lost
>>> > track of what was agreed it would be and from the uses in the
>>> > vectorizer I was never quite sure how to utilize it in other passes.
>>>
>>> Cc'ing Sharad who implemented this - Sharad, is this documented on a
>>> wiki or elsewhere?
>>
>> Thanks
>>
>>>
>>> >
>>> > I'd also like to point out two other minor things inline:
>>> >
>>> > [...]
>>> >
>>> >> 2013-08-06  Teresa Johnson  
>>> >> Dehao Chen  
>>> >>
>>> >> * dumpfile.c (dump_loc): Add column number to output, make 
>>> >> newlines
>>> >> consistent.
>>> >> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>>> >> * ipa-inline-transform.c (clone_inlined_nodes):
>>> >> (cgraph_node_opt_info): New function.
>>> >> (cgraph_node_call_chain): Ditto.
>>> >> (dump_inline_decision): Ditto.
>>> >> (inline_call): Invoke dump_inline_decision.
>>> >> * doc/invoke.texi: Document optall -fopt-info flag.
>>> >> * profile.c (read_profile_edge_counts): Use new dump framework.
>>> >> (compute_branch_probabilities): Ditto.
>>> >> * passes.c (pass_manager::register_one_dump_file): Use 
>>> >> OPTGROUP_OTHER
>>> >> when pass not in any opt group.
>>> >> * value-prof.c (check_counter): Use new dump framework.
>>> >> (find_func_by_funcdef_no): Ditto.
>>> >> (check_ic_target): Ditto.
>>> >> * coverage.c (get_coverage_counts): Ditto.
>>> >> (coverage_init): Setup new dump framework.
>>> >> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>>> >> * ipa-inline.h (is_in_ipa_inline): Declare.
>>> >>
>>> >> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>>> >> * testsuite/gcc.dg/pr26570.c: Ditto.
>>> >> * testsuite/gcc.dg/pr32773.c: Ditto.
>>> >> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>> >>
>>> >
>>> > [...]
>>> >
>>> >> Index: ipa-inline-transform.c
>>> >> ===
>>> >> --- ipa-inline-transform.c  (revision 201461)
>>> >> +++ ipa-inline-transform.c  (working copy)
>>> >> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>>> >>  }
>>> >>
>>> >>
>>> >> +#define MAX_INT_LENGTH 20
>>> >> +
>>> >> +/* Return NODE's name and profile count, if available.  */
>>> >> +
>>> >> +static const char *
>>> >> +cgraph_node_opt_info (struct cgraph_node *node)
>>> >> +{
>>> >> +  char *buf;
>>> >> +  size_t buf_size;
>>> >> +  const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>>> >> +
>>> >> +  if (!bfd_name)
>>> >> +bfd_name = "unknown";
>>> >> +
>>> >> +  buf_size = strlen (bfd_name) + 1;
>>> >> +  if (profile_info)
>>> >> +buf_size += (MAX_INT_LENGTH + 3);
>>> >> +
>>> >> +  buf = (char *) xmalloc (buf_size);
>>> >> +
>>> >> +  strcpy (buf, bfd_name);
>>> >> +
>>> >> +  if (profile_info)
>>> >> +sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>>> >> +  return buf;
>>> >> +}
>>> >
>>> > I'm not sure if output of this function is aimed only at the user or
>>> > if it is supposed to be used by gcc developers as well.  If the
>>> > latter, an incredibly useful thing is to also dump node->symbol.order
>>> > too.  We usually dump it after "/" sign separating it from node name.
>>> > It is invaluable when examining decisions in C++ code where you can
>>> > have lots of clones of a node (and also because existing dumps print
>>> > it, it is easy to combine them).
>>>
>>> The output is useful for both power users doing performance tuning of
>>> their application, and by gcc developers. Adding the id is not so
>>> useful for the former, but I agree that it is very useful for compiler
>>> developers. In fact, in the google branch version we emit more verbose
>>> information (the lipo module id and the funcdef_no) to help uniquely
>>> identify the routines and to aid in post-processing by humans and
>>> tools. So it is probably useful to add something similar here too. Is
>>> the node->symbol.order more or less unique than the funcdef_no? I see
>>> that you added a patch a few months ago to print the
>>> node->symbol.order in the function header, and it also has the
>>> advantage as you note of matching up with existing ipa dumps.
>>
>> node->symbol.order is unique and if I remember correctly, it is not
>> even recycled.  Clones, inline clones, thunks, every symbol table node
>> gets its own symbol order so it should be more unique than funcdef_no.
>> On