Re: Using gen_int_mode instead of GEN_INT minor testsuite fallout on MIPS

[Richard Sandiford]

Mike Stump  writes:
> Here is the patch to add precision to partial int modes.  I included
> switching over the rs6000 port to the new scheme so one can get a feel
> for that it would look like.
>
> I could trivially convert the remaining ports:
>
> config/bfin/bfin-modes.def:PARTIAL_INT_MODE (DI);
> config/m32c/m32c-modes.def:PARTIAL_INT_MODE (SI);
> config/msp430/msp430-modes.def:PARTIAL_INT_MODE (SI);
> config/rs6000/rs6000-modes.def:PARTIAL_INT_MODE_N (TI, 128);
> config/sh/sh-modes.def:PARTIAL_INT_MODE (SI);
> config/sh/sh-modes.def:PARTIAL_INT_MODE (DI);
>
> to the new scheme so that _all_ ports would have a precision with all
> partial int modes.  The newest port that I had not audited for how they
> use it (msp430):
>
> /* 20-bit address */
> PARTIAL_INT_MODE (SI);
>
> So, this, trivially would be SI, 20… I would define PARTIAL_INT_MODE
> with two parameters and not define PARTIAL_INT_MODE_N, if we did it for
> real.  No need for partial conversions here, too easy to convert things.

How about a three-operand version: the name of the partial mode,
the name of the memory mode, and the precision?  "PTI" and "P128TI"
are a bit confusing for the (hacky) powerpc usage, since 128 bits
isn't partial.  With three operands we could pick a more descriptive name.

Thanks,
Richard

[v3] More noexcept for vectors

[Marc Glisse]

Hello,

this patch passes bootstrap+testsuite. The guarantees given by the 
standard on allocators are a bit weird, but I see there is already DR2016 
taking care of it.


2013-09-14  Marc Glisse  

PR libstdc++/58338
* include/bits/stl_vector.h
(_Vector_impl::_Vector_impl(_Tp_alloc_type const&),
_Vector_impl::_Vector_impl(_Tp_alloc_type&&),
_Vector_impl::_M_swap_data,
_Vector_base::_Vector_base(const allocator_type&),
_Vector_base::_Vector_base(allocator_type&&),
_Vector_base::_Vector_base(_Vector_base&&),
vector::vector(const allocator_type&), vector::operator[],
vector::operator[] const, vector::front, vector::front const,
vector::back, vector::back const, vector::pop_back,
vector::_M_erase_at_end): Mark as noexcept.
(vector::~vector): Remove useless noexcept.


--
Marc GlisseIndex: include/bits/stl_vector.h
===
--- include/bits/stl_vector.h   (revision 202588)
+++ include/bits/stl_vector.h   (working copy)
@@ -80,32 +80,32 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   : public _Tp_alloc_type
   {
pointer _M_start;
pointer _M_finish;
pointer _M_end_of_storage;
 
_Vector_impl()
: _Tp_alloc_type(), _M_start(0), _M_finish(0), _M_end_of_storage(0)
{ }
 
-   _Vector_impl(_Tp_alloc_type const& __a)
+   _Vector_impl(_Tp_alloc_type const& __a) _GLIBCXX_NOEXCEPT
: _Tp_alloc_type(__a), _M_start(0), _M_finish(0), _M_end_of_storage(0)
{ }
 
 #if __cplusplus >= 201103L
-   _Vector_impl(_Tp_alloc_type&& __a)
+   _Vector_impl(_Tp_alloc_type&& __a) noexcept
: _Tp_alloc_type(std::move(__a)),
  _M_start(0), _M_finish(0), _M_end_of_storage(0)
{ }
 #endif
 
-   void _M_swap_data(_Vector_impl& __x)
+   void _M_swap_data(_Vector_impl& __x) _GLIBCXX_NOEXCEPT
{
  std::swap(_M_start, __x._M_start);
  std::swap(_M_finish, __x._M_finish);
  std::swap(_M_end_of_storage, __x._M_end_of_storage);
}
   };
   
 public:
   typedef _Alloc allocator_type;
 
