Re: [PATCH] Implement new hook for max_align_t_align

[Carlos O'Donell]

On 10/11/2016 04:04 PM, John David Anglin wrote:
> On 2016-10-11 2:50 PM, Jason Merrill wrote:
>> /* Alignment, in bits, a C conformant malloc implementation has to
>> provide.
>> The HP-UX malloc implementation provides a default alignment of 8
>> bytes.
>> This can be increased with mallopt.  The glibc implementation also
>> provides
>> 8-byte alignment.  Note that this isn't enough for various POSIX
>> types such
>> as pthread_mutex_t.  However, since we no longer need the 16-byte
>> alignment
>> for atomic operations, we ignore the nominal alignment specified
>> for these
>> types.  The same is true for long double on 64-bit HP-UX.  */
>>
>> If PA malloc doesn't actually provide 16-byte alignment, this change
>> seems problematic; it will mean any type that wants 16-byte alignment
>> will silently get 8-byte alignment instead.
> 
> I agree the situation is something of a mess.  On linux, we could bump the 
> alignment
> of malloc to 16-bytes.  However, Carlos argued that we don't need to and I 
> think doing
> so would be detrimental to performance.

Correct, we do not need a 16-byte alignment at runtime.

> The 16-byte alignment was used originally because the ldcw instruction used 
> for atomic
> operations in linux threads needs 16-byte alignment.  However, the nptl 
> pthread
> implementation now uses a kernel helper for atomic operations.  It only needs
> 4-byte alignment.  The largest alignment actually needed is for long double 
> (8 bytes).
> However, we can't change the 16-byte alignment without affecting the layout 
> of various
> structures.

Correct, the structure padding needs to continue to be there to match the 
original ABI.

> The same is true for long double on HPUX.  Originally, it was planned to 
> implement it
> in hardware and that would have required 16-byte alignment.  It was only 
> implemented
> in software with an 8-byte alignment requirement.  Somehow, it ended up with 
> 8 and
> 16-byte alignment in the HP 32 and 64-bit compilers, respectively. In both 
> cases, malloc
> has 8-byte alignment.  It is possible to increase the "grain" size of HP 
> malloc to 16 bytes.
> 
> Thus, I don't think the silent reduction to 8-byte alignment matters.  
> Without the change,
> we are faced with spurious warnings from new.

I suggested disabling the warnings.
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01445.html

Which is what Jason suggests here:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00786.html

Though Florian Weimer suggests just bumping malloc to return 16-byte aligned 
objects and
raising max_align_t, since conceptually that's simple (but will impact 
performance):
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01446.html

I think the case where a user specifically requests a larger aligment is still 
always
bound to fail if they exceed malloc's aligment. So removing the warning just 
leaves
hppa where it is today. No warning. Working with existing malloc alignment. But 
unable
to warn users of legitimate cases where this might matter?

I still suggest disabling the warning.

-- 
Cheers,
Carlos.

Re: [RFC][VRP] Improve intersect_ranges

[Richard Biener]

On Wed, Oct 12, 2016 at 8:35 AM, kugan
 wrote:
> Hi Richard,
>
>
> On 12/10/16 00:14, Richard Biener wrote:
>>
>> On Tue, Oct 11, 2016 at 2:57 AM, kugan
>>  wrote:
>>>
>>> Hi Richard,
>>> Hi Richard,
>>>
>>> On 10/10/16 20:13, Richard Biener wrote:


 On Sat, Oct 8, 2016 at 9:38 PM, kugan
 
 wrote:
>
>
> Hi Richard,
>
> Thanks for the review.
> On 07/10/16 20:11, Richard Biener wrote:
>>
>>
>>
>> On Fri, Oct 7, 2016 at 12:00 AM, kugan
>>  wrote:
>>>
>>>
>>>
>>> Hi,
>>>
>>> In vrp intersect_ranges, Richard recently changed it to create
>>> integer
>>> value
>>> ranges when it is integer singleton.
>>>
>>> Maybe we should do the same when the other range is a complex ranges
>>> with
>>> SSA_NAME (like [x+2, +INF])?
>>>
>>> Attached patch tries to do this. There are cases where it will be
>>> beneficial
>>> as the  testcase in the patch. (For this testcase to work with Early
>>> VRP,
>>> we
>>> need the patch posted at
>>> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00413.html)
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions.
>>
>>
>>
>>
>> This is not clearly a win, in fact it can completely lose an
>> ASSERT_EXPR
>> because there is no way to add its effect back as an equivalence.  The
>> current choice of always using the "left" keeps the ASSERT_EXPR range
>> and is able to record the other range via an equivalence.
>
>
>
>
> How about changing the order in Early VRP when we are dealing with the
> same
> SSA_NAME in inner and outer scope. Here is a patch that does this. Is
> this
> OK if no new regressions?



 I'm not sure if this is a good way forward.  The failure with the
 testcase
 is
 that we don't extract a range for k from if (j < k) which I believe
 another
 patch from you addresses?
>>>
>>>
>>>
>>> Yes,  I have committed that. I am trying to add test cases for this and
>>> thats when I stumbled on this:
>>>
>>> For:
>>> foo (int k, int j)
>>> {
>>>:
>>>if (j_1(D) > 9)
>>>  goto ;
>>>else
>>>  goto ;
>>>
>>>:
>>>if (j_1(D) < k_2(D))
>>>  goto ;
>>>else
>>>  goto ;
>>>
>>>:
>>>k_3 = k_2(D) + 1;
>>>if (k_2(D) <= 8)
>>>  goto ;
>>>else
>>>  goto ;
>>>
>>>:
>>>abort ();
>>>
>>>:
>>>return j_1(D);
>>>
>>> }
>>>
>>> Before we look at - if (j_1(D) < k_2(D))
>>> j_1 (D) has [10, +INF]  EQUIVALENCES: { j_1(D) } (1 elements)
>>>
>>> When we look at  if (j_1(D) < k_2(D))
>>> The range is [-INF, k_2(D) + -1]  EQUIVALENCES: { j_1(D) } (1 elements)
>>>
>>> We intersect:
>>> [-INF, k_2(D) + -1]  EQUIVALENCES: { j_1(D) } (1 elements)
>>> and
>>> [10, +INF]  EQUIVALENCES: { j_1(D) } (1 elements)
>>>
>>> to
>>> [-INF, k_2(D) + -1]  EQUIVALENCES: { j_1(D) } (1 elements)
>>>
>>> Due to this, in if (j_1(D) < k_2(D)) , we get pessimistic value range for
>>> k_2(D)
>>
>>
>> Ah, but that is because when generating the range for k from j < k we
>> use the updated range for j.  That obviously doens't make too much sense.
>>
>> @@ -10650,7 +10661,7 @@ public:
>>virtual void after_dom_children (basic_block);
>>void push_value_range (const_tree var, value_range *vr);
>>value_range *pop_value_range (const_tree var);
>> -  void try_add_new_range (tree op, tree_code code, tree limit);
>> +  value_range *try_add_new_range (tree op, tree_code code, tree limit);
>>
>>/* Cond_stack holds the old VR.  */
>>auto_vec > stack;
>> @@ -10661,7 +10672,7 @@ public:
>>
>>  /*  Add new range to OP such that (OP CODE LIMIT) is true.  */
>>
>> -void
>> +value_range *
>>  evrp_dom_walker::try_add_new_range (tree op, tree_code code, tree limit)
>>  {
>>value_range vr = VR_INITIALIZER;
>> @@ -10678,8 +10689,9 @@ evrp_dom_walker::try_add_new_range (tree
>>  {
>>value_range *new_vr = vrp_value_range_pool.allocate ();
>>*new_vr = vr;
>> -  push_value_range (op, new_vr);
>> +  return new_vr;
>>  }
>> +  return NULL;
>>  }
>>
>>  /* See if there is any new scope is entered with new VR and set that VR
>> to
>> @@ -10715,7 +10727,7 @@ evrp_dom_walker::before_dom_children (ba
>> code = invert_tree_comparison (gimple_cond_code (stmt),
>>HONOR_NANS (op0));
>>   /* Add VR when (OP0 CODE OP1) condition is true.  */
>> - try_add_new_range (op0, code, op1);
>> + value_range *op0_range = try_add_new_range (op0, code, op1);
>>
>>   /* Register ranges for y in x < y where
>>  y might have ranges that are useful.  */
>> @@ -10728,8 +10740,13 @@ evrp_dom_walker::before_dom_children (ba
>>   &new_code,
>> &limit))
>> {
>>   /* Add VR when (OP1 NEW_CO

[PATCH] Fix PR77920

[Richard Biener]

VRP needs to make sure to update the iterator it gets to it stmt folding
callback.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2016-10-12  Richard Biener  

PR tree-optimization/77920
* tree-vrp.c (simplify_div_or_mod_using_ranges): Simplify.
(simplify_min_or_max_using_ranges): Pass in gsi and use it.
(simplify_abs_using_ranges): Likewise.
(simplify_conversion_using_ranges): Likewise.
(simplify_stmt_using_ranges): Adjust.

* gcc.dg/torture/pr77920.c: New testcase.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 240964)
+++ gcc/tree-vrp.c  (working copy)
@@ -9093,9 +9093,7 @@ simplify_div_or_mod_using_ranges (gimple
{
  /* If op0 already has the range op0 % op1 has,
 then TRUNC_MOD_EXPR won't change anything.  */
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- gimple_assign_set_rhs_from_tree (&gsi, op0);
- update_stmt (stmt);
+ gimple_assign_set_rhs_from_tree (gsi, op0);
  return true;
}
 }
@@ -9171,7 +9169,7 @@ simplify_div_or_mod_using_ranges (gimple
disjoint.   Return true if we do simplify.  */
 
 static bool
-simplify_min_or_max_using_ranges (gimple *stmt)
+simplify_min_or_max_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
 {
   tree op0 = gimple_assign_rhs1 (stmt);
   tree op1 = gimple_assign_rhs2 (stmt);
@@ -9206,10 +9204,7 @@ simplify_min_or_max_using_ranges (gimple
 VAL == FALSE -> OP0 > or >= op1.  */
   tree res = ((gimple_assign_rhs_code (stmt) == MAX_EXPR)
  == integer_zerop (val)) ? op0 : op1;
-  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
-  gimple_assign_set_rhs_from_tree (&gsi, res);
-  update_stmt (stmt);
-  fold_stmt (&gsi, follow_single_use_edges);
+  gimple_assign_set_rhs_from_tree (gsi, res);
   return true;
 }
 
@@ -9221,7 +9216,7 @@ simplify_min_or_max_using_ranges (gimple
ABS_EXPR into a NEGATE_EXPR.  */
 
 static bool
-simplify_abs_using_ranges (gimple *stmt)
+simplify_abs_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
 {
   tree op = gimple_assign_rhs1 (stmt);
   value_range *vr = get_value_range (op);
@@ -9262,8 +9257,7 @@ simplify_abs_using_ranges (gimple *stmt)
  else
gimple_assign_set_rhs_code (stmt, NEGATE_EXPR);
  update_stmt (stmt);
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- fold_stmt (&gsi, follow_single_use_edges);
+ fold_stmt (gsi, follow_single_use_edges);
  return true;
}
 }
@@ -9844,7 +9838,7 @@ simplify_switch_using_ranges (gswitch *s
 /* Simplify an integral conversion from an SSA name in STMT.  */
 
 static bool
-simplify_conversion_using_ranges (gimple *stmt)
+simplify_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
 {
   tree innerop, middleop, finaltype;
   gimple *def_stmt;
@@ -9914,8 +9908,7 @@ simplify_conversion_using_ranges (gimple
 return false;
 
   gimple_assign_set_rhs1 (stmt, innerop);
-  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
-  fold_stmt (&gsi, follow_single_use_edges);
+  fold_stmt (gsi, follow_single_use_edges);
   return true;
 }
 
@@ -10218,7 +10211,7 @@ simplify_stmt_using_ranges (gimple_stmt_
case ABS_EXPR:
  if (TREE_CODE (rhs1) == SSA_NAME
  && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
-   return simplify_abs_using_ranges (stmt);
+   return simplify_abs_using_ranges (gsi, stmt);
  break;
 
case BIT_AND_EXPR:
@@ -10233,7 +10226,7 @@ simplify_stmt_using_ranges (gimple_stmt_
CASE_CONVERT:
  if (TREE_CODE (rhs1) == SSA_NAME
  && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
-   return simplify_conversion_using_ranges (stmt);
+   return simplify_conversion_using_ranges (gsi, stmt);
  break;
 
case FLOAT_EXPR:
@@ -10244,7 +10237,7 @@ simplify_stmt_using_ranges (gimple_stmt_
 
case MIN_EXPR:
case MAX_EXPR:
- return simplify_min_or_max_using_ranges (stmt);
+ return simplify_min_or_max_using_ranges (gsi, stmt);
 
default:
  break;
Index: gcc/testsuite/gcc.dg/torture/pr77920.c
===
--- gcc/testsuite/gcc.dg/torture/pr77920.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr77920.c  (working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+int a, b;
+void fn1()
+{
+  int c;
+  for (; b < 0;)
+{
+   {
+ int d = 56, e = (b >> 3) - (d >> 3) > 0 ? (b >> 3) - (d >> 3)
+ : -((b >> 3) - (d >> 3));
+ c = 1 >= e;
+   }
+  if (c)
+   a = 0;
+}
+}

Re: [PATCH][2/2] Enable SSA propagator "DCE" for VRP

[Richard Biener]

On Fri, 7 Oct 2016, Richard Biener wrote:

> 
> This turns the switch (which also requires propagating into ASSERT_EXPRs,
> otherwise those will end up with released SSA names eventually).
> 
> A [3/2] would be to let ASSERT_EXPRs be removed by the propagator
> which would a) require VRP to fix up its lattice for this task,
> b) make match-and-simplify not be confused by them during
> substitute-and-fold folding.  This also requires (I think) dealing
> with symbolic valueizations during substitute-and-fold in a
> way FRE does (track availability) -- currently propagators restrict
> themselves to constants due to that (avoid substituting to where
> the use does not dominate the definition).
> 
> Re-bootstrap / regtest pending on x86_64-unknown-linux-gnu.

The following is what I have applied -- for now we can't DCE ASSERT_EXPRs
because VRP jump threading relies on them (in fact, it looks like VRPs
jump threading "implementation" is nothing but backwards threading,
looking for ASSERT_EXPRs rather than dominating conditions ... Jeff,
you may be more familiar with the VRP threading code, it looks like
we might be able to rip it out without regressions?)

Richard.

2016-10-07  Richard Biener  

* tree-ssa-propagate.c
(substitute_and_fold_dom_walker::before_dom_children): Do not
ignore ASSERT_EXPRs but only preserve them.
* tree-vrp.c (remove_range_assertions): Deal with ASSERT_EXPRs
that have been propagated into.
(vrp_finalize): Enable DCE for substitute_and_fold.

* gcc.dg/tree-ssa/vrp35.c: Adjust.
* gcc.dg/tree-ssa/vrp36.c: Likewise.
* gcc.dg/tree-ssa/vrp46.c: Likewise.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 240855)
+++ gcc/tree-vrp.c  (working copy)
@@ -6894,9 +6894,9 @@ remove_range_assertions (void)
imm_use_iterator iter;
 
var = ASSERT_EXPR_VAR (rhs);
-   gcc_assert (TREE_CODE (var) == SSA_NAME);
 
-   if (!POINTER_TYPE_P (TREE_TYPE (lhs))
+   if (TREE_CODE (var) == SSA_NAME
+   && !POINTER_TYPE_P (TREE_TYPE (lhs))
&& SSA_NAME_RANGE_INFO (lhs))
  {
if (is_unreachable == -1)
@@ -6928,8 +6928,11 @@ remove_range_assertions (void)
 
/* Propagate the RHS into every use of the LHS.  */
FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
- FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
-   SET_USE (use_p, var);
+ {
+   FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
+ SET_USE (use_p, var);
+   update_stmt (use_stmt);
+ }
 
/* And finally, remove the copy, it is not needed.  */
gsi_remove (&si, true);
@@ -10608,7 +10611,7 @@ vrp_finalize (bool warn_array_bounds_p)
   }
 
   substitute_and_fold (op_with_constant_singleton_value_range,
-  vrp_fold_stmt, false);
+  vrp_fold_stmt, true);
 
   if (warn_array_bounds && warn_array_bounds_p)
 check_all_array_refs ();
Index: gcc/tree-ssa-propagate.c
===
--- gcc/tree-ssa-propagate.c(revision 240964)
+++ gcc/tree-ssa-propagate.c(working copy)
@@ -1035,15 +1035,6 @@ substitute_and_fold_dom_walker::before_d
 {
   bool did_replace;
   gimple *stmt = gsi_stmt (i);
-  enum gimple_code code = gimple_code (stmt);
-
-  /* Ignore ASSERT_EXPRs.  They are used by VRP to generate
-range information for names and they are discarded
-afterwards.  */
-
-  if (code == GIMPLE_ASSIGN
- && TREE_CODE (gimple_assign_rhs1 (stmt)) == ASSERT_EXPR)
-   continue;
 
   /* No point propagating into a stmt we have a value for we
  can propagate into all uses.  Mark it for removal instead.  */
@@ -1056,7 +1047,10 @@ substitute_and_fold_dom_walker::before_d
  && sprime != lhs
  && may_propagate_copy (lhs, sprime)
  && !stmt_could_throw_p (stmt)
- && !gimple_has_side_effects (stmt))
+ && !gimple_has_side_effects (stmt)
+ /* We have to leave ASSERT_EXPRs around for jump-threading.  */
+ && (!is_gimple_assign (stmt)
+ || gimple_assign_rhs_code (stmt) != ASSERT_EXPR))
{
  stmts_to_remove.safe_push (stmt);
  continue;
Index: gcc/testsuite/gcc.dg/tree-ssa/vrp35.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/vrp35.c   (revision 240964)
+++ gcc/testsuite/gcc.dg/tree-ssa/vrp35.c   (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
 
 int test1(int i, int k)
 {
@@ -11,4 +11,4 @@ int test1(int i, int k)
   return 1;
 }
 
-/* { dg-final { scan-tree-dump "Folding pred

[PATCH] Remove constraint on (X /[ex] A) * A -> X

[Richard Biener]

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2016-10-12  Richard Biener  

* match.pd ((X /[ex] A) * A -> X): Remove unnecessary constraint
on the conversion.

Index: gcc/match.pd
===
--- gcc/match.pd(revision 241019)
+++ gcc/match.pd(working copy)
@@ -1783,10 +1783,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
   (mult (convert1? (exact_div @0 @1)) (convert2? @2))
   /* We cannot use matching captures here, since in the case of
- constants we don't see the second conversion.  Look through
- a sign-changing or widening conversions.  */
-  (if (operand_equal_p (@1, @2, 0)
-   && element_precision (@0) <= element_precision (type))
+ constants we don't see the second conversion.  */
+  (if (operand_equal_p (@1, @2, 0))
(convert @0)))
 
 /* Canonicalization of binary operations.  */

Re: [PATCH] Implement new hook for max_align_t_align

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 03:01:51AM -0400, Carlos O'Donell wrote:
> On 10/11/2016 04:04 PM, John David Anglin wrote:
> > On 2016-10-11 2:50 PM, Jason Merrill wrote:
> >> /* Alignment, in bits, a C conformant malloc implementation has to
> >> provide.
> >> The HP-UX malloc implementation provides a default alignment of 8
> >> bytes.
> >> This can be increased with mallopt.  The glibc implementation also
> >> provides
> >> 8-byte alignment.  Note that this isn't enough for various POSIX
> >> types such
> >> as pthread_mutex_t.  However, since we no longer need the 16-byte
> >> alignment
> >> for atomic operations, we ignore the nominal alignment specified
> >> for these
> >> types.  The same is true for long double on 64-bit HP-UX.  */
> >>
> >> If PA malloc doesn't actually provide 16-byte alignment, this change
> >> seems problematic; it will mean any type that wants 16-byte alignment
> >> will silently get 8-byte alignment instead.
> > 
> > I agree the situation is something of a mess.  On linux, we could bump the 
> > alignment
> > of malloc to 16-bytes.  However, Carlos argued that we don't need to and I 
> > think doing
> > so would be detrimental to performance.
> 
> Correct, we do not need a 16-byte alignment at runtime.

The problem with cheating is that gcc will then assume the structure is
properly aligned and optimize based on that (optimize away alignment checks
etc.).
> 
> > The 16-byte alignment was used originally because the ldcw instruction used 
> > for atomic
> > operations in linux threads needs 16-byte alignment.  However, the nptl 
> > pthread
> > implementation now uses a kernel helper for atomic operations.  It only 
> > needs
> > 4-byte alignment.  The largest alignment actually needed is for long double 
> > (8 bytes).
> > However, we can't change the 16-byte alignment without affecting the layout 
> > of various
> > structures.
> 
> Correct, the structure padding needs to continue to be there to match the 
> original ABI.

No, you can just drop the aligned attributes for HPUX 32-bit, basically
introduce a new ABI.  If needed, you could add new symbol versions for
pthread_mutex_* etc. (though, if the current code doesn't care about the
alignment, perhaps you could get away without bumping that).

> I think the case where a user specifically requests a larger aligment is 
> still always
> bound to fail if they exceed malloc's aligment. So removing the warning just 
> leaves

If users use posix_memalign/memalign/aligned_alloc or the C++17 aligned new,
they should be fine.

Jakub

Re: [PATCH] Implement new hook for max_align_t_align

[Florian Weimer]

On 10/12/2016 09:25 AM, Jakub Jelinek wrote:


No, you can just drop the aligned attributes for HPUX 32-bit, basically
introduce a new ABI.  If needed, you could add new symbol versions for
pthread_mutex_* etc. (though, if the current code doesn't care about the
alignment, perhaps you could get away without bumping that).


This is not something which can be solved with symbol versioning.  It is 
fairly common to embed mutexes into other objects, like this:


  struct client {
pthread_mutex_t lock;
struct client *next;
size_t attachment_count;
  };

The layout above is fine with the alignment change, but if the 
programmer writes this instead:


  struct client {
struct client *next;
pthread_mutex_t lock;
size_t attachment_count;
  };

dropping the alignment means that the padding before the lock member 
vanishes.  Consequently, we have just created a silent ABI change in 
application code, which is a big no-no.


Since this is PA-RISC, which is essentially dead (neither HPE nor Debian 
ship it anymore), I stand by my suggestion to bump the fundamental 
alignment instead.  Sure, it is a bit inefficient, but this will only 
affect PA-RISC users.  It does not even cause work for PA-RISC porters. 
Conversely, if we work on this to come up with a different fix, many 
more people will be affected (because they don't get all the nice things 
we could work on instead), and we may need to maintain a special GCC 
kludge for the alternative solution, impacting GCC developers in particular.


Thanks,
Florian

Re: [PATCH v2] aarch64: Add split-stack initial support

[Kyrill Tkachov]

On 11/10/16 20:39, Adhemerval Zanella wrote:









diff --git a/gcc/testsuite/gcc.dg/split-3.c b/gcc/testsuite/gcc.dg/split-3.c
index 64bbb8c..5ba7616 100644
--- a/gcc/testsuite/gcc.dg/split-3.c
+++ b/gcc/testsuite/gcc.dg/split-3.c
@@ -40,6 +40,7 @@ down (int i, ...)
 || va_arg (ap, int) != 9
 || va_arg (ap, int) != 10)
   abort ();
+  va_end (ap);
   if (i > 0)
   {
diff --git a/gcc/testsuite/gcc.dg/split-6.c b/gcc/testsuite/gcc.dg/split-6.c
index b32cf8d..b3016ba 100644
--- a/gcc/testsuite/gcc.dg/split-6.c
+++ b/gcc/testsuite/gcc.dg/split-6.c
@@ -37,6 +37,7 @@ down (int i, ...)
 || va_arg (ap, int) != 9
 || va_arg (ap, int) != 10)
   abort ();
+  va_end (ap);
   if (i > 0)
   {

Is this just undefined behaviour in existing tests?
If so, I recommend you split these testsuite fixes into a separate patch
and submit that separately. It can probably go in much sooner independently
of slit stack support.

Also, I think some tests for split-stack support are appropriate.
At least something that checks that gcc generates the jumps to the appropriate
helper functions?

In theory yes, although it does not really invalidate the test since the
idea is to check for the variadic arguments.  Based on your comment I think
it would better to split this patch in 2 with this modifications at first.

Regarding the tests, the go testsuite itself covers pretty much all the
morestack call usage (it indeed showed a lot of issues with some initial
drafts for this patches). So I am not sure how more extensible testsuite
we would like to push for this.


Ah ok then. I won't insist on more calls then.
Thanks,
Kyrill




Thanks,
Kyrill

Re: [PATCH] Implement new hook for max_align_t_align

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 09:52:04AM +0200, Florian Weimer wrote:
> dropping the alignment means that the padding before the lock member
> vanishes.  Consequently, we have just created a silent ABI change in
> application code, which is a big no-no.

Sure, it would be an ABI change, but how many users would it affect?

> Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
> ship it anymore), I stand by my suggestion to bump the fundamental alignment

Or just drop support for a dead arch?

> instead.  Sure, it is a bit inefficient, but this will only affect PA-RISC
> users.  It does not even cause work for PA-RISC porters. Conversely, if we
> work on this to come up with a different fix, many more people will be
> affected (because they don't get all the nice things we could work on
> instead), and we may need to maintain a special GCC kludge for the
> alternative solution, impacting GCC developers in particular.

But sure, bumping malloc alignment is probably easiest.  And people who want
performance have better options than to stay on 32-bit PA-RISC anyway.

Jakub

Re: [PATCH GCC]New vectorization pattern turning cond_expr into max/min and plus/minus

[Richard Biener]

On Tue, Oct 11, 2016 at 5:03 PM, Bin Cheng  wrote:
> Hi,
> Given below test case,
> int foo (unsigned short a[], unsigned int x)
> {
>   unsigned int i;
>   for (i = 0; i < 1000; i++)
> {
>   x = a[i];
>   a[i] = (unsigned short)(x >= 32768 ? x - 32768 : 0);
> }
>   return x;
> }
>
> it now can be vectorized on AArch64, but generated assembly is way from 
> optimal:
> .L4:
> ldr q4, [x3, x1]
> add w2, w2, 1
> cmp w2, w0
> ushll   v1.4s, v4.4h, 0
> ushll2  v0.4s, v4.8h, 0
> add v3.4s, v1.4s, v6.4s
> add v2.4s, v0.4s, v6.4s
> cmhiv1.4s, v1.4s, v5.4s
> cmhiv0.4s, v0.4s, v5.4s
> and v1.16b, v3.16b, v1.16b
> and v0.16b, v2.16b, v0.16b
> xtn v2.4h, v1.4s
> xtn2v2.8h, v0.4s
> str q2, [x3, x1]
> add x1, x1, 16
> bcc .L4
>
> The vectorized loop has 15 instructions, which can be greatly simplified by 
> turning cond_expr into max_expr, as below:
> .L4:
> ldr q1, [x3, x1]
> add w2, w2, 1
> cmp w2, w0
> umaxv0.8h, v1.8h, v2.8h
> add v0.8h, v0.8h, v2.8h
> str q0, [x3, x1]
> add x1, x1, 16
> bcc .L4
>
> This patch addresses the issue by adding new vectorization pattern.
> Bootstrap and test on x86_64 and AArch64.  Is it OK?

So the COND_EXPRs are generated this way by if-conversion, right?  I
believe that
the MAX/MIN_EXPR form is always preferrable and thus it looks like if-conversion
might want to either directly generate it or make sure to fold the
introduced stmts
(and have a match.pd pattern catching this).

Richard.

> Thanks,
> bin
>
> 2016-10-11  Bin Cheng  
>
> * tree-vect-patterns.c (vect_recog_min_max_modify_pattern): New.
> (vect_vect_recog_func_ptrs): New element for above pattern.
> * tree-vectorizer.h (NUM_PATTERNS): Increase by 1.
>
> gcc/testsuite/ChangeLog
> 2016-10-11  Bin Cheng  
>
> * gcc.dg/vect/vect-umax-modify-pattern.c: New test.
> * gcc.dg/vect/vect-umin-modify-pattern.c: New test.

Re: [PATCH, PING] DWARF: space-optimize loc. descr. for integer literals on 32-bit targets

[Richard Biener]

On Wed, Aug 31, 2016 at 1:41 PM, Pierre-Marie de Rodat
 wrote:
> Hello,
>
> Ping for the patch submitted at
> . Also, here’s the
> update that Trevor suggested.

Ok.

Thanks,
Richard.

> Thanks!
>
> --
> Pierre-Marie de Rodat

Re: [PATCH, PING] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Richard Biener]

On Tue, Oct 11, 2016 at 4:39 PM, Pierre-Marie de Rodat
 wrote:
> On 09/28/2016 09:48 AM, Richard Biener wrote:
>>
>> Hmm, interesting approach.  It might work reliably at this point of
>> the compilation
>> but we do actually recycle cgraph nodes.
>
>
> Arf, what a pity. ;-)
>
>> So I wonder if given we do have
>> ->origin / ->next_nested in the cgraph if we can simply perform a walk in
>> the
>> appropriate order.
>
>
>> But then OTOH in my early LTO debug patchset I have
>> […]
>> which is not 100% equivalent (looking at the abstract origin) but it
>> sounds like a
>> similar issue.  So I wonder if doing the same for DECL_CONTEXT being a
>> function
>> makes sense here and would also fix your particular issue in a more
>> reliable way.
>
>
> I think it makes sense, and it seems to work, so many thanks for the
> suggestion! Bootstrapped and regtested successfuly on x86_64-linux. And this
> time, I did not forgot to attach the new testcase!

Ok.

Thanks,
Richard.

> --
> Pierre-Marie de Rodat

Re: [PATCH] Fold __builtin_str{n}{case}cmp functions (version 3)

[Richard Biener]

On Tue, Oct 11, 2016 at 11:33 AM, Martin Liška  wrote:
> Changes from the previous version:
>
> 1) Handle BUILT_IN_STRNCMP with length == -1.
> 2) Direct gimple stmts creation and usage gsi_replace_with_seq_vops.
> (hope using of replace_call_with_value is fine if replacing with a cst?)

yes

> 3) lhs == NULL cases are handled (Or is it fine to replace with a nop in 
> general?
> Can change a semantic as it may touch a memory.)
> 4) CFN_BUILT_IN_STRNCMP can handle strncmp (x, y, 0).
> 5) Handling of CFN_BUILT_IN_STRNCASECMP is added.
>
> Testing of the whole series has been running.

gimple_load_first_char needs a comment.

+  tree var = gimple_load_first_char (loc, str1, &stmts);
+  if (lhs)
+   stmt = gimple_build_assign (lhs, NOP_EXPR, var);
+  else
+   stmt = gimple_build_nop ();

I think you don't need the nop() as you have at least one stmt
from the load.  Likewise for the other cases below.

Otherwise this patch looks ok now.

Richard.



> Martin

Re: [PATCH GCC]New vectorization pattern turning cond_expr into max/min and plus/minus

[Bin.Cheng]

On Wed, Oct 12, 2016 at 9:12 AM, Richard Biener
 wrote:
> On Tue, Oct 11, 2016 at 5:03 PM, Bin Cheng  wrote:
>> Hi,
>> Given below test case,
>> int foo (unsigned short a[], unsigned int x)
>> {
>>   unsigned int i;
>>   for (i = 0; i < 1000; i++)
>> {
>>   x = a[i];
>>   a[i] = (unsigned short)(x >= 32768 ? x - 32768 : 0);
>> }
>>   return x;
>> }
>>
>> it now can be vectorized on AArch64, but generated assembly is way from 
>> optimal:
>> .L4:
>> ldr q4, [x3, x1]
>> add w2, w2, 1
>> cmp w2, w0
>> ushll   v1.4s, v4.4h, 0
>> ushll2  v0.4s, v4.8h, 0
>> add v3.4s, v1.4s, v6.4s
>> add v2.4s, v0.4s, v6.4s
>> cmhiv1.4s, v1.4s, v5.4s
>> cmhiv0.4s, v0.4s, v5.4s
>> and v1.16b, v3.16b, v1.16b
>> and v0.16b, v2.16b, v0.16b
>> xtn v2.4h, v1.4s
>> xtn2v2.8h, v0.4s
>> str q2, [x3, x1]
>> add x1, x1, 16
>> bcc .L4
>>
>> The vectorized loop has 15 instructions, which can be greatly simplified by 
>> turning cond_expr into max_expr, as below:
>> .L4:
>> ldr q1, [x3, x1]
>> add w2, w2, 1
>> cmp w2, w0
>> umaxv0.8h, v1.8h, v2.8h
>> add v0.8h, v0.8h, v2.8h
>> str q0, [x3, x1]
>> add x1, x1, 16
>> bcc .L4
>>
>> This patch addresses the issue by adding new vectorization pattern.
>> Bootstrap and test on x86_64 and AArch64.  Is it OK?
>
> So the COND_EXPRs are generated this way by if-conversion, right?  I
Though ?: is used in source code, yes, it is if-conv regenerated COND_EXPR.
> believe that
> the MAX/MIN_EXPR form is always preferrable and thus it looks like 
> if-conversion
> might want to either directly generate it or make sure to fold the
> introduced stmts
> (and have a match.pd pattern catching this).
Hmm, I also noticed saturation cases which should be better
transformed before vectorization in scalar optimizers.  But this case
is a bit different because there is additional computation involved
other than type conversion.  We need to prove the computation can be
done in either large or small types.  It is quite specific case and I
don't see good (general) solution in if-conv.  Vect-pattern looks like
a natural place doing this.  I am also looking at general saturation
cases, but this one is different?

Thanks,
bin
>
> Richard.
>
>> Thanks,
>> bin
>>
>> 2016-10-11  Bin Cheng  
>>
>> * tree-vect-patterns.c (vect_recog_min_max_modify_pattern): New.
>> (vect_vect_recog_func_ptrs): New element for above pattern.
>> * tree-vectorizer.h (NUM_PATTERNS): Increase by 1.
>>
>> gcc/testsuite/ChangeLog
>> 2016-10-11  Bin Cheng  
>>
>> * gcc.dg/vect/vect-umax-modify-pattern.c: New test.
>> * gcc.dg/vect/vect-umin-modify-pattern.c: New test.

Re: [PATCH] Fold __builtin_memchr (version 3)

[Richard Biener]

On Tue, Oct 11, 2016 at 11:38 AM, Martin Liška  wrote:
> On 10/07/2016 01:01 PM, Richard Biener wrote:
>> On Fri, Oct 7, 2016 at 10:41 AM, Martin Liška  wrote:
>>> Resending the patch, where I implemented folding in gimple-fold.c
>>>
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> +/* Fold a call to the str{n}{case}cmp builtin pointed by GSI iterator.
>> +   FCODE is the name of the builtin.  */
>>
>> wrong comment
>
> Fixed.
>
>>
>> +static bool
>> +gimple_fold_builtin_memchr (gimple_stmt_iterator *gsi)
>>
>> +   {
>> + replace_call_with_value (gsi, build_int_cst (type, 0));
>>
>> use ptr_type_node (void *) instead of type here and below.
>
> Done.
>
>>
>> + HOST_WIDE_INT offset = r - p1;
>> + if (compare_tree_int (len, offset) <= 0)
>> +   {
>>
>> == 0 can occur in which case we have to return a pointer to the
>> first char.  I think len < offset can't happen with memchr?
>
> Here I reworked the patch as it's not desired to trigger an undefined behavior
> in a host compiler for cases like: memchr ("", 'x', 5). Thus I switched to 
> strchr
> and aforementioned hunk would make sense.
>
>>
>> + replace_call_with_value (gsi, build_int_cst (type, 0));
>> + return true;
>> +   }
>> + else
>> +   {
>> + tree temp = fold_build_pointer_plus_hwi_loc (loc, arg1, 
>> offset);
>> + replace_call_with_value (gsi, temp);
>>
>> That yields valid GIMPLE by chance, I'd prefer if you'd built that to a
>> stmt and use the replace-with-vops.
>
> Done.
>
> Apart from that I added handling of lhs and the patch supports folding
> of CFN_BUILT_IN_MEMCHR.
>
> One question that comes to my mind is whether there's a possibility
> to fully test gimple folding of all cases if some of them are already
> eaten by generic folding?
>
> Tests of the series have been running.

Ok.

Thanks,
Richard.

> Martin
>
>>
>> + return true;
>> +   }
>>
>>
>>> Ready to be installed?
>>> Martin
>

Re: [PATCH] Test folding of str{n}{case}cmp and memchr (version 3)

[Richard Biener]

On Tue, Oct 11, 2016 at 11:38 AM, Martin Liška  wrote:
> Third iteration of tests, where I added both GENERIC and GIMPLE folding
> tests.

They should work already with -O1?

Otherwise ok.

Richard.

> Martin

Re: [PATCH] Fold __builtin_memchr (version 3)

[Richard Biener]

On Tue, Oct 11, 2016 at 11:38 AM, Martin Liška  wrote:
> One question that comes to my mind is whether there's a possibility
> to fully test gimple folding of all cases if some of them are already
> eaten by generic folding?

The only way is to make GENERIC folding not trigger by pushing
constants to temporaries.

Richard.

Re: [PATCH, PING] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Pierre-Marie de Rodat]

On 10/12/2016 10:15 AM, Richard Biener wrote:

Ok.

Thanks,


Committed. Thank you!

--
Pierre-Marie de Rodat

Re: [PATCH, PING] DWARF: space-optimize loc. descr. for integer literals on 32-bit targets

[Pierre-Marie de Rodat]

On 10/12/2016 10:13 AM, Richard Biener wrote:

Ok.

Thanks,


Committed. Thank you!

--
Pierre-Marie de Rodat

Re: [Patch 3/11] Implement TARGET_C_EXCESS_PRECISION for s390

[Andreas Krebbel]

On 10/07/2016 03:11 PM, Joseph Myers wrote:
> On Fri, 7 Oct 2016, Andreas Krebbel wrote:
> 
>> wrt (c): float_t appears to be more widely used than I expected. But the 
>> only hits which might indicate potential ABI problems where in clucene 
>> and libassa. (I've scanned the header files of about 25k Ubuntu source 
>> packages).
> 
> If it's two out of 25000 source packages whose ABIs might be affected, I 
> think that shows it's much safer as a change in glibc than moving to 
> _FILE_OFFSET_BITS=64 as a default (which I expect will happen when someone 
> puts the work in).  And probably safer than many past changes done through 
> symbol versioning.

Regarding (c) imagemagick is also affected (it wasn't really clear from my last 
email). Since it is
a widely used lib I think this counts as a blocker. The ABI relevant 
MagickRealType depends on the
size of float_t:

/*
  Float_t is not an ABI type.
*/
#if MAGICKCORE_SIZEOF_FLOAT_T == 0
typedef float MagickRealType;
#elif (MAGICKCORE_SIZEOF_FLOAT_T == MAGICKCORE_SIZEOF_FLOAT)
typedef float MagickRealType;
#elif (MAGICKCORE_SIZEOF_FLOAT_T == MAGICKCORE_SIZEOF_DOUBLE)
typedef double MagickRealType;
#elif (MAGICKCORE_SIZEOF_FLOAT_T == MAGICKCORE_SIZEOF_LONG_DOUBLE)
typedef long double MagickRealType;
#else
# error Your float_t type is neither a float, nor a double, nor a long double
#endif

So I would prefer (b) which looks like a good compromise to me.

-Andreas-

gcc-patches@gcc.gnu.org

[Richard Biener]

On Tue, Oct 11, 2016 at 11:34 PM, Marc Glisse  wrote:
> On Tue, 11 Oct 2016, Bin Cheng wrote:
>
>> We missed folding (convert)(X op const) -> (convert)X op (convert)const
>> for unsigned narrowing because of reason reported at
>> https://gcc.gnu.org/ml/gcc/2016-07/msg00126.html
>> This patch fixes the issue by adding new match&simplify pattern, it also
>> adds a test case.  This is the prerequisite patch for next patch adding new
>> vectorization pattern.
>
>
> Some technical comments below. I am sure Jeff and/or Richi will have more to
> say on the approach. I am a bit surprised to see it as adding a new
> transformation, instead of moving an old one.

The "old one" would be c-family/c-common.c:shorten_binary_op.  It's generally
prefered to move stuff, preserving semantics.

We do have a similar transform for MULT_EXPR in fold-const.c which also
applies to non-constant 2nd operand (likewise for shorten_binary_op).
The general issue with these "narrowings" is that it loses overflow information
if the original op was carried out in a TYPE_OVERFLOW_UNDEFINED type.

There is also already a bunch of similar match.pd patterns here:

/* Narrowing of arithmetic and logical operations.

   These are conceptually similar to the transformations performed for
   the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
   term we want to move all that code out of the front-ends into here.  */

/* If we have a narrowing conversion of an arithmetic operation where
   both operands are widening conversions from the same type as the outer
   narrowing conversion.  Then convert the innermost operands to a suitable
   unsigned type (to avoid introducing undefined behavior), perform the
   operation and convert the result to the desired type.  */
(for op (plus minus)
  (simplify
(convert (op:s (convert@2 @0) (convert@3 @1)))
...

so it might be possible to amend these or at least move your pattern next
to those.

> +/* (convert (X op C)) -> ((convert)X op (convert)C) if it is narrowing
> +   conversion and both types wrap when overflow.  */
> +(for op (plus minus)
> +  (simplify
>
> We used to indent by a single space in this file, but I see that other
> transforms have made it in with double spacing, so I guess it doesn't
> matter.
>
> +(convert (op @0 @1))
> +(if (INTEGRAL_TYPE_P (type)
> +&& TYPE_OVERFLOW_WRAPS (type)
> +&& TREE_CODE (@1) == INTEGER_CST
>
> You can write (convert (op @0 INTEGER_CST@1)) and skip this line.
>
> +&& INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +&& TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
>
> This seems quite restrictive, TYPE_OVERFLOW_UNDEFINED should also be fine
> for this type. I guess you are trying to avoid saturating / trapping types?

Maybe he's avoiding the dropping of overflow info ... at least it
warrants a comment.

Richard.

> +&& TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0)))
> + (op (convert @0) (convert @1)
> +
>  /* If we have a narrowing conversion to an integral type that is fed by a
> BIT_AND_EXPR, we might be able to remove the BIT_AND_EXPR if it merely
> masks off bits outside the final type (and nothing else).  */
> diff --git a/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> b/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> new file mode 100644
> index 000..aff96a9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-gimple" } */
> +
> +unsigned char foo (unsigned short s)
> +{
> +  return (unsigned char)(s + 65530);
> +}
> +/* { dg-final { scan-tree-dump-not " 65530" "gimple" } } */
>
> As I understand it, C says that s is promoted to int and added to 65530, but
> int is not TYPE_OVERFLOW_WRAPS so your transformation doesn't apply (the
> test already passes without your patch). It is better to write tests for the
> gimple version of transformations, i.e. don't write everything as a single
> expression, use intermediate variables.
>
> --
> Marc Glisse

Re: [PATCH GCC]New vectorization pattern turning cond_expr into max/min and plus/minus

[Richard Biener]

On Wed, Oct 12, 2016 at 10:29 AM, Bin.Cheng  wrote:
> On Wed, Oct 12, 2016 at 9:12 AM, Richard Biener
>  wrote:
>> On Tue, Oct 11, 2016 at 5:03 PM, Bin Cheng  wrote:
>>> Hi,
>>> Given below test case,
>>> int foo (unsigned short a[], unsigned int x)
>>> {
>>>   unsigned int i;
>>>   for (i = 0; i < 1000; i++)
>>> {
>>>   x = a[i];
>>>   a[i] = (unsigned short)(x >= 32768 ? x - 32768 : 0);
>>> }
>>>   return x;
>>> }
>>>
>>> it now can be vectorized on AArch64, but generated assembly is way from 
>>> optimal:
>>> .L4:
>>> ldr q4, [x3, x1]
>>> add w2, w2, 1
>>> cmp w2, w0
>>> ushll   v1.4s, v4.4h, 0
>>> ushll2  v0.4s, v4.8h, 0
>>> add v3.4s, v1.4s, v6.4s
>>> add v2.4s, v0.4s, v6.4s
>>> cmhiv1.4s, v1.4s, v5.4s
>>> cmhiv0.4s, v0.4s, v5.4s
>>> and v1.16b, v3.16b, v1.16b
>>> and v0.16b, v2.16b, v0.16b
>>> xtn v2.4h, v1.4s
>>> xtn2v2.8h, v0.4s
>>> str q2, [x3, x1]
>>> add x1, x1, 16
>>> bcc .L4
>>>
>>> The vectorized loop has 15 instructions, which can be greatly simplified by 
>>> turning cond_expr into max_expr, as below:
>>> .L4:
>>> ldr q1, [x3, x1]
>>> add w2, w2, 1
>>> cmp w2, w0
>>> umaxv0.8h, v1.8h, v2.8h
>>> add v0.8h, v0.8h, v2.8h
>>> str q0, [x3, x1]
>>> add x1, x1, 16
>>> bcc .L4
>>>
>>> This patch addresses the issue by adding new vectorization pattern.
>>> Bootstrap and test on x86_64 and AArch64.  Is it OK?
>>
>> So the COND_EXPRs are generated this way by if-conversion, right?  I
> Though ?: is used in source code, yes, it is if-conv regenerated COND_EXPR.
>> believe that
>> the MAX/MIN_EXPR form is always preferrable and thus it looks like 
>> if-conversion
>> might want to either directly generate it or make sure to fold the
>> introduced stmts
>> (and have a match.pd pattern catching this).
> Hmm, I also noticed saturation cases which should be better
> transformed before vectorization in scalar optimizers.  But this case
> is a bit different because there is additional computation involved
> other than type conversion.  We need to prove the computation can be
> done in either large or small types.  It is quite specific case and I
> don't see good (general) solution in if-conv.  Vect-pattern looks like
> a natural place doing this.  I am also looking at general saturation
> cases, but this one is different?

(vect-patterns should go away ...)

But as if-conversion results may also prevail for scalar code doing the
pattern in match.pd would be better - that is, "apply" the pattern
already during if-conversion.

Yes, if-conversion fails to fold the stmts it generates (it only uses
generic folding on the trees it builds - it can need some TLC here).

Richard.

> Thanks,
> bin
>>
>> Richard.
>>
>>> Thanks,
>>> bin
>>>
>>> 2016-10-11  Bin Cheng  
>>>
>>> * tree-vect-patterns.c (vect_recog_min_max_modify_pattern): New.
>>> (vect_vect_recog_func_ptrs): New element for above pattern.
>>> * tree-vectorizer.h (NUM_PATTERNS): Increase by 1.
>>>
>>> gcc/testsuite/ChangeLog
>>> 2016-10-11  Bin Cheng  
>>>
>>> * gcc.dg/vect/vect-umax-modify-pattern.c: New test.
>>> * gcc.dg/vect/vect-umin-modify-pattern.c: New test.

Re: [ARM] PR 67591 ARM v8 Thumb IT blocks are deprecated

[Christophe Lyon]

Hi Kyrill,

On 7 October 2016 at 17:00, Kyrill Tkachov  wrote:
> Hi Christophe,
>
>
> On 07/09/16 21:05, Christophe Lyon wrote:
>>
>> Hi,
>>
>> The attached patch is a first part to solve PR 67591: it removes
>> several occurrences of "IT blocks containing 32-bit Thumb
>> instructions are deprecated in ARMv8" messages in the
>> gcc/g++/libstdc++/fortran testsuites.
>>
>> It does not remove them all yet. This patch only modifies the
>> *cmp_and, *cmp_ior, *ior_scc_scc, *ior_scc_scc_cmp,
>> *and_scc_scc and *and_scc_scc_cmp patterns.
>> Additional work is required in sub_shiftsi etc, at least.
>> I've started looking at these, but I decided I could already
>> post this self-contained patch to check if this implementation
>> is OK.
>>
>> Regarding *cmp_and and *cmp_ior patterns, the addition of the
>> enabled_for_depr_it attribute is aggressive in the sense that it keeps
>> only the alternatives with 'l' and 'Py' constraints, while in some
>> cases the constraints could be relaxed. Indeed, these 2 patterns can
>> swap their input comparisons, meaning that any of them can be emitted
>> in the IT-block, and is thus subject to the ARMv8 deprecation.
>> The generated code is possibly suboptimal in the cases where the
>> operands are not swapped, since 'r' could be used.
>>
>> Cross-tested on arm-none-linux-gnueabihf with -mthumb/-march=armv8-a
>> and --with-cpu=cortex-a57 --with-mode=thumb, showing only improvements:
>>
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/239850-depr-it-4/report-build-info.html
>>
>> Bootstrapped OK on armv8l HW.
>>
>> Is this OK?
>>
>> Thanks,
>>
>> Christophe
>
>
>  (define_insn_and_split "*ior_scc_scc"
> -  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
> +  [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
> (ior:SI (match_operator:SI 3 "arm_comparison_operator"
> -[(match_operand:SI 1 "s_register_operand" "r")
> - (match_operand:SI 2 "arm_add_operand" "rIL")])
> +[(match_operand:SI 1 "s_register_operand" "r,l")
> + (match_operand:SI 2 "arm_add_operand" "rIL,lPy")])
> (match_operator:SI 6 "arm_comparison_operator"
> -[(match_operand:SI 4 "s_register_operand" "r")
> - (match_operand:SI 5 "arm_add_operand" "rIL")])))
> +[(match_operand:SI 4 "s_register_operand" "r,l")
> + (match_operand:SI 5 "arm_add_operand" "rIL,lPy")])))
>
> Can you please put the more restrictive alternatives (lPy) first?
> Same with the other patterns your patch touches.
> Ok with that change if a rebased testing run is ok.
> Sorry for the delay in reviewing.
>

OK, I will update my patch accordingly.

However, when I discussed this with Ramana during the Cauldron,
he requested benchmark results. So far, I was able to run spec2006
on an APM machine, and I'm seeing performance changes in the
range 11% improvement (465.tonto) to 7% degradation (433.milc).

Would that be acceptable?

The number of warnings (IT blocks containing 32-bit Thumb instructions
are deprecated in ARMv8)
was 712 without my patch and 122 with it. (using the hosts's binutils
2.24/ubuntu).
I expected some warning, since as I said earlier other patterns need
to be updated.

Christophe


> Thanks for improving this area!
> Kyrill
>

Re: [ARM] PR 67591 ARM v8 Thumb IT blocks are deprecated

[Kyrill Tkachov]

On 12/10/16 09:59, Christophe Lyon wrote:

Hi Kyrill,

On 7 October 2016 at 17:00, Kyrill Tkachov  wrote:

Hi Christophe,


On 07/09/16 21:05, Christophe Lyon wrote:

Hi,

The attached patch is a first part to solve PR 67591: it removes
several occurrences of "IT blocks containing 32-bit Thumb
instructions are deprecated in ARMv8" messages in the
gcc/g++/libstdc++/fortran testsuites.

It does not remove them all yet. This patch only modifies the
*cmp_and, *cmp_ior, *ior_scc_scc, *ior_scc_scc_cmp,
*and_scc_scc and *and_scc_scc_cmp patterns.
Additional work is required in sub_shiftsi etc, at least.
I've started looking at these, but I decided I could already
post this self-contained patch to check if this implementation
is OK.

Regarding *cmp_and and *cmp_ior patterns, the addition of the
enabled_for_depr_it attribute is aggressive in the sense that it keeps
only the alternatives with 'l' and 'Py' constraints, while in some
cases the constraints could be relaxed. Indeed, these 2 patterns can
swap their input comparisons, meaning that any of them can be emitted
in the IT-block, and is thus subject to the ARMv8 deprecation.
The generated code is possibly suboptimal in the cases where the
operands are not swapped, since 'r' could be used.

Cross-tested on arm-none-linux-gnueabihf with -mthumb/-march=armv8-a
and --with-cpu=cortex-a57 --with-mode=thumb, showing only improvements:

http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/239850-depr-it-4/report-build-info.html

Bootstrapped OK on armv8l HW.

Is this OK?

Thanks,

Christophe


  (define_insn_and_split "*ior_scc_scc"
-  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
+  [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
 (ior:SI (match_operator:SI 3 "arm_comparison_operator"
-[(match_operand:SI 1 "s_register_operand" "r")
- (match_operand:SI 2 "arm_add_operand" "rIL")])
+[(match_operand:SI 1 "s_register_operand" "r,l")
+ (match_operand:SI 2 "arm_add_operand" "rIL,lPy")])
 (match_operator:SI 6 "arm_comparison_operator"
-[(match_operand:SI 4 "s_register_operand" "r")
- (match_operand:SI 5 "arm_add_operand" "rIL")])))
+[(match_operand:SI 4 "s_register_operand" "r,l")
+ (match_operand:SI 5 "arm_add_operand" "rIL,lPy")])))

Can you please put the more restrictive alternatives (lPy) first?
Same with the other patterns your patch touches.
Ok with that change if a rebased testing run is ok.
Sorry for the delay in reviewing.


OK, I will update my patch accordingly.

However, when I discussed this with Ramana during the Cauldron,
he requested benchmark results. So far, I was able to run spec2006
on an APM machine, and I'm seeing performance changes in the
range 11% improvement (465.tonto) to 7% degradation (433.milc).

Would that be acceptable?


Those sound like quite large swings.
Are you sure the machine was not running anything else at the time
or playing tricks with frequency scaling?
Did all iterations of SPEC show a consistent difference?

If the changes are consistent, could you have a look at the codegen
to see if there are any clues to the differences?

I'd like to get an explanation for these differences before committing
this patch. If they are just an effect of the more restrictive constraints
then there may be not much we can do, but I'd like to make sure there's not
anything else unexpected going on.



The number of warnings (IT blocks containing 32-bit Thumb instructions
are deprecated in ARMv8)
was 712 without my patch and 122 with it. (using the hosts's binutils
2.24/ubuntu).
I expected some warning, since as I said earlier other patterns need
to be updated.


Understood. That's fine.

Thanks,
Kyrill



Christophe



Thanks for improving this area!
Kyrill

Re: [ARM] PR 67591 ARM v8 Thumb IT blocks are deprecated

[Christophe Lyon]

On 12 October 2016 at 11:14, Kyrill Tkachov  wrote:
>
> On 12/10/16 09:59, Christophe Lyon wrote:
>>
>> Hi Kyrill,
>>
>> On 7 October 2016 at 17:00, Kyrill Tkachov 
>> wrote:
>>>
>>> Hi Christophe,
>>>
>>>
>>> On 07/09/16 21:05, Christophe Lyon wrote:

 Hi,

 The attached patch is a first part to solve PR 67591: it removes
 several occurrences of "IT blocks containing 32-bit Thumb
 instructions are deprecated in ARMv8" messages in the
 gcc/g++/libstdc++/fortran testsuites.

 It does not remove them all yet. This patch only modifies the
 *cmp_and, *cmp_ior, *ior_scc_scc, *ior_scc_scc_cmp,
 *and_scc_scc and *and_scc_scc_cmp patterns.
 Additional work is required in sub_shiftsi etc, at least.
 I've started looking at these, but I decided I could already
 post this self-contained patch to check if this implementation
 is OK.

 Regarding *cmp_and and *cmp_ior patterns, the addition of the
 enabled_for_depr_it attribute is aggressive in the sense that it keeps
 only the alternatives with 'l' and 'Py' constraints, while in some
 cases the constraints could be relaxed. Indeed, these 2 patterns can
 swap their input comparisons, meaning that any of them can be emitted
 in the IT-block, and is thus subject to the ARMv8 deprecation.
 The generated code is possibly suboptimal in the cases where the
 operands are not swapped, since 'r' could be used.

 Cross-tested on arm-none-linux-gnueabihf with -mthumb/-march=armv8-a
 and --with-cpu=cortex-a57 --with-mode=thumb, showing only improvements:


 http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/239850-depr-it-4/report-build-info.html

 Bootstrapped OK on armv8l HW.

 Is this OK?

 Thanks,

 Christophe
>>>
>>>
>>>   (define_insn_and_split "*ior_scc_scc"
>>> -  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
>>> +  [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
>>>  (ior:SI (match_operator:SI 3 "arm_comparison_operator"
>>> -[(match_operand:SI 1 "s_register_operand" "r")
>>> - (match_operand:SI 2 "arm_add_operand" "rIL")])
>>> +[(match_operand:SI 1 "s_register_operand" "r,l")
>>> + (match_operand:SI 2 "arm_add_operand" "rIL,lPy")])
>>>  (match_operator:SI 6 "arm_comparison_operator"
>>> -[(match_operand:SI 4 "s_register_operand" "r")
>>> - (match_operand:SI 5 "arm_add_operand" "rIL")])))
>>> +[(match_operand:SI 4 "s_register_operand" "r,l")
>>> + (match_operand:SI 5 "arm_add_operand" "rIL,lPy")])))
>>>
>>> Can you please put the more restrictive alternatives (lPy) first?
>>> Same with the other patterns your patch touches.
>>> Ok with that change if a rebased testing run is ok.
>>> Sorry for the delay in reviewing.
>>>
>> OK, I will update my patch accordingly.
>>
>> However, when I discussed this with Ramana during the Cauldron,
>> he requested benchmark results. So far, I was able to run spec2006
>> on an APM machine, and I'm seeing performance changes in the
>> range 11% improvement (465.tonto) to 7% degradation (433.milc).
>>
>> Would that be acceptable?
>
>
> Those sound like quite large swings.
Indeed, but most are in the -1%-+1% range.

> Are you sure the machine was not running anything else at the time
> or playing tricks with frequency scaling?
No, I had no such guarantee. I used this machine temporarily,
first to check that bootstrap worked. I planed to use another
board with an A57 "standard" microarch for proper
benchmarking, but I'm not sure when I'll have access to it
(wrt to e/o gcc stage1), that's why I reported these early
figures.

> Did all iterations of SPEC show a consistent difference?
>
> If the changes are consistent, could you have a look at the codegen
> to see if there are any clues to the differences?
I will update my patch according to your comment, re-run the bench
and have a deeper look at the codegen differences.

> I'd like to get an explanation for these differences before committing
> this patch. If they are just an effect of the more restrictive constraints
> then there may be not much we can do, but I'd like to make sure there's not
> anything else unexpected going on.
>
Thanks,

Christophe

>>
>> The number of warnings (IT blocks containing 32-bit Thumb instructions
>> are deprecated in ARMv8)
>> was 712 without my patch and 122 with it. (using the hosts's binutils
>> 2.24/ubuntu).
>> I expected some warning, since as I said earlier other patterns need
>> to be updated.
>
>
> Understood. That's fine.
>
> Thanks,
> Kyrill
>
>
>>
>> Christophe
>>
>>
>>> Thanks for improving this area!
>>> Kyrill
>>>
>

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Markus Trippelsdorf]

On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:
> On 10/11/2016 11:52 PM, Jakub Jelinek wrote:
> > The following patch introduces difference warning levels for
> > -Wimplicit-fallthrough warning, so projects can choose if they want to
> > honor only attributes (-Wimplicit-fallthrough=5), or what kind of comments.
> > =4 is very picky and accepts only very small amount of comments, =3 is what
> > we had before this patch, =2 looks case insensitively for falls?[ 
> > \t-]*thr(u|ough)
> > anywhere in the comment, =1 accepts any comment, =0 is the same as
> > -Wno-implicit-fallthrough - disables the warning.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> I think this is ok, and thank you very much for doing this.
> 
> > The patch keeps as the default the current forms, I'm not against changing
> > it to =2 if there is consensus on it, but would strongly prefer doing that
> > incrementally, as e.g. we'll need to adjust the testsuite for that, and
> > perhaps also use =3 as the warning for gcc bootstraps when we are already =3
> > mode clear.
> 
> It's a discussion we should have, but I agree it should be done
> incrementally. I would argue for =1 as the default.

Here are some numbers for an allmodconfig Linux kernel on pcc64le:

-Wimplicit-fallthrough=1 :  951 warnings
-Wimplicit-fallthrough=2 : 1087 warnings
-Wimplicit-fallthrough=3 : 1209 warnings

I randomly looked at the differences and almost all additional
-Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
And _all_ additional -Wimplicit-fallthrough=3 warnings appear
to be bogus.

-- 
Markus

PING! [Fortran, Patch, PR72832, v1] [6/7 Regression] [OOP] ALLOCATE with SOURCE fails to allocate requested dimensions

[Andre Vehreschild]

Ping!

Updated patch with the comments gotten so far.

Ok for trunk?

- Andre

On Fri, 2 Sep 2016 09:59:19 +0200
Andre Vehreschild  wrote:

> Hi all,
> 
> attached patch fixes the issue raised by PR72832. The issue was that
> the array descriptor of the SOURCE= in an ALLOCATE () was used to
> allocate an array object although an explicit array spec had been
> given.
> 
> The initial test showed a second issue when a class array was copied.
> Compiling the code with -fcheck=bounds showed that no boundary check
> was generated for class array copying using gfc_copy_class_to_class().
> I have added the generation of a runtime boundary check when the
> -fcheck=bounds flag is set to locate the current issue. The test
> allocate_with_source_23 is compiled with fcheck=bounds and fails as
> expected ({ xfail *-*-* } set).
> 
> Fixing the both issues unfortunately raised the next one, when trying
> to get the size of a class array returned from a function (testcase:
> allocate_with_source_11.f08). Here the issue was that for functions
> returning class arrays gfc_conv_expr_descriptor () relied on the
> descriptor being magicked into the scalarizer, which did not work in
> this use case. Due to the first issue this bug did not raise beforehand.
> Because I could not figure how to do it right in
> gfc_conv_expr_descriptor (), I found a way to circumvent the issue by
> getting the reference of the result of the function returning a class
> array and massaging it to be ok for size (). This works quite neatly,
> but may be someone with better knowledge of conv_expr_descriptor and
> the scalarizer might want to fix it there. I suppose there are more
> locations in the code, that work around this issue.
> 
> Bootstrapped and regtests ok on x86_64-linux-gnu/F23 for trunk and
> gcc-6. Ok for both?
> 
> - Andre


-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
gcc/fortran/ChangeLog:

2016-09-01  Andre Vehreschild  

PR fortran/72832
* trans-expr.c (gfc_copy_class_to_class): Add generation of
runtime array bounds check.
* trans-intrinsic.c (gfc_conv_intrinsic_size): Add a crutch to
get the descriptor of a function returning a class object.
* trans-stmt.c (gfc_trans_allocate): Use the array spec on the
array to allocate instead of the array spec from source=.

gcc/testsuite/ChangeLog:

2016-09-01  Andre Vehreschild  

PR fortran/72832
* gfortran.dg/allocate_with_source_22.f03: New test.
* gfortran.dg/allocate_with_source_23.f03: New test.  Expected to
fail.


diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 1de2818..5486ec6 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -1237,6 +1237,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
   stmtblock_t body;
   stmtblock_t ifbody;
   gfc_loopinfo loop;
+  tree orig_nelems = nelems; /* Needed for bounds check.  */
 
   gfc_init_block (&body);
   tmp = fold_build2_loc (input_location, MINUS_EXPR,
@@ -1264,6 +1265,31 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
 	}
   vec_safe_push (args, to_ref);
 
+  /* Add bounds check.  */
+  if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) > 0 && is_from_desc)
+	{
+	  char *msg;
+	  const char *name = "<>";
+	  tree from_len;
+
+	  if (DECL_P (to))
+	name = (const char *)(DECL_NAME (to)->identifier.id.str);
+
+	  from_len = gfc_conv_descriptor_size (from_data, 1);
+	  tmp = fold_build2_loc (input_location, NE_EXPR,
+  boolean_type_node, from_len, orig_nelems);
+	  msg = xasprintf ("Array bound mismatch for dimension %d "
+			   "of array '%s' (%%ld/%%ld)",
+			   1, name);
+
+	  gfc_trans_runtime_check (true, false, tmp, &body,
+   &gfc_current_locus, msg,
+			 fold_convert (long_integer_type_node, orig_nelems),
+			   fold_convert (long_integer_type_node, from_len));
+
+	  free (msg);
+	}
+
   tmp = build_call_vec (fcn_type, fcn, args);
 
   /* Build the body of the loop.  */
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index a499c32..9d5e33c 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -6544,9 +6544,20 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr)
   if (actual->expr->ts.type == BT_CLASS)
 gfc_add_class_array_ref (actual->expr);
 
-  argse.want_pointer = 1;
   argse.data_not_needed = 1;
-  gfc_conv_expr_descriptor (&argse, actual->expr);
+  if (gfc_is_alloc_class_array_function (actual->expr))
+{
+  /* For functions that return a class array conv_expr_descriptor is not
+	 able to get the descriptor right.  Therefore this special case.  */
+  gfc_conv_expr_reference (&argse, actual->expr);
+  argse.expr = gfc_build_addr_expr (NULL_TREE,
+	gfc_class_data_get (argse.expr));
+}
+  else
+{
+  argse.want_pointer = 1;
+  gfc_conv_expr_descriptor (&argse, actual->expr);
+}
  

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Bernd Schmidt]

On 10/12/2016 11:31 AM, Markus Trippelsdorf wrote:

On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:

It's a discussion we should have, but I agree it should be done
incrementally. I would argue for =1 as the default.


Here are some numbers for an allmodconfig Linux kernel on pcc64le:

-Wimplicit-fallthrough=1 :  951 warnings
-Wimplicit-fallthrough=2 : 1087 warnings
-Wimplicit-fallthrough=3 : 1209 warnings

I randomly looked at the differences and almost all additional
-Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
And _all_ additional -Wimplicit-fallthrough=3 warnings appear
to be bogus.


And that's for a codebase that was written in English to begin with. 
Would you mind posting one or two examples if you saw interesting ones, 
for reference?


This result suggests that we should probably collapse levels 3-5 into a 
single strict one that doesn't try to be clever, and definitely make at 
most level 1 the default.


Another thing, was it ever resolved what this warning does to tools like 
ccache which like to operate on preprocessed files?



Bernd

Re: [PATCH][check_GNU_style.sh] More aggressively ignore dg-xxx directives

[Kyrill Tkachov]

On 11/10/16 20:19, Jakub Jelinek wrote:

On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:

Also, the pattern that starts with "/\+\+\+" looks like it's missing
the ^ anchor.  Presumably it should be "/^\+\+\+ \/testsuite\//".

No, it will be almost never +++ /testsuite/
There needs to be .* in between "+++ " and "/testsuite/", and perhaps
it should also ignore "+++ testsuite/".
So /^\+\+\+ (.*\/)?testsuite\// ?
Also, normally (when matching $0) there won't be newlines in the text.

Jakub


Thanks.
Here is the updated patch with your suggestions.

2016-10-12  Kyrylo Tkachov  

* check_GNU_style.sh (remove_testsuite): New function.
Use it to remove testsuite from the diff.
commit e4af5426c86a00a05fc08109fa84bfe30285f2a2
Author: Kyrylo Tkachov 
Date:   Tue Oct 11 11:33:50 2016 +0100

[check_GNU_style.sh] Ignore testsuite

diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index 87a276c..2787e9b 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -81,7 +81,17 @@ if [ $nfiles -eq 1 ]; then
 else
 format="-nH"
 fi
+
+# Remove the testsuite part of the diff.  We don't care about GNU style
+# in testcases and the dg-* directives give too many false positives.
+remove_testsuite ()
+{
+  awk 'BEGIN{testsuite=0} /^\+\+\+ / && ! /testsuite\//{testsuite=0} \
+   {if (!testsuite) print} /^\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
+}
+
 grep $format '^+' $files \
+| remove_testsuite \
 | grep -v ':+++' \
 > $inp
 
@@ -160,8 +170,9 @@ col (){
 	fi
 
 	# Don't reuse $inp, which may be generated using -H and thus contain a
-	# file prefix.
-	grep -n '^+' $f \
+	# file prefix.  Re-remove the testsuite since we're not using $inp.
+	remove_testsuite $f \
+	| grep -n '^+' \
 	| grep -v ':+++' \
 	> $tmp
 
@@ -174,11 +185,10 @@ col (){
 	# Expand tabs to spaces according to tab positions.
 	# Keep long lines, make short lines empty.  Print the part past 80 chars
 	# in red.
-# Don't complain about dg-xxx directives in tests.
 	cat "$tmp" \
 	| sed 's/^[0-9]*:+//' \
 	| expand \
-	| awk '$0 !~ /{[[:space:]]*dg-(error|warning|message)[[:space:]]/ { \
+	| awk '{ \
 		 if (length($0) > 80) \
 		   printf "%s\033[1;31m%s\033[0m\n", \
 			  substr($0,1,80), \

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Markus Trippelsdorf]

On 2016.10.12 at 11:52 +0200, Bernd Schmidt wrote:
> On 10/12/2016 11:31 AM, Markus Trippelsdorf wrote:
> > On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:
> > > It's a discussion we should have, but I agree it should be done
> > > incrementally. I would argue for =1 as the default.
> >
> > Here are some numbers for an allmodconfig Linux kernel on pcc64le:
> >
> > -Wimplicit-fallthrough=1 :  951 warnings
> > -Wimplicit-fallthrough=2 : 1087 warnings
> > -Wimplicit-fallthrough=3 : 1209 warnings
> >
> > I randomly looked at the differences and almost all additional
> > -Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
> > And _all_ additional -Wimplicit-fallthrough=3 warnings appear
> > to be bogus.
>
> And that's for a codebase that was written in English to begin with. Would
> you mind posting one or two examples if you saw interesting ones, for
> reference?

Actually looking more closely it appears that all of the 136 additional
warnings for level 2 are bogus, too. Here is an example:

 switch (class) {
 case ATA_DEV_SEMB:
 class = ATA_DEV_ATA;/* some hard drives report SEMB sig */
 case ATA_DEV_ATA:
 case ATA_DEV_ZAC:
 tf.command = ATA_CMD_ID_ATA;
 break;

> This result suggests that we should probably collapse levels 3-5 into a
> single strict one that doesn't try to be clever, and definitely make at most
> level 1 the default.

Yes, I agree. 

--
Markus

Re: [Patch 3/11] Implement TARGET_C_EXCESS_PRECISION for s390

[Joseph Myers]

On Wed, 12 Oct 2016, Andreas Krebbel wrote:

> Regarding (c) imagemagick is also affected (it wasn't really clear from 
> my last email). Since it is a widely used lib I think this counts as a 
> blocker. The ABI relevant MagickRealType depends on the size of float_t:

I think distributions manage ABI transitions for such libraries all the 
time (and could well choose to patch it locally to defer the change until 
it next changes SONAME upstream anyway).

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

Re: [PATCH][check_GNU_style.sh] More aggressively ignore dg-xxx directives

[Kyrill Tkachov]

On 12/10/16 10:57, Kyrill Tkachov wrote:


On 11/10/16 20:19, Jakub Jelinek wrote:

On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:

Also, the pattern that starts with "/\+\+\+" looks like it's missing
the ^ anchor.  Presumably it should be "/^\+\+\+ \/testsuite\//".

No, it will be almost never +++ /testsuite/
There needs to be .* in between "+++ " and "/testsuite/", and perhaps
it should also ignore "+++ testsuite/".
So /^\+\+\+ (.*\/)?testsuite\// ?
Also, normally (when matching $0) there won't be newlines in the text.

Jakub


Thanks.
Here is the updated patch with your suggestions.



Actually, I've encountered a problem:

 85 # Remove the testsuite part of the diff.  We don't care about GNU style
 86 # in testcases and the dg-* directives give too many false positives.
 87 remove_testsuite ()
 88 {
 89   awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
 90{if (!testsuite) print} /^\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
 91 }
 92
 93 grep $format '^+' $files \
 94 | remove_testsuite \
 95 | grep -v ':+++' \
 96 > $inp


The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor is used.
The awk command matches fine by itself but not when fed from the "grep $format '^+' 
$files"
command because grep adds the line numbers and file names.
So is it okay to omit the ^ here?

Thanks,
Kyrill

[PATCH][AArch64] Add function comments to some prologue/epilogue helpers

[Kyrill Tkachov]

Hi all,

I'm looking at the aarch64 prologue and epilogue generation code and I noticed 
many of the helper
functions don't have function comments so it makes it harder than it has to to 
understand what's going on.
This patch adds function comments to some of them. I hope I understood the 
functions correctly.

Is this ok for trunk?

Thanks,
Kyrill

2016-10-12  Kyrylo Tkachov  

* config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add
function comment.
(aarch64_next_callee_save): Likewise.
(aarch64_pushwb_single_reg): Likewise.
(aarch64_gen_storewb_pair): Likewise.
(aarch64_push_regs): Likewise.
(aarch64_gen_loadwb_pair): Likewise.
(aarch64_pop_regs): Likewise.
(aarch64_gen_store_pair): Likewise.
(aarch64_gen_load_pair): Likewise.
(aarch64_save_callee_saves): Likewise.
(aarch64_restore_callee_saves): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e7556632901177c04f9884be4f3ee40e5f677917..87bb5407245a7e902a2153a3ed963cead3aa4bcc 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2896,12 +2896,18 @@ aarch64_layout_frame (void)
   cfun->machine->frame.laid_out = true;
 }
 
+/* Return true if the register REGNO is saved on entry to
+   the current function.  */
+
 static bool
 aarch64_register_saved_on_entry (int regno)
 {
   return cfun->machine->frame.reg_offset[regno] >= 0;
 }
 
+/* Return the next register up from REGNO up to LIMIT for the callee
+   to save.  */
+
 static unsigned
 aarch64_next_callee_save (unsigned regno, unsigned limit)
 {
@@ -2910,6 +2916,9 @@ aarch64_next_callee_save (unsigned regno, unsigned limit)
   return regno;
 }
 
+/* Push the register number REGNO of mode MODE to the stack with write-back
+   adjusting the stack by ADJUSTMENT.  */
+
 static void
 aarch64_pushwb_single_reg (machine_mode mode, unsigned regno,
 			   HOST_WIDE_INT adjustment)
@@ -2926,6 +2935,10 @@ aarch64_pushwb_single_reg (machine_mode mode, unsigned regno,
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
+/* Generate and return an instruction to store the pair of registers
+   REG and REG2 of mode MODE to location BASE with write-back adjusting
+   the stack location BASE by ADJUSTMENT.  */
+
 static rtx
 aarch64_gen_storewb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2,
 			  HOST_WIDE_INT adjustment)
@@ -2945,6 +2958,9 @@ aarch64_gen_storewb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2,
 }
 }
 
+/* Push registers numbered REGNO1 and REGNO2 to the stack, adjusting the
+   stack pointer by ADJUSTMENT.  */
+
 static void
 aarch64_push_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment)
 {
@@ -2964,6 +2980,9 @@ aarch64_push_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment)
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
+/* Load the pair of register REG, REG2 of mode MODE from stack location BASE,
+   adjusting it by ADJUSTMENT afterwards.  */
+
 static rtx
 aarch64_gen_loadwb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2,
 			 HOST_WIDE_INT adjustment)
@@ -2981,6 +3000,10 @@ aarch64_gen_loadwb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2,
 }
 }
 
+/* Pop the two registers numbered REGNO1, REGNO2 from the stack, adjusting it
+   afterwards by ADJUSTMENT and writing the appropriate REG_CFA_RESTORE notes
+   into CFI_OPS.  */
+
 static void
 aarch64_pop_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment,
 		  rtx *cfi_ops)
@@ -3005,6 +3028,9 @@ aarch64_pop_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment,
 }
 }
 
+/* Generate and return a store pair instruction of mode MODE to store
+   register REG1 to MEM1 and register REG2 to MEM2.  */
+
 static rtx
 aarch64_gen_store_pair (machine_mode mode, rtx mem1, rtx reg1, rtx mem2,
 			rtx reg2)
@@ -3022,6 +3048,9 @@ aarch64_gen_store_pair (machine_mode mode, rtx mem1, rtx reg1, rtx mem2,
 }
 }
 
+/* Generate and regurn a load pair isntruction of mode MODE to load register
+   REG1 from MEM1 and register REG2 from MEM2.  */
+
 static rtx
 aarch64_gen_load_pair (machine_mode mode, rtx reg1, rtx mem1, rtx reg2,
 		   rtx mem2)
@@ -3039,6 +3068,9 @@ aarch64_gen_load_pair (machine_mode mode, rtx reg1, rtx mem1, rtx reg2,
 }
 }
 
+/* Emit code to save the callee-saved registers from register number START
+   to LIMIT to the stack at the location starting at offset START_OFFSET,
+   skipping any write-back candidates if SKIP_WB is true.  */
 
 static void
 aarch64_save_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset,
@@ -3097,6 +3129,11 @@ aarch64_save_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset,
 }
 }
 
+/* Emit code to restore the callee registers of mode MODE from register
+   number START up to and including LIMIT.  Restore from the stack offset
+   START_OFFSET, skipping any write-back candidates if SKIP_WB is true.
+   Write the appropriate REG_CFA_RESTORE notes into CFI_OPS.  */
+
 static void
 aarch64_restore_call

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Bernd Schmidt]

On 10/12/2016 12:08 PM, Markus Trippelsdorf wrote:


Actually looking more closely it appears that all of the 136 additional
warnings for level 2 are bogus, too. Here is an example:

 switch (class) {
 case ATA_DEV_SEMB:
 class = ATA_DEV_ATA;/* some hard drives report SEMB sig */
 case ATA_DEV_ATA:
 case ATA_DEV_ZAC:
 tf.command = ATA_CMD_ID_ATA;
 break;


Another interesting question would be, how many of the level 1 warnings 
are false positives? But I'm not going to make you go through all 951 of 
them.



Bernd

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 11:52:02AM +0200, Bernd Schmidt wrote:
> On 10/12/2016 11:31 AM, Markus Trippelsdorf wrote:
> >On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:
> >>It's a discussion we should have, but I agree it should be done
> >>incrementally. I would argue for =1 as the default.
> >
> >Here are some numbers for an allmodconfig Linux kernel on pcc64le:
> >
> >-Wimplicit-fallthrough=1 :  951 warnings
> >-Wimplicit-fallthrough=2 : 1087 warnings
> >-Wimplicit-fallthrough=3 : 1209 warnings
> >
> >I randomly looked at the differences and almost all additional
> >-Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
> >And _all_ additional -Wimplicit-fallthrough=3 warnings appear
> >to be bogus.
> 
> And that's for a codebase that was written in English to begin with. Would
> you mind posting one or two examples if you saw interesting ones, for
> reference?
> 
> This result suggests that we should probably collapse levels 3-5 into a
> single strict one that doesn't try to be clever, and definitely make at most
> level 1 the default.

What do you mean at most?  level 0 is the warning disabled, that is the
default except for -Wextra.  The difference between =1 and =2 is very small
amount of warnings, one will need to annotate or add break; to those 951
spots anyway to make it -Wextra clean (those that don't have any kind of
comment at all), so just handling the additional 136 ones as well is not
that big deal.  It would be interesting to see those 136 comments though,
whether anything in them is about the intentional fall through or if they
are just unrelated.

Collapsing 3-5 levels is a bad idea, various projects will want to only
rely on attributes, especially if other compilers only support attributes
and nothing else, which is why there is level 5.  Levels 4 and 3 give choice
on how much free form the comments are.

> Another thing, was it ever resolved what this warning does to tools like
> ccache which like to operate on preprocessed files?

We already have 2 real-world PRs about cases like:
  case 2:
bar ();
/* FALLTHRU */
#undef X
  case 3:
#ifdef Y
bar ();
/* FALLTHRU */
#endif
  case 4:
bar ();
not being handled, which are extremely difficult to handle the current way
in libcpp, there can be many tokens in between the fallthrough_comment_p 
comments
and the case/default/label tokens.  It should work fine with -C or -CC.
So I'm wondering if a better approach wouldn't be that for
-Wimplicit-fallthrough={1,2,3,4,5} we'd let the fallthrough_comment_p
comments get through (perhaps normalized if not -C/-CC) as CPP_COMMENT with
the PREV_FALLTHROUGH flag and perhaps also another one that would indicate
it is really whitespace for other purposes.  The -C/-CC description talks
about significant differences though, e.g.
/*FALLTHROUGH*/#define A 1
A
is preprocessed differently with -C/-CC and without, so if we want to go
that way, we'd also need to special case the non-C/CC added CPP_COMMENT.
I have no idea what else it would affect though :(, I'm worried about token
pasting and lots of other cases.
If that is resolved, we could just emit the normalized /*FALLTHROUGH*/
comments if {-Wextra,-W,-Wimplicit-fallthrough{,=1,=2,=3,=4}} into -E output
too and just document that we do that.

Of course, for ccache I keep suggesting that they use -fdirectives-only
preprocessing instead, because anything else breaks miserably tons of other
stuff we have added into GCC over the last decade (-Wmisleading-indentation,
-Wsystem-headers vs. macro contexts, etc.), but the maintainers disagree.

Jakub

[Ada] Call memmove instead of memcpy in more cases

[Arnaud Charlet]

This changes the logic of Is_Non_Local_Array to return true for anything that
is not a component or a slice of a local entity, for example a dereference, so
as to generate a call to memmove instead of memcpy in more cases, e.g.:

package P is

  type String_Access is access all String;

  type Rec1 is record
A : String_Access;
  end record;

  procedure Proc (R : Rec1; S : String);

end P;
package body P is

  procedure Proc (R : Rec1; S : String) is
  begin
R.A (1 .. S'Length) := S;
  end;

end P;

This will also fix PR ada/64057.

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Eric Botcazou  

PR ada/64057.
* exp_ch5.adb (Is_Non_Local_Array): Return true for every array
that is not a component or slice of an entity in the current
scope.

Index: exp_ch5.adb
===
--- exp_ch5.adb (revision 241024)
+++ exp_ch5.adb (working copy)
@@ -278,9 +278,9 @@
 
   function Is_Non_Local_Array (Exp : Node_Id) return Boolean;
   --  Determine if Exp is a reference to an array variable which is other
-  --  than an object defined in the current scope, or a slice of such
-  --  an object. Such objects can be aliased to parameters (unlike local
-  --  array references).
+  --  than an object defined in the current scope, or a component or a
+  --  slice of such an object. Such objects can be aliased to parameters
+  --  (unlike local array references).
 
   ---
   -- Apply_Dereference --
@@ -327,10 +327,14 @@
 
   function Is_Non_Local_Array (Exp : Node_Id) return Boolean is
   begin
- return (Is_Entity_Name (Exp)
-   and then Scope (Entity (Exp)) /= Current_Scope)
-or else (Nkind (Exp) = N_Slice
-   and then Is_Non_Local_Array (Prefix (Exp)));
+ case Nkind (Exp) is
+when N_Indexed_Component | N_Selected_Component | N_Slice =>
+   return Is_Non_Local_Array (Prefix (Exp));
+when others =>
+   return
+ not (Is_Entity_Name (Exp) and then
+Scope (Entity (Exp)) = Current_Scope);
+ end case;
   end Is_Non_Local_Array;
 
   --  Determine if Lhs, Rhs are formal arrays or nonlocal arrays

[Ada] Spurious errors on instances of formal packages with defaults.

[Arnaud Charlet]

This patch fixes spurious errors on instantiations of formal packages that
have defaulted formals that include tagged private types and array types.

The following must compile quietly:

   gcc -c lis3dsh_spi.ads

---
with SPI_Bus_Driver;
with LIS3DSH;

package LIS3DSH_SPI is new LIS3DSH (IO => SPI_Bus_Driver);

package HAL.SPI is

   type SPI_Data_8b is array (Natural range <>) of Byte;

   type SPI_Port is limited interface;

end HAL.SPI;
---
with Interfaces;

package HAL is
   pragma Pure;

   subtype Word is Interfaces.Unsigned_32;
   subtype Short is Interfaces.Unsigned_16;
   subtype Byte is Interfaces.Unsigned_8;

end HAL;
with HAL;
with Peripheral_Bus_Driver;  -- signature

generic
   --  This device can be connected through I2C or SPI.
   with package IO is new Peripheral_Bus_Driver
 (Address => HAL.Byte,
  Data=> HAL.Byte,
  others  => <>);
package LIS3DSH is

   --  various routines that will call IO routines ...

end LIS3DSH;
---
generic

   type Device_Bus is abstract tagged limited private;

   type Address is private;

   type Data is private;

   type Buffer is array (Natural range <>) of Data;

   with procedure Read
 (This   : in out Device_Bus'Class;
  Value  : out Data;
  Source : Address);

   with procedure Write
 (This: in out Device_Bus'Class;
  Value   : Data;
  Destination : Address);

   with procedure Read_Buffer
 (This   : in out Device_Bus'Class;
  Value  : out Buffer;
  Source : Address);

   with procedure Write_Buffer
 (This: in out Device_Bus'Class;
  Value   : Buffer;
  Destination : Address);

package Peripheral_Bus_Driver is end;
---
with HAL.SPI;
with SPI_Byte_IO;
with Peripheral_Bus_Driver;

package SPI_Bus_Driver is new Peripheral_Bus_Driver
  (Device_Bus   => HAL.SPI.SPI_Port,
   Address  => HAL.Byte,
   Data => HAL.Byte,
   Buffer   => HAL.SPI.SPI_Data_8b,
   Read => SPI_Byte_IO.Read,
   Write=> SPI_Byte_IO.Write,
   Read_Buffer  => SPI_Byte_IO.Read_Buffer,
   Write_Buffer => SPI_Byte_IO.Write_Buffer);

---
with HAL.SPI;   use HAL.SPI;
use HAL;

package SPI_Byte_IO is

   procedure Read
 (This   : in out SPI_Port'Class;
  Value  : out Byte;
  Source : Byte);

   procedure Write
 (This: in out SPI_Port'Class;
  Value   : Byte;
  Destination : Byte);

   procedure Read_Buffer
 (This   : in out SPI_Port'Class;
  Value  : out SPI_Data_8b;
  Source : Byte);

   procedure Write_Buffer
 (This: in out SPI_Port'Class;
  Value   : SPI_Data_8b;
  Destination : Byte);

end SPI_Byte_IO;

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Ed Schonberg  

* sem_ch12.adb (Check_Formal_Package_Instance): Handle properly
an instance of a formal package with defaults, when defaulted
parameters include tagged private types and array types.

Index: sem_ch12.adb
===
--- sem_ch12.adb(revision 241024)
+++ sem_ch12.adb(working copy)
@@ -5787,8 +5787,9 @@
  (Formal_Pack : Entity_Id;
   Actual_Pack : Entity_Id)
is
-  E1 : Entity_Id := First_Entity (Actual_Pack);
-  E2 : Entity_Id := First_Entity (Formal_Pack);
+  E1  : Entity_Id := First_Entity (Actual_Pack);
+  E2  : Entity_Id := First_Entity (Formal_Pack);
+  Prev_E1 : Entity_Id;
 
   Expr1 : Node_Id;
   Expr2 : Node_Id;
@@ -5954,6 +5955,7 @@
--  Start of processing for Check_Formal_Package_Instance
 
begin
+  Prev_E1 := E1;
   while Present (E1) and then Present (E2) loop
  exit when Ekind (E1) = E_Package
and then Renamed_Entity (E1) = Renamed_Entity (Actual_Pack);
@@ -5983,6 +5985,14 @@
  if No (E1) then
 return;
 
+ --  Entities may be declared without full declaration, such as
+ --  itypes and predefined operators (concatenation for arrays, eg).
+ --  Skip it and keep the formal entity to find a later match for it.
+
+ elsif No (Parent (E2)) then
+E1 := Prev_E1;
+goto Next_E;
+
  --  If the formal entity comes from a formal declaration, it was
  --  defaulted in the formal package, and no check is needed on it.
 
@@ -5990,6 +6000,13 @@
  N_Formal_Object_Declaration,
  N_Formal_Type_Declaration)
  then
+--  If the formal is a tagged type the corresponding class-wide
+--  type has been generated as well, and it must be skipped.
+
+if Is_Type (E2) and then Is_Tagged_Type (E2) then
+   Next_Entity (E2);
+end if;
+
 goto Next_E;
 
  --  Ditto for defaulted formal subprograms.
@@ -6144,6 +6161,7 @@
  end if;
 
  <>
+Prev_E1 := E1;
 Next_Entity (E1);
 Next_Entity (E2);
   end lo

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 12:08:40PM +0200, Markus Trippelsdorf wrote:
> On 2016.10.12 at 11:52 +0200, Bernd Schmidt wrote:
> > On 10/12/2016 11:31 AM, Markus Trippelsdorf wrote:
> > > On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:
> > > > It's a discussion we should have, but I agree it should be done
> > > > incrementally. I would argue for =1 as the default.
> > >
> > > Here are some numbers for an allmodconfig Linux kernel on pcc64le:
> > >
> > > -Wimplicit-fallthrough=1 :  951 warnings
> > > -Wimplicit-fallthrough=2 : 1087 warnings
> > > -Wimplicit-fallthrough=3 : 1209 warnings
> > >
> > > I randomly looked at the differences and almost all additional
> > > -Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
> > > And _all_ additional -Wimplicit-fallthrough=3 warnings appear
> > > to be bogus.
> >
> > And that's for a codebase that was written in English to begin with. Would
> > you mind posting one or two examples if you saw interesting ones, for
> > reference?
> 
> Actually looking more closely it appears that all of the 136 additional
> warnings for level 2 are bogus, too. Here is an example:
> 
>  switch (class) {
>  case ATA_DEV_SEMB:
>  class = ATA_DEV_ATA;/* some hard drives report SEMB sig 
> */
>  case ATA_DEV_ATA:
>  case ATA_DEV_ZAC:
>  tf.command = ATA_CMD_ID_ATA;
>  break;

This is a clear argument for -Wimplicit-fallthrough default to =2 or higher.
The comment is completely random, doesn't express in any way the intent to
fall through, and if the comment appeared one line earlier (just slightly
different coding style), we'd warn.  I don't see a difference between why
we shouldn't warn by default (still talking about -Wextra) for the above
and should for:
switch (class) {
case ATA_DEV_SEMB:
class = ATA_DEV_ATA;
case ATA_DEV_ATA:
case ATA_DEV_ZAC:
tf.command = ATA_CMD_ID_ATA;
break;
In neither case the warning is bogus, it isn't even clear without additional
code analysis whether it is a code bug or intentational fallthrough not
marked that way.  The reason for adding =1 mode has been to support projects
that can't be bothered to express their intent in any standardized way and
want to express it free form in arbitrary languages, using various wordings
etc.  At the expense of missing cases like the above, where they actually
don't express the intent in any way.

Jakub

[Ada] Create a new internal profile

[Arnaud Charlet]

This new profile is not user visible and is the base to switch to the
restricted tasking runtime. It replaces the Restricted profile for that
purpose and is weaker than it.  The Restricted profile remains unchanged.
The purpose of that change is to lift some restrictions of the
GNAT_Extended_Ravenscar profile but still using the Restricted tasking.

No functional changes.

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Tristan Gingold  

* restrict.ads, restrict.adb (Restricted_Profile): Adjust
comment, use Restricted_Tasking to compare restrictions.
* s-rident.ads (Profile_Name): Add Restricted_Tasking and
reorder literals.
(Profile_Info): Set restrictions for Restricted_Tasking.

Index: restrict.adb
===
--- restrict.adb(revision 241024)
+++ restrict.adb(working copy)
@@ -1194,8 +1194,10 @@
  Restricted_Profile_Cached := True;
 
  declare
-R : Restriction_Flags  renames Profile_Info (Restricted).Set;
-V : Restriction_Values renames Profile_Info (Restricted).Value;
+R : Restriction_Flags  renames
+   Profile_Info (Restricted_Tasking).Set;
+V : Restriction_Values renames
+   Profile_Info (Restricted_Tasking).Value;
  begin
 for J in R'Range loop
if R (J)
Index: restrict.ads
===
--- restrict.ads(revision 241024)
+++ restrict.ads(working copy)
@@ -6,7 +6,7 @@
 --  --
 -- S p e c  --
 --  --
---  Copyright (C) 1992-2015, Free Software Foundation, Inc. --
+--  Copyright (C) 1992-2016, Free Software Foundation, Inc. --
 --  --
 -- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
@@ -424,10 +424,10 @@
--  executing this code only if needed.
 
function Restricted_Profile return Boolean;
-   --  Tests if set of restrictions corresponding to Profile (Restricted) is
-   --  currently in effect (set by pragma Profile, or by an appropriate set of
-   --  individual Restrictions pragmas). Returns True only if all the required
-   --  restrictions are set.
+   --  Tests if set of restrictions corresponding to Restricted_Tasking profile
+   --  is currently in effect (set by pragma Profile, or by an appropriate set
+   --  of individual Restrictions pragmas). Returns True only if all the
+   --  required restrictions are set.
 
procedure Set_Hidden_Part_In_SPARK (Loc1, Loc2 : Source_Ptr);
--  Insert a new hidden region range in the SPARK hides table. The effect
Index: s-rident.ads
===
--- s-rident.ads(revision 241024)
+++ s-rident.ads(working copy)
@@ -378,15 +378,19 @@
type Profile_Name is
  (No_Profile,
   No_Implementation_Extensions,
+  Restricted_Tasking,
+  Restricted,
   Ravenscar,
-  GNAT_Extended_Ravenscar,
-  Restricted);
+  GNAT_Extended_Ravenscar);
--  Names of recognized profiles. No_Profile is used to indicate that a
--  restriction came from pragma Restrictions[_Warning], as opposed to
-   --  pragma Profile[_Warning].
+   --  pragma Profile[_Warning]. Restricted_Tasking is a non-user profile that
+   --  contaings the minimal set of restrictions to trigger the user of the
+   --  restricted tasking runtime. Restricted is the corresponding user profile
+   --  that also restrict protected types.
 
subtype Profile_Name_Actual is Profile_Name
- range No_Implementation_Extensions .. Restricted;
+ range No_Implementation_Extensions .. GNAT_Extended_Ravenscar;
--  Actual used profile names
 
type Profile_Data is record
@@ -422,6 +426,37 @@
 Value =>
   (others  => 0)),
 
+ --  Restricted_Tasking Profile
+
+ Restricted_Tasking =>
+
+--  Restrictions for Restricted_Tasking profile
+
+   (Set   =>
+  (No_Abort_Statements => True,
+   No_Asynchronous_Control => True,
+   No_Dynamic_Attachment   => True,
+   No_Dynamic_Priorities   => True,
+   No_Local_Protected_Objects  => True,
+   No_Protected_Type_Allocators=> True,
+   No_Requeue_Statements 

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Bernd Schmidt]

On 10/12/2016 12:26 PM, Jakub Jelinek wrote:

What do you mean at most?  level 0 is the warning disabled, that is the
default except for -Wextra.


I think level 0 has to be on the table for -Wextra as well, depending on 
the signal-to-noise ratio of level 1.



Collapsing 3-5 levels is a bad idea, various projects will want to only
rely on attributes, especially if other compilers only support attributes
and nothing else, which is why there is level 5.  Levels 4 and 3 give choice
on how much free form the comments are.


But I think the data we have points towards that being a meaningless 
choice. So maybe I got the levels wrong and 2-4 is what we should be 
collapsing.



Another thing, was it ever resolved what this warning does to tools like
ccache which like to operate on preprocessed files?


We already have 2 real-world PRs about cases like:


Yeah. Another reason why I think level 0 (i.e. off) should be seriously 
considered for -Wextra.



Bernd

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Markus Trippelsdorf]

On 2016.10.12 at 12:26 +0200, Jakub Jelinek wrote:
> On Wed, Oct 12, 2016 at 11:52:02AM +0200, Bernd Schmidt wrote:
> > On 10/12/2016 11:31 AM, Markus Trippelsdorf wrote:
> > >On 2016.10.12 at 00:34 +0200, Bernd Schmidt wrote:
> > >>It's a discussion we should have, but I agree it should be done
> > >>incrementally. I would argue for =1 as the default.
> > >
> > >Here are some numbers for an allmodconfig Linux kernel on pcc64le:
> > >
> > >-Wimplicit-fallthrough=1 :  951 warnings
> > >-Wimplicit-fallthrough=2 : 1087 warnings
> > >-Wimplicit-fallthrough=3 : 1209 warnings
> > >
> > >I randomly looked at the differences and almost all additional
> > >-Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
> > >And _all_ additional -Wimplicit-fallthrough=3 warnings appear
> > >to be bogus.
> > 
> > And that's for a codebase that was written in English to begin with. Would
> > you mind posting one or two examples if you saw interesting ones, for
> > reference?
> > 
> > This result suggests that we should probably collapse levels 3-5 into a
> > single strict one that doesn't try to be clever, and definitely make at most
> > level 1 the default.
> 
> What do you mean at most?  level 0 is the warning disabled, that is the
> default except for -Wextra.  The difference between =1 and =2 is very small
> amount of warnings, one will need to annotate or add break; to those 951
> spots anyway to make it -Wextra clean (those that don't have any kind of
> comment at all), so just handling the additional 136 ones as well is not
> that big deal.  It would be interesting to see those 136 comments though,
> whether anything in them is about the intentional fall through or if they
> are just unrelated.

Here are some examples:

 /* Don't break */
 /* drop through */  //very popular
 /* fall though */ 
 /* pass through... */
 /* break intentionally omitted */ 
 /* fallthough */
 /* Don't break, this is a failed frame! */
 /* leave break out intentionly */ 
 /* else: Fall trough */  
 /* passthrough */ 
 /* NOBREAK */
 /* follow thru */ 
 /* go through here */ 

-- 
Markus

[diagnostic] fix formatting

[Nathan Sidwell]

I noticed some extraneous braces and extra long lines.  Applied as obvious.

nathan
2016-10-12  Nathan Sidwell  

	* diagnostic.c (diagnostc_report_diagnostic): Fix formatting.

Index: diagnostic.c
===
--- diagnostic.c	(revision 241024)
+++ diagnostic.c	(working copy)
@@ -877,13 +877,15 @@ diagnostic_report_diagnostic (diagnostic
 		}
 	}
 	}
+
   /* This tests if the user provided the appropriate -Werror=foo
 	 option.  */
   if (diag_class == DK_UNSPECIFIED
-	  && context->classify_diagnostic[diagnostic->option_index] != DK_UNSPECIFIED)
-	{
-	  diagnostic->kind = context->classify_diagnostic[diagnostic->option_index];
-	}
+	  && (context->classify_diagnostic[diagnostic->option_index]
+	  != DK_UNSPECIFIED))
+	diagnostic->kind
+	  = context->classify_diagnostic[diagnostic->option_index];
+
   /* This allows for future extensions, like temporarily disabling
 	 warnings for ranges of source code.  */
   if (diagnostic->kind == DK_IGNORED)

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Bernd Schmidt]

On 10/12/2016 12:36 PM, Markus Trippelsdorf wrote:

Here are some examples:

 /* Don't break */
 /* drop through */  //very popular
 /* fall though */
 /* pass through... */
 /* break intentionally omitted */
 /* fallthough */
 /* Don't break, this is a failed frame! */
 /* leave break out intentionly */
 /* else: Fall trough */
 /* passthrough */
 /* NOBREAK */
 /* follow thru */
 /* go through here */


Yeah. As I said earlier, this is a losing battle without real AI, even 
when we're only considering English as the comment language.



Bernd

[PATCH] print_rtx_function: integrate dumping of the CFG into the insn chain

[David Malcolm]

On Fri, 2016-10-07 at 15:58 +0200, Bernd Schmidt wrote:
> On 10/07/2016 03:26 PM, David Malcolm wrote:
> > 
> > We could simply print the INSN_UID for CODE_LABELs; something like
> > this
> > (see the "(code_label 16" below):
> 
> I think that should work.
> 
> > You appear to have trimmed the idea of enclosing the insns with
> > (basic-block) directives without commenting on it.  Did you like
> > this
> > idea?
> 
> Sorry - I appear to have completely missed it.
> 
> > It would make the above look like:
> > 
> >   (basic-block 2
> > ;; insns snipped
> > (jump_insn (set (pc)
> > (if_then_else (ge (reg:CCGC 17 flags)
> > (const_int 0 [0]))
> > (label_ref 16)
> > (pc))) "test.c":3
> >-> 16)
> >   ) ;; basic-block 2
> >   (basic-block 4
> > (note [bb 4] NOTE_INSN_BASIC_BLOCK)
> > ;; insns snipped
> > (jump_insn (set (pc) (label_ref 20)) "test.c":4
> >  -> 20)
> >   ) ;; basic-block 4
> >   (barrier)
> >   (basic-block 5
> > (code_label 16 [1 uses])
> > (note [bb 5] NOTE_INSN_BASIC_BLOCK)
> > ;; etc
> >   ) ;; basic-block 5
> > 
> > Note how the above format expresses clearly that:
> > * the (barrier) is part of the insn chain, but not in a basic
> > block, and
> > * some insns can happen in a basic block
> 
> That looks really nice IMO. Except maybe drop the "-> 16" bit for the
> jump_insn (that's the JUMP_LABEL, isn't it?)
> 
> > Taking this idea further: if we have (basic-block) directives
> > integrated into the insn-chain like this, we could express the CFG
> > by
> > adding (edge) directives. Here's a suggestion for doing it with
> > (edge-from) and (edge-to) directives, expressing the predecessor
> > and
> > successor edges in the CFG, along with :
> 
> That also looks reasonable. Probably a small but maybe not a huge
> improvement over the other syntax. Having both from and to edges
> seems
> redundant but might help readability. The reader should check
> consistency in that case.
> 
> > Should we spell "0" and "1" as "entry" and "exit" when
> > parsing/dumping
> > basic block indices? e.g.:
> > 
> >   (basic-block 2
> > (edge-from entry)
> 
> If that can be done it would be an improvement.

The following patch implements the above idea, integrating (block)
directives into the (insn-chain) which wrap those rtx_insn * that
have a basic block.

It doesn't yet suppress printing the various INSN_UID fields, along
with the other ideas in this thread; I plan to do these as followups.

> > > I think maybe you want a separate compact form of insns and notes
> > > (cinsn/cnote maybe), with a flag selecting which gets written out
> > > in
> > > the
> > > dumper. The reader could then read them in like any other rtx
> > > code,
> > > and
> > > there'd be a postprocessing stage to reconstruct the insn chain.
> > 
> > By this separate compact form, do you mean the form we've been
> > discussing above, with no INSN_UID/PREV/NEXT, etc?  Or something
> > else?
> 
> Yes, the form we're discussing, except instead of (insn ...) you'd
> have
> (cinsn ...), which I assume would make it easier for the reader and
> less
> ambiguous overall.
> 
> > As for "cinsn", "cnote", how about just "insn" and "note", and
> > having
> > the compactness be expressed at the top of the dump e.g. implicitly
> > by
> > the presence of a "(function" directive.  Could even have a format
> > version specifier in the function directive, to give us some future
> > -proofing e.g.
> >   (function (format 20161007)
> > or somesuch.
> 
> Having it implicit should also be ok - I have no strong preference
> really. I'm not sure we want versioning - better to have an automatic
> conversion if we ever feel we need to change the format.
> 
> > Do you want to want to try hand-edited a test case, using some of
> > the
> > format ideas we've been discussing?  That might suggest further
> > improvements to the format.
> 
> We'll definitely want to have a look at one or two. Also, we ought to
> try to set up situations we haven't discussed: ADDR_VECs (in light of
> the basic block dumping) and ASMs maybe. I'm probably forgetting
> some.
> 
> One other thing in terms of format is the printout of CONST_INT - I
> think it should suffice to have either decimal, or hex, but not
> really
> both. The reader should maybe accept either.
> 
> I think all hosts have 64-bit HWI these days, so CONST_INT ought to
> always stay valid through a roundtrip.
> 
> I may have missed it, but is there any kind of provision yet for
> providing an "after" dump for what is expected after a pass is run?
> Might be worth thinking about whether the reader could have a mode
> where
> it matches internal RTL against an input.
> 
> > OK.  If so, do we need to print the regno for hard registers?  Or
> > should we just print the name for those, for consistency with
> > virtual
> > regs?  (i.e. have it be controlled by the same flag?)
> 
> Just the name should work, leaving only pseudos wi

Re: [PATCH] print_rtx_function: integrate dumping of the CFG into the insn chain

[Bernd Schmidt]

On 10/12/2016 01:15 PM, David Malcolm wrote:

+/* Determine if INSN is of a kind that can have a basic block.  */
+
+static bool
+can_have_basic_block_p (const rtx_insn *insn)
+{
+  return GET_RTX_FORMAT (GET_CODE (insn))[2] == 'B';
+}


Oof. I suppose they're all at least that long, but still - we're looking 
only for barriers, right? Might be better to encode this expectation 
using an assert, like so:


rtx_code code = GET_CODE (insn);
if (code == BARRIER)
  return false;
gcc_assert (GET_RTX_FORMAT (code)[2] == 'B');

Otherwise, nice, and ok.


Bernd

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 12:33:58PM +0200, Bernd Schmidt wrote:
> On 10/12/2016 12:26 PM, Jakub Jelinek wrote:
> >What do you mean at most?  level 0 is the warning disabled, that is the
> >default except for -Wextra.
> 
> I think level 0 has to be on the table for -Wextra as well, depending on the
> signal-to-noise ratio of level 1.

-Wextra includes various style warnings (we have some of them even in
-Wall), and the intent with this warning is to improve the code base,
let users look at places where there are implicit fallthrough not explicitly
marked so, and spend the few seconds to mark intentional fallthrough if it
is indeed intentional.
So for this warning, the question is what is a false positive; I really
don't expect the code in the wild broken so much that every place the
warning is emitted is a missing break;, 1:50 or 1:100 ratio is just fine,
by adding (more or less free form, which is why we have levels)
comments about it you express the intent and that will survive often for a
long time without need for another analysis.

If -Wimplicit-fallthrough isn't included in -Wextra, most of the people will
not even know there is this warning, and the code base in the wild will
remain with lots of bugs.  Remember how many bugs in gcc sources alone this
warning allowed to fix.

Jakub

Re: [PATCH] Add a helper function: create_tmp

[Martin Liška]

On 10/11/2016 12:31 PM, Richard Biener wrote:
> Hrm.  Too easy to confuse with create_tmp_reg ... so maybe
> create_tmp_reg_or_ssa_name?

Yep, renamed function patch installed as r241030.

Thanks,
Martin

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 12:36:29PM +0200, Markus Trippelsdorf wrote:
> Here are some examples:
> 
>  /* Don't break */
>  /* drop through */  //very popular
>  /* fall though */ 
>  /* pass through... */
>  /* break intentionally omitted */ 
>  /* fallthough */
>  /* Don't break, this is a failed frame! */
>  /* leave break out intentionly */ 
>  /* else: Fall trough */  
>  /* passthrough */ 
>  /* NOBREAK */
>  /* follow thru */ 
>  /* go through here */ 

It would be nice to see all those 136 comments, perhaps sort | uniq -c
merged together.
Letting people fix fall though or fallthough or Fall trough misspellings
is a good thing.  And for the others, I think the comment adjustment won't
take more than a few seconds each, compared to having to analyze the 900+
cases.

Jakub

Re: [PATCH] Introduce -Wimplicit-fallthrough={0,1,2,3,4,5}

[Markus Trippelsdorf]

On 2016.10.12 at 12:54 +0200, Jakub Jelinek wrote:
> On Wed, Oct 12, 2016 at 12:36:29PM +0200, Markus Trippelsdorf wrote:
> > Here are some examples:
> > 
> >  /* Don't break */
> >  /* drop through */  //very popular
> >  /* fall though */ 
> >  /* pass through... */
> >  /* break intentionally omitted */ 
> >  /* fallthough */
> >  /* Don't break, this is a failed frame! */
> >  /* leave break out intentionly */ 
> >  /* else: Fall trough */  
> >  /* passthrough */ 
> >  /* NOBREAK */
> >  /* follow thru */ 
> >  /* go through here */ 
> 
> It would be nice to see all those 136 comments, perhaps sort | uniq -c
> merged together.
> Letting people fix fall though or fallthough or Fall trough misspellings
> is a good thing.  And for the others, I think the comment adjustment won't
> take more than a few seconds each, compared to having to analyze the 900+
> cases.

I'm more concerned about the first impression that people will get from
this warning. If the fist couple of samples they will look at are
clearly bogus (as with the example comments above) they will very
quickly disable the warning.

And if a small one digit percentage of all potential issues falls
through the cracks with -Wimplicit-fallthrough=1 , so be it.

-- 
Markus

Re: [ipa-vrp] Use get/set_ptr_nonnull in ipa-vrp

[Jan Hubicka]

> Hi,
> 
> This patch uses the get/set_ptr_nonnull so that ipa-vrp also
> propagates nonnull ranges for pinter.
> 
> Bootstrapped and regression tested this with other patched without
> any new regressions on x86_64-linux-gnu.
> 
> Is this OK for trunk?
> 
> Thanks,
> Kugan
> 
> 
> 
> 
> gcc/ChangeLog:
> 
> 2016-10-12  Kugan Vivekanandarajah  
> 
>   * ipa-prop.c (ipa_compute_jump_functions_for_edge): Set value range
> for pointer type too.
>   (ipcp_update_vr): set_ptr_nonnull for pointer.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-10-12  Kugan Vivekanandarajah  
> 
>   * gcc.dg/ipa/vrp4.c: New test.
OK, thank you!
We should be able to derive a lot of (useful) non-null information from the
fact that the pointers are dereferenced either prior the function call or in a
statement that postdominate the function entry.  I guess we could also give 
(semi)
useful -Wmissing-attribute=nonnull hints in that case.

Honza

> 

> >From f773226855968cc652fa6f2b2d9c70d2a5d7acdb Mon Sep 17 00:00:00 2001
> From: Kugan Vivekanandarajah 
> Date: Wed, 12 Oct 2016 13:54:34 +1100
> Subject: [PATCH 2/3] Set-nonnull-range-for-pointer-type
> 
> ---
>  gcc/ipa-prop.c  | 57 
> +
>  gcc/testsuite/gcc.dg/ipa/vrp4.c | 27 +++
>  2 files changed, 68 insertions(+), 16 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/vrp4.c
> 
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index a1d7619..353b638 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1668,7 +1668,22 @@ ipa_compute_jump_functions_for_edge (struct 
> ipa_func_body_info *fbi,
>   useful_context = true;
>   }
>  
> -  if (!POINTER_TYPE_P (TREE_TYPE (arg)))
> +  if (POINTER_TYPE_P (TREE_TYPE (arg)))
> + {
> +   if (TREE_CODE (arg) == SSA_NAME
> +   && param_type
> +   && get_ptr_nonnull (arg))
> + {
> +   jfunc->vr_known = true;
> +   jfunc->m_vr.type = VR_ANTI_RANGE;
> +   jfunc->m_vr.min = build_int_cst (TREE_TYPE (arg), 0);
> +   jfunc->m_vr.max = build_int_cst (TREE_TYPE (arg), 0);
> +   jfunc->m_vr.equiv = NULL;
> + }
> +   else
> + gcc_assert (!jfunc->vr_known);
> + }
> +  else
>   {
> wide_int min, max;
> value_range_type type;
> @@ -5602,27 +5617,37 @@ ipcp_update_vr (struct cgraph_node *node)
>   continue;
>  
>if (vr[i].known
> -   && INTEGRAL_TYPE_P (TREE_TYPE (ddef))
> -   && !POINTER_TYPE_P (TREE_TYPE (ddef))
> && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE))
>   {
> tree type = TREE_TYPE (ddef);
> unsigned prec = TYPE_PRECISION (type);
> -   if (dump_file)
> +   if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
> + {
> +   if (dump_file)
> + {
> +   fprintf (dump_file, "Setting value range of param %u ", i);
> +   fprintf (dump_file, "%s[",
> +(vr[i].type == VR_ANTI_RANGE) ? "~" : "");
> +   print_decs (vr[i].min, dump_file);
> +   fprintf (dump_file, ", ");
> +   print_decs (vr[i].max, dump_file);
> +   fprintf (dump_file, "]\n");
> + }
> +   set_range_info (ddef, vr[i].type,
> +   wide_int_storage::from (vr[i].min, prec,
> +   TYPE_SIGN (type)),
> +   wide_int_storage::from (vr[i].max, prec,
> +   TYPE_SIGN (type)));
> + }
> +   else if (POINTER_TYPE_P (TREE_TYPE (ddef))
> +&& vr[i].type == VR_ANTI_RANGE
> +&& wi::eq_p (vr[i].min, 0)
> +&& wi::eq_p (vr[i].max, 0))
>   {
> -   fprintf (dump_file, "Setting value range of param %u ", i);
> -   fprintf (dump_file, "%s[",
> -(vr[i].type == VR_ANTI_RANGE) ? "~" : "");
> -   print_decs (vr[i].min, dump_file);
> -   fprintf (dump_file, ", ");
> -   print_decs (vr[i].max, dump_file);
> -   fprintf (dump_file, "]\n");
> +   if (dump_file)
> + fprintf (dump_file, "Setting nonnull for %u\n", i);
> +   set_ptr_nonnull (ddef);
>   }
> -   set_range_info (ddef, vr[i].type,
> -   wide_int_storage::from (vr[i].min, prec,
> -   TYPE_SIGN (type)),
> -   wide_int_storage::from (vr[i].max, prec,
> -   TYPE_SIGN (type)));
>   }
>  }
>  }
> diff --git a/gcc/testsuite/gcc.dg/ipa/vrp4.c b/gcc/testsuite/gcc.dg/ipa/vrp4.c
> new file mode 100644
> index 000..d7e1f26
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/vrp4.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-cp-details" } */
> +
> +static __attribute__((noin

[PATCH] Do not copy std:call_once arguments (LWG 2442)

[Jonathan Wakely]

This replaces the use of std::__bind_simple by direct calls to
std::__invoke so that the arguments are not decay-copied.

* doc/xml/manual/intro.xml: Document LWG 2442 status.
* include/std/mutex [_GLIBCXX_HAVE_TLS] (__once_call_impl): Remove.
[_GLIBCXX_HAVE_TLS] (_Once_call): Declare primary template and define
partial specialization to unpack args and forward to std::invoke.
(call_once) [_GLIBCXX_HAVE_TLS]: Use forward_as_tuple and _Once_call
instead of __bind_simple and __once_call_impl.
(call_once) [!_GLIBCXX_HAVE_TLS]: Use __invoke instead of
__bind_simple.
* testsuite/30_threads/call_once/dr2442.cc: New test.

Tested powerpc64le-linux, committed to trunk.

commit 67819bb3bee8fb170e46b324c65d35b856136cd1
Author: redi 
Date:   Wed Oct 12 11:20:25 2016 +

Do not copy std:call_once arguments (LWG 2442)

* doc/xml/manual/intro.xml: Document LWG 2442 status.
* include/std/mutex [_GLIBCXX_HAVE_TLS] (__once_call_impl): Remove.
[_GLIBCXX_HAVE_TLS] (_Once_call): Declare primary template and define
partial specialization to unpack args and forward to std::invoke.
(call_once) [_GLIBCXX_HAVE_TLS]: Use forward_as_tuple and _Once_call
instead of __bind_simple and __once_call_impl.
(call_once) [!_GLIBCXX_HAVE_TLS]: Use __invoke instead of
__bind_simple.
* testsuite/30_threads/call_once/dr2442.cc: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241031 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/doc/xml/manual/intro.xml 
b/libstdc++-v3/doc/xml/manual/intro.xml
index 22b792a..528b192 100644
--- a/libstdc++-v3/doc/xml/manual/intro.xml
+++ b/libstdc++-v3/doc/xml/manual/intro.xml
@@ -1043,6 +1043,13 @@ requirements of the license of GCC.
 Define the typedefs.
 
 
+http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2442">2442:
+   call_once() shouldn't 
DECAY_COPY()
+
+Remove indirection through call wrapper that made copies
+of arguments and forward arguments straight to std::invoke.
+
+
 http://www.w3.org/1999/xlink"; 
xlink:href="../ext/lwg-defects.html#2454">2454:
Add raw_storage_iterator::base() member

diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 7a7bd2e..4c6f036 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -580,12 +580,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   extern __thread void* __once_callable;
   extern __thread void (*__once_call)();
 
-  template
-inline void
-__once_call_impl()
+  template::value>::__type>
+struct _Once_call;
+
+  template
+struct _Once_call<_Tuple, _Index_tuple<_Ind...>>
 {
-  (*(_Callable*)__once_callable)();
-}
+  static void
+  _S_call()
+  {
+   auto& __f_args = *static_cast<_Tuple*>(__once_callable);
+   std::__invoke(std::get<_Ind>(std::move(__f_args))...);
+  }
+};
 #else
   extern function __once_functor;
 
@@ -603,16 +611,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 void
 call_once(once_flag& __once, _Callable&& __f, _Args&&... __args)
 {
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 2442. call_once() shouldn't DECAY_COPY()
 #ifdef _GLIBCXX_HAVE_TLS
-  auto __bound_functor = std::__bind_simple(std::forward<_Callable>(__f),
-  std::forward<_Args>(__args)...);
-  __once_callable = std::__addressof(__bound_functor);
-  __once_call = &__once_call_impl;
+  auto __f_args = std::forward_as_tuple(
+ std::forward<_Callable>(__f), std::forward<_Args>(__args)...);
+  __once_callable = std::__addressof(__f_args);
+  __once_call = _Once_call::_S_call;
 #else
   unique_lock __functor_lock(__get_once_mutex());
-  auto __callable = std::__bind_simple(std::forward<_Callable>(__f),
-  std::forward<_Args>(__args)...);
-  __once_functor = [&]() { __callable(); };
+  __once_functor = [&] {
+ std::__invoke(std::forward<_Callable>(__f),
+   std::forward<_Args>(__args)...);
+  };
   __set_once_functor_lock_ptr(&__functor_lock);
 #endif
 
diff --git a/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc 
b/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc
new file mode 100644
index 000..5b66687
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc
@@ -0,0 +1,45 @@
+// { dg-do run { target *-*-freebsd* *-*-dragonfly* *-*-netbsd* *-*-linux* 
*-*-gnu* *-*-solaris* *-*-cygwin *-*-rtems* *-*-darwin* powerpc-ibm-aix* } }
+// { dg-options "-pthread" { target *-*-freebsd* *-*-dragonfly* *-*-netbsd* 
*-*-linux* *-*-gnu* *-*-solaris* powerpc-ibm-aix* } }
+// { dg-require-effective-target c++11 }
+// { dg-require-cstdint "" }
+// { dg-require-gthreads "" }
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+//

[PATCH] Remove 'return' from noreturn function

[Jonathan Wakely]

This just fixes a -Wsystem-headers warning, because this function is
[[noreturn]].

* libsupc++/nested_exception.h (throw_with_nested): Remove return.

Tested powerpc64le-linux, committed to trunk.

commit 9720fce68641461a1d17bc42bc04d77f5cf18e0e
Author: redi 
Date:   Wed Oct 12 11:20:29 2016 +

Remove 'return' from noreturn function

* libsupc++/nested_exception.h (throw_with_nested): Remove return.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241032 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/libsupc++/nested_exception.h 
b/libstdc++-v3/libsupc++/nested_exception.h
index 0c00d74..078af0e 100644
--- a/libstdc++-v3/libsupc++/nested_exception.h
+++ b/libstdc++-v3/libsupc++/nested_exception.h
@@ -122,7 +122,7 @@ namespace std
  "throw_with_nested argument must be CopyConstructible");
   using __nest = __and_, __bool_constant,
__not_>>;
-  return std::__throw_with_nested_impl(std::forward<_Tp>(__t), __nest{});
+  std::__throw_with_nested_impl(std::forward<_Tp>(__t), __nest{});
 }
 
   // Determine if dynamic_cast would be well-formed.

[PATCH] Add missing return to uninitialized_default_construct_n

[Jonathan Wakely]

This non-void function was missing a return statement.

* include/bits/stl_uninitialized.h
(__uninitialized_default_novalue_n_1): Add missing return.
* testsuite/20_util/specialized_algorithms/memory_management_tools/
1.cc: Check return values of uninitialized_xxx_n algorithms.

Tested powerpc64le-linux, committed to trunk.

commit fe1f02d734ca135872688f8371542ca7f17aae7e
Author: redi 
Date:   Wed Oct 12 11:20:35 2016 +

Add missing return to uninitialized_default_construct_n

* include/bits/stl_uninitialized.h
(__uninitialized_default_novalue_n_1): Add missing return.
* testsuite/20_util/specialized_algorithms/memory_management_tools/
1.cc: Check return values of uninitialized_xxx_n algorithms.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241033 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h 
b/libstdc++-v3/include/bits/stl_uninitialized.h
index ef2e584..07370c9 100644
--- a/libstdc++-v3/include/bits/stl_uninitialized.h
+++ b/libstdc++-v3/include/bits/stl_uninitialized.h
@@ -705,8 +705,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
static _ForwardIterator
__uninit_default_novalue_n(_ForwardIterator __first, _Size __n)
-   {
-   }
+   { return std::next(__first, __n); }
 };
 
   // __uninitialized_default_novalue
diff --git 
a/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
 
b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
index 84a6857..a86e61a 100644
--- 
a/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
+++ 
b/libstdc++-v3/testsuite/20_util/specialized_algorithms/memory_management_tools/1.cc
@@ -74,15 +74,17 @@ void test02()
 void test03()
 {
   char test_data[] = "123456";
-  std::uninitialized_default_construct_n(std::begin(test_data), 6);
+  auto end = std::uninitialized_default_construct_n(std::begin(test_data), 6);
   VERIFY(std::string(test_data) == "123456");
+  VERIFY( end == test_data + 6 );
 }
 
 void test04()
 {
   char test_data[] = "123456";
-  std::uninitialized_value_construct_n(std::begin(test_data), 6);
-  VERIFY(std::string(test_data, 6) == std::string("\0\0\0\0\0\0", 6));
+  auto end = std::uninitialized_value_construct_n(std::begin(test_data), 5);
+  VERIFY(std::string(test_data, 6) == std::string("\0\0\0\0\0" "6", 6));
+  VERIFY( end == test_data + 5 );
 }
 
 void test05()
@@ -112,8 +114,9 @@ void test07()
   del_count = 0;
   DelCount* x = (DelCount*)malloc(sizeof(DelCount)*10);
   for (int i = 0; i < 10; ++i) new (x+i) DelCount;
-  std::destroy_n(x, 10);
+  auto end = std::destroy_n(x, 10);
   VERIFY(del_count == 10);
+  VERIFY( end == x + 10 );
   del_count = 0;
   free(x);
 }
@@ -127,7 +130,8 @@ void test08()
   std::uninitialized_move(source.begin(), source.end(), target);
   for (const auto& x : source) VERIFY(!x);
   for (int i = 0; i < 10; ++i) VERIFY(bool(*(target+i)));
-  std::destroy_n(target, 10);
+  auto end = std::destroy_n(target, 10);
+  VERIFY( end == target + 10 );
   free(target);
 }
 
@@ -137,10 +141,13 @@ void test09()
   for (int i = 0; i < 10; ++i) source.push_back(std::make_unique(i));
   std::unique_ptr* target =
 (std::unique_ptr*)malloc(sizeof(std::unique_ptr)*10);
-  std::uninitialized_move_n(source.begin(), 10, target);
+  auto end = std::uninitialized_move_n(source.begin(), 10, target);
+  VERIFY( end.first == source.begin() + 10 );
+  VERIFY( end.second == target + 10 );
   for (const auto& x : source) VERIFY(!x);
   for (int i = 0; i < 10; ++i) VERIFY(bool(*(target+i)));
-  std::destroy_n(target, 10);
+  auto end2 = std::destroy_n(target, 10);
+  VERIFY( end2 == target + 10 );
   free(target);
 }
 
@@ -156,7 +163,8 @@ void test11()
 {
   char* x = (char*)malloc(sizeof(char)*10);
   for (int i = 0; i < 10; ++i) new (x+i) char;
-  std::destroy_n(x, 10);
+  auto end = std::destroy_n(x, 10);
+  VERIFY( end == x + 10 );
   free(x);
 }
 
@@ -285,10 +293,12 @@ void test19()
 {
   char test_source[] = "123456";
   char test_target[] = "00";
-  std::uninitialized_move_n(std::begin(test_source),
-   6,
-   test_target);
+  auto end = std::uninitialized_move_n(std::begin(test_source),
+   6,
+   test_target);
   VERIFY(std::string(test_target) == "123456");
+  VERIFY( end.first == test_source + 6 );
+  VERIFY( end.second == test_target + 6 );
 }
 
 int main()

[patch,avr] Introduce absdata attribute to allow LDS /STS on Tiny

[Georg-Johann Lay]

This patch introduces a new variable attribute "absdata".

Reduced Tiny cores have only a limited range of addresses 0x40..0xbf which can 
be handled by LDS / STS directly.  The attribute allows the user to assert that 
it is legitimate to use absolute addressing for such addresses and that there 
is no need for bulky load of address + indirect addressing.


Allowed addresses actually range from 0x0 to 0xbf because IN / OUT can handle 
absolute addresses in the lower range 0x0..0x3f.


Ok for trunk?

Johann


gcc/
* doc/extend.texi (AVR Variable Attributes) [absdata]: Document it.
* config/avr/avr.c (AVR_SYMBOL_FLAG_TINY_ABSDATA): New macro.
(avr_address_tiny_absdata_p): New static function.
(avr_legitimate_address_p, avr_legitimize_address) [AVR_TINY]: Use
it to determine validity of constant addresses.
(avr_attribute_table) [absdata]: New variable attribute...
(avr_handle_absdata_attribute): ...and handler.
(avr_decl_absdata_p): New static function.
(avr_encode_section_info) [AVR_TINY]: Use it to add flag
AVR_SYMBOL_FLAG_TINY_ABSDATA to respective symbols_refs.
(avr_address_cost) [AVR_TINY]: absdata addresses cost 2.

gcc/testsuite/
* lib/target-supports.exp (check_effective_target_avr_tiny): New proc.
* gcc.target/avr/torture/tiny-absdata-1.c: New test.
Index: doc/extend.texi
===
--- doc/extend.texi	(revision 240915)
+++ doc/extend.texi	(working copy)
@@ -5973,6 +5973,33 @@ memory-mapped peripherals that may lie o
 volatile int porta __attribute__((address (0x600)));
 @end smallexample
 
+@item absdata
+@cindex @code{absdata} variable attribute, AVR
+Variables in static storage and with the @code{absdata} attribute can
+be accessed by the @code{LDS} and @code{STS} instructions which take
+absolute addresses.
+
+@itemize @bullet
+@item
+This attribute is only supported for the reduced AVR Tiny core
+like ATtiny40.
+
+@item
+You must make sure that respective data is located in the
+address range @code{0x40}@dots{}@code{0xbf} accessible by
+@code{LDS} and @code{STS}.  One way to achieve this as an
+appropriate linker description file.
+
+@item
+If the location does not fit the address range of @code{LDS}
+and @code{STS}, there is currently (Binutils 2.26) just an unspecific
+warning like
+@quotation
+@code{module.c:(.text+0x1c): warning: internal error: out of range error}
+@end quotation
+
+@end itemize
+
 @end table
 
 @node Blackfin Variable Attributes
Index: config/avr/avr.c
===
--- config/avr/avr.c	(revision 240966)
+++ config/avr/avr.c	(working copy)
@@ -84,6 +84,10 @@
 #define AVR_SYMBOL_FLAG_TINY_PM \
   (SYMBOL_FLAG_MACH_DEP << 7)
 
+/* (AVR_TINY only): Symbol has attribute absdata */
+#define AVR_SYMBOL_FLAG_TINY_ABSDATA \
+  (SYMBOL_FLAG_MACH_DEP << 8)
+
 #define TINY_ADIW(REG1, REG2, I)\
 "subi " #REG1 ",lo8(-(" #I "))" CR_TAB  \
 "sbci " #REG2 ",hi8(-(" #I "))"
@@ -1790,6 +1794,28 @@ avr_mode_dependent_address_p (const_rtx
 }
 
 
+/* Return true if rtx X is a CONST_INT, CONST or SYMBOL_REF
+   address with the `absdata' variable attribute, i.e. respective
+   data can be read / written by LDS / STS instruction.
+   This is used only for AVR_TINY.  */
+
+static bool
+avr_address_tiny_absdata_p (rtx x, machine_mode mode)
+{
+  if (CONST == GET_CODE (x))
+x = XEXP (XEXP (x, 0), 0);
+
+  if (SYMBOL_REF_P (x))
+return SYMBOL_REF_FLAGS (x) & AVR_SYMBOL_FLAG_TINY_ABSDATA;
+
+  if (CONST_INT_P (x)
+  && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode)))
+return true;
+
+  return false;
+}
+
+
 /* Helper function for `avr_legitimate_address_p'.  */
 
 static inline bool
@@ -1874,8 +1900,7 @@ avr_legitimate_address_p (machine_mode m
   /* avrtiny's load / store instructions only cover addresses 0..0xbf:
  IN / OUT range is 0..0x3f and LDS / STS can access 0x40..0xbf.  */
 
-  ok = (CONST_INT_P (x)
-&& IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode)));
+  ok = avr_address_tiny_absdata_p (x, mode);
 }
 
   if (avr_log.legitimate_address_p)
@@ -1917,8 +1942,7 @@ avr_legitimize_address (rtx x, rtx oldx,
   if (AVR_TINY)
 {
   if (CONSTANT_ADDRESS_P (x)
-  && !(CONST_INT_P (x)
-   && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode
+  && ! avr_address_tiny_absdata_p (x, mode))
 {
   x = force_reg (Pmode, x);
 }
@@ -9148,6 +9172,32 @@ avr_handle_fntype_attribute (tree *node,
 }
 
 static tree
+avr_handle_absdata_attribute (tree *node, tree name, tree /* args */,
+  int /* flags */, bool *no_add)
+{
+  location_t loc = DECL_SOURCE_LOCATION (*node);
+
+  if (AVR_TINY)
+{
+  if (TREE_CODE (*node) != VAR_DECL
+  || (!TREE_STATIC (*node) && !DECL_EXTERNAL (

Re: [PATCH] Fix PR77826

[Richard Biener]

On Tue, 11 Oct 2016, Marc Glisse wrote:

> On Tue, 11 Oct 2016, Richard Biener wrote:
> 
> > > An example that regressed at -O (looking at the .optimized dump)
> > > 
> > > int f(int a, unsigned b){
> > >   a &= 1;
> > >   b &= 1;
> > >   return a&b;
> > > }
> > 
> > Yeah, but it usually also shows a badly written pattern:
> > 
> > /* (X & Y) & (X & Z) -> (X & Y) & Z
> >   (X | Y) | (X | Z) -> (X | Y) | Z  */
> > (for op (bit_and bit_ior)
> > (simplify
> >  (op:c (convert1?@3 (op:c@4 @0 @1)) (convert2?@5 (op:c@6 @0 @2)))
> >  (if (tree_nop_conversion_p (type, TREE_TYPE (@1))
> >   && tree_nop_conversion_p (type, TREE_TYPE (@2)))
> > 
> > so how could we ever match the @0s when we have either of the two
> > conversions not present?  We could only do this then facing constants
> > (due to using operand_equal_p).  We for example fail to handle
> > 
> > (X & Y) & (unsigned) ((singed)X & Z)
> 
> Indeed... (oups, looks like I wrote that one)
> 
> Trying to find other examples
> 
> /* Fold A - (A & B) into ~B & A.  */
> (simplify
>  (minus (convert? @0) (convert?:s (bit_and:cs @0 @1)))
>  (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
>   && tree_nop_conversion_p (type, TREE_TYPE (@1)))
>   (convert (bit_and (bit_not @1) @0
> 
> Hmm, should be convert1/convert2 to handle the case where @0 is a constant.
> 
> /* (X | Y) ^ X -> Y & ~ X*/
> (simplify
>  (bit_xor:c (convert? (bit_ior:c @0 @1)) (convert? @0))
>  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
>   (convert (bit_and @1 (bit_not @0)
> 
> Again, will never match when there is a convert and @0 is a constant.
> 
> (for op (bit_and bit_ior bit_xor)
>  rop (bit_ior bit_and bit_and)
>  (simplify
>   (op (convert? (rop:c @0 @1)) (convert? (rop:c @0 @2)))
> ...
> 
> Again won't match for constant @0 that has a different type in both parts.
> 
> /* (X & Y) & Y -> X & Y
>(X | Y) | Y -> X | Y  */
> (for op (bit_and bit_ior)
>  (simplify
>   (op:c (convert?@2 (op:c @0 @1)) (convert? @1))
>   @2))
> 
> Same issue.
> 
> Ok, not many transformations are concerned, and most need a rewrite anyway...
> 
> In the other direction, looking at the transformations for which we used
> explicitly operand_equal_p as a workaround for lax integer constant matches,
> it doesn't look like changing them back to use matching captures would help,
> it would require duplicating the pattern for constants.

So with doing the same on GENERIC I hit

FAIL: g++.dg/cpp1y/constexpr-array4.C  -std=c++14 (test for excess errors)

with the pattern

  /* (T)(P + A) - (T)P -> (T) A */
  (for add (plus pointer_plus)
   (simplify
(minus (convert (add @0 @1))
 (convert @0))
...
 (convert @1


no longer applying to

(long int) ((int *) &ar + 12) - (long int) &ar

because while (int *) &ar is equal to (long int) &ar (operand_equal_p
does STRIP_NOPS) they obviously do not have the same type.  So on
GENERIC we have to consider that we feed operand_equal_p with
non-ATOMs (arbitrary expressions).  The pattern above is "safe" as
it doesn't reference @0 in the result (not sure if we should take
advantage of that in genmatch).

The other FAILs with doing the same on GENERIC are

FAIL: g++.dg/gomp/declare-simd-3.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/torture/pr71448.C   -O0  (test for excess errors)
FAIL: g++.dg/vect/simd-clone-6.cc  -std=c++11 (test for excess errors)

the simd ones are 'warning: ignoring large linear step' and the pr71448.C
case is very similar to the above.

> > > If we stick to the old behavior, maybe we could have some genmatch magic
> > > to
> > > help with the constant capture weirdness. With matching captures, we could
> > > select which operand (among those supposed to be equivalent) is actually
> > > captured more cleverly, either with an explicit marker, or by giving
> > > priority
> > > to the one that is not immediatly below convert? in the pattern.
> > 
> > This route is a difficult one to take
> 
> The simplest version I was thinking about was @0 for a true capture, and @@0
> for something that just has to be checked for equality with @0.

Hmm, ok.  So you'd have @@0 having to match @0 and we'd get the @0 for
the result in a reliable way?  Sounds like a reasonable idea, I'll see
how that works out (we could auto-detect '@@' if the capture is not
used in the result, see above).

> > -- what would be possible is to
> > capture a specific operand.  Like allow one to write
> > 
> > (op (op @0@4 @1) (op @0@3 @2))
> > 
> > and thus actually have three "names" for @0.  We have this internally
> > already when you write
> > 
> > (convert?@0 @1)
> > 
> > for the case where there is no conversion.  @0 and @1 are the same
> > in this case.
> 
> Looks nice and convenient (assuming lax constant matching).

Yes, w/o lax matching it has of course little value.

> > Not sure if this helps enough cases.
> 
> IIRC, in all cases where we had trouble with operand_equal_p, chosing which
> operand to capture would have solved the issue.

Y

[patch] Comment in rtl.h how rtx flags are dumped in rtl dumps.

[Georg-Johann Lay]

This is a no-op change that just adds comments how the various RTX flags will 
appear in RTL dumps.



Ok for trunk?

Johann

* rtl.h (struct rtx_def): Comment how RTX_FLAGS will be
dumped in RTL dumps.
Index: rtl.h
===
--- rtl.h	(revision 241033)
+++ rtl.h	(working copy)
@@ -317,12 +317,14 @@ struct GTY((desc("0"), tag("0"),
  1 in a CONCAT is VAL_EXPR_IS_COPIED in var-tracking.c.
  1 in a VALUE is SP_BASED_VALUE_P in cselib.c.
  1 in a SUBREG generated by LRA for reload insns.
- 1 in a CALL for calls instrumented by Pointer Bounds Checker.  */
+ 1 in a CALL for calls instrumented by Pointer Bounds Checker.
+ Dumped as "/j" in RTL dumps.  */
   unsigned int jump : 1;
   /* In a CODE_LABEL, part of the two-bit alternate entry field.
  1 in a MEM if it cannot trap.
  1 in a CALL_INSN logically equivalent to
-   ECF_LOOPING_CONST_OR_PURE and DECL_LOOPING_CONST_OR_PURE_P. */
+   ECF_LOOPING_CONST_OR_PURE and DECL_LOOPING_CONST_OR_PURE_P.
+ Dumped as "/c" in RTL dumps.  */
   unsigned int call : 1;
   /* 1 in a REG, MEM, or CONCAT if the value is set at most once, anywhere.
  1 in a SUBREG used for SUBREG_PROMOTED_UNSIGNED_P.
@@ -333,7 +335,8 @@ struct GTY((desc("0"), tag("0"),
  1 in a JUMP_INSN of an annulling branch.
  1 in a CONCAT is VAL_EXPR_IS_CLOBBERED in var-tracking.c.
  1 in a preserved VALUE is PRESERVED_VALUE_P in cselib.c.
- 1 in a clobber temporarily created for LRA.  */
+ 1 in a clobber temporarily created for LRA.
+ Dumped as "/u" in RTL dumps.  */
   unsigned int unchanging : 1;
   /* 1 in a MEM or ASM_OPERANDS expression if the memory reference is volatile.
  1 in an INSN, CALL_INSN, JUMP_INSN, CODE_LABEL, BARRIER, or NOTE
@@ -344,9 +347,10 @@ struct GTY((desc("0"), tag("0"),
  1 in a LABEL_REF, REG_LABEL_TARGET or REG_LABEL_OPERAND note for a
  non-local label.
  In a SYMBOL_REF, this flag is used for machine-specific purposes.
- In a PREFETCH, this flag indicates that it should be considered a scheduling
- barrier.
- 1 in a CONCAT is VAL_NEEDS_RESOLUTION in var-tracking.c.  */
+ In a PREFETCH, this flag indicates that it should be considered a
+ scheduling barrier.
+ 1 in a CONCAT is VAL_NEEDS_RESOLUTION in var-tracking.c.
+ Dumped as "/v" in RTL dumps.  */
   unsigned int volatil : 1;
   /* 1 in a REG if the register is used only in exit code a loop.
  1 in a SUBREG expression if was generated from a variable with a
@@ -360,7 +364,8 @@ struct GTY((desc("0"), tag("0"),
  cleared before used.
 
  The name of the field is historical.  It used to be used in MEMs
- to record whether the MEM accessed part of a structure.  */
+ to record whether the MEM accessed part of a structure.
+ Dumped as "/s" in RTL dumps.  */
   unsigned int in_struct : 1;
   /* At the end of RTL generation, 1 if this rtx is used.  This is used for
  copying shared structure.  See `unshare_all_rtl'.
@@ -377,13 +382,15 @@ struct GTY((desc("0"), tag("0"),
  1 in a REG or MEM if it is a pointer.
  1 in a SYMBOL_REF if it addresses something in the per-function
  constant string pool.
- 1 in a VALUE is VALUE_CHANGED in var-tracking.c.  */
+ 1 in a VALUE is VALUE_CHANGED in var-tracking.c.
+ Dumped as "/f" in RTL dumps.  */
   unsigned frame_related : 1;
   /* 1 in a REG or PARALLEL that is the current function's return value.
  1 in a SYMBOL_REF for a weak symbol.
  1 in a CALL_INSN logically equivalent to ECF_PURE and DECL_PURE_P.
  1 in a CONCAT is VAL_EXPR_HAS_REVERSE in var-tracking.c.
- 1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.c.  */
+ 1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.c.
+ Dumped as "/i" in RTL dumps.  */
   unsigned return_val : 1;
 
   union {

Re: [patch] Comment in rtl.h how rtx flags are dumped in rtl dumps.

[Bernd Schmidt]

On 10/12/2016 01:59 PM, Georg-Johann Lay wrote:

This is a no-op change that just adds comments how the various RTX flags
will appear in RTL dumps.


Ok.


Bernd

Questionable code in gcov-io.c

[Marek Polacek]

While implementing a warning I noticed this in gcov-io.c:

 187   else if (mode == 0)
 188 {
 189   struct stat st;
 190 
 191   if (fstat (fd, &st) < 0)
 192 {
 193   fclose (gcov_var.file);
 194   gcov_var.file = 0;
 195   return 0;
 196 }
 197   if (st.st_size != 0)
 198 gcov_var.mode = 1;
 199   else
 200 gcov_var.mode = mode * 2 + 1;
 201 }
 202   else
 203 gcov_var.mode = mode * 2 + 1;

It seems that lines 198 and 200 do the same thing, at line 200 we know that
mode == 0, so we just assign 1.  Should we just remove the condition on line 
197?

This has been introduced in Jakub's r78281.

Marek

Re: [PATCH] Implement new hook for max_align_t_align

[John David Anglin]

On 2016-10-12, at 4:02 AM, Jakub Jelinek wrote:

>> Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
>> ship it anymore), I stand by my suggestion to bump the fundamental alignment
> 
> Or just drop support for a dead arch?

Hardware is still available on the second hand market.

Linux is available from Debian although not as a release architecture:
https://buildd.debian.org/status/architecture.php?a=hppa&suite=sid
I know from personal use that it is more functional than it has ever been.

It is also still available from gentoo as far as I know.

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

Re: Questionable code in gcov-io.c

[Nathan Sidwell]

On 10/12/16 08:10, Marek Polacek wrote:

While implementing a warning I noticed this in gcov-io.c:

 187   else if (mode == 0)
 188 {
 189   struct stat st;
 190
 191   if (fstat (fd, &st) < 0)
 192 {
 193   fclose (gcov_var.file);
 194   gcov_var.file = 0;
 195   return 0;
 196 }
 197   if (st.st_size != 0)
 198 gcov_var.mode = 1;
 199   else
 200 gcov_var.mode = mode * 2 + 1;
 201 }
 202   else
 203 gcov_var.mode = mode * 2 + 1;

It seems that lines 198 and 200 do the same thing, at line 200 we know that
mode == 0, so we just assign 1.  Should we just remove the condition on line 
197?


ITYM lines 197 -> 203.  I.e. remove the entire if that;s inside the 'mode == 0' 
branch and make line 203 unconditional.


nathan

Re: [PATCH] Do not copy std:call_once arguments (LWG 2442)

[Jonathan Wakely]

On 12/10/16 12:22 +0100, Jonathan Wakely wrote:

This replaces the use of std::__bind_simple by direct calls to
std::__invoke so that the arguments are not decay-copied.

* doc/xml/manual/intro.xml: Document LWG 2442 status.
* include/std/mutex [_GLIBCXX_HAVE_TLS] (__once_call_impl): Remove.
[_GLIBCXX_HAVE_TLS] (_Once_call): Declare primary template and define
partial specialization to unpack args and forward to std::invoke.
(call_once) [_GLIBCXX_HAVE_TLS]: Use forward_as_tuple and _Once_call
instead of __bind_simple and __once_call_impl.
(call_once) [!_GLIBCXX_HAVE_TLS]: Use __invoke instead of
__bind_simple.
* testsuite/30_threads/call_once/dr2442.cc: New test.

Tested powerpc64le-linux, committed to trunk.


I don't know why I over-complicated it by packing and unpacking the
args in a tuple, this is much simpler, and makes the TLS and non-TLS
implementations more similar.

Tested powerpc64le-linux, committed to trunk.


commit f4f65251fbf84547db29d04bfa60ffcf0540a188
Author: Jonathan Wakely 
Date:   Wed Oct 12 12:30:35 2016 +0100

Simplify std::call_once implementation

	* include/std/mutex [_GLIBCXX_HAVE_TLS] (_Once_call): Remove.
	(call_once) [_GLIBCXX_HAVE_TLS]: Simplify by removing _Once_call.

diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 4c6f036..e90006f 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -579,21 +579,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #ifdef _GLIBCXX_HAVE_TLS
   extern __thread void* __once_callable;
   extern __thread void (*__once_call)();
-
-  template::value>::__type>
-struct _Once_call;
-
-  template
-struct _Once_call<_Tuple, _Index_tuple<_Ind...>>
-{
-  static void
-  _S_call()
-  {
-	auto& __f_args = *static_cast<_Tuple*>(__once_callable);
-	std::__invoke(std::get<_Ind>(std::move(__f_args))...);
-  }
-};
 #else
   extern function __once_functor;
 
@@ -613,17 +598,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // 2442. call_once() shouldn't DECAY_COPY()
-#ifdef _GLIBCXX_HAVE_TLS
-  auto __f_args = std::forward_as_tuple(
-	  std::forward<_Callable>(__f), std::forward<_Args>(__args)...);
-  __once_callable = std::__addressof(__f_args);
-  __once_call = _Once_call::_S_call;
-#else
-  unique_lock __functor_lock(__get_once_mutex());
-  __once_functor = [&] {
+  auto __callable = [&] {
 	  std::__invoke(std::forward<_Callable>(__f),
 			std::forward<_Args>(__args)...);
   };
+#ifdef _GLIBCXX_HAVE_TLS
+  __once_callable = std::__addressof(__callable);
+  __once_call = []{ (*(decltype(__callable)*)__once_callable)(); };
+#else
+  unique_lock __functor_lock(__get_once_mutex());
+  __once_functor = __callable;
   __set_once_functor_lock_ptr(&__functor_lock);
 #endif
 

Re: [SPARC] Tidy up Condition Code support and more

[Jose E. Marchesi]

> Note that the ADDXC, ADDXCCC, SUBXC and SUBXCCC instructions do not
> support immediate operands.  Hence the patch breaks vis3-enabling arches
> and niagara-7.

Egad.  I totally overlooked this (yet the -mcpu=niagara7 -m64 testsuite is 
rather explicit) and this is a bit of a shame, especially for SUBXC which 
becomes less useful, you need something like:

long foo2 (long a, long i)
{
  return a - (i != 0);
}

to generate it:

cmp %g0, %o1
jmp %o7+8
 subxc  %o0, %g0, %o0


Yes, it is unfortunate for these instructions to not support 1,i,d
formats.

I am bringing this up with the hardware chaps to see if this could be
fixed in future revisions of the architecture.  Will keep you posted.

> (Yes, ADXCC and SUBXCC both support immediate operands, it can be
> confusing :/)

The mere (re-)naming is already confusing. ;-)

Thanks for the heads up.  I have installed the attached corrective patch.

Thanks!  I am testing it right now.

Re: Set nonnull attribute to ptr_info_def based on VRP

[Richard Biener]

On Wed, Oct 12, 2016 at 8:46 AM, kugan
 wrote:
> Hi Richard,
>
>
>
> On 07/10/16 21:03, Richard Biener wrote:
>>
>> On Fri, Oct 7, 2016 at 2:53 AM, kugan 
>> wrote:
>>>
>>> Hi Richard,
>>>
>>> Thanks for the review.
>>>
>>>
>>> On 09/08/16 18:58, Richard Biener wrote:


 On Tue, Aug 9, 2016 at 12:58 AM, kugan
  wrote:
>
>
> Hi Jakub,
>
> Thanks for the review.
>
> On 08/08/16 16:40, Jakub Jelinek wrote:
>>
>>
>>
>> On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
>>>
>>>
>>>
>>> diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
>>> index c81b1a1..6e34433 100644
>>> --- a/gcc/tree-ssanames.h
>>> +++ b/gcc/tree-ssanames.h
>>> @@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
>>>   above alignment.  Access only through the same helper functions
>>> as
>>> align
>>>   above.  */
>>>unsigned int misalign;
>>> +  /* When this pointer is knonw to be nnonnull this would be true
>>> otherwise
>>> + false.  */
>>> +  bool  nonnull_p;
>>>  };
>>
>>
>>
>>
>> Why do you need this?  Doesn't the pt.null bit represent that already?
>
>
>
>
> It looks like I can use this. As in gcc/tree-ssa-alias.h:
>
>   /* Nonzero if the points-to set includes 'nothing', the points-to set
>  includes memory at address NULL.  */
>   unsigned int null : 1;
>
> But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following
> comment
> which says:
>
> /* ???  We'd like to handle ptr1 != NULL and ptr1 != ptr2
>  but those require pt.null to be conservatively correct.  */
>
> Does that means it can be wrong at times? I haven't looked it in detail
> yet
> but if it is, it would be a problem.



 Yes, this bit is not correctly computed for all cases (well, it works
 for trivial
 ones but for example misses weaks and maybe some other corner-cases).

 Currently there are no consumers of this bit so the incorrectness
 doesn't matter.

 I suggest you simply use that bit but set it conservatively from PTA (to
 true)
 for now and adjust it from VRP only.

 I do have some patches for fixinig some of the .nonnull_p issues in PTA
 but
 they come at a cost of more constraints.
>>>
>>>
>>>
>>> Here is a version of the patch that does this. I have a follow up patch
>>> to
>>> use this in ipa-vrp. I will send it for review once this is OK.
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new
>>> regressions. Is this OK for trunk?
>>
>>
>> @@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t
>> orig_vi)
>>if (vi->is_artificial_var)
>> {
>>   if (vi->id == nothing_id)
>> -   pt->null = 1;
>> +   ;
>>
>> please leave this here.
>
> Done.
>
>>
>>  pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
>>  {
>>if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
>> -  || pt->null || pt->vars == NULL
>> +  || pt->vars == NULL
>>|| !bitmap_single_bit_set_p (pt->vars))
>>  return false;
>>
>> humm... this is a correctness problem I guess.  A latent one currently
>> depending on who relies on it.
>> There is a single use of the above predicate which looks for the
>> points-to solution of a __builtin_alloca call.  We should somehow
>> arrange for its solution to not include ->null.  Or, simpler, change
>> the predicates name to pt_solution_singleton_or_null_p () as
>> the use does not care for pt->null.
>
>
> I have changed the name.
>
>>
>> +void
>> +set_ptr_nonnull (tree name)
>> +{
>> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
>> +  struct ptr_info_def *pi = get_ptr_info (name);
>> +  pi->pt.null = 0;
>>
>>  = false;
>>
>> +}
>>
>> There is the question on how pt.null semantics should be
>> with respect to pt.anything, pt.escaped and pt.nonlocal.
>> I think get_ptr_nonnull needs to return false if any
>> of those are set.
>
>
> I have added pt.anything.
>
> I am assuming if pt.escaped, it cannot become null?

Sure can it become null.

> My intention is to eliminate for pt.nonlocal too. Like in:
>
> static __attribute__((noinline, noclone))
> int foo (int *p)
> {
>   if (!p)
> return 0;
>   *p = 1;
> }
>
> struct st
> {
>   int a;
>   int b;
> };
>
> int bar (struct st *s)
> {
>
>   if (!s)
> return 0;
>   foo (&s->a);
>   foo (&s->b);
> }

I don't think this is sth PTA handles.

> And also in find_what_p_points_to we can overwrite pt.null. So I have
> changed that too.
>
> Please find the updated patch attached. Does this look better?

@@ -515,8 +515,8 @@ dump_points_to_solution (FILE *file, struct pt_solution *pt)
   if (pt->ipa_escaped)
 fprintf (file, ", points-to unit escaped");

-  if (pt->null)
-fprintf (file, ", points-to NULL");
+  if (!pt->null)
+fprintf (file, ", points-to n

Re: Questionable code in gcov-io.c

[Bernd Schmidt]

On 10/12/2016 02:10 PM, Marek Polacek wrote:

While implementing a warning I noticed this in gcov-io.c:

 187   else if (mode == 0)
 188 {
 189   struct stat st;
 190
 191   if (fstat (fd, &st) < 0)
 192 {
 193   fclose (gcov_var.file);
 194   gcov_var.file = 0;
 195   return 0;
 196 }
 197   if (st.st_size != 0)
 198 gcov_var.mode = 1;
 199   else
 200 gcov_var.mode = mode * 2 + 1;
 201 }
 202   else
 203 gcov_var.mode = mode * 2 + 1;

It seems that lines 198 and 200 do the same thing, at line 200 we know that
mode == 0, so we just assign 1.  Should we just remove the condition on line 
197?


The intention seems to be that a negative gcov_var.mode means we're 
writing, so for a size zero file maybe that's the expected result. But 
of course none of the existing code is going to expect that.


There are more oddities here...

Before the quoted piece of code, we test for mode > 0, so in line 203 we 
know that mode is < 0. I don't really see what the "mode * 2 + 1" 
expression is supposed to do anyway, given that the caller could pass in 
any positive/negative number and expect the same results as for 1 and 
-1. I think line 203 should just use -1. There's one further occurrence 
of that expression which should probably be modified.


The function comment also seems to have an issue:

/* Open a gcov file. NAME is the name of the file to open and MODE
   indicates whether a new file should be created, or an existing file
   opened. If MODE is >= 0 an existing file will be opened, if
   possible, and if MODE is <= 0, a new file will be created. Use
   MODE=0 to attempt to reopen an existing file and then fall back on
   creating a new one.  If MODE < 0, the file will be opened in
   read-only mode.  Otherwise it will be opened for modification.
   Return zero on failure, >0 on opening an existing file and <0 on
   creating a new one.  */

This suggests that with MODE < 0, we'll create a file in read-only mode, 
which is nonsensical. The code suggests that the comment should read " > 0".



Bernd

Re: [vrp] use get_ptr_nonnull in tree-vrp

[Richard Biener]

On Wed, Oct 12, 2016 at 8:56 AM, kugan
 wrote:
> Hi,
>
> This patch uses get_ptr_nonnull in tree-vrp.
>
> Bootstrapped and regression tested this with other patched without any
> new regressions on x86_64-linux-gnu.
>
> Is this OK for trunk?

Um.  Doesn't make much sense given nothing provides this info before EVRP?
And if it makes sense then it makes sense not only for PARM_DECL SSA names.

Richard.

> Thanks,
> Kugan
>
> gcc/testsuite/ChangeLog:
>
> 2016-10-12  Kugan Vivekanandarajah  
>
> * gcc.dg/ipa/vrp4.c: Adjust testcase.
>
> gcc/ChangeLog:
>
> 2016-10-12  Kugan Vivekanandarajah  
>
> * tree-vrp.c (get_value_range): Check get_ptr_nonnull.

[Ada] Preliminary work to support relative delays on extended ravenscar

[Arnaud Charlet]

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Tristan Gingold  

* exp_ch9.adb (Expand_N_Delay_Relative_Statement): Add support
for a secondary procedure in case of missing Ada.Calendar.Delays
* rtsfind.ads (RTU_Id): Add System_Relative_Delays.
(RE_Id): Add RO_RD_Delay_For.
* rtsfind.adb (Output_Entity_Name): Handle correctly units RO_XX.
* s-rident.ads: Remove No_Relative_Delays
restriction for GNAT_Extended_Ravenscar.

Index: exp_ch9.adb
===
--- exp_ch9.adb (revision 241029)
+++ exp_ch9.adb (working copy)
@@ -8388,11 +8388,23 @@
--  simple delays imposed by the use of Protected Objects.
 
procedure Expand_N_Delay_Relative_Statement (N : Node_Id) is
-  Loc : constant Source_Ptr := Sloc (N);
+  Loc  : constant Source_Ptr := Sloc (N);
+  Proc : Entity_Id;
begin
+  if RTE_Available (RO_RD_Delay_For) then
+ --  Try to use System.Relative_Delays.Delay_For only if available.
+ --  This is the implementation used on restricted platforms when
+ --  Ada.Calendar is not available.
+ Proc := RTE (RO_RD_Delay_For);
+  else
+ --  Otherwise, use Ada.Calendar.Delays.Delay_For and emit an error
+ --  message if not available.
+ Proc := RTE (RO_CA_Delay_For);
+  end if;
+
   Rewrite (N,
 Make_Procedure_Call_Statement (Loc,
-  Name => New_Occurrence_Of (RTE (RO_CA_Delay_For), Loc),
+  Name => New_Occurrence_Of (Proc, Loc),
   Parameter_Associations => New_List (Expression (N;
   Analyze (N);
end Expand_N_Delay_Relative_Statement;
Index: rtsfind.adb
===
--- rtsfind.adb (revision 241024)
+++ rtsfind.adb (working copy)
@@ -6,7 +6,7 @@
 --  --
 -- B o d y  --
 --  --
---  Copyright (C) 1992-2015, Free Software Foundation, Inc. --
+--  Copyright (C) 1992-2016, Free Software Foundation, Inc. --
 --  --
 -- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
@@ -1144,6 +1144,9 @@
   --  M (1 .. P) is current message to be output
 
   RE_Image : constant String := RE_Id'Image (Id);
+  S : Natural;
+  --  RE_Image (S .. RE_Image'Last) is the name of the entity without the
+  --  "RE_" or "RO_XX_" prefix.
 
begin
   if Id = RE_Null then
@@ -1168,8 +1171,15 @@
 
   --  Add entity name and closing quote to message
 
-  Name_Len := RE_Image'Length - 3;
-  Name_Buffer (1 .. Name_Len) := RE_Image (4 .. RE_Image'Length);
+  if RE_Image (2) = 'E' then
+ --  Strip "RE"
+ S := 4;
+  else
+ --  Strip "RO_XX"
+ S := 7;
+  end if;
+  Name_Len := RE_Image'Length - S + 1;
+  Name_Buffer (1 .. Name_Len) := RE_Image (S .. RE_Image'Last);
   Set_Casing (Mixed_Case);
   M (P + 1 .. P + Name_Len) := Name_Buffer (1 .. Name_Len);
   P := P + Name_Len;
Index: rtsfind.ads
===
--- rtsfind.ads (revision 241024)
+++ rtsfind.ads (working copy)
@@ -6,7 +6,7 @@
 --  --
 -- S p e c  --
 --  --
---  Copyright (C) 1992-2015, Free Software Foundation, Inc. --
+--  Copyright (C) 1992-2016, Free Software Foundation, Inc. --
 --  --
 -- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
@@ -349,6 +349,7 @@
   System_Pool_Empty,
   System_Pool_Local,
   System_Pool_Size,
+  System_Relative_Delays,
   System_RPC,
   System_Scalar_Values,
   System_Secondary_Stack,
@@ -1403,6 +1404,8 @@
  RE_Tk_Objref,   -- System.Partition_Interface
  RE_Tk_Union,-- System.Partition_Interface
 
+ RO_RD_Delay_For,-- System.Relative_Delays
+
  RE_IS_Is1,  -- System.Scalar_Values
  RE_IS_Is2,  -- System.Scalar_Values
  RE_IS_Is4,  -- System.Scalar_Values
@@ -2635,6 +2638,8 @@
 
  RE_Stack_Bounded_Pool   => System_Pool_Size,
 
+ RO_RD_Delay_For => Sys

[Ada] Renaming of functions to check file's readability/writability

[Arnaud Charlet]

Old functions to check readability/writability of file based
on file's attributes had been replaced by new more precise
functions. Old functions are provided under new names.

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Vadim Godunko  

* s-os_lib.ads (Is_Owner_Readable_File): Renamed from
Is_Readable_File.
(Is_Owner_Writable_File): Renamed from Is_Writable_File.
(Is_Readable_File): Renames Is_Read_Accessible_File.
(Is_Writable_File): Renames Is_Write_Accessible_File.

Index: s-os_lib.adb
===
--- s-os_lib.adb(revision 241024)
+++ s-os_lib.adb(working copy)
@@ -1510,24 +1510,24 @@
   return Is_Read_Accessible_File (F_Name'Address) /= 0;
end Is_Read_Accessible_File;
 
-   --
-   -- Is_Readable_File --
-   --
+   
+   -- Is_Owner_Readable_File --
+   
 
-   function Is_Readable_File (Name : C_File_Name) return Boolean is
+   function Is_Owner_Readable_File (Name : C_File_Name) return Boolean is
   function Is_Readable_File (Name : Address) return Integer;
   pragma Import (C, Is_Readable_File, "__gnat_is_readable_file");
begin
   return Is_Readable_File (Name) /= 0;
-   end Is_Readable_File;
+   end Is_Owner_Readable_File;
 
-   function Is_Readable_File (Name : String) return Boolean is
+   function Is_Owner_Readable_File (Name : String) return Boolean is
   F_Name : String (1 .. Name'Length + 1);
begin
   F_Name (1 .. Name'Length) := Name;
   F_Name (F_Name'Last)  := ASCII.NUL;
-  return Is_Readable_File (F_Name'Address);
-   end Is_Readable_File;
+  return Is_Owner_Readable_File (F_Name'Address);
+   end Is_Owner_Readable_File;
 

-- Is_Executable_File --
@@ -1601,24 +1601,24 @@
   return Is_Write_Accessible_File (F_Name'Address) /= 0;
end Is_Write_Accessible_File;
 
-   --
-   -- Is_Writable_File --
-   --
+   
+   -- Is_Owner_Writable_File --
+   
 
-   function Is_Writable_File (Name : C_File_Name) return Boolean is
+   function Is_Owner_Writable_File (Name : C_File_Name) return Boolean is
   function Is_Writable_File (Name : Address) return Integer;
   pragma Import (C, Is_Writable_File, "__gnat_is_writable_file");
begin
   return Is_Writable_File (Name) /= 0;
-   end Is_Writable_File;
+   end Is_Owner_Writable_File;
 
-   function Is_Writable_File (Name : String) return Boolean is
+   function Is_Owner_Writable_File (Name : String) return Boolean is
   F_Name : String (1 .. Name'Length + 1);
begin
   F_Name (1 .. Name'Length) := Name;
   F_Name (F_Name'Last)  := ASCII.NUL;
-  return Is_Writable_File (F_Name'Address);
-   end Is_Writable_File;
+  return Is_Owner_Writable_File (F_Name'Address);
+   end Is_Owner_Writable_File;
 
--
-- Kill --
Index: s-os_lib.ads
===
--- s-os_lib.ads(revision 241024)
+++ s-os_lib.ads(working copy)
@@ -425,7 +425,7 @@
--  not actually be readable due to some other process having exclusive
--  access.
 
-   function Is_Readable_File (Name : String) return Boolean;
+   function Is_Owner_Readable_File (Name : String) return Boolean;
--  Determines if the given string, Name, is the name of an existing file
--  that is readable. Returns True if so, False otherwise. Note that this
--  function simply interrogates the file attributes (e.g. using the C
@@ -449,7 +449,7 @@
--  contains the name of the file to which it is linked. Symbolic links may
--  span file systems and may refer to directories.
 
-   function Is_Writable_File (Name : String) return Boolean;
+   function Is_Owner_Writable_File (Name : String) return Boolean;
--  Determines if the given string, Name, is the name of an existing file
--  that is writable. Returns True if so, False otherwise. Note that this
--  function simply interrogates the file attributes (e.g. using the C
@@ -465,6 +465,14 @@
--  Determines if the given string, Name, is the name of an existing file
--  that is writable. Returns True if so, False otherwise.
 
+   function Is_Readable_File (Name : String) return Boolean
+ renames Is_Read_Accessible_File;
+   function Is_Writable_File (Name : String) return Boolean
+ renames Is_Write_Accessible_File;
+   --  These subprograms provided for backward compatibility and should not be
+   --  used. Use Is_Owner_Readable_File/Is_Owner_Writable_File or
+   --  Is_Read_Accessible_File/Is_Write_Accessible_File instead.
+
function Locate_Exec_On_Path (Exec_Name : String) return String_Access;
--  Try to locate an executable whose name is given by Exec_Name in the
--  directories listed in 

[PATCH, Fortran] Four small DEC extensions

[Fritz Reese]

Here I submit for review four small extensions to the GNU Fortran
frontend for compatibility with legacy code. I figure it might be a
nice change of pace from my larger patches. Never fear, for I have
more large patches to come, which I will continue to submit
one-at-a-time.

Each extension is enabled with its own compile flag, and each are
enabled along with -fdec. They are fairly self-explanatory.

1. [-fdec-feed] Treat form feed characters as whitespace.
2. [-fdec-type-print] Treat TYPE as an alias for PRINT where applicable.
3. [-fdec-loc-rval] Allow %LOC() as an rvalue, equivalent to using the
LOC() intrinsic.
4. [-fdec-logical-xor] Enable .XOR. as a logical operator.

Feel free to comment on/question/approve each patch individually as
desired. They are attached as sequential patch files for ease of
review. Bootstraps & regtests on x86_64-redhat-
linux. OK for trunk?

---
Fritz Reese


0001-dec-feed.patch
Description: Binary data


0002-dec-type-print.patch
Description: Binary data


0003-dec-loc-rval.patch
Description: Binary data


0004-dec-logical-xor.patch
Description: Binary data

Re: [PATCH] Implement new hook for max_align_t_align

[Bernd Schmidt]

On 10/12/2016 02:12 PM, John David Anglin wrote:

On 2016-10-12, at 4:02 AM, Jakub Jelinek wrote:


Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
ship it anymore), I stand by my suggestion to bump the fundamental alignment


Or just drop support for a dead arch?


Hardware is still available on the second hand market.


So is the Commodore 64, but is that enough though to keep supporting PA 
in gcc? Anyone who wants to do retrocomputing can still use gcc-6 or 
earlier versions.



Bernd

[Ada] Crash on overloaded function call with limited view

[Arnaud Charlet]

This patch fixes a compiler abort on a call that is initially overloaded,
when the resolved function returns the limited view of a type.

The folllowing must compile quietly:

   gcc -c p.adb

---
with R;

package body P is

   function Get (A : Q.Ptr) return Integer is
   begin
  return R.Conv (A.Get);
   end;

end P;
---
with Q;

package P is

   function Get (A : Q.Ptr) return Integer;

end P;
---
limited with R;

package Q is

   type Int1 is limited interface;

   function Get (Self : access Int1) return Integer is abstract;

   type Int2 is limited interface and Int1;

   function Get (Self : access Int2) return R.Rec2 is abstract;

   type Ptr is access all Int2'Class;

end Q;
---
package R is

   type Rec1 is tagged null record;

   function Conv (Item : Rec1) return Integer;

   type Rec2 is new Rec1 with null record;

end R;

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Ed Schonberg  

* sem_res.adb (Resolve_Call): If a function call returns a
limited view of a type replace it with the non-limited view,
which must be available when compiling call.  This was already
done elsewhere for non-overloaded calls, but needs to be done
after resolution if function name is overloaded.

Index: sem_res.adb
===
--- sem_res.adb (revision 241024)
+++ sem_res.adb (working copy)
@@ -6034,6 +6034,15 @@
  end;
 
   else
+ --  If the function returns the limited view of type, the call must
+ --  appear in a context in which the non-limited view is available.
+ --  As is done in Try_Object_Operation, use the available view to
+ --  prevent back-end confusion.
+
+ if From_Limited_With (Etype (Nam)) then
+Set_Etype (Nam, Available_View (Etype (Nam)));
+ end if;
+
  Set_Etype (N, Etype (Nam));
   end if;
 

[Ada] Illegal overriding indicators not flagged for procedure named Initialize

[Arnaud Charlet]

In cases of dispatching operations with names Initialize, Adjust, and Finalize
that will override an inherited procedure at the point of the full type but
not at their point of declaration, the compiler was not properly flagging
an overriding_indicator on such a procedure as illegal, because procedures with
such names are treated specially when privately inherited. The inherited parent
operations shouldn't be visible, but when they come from a controlled type
they're visibly inherited because there's currently a dependence on being
able to find them when generating implicit controlled calls. We add a test
for the case of having a controlled parent, so that at least in noncontrolled
cases an operation with a name such as Initialize will be privately inherited
and a homographic overriding procedure won't override the inherited procedure
at the point of its declaration. This ensures that legality of overriding_
indicators is checked properly in that case.

The test below should produce the following errors when parent-overriding_child
is compiled:

$ gcc -c -gnatj70 parent-overriding_child.adb

parent-overriding_child.ads:11:04: subprogram "Private_Primitive" is
   not overriding
parent-overriding_child.ads:14:04: subprogram "Initialize" is not
   overriding
parent-overriding_child.ads:17:04: subprogram "Adjust" is not
   overriding
parent-overriding_child.ads:20:04: subprogram "Finalize" is not
   overriding



package Parent is

   type TT is tagged private;

   procedure Visible_Primitive (X : TT);

private

   type TT is tagged null record;

   procedure Private_Primitive (X : TT);

   procedure Initialize (X : in out TT);

   procedure Adjust (X : in out TT);

   procedure Finalize (X : in out TT);

   procedure Other_Primitive (X : TT);

end Parent;

package Parent.Overriding_Child is

   type NTT is new TT with private;

private

   overriding
   procedure Visible_Primitive (X : NTT);  -- OK (overridden procedure visible)

   overriding
   procedure Private_Primitive (X : NTT);  -- ERROR (too early)

   overriding
   procedure Initialize (X : in out NTT);  -- ERROR (too early)

   overriding
   procedure Adjust (X : in out NTT);  -- ERROR (too early)

   overriding
   procedure Finalize (X : in out NTT);-- ERROR (too early)

   type NTT is new TT with null record;

   overriding
   procedure Other_Primitive (X : NTT);-- OK (overridden procedure visible)

end Parent.Overriding_Child;

package body Parent.Overriding_Child is

   overriding
   procedure Visible_Primitive (X : NTT) is
   begin
  null;
   end Visible_Primitive;

   overriding
   procedure Private_Primitive (X : NTT) is
   begin
  null;
   end Private_Primitive;

   overriding
   procedure Initialize (X : in out NTT) is
   begin
  null;
   end Initialize;

   overriding
   procedure Adjust (X : in out NTT) is
   begin
  null;
   end Adjust;

   overriding
   procedure Finalize (X : in out NTT) is
   begin
  null;
   end Finalize;

   overriding
   procedure Other_Primitive (X : NTT) is
   begin
  null;
   end Other_Primitive;

end Parent.Overriding_Child;

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Gary Dismukes  

* sem_ch3.adb (Derive_Subprogram): Add test
for Is_Controlled of Parent_Type when determining whether an
inherited subprogram with one of the special names Initialize,
Adjust, or Finalize should be derived with its normal name even
when inherited as a private operation (which would normally
result in the inherited operation having a special "hidden" name).

Index: sem_ch3.adb
===
--- sem_ch3.adb (revision 241026)
+++ sem_ch3.adb (working copy)
@@ -14757,9 +14757,10 @@
 or else Is_Internal (Parent_Subp)
 or else Is_Private_Overriding
 or else Is_Internal_Name (Chars (Parent_Subp))
-or else Nam_In (Chars (Parent_Subp), Name_Initialize,
- Name_Adjust,
- Name_Finalize)
+or else (Is_Controlled (Parent_Type)
+  and then Nam_In (Chars (Parent_Subp), Name_Initialize,
+Name_Adjust,
+Name_Finalize))
   then
  Set_Derived_Name;
 

Re: [PATCH][check_GNU_style.sh] More aggressively ignore dg-xxx directives

[Kyrill Tkachov]

On 12/10/16 11:18, Kyrill Tkachov wrote:


On 12/10/16 10:57, Kyrill Tkachov wrote:


On 11/10/16 20:19, Jakub Jelinek wrote:

On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:

Also, the pattern that starts with "/\+\+\+" looks like it's missing
the ^ anchor.  Presumably it should be "/^\+\+\+ \/testsuite\//".

No, it will be almost never +++ /testsuite/
There needs to be .* in between "+++ " and "/testsuite/", and perhaps
it should also ignore "+++ testsuite/".
So /^\+\+\+ (.*\/)?testsuite\// ?
Also, normally (when matching $0) there won't be newlines in the text.

Jakub


Thanks.
Here is the updated patch with your suggestions.



Actually, I've encountered a problem:

 85 # Remove the testsuite part of the diff.  We don't care about GNU style
 86 # in testcases and the dg-* directives give too many false positives.
 87 remove_testsuite ()
 88 {
 89   awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
 90{if (!testsuite) print} /^\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
 91 }
 92
 93 grep $format '^+' $files \
 94 | remove_testsuite \
 95 | grep -v ':+++' \
 96 > $inp


The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor is used.
The awk command matches fine by itself but not when fed from the "grep $format '^+' 
$files"
command because grep adds the line numbers and file names.
So is it okay to omit the ^ here?



Here is the patch omitting the ^.
Thanks,
Kyrill

2016-10-12  Kyrylo Tkachov  

* check_GNU_style.sh (remove_testsuite): New function.
Use it to remove testsuite from the diff.


Thanks,
Kyrill



diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index 87a276c9cf47b5e07c4407f740ce05dce1928c30..92ce2eee2067b067c002f60b4a81eb3a5fb98ec5 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -81,7 +81,17 @@ if [ $nfiles -eq 1 ]; then
 else
 format="-nH"
 fi
+
+# Remove the testsuite part of the diff.  We don't care about GNU style
+# in testcases and the dg-* directives give too many false positives.
+remove_testsuite ()
+{
+  awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
+   {if (!testsuite) print} /\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
+}
+
 grep $format '^+' $files \
+| remove_testsuite \
 | grep -v ':+++' \
 > $inp
 
@@ -160,8 +170,9 @@ col (){
 	fi
 
 	# Don't reuse $inp, which may be generated using -H and thus contain a
-	# file prefix.
-	grep -n '^+' $f \
+	# file prefix.  Re-remove the testsuite since we're not using $inp.
+	remove_testsuite $f \
+	| grep -n '^+' \
 	| grep -v ':+++' \
 	> $tmp
 
@@ -174,11 +185,10 @@ col (){
 	# Expand tabs to spaces according to tab positions.
 	# Keep long lines, make short lines empty.  Print the part past 80 chars
 	# in red.
-# Don't complain about dg-xxx directives in tests.
 	cat "$tmp" \
 	| sed 's/^[0-9]*:+//' \
 	| expand \
-	| awk '$0 !~ /{[[:space:]]*dg-(error|warning|message)[[:space:]]/ { \
+	| awk '{ \
 		 if (length($0) > 80) \
 		   printf "%s\033[1;31m%s\033[0m\n", \
 			  substr($0,1,80), \

Re: [PATCH] Implement new hook for max_align_t_align

[Richard Biener]

On Wed, Oct 12, 2016 at 2:33 PM, Bernd Schmidt  wrote:
> On 10/12/2016 02:12 PM, John David Anglin wrote:
>>
>> On 2016-10-12, at 4:02 AM, Jakub Jelinek wrote:
>>
 Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
 ship it anymore), I stand by my suggestion to bump the fundamental
 alignment
>>>
>>>
>>> Or just drop support for a dead arch?
>>
>>
>> Hardware is still available on the second hand market.
>
>
> So is the Commodore 64, but is that enough though to keep supporting PA in
> gcc? Anyone who wants to do retrocomputing can still use gcc-6 or earlier
> versions.

I'd say what applies to PA should apply equally well to the pdp11 and
the alpha port ...

But usually the question is just whether the port has a maintainer
and/or whether it is
a maintainance burden to keep it (say, last user of obsolete feature X).

Richard.

>
> Bernd

Re: [PATCH] Implement new hook for max_align_t_align

[Bernd Schmidt]

On 10/12/2016 02:43 PM, Richard Biener wrote:

I'd say what applies to PA should apply equally well to the pdp11 and
the alpha port ...

But usually the question is just whether the port has a maintainer
and/or whether it is
a maintainance burden to keep it (say, last user of obsolete feature X).


Well, we seem to be running into a problem with PA, and pdp11 is a cc0 port.


Bernd

Re: [PATCH] Fix PR77826

[Marc Glisse]

On Wed, 12 Oct 2016, Richard Biener wrote:


So with doing the same on GENERIC I hit

FAIL: g++.dg/cpp1y/constexpr-array4.C  -std=c++14 (test for excess errors)

with the pattern

 /* (T)(P + A) - (T)P -> (T) A */
 (for add (plus pointer_plus)
  (simplify
   (minus (convert (add @0 @1))
(convert @0))


Ah, grep missed that one because it is on 2 lines :-(


   ...
(convert @1


no longer applying to

(long int) ((int *) &ar + 12) - (long int) &ar

because while (int *) &ar is equal to (long int) &ar (operand_equal_p
does STRIP_NOPS) they obviously do not have the same type.


I believe we are comparing (int *) &ar to &ar, not to (long int) &ar. But 
yes, indeed.


So on GENERIC we have to consider that we feed operand_equal_p with 
non-ATOMs (arbitrary expressions).  The pattern above is "safe" as it 
doesn't reference @0 in the result (not sure if we should take advantage 
of that in genmatch).


Since we are in the process of defining an 
operand_equal_for_(generic|gimple)_match_p, we could tweak it to check the 
type only for INTEGER_CST, or to still STRIP_NOPS, or similar.


Or we could remain very strict and refine the pattern, allowing a convert 
on the pointer (we might want to split the plus and pointer_plus versions 
then, for clarity). There are not many optimizations that are mandated by 
front-ends and for which this is an issue.



The other FAILs with doing the same on GENERIC are

FAIL: g++.dg/gomp/declare-simd-3.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/torture/pr71448.C   -O0  (test for excess errors)
FAIL: g++.dg/vect/simd-clone-6.cc  -std=c++11 (test for excess errors)

the simd ones are 'warning: ignoring large linear step' and the pr71448.C
case is very similar to the above.


Yes, I expect they all come from the same 1 or 2 transformations.

If we stick to the old behavior, maybe we could have some genmatch 
magic to help with the constant capture weirdness. With matching 
captures, we could select which operand (among those supposed to be 
equivalent) is actually captured more cleverly, either with an 
explicit marker, or by giving priority to the one that is not 
immediatly below convert? in the pattern.


This route is a difficult one to take


The simplest version I was thinking about was @0 for a true capture, and @@0
for something that just has to be checked for equality with @0.


Hmm, ok.  So you'd have @@0 having to match @0 and we'd get the @0 for
the result in a reliable way?  Sounds like a reasonable idea, I'll see
how that works out (we could auto-detect '@@' if the capture is not
used in the result, see above).


It probably doesn't bring much compared to the @0@4 syntax you were 
suggesting below, so if that is easier...



-- what would be possible is to
capture a specific operand.  Like allow one to write

(op (op @0@4 @1) (op @0@3 @2))

and thus actually have three "names" for @0.  We have this internally
already when you write

(convert?@0 @1)

for the case where there is no conversion.  @0 and @1 are the same
in this case.


Looks nice and convenient (assuming lax constant matching).


Yes, w/o lax matching it has of course little value.


Not sure if this helps enough cases.


IIRC, in all cases where we had trouble with operand_equal_p, chosing which
operand to capture would have solved the issue.


Yes.  We'd still need to actually catch all those cases...


Being forced to chose which operand we capture (say with @ vs @@, making 2 
occurences of @0 a syntax error) might help, but it would also require 
updating many patterns for which this was never an issue (easy but 
boring).



I still think that having two things matched that are not really
the same is werid (and a possible source of errors as in, ICEs,
rather than missed optimizations).


Ok. Let's go with the strict matching, it is indeed less confusing.


I'll hold off with the GENERIC strict matching and will investigate
the @@ idea (plus auto-detecting it).


And if we move to stricter matching, maybe genmatch could be updated so
convert could also match integer constants in some cases.


You mean when trying to match @0 ... (convert @0) and @0 is an INTEGER_CST
allow then (convert ...) case to match an INTEGER_CST of different type?
That's an interesting idea (not sure how to implement that though).


Yes, though I am not sure of the exact semantics that would work best.

On a related note, seeing duplicated patterns for constants, I tried several
variants of

(match (mynot @0)
 (bit_not @0))
(match (mynot @0)
 INTEGER_CST@0
 (if (@0 = wide_int_to_tree (TREE_TYPE (@0), wi::bit_not (@0)

(simplify
 (minus (bit_and:cs @0 (mynot @1)) (bit_and:cs @0 @1))
  (minus (bit_xor @0 @1) @1))

This kind of hack feels wrong, but I don't see a proper way to write it.


Yes.  The above is very much a hack.  Must have been me inventing it
just to avoid duplicating the pattern.


Uh, no, don't worry, we don't have that hack in match.pd, we have 2 
patterns. The hack is just me trying to rem

Re: Questionable code in gcov-io.c

[Jakub Jelinek]

On Wed, Oct 12, 2016 at 02:23:36PM +0200, Bernd Schmidt wrote:
> >It seems that lines 198 and 200 do the same thing, at line 200 we know that
> >mode == 0, so we just assign 1.  Should we just remove the condition on line 
> >197?
> 
> The intention seems to be that a negative gcov_var.mode means we're writing,
> so for a size zero file maybe that's the expected result. But of course none
> of the existing code is going to expect that.
> 
> There are more oddities here...

Unfortunately I really don't remember, and it seems we don't have the
posting in gcc-patches archive at all.
Found it in http://marc.info/?l=gcc-patches&m=107747608611324 though.

Jakub

[Ada] Spurious use visibility with limited with and child packages

[Arnaud Charlet]

This patch corrects a minor issue whereby declarations that occur
immediately within package P are erroneously treated as potentially
use-visible when a parent package has a limited with clause on P, a
child package has a with clause on P, and the child package is a
client of another unit. 


-- Source --


--  buffer.ads

package Buffer is
   Junk : Integer;
end;

--  d.ads

limited with Buffer;
package D is
   Junk : Integer;
end;

--  d-te.ads

with Buffer;
package D.Te is
   Junk : Integer;
end;

-- f.ads

with D.Te;
package F is
   use Buffer; -- This should cause an error
end;


-- Compilation and output --


$ gcc -c f.ads
f.ads:3:08: "Buffer" is not visible
f.ads:3:08: non-visible declaration at buffer.ads:1

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Justin Squirek  

* sem_ch10.adb (Remove_Limited_With_Clause): Add a check to
detect accidental visibility.

Index: sem_ch10.adb
===
--- sem_ch10.adb(revision 241024)
+++ sem_ch10.adb(working copy)
@@ -6377,6 +6377,13 @@
   --  Limited_Withed_Unit.
 
   else
+ --  If the limited_with_clause is in some other unit in the context
+ --  then it is not visible in the main unit.
+
+ if not In_Extended_Main_Source_Unit (N) then
+Set_Is_Immediately_Visible (P, False);
+ end if;
+
  --  Real entities that are type or subtype declarations were hidden
  --  from visibility at the point of installation of the limited-view.
  --  Now we recover the previous value of the hidden attribute.

Re: [PATCH] More trivial bits from early LTO debug merge

[Thomas Schwinge]

Hi!

On Mon, 19 Sep 2016 09:53:13 +0200 (CEST), Richard Biener  
wrote:
>   * debug.h (gcc_debug_hooks): Add filename parameter to early_finish
>   hook.
>   [...]

With contrib/config-list.mk testing, I noticed another one that needed to
be updated; log/alpha-dec-vms-make.out, log/alpha64-dec-vms-make.out,
log/ia64-hp-vms-make.out:

[...]/source-gcc/gcc/vmsdbgout.c:209:1: error: invalid conversion from 
'void (*)()' to 'void (*)(const char*)' [-fpermissive]
 };
 ^
make[2]: *** [vmsdbgout.o] Error 1

As obvious, committed to trunk in r241042:

commit 706cf22169f5881765aa12affda63d80b372e47d
Author: tschwinge 
Date:   Wed Oct 12 13:08:53 2016 +

Fix gcc/vmsdbgout.c:vmsdbg_debug_hooks for "More trivial bits from early 
LTO debug merge"

gcc/
* vmsdbgout.c (vmsdbg_debug_hooks): Add filename parameter to
early_finish hook.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241042 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog   | 5 +
 gcc/vmsdbgout.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index 8044af0..e57438e 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,8 @@
+2016-10-12  Thomas Schwinge  
+
+   * vmsdbgout.c (vmsdbg_debug_hooks): Add filename parameter to
+   early_finish hook.
+
 2016-10-12  Georg-Johann Lay  
 
* rtl.h (struct rtx_def): Comment how RTX_FLAGS will be
diff --git gcc/vmsdbgout.c gcc/vmsdbgout.c
index 7c6d64d..23f7631 100644
--- gcc/vmsdbgout.c
+++ gcc/vmsdbgout.c
@@ -174,7 +174,7 @@ static void vmsdbgout_abstract_function (tree);
 const struct gcc_debug_hooks vmsdbg_debug_hooks
 = {vmsdbgout_init,
vmsdbgout_finish,
-   debug_nothing_void,
+   debug_nothing_charstar,
vmsdbgout_assembly_start,
vmsdbgout_define,
vmsdbgout_undef,


Grüße
 Thomas

Re: [BUILDROBOT] Selftest failed for i686-wrs-vxworks

[Thomas Schwinge]

Hi!

On Wed, 5 Oct 2016 16:36:01 +0200, Bernd Schmidt  wrote:
> On 10/05/2016 04:14 PM, David Malcolm wrote:
> > Thanks.  I'm not able to formally approve these changes, but FWIW these
> > patches look good to me (assuming usual testing).
> 
> LGTM too, so OK.

Without changes, committed to trunk in r241043 and r241044:

commit 441751466e69eaf8d8da7c0d388055509b35bc63
Author: tschwinge 
Date:   Wed Oct 12 13:09:06 2016 +

In gcc/Makefile.in, factor out SELFTEST_FLAGS

gcc/
* Makefile.in (SELFTEST_FLAGS): New variable.
(s-selftest, selftest-gdb, selftest-valgrind): Use it.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241043 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog   |  3 +++
 gcc/Makefile.in | 11 ---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index e57438e..003316f 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,5 +1,8 @@
 2016-10-12  Thomas Schwinge  
 
+   * Makefile.in (SELFTEST_FLAGS): New variable.
+   (s-selftest, selftest-gdb, selftest-valgrind): Use it.
+
* vmsdbgout.c (vmsdbg_debug_hooks): Add filename parameter to
early_finish hook.
 
diff --git gcc/Makefile.in gcc/Makefile.in
index 2914605..02d0c45 100644
--- gcc/Makefile.in
+++ gcc/Makefile.in
@@ -1877,6 +1877,10 @@ endif
 # This does the things that can't be done on the host machine.
 rest.cross: specs
 
+# GCC's selftests.
+# Specify a dummy input file to placate the driver.
+SELFTEST_FLAGS = -x c /dev/null -S -fself-test
+
 # Run the selftests during the build once we have a driver and a cc1,
 # so that self-test failures are caught as early as possible.
 # Use "s-selftest" to ensure that we only run the selftests if the
@@ -1884,18 +1888,19 @@ rest.cross: specs
 .PHONY: selftest
 selftest: s-selftest
 s-selftest: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-   $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test
+   $(GCC_FOR_TARGET) $(SELFTEST_FLAGS)
$(STAMP) $@
 
 # Convenience method for running selftests under gdb:
 .PHONY: selftest-gdb
 selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-   $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test -wrapper gdb,--args
+   $(GCC_FOR_TARGET) $(SELFTEST_FLAGS) \
+ -wrapper gdb,--args
 
 # Convenience method for running selftests under valgrind:
 .PHONY: selftest-valgrind
 selftest-valgrind: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-   $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test \
+   $(GCC_FOR_TARGET) $(SELFTEST_FLAGS) \
  -wrapper valgrind,--leak-check=full
 
 # Recompile all the language-independent object files.

commit c2d86129ef8141bb214958ca90be15fbeb42f4b1
Author: tschwinge 
Date:   Wed Oct 12 13:09:17 2016 +

Make GCC selftests work for *-wrs-vxworks-* targets

gcc/
* Makefile.in (SELFTEST_FLAGS): Add -nostdinc.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241044 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog   | 2 ++
 gcc/Makefile.in | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index 003316f..d6880d9 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,5 +1,7 @@
 2016-10-12  Thomas Schwinge  
 
+   * Makefile.in (SELFTEST_FLAGS): Add -nostdinc.
+
* Makefile.in (SELFTEST_FLAGS): New variable.
(s-selftest, selftest-gdb, selftest-valgrind): Use it.
 
diff --git gcc/Makefile.in gcc/Makefile.in
index 02d0c45..f1ff782 100644
--- gcc/Makefile.in
+++ gcc/Makefile.in
@@ -1879,7 +1879,9 @@ rest.cross: specs
 
 # GCC's selftests.
 # Specify a dummy input file to placate the driver.
-SELFTEST_FLAGS = -x c /dev/null -S -fself-test
+# Specify -nostdinc to work around missing WIND_BASE environment variable
+# required for *-wrs-vxworks-* targets.
+SELFTEST_FLAGS = -nostdinc -x c /dev/null -S -fself-test
 
 # Run the selftests during the build once we have a driver and a cc1,
 # so that self-test failures are caught as early as possible.


Grüße
 Thomas

Re: [PATCH] Check \0-termination of string in c_getstr

[Martin Liška]

On 10/11/2016 12:28 PM, Richard Biener wrote:
> On Tue, Oct 11, 2016 at 11:27 AM, Martin Liška  wrote:
>> As mentioned in the email that I reply to, c_getstr should check
>> null termination of string constants.
>>
>> Tests of the whole series have been running.
> 
> Looks ok to me (if testing passes).

Thanks for the review, however I decided to enhance the API to support
a requested length argument (if a string is not null terminated)
and to return length of the string(usable for strn* functions folding).

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests as
a whole series.

Martin

> 
> Thanks,
> Richard.
> 
>> Thanks,
>> Martin

>From e6c16ea038104ef15b087ff9fca23d9b2406e65e Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 10 Oct 2016 12:13:12 +0200
Subject: [PATCH] Enhance c_getstr API

gcc/ChangeLog:

2016-10-10  Martin Liska  

	* fold-const.c (c_getstr): Guard string termination, or validate
	that requested length is within a string constant.
	* fold-const.h (c_getstr): Set default value for the new arg.
---
 gcc/fold-const.c | 44 +++-
 gcc/fold-const.h |  3 ++-
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 02aa484..eb53e84 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -14440,24 +14440,50 @@ fold_build_pointer_plus_hwi_loc (location_t loc, tree ptr, HOST_WIDE_INT off)
 }
 
 /* Return a char pointer for a C string if it is a string constant
-   or sum of string constant and integer constant.  */
+   or sum of string constant and integer constant.
+   If the string constant is properly zero-terminated, the constant is returned.
+   Otherwise, if REQ_LENGTH is a non-negative number, the constant
+   is returned if the string length is greater or equal to REQ_LENGTH.
+   If STRLEN is a valid pointer, length (including terminatinch character)
+   of returned string is stored to the argument.  */
 
 const char *
-c_getstr (tree src)
+c_getstr (tree src, HOST_WIDE_INT req_length, unsigned HOST_WIDE_INT *strlen)
 {
   tree offset_node;
 
+  if (strlen)
+*strlen = 0;
+
   src = string_constant (src, &offset_node);
   if (src == 0)
-return 0;
+return NULL;
 
-  if (offset_node == 0)
-return TREE_STRING_POINTER (src);
-  else if (!tree_fits_uhwi_p (offset_node)
-	   || compare_tree_int (offset_node, TREE_STRING_LENGTH (src) - 1) > 0)
-return 0;
+  unsigned HOST_WIDE_INT offset = 0;
+  if (offset_node != NULL_TREE)
+{
+  if (!tree_fits_uhwi_p (offset_node))
+	return NULL;
+  else
+	offset = tree_to_uhwi (offset_node);
+}
 
-  return TREE_STRING_POINTER (src) + tree_to_uhwi (offset_node);
+  unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src);
+  const char *string = TREE_STRING_POINTER (src);
+  if (offset > string_length)
+return NULL;
+
+  /* If the string is properly '\0' character terminated, return it.  */
+  if ((string_length > 0 && string[string_length - 1] == 0)
+  || (req_length != -1
+	  && (unsigned HOST_WIDE_INT)req_length <= string_length - offset))
+{
+  if (strlen)
+	*strlen = string_length - offset;
+  return string + offset;
+}
+  else
+return NULL;
 }
 
 #if CHECKING_P
diff --git a/gcc/fold-const.h b/gcc/fold-const.h
index 637e46b..bbf831a 100644
--- a/gcc/fold-const.h
+++ b/gcc/fold-const.h
@@ -182,7 +182,8 @@ extern bool expr_not_equal_to (tree t, const wide_int &);
 extern tree const_unop (enum tree_code, tree, tree);
 extern tree const_binop (enum tree_code, tree, tree, tree);
 extern bool negate_mathfn_p (combined_fn);
-extern const char *c_getstr (tree);
+extern const char *c_getstr (tree src, HOST_WIDE_INT req_length = -1,
+			 unsigned HOST_WIDE_INT *strlen = NULL);
 
 /* Return OFF converted to a pointer offset type suitable as offset for
POINTER_PLUS_EXPR.  Use location LOC for this conversion.  */
-- 
2.9.2

Re: [PATCH] Fold __builtin_str{n}{case}cmp functions (version 3)

[Martin Liška]

On 10/12/2016 10:30 AM, Richard Biener wrote:
> On Tue, Oct 11, 2016 at 11:33 AM, Martin Liška  wrote:
>> Changes from the previous version:
>>
>> 1) Handle BUILT_IN_STRNCMP with length == -1.
>> 2) Direct gimple stmts creation and usage gsi_replace_with_seq_vops.
>> (hope using of replace_call_with_value is fine if replacing with a cst?)
> 
> yes
> 
>> 3) lhs == NULL cases are handled (Or is it fine to replace with a nop in 
>> general?
>> Can change a semantic as it may touch a memory.)
>> 4) CFN_BUILT_IN_STRNCMP can handle strncmp (x, y, 0).
>> 5) Handling of CFN_BUILT_IN_STRNCASECMP is added.
>>
>> Testing of the whole series has been running.
> 
> gimple_load_first_char needs a comment.
> 
> +  tree var = gimple_load_first_char (loc, str1, &stmts);
> +  if (lhs)
> +   stmt = gimple_build_assign (lhs, NOP_EXPR, var);
> +  else
> +   stmt = gimple_build_nop ();
> 
> I think you don't need the nop() as you have at least one stmt
> from the load.  Likewise for the other cases below.

Thanks, both nits are fixed in the new version.
Apart from that, I added usages of enhanced c_getstr API.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests
as a whole series.

Martin

> 
> Otherwise this patch looks ok now.
> 
> Richard.
> 
> 
> 
>> Martin

>From ec78ca3e97984acd8ef6330424798218568bd6c8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 5 Oct 2016 13:18:35 +0200
Subject: [PATCH 3/5] Fold __builtin_str{n}{case}cmp functions

gcc/ChangeLog:

2016-10-06  Martin Liska  

	* builtins.c (fold_builtin_strcmp): Remove function.
	(fold_builtin_strncmp): Likewise.
	(fold_builtin_2): Do not call fold_builtin_strcmp.
	(fold_builtin_3): Do not call fold_builtin_strncmp.
	* fold-const-call.c: Make build_cmp_result global fn.
	* fold-const-call.h: Likewise.
	* gimple-fold.c (gimple_fold_builtin_string_compare): New
	function.
	(gimple_fold_builtin): Call the function.
	* fold-const-call.c (fold_const_call): Handle
	CFN_BUILT_IN_STRCASECMP, CFN_BUILT_IN_STRNCMP and
	CFN_BUILT_IN_STRNCASECMP.
---
 gcc/builtins.c| 138 
 gcc/fold-const-call.c |  43 +---
 gcc/fold-const-call.h |   1 +
 gcc/gimple-fold.c | 189 +-
 4 files changed, 224 insertions(+), 147 deletions(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 6c68198..6696f28 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -150,8 +150,6 @@ static rtx expand_builtin_fabs (tree, rtx, rtx);
 static rtx expand_builtin_signbit (tree, rtx);
 static tree fold_builtin_memchr (location_t, tree, tree, tree, tree);
 static tree fold_builtin_memcmp (location_t, tree, tree, tree);
-static tree fold_builtin_strcmp (location_t, tree, tree);
-static tree fold_builtin_strncmp (location_t, tree, tree, tree);
 static tree fold_builtin_isascii (location_t, tree);
 static tree fold_builtin_toascii (location_t, tree);
 static tree fold_builtin_isdigit (location_t, tree);
@@ -7333,136 +7331,6 @@ fold_builtin_memcmp (location_t loc, tree arg1, tree arg2, tree len)
   return NULL_TREE;
 }
 
-/* Fold function call to builtin strcmp with arguments ARG1 and ARG2.
-   Return NULL_TREE if no simplification can be made.  */
-
-static tree
-fold_builtin_strcmp (location_t loc, tree arg1, tree arg2)
-{
-  if (!validate_arg (arg1, POINTER_TYPE)
-  || !validate_arg (arg2, POINTER_TYPE))
-return NULL_TREE;
-
-  /* If ARG1 and ARG2 are the same (and not volatile), return zero.  */
-  if (operand_equal_p (arg1, arg2, 0))
-return integer_zero_node;
-
-  /* If the second arg is "", return *(const unsigned char*)arg1.  */
-  const char *p2 = c_getstr (arg2);
-  if (p2 && *p2 == '\0')
-{
-  tree cst_uchar_node = build_type_variant (unsigned_char_type_node, 1, 0);
-  tree cst_uchar_ptr_node
-	= build_pointer_type_for_mode (cst_uchar_node, ptr_mode, true);
-
-  return fold_convert_loc (loc, integer_type_node,
-			   build1 (INDIRECT_REF, cst_uchar_node,
-   fold_convert_loc (loc,
-			 cst_uchar_ptr_node,
-			 arg1)));
-}
-
-  /* If the first arg is "", return -*(const unsigned char*)arg2.  */
-  const char *p1 = c_getstr (arg1);
-  if (p1 && *p1 == '\0')
-{
-  tree cst_uchar_node = build_type_variant (unsigned_char_type_node, 1, 0);
-  tree cst_uchar_ptr_node
-	= build_pointer_type_for_mode (cst_uchar_node, ptr_mode, true);
-
-  tree temp
-	= fold_convert_loc (loc, integer_type_node,
-			build1 (INDIRECT_REF, cst_uchar_node,
-fold_convert_loc (loc,
-		  cst_uchar_ptr_node,
-		  arg2)));
-  return fold_build1_loc (loc, NEGATE_EXPR, integer_type_node, temp);
-}
-
-  return NULL_TREE;
-}
-
-/* Fold function call to builtin strncmp with arguments ARG1, ARG2, and LEN.
-   Return NULL_TREE if no simplification can be made.  */
-
-static tree
-fold_builtin_strncmp (location_t loc, tree arg1, tree arg2, tree len)
-{
-  if (!validate_arg (arg1, POINTER_TYPE)
-  || !val

Re: [PATCH] Fold __builtin_memchr (version 3)

[Martin Liška]

On 10/12/2016 10:35 AM, Richard Biener wrote:
> On Tue, Oct 11, 2016 at 11:38 AM, Martin Liška  wrote:
>> One question that comes to my mind is whether there's a possibility
>> to fully test gimple folding of all cases if some of them are already
>> eaten by generic folding?
> 
> The only way is to make GENERIC folding not trigger by pushing
> constants to temporaries.
> 
> Richard.
> 

Good idea, I've done that in the patch with tests. I made a small revision to
patch, where I utilize the new c_getstr function arguments to handle more
cases.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests
as a whole series.

Martin
>From bc96a7b30764a098d47fa65bd1682005111febdf Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 6 Oct 2016 17:52:45 +0200
Subject: [PATCH 4/5] Fold __builtin_memchr function

gcc/ChangeLog:

2016-10-06  Martin Liska  

	* builtins.h(target_char_cst_p): Declare the function.
	* builtins.c (fold_builtin_memchr): Remove.
	(target_char_cst_p): Move the function from gimple-fold.c.
	(fold_builtin_3): Do not call the function.
	* gimple-fold.c (gimple_fold_builtin_memchr): New function.
	(gimple_fold_builtin): Call the function.
	* fold-const-call.c (fold_const_call_1): Handle CFN_BUILT_IN_MEMCHR.
---
 gcc/builtins.c| 59 +---
 gcc/builtins.h|  1 +
 gcc/fold-const-call.c | 41 +
 gcc/gimple-fold.c | 83 ++-
 4 files changed, 125 insertions(+), 59 deletions(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 6696f28..385e78e0 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -148,7 +148,6 @@ static tree rewrite_call_expr (location_t, tree, int, tree, int, ...);
 static bool validate_arg (const_tree, enum tree_code code);
 static rtx expand_builtin_fabs (tree, rtx, rtx);
 static rtx expand_builtin_signbit (tree, rtx);
-static tree fold_builtin_memchr (location_t, tree, tree, tree, tree);
 static tree fold_builtin_memcmp (location_t, tree, tree, tree);
 static tree fold_builtin_isascii (location_t, tree);
 static tree fold_builtin_toascii (location_t, tree);
@@ -7244,47 +7243,6 @@ fold_builtin_sincos (location_t loc,
 			 fold_build1_loc (loc, REALPART_EXPR, type, call)));
 }
 
-/* Fold function call to builtin memchr.  ARG1, ARG2 and LEN are the
-   arguments to the call, and TYPE is its return type.
-   Return NULL_TREE if no simplification can be made.  */
-
-static tree
-fold_builtin_memchr (location_t loc, tree arg1, tree arg2, tree len, tree type)
-{
-  if (!validate_arg (arg1, POINTER_TYPE)
-  || !validate_arg (arg2, INTEGER_TYPE)
-  || !validate_arg (len, INTEGER_TYPE))
-return NULL_TREE;
-  else
-{
-  const char *p1;
-
-  if (TREE_CODE (arg2) != INTEGER_CST
-	  || !tree_fits_uhwi_p (len))
-	return NULL_TREE;
-
-  p1 = c_getstr (arg1);
-  if (p1 && compare_tree_int (len, strlen (p1) + 1) <= 0)
-	{
-	  char c;
-	  const char *r;
-	  tree tem;
-
-	  if (target_char_cast (arg2, &c))
-	return NULL_TREE;
-
-	  r = (const char *) memchr (p1, c, tree_to_uhwi (len));
-
-	  if (r == NULL)
-	return build_int_cst (TREE_TYPE (arg1), 0);
-
-	  tem = fold_build_pointer_plus_hwi_loc (loc, arg1, r - p1);
-	  return fold_convert_loc (loc, type, tem);
-	}
-  return NULL_TREE;
-}
-}
-
 /* Fold function call to builtin memcmp with arguments ARG1 and ARG2.
Return NULL_TREE if no simplification can be made.  */
 
@@ -8340,9 +8298,6 @@ fold_builtin_3 (location_t loc, tree fndecl,
 	return do_mpfr_remquo (arg0, arg1, arg2);
 break;
 
-case BUILT_IN_MEMCHR:
-  return fold_builtin_memchr (loc, arg0, arg1, arg2, type);
-
 case BUILT_IN_BCMP:
 case BUILT_IN_MEMCMP:
   return fold_builtin_memcmp (loc, arg0, arg1, arg2);;
@@ -9908,3 +9863,17 @@ is_inexpensive_builtin (tree decl)
 
   return false;
 }
+
+/* Return true if T is a constant and the value cast to a target char
+   can be represented by a host char.
+   Store the casted char constant in *P if so.  */
+
+bool
+target_char_cst_p (tree t, char *p)
+{
+  if (!tree_fits_uhwi_p (t) || CHAR_TYPE_SIZE != HOST_BITS_PER_CHAR)
+return false;
+
+  *p = (char)tree_to_uhwi (t);
+  return true;
+}
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 8d0acd0..5e83646 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -97,6 +97,7 @@ extern unsigned HOST_WIDE_INT target_percent;
 extern char target_percent_s[3];
 extern char target_percent_c[3];
 extern char target_percent_s_newline[4];
+extern bool target_char_cst_p (tree t, char *p);
 
 extern internal_fn associated_internal_fn (tree);
 extern internal_fn replacement_internal_fn (gcall *);
diff --git a/gcc/fold-const-call.c b/gcc/fold-const-call.c
index 993769c..d84af48 100644
--- a/gcc/fold-const-call.c
+++ b/gcc/fold-const-call.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "fold-const-call.h"
 #include "case-cfn-macros.h"
 #include "tm.h" /* For C[LT]Z_DEFINED_AT_Z

Re: [PATCH] Test folding of str{n}{case}cmp and memchr (version 3)

[Martin Liška]

On 10/12/2016 10:34 AM, Richard Biener wrote:
> On Tue, Oct 11, 2016 at 11:38 AM, Martin Liška  wrote:
>> Third iteration of tests, where I added both GENERIC and GIMPLE folding
>> tests.
> 
> They should work already with -O1?

Yes, they work. Sending new version where I also added few situations
that trigger an undefined behavior (and should not be folded).

Martin

> 
> Otherwise ok.
> 
> Richard.
> 
>> Martin

>From bae7f32a3f602d111ac67019c6cd6dd8efacae00 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 16 Aug 2016 15:56:01 +0200
Subject: [PATCH 5/5] Test folding of str{n}{case}cmp and memchr

gcc/testsuite/ChangeLog:

2016-08-16  Martin Liska  

	* gcc.dg/tree-ssa/builtins-folding-generic.c: New test.
	* gcc.dg/tree-ssa/builtins-folding-gimple.c: Likewise.
	* gcc.dg/tree-ssa/builtins-folding-gimple-ub.c: Likewise.
---
 .../gcc.dg/tree-ssa/builtins-folding-generic.c |  76 +++
 .../gcc.dg/tree-ssa/builtins-folding-gimple-ub.c   |  23 
 .../gcc.dg/tree-ssa/builtins-folding-gimple.c  | 140 +
 3 files changed, 239 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-generic.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-generic.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-generic.c
new file mode 100644
index 000..175feff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-generic.c
@@ -0,0 +1,76 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -fdump-tree-original" } */
+
+char *buffer1;
+char *buffer2;
+
+#define SIZE 1000
+
+int
+main (void)
+{
+  const char* const foo1 = "hello world";
+
+  buffer1 = __builtin_malloc (SIZE);
+  __builtin_strcpy (buffer1, foo1);
+  buffer2 = __builtin_malloc (SIZE);
+  __builtin_strcpy (buffer2, foo1);
+
+  /* MEMCHR.  */
+  if (__builtin_memchr ("hello world", 'x', 11))
+__builtin_abort ();
+  if (__builtin_memchr ("hello world", 'x', 0) != 0)
+__builtin_abort ();
+  if (__builtin_memchr ("hello world", 'w', 2))
+__builtin_abort ();
+  if (__builtin_memchr ("hello world", 'd', 10))
+__builtin_abort ();
+  if (__builtin_memchr ("hello world", '\0', 11))
+__builtin_abort ();
+
+  /* STRCMP.  */
+  if (__builtin_strcmp ("hello", "a") <= 0)
+__builtin_abort ();
+  if (__builtin_strcmp ("a", "a") != 0)
+__builtin_abort ();
+  if (__builtin_strcmp ("a", "") <= 0)
+__builtin_abort ();
+  if (__builtin_strcmp ("", "a") >= 0)
+__builtin_abort ();
+  if (__builtin_strcmp ("ab", "ba") >= 0)
+__builtin_abort ();
+
+  /* STRNCMP.  */
+  if (__builtin_strncmp ("hello", "a", 0) != 0)
+__builtin_abort ();
+  if (__builtin_strncmp ("a", "a", 100) != 0)
+__builtin_abort ();
+  if (__builtin_strncmp ("a", "", 100) <= 0)
+__builtin_abort ();
+  if (__builtin_strncmp ("", "a", 100) >= 0)
+__builtin_abort ();
+  if (__builtin_strncmp ("ab", "ba", 1) >= 0)
+__builtin_abort ();
+  if (__builtin_strncmp ("aab", "aac", 2) != 0)
+__builtin_abort ();
+
+  /* STRCASECMP.  */
+  if (__builtin_strcasecmp ("a", "a") != 0)
+__builtin_abort ();
+
+  /* STRNCASECMP.  */
+  if (__builtin_strncasecmp ("hello", "a", 0) != 0)
+__builtin_abort ();
+  if (__builtin_strncasecmp ("a", "a", 100) != 0)
+__builtin_abort ();
+  if (__builtin_strncasecmp ("aab", "aac", 2) != 0)
+__builtin_abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "__builtin_strcmp" "original" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_strcasecmp" "original" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_strncmp" "original" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_strncasecmp" "original" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_memchr" "original" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
new file mode 100644
index 000..df0ede2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple-ub.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+char *buffer1;
+char *buffer2;
+
+#define SIZE 1000
+
+int
+main (void)
+{
+  const char* const foo1 = "hello world";
+
+  /* MEMCHR.  */
+  if (__builtin_memchr ("", 'x', 1000)) /* Not folded away.  */
+__builtin_abort ();
+  if (__builtin_memchr (foo1, 'x', 1000)) /* Not folded away.  */
+__builtin_abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin_memchr" 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
new file mode 100644
index 000..4cac93a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtins-folding-gimple.c
@@ -0,0 +1,140 @@
+/* { dg-do run } */
+

Re: Un-break dwarf2out for DWARF2_LINENO_DEBUGGING_INFO configurations (was: [PATCH] Refactor section/label init for early LTO debug)

[Thomas Schwinge]

Hi!

On Wed, 28 Sep 2016 11:28:16 +0200 (CEST), Richard Biener  
wrote:
> On Wed, 28 Sep 2016, Thomas Schwinge wrote:
> > ([...] got committed in r240545.)  For DWARF2_LINENO_DEBUGGING_INFO
> > configurations (that is, nvptx; Bernd CCed, who originally authored the
> > DWARF2_LINENO_DEBUGGING_INFO support), this breaks things because of
> > uninitialized text_section_line_info/cur_line_info_table.  OK to fix as
> > follows?
> 
> Ok.

Without changes committed to trunk in r241045:

commit 84cb1b769377413a1b7cef73fc4f6022e046b0a9
Author: tschwinge 
Date:   Wed Oct 12 13:09:26 2016 +

Un-break dwarf2out for DWARF2_LINENO_DEBUGGING_INFO configurations

gcc/
* dwarf2out.c (dwarf2_lineno_debug_hooks): Use
dwarf2out_assembly_start.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241045 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog   | 3 +++
 gcc/dwarf2out.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index d6880d9..c66b7a7 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,5 +1,8 @@
 2016-10-12  Thomas Schwinge  
 
+   * dwarf2out.c (dwarf2_lineno_debug_hooks): Use
+   dwarf2out_assembly_start.
+
* Makefile.in (SELFTEST_FLAGS): Add -nostdinc.
 
* Makefile.in (SELFTEST_FLAGS): New variable.
diff --git gcc/dwarf2out.c gcc/dwarf2out.c
index b5787ef..98deeef 100644
--- gcc/dwarf2out.c
+++ gcc/dwarf2out.c
@@ -2540,7 +2540,7 @@ const struct gcc_debug_hooks dwarf2_lineno_debug_hooks =
   dwarf2out_init,
   debug_nothing_charstar,
   debug_nothing_charstar,
-  debug_nothing_void,
+  dwarf2out_assembly_start,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,


Grüße
 Thomas

Re: [PATCH] Fix PR77826

[Richard Biener]

On Wed, 12 Oct 2016, Marc Glisse wrote:

> On Wed, 12 Oct 2016, Richard Biener wrote:
> 
> > So with doing the same on GENERIC I hit
> > 
> > FAIL: g++.dg/cpp1y/constexpr-array4.C  -std=c++14 (test for excess errors)
> > 
> > with the pattern
> > 
> >  /* (T)(P + A) - (T)P -> (T) A */
> >  (for add (plus pointer_plus)
> >   (simplify
> >(minus (convert (add @0 @1))
> > (convert @0))
> 
> Ah, grep missed that one because it is on 2 lines :-(
> 
> >...
> > (convert @1
> > 
> > 
> > no longer applying to
> > 
> > (long int) ((int *) &ar + 12) - (long int) &ar
> > 
> > because while (int *) &ar is equal to (long int) &ar (operand_equal_p
> > does STRIP_NOPS) they obviously do not have the same type.
> 
> I believe we are comparing (int *) &ar to &ar, not to (long int) &ar. But yes,
> indeed.
> 
> > So on GENERIC we have to consider that we feed operand_equal_p with
> > non-ATOMs (arbitrary expressions).  The pattern above is "safe" as it
> > doesn't reference @0 in the result (not sure if we should take advantage of
> > that in genmatch).
> 
> Since we are in the process of defining an
> operand_equal_for_(generic|gimple)_match_p, we could tweak it to check the
> type only for INTEGER_CST, or to still STRIP_NOPS, or similar.
> 
> Or we could remain very strict and refine the pattern, allowing a convert on
> the pointer (we might want to split the plus and pointer_plus versions then,
> for clarity). There are not many optimizations that are mandated by front-ends
> and for which this is an issue.
> 
> > The other FAILs with doing the same on GENERIC are
> > 
> > FAIL: g++.dg/gomp/declare-simd-3.C  -std=gnu++11 (test for excess errors)
> > FAIL: g++.dg/torture/pr71448.C   -O0  (test for excess errors)
> > FAIL: g++.dg/vect/simd-clone-6.cc  -std=c++11 (test for excess errors)
> > 
> > the simd ones are 'warning: ignoring large linear step' and the pr71448.C
> > case is very similar to the above.
> 
> Yes, I expect they all come from the same 1 or 2 transformations.
> 
> > > > > If we stick to the old behavior, maybe we could have some genmatch
> > > > > magic to help with the constant capture weirdness. With matching
> > > > > captures, we could select which operand (among those supposed to be
> > > > > equivalent) is actually captured more cleverly, either with an
> > > > > explicit marker, or by giving priority to the one that is not
> > > > > immediatly below convert? in the pattern.
> > > > 
> > > > This route is a difficult one to take
> > > 
> > > The simplest version I was thinking about was @0 for a true capture, and
> > > @@0
> > > for something that just has to be checked for equality with @0.
> > 
> > Hmm, ok.  So you'd have @@0 having to match @0 and we'd get the @0 for
> > the result in a reliable way?  Sounds like a reasonable idea, I'll see
> > how that works out (we could auto-detect '@@' if the capture is not
> > used in the result, see above).
> 
> It probably doesn't bring much compared to the @0@4 syntax you were 
> suggesting below, so if that is easier...

Will figure that out ...

> > > > -- what would be possible is to
> > > > capture a specific operand.  Like allow one to write
> > > > 
> > > > (op (op @0@4 @1) (op @0@3 @2))
> > > > 
> > > > and thus actually have three "names" for @0.  We have this internally
> > > > already when you write
> > > > 
> > > > (convert?@0 @1)
> > > > 
> > > > for the case where there is no conversion.  @0 and @1 are the same
> > > > in this case.
> > > 
> > > Looks nice and convenient (assuming lax constant matching).
> > 
> > Yes, w/o lax matching it has of course little value.
> > 
> > > > Not sure if this helps enough cases.
> > > 
> > > IIRC, in all cases where we had trouble with operand_equal_p, chosing
> > > which
> > > operand to capture would have solved the issue.
> > 
> > Yes.  We'd still need to actually catch all those cases...
> 
> Being forced to chose which operand we capture (say with @ vs @@, making 2
> occurences of @0 a syntax error) might help, but it would also require
> updating many patterns for which this was never an issue (easy but boring).

We can even have today

 (plus (minus @0 @1) (plus @0 @2) @0)

thus three matching @0.  Now if we have @@ telling to use value equality
rather than "node equality" _and_ making the @@ select the canonical
operand to choose then we'd have at most one @@ per ID.  Somewhat tricky
but not impossible to implement I think.

> > > > I still think that having two things matched that are not really
> > > > the same is werid (and a possible source of errors as in, ICEs,
> > > > rather than missed optimizations).
> > > 
> > > Ok. Let's go with the strict matching, it is indeed less confusing.
> > 
> > I'll hold off with the GENERIC strict matching and will investigate
> > the @@ idea (plus auto-detecting it).
> > 
> > > > > And if we move to stricter matching, maybe genmatch could be updated
> > > > > so
> > > > > convert could also match integer constants in some cases.
> > > > 
> > >

[PATCH] Make test helper work with non-copyable types

[Jonathan Wakely]

This test utility does: "T t = T();" to guarantee value-init, and then
some std::atomic tests have dg-error directives to handle the errors
that happen because atomics are non-copyable. In C++17 there is no
error, because of guaranteed elision, so the dg-errors FAIL.

For C++11 we can us "T t{};" instead, which means there are no errors
for non-copyable types, and we don't get different errors for C++11
and C++17.

* testsuite/util/testsuite_common_types.h
(bitwise_assignment_operators): Use direct-initialization for C++11
and later, to avoid CopyConstructible requirement.
* testsuite/29_atomics/atomic/cons/assign_neg.cc: Adjust dg-error
line numbers.
* testsuite/29_atomics/atomic/cons/copy_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/cons/assign_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/cons/copy_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc:
Adjust expected errors and line numbers.

Tested powerpc64le-linux, committed to trunk.

commit 588104840e79d4fdad1eba26ac83a511d9b6ab27
Author: Jonathan Wakely 
Date:   Wed Oct 12 13:17:05 2016 +0100

Make test helper work with non-copyable types

* testsuite/util/testsuite_common_types.h
(bitwise_assignment_operators): Use direct-initialization for C++11
and later, to avoid CopyConstructible requirement.
* testsuite/29_atomics/atomic/cons/assign_neg.cc: Adjust dg-error
line numbers.
* testsuite/29_atomics/atomic/cons/copy_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/cons/assign_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/cons/copy_neg.cc: Likewise.
* testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc:
Adjust expected errors and line numbers.

diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/cons/assign_neg.cc 
b/libstdc++-v3/testsuite/29_atomics/atomic/cons/assign_neg.cc
index eb0111d..4136944 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/cons/assign_neg.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/cons/assign_neg.cc
@@ -27,5 +27,5 @@ int main()
   return 0;
 }
 
-// { dg-error "deleted" "" { target *-*-* } 615 }
+// { dg-error "deleted" "" { target *-*-* } 620 }
 // { dg-prune-output "include" }
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_neg.cc 
b/libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_neg.cc
index 546ac50..ffc2dc2 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_neg.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/cons/copy_neg.cc
@@ -27,5 +27,5 @@ int main()
   return 0;
 }
 
-// { dg-error "deleted" "" { target *-*-* } 654 }
+// { dg-error "deleted" "" { target *-*-* } 659 }
 // { dg-prune-output "include" }
diff --git 
a/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/assign_neg.cc 
b/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/assign_neg.cc
index c5b6103..20f263c 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/assign_neg.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/assign_neg.cc
@@ -28,5 +28,5 @@ int main()
   return 0;
 }
 
-// { dg-error "deleted" "" { target *-*-* } 615 }
+// { dg-error "deleted" "" { target *-*-* } 620 }
 // { dg-prune-output "include" }
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_neg.cc 
b/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_neg.cc
index 49e1eb1..bf7cda8 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_neg.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_integral/cons/copy_neg.cc
@@ -28,5 +28,5 @@ int main()
   return 0;
 }
 
-// { dg-error "deleted" "" { target *-*-* } 654 }
+// { dg-error "deleted" "" { target *-*-* } 659 }
 // { dg-prune-output "include" }
diff --git 
a/libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc 
b/libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc
index db0c3c1..731513b 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_integral/operators/bitwise_neg.cc
@@ -26,10 +26,8 @@ int main()
   return 0;
 }
 
-// { dg-error "deleted" "" { target *-*-* } 469 }
-// { dg-error "deleted" "" { target *-*-* } 470 }
-// { dg-error "operator" "" { target *-*-* } 471 }
-// { dg-error "operator" "" { target *-*-* } 472 }
-// { dg-error "operator" "" { target *-*-* } 473 }
+// { dg-error "operator" "" { target *-*-* } 476 }
+// { dg-error "operator" "" { target *-*-* } 477 }
+// { dg-error "operator" "" { target *-*-* } 478 }
 
 // { dg-prune-output "declared here" }
diff --git a/libstdc++-v3/testsuite/util/testsuite_common_types.h 
b/libstdc++-v3/testsuite/util/testsuite_common_types.h
index 2f3732e..3233676 100644
--- a/libstdc++-v3/testsuite/util/testsuite_common_types.h
+++ b/libstdc++-v3/testsuite/util/testsuite_common_types.h
@@ -46

Re: Compile-time improvement for if conversion.

[Yuri Rumyantsev]

Richard,

Here is updated patch . I avoided creation of new entry/exit blocks
but instead add check to border cases - do not consider also blocks
which are out of region.

Any comments will be appreciated.

2016-10-11 16:48 GMT+03:00 Richard Biener :
> On Tue, Oct 11, 2016 at 3:23 PM, Yuri Rumyantsev  wrote:
>> Richard,
>>
>> I implemented this by passing callback function in_region which
>> returns true if block belongs to region.
>> I am testing it now
>>
>> I attach modified patch for your quick review.
>
> +  FOR_EACH_VEC_ELT (region, i, bb)
> +{
> +  bb->dom[dir_index] = et_new_tree (bb);
> +}
> +  /* Check that region is SESE region.  */
> +  entry = region[0];
> +  if ( EDGE_COUNT (entry->succs) > 1)
> +{
> +  /* Create new entry block with the only successor.  */
> +  basic_block succ = NULL;
> +  bool found = false;
> +  FOR_EACH_EDGE (e, ei, entry->succs)
> +   if (in_region (region, e->dest))
> + {
> +   gcc_assert (!found);
>
> is that check equal to e->dest->dom[dir_index] != NULL?  I think we
> shouldn't need the callback.
>
> +new_entry = create_empty_bb (entry);
> +unchecked_make_edge (new_entry, succ, 0);
>
> I still think this is somewhat gross and we should try to avoid it
> somehow - at least it's well-hidden now ;)
>
> +/* Put it to region as entry node.  */
> +region[0] = new_entry;
>
> so region[0] is overwritten now?
>
> As far as I understand calc_dfs_tree it should be possible to compute
> the region on-the-fly
> from calling calc_dfs_tree_nonrec on the entry/exit (also maybe
> avoiding some of the still
> "large" complexity of zeroing arrays in the constructor).
>
> And if we use that DFS walk directly we should be able to avoid
> creating those fake entry/exit
> blocks by using entry/exit edges instead... (somehow).
>
> Richard.
>
>
>
>> Thanks.
>>
>> 2016-10-11 13:33 GMT+03:00 Richard Biener :
>>> On Mon, Oct 10, 2016 at 4:17 PM, Yuri Rumyantsev  wrote:
 Richard,

 If "fake" exit or entry block is created in dominance how we can
 determine what is its the only  predecessor or successor without using
 a notion of loop?
>>>
>>> The caller passes in an entry and exit edge instead of a block or loop.
>>>
>>> Richard.
>>>
 2016-10-10 15:00 GMT+03:00 Richard Biener :
> On Mon, Oct 10, 2016 at 1:42 PM, Yuri Rumyantsev  
> wrote:
>> Thanks Richard for your comments.
>> I'd like to answer on your last comment regarding use split_edge()
>> instead of creating fake post-header. I started with this splitting
>> but it requires to fix-up closed ssa form by creating additional phi
>> nodes, so I decided to use only cfg change without updating ssa form.
>> Other changes look reasonable and will fix them.
>
> Ah.  In this case can you investigate what it takes to make the entry/exit
> edges rather than BBs?  That is, introduce those "fakes" only internally
> in dominance.c?
>
>> 2016-10-10 12:52 GMT+03:00 Richard Biener :
>>> On Wed, Oct 5, 2016 at 3:22 PM, Yuri Rumyantsev  
>>> wrote:
 Hi All,

 Here is implementation of Richard proposal:

 < For general infrastructure it would be nice to expose a 
 (post-)dominator
 < compute for MESE (post-dominators) / SEME (dominators) regions.  I 
 believe
 < what makes if-conversion expensive is the post-dom compute which 
 happens
 < for each loop for the whole function.  It shouldn't be very difficult
 < to write this,
 < sharing as much as possible code with the current DOM code might need
 < quite some refactoring though.

 I implemented this proposal by adding calculation of dominance info
 for SESE regions and incorporate this change to if conversion pass.
 SESE region is built by adding loop pre-header and possibly fake
 post-header blocks to loop body. Fake post-header is deleted after
 predication completion.

 Bootstrapping and regression testing did not show any new failures.

 Is it OK for trunk?
>>>
>>> It's mostly reasonable but I have a few comments.  First, re-using
>>> bb->dom[] for the dominator info is somewhat fragile but indeed
>>> a requirement to make the patch reasonably small.  Please,
>>> in calculate_dominance_info_for_region, make sure that
>>> !dom_info_available_p (dir).
>>>
>>> You pass loop * everywhere but require ->aux to be set up as
>>> an array of BBs forming the region with special BBs at array ends.
>>>
>>> Please instead pass in a vec which avoids using ->aux
>>> and also allows other non-loop-based SESE regions to be used
>>> (I couldn't spot anything that relies on this being a loop).
>>>
>>> Adding a convenience wrapper for loop  * would be of course nice,
>>> to cover the special pre/post-header code in tr

Re: Fix LTO_STREAMER_DEBUG build

[Thomas Schwinge]

Hi!

On Thu, 29 Sep 2016 17:29:29 +0200, Richard Biener  
wrote:
> On September 29, 2016 4:54:36 PM GMT+02:00, Thomas Schwinge 
>  wrote:
> >While working on something else, I found LTO_STREAMER_DEBUG broken.
> >Enabling (#define) gcc/lto-stream.h:LTO_STREAMER_DEBUG, some further
> >checking is done on the LTO streamer (writer/reader), and some
> >additional
> >debugging hooks are provided.  In the end, I didn't use this facility
> >for
> >my debugging, but here is anyway the patch to un-break it.  Not yet
> >thoroughly tested, will do that later -- OK for trunk then?
> 
> Works for me though I'd expect the checking code to be broken (I foubd it 
> hardly useful)

;-) Well, I just filed  "LTO_STREAMER_DEBUG
ICE with OpenMP SIMD clones".

Without changes committed in r241046:

commit 2e5bb449b8f67130b235ea4e3643929041bb495e
Author: tschwinge 
Date:   Wed Oct 12 13:18:36 2016 +

Fix LTO_STREAMER_DEBUG build

gcc/
* lto-streamer.c: Fix LTO_STREAMER_DEBUG build.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@241046 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog  |  2 ++
 gcc/lto-streamer.c | 12 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index c66b7a7..e709adb 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,5 +1,7 @@
 2016-10-12  Thomas Schwinge  
 
+   * lto-streamer.c: Fix LTO_STREAMER_DEBUG build.
+
* dwarf2out.c (dwarf2_lineno_debug_hooks): Use
dwarf2out_assembly_start.
 
diff --git gcc/lto-streamer.c gcc/lto-streamer.c
index bfde1fe..a44a916 100644
--- gcc/lto-streamer.c
+++ gcc/lto-streamer.c
@@ -267,23 +267,23 @@ struct tree_hash_entry
 
 struct tree_entry_hasher : nofree_ptr_hash 
 {
-  static inline hashval_t hash (const value_type *);
-  static inline bool equal (const value_type *, const compare_type *);
+  static inline hashval_t hash (const tree_hash_entry *);
+  static inline bool equal (const tree_hash_entry *, const tree_hash_entry *);
 };
 
 inline hashval_t
-tree_entry_hasher::hash (const value_type *e)
+tree_entry_hasher::hash (const tree_hash_entry *e)
 {
   return htab_hash_pointer (e->key);
 }
 
 inline bool
-tree_entry_hasher::equal (const value_type *e1, const compare_type *e2)
+tree_entry_hasher::equal (const tree_hash_entry *e1, const tree_hash_entry *e2)
 {
   return (e1->key == e2->key);
 }
 
-static hash_table *tree_htab;
+static hash_table *tree_htab;
 #endif
 
 /* Initialization common to the LTO reader and writer.  */
@@ -299,7 +299,7 @@ lto_streamer_init (void)
 streamer_check_handled_ts_structures ();
 
 #ifdef LTO_STREAMER_DEBUG
-  tree_htab = new hash_table (31);
+  tree_htab = new hash_table (31);
 #endif
 }
 


Grüße
 Thomas

Re: Questionable code in gcov-io.c

[Marek Polacek]

On Wed, Oct 12, 2016 at 08:14:58AM -0400, Nathan Sidwell wrote:
> On 10/12/16 08:10, Marek Polacek wrote:
> > While implementing a warning I noticed this in gcov-io.c:
> > 
> >  187   else if (mode == 0)
> >  188 {
> >  189   struct stat st;
> >  190
> >  191   if (fstat (fd, &st) < 0)
> >  192 {
> >  193   fclose (gcov_var.file);
> >  194   gcov_var.file = 0;
> >  195   return 0;
> >  196 }
> >  197   if (st.st_size != 0)
> >  198 gcov_var.mode = 1;
> >  199   else
> >  200 gcov_var.mode = mode * 2 + 1;
> >  201 }
> >  202   else
> >  203 gcov_var.mode = mode * 2 + 1;
> > 
> > It seems that lines 198 and 200 do the same thing, at line 200 we know that
> > mode == 0, so we just assign 1.  Should we just remove the condition on 
> > line 197?
> 
> ITYM lines 197 -> 203.  I.e. remove the entire if that;s inside the 'mode ==
> 0' branch and make line 203 unconditional.

Yes, sorry for sloppy wording.  I'm testing a patch.

Thanks,

Marek

Re: PING [PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])

[Jason Merrill]

On Tue, Oct 11, 2016 at 9:45 PM, Martin Sebor  wrote:
> Are there any other changes you want me to make to the patch?
> I leave this weekend for the WG14 meeting and would like to
> get this change finalized and hopefully committed before then.
>
>   https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00410.html

OK, thanks.  Sorry I overlooked your earlier mail.

Jason

Re: Questionable code in gcov-io.c

[Nathan Sidwell]

On 10/12/16 09:43, Marek Polacek wrote:


ITYM lines 197 -> 203.  I.e. remove the entire if that;s inside the 'mode ==
0' branch and make line 203 unconditional.


Yes, sorry for sloppy wording.  I'm testing a patch.


heh, so am I :)

nathan

[RFC] Possible folding opportunities for string built-ins

[Martin Liška]

Hi.

As you probably mentioned, simple folding improvement has grown to multiple 
patches
and multiple iterations. Apart from that, I also noticed that we do not do the 
best
for couple of cases and I would like to have a feedback if it worth to improve 
or not?

$ cat /tmp/string-folding-missing.c 
const char global_1[4] = {'a', 'b', 'c', 'd' };
const char global_2[6] = "abcdefghijk";

int main()
{
  const char local1[] = "asdfasdfasdf";

  /* Case 1 */
  __builtin_memchr (global_1, 'c', 5);

  /* Case 2 */
  __builtin_memchr (global_2, 'c', 5);

  /* Case 3 */
  __builtin_memchr (local1, 'a', 5);

  return 0;
}

Cases:
1) Currently, calling c_getstr (which calls string_constant) can't handle 
CONSTRUCTOR. Potential
solution can be to create on demand STRING_CST, however as string_constant is 
called multiple times,
it can be overkill.
2) /tmp/x.c:2:26: warning: initializer-string for array of chars is too long
 const char global_2[6] = "abcdefghijk";
Here I'm not sure whether one can consider global_2 == "abcdef" (w/o trailing 
zero char) or not?
If so, adding new output argument (string_length) to string_constant can be 
solution.
3) Currently, ctor_for_folding return error_mark_node for local variables. I'm 
wondering whether returning
DECL_INITIAL for these would be doable? Will it make any issue for LTO?

Last question is whether one can aggressively fold strcasecmp in a host 
compiler? Or are there any situations
where results depends on locale?

Thanks for thoughts.
Martin

Re: [PATCH] Implement new hook for max_align_t_align

[Jason Merrill]

On Wed, Oct 12, 2016 at 4:02 AM, Jakub Jelinek  wrote:
> On Wed, Oct 12, 2016 at 09:52:04AM +0200, Florian Weimer wrote:
>> dropping the alignment means that the padding before the lock member
>> vanishes.  Consequently, we have just created a silent ABI change in
>> application code, which is a big no-no.
>
> Sure, it would be an ABI change, but how many users would it affect?
>
>> Since this is PA-RISC, which is essentially dead (neither HPE nor Debian
>> ship it anymore), I stand by my suggestion to bump the fundamental alignment
>
> Or just drop support for a dead arch?
>
>> instead.  Sure, it is a bit inefficient, but this will only affect PA-RISC
>> users.  It does not even cause work for PA-RISC porters. Conversely, if we
>> work on this to come up with a different fix, many more people will be
>> affected (because they don't get all the nice things we could work on
>> instead), and we may need to maintain a special GCC kludge for the
>> alternative solution, impacting GCC developers in particular.
>
> But sure, bumping malloc alignment is probably easiest.  And people who want
> performance have better options than to stay on 32-bit PA-RISC anyway.

Or we could do nothing and tell people to ignore the harmless warning.

Jason

[Ada] Allow Refined_Global/Depends contracts with partial refinement

[Arnaud Charlet]

Pragmas/aspects Refined_Global and Refined_Depends may be applied in
a scope where only partial refinement of abstract states is visible,
as defined in SPARK RM 7.2.4. In such a case, the partial refinement
should be used instead of the full refinement for checking the refined
contracts.

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Yannick Moy  

* einfo.adb, einfo.ads (Has_Partial_Visible_Refinement): New flag
in abtract states.
(Has_Non_Null_Visible_Refinement): Return true for patial refinement.
(Partial_Refinement_Constituents): New function returns the full or
partial refinement constituents depending on scope.
* sem_ch3.adb (Analyze_Declarations): Remove partial visible
refinements when exiting the scope of a package spec or body
and those partial refinements are not in scope afterwards.
* sem_ch7.adb, sem_ch7.ads (Install_Partial_Declarations): Mark
abstract states of parent units with partial refinement so that
it is visible.
* sem_prag.adb (Analyze_Part_Of_In_Decl_Part): Mark enclosing
abstract state if any as having partial refinement in that scope.
(Analyze_Refined_Global_In_Decl_Part): Check constituent usage
based on full or partial refinement depending on scope.

Index: sem_ch3.adb
===
--- sem_ch3.adb (revision 241041)
+++ sem_ch3.adb (working copy)
@@ -2178,10 +2178,17 @@
   --  case, add a proper spec if the body lacks one. The spec is inserted
   --  before Body_Decl and immediately analyzed.
 
+  procedure Remove_Partial_Visible_Refinements (Spec_Id : Entity_Id);
+  --  Spec_Id is the entity of a package that may define abstract states,
+  --  and in the case of a child unit, whose ancestors may define abstract
+  --  states. If the states have partial visible refinement, remove the
+  --  partial visibility of each constituent at the end of the package
+  --  spec and body declarations.
+
   procedure Remove_Visible_Refinements (Spec_Id : Entity_Id);
   --  Spec_Id is the entity of a package that may define abstract states.
   --  If the states have visible refinement, remove the visibility of each
-  --  constituent at the end of the package body declarations.
+  --  constituent at the end of the package body declaration.
 
   -
   -- Adjust_Decl --
@@ -2335,6 +2342,29 @@
  Insert_Before_And_Analyze (Body_Decl, Decl);
   end Handle_Late_Controlled_Primitive;
 
+  
+  -- Remove_Partial_Visible_Refinements --
+  
+
+  procedure Remove_Partial_Visible_Refinements (Spec_Id : Entity_Id) is
+ State_Elmt : Elmt_Id;
+  begin
+ if Present (Abstract_States (Spec_Id)) then
+State_Elmt := First_Elmt (Abstract_States (Spec_Id));
+while Present (State_Elmt) loop
+   Set_Has_Partial_Visible_Refinement (Node (State_Elmt), False);
+   Next_Elmt (State_Elmt);
+end loop;
+ end if;
+
+ --  For a child unit, also hide the partial state refinement from
+ --  ancestor packages.
+
+ if Is_Child_Unit (Spec_Id) then
+Remove_Partial_Visible_Refinements (Scope (Spec_Id));
+ end if;
+  end Remove_Partial_Visible_Refinements;
+
   
   -- Remove_Visible_Refinements --
   
@@ -2576,6 +2606,15 @@
 --  restore the original state conditions.
 
 Remove_Visible_Refinements (Corresponding_Spec (Context));
+Remove_Partial_Visible_Refinements (Corresponding_Spec (Context));
+
+ elsif Nkind (Context) = N_Package_Declaration then
+
+--  Partial state refinements are visible up to the end of the
+--  package spec declarations. Hide the partial state refinements
+--  from visibility to restore the original state conditions.
+
+Remove_Partial_Visible_Refinements (Corresponding_Spec (Context));
  end if;
 
  --  Verify that all abstract states found in any package declared in
Index: sem_ch7.adb
===
--- sem_ch7.adb (revision 241024)
+++ sem_ch7.adb (working copy)
@@ -2275,6 +2275,34 @@
  Next_Entity (Id);
   end loop;
 
+  --  An abstract state is partially refined when it has at least one
+  --  Part_Of constituent. Since these constituents are being installed
+  --  into visibility, update the partial refinement status of any state
+  --  defined in the associated package, subject to at least one Part_Of
+  --  constituent.
+
+  if Ekind_In (P, E_Generic_Package, E_Package) then
+ declare
+States : constant Elist_Id

[Ada] Spurious error on renaming of type conversion with invariant.

[Arnaud Charlet]

This patch fixes a spurious error on a renaming of a conversion of the
designated object of a pointer to class-wide type when the target type has
an invariant aspect.

The following must execute quietly:

   gnatmake -gnata -q main
   main

---
with System.Assertions; use System.Assertions;
with Gd; use Gd;
procedure main is
begin
   Foo;
   raise Program_Error;
exception
   when Assert_Failure => null;
end;
--
package CN is
   type CN_Type is private;
private
   type CN_Type is record
  V : Integer := 27;   -- wrong initialization
   end record  with Type_Invariant => V mod 7 = 0;
end;
---
package HD is
   type HD_Type is tagged null record;
   type HD_Class_Pointer is access HD_Type'Class;
end;
---
with CN;
with HD;
package GD is
   type XT is new HD.HD_Type with record
  X : aliased CN.CN_Type;
   end record;
   procedure Foo;
end;
--
package body GD is
   procedure Foo is
  DHP : constant HD.HD_Class_Pointer := new XT;
  DH  : XT renames XT (DHP.all);
   begin
  null;
   end;
end;

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Ed Schonberg  

* exp_ch4.adb (Expand_N_Type_Conversion): If the target type
has an invariant aspect, insert invariant call at the proper
place in the code rather than rewriting the expression as an
expression with actions, to prevent spurious semantic errors on
the rewritten conversion when it is the object in a renaming.

Index: exp_ch4.adb
===
--- exp_ch4.adb (revision 241041)
+++ exp_ch4.adb (working copy)
@@ -10577,16 +10577,17 @@
   end if;
 
   --  Check for case of converting to a type that has an invariant
-  --  associated with it. This required an invariant check. We convert
+  --  associated with it. This requires an invariant check. We insert
+  --  a call:
 
-  --typ (expr)
+  --invariant_check (typ (expr))
 
-  --  into
+  --  in the code, after removing side effects from the expression.
+  --  This is clearer than replacing the conversion into an expression
+  --  with actions, because the context may impose additional actions
+  --  (tag checks, membership tests, etc.) that conflict with this
+  --  rewriting (used previously).
 
-  --do invariant_check (typ (expr)) in typ (expr);
-
-  --  using Duplicate_Subexpr to avoid multiple side effects
-
   --  Note: the Comes_From_Source check, and then the resetting of this
   --  flag prevents what would otherwise be an infinite recursion.
 
@@ -10595,12 +10596,8 @@
 and then Comes_From_Source (N)
   then
  Set_Comes_From_Source (N, False);
- Rewrite (N,
-   Make_Expression_With_Actions (Loc,
- Actions=> New_List (
-   Make_Invariant_Call (Duplicate_Subexpr (N))),
- Expression => Duplicate_Subexpr_No_Checks (N)));
- Analyze_And_Resolve (N, Target_Type);
+ Remove_Side_Effects (N);
+ Insert_Action (N, Make_Invariant_Call (Duplicate_Subexpr (N)));
  goto Done;
   end if;
 

[Ada] Premature finalization of controlled array component

[Arnaud Charlet]

This patch modifies the processing of transient array components to properly
handle the finalization of the temporary controlled function result when the
call initializes a component choice list or an "others" choice.


-- Source --


--  aggregates.adb

with Ada.Strings.Unbounded; use Ada.Strings.Unbounded;
with Ada.Text_IO;   use Ada.Text_IO;

procedure Aggregates is
begin
   declare
  Arr : array (1 .. 3) of Unbounded_String :=
  (2  => To_Unbounded_String ("two"),
   others => To_Unbounded_String ("others"));
   begin
  Put ("others "); Put_Line (To_String (Arr (1)));
  Put ("two"); Put_Line (To_String (Arr (2)));
  Put ("others "); Put_Line (To_String (Arr (3)));
   end;

   declare
  Arr : array (1 .. 4) of Unbounded_String :=
  (1 | 3 | 4 => To_Unbounded_String ("one_three_four"),
   2 => To_Unbounded_String ("two"));
   begin
  Put ("one_three_four "); Put_Line (To_String (Arr (1)));
  Put ("two"); Put_Line (To_String (Arr (2)));
  Put ("one_three_four "); Put_Line (To_String (Arr (3)));
  Put ("one_three_four "); Put_Line (To_String (Arr (4)));
   end;

   declare
  Arr : array (1 .. 3) of Unbounded_String :=
  (1 .. 2 => To_Unbounded_String ("one_two"),
   others => To_Unbounded_String ("others"));
   begin
  Put ("one_two"); Put_Line (To_String (Arr (1)));
  Put ("one_two"); Put_Line (To_String (Arr (2)));
  Put ("others "); Put_Line (To_String (Arr (3)));
   end;

   declare
  Arr : array (1 .. 4) of Unbounded_String :=
  (1  => To_Unbounded_String ("one"),
   2 .. 4 => To_Unbounded_String ("two_four"));
   begin
  Put ("one"); Put_Line (To_String (Arr (1)));
  Put ("two_four   "); Put_Line (To_String (Arr (2)));
  Put ("two_four   "); Put_Line (To_String (Arr (3)));
  Put ("two_four   "); Put_Line (To_String (Arr (4)));
   end;

   declare
  Arr : array (1 .. 5) of Unbounded_String :=
  (1 .. 2 => To_Unbounded_String ("one_two"),
   4 | 5  => To_Unbounded_String ("four_five"),
   others => To_Unbounded_String ("others"));
   begin
  Put ("one_two"); Put_Line (To_String (Arr (1)));
  Put ("one_two"); Put_Line (To_String (Arr (2)));
  Put ("others "); Put_Line (To_String (Arr (3)));
  Put ("four_five  "); Put_Line (To_String (Arr (4)));
  Put ("four_five  "); Put_Line (To_String (Arr (5)));
   end;
end Aggregates;


-- Compilation and output --


$ gnatmake -q aggregates.adb
$ ./aggregates
others others
twotwo
others others
one_three_four one_three_four
twotwo
one_three_four one_three_four
one_three_four one_three_four
one_twoone_two
one_twoone_two
others others
oneone
two_four   two_four
two_four   two_four
two_four   two_four
one_twoone_two
one_twoone_two
others others
four_five  four_five
four_five  four_five

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Hristian Kirtchev  

* exp_aggr.adb (Initialize_Ctrl_Array_Component):
Create a copy of the initialization expression to avoid sharing
it between multiple components.

Index: exp_aggr.adb
===
--- exp_aggr.adb(revision 241024)
+++ exp_aggr.adb(working copy)
@@ -1277,6 +1277,7 @@
  is
 Act_Aggr   : Node_Id;
 Act_Stmts  : List_Id;
+Expr   : Node_Id;
 Fin_Call   : Node_Id;
 Hook_Clear : Node_Id;
 
@@ -1285,20 +1286,29 @@
 --  in-place expansion.
 
  begin
+--  Duplicate the initialization expression in case the context is
+--  a multi choice list or an "others" choice which plugs various
+--  holes in the aggregate. As a result the expression is no longer
+--  shared between the various components and is reevaluated for
+--  each such component.
+
+Expr := New_Copy_Tree (Init_Expr);
+Set_Parent (Expr, Parent (Init_Expr));
+
 --  Perform a preliminary analysis and resolution to determine what
 --  the initialization expression denotes. An unanalyzed function
 --  call may appear as an identifier or an indexed component.
 
-if Nkind_In (Init_Expr, N_Function_Call,
-N_Identifier,
-N_Indexed_Component)
-  and then not Analyzed (Init_Expr)
+if Nkind_In (Expr, N_Function_Call,
+   N_Identifier,
+   N_In

[Ada] Missing predicate check with explicit policy pragma

[Arnaud Charlet]

This patch fixes an omission in the generation of predicate checks, when there
is a configuration pragma that sets Assertion_Policy to Ignore and a subsequent
configuration pragma that enables predicate checking.

Executing:

   gnatmake -q main
   main

must yield

raised SYSTEM.ASSERTIONS.ASSERT_FAILURE :
Dynamic_Predicate failed at main.adb:5

---
pragma Assertion_Policy (Ignore);
 pragma Assertion_Policy
   (Static_Predicate => Check,
   Dynamic_Predicate => Check,
   Pre => Check, Pre'Class => Check);
---
with Pred;
procedure Main is
begin
  Pred.Foo ("");
end;
---
package Pred is
  subtype Not_Empty_String is String
with Dynamic_Predicate => Not_Empty_String /= "";
  procedure Foo (S : Not_Empty_String);
end;
---
package body Pred is
  procedure Foo (S : Not_Empty_String) is begin null; end;
end;

Tested on x86_64-pc-linux-gnu, committed on trunk

2016-10-12  Ed Schonberg  

* sem_prag.adb (Analyze_Pragma, case Dynamic_Predicate):
Check properly whether there is an explicit assertion policy
for predicate checking, even in the presence of a general Ignore
assertion policy.

Index: sem_prag.adb
===
--- sem_prag.adb(revision 241048)
+++ sem_prag.adb(working copy)
@@ -19136,15 +19136,17 @@
 --  the rep item chain, for processing when the type is frozen.
 --  This is accomplished by a call to Rep_Item_Too_Late. We also
 --  mark the type as having predicates.
---  If the current policy is Ignore mark the subtype accordingly.
---  In the case of predicates we consider them enabled unless an
---  Ignore is specified, to preserve existing warnings.
 
+--  If the current policy for predicate checking is Ignore mark the
+--  subtype accordingly. In the case of predicates we consider them
+--  enabled unless Ignore is specified (either directly or with a
+--  general Assertion_Policy pragma) to preserve existing warnings.
+
 Set_Has_Predicates (Typ);
 Set_Predicates_Ignored (Typ,
   Present (Check_Policy_List)
 and then
-  Policy_In_Effect (Name_Assertion_Policy) = Name_Ignore);
+  Policy_In_Effect (Name_Dynamic_Predicate) = Name_Ignore);
 Discard := Rep_Item_Too_Late (Typ, N, FOnly => True);
  end Predicate;
 

Re: [RFC] Speed-up -fprofile-update=atomic

[Martin Liška]

On 10/04/2016 11:45 AM, Richard Biener wrote:
> On Thu, Sep 15, 2016 at 12:00 PM, Martin Liška  wrote:
>> On 09/07/2016 02:09 PM, Richard Biener wrote:
>>> On Wed, Sep 7, 2016 at 1:37 PM, Martin Liška  wrote:
 On 08/18/2016 06:06 PM, Richard Biener wrote:
> On August 18, 2016 5:54:49 PM GMT+02:00, Jakub Jelinek  
> wrote:
>> On Thu, Aug 18, 2016 at 08:51:31AM -0700, Andi Kleen wrote:
 I'd prefer to make updates atomic in multi-threaded applications.
 The best proxy we have for that is -pthread.

 Is it slower, most definitely, but odds are we're giving folks
 garbage data otherwise, which in many ways is even worse.
>>>
>>> It will likely be catastrophically slower in some cases.
>>>
>>> Catastrophically as in too slow to be usable.
>>>
>>> An atomic instruction is a lot more expensive than a single
>> increment. Also
>>> they sometimes are really slow depending on the state of the machine.
>>
>> Can't we just have thread-local copies of all the counters (perhaps
>> using
>> __thread pointer as base) and just atomically merge at thread
>> termination?
>
> I suggested that as well but of course it'll have its own class of issues 
> (short lived threads, so we need to somehow re-use counters from 
> terminated threads, large number of threads and thus using too much 
> memory for the counters)
>
> Richard.

 Hello.

 I've got written the approach on my TODO list, let's see whether it would 
 be doable in a reasonable amount of time.

 I've just finished some measurements to illustrate slow-down of 
 -fprofile-update=atomic approach.
 All numbers are: no profile, -fprofile-generate, -fprofile-generate 
 -fprofile-update=atomic
 c-ray benchmark (utilizing 8 threads, -O3): 1.7, 15.5., 38.1s
 unrar (utilizing 8 threads, -O3): 3.6, 11.6, 38s
 tramp3d (1 thread, -O3): 18.0, 46.6, 168s

 So the slow-down is roughly 300% compared to -fprofile-generate. I'm not 
 having much experience with default option
 selection, but these numbers can probably help.

 Thoughts?
>>>
>>> Look at the generated code for an instrumented simple loop and see that for
>>> the non-atomic updates we happily apply store-motion to the counter update
>>> and thus we only get one counter update per loop exit rather than one per
>>> loop iteration.  Now see what happens for the atomic case (I suspect you
>>> get one per iteration).
>>>
>>> I'll bet this accounts for most of the slowdown.
>>>
>>> Back in time ICC which had atomic counter updates (but using function
>>> calls - ugh!) had a > 1000% overhead with FDO for tramp3d (they also
>>> didn't have early inlining -- removing abstraction helps reducing the number
>>> of counters significantly).
>>>
>>> Richard.
>>
>> Hi.
>>
>> During Cauldron I discussed with Richi approaches how to speed-up ARCS
>> profile counter updates. My first attempt is to utilize TLS storage, where
>> every function is accumulating arcs counters. These are eventually added
>> (using atomic operations) to the global one at the very end of a function.
>> Currently I rely on target support of TLS, which is questionable whether
>> to have such a requirement for -fprofile-update=atomic, or to add a new 
>> option value
>> like -fprofile-update=atomic-tls?
>>
>> Running the patch on tramp3d, compared to previous numbers, it takes 88s to 
>> finish.
>> Time shrinks to 50%, compared to the current implementation.
>>
>> Thoughts?
> 
> Hmm, I thought I suggested that you can simply use automatic storage
> (which effectively
> is TLS...) for regions that are not forked or abnormally left (which
> means SESE regions
> that have no calls that eventually terminate or throw externally).
> 
> So why did you end up with TLS?

Hi.

Usage for TLS does not makes sense, stupid mistake ;)

By using SESE regions, do you mean the infrastructure that is utilized
by Graphite machinery?

Thanks,
Martin

> 
> Richard.
> 
>> Martin
>>
>>>
 Martin

>
>>  Jakub
>
>

>>

Re: Move OVERRIDE/FINAL from gcc/coretypes.h to include/ansidecl.h

[Bernd Schmidt]

On 10/10/2016 09:32 PM, Pedro Alves wrote:

From: Pedro Alves 
Date: 2016-10-10 19:25:47 +0100

Move OVERRIDE/FINAL from gcc/coretypes.h to include/ansidecl.h

So that GDB and other projects that share the top level can use them.


Seems reasonable. OK unless someone objects by the end of the week.


Bernd

Re: [PATCH 1/2] [ARC] Update INSN_LENGTH_ALIGNMENT.

[Andrew Burgess]

* Claudiu Zissulescu  [2016-06-30 12:24:11 
+0200]:

> Update the INSN_LENGTH_ALIGNMENT macro to handle jump tables placed in
> program memory.
> 
> Ok to apply?
> Claudiu

This looks good to me.


Thanks,
Andrew


> 
> 
> gcc/
> 2016-06-20  Claudiu Zissulescu  
> 
>   * config/arc/arc.h (INSN_LENGTH_ALIGNMENT): Change.
> ---
>  gcc/config/arc/arc.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> index 3376ad8..d2adf4d 100644
> --- a/gcc/config/arc/arc.h
> +++ b/gcc/config/arc/arc.h
> @@ -1519,10 +1519,10 @@ extern int arc_return_address_regs[4];
>  #define ASM_OUTPUT_BEFORE_CASE_LABEL(FILE, PREFIX, NUM, TABLE) \
>ASM_OUTPUT_ALIGN ((FILE), ADDR_VEC_ALIGN (TABLE));
>  
> -#define INSN_LENGTH_ALIGNMENT(INSN) \
> -  ((JUMP_P (INSN) \
> +#define INSN_LENGTH_ALIGNMENT(INSN)\
> +  ((JUMP_TABLE_DATA_P (INSN)   \
>  && GET_CODE (PATTERN (INSN)) == ADDR_DIFF_VEC \
> -&& GET_MODE (PATTERN (INSN)) == QImode) \
> +&& GET_MODE (PATTERN (INSN)) == QImode)\
> ? 0 : length_unit_log)
>  
>  /* Define if operations between registers always perform the operation
> -- 
> 1.9.1
>