[committed] RISC-V: Drop some commited accidentally code.

2020-11-30 Thread Kito Cheng
gcc/ChangeLog:

* config.gcc (riscv*-*-*): Drop some commited accidentally code.
---
 gcc/config.gcc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index c348596b1ac..4808b698f3a 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4615,7 +4615,6 @@ case "${target}" in
exit 1
;;
esac
-   with_arch=`${srcdir}/config/riscv/arch-canonicalize 
${with_arch}`
tm_defines="${tm_defines} 
TARGET_RISCV_DEFAULT_ARCH=${with_arch}"
 
# Make sure --with-abi is valid.  If it was not specified,
-- 
2.29.2



[PATCH] Add unordered containers heterogeneous lookup

2020-11-30 Thread François Dumont via Gcc-patches
Let me know if I need to reference a specific paper or any other 
Standard reference here. Maybe P1690R1 I used here ?


I tried to allow the same partition trick you can have on ordered 
containers (see Partition in tests) even if here elements are not 
ordered so I aren't sure there can be any usage of it.


    libstdc++: Add unordered containers heterogeneous lookup

    Add unordered containers heterogeneous lookup member functions 
find, count, contains and
    equal_range in C++20. Those members are considered for overload 
resolution only if hash and
    equal functors used to instantiate the container have a nested 
is_transparent type.


    libstdc++-v3/ChangeLog:

    * include/bits/stl_tree.h
    (__has_is_transparent, __has_is_transparent_t): Move...
    * include/bits/stl_function.h: ...here.
    * include/bits/hashtable_policy.h 
(_Hash_code_base<>::_M_hash_code):

    Use template key type.
    (_Hashtable_base<>::_M_equals): Likewise.
    * include/bits/hashtable.h (_Hashtable<>::_M_find_tr, 
_Hashtable<>::_M_count_tr,
    _Hashtable<>::_M_equal_range_tr): New member function 
templates to perform

    heterogeneous lookup.
    (_Hashtable<>::_M_find_before_node): Use template key type.
    (_Hashtable<>::_M_find_node): Likewise.
    * include/bits/unordered_map.h (unordered_map::find<>, 
unordered_map::count<>,
    unordered_map::contains<>, unordered_map::equal_range<>): 
New member function

    templates to perform heterogeneous lookup.
    (unordered_multimap::find<>, unordered_multimap::count<>,
    unordered_multimap::contains<>, 
unordered_multimap::equal_range<>): Likewise.
    * include/bits/unordered_set.h (unordered_set::find<>, 
unordered_set::count<>,
    unordered_set::contains<>, unordered_set::equal_range<>): 
Likewise.

    (unordered_multiset::find<>, unordered_multiset::count<>,
    unordered_multiset::contains<>, 
unordered_multiset::equal_range<>): Likewise.

    * include/debug/unordered_map
    (unordered_map::find<>, unordered_map::equal_range<>): 
Likewise.
    (unordered_multimap::find<>, 
unordered_multimap::equal_range<>): Likewise.

    * include/debug/unordered_set
    (unordered_set::find<>, unordered_set::equal_range<>): 
Likewise.
    (unordered_multiset::find<>, 
unordered_multiset::equal_range<>): Likewise.
    * testsuite/23_containers/unordered_map/operations/1.cc: 
New test.
    * 
testsuite/23_containers/unordered_multimap/operations/1.cc: New test.
    * 
testsuite/23_containers/unordered_multiset/operations/1.cc: New test.
    * testsuite/23_containers/unordered_set/operations/1.cc: 
New test.


Tested under Linux x86_64 normal and debug modes.

François

diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index 6c6c5edde0b..30d4ee58100 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -724,6 +724,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   std::pair
   equal_range(const key_type& __k) const;
 
+#if __cplusplus > 201702L
+  template,
+	   typename = __has_is_transparent_t<_Equal, _Kt>>
+	iterator
+	_M_find_tr(const _Kt& __k);
+
+  template,
+	   typename = __has_is_transparent_t<_Equal, _Kt>>
+	const_iterator
+	_M_find_tr(const _Kt& __k) const;
+
+  template,
+	   typename = __has_is_transparent_t<_Equal, _Kt>>
+	size_type
+	_M_count_tr(const _Kt& __k) const;
+
+  template,
+	   typename = __has_is_transparent_t<_Equal, _Kt>>
+	pair
+	_M_equal_range_tr(const _Kt& __k);
+
+  template,
+	   typename = __has_is_transparent_t<_Equal, _Kt>>
+	pair
+	_M_equal_range_tr(const _Kt& __k) const;
+#endif
+
 private:
   // Bucket index computation helpers.
   size_type
@@ -736,12 +768,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Find and insert helper functions and types
   // Find the node before the one matching the criteria.
+  template
 	__node_base_ptr
-  _M_find_before_node(size_type, const key_type&, __hash_code) const;
+	_M_find_before_node(size_type, const _Kt&, __hash_code) const;
 
+  template
 	__node_ptr
-  _M_find_node(size_type __bkt, const key_type& __key,
-		   __hash_code __c) const
+	_M_find_node(size_type __bkt, const _Kt& __key, __hash_code __c) const
 	{
 	  __node_base_ptr __before_n = _M_find_before_node(__bkt, __key, __c);
 	  if (__before_n)
@@ -1532,6 +1565,40 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return const_iterator(_M_find_node(__bkt, __k, __code));
 }
 
+#if __cplusplus > 201703L
+  template
+template
+  auto
+  _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
+		 _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::
+  _M_find_tr(const _Kt& __k)
+  -> iterator
+  {
+	__hash_code __code 

Re: [PATCH] [tree-optimization] Optimize max/min pattern with comparison

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 7:00 PM, Eugene Rozenfeld wrote:
> Thank you for the review Jeff.
>
> I don't need to look at the opcode to know the result. The pattern will be 
> matched only in these 4 cases:
>
> X <= MAX(X, Y) -> true
> X > MAX(X, Y) -> false
> X >= MIN(X, Y) -> true
> X < MIN(X, Y) -> false
>
> So, the result will be true for GE_EXPR and LE_EXPR and false otherwise.
>
> I added two test files: one for positive cases and one for negative cases. 
> The updated patch is attached.
Nuts, they weren't nested FORs, sorry for mis-reading.  I'll take
another close look tomorrow.

jeff



[PATCH] [Refactor] [AVX512] Combine VI12_AVX512VL with VI48_AVX512VL into VI_AVX512VLBW

2020-11-30 Thread Hongtao Liu via Gcc-patches
Hi:
There're many pairs of define_insn/define_expand that are very similar
to each other except mode iterator and condition. For these patterns
VI12_AVX512VL are used under condition TARGET_AVX512BW, and
VI48_AVX512VL are used under condition TARGET_AVX512F.

This patch is about to introduce a new iterator VI_AVX512VLBW to
combine a pair of those patterns into one.

There are no functional changes, just code refactoring.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

gcc/ChangeLog

* config/i386/sse.md (VI_AVX512VLBW): New mode iterator.
(_ucmp3): Combine
two patterns with mode iterator VI12_AVX512VL and VI48_AVX512VL
into one pattern with mode iterator VI_AVX512VLBW.
(vec_cmpu): Ditto.
(_cvt2mask): Ditto.
(_cvtmask2): Ditto.
(*_cvtmask2): Ditto.
(3_mask): Ditto.
(*3_mask): Ditto.
(_eq3): Ditto.
(_eq3_1): Ditto.
(_gt3): Ditto.
(_andnot3_mask): Ditto.
(abs2_mask): Ditto.
(*_3): Combine from ...
(*avx512f_3)
and (3).


-- 
BR,
Hongtao
From e55528fb9a0346365327e7b1cdebadec7c71be15 Mon Sep 17 00:00:00 2001
From: liuhongt 
Date: Mon, 30 Nov 2020 13:24:45 +0800
Subject: [PATCH] Combine VI12_AVX512VL with VI48_AVX512VL into VI_AVX512VLBW.

There're many pairs of define_insn/define_expand that are very similar
to each other except mode iterator and condition. For these patterns
VI12_AVX512VL are used under condition TARGET_AVX512BW, and
VI48_AVX512VL are used under condition TARGET_AVX512F.

This patch is about to introduce a new iterator VI_AVX512VLBW to
combine a pair of those patterns into one.

There're no functional changed, just code refactoring.

gcc/ChangeLog

	* config/i386/sse.md (VI_AVX512VLBW): New mode iterator.
	(_ucmp3): Combine
	two patterns with mode iterator VI12_AVX512VL and VI48_AVX512VL
	into one pattern with mode iterator VI_AVX512VLBW.
	(vec_cmpu): Ditto.
	(_cvt2mask): Ditto.
	(_cvtmask2): Ditto.
	(*_cvtmask2): Ditto.
	(3_mask): Ditto.
	(*3_mask): Ditto.
	(_eq3): Ditto.
	(_eq3_1): Ditto.
	(_gt3): Ditto.
	(_andnot3_mask): Ditto.
	(abs2_mask): Ditto.
	(*_3): Combine from ...
	(*avx512f_3)
	and (3).
---
 gcc/config/i386/sse.md | 312 -
 1 file changed, 89 insertions(+), 223 deletions(-)

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 4aad462f882..c761a018e86 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -335,6 +335,14 @@ (define_mode_iterator VI48_AVX512VL
   [V16SI (V8SI  "TARGET_AVX512VL") (V4SI  "TARGET_AVX512VL")
V8DI  (V4DI  "TARGET_AVX512VL") (V2DI  "TARGET_AVX512VL")])
 
+(define_mode_iterator VI_AVX512VLBW
+  [(V16SI "TARGET_AVX512F") (V8SI  "TARGET_AVX512VL") (V4SI  "TARGET_AVX512VL")
+   (V8DI "TARGET_AVX512F") (V4DI  "TARGET_AVX512VL") (V2DI  "TARGET_AVX512VL")
+   (V64QI "TARGET_AVX512BW") (V32QI "TARGET_AVX512VL && TARGET_AVX512BW")
+   (V16QI "TARGET_AVX512BW && TARGET_AVX512VL")
+   (V32HI "TARGET_AVX512BW") (V16HI "TARGET_AVX512VL && TARGET_AVX512BW")
+   (V8HI "TARGET_AVX512VL && TARGET_AVX512BW")])
+
 (define_mode_iterator VF_AVX512VL
   [V16SF (V8SF "TARGET_AVX512VL") (V4SF "TARGET_AVX512VL")
V8DF (V4DF "TARGET_AVX512VL") (V2DF "TARGET_AVX512VL")])
@@ -2981,25 +2989,11 @@ (define_insn "_cmp3"
 (define_insn "_ucmp3"
   [(set (match_operand: 0 "register_operand" "=k")
 	(unspec:
-	  [(match_operand:VI12_AVX512VL 1 "register_operand" "v")
-	   (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm")
+	  [(match_operand:VI_AVX512VLBW 1 "register_operand" "v")
+	   (match_operand:VI_AVX512VLBW 2 "nonimmediate_operand" "vm")
 	   (match_operand:SI 3 "const_0_to_7_operand" "n")]
 	  UNSPEC_UNSIGNED_PCMP))]
-  "TARGET_AVX512BW"
-  "vpcmpu\t{%3, %2, %1, %0|%0, %1, %2, %3}"
-  [(set_attr "type" "ssecmp")
-   (set_attr "length_immediate" "1")
-   (set_attr "prefix" "evex")
-   (set_attr "mode" "")])
-
-(define_insn "_ucmp3"
-  [(set (match_operand: 0 "register_operand" "=k")
-	(unspec:
-	  [(match_operand:VI48_AVX512VL 1 "register_operand" "v")
-	   (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm")
-	   (match_operand:SI 3 "const_0_to_7_operand" "n")]
-	  UNSPEC_UNSIGNED_PCMP))]
-  "TARGET_AVX512F"
+  ""
   "vpcmpu\t{%3, %2, %1, %0|%0, %1, %2, %3}"
   [(set_attr "type" "ssecmp")
(set_attr "length_immediate" "1")
@@ -3149,22 +3143,9 @@ (define_expand "vec_cmp"
 (define_expand "vec_cmpu"
   [(set (match_operand: 0 "register_operand")
 	(match_operator: 1 ""
-	  [(match_operand:VI48_AVX512VL 2 "register_operand")
-	   (match_operand:VI48_AVX512VL 3 "nonimmediate_operand")]))]
-  "TARGET_AVX512F"
-{
-  bool ok = ix86_expand_mask_vec_cmp (operands[0], GET_CODE (operands[1]),
-  operands[2], operands[3]);
-  gcc_assert (ok);
-  DONE;
-})
-
-(define_expand "vec_cmpu"
-  [(set (match_operand: 0 "register_operand")
-	(match_operator: 1 ""
-	  [(match_operand:VI12_AVX512VL 2 "register_operand")
-	   

Re: [PATCH] i386: Optimize vpsubusw compared to 0 into vpcmpleuw or vpcmpnltuw[PR96906]

2020-11-30 Thread Hongtao Liu via Gcc-patches
On Mon, Nov 30, 2020 at 9:46 PM Jakub Jelinek  wrote:
>
> On Mon, Nov 30, 2020 at 09:11:10PM +0800, Hongtao Liu wrote:
> > +;; PR96906 - optimize vpsubusw compared to 0 into vpcmpleuw or vpcmpnltuw.
> > +(define_split
> > +  [(set (match_operand: 0 "register_operand")
> > +(unspec:
> > +  [(us_minus:VI12_AVX512VL
> > + (match_operand:VI12_AVX512VL 1 "vector_operand")
> > + (match_operand:VI12_AVX512VL 2 "vector_operand"))
> > +   (match_operand:VI12_AVX512VL 3 "const0_operand")
> > +   (match_operand:SI 4 "const0_operand")]
> > +  UNSPEC_PCMP))]
> > +  "TARGET_AVX512BW && ix86_binary_operator_ok (US_MINUS, mode, 
> > operands)"
>
> Too long line, please wrap it.
> Also, INTVAL (operands[4]) == 0 is EQ comparison, can't we handle also
> NE (i.e. INTVAL (operands[4]) == 4?
> I.e. replace the "const0_operand" in there with "const_0_to_7_operand"
> and check in conditions that (INTVAL (operands[4]) & 3) == 0.
>
> > +  [(const_int 0)]
> > +  {
> > +/* LE: 2, NLT: 5.  */
> > +rtx cmp_predicate = GEN_INT (2);
> > +if (MEM_P (operands[1]))
> > +  {
> > +std::swap (operands[1], operands[2]);
> > +cmp_predicate = GEN_INT (5);
>
> For INTVAL (operands[4]) == 4 it would then be cmp_predictate NLE: 4 resp.
> LT: 3 I think.
>
> Also, this handles only UNSPEC_PCMP, can't we handle UNSPEC_UNSIGNED_PCMP
> too?  I mean, for equality comparisons it doesn't really matter if we have
> signed or unsigned == or !=.  And for unsigned
> x == 0U is equivalent to x <= 0U, and x != 0U equivalent to x > 0U.
>
> Jakub
>

Yes, Update patch.

+(define_int_iterator UNSPEC_PCMP_ITER
+  [UNSPEC_PCMP UNSPEC_UNSIGNED_PCMP])
+
+(define_int_attr pcmp_signed_mask
+  [(UNSPEC_PCMP "3") (UNSPEC_UNSIGNED_PCMP "1")])
+
+;; PR96906 - optimize vpsubusw compared to 0 into vpcmpleuw or vpcmpnltuw.
+;; For signed comparison, handle EQ 0: NEQ 4,
+;; for unsigned comparison extra handle LE:2, NLE:6, equivalent to EQ and NEQ.
+
+(define_split
+  [(set (match_operand: 0 "register_operand")
+   (unspec:
+ [(us_minus:VI12_AVX512VL
+(match_operand:VI12_AVX512VL 1 "vector_operand")
+(match_operand:VI12_AVX512VL 2 "vector_operand"))
+  (match_operand:VI12_AVX512VL 3 "const0_operand")
+  (match_operand:SI 4 "const_0_to_7_operand")]
+ UNSPEC_PCMP_ITER))]
+  "TARGET_AVX512BW
+  && ix86_binary_operator_ok (US_MINUS, mode, operands)
+  && (INTVAL (operands[4]) & ) == 0"
+  [(const_int 0)]
+  {
+bool neq_p = INTVAL (operands[4]) >> 2;
+/* LE: 2, NLT: 5, NLE: 6, LT: 1  */
+rtx cmp_predicate = neq_p ? GEN_INT (6) : GEN_INT (2);
+if (MEM_P (operands[1]))
+  {
+   std::swap (operands[1], operands[2]);
+   cmp_predicate = neq_p ? GEN_INT (1) : GEN_INT (5);
+  }
+emit_insn (gen__ucmp3 (operands[0], operands[1],
+   operands[2], cmp_predicate));
+DONE;
+  })
+


-- 
BR,
Hongtao
From e3eb61066ee665325cba8e231b991f9a1dda07df Mon Sep 17 00:00:00 2001
From: liuhongt 
Date: Mon, 30 Nov 2020 13:27:16 +0800
Subject: [PATCH]  Optimize vpsubusw compared to 0 into vpcmpleuw or vpcmpnleuw
 [PR96906]

For signed comparisons, it handles cases that are eq or neq to 0.
For unsigned comparisons, it additionaly handles cases that are le or
gt to 0(equivilent to eq or neq to 0). Transform case eq to leu,
case neq to gtu.

.i.e. for -mavx512bw -mavx512vl transform eq case code from

	vpsubusw%xmm1, %xmm0, %xmm0
	vpxor   %xmm1, %xmm1, %xmm1
	vpcmpeqw  %xmm1, %xmm0, %k0
to
	vpcmpleuw   %xmm1, %xmm0, %k0

.i.e. for -mavx512bw -mavx512vl transform neq case code from

	vpsubusw%xmm1, %xmm0, %xmm0
	vpxor   %xmm1, %xmm1, %xmm1
	vpcmpneqw  %xmm1, %xmm0, %k0
to
	vpcmpnleuw   %xmm1, %xmm0, %k0

gcc/ChangeLog
	PR target/96906
	* config/i386/sse.md
	(_ucmp3): Add a new
	define_split after this insn.

gcc/testsuite/ChangeLog

	* gcc.target/i386/avx512bw-pr96906-1.c: New test.
	* gcc.target/i386/pr96906-1.c: Add -mno-avx512f.
---
 gcc/config/i386/sse.md| 37 ++
 .../gcc.target/i386/avx512bw-pr96906-1.c  | 68 +++
 gcc/testsuite/gcc.target/i386/pr96906-1.c |  2 +-
 3 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/avx512bw-pr96906-1.c

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 4aad462f882..7a4dafea1ed 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -3006,6 +3006,43 @@ (define_insn "_ucmp3"
(set_attr "prefix" "evex")
(set_attr "mode" "")])
 
+(define_int_iterator UNSPEC_PCMP_ITER
+  [UNSPEC_PCMP UNSPEC_UNSIGNED_PCMP])
+
+(define_int_attr pcmp_signed_mask
+  [(UNSPEC_PCMP "3") (UNSPEC_UNSIGNED_PCMP "1")])
+
+;; PR96906 - optimize vpsubusw compared to 0 into vpcmpleuw or vpcmpnltuw.
+;; For signed comparison, handle EQ 0: NEQ 4,
+;; for unsigned comparison extra handle LE:2, NLE:6, equivalent to EQ 

Re: [PATCH] [tree-optimization] Optimize max/min pattern with comparison

2020-11-30 Thread Eugene Rozenfeld via Gcc-patches
Thank you for the review Jeff.

I don't need to look at the opcode to know the result. The pattern will be 
matched only in these 4 cases:

X <= MAX(X, Y) -> true
X > MAX(X, Y) -> false
X >= MIN(X, Y) -> true
X < MIN(X, Y) -> false

So, the result will be true for GE_EXPR and LE_EXPR and false otherwise.

I added two test files: one for positive cases and one for negative cases. The 
updated patch is attached.

Thanks,

Eugene

-Original Message-
From: Jeff Law  
Sent: Monday, November 30, 2020 9:51 AM
To: Eugene Rozenfeld ; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] [tree-optimization] Optimize max/min pattern with 
comparison



On 11/25/20 3:04 PM, Eugene Rozenfeld via Gcc-patches wrote:
> Make the following simplifications:
> X <= MAX(X, Y) -> true
> X > MAX(X, Y) -> false
> X >= MIN(X, Y) -> true
> X < MIN(X, Y) -> false
> 
> This fixes PR96708.
> 
> Tested on x86_64-pc-linux-gnu.
>
> bool f(int a, int b)
> {
> int tmp = (a < b) ? b : a;
> return tmp >= a;
> }
>
> Code without the patch:
>
> vmovd  xmm0,edi
> vmovd  xmm1,esi
> vpmaxsd xmm0,xmm0,xmm1
> vmovd  eax,xmm0
> cmpeax,edi
> setge  al
> ret
>
> Code with the patch:
>
> moveax,0x1
> ret
>
> Eugene
>
> 0001-Optimize-max-pattern-with-comparison.patch
>
> From f6391c197b670b516238ac7707512c1358336520 Mon Sep 17 00:00:00 2001
> From: Eugene Rozenfeld 
> Date: Sat, 21 Nov 2020 01:08:50 -0800
> Subject: [PATCH] Optimize max pattern with comparison
>
> Make the following simplifications:
> X <= MAX(X, Y) -> true
> X > MAX(X, Y) -> false
> X >= MIN(X, Y) -> true
> X < MIN(X, Y) -> false
>
> This fixes PR96708.
>
> gcc/
> * match.pd : New patterns.
> ---
>  gcc/match.pd | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd index 
> cbb4bf0b32d..75237741946 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2851,6 +2851,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>(cmp (minmax @0 INTEGER_CST@1) INTEGER_CST@2)
>(comb (cmp @0 @2) (cmp @1 @2
>  
> +/* X <= MAX(X, Y) -> true
> +   X > MAX(X, Y) -> false 
> +   X >= MIN(X, Y) -> true
> +   X < MIN(X, Y) -> false */
> +(for minmax (min min max max )
> + cmp(ge  lt  le  gt  )
> + (simplify
> +  (cmp @0 (minmax:c @0 @1))
> +  { constant_boolean_node (cmp == GE_EXPR || cmp == LE_EXPR, type); } 
> +))
Don't you need to look at the opcode (MIN vs MAX) vs CMP to know the result?  
I'd really like to see some tests for the testsuite.    In particular I'd 
like to see positive tests where we should apply the optimization and negative 
tests when we should not apply the optimization.

I also wonder if there's value in handling this in Ranger and/or DOM. Though 
I'd probably wait to see if fixing in match.pd is sufficient to cover the cases 
I'm thinking of in Ranger & DOM.

Jeff




0001-Optimize-min-and-max-patterns-with-comparison.patch
Description: 0001-Optimize-min-and-max-patterns-with-comparison.patch


Re: [PATCH][PR target/97770] x86: Add missing popcount2 expander

2020-11-30 Thread Hongyu Wang via Gcc-patches
> OK.  Presumably once this is applied Richi is going to  look at the
> higher level issues in the vectorizer which inhibit creating the HI/QI
> vector popcounts?
>

Yes, this is the prerequisite to look at the vectorization issue. I'll
ask Hongtao to
help check-in this patch. Thanks for the approval.

Jeff Law  于2020年12月1日周二 上午12:17写道:

>
>
>
> On 11/11/20 6:54 PM, Hongyu Wang via Gcc-patches wrote:
> > Hi,
> >
> > According to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97770, x86
> > backend need popcount2 expander so __builtin_popcount could be
> > auto vectorized with AVX512BITALG/AVX512VPOPCNTDQ targets.
> >
> > For DImode the middle-end vectorizer could not generate expected code,
> > and for QI/HImode there is no corresponding IFN, xfails are added for
> > these tests.
> >
> > Bootstrap/regression test for x86 backend is OK.
> >
> > OK for master?
> >
> > gcc/ChangeLog
> >
> > PR target/97770
> > * gcc/config/i386/sse.md (popcount2): New expander
> > for SI/DI vector modes.
> > (popcount2): Likewise for QI/HI vector modes.
> >
> > gcc/testsuite/ChangeLog
> >
> > PR target/97770
> > * gcc.target/i386/avx512bitalg-pr97770-1.c: New test.
> > * gcc.target/i386/avx512vpopcntdq-pr97770-1.c: Likewise.
> > * gcc.target/i386/avx512vpopcntdq-pr97770-2.c: Likewise.
> > * gcc.target/i386/avx512vpopcntdqvl-pr97770-1.c: Likewise.
> >
> >
> > 0001-Add-popcount-mode-expander-to-enable-popcount-auto-v.patch
> >
> OK.  Presumably once this is applied Richi is going to  look at the
> higher level issues in the vectorizer which inhibit creating the HI/QI
> vector popcounts?
>
> Jeff
>


Re: [PATCH] handle conditionals in -Wstringop-overflow et al. (PR 92936)

2020-11-30 Thread Martin Sebor via Gcc-patches

On 11/30/20 1:49 PM, Jeff Law wrote:



On 11/29/20 3:27 PM, Martin Sebor wrote:

On 11/13/20 2:34 PM, Jeff Law wrote:


On 11/2/20 7:24 PM, Martin Sebor wrote:

The attached patch extends compute_objsize() to handle conditional
expressions represented either as PHIs or MIN_EXPR and MAX_EXPR.

To simplify the handling of the -Wstringop-overflow/-overread
warnings the change factors this code out of tree-ssa-strlen.c
and into inform_access() in builtins.c, making it a member of
access_ref.  Besides eliminating a decent amount of code
duplication this also improves the consistency of the warnings.

Finally, the change introduces a distinction between the definite
kinds of -Wstringop-overflow (and -Wstringop-overread) warnings
and the maybe kind.  The latter are currently only being issued
for function array parameters but I expect to make use of them
more extensively in the future.

Besides the usual GCC bootstrap/regtest I have tested the change
with Binutils/GDB and Glibc and verified that it doesn't introduce
any false positives.

Martin

gcc-92936.diff

PR middle-end/92936 - missing warning on a past-the-end store to a PHI
PR middle-end/92940 - incorrect offset and size in
-Wstringop-overflow for out-of-bounds store into VLA and two offset
ranges
PR middle-end/89428 - missing -Wstringop-overflow on a PHI with
variable offset

gcc/ChangeLog:

 PR middle-end/92936
 PR middle-end/92940
 PR middle-end/89428
 * builtins.c (access_ref::access_ref): Initialize member.
 (access_ref::phi): New function.
 (access_ref::get_ref): New function.
 (access_ref::add_offset): Remove duplicate assignment.
 (maybe_warn_for_bound): Add "maybe" kind of warning messages.
 (warn_for_access): Same.
 (inform_access): Rename...
 (access_ref::inform_access): ...to this.  Print PHI arguments.
Format
 offset the same as size and simplify.  Improve printing of
allocation
 functions and VLAs.
 (check_access): Adjust to the above.
 (gimple_parm_array_size): Change argument.
 (handle_min_max_size): New function.
 * builtins.h (struct access_ref): Declare new members.
 (gimple_parm_array_size): Change argument.
 * tree-ssa-strlen.c (maybe_warn_overflow): Use access_ref and
simplify.
 (handle_builtin_memcpy): Correct argument passed to
maybe_warn_overflow.
 (handle_builtin_memset): Same.

gcc/testsuite/ChangeLog:

 PR middle-end/92936
 PR middle-end/92940
 PR middle-end/89428
 * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected
 informational notes.
 * gcc.dg/Wstringop-overflow-11.c: Remove xfails.
 * gcc.dg/Wstringop-overflow-12.c: Same.
 * gcc.dg/Wstringop-overflow-17.c: Adjust text of expected messages.
 * gcc.dg/Wstringop-overflow-27.c: Same.  Remove xfails.
 * gcc.dg/Wstringop-overflow-28.c: Adjust text of expected messages.
 * gcc.dg/Wstringop-overflow-29.c: Same.
 * gcc.dg/Wstringop-overflow-37.c: Same.
 * gcc.dg/Wstringop-overflow-46.c: Same.
 * gcc.dg/Wstringop-overflow-47.c: Same.
 * gcc.dg/Wstringop-overflow-54.c: Same.
 * gcc.dg/warn-strnlen-no-nul.c: Add expected warning.
 * gcc.dg/Wstringop-overflow-58.c: New test.
 * gcc.dg/Wstringop-overflow-59.c: New test.
 * gcc.dg/Wstringop-overflow-60.c: New test.
 * gcc.dg/Wstringop-overflow-61.c: New test.
 * gcc.dg/Wstringop-overflow-62.c: New test.


So my only significant concern here is the recursive nature and the
lack of a limiter for pathological cases.  We certainly run into
cases with thousands of PHI arguments and deep chains of PHIs feeding
other PHIs.  Can you put in a PARAM to limit the amount of recursion
and and PHI arguments you look at?  With that I think this is fine --
I consider it unlikely this patch is the root cause of the ICEs I
sent you earlier today from the tester since those failures are in
the array bounds checking bits.


I've reused the same approach/class as in tree-ssa-strlen.c and
after adjusting things that need to be adjusted and retesting
with Binutils/GDB and Glibc committed the attached patch in
r11-5523.

That said, although the recursion hasn't been a problem (there's
still code elsewhere that does this sort of traversal of the use-
def chains that doesn't use the parameter), the subsequent patch
that adds the cache makes it possible to reduce it to just trees
(when the function is called in a GIMPLE pass to cache each
pointer assignment).

Martin

gcc-92936.diff


OK.  I didn't necessarily expect that the recursion is causing problems
in our code base or in the testsuite.  It's not terribly unusual for
tests which cause these kinds of problems to be too big to include in
the testsuite.  It's also the case that these problems typically aren't
reported until after a release gets out into the wild and someone tries
there machine generated torturous code on the latest bits :-)


What I meant was that the recursion in compute_objsize has been
there in various forms since 

Re: [00/23] Make fwprop use an on-the-side RTL SSA representation

2020-11-30 Thread Michael Matz
Hello,

On Mon, 30 Nov 2020, Jeff Law wrote:

> >> So, then let's start with one of 
> >> the prime examples of SSA deconstruction problems, the lost swap, and how 
> >> it comes to be: we start with a swap:
> >>
> >>   x = ..., y = ...
> >>   if (cond)
> >> tmp=x, x=y, y=tmp
> >>
> >> (1) into SSA:
> >>
> >>   x0 = ..., y0 = ...
> >>   if (cond)
> >> tmp = x0, x1=y0, y1=tmp;
> >>   x2 = PHI(x0,x1),  y2 = PHI(y0,y1)
> >>
> >> (2) copy-prop:
> >>
> >>   x0 = ..., y0 = ...
> >>   if (cond)
> >> ;
> >>   x2 = PHI(x0,y0),  y2 = PHI(y0,x0)
> > So the point is that this isn't what the RTL would look like even
> > when using RTL SSA.  Putting y0 in x2 PHI and x0 in the y2 PHI is
> > representationally invalid.
> >
> > Like I say, this isn't a “native” SSA form: it's just using SSA
> > constructs to represent dataflow in normal RTL.
> It appears that the PHI arguments have to be different instances of the
> result.  So the case above can't happen, which helps, but I'm not sure
> it's necessarily sufficient to avoid all the problems in this space.
> IIRC you can get into a similar scenario by transformations that result
> in overlapping lifetimes for different instances of the same object. 
> They didn't necessarily overlap when the SSA form was created, but may
> after things like CSE or copy propagation.

I think the reasoning why this can't (or should not) happen is the 
following: if different instances of the same objects (say, one before, 
one after a modification) exist, they must necessarily be stored in 
different pseudos (otherwise the RTL transformation itself was already 
invalid), and that causes them to be invalid operands of the same PHI 
node.  Ala:

input:

   regA =  /1
   use1(regA)  /2
   regA += ... /3
   use2(regA)  /4

let's try creating different instances of regA (from point 2 and 4) that 
overlap, e.g. by swapping insns 2 and 3.  We _have_ to rename regA from 
insn 3 into a new pseudo, otherwise the uses of 2 and 4 can't be 
differentiated anymore, so:

   regA  =  /1
   regA' = regA
   regA' += /3'
   use1(regA)   /2
   use2(regA')  /4'

So if Richards model constrains the pseudo PHI nodes such that regA and 
regA' can't be operands of one, that might solve the issue, as both the 
lost copy and the swap problem need overlaps of different values to occur.

> The fact that passes don't directly manipulate the PHIs definitely helps
> as well.  But I've still got some reading to do in this space to refresh
> my memory of the issues.

AFAIU Richards approach is more comparable to factored def-use chains than 
to real SSA, which might indeed have no issues, though I then think the 
problem moves into keeping _those_ consistent with the real instruction 
stream as it changes.


Ciao,
Michael.


Re: [PATCH] libstdc++: Add C++ runtime support for new 128-bit long double format

2020-11-30 Thread Segher Boessenkool
Hi!

Thank you for all this.

On Wed, Nov 11, 2020 at 09:50:01PM +, Jonathan Wakely wrote:
> This adds support for the new __ieee128 long double format on
> powerpc64le targets.

> * testsuite/27_numerics/complex/abi_tag.cc: Add u9___ieee128 to
> regex matching expected symbols.

One less underscore.

> libstdc++-v3/ChangeLog:
> 
> * Makefile.in:
> * config.h.in:

(etc.)  Something missing here?

All the actual patch looks fine to me, but that doesn't say much :-)


Segher


Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Qing Zhao via Gcc-patches
On Nov 30, 2020, at 11:18 AM, Martin Sebor  wrote:
 Does gcc provide an iterator to traverse all the local variables that 
 are declared in the current routine?
 
 If not, what’s the best way to traverse the local variables?
>>> 
>>> Depends on what for.  There's the source level view you get by walking
>>> BLOCK_VARS of the
>>> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
>>> there's SSA names
>>> (FOR_EACH_SSA_NAME).
>> 
>> I am planing to add a new phase immediately after 
>> “pass_late_warn_uninitialized” to initialize all auto-variables that are
>> not explicitly initialized in the declaration, the basic idea is 
>> following:
>> 
>> ** The proposal:
>> 
>> A. add a new GCC option: (same name and meaning as CLANG)
>> -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
>> 
>> B. add a new attribute for variable:
>> __attribute((uninitialized)
>> the marked variable is uninitialized intentionaly for performance 
>> purpose.
>> 
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language".
>> 
>> 
>> ** The implementation:
>> 
>> There are two major requirements for the implementation:
>> 
>> 1. all auto-variables that do not have an explicit initializer should be 
>> initialized to
>> zero by this option.  (Same behavior as CLANG)
>> 
>> 2. keep the current static warning on uninitialized variables untouched.
>> 
>> In order to satisfy 1, we should check whether an auto-variable has 
>> initializer
>> or not;
>> In order to satisfy 2, we should add this new transformation after
>> "pass_late_warn_uninitialized".
>> 
>> So, we should be able to check whether an auto-variable has initializer 
>> or not after “pass_late_warn_uninitialized”,
>> If Not, then insert an initialization for it.
>> 
>> For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
> 
> Yes, but do you want to catch variables promoted to register as well
> or just variables
> on the stack?
 I think both as long as they are source-level auto-variables. Then which 
 one is better?
> 
>> Another issue is, in order to check whether an auto-variable has 
>> initializer, I plan to add a new bit in “decl_common” as:
>>  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
>>  unsigned decl_is_initialized :1;
>> 
>> /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
>> #define DECL_IS_INITIALIZED(NODE) \
>>  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
>> 
>> set this bit when setting DECL_INITIAL for the variables in FE. then 
>> keep it
>> even though DECL_INITIAL might be NULLed.
> 
> For locals it would be more reliable to set this flag during 
> gimplification.
 You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the 
 routine “gimpley_decl_expr” (gimplify.c) as following:
   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
 {
   tree init = DECL_INITIAL (decl);
 ...
   if (init && init != error_mark_node)
 {
   if (!TREE_STATIC (decl))
 {
   DECL_IS_INITIALIZED(decl) = 1;
 }
 Is this enough for all Frontends? Are there other places that I need to 
 maintain this bit?
> 
>> Do you have any comment and suggestions?
> 
> As said above - do you want to cover registers as well as locals?
 All the locals from the source-code point of view should be covered.   
 (From my study so far,  looks like that Clang adds that phase in FE).
 If GCC adds this phase in FE, then the following design requirement
 C. The implementation needs to keep the current static warning on 
 uninitialized
 variables untouched in order to avoid "forking the language”.
 cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
 applied quite late.
 So, we have to add this new phase after “pass_late_warn_uninitialized”.
>  I'd do
> the actual zeroing during RTL expansion instead since otherwise you
> have to figure youself whether a local is actually used (see 
> expand_stack_vars)
 Adding  this new transformation during RTL expansion is okay.  I will 
 check on this in more details to see how to add it to RTL expansion phase.
> 
> Note that optimization will already made have use of "uninitialized" state
> of locals so depending on what the actual goal is here "late" may be too 
> late.
 This is a really good point…
 In order to avoid optimization  to use the “uninitialized” state of 
 locals, we should add the zeroing phase as early as possible (adding it in 
 FE might 

Re: [PATCH] detect allocation/deallocation mismatches in user-defined functions (PR94527)

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/13/20 2:45 PM, Martin Sebor via Gcc-patches wrote:
> Bug 94527 is request from the kernel developers for an attribute
> to indicate that a user-defined function deallocates an object
> allocated by an earlier call to an allocation function.  Their
> goal is to detect misuses of such functions and the pointers or
> objects returned from them.
>
> The recently submitted patches(*) enable the detection of a subset
> of such misuses for standard allocation functions like malloc and
> free, but those are just a small fraction of allocation/deallocation
> functions used in practice, and only rarely used in the kernel
> (mostly in utility programs). The attached patch extends attribute
> malloc to enable this detection also for user-defined functions.
>
> The design extends attribute malloc to accept one or two optional
> arguments: one naming a deallocation function that deallocates
> pointers returned from the malloc-like function, and another to
> denote the position of the pointer argument in the deallocation
> functions parameter list.  Any number of deallocators can be
> associated with any number of allocators.  This makes it possible
> to annotate, for example, all the POSIX  functions that
> open and close FILE streams and detect mismatches between any
> pairs that aren't suitable (in addition to calling free on
> a FILE* returned from fopen, for instance).
>
> An association with an allocator results in adding an internal
> "*dealloc" attribute to the deallocator so that the former can
> be quickly looked up based on a call to the latter.
>
> Tested on x86_64-linux + Glibc & Binutils/GDB (no instances
> of the new warnings).
>
> Martin
>
> [*] Prerequisite patch
> add -Wmismatched-new-delete to middle end (PR 90629)
> https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557987.html
>
> PS In pr94527 Jonathan notes that failing to properly match pairs
> of calls isn't limited to APIs that return pointers and applies
> to other kinds of "handles" including integers (e.g., the POSIX
> open/close APIs), and a detection of such mismatches would be
> helpful as well.  David submitted a prototype of this for
> the analyzer here:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/44.html
> I chose not to implement nonpointer detection for some of the same
> reasons as mentioned in comment #8 on the bug (and also because
> there's no support for it in the machinery I use).  I also didn't
> use the same attribute as David, in part because I think it's better
> to provide separate attributes for pointer APIs and for others
> (integers), and in part also because the deallocated_by attribute
> design as is cannot accommodate my goal of supporting app standard
> functions (including the  freopen which "deallocates"
> the third argument).
>
> gcc-94527.diff
>
> PR middle-end/94527 - Add an __attribute__ that marks a function as freeing 
> an object
>
> gcc/ChangeLog:
>
>   PR middle-end/94527
>   * builtins.c (gimple_call_alloc_p): Handle user-defined functions.
>   (fndecl_alloc_p): New helper.
>   (call_dealloc_argno): New helper.
>   (gimple_call_dealloc_p): Call it.
>   (call_dealloc_p): Same.
>   (matching_alloc_calls_p): Handle user-defined functions.
>   (maybe_emit_free_warning): Same.
>   * doc/extend.texi (attribute malloc): Update.
>   * doc/invoke.texi (-Wmismatched-dealloc): Document new option.
>
> gcc/c-family/ChangeLog:
>
>   PR middle-end/94527
>   * c-attribs.c (handle_dealloc_attribute): New function.
>   (handle_malloc_attribute): Handle argument forms of attribute.
>   * c.opt (-Wmismatched-dealloc): New option.
>   (-Wmismatched-new-delete): Update description.
>
> gcc/testsuite/ChangeLog:
>
>   PR middle-end/94527
>   * g++.dg/warn/Wmismatched-dealloc-2.C: New test.
>   * g++.dg/warn/Wmismatched-dealloc.C: New test.
>   * gcc.dg/Wmismatched-dealloc.c: New test.
>   * gcc.dg/attr-malloc.c: New test.
OK once prereq is wrapped up.

I realize the dealloc attribute is currently internal only to make
lookups quick.  Any thoughts on whether or not we might want to make it
a user visible attribute at some point? 


Jeff



Re: [PATCH] Unbreak build with --disable-analyzer

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 3:39 PM, David Malcolm wrote:
> On Mon, 2020-11-30 at 16:19 -0500, David Malcolm wrote:
>> I broke the build with --disable-analyzer with
>> g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:
>>
>> ../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual
>> unsigned int {anonymous}::pass_analyzer::execute(function*)':
>> ../../src/gcc/analyzer/analyzer-pass.cc:86:3: error:
>> 'sorry_no_analyzer' was not declared in this scope
>>86 |   sorry_no_analyzer ();
>>   |   ^
>>
>> Fixed by including the relevant header file.
>>
>> Successfully built stage 1 compiler with and without --disable-
>> analyzer;
>> full bootstrap in progress; I'll commit it if/when that succeeds.
> The full bootstrap succeeded, so I've pushed this.
Thanks.  I'll un-pause the tester and resubmit the failed builds ;-)

jeff



Re: [PATCH] Unbreak build with --disable-analyzer

2020-11-30 Thread David Malcolm via Gcc-patches
On Mon, 2020-11-30 at 16:19 -0500, David Malcolm wrote:
> I broke the build with --disable-analyzer with
> g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:
> 
> ../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual
> unsigned int {anonymous}::pass_analyzer::execute(function*)':
> ../../src/gcc/analyzer/analyzer-pass.cc:86:3: error:
> 'sorry_no_analyzer' was not declared in this scope
>86 |   sorry_no_analyzer ();
>   |   ^
> 
> Fixed by including the relevant header file.
> 
> Successfully built stage 1 compiler with and without --disable-
> analyzer;
> full bootstrap in progress; I'll commit it if/when that succeeds.

The full bootstrap succeeded, so I've pushed this.

Dave



[PATCH][GCC] aarch64: Add +flagm to -march

2020-11-30 Thread Przemyslaw Wirkus via Gcc-patches
New +flagm (Condition flag manipulation from Armv8.4-A) feature option for
-march command line option.

Please note that FLAGM stays an Armv8.4-A feature but now can be
assigned to other architectures or CPUs.

OK for master?

gcc/ChangeLog:

* config/aarch64/aarch64-option-extensions.def
(AARCH64_OPT_EXTENSION): New +flagm option in -march for AArch64.
* config/aarch64/aarch64.h (AARCH64_FL_FLAGM): Add new flagm extension 
bit
mask.
(AARCH64_FL_FOR_ARCH8_4): Add flagm to Armv8.4-A.


rb13807.patch
Description: rb13807.patch


Re: [C++ patch] Re: Free more of CFG in release_function_body

2020-11-30 Thread Jason Merrill via Gcc-patches

On 11/27/20 8:26 AM, Jan Hubicka wrote:

On Wed, Nov 25, 2020 at 3:11 PM Jan Hubicka  wrote:



On Tue, 24 Nov 2020, Jan Hubicka wrote:


Hi,
at the end of processing function body we loop over basic blocks and
free all edges while we do not free the rest.  I think this is leftover
from time eges was not garbage collected and we was not using ggc_free.
It makes more sense to free all associated structures (which is
importnat for WPA memory footprint).

Bootstrapped/regtested x86_64-linux, OK?


OK.


Unforutnately the patch does not surive LTO bootstrap.  The problem is
that we keep DECL_INITIAL that points to blocks and blocks points to
var_decls and these points to SSA_NAMES that points to statements and
those points to basic blocks.


VAR_DECLs point to SSA_NAMEs?  It's the other way around.  We for sure
free SSA_NAMEs (well, maybe not explicitely with ggc_free).


I am going to debug this more carefully now.  I think it was VAR_DECL
with variadic type pointing to SSA_NAME.  Should be easy to reduct with
gcac compiler.


Hi,
it turns out that the pointers to statements leaks through saved scopes
in C++ FE.  Scopes seems to point to internal blocks of functions even
after we finish their compiling.

This patch adds code to free pointers.  I tried to clear saved_blocks
but it breaks since C++ finalization uses them, but it does not look
into previous class levels.

Patch lto-bootstraps/regtestes x86_64-linux with all languages. OK?

Honza

* cfg.c (free_block): Call ggc_free on BB itself.

* cp-tre.eh (cp_tree_c_finish_parsing): Declare.
* semantics.c (finish_translation_unit): Call finish_parsing
* tree.c (cp_tree_c_finish_parsing): New function.
diff --git a/gcc/cfg.c b/gcc/cfg.c
index 529b6ed2105..e8bd1456c9f 100644
--- a/gcc/cfg.c
+++ b/gcc/cfg.c
@@ -102,8 +102,7 @@ free_block (basic_block bb)
 bb->succs = NULL;
 vec_free (bb->preds);
 bb->preds = NULL;
-   /* Do not free BB itself yet since we leak pointers to dead statements
-  that points to dead basic blocks.  */
+   ggc_free (bb);
  }
  
  /* Free the memory associated with the CFG in FN.  */

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 021de76e142..665d171d9b0 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7986,6 +7986,8 @@ struct uid_sensitive_constexpr_evaluation_checker
bool evaluation_restricted_p () const;
  };
  
+void cp_tree_c_finish_parsing ();

+
  /* In cp-ubsan.c */
  extern void cp_ubsan_maybe_instrument_member_call (tree);
  extern void cp_ubsan_instrument_member_accesses (tree *);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 5ff70ff4844..e9d17c21985 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3094,6 +3094,7 @@ finish_translation_unit (void)
   "%<#pragma omp end declare target%>");
scope_chain->omp_declare_target_attribute = 0;
  }
+  cp_tree_c_finish_parsing ();


This is too soon for this call; it should be from c_parse_final_cleanups 
by the call to fini_constexpr, so that it follows template instantiation 
at EOF.



  }
  
  /* Finish a template type parameter, specified as AGGR IDENTIFIER.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 28e591086b3..e63d383c0a3 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5844,6 +5844,19 @@ maybe_warn_zero_as_null_pointer_constant (tree expr, 
location_t loc)
return false;
  }
  
+/* Release memory we no longer need after parsing.  */
+void
+cp_tree_c_finish_parsing ()
+{
+  saved_scope *chain = scope_chain;
+  while (chain)
+{
+  chain->x_previous_class_level = NULL;
+  chain = chain->prev;
+}


This can just be invalidate_class_lookup_cache ().  scope_chain->prev 
will always be NULL at this point.


OK with those changes.


+  deleted_copy_types = NULL;
+}
+
  #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
  /* Complain that some language-specific thing hanging off a tree
 node has been accessed improperly.  */




Re: [PATCH 3/3] Improve efficiency of copying section from another tree

2020-11-30 Thread Jeff Law via Gcc-patches


On 11/30/20 5:16 AM, Martin Liška wrote:
> On 11/13/19 7:29 AM, Strager Neds wrote:
>> -/* Worker for set_section.  */
>> +void
>> +symtab_node::set_section_for_node (const symtab_node )
>> +{
>> +  if (x_section == other.x_section)
>> +    return;
>> +  if (get_section () && other.get_section ())
>> +    gcc_checking_assert (strcmp (get_section (), other.get_section
>> ()) != 0);
>> +  release_section_hash_entry (x_section);
>> +  if (other.x_section)
>> +    x_section = retain_section_hash_entry (other.x_section);
>> +  else
>> +    x_section = NULL;
>> +}
>> +
>> +/* Workers for set_section.  */
>>
>>   bool
>> -symtab_node::set_section (symtab_node *n, void *s)
>> +symtab_node::set_section_from_string (symtab_node *n, void *s)
>>   {
>>     n->set_section_for_node ((char *)s);
>>     return false;
>>   }
>>
>> +bool
>> +symtab_node::set_section_from_node (symtab_node *n, void *o)
>> +{
>> +  const symtab_node  = *static_cast (o);
>> +  n->set_section_for_node (other);
>> +  return false;
>> +}
>> +
>>   /* Set section of symbol and its aliases.  */
>
> Hello.
>
> Apparently, the patch caused the following regression:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98057
>
> I've got a fix for it, but I would appreciate function comments
> for the
>
> void
> symtab_node::set_section_for_node (const symtab_node )
>
> and
> bool
> symtab_node::set_section_from_node (symtab_node *n, void *o)
>
> Can you please add it?
I'll take care of it with the attached patch.

Jeff

commit dccae0f42e9e052b7721e805858d10d3ec345685
Author: Jeff Law 
Date:   Mon Nov 30 15:21:38 2020 -0700

Add function comments for recently added member functions.

gcc/
* symtab.c (set_section_for_node): Add function comment.
(set_section_from_node): Likewise.

diff --git a/gcc/symtab.c b/gcc/symtab.c
index 393d6b07870..fd7d553c112 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -1668,6 +1668,10 @@ symtab_node::set_section_for_node (const char *section)
 }
 }
 
+/* Set the section of node THIS to be the same as the section
+   of node OTHER.  Keep reference counts of the sections
+   up-to-date as needed.  */
+
 void
 symtab_node::set_section_for_node (const symtab_node )
 {
@@ -1691,6 +1695,9 @@ symtab_node::set_section_from_string (symtab_node *n, 
void *s)
   return false;
 }
 
+/* Set the section of node N to be the same as the section
+   of node O.  */
+
 bool
 symtab_node::set_section_from_node (symtab_node *n, void *o)
 {


Re: [PATCH] introduce --param max-object-size

2020-11-30 Thread Martin Sebor via Gcc-patches

On 11/30/20 1:29 PM, Jeff Law wrote:



On 11/17/20 7:09 PM, Martin Sebor wrote:

On 11/16/20 4:54 PM, Jeff Law wrote:


On 11/16/20 2:04 AM, Richard Biener via Gcc-patches wrote:

On Sun, Nov 15, 2020 at 1:46 AM Martin Sebor via Gcc-patches
 wrote:

GCC considers PTRDIFF_MAX - 1 to be the size of the largest object
so that the difference between a pointer to the byte just past its
end and the first one is no more than PTRDIFF_MAX.  This is too
liberal in LP64 on most systems because the size of the address
space is constrained to much less than that, both by the width
of the address bus for physical memory and by the practical
limitations of disk sizes for swap files.

Shouldn't this be a target hook like MAX_OFILE_ALIGNMENT then?


I think one could argue either way.  Yes, the absolutes are a function
of the underlying hardware and it can change over the lifetime of a
processor family which likey differs from MAX_OFILE_ALIGNMENT.


A PARAM gives the developer  a way to specify the limit which is more
flexible.


What I'm not really not sure of is whether is really matters in practice
for end users.


I'd like to do two things with this change: 1) make it easier
(and encourage users) to detect as early as possible more bugs
due to excessive sizes in various function calls (malloc, memcpy,
etc.), and 2) verify that GCC code uses the limit consistently
and correctly.

I envision the first would appeal to security-minded distros
and other organizations that use GCC as their system compiler.
For those, a target hook would be more convenient than a --param.
But I also expect individual projects wanting to impose stricter
limits than distros select.  For those, a --param is the only
choice (aside from individual -Wlarger-than- options(*)).

With this in mind, I think providing both a target hook and
a --param has the best chance of achieving these goals.

The attached patch does that.

Martin

[*] To enforce more realistic object size limits than PTRDIFF_MAX,
GCC users today have to set no fewer than five (or six if we count
-Wstack-usage) options: -Walloca-larger-than,
-Walloc-size-larger-than, -Wframe-larger-than, -Wlarger-than, and
-Wvla-larger-than.  The limits are all independent of one another.
I expect providing a single configurable baseline value for all
these options to use and refine to be helpful to these users.

gcc-max-objsize.diff


The more I think about this, the more I think it's not really useful in
practice.

I don't see distros using this flag as there's likely no good values a
distro could use that would likely catch bogus code without needlessly
flagging valid code.


Red Hat documents 128TB of maximum x86_64 per-process virtual address
space:

  https://access.redhat.com/articles/rhel-limits

My understanding is that no x86_64 implementation exists that supports
objects larger than 2^48 bytes.  AFAIK, other 64-bit architectures and
operating system have similar limits.  The Red Hat page mentions limits
for all our supported architectures.



I don't see individual projects using this code either -- for the most
part I would not expect a project developer to be able to accurately
predict the maximum size of allocations they potentially perform and
then bake that into their build system.  There are exceptions (kernel &
embedded systems come immediately to mind).


They can refer to documentation like the RHEL link above to figure
that out.  But even with 64-bit addresses, the size of the virtual
address space is limited by the amount of physical memory and
the size of the swap file (limited by the size of the disk), and
for practical purposes, by the transfer rate of the disk.  So with
some math, those who care about these things can easily come up
with a more realistic limit for their application than PTRDIFF_MAX.



And finally, I'm really not a fan of --params for end-user needs.  Those
feel much more like options that we as GCC developers use to help
ourselves rather than something we encourage others to use.


I don't insist on it to be a parameter.  Whatever other knob will
work as well.  I just thought this is what parameters were for(*).

Will you approve the patch with the parameter changed to an option?
Say -fmax-object-size, or would I be wasting my time?

Martin

[*] Options intended for GCC developers are documented in section
named GCC Developer Options.  --params are documented in section
named Options That Control Optimization.  If --params are mainly
meant for GCC developers then they're in the wrong section.


Re: [PATCH] libstdc++: Add C++ runtime support for new 128-bit long double format

2020-11-30 Thread Michael Meissner via Gcc-patches
Jonathan, could you send a fresh set of patches (or at least replacements)?  I
tried installing the patches on a master branch I checked out this morning, and
I got two rejects:

--- libstdc++-v3/testsuite/util/testsuite_abi.cc
+++ libstdc++-v3/testsuite/util/testsuite_abi.cc
@@ -207,6 +207,7 @@
   known_versions.push_back("GLIBCXX_3.4.24");
   known_versions.push_back("GLIBCXX_3.4.25");
   known_versions.push_back("GLIBCXX_3.4.26");
+  known_versions.push_back("GLIBCXX_IEEE128_3.4.26");
   known_versions.push_back("CXXABI_1.3");
   known_versions.push_back("CXXABI_LDBL_1.3");
   known_versions.push_back("CXXABI_1.3.1");
@@ -220,6 +221,8 @@
   known_versions.push_back("CXXABI_1.3.9");
   known_versions.push_back("CXXABI_1.3.10");
   known_versions.push_back("CXXABI_1.3.11");
+  known_versions.push_back("CXXABI_1.3.12");
+  known_versions.push_back("CXXABI_IEEE128_1.3.12");
   known_versions.push_back("CXXABI_TM_1");
   known_versions.push_back("CXXABI_FLOAT128");
 }
@@ -238,7 +241,7 @@
 
   // Check that added symbols are added in the latest pre-release version.
   bool latestp = (test.version_name == "GLIBCXX_3.4.26"
-|| test.version_name == "CXXABI_1.3.11"
+|| test.version_name == "CXXABI_1.3.12"
 || test.version_name == "CXXABI_FLOAT128"
 || test.version_name == "CXXABI_TM_1");
   if (added && !latestp)
--- libstdc++-v3/include/bits/locale_facets.h
+++ libstdc++-v3/include/bits/locale_facets.h
@@ -51,16 +51,19 @@
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-  // NB: Don't instantiate required wchar_t facets if no wchar_t support.
-#ifdef _GLIBCXX_USE_WCHAR_T
-# define  _GLIBCXX_NUM_FACETS 28
-# define  _GLIBCXX_NUM_CXX11_FACETS 16
-#else
-# define  _GLIBCXX_NUM_FACETS 14
-# define  _GLIBCXX_NUM_CXX11_FACETS 8
-#endif
+// Number of standard facets (for narrow characters only)
+#define  _GLIBCXX_NUM_FACETS 14
+
+// Number of duplicated facets for cxx11 ABI
+#define  _GLIBCXX_NUM_CXX11_FACETS (_GLIBCXX_USE_DUAL_ABI ? 8 : 0)
+
+// codecvt and codecvt
 #define _GLIBCXX_NUM_UNICODE_FACETS 2
 
+// Facets duplicated for alt128 long double format
+// num_get, num_put, money_get, money_put (+ cxx11 money_get, money_put)
+#define _GLIBCXX_NUM_LBDL_ALT128_FACETS (4 + (_GLIBCXX_USE_DUAL_ABI ? 2 : 0))
+
   // Convert string to numeric value of type _Tp and store results.
   // NB: This is specialized for all required types, there is no
   // generic definition.


I tried to fix it with the following patches, but I get the error when I build
libstdc++:

.../bin/ld: duplicate version tag `CXXABI_1.3.12'

diff --git a/libstdc++-v3/testsuite/util/testsuite_abi.cc 
b/libstdc++-v3/testsuite/util/testsuite_abi.cc
index 33b9ec15935..bbe397ae8a4 100644
--- a/libstdc++-v3/testsuite/util/testsuite_abi.cc
+++ b/libstdc++-v3/testsuite/util/testsuite_abi.cc
@@ -207,6 +207,7 @@ check_version(symbol& test, bool added)
   known_versions.push_back("GLIBCXX_3.4.24");
   known_versions.push_back("GLIBCXX_3.4.25");
   known_versions.push_back("GLIBCXX_3.4.26");
+  known_versions.push_back("GLIBCXX_IEEE128_3.4.26");
   known_versions.push_back("GLIBCXX_3.4.27");
   known_versions.push_back("GLIBCXX_3.4.28");
   known_versions.push_back("GLIBCXX_3.4.29");
@@ -225,6 +226,7 @@ check_version(symbol& test, bool added)
   known_versions.push_back("CXXABI_1.3.10");
   known_versions.push_back("CXXABI_1.3.11");
   known_versions.push_back("CXXABI_1.3.12");
+  known_versions.push_back("CXXABI_IEEE128_1.3.12");
   known_versions.push_back("CXXABI_1.3.13");
   known_versions.push_back("CXXABI_TM_1");
   known_versions.push_back("CXXABI_FLOAT128");
@@ -260,7 +262,17 @@ check_version(symbol& test, bool added)
  && test.demangled_name.find("std::__cxx11::") != 0)
{
  if (test.version_name.find("_LDBL_") == std::string::npos
- && test.version_name.find("_FLOAT128") == std::string::npos)
+ && test.version_name.find("_FLOAT128") == std::string::npos
+ && test.version_name.find("_IEEE128") == std::string::npos)
+   test.version_status = symbol::incompatible;
+   }
+
+  // Check that IEEE128 long double compatibility symbols demangled as
+  // __ieee128 are put into some _LDBL_IEEE version name.
+  // XXX is this right? might not want *everything* for __ieee128 in here.
+  if (added && test.demangled_name.find("__ieee128") != std::string::npos)
+   {
+ if (test.version_name.find("_IEEE128") == std::string::npos)
test.version_status = symbol::incompatible;
}
diff --git a/libstdc++-v3/include/bits/locale_facets.h 
b/libstdc++-v3/include/bits/locale_facets.h
index 3e0ae8776c9..9b262c2d228 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -65,6 +65,10 @@ 

[PATCH] Unbreak build with --disable-analyzer

2020-11-30 Thread David Malcolm via Gcc-patches
I broke the build with --disable-analyzer with
g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:

../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual unsigned 
int {anonymous}::pass_analyzer::execute(function*)':
../../src/gcc/analyzer/analyzer-pass.cc:86:3: error: 'sorry_no_analyzer' was 
not declared in this scope
   86 |   sorry_no_analyzer ();
  |   ^

Fixed by including the relevant header file.

Successfully built stage 1 compiler with and without --disable-analyzer;
full bootstrap in progress; I'll commit it if/when that succeeds.

Spotted by Jeff [CCed]

Sorry about the breakage.

gcc/analyzer/ChangeLog:
* analyzer-pass.cc: Include "analyzer/analyzer.h" for the
declaration of sorry_no_analyzer; include "tree.h" and
"function.h" as these are needed by it.
---
 gcc/analyzer/analyzer-pass.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/analyzer/analyzer-pass.cc b/gcc/analyzer/analyzer-pass.cc
index 1f65bf8b154..333f87b7897 100644
--- a/gcc/analyzer/analyzer-pass.cc
+++ b/gcc/analyzer/analyzer-pass.cc
@@ -25,6 +25,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "diagnostic.h"
 #include "options.h"
+#include "tree.h"
+#include "function.h"
+#include "analyzer/analyzer.h"
 #include "analyzer/engine.h"
 
 namespace {
-- 
2.26.2



Re: [00/23] Make fwprop use an on-the-side RTL SSA representation

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/27/20 9:31 AM, Richard Sandiford via Gcc-patches wrote:
> Michael Matz  writes:
>> Hello,
>>
>> On Thu, 26 Nov 2020, Richard Sandiford via Gcc-patches wrote:
>>
> The aim is only to provide a different view of existing RTL instructions.
> Unlike gimple, and unlike (IIRC) the old RTL SSA project from way back,
> the new framework isn't a “native” SSA representation.  This means that
> all inputs to a phi node for a register R are also definitions of
> register R; no move operation is “hidden” in the phi node.
 Hmm, I'm trying to parse what the last phrase means.  Does it mean that
 the "hidden copy" problem for out-of-ssa is avoided?  And if so, how is
 that maintained over time.  Things like copy-prop will tend to introduce
 those issues even if they didn't originally exist.
>>> Yeah, the phi nodes simply say which definition of register R provides
>>> the value of R on a particular incoming edge.  That definition will
>>> itself be a phi node for R, an artificial definition of R created by DF
>>> (e.g. for incoming function arguments or for EH data registers), or an
>>> actual instruction that sets R.
>>>
>>> In other words, the SSA form is a purely on-the-side thing and the
>>> underlying RTL instructions are maintained in the same way as normal.
>>> The SSA form can be deleted at any time without performing a separate
>>> out-of-ssa step.  In that respect it's different from cfglayout,
>>> for example.
>> Hmm, I don't see how that answers Jeffs question, if I got it correctly.  
>> If I didn't get it correctly let me ask my own version of the question :)
>>
>> (I haven't studied your implementation in detail, if I had maybe answers 
>> to the below would become obvious, sorry if so :) )
>>  
>> So, you're saying that in your implementation the operands of PHIs can be 
>> PHIs and real defs.
> Specifically real defs of the same register (or memory, for memory phis).
>
>> Further you say nothing about any restriction in RTL 
>> instruction moving and/or propagation.
> The RTL SSA form doesn't add any extra restrictions beyond those that apply
> to non-SSA RTL passes.  But it also doesn't take any restrictions away.
> In other words, the changes that RTL SSA passes make to RTL instructions
> are the same as those that non-SSA RTL passes would make.  The SSA form
> is just there to make it easier to process use-def chains (and also
> to process live ranges, to a limited extent).
>
>> So, then let's start with one of 
>> the prime examples of SSA deconstruction problems, the lost swap, and how 
>> it comes to be: we start with a swap:
>>
>>   x = ..., y = ...
>>   if (cond)
>> tmp=x, x=y, y=tmp
>>
>> (1) into SSA:
>>
>>   x0 = ..., y0 = ...
>>   if (cond)
>> tmp = x0, x1=y0, y1=tmp;
>>   x2 = PHI(x0,x1),  y2 = PHI(y0,y1)
>>
>> (2) copy-prop:
>>
>>   x0 = ..., y0 = ...
>>   if (cond)
>> ;
>>   x2 = PHI(x0,y0),  y2 = PHI(y0,x0)
> So the point is that this isn't what the RTL would look like even
> when using RTL SSA.  Putting y0 in x2 PHI and x0 in the y2 PHI is
> representationally invalid.
>
> Like I say, this isn't a “native” SSA form: it's just using SSA
> constructs to represent dataflow in normal RTL.
It appears that the PHI arguments have to be different instances of the
result.  So the case above can't happen, which helps, but I'm not sure
it's necessarily sufficient to avoid all the problems in this space.  
IIRC you can get into a similar scenario by transformations that result
in overlapping lifetimes for different instances of the same object. 
They didn't necessarily overlap when the SSA form was created, but may
after things like CSE or copy propagation.

The fact that passes don't directly manipulate the PHIs definitely helps
as well.  But I've still got some reading to do in this space to refresh
my memory of the issues.

jeff




Re: [PATCH] Objective-C++ : Allow prefix attrs on linkage specs.

2020-11-30 Thread Jason Merrill via Gcc-patches

On 11/27/20 6:08 PM, Iain Sandoe wrote:

Jason Merrill  wrote:


On Mon, Nov 23, 2020 at 8:52 AM Iain Sandoe  wrote:
Jason Merrill  wrote:



(NOTE: likewise,   ^~~ starting indent is below ‘int’ for a fixed spacing
font)

===

I’m inclined to think that the second is more useful,
but have patches for both,
which (or something else) do you prefer?

I agree that the second is preferable, thanks.  But let's not 
underline all of "int" there, just the caret is sufficient.  I'd also 
drop the mention of Obj-C++.


t.C:2:1: warning: attributes are not permitted in this position 
[-Wattributes]

    2 | __attribute__(())
  | ^
t.C:3:11: note: attributes may be inserted here
    3 | extern "C" int foo (void);
  |   ^

(the caret _is_ below the space)

(cool, I got to find out how to make a diagnostic point to the space 
between two tokens)


OK?
thanks
Iain


OK, thanks.


[PATCH] C++ : Adjust warning for misplaced attributes.

This removes the reference to Objective-C++ for the warning that
attributes may not be placed before linkage specifications.  It also
adds a note that they may be placed after that.



gcc/cp/ChangeLog:

* parser.c (cp_parser_declaration): Add a not about where
attributes may be placed.
---
gcc/cp/parser.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7a83bf4a2a7..fe1dffc391f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -13567,9 +13567,12 @@ cp_parser_declaration (cp_parser* parser, tree 
prefix_attrs)

  /* We might have already been here.  */
  if (!c_dialect_objc ())
    {
+ location_t where = get_finish (t2->location);
  warning_at (token1->location, OPT_Wattributes, "attributes are"
- " only permitted in this position for Objective-C++,"
- " ignored");
+ " not permitted in this position");
+ where = linemap_position_for_loc_and_offset (line_table,
+  where, 1);
+ inform (where, "attributes may be inserted here");
  attributes = NULL_TREE;
    }
  token1 = t1;
--
2.24.1






Re: [PING] [PATCH] Avoid atomic for guard acquire when that is expensive

2020-11-30 Thread Jason Merrill via Gcc-patches

On 11/30/20 3:08 PM, Bernd Edlinger wrote:

Hi,

I'd like to ping for this patch:


I reviewed it on the 24th:

https://gcc.gnu.org/pipermail/gcc-patches/2020-November/560118.html



https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559882.html

Thanks
Bernd.

On 11/22/20 9:05 AM, Bernd Edlinger wrote:

Hi,

this avoids the need to use -fno-threadsafe-statics on
arm-none-eabi or working around that problem by supplying
a dummy __sync_synchronize function which might
just lead to silent code failure of the worst kind
(non-reproducable, racy) at runtime, as was pointed out
on previous discussions here.

When the atomic access involves a call to __sync_synchronize
it is better to call __cxa_guard_acquire unconditionally,
since it handles the atomics too, or is a non-threaded
implementation when there is no gthread support for this target.

This fixes also a bug for the ARM EABI big-endian target,
that is, previously the wrong bit was checked.


Regression tested successfully on arm-none-eabi with newlib-3.3.0.

Is it OK for trunk?


Thanks
Bernd.







Re: [PATCH] Remove redundant builtins for avx512f scalar instructions.

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 9:26 AM, Jakub Jelinek wrote:
> On Mon, Nov 30, 2020 at 09:23:15AM -0700, Jeff Law wrote:
>>
>> On 11/12/20 11:21 PM, Hongyu Wang wrote:
>>> Hi
>>>
>>> Thanks for reminding me about this patch. I didn't remove any existing
>>> intrinsics, just remove redundant builtin functions that end-users
>>> would not likely to use.
>>>
>>> Also I'm OK to keep current implementation, in case there might be
>>> someone using the builtin directly.
>> That seems wise -- we can't reasonably predict if users are using those
>> builtins directly. 
>>
>> So if we can clean things up and keep the redundant builtins that seems
>> best.  Or just leave things as-is. 
>>
>> The other possibility would be to deprecate the redundant builtins this
>> release and remove them in gcc-12.  I haven't looked at how difficult
>> that might be, but the idea here would be to give users a warning if
>> they use those builtins directly and enough time to resolve the issue
>> before we remove them.
> In the past we've removed the builtins without any warning, we state all the
> time that the builtins used to implement the intrinsics themselves aren't
> supported, only the intrinsics declared in the headers.
In that case, I don't mind removing those builtins.
jeff



Re: [PATCH v2] rs6000: Disable HTM for Power10 and later

2020-11-30 Thread Segher Boessenkool
Hi!

On Mon, Nov 30, 2020 at 06:36:30PM +0800, Kewen.Lin wrote:
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -51,7 +51,6 @@
>| OPTION_MASK_CRYPTO   \
>| OPTION_MASK_DIRECT_MOVE  \
>| OPTION_MASK_EFFICIENT_UNALIGNED_VSX  \
> -  | OPTION_MASK_HTM  \
>| OPTION_MASK_QUAD_MEMORY  \
>| OPTION_MASK_QUAD_MEMORY_ATOMIC)

(this is in #define ISA_2_7_MASKS_SERVER)

That looks fine.

> -RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | 
> ISA_2_7_MASKS_SERVER)
> -RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | 
> ISA_3_0_MASKS_SERVER)
> +RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
> + | OPTION_MASK_HTM)
> +RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
> + | OPTION_MASK_HTM)

> -RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | 
> ISA_2_7_MASKS_SERVER)
> +RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | 
> ISA_2_7_MASKS_SERVER
> + | OPTION_MASK_HTM)

This, too.

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3807,7 +3807,8 @@ rs6000_option_override_internal (bool global_init_p)
>/* If little-endian, default to -mstrict-align on older processors.
>   Testing for htm matches power8 and later.  */
>if (!BYTES_BIG_ENDIAN
> -  && !(processor_target_table[tune_index].target_enable & 
> OPTION_MASK_HTM))
> +  && !(processor_target_table[tune_index].target_enable
> +& OPTION_MASK_CRYPTO))
>  rs6000_isa_flags |= ~rs6000_isa_flags_explicit & 
> OPTION_MASK_STRICT_ALIGN;

But not this.  Not all ISA 2.07 processors implement the crypto
category.  You could use OPTION_MASK_DIRECT_MOVE, instead?

Okay for trunk with that change (if that works ;-) )  Thanks!


Segher


Re: [patch][rtl-optimization][i386][pr97777] Fix a reg-stack df maintenance bug triggered by zero-call-used-regs pass.

2020-11-30 Thread Qing Zhao via Gcc-patches
Hi, Jeff,

Sorry for the late reply due to thanksgiving long weekend. 

> On Nov 25, 2020, at 1:37 PM, Jeff Law  wrote:
> 
> 
> 
> On 11/19/20 8:59 AM, Qing Zhao via Gcc-patches wrote:
>> Hi, 
>> 
>> PR9 - ICE: in df_refs_verify, at df-scan.c:3991 with -O 
>> -ffinite-math-only -fzero-call-used-regs=all
>> 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=9
>> 
>> Is a bug triggered by the new pass zero-call-used-regs, however, it’s an old 
>> bug in the pass “reg-stack”.
>> This pass does not correctly maintain the df information after 
>> transformation. 
>> 
>> Since the transformation is reg-stack pass is quite complicate, involving 
>> both instruction changes and control
>> Flow changes, I called “df_insn_rescan_all” after the transformation is done.
>> 
>> The patch has been tested with bootstrap with 
>> --enable-checking=yes,rtl,df,extra, no regression. 
>> 
>> Okay for commit?
>> 
>> Qing
>> 
>> From c2573c6c8552b7b4c2eedb0684ce48b5c11436ec Mon Sep 17 00:00:00 2001
>> From: qing zhao 
>> Date: Thu, 19 Nov 2020 16:46:50 +0100
>> Subject: [PATCH] rtl-optimization: Fix data flow maintenance bug in
>> reg-stack.c [pr9]
>> 
>> reg-stack pass does not maintain the data flow information correctly.
>> call df_insn_rescan_all after the transformation is done.
>> 
>>gcc/
>>  PR rtl-optimization/9
>>  * reg-stack.c (rest_of_handle_stack_regs): call
>>  df_insn_rescan_all if reg_to_stack return true.
>> 
>>  gcc/testsuite/
>>  PR rtl-optimization/9
>>  * gcc.target/i386/pr9.c: New test.
> I'd like to see more analysis here.
> 
> ie, precisely what data is out of date and why?

For the simple testing case, what happened is, for the insn #6:

(gdb) call debug_rtx(insn)
(insn 6 26 7 2 (set (reg:XF 8 st [84])
(reg:XF 9 st(1) [85])) "t.c":4:10 134 {*movxf_internal}
 (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
(nil)))

After the following statement in reg-stack.c:
   3080   control_flow_insn_deleted |= subst_stack_regs (insn, 
);

This insn # 6 becomes:
(gdb) call debug_rtx(insn)
(insn 6 26 7 2 (set (reg:XF 8 st)
(reg:XF 8 st)) "t.c":4:10 134 {*movxf_internal}
 (expr_list:REG_EQUAL (const_double:XF 0.0 [0x0.0p+0])
(nil)))

However, there is no any df maintenance routine (for example, df_insn_rescan, 
etc) is called for this changed insn. 

As I checked, the transformation for this pass “stack” is quite complicated. In 
addition to the above register replacement,
New insns might be inserted, and control flow might be changed, but for most of 
the transformations applied in this pass,
There is no corresponding df maintenance routine is added for deferred df 
rescanning. 

Therefore, I called the “df_insn_rescan_all” after the whole transformation is 
done to maintain the df information. 

Another solution is to check all the details of the transformations of this 
pass, and add the df maintenance routine case by
case for each of the transformation.  Since the transformation of the pass 
“stack” is quite extensive and complicate, I feel 
that calling “df_insn_rescan_all” might be a better solution. 

Let me know your suggestions.

thanks.

Qing



> 
> Jeff



Re: [PATCH] handle conditionals in -Wstringop-overflow et al. (PR 92936)

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/29/20 3:27 PM, Martin Sebor wrote:
> On 11/13/20 2:34 PM, Jeff Law wrote:
>>
>> On 11/2/20 7:24 PM, Martin Sebor wrote:
>>> The attached patch extends compute_objsize() to handle conditional
>>> expressions represented either as PHIs or MIN_EXPR and MAX_EXPR.
>>>
>>> To simplify the handling of the -Wstringop-overflow/-overread
>>> warnings the change factors this code out of tree-ssa-strlen.c
>>> and into inform_access() in builtins.c, making it a member of
>>> access_ref.  Besides eliminating a decent amount of code
>>> duplication this also improves the consistency of the warnings.
>>>
>>> Finally, the change introduces a distinction between the definite
>>> kinds of -Wstringop-overflow (and -Wstringop-overread) warnings
>>> and the maybe kind.  The latter are currently only being issued
>>> for function array parameters but I expect to make use of them
>>> more extensively in the future.
>>>
>>> Besides the usual GCC bootstrap/regtest I have tested the change
>>> with Binutils/GDB and Glibc and verified that it doesn't introduce
>>> any false positives.
>>>
>>> Martin
>>>
>>> gcc-92936.diff
>>>
>>> PR middle-end/92936 - missing warning on a past-the-end store to a PHI
>>> PR middle-end/92940 - incorrect offset and size in
>>> -Wstringop-overflow for out-of-bounds store into VLA and two offset
>>> ranges
>>> PR middle-end/89428 - missing -Wstringop-overflow on a PHI with
>>> variable offset
>>>
>>> gcc/ChangeLog:
>>>
>>> PR middle-end/92936
>>> PR middle-end/92940
>>> PR middle-end/89428
>>> * builtins.c (access_ref::access_ref): Initialize member.
>>> (access_ref::phi): New function.
>>> (access_ref::get_ref): New function.
>>> (access_ref::add_offset): Remove duplicate assignment.
>>> (maybe_warn_for_bound): Add "maybe" kind of warning messages.
>>> (warn_for_access): Same.
>>> (inform_access): Rename...
>>> (access_ref::inform_access): ...to this.  Print PHI arguments. 
>>> Format
>>> offset the same as size and simplify.  Improve printing of
>>> allocation
>>> functions and VLAs.
>>> (check_access): Adjust to the above.
>>> (gimple_parm_array_size): Change argument.
>>> (handle_min_max_size): New function.
>>> * builtins.h (struct access_ref): Declare new members.
>>> (gimple_parm_array_size): Change argument.
>>> * tree-ssa-strlen.c (maybe_warn_overflow): Use access_ref and
>>> simplify.
>>> (handle_builtin_memcpy): Correct argument passed to
>>> maybe_warn_overflow.
>>> (handle_builtin_memset): Same.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR middle-end/92936
>>> PR middle-end/92940
>>> PR middle-end/89428
>>> * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected
>>> informational notes.
>>> * gcc.dg/Wstringop-overflow-11.c: Remove xfails.
>>> * gcc.dg/Wstringop-overflow-12.c: Same.
>>> * gcc.dg/Wstringop-overflow-17.c: Adjust text of expected messages.
>>> * gcc.dg/Wstringop-overflow-27.c: Same.  Remove xfails.
>>> * gcc.dg/Wstringop-overflow-28.c: Adjust text of expected messages.
>>> * gcc.dg/Wstringop-overflow-29.c: Same.
>>> * gcc.dg/Wstringop-overflow-37.c: Same.
>>> * gcc.dg/Wstringop-overflow-46.c: Same.
>>> * gcc.dg/Wstringop-overflow-47.c: Same.
>>> * gcc.dg/Wstringop-overflow-54.c: Same.
>>> * gcc.dg/warn-strnlen-no-nul.c: Add expected warning.
>>> * gcc.dg/Wstringop-overflow-58.c: New test.
>>> * gcc.dg/Wstringop-overflow-59.c: New test.
>>> * gcc.dg/Wstringop-overflow-60.c: New test.
>>> * gcc.dg/Wstringop-overflow-61.c: New test.
>>> * gcc.dg/Wstringop-overflow-62.c: New test.
>>
>> So my only significant concern here is the recursive nature and the
>> lack of a limiter for pathological cases.  We certainly run into
>> cases with thousands of PHI arguments and deep chains of PHIs feeding
>> other PHIs.  Can you put in a PARAM to limit the amount of recursion
>> and and PHI arguments you look at?  With that I think this is fine --
>> I consider it unlikely this patch is the root cause of the ICEs I
>> sent you earlier today from the tester since those failures are in
>> the array bounds checking bits.
>
> I've reused the same approach/class as in tree-ssa-strlen.c and
> after adjusting things that need to be adjusted and retesting
> with Binutils/GDB and Glibc committed the attached patch in
> r11-5523.
>
> That said, although the recursion hasn't been a problem (there's
> still code elsewhere that does this sort of traversal of the use-
> def chains that doesn't use the parameter), the subsequent patch
> that adds the cache makes it possible to reduce it to just trees
> (when the function is called in a GIMPLE pass to cache each
> pointer assignment).
>
> Martin
>
> gcc-92936.diff
>
OK.  I didn't necessarily expect that the recursion is causing problems
in our code base or in the testsuite.  It's not terribly unusual for
tests which cause these kinds of problems to be too big to include in
the 

Re: [PATCH] IPA: drop implicit_section again

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 5:48 AM, Martin Liška wrote:
> As mentioned in the PR, since 4656461585bfd0b9 implicit_section
> was not set to false when set_section was called with the argument
> equal to NULL.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
>     PR ipa/98057
>     * symtab.c (symtab_node::set_section_for_node): Drop
>     implicit_section if x_section is NULL.
>
> gcc/testsuite/ChangeLog:
>
>     PR ipa/98057
>     * g++.dg/ipa/pr98057.C: New test.
OK
jeff



Re: [PATCH] VAX: Fix the LTO compiler downgrading code to non-PIC model

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 8:11 AM, Maciej W. Rozycki wrote:
> Fix a testsuite failure:
>
> /tmp/ccL65Mmt.s: Assembler messages:
> /tmp/ccL65Mmt.s:36: Warning: Symbol n used as immediate operand in PIC mode.
> FAIL: gcc.dg/lto/pr55660 c_lto_pr55660_0.o-c_lto_pr55660_1.o link, -O0 -flto 
> -flto-partition=none -fuse-linker-plugin
>
> where non-PIC code is substituted by the LTO compiler at the link stage 
> for what used to be PIC code in the original compilation.  This happens 
> because in the de-facto VAX ELF psABI we rely on code being PIC for GOT 
> support in dynamic executables and arrange that by having `-fPIC' passed 
> to the compiler by default by means of a specs recipe.
>
> That is however canceled where the LTO wrapper is used, by an internal 
> arrangement in the LTO compiler that clears the PIC flag whenever the 
> `-flinker-output=exec' option has been used.  This has been deliberately 
> introduced with commit 1ff9ed6fb282 ("re PR lto/67548 (LTO drops weak 
> binding with "ld -r")")[1]:
>
> "In the log of PR67548 HJ actually pointed out that we do have API at 
> linker plugin side which says what type of output is done.  This is cool 
> because we can also use it to drop -fpic when building static binary. 
> This is common in Firefox, where some objects are built with -fpic and 
> linked to both binaries and libraries."
>
> with this code:
>
> case LTO_LINKER_OUTPUT_EXEC: /* Normal executable */
>   flag_pic = 0;
>   flag_pie = 0;
>   flag_shlib = 0;
>   break;
>
> Consequently code like:
>
> .L6:
>   addl3 -8(%fp),$n,%r0
>   pushl %r0
>   calls $1,foo
>   addl2 %r0,-12(%fp)
>   incl -8(%fp)
> .L5:
>
> is produced by the LTO compiler, where a reference to `n' is used that 
> is invalid in PIC code, because it uses the immediate addressing mode, 
> denoted by the `$' prefix.
>
> For that not to happen we must never pass `-flinker-output=exec' to the 
> LTO compiler unless non-PIC code has been explicitly requested.  Using 
> `-flinker-output=dyn' except for relocatable output would seem the 
> simplest approach, as it does not fiddle with any of the internal code 
> model settings beyond what the command-line options have arranged and 
> therefore lets them remain the same as with the original compilation, 
> but it breaks as well causing PR lto/69866 to retrigger, as that code 
> seems sensitive to `flag_shlib':
>
> lto1: internal compiler error: in add_symbol_to_partition_1, at 
> lto/lto-partition.c:152
> 0x105be1cb add_symbol_to_partition_1
>   .../gcc/lto/lto-partition.c:152
> 0x105be443 add_symbol_to_partition_1
>   .../gcc/lto/lto-partition.c:194
> 0x105be80f add_symbol_to_partition
>   .../gcc/lto/lto-partition.c:270
> 0x105bee6f add_sorted_nodes
>   .../gcc/lto/lto-partition.c:395
> 0x105c0903 lto_balanced_map(int, int)
>   .../gcc/lto/lto-partition.c:815
> 0x105aa91f do_whole_program_analysis
>   .../gcc/lto/lto.c:499
> 0x105aac97 lto_main()
>   .../gcc/lto/lto.c:637
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> lto-wrapper: fatal error: .../gcc/xgcc returned 1 exit status
> compilation terminated.
> .../usr/bin/vax-netbsdelf-ld: error: lto-wrapper failed
> collect2: error: ld returned 1 exit status
> compiler exited with status 1
> FAIL: gcc.dg/lto/pr69866 c_lto_pr69866_0.o-c_lto_pr69866_1.o link, -O0 -flto 
> -fuse-linker-plugin -fno-fat-lto-objects  (internal compiler error)
>
> Substitute `-flinker-output=pie' for `-flinker-output=exec' in the specs 
> then unless `-no-pie' has also been used, preserving the original intent 
> of emitting PIC code by default for executables while keeping the linker 
> arrangement unchanged.  The LTO compiler uses the `cc1' spec, so keep 
> `cc1plus' unmodified.
>
> This makes code like:
>
> .L6:
>   movab n,%r0
>   addl2 -8(%fp),%r0
>   pushl %r0
>   calls $1,foo
>   addl2 %r0,-12(%fp)
>   incl -8(%fp)
> .L5:
>
> be produced instead corresponding to the fragment quoted above, which is 
> valid PIC code as it uses the PC-relative addressing mode denoted by the 
> absence of a prefix to `n' (which can be redirected to GOT as required, 
> by changing the addressing mode to PC-relative indirect in the operand 
> specifier).
>
> Ideally we would instead default to the PIE model for executables, but 
> that triggers a BFD bug where for a change the LTO wrapper is not used:
>
> .../usr/bin/vax-netbsdelf-ld: /tmp/ccV2sWQt.ltrans0.ltrans.o: warning: GOT 
> addend of 3 to `n' does not match previous GOT addend of 0
> FAIL: gcc.dg/lto/pr55660 c_lto_pr55660_0.o-c_lto_pr55660_1.o link, -O2 -flto 
> -flto-partition=1to1 -fno-use-linker-plugin
>
> which is due to assembly code like:
>
> main:
>   .word 0
>   subl2 $4,%sp
>   movab n,%r0
>   movab n+3,%r2
>   clrl %r3
>   movb $98,%r1
> .L4:
>
> and consequently 

Re: [PATCH] introduce --param max-object-size

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/17/20 7:09 PM, Martin Sebor wrote:
> On 11/16/20 4:54 PM, Jeff Law wrote:
>>
>> On 11/16/20 2:04 AM, Richard Biener via Gcc-patches wrote:
>>> On Sun, Nov 15, 2020 at 1:46 AM Martin Sebor via Gcc-patches
>>>  wrote:
 GCC considers PTRDIFF_MAX - 1 to be the size of the largest object
 so that the difference between a pointer to the byte just past its
 end and the first one is no more than PTRDIFF_MAX.  This is too
 liberal in LP64 on most systems because the size of the address
 space is constrained to much less than that, both by the width
 of the address bus for physical memory and by the practical
 limitations of disk sizes for swap files.
>>> Shouldn't this be a target hook like MAX_OFILE_ALIGNMENT then?
>>
>> I think one could argue either way.  Yes, the absolutes are a function
>> of the underlying hardware and it can change over the lifetime of a
>> processor family which likey differs from MAX_OFILE_ALIGNMENT.
>>
>>
>> A PARAM gives the developer  a way to specify the limit which is more
>> flexible.
>>
>>
>> What I'm not really not sure of is whether is really matters in practice
>> for end users.
>
> I'd like to do two things with this change: 1) make it easier
> (and encourage users) to detect as early as possible more bugs
> due to excessive sizes in various function calls (malloc, memcpy,
> etc.), and 2) verify that GCC code uses the limit consistently
> and correctly.
>
> I envision the first would appeal to security-minded distros
> and other organizations that use GCC as their system compiler.
> For those, a target hook would be more convenient than a --param.
> But I also expect individual projects wanting to impose stricter
> limits than distros select.  For those, a --param is the only
> choice (aside from individual -Wlarger-than- options(*)).
>
> With this in mind, I think providing both a target hook and
> a --param has the best chance of achieving these goals.
>
> The attached patch does that.
>
> Martin
>
> [*] To enforce more realistic object size limits than PTRDIFF_MAX,
> GCC users today have to set no fewer than five (or six if we count
> -Wstack-usage) options: -Walloca-larger-than,
> -Walloc-size-larger-than, -Wframe-larger-than, -Wlarger-than, and
> -Wvla-larger-than.  The limits are all independent of one another.
> I expect providing a single configurable baseline value for all
> these options to use and refine to be helpful to these users.
>
> gcc-max-objsize.diff
>
The more I think about this, the more I think it's not really useful in
practice.

I don't see distros using this flag as there's likely no good values a
distro could use that would likely catch bogus code without needlessly
flagging valid code.

I don't see individual projects using this code either -- for the most
part I would not expect a project developer to be able to accurately
predict the maximum size of allocations they potentially perform and
then bake that into their build system.  There are exceptions (kernel &
embedded systems come immediately to mind).

And finally, I'm really not a fan of --params for end-user needs.  Those
feel much more like options that we as GCC developers use to help
ourselves rather than something we encourage others to use.

Jeff




libgo patch committed: Don't define CacheLinePadSize for mips64x

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This libgo patch changes the internal/cpu package to not define
CacheLinePadSize for mips64x.  For libgo the definition always comes
from the generated file cpugen.go.  This fixes GCC PR 98041.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
eafb46ce90c23efd22c61d941face060bb9f11f3
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index eec3d0708e5..45f62b3bec5 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-84506e0f6bf282765856cb5aeb17124222f73042
+2cc5c746ddfbaeb731f10f2232b9a488df12b71e
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/libgo/go/internal/cpu/cpu_mips64x.go 
b/libgo/go/internal/cpu/cpu_mips64x.go
index 0c4794a70ac..af10a5071ea 100644
--- a/libgo/go/internal/cpu/cpu_mips64x.go
+++ b/libgo/go/internal/cpu/cpu_mips64x.go
@@ -6,8 +6,6 @@
 
 package cpu
 
-const CacheLinePadSize = 32
-
 // This is initialized by archauxv and should not be changed after it is
 // initialized.
 var HWCap uint


Go patch committed: Check len and cap for append(s, make(T, I)...)

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This patch to the Go frontend and runtime checks both len and cap when
handling the case of append(s, make(T, I)...).  The overflow checks
done in growslice always reported an error for the capacity argument,
even if it was the length argument that overflowed.  This change lets
the code pass the current issue4085b.go test.  Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
9ebad4b01c22d4c03a3552fd6b0e86c9de0ce6bd
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index e8cf468d8fc..eec3d0708e5 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-be1738f1fff0e817d921ed568791f9b0585a6982
+84506e0f6bf282765856cb5aeb17124222f73042
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index d1546300d0e..23caf61db93 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -8893,8 +8893,8 @@ Builtin_call_expression::flatten_append(Gogo* gogo, 
Named_object* function,
   // We will optimize this to directly zeroing the tail,
   // instead of allocating a new slice then copy.
 
-  // Retrieve the length. Cannot reference s2 as we will remove
-  // the makeslice call.
+  // Retrieve the length and capacity. Cannot reference s2 as
+  // we will remove the makeslice call.
   Expression* len_arg = makecall->args()->at(1);
   len_arg = Expression::make_cast(int_type, len_arg, loc);
   l2tmp = Statement::make_temporary(int_type, len_arg, loc);
@@ -8907,28 +8907,19 @@ Builtin_call_expression::flatten_append(Gogo* gogo, 
Named_object* function,
   inserter->insert(c2tmp);
 
   // Check bad len/cap here.
-  // if len2 < 0 { panicmakeslicelen(); }
+ // checkmakeslice(type, len, cap)
+ // (Note that if len and cap are constants, we won't see a
+ // makeslice call here, as it will be rewritten to a stack
+ // allocated array by Mark_address_taken::expression.)
+ Expression* elem = Expression::make_type_descriptor(element_type,
+ loc);
   len2 = Expression::make_temporary_reference(l2tmp, loc);
-  Expression* zero = Expression::make_integer_ul(0, int_type, loc);
-  Expression* cond = Expression::make_binary(OPERATOR_LT, len2,
- zero, loc);
- Expression* call = Runtime::make_call(Runtime::PANIC_MAKE_SLICE_LEN,
-   loc, 0);
-  cond = Expression::make_conditional(cond, call, zero->copy(), loc);
-  gogo->lower_expression(function, inserter, );
-  gogo->flatten_expression(function, inserter, );
-  Statement* s = Statement::make_statement(cond, false);
-  inserter->insert(s);
-
-  // if cap2 < 0 { panicmakeslicecap(); }
   Expression* cap2 = Expression::make_temporary_reference(c2tmp, loc);
-  cond = Expression::make_binary(OPERATOR_LT, cap2,
- zero->copy(), loc);
- call = Runtime::make_call(Runtime::PANIC_MAKE_SLICE_CAP, loc, 0);
-  cond = Expression::make_conditional(cond, call, zero->copy(), loc);
-  gogo->lower_expression(function, inserter, );
-  gogo->flatten_expression(function, inserter, );
-  s = Statement::make_statement(cond, false);
+ Expression* check = Runtime::make_call(Runtime::CHECK_MAKE_SLICE,
+loc, 3, elem, len2, cap2);
+  gogo->lower_expression(function, inserter, );
+  gogo->flatten_expression(function, inserter, );
+  Statement* s = Statement::make_statement(check, false);
   inserter->insert(s);
 
   // Remove the original makeslice call.
diff --git a/gcc/go/gofrontend/runtime.def b/gcc/go/gofrontend/runtime.def
index b608766cdaf..9a3c6809130 100644
--- a/gcc/go/gofrontend/runtime.def
+++ b/gcc/go/gofrontend/runtime.def
@@ -265,6 +265,11 @@ DEF_GO_RUNTIME(GROWSLICE, "runtime.growslice",
P5(TYPE, POINTER, INT, INT, INT), R1(SLICE))
 
 
+// Check the length and cap passed to make, without making a slice.
+// This is used for apend(s, make([]T, len)...).
+DEF_GO_RUNTIME(CHECK_MAKE_SLICE, "runtime.checkMakeSlice", P3(TYPE, INT, INT),
+  R1(UINTPTR))
+
 // Register roots (global variables) for the garbage collector.
 DEF_GO_RUNTIME(REGISTER_GC_ROOTS, "runtime.registerGCRoots", P1(POINTER), R0())
 
diff --git a/libgo/go/runtime/slice.go b/libgo/go/runtime/slice.go
index 97b26594dad..ddd588ed5b9 100644
--- a/libgo/go/runtime/slice.go
+++ b/libgo/go/runtime/slice.go
@@ -15,6 +15,7 @@ import (
 //go:linkname panicmakeslicelen
 //go:linkname panicmakeslicecap
 //go:linkname makeslice

libgo patch committed: Define SO_RCVTIMEO on 32-bit GNU/Linux

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This libgo patch defines SO_RCVTIMEO on 32-bit GNU/Linux.  It was not
being defined before because it is defined as a conditional expression
that is too complicated for -fdump-go-spec to handle.  This fixes
https://golang.org/issue/42872.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
e848a83f46f15280ad654f05545cc5ec4f5b8e50
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 41246030f13..e8cf468d8fc 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-45461eeba1db1a3b4194dc8ecc331c0e92f5ad4c
+be1738f1fff0e817d921ed568791f9b0585a6982
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/libgo/mksysinfo.sh b/libgo/mksysinfo.sh
index deac5ce8d67..b32a0266b71 100755
--- a/libgo/mksysinfo.sh
+++ b/libgo/mksysinfo.sh
@@ -194,6 +194,7 @@ fi
 
 # Networking constants.
 egrep '^const _(AF|ARPHRD|ETH|IN|SOCK|SOL|SO|IPPROTO|TCP|IP|IPV6)_' 
gen-sysinfo.go |
+  grep -v '_val =' |
   sed -e 's/^\(const \)_\([^= ]*\)\(.*\)$/\1\2 = _\2/' >> ${OUT}
 grep '^const _SOMAXCONN' gen-sysinfo.go |
   sed -e 's/^\(const \)_\(SOMAXCONN[^= ]*\)\(.*\)$/\1\2 = _\2/' \
@@ -213,6 +214,14 @@ for m in SOCK_CLOEXEC SOCK_NONBLOCK; do
   fi
 done
 
+# On 32-bit GNU/Linux the expression for SO_RCVTIMEO is too complicated
+# for -fdump-go-spec.
+if ! grep '^const SO_RCVTIMEO ' ${OUT} >/dev/null 2>&1; then
+  if grep '^const _SO_RCVTIMEO_val' ${OUT} >/dev/null 2>&1; then
+echo 'const SO_RCVTIMEO = _SO_RCVTIMEO_val' >> ${OUT}
+  fi
+fi
+
 # The syscall package requires AF_LOCAL.
 if ! grep '^const AF_LOCAL ' ${OUT} >/dev/null 2>&1; then
   if grep '^const AF_UNIX ' ${OUT} >/dev/null 2>&1; then
diff --git a/libgo/sysinfo.c b/libgo/sysinfo.c
index 7086381a14c..a060ea867a5 100644
--- a/libgo/sysinfo.c
+++ b/libgo/sysinfo.c
@@ -337,6 +337,9 @@ enum {
 #ifdef BIOCVERSION
   BIOCVERSION_val = BIOCVERSION,
 #endif
+#ifdef SO_RCVTIMEO
+  SO_RCVTIMEO_val = SO_RCVTIMEO,
+#endif
 };
 
 // SIOCGIFMTU can't be added in the above enum as it might


Go patch committed: Improve error messages for expected curly brace

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This Go frontend patch improves the error messages for an expected
curly brace.  This is for https://golang.org/issue/17328.  This
requires updating some tests to the current sources in the main repo.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
5ba975e6680cc5f9c2be7ee34b1cacdba3eb3347
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index e2fc0b5560b..41246030f13 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-213abeedc85ed638a878f9457e422897fda3a111
+45461eeba1db1a3b4194dc8ecc331c0e92f5ad4c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/parse.cc b/gcc/go/gofrontend/parse.cc
index 1dac0029feb..c9a5485049f 100644
--- a/gcc/go/gofrontend/parse.cc
+++ b/gcc/go/gofrontend/parse.cc
@@ -4422,7 +4422,7 @@ Parse::if_stat()
 {
   Location semi_loc = this->location();
   if (this->advance_token()->is_op(OPERATOR_LCURLY))
-   go_error_at(semi_loc, "missing %<{%> after if clause");
+   go_error_at(semi_loc, "unexpected semicolon or newline, expecting %<{%> 
after if clause");
   // Otherwise we will get an error when we call this->block
   // below.
 }
@@ -5359,7 +5359,7 @@ Parse::for_stat(Label* label)
 {
   Location semi_loc = this->location();
   if (this->advance_token()->is_op(OPERATOR_LCURLY))
-   go_error_at(semi_loc, "missing %<{%> after for clause");
+   go_error_at(semi_loc, "unexpected semicolon or newline, expecting %<{%> 
after for clause");
   // Otherwise we will get an error when we call this->block
   // below.
 }
@@ -5430,7 +5430,7 @@ Parse::for_clause(Expression** cond, Block** post)
 *cond = NULL;
   else if (this->peek_token()->is_op(OPERATOR_LCURLY))
 {
-  go_error_at(this->location(), "missing %<{%> after for clause");
+  go_error_at(this->location(), "unexpected semicolon or newline, 
expecting %<{%> after for clause");
   *cond = NULL;
   *post = NULL;
   return;
diff --git a/gcc/testsuite/go.test/test/syntax/semi1.go 
b/gcc/testsuite/go.test/test/syntax/semi1.go
index 6e0428121ff..8eed05c1ca1 100644
--- a/gcc/testsuite/go.test/test/syntax/semi1.go
+++ b/gcc/testsuite/go.test/test/syntax/semi1.go
@@ -1,13 +1,13 @@
 // errorcheck
 
-// Copyright 2010 The Go Authors.  All rights reserved.
+// Copyright 2010 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package main
 
 func main() {
-   if x; y // ERROR "missing .*{.* after if clause|undefined"
+   if x; y // ERROR "expected .*{.* after if clause|undefined"
{
z   // GCCGO_ERROR "undefined"
 
diff --git a/gcc/testsuite/go.test/test/syntax/semi3.go 
b/gcc/testsuite/go.test/test/syntax/semi3.go
index ca070d8a577..d064ce631c9 100644
--- a/gcc/testsuite/go.test/test/syntax/semi3.go
+++ b/gcc/testsuite/go.test/test/syntax/semi3.go
@@ -1,13 +1,13 @@
 // errorcheck
 
-// Copyright 2010 The Go Authors.  All rights reserved.
+// Copyright 2010 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package main
 
 func main() {
-   for x; y; z // ERROR "missing .*{.* after for clause|undefined"
+   for x; y; z // ERROR "expected .*{.* after for clause|undefined"
{
z   // GCCGO_ERROR "undefined"
 
diff --git a/gcc/testsuite/go.test/test/syntax/semi4.go 
b/gcc/testsuite/go.test/test/syntax/semi4.go
index 99c2d22561b..08c354751b6 100644
--- a/gcc/testsuite/go.test/test/syntax/semi4.go
+++ b/gcc/testsuite/go.test/test/syntax/semi4.go
@@ -1,6 +1,6 @@
 // errorcheck
 
-// Copyright 2010 The Go Authors.  All rights reserved.
+// Copyright 2010 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -8,7 +8,5 @@ package main
 
 func main() {
for x   // GCCGO_ERROR "undefined"
-   {   // ERROR "missing .*{.* after for clause"
+   {   // ERROR "unexpected {, expecting for loop 
condition|expecting .*{.* after for clause"
z   // GCCGO_ERROR "undefined"
-
-


Go patch committed: Use correct assignment order for type assertions

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This patch to the Go frontend uses the use correct assignment order
for type assertions.  For "a, b := v.(T)" we must set a before b.
This is for https://golang.org/issue/13433.  Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
c7f272e05e1cf8c7d7caefe5ee542845cf4cc7c8
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 698969fc8c8..e2fc0b5560b 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-af683486b4de5503b2b6d9ae974a2ab1eeb92290
+213abeedc85ed638a878f9457e422897fda3a111
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc
index ad898070f6e..25e25364cee 100644
--- a/gcc/go/gofrontend/statements.cc
+++ b/gcc/go/gofrontend/statements.cc
@@ -1985,18 +1985,42 @@ 
Tuple_type_guard_assignment_statement::lower_to_object_type(
NULL, loc);
   b->add_statement(val_temp);
 
-  // ok = CODE(type_descriptor, expr, _temp)
+  // var ok_temp bool
+  Temporary_statement* ok_temp = NULL;
+  if (!this->ok_->is_sink_expression())
+{
+  ok_temp = Statement::make_temporary(this->ok_->type(),
+ NULL, loc);
+  b->add_statement(ok_temp);
+}
+
+  // ok_temp = CODE(type_descriptor, expr, _temp)
   Expression* p1 = Expression::make_type_descriptor(this->type_, loc);
   Expression* ref = Expression::make_temporary_reference(val_temp, loc);
   Expression* p3 = Expression::make_unary(OPERATOR_AND, ref, loc);
   Expression* call = Runtime::make_call(code, loc, 3, p1, this->expr_, p3);
-  Statement* s = Statement::make_assignment(this->ok_, call, loc);
+  Statement* s;
+  if (ok_temp == NULL)
+s = Statement::make_statement(call, true);
+  else
+{
+  Expression* ok_ref = Expression::make_temporary_reference(ok_temp, loc);
+  s = Statement::make_assignment(ok_ref, call, loc);
+}
   b->add_statement(s);
 
   // val = val_temp
   ref = Expression::make_temporary_reference(val_temp, loc);
   s = Statement::make_assignment(this->val_, ref, loc);
   b->add_statement(s);
+
+  // ok = ok_temp
+  if (ok_temp != NULL)
+{
+  ref = Expression::make_temporary_reference(ok_temp, loc);
+  s = Statement::make_assignment(this->ok_, ref, loc);
+  b->add_statement(s);
+}
 }
 
 // Dump the AST representation for a tuple type guard statement.


[PING] [PATCH] Avoid atomic for guard acquire when that is expensive

2020-11-30 Thread Bernd Edlinger
Hi,

I'd like to ping for this patch:

https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559882.html

Thanks
Bernd.

On 11/22/20 9:05 AM, Bernd Edlinger wrote:
> Hi,
> 
> this avoids the need to use -fno-threadsafe-statics on
> arm-none-eabi or working around that problem by supplying
> a dummy __sync_synchronize function which might
> just lead to silent code failure of the worst kind
> (non-reproducable, racy) at runtime, as was pointed out
> on previous discussions here.
> 
> When the atomic access involves a call to __sync_synchronize
> it is better to call __cxa_guard_acquire unconditionally,
> since it handles the atomics too, or is a non-threaded
> implementation when there is no gthread support for this target.
> 
> This fixes also a bug for the ARM EABI big-endian target,
> that is, previously the wrong bit was checked.
> 
> 
> Regression tested successfully on arm-none-eabi with newlib-3.3.0.
> 
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 


Go patch committed: Always use int context for index values

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This patch changes the Go frontend to always use a context that
expects an int type when determining the type of an index value.  This
is for https://golang.org/issue/14844.  This requires updating one of
the tests in the testsuite to the source version.  Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
8d8fea8a57068a0c5f0c1df766679a25f4272481
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index af4d0526b2f..698969fc8c8 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-68b1c7659a6b25d537a4ff3365ab070fa6215b0b
+af683486b4de5503b2b6d9ae974a2ab1eeb92290
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc
index dc7399ebb3a..d1546300d0e 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -12802,24 +12802,11 @@ Array_index_expression::do_determine_type(const 
Type_context*)
   this->array_->determine_type_no_context();
 
   Type_context index_context(Type::lookup_integer_type("int"), false);
-  if (this->start_->is_constant())
-this->start_->determine_type(_context);
-  else
-this->start_->determine_type_no_context();
+  this->start_->determine_type(_context);
   if (this->end_ != NULL)
-{
-  if (this->end_->is_constant())
-this->end_->determine_type(_context);
-  else
-this->end_->determine_type_no_context();
-}
+this->end_->determine_type(_context);
   if (this->cap_ != NULL)
-{
-  if (this->cap_->is_constant())
-this->cap_->determine_type(_context);
-  else
-this->cap_->determine_type_no_context();
-}
+this->cap_->determine_type(_context);
 }
 
 // Check types of an array index.
@@ -13488,17 +13475,9 @@ String_index_expression::do_determine_type(const 
Type_context*)
   this->string_->determine_type_no_context();
 
   Type_context index_context(Type::lookup_integer_type("int"), false);
-  if (this->start_->is_constant())
-this->start_->determine_type(_context);
-  else
-this->start_->determine_type_no_context();
+  this->start_->determine_type(_context);
   if (this->end_ != NULL)
-{
-  if (this->end_->is_constant())
-this->end_->determine_type(_context);
-  else
-this->end_->determine_type_no_context();
-}
+this->end_->determine_type(_context);
 }
 
 // Check types of a string index.
diff --git a/gcc/testsuite/go.test/test/shift1.go 
b/gcc/testsuite/go.test/test/shift1.go
index 04f5321b73f..d6a6c38839f 100644
--- a/gcc/testsuite/go.test/test/shift1.go
+++ b/gcc/testsuite/go.test/test/shift1.go
@@ -1,6 +1,6 @@
 // errorcheck
 
-// Copyright 2011 The Go Authors.  All rights reserved.
+// Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -18,13 +18,13 @@ func h(x float64) int { return 0 }
 var (
s uint= 33
u = 1.0 << s // ERROR "invalid operation|shift of non-integer 
operand"
-   v float32 = 1 << s   // ERROR "invalid" "as type float32"
+   v float32 = 1 << s   // ERROR "invalid"
 )
 
 // non-constant shift expressions
 var (
-   e1   = g(2.0 << s) // ERROR "invalid|shift of non-integer operand" 
"as type interface"
-   f1   = h(2 << s)   // ERROR "invalid" "as type float64"
+   e1   = g(2.0 << s) // ERROR "invalid|shift of non-integer operand"
+   f1   = h(2 << s)   // ERROR "invalid"
g1 int64 = 1.1 << s// ERROR "truncated"
 )
 
@@ -66,8 +66,15 @@ func _() {
u2 = 1<

Go patch committed: Better error messages for missing interface method

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This patch to the Go frontend generates better error messages for a
missing interface method.  This is for https://golang.org/issue/10700.
This requires updating one test to the one in the source repo.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
38f10841817a9fee28ee97c7115b6f4d24f5245d
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index c14a10f1265..af4d0526b2f 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-534fb907c821b052dc430330708d7fa555b91fe3
+68b1c7659a6b25d537a4ff3365ab070fa6215b0b
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/types.cc b/gcc/go/gofrontend/types.cc
index 286ecc16366..23d1647c1fa 100644
--- a/gcc/go/gofrontend/types.cc
+++ b/gcc/go/gofrontend/types.cc
@@ -12052,6 +12052,15 @@ Type::bind_field_or_method(Gogo* gogo, const Type* 
type, Expression* expr,
ambig2.c_str());
   else if (found_pointer_method)
go_error_at(location, "method requires a pointer receiver");
+  else if (it != NULL && it->is_empty())
+   go_error_at(location,
+   "reference to method %qs in interface with no methods",
+   Gogo::message_name(name).c_str());
+  else if (it == NULL && type->deref()->interface_type() != NULL)
+   go_error_at(location,
+   ("reference to method %qs in type that is "
+"pointer to interface, not interface"),
+   Gogo::message_name(name).c_str());
   else if (nt == NULL && st == NULL && it == NULL)
go_error_at(location,
("reference to field %qs in object which "
diff --git a/gcc/testsuite/go.test/test/fixedbugs/bug340.go 
b/gcc/testsuite/go.test/test/fixedbugs/bug340.go
index d996ab64cd4..8c543c98d97 100644
--- a/gcc/testsuite/go.test/test/fixedbugs/bug340.go
+++ b/gcc/testsuite/go.test/test/fixedbugs/bug340.go
@@ -1,6 +1,6 @@
 // errorcheck
 
-// Copyright 2011 The Go Authors.  All rights reserved.
+// Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -12,6 +12,6 @@ func main() {
var x interface{}
switch t := x.(type) {
case 0: // ERROR "type"
-   t.x = 1 // ERROR "type interface \{\}|reference to undefined 
field or method"
+   t.x = 1 // ERROR "type interface \{\}|reference to undefined 
field or method|interface with no methods"
}
 }


Re: [15/23] recog: Add a validate_change_xveclen function

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/13/20 1:19 AM, Richard Sandiford via Gcc-patches wrote:
> A later patch wants to be able to use the validate_change machinery
> to reduce the XVECLEN of a PARALLEL.  This should be more efficient
> than allocating a separate PARALLEL at a possibly distant memory
> location, especially since the new PARALLEL would be garbage rtl if
> the new pattern turns out not to match.  Combine already pulls this
> trick with SUBST_INT.
>
> This patch adds a general helper for doing that.
>
> gcc/
>   * recog.h (validate_change_xveclen): Declare.
>   * recog.c (change_t::old_len): New field.
>   (validate_change_1): Add a new_len parameter.  Conditionally
>   replace the XVECLEN of an rtx, avoiding single-element PARALLELs.
>   (validate_change_xveclen): New function.
>   (cancel_changes): Undo changes made by validate_change_xveclen.
OK.

Presumably it's better to reduce the length of the existing vector,
possibly losing a bit of memory off the end rather than releasing the
existing vector and allocating a new one of the desired length (as you
say it'd likely have no good locality).

And note that when I say lose, I mean lose the ability to use those
slots after we shrink the vector (say if we later wanted to lengthen
it).  The memory doesn't actually leak as it'll be released if we ever
release the vector.


Jeff



Go patch committed: Improve error for import of non-string

2020-11-30 Thread Ian Lance Taylor via Gcc-patches
This patch to the Go frontend improves the error message when using an
import statement with something other than a string.  This requires
updating one of the tests to the current version in the main repo.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
4f32eced9d01c610bd53e8c59b83bcb5402eddb5
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 46959070e85..c14a10f1265 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-16ab9b001c214cf831bc52a7bca5a2d18e9e4f3c
+534fb907c821b052dc430330708d7fa555b91fe3
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/gcc/go/gofrontend/parse.cc b/gcc/go/gofrontend/parse.cc
index aa157e86cb2..1dac0029feb 100644
--- a/gcc/go/gofrontend/parse.cc
+++ b/gcc/go/gofrontend/parse.cc
@@ -5788,7 +5788,7 @@ Parse::import_spec(void*, unsigned int pragmas)
 
   if (!token->is_string())
 {
-  go_error_at(this->location(), "import statement not a string");
+  go_error_at(this->location(), "import path must be a string");
   this->advance_token();
   return;
 }
diff --git a/gcc/testsuite/go.test/test/import5.go 
b/gcc/testsuite/go.test/test/import5.go
index 6480acff92c..8fdc8c37574 100644
--- a/gcc/testsuite/go.test/test/import5.go
+++ b/gcc/testsuite/go.test/test/import5.go
@@ -21,35 +21,7 @@ import _ "go/parser"
 //import "greek/αβ"
 
 // Import paths must be strings.
-import 42// ERROR "import statement"
-import 'a'   // ERROR "import statement"
-import 3.14  // ERROR "import statement"
-import 0.25i // ERROR "import statement"
-
-// Each of these pairs tests both `` vs "" strings
-// and also use of invalid characters spelled out as
-// escape sequences and written directly.
-// For example `"\x00"` tests import "\x00"
-// while "`\x00`" tests import ``.
-import "" // ERROR "import path"
-import `` // ERROR "import path"
-import "\x00" // ERROR "import path"
-import `\x00` // ERROR "import path"
-import "\x7f" // ERROR "import path"
-import `\x7f` // ERROR "import path"
-import "a!"   // ERROR "import path"
-import `a!`   // ERROR "import path"
-import "a b"  // ERROR "import path"
-import `a b`  // ERROR "import path"
-import "a\\b" // ERROR "import path"
-import `a\\b` // ERROR "import path"
-import "\"`a`\""  // ERROR "import path"
-import `\"a\"`// ERROR "import path"
-import "\x80\x80" // ERROR "import path"
-import `\x80\x80` // ERROR "import path"
-import "\xFFFD"   // ERROR "import path"
-import `\xFFFD`   // ERROR "import path"
-
-// Invalid local imports.
-import "/foo"  // ERROR "import path cannot be absolute path"
-import "c:/foo"  // ERROR "import path contains invalid character"
+import 42// ERROR "import path must be a string"
+import 'a'   // ERROR "import path must be a string"
+import 3.14  // ERROR "import path must be a string"
+import 0.25i // ERROR "import path must be a string"


Re: [09/23] Add a cut-down version of std::span (array_slice)

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/13/20 1:15 AM, Richard Sandiford via Gcc-patches wrote:
> A later patch wants to be able to pass around subarray views of an
> existing array.  The standard class to do that is std::span, but it's
> a C++20 thing.  This patch just adds a cut-down version of it.
>
> The intention is just to provide what's currently needed.
>
> gcc/
>   * vec.h (array_slice): New class.
OK.  Obviously we can add more capabilities as we need them.

jeff



Re: [14/23] simplify-rtx: Put simplify routines into a class

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/13/20 1:18 AM, Richard Sandiford via Gcc-patches wrote:
> One of the recurring warts of RTL is that multiplication by a power
> of 2 is represented as a MULT inside a MEM but as an ASHIFT outside
> a MEM.  It would obviously be better if we didn't have this kind of
> context sensitivity, but it would be difficult to remove.
>
> Currently the simplify-rtx.c routines are hard-coded for the
> ASHIFT form.  This means that some callers have to convert the
> ASHIFTs “back” into MULTs after calling the simplify-rtx.c
> routines; see fwprop.c:canonicalize_address for an example.
>
> I think we can relieve some of the pain by wrapping the simplify-rtx.c
> routines in a simple class that tracks whether the expression occurs
> in a MEM or not, so that no post-processing is needed.
>
> An obvious concern is whether passing the “this” pointer around
> will slow things down or bloat the code.  I can't measure any
> increase in compile time after applying the patch.  Sizewise,
> simplify-rtx.o text increases by 2.3% in default-checking builds
> and 4.1% in release-checking builds.
>
> I realise the MULT/ASHIFT thing isn't the most palatable
> reason for doing this, but I think it might be useful for
> other things in future, such as using local nonzero_bits
> hooks/virtual functions instead of the global hooks.
>
> The obvious alternative would be to add a static variable
> and hope that it is always updated correctly.
I think a static would be step in the wrong direction and I'm generally
in favor using classes, particularly when there's contextual information
we want to query.  Shoving things into a class allows us to reason about
when/how that contextual information is used.



>
> Later patches make use of this.
>
> gcc/
>   * rtl.h (simplify_context): New class.
>   (simplify_unary_operation, simplify_binary_operation): Use it.
>   (simplify_ternary_operation, simplify_relational_operation): Likewise.
>   (simplify_subreg, simplify_gen_unary, simplify_gen_binary): Likewise.
>   (simplify_gen_ternary, simplify_gen_relational): Likewise.
>   (simplify_gen_subreg, lowpart_subreg): Likewise.
>   * simplify-rtx.c (simplify_gen_binary): Turn into a member function
>   of simplify_context.
>   (simplify_gen_unary, simplify_gen_ternary, simplify_gen_relational)
>   (simplify_truncation, simplify_unary_operation): Likewise.
>   (simplify_unary_operation_1, simplify_byte_swapping_operation)
>   (simplify_associative_operation, simplify_logical_relational_operation)
>   (simplify_binary_operation, simplify_binary_operation_series)
>   (simplify_distributive_operation, simplify_plus_minus): Likewise.
>   (simplify_relational_operation, simplify_relational_operation_1)
>   (simplify_cond_clz_ctz, simplify_merge_mask): Likewise.
>   (simplify_ternary_operation, simplify_subreg, simplify_gen_subreg)
>   (lowpart_subreg): Likewise.
>   (simplify_binary_operation_1): Likewise.  Test mem_depth when
>   deciding whether the ASHIFT or MULT form is canonical.
>   (simplify_merge_mask): Use simplify_context.
I like it.  OK for the trunk.

jeff



Re: [PATCH] dse: Cope with bigger-than-integer modes [PR98037]

2020-11-30 Thread Richard Biener via Gcc-patches
On November 30, 2020 7:18:37 PM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On November 30, 2020 4:29:41 PM GMT+01:00, Richard Sandiford via
>Gcc-patches  wrote:
>>>dse.c:find_shift_sequence tries to represent a store and load
>>>back as a shift right followed by a truncation.  It therefore
>>>needs to find an integer mode in which to do the shift right.
>>>The loop it uses has the form:
>>>
>>>  FOR_EACH_MODE_FROM (new_mode_iter,
>>>   smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>>>
>>>which implicitly assumes that read_mode has an equivalent integer
>mode.
>>>As shown in the testcase, not all modes have such an integer mode.
>>
>> But if no such mode exists iterating won't help either? So why not
>simply fail when the mode does not exist?
>
>You mean test against the maximum integer mode before the loop,
>but keep the smallest_int_mode_for_size call as-is?  I'm not sure
>that's going to be more efficient in practice, and it seems less
>obviously correct.
>
>Alternatively, we could have a version of smallest_int_mode_for_size
>that returns failure instead of asserting. 

OH, didn't remember this one asserts... 

 But
>smallest_int_mode_for_size
>is itself a FOR_EACH_MODE_* iterator, so it would iterate just as much
>as
>the patch does.
>
>smallest_int_mode_for_size also has some __int20 etc. handling that I
>don't think we want here, and probably avoid by luck.  (We already know
>at this point that any shift value would be nonzero, which probably
>weeds out most of the problematic cases for PSImode targets.)

Ok, I see. 

>find_shift_sequence already iterates over the modes itself and it
>already has custom requirements in terms of when to break and when
>to continue.  It just seems simpler and more obvious for the loop to
>iterate over all the modes directly rather than delegate part of the
>iteration to another function.

Fine. Jeff has already acked the patch. 

Richard. 

>Thanks,
>Richard
>
>
>
>>
>>>This patch just makes the code start from the smallest integer mode
>and
>>>skip modes that are too small.  The loop already breaks at the first
>>>mode wider than word_mode.
>>>
>>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
>>>GCC 10 branch?
>>>
>>>Richard
>>>
>>>
>>>gcc/
>>> PR rtl-optimization/98037
>>> * dse.c (find_shift_sequence): Iterate over all integers and
>>> skip modes that are too small.
>>>
>>>gcc/testsuite/
>>> PR rtl-optimization/98037
>>> * gcc.target/aarch64/sve/acle/general/pr98037.c: New test.
>>>---
>>> gcc/dse.c   | 5
>+++--
>>> gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c | 6
>++
>>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>>create mode 100644
>>>gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>>
>>>diff --git a/gcc/dse.c b/gcc/dse.c
>>>index d65266b5476..651e6e7e71e 100644
>>>--- a/gcc/dse.c
>>>+++ b/gcc/dse.c
>>>@@ -1757,8 +1757,7 @@ find_shift_sequence (poly_int64 access_size,
>>>  the machine.  */
>>> 
>>>   opt_scalar_int_mode new_mode_iter;
>>>-  FOR_EACH_MODE_FROM (new_mode_iter,
>>>-  smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>>>+  FOR_EACH_MODE_IN_CLASS (new_mode_iter, MODE_INT)
>>> {
>>>   rtx target, new_reg, new_lhs;
>>>   rtx_insn *shift_seq, *insn;
>>>@@ -1767,6 +1766,8 @@ find_shift_sequence (poly_int64 access_size,
>>>   new_mode = new_mode_iter.require ();
>>>   if (GET_MODE_BITSIZE (new_mode) > BITS_PER_WORD)
>>> break;
>>>+  if (maybe_lt (GET_MODE_SIZE (new_mode), GET_MODE_SIZE
>>>(read_mode)))
>>>+continue;
>>> 
>>>   /* Try a wider mode if truncating the store mode to NEW_MODE
>>>  requires a real instruction.  */
>>>diff --git
>>>a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>>b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>>new file mode 100644
>>>index 000..b91e940b18e
>>>--- /dev/null
>>>+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>>@@ -0,0 +1,6 @@
>>>+/* { dg-options "-msve-vector-bits=1024 -O3" } */
>>>+
>>>+typedef __SVInt8_t vec __attribute__((arm_sve_vector_bits(1024)));
>>>+struct pair { vec v[2]; };
>>>+void use (struct pair *);
>>>+vec f (struct pair p) { vec v = p.v[1]; use (); return v; }



Re: Add 'dg-note' next to 'dg-optimized', 'dg-missed' (was: [PATCH] dumpfile.c: use prefixes other that 'note: ' for MSG_{OPTIMIZED_LOCATIONS|MISSED_OPTIMIZATION})

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/24/20 2:53 AM, Thomas Schwinge wrote:
> Hi!
>
> Ping.
>
>
> Grüße
>  Thomas
>
>
> On 2020-11-06T10:26:46+0100, I wrote:
>> Hi, again!
>>
>> On 2018-09-25T16:00:14-0400, David Malcolm  wrote:
>>> As noted at Cauldron, dumpfile.c currently emits "note: " for all kinds
>>> of dump message, so that (after filtering) there's no distinction between
>>> MSG_OPTIMIZED_LOCATIONS vs MSG_NOTE vs MSG_MISSED_OPTIMIZATION in the
>>> textual output.
>>>
>>> This patch changes dumpfile.c so that the "note: " varies to show
>>> which MSG_* was used, with the string prefix matching that used for
>>> filtering in -fopt-info, hence e.g.
>>>   directive_unroll_3.f90:24:0: optimized: loop unrolled 7 times
>>> and:
>>>   pr19210-1.c:24:3: missed: missed loop optimization: niters analysis ends 
>>> up with assumptions.
>> (However, 'MSG_NOTE'/'note: ' also still remains used for "general
>> optimization info".)
>>
>>> The patch adds "dg-optimized" and "dg-missed" directives
>>> --- a/gcc/testsuite/lib/gcc-dg.exp
>>> +++ b/gcc/testsuite/lib/gcc-dg.exp
>>> +# Handle output from -fopt-info for MSG_OPTIMIZED_LOCATIONS:
>>> +# a successful optimization.
>>> +
>>> +proc dg-optimized { args } {
>>> +# Make this variable available here and to the saved proc.
>>> +upvar dg-messages dg-messages
>>> +
>>> +process-message saved-dg-error "optimized: " "$args"
>>> +}
>>> +
>>> +# Handle output from -fopt-info for MSG_MISSED_OPTIMIZATION:
>>> +# a missed optimization.
>>> +
>>> +proc dg-missed { args } {
>>> +# Make this variable available here and to the saved proc.
>>> +upvar dg-messages dg-messages
>>> +
>>> +process-message saved-dg-error "missed: " "$args"
>>> +}
>>> +
>> Next to these, I'm proposing to add 'dg-note', see attached "[WIP] Add
>> 'dg-note' next to 'dg-optimized'", which may be used instead of generic
>> 'dg-message' (which in current uses in testcases often doesn't scan for
>> the 'note: ' prefix, by the way).
>>
>> The proposed 'dg-note' has the additional property that "if dg-note is
>> used once, [notes] must *all* be handled explicitly".  The rationale is
>> that either you're not interested in notes at all (default behavior of
>> pruning all notes), but often, when you're interested in one note, you're
>> in fact interested in all notes, and especially interested if
>> *additional* notes appear over time, as GCC evolves.  It seemed somewhat
>> useful, but I'm not insisting on coupling the disabling of notes pruning
>> on 'dg-note' usage, so if anyone feels strongly about that, please speak
>> up.
>>
>> TODO document (also 'dg-optimized', 'dg-missed')
>>
>> TODO 'gcc/testsuite/lib/lto.exp' change necessary/desirable?
>>
>> The latter got added in commit 824721f0905478ebc39e6a295cc8e95c22fa9d17
>> "lto, testsuite: Fix ICE in -Wodr (PR lto/83121)".  David, do you happen
>> to have an opinion on that one?
>>
>>
>> Grüße
>>  Thomas
>>
>>
>> From bb293fff7580025a3b78fc1619d8bf0d8f8b8a1a Mon Sep 17 00:00:00 2001
>> From: Thomas Schwinge 
>> Date: Fri, 6 Nov 2020 09:01:26 +0100
>> Subject: [PATCH] [WIP] Add 'dg-note' next to 'dg-optimized', 'dg-missed'
>>
>> TODO document (also 'dg-optimized', 'dg-missed')
>>
>> TODO 'gcc/testsuite/lib/lto.exp' change necessary/desirable?
I think this is generally fine with the proper doc updates.  I wouldn't
think we'd want/need to treat LTO differently, so I'm not so sure about
that specific hunk.


Jeff



Re: Updating the backend status for h8300 on the wiki

2020-11-30 Thread Jeff Law via Gcc-patches


On 11/30/20 7:37 AM, John Paul Adrian Glaubitz wrote:
> Hi!
>
> Now that h8300 has been converted to use MODE_CC, it might be a good idea
> to update the documentation on the wiki [1] which still lists h8300 as
> being cc0.
Thanks.  I'd been meaning to update that.

I also took care of updating cris and mn103 status WRT cc0.  Attached is
what was pushed to the gcc-wwwdocs trunk.

jeff

commit 50b60e27027f494487deec31cbb0e4133473de34
Author: Jeff Law 
Date:   Mon Nov 30 12:21:24 2020 -0700

Update H8, mn103 and cris state WRT cc0

diff --git a/htdocs/backends.html b/htdocs/backends.html
index 2de21fee..635261ef 100644
--- a/htdocs/backends.html
+++ b/htdocs/backends.html
@@ -77,13 +77,13 @@ avr|L  FIl  cp   g
 bfin   |   F gi
 c6x|   S CB  gi
 cr16   |L  F C   gs
-cris   |   F  B cgi   s
+cris   |   F  B  gi   s
 csky   |  b   ia
 epiphany   | C   gi   s
 fr30   | ??FI B  pb mgs
 frv| ??   B   b   i   s
 gcn|   S C D  qa e
-h8300  |   FI B cgs
+h8300  |   FI B  gs
 i386   | Qq   b   ia
 ia64   |   ? Q   Cqr  b m i
 iq2000 | ???   FICB   b  g  t
@@ -96,7 +96,7 @@ mep|   F Cb  g  t s
 microblaze | CB   i   s
 mips   | Q   CB   qr  ia  s
 mmix   | HM  Q   Cq   i  e
-mn10300| ?? cgi   s
+mn10300| ??  gi   s
 moxie  |   F g  t s
 msp430 |L  FIlb  gs
 nds32  |   F Cia  s


[committed] Remove dead cc0 code from H8 port

2020-11-30 Thread Jeff Law via Gcc-patches
This patch removes a bit of now dead cc0 code from the H8 port.

In particular it drops the "cc" attribute various insns and removes
notice_update_cc.  We keep the attribute, but call it "old_cc" for now. 
The compute_*_cc routines get re-purposed in the optimization patches
(not yet submitted), though I expect those will go through some further
iteration as well and perhaps we'll end up dropping the compute_*_cc
routines as well.  It also removes an accidentally committed/pushed
"save.md" file.



Tested with no regressions on the H8 simulator.  Committed to the trunk.


jeff
commit 5ddb6eca28a2b58656c5d786a4462024ab74618f
Author: Jeff Law 
Date:   Mon Nov 30 12:11:34 2020 -0700

Remove dead cc0 code from H8 port

gcc/
* config/h8300/bitfield.md: Remove "cc" attribute on any
insns where it remained.
* config/h8300/combiner.md: Likewise.
* config/h8300/jumpcall.md: Likewise.
* config/h8300/logical.md: Likewise.
* config/h8300/testcompare.md: Likewise.
* config/h8300/h8300.md (old_cc attr): Renamed from cc attr.
* config/h8300/h8300.c (notice_update_cc): Remove.
(compute_plussi_cc): Change references to CC_* to OLD_CC_.
(compute_logical_op_cc): Likewise.
(shift_one, shift_two): Likewise.
(compute_a_shift_cc): Likewise.
(get_shift_alg): Likewise.
(struct shift_insn): Change type of cc_valid field.
(struct shift_info): Likewise.
* config/h8300/save.md: Remove accidentially created file.

diff --git a/gcc/config/h8300/bitfield.md b/gcc/config/h8300/bitfield.md
index 8fa6fde433a..722c147fe4f 100644
--- a/gcc/config/h8300/bitfield.md
+++ b/gcc/config/h8300/bitfield.md
@@ -69,8 +69,7 @@
 {
   return output_simode_bld (0, operands);
 }
-  [(set_attr "cc" "set_znv,set_znv")
-   (set_attr "length" "8,6")])
+  [(set_attr "length" "8,6")])
 
 ;;
 ;; Inverted loads with a 32bit destination.
@@ -104,8 +103,7 @@
 {
   return output_simode_bld (1, operands);
 }
-  [(set_attr "cc" "set_znv,set_znv")
-   (set_attr "length" "8,6")])
+  [(set_attr "length" "8,6")])
 
 (define_expand "insv"
   [(set (zero_extract:HI (match_operand:HI 0 "general_operand" "")
@@ -312,8 +310,7 @@
 - (1 << INTVAL (operands[3])));
   return "bfld %2,%1,%R0";
 }
-  [(set_attr "cc" "none_0hit")
-   (set_attr "length_table" "bitfield")])
+  [(set_attr "length_table" "bitfield")])
 
 (define_insn_and_split "bfst"
   [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU")
@@ -339,8 +336,7 @@
 - (1 << INTVAL (operands[3])));
   return "bfst %R1,%2,%0";
 }
-  [(set_attr "cc" "none_0hit")
-   (set_attr "length_table" "bitfield")])
+  [(set_attr "length_table" "bitfield")])
 
 ;;(define_expand "cstore4"
 ;;  [(use (match_operator 1 "eqne_operator"
@@ -357,8 +353,7 @@
 ;;  [(set (match_operand:HI 0 "register_operand" "=r")
 ;; (match_operator:HI 1 "eqne_operator" [(cc0) (const_int 0)]))]
 ;;  "TARGET_H8300SX"
-;;  "mulu.w#0,%T0\;b%k1.Lh8BR%=\;inc.w #1,%T0\\n.Lh8BR%=:"
-;;  [(set_attr "cc" "clobber")])
+;;  "mulu.w#0,%T0\;b%k1.Lh8BR%=\;inc.w #1,%T0\\n.Lh8BR%=:")
 
 ;;(define_insn_and_split "*cmpstz"
 ;;  [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,WU")
@@ -379,8 +374,7 @@
 ;; (match_op_dup:QI 2 [(cc0) (const_int 0)]))]
 ;;  {
 ;;operands[5] = gen_rtx_COMPARE (VOIDmode, operands[3], operands[4]);
-;;  }
-;;  [(set_attr "cc" "set_znv,compare")])
+;;  })
 
 ;;(define_insn "*bstz"
 ;;  [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU")
@@ -389,8 +383,7 @@
 ;; (eq:QI (cc0) (const_int 0)))]
 ;;  "TARGET_H8300SX && reload_completed"
 ;;  "bstz  %1,%0"
-;;  [(set_attr "cc" "none_0hit")
-;;   (set_attr "length_table" "unary")])
+;;  [(set_attr "length_table" "unary")])
 
 ;;(define_insn "*bistz"
 ;;  [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU")
@@ -399,8 +392,7 @@
 ;; (ne:QI (cc0) (const_int 0)))]
 ;;  "TARGET_H8300SX && reload_completed"
 ;;  "bistz %1,%0"
-;;  [(set_attr "cc" "none_0hit")
-;;   (set_attr "length_table" "unary")])
+;;  [(set_attr "length_table" "unary")])
 
 ;;(define_insn_and_split "*cmpcondbset"
 ;;  [(set (match_operand:QI 0 "nonimmediate_operand" "=WU,WU")
@@ -420,8 +412,7 @@
 ;;  (match_dup 4)))]
 ;;  {
 ;;operands[6] = gen_rtx_COMPARE (VOIDmode, operands[2], operands[3]);
-;;  }
-;; [(set_attr "cc" "set_znv,compare")])
+;;  })
 
 ;;(define_insn "*condbset"
 ;;  [(set (match_operand:QI 0 "bit_memory_operand" "=WU")
@@ -432,8 +423,7 @@
 ;;  (match_dup 3)))]
 ;;  "TARGET_H8300SX && reload_completed"
 ;;  "bset/%j2\t%V1,%0"
-;;  [(set_attr "cc" "none_0hit")
-;;   (set_attr "length_table" "logicb")])
+;;  [(set_attr "length_table" "logicb")])
 
 ;;(define_insn_and_split "*cmpcondbclr"
 ;;  [(set (match_operand:QI 0 

Re: [PATCH] testsuite: remove LIT annotation and reduce

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 7:02 AM, Martin Liška wrote:
> I noticed the test-case contains LIT annotation and
> it is possible to reduce. I did that with compiler that
> was affected by the PR.
>
> Ready for master?
> Tahnks,
> Martin
>
> gcc/testsuite/ChangeLog:
>
>     * g++.dg/torture/pr93347.C: Reduce and remove LIT keywords.
OK
jeff



Re: [PATCH v3 01/31] PR target/58901: reload: Handle SUBREG of MEM with a mode-dependent address

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/27/20 1:50 PM, Maciej W. Rozycki wrote:
> From: Matt Thomas 
>
> Fix an ICE with the handling of RTL expressions like:
>
> (subreg:QI (mem/c:SI (plus:SI (plus:SI (mult:SI (reg/v:SI 0 %r0 [orig:67 i ] 
> [67])
> (const_int 4 [0x4]))
> (reg/v/f:SI 7 %r7 [orig:59 doacross ] [59]))
> (const_int 40 [0x28])) [1 MEM[(unsigned int *)doacross_63 + 40B + 
> i_106 * 4]+0 S4 A32]) 0)
>
> that causes the compilation of libgomp to fail:
>
> during RTL pass: reload
> .../libgomp/ordered.c: In function 'GOMP_doacross_wait':
> .../libgomp/ordered.c:507:1: internal compiler error: in change_address_1, at 
> emit-rtl.c:2275
>   507 | }
>   | ^
> 0x10a3462b change_address_1
>   .../gcc/emit-rtl.c:2275
> 0x10a353a7 adjust_address_1(rtx_def*, machine_mode, poly_int<1u, long>, int, 
> int, int, poly_int<1u, long>)
>   .../gcc/emit-rtl.c:2409
> 0x10ae2993 alter_subreg(rtx_def**, bool)
>   .../gcc/final.c:3368
> 0x10ae25cf cleanup_subreg_operands(rtx_insn*)
>   .../gcc/final.c:3322
> 0x110922a3 reload(rtx_insn*, int)
>   .../gcc/reload1.c:1232
> 0x10de2bf7 do_reload
>   .../gcc/ira.c:5812
> 0x10de3377 execute
>   .../gcc/ira.c:5986
>
> in a `vax-netbsdelf' build, where an attempt is made to change the mode
> of the contained memory reference to the mode of the containing SUBREG.
> Such RTL expressions are produced by the VAX shift and rotate patterns
> (`ashift', `ashiftrt', `rotate', `rotatert') where the count operand
> always has the QI mode regardless of the mode, either SI or DI, of the
> datum shifted or rotated.
>
> Such a mode change cannot work where the memory reference uses the
> indexed addressing mode, where a multiplier is implied that in the VAX
> ISA depends on the width of the memory access requested and therefore
> changing the machine mode would change the address calculation as well.
>
> Avoid the attempt then by forcing the reload of any SUBREGs containing
> a mode-dependent memory reference, also fixing these regressions:
>
> FAIL: gcc.c-torture/compile/pr46883.c   -Os  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr46883.c   -Os  (test for excess errors)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2  (internal compiler error)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2  (test for excess errors)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -fomit-frame-pointer 
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (internal compiler 
> error)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -fomit-frame-pointer 
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess 
> errors)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -g  (internal compiler error)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -g  (test for excess errors)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fno-use-linker-plugin 
> -flto-partition=none  (internal compiler error)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fno-use-linker-plugin 
> -flto-partition=none  (test for excess errors)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fuse-linker-plugin 
> -fno-fat-lto-objects  (internal compiler error)
> FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fuse-linker-plugin 
> -fno-fat-lto-objects  (test for excess errors)
> FAIL: gcc.dg/20050629-1.c (internal compiler error)
> FAIL: gcc.dg/20050629-1.c (test for excess errors)
> FAIL: c-c++-common/torture/pr53505.c   -Os  (internal compiler error)
> FAIL: c-c++-common/torture/pr53505.c   -Os  (test for excess errors)
> FAIL: gfortran.dg/coarray_failed_images_1.f08   -Os  (internal compiler error)
> FAIL: gfortran.dg/coarray_stopped_images_1.f08   -Os  (internal compiler 
> error)
>
> With test case #0 included it causes a reload with:
>
> (insn 15 14 16 4 (set (reg:SI 31)
> (ashift:SI (const_int 1 [0x1])
> (subreg:QI (reg:SI 30 [ MEM[(int *)s_8(D) + 4B + _5 * 4] ]) 0))) 
> "pr58901-0.c":15:12 94 {ashlsi3}
>  (expr_list:REG_DEAD (reg:SI 30 [ MEM[(int *)s_8(D) + 4B + _5 * 4] ])
> (nil)))
>
> as follows:
>
> Reloads for insn # 15
> Reload 0: reload_in (SI) = (reg:SI 30 [ MEM[(int *)s_8(D) + 4B + _5 * 4] ])
>   ALL_REGS, RELOAD_FOR_INPUT (opnum = 2)
>   reload_in_reg: (reg:SI 30 [ MEM[(int *)s_8(D) + 4B + _5 * 4] ])
>   reload_reg_rtx: (reg:SI 5 %r5)
>
> resulting in:
>
> (insn 37 14 15 4 (set (reg:SI 5 %r5)
> (mem/c:SI (plus:SI (plus:SI (mult:SI (reg/v:SI 1 %r1 [orig:25 i ] 
> [25])
> (const_int 4 [0x4]))
> (reg/v/f:SI 4 %r4 [orig:29 s ] [29]))
> (const_int 4 [0x4])) [1 MEM[(int *)s_8(D) + 4B + _5 * 4]+0 S4 
> A32])) "pr58901-0.c":15:12 12 {movsi_2}
>  (nil))
> (insn 15 37 16 4 (set (reg:SI 2 %r2 [31])
> (ashift:SI (const_int 1 [0x1])
> (reg:QI 5 %r5))) "pr58901-0.c":15:12 94 {ashlsi3}
>  (nil))
>
> and assembly like:
>
> .L3:
>   movl 4(%r4)[%r1],%r5
>   ashl %r5,$1,%r2
>  

Re: [PATCH 25/31] VAX: Fix predicates for widening multiply and multiply-add insns

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 9:02 AM, Maciej W. Rozycki wrote:
> On Fri, 20 Nov 2020, Jeff Law wrote:
>
>> ps.  Yes, I skipped the insv/extv changes.  They're usually a rats nest
>> of special cases.  We'll come back to them.
>  I've thought of actually reducing the number of patterns to the minimum 
> possible by folding the existing ones together, and then getting the 
> alternatives sorted by fine-grained constraints at reload.
That might work, but I've generally found insv/extv to be fairly painful
through the years, more because of limitations on the generic bits
rather than the target bits.

>
>  There is that complication caused by INSV machine instruction preserving 
> condition codes (understandably), so keeping it together with alternative 
> code sequences that do set the codes in a single RTL insn would cause 
> trouble with getting a matching insn for the comparison elimination pass.  
> Or so I think.
Yea.  I think that is (in general) not a well solved problem.  We have
similar issues on the H8 where we have multiple ways to implement
certain operations -- some of which set/clobber flags while others don't
touch them.  Depending on the context one form may be preferable to the
other.  I've punted this problem so far as I strongly suspect the gains
in handling this scenario well are marginal at best.

Jeff



Re: [PATCH] dse: Cope with bigger-than-integer modes [PR98037]

2020-11-30 Thread Richard Sandiford via Gcc-patches
Richard Biener  writes:
> On November 30, 2020 4:29:41 PM GMT+01:00, Richard Sandiford via Gcc-patches 
>  wrote:
>>dse.c:find_shift_sequence tries to represent a store and load
>>back as a shift right followed by a truncation.  It therefore
>>needs to find an integer mode in which to do the shift right.
>>The loop it uses has the form:
>>
>>  FOR_EACH_MODE_FROM (new_mode_iter,
>>smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>>
>>which implicitly assumes that read_mode has an equivalent integer mode.
>>As shown in the testcase, not all modes have such an integer mode.
>
> But if no such mode exists iterating won't help either? So why not simply 
> fail when the mode does not exist?

You mean test against the maximum integer mode before the loop,
but keep the smallest_int_mode_for_size call as-is?  I'm not sure
that's going to be more efficient in practice, and it seems less
obviously correct.

Alternatively, we could have a version of smallest_int_mode_for_size
that returns failure instead of asserting.  But smallest_int_mode_for_size
is itself a FOR_EACH_MODE_* iterator, so it would iterate just as much as
the patch does.

smallest_int_mode_for_size also has some __int20 etc. handling that I
don't think we want here, and probably avoid by luck.  (We already know
at this point that any shift value would be nonzero, which probably
weeds out most of the problematic cases for PSImode targets.)

find_shift_sequence already iterates over the modes itself and it
already has custom requirements in terms of when to break and when
to continue.  It just seems simpler and more obvious for the loop to
iterate over all the modes directly rather than delegate part of the
iteration to another function.

Thanks,
Richard



>
>>This patch just makes the code start from the smallest integer mode and
>>skip modes that are too small.  The loop already breaks at the first
>>mode wider than word_mode.
>>
>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
>>GCC 10 branch?
>>
>>Richard
>>
>>
>>gcc/
>>  PR rtl-optimization/98037
>>  * dse.c (find_shift_sequence): Iterate over all integers and
>>  skip modes that are too small.
>>
>>gcc/testsuite/
>>  PR rtl-optimization/98037
>>  * gcc.target/aarch64/sve/acle/general/pr98037.c: New test.
>>---
>> gcc/dse.c   | 5 +++--
>> gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c | 6 ++
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>create mode 100644
>>gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>
>>diff --git a/gcc/dse.c b/gcc/dse.c
>>index d65266b5476..651e6e7e71e 100644
>>--- a/gcc/dse.c
>>+++ b/gcc/dse.c
>>@@ -1757,8 +1757,7 @@ find_shift_sequence (poly_int64 access_size,
>>  the machine.  */
>> 
>>   opt_scalar_int_mode new_mode_iter;
>>-  FOR_EACH_MODE_FROM (new_mode_iter,
>>-   smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>>+  FOR_EACH_MODE_IN_CLASS (new_mode_iter, MODE_INT)
>> {
>>   rtx target, new_reg, new_lhs;
>>   rtx_insn *shift_seq, *insn;
>>@@ -1767,6 +1766,8 @@ find_shift_sequence (poly_int64 access_size,
>>   new_mode = new_mode_iter.require ();
>>   if (GET_MODE_BITSIZE (new_mode) > BITS_PER_WORD)
>>  break;
>>+  if (maybe_lt (GET_MODE_SIZE (new_mode), GET_MODE_SIZE
>>(read_mode)))
>>+ continue;
>> 
>>   /* Try a wider mode if truncating the store mode to NEW_MODE
>>   requires a real instruction.  */
>>diff --git
>>a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>new file mode 100644
>>index 000..b91e940b18e
>>--- /dev/null
>>+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>>@@ -0,0 +1,6 @@
>>+/* { dg-options "-msve-vector-bits=1024 -O3" } */
>>+
>>+typedef __SVInt8_t vec __attribute__((arm_sve_vector_bits(1024)));
>>+struct pair { vec v[2]; };
>>+void use (struct pair *);
>>+vec f (struct pair p) { vec v = p.v[1]; use (); return v; }


[PATCH] Add feature test macro for atomic::wait

2020-11-30 Thread Thomas Rodgers
From: Thomas Rodgers 

Adds __cpp_lib_atomic_wait feature test macro which was overlooked in
the initial commit of this feature. Replaces uses of
_GLIBCXX_HAVE_ATOMIC_WAIT.

libstdc++-v3/ChangeLog:

* include/bits/atomic_base.h: Replace usage of
_GLIBCXX_HAVE_ATOMIC_WAIT with __cpp_lib_atomic_wait.
* include/bits/atomic_timed_wait.h: Likewise.
* include/bits/atomic_wait.h: Define __cpp_lib_atomic_wait
feature test macro.
* include/bits/semaphore_base: Replace usage of
_GLIBCXX_HAVE_ATOMIC_WAIT with __cpp_lib_atomic_wait.
* include/std/atomic: Likewise.
* include/std/latch: Likewise.
* include/std/semaphore: Likewise.
* include/std/version: Define __cpp_lib_atomic wait
feature test macro and replace usage of
_GLIBCXX_HAVE_ATOMIC_WAIT.
* testsuite/29_atomics/atomic/wait_notify/1.cc: New test.
* testsuite/29_atomics/atomic/wait_notify/2.cc: Likewise.

---
 libstdc++-v3/include/bits/atomic_base.h   | 36 +--
 libstdc++-v3/include/bits/atomic_timed_wait.h |  5 +--
 libstdc++-v3/include/bits/atomic_wait.h   |  3 +-
 libstdc++-v3/include/bits/semaphore_base.h|  4 +--
 libstdc++-v3/include/std/atomic   | 16 -
 libstdc++-v3/include/std/latch|  4 +--
 libstdc++-v3/include/std/semaphore|  4 +--
 libstdc++-v3/include/std/version  |  7 ++--
 .../29_atomics/atomic/wait_notify/1.cc| 28 +++
 .../29_atomics/atomic/wait_notify/2.cc| 29 +++
 10 files changed, 98 insertions(+), 38 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/29_atomics/atomic/wait_notify/1.cc
 create mode 100644 libstdc++-v3/testsuite/29_atomics/atomic/wait_notify/2.cc

diff --git a/libstdc++-v3/include/bits/atomic_base.h 
b/libstdc++-v3/include/bits/atomic_base.h
index d0d962d3047..ad4e24b4d20 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -230,7 +230,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return __v == __GCC_ATOMIC_TEST_AND_SET_TRUEVAL;
 }
 
-#ifdef _GLIBCXX_HAVE_ATOMIC_WAIT
+#ifdef __cpp_lib_atomic_wait
 _GLIBCXX_ALWAYS_INLINE void
 wait(bool __old,
memory_order __m = memory_order_seq_cst) const noexcept
@@ -253,7 +253,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 { std::__atomic_notify(&_M_i, true); }
 
 // TODO add const volatile overload
-#endif // HAVE_ATOMIC_WAIT
+#endif // __cpp_lib_atomic_wait 
 #endif // C++20
 
 _GLIBCXX_ALWAYS_INLINE void
@@ -604,7 +604,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __cmpexch_failure_order(__m));
   }
 
-#if __cplusplus > 201703L && defined _GLIBCXX_HAVE_ATOMIC_WAIT
+#if __cpp_lib_atomic_wait
   _GLIBCXX_ALWAYS_INLINE void
   wait(__int_type __old,
  memory_order __m = memory_order_seq_cst) const noexcept
@@ -627,7 +627,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { std::__atomic_notify(&_M_i, true); }
 
   // TODO add const volatile overload
-#endif // C++20 && HAVE_ATOMIC_WAIT
+#endif // __cpp_lib_atomic_wait 
 
   _GLIBCXX_ALWAYS_INLINE __int_type
   fetch_add(__int_type __i,
@@ -898,7 +898,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   int(__m1), int(__m2));
   }
 
-#if __cplusplus > 201703L && defined _GLIBCXX_HAVE_ATOMIC_WAIT
+#if __cpp_lib_atomic_wait 
   _GLIBCXX_ALWAYS_INLINE void
   wait(__pointer_type __old,
   memory_order __m = memory_order_seq_cst) noexcept
@@ -921,7 +921,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { std::__atomic_notify(&_M_p, true); }
 
   // TODO add const volatile overload
-#endif // C++20 && HAVE_ATOMIC_WAIT
+#endif // __cpp_lib_atomic_wait
 
   _GLIBCXX_ALWAYS_INLINE __pointer_type
   fetch_add(ptrdiff_t __d,
@@ -1011,7 +1011,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 int(__success), int(__failure));
   }
 
-#if __cplusplus > 201703L && defined _GLIBCXX_HAVE_ATOMIC_WAIT
+#if __cpp_lib_atomic_wait
 template
   _GLIBCXX_ALWAYS_INLINE void
   wait(const _Tp* __ptr, _Val<_Tp> __old,
@@ -1036,7 +1036,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { std::__atomic_notify(__ptr, true); }
 
   // TODO add const volatile overload
-#endif // C++20 && HAVE_ATOMIC_WAIT
+#endif // __cpp_lib_atomic_wait
 
 template
   _GLIBCXX_ALWAYS_INLINE _Tp
@@ -1291,7 +1291,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __cmpexch_failure_order(__order));
   }
 
-#ifdef _GLIBCXX_HAVE_ATOMIC_WAIT
+#ifdef __cpp_lib_atomic_wait
   _GLIBCXX_ALWAYS_INLINE void
   wait(_Fp __old, memory_order __m = memory_order_seq_cst) const noexcept
   { __atomic_impl::wait(&_M_fp, __old, __m); }
@@ -1309,7 +1309,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { __atomic_impl::notify_all(&_M_fp); }
 
   // TODO add const volatile overload
-#endif // 

Re: [PATCH] profopt-execute: unset testname_with_flags if create_gcov fails

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/26/20 12:22 PM, Ilya Leoshkevich via Gcc-patches wrote:
> Bootstrapped and regtested on x86_64-redhat-linux and
> s390x-redhat-linux.  Ok for master?
>
>
>
> When diffing test results, there sometimes occur spurious "New tests
> that PASS" / "Old tests that passed, that have disappeared" messages.
> The reason is that if create_gcov is not installed, then the cached
> testname_with_flags is not cleared and is carried over to the next
> test.
>
> gcc/testsuite/ChangeLog:
>
> 2020-11-26  Ilya Leoshkevich  
>
>   * lib/profopt.exp: Unset testname_with_flags if create_gcov
>   fails.

OK.

Thanks,
Jeff




Re: [PATCH] gcc-11/changes: Document new configure flag --enable-s390-excess-float-precision

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/26/20 2:58 AM, Marius Hillenbrand via Gcc-patches wrote:
> Hi,
>
> To document the new behavior around FLT_EVAL_METHOD and configure flag
> --enable-s390-excess-float-precision on s390, I propose this update to the
> Release Notes. Please commit to git-wwwdocs if you agree.
>
> Checked against the w3c validator.
>
> Thanks,
> Marius
> ---
>  htdocs/gcc-11/changes.html | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
Thanks.  Pushed to the trunk.
jeff



Re: [PATCH] [tree-optimization] Optimize max/min pattern with comparison

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/25/20 3:04 PM, Eugene Rozenfeld via Gcc-patches wrote:
> Make the following simplifications:
> X <= MAX(X, Y) -> true
> X > MAX(X, Y) -> false
> X >= MIN(X, Y) -> true
> X < MIN(X, Y) -> false
> 
> This fixes PR96708.
> 
> Tested on x86_64-pc-linux-gnu.
>
> bool f(int a, int b)
> {
> int tmp = (a < b) ? b : a;
> return tmp >= a;
> }
>
> Code without the patch:
>
> vmovd  xmm0,edi
> vmovd  xmm1,esi
> vpmaxsd xmm0,xmm0,xmm1
> vmovd  eax,xmm0
> cmpeax,edi
> setge  al
> ret
>
> Code with the patch:
>
> moveax,0x1
> ret
>
> Eugene
>
> 0001-Optimize-max-pattern-with-comparison.patch
>
> From f6391c197b670b516238ac7707512c1358336520 Mon Sep 17 00:00:00 2001
> From: Eugene Rozenfeld 
> Date: Sat, 21 Nov 2020 01:08:50 -0800
> Subject: [PATCH] Optimize max pattern with comparison
>
> Make the following simplifications:
> X <= MAX(X, Y) -> true
> X > MAX(X, Y) -> false
> X >= MIN(X, Y) -> true
> X < MIN(X, Y) -> false
>
> This fixes PR96708.
>
> gcc/
> * match.pd : New patterns.
> ---
>  gcc/match.pd | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index cbb4bf0b32d..75237741946 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2851,6 +2851,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>(cmp (minmax @0 INTEGER_CST@1) INTEGER_CST@2)
>(comb (cmp @0 @2) (cmp @1 @2
>  
> +/* X <= MAX(X, Y) -> true
> +   X > MAX(X, Y) -> false 
> +   X >= MIN(X, Y) -> true
> +   X < MIN(X, Y) -> false */
> +(for minmax (min min max max )
> + cmp(ge  lt  le  gt  )
> + (simplify
> +  (cmp @0 (minmax:c @0 @1))
> +  { constant_boolean_node (cmp == GE_EXPR || cmp == LE_EXPR, type); } ))
Don't you need to look at the opcode (MIN vs MAX) vs CMP to know the
result?  I'd really like to see some tests for the testsuite.    In
particular I'd like to see positive tests where we should apply the
optimization and negative tests when we should not apply the optimization.

I also wonder if there's value in handling this in Ranger and/or DOM. 
Though I'd probably wait to see if fixing in match.pd is sufficient to
cover the cases I'm thinking of in Ranger & DOM.

Jeff




Re: [PATCH] dse: Cope with bigger-than-integer modes [PR98037]

2020-11-30 Thread Richard Biener via Gcc-patches
On November 30, 2020 4:29:41 PM GMT+01:00, Richard Sandiford via Gcc-patches 
 wrote:
>dse.c:find_shift_sequence tries to represent a store and load
>back as a shift right followed by a truncation.  It therefore
>needs to find an integer mode in which to do the shift right.
>The loop it uses has the form:
>
>  FOR_EACH_MODE_FROM (new_mode_iter,
> smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>
>which implicitly assumes that read_mode has an equivalent integer mode.
>As shown in the testcase, not all modes have such an integer mode.

But if no such mode exists iterating won't help either? So why not simply fail 
when the mode does not exist?

>This patch just makes the code start from the smallest integer mode and
>skip modes that are too small.  The loop already breaks at the first
>mode wider than word_mode.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
>GCC 10 branch?
>
>Richard
>
>
>gcc/
>   PR rtl-optimization/98037
>   * dse.c (find_shift_sequence): Iterate over all integers and
>   skip modes that are too small.
>
>gcc/testsuite/
>   PR rtl-optimization/98037
>   * gcc.target/aarch64/sve/acle/general/pr98037.c: New test.
>---
> gcc/dse.c   | 5 +++--
> gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c | 6 ++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>create mode 100644
>gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>
>diff --git a/gcc/dse.c b/gcc/dse.c
>index d65266b5476..651e6e7e71e 100644
>--- a/gcc/dse.c
>+++ b/gcc/dse.c
>@@ -1757,8 +1757,7 @@ find_shift_sequence (poly_int64 access_size,
>  the machine.  */
> 
>   opt_scalar_int_mode new_mode_iter;
>-  FOR_EACH_MODE_FROM (new_mode_iter,
>-smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>+  FOR_EACH_MODE_IN_CLASS (new_mode_iter, MODE_INT)
> {
>   rtx target, new_reg, new_lhs;
>   rtx_insn *shift_seq, *insn;
>@@ -1767,6 +1766,8 @@ find_shift_sequence (poly_int64 access_size,
>   new_mode = new_mode_iter.require ();
>   if (GET_MODE_BITSIZE (new_mode) > BITS_PER_WORD)
>   break;
>+  if (maybe_lt (GET_MODE_SIZE (new_mode), GET_MODE_SIZE
>(read_mode)))
>+  continue;
> 
>   /* Try a wider mode if truncating the store mode to NEW_MODE
>requires a real instruction.  */
>diff --git
>a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>new file mode 100644
>index 000..b91e940b18e
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
>@@ -0,0 +1,6 @@
>+/* { dg-options "-msve-vector-bits=1024 -O3" } */
>+
>+typedef __SVInt8_t vec __attribute__((arm_sve_vector_bits(1024)));
>+struct pair { vec v[2]; };
>+void use (struct pair *);
>+vec f (struct pair p) { vec v = p.v[1]; use (); return v; }



Re: [PATCH] ipa-cp: Avoid unwanted multiple propagations (PR 97816)

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/20/20 12:41 PM, Martin Jambor wrote:
> Hi,
>
> this is an updated patch based on our conversation on IRC today.  So far
> I have had a look at the effects on only tramp3d and although it makes
> the heuristics more pessimistic more times than optimistic (number of
> clones at -Ofast drops from 559 to 557), there are also lattices which
> are massively boosted.
I don't think that's inherently a bad thing.

>
> When looking at the testcase of PR 97816 I realized that the reason
> why we were hitting overflows in size growth estimates in IPA-CP is
> not because the chains of how lattices feed values to each other are
> so long but mainly because we add estimates in callee lattices to
> caller lattices for each value source, which roughly corresponds to a
> call graph edge, and therefore if there are multiple calls between two
> functions passing the same value in a parameter we end up doing it
> more than once, sometimes actually quite many times.
>
> This patch avoids it by using a has_set to remember the source values
> we have already updated and not increasing their size again.
> Furhtermore, to improve estimation of times we scale the propagated
> time benefits with edge frequencies as we accumulate them.
>
> This should make any overflows very unlikely but not impossible, so I
> still included checks for overflows but decided to restructure the
> code to only need it in the propagate_effects function and modified it
> so that it does not need to perform the check before each sum.
>
> This is because I decided to add local estimates to propagated
> estimates already in propagate_effects and not at the evaluation time.
> The function can then do the sums in a wide type and discard them in
> the unlikely case of an overflow.  I also decided to use the
> opportunity to make propagated effect stats now include stats from
> other values in the same SCCs.  In the dumps I have seen this tended
> to increase size cost a tiny bit more than the estimated time benefit
> but both increases were small.
>
> Bootstrapped and LTO bootstrapped on x86_64-linux.  OK for trunk?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2020-11-20  Martin Jambor  
>
>   PR ipa/97816
>   * ipa-cp.c (safe_add): Removed.
>   (good_cloning_opportunity_p): Remove special handling of INT_MAX.
>   (value_topo_info::propagate_effects): Take care not to
>   propagate from size one value to another through more sources.  Scale
>   propagated times with edge frequencies.  Include local time and size
>   in propagates ones here.  Take care not to overflow size.
>   (decide_about_value): Do not add local and propagated effects when
>   passing them to good_cloning_opportunity_p.
OK
Jeff



Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Martin Sebor via Gcc-patches

On 11/30/20 9:23 AM, Qing Zhao wrote:

Hi, Martin,

Thanks a lot for your suggestion.

On Nov 25, 2020, at 6:08 PM, Martin Sebor > wrote:


On 11/24/20 9:54 AM, Qing Zhao via Gcc-patches wrote:
On Nov 24, 2020, at 9:55 AM, Richard Biener 
mailto:richard.guent...@gmail.com>> wrote:


On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao > wrote:




On Nov 24, 2020, at 1:32 AM, Richard Biener 
mailto:richard.guent...@gmail.com>> 
wrote:


On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
mailto:gcc-patches@gcc.gnu.org>> wrote:


Hi,

Does gcc provide an iterator to traverse all the local variables 
that are declared in the current routine?


If not, what’s the best way to traverse the local variables?


Depends on what for.  There's the source level view you get by walking
BLOCK_VARS of the
scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
there's SSA names
(FOR_EACH_SSA_NAME).


I am planing to add a new phase immediately after 
“pass_late_warn_uninitialized” to initialize all auto-variables 
that are
not explicitly initialized in the declaration, the basic idea is 
following:


** The proposal:

A. add a new GCC option: (same name and meaning as CLANG)
-ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;

B. add a new attribute for variable:
__attribute((uninitialized)
the marked variable is uninitialized intentionaly for performance 
purpose.


C. The implementation needs to keep the current static warning on 
uninitialized

variables untouched in order to avoid "forking the language".


** The implementation:

There are two major requirements for the implementation:

1. all auto-variables that do not have an explicit initializer 
should be initialized to

zero by this option.  (Same behavior as CLANG)

2. keep the current static warning on uninitialized variables 
untouched.


In order to satisfy 1, we should check whether an auto-variable has 
initializer

or not;
In order to satisfy 2, we should add this new transformation after
"pass_late_warn_uninitialized".

So, we should be able to check whether an auto-variable has 
initializer or not after “pass_late_warn_uninitialized”,

If Not, then insert an initialization for it.

For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?


Yes, but do you want to catch variables promoted to register as well
or just variables
on the stack?
I think both as long as they are source-level auto-variables. Then 
which one is better?


Another issue is, in order to check whether an auto-variable has 
initializer, I plan to add a new bit in “decl_common” as:

 /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
 unsigned decl_is_initialized :1;

/* IN VAR_DECL, set when the decl is initialized at the 
declaration.  */

#define DECL_IS_INITIALIZED(NODE) \
 (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)

set this bit when setting DECL_INITIAL for the variables in FE. 
then keep it

even though DECL_INITIAL might be NULLed.


For locals it would be more reliable to set this flag during 
gimplification.
You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the 
routine “gimpley_decl_expr” (gimplify.c) as following:

  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
    {
      tree init = DECL_INITIAL (decl);
...
      if (init && init != error_mark_node)
        {
          if (!TREE_STATIC (decl))
    {
      DECL_IS_INITIALIZED(decl) = 1;
    }
Is this enough for all Frontends? Are there other places that I need 
to maintain this bit?



Do you have any comment and suggestions?


As said above - do you want to cover registers as well as locals?
All the locals from the source-code point of view should be covered. 
  (From my study so far,  looks like that Clang adds that phase in FE).

If GCC adds this phase in FE, then the following design requirement
C. The implementation needs to keep the current static warning on 
uninitialized

variables untouched in order to avoid "forking the language”.
cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
applied quite late.

So, we have to add this new phase after “pass_late_warn_uninitialized”.

 I'd do
the actual zeroing during RTL expansion instead since otherwise you
have to figure youself whether a local is actually used (see 
expand_stack_vars)
Adding  this new transformation during RTL expansion is okay.  I will 
check on this in more details to see how to add it to RTL expansion 
phase.


Note that optimization will already made have use of "uninitialized" 
state
of locals so depending on what the actual goal is here "late" may be 
too late.

This is a really good point…
In order to avoid optimization  to use the “uninitialized” state of 
locals, we should add the zeroing phase as early as possible (adding 
it in FE might be best

for this issue). However, if we have to met the following requirement:
C. The implementation needs to keep the current static warning on 
uninitialized


Re: [PATCH] driver: Don't imply -dD for -g3 -g0 [PR97989]

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/26/20 1:39 AM, Jakub Jelinek via Gcc-patches wrote:
> Hi!
>
> The driver enables -dD when preprocessing when -g3 is specified, for obvious 
> reasons
> that we need the macros to be preserved somewhere for them to make up the 
> debug
> info.  But it enables it even if -g3 is later overridden to -g2, -g1 or -g0,
> where we in the end don't emit .debug_mac{ros,info}.
>
> The following patch passes -dD only if we'll need it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-11-26  Jakub Jelinek  
>
>   PR debug/97989
>   * gcc.c (cpp_unique_options): Add -dD if %:debug-level-gt(2)
>   rather than g3|ggdb3|gstabs3|gxcoff3|gvms3.
>
>   * gcc.dg/cpp/pr97989-1.c: New test.
>   * gcc.dg/cpp/pr97989-2.c: New test.
OK
jeff



Re: [PATCH v2] C-family : Add attribute 'unavailable'.

2020-11-30 Thread Martin Sebor via Gcc-patches

On 11/29/20 6:56 PM, Iain Sandoe wrote:

Hi Martin,

Martin Sebor via Gcc-patches  wrote:


On 11/10/20 12:38 PM, Iain Sandoe wrote:



—— commit message.
If an interface is marked 'deprecated' then, presumably, at some 
point it

will be withdrawn and no longer available.  The 'unavailable' attribute
makes it possible to mark up interfaces to indicate this status.



Making an interface unavailable isn't the intent of deprecation in
standards like C, C++, or POSIX.  Rather, the intended next stage
after deprecation is to make the interface available for other uses,
either by the standards themselves, or by implementations, or by
programs (if its name is not in the reserved namespace).  So unless
you have other kinds of deprecation in mind this doesn't seem like
a fitting use case.


coming at things from the standards perspective .. perhaps.

.. however, in the first set of cases above, one never needs to indicate
   unavailability  since the entity never becomes unavailable, it changes
   meaning.

In practice, we don’t have markup of keywords etc. to indicate this (such
deprecation has been handled specifically in the FEs).

The practical (and actual) use of these attributes is in describing the 
lifecycle

of system APIs.

In that context, one could see it being used to describe APIs withdrawn 
in, say,

a Posix standard (as implemented by a given system or according to specific
compile-time flags).


Just for the sake of clarity, to make sure we are both talking about
the same thing, the term POSIX uses for "withdrawn" when referring
to APIs is removed.  Withdrawn is a term that ISO tends to apply to
whole standards.  For example, ISO 9945:2003 AKA SUSv3 is a withdrawn
revision of POSIX (likewise, C99 and C11 are withdrawn revisions of
C).

With that, the APIs that have been removed in recent POSIX versions
would not be appropriately described by the attribute, either on
paper, in the POSIX spec (if the hypothetical case when the attribute
was specified the C standard), or in implementations.  The APIs simply
don't exist and so are open to use by programs (unless they are in
the reserved namespace), or by implementations as extensions.
The latter typically means that the APIs are defined (i.e., can
continue to be linked to by legacy applications) but not declared
in the system's header so they can be defined by programs.  Declaring
removed APIs with the unavailable attribute would prevent that and so
would not be conforming.

An example from C (not yet POSIX) is the gets() function that was
removed in C11 (it's still SUSv4).  It's only declared in 
when targeting older versions of C and C++.  Because in more recent
versions of the standards gets is not a reserved name it's valid
for programs to declare symbols named gets (of any kind).  So
declaring gets in  with attribute unavailable would be
nonconforming.

My point here is not necessarily to object to the idea behind
the attribute but to draw attention to the fact that it's not
suitable for standard APIs.


It is used
quite extensively in some codebases where a single set of headers can 
be used

to permit code generation for multiple system versions.


This sounds like a different use case than the next stage after
deprecation.  I haven't come across it but I imagine its purpose
is to foster portability between variants or flavors (rather
than versions) of APSs?  Say one set of APIs for a feature-rich
desktop variant of an OS versus a subset of the same APIs for
an embedded, more limited variant of the same OS.


In the case of Darwin, the compilers are capable of targeting multiple 
versions
of the system (one doesn’t need a separate GCC or clang to target each 
version,
there is a -mmacosx-version-min= flag that allows the target version to 
be specified

on the command line).

Rather than install many versions of headers (and libraries) for all the 
system
versions, the designers elected to have one copy with markup that 
describes the

availability of APIs.

the lifecycle is typically:

introduced from version P.Q (ergo, unavailable before that)
perhaps deprecated at version R.S (ergo the user should be warned)
withdrawn at version X.Y (and unavailable thereafter).

The headers contain macros that are expanded according to the version
for which the code is being compiled - to produce deprecation warnings or
unavailability errors if such APIs are *used* in the code.


I see.  That seems like a Darwin-specific use case, or one that may
be suitable for proprietary APIs not covered by the three language
standards (C, C++, or POSIX) where the attribute could only be used
this way as a non-conforming extension.  I suppose it might also be
applicable in the case of Linux distributions whose vendors support
multiple versions simultaneously, although I haven't come across
this use case.



From a configuration perspective, it also allows a compile test to 
determine

that an interface is missing - rather than requiring a link test.
The implementation 

Re: [PATCH] dse: Cope with bigger-than-integer modes [PR98037]

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 8:29 AM, Richard Sandiford via Gcc-patches wrote:
> dse.c:find_shift_sequence tries to represent a store and load
> back as a shift right followed by a truncation.  It therefore
> needs to find an integer mode in which to do the shift right.
> The loop it uses has the form:
>
>   FOR_EACH_MODE_FROM (new_mode_iter,
> smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
>
> which implicitly assumes that read_mode has an equivalent integer mode.
> As shown in the testcase, not all modes have such an integer mode.
>
> This patch just makes the code start from the smallest integer mode and
> skip modes that are too small.  The loop already breaks at the first
> mode wider than word_mode.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
> GCC 10 branch?
>
> Richard
>
>
> gcc/
>   PR rtl-optimization/98037
>   * dse.c (find_shift_sequence): Iterate over all integers and
>   skip modes that are too small.
>
> gcc/testsuite/
>   PR rtl-optimization/98037
>   * gcc.target/aarch64/sve/acle/general/pr98037.c: New test.
OK
jeff



Re: [PATCH] [tree-optimization] Optimize or+and+or pattern to and+or

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/22/20 6:35 PM, Eugene Rozenfeld via Gcc-patches wrote:
> Simplify
> ((b | c) & a) | b
> to 
> (a & c) | b.
>
> This fixes PR96679.
>
> Tested on x86_64-pc-linux-gnu.
>
> int f(int a, int b, int c)
> {
> return ((b | c) & a) | b;
> }
>
> Code without the patch:
> or edx,esi
> andedx,edi
> moveax,edx
> or eax,esi
> ret
>
> Code with the patch:
> andedi,edx
> moveax,edi
> or eax,esi
> ret
>
> Eugene
>
> 0001-Optimize-or-and-or-pattern-to-and-or.patch
>
> From 326f186e084c1788c428d581aa3c1a20344ed5b4 Mon Sep 17 00:00:00 2001
> From: Eugene Rozenfeld 
> Date: Fri, 20 Nov 2020 18:05:15 -0800
> Subject: [PATCH] Optimize or+and+or pattern to and+or
>
> Simplify ((b | c) & a) | b to (a & c) | b.
> This fixes PR96679.
>
> gcc/
> * match.pd : New patterns.
Thanks.  I've expanded the ChangeLog a bit and pushed this to the trunk.

For future patches, please also include testcases.  While they aren't a
strict requirement, we've found them incredibly useful, even for minor
changes like this.

Jeff
> ---
>  gcc/match.pd | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index cbb4bf0b32d..00294665c98 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1433,6 +1433,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>(bitop:c (rbitop:c (bit_not @0) @1) @0)
>(bitop @0 @1)))
>  
> +/* ((x | y) & z) | x -> (z & y) | x */
> +(simplify
> +  (bit_ior:c (bit_and:cs (bit_ior:cs @0 @1) @2) @0)
> +  (bit_ior (bit_and @2 @1) @0))
> +
>  /* (x | CST1) & CST2 -> (x & CST2) | (CST1 & CST2) */
>  (simplify
>(bit_and (bit_ior @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)



Re: [PATCH v3 1/2] generate EH info for volatile asm statements (PR93981)

2020-11-30 Thread J.W. Jagersma via Gcc-patches
On 2020-11-23 09:20, Richard Biener wrote:
> On Sun, Nov 22, 2020 at 5:38 PM J.W. Jagersma  wrote:
>>
>> On 2020-11-21 12:27, J.W. Jagersma wrote:
>>> ...
>>> Another idea I had is to introduce a new operand modifier, eg. '-', which
>>> would signify that the output *must* be considered clobbered on exception,
>>> and it would be an error if a copy is not possible.  Then the meaning of '+'
>>> can be extended to say that the output must be valid regardless of whether 
>>> an
>>> exception occured or not.  Modifier '=' would mean the same as '+' with the
>>> additional implication that it is okay to eliminate earlier assignments, so
>>> that its value on the EH edge would be undefined.
>>
>> I've been working on implementing this, but I ran into an issue:
>>
>> First of all, in the first version of this patch I had to make sure that
>> debug stmts were inserted across the fallthrough edge, so that they don't
>> appear at the end a basic block.  I was surprised to find that this is no
>> longer necessary.
>>
>> Next I discovered that the following test case fails (returns -1):
>>
>> int f ()
>> {
>>   int i = 0;
>>   try { asm ("mov %0, 1; ud2" : "+r" (i)); }
>>   catch (...) { }
>>   return i - 1;
>> }
>>
>> And this does succeed with a memory operand.
>>
>> It seems that something changed in the past few months, and now asm register
>> outputs are already being assigned via a temporary variable, somewhere.  Does
>> anyone know where that might be?
> 
> It's likely out-of SSA / RTL expansion inserting a bogus copy or doing
> coalescing on the EH edge.  Semantics of throwing asm stmts need to be
> honored there.
> 
> Richard.

Quick update.  I have a mostly working implementation now, the only case where
it fails is when an asm clobbers all callee-saved registers.  In this one case
the output is not available on the exception edge.  However this same case does
work correctly with an 'asm goto' that throws.

The problem I think is in lra_process_new_insns (lra.c:1878) where the condition
to insert new insns across edges is "JUMP_P (insn)", but this should happen for
throwing asms as well.  I figured "insn == BB_END (BLOCK_FOR_INSN (insn))" would
make sense but that causes all sorts of trouble (segfaults during bootstrap).

Would appreciate any advice on what to do here.


Re: [PATCH][AVX512]Lower AVX512 vector compare to AVX version when dest is vector

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/16/20 8:10 PM, Hongtao Liu wrote:
> On Tue, Nov 17, 2020 at 8:05 AM Jeff Law  wrote:
>>
>> On 9/2/20 3:34 AM, Hongtao Liu via Gcc-patches wrote:
>>> Hi:
>>>   Add define_peephole2 to eliminate potential redundant conversion
>>> from mask to vector.
>>>   Bootstrap is ok, regression test is ok for i386/x86-64 backend.
>>>   Ok for trunk?
>>>
>>> gcc/ChangeLog:
>>> PR target/96891
>>> * config/i386/sse.md (VI_128_256): New mode iterator.
>>> (define_peephole2): Lower avx512 vector compare to avx version
>>> when dest is vector.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/i386/avx512bw-pr96891-1.c: New test.
>>> * gcc.target/i386/avx512f-pr96891-1.c: New test.
>>> * gcc.target/i386/avx512f-pr96891-2.c: New test.
>> Aren't these the two insns in question:
>>
>>
>> (insn 7 4 8 2 (set (reg:QI 86)
>> (unspec:QI [
>> (reg:V8SF 90)
>> (reg:V8SF 89)
>> (const_int 2 [0x2])
>> ] UNSPEC_PCMP)) "j.c":4:14 1911 {avx512vl_cmpv8sf3}
>>  (expr_list:REG_DEAD (reg:V8SF 90)
>> (expr_list:REG_DEAD (reg:V8SF 89)
>> (nil
>> (note 8 7 9 2 NOTE_INSN_DELETED)
>> (insn 9 8 14 2 (set (reg:V8SI 82 [ _2 ])
>> (vec_merge:V8SI (const_vector:V8SI [
>> (const_int -1 [0x]) repeated x8
>> ])
>> (const_vector:V8SI [
>> (const_int 0 [0]) repeated x8
>> ])
>> (reg:QI 86))) "j.c":4:14 2705 {*avx512vl_cvtmask2dv8si}
>>  (expr_list:REG_DEAD (reg:QI 86)
>> (nil)))
>>
>>
>> Note there's a data dependency between them.  insn 7 feeds insn 9.  When
>> there's a data dependency, combiner patterns are usually the better
>> choice than peepholes.  I think you'd be looking to match something
>> likethis (from the . combine dump):
>>
>> (set (reg:V8SI 82 [ _2 ])
>> (vec_merge:V8SI (const_vector:V8SI [
>> (const_int -1 [0x]) repeated x8
>> ])
>> (const_vector:V8SI [
>> (const_int 0 [0]) repeated x8
>> ])
>> (unspec:QI [
>> (reg:V8SF 90)
>> (reg:V8SF 89)
>> (const_int 2 [0x2])
>> ] UNSPEC_PCMP)))
>>
>>
>> Jeff
>>
> Yes, as discussed in [1], maybe it's better to refactor avx512 integer
> mask with VnBImode. Then unspec_pcmp could be dropped and simplify_rtx
> could handle vector comparison more effectively.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97521#c4
Thanks for the pointer.   I didn't realize this patch was essentially
abandoned.

Jeff



Re: [PATCH] ipa: dump symtab to emergency dump file

2020-11-30 Thread Martin Liška

On 11/30/20 4:50 PM, Jan Hubicka wrote:



On 11/30/20 7:04 AM, Martin Liška wrote:

Hi.

It's handy to have symbol table when we dump emergency dump.
It's likely next stage1 material.

Thoughts?
Martin

gcc/ChangeLog:

    * passes.c (emergency_dump_function): Dump symtab when
    we are in an IPA pass.

Your call now or next stage1.   We should keep in mind that if the
symtab structures are inconsistent/corrupted that we can ICE or worse
yet spin while dumping the symtab.


We already dump function bodies that has same problem, so I think
dumping symtab is fine (and I have no problem with it going to trunk
this stage3)


Thank you, I install the patch now.

Martin



Honza


Jeff





Re: [PATCH] Remove redundant builtins for avx512f scalar instructions.

2020-11-30 Thread Jakub Jelinek via Gcc-patches
On Mon, Nov 30, 2020 at 09:23:15AM -0700, Jeff Law wrote:
> 
> 
> On 11/12/20 11:21 PM, Hongyu Wang wrote:
> > Hi
> >
> > Thanks for reminding me about this patch. I didn't remove any existing
> > intrinsics, just remove redundant builtin functions that end-users
> > would not likely to use.
> >
> > Also I'm OK to keep current implementation, in case there might be
> > someone using the builtin directly.
> That seems wise -- we can't reasonably predict if users are using those
> builtins directly. 
> 
> So if we can clean things up and keep the redundant builtins that seems
> best.  Or just leave things as-is. 
> 
> The other possibility would be to deprecate the redundant builtins this
> release and remove them in gcc-12.  I haven't looked at how difficult
> that might be, but the idea here would be to give users a warning if
> they use those builtins directly and enough time to resolve the issue
> before we remove them.

In the past we've removed the builtins without any warning, we state all the
time that the builtins used to implement the intrinsics themselves aren't
supported, only the intrinsics declared in the headers.

Jakub



Re: How to traverse all the local variables that declared in the current routine?

2020-11-30 Thread Qing Zhao via Gcc-patches
Hi, Martin,

Thanks a lot for your suggestion.

> On Nov 25, 2020, at 6:08 PM, Martin Sebor  wrote:
> 
> On 11/24/20 9:54 AM, Qing Zhao via Gcc-patches wrote:
>>> On Nov 24, 2020, at 9:55 AM, Richard Biener  
>>> wrote:
>>> 
>>> On Tue, Nov 24, 2020 at 4:47 PM Qing Zhao  wrote:
 
 
 
> On Nov 24, 2020, at 1:32 AM, Richard Biener  
> wrote:
> 
> On Tue, Nov 24, 2020 at 12:05 AM Qing Zhao via Gcc-patches
>  wrote:
>> 
>> Hi,
>> 
>> Does gcc provide an iterator to traverse all the local variables that 
>> are declared in the current routine?
>> 
>> If not, what’s the best way to traverse the local variables?
> 
> Depends on what for.  There's the source level view you get by walking
> BLOCK_VARS of the
> scope tree, theres cfun->local_variables (FOR_EACH_LOCAL_DECL) and
> there's SSA names
> (FOR_EACH_SSA_NAME).
 
 I am planing to add a new phase immediately after 
 “pass_late_warn_uninitialized” to initialize all auto-variables that are
 not explicitly initialized in the declaration, the basic idea is following:
 
 ** The proposal:
 
 A. add a new GCC option: (same name and meaning as CLANG)
 -ftrivial-auto-var-init=[pattern|zero], similar pattern init as CLANG;
 
 B. add a new attribute for variable:
 __attribute((uninitialized)
 the marked variable is uninitialized intentionaly for performance purpose.
 
 C. The implementation needs to keep the current static warning on 
 uninitialized
 variables untouched in order to avoid "forking the language".
 
 
 ** The implementation:
 
 There are two major requirements for the implementation:
 
 1. all auto-variables that do not have an explicit initializer should be 
 initialized to
 zero by this option.  (Same behavior as CLANG)
 
 2. keep the current static warning on uninitialized variables untouched.
 
 In order to satisfy 1, we should check whether an auto-variable has 
 initializer
 or not;
 In order to satisfy 2, we should add this new transformation after
 "pass_late_warn_uninitialized".
 
 So, we should be able to check whether an auto-variable has initializer or 
 not after “pass_late_warn_uninitialized”,
 If Not, then insert an initialization for it.
 
 For this purpose, I guess that “FOR_EACH_LOCAL_DECL” might be better?
>>> 
>>> Yes, but do you want to catch variables promoted to register as well
>>> or just variables
>>> on the stack?
>> I think both as long as they are source-level auto-variables. Then which one 
>> is better?
>>> 
 Another issue is, in order to check whether an auto-variable has 
 initializer, I plan to add a new bit in “decl_common” as:
  /* In a VAR_DECL, this is DECL_IS_INITIALIZED.  */
  unsigned decl_is_initialized :1;
 
 /* IN VAR_DECL, set when the decl is initialized at the declaration.  */
 #define DECL_IS_INITIALIZED(NODE) \
  (DECL_COMMON_CHECK (NODE)->decl_common.decl_is_initialized)
 
 set this bit when setting DECL_INITIAL for the variables in FE. then keep 
 it
 even though DECL_INITIAL might be NULLed.
>>> 
>>> For locals it would be more reliable to set this flag during gimplification.
>> You mean I can set the flag “DECL_IS_INITIALIZED (decl)”  inside the routine 
>> “gimpley_decl_expr” (gimplify.c) as following:
>>   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>> {
>>   tree init = DECL_INITIAL (decl);
>> ...
>>   if (init && init != error_mark_node)
>> {
>>   if (!TREE_STATIC (decl))
>>  {
>>DECL_IS_INITIALIZED(decl) = 1;
>>  }
>> Is this enough for all Frontends? Are there other places that I need to 
>> maintain this bit?
>>> 
 Do you have any comment and suggestions?
>>> 
>>> As said above - do you want to cover registers as well as locals?
>> All the locals from the source-code point of view should be covered.   (From 
>> my study so far,  looks like that Clang adds that phase in FE).
>> If GCC adds this phase in FE, then the following design requirement
>> C. The implementation needs to keep the current static warning on 
>> uninitialized
>> variables untouched in order to avoid "forking the language”.
>> cannot be satisfied.  Since gcc’s uninitialized variables analysis is 
>> applied quite late.
>> So, we have to add this new phase after “pass_late_warn_uninitialized”.
>>>  I'd do
>>> the actual zeroing during RTL expansion instead since otherwise you
>>> have to figure youself whether a local is actually used (see 
>>> expand_stack_vars)
>> Adding  this new transformation during RTL expansion is okay.  I will check 
>> on this in more details to see how to add it to RTL expansion phase.
>>> 
>>> Note that optimization will already made have use of "uninitialized" state
>>> of locals so depending on what the actual goal is here "late" may be too 
>>> late.
>> 

Re: [PATCH] Remove redundant builtins for avx512f scalar instructions.

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/12/20 11:21 PM, Hongyu Wang wrote:
> Hi
>
> Thanks for reminding me about this patch. I didn't remove any existing
> intrinsics, just remove redundant builtin functions that end-users
> would not likely to use.
>
> Also I'm OK to keep current implementation, in case there might be
> someone using the builtin directly.
That seems wise -- we can't reasonably predict if users are using those
builtins directly. 

So if we can clean things up and keep the redundant builtins that seems
best.  Or just leave things as-is. 

The other possibility would be to deprecate the redundant builtins this
release and remove them in gcc-12.  I haven't looked at how difficult
that might be, but the idea here would be to give users a warning if
they use those builtins directly and enough time to resolve the issue
before we remove them.

jeff



[PATCH] changelog: add hint for a file mismatch

2020-11-30 Thread Martin Liška

Pushed to master. It's supposed to provide a hint, e.g.

ERR: unchanged file mentioned in a ChangeLog (did you mean 
"gcc/testsuite/gfortran.dg/goacc-gomp/free-1.f90"?):"gcc/testsuite/gfortran.dg/goacc-gomp/free-1.f"

Martin


contrib/ChangeLog:

* gcc-changelog/git_commit.py: Suggest close file for
'unchanged file mentioned in a ChangeLog' error.
* gcc-changelog/test_email.py: Test it.
---
 contrib/gcc-changelog/git_commit.py | 4 
 contrib/gcc-changelog/test_email.py | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/gcc-changelog/git_commit.py 
b/contrib/gcc-changelog/git_commit.py
index 57fba756d32..0c438165516 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -16,6 +16,7 @@
 # along with GCC; see the file COPYING3.  If not see
 # .  */
 
+import difflib

 import os
 import re
 
@@ -576,6 +577,9 @@ class GitCommit:

 changed_files = set(cand)
 for file in sorted(mentioned_files - changed_files):
 msg = 'unchanged file mentioned in a ChangeLog'
+candidates = difflib.get_close_matches(file, changed_files, 1)
+if candidates:
+msg += f' (did you mean "{candidates[0]}"?)'
 self.errors.append(Error(msg, file))
 for file in sorted(changed_files - mentioned_files):
 if not self.in_ignored_location(file):
diff --git a/contrib/gcc-changelog/test_email.py 
b/contrib/gcc-changelog/test_email.py
index 48ecc1ee6a6..8f5129edb12 100755
--- a/contrib/gcc-changelog/test_email.py
+++ b/contrib/gcc-changelog/test_email.py
@@ -113,7 +113,9 @@ class TestGccChangelog(unittest.TestCase):
 email = self.from_patch_glob('0096')
 assert email.errors
 err = email.errors[0]
-assert err.message == 'unchanged file mentioned in a ChangeLog'
+assert err.message == 'unchanged file mentioned in a ChangeLog (did ' \
+'you mean "gcc/testsuite/gcc.target/aarch64/' \
+'advsimd-intrinsics/vdot-3-1.c"?)'
 assert err.line == 'gcc/testsuite/gcc.target/aarch64/' \
'advsimd-intrinsics/vdot-compile-3-1.c'
 
--

2.29.2



Re: [24/32] module mapper

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/12/20 9:24 AM, Nathan Sidwell wrote:
> On 11/3/20 4:17 PM, Nathan Sidwell wrote:
>> this is the module mapper client and server pieces.  It features a
>> default resolver that can read a text file, or generate default
>> mappings from module name to cmi name.
>
> Richard rightly suggested on IRC that the sample server for the module
> mapper shouldn't be in the gcc/cp dir.  It happened to be that way
> because it started out much more closely coupled, but then it grew legs.
>
> So this patch creates a new c++tools toplevel directory and places the
> mapper-server and its default resolver there.  That means more changes
> to the toplevel Makefile.def and Makefile.tpl (I've not included the
> regenerated Makefile.in, nor other generated files in gcc/ and
> c++tools in this diff.)
>
> We still need to build the default resolver when building cc1plus, and
> I've placed mapper-resolver.cc there, as a simple #include forwarder
> to the source in c++tools.  I also replace 'gcc/cp/mapper.h' with a
> client-specific 'gcc/cp/mapper-client.h'.  (mapper-client is only
> linked into cc1plus, so gcc/cp seems the right place for it.)
>
> The sample server relies on gcc/version.o to pick up its version
> number, and I place it in the libexecsubdir that we place cc1plus.  I
> wasn't comfortable placing it in the install location of g++ itself. 
> I call it a sample server for a reason :)
>
> I will of course provide changelog when committing.
>
> nathan
>
>
> 24a-c++-mapper.diff
>


So I think you should just own these bits.  You're going to be far more
familiar with them than anyone else involved in GCC work :-)  So, OK for
the trunk as well as any followups into the module mapper.

jeff



Re: [PATCH][PR target/97770] x86: Add missing popcount2 expander

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/11/20 6:54 PM, Hongyu Wang via Gcc-patches wrote:
> Hi,
>
> According to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97770, x86
> backend need popcount2 expander so __builtin_popcount could be
> auto vectorized with AVX512BITALG/AVX512VPOPCNTDQ targets.
>
> For DImode the middle-end vectorizer could not generate expected code,
> and for QI/HImode there is no corresponding IFN, xfails are added for
> these tests.
>
> Bootstrap/regression test for x86 backend is OK.
>
> OK for master?
>
> gcc/ChangeLog
>
> PR target/97770
> * gcc/config/i386/sse.md (popcount2): New expander
> for SI/DI vector modes.
> (popcount2): Likewise for QI/HI vector modes.
>
> gcc/testsuite/ChangeLog
>
> PR target/97770
> * gcc.target/i386/avx512bitalg-pr97770-1.c: New test.
> * gcc.target/i386/avx512vpopcntdq-pr97770-1.c: Likewise.
> * gcc.target/i386/avx512vpopcntdq-pr97770-2.c: Likewise.
> * gcc.target/i386/avx512vpopcntdqvl-pr97770-1.c: Likewise.
>
>
> 0001-Add-popcount-mode-expander-to-enable-popcount-auto-v.patch
>
OK.  Presumably once this is applied Richi is going to  look at the
higher level issues in the vectorizer which inhibit creating the HI/QI
vector popcounts?

Jeff



[committed] Fix non-unique testnames

2020-11-30 Thread Jeff Law via Gcc-patches
This patch fixes a handful of tests with non-unique names which confuse
the living hell out of compare_tests, particularly if one of two tests
[x]fail while the other is [x]pass which compare_tests will flag as a
regression each and every run.

No doubt there's a lot more of these lying around and I'm going to be
watching for this much more closely going forward.

Committing to the trunk,

Jeff
commit e40fece7c9b3731f4cff060f712c132ff100cab4
Author: Jeff Law 
Date:   Mon Nov 30 08:59:23 2020 -0700

Fix non-unique testnames

gcc/testsuite

* g++.dg/warn/Wnonnull5.C: Fix non-unique testnames.
* g++.dg/warn/Wplacement-new-size-8.C: Likewise.

diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull5.C 
b/gcc/testsuite/g++.dg/warn/Wnonnull5.C
index 8b25d2d9f86..78862d48993 100644
--- a/gcc/testsuite/g++.dg/warn/Wnonnull5.C
+++ b/gcc/testsuite/g++.dg/warn/Wnonnull5.C
@@ -36,7 +36,7 @@ struct S
 void warn_nullptr_this ()
 {
   ((S*)nullptr)->f0 ("");// { dg-warning "3:'this' pointer null" 
"pr86568" { xfail *-*-* } }
- // { dg-warning "this' pointer null" 
"pr86568" { target *-*-* } .-1 }
+ // { dg-warning "this' pointer null" "pr86568 
second variant" { target *-*-* } .-1 }
 }
 
 void warn_null_this_cst ()
@@ -49,15 +49,15 @@ void warn_null_this_var ()
 {
   S* null = 0;
   null->f2 ();   // { dg-warning "3:'this' pointer null" 
"pr86568" { xfail *-*-* } }
-  // { dg-warning "'this' pointer null" 
"pr86568" { target *-*-* } .-1 }
+  // { dg-warning "'this' pointer null" 
"pr86568 second variant" { target *-*-* } .-1 }
 }
 
 void warn_nullptr (S s)
 {
   s.f3 (nullptr, );  // { dg-warning "9:argument 1 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-   // { dg-warning "argument 1 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+   // { dg-warning "argument 1 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
   s.f3 (, nullptr);  // { dg-warning "13:argument 2 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-   // { dg-warning "argument 2 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+   // { dg-warning "argument 2 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
 }
 
 
@@ -72,9 +72,9 @@ void warn_null_var (S s)
 {
   void* null = 0;
   s.f5 (null, );// { dg-warning "9:argument 1 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-  // { dg-warning "argument 1 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+  // { dg-warning "argument 1 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
   s.f5 (, null);// { dg-warning "16:argument 2 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-  // { dg-warning "argument 2 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+  // { dg-warning "argument 2 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
 }
 
 void warn_null_cond (S s, void *null)
@@ -83,9 +83,9 @@ void warn_null_cond (S s, void *null)
 return;
 
   s.f6 (null, );// { dg-warning "9:argument 1 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-  // { dg-warning "argument 1 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+  // { dg-warning "argument 1 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
   s.f6 (, null);// { dg-warning "13:argument 2 null where 
non-null expected" "pr86568" { xfail *-*-* } }
-  // { dg-warning "argument 2 null where 
non-null expected" "pr86568" { target *-*-* } .-1 }
+  // { dg-warning "argument 2 null where 
non-null expected" "pr86568 second variant" { target *-*-* } .-1 }
 }
 
 
diff --git a/gcc/testsuite/g++.dg/warn/Wplacement-new-size-8.C 
b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-8.C
index 77bd3314a19..12cd4cda89d 100644
--- a/gcc/testsuite/g++.dg/warn/Wplacement-new-size-8.C
+++ b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-8.C
@@ -43,7 +43,7 @@ void test_cst_off ()
 /* Offsets are treated as signed so SIZE_MAX is indistinguishable
from -1.  */
 char ca1[1];// { dg-message "at offset \\d+ from 'ca1' 
declared here" "note" { xfail *-*-* } }
-// { dg-message "at offset -1 from 'ca1' 
declared here" "note" { target *-*-* } .-1 }
+// { dg-message "at offset -1 

Re: [PATCH 25/31] VAX: Fix predicates for widening multiply and multiply-add insns

2020-11-30 Thread Maciej W. Rozycki
On Fri, 20 Nov 2020, Jeff Law wrote:

> ps.  Yes, I skipped the insv/extv changes.  They're usually a rats nest
> of special cases.  We'll come back to them.

 I've thought of actually reducing the number of patterns to the minimum 
possible by folding the existing ones together, and then getting the 
alternatives sorted by fine-grained constraints at reload.

 There is that complication caused by INSV machine instruction preserving 
condition codes (understandably), so keeping it together with alternative 
code sequences that do set the codes in a single RTL insn would cause 
trouble with getting a matching insn for the comparison elimination pass.  
Or so I think.

 As you say, we can sort it later.  It's an optimisation only anyway, the 
VAX EXTV/EXTZV/INSV machine instructions are flexible enough to handle 
just about anything, except that reportedly at a slow pace, at least with 
some implementations (I'd have to review that NVAX manual for the details, 
but it's hefty 1022 pages).  So at worst we could use those instructions 
in all cases.

  Maciej


Re: [PATCH] ipa: dump symtab to emergency dump file

2020-11-30 Thread Jan Hubicka
> 
> 
> On 11/30/20 7:04 AM, Martin Liška wrote:
> > Hi.
> >
> > It's handy to have symbol table when we dump emergency dump.
> > It's likely next stage1 material.
> >
> > Thoughts?
> > Martin
> >
> > gcc/ChangeLog:
> >
> >     * passes.c (emergency_dump_function): Dump symtab when
> >     we are in an IPA pass.
> Your call now or next stage1.   We should keep in mind that if the
> symtab structures are inconsistent/corrupted that we can ICE or worse
> yet spin while dumping the symtab.

We already dump function bodies that has same problem, so I think
dumping symtab is fine (and I have no problem with it going to trunk
this stage3)

Honza
> 
> Jeff
> 


Re: [PATCH] ipa: dump symtab to emergency dump file

2020-11-30 Thread Jeff Law via Gcc-patches



On 11/30/20 7:04 AM, Martin Liška wrote:
> Hi.
>
> It's handy to have symbol table when we dump emergency dump.
> It's likely next stage1 material.
>
> Thoughts?
> Martin
>
> gcc/ChangeLog:
>
>     * passes.c (emergency_dump_function): Dump symtab when
>     we are in an IPA pass.
Your call now or next stage1.   We should keep in mind that if the
symtab structures are inconsistent/corrupted that we can ICE or worse
yet spin while dumping the symtab.

Jeff



[PATCH] dse: Cope with bigger-than-integer modes [PR98037]

2020-11-30 Thread Richard Sandiford via Gcc-patches
dse.c:find_shift_sequence tries to represent a store and load
back as a shift right followed by a truncation.  It therefore
needs to find an integer mode in which to do the shift right.
The loop it uses has the form:

  FOR_EACH_MODE_FROM (new_mode_iter,
  smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))

which implicitly assumes that read_mode has an equivalent integer mode.
As shown in the testcase, not all modes have such an integer mode.

This patch just makes the code start from the smallest integer mode and
skip modes that are too small.  The loop already breaks at the first
mode wider than word_mode.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
GCC 10 branch?

Richard


gcc/
PR rtl-optimization/98037
* dse.c (find_shift_sequence): Iterate over all integers and
skip modes that are too small.

gcc/testsuite/
PR rtl-optimization/98037
* gcc.target/aarch64/sve/acle/general/pr98037.c: New test.
---
 gcc/dse.c   | 5 +++--
 gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c | 6 ++
 2 files changed, 9 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c

diff --git a/gcc/dse.c b/gcc/dse.c
index d65266b5476..651e6e7e71e 100644
--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -1757,8 +1757,7 @@ find_shift_sequence (poly_int64 access_size,
  the machine.  */
 
   opt_scalar_int_mode new_mode_iter;
-  FOR_EACH_MODE_FROM (new_mode_iter,
- smallest_int_mode_for_size (GET_MODE_BITSIZE (read_mode)))
+  FOR_EACH_MODE_IN_CLASS (new_mode_iter, MODE_INT)
 {
   rtx target, new_reg, new_lhs;
   rtx_insn *shift_seq, *insn;
@@ -1767,6 +1766,8 @@ find_shift_sequence (poly_int64 access_size,
   new_mode = new_mode_iter.require ();
   if (GET_MODE_BITSIZE (new_mode) > BITS_PER_WORD)
break;
+  if (maybe_lt (GET_MODE_SIZE (new_mode), GET_MODE_SIZE (read_mode)))
+   continue;
 
   /* Try a wider mode if truncating the store mode to NEW_MODE
 requires a real instruction.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c 
b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
new file mode 100644
index 000..b91e940b18e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr98037.c
@@ -0,0 +1,6 @@
+/* { dg-options "-msve-vector-bits=1024 -O3" } */
+
+typedef __SVInt8_t vec __attribute__((arm_sve_vector_bits(1024)));
+struct pair { vec v[2]; };
+void use (struct pair *);
+vec f (struct pair p) { vec v = p.v[1]; use (); return v; }


Re: [PATCH] match.pd: Use ranges to optimize some x * y / y to x [PR97997]

2020-11-30 Thread Andrew MacLeod via Gcc-patches

On 11/26/20 3:52 AM, Jakub Jelinek wrote:

Hi!

For signed integers with undefined overflow we already optimize x * y / y
into x, but for signed integers with -fwrapv or unsigned integers we don't.
The following patch allows optimizing that into just x if value ranges
prove that x * y will never overflow.
It uses the global SSA_NAME_RANGE_INFO only, because like mentioned
in another PR we don't currently have a way to tell the ranger from match.pd
the use stmt (and we'd need in that case to tell ranger to only follow
SSA_NAME_DEF_STMTs + SSA_NAME_RANGE_INFO and never go in the other
direction, as following immediate uses seems forbidden in match.pd).


as an FYI, ranger only uses immediate-uses to try to track non-null 
pointer references. so

   a) if its not a pointer, it'll never follow immediate uses, and
   b) we can look at disabling that functionality as needed, or better 
yet, replace the non-null pointer processing facility to eventually not 
need immediate_uses.  I will add that to the worklist.


Andrew



[PATCH] VAX: Fix the LTO compiler downgrading code to non-PIC model

2020-11-30 Thread Maciej W. Rozycki
Fix a testsuite failure:

/tmp/ccL65Mmt.s: Assembler messages:
/tmp/ccL65Mmt.s:36: Warning: Symbol n used as immediate operand in PIC mode.
FAIL: gcc.dg/lto/pr55660 c_lto_pr55660_0.o-c_lto_pr55660_1.o link, -O0 -flto 
-flto-partition=none -fuse-linker-plugin

where non-PIC code is substituted by the LTO compiler at the link stage 
for what used to be PIC code in the original compilation.  This happens 
because in the de-facto VAX ELF psABI we rely on code being PIC for GOT 
support in dynamic executables and arrange that by having `-fPIC' passed 
to the compiler by default by means of a specs recipe.

That is however canceled where the LTO wrapper is used, by an internal 
arrangement in the LTO compiler that clears the PIC flag whenever the 
`-flinker-output=exec' option has been used.  This has been deliberately 
introduced with commit 1ff9ed6fb282 ("re PR lto/67548 (LTO drops weak 
binding with "ld -r")")[1]:

"In the log of PR67548 HJ actually pointed out that we do have API at 
linker plugin side which says what type of output is done.  This is cool 
because we can also use it to drop -fpic when building static binary. 
This is common in Firefox, where some objects are built with -fpic and 
linked to both binaries and libraries."

with this code:

case LTO_LINKER_OUTPUT_EXEC: /* Normal executable */
  flag_pic = 0;
  flag_pie = 0;
  flag_shlib = 0;
  break;

Consequently code like:

.L6:
addl3 -8(%fp),$n,%r0
pushl %r0
calls $1,foo
addl2 %r0,-12(%fp)
incl -8(%fp)
.L5:

is produced by the LTO compiler, where a reference to `n' is used that 
is invalid in PIC code, because it uses the immediate addressing mode, 
denoted by the `$' prefix.

For that not to happen we must never pass `-flinker-output=exec' to the 
LTO compiler unless non-PIC code has been explicitly requested.  Using 
`-flinker-output=dyn' except for relocatable output would seem the 
simplest approach, as it does not fiddle with any of the internal code 
model settings beyond what the command-line options have arranged and 
therefore lets them remain the same as with the original compilation, 
but it breaks as well causing PR lto/69866 to retrigger, as that code 
seems sensitive to `flag_shlib':

lto1: internal compiler error: in add_symbol_to_partition_1, at 
lto/lto-partition.c:152
0x105be1cb add_symbol_to_partition_1
.../gcc/lto/lto-partition.c:152
0x105be443 add_symbol_to_partition_1
.../gcc/lto/lto-partition.c:194
0x105be80f add_symbol_to_partition
.../gcc/lto/lto-partition.c:270
0x105bee6f add_sorted_nodes
.../gcc/lto/lto-partition.c:395
0x105c0903 lto_balanced_map(int, int)
.../gcc/lto/lto-partition.c:815
0x105aa91f do_whole_program_analysis
.../gcc/lto/lto.c:499
0x105aac97 lto_main()
.../gcc/lto/lto.c:637
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
lto-wrapper: fatal error: .../gcc/xgcc returned 1 exit status
compilation terminated.
.../usr/bin/vax-netbsdelf-ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: gcc.dg/lto/pr69866 c_lto_pr69866_0.o-c_lto_pr69866_1.o link, -O0 -flto 
-fuse-linker-plugin -fno-fat-lto-objects  (internal compiler error)

Substitute `-flinker-output=pie' for `-flinker-output=exec' in the specs 
then unless `-no-pie' has also been used, preserving the original intent 
of emitting PIC code by default for executables while keeping the linker 
arrangement unchanged.  The LTO compiler uses the `cc1' spec, so keep 
`cc1plus' unmodified.

This makes code like:

.L6:
movab n,%r0
addl2 -8(%fp),%r0
pushl %r0
calls $1,foo
addl2 %r0,-12(%fp)
incl -8(%fp)
.L5:

be produced instead corresponding to the fragment quoted above, which is 
valid PIC code as it uses the PC-relative addressing mode denoted by the 
absence of a prefix to `n' (which can be redirected to GOT as required, 
by changing the addressing mode to PC-relative indirect in the operand 
specifier).

Ideally we would instead default to the PIE model for executables, but 
that triggers a BFD bug where for a change the LTO wrapper is not used:

.../usr/bin/vax-netbsdelf-ld: /tmp/ccV2sWQt.ltrans0.ltrans.o: warning: GOT 
addend of 3 to `n' does not match previous GOT addend of 0
FAIL: gcc.dg/lto/pr55660 c_lto_pr55660_0.o-c_lto_pr55660_1.o link, -O2 -flto 
-flto-partition=1to1 -fno-use-linker-plugin

which is due to assembly code like:

main:
.word 0
subl2 $4,%sp
movab n,%r0
movab n+3,%r2
clrl %r3
movb $98,%r1
.L4:

and consequently object code like:

 :
   0:   00 00   .word 0x # Entry mask: < >
   2:   c2 04 5esubl2 $0x4,sp
   5:   9e ef 00 00 movab b ,r0
   9:   00 00 50 
7: R_VAX_GOT32  n
   c:   9e ef 

[committed] libstdc++: Add new C++20 headers to Doxygen settings

2020-11-30 Thread Jonathan Wakely via Gcc-patches
This doesn't actually have any effect unless you also change the
predefined value of __cplusplus, as it's currently 201703L. But if
somebody does want to do that, the new headers will get processed now.

libstdc++-v3/ChangeLog:

* doc/doxygen/user.cfg.in (INPUT): Add  and .

Tested x86_64-linux. Committed to trunk.

commit 82ac923da681dbde2af1bdb353053ec9ee8ef18b
Author: Jonathan Wakely 
Date:   Mon Nov 30 15:02:03 2020

libstdc++: Add new C++20 headers to Doxygen settings

This doesn't actually have any effect unless you also change the
predefined value of __cplusplus, as it's currently 201703L. But if
somebody does want to do that, the new headers will get processed now.

libstdc++-v3/ChangeLog:

* doc/doxygen/user.cfg.in (INPUT): Add  and .

diff --git a/libstdc++-v3/doc/doxygen/user.cfg.in 
b/libstdc++-v3/doc/doxygen/user.cfg.in
index 320f6dea6881..1966055e675a 100644
--- a/libstdc++-v3/doc/doxygen/user.cfg.in
+++ b/libstdc++-v3/doc/doxygen/user.cfg.in
@@ -870,6 +870,7 @@ INPUT  = @srcdir@/doc/doxygen/doxygroups.cc 
\
  include/iostream \
  include/istream \
  include/iterator \
+ include/latch \
  include/limits \
  include/list \
  include/locale \
@@ -887,6 +888,7 @@ INPUT  = @srcdir@/doc/doxygen/doxygroups.cc 
\
  include/ratio \
  include/regex \
  include/scoped_allocator \
+ include/semaphore \
  include/set \
  include/shared_mutex \
  include/span \


Re: [committed] libstdc++: Set dg-timeout-factor for some slow tests

2020-11-30 Thread Jonathan Wakely via Gcc-patches

On 26/11/20 16:25 +, Jonathan Wakely wrote:

These tests are very, very slow to compile. If the testsuite is run with
a low tool_timeout value they are likely to fail. By adding a
multiplication factor to those tests, it's still possible to use a low
timeout without spurious failures.

libstdc++-v3/ChangeLog:

* testsuite/28_regex/algorithms/regex_match/basic/string_range_01_03.cc:
Add dg-timeout-factor directive.
* testsuite/28_regex/algorithms/regex_match/cstring_bracket_01.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/ecma/char/backref.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/63199.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/anymatcher.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/cjk_match.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/hex.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_match/extended/wstring_locale.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_search/61720.cc: Likewise.
* testsuite/28_regex/algorithms/regex_search/ecma/assertion.cc:
Likewise.
* testsuite/28_regex/algorithms/regex_search/ecma/string_01.cc:
Likewise.
* testsuite/28_regex/basic_regex/ctors/deduction.cc: Likewise.



I've been experimenting with much shorter tool_timeout values for the
testsuite. We already knew that the PSTL and  tests are slow,
and indeed they are more prone to FAILing if the default timeout is
too short.

This adds dg-timeout-factor to lots more tests, so that it's practical
to use "set tool_timeout 120" or even "set tool_timeout 60" without
too many problems.

Tested powerpc64le-linux, committed to trunk.


commit b6a8e3479ea924c424a4939d7129d744a9c7b273
Author: Jonathan Wakely 
Date:   Mon Nov 30 14:39:54 2020

libstdc++: Set dg-timeout-factor for more slow tests

As in r11-5449, this adds a muliplier to the timeout for slow tests.
This covers the majority of the  and PSTL tests.

libstdc++-v3/ChangeLog:

* testsuite/20_util/specialized_algorithms/pstl/*: Add
dg-timeout-factor.
* testsuite/25_algorithms/pstl/*: Likewise.
* testsuite/26_numerics/pstl/*: Likewise.
* testsuite/28_regex/*: Likewise.

diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_construct.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_construct.cc
index 4f1911e51f32..a6dbe69b60cc 100644
--- a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_construct.cc
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_construct.cc
@@ -1,6 +1,7 @@
 // -*- C++ -*-
 // { dg-options "-std=gnu++17 -ltbb" }
 // { dg-do run { target c++17 } }
+// { dg-timeout-factor 3 }
 // { dg-require-effective-target tbb-backend }
 
 //===-- uninitialized_construct.pass.cpp --===//
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_copy_move.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_copy_move.cc
index 2ecfde28591e..48b6a6a0ce3d 100644
--- a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_copy_move.cc
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_copy_move.cc
@@ -1,6 +1,7 @@
 // -*- C++ -*-
 // { dg-options "-std=gnu++17 -ltbb" }
 // { dg-do run { target c++17 } }
+// { dg-timeout-factor 3 }
 // { dg-require-effective-target tbb-backend }
 
 //===-- uninitialized_copy_move.pass.cpp --===//
diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_fill_destroy.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_fill_destroy.cc
index 31ea484897b9..16a6ef24f962 100644
--- a/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_fill_destroy.cc
+++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/pstl/uninitialized_fill_destroy.cc
@@ -1,6 +1,7 @@
 // -*- C++ -*-
 // { dg-options "-std=gnu++17 -ltbb" }
 // { dg-do run { target c++17 } }
+// { dg-timeout-factor 3 }
 // { dg-require-effective-target tbb-backend }
 
 //===-- uninitialized_fill_destroy.pass.cpp ---===//
diff --git a/libstdc++-v3/testsuite/25_algorithms/pstl/alg_merge/inplace_merge.cc b/libstdc++-v3/testsuite/25_algorithms/pstl/alg_merge/inplace_merge.cc
index a20bdf9afa68..74af960901a2 100644
--- a/libstdc++-v3/testsuite/25_algorithms/pstl/alg_merge/inplace_merge.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/pstl/alg_merge/inplace_merge.cc
@@ -1,6 +1,7 @@
 // -*- C++ -*-
 // { dg-options "-std=gnu++17 -ltbb" }
 // { dg-do run { target c++17 } }
+// { dg-timeout-factor 3 }
 // { dg-require-effective-target 

Re: [RFC] Decrease default timeout for libstdc++ tests to 6 minutes

2020-11-30 Thread Jonathan Wakely via Gcc-patches

On 27/11/20 21:17 +0100, Christophe Lyon via Libstdc++ wrote:

On Fri, 27 Nov 2020 at 17:13, Jonathan Wakely via Gcc-patches
 wrote:


The default for the GCC testsuite is 300, i.e. 5 minutes, which is the
same as the DejaGnu default.

Libstdc++ overrides this to 600, i.e. 10 minutes.

This seems ridiculously long. If any test takes that long on modern
hardware, something is wrong. We've seen this a lot recently with
buggy tests, and waiting for them to FAIL is tedious.

I've already made libstdc++.exp respect the user's setting in
~/.dejagnurc or the global site.exp file. This means anybody testing
on slow simulators or old hardware can choose their own timeout.

I've added dg-timeout-factor to the slowest std::regex tests and have
a patch to do it for the PSTL tests, which both take far too long to
compile. That means you can choose a sensible timeout appropriate for
most tests (e.g. 60 seconds) and not get spurious failures from the
few dozen tests which are just very slow.

I'd like to change the default to 6 minutes. If that goes well, I'd
like to lower it even further.

The main benefit of this will be that buggy tests which hang will get
killed sooner, so we waste less time waiting for the inevitable
timeout.



I think that's a good idea, I did have problems sometimes when
many tests timed out, causing the whole 'make check' to be
killed before completion by our compute farm management system.


Thanks for the feedback. I've pushed this patch now.

It's been tested on powercp64le-linux, x86_64-linux, aarch64-linux,
sparc-solaris and powerpc-aix. They were all fine with much lower
defaults (e.g. 120 seconds). Let's see how this goes for people
testing on older or less powerful hardware.


commit 637800c7bbb222a629076a5d20afda03c2dd1043
Author: Jonathan Wakely 
Date:   Mon Nov 30 14:39:54 2020

libstdc++: Reduce default test timeout to 360 seconds

The current default of 10 minutes is much longer than most tests need on
common hardware. The slow tests all now have a dg-timeout-factor
directive that gives them more time to run relative to the default. The
default can also be overridden in ~/.dejagnurc or DEJAGNU=site.exp, so
it seems unnecessary to have such a large default.

This reduces the default from 10 minutes to 6 minutes, which still seems
more than enough.

libstdc++-v3/ChangeLog:

* testsuite/lib/libstdc++.exp (libstdc++_init): Reduce
default tool_timeout to 360.

diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index e000dba968f7..35817a8870a4 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -248,7 +248,7 @@ proc libstdc++_init { testfile } {
 # Set the default timeout for v3 tests.
 # You can override this in ~/.dejagnurc or a .exp file named by $DEJAGNU.
 if {![info exists tool_timeout]} {
-  set tool_timeout 600
+  set tool_timeout 360
 }
 
 # Default settings.


Re: Fortran: With OpenACC, ignore OpenMP's cond comp sentinels [PR98011]

2020-11-30 Thread Tobias Burnus

... and now committed as r11-5572-g1d6f6ac693a8601bef9fe4ba72eb6fbf7b60b5cd.

Thanks again for the suggestions!

Tobias

On 27.11.20 23:14, Tobias Burnus wrote:

On 27.11.20 18:31, Jakub Jelinek via Fortran wrote:

Depends on what does the OpenACC standard say.
If it has similar wording to OpenMP that '!$ ' stands ...


It only has '!$acc' (free) and !$acc + c$acc + *$acc (fixed).
cf.
https://www.openacc.org/sites/default/files/inline-images/Specification/OpenACC-3.1-final.pdf
(2.1 Directive Form)


.. if it is silent on that, then the patch is correct.

What about fixed-form parsing?


Missed that somehow. I have now added two fixed-form testcases
(goacc + goacc-gomp) and a free one (goacc-gomp).

Thanks,

Tobias


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter


Updating the backend status for h8300 on the wiki

2020-11-30 Thread John Paul Adrian Glaubitz
Hi!

Now that h8300 has been converted to use MODE_CC, it might be a good idea
to update the documentation on the wiki [1] which still lists h8300 as
being cc0.

Adrian

> [1] https://gcc.gnu.org/backends.html

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


Re: [Patch] Fortran: -fno-automatic and -fopenacc / recusion check cleanup

2020-11-30 Thread Tobias Burnus

Now installed as r11-5571-gf4e7ea81d1369d4d6cb6d8e440aefb3407142e05.

Tobias

On 27.11.20 11:16, Tobias Burnus wrote:

Two fixes – simple, see patch + commit text.

Longer description:

* options:
Background:
- OpenMP, OpenACC and imply that a function is called
concurrently and -frecursive implies recusive. In all those
cases, the code may fail if a local variable is in static memory
instead of stack or heap. – If a user specified 'save', we
can assume/hope that they will deal with it - but with
-fno-automatic (→ 'save' implied), the flags clash.

- Additionally, to avoid placing local arrays in static memory,
for -fopenmp/-fopenacc -frecursive and 'unlimited' stack size
use for const-size arrays is implied.

This patch:
- Handle OpenACC as OpenMP (before it didn't imply -frecursive.


* Recursive run-time check. The current code currently generates:

  subroutine foo()
logical, save :: currently_called = .false.
if (currently_called) error_stop "No recursive but called"
currently_called = .true.
...
... ! Rest of code, which could indirectly call this proc again
...
currently_called = .false.
  end

This works well for recursive calls but less so for concurrency
(→ OpenMP, OpenACC).
As noted above, by default OpenMP/OpenACC implies -frecursive
and, hence, there is no recursive check generated.
The question is what code should be generated for, e.g.
  -fno-automatic -fopenmp or -fopenacc -fmax-stack-var-size=20

In that case, -frecursive is unset. We have two choices:
- Either still always reset, which may not detect concurrent
  use (including recursive + concurrent use) do to a race condition.
- Or avoid resetting the flag
  But then calling the procedure twice (e.g. beginning + end of the
  program) will generate a bogus error.

The current code does the second – at least for -fopenmp.

→ PATCH: Simply use the same condition twice instead of a complicated
  test; do so via: 'if (recurcheckvar == NULL_TREE)'.

Current code:

tree recurcheckvar = NULL_TREE;
...
  if ((gfc_option.rtcheck & GFC_RTCHECK_RECURSION)
  && !is_recursive && !flag_recursive && !sym->attr.artificial)
...  // declare 'recurcheckvar', generate error-message code etc.
...
  /* Reset recursion-check variable.  */
  if ((gfc_option.rtcheck & GFC_RTCHECK_RECURSION)
   && !is_recursive && !flag_openmp && recurcheckvar != NULL_TREE)


Comments? Suggestions? – If not, I will commit it as obvious in the next
days.

Tobias


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter


[Patch, committed] Fortran's dump-parse-tree.c: Use '==' not '=' for '.eq.'.

2020-11-30 Thread Tobias Burnus

I was a bit confused by the '=' (assignment?) in the dump; hence,
in line with /=, >= etc. it now is '==' for '==' or '.eq.'.

Committed as r11-5570-g2610c786f7496c5006bb68d6801ef7450bd231a9

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
commit 2610c786f7496c5006bb68d6801ef7450bd231a9
Author: Tobias Burnus 
Date:   Mon Nov 30 15:19:39 2020 +0100

Fortran's dump-parse-tree.c: Use '==' not '=' for '.eq.'.

gcc/fortran/
* dump-parse-tree.c (show_expr): Use '==' not '=' for '.eq.'.

diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index cab0fb2979f..1012b11fb98 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -647,9 +647,9 @@ show_expr (gfc_expr *p)
 	  fputs ("NEQV ", dumpfile);
 	  break;
 	case INTRINSIC_EQ:
 	case INTRINSIC_EQ_OS:
-	  fputs ("= ", dumpfile);
+	  fputs ("== ", dumpfile);
 	  break;
 	case INTRINSIC_NE:
 	case INTRINSIC_NE_OS:
 	  fputs ("/= ", dumpfile);


Re: [PATCH] c++, debug: Treat -std=c++20 -gdwarf-5 like C++14 rather than C++98

2020-11-30 Thread Richard Biener via Gcc-patches
On Mon, Nov 30, 2020 at 3:12 PM Jakub Jelinek via Gcc-patches
 wrote:
>
> Hi!
>
> I have noticed that while we use DW_LANG_C_plus_plus_14 for -std=c++17 
> -gdwarf-5,
> we use DW_LANG_C_plus_plus (aka C++98) for -std=c++20 -gdwarf-5.  The
> following patch makes those two match.
>
> Ok for trunk if it passes bootstrap/regtest?

OK.

> 2020-11-30  Jakub Jelinek  
>
> * dwarf2out.c (gen_compile_unit_die): Treat GNU C++20
> like C++14 for -gdwarf-5.
>
> * g++.dg/debug/dwarf2/lang-cpp17.C: New test.
> * g++.dg/debug/dwarf2/lang-cpp20.C: New test.
>
> --- gcc/dwarf2out.c.jj  2020-11-26 16:22:29.111352844 +0100
> +++ gcc/dwarf2out.c 2020-11-30 15:05:52.601317310 +0100
> @@ -24646,7 +24646,8 @@ gen_compile_unit_die (const char *filena
> language = DW_LANG_C_plus_plus_11;
>   else if (strcmp (language_string, "GNU C++14") == 0)
> language = DW_LANG_C_plus_plus_14;
> - else if (strcmp (language_string, "GNU C++17") == 0)
> + else if (strcmp (language_string, "GNU C++17") == 0
> +  || strcmp (language_string, "GNU C++20") == 0)
> /* For now.  */
> language = DW_LANG_C_plus_plus_14;
> }
> --- gcc/testsuite/g++.dg/debug/dwarf2/lang-cpp17.C.jj   2020-11-30 
> 15:06:35.631836747 +0100
> +++ gcc/testsuite/g++.dg/debug/dwarf2/lang-cpp17.C  2020-11-30 
> 15:07:19.160350623 +0100
> @@ -0,0 +1,7 @@
> +// { dg-do compile }
> +// { dg-options "-O -std=c++17 -gdwarf-5 -dA" }
> +// For -gdwarf-6 hopefully DW_LANG_C_plus_plus_17
> +// DW_LANG_C_plus_plus_14 = 0x0021
> +// { dg-final { scan-assembler "0x21\[^\n\r]* DW_AT_language" } } */
> +
> +int version;
> --- gcc/testsuite/g++.dg/debug/dwarf2/lang-cpp20.C.jj   2020-11-30 
> 15:07:27.436258201 +0100
> +++ gcc/testsuite/g++.dg/debug/dwarf2/lang-cpp20.C  2020-11-30 
> 15:07:37.801142442 +0100
> @@ -0,0 +1,7 @@
> +// { dg-do compile }
> +// { dg-options "-O -std=c++20 -gdwarf-5 -dA" }
> +// For -gdwarf-6 hopefully DW_LANG_C_plus_plus_20
> +// DW_LANG_C_plus_plus_14 = 0x0021
> +// { dg-final { scan-assembler "0x21\[^\n\r]* DW_AT_language" } } */
> +
> +int version;
>
> Jakub
>


[Ada] Remove all ^L characters

2020-11-30 Thread Pierre-Marie de Rodat
These control characters are mainly causing confusion at this stage,
so remove them.

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

gcc/ada/

* gcc-interface/Makefile.in, gcc-interface/trans.c: Remove ^L
characters.diff --git a/gcc/ada/gcc-interface/Makefile.in b/gcc/ada/gcc-interface/Makefile.in
--- a/gcc/ada/gcc-interface/Makefile.in
+++ b/gcc/ada/gcc-interface/Makefile.in
@@ -216,7 +216,7 @@ endif
 ifneq ($(xmake_file),)
 include $(xmake_file)
 endif
-

+
 # Now figure out from those variables how to compile and link.
 
 all.indirect: Makefile ../gnat1$(exeext)
@@ -311,7 +311,7 @@ Makefile: ../config.status $(srcdir)/ada/gcc-interface/Makefile.in $(srcdir)/ada
 # This tells GNU make version 3 not to export all the variables
 # defined in this file into the environment.
 .NOEXPORT:
-

+
 # Lists of files for various purposes.
 
 GNATLINK_OBJS = gnatlink.o \


diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -1333,7 +1333,7 @@ Identifier_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p)
 
   return gnu_result;
 }
-

+
 /* Subroutine of gnat_to_gnu to process gnat_node, an N_Pragma.  Return
any statements we generate.  */
 




[Ada] Wrong replacement of Component.Discriminant

2020-11-30 Thread Pierre-Marie de Rodat
The procedure Replace_Discr_Ref added a few years ago is overzealous and
triggers in too many cases in the case of multiple records with
discriminants involved. It appears that this procedure is no longer
needed at this stage, so simply remove it.

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

gcc/ada/

* exp_ch3.adb (Replace_Discr_Ref): Removed, no longer needed.diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -2008,47 +2008,6 @@ package body Exp_Ch3 is
  Lhs  : Node_Id;
  Res  : List_Id;
 
- function Replace_Discr_Ref (N : Node_Id) return Traverse_Result;
- --  Analysis of the aggregate has replaced discriminants by their
- --  corresponding discriminals, but these are irrelevant when the
- --  component has a mutable type and is initialized with an aggregate.
- --  Instead, they must be replaced by the values supplied in the
- --  aggregate, that will be assigned during the expansion of the
- --  assignment.
-
- ---
- -- Replace_Discr_Ref --
- ---
-
- function Replace_Discr_Ref (N : Node_Id) return Traverse_Result is
-Val : Node_Id;
-
- begin
-if Is_Entity_Name (N)
-  and then Present (Entity (N))
-  and then Is_Formal (Entity (N))
-  and then Present (Discriminal_Link (Entity (N)))
-then
-   Val :=
- Make_Selected_Component (Default_Loc,
-   Prefix=> New_Copy_Tree (Lhs),
-   Selector_Name =>
- New_Occurrence_Of
-   (Discriminal_Link (Entity (N)), Default_Loc));
-
-   if Present (Val) then
-  Rewrite (N, New_Copy_Tree (Val));
-   end if;
-end if;
-
-return OK;
- end Replace_Discr_Ref;
-
- procedure Replace_Discriminant_References is
-   new Traverse_Proc (Replace_Discr_Ref);
-
-  --  Start of processing for Build_Assignment
-
   begin
  Lhs :=
Make_Selected_Component (Default_Loc,
@@ -2056,22 +2015,6 @@ package body Exp_Ch3 is
  Selector_Name => New_Occurrence_Of (Id, Default_Loc));
  Set_Assignment_OK (Lhs);
 
- if Nkind (Exp) = N_Aggregate
-   and then Has_Discriminants (Typ)
-   and then not Is_Constrained (Base_Type (Typ))
- then
---  The aggregate may provide new values for the discriminants
---  of the component, and other components may depend on those
---  discriminants. Previous analysis of those expressions have
---  replaced the discriminants by the formals of the initialization
---  procedure for the type, but these are irrelevant in the
---  enclosing initialization procedure: those discriminant
---  references must be replaced by the values provided in the
---  aggregate.
-
-Replace_Discriminant_References (Exp);
- end if;
-
  --  Case of an access attribute applied to the current instance.
  --  Replace the reference to the type by a reference to the actual
  --  object. (Note that this handles the case of the top level of




[Ada] Fix internal error on extended return and fixed-point result

2020-11-30 Thread Pierre-Marie de Rodat
This prevents the expander from creating an access before elaboration
issue for a temporary generated for a range check applied to the
expression of a degenerate extended return statement (an extended return
statement without an explicit do/end construct).

Expand_N_Extended_Return_Statement has got a fast track for this case,
but it does not do what it is supposed to do and ends up generating both
the declaration of the return object with the expresion and the direct
return of the expression, which is problematic if the direct return is
analyzed again and subject to a range check.

The patch reimplements the fast track as documented and makes sure that
it is used only when the declaration of the return object can be
dropped.  It also contains a fixlet for
Check_Type_Or_Object_External_Properties.

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

gcc/ada/

* contracts.adb (Check_Type_Or_Object_External_Properties): Make
sure to exclude all return objects from the SPARK legality rule
on effectively volatile variables.
* exp_ch6.adb (Expand_N_Extended_Return_Statement): Use the fast
track only when the declaration of the return object can be
dropped.diff --git a/gcc/ada/contracts.adb b/gcc/ada/contracts.adb
--- a/gcc/ada/contracts.adb
+++ b/gcc/ada/contracts.adb
@@ -905,9 +905,12 @@ package body Contracts is
 
   --  The following checks are relevant only when SPARK_Mode is on, as
   --  they are not standard Ada legality rules. Internally generated
-  --  temporaries are ignored.
+  --  temporaries are ignored, as well as return objects.
 
-  if SPARK_Mode = On and then Comes_From_Source (Type_Or_Obj_Id) then
+  if SPARK_Mode = On
+and then Comes_From_Source (Type_Or_Obj_Id)
+and then not Is_Return_Object (Type_Or_Obj_Id)
+  then
  if Is_Effectively_Volatile (Type_Or_Obj_Id) then
 
 --  The declaration of an effectively volatile object or type must


diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb
--- a/gcc/ada/exp_ch6.adb
+++ b/gcc/ada/exp_ch6.adb
@@ -5384,13 +5384,15 @@ package body Exp_Ch6 is
   end if;
 
   --  Build a simple_return_statement that returns the return object when
-  --  there is a statement sequence, or no expression, or the result will
-  --  be built in place. Note however that we currently do this for all
-  --  composite cases, even though not all are built in place.
+  --  there is a statement sequence, or no expression, or the analysis of
+  --  the return object declaration generated extra actions, or the result
+  --  will be built in place. Note however that we currently do this for
+  --  all composite cases, even though they are not built in place.
 
   if Present (HSS)
-or else Is_Composite_Type (Ret_Typ)
 or else No (Exp)
+or else List_Length (Return_Object_Declarations (N)) > 1
+or else Is_Composite_Type (Ret_Typ)
   then
  if No (HSS) then
 Stmts := New_List;
@@ -6058,16 +6060,11 @@ package body Exp_Ch6 is
 end;
  end if;
 
-  --  Case where we do not build a block
-
-  else
- --  We're about to drop Return_Object_Declarations on the floor, so
- --  we need to insert it, in case it got expanded into useful code.
- --  Remove side effects from expression, which may be duplicated in
- --  subsequent checks (see Expand_Simple_Function_Return).
+  --  Case where we do not need to build a block. But we're about to drop
+  --  Return_Object_Declarations on the floor, so assert that it contains
+  --  only the return object declaration.
 
- Insert_List_Before (N, Return_Object_Declarations (N));
- Remove_Side_Effects (Exp);
+  else pragma Assert (List_Length (Return_Object_Declarations (N)) = 1);
 
  --  Build simple_return_statement that returns the expression directly
 




[Ada] Implement inheritance for Default_Initial_Condition and address other gaps

2020-11-30 Thread Pierre-Marie de Rodat
The existing (SPARK-based) implementation of Default_Initial_Condition
aspects only applies DIC checks based on a type's own DIC (or only one
DIC from an ancestor if the type doesn't specify one, but not from
further up the ancestor chain), but Ada 202x (AI12-0265) specifies that
all DICs of parent types apply to the descendant type (so they're
basically "additive"). This is similar to what's already done for types
that have Type_Invariant'Class aspects, and the implementation is
modeled on what's done for Type_Invariant, which generates "partial"
checking procedures to apply checks for any DIC specified directly on
the partial (private) view of a type. The "full" DIC procedure calls the
type's partial DIC procedure if any, and applies any DIC checks
inherited from parent types.  Calls to DIC procedures are suppressed in
various cases when the procedure body is null (such as when assertions
are not enabled).

Also, checks of Default_Initial_Conditions were missing in a number of
cases where such checks are required, and these changes address those
gaps. Specifically, checks are now done for record and array components
where the components are default initialized, for default-initialized
allocators, for ancestor parts of extension aggregates, and for
components that are default-initialized by aggregate component
associations specified with a box. Note that components whose
declarations specify an initialization expression are not defined to be
"initialized by default".

For the case of box associations, it was necessary to add a flag to
N_Component_Association nodes (Was_Default_Init_Box_Association),
to mark such associations as having come from a box association that
requires default initialization, because new associations are created
in the analyzer and the original associations are no longer available
at the point where aggregate expansion occurs (Original_Node doesn't
apply).

Note that these changes do not complete the work for implementing
Default_Initial_Condition, which also requires support for DIC aspects
on generic formal types (part of AI12-0265), and resolution of DIC
expressions as specified in AI12-0397.

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

gcc/ada/

* einfo.ads (Is_Partial_DIC_Procedure): New function.
(Partial_DIC_Procedure): New procedure.
* einfo.adb (Is_Partial_DIC_Procedure): New function to return
whether a subprogram is a partial Default_Initial_Condition
procedure by checking the name (to avoid adding a new field).
(DIC_Procedure): Add a test that excludes partial DIC procedures
from being returned.
(Partial_DIC_Procedure): New procedure to return the partial DIC
procedure of a type, if it has one (otherwise returns Empty).
(Set_DIC_Procedure): Remove check for duplicate DIC procedures.
* exp_aggr.adb (Gen_Assign): Generate a call to the type's DIC
procedure in the case where an array component is default
initialized (due to an association with a box).
(Build_Record_Aggr_Code): For an extension aggregate, generate a
call to the ancestor type's DIC procedure (if any) when the
ancestor part is a subtype mark. For a record component
association that was specified with a box (tested for by
checking the new flag Was_Default_Init_Box_Association),
generate a call to the component type's DIC procedure (if it has
one).
* exp_ch4.adb (Expand_N_Allocator): When the allocated object is
default initialized and the designated type has a DIC aspect,
generate a call to the DIC procedure.
* exp_util.ads (Build_DIC_Call): Change the formal Obj_Id to
name Obj_Name, and change its type from Entity_Id to Node_Id
(and update comment).
(Build_DIC_Procedure_Body): Add formal Partial_DIC, remove
formal For_Freeze, and update comment accordingly.
(Build_DIC_Procedure_Declaration): Add formal Partial_DIC and
update comment.
* exp_util.adb
(Build_DIC_Call): Revised to use its Obj_Name (formerly Obj_Id)
formal directly rather than calling New_Occurrence_Of on it, to
allow arbitrary names to be passed rather than being limited to
Entity_Ids.
(Build_DIC_Procedure_Body): Call Add_Parent_DICs to generate
checks for DICs associated with any parent types, implementing
the required "additive" semantics for DICs. When building a DIC
procedure body for a partial view (when Partial_DIC is True),
call Add_Own_DIC when the type has its own DIC.  In the case of
"full" DIC procedures, a call is generated to any partial DIC
procedure of the type (unless the procedure has a null body),
along with checks for any DICs inherited by the full view.
(Build_DIC_Procedure_Declaration): Add handling for partial DIC
procedures.  For the suffix of a regular DIC 

[Ada] Expand integer-only implementation of ordinary fixed-point types

2020-11-30 Thread Pierre-Marie de Rodat
This change relaxes the conditions under which the implementation of
ordinary fixed-point types uses only underlying integer instructions.

These conditions are on the Small of the ordinary fixed-point types,
which must now be a rational number whose factors are of a magnitude
bounded relatively to the size of the mantissa.

The algorithms of Image and Value are not modified, only that of Fore
is genezalized in a straightforward way.  Two new attributes are added
to the compiler, Small_Denominator and Small_Numerator, in order for
the Text_IO.Fixed_IO package to benefit from the enhancement.

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

gcc/ada/

* doc/gnat_rm/implementation_defined_attributes.rst (Pool_Address):
Fix pasto.
(Small_Denominator): New entry.
(Small_Numerator): Likewise.
* doc/gnat_rm/implementation_defined_characteristics.rst (3.5.9):
Relax conditions on 128-bit smalls and integer-only implementation.
* gnat_rm.texi: Regenerate.
* exp_attr.adb (Expand_N_Attribute_Reference) :
Relax conditions on integer implementation for ordinary fixed-point
types and pass a third parameter to the routine.
: Raise Program_Error.
: Likewise.
* exp_fixd.adb (Expand_Convert_Fixed_To_Fixed): Use a scaled divide
if the numerator and denominator of the small ratio are sufficiently
small integers.
(Expand_Convert_Fixed_To_Integer): Use a scaled divide if numerator
and denominator of the small value are sufficiently small integers.
(Expand_Convert_Integer_To_Fixed): Likewise.
* exp_imgv.adb (Expand_Image_Attribute): Relax the conditions on the
integer implementation for ordinary fixed-point types.
(Expand_Value_Attribute): Likewise.
* freeze.adb (Freeze_Fixed_Point_Type): Relax conditions on 128-bit
smalls.
* sem_attr.adb (Analyze_Attribute) :
Check no arguments, fixed-point and set type to Universal_Integer.
: Likewise.
(Eval_Attribute) : Fold statically.
: Likewise.
* snames.ads-tmpl (Name_Small_Denominator): New attribute name.
(Name_Small_Numerator): Likewise.
(Attribute_Id): Add Attribute_Small_{Denominator,Numerator}.
* libgnat/a-tifiio.adb (Exact): Delete.
(Need_64): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/a-tifiio__128.adb (Exact): Delete.
(Need_64): Likewise.
(Need_128): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(OK_Get_128): Likewise.
(OK_Put_128): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/a-wtfiio.adb (Exact): Delete.
(Need_64): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/a-wtfiio__128.adb (Exact): Delete.
(Need_64): Likewise.
(Need_128): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(OK_Get_128): Likewise.
(OK_Put_128): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/a-ztfiio.adb (Exact): Delete.
(Need_64): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/a-ztfiio__128.adb (Exact): Delete.
(Need_64): Likewise.
(Need_128): Likewise.
(OK_Get_32): New boolean constant.
(OK_Put_32): Likewise.
(OK_Get_64): Likewise.
(OK_Put_64): Likewise.
(OK_Get_128): Likewise.
(OK_Put_128): Likewise.
(E): Adjust.
(Get procedures): Likewise.
(Put procedures): Likewise.
* libgnat/s-fore_f.ads (Fore_Fixed): Adjust signature.
* libgnat/s-fore_f.adb (Fore_Fixed): Reimplement.
* libgnat/s-fofi32.ads (Fore_Fixed32): Adjust signature.
* libgnat/s-fofi64.ads (Fore_Fixed64): Likewise.
* libgnat/s-fofi128.ads (Fore_Fixed128): Likewise.
* libgnat/s-imagef.ads: Adjust description.
* libgnat/s-imagef.adb (Maxdigs): Move around.
(Set_Image_Integer): Remove assertion.
* libgnat/s-valuef.ads: Adjust description.
* libgnat/s-valuef.adb (Integer_To_Fixed): 

[Ada] Enable checks on runtime by default

2020-11-30 Thread Pierre-Marie de Rodat
These checks are not very costly these days and bring additional safety
and security guarantees built into the Ada language, so enable them by
default.

It turns out that enabling checks on s-bitfie.adb makes a latent
visibility bug appeared on strict alignment platform (related to
alignment checks most likely). Workaround it for the time being by
suppressing checks locally.

It also makes visible a latent issue with secondary stack allocation of
0 bytes, now possible with another recent change to allocate the result
of string concatenation sometimes on the secondary stack.

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

gcc/ada/

* gcc-interface/Makefile.in (GNATLIBFLAGS): Enable checks by
default.
* libgnat/s-bitfie.ads: Suppress alignment checks.
* libgnat/s-bituti.adb: Minor reformatting.
* libgnat/s-secsta.adb (SS_Allocate): Support Size = 0.diff --git a/gcc/ada/gcc-interface/Makefile.in b/gcc/ada/gcc-interface/Makefile.in
--- a/gcc/ada/gcc-interface/Makefile.in
+++ b/gcc/ada/gcc-interface/Makefile.in
@@ -110,7 +110,7 @@ NO_INLINE_ADAFLAGS = -fno-inline
 NO_OMIT_ADAFLAGS = -fno-omit-frame-pointer
 NO_SIBLING_ADAFLAGS = -fno-optimize-sibling-calls
 NO_REORDER_ADAFLAGS = -fno-toplevel-reorder
-GNATLIBFLAGS = -W -Wall -gnatpg -nostdinc
+GNATLIBFLAGS = -W -Wall -gnatg -nostdinc
 GNATLIBCFLAGS = -g -O2
 # Pretend that _Unwind_GetIPInfo is available for the target by default.  This
 # should be autodetected during the configuration of libada and passed down to


diff --git a/gcc/ada/libgnat/s-bitfie.ads b/gcc/ada/libgnat/s-bitfie.ads
--- a/gcc/ada/libgnat/s-bitfie.ads
+++ b/gcc/ada/libgnat/s-bitfie.ads
@@ -47,6 +47,12 @@ package System.Bitfields is
pragma Provide_Shift_Operators (Val_2);
type Val is mod 2**Val_Bits with Alignment => Val_Bytes;
 
+   --  ??? It turns out that enabling checks on the instantiation of
+   --  System.Bitfield_Utils.G makes a latent visibility bug appear on strict
+   --  alignment platforms related to alignment checks. Work around it by
+   --  suppressing these checks explicitly.
+
+   pragma Suppress (Alignment_Check);
package Utils is new System.Bitfield_Utils.G (Val, Val_2);
 
procedure Copy_Bitfield


diff --git a/gcc/ada/libgnat/s-bituti.adb b/gcc/ada/libgnat/s-bituti.adb
--- a/gcc/ada/libgnat/s-bituti.adb
+++ b/gcc/ada/libgnat/s-bituti.adb
@@ -317,6 +317,7 @@ package body System.Bitfield_Utils is
  Get_Val_2 (S_Addr, S_Off, Initial_Size);
Initial_Val : constant Val :=
  Get_Bitfield (Initial_Val_2, S_Off, Initial_Size);
+
 begin
Set_Bitfield
  (Initial_Val, D_Addr, D_Off, Initial_Size);


diff --git a/gcc/ada/libgnat/s-secsta.adb b/gcc/ada/libgnat/s-secsta.adb
--- a/gcc/ada/libgnat/s-secsta.adb
+++ b/gcc/ada/libgnat/s-secsta.adb
@@ -587,15 +587,18 @@ package body System.Secondary_Stack is
--  Start of processing for SS_Allocate
 
begin
-  --  It should not be possible to request an allocation of negative or
-  --  zero size.
-
-  pragma Assert (Storage_Size > 0);
-
   --  Round the requested size up to the nearest multiple of the maximum
   --  alignment to ensure efficient access.
 
-  Mem_Size := Round_Up (Storage_Size);
+  if Storage_Size = 0 then
+ Mem_Size := Memory_Alignment;
+  else
+ --  It should not be possible to request an allocation of negative
+ --  size.
+
+ pragma Assert (Storage_Size >= 0);
+ Mem_Size := Round_Up (Storage_Size);
+  end if;
 
   if Sec_Stack_Dynamic then
  Allocate_Dynamic (Stack, Mem_Size, Addr);




[Ada] Reimplement Ada.Numerics.Big_Numbers.Big_Reals.Fixed_Conversions

2020-11-30 Thread Pierre-Marie de Rodat
This reimplements the aforementioned generic package according to the
requirements of the Ada 2020 RM, namely that To_Big_Real be exact and
that From_Big_Real use the common conversion rules.

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

gcc/ada/

* libgnat/a-nbnbre.adb (Float_Conversions): Instantiate Conv
package only once in the body.
(Fixed_Conversions.Float_Aux): New instance.
(Fixed_Conversions.Conv_I): Likewise.
(Fixed_Conversions.Conv_U): Likewise.
(Fixed_Conversions.LLLI): New subtype.
(Fixed_Conversions.LLLU): Likewise.
(Fixed_Conversions.Too_Large): New constant.
(Fixed_Conversions.To_Big_Real): Reimplement.
(Fixed_Conversions.From_Big_Real): Likewise.diff --git a/gcc/ada/libgnat/a-nbnbre.adb b/gcc/ada/libgnat/a-nbnbre.adb
--- a/gcc/ada/libgnat/a-nbnbre.adb
+++ b/gcc/ada/libgnat/a-nbnbre.adb
@@ -118,6 +118,9 @@ package body Ada.Numerics.Big_Numbers.Big_Reals is
 
package body Float_Conversions is
 
+  package Conv is new
+Big_Integers.Unsigned_Conversions (Long_Long_Unsigned);
+
   -
   -- To_Big_Real --
   -
@@ -130,9 +133,6 @@ package body Ada.Numerics.Big_Numbers.Big_Reals is
 
   function To_Big_Real (Arg : Num) return Valid_Big_Real is
 
- package Conv is new
-   Big_Integers.Unsigned_Conversions (Long_Long_Unsigned);
-
  A : constant Num'Base := abs (Arg);
  E : constant Integer  := Num'Exponent (A);
  F : constant Num'Base := Num'Fraction (A);
@@ -182,9 +182,6 @@ package body Ada.Numerics.Big_Numbers.Big_Reals is
 
   function From_Big_Real (Arg : Big_Real) return Num is
 
- package Conv is new
-   Big_Integers.Unsigned_Conversions (Long_Long_Unsigned);
-
  M: constant Natural := Num'Machine_Mantissa;
  One  : constant Big_Real:= To_Real (1);
  Two  : constant Big_Real:= To_Real (2);
@@ -310,22 +307,78 @@ package body Ada.Numerics.Big_Numbers.Big_Reals is
 
package body Fixed_Conversions is
 
+  package Float_Aux is new Float_Conversions (Long_Long_Float);
+
+  subtype LLLI is Long_Long_Long_Integer;
+  subtype LLLU is Long_Long_Long_Unsigned;
+
+  Too_Large : constant Boolean :=
+Num'Small_Numerator > LLLU'Last
+  or else Num'Small_Denominator > LLLU'Last;
+  --  True if the Small is too large for Long_Long_Long_Unsigned, in which
+  --  case we convert to/from Long_Long_Float as an intermediate step.
+
+  package Conv_I is new Big_Integers.Signed_Conversions (LLLI);
+  package Conv_U is new Big_Integers.Unsigned_Conversions (LLLU);
+
   -
   -- To_Big_Real --
   -
 
+  --  We just compute V * N / D where V is the mantissa value of the fixed
+  --  point number, and N resp. D is the numerator resp. the denominator of
+  --  the Small of the fixed-point type.
+
   function To_Big_Real (Arg : Num) return Valid_Big_Real is
+ N, D, V : Big_Integer;
+
   begin
- return From_String (Arg'Image);
+ if Too_Large then
+return Float_Aux.To_Big_Real (Long_Long_Float (Arg));
+ end if;
+
+ N := Conv_U.To_Big_Integer (Num'Small_Numerator);
+ D := Conv_U.To_Big_Integer (Num'Small_Denominator);
+ V := Conv_I.To_Big_Integer (LLLI'Integer_Value (Arg));
+
+ return V * N / D;
   end To_Big_Real;
 
   ---
   -- From_Big_Real --
   ---
 
+  --  We first compute A / B = Arg * D / N where N resp. D is the numerator
+  --  resp. the denominator of the Small of the fixed-point type. Then we
+  --  divide A by B and convert the result to the mantissa value.
+
   function From_Big_Real (Arg : Big_Real) return Num is
+ N, D, A, B, Q, X : Big_Integer;
+
   begin
- return Num'Value (To_String (Arg));
+ if Too_Large then
+return Num (Float_Aux.From_Big_Real (Arg));
+ end if;
+
+ N := Conv_U.To_Big_Integer (Num'Small_Numerator);
+ D := Conv_U.To_Big_Integer (Num'Small_Denominator);
+ A := Numerator (Arg) * D;
+ B := Denominator (Arg) * N;
+
+ Q := A / B;
+
+ --  Round to nearest, ties to away, by comparing twice the remainder
+
+ X := (A - Q * B) * To_Big_Integer (2);
+
+ if X >= B then
+Q := Q + To_Big_Integer (1);
+
+ elsif X <= -B then
+Q := Q - To_Big_Integer (1);
+ end if;
+
+ return Num'Fixed_Value (Conv_I.From_Big_Integer (Q));
   end From_Big_Real;
 
end Fixed_Conversions;




[Ada] Implement -gnateb switch

2020-11-30 Thread Pierre-Marie de Rodat
The -gnateb switch instructs gnat to store configuration pragma files by
their basename in ALI files instead of using absolute paths.

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

gcc/ada/

* doc/gnat_ugn/building_executable_programs_with_gnat.rst:
Describe -gnateb switch.
* doc/gnat_ugn/the_gnat_compilation_model.rst: Mention -gnateb
switch in configuration pragma files section.
* gnat_ugn.texi: Regenerate.
* lib-writ.adb (Write_ALI): Strip directories from configuration
files path if needed.
* opt.ads: Declare Config_Files_Store_Basename option.
* par.adb (Par): Save configuration file checksum.
* switch-c.adb (Scan_Front_End_Switches): Set
Config_Files_Store_Basename true if -gnateb is present.diff --git a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
--- a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
+++ b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
@@ -1517,6 +1517,13 @@ Alphabetical List of All Switches
   an exception because ``Self(Obj)`` produces an anonymous object which does
   not share the memory location of ``Obj``.
 
+.. index:: -gnateb  (gcc)
+
+:switch:`-gnateb`
+  Store configuration files by their basename in ALI files. This switch is
+  used for instance by gprbuild for distributed builds in order to prevent
+  issues where machine-specific absolute paths could end up being stored in
+  ALI files.
 
 .. index:: -gnatec  (gcc)
 


diff --git a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
--- a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
+++ b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
@@ -1560,6 +1560,10 @@ temporary files that are immediately deleted; it doesn't make sense to
 depend on a file that no longer exists. Such tools include
 ``gprbuild``, ``gnatmake``, and ``gnatcheck``.
 
+By default, configuration pragma files are stored by their absolute paths in
+ALI files. You can use the :switch:`-gnateb` switch in order to store them by
+their basename instead.
+
 If you are using project file, a separate mechanism is provided using
 project attributes.
 


diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi
--- a/gcc/ada/gnat_ugn.texi
+++ b/gcc/ada/gnat_ugn.texi
@@ -2970,6 +2970,10 @@ temporary files that are immediately deleted; it doesn't make sense to
 depend on a file that no longer exists. Such tools include
 @code{gprbuild}, @code{gnatmake}, and @code{gnatcheck}.
 
+By default, configuration pragma files are stored by their absolute paths in
+ALI files. You can use the @code{-gnateb} switch in order to store them by
+their basename instead.
+
 If you are using project file, a separate mechanism is provided using
 project attributes.
 
@@ -8968,6 +8972,19 @@ an exception because @code{Self(Obj)} produces an anonymous object which does
 not share the memory location of @code{Obj}.
 @end table
 
+@geindex -gnateb (gcc)
+
+
+@table @asis
+
+@item @code{-gnateb}
+
+Store configuration files by their basename in ALI files. This switch is
+used for instance by gprbuild for distributed builds in order to prevent
+issues where machine-specific absolute paths could end up being stored in
+ALI files.
+@end table
+
 @geindex -gnatec (gcc)
 
 


diff --git a/gcc/ada/lib-writ.adb b/gcc/ada/lib-writ.adb
--- a/gcc/ada/lib-writ.adb
+++ b/gcc/ada/lib-writ.adb
@@ -1476,11 +1476,8 @@ package body Lib.Writ is
 --  Normal case of a unit entry with a source index
 
 if Sind > No_Source_File then
-   --  We never want directory information in ALI files
-   --  ???But back out this change temporarily until
-   --  gprbuild is fixed.
 
-   if False then
+   if Config_Files_Store_Basename then
   Fname := Strip_Directory (File_Name (Sind));
else
   Fname := File_Name (Sind);


diff --git a/gcc/ada/opt.ads b/gcc/ada/opt.ads
--- a/gcc/ada/opt.ads
+++ b/gcc/ada/opt.ads
@@ -364,6 +364,11 @@ package Opt is
--  GNAT
--  Names of configuration pragmas files (given by switches -gnatec)
 
+   Config_Files_Store_Basename : Boolean := False;
+   --  GNAT
+   --  Set True for -gnateb. Tells GNAT that config files should be referred to
+   --  by their basename and their checksums computed in ALI files.
+
Configurable_Run_Time_Mode : Boolean := False;
--  GNAT, GNATBIND
--  Set True if the compiler is operating in configurable run-time mode.


diff --git a/gcc/ada/par.adb b/gcc/ada/par.adb
--- a/gcc/ada/par.adb
+++ b/gcc/ada/par.adb
@@ -1546,6 +1546,10 @@ begin
  end loop;
   end;
 
+  if Config_Files_Store_Basename then
+ Complete_Source_File_Entry;
+  end if;
+
--  Normal case of compilation unit
 
else


diff --git 

[Ada] Add stream-oriented attributes support for 128-bit integer types

2020-11-30 Thread Pierre-Marie de Rodat
This was overlooked in the original implementation of these types.

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

gcc/ada/

* doc/gnat_ugn/building_executable_programs_with_gnat.rst (-xdr):
Document that XDR is not supported for 128-bit integer types.
* gnat_ugn.texi: Regenerate.
* exp_strm.adb (Build_Elementary_Input_Call): Deal with types
larger than Long_Long_Integer.
(Build_Elementary_Write_Call): Likewise.
* rtsfind.ads (RE_Id): Add RE_I_LLL{I,U] and RE_W_LLL{I,U}.
(RE_Unit_Table): Add entries for them.
* libgnat/s-stratt.ads (I_LLLI): New inline function.
(I_LLLU): Likewise.
(W_LLLI): New inline procedure.
(W_LLLU): Likewise.
* libgnat/s-stratt.adb (S_LLLI): New subtype of SEA.
(S_LLLU): Likewise.
(From_LLLI): New instance of Unchecked_Conversion.
(From_LLLU): Likewise.
(To_LLLI): Likewise.
(To_LLLU): Likewise.
(I_LLLI): Implement.
(I_LLLU): Likewise.
(W_LLLI): Likewise.
(W_LLLU): Likewise.diff --git a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
--- a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
+++ b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
@@ -6704,6 +6704,9 @@ be presented in subsequent sections.
   Use the target-independent XDR protocol for stream oriented attributes
   instead of the default implementation which is based on direct binary
   representations and is therefore target-and endianness-dependent.
+  However it does not support 128-bit integer types and the exception
+  ``Ada.IO_Exceptions.Device_Error`` is raised if any attempt is made
+  at streaming 128-bit integer types with it.
 
 
   .. index:: -Xnnn  (gnatbind)


diff --git a/gcc/ada/exp_strm.adb b/gcc/ada/exp_strm.adb
--- a/gcc/ada/exp_strm.adb
+++ b/gcc/ada/exp_strm.adb
@@ -578,8 +578,11 @@ package body Exp_Strm is
  elsif P_Size <= Standard_Long_Integer_Size then
 Lib_RE := RE_I_LI;
 
- else
+ elsif P_Size <= Standard_Long_Long_Integer_Size then
 Lib_RE := RE_I_LLI;
+
+ else
+Lib_RE := RE_I_LLLI;
  end if;
 
   --  Unsigned integer types, also includes unsigned fixed-point types
@@ -609,8 +612,11 @@ package body Exp_Strm is
  elsif P_Size <= Standard_Long_Integer_Size then
 Lib_RE := RE_I_LU;
 
- else
+ elsif P_Size <= Standard_Long_Long_Integer_Size then
 Lib_RE := RE_I_LLU;
+
+ else
+Lib_RE := RE_I_LLLU;
  end if;
 
   else pragma Assert (Is_Access_Type (U_Type));
@@ -802,16 +808,24 @@ package body Exp_Strm is
   then
  if P_Size <= Standard_Short_Short_Integer_Size then
 Lib_RE := RE_W_SSI;
+
  elsif P_Size <= Standard_Short_Integer_Size then
 Lib_RE := RE_W_SI;
+
  elsif P_Size = 24 then
 Lib_RE := RE_W_I24;
+
  elsif P_Size <= Standard_Integer_Size then
 Lib_RE := RE_W_I;
+
  elsif P_Size <= Standard_Long_Integer_Size then
 Lib_RE := RE_W_LI;
- else
+
+ elsif P_Size <= Standard_Long_Long_Integer_Size then
 Lib_RE := RE_W_LLI;
+
+ else
+Lib_RE := RE_W_LLLI;
  end if;
 
   --  Unsigned integer types, also includes unsigned fixed-point types
@@ -828,16 +842,24 @@ package body Exp_Strm is
   then
  if P_Size <= Standard_Short_Short_Integer_Size then
 Lib_RE := RE_W_SSU;
+
  elsif P_Size <= Standard_Short_Integer_Size then
 Lib_RE := RE_W_SU;
+
  elsif P_Size = 24 then
 Lib_RE := RE_W_U24;
+
  elsif P_Size <= Standard_Integer_Size then
 Lib_RE := RE_W_U;
+
  elsif P_Size <= Standard_Long_Integer_Size then
 Lib_RE := RE_W_LU;
- else
+
+ elsif P_Size <= Standard_Long_Long_Integer_Size then
 Lib_RE := RE_W_LLU;
+
+ else
+Lib_RE := RE_W_LLLU;
  end if;
 
   else pragma Assert (Is_Access_Type (U_Type));


diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi
--- a/gcc/ada/gnat_ugn.texi
+++ b/gcc/ada/gnat_ugn.texi
@@ -21,7 +21,7 @@
 
 @copying
 @quotation
-GNAT User's Guide for Native Platforms , Nov 19, 2020
+GNAT User's Guide for Native Platforms , Nov 20, 2020
 
 AdaCore
 
@@ -15919,6 +15919,9 @@ Exclude source files (check object consistency only).
 Use the target-independent XDR protocol for stream oriented attributes
 instead of the default implementation which is based on direct binary
 representations and is therefore target-and endianness-dependent.
+However it does not support 128-bit integer types and the exception
+@code{Ada.IO_Exceptions.Device_Error} is raised if any attempt is made
+at streaming 128-bit integer types with it.
 
 

[Ada] Compiler crash on limited conditional expressions

2020-11-30 Thread Pierre-Marie de Rodat
This patch fixes a bug in which if the expression of an expression
function is a conditional expression of limited type, the compiler
crashes.

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

gcc/ada/

* exp_ch3.adb (Expand_N_Object_Declaration): Avoid crash in case
of conditional expression.diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb
--- a/gcc/ada/exp_ch3.adb
+++ b/gcc/ada/exp_ch3.adb
@@ -7403,12 +7403,12 @@ package body Exp_Ch3 is
 
--  If we cannot convert the expression into a renaming we must
--  consider it an internal error because the backend does not
-   --  have support to handle it. Also, when a raise expression is
-   --  encountered we ignore it since it doesn't return a value and
-   --  thus cannot trigger a copy.
+   --  have support to handle it. But avoid crashing on a raise
+   --  expression or conditional expression.
 
-   elsif Nkind (Original_Node (Expr_Q)) /= N_Raise_Expression then
-  pragma Assert (False);
+   elsif Nkind (Original_Node (Expr_Q)) not in
+ N_Raise_Expression | N_If_Expression | N_Case_Expression
+   then
   raise Program_Error;
end if;
 




[Ada] Improve error recovery

2020-11-30 Thread Pierre-Marie de Rodat
Function P_Formal_Part was unnecessarily calling Scan after calling
T_Semicolon: T_Semicolon is already calling Scan to go past the
comma-used-insted-of-semicolon.

This avoids spurious cascaded errors in case of e.g:

  procedure P (A : Integer, B : Integer, C : Integer) is

where ',' is used instead of ';'.

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

gcc/ada/

* par-ch6.adb (P_Formal_Part): Remove extra call to Scan.
* par-tchk.adb: Minor reformatting.diff --git a/gcc/ada/par-ch6.adb b/gcc/ada/par-ch6.adb
--- a/gcc/ada/par-ch6.adb
+++ b/gcc/ada/par-ch6.adb
@@ -1650,7 +1650,6 @@ package body Ch6 is
 
  elsif Token = Tok_Comma then
 T_Semicolon;
-Scan; -- past comma
 
  --  Special check for omitted separator
 


diff --git a/gcc/ada/par-tchk.adb b/gcc/ada/par-tchk.adb
--- a/gcc/ada/par-tchk.adb
+++ b/gcc/ada/par-tchk.adb
@@ -436,7 +436,6 @@ package body Tchk is
 
procedure T_Semicolon is
begin
-
   if Token = Tok_Semicolon then
  Scan;
 




[Ada] Fix folding of comparison operators in GNATprove mode

2020-11-30 Thread Pierre-Marie de Rodat
Folding of comparison operators was disabled for GNATprove in assertion
expressions (except for some special cases). However, folding itself is
harmless. The problem was only in the current value optimizer, which
interferes with assertions that act as cut points for proof.

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

gcc/ada/

* exp_util.adb (Get_Current_Value_Condition): Don't use current
value tracking in GNATprove mode.
* sem_res.adb (Resolve_Comparison_Op): Remove incomplete
special-casing for folding in GNATprove mode.diff --git a/gcc/ada/exp_util.adb b/gcc/ada/exp_util.adb
--- a/gcc/ada/exp_util.adb
+++ b/gcc/ada/exp_util.adb
@@ -6360,6 +6360,17 @@ package body Exp_Util is
  return;
   end if;
 
+  --  In GNATprove mode we don't want to use current value optimizer, in
+  --  particular for loop invariant expressions and other assertions that
+  --  act as cut points for proof. The optimizer often folds expressions
+  --  into True/False where they trivially follow from the previous
+  --  assignments, but this deprives proof from the information needed to
+  --  discharge checks that are beyond the scope of the value optimizer.
+
+  if GNATprove_Mode then
+ return;
+  end if;
+
   --  Otherwise examine current value
 
   declare


diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -7457,24 +7457,7 @@ package body Sem_Res is
 
   Analyze_Dimension (N);
 
-  --  Evaluate the relation (note we do this after the above check since
-  --  this Eval call may change N to True/False). Skip this evaluation
-  --  inside assertions, in order to keep assertions as written by users
-  --  for tools that rely on these, e.g. GNATprove for loop invariants.
-  --  Except evaluation is still performed even inside assertions for
-  --  comparisons between values of universal type, which are useless
-  --  for static analysis tools, and not supported even by GNATprove.
-  --  ??? It is suspicious to disable evaluation only for comparison
-  --  operators and not, let's say, for calls to static functions.
-
-  if not GNATprove_Mode
-or else In_Assertion_Expr = 0
-or else (Is_Universal_Numeric_Type (Etype (L))
-   and then
- Is_Universal_Numeric_Type (Etype (R)))
-  then
- Eval_Relational_Op (N);
-  end if;
+  Eval_Relational_Op (N);
end Resolve_Comparison_Op;
 





[Ada] Fix serial port control setting on GNU/Linux

2020-11-30 Thread Pierre-Marie de Rodat
This fixes an issue when setting the control flags of the serial
communication port. The c_cflag termios field should not be set with the
baud rate. The speed of the communication is set using the c_ispeed and
c_ospeed fields. Setting the baud rate into the c_cflag has unexpected
results as it will wrongly set some bits used to control other aspects
of the communication channel (bits, parity...).

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

gcc/ada/

* libgnat/g-sercom__linux.adb (Set): Fix control flags of the
serial port setting.diff --git a/gcc/ada/libgnat/g-sercom__linux.adb b/gcc/ada/libgnat/g-sercom__linux.adb
--- a/gcc/ada/libgnat/g-sercom__linux.adb
+++ b/gcc/ada/libgnat/g-sercom__linux.adb
@@ -52,34 +52,6 @@ package body GNAT.Serial_Communications is
function fcntl (fd : int; cmd : int; value : int) return int;
pragma Import (C, fcntl, "fcntl");
 
-   C_Data_Rate : constant array (Data_Rate) of unsigned :=
-   (B75  => OSC.B75,
-B110 => OSC.B110,
-B150 => OSC.B150,
-B300 => OSC.B300,
-B600 => OSC.B600,
-B1200=> OSC.B1200,
-B2400=> OSC.B2400,
-B4800=> OSC.B4800,
-B9600=> OSC.B9600,
-B19200   => OSC.B19200,
-B38400   => OSC.B38400,
-B57600   => OSC.B57600,
-B115200  => OSC.B115200,
-B230400  => OSC.B230400,
-B460800  => OSC.B460800,
-B50  => OSC.B50,
-B576000  => OSC.B576000,
-B921600  => OSC.B921600,
-B100 => OSC.B100,
-B1152000 => OSC.B1152000,
-B150 => OSC.B150,
-B200 => OSC.B200,
-B250 => OSC.B250,
-B300 => OSC.B300,
-B350 => OSC.B350,
-B400 => OSC.B400);
-
C_Bits  : constant array (Data_Bits) of unsigned :=
(CS7 => OSC.CS7, CS8 => OSC.CS8);
 
@@ -229,8 +201,7 @@ package body GNAT.Serial_Communications is
 
   --  Change settings now
 
-  Current.c_cflag := C_Data_Rate (Rate)
-   or C_Bits (Bits)
+  Current.c_cflag := C_Bits (Bits)
or C_Stop_Bits (Stop_Bits)
or C_Parity (Parity)
or CREAD;




[Ada] Spurious error on iterator over container with modified private part

2020-11-30 Thread Pierre-Marie de Rodat
The compiler rejects an element iterator (for .. of) over a container
that extends a predefined container with additional operations in the
private part, when these added operations overload existing
GNAT-specific private operations used to optimize such loops.

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

gcc/ada/

* exp_ch5.adb (Expand_Iterator_Loop_Over_Container): Check the
signature of the private operation Get_Element_Access to prevent
accidental use of a user-defined homonym subprogram.diff --git a/gcc/ada/exp_ch5.adb b/gcc/ada/exp_ch5.adb
--- a/gcc/ada/exp_ch5.adb
+++ b/gcc/ada/exp_ch5.adb
@@ -4346,10 +4346,21 @@ package body Exp_Ch5 is
 Iter_Pack := Scope (Root_Type (Etype (Iter_Type)));
 
 --  Find declarations needed for "for ... of" optimization
+--  These declarations come from GNAT sources or sources
+--  derived from them. User code may include additional
+--  overloadings with similar names, and we need to perforn
+--  some reasonable resolution to find the needed primitives.
+--  It is unclear whether this mechanism is fragile if a user
+--  makes arbitrary changes to the private part of a package
+--  that supports iterators.
 
 Ent := First_Entity (Pack);
 while Present (Ent) loop
-   if Chars (Ent) = Name_Get_Element_Access then
+   if Chars (Ent) = Name_Get_Element_Access
+ and then Present (First_Formal (Ent))
+ and then Chars (First_Formal (Ent)) = Name_Position
+ and then No (Next_Formal (First_Formal (Ent)))
+   then
   Fast_Element_Access_Op := Ent;
 
elsif Chars (Ent) = Name_Step




[Ada] Potential read of uninitialized variable in exp_dist.adb

2020-11-30 Thread Pierre-Marie de Rodat
Found by using -gnatVa and Initialize_Scalars on GNAT sources.

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

gcc/ada/

* exp_dist.adb (RCI_Cache): Initialize.diff --git a/gcc/ada/exp_dist.adb b/gcc/ada/exp_dist.adb
--- a/gcc/ada/exp_dist.adb
+++ b/gcc/ada/exp_dist.adb
@@ -902,7 +902,7 @@ package body Exp_Dist is
-- Local variables and structures --

 
-   RCI_Cache : Node_Id;
+   RCI_Cache : Node_Id := Empty;
--  Needs comments ???
 
Output_From_Constrained : constant array (Boolean) of Name_Id :=




  1   2   >