@@ -117,36 +117,36 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   _M_get_Tp_allocator() const _GLIBCXX_NOEXCEPT
   { return *static_cast(&this->_M_impl); }
 
   allocator_type
   get_allocator() const _GLIBCXX_NOEXCEPT
   { return allocator_type(_M_get_Tp_allocator()); }
 
   _Vector_base()
   : _M_impl() { }
 
-  _Vector_base(const allocator_type& __a)
+  _Vector_base(const allocator_type& __a) _GLIBCXX_NOEXCEPT
   : _M_impl(__a) { }
 
   _Vector_base(size_t __n)
   : _M_impl()
   { _M_create_storage(__n); }
 
   _Vector_base(size_t __n, const allocator_type& __a)
   : _M_impl(__a)
   { _M_create_storage(__n); }
 
 #if __cplusplus >= 201103L
-  _Vector_base(_Tp_alloc_type&& __a)
+  _Vector_base(_Tp_alloc_type&& __a) noexcept
   : _M_impl(std::move(__a)) { }
 
-  _Vector_base(_Vector_base&& __x)
+  _Vector_base(_Vector_base&& __x) noexcept
   : _M_impl(std::move(__x._M_get_Tp_allocator()))
   { this->_M_impl._M_swap_data(__x._M_impl); }
 
   _Vector_base(_Vector_base&& __x, const allocator_type& __a)
   : _M_impl(__a)
   {
if (__x.get_allocator() == __a)
  this->_M_impl._M_swap_data(__x._M_impl);
else
  {
@@ -246,21 +246,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*  @brief  Default constructor creates no elements.
*/
   vector()
   : _Base() { }
 
   /**
*  @brief  Creates a %vector with no elements.
*  @param  __a  An allocator object.
*/
   explicit
-  vector(const allocator_type& __a)
+  vector(const allocator_type& __a) _GLIBCXX_NOEXCEPT
   : _Base(__a) { }
 
 #if __cplusplus >= 201103L
   /**
*  @brief  Creates a %vector with default constructed elements.
*  @param  __n  The number of elements to initially create.
*  @param  __a  An allocator.
*
*  This constructor fills the %vector with @a __n default
*  constructed elements.
@@ -404,21 +404,21 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
  _M_initialize_dispatch(__first, __last, _Integral());
}
 #endif
 
   /**
*  The dtor only erases the elements, and note that if the
*  elements themselves are pointers, the pointed-to memory is
*  not touched in any way.  Managing the pointer is the user's
*  responsibility.
*/
-  ~vector() _GLIBCXX_NOEXCEPT
+  ~vector()
   { std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
  _M_get_Tp_allocator()); }
 
   /**
*  @brief  %Vector assignment operator.
*  @param  __x  A %vector of identical element and allocator types.
*
*  All the elements of @a __x are copied, but any extra memory in
 

[buildrobot] [PATCH] r202527: ssa / ssanames restructure broke alpha-linux

[Jan-Benedict Glaw]

Hi!

My Build Robot[1] found this recent commit to break Alpha:

* tree-flow.h (FREE_SSANAMES): Move to tree-ssanames.c
(SSANAMES, MODIFIED_NORETURN_CALLS, DEFAULT_DEFS, ptr_info_def,
num_ssa_names, ssa_name): Move to tree-ssanames.h + prototypes.
* tree-flow-inline.h (make_ssa_name, copy_ssa_name, duplicate_ssa_name,
make_temp_ssa_name): move to tree-ssanames.h
* tree-ssa-alias.h: Move prototype.
* tree-ssa.h: Include tree-ssanames.h.
* tree-ssanames.c (FREE_SSANAMES): Move to here.
* tree-ssanames.h: New.  Move items from tree-flow*.h
* Makefile.in (tree-ssanames.h): Add to tree-ssanames.o and GTFILES.

See for example this build log:
http://toolchain.lug-owl.de/buildbot/showlog.php?id=11663&mode=view

I suggest this patch, which fixes an alpha-linux build for me:

2013-09-13  Jan-Benedict Glaw  

* config/alpha.c: Include tree-ssa.h.

diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c
index a8fb929..3759205 100644
--- a/gcc/config/alpha/alpha.c
+++ b/gcc/config/alpha/alpha.c
@@ -50,6 +50,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "splay-tree.h"
 #include "gimple.h"
 #include "tree-flow.h"
+#include "tree-ssa.h"
 #include "tree-stdarg.h"
 #include "tm-constrs.h"
 #include "df.h"


Ok?

MfG, JBG

[1] http://toolchain.lug-owl.de/buildbot/
http://toolchain.lug-owl.de/buildbot/timeline.php
-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
 Signature of:If it doesn't work, force it.
 the second  :   If it breaks, it needed replacing anyway.


signature.asc
Description: Digital signature

Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (2).

[Richard Sandiford]

Some comments for part 2.

Chung-Ju Wu  writes:
> +;; Include intrinsic functions definition.
> +(include "nds32.intrinsic.md")
> +
> +;; Include block move for nds32 multiple load/store behavior.
> +(include "nds32.multiple.md")
> +
> +;; Include DImode/DFmode operations.
> +(include "nds32.doubleword.md")
> +
> +;; Include peephole patterns.
> +(include "nds32.peephole2.md")

Usual gcc style is to use "-" rather than "." as a word separator in
filenames.

> +(define_insn "*store_si"
> +  [(set (match_operand:SI 0 "memory_operand"   "=U45, U33, U37, U45, m")
> + (match_operand:SI 1 "register_operand" "   l,   l,   l,   d, r"))]
> +  ""

Loads, stores, register moves and constant moves should normally be in
the same pattern, so that anything operating on constraints can see all
the alternatives at once.  This might not be as important for LRA as it
was for reload, but it still seems like good practice.

> +(define_insn "*mov"
> +  [(set (match_operand:QIHISI 0 "register_operand" "=r, m, r")
> + (match_operand:QIHISI 1 "register_operand" " r, r, m"))]
> +  ""
> +{
> +  switch (which_alternative)
> +{
> +case 0:
> +  if (get_attr_length (insn) == 2)
> + return "mov55\t%0, %1";
> +  else
> + return "ori\t%0, %1, 0";
> +case 1:
> +  return nds32_output_32bit_store (operands, );
> +case 2:
> +  return nds32_output_32bit_load (operands, );
> +
> +default:
> +  gcc_unreachable ();
> +}
> +}
> +  [(set_attr "type" "alu,store,load")
> +   (set_attr "enabled" "1")
> +   (set_attr_alternative "length"
> + [
> +   ;; Alternative 0
> +   (if_then_else (match_test "TARGET_16_BIT")
> +  (const_int 2)
> +  (const_int 4))
> +   ;; Alternative 1
> +   (const_int 4)
> +   ;; Alternative 2
> +   (const_int 4)
> + ])])

The style used in the load and store patterns was:

(define_insn "*mov"
  [(set (match_operand:QIHISI 0 "register_operand" "=r, r, m, r")
(match_operand:QIHISI 1 "register_operand" " r, r, r, m"))]
  ""
{
  switch (which_alternative)
{
case 0:
  return "mov55\t%0, %1";
case 1:
  return "ori\t%0, %1, 0";
case 2:
  return nds32_output_32bit_store (operands, );
case 3:
  return nds32_output_32bit_load (operands, );
default:
  gcc_unreachable ();
}
}
  [(set_attr "type" "alu,alu,store,load")
   (set_attr "length" "2,4,4,4")])

which seems neater.  Did you try that but find that it didn't work here?

Same comment for other instructions where:

   (if_then_else (match_test "TARGET_16_BIT")
 (const_int 2)
 (const_int 4))

occurs (except for the special case of relaxable branch instructions,
where using the if_then_else is good).

> +;; We use nds32_symbolic_operand to limit that only 
> CONST/SYMBOL_REF/LABEL_REF
> +;; are able to match such instruction template.
> +(define_insn "*move_addr"
> +  [(set (match_operand:SI 0 "register_operand"   "=l, r")
> + (match_operand:SI 1 "nds32_symbolic_operand" " i, i"))]
> +  ""
> +  "la\t%0, %1"
> +  [(set_attr "type" "move")
> +   (set_attr "length"  "8")])
> +
> +
> +(define_insn "*sethi"
> +  [(set (match_operand:SI 0 "register_operand"   "=r")
> + (high:SI (match_operand:SI 1 "immediate_operand" " i")))]
> +  ""
> +{
> +  return "sethi\t%0, hi20(%1)";
> +}
> +  [(set_attr "type" "alu")
> +   (set_attr "length" "4")])
> +
> +
> +(define_insn "*lo_sum"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> + (lo_sum:SI (match_operand:SI 1 "register_operand"  " 0")
> +(match_operand:SI 2 "immediate_operand" " i")))]
> +  ""
> +  "ori\t%0, %1, lo12(%2)"
> +  [(set_attr "type" "alu")
> +   (set_attr "length" "4")])

Very minor, but "nds32_symbolic_operand" seems like a better choice for
*sethi and *lo_sum too, since (high ...) and (lo_sum ...) shouldn't be
used for const_ints.

Any pass would be in its rights to fuse a *sethi and *lo_sum pair back
into a single *move_addr.  Is that something you want to allow?
(That's a genuine question rather than a review comment btw.)

Is the "0" constraint on the *lo_sum really necessary?  It looks from
the later OR patterns as though this form of ORI allows the source and
destination registers to be different.

> +;; Zero extension instructions.
> +
> +(define_expand "zero_extendsi2"
> +  [(set (match_operand:SI 0 "general_operand" "")
> + (zero_extend:SI (match_operand:QIHI 1 "general_operand" "")))]
> +  ""
> +{
> +  rtx tmp_reg;
> +
> +  /* We need to make sure operands[1] is a register.  */
> +  if (!REG_P (operands[1]))
> +operands[1] = force_reg (GET_MODE (operands[1]), operands[1]);

Why do you need this?  It looks like the architecture has zero-extending loads.

> +
> +  /* If the pattern is "(mem X) <- (zero_extend (reg Y))",
> + we create two rtx patterns:
> +   (reg:SI K) <- (zero_extend:SI (reg Y))
> +   (mem:SI X) <- (reg:SI K)
> +   

Re: [v3] More noexcept for vectors

[Paolo Carlini]

Hi,

Marc Glisse  ha scritto:
>Hello,
>
>this patch passes bootstrap+testsuite. The guarantees given by the
>standard on allocators are a bit weird, but I see there is already
>DR2016
>taking care of it.

Patch looks good to me, thanks!

Paolo

Re: [v3] More noexcept for vectors

[Paolo Carlini]

... what about debug-mode (and profile-mode)? Is in principle possible to 
detect the noexcepts? If we can't exclude that, we should probably change at 
the same time the special modes too. Otherwise seems just matter of consistency?

Paolo

Re: [v3] More noexcept for vectors

[Marc Glisse]

On Sat, 14 Sep 2013, Paolo Carlini wrote:

... what about debug-mode (and profile-mode)? Is in principle possible 
to detect the noexcepts? If we can't exclude that, we should probably 
change at the same time the special modes too. Otherwise seems just 
matter of consistency?


I was going one file at a time, and the priority is clearly 
"release"-mode, since this is about performance. I don't think there is 
anything wrong with debug-mode having a different exception specification 
(it is already binary-incompatible with the default mode) but I understand 
why people may want to keep some consistency. I can do vector in the other 
modes next if you want.


--
Marc Glisse

Re: [v3] More noexcept for vectors

[Paolo Carlini]

Hi,

>I was going one file at a time, and the priority is clearly
>"release"-mode, since this is about performance. I don't think there is
>
>anything wrong with debug-mode having a different exception
>specification
>(it is already binary-incompatible with the default mode) but I
>understand 
>why people may want to keep some consistency. I can do vector in the
>other
>modes next if you want.

Yes, please. As boring as it may seem, I'm trying to convince myself and the 
other contributors to keep the modes up to date and consistent as much as 
possible. If one day we figure out that we can't, we don't have enough man 
power it will mean we have to remove the special modes (or only some of them) 
from svn.

Thanks!
Paolo

Re: Using gen_int_mode instead of GEN_INT minor testsuite fallout on MIPS

[Mike Stump]

On Sep 14, 2013, at 1:57 AM, Richard Sandiford  
wrote:
> How about a three-operand version: the name of the partial mode,
> the name of the memory mode, and the precision?

I like creating the name via algorithm.  It leads to predictable names, and 
easy to understand names.  P60DI is obvious to all…  P20SI equally is obvious 
to all.  But, if people like the ability to name the mode, certainly adding a 
parameter for the name is trivial enough to do.

> "PTI" and "P128TI" are a bit confusing for the (hacky) powerpc usage, since 
> 128 bits
> isn't partial.

Arguably what rs6000 is doing is unholy.  I'd welcome someone to find the right 
solution to their problem and fix it in a better way; the use of partial isn't 
the right solution.

Re: [RFC] Offloading Support in libgomp

[Jakub Jelinek]

On Fri, Sep 13, 2013 at 05:41:03PM +0200, Marek Polacek wrote:
> On Fri, Sep 13, 2013 at 05:35:27PM +0200, Jakub Jelinek wrote:
> > On Fri, Sep 13, 2013 at 03:15:56PM +0200, Jakub Jelinek wrote:
> > > On Fri, Sep 13, 2013 at 05:11:09PM +0400, Michael V. Zolotukhin wrote:
> > > > > FYI, I'm attaching a WIP patch with the splay tree stuff.
> > > > Thanks, I'll take a look.  By the way, isn't it better to move 
> > > > splay-tree
> > > > implementation to a separate file?
> > > 
> > > As it is just a few routines, heavily modified from include/splay-tree.h
> > > (e.g. the data structures contain all the target.c specific stuff), and 
> > > will be
> > > used just in target.c, I think it is fine to keep it in target.c.
> > 
> > Anyway, here is an updated patch that moves the splay stuff into
> > splay-tree.h and cleans up a bunch of other things.
> > 
> > Will commit once the http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01044.html
> > issue is resolved.
> > 
> > 2013-09-13  Jakub Jelinek  
> > 
> > * ipa-prop.c (ipa_compute_jump_functions_for_edge): Return early
> > for internal calls.
> 
> Seems like a wrong patch is attached.

You're right, here is the right one.

2013-09-13  Jakub Jelinek  

* splay-tree.h: New file.
* target.c: Include stdbool.h.
(splay_tree_node, splay_tree, splay_tree_key): New typedefs.
(struct target_mem_desc, struct splay_tree_key_s): New structures.
(splay_compare): New inline function.
(gomp_get_num_devices): New function.
(resolve_device): Use default_device_var ICV.  Add temporarily
magic testing device number 257.
(dev_splay_tree, dev_env_lock): New variables.
(gomp_map_vars_existing, gomp_map_vars, gomp_unmap_tgt,
gomp_unmap_vars, gomp_update): New functions.
(GOMP_target, GOMP_target_data, GOMP_target_end_data,
GOMP_target_update): Add support for magic testing device number 257.
* libgomp.h (struct target_mem_desc): Forward declare.
(struct gomp_task_icv): Add default_device_var and target_data.
(gomp_get_num_devices): New prototype.
* env.c (gomp_global_icv): Add default_device_var initializer.
(parse_int): New function.
(handle_omp_display_env): Print OMP_DEFAULT_DEVICE.
(initialize_env): Initialize default_device_var.
(omp_set_default_device): Set default_device_var ICV.
(omp_get_default_device): Query default_device_var ICV.
(omp_get_num_devices): Call gomp_get_num_devices.
(omp_get_num_teams, omp_get_team_num, omp_is_initial_device): Add
comments.

--- libgomp/splay-tree.h.jj 2013-09-13 16:32:48.381973559 +0200
+++ libgomp/splay-tree.h2013-09-13 16:41:38.059701560 +0200
@@ -0,0 +1,232 @@
+/* A splay-tree datatype.
+   Copyright 1998-2013
+   Free Software Foundation, Inc.
+   Contributed by Mark Mitchell (m...@markmitchell.com).
+
+   This file is part of the GNU OpenMP Library (libgomp).
+
+   Libgomp 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.
+
+   Libgomp 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
+   .  */
+
+/* The splay tree code copied from include/splay-tree.h and adjusted,
+   so that all the data lives directly in splay_tree_node_s structure
+   and no extra allocations are needed.
+
+   Files including this header should before including it add:
+typedef struct splay_tree_node_s *splay_tree_node;
+typedef struct splay_tree_s *splay_tree;
+typedef struct splay_tree_key_s *splay_tree_key;
+   define splay_tree_key_s structure, and define
+   splay_compare inline function.  */
+
+/* For an easily readable description of splay-trees, see:
+
+ Lewis, Harry R. and Denenberg, Larry.  Data Structures and Their
+ Algorithms.  Harper-Collins, Inc.  1991.
+
+   The major feature of splay trees is that all basic tree operations
+   are amortized O(log n) time for a tree with n nodes.  */
+
+/* The nodes in the splay tree.  */
+struct splay_tree_node_s {
+  struct splay_tree_key_s key;
+  /* The left and right children, respectively.  */
+  splay_tree_node left;
+  splay_tree_node right;
+};
+
+/* The splay tree.  */
+struct splay_tree_s {
+  splay

[committed] Fix incorrect mode used in pa_expand_prologue

[John David Anglin]

The attached change fixes a bug in pa_expand_prologue.  The mode  
argument in the calls

to "gen_rtx_POST_INC" should be the same as "tmpreg" (word_mode).

Tested on hppa2.0w-hp-hpux11.11, hppa64-hp-hpux11.11 and hppa-unknown- 
linux-gnu.

Committed to trunk, 4.8 and 4.7.

Dave
--
John David Anglin   dave.ang...@bell.net


2013-09-14  John David Anglin  

PR target/58382
* config/pa/pa.c (pa_expand_prologue): Change mode in gen_rtx_POST_INC
calls to word_mode.

Index: config/pa/pa.c
===
--- config/pa/pa.c  (revision 202481)
+++ config/pa/pa.c  (working copy)
@@ -4035,7 +4041,8 @@
  || (! TARGET_64BIT && df_regs_ever_live_p (i + 1)))
{
  rtx addr, insn, reg;
- addr = gen_rtx_MEM (DFmode, gen_rtx_POST_INC (DFmode, tmpreg));
+ addr = gen_rtx_MEM (DFmode,
+ gen_rtx_POST_INC (word_mode, tmpreg));
  reg = gen_rtx_REG (DFmode, i);
  insn = emit_move_insn (addr, reg);
  if (DO_FRAME_NOTES)
@@ -4328,7 +4335,8 @@
if (df_regs_ever_live_p (i)
|| (! TARGET_64BIT && df_regs_ever_live_p (i + 1)))
  {
-   rtx src = gen_rtx_MEM (DFmode, gen_rtx_POST_INC (DFmode, tmpreg));
+   rtx src = gen_rtx_MEM (DFmode,
+  gen_rtx_POST_INC (word_mode, tmpreg));
rtx dest = gen_rtx_REG (DFmode, i);
emit_move_insn (dest, src);
  }