Re: [PATCH 0/2] Future warnings not treated as errors

[Alex Henrie]

On Tue, Mar 19, 2019 at 11:03 AM Martin Liška  wrote:
>
> What's Joseph telling is that right now the patch can't be approved (and 
> installed).
> It's due to fact that we are close to the release in stage4:
> https://www.gnu.org/software/gcc/develop.html#stage4
>
> So that a proper review will happen in couple of weeks.
>
> Thanks for understanding,

Sure. I look forward to hearing back from you guys in a couple of weeks.

-Alex

[PATCH v2 1/2] PR c/65403 - Ignore -Wno-error=

[Alex Henrie]

From: Manuel López-Ibáñez 

* opts.c: Ignore -Wno-error= except if there are
other diagnostics.
---
 gcc/opts-common.c |  2 ++
 gcc/opts-global.c | 10 +++---
 gcc/opts.c|  5 -
 gcc/opts.h|  2 ++
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index edbb3ac9b6d..36e06e2372a 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "spellcheck.h"
 
+vec ignored_wnoerror_options;
+
 static void prune_options (struct cl_decoded_option **, unsigned int *);
 
 /* An option that is undocumented, that takes a joined argument, and
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index a5e9ef0237a..bef020a4c46 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -132,12 +132,16 @@ print_ignored_options (void)
 {
   while (!ignored_options.is_empty ())
 {
-  const char *opt;
-
-  opt = ignored_options.pop ();
+  const char * opt = ignored_options.pop ();
   warning_at (UNKNOWN_LOCATION, 0,
  "unrecognized command line option %qs", opt);
 }
+  while (!ignored_wnoerror_options.is_empty ())
+{
+  const char * opt = ignored_wnoerror_options.pop ();
+  warning_at (UNKNOWN_LOCATION, 0,
+ "%<-Werror=%s%>: no option -W%s", opt, opt);
+}
 }
 
 /* Handle an unknown option DECODED, returning true if an error should
diff --git a/gcc/opts.c b/gcc/opts.c
index 02f6b4656e1..492a50ba326 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3087,7 +3087,10 @@ enable_warning_as_error (const char *arg, int value, 
unsigned int lang_mask,
   strcpy (new_option + 1, arg);
   option_index = find_opt (new_option, lang_mask);
   if (option_index == OPT_SPECIAL_unknown)
-error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+if (value)
+  error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+else
+  ignored_wnoerror_options.safe_push (arg);
   else if (!(cl_options[option_index].flags & CL_WARNING))
 error_at (loc, "%<-Werror=%s%>: -%s is not an option that controls "
  "warnings", arg, new_option);
diff --git a/gcc/opts.h b/gcc/opts.h
index f14d9bcb896..b18504e0bc3 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -456,4 +456,6 @@ extern bool parse_and_check_align_values (const char *flag,
  bool report_error,
  location_t loc);
 
+extern vec ignored_wnoerror_options;
+
 #endif
-- 
2.21.0

[PATCH v2 2/2] PR c/65403 - Add tests for -Wno-error=

[Alex Henrie]

---
 gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
 gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
 2 files changed, 25 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
 create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c

diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c 
b/gcc/testsuite/c-c++-common/pr65403-1.c
new file mode 100644
index 000..fbe004a1f78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-1.c
@@ -0,0 +1,10 @@
+/* PR c/65403 */
+/* Test an unrecognized -Wno-error option in the absence of any other
+   diagnostics. The -Wno-error option should be ignored. */
+
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c 
b/gcc/testsuite/c-c++-common/pr65403-2.c
new file mode 100644
index 000..128d4f694a6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-2.c
@@ -0,0 +1,15 @@
+/* PR c/65403 */
+/* Test a warning, treated as an error, that some future -Wno-error option
+   might downgrade back to a warning. The -Wno-error option should produce a
+   warning in this case. */
+
+/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+  int foo; /* { dg-error "unused variable 'foo'" } */
+  return 0;
+}
+
+/* { dg-error "no option -Wsome-future-warning" "" { target *-*-* } 0 } */
+/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 } 
*/
-- 
2.21.0

Re: [PATCH 1/2] PR c/65403 - Ignore -Wno-error=

[Alex Henrie]

On Tue, Mar 19, 2019 at 11:26 AM Martin Sebor  wrote:
>
> Please remember to quote the command line options in the message
> (same as in the error below):

I'll send a new version. Thanks for the feedback!

-Alex

Re: PING [PATCH] fix ICE in __builtin_has_attribute (PR 88383 and 89288)

[Jeff Law]

On 3/19/19 8:22 PM, Joseph Myers wrote:
> On Tue, 19 Mar 2019, Jeff Law wrote:
> 
>> I'll note that our documentation clearly states that attributes can be
>> applied to functions, variables, labels, enums, statements and types.
> 
> A key thing here is that they can be applied to fields - that is, they can 
> be properties of a FIELD_DECL.  Referring to a field either requires an 
> expression referring to that field, or some other custom syntax that uses 
> both a type name and a field name (like in __builtin_offsetof).
> 
Thanks for chiming in, your opinions on this kind of thing are greatly
appreciated.

Understood WRT applying to fields, and I think that's consistent with
some of what Jakub has expressed elsewhere -- specifically that we
should consider allowing COMPONENT_REFs as the exception, returning the
attributes of the associated FIELD_DECL in that case.

Is there a need to call out BIT_FIELD_REFs where we can't actually get
to the underlying DECL?  And if so, how do we do that in the end user
documentation that is clear and consistent?

One of the big worries I've got here is that documenting when an
expression as an argument to __builtin_has_attribute (or any attribute
query) can be expected to work.  It's always easier on our end users to
loosen semantics of extensions over time than it is to tighten them.


Jeff

[PATCH V3] PR88497 - Extend reassoc for vector bit_field_ref

[Kewen.Lin]

Hi,

Please refer to below link for previous threads.
https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00348.html

Comparing to patch v2, I've moved up the vector operation target 
check upward together with vector type target check.  Besides, I
ran bootstrap and regtest on powerpc64-linux-gnu (BE), updated 
testcases' requirements and options for robustness.

Is it OK for GCC10?


gcc/ChangeLog

2019-03-20  Kewen Lin  

PR target/88497
* tree-ssa-reassoc.c (reassociate_bb): Swap the positions of 
GIMPLE_BINARY_RHS check and gimple_visited_p check, call new 
function undistribute_bitref_for_vector.
(undistribute_bitref_for_vector): New function.
(cleanup_vinfo_map): Likewise.
(unsigned_cmp): Likewise.

gcc/testsuite/ChangeLog

2019-03-20  Kewen Lin  

* gcc.dg/tree-ssa/pr88497-1.c: New test.
* gcc.dg/tree-ssa/pr88497-2.c: Likewise.
* gcc.dg/tree-ssa/pr88497-3.c: Likewise.
* gcc.dg/tree-ssa/pr88497-4.c: Likewise.
* gcc.dg/tree-ssa/pr88497-5.c: Likewise.

---
 gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c |  44 +
 gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c |  33 
 gcc/testsuite/gcc.dg/tree-ssa/pr88497-3.c |  33 
 gcc/testsuite/gcc.dg/tree-ssa/pr88497-4.c |  33 
 gcc/testsuite/gcc.dg/tree-ssa/pr88497-5.c |  33 
 gcc/tree-ssa-reassoc.c| 306 +-
 6 files changed, 477 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88497-3.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88497-4.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88497-5.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c
new file mode 100644
index 000..99c9af8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_double } */
+/* { dg-require-effective-target powerpc_vsx_ok { target { powerpc*-*-* } } } 
*/
+/* { dg-options "-O2 -ffast-math" } */
+/* { dg-options "-O2 -ffast-math -mvsx -fdump-tree-reassoc1" { target { 
powerpc*-*-* } } } */
+
+/* To test reassoc can undistribute vector bit_field_ref summation.
+
+   arg1 and arg2 are two arrays whose elements of type vector double.
+   Assuming:
+ A0 = arg1[0], A1 = arg1[1], A2 = arg1[2], A3 = arg1[3],
+ B0 = arg2[0], B1 = arg2[1], B2 = arg2[2], B3 = arg2[3],
+
+   Then:
+ V0 = A0 * B0, V1 = A1 * B1, V2 = A2 * B2, V3 = A3 * B3,
+
+   reassoc transforms
+
+ accumulator += V0[0] + V0[1] + V1[0] + V1[1] + V2[0] + V2[1]
+ + V3[0] + V3[1];
+
+   into:
+
+ T = V0 + V1 + V2 + V3
+ accumulator += T[0] + T[1];
+
+   Fewer bit_field_refs, only two for 128 or more bits vector.  */
+
+typedef double v2df __attribute__ ((vector_size (16)));
+double
+test (double accumulator, v2df arg1[], v2df arg2[])
+{
+  v2df temp;
+  temp = arg1[0] * arg2[0];
+  accumulator += temp[0] + temp[1];
+  temp = arg1[1] * arg2[1];
+  accumulator += temp[0] + temp[1];
+  temp = arg1[2] * arg2[2];
+  accumulator += temp[0] + temp[1];
+  temp = arg1[3] * arg2[3];
+  accumulator += temp[0] + temp[1];
+  return accumulator;
+}
+/* { dg-final { scan-tree-dump-times "BIT_FIELD_REF" 2 "reassoc1" { target { 
powerpc*-*-* } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c
new file mode 100644
index 000..61ed0bf5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_float } */
+/* { dg-require-effective-target powerpc_altivec_ok { target { powerpc*-*-* } 
} } */
+/* { dg-options "-O2 -ffast-math" } */
+/* { dg-options "-O2 -ffast-math -maltivec -fdump-tree-reassoc1" { target { 
powerpc*-*-* } } } */
+
+/* To test reassoc can undistribute vector bit_field_ref on multiplication.
+
+   v1, v2, v3, v4 of type vector float.
+
+   reassoc transforms
+
+ accumulator *= v1[0] * v1[1] * v1[2] * v1[3] *
+v2[0] * v2[1] * v2[2] * v2[3] *
+v3[0] * v3[1] * v3[2] * v3[3] *
+v4[0] * v4[1] * v4[2] * v4[3] ;
+
+   into:
+
+ T = v1 * v2 * v3 * v4;
+ accumulator *= T[0] * T[1] * T[2] * T[3];
+
+   Fewer bit_field_refs, only four for 128 or more bits vector.  */
+
+typedef float v4si __attribute__((vector_size(16)));
+float test(float accumulator, v4si v1, v4si v2, v4si v3, v4si v4) {
+  accumulator *= v1[0] * v1[1] * v1[2] * v1[3];
+  accumulator *= v2[0] * v2[1] * v2[2] * v2[3];
+  accumulator *= v3[0] * v3[1] * v3[2] * v3[3];
+  accumulator *= v4[0] * v4[1] * v4[2] * v4[3];
+  return accumulator;
+}
+/* { dg-final { scan-tree-dump-times "BIT_FIELD_REF" 4 "reassoc1" { target { 
powerpc*-*-* } } } } */
diff --git 

Re: PING [PATCH] fix ICE in __builtin_has_attribute (PR 88383 and 89288)

[Joseph Myers]

On Tue, 19 Mar 2019, Jeff Law wrote:

> I'll note that our documentation clearly states that attributes can be
> applied to functions, variables, labels, enums, statements and types.

A key thing here is that they can be applied to fields - that is, they can 
be properties of a FIELD_DECL.  Referring to a field either requires an 
expression referring to that field, or some other custom syntax that uses 
both a type name and a field name (like in __builtin_offsetof).

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

[SVE ACLE] svbic implementation

[Kugan Vivekanandarajah]

I have committed attached patch to aarch64/sve-acle-branch branch
which implements svbic.

Thanks,
Kugan
From 182bd15334874844bef5e317f55a6497f77e12ff Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah 
Date: Thu, 24 Jan 2019 20:57:19 +1100
Subject: [PATCH 1/3] svbic

Change-Id: I819490ec63ee38b9cdc7c5e342436b7afdee2973
---
 gcc/config/aarch64/aarch64-sve-builtins.c  |  30 ++
 gcc/config/aarch64/aarch64-sve-builtins.def|   1 +
 gcc/config/aarch64/aarch64-sve.md  |  54 ++-
 .../gcc.target/aarch64/sve-acle/asm/bic_s16.c  | 398 +
 .../gcc.target/aarch64/sve-acle/asm/bic_s32.c  | 394 
 .../gcc.target/aarch64/sve-acle/asm/bic_s64.c  | 394 
 .../gcc.target/aarch64/sve-acle/asm/bic_s8.c   | 317 
 .../gcc.target/aarch64/sve-acle/asm/bic_u16.c  | 398 +
 .../gcc.target/aarch64/sve-acle/asm/bic_u32.c  | 394 
 .../gcc.target/aarch64/sve-acle/asm/bic_u64.c  | 394 
 .../gcc.target/aarch64/sve-acle/asm/bic_u8.c   | 317 
 11 files changed, 3087 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_s16.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_s32.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_s64.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_s8.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_u16.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_u32.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_u64.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve-acle/asm/bic_u8.c

diff --git a/gcc/config/aarch64/aarch64-sve-builtins.c b/gcc/config/aarch64/aarch64-sve-builtins.c
index 0e3db66..106f21e 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.c
+++ b/gcc/config/aarch64/aarch64-sve-builtins.c
@@ -166,6 +166,7 @@ enum function {
   FUNC_svadd,
   FUNC_svand,
   FUNC_svasrd,
+  FUNC_svbic,
   FUNC_svdiv,
   FUNC_svdivr,
   FUNC_svdot,
@@ -474,6 +475,7 @@ private:
   rtx expand_add (unsigned int);
   rtx expand_and ();
   rtx expand_asrd ();
+  rtx expand_bic ();
   rtx expand_div (bool);
   rtx expand_dot ();
   rtx expand_dup ();
@@ -1214,6 +1216,7 @@ arm_sve_h_builder::get_attributes (const function_instance )
 case FUNC_svadd:
 case FUNC_svand:
 case FUNC_svasrd:
+case FUNC_svbic:
 case FUNC_svdiv:
 case FUNC_svdivr:
 case FUNC_svdot:
@@ -1887,6 +1890,7 @@ gimple_folder::fold ()
 case FUNC_svadd:
 case FUNC_svand:
 case FUNC_svasrd:
+case FUNC_svbic:
 case FUNC_svdiv:
 case FUNC_svdivr:
 case FUNC_svdot:
@@ -1990,6 +1994,9 @@ function_expander::expand ()
 case FUNC_svdot:
   return expand_dot ();
 
+case FUNC_svbic:
+  return expand_bic ();
+
 case FUNC_svdup:
   return expand_dup ();
 
@@ -2133,6 +2140,29 @@ function_expander::expand_dot ()
 return expand_via_unpred_direct_optab (sdot_prod_optab);
 }
 
+/* Expand a call to svbic.  */
+rtx
+function_expander::expand_bic ()
+{
+  if (CONST_INT_P (m_args[2]))
+{
+  machine_mode mode = GET_MODE_INNER (get_mode (0));
+  m_args[2] = simplify_unary_operation (NOT, mode, m_args[2], mode);
+  return expand_and ();
+}
+
+  if (m_fi.pred == PRED_x)
+{
+  insn_code icode = code_for_aarch64_bic (get_mode (0));
+  return expand_via_unpred_insn (icode);
+}
+  else
+{
+  insn_code icode = code_for_cond_bic (get_mode (0));
+  return expand_via_pred_insn (icode);
+}
+}
+
 /* Expand a call to svdup.  */
 rtx
 function_expander::expand_dup ()
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.def b/gcc/config/aarch64/aarch64-sve-builtins.def
index 8322c4b..4af06ac 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.def
+++ b/gcc/config/aarch64/aarch64-sve-builtins.def
@@ -65,6 +65,7 @@ DEF_SVE_FUNCTION (svabs, unary, all_signed_and_float, mxz)
 DEF_SVE_FUNCTION (svadd, binary_opt_n, all_data, mxz)
 DEF_SVE_FUNCTION (svand, binary_opt_n, all_integer, mxz)
 DEF_SVE_FUNCTION (svasrd, shift_right_imm, all_signed, mxz)
+DEF_SVE_FUNCTION (svbic, binary_opt_n, all_integer, mxz)
 DEF_SVE_FUNCTION (svdiv, binary_opt_n, all_sdi_and_float, mxz)
 DEF_SVE_FUNCTION (svdivr, binary_opt_n, all_sdi_and_float, mxz)
 DEF_SVE_FUNCTION (svdot, ternary_qq_opt_n, sdi, none)
diff --git a/gcc/config/aarch64/aarch64-sve.md b/gcc/config/aarch64/aarch64-sve.md
index d480289..5e629de 100644
--- a/gcc/config/aarch64/aarch64-sve.md
+++ b/gcc/config/aarch64/aarch64-sve.md
@@ -1360,6 +1360,52 @@
   [(set_attr "movprfx" "*,yes,*")]
 )
 
+;; Predicated BIC with select.
+(define_expand "@cond_bic"
+  [(set (match_operand:SVE_I 0 "register_operand")
+	(unspec:SVE_I
+	  [(match_operand: 1 "register_operand")
+	   (and:SVE_I
+	 (not:SVE_I (match_operand:SVE_I 3 

[Patch, Fortran, F03] PR 71861: [7/8/9 Regression] ICE in write_symbol(): bad module symbol

[Janus Weil]

Hi all,

the attached one-line patch fixes an ICE-on-invalid regression with
abstract interfaces. Regtests cleanly on x86_64-linux-gnu. Ok for
trunk and the release branches (7 and 8)?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 4dd35ec6030..cee2b2cfd15 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-19  Janus Weil  
+
+	PR fortran/71861
+	* symbol.c (check_conflict): ABSTRACT attribute conflicts with
+	INTRINSIC attribute.
+
 2019-03-18  Thomas Koenig  
 
 	PR fortran/68009
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index c342d62ead1..ec753229a98 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -557,6 +557,7 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
 
   conf (external, intrinsic);
   conf (entry, intrinsic);
+  conf (abstract, intrinsic);
 
   if ((attr->if_source == IFSRC_DECL && !attr->procedure) || attr->contained)
 conf (external, subroutine);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index cf01b2fea19..4649e282f64 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-03-19  Janus Weil  
+
+	PR fortran/71861
+	* gfortran.dg/interface_abstract_5.f90: New test case.
+
 2019-03-19  Martin Sebor  
 
 	PR tree-optimization/89644
diff --git a/gcc/testsuite/gfortran.dg/interface_abstract_5.f90 b/gcc/testsuite/gfortran.dg/interface_abstract_5.f90
new file mode 100644
index 000..fddf6b89d27
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/interface_abstract_5.f90
@@ -0,0 +1,32 @@
+! { dg-do compile }
+!
+! PR 71861: [7/8/9 Regression] [F03] ICE in write_symbol(): bad module symbol
+!
+! Contributed by Gerhard Steinmetz 
+
+module m1
+   intrinsic abs
+   abstract interface
+  function abs(x)! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+ real :: abs, x
+  end
+   end interface
+end
+
+module m2
+   abstract interface
+  function abs(x)
+ real :: abs, x
+  end
+   end interface
+   intrinsic abs! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+end
+
+module m3
+   abstract interface
+  function f(x)
+ real :: f, x
+  end
+   end interface
+   intrinsic f! { dg-error "ABSTRACT attribute conflicts with INTRINSIC attribute" }
+end

[PATCH] RISC-V: Fix %lo overflow with BLKmode references.

[Jim Wilson]

This fixes the bug reported in PR 89411 where we accidentally generated a
%lo with an offset that overflowed at link time if the symbol was allocated
to an address just below the 2KB limit for %lo.  This solves the problem
by checking for BLKmode references, pulling out the SYMBOL_REF_DECL, and
then getting size and alignment from the decl instead of from the mode.

Tested with 32-bit/64-bit elf/linux cross builds and checks.  There were no
regressions.  The new testcase fails without the patch and works with the
patch.

Committed.

Jim

gcc/
PR target/89411
* config/riscv/riscv.c (riscv_valid_lo_sum_p): New arg x.  New locals
align, size, offset.  Use them to handle a BLKmode reference.  Update
comment.
(riscv_classify_address): Pass info->offset to riscv_valid_lo_sum_p.

gcc/testsuite/
PR target/89411
* gcc.target/riscv/losum-overflow.c: New test.
---
 gcc/config/riscv/riscv.c  | 43 ---
 .../gcc.target/riscv/losum-overflow.c | 29 +
 2 files changed, 66 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/losum-overflow.c

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 8e78ab76375..d8446f82b96 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -708,11 +708,15 @@ riscv_split_symbol_type (enum riscv_symbol_type 
symbol_type)
 }
 
 /* Return true if a LO_SUM can address a value of mode MODE when the
-   LO_SUM symbol has type SYM_TYPE.  */
+   LO_SUM symbol has type SYM_TYPE.  X is the LO_SUM second operand, which
+   is used when the mode is BLKmode.  */
 
 static bool
-riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, machine_mode mode)
+riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, machine_mode mode,
+ rtx x)
 {
+  int align, size;
+
   /* Check that symbols of type SYMBOL_TYPE can be used to access values
  of mode MODE.  */
   if (riscv_symbol_insns (sym_type) == 0)
@@ -722,11 +726,38 @@ riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, 
machine_mode mode)
   if (!riscv_split_symbol_type (sym_type))
 return false;
 
+  /* We can't tell size or alignment when we have BLKmode, so try extracing a
+ decl from the symbol if possible.  */
+  if (mode == BLKmode)
+{
+  rtx offset;
+
+  /* Extract the symbol from the LO_SUM operand, if any.  */
+  split_const (x, , );
+
+  /* Might be a CODE_LABEL.  We can compute align but not size for that,
+so don't bother trying to handle it.  */
+  if (!SYMBOL_REF_P (x))
+   return false;
+
+  /* Use worst case assumptions if we don't have a SYMBOL_REF_DECL.  */
+  align = (SYMBOL_REF_DECL (x)
+  ? DECL_ALIGN (SYMBOL_REF_DECL (x))
+  : 1);
+  size = (SYMBOL_REF_DECL (x) && DECL_SIZE (SYMBOL_REF_DECL (x))
+ ? tree_to_uhwi (DECL_SIZE (SYMBOL_REF_DECL (x)))
+ : 2*BITS_PER_WORD);
+}
+  else
+{
+  align = GET_MODE_ALIGNMENT (mode);
+  size = GET_MODE_BITSIZE (mode);
+}
+
   /* We may need to split multiword moves, so make sure that each word
  can be accessed without inducing a carry.  */
-  if (GET_MODE_SIZE (mode) > UNITS_PER_WORD
-  && (!TARGET_STRICT_ALIGN
- || GET_MODE_BITSIZE (mode) > GET_MODE_ALIGNMENT (mode)))
+  if (size > BITS_PER_WORD
+  && (!TARGET_STRICT_ALIGN || size > align))
 return false;
 
   return true;
@@ -772,7 +803,7 @@ riscv_classify_address (struct riscv_address_info *info, 
rtx x,
   info->symbol_type
= riscv_classify_symbolic_expression (info->offset);
   return (riscv_valid_base_register_p (info->reg, mode, strict_p)
- && riscv_valid_lo_sum_p (info->symbol_type, mode));
+ && riscv_valid_lo_sum_p (info->symbol_type, mode, info->offset));
 
 case CONST_INT:
   /* Small-integer addresses don't occur very often, but they
diff --git a/gcc/testsuite/gcc.target/riscv/losum-overflow.c 
b/gcc/testsuite/gcc.target/riscv/losum-overflow.c
new file mode 100644
index 000..9c01c7feb54
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/losum-overflow.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32gc -mabi=ilp32 -O2 -fno-section-anchors" } */
+
+/* Check for %lo overflow.  Adding an offset larger than the alignment can
+   overflow if the data is allocated to an address mod 4KB that is between
+   2KB-offset+1 and 2KB-1.  */
+typedef long long int int64_t;
+
+#pragma pack(push)
+#pragma pack(1)
+struct S0 {
+   signed f0 : 4;
+   const volatile int64_t  f1;
+   volatile signed f2 : 1;
+   signed f3 : 31;
+   unsigned f4 : 8;
+   signed f5 : 20;
+   unsigned f6 : 5;
+};
+#pragma pack(pop)
+
+struct S0 g_3030 = {0,-9L,-0,-22553,7,-841,1};
+
+int64_t
+sub (void)
+{
+  return g_3030.f1;
+}
+/* { dg-final { scan-assembler-not "%lo\\(g_3030\\+4\\)" } } */
-- 
2.17.1

Re: [PATCH] fix ice in attribute copy (PR 89685)

[Martin Sebor]

On 3/19/19 12:42 PM, Jeff Law wrote:

On 3/14/19 7:47 PM, Martin Sebor wrote:

To copy type attributes from struct A, the copy attribute (new
in GCC 9) accepts a pointer argument such as (struct A*)0, but
it isn't prepared for anything much more complicated than that.
So for example when it's passed something like (struct A*)(0, 1)
as the test case in PR 89685 does (a P1 regression), it fails
with an ICE.

The attached patch makes this handling more robust by letting
it accept all forms of type and member references.

Tested on x86_64-linux.

Martin

gcc-89685.diff

PR c/89685 - ICE on attribute copy with a compound expression

gcc/c-family/ChangeLog:

PR c/89685
* c-attribs.c (handle_copy_attribute): Handle references and
non-constant expressions.

gcc/testsuite/ChangeLog:

PR c/89685
* gcc.dg/attr-copy-8.c: New test.
* g++.dg/ext/attr-copy-2.C: New test.I think this is in the same state 
as the __builtin_has_attribute bits --

you're trying to support attributes on expressions.  We should reject
those as syntax errors right now.


There is no way for attribute copy to refer to a type but by
mentioning an expression:

  struct __attribute__ ((aligned (8))) A { ... };
  struct __attribute__ ((copy ((struct S*)0))) struct B { };

Copying type attributes is one third of the feature's documented
purpose:

  copy
  copy (expression)

The copy attribute applies the set of attributes with which
the type of the expression has been declared to the declaration
of the type to which the attribute is applied.

Why are you so determined to break these features?

Martin

Re: Implement C++20 constexpr , , and

[Ed Smith-Rowland via gcc-patches]

On 3/19/19 4:57 PM, Ed Smith-Rowland via libstdc++ wrote:

On 3/18/19 6:18 PM, Jonathan Wakely wrote:

On 17/03/19 22:54 -0400, Ed Smith-Rowland via libstdc++ wrote:

Greetings,

This patch implements C++20 p0202 - Add Constexpr Modifiers to 
Functions in  and  Headers


and C++20 p1023 - constexpr comparison operators for std::array.


The patch is large because of the testsuite additions. Basically, 
the algorithms and the array comparison operators are all marked 
constexpr for C++20.  This has been built and tested on x86_64-linux.


As discussed on IRC< the tests need to be run with -std=gnu++2a to
ensure everything we test (which is not exhaustive, but is the best we
can do) still works in C++2a mode.

The indentation went bad here:

  template_BinaryPredicate>

-    _FIter1
+    _GLIBCXX20_CONSTEXPR
+   _FIter1
    find_first_of(_FIter1, _FIter1, _FIter2, _FIter2, _BinaryPredicate);

Fixed.


I would have to look through old emails but I think there's a reason
the function objects like _Iter_comp_to_val take their arguments by
non-const reference.

I'm very surprised that none of the algos that dispatch to
__builtin_memove need changes, because those optimizations won't work
in constant expressions. I would expect to have to use
std::is_constant_evaluated to disable the optimizations when used in
constant expressions.


As am I. As I mentioned on IRC I could roll a constexpr memmove.

I was wondering if somehow I'm not checking what I think I'm checking 
(but I don't see how.)


I wonder if the builtins are handled differently somehow by the front 
end.  I'm still not sure why __builtin_memcmp is OK for array == array 
but not array != array.  In that case I just do element by element 
compare for constexpr now anyway.


Ed


Ignore this patch,

I'm not actually getting constexpr.

Ed

Re: PING [PATCH] fix ICE in __builtin_has_attribute (PR 88383 and 89288)

[Martin Sebor]

On 3/19/19 12:30 PM, Jeff Law wrote:

On 2/22/19 9:42 AM, Marek Polacek wrote:

On Thu, Feb 21, 2019 at 03:39:21PM -0700, Martin Sebor wrote:

On 2/21/19 1:27 PM, Jeff Law wrote:

On 2/21/19 1:12 PM, Martin Sebor wrote:

On 2/21/19 12:08 PM, Jeff Law wrote:

On 2/18/19 7:53 PM, Martin Sebor wrote:

Please let me know what it will take to get the fix for these two
issues approved.  I've answered the questions so I don't know what
else I'm expected to do here.

     https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00793.html

I think there is still a fundamental disagreement about whether or not
this should be handling expressions.  Without an agreement on that it's
hard to see how this could go forward.


I think it's wrong to hold up a fix for an ICE because you don't
like the current design.  The built-in successfully handles common
expressions (see c-c++-common/builtin-has-attribute-5.c).  It just
fails for some of the less common ones.  Not fixing those will only
penalize users who run into the ICE, and cast a bad light on
the quality of the release.   Any design questions should be
settled separately of these ICEs (should have been when
the feature was being reviewed).

That said, I have explained the rationale for the current design.
Neither you nor Jakub has explained what you find wrong with it or
why either of your alternatives is preferable.  You said it should
be an error to apply the built-in to expressions (why?).  Jakub
thought there perhaps should be two built-ins, one for expressions
and another for decls.  His rationale?  The current design is not
good.  Clearly,  the two of you don't agree on what you'd like to
see; the only thing you agree on is that you don't like what's
there now.  What do you expect me to do with that, now at the end
of stage 4?

Fix the ice in another way.  Handling expressions here seems
fundamentally wrong.  ISTM that making this query on an expression
should result in an immediate error.


Sorry but "it seems wrong" is not a compelling argument.  You need
to explain what you think is wrong with it.


"Attributes apply to decls and types, not expressions" seems like an
argument strong enough.  (There are also special attributes for enums
  and ';' and labels.)

So I think Martin's argument is that for an expression we can look at
the type of the expression and see if the attribute is on that type.

That leads to some inconsistencies (that Jakub has pointed out) where an
unfolded expression could produce a different result than it's folded
result if the result was a constant.


The built-in is evaluated during parsing (same as __alignof__)
so I'm not sure I understand what inconsistencies it could be
subject to that __alignof__ isn't.  I'm open to examples, I just
can't think of any.



AFAICT the goal (outside handling just types) here is to be able to ask
things like is a particular field within a structure packed, aliasing
properties of objects, etc.


Yes.  The goal is to query functions, variables (including members),
and types for most attributes known to GCC that may be attached to
them.



My next thought was to use typeof to extract the type of the expression
and pass that through (independent of Marek suggesting the same).
Martin points out in a subsequent email that won't work because p->a in
the supplied example returns an int rather than a packed int.   One
could argue that's the wrong return value for typeof, but I don't think
it's that clear cut.  In this specific instance I'd claim it's wrong,
but I'd hazard a guess we don't have clear semantics in this space.


__alignof__ is documented to work this way:

  If the operand of __alignof__ is an lvalue rather than a type,
  its value is the required alignment for its type, taking into
  account any minimum alignment specified with GCC's __attribute__
  extension (see Variable Attributes). For example, after this
  declaration:

 struct foo { int x; char y; } foo1;

  the value of __alignof__ (foo1.y) is 1, even though its actual
  alignment is probably 2 or 4, the same as __alignof__ (int).

What isn't documented is the semantics for expressions that aren't
lvalues.  They are nevertheless accepted and, AFAICT, __alignof__
returns the alignment of their type.



I'll note that our documentation clearly states that attributes can be
applied to functions, variables, labels, enums, statements and types.
No provision AFAICT is made for expressions.  Users have never had the
ability or assumption that they can ask for attributes on an expression.


Not quite.  As I said above, __alignof__ accepts expressions
because there is no way to apply the operator to a member without
using an expression of some sort.  But it also accepts any other
expressions.


Martin's patch essentially allows for asking indirectly -- but I suspect
it's going to be inconsistent in more ways than Jakub pointed out as the
type system in gimple is "loose" in certain ways and the result could
well change as we go through the optimizer 

Re: Fix two ICEs with C++ destructors and LTO

[Rainer Orth]

Hi Jan,

> these two PRs are about C++ destructors that are not virtual but have
> virtual alias. The devirtualization machinery walks from alias to symbol
> and is then confused by not knowing the class symbol belongs to.
>
> I think it would make more sense to avoid these walks but that require
> more of surgery, so for GCC9 I think it is best to stop freeing contexts
> for desctructors. This does not save any stremaing because THIS pointer
> of the destructor has the same type.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
>   PR lto/87809
>   PR lto/89335
>   * tree.c (free_lang_data_in_decl): Do not free context of C++
>   destrutors.
>
>   * g++.dg/lto/pr87089_0.C: New testcase.
>   * g++.dg/lto/pr87089_1.C: New testcase.
>   * g++.dg/lto/pr89335_0.C: New testcase.

this test FAILs on Solaris with the native ld:

FAIL: g++.dg/lto/pr89335 cp_lto_pr89335_0.o assemble, -O2 -flto 
-Wsuggest-final-methods

/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/lto/pr89335_0.C:9:7: warning: 
Declaring virtual destructor of 'struct List' final would enable 
devirtualization of 1 call [-Wsuggest-final-methods]

The same failure can be seen on Linux/x86_64 with -fno-use-linker-plugin.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Implement C++20 constexpr , , and

[Ed Smith-Rowland via gcc-patches]

On 3/18/19 6:18 PM, Jonathan Wakely wrote:

On 17/03/19 22:54 -0400, Ed Smith-Rowland via libstdc++ wrote:

Greetings,

This patch implements C++20 p0202 - Add Constexpr Modifiers to 
Functions in  and  Headers


and C++20 p1023 - constexpr comparison operators for std::array.


The patch is large because of the testsuite additions. Basically, the 
algorithms and the array comparison operators are all marked 
constexpr for C++20.  This has been built and tested on x86_64-linux.


As discussed on IRC< the tests need to be run with -std=gnu++2a to
ensure everything we test (which is not exhaustive, but is the best we
can do) still works in C++2a mode.

The indentation went bad here:

  template
-    _FIter1
+    _GLIBCXX20_CONSTEXPR
+   _FIter1
    find_first_of(_FIter1, _FIter1, _FIter2, _FIter2, _BinaryPredicate);

Fixed.


I would have to look through old emails but I think there's a reason
the function objects like _Iter_comp_to_val take their arguments by
non-const reference.

I'm very surprised that none of the algos that dispatch to
__builtin_memove need changes, because those optimizations won't work
in constant expressions. I would expect to have to use
std::is_constant_evaluated to disable the optimizations when used in
constant expressions.


As am I. As I mentioned on IRC I could roll a constexpr memmove.

I was wondering if somehow I'm not checking what I think I'm checking 
(but I don't see how.)


I wonder if the builtins are handled differently somehow by the front 
end.  I'm still not sure why __builtin_memcmp is OK for array == array 
but not array != array.  In that case I just do element by element 
compare for constexpr now anyway.


Ed


2019-03-19  Edward Smith-Rowland  <3dw...@verizon.net>

Implement C++20 p0202 - Add Constexpr Modifiers to Functions
in  and  Headers.
Implement C++20 p1023 - constexpr comparison operators for std::array.
* include/bits/algorithmfwd.h (all_of, any_of, binary_search, copy,
copy_backward, copy_if, copy_n, equal_range, fill, fill_n, find_end,
find_if_not, includes, is_heap, is_heap_until, is_partitioned,
is_permutation, is_sorted, is_sorted_until, lower_bound, none_of,
partition_copy, partition_point, remove, remove_if, remove_copy,
remove_copy_if, replace_copy, replace_copy_if, reverse_copy,
rotate_copy, unique, upper_bound, adjacent_find, count, count_if, equal,
find, find_first_of, find_if, for_each, generate, generate_n,
lexicographical_compare, merge, mismatch, replace, replace_if, search,
search_n, set_difference, set_intersection, set_symmetric_difference,
set_union, transform, unique_copy): Mark constexpr.
Make versions of operator() const.
* include/bits/cpp_type_traits.h (__miter_base): Make constexpr.
* include/bits/predefined_ops.h (_Iter_less_val, __iter_comp_val,
_Val_less_iter, __val_less_iter, __val_comp_iter, _Iter_equal_to_iter,
__iter_equal_to_iter, _Iter_equal_to_val, __iter_equal_to_val,
_Iter_comp_val, __iter_comp_val, _Val_comp_iter, __val_comp_iter,
_Iter_equals_val, __iter_equals_val, _Iter_equals_iter,
__iter_comp_iter, __pred_iter, __iter_comp_val, __negate): Constexpr
ctors. Add const operator().
* include/bits/stl_algo.h (__find_if, __find_if_not, __find_if_not_n,
__search, __search_n_aux, __search_n, __find_end, find_end, all_of
none_of, any_of, find_if_not, is_partitioned, partition_point,
__remove_copy_if, remove_copy, remove_copy_if, copy_if, __copy_n,
copy_n, partition_copy, __remove_if, remove, remove_if, __adjacent_find,
__unique, unique, __unique_copy, reverse_copy, rotate_copy,
__unguarded_linear_insert, __insertion_sort, __unguarded_insertion_sort,
__final_insertion_sort, lower_bound, __upper_bound, upper_bound,
__equal_range, equal_range, binary_search, __includes, includes,
__next_permutation, __prev_permutation, __replace_copy_if, replace_copy,
replace_copy_if, __count_if, is_sorted, __is_sorted_until,
is_sorted_until, __is_permutation, is_permutation, for_each, find,
find_if, find_first_of, adjacent_find, count, count_if, search,
search_n, transform, replace, replace_if, generate, generate_n,
unique_copy, __merge, merge, __set_union, set_union, __set_intersection,
set_intersection, __set_difference, set_difference,
__set_symmetric_difference, set_symmetric_difference): Constexpr.
* include/bits/stl_algobase.h (__niter_base, __niter_wrap, __copy_m,
__copy_move_a, copy, move, __copy_move_backward::__copy_move_b,
__copy_move_backward_a, __copy_move_backward_a2, copy_backward,
move_backward, fill, __fill_n_a, fill_n, __equal::equal, __equal_aux,
__newlast1, __cnd2, __newlast1, __cnd2, __lexicographical_compare_impl
__lexicographical_compare::__lc, 

Re: RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.

[Jeff Law]

On 3/5/19 3:40 PM, Jakub Jelinek wrote:
> On Tue, Mar 05, 2019 at 05:33:45PM -0500, Jason Merrill wrote:
>> The x86_64 psABI says that an empty class isn't passed or returned in memory
>> or registers, so we shouldn't set %eax in this function.  This seems like a
>> missed case from the GCC 8 TYPE_EMPTY_P changes.
>>
>> Shall I apply this for GCC 9, or hold it for stage 1?
>>
>>  * function.c (expand_function_end): Don't copy a TYPE_EMPTY_P
>> result.
> 
> I'd go for it now.  See below for a testsuite nit:
Agreed.

> 
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/opt/empty3.C
>> @@ -0,0 +1,8 @@
>> +// PR c++/88529
>> +// { dg-do compile { target c++11 } }
>> +// { dg-final { scan-assembler-not mov { target x86_64-*-* } } }
> 
> But this probably needs to be { target { { i?86-*-* x86_64-*-* } && { ! { 
> ia32 } } } }
> or similar instead, so that it is done for x86_64 -m64 and -mx32, but not
> -m32.
> 
>> +// The x86_64 psABI says that f() doesn't put the return value anywhere.
>> +
>> +class A{};
>> +
>> +A f() { return {}; }
> 
>   Jakub
> 

Re: [PATCH] PR libgcc/60790: Avoid IFUNC resolver access to uninitialized data

[Jeff Law]

On 3/19/19 9:00 AM, Florian Weimer wrote:
> * Florian Weimer:
> 
>> * Jeff Law:
>>
>>> On 03/29/2018 08:00 AM, Florian Weimer wrote:
 This patch performs lazy initialization of the relevant CPUID feature
 register value.  It will needlessly invoke the CPUID determination code
 on architectures which lack CPUID support or support for the feature
 register, but I think it's not worth to avoid the complexity for that.

 I verified manually that the CMPXCHG16B implementation is still selected
 for a 128-bit load after the change.  I don't know how to write an
 automated test for that.

 Thanks,
 Florian

 pr60790.patch


 Index: libatomic/ChangeLog
 ===
 --- libatomic/ChangeLog(revision 258952)
 +++ libatomic/ChangeLog(working copy)
 @@ -1,3 +1,18 @@
 +2018-03-29  Florian Weimer  
 +
 +  PR libgcc/60790
 +  x86: Do not assume ELF constructors run before IFUNC resolvers.
 +  * config/x86/host-config.h (libat_feat1_ecx, libat_feat1_edx):
 +  Remove declarations.
 +  (__libat_feat1, __libat_feat1_init): Declare.
 +  (FEAT1_REGISTER): Define.
 +  (load_feat1): New function.
 +  (IFUNC_COND_1): Adjust.
 +  * config/x86/init.c (libat_feat1_ecx, libat_feat1_edx)
 +  (init_cpuid): Remove definitions.
 +  (__libat_feat1): New variable.
 +  (__libat_feat1_init): New function.
>>> OK.
>>
>> Here is the backport to gcc-8-branch.
> 
> Ping?  
OK for the branch as well.  I didn't know you were waiting on me :-)
jeff

Re: Fixing ifcvt issue as exposed by BZ89430

[Jeff Law]

On 2/28/19 5:26 AM, JiangNing OS wrote:
> To solve BZ89430 the followings are needed,
> 
> (1) The code below in noce_try_cmove_arith needs to be fixed.
> 
>   /* ??? We could handle this if we knew that a load from A or B could
>  not trap or fault.  This is also true if we've already loaded
>  from the address along the path from ENTRY.  */
>   else if (may_trap_or_fault_p (a) || may_trap_or_fault_p (b))
> return FALSE;
> 
> Finding dominating memory access could help to decide whether the memory 
> access following the conditional move is valid or not. 
> * If there is a dominating memory write to the same memory address in test_bb 
> as the one from x=a, it is always safe.
> * When the dominating memory access to the same memory address in test_bb is 
> a read, for the load out of x=a, it is always safe. For the store out of x=a, 
> if the memory is on stack, it is still safe.
> 
> Besides, there is a bug around rearranging the then_bb and else_bb layout, 
> which needs to be fixed.
> 
> (2) The fix by https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02998.html  is 
> an overkill. If the write target following conditional move is a memory 
> access, it exits shortly.
> 
>   if (!set_b && MEM_P (orig_x))
> /* We want to avoid store speculation to avoid cases like
>  if (pthread_mutex_trylock(mutex))
>++global_variable;
>Rather than go to much effort here, we rely on the SSA optimizers,
>which do a good enough job these days.  */
> return FALSE;
> 
> It looks a bit hard for compiler to know the program semantic is related to 
> mutex and avoid store speculation. Without removing this overkill, the fix 
> mentioned by (1) would not work. Any idea? An alternative solution is to 
> detect the following pattern conservatively,
> 
>  if (a_function_call(...))
>++local_variable;
> 
> If the local_variable doesn't have address taken at all in current function, 
> it mustn't be a pthread mutex lock semantic, and then the following patch 
> would work to solve (1) and pass the last case as described in 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89430. 
Just a note. I'm deferring this to gcc-10.
jeff

Re: [PATCH] Integration of parallel standard algorithms for c++17

[Jonathan Wakely]

On 11/03/19 21:24 -0700, Thomas Rodgers wrote:

Let's try this patch -




The feature test macro should be 201603L (in  and
):

+// Feature test macro for parallel algorithms
+# define __cpp_lib_parallel_algorithm 201703L

***

The new files have copyright dates of 2018, but it's taken so long to
get the licensing changes done and for me to review it that they need
to say "2018-2019" now:

+++ b/libstdc++-v3/include/std/execution
@@ -0,0 +1,58 @@
+//  -*- C++ -*-
+
+// Copyright (C) 2018 Free Software Foundation, Inc.

***

The  header warns if included pre-C++17 but it should just
not define anything:

+#if __cplusplus < 201703L
+# include 
+#else

We only give that warning for C++11 headers, but for anything newer it
should be just:

+#if __cplusplus >= 201703L

***

There are still a couple of un-uglified names I noticed:
parallel_set_union_op, is_heap_until_local

***

The copyright notices at the top of each file seem a bit out of place
in the GCC tree:

+//===-- execution_defs.h 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//

I wonder if we should put another comment before that, saying GCC uses
the PSTL code from the LLVM upstream, or something like that. That can
wait though.

Re: A bug in vrp_meet?

[Jeff Law]

On 3/6/19 3:05 AM, Richard Biener wrote:
> On Tue, Mar 5, 2019 at 10:36 PM Jeff Law  wrote:
>>
>> On 3/5/19 7:44 AM, Richard Biener wrote:
>>
>>> So fixing it properly with also re-optimize_stmt those stmts so we'd CSE
>>> the MAX_EXPR introduced by folding makes it somewhat ugly.
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> Any ideas how to make it less so?  I can split out making optimize_stmt
>>> take a gsi * btw, in case that's a more obvious change and it makes the
>>> patch a little smaller.
>>>
>>> Richard.
>>>
>>> 2019-03-05  Richard Biener  
>>>
>>> PR tree-optimization/89595
>>> * tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Take
>>> stmt iterator as reference, take boolean output parameter to
>>> indicate whether the stmt was removed and thus the iterator
>>> already advanced.
>>> (dom_opt_dom_walker::before_dom_children): Re-iterate over
>>> stmts created by folding.
>>>
>>> * gcc.dg/torture/pr89595.c: New testcase.
>>>
>>
>> Well, all the real logic changs are in the before_dom_children method.
>> The bits in optimize_stmt are trivial enough to effectively ignore.
>>
>> I don't see a better way to discover and process statements that are
>> created in the bowels of fold_stmt.
> 
> I'm not entirely happy so I created the following alternative which
> is a bit larger and slower due to the pre-pass clearing the visited flag
> but is IMHO easier to follow.  I guess there's plenty of TLC opportunity
> here but then I also hope to retire the VN parts of DOM in favor
> of the non-iterating RPO-VN code...
> 
> So - I'd lean to this variant even though it has the extra loop over stmts,
> would you agree?
> 
> Bootstrap / regtest running on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 2019-03-06  Richard Biener  
> 
> PR tree-optimization/89595
> * tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Take
> stmt iterator as reference, take boolean output parameter to
> indicate whether the stmt was removed and thus the iterator
> already advanced.
> (dom_opt_dom_walker::before_dom_children): Re-iterate over
> stmts created by folding.
> 
> * gcc.dg/torture/pr89595.c: New testcase.
This one is easier to follow from a logic standpoint.  I don't think the
gimple_set_visited bits are going to be terribly expensive in general.

Is that flag in a known state for new statements?  I'm guessing it's
cleared by some structure-sized memset as we create the raw statement?

Might be worth clarifying that in the comments in gimple.h.

jeff

Re: [PATCH] Fix RTL loop-unroll ICE (PR rtl-optimization/89768)

[Jeff Law]

On 3/19/19 11:54 AM, Jakub Jelinek wrote:
> Hi!
> 
> On the testcase in the PR we ICE because we create non-canonical CONST_INTs
> (e.g. for HImode niter 0xfffe) and the compare_and_jump_seq code then ICEs
> on that.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> Testcase not included, because it creates 48 basic blocks and we handle
> that with O(n^2) complexity.
> 
> 2019-03-19  Jakub Jelinek  
> 
>   PR rtl-optimization/89768
>   * loop-unroll.c (unroll_loop_constant_iterations): Use gen_int_mode
>   instead of GEN_INT.
>   (unroll_loop_runtime_iterations): Likewise.
OK
jeff

Re: [PATCH] improve constant string length folding (PR 89688)

[Jeff Law]

On 3/13/19 8:17 PM, Martin Sebor wrote:
> PR 89688 points out a bogus warning about an unterminated
> character array argument to strlen.  The root cause is
> an oversight in the transformation of braced initializer lists
> to STRING_CSTs where the solution implemented last summer only
> considers one-dimensional arrays and skips more complex aggregates
> such as multi-dimensional arrays or structs.
> 
> The folder (string_constant), on the other hand, assumes that
> every a constant character array with an initializer is either
> a properly nul-terminated string (i.e., STRING_CST), or
> an unterminated array or a single character.  If the latter
> then unless the character value is zero it indicates to its
> caller that the constant is not a string.  As a result, we
> end up with a warning.
> 
> To avoid the false positives the attached patch extends
> the solution to those other kinds of aggregates.
> 
> Martin
> 
> gcc-89688.diff
> 
> PR tree-optimization/89688 - -Wstringop-overflow confused by const 2D array 
> of char
> 
> gcc/c/ChangeLog:
> 
>   PR tree-optimization/89688
>   * c-decl.c (finish_decl): Call braced_lists_to_string for more
>   kinds of initializers.
> 
> gcc/c-family/ChangeLog:
> 
>   PR tree-optimization/89688
>   * c-common.c (braced_list_to_string): Make static.
>   (braced_lists_to_strings): Define new function.
>   * c-common.h (braced_list_to_string): Remove.
>   (braced_lists_to_strings): Declare.
> 
> gcc/cp/ChangeLog:
> 
>   PR tree-optimization/89688
>   * typeck2.c (store_init_value): Call braced_lists_to_string for more
>   kinds of initializers.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/89688
>   * gcc.dg/strlenopt-61.c: New test.
>   * g++.dg/warn/Wstringop-overflow-2.C: New test.
OK.
jeff
> 

Re: [PATCH] Fix LRA ICE with BLKmode (PR target/89752)

[Vladimir Makarov]

On 3/19/19 2:00 PM, Jakub Jelinek wrote:

Hi!

As mentioned in the PR, BLKmode operands (such as in the testcase C++
variables with TREE_ADDRESSABLE types) may only live in a MEM, trying to
reload those in a register will always ICE.  process_alt_operands already
has if (mode == BLKmode) break; in it, so that it never claims a BLKmode
wins when seen a register constraint, but it still updated this_alternative
and this_alternative_set, so if a register constraint (or more of them)
is accompanied also by memory constraint that matches, we still let the
caller believe it could try to reload in registers.  It can't.
This patch ensures that we ignore those register constraints completely.

Bootstrapped/regtested on x86_64-linux and i686-linux, further bootstraps
pending on {powerpc64le,aarch64,s390x,armv7hl}-linux, ok for trunk if those
pass?


Yes, sure. Thanks for working on the PR, Jakub.

Go patch committed: Pass slice by elements to growslice

[Ian Lance Taylor]

This patch by Cherry Zhang changes the Go frontend and libgo to pass
the old slice's ptr/len/cap by value to growslice.  In the C calling
convention, on AMD64, and probably a number of other architectures, a
3-word struct argument is passed on stack.  This is less efficient
than passing in three registers.  Further, this may affect the code
generation in other part of the program, even if the function is not
actually called.

Slices are common in Go and append is a common slice operation, which
calls growslice in the growing path.  To improve the codegeneration,
pass the slice header's three fields as separate values, instead of a
struct, to growslice.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 269805)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-069afe85f38c099660c5d81950d65248ed4fc516
+6e5ff227d4e77d340e86bd2c5e045d5532c2d7d7
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 269805)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -7986,23 +7986,33 @@ Builtin_call_expression::flatten_append(
   // Using uint here means that if the computation of ntmp overflowed,
   // we will call growslice which will panic.
 
-  Expression* left = Expression::make_temporary_reference(ntmp, loc);
-  left = Expression::make_cast(uint_type, left, loc);
-
   Named_object* capfn = gogo->lookup_global("cap");
   Expression* capref = Expression::make_func_reference(capfn, NULL, loc);
   call_args = new Expression_list();
   call_args->push_back(Expression::make_temporary_reference(s1tmp, loc));
-  Expression* right = Expression::make_call(capref, call_args, false, loc);
+  Expression* cap = Expression::make_call(capref, call_args, false, loc);
+  gogo->lower_expression(function, inserter, );
+  gogo->flatten_expression(function, inserter, );
+  Temporary_statement* c1tmp = Statement::make_temporary(int_type, cap, loc);
+  inserter->insert(c1tmp);
+
+  Expression* left = Expression::make_temporary_reference(ntmp, loc);
+  left = Expression::make_cast(uint_type, left, loc);
+  Expression* right = Expression::make_temporary_reference(c1tmp, loc);
   right = Expression::make_cast(uint_type, right, loc);
 
   Expression* cond = Expression::make_binary(OPERATOR_GT, left, right, loc);
 
+  Type* unsafe_ptr_type = Type::make_pointer_type(Type::make_void_type());
   Expression* a1 = Expression::make_type_descriptor(element_type, loc);
   Expression* a2 = Expression::make_temporary_reference(s1tmp, loc);
-  Expression* a3 = Expression::make_temporary_reference(ntmp, loc);
-  Expression* call = Runtime::make_call(Runtime::GROWSLICE, loc, 3,
-   a1, a2, a3);
+  a2 = slice_type->array_type()->get_value_pointer(gogo, a2, false);
+  a2 = Expression::make_cast(unsafe_ptr_type, a2, loc);
+  Expression* a3 = Expression::make_temporary_reference(l1tmp, loc);
+  Expression* a4 = Expression::make_temporary_reference(c1tmp, loc);
+  Expression* a5 = Expression::make_temporary_reference(ntmp, loc);
+  Expression* call = Runtime::make_call(Runtime::GROWSLICE, loc, 5,
+   a1, a2, a3, a4, a5);
   call = Expression::make_unsafe_cast(slice_type, call, loc);
 
   ref = Expression::make_temporary_reference(s1tmp, loc);
Index: gcc/go/gofrontend/runtime.def
===
--- gcc/go/gofrontend/runtime.def   (revision 269805)
+++ gcc/go/gofrontend/runtime.def   (working copy)
@@ -202,7 +202,8 @@ DEF_GO_RUNTIME(TYPEDSLICECOPY, "runtime.
 
 
 // Grow a slice for append.
-DEF_GO_RUNTIME(GROWSLICE, "runtime.growslice", P3(TYPE, SLICE, INT), R1(SLICE))
+DEF_GO_RUNTIME(GROWSLICE, "runtime.growslice",
+   P5(TYPE, POINTER, INT, INT, INT), R1(SLICE))
 
 
 // Register roots (global variables) for the garbage collector.
Index: libgo/go/runtime/slice.go
===
--- libgo/go/runtime/slice.go   (revision 269805)
+++ libgo/go/runtime/slice.go   (working copy)
@@ -77,31 +77,31 @@ func makeslice64(et *_type, len64, cap64
 // and it returns a new slice with at least that capacity, with the old data
 // copied into it.
 // The new slice's length is set to the requested capacity.
-func growslice(et *_type, old slice, cap int) slice {
+func growslice(et *_type, oldarray unsafe.Pointer, oldlen, oldcap, cap int) 
slice {
if raceenabled {
callerpc := getcallerpc()
-   racereadrangepc(old.array, uintptr(old.len*int(et.size)), 
callerpc, funcPC(growslice))
+   racereadrangepc(oldarray, 

Re: [PATCH] fix ice in attribute copy (PR 89685)

[Jeff Law]

On 3/14/19 7:47 PM, Martin Sebor wrote:
> To copy type attributes from struct A, the copy attribute (new
> in GCC 9) accepts a pointer argument such as (struct A*)0, but
> it isn't prepared for anything much more complicated than that.
> So for example when it's passed something like (struct A*)(0, 1)
> as the test case in PR 89685 does (a P1 regression), it fails
> with an ICE.
> 
> The attached patch makes this handling more robust by letting
> it accept all forms of type and member references.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-89685.diff
> 
> PR c/89685 - ICE on attribute copy with a compound expression
> 
> gcc/c-family/ChangeLog:
> 
>   PR c/89685
>   * c-attribs.c (handle_copy_attribute): Handle references and
>   non-constant expressions.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/89685
>   * gcc.dg/attr-copy-8.c: New test.
>   * g++.dg/ext/attr-copy-2.C: New test.I think this is in the same state 
> as the __builtin_has_attribute bits --
you're trying to support attributes on expressions.  We should reject
those as syntax errors right now.

jeff
> 

Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Jeff Law]

On 3/19/19 12:37 PM, Martin Sebor wrote:
> On 3/19/19 12:01 PM, Martin Sebor wrote:
>> On 3/19/19 8:33 AM, Jeff Law wrote:
>>> On 3/11/19 8:27 PM, Martin Sebor wrote:
 The -Wstringop-truncation handling for strncpy/stpncpy neglects
 to consider that character arrays tracked by the strlen pass
 are not necessarily nul-terminated.  It unconditionally adds
 one when computing the size of each sequence to account for
 the nul.  This leads to false positive warnings when checking
 the validity of indices/pointers computed by the built-ins.

 The attached patch corrects this by adding one for the nul
 only when the character array is known to be nul-terminated.

 Since GCC 7 does not issue the warning this is a 8/9 regression
 that I would like to fix in both releases.  Is the patch okay
 for trunk/gcc-8-branch?

 Tested on x86_64-linux.

 Martin

 gcc-89644.diff

 PR tree-optimization/89644 - False-positive -Warray-bounds
 diagnostic on strncpy

 gcc/ChangeLog:

 PR tree-optimization/89644
 * tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
 arrays in determining sequence sizes in strncpy and stpncpy.

 gcc/testsuite/ChangeLog:

 PR tree-optimization/89644
 * gcc.dg/Wstringop-truncation-8.c: New test.
>>> OK for the trunk as well as the affected release branches.
>>
>> I just noticed some some -Wrestrict test failures that I had missed
>> in the test report.  The patch wasn't quite right.  Sorry about that.
>> I'll fix it shortly.
> 
> The variable to use to check for nul-termination is full_string_p
> rather than endptr.  I've fixed that in 269809.
ACK.  I'll restart those targets once 269809 lands in the git mirror.

jeff

Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Jeff Law]

On 3/19/19 12:01 PM, Martin Sebor wrote:
> On 3/19/19 8:33 AM, Jeff Law wrote:
>> On 3/11/19 8:27 PM, Martin Sebor wrote:
>>> The -Wstringop-truncation handling for strncpy/stpncpy neglects
>>> to consider that character arrays tracked by the strlen pass
>>> are not necessarily nul-terminated.  It unconditionally adds
>>> one when computing the size of each sequence to account for
>>> the nul.  This leads to false positive warnings when checking
>>> the validity of indices/pointers computed by the built-ins.
>>>
>>> The attached patch corrects this by adding one for the nul
>>> only when the character array is known to be nul-terminated.
>>>
>>> Since GCC 7 does not issue the warning this is a 8/9 regression
>>> that I would like to fix in both releases.  Is the patch okay
>>> for trunk/gcc-8-branch?
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-89644.diff
>>>
>>> PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic
>>> on strncpy
>>>
>>> gcc/ChangeLog:
>>>
>>> PR tree-optimization/89644
>>> * tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
>>> arrays in determining sequence sizes in strncpy and stpncpy.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR tree-optimization/89644
>>> * gcc.dg/Wstringop-truncation-8.c: New test.
>> OK for the trunk as well as the affected release branches.
> 
> I just noticed some some -Wrestrict test failures that I had missed
> in the test report.  The patch wasn't quite right.  Sorry about that.
> I'll fix it shortly.
Something like this perhaps?

> # Comparing 4 common sum files
> ## /bin/sh gcc/contrib/compare_tests  /tmp/gxx-sum1.6751 /tmp/gxx-sum2.6751
> Tests that now fail, but worked before (15 tests):
> 
> c-c++-common/Wrestrict.c  -Wc++-compat  (test for excess errors)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 811)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 812)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 813)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 819)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 820)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 827)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 828)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 867)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 875)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 876)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 884)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 885)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 893)
> c-c++-common/Wrestrict.c  -Wc++-compat  strncpy (test for warnings, line 894)

My testers just spit this out on a couple of targets in the last half
hour or so.

jeff

Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Martin Sebor]

On 3/19/19 12:01 PM, Martin Sebor wrote:

On 3/19/19 8:33 AM, Jeff Law wrote:

On 3/11/19 8:27 PM, Martin Sebor wrote:

The -Wstringop-truncation handling for strncpy/stpncpy neglects
to consider that character arrays tracked by the strlen pass
are not necessarily nul-terminated.  It unconditionally adds
one when computing the size of each sequence to account for
the nul.  This leads to false positive warnings when checking
the validity of indices/pointers computed by the built-ins.

The attached patch corrects this by adding one for the nul
only when the character array is known to be nul-terminated.

Since GCC 7 does not issue the warning this is a 8/9 regression
that I would like to fix in both releases.  Is the patch okay
for trunk/gcc-8-branch?

Tested on x86_64-linux.

Martin

gcc-89644.diff

PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic 
on strncpy


gcc/ChangeLog:

PR tree-optimization/89644
* tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
arrays in determining sequence sizes in strncpy and stpncpy.

gcc/testsuite/ChangeLog:

PR tree-optimization/89644
* gcc.dg/Wstringop-truncation-8.c: New test.

OK for the trunk as well as the affected release branches.


I just noticed some some -Wrestrict test failures that I had missed
in the test report.  The patch wasn't quite right.  Sorry about that.
I'll fix it shortly.


The variable to use to check for nul-termination is full_string_p
rather than endptr.  I've fixed that in 269809.

Martin

Re: PING [PATCH] fix ICE in __builtin_has_attribute (PR 88383 and 89288)

[Jeff Law]

On 2/22/19 9:42 AM, Marek Polacek wrote:
> On Thu, Feb 21, 2019 at 03:39:21PM -0700, Martin Sebor wrote:
>> On 2/21/19 1:27 PM, Jeff Law wrote:
>>> On 2/21/19 1:12 PM, Martin Sebor wrote:
 On 2/21/19 12:08 PM, Jeff Law wrote:
> On 2/18/19 7:53 PM, Martin Sebor wrote:
>> Please let me know what it will take to get the fix for these two
>> issues approved.  I've answered the questions so I don't know what
>> else I'm expected to do here.
>>
>>     https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00793.html
> I think there is still a fundamental disagreement about whether or not
> this should be handling expressions.  Without an agreement on that it's
> hard to see how this could go forward.

 I think it's wrong to hold up a fix for an ICE because you don't
 like the current design.  The built-in successfully handles common
 expressions (see c-c++-common/builtin-has-attribute-5.c).  It just
 fails for some of the less common ones.  Not fixing those will only
 penalize users who run into the ICE, and cast a bad light on
 the quality of the release.   Any design questions should be
 settled separately of these ICEs (should have been when
 the feature was being reviewed).

 That said, I have explained the rationale for the current design.
 Neither you nor Jakub has explained what you find wrong with it or
 why either of your alternatives is preferable.  You said it should
 be an error to apply the built-in to expressions (why?).  Jakub
 thought there perhaps should be two built-ins, one for expressions
 and another for decls.  His rationale?  The current design is not
 good.  Clearly,  the two of you don't agree on what you'd like to
 see; the only thing you agree on is that you don't like what's
 there now.  What do you expect me to do with that, now at the end
 of stage 4?
>>> Fix the ice in another way.  Handling expressions here seems
>>> fundamentally wrong.  ISTM that making this query on an expression
>>> should result in an immediate error.
>>
>> Sorry but "it seems wrong" is not a compelling argument.  You need
>> to explain what you think is wrong with it.
> 
> "Attributes apply to decls and types, not expressions" seems like an
> argument strong enough.  (There are also special attributes for enums
>  and ';' and labels.)
So I think Martin's argument is that for an expression we can look at
the type of the expression and see if the attribute is on that type.

That leads to some inconsistencies (that Jakub has pointed out) where an
unfolded expression could produce a different result than it's folded
result if the result was a constant.

AFAICT the goal (outside handling just types) here is to be able to ask
things like is a particular field within a structure packed, aliasing
properties of objects, etc.

My next thought was to use typeof to extract the type of the expression
and pass that through (independent of Marek suggesting the same).
Martin points out in a subsequent email that won't work because p->a in
the supplied example returns an int rather than a packed int.   One
could argue that's the wrong return value for typeof, but I don't think
it's that clear cut.  In this specific instance I'd claim it's wrong,
but I'd hazard a guess we don't have clear semantics in this space.

I'll note that our documentation clearly states that attributes can be
applied to functions, variables, labels, enums, statements and types.
No provision AFAICT is made for expressions.  Users have never had the
ability or assumption that they can ask for attributes on an expression.

Martin's patch essentially allows for asking indirectly -- but I suspect
it's going to be inconsistent in more ways than Jakub pointed out as the
type system in gimple is "loose" in certain ways and the result could
well change as we go through the optimizer pipeline (ie, this is bigger
than just constant folding causing inconsistencies).

Given that we're talking about a new feature with no existing end user
expectations and the fact that attributes are documented as applying
only to things which are decls or types, I'm more inclined to stay
conservative at this point and reject expressions as syntax errors.

We can revisit in gcc-10 and see if there's a reasonable way to tighten
semantics around typeof and/or loosen the requirement that we can only
ask about attributes of DECL/TYPE nodes and define good semantics around
that if we try.  I'm also willing to revisit if other reviewers chime in
with other opinions -- this kind of stuff isn't my strongest area.


Jeff

Re: [PATCH] Fix RTL loop-unroll ICE (PR rtl-optimization/89768)

[Richard Biener]

On March 19, 2019 6:54:33 PM GMT+01:00, Jakub Jelinek  wrote:
>Hi!
>
>On the testcase in the PR we ICE because we create non-canonical
>CONST_INTs
>(e.g. for HImode niter 0xfffe) and the compare_and_jump_seq code then
>ICEs
>on that.
>
>Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
>for
>trunk?

OK. 

Richard. 

>Testcase not included, because it creates 48 basic blocks and we
>handle
>that with O(n^2) complexity.
>
>2019-03-19  Jakub Jelinek  
>
>   PR rtl-optimization/89768
>   * loop-unroll.c (unroll_loop_constant_iterations): Use gen_int_mode
>   instead of GEN_INT.
>   (unroll_loop_runtime_iterations): Likewise.
>
>--- gcc/loop-unroll.c.jj   2019-03-19 09:09:32.686006683 +0100
>+++ gcc/loop-unroll.c  2019-03-19 10:15:50.319343904 +0100
>@@ -652,7 +652,7 @@ unroll_loop_constant_iterations (struct
>   if (loop->any_likely_upper_bound)
> loop->nb_iterations_likely_upper_bound
>= wi::udiv_trunc (loop->nb_iterations_likely_upper_bound, max_unroll +
>1);
>-  desc->niter_expr = GEN_INT (desc->niter);
>+  desc->niter_expr = gen_int_mode (desc->niter, desc->mode);
> 
>   /* Remove the edges.  */
>   FOR_EACH_VEC_ELT (remove_edges, i, e)
>@@ -1020,9 +1020,9 @@ unroll_loop_runtime_iterations (struct l
>   preheader = split_edge (loop_preheader_edge (loop));
>   /* Add in count of edge from switch block.  */
>   preheader->count += iter_count;
>-  branch_code = compare_and_jump_seq (copy_rtx (niter), GEN_INT
>(j), EQ,
>-block_label (preheader), p,
>-NULL);
>+  branch_code = compare_and_jump_seq (copy_rtx (niter),
>+gen_int_mode (j, desc->mode), EQ,
>+block_label (preheader), p, NULL);
> 
>/* We rely on the fact that the compare and jump cannot be optimized
>out,
>and hence the cfg we create is correct.  */
>
>   Jakub

[C++ PATCH] Fix lambda capture duplicate handling (PR c++/89767)

[Jakub Jelinek]

Hi!

add_capture when parsing a lambda introducer uses the IDENTIFIER_MARKED
bit to detect duplicate captures.
I guess in strict C++11 that could have worked, if the introducer could
contain just identifiers, but in C++14 it has 2 problems:

1) lambda initializers can contain arbitrary expressions and
IDENTIFIER_MARKED is used during the name lookup, if we set it, then
anything we do while parsing something in the initializer will affect
name lookup of __X identifiers nested inside if there is X capture
(see the last testcase, where there is t capture and __t argument
of a constexpr template function called in the lambda initializer
and another __t argument in another constexpr template function that calls)

2) if the lambda initializer contains another lambda, the IDENTIFIER_MARKED
bit will affect also this nested lambda while it shouldn't (see
lambda-init18.C testcase)

The following patch stops using IDENTIFIER_MARKED for this and uses a
hash_set instead.  But, as I believe lambdas with 0 (or very few) explicit
captures are extremely common, I've tried not to slow down those with
allocation of a hash_set and deallocating it again, so it uses
a linear search if there are up to 8 captures and starts using a hash_set
only when getting above that count.

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

Or do you think allocation of an hash_set and deallocation would be in a
noise?

2019-03-19  Jakub Jelinek  

PR c++/89767
* cp-tree.h (add_capture): Add ids argument.
* parser.c (cp_parser_lambda_introducer): Add ids variable and pass
its address to add_capture calls, delete it at the end.
* lambda.c (add_capture): Add ids argument, don't use
IDENTIFIER_MARKED, instead walk capture list linearly to find
duplicates if short, or use hash_set otherwise.
(register_capture_members): Don't clear IDENTIFIER_MARKED here.
(add_default_capture): Adjust add_capture caller.

* g++.dg/cpp1y/lambda-init18.C: New test.
* g++.dg/cpp1y/lambda-init19.C: New test.
* g++.dg/cpp1y/pr89767.C: New test.

--- gcc/cp/cp-tree.h.jj 2019-03-06 19:45:40.370751592 +0100
+++ gcc/cp/cp-tree.h2019-03-19 16:06:13.989660106 +0100
@@ -7150,7 +7150,8 @@ extern tree lambda_return_type(tree);
 extern tree lambda_proxy_type  (tree);
 extern tree lambda_function(tree);
 extern void apply_deduced_return_type   (tree, tree);
-extern tree add_capture (tree, tree, tree, bool, bool);
+extern tree add_capture (tree, tree, tree, bool, bool,
+hash_set **);
 extern tree add_default_capture (tree, tree, tree);
 extern void insert_capture_proxy   (tree);
 extern void insert_pending_capture_proxies (void);
--- gcc/cp/parser.c.jj  2019-03-19 07:46:44.293812030 +0100
+++ gcc/cp/parser.c 2019-03-19 16:53:00.884434656 +0100
@@ -10547,6 +10547,7 @@ cp_parser_lambda_introducer (cp_parser*
error ("non-local lambda expression cannot have a capture-default");
 }
 
+  hash_set *ids = NULL;
   while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_SQUARE))
 {
   cp_token* capture_token;
@@ -10586,7 +10587,7 @@ cp_parser_lambda_introducer (cp_parser*
   /*id=*/this_identifier,
   /*initializer=*/finish_this_expr (),
   /*by_reference_p=*/true,
-  explicit_init_p);
+  explicit_init_p, );
  continue;
}
 
@@ -10604,7 +10605,7 @@ cp_parser_lambda_introducer (cp_parser*
   /*id=*/this_identifier,
   /*initializer=*/finish_this_expr (),
   /*by_reference_p=*/false,
-  explicit_init_p);
+  explicit_init_p, );
  continue;
}
 
@@ -10757,7 +10758,7 @@ cp_parser_lambda_introducer (cp_parser*
   capture_id,
   capture_init_expr,
   /*by_reference_p=*/capture_kind == BY_REFERENCE,
-  explicit_init_p);
+  explicit_init_p, );
 
   /* If there is any qualification still in effect, clear it
 now; we will be starting fresh with the next capture.  */
@@ -10767,6 +10768,9 @@ cp_parser_lambda_introducer (cp_parser*
 }
 
   cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
+
+  if (ids)
+delete ids;
 }
 
 /* Parse the (optional) middle of a lambda expression.
--- gcc/cp/lambda.c.jj  2019-03-01 09:05:59.692052025 +0100
+++ gcc/cp/lambda.c 2019-03-19 16:32:44.534844789 +0100
@@ -500,11 +500,12 @@ vla_capture_type (tree array_type)
 /* From an ID and INITIALIZER, create a capture (by reference if
BY_REFERENCE_P is true), add it to the capture-list for LAMBDA,
and return it.  If ID is `this', BY_REFERENCE_P 

Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Martin Sebor]

On 3/19/19 8:33 AM, Jeff Law wrote:

On 3/11/19 8:27 PM, Martin Sebor wrote:

The -Wstringop-truncation handling for strncpy/stpncpy neglects
to consider that character arrays tracked by the strlen pass
are not necessarily nul-terminated.  It unconditionally adds
one when computing the size of each sequence to account for
the nul.  This leads to false positive warnings when checking
the validity of indices/pointers computed by the built-ins.

The attached patch corrects this by adding one for the nul
only when the character array is known to be nul-terminated.

Since GCC 7 does not issue the warning this is a 8/9 regression
that I would like to fix in both releases.  Is the patch okay
for trunk/gcc-8-branch?

Tested on x86_64-linux.

Martin

gcc-89644.diff

PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic on strncpy

gcc/ChangeLog:

PR tree-optimization/89644
* tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
arrays in determining sequence sizes in strncpy and stpncpy.

gcc/testsuite/ChangeLog:

PR tree-optimization/89644
* gcc.dg/Wstringop-truncation-8.c: New test.

OK for the trunk as well as the affected release branches.


I just noticed some some -Wrestrict test failures that I had missed
in the test report.  The patch wasn't quite right.  Sorry about that.
I'll fix it shortly.

Martin

[PATCH] Fix LRA ICE with BLKmode (PR target/89752)

[Jakub Jelinek]

Hi!

As mentioned in the PR, BLKmode operands (such as in the testcase C++
variables with TREE_ADDRESSABLE types) may only live in a MEM, trying to
reload those in a register will always ICE.  process_alt_operands already
has if (mode == BLKmode) break; in it, so that it never claims a BLKmode
wins when seen a register constraint, but it still updated this_alternative
and this_alternative_set, so if a register constraint (or more of them)
is accompanied also by memory constraint that matches, we still let the
caller believe it could try to reload in registers.  It can't.
This patch ensures that we ignore those register constraints completely.

Bootstrapped/regtested on x86_64-linux and i686-linux, further bootstraps
pending on {powerpc64le,aarch64,s390x,armv7hl}-linux, ok for trunk if those
pass?

2019-03-19  Jakub Jelinek  

PR target/89752
* lra-constraints.c (process_alt_operands) : For BLKmode, don't
update this_alternative nor this_alternative_set.

* g++.target/aarch64/aarch64.exp: New file.
* g++.target/aarch64/pr89752.C: New test.

--- gcc/lra-constraints.c.jj2019-03-16 22:17:21.060937047 +0100
+++ gcc/lra-constraints.c   2019-03-19 11:49:11.982058568 +0100
@@ -2350,6 +2350,8 @@ process_alt_operands (int only_alternati
  break;
 
reg:
+ if (mode == BLKmode)
+   break;
  this_alternative = reg_class_subunion[this_alternative][cl];
  IOR_HARD_REG_SET (this_alternative_set,
reg_class_contents[cl]);
@@ -2360,8 +2362,6 @@ process_alt_operands (int only_alternati
  IOR_HARD_REG_SET (this_costly_alternative_set,
reg_class_contents[cl]);
}
- if (mode == BLKmode)
-   break;
  winreg = true;
  if (REG_P (op))
{
--- gcc/testsuite/g++.target/aarch64/aarch64.exp.jj 2019-03-19 
11:55:06.539427568 +0100
+++ gcc/testsuite/g++.target/aarch64/aarch64.exp2019-03-19 
11:57:08.302493754 +0100
@@ -0,0 +1,44 @@
+#  Specific regression driver for AArch64.
+#  Copyright (C) 2009-2019 Free Software Foundation, Inc.
+#
+#  This file is part of GCC.
+#
+#  GCC is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 3, or (at your option)
+#  any later version.
+#
+#  GCC is distributed in the hope that it will be useful, but
+#  WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#  General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with GCC; see the file COPYING3.  If not see
+#  .  */
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't an AArch64 target.
+if {![istarget aarch64*-*-*] } then {
+  return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+global DEFAULT_CXXFLAGS
+if ![info exists DEFAULT_CXXFLAGS] then {
+set DEFAULT_CXXFLAGS " -pedantic-errors"
+}
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] \
+"" $DEFAULT_CXXFLAGS
+
+# All done.
+dg-finish
+
--- gcc/testsuite/g++.target/aarch64/pr89752.C.jj   2019-03-19 
12:00:24.890371590 +0100
+++ gcc/testsuite/g++.target/aarch64/pr89752.C  2019-03-19 11:54:23.162116476 
+0100
@@ -0,0 +1,11 @@
+// PR target/89752
+// { dg-do compile }
+
+struct A { A (); ~A (); short c; };
+
+void
+foo ()
+{
+  A a0, a1;
+  __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));   // { 
dg-error "inconsistent operand constraints in an 'asm'" }
+}

Jakub

[PATCH] Fix RTL loop-unroll ICE (PR rtl-optimization/89768)

[Jakub Jelinek]

Hi!

On the testcase in the PR we ICE because we create non-canonical CONST_INTs
(e.g. for HImode niter 0xfffe) and the compare_and_jump_seq code then ICEs
on that.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

Testcase not included, because it creates 48 basic blocks and we handle
that with O(n^2) complexity.

2019-03-19  Jakub Jelinek  

PR rtl-optimization/89768
* loop-unroll.c (unroll_loop_constant_iterations): Use gen_int_mode
instead of GEN_INT.
(unroll_loop_runtime_iterations): Likewise.

--- gcc/loop-unroll.c.jj2019-03-19 09:09:32.686006683 +0100
+++ gcc/loop-unroll.c   2019-03-19 10:15:50.319343904 +0100
@@ -652,7 +652,7 @@ unroll_loop_constant_iterations (struct
   if (loop->any_likely_upper_bound)
 loop->nb_iterations_likely_upper_bound
   = wi::udiv_trunc (loop->nb_iterations_likely_upper_bound, max_unroll + 
1);
-  desc->niter_expr = GEN_INT (desc->niter);
+  desc->niter_expr = gen_int_mode (desc->niter, desc->mode);
 
   /* Remove the edges.  */
   FOR_EACH_VEC_ELT (remove_edges, i, e)
@@ -1020,9 +1020,9 @@ unroll_loop_runtime_iterations (struct l
   preheader = split_edge (loop_preheader_edge (loop));
   /* Add in count of edge from switch block.  */
   preheader->count += iter_count;
-  branch_code = compare_and_jump_seq (copy_rtx (niter), GEN_INT (j), EQ,
- block_label (preheader), p,
- NULL);
+  branch_code = compare_and_jump_seq (copy_rtx (niter),
+ gen_int_mode (j, desc->mode), EQ,
+ block_label (preheader), p, NULL);
 
   /* We rely on the fact that the compare and jump cannot be optimized out,
 and hence the cfg we create is correct.  */

Jakub

Re: [patch] Fix wrong code for boolean negation in condition at -O

[Jeff Law]

On 2/25/19 2:07 AM, Eric Botcazou wrote:
> Hi,
> 
> this is a regression present on the mainline and 8 branch, introduced by the 
> new code in edge_info::derive_equivalences dealing with BIT_AND_EXPR for SSA 
> names with boolean range:
> 
> /* If either operand has a boolean range, then we
>know its value must be one, otherwise we just know it
>is nonzero.  The former is clearly useful, I haven't
>seen cases where the latter is helpful yet.  */
> if (TREE_CODE (rhs1) == SSA_NAME)
>   {
> if (ssa_name_has_boolean_range (rhs1))
>   {
> value = build_one_cst (TREE_TYPE (rhs1));
> derive_equivalences (rhs1, value, recursion_limit - 1);
>   }
>   }
> if (TREE_CODE (rhs2) == SSA_NAME)
>   {
> if (ssa_name_has_boolean_range (rhs2))
>   {
> value = build_one_cst (TREE_TYPE (rhs2));
> derive_equivalences (rhs2, value, recursion_limit - 1);
>   }
>   }
> 
> and visible on the attached Ada testcase at -O1 or above.
> 
> The sequence of events is as follows: for boolean types with precision > 1 
> (the normal boolean types in Ada), the gimplifier turns a TRUTH_NOT_EXPR into 
> a BIT_XOR_EXPR with 1 in order to preserve the 0-or-1-value invariant:
> 
>   /* The parsers are careful to generate TRUTH_NOT_EXPR
>  only with operands that are always zero or one.
>  We do not fold here but handle the only interesting case
>  manually, as fold may re-introduce the TRUTH_NOT_EXPR.  */
>   *expr_p = gimple_boolify (*expr_p);
>   if (TYPE_PRECISION (TREE_TYPE (*expr_p)) == 1)
> *expr_p = build1_loc (input_location, BIT_NOT_EXPR,
>   TREE_TYPE (*expr_p),
>   TREE_OPERAND (*expr_p, 0));
>   else
> *expr_p = build2_loc (input_location, BIT_XOR_EXPR,
>   TREE_TYPE (*expr_p),
>   TREE_OPERAND (*expr_p, 0),
>   build_int_cst (TREE_TYPE (*expr_p), 1));
> 
> Now this TRUTH_NOT_EXPR is part of a conjunction which has been turned into a 
> BIT_AND_EXPR by the folder, so this gives BIT_AND_EXPR >.
> 
> After some optimization passes, the second operand of the BIT_AND_EXPR is 
> also 
> folded into 1 and, consequently, the following match.pd pattern kicks in:
> 
> /* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y.  */
> (for opo (bit_and bit_xor)
>  opi (bit_xor bit_and)
>  (simplify
>   (opo:c (opi:c @0 @1) @1) 
>   (bit_and (bit_not @0) @1)))
> 
> and yields BIT_AND_EXPR .  This is still correct, in the 
> sense that the 0-or-1-value invariant is preserved.
> 
> Then the new code in edge_info::derive_equivalences above deduces from this 
> that the BIT_NOT_EXPR has value 1 on one of the edges.  But the same function 
> also handles the BIT_NOT_EXPR itself and further deduces that its operand has 
> value ~1 or 254 (the precision of boolean types is 8) on this edge, which 
> breaks the 0-or-1-value invariant and leads to wrong code downstream.
> 
> Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for 
> boolean types, I think that the same special treatment must be added for 
> boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value invariant.
> 
> Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch?
> 
> 
> 2019-02-25  Eric Botcazou  
> 
>   * tree-ssa-dom.c (edge_info::derive_equivalences) : Fix
>   and move around comment.
>   : Likewise.
>   : Add specific handling for boolean types.
> 
> 
> 2019-02-25  Eric Botcazou  
> 
>   * gnat.dg/opt77.adb: New test.
>   * gnat.dg/opt77_pkg.ad[sb]: Likewise.
Umm, did you look at ssa_name_has_boolean_range?  Isn't the problem that
Ada's BOOLEAN_TYPE has a wider range than [0..1] and thus this is the
wrong bit of code:

  /* Boolean types always have a range [0..1].  */
  if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
return true;

IIRC there are other places that have similar logic and rely on
ssa_name_has_boolean_range to filter out anything undesirable.

jeff

Re: [PATCH 1/2] PR c/65403 - Ignore -Wno-error=

[Martin Sebor]

On 3/18/19 8:46 PM, Alex Henrie wrote:

From: Manuel López-Ibáñez 

* opts.c: Ignore -Wno-error= except if there are
other diagnostics.
---
  gcc/opts-common.c |  2 ++
  gcc/opts-global.c | 10 +++---
  gcc/opts.c|  5 -
  gcc/opts.h|  2 ++
  4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index edbb3ac9b6d..36e06e2372a 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3.  If not see
  #include "diagnostic.h"
  #include "spellcheck.h"
  
+vec ignored_wnoerror_options;

+
  static void prune_options (struct cl_decoded_option **, unsigned int *);
  
  /* An option that is undocumented, that takes a joined argument, and

diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index a5e9ef0237a..0b2550a4855 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -132,12 +132,16 @@ print_ignored_options (void)
  {
while (!ignored_options.is_empty ())
  {
-  const char *opt;
-
-  opt = ignored_options.pop ();
+  const char * opt = ignored_options.pop ();
warning_at (UNKNOWN_LOCATION, 0,
  "unrecognized command line option %qs", opt);
  }
+  while (!ignored_wnoerror_options.is_empty ())
+{
+  const char * opt = ignored_wnoerror_options.pop ();
+  warning_at (UNKNOWN_LOCATION, 0,
+ "-Wno-error=%s: no option -W%s", opt, opt);


Please remember to quote the command line options in the message
(same as in the error below):


+}
  }
  
  /* Handle an unknown option DECODED, returning true if an error should

diff --git a/gcc/opts.c b/gcc/opts.c
index 3161e0b6753..4fa79306e30 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3079,7 +3079,10 @@ enable_warning_as_error (const char *arg, int value, 
unsigned int lang_mask,
strcpy (new_option + 1, arg);
option_index = find_opt (new_option, lang_mask);
if (option_index == OPT_SPECIAL_unknown)
-error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+if (value)
+  error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);

 ^^

Like that.

Thanks
Martin



+else
+  ignored_wnoerror_options.safe_push (arg);
else if (!(cl_options[option_index].flags & CL_WARNING))
  error_at (loc, "%<-Werror=%s%>: -%s is not an option that controls "
  "warnings", arg, new_option);
diff --git a/gcc/opts.h b/gcc/opts.h
index f14d9bcb896..b18504e0bc3 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -456,4 +456,6 @@ extern bool parse_and_check_align_values (const char *flag,
  bool report_error,
  location_t loc);
  
+extern vec ignored_wnoerror_options;

+
  #endif

Re: [PATCH] Fix set of even probabilities (PR middle-end/89737).

[Martin Liška]

On 3/19/19 3:52 PM, Jakub Jelinek wrote:
> On Tue, Mar 19, 2019 at 09:02:27AM +0100, Martin Liška wrote:
>> 2019-03-18  Martin Liska  
>>
>>  PR middle-end/89737
>>  * predict.c (combine_predictions_for_bb): Empty likely_edges and
>>  unlikely_edges if there's an edge that belongs to both these sets.
> 
> I admit I don't know this code at all, just wonder if if you find
> an intersection between likely_edges and unlikely_edges, couldn't you just
> remove the intersection from both, rather than all other edges too?

Having a contradict edge makes any assumption about the likeliness of the
others misleading. Plus it's very rare case this happens.

I'm going to install the patch.
Thanks,
Martin

> 
>   Jakub
> 

Re: [PATCH 0/2] Future warnings not treated as errors

[Martin Liška]

On 3/19/19 5:35 PM, Alex Henrie wrote:
> On Tue, Mar 19, 2019 at 3:19 AM Martin Liška  wrote:
>>
>> On 3/19/19 3:46 AM, Alex Henrie wrote:
>>> Hello,
>>>
>>> I have received permission to contribute to GCC, but I do not have
>>> commit access. Could one of you please merge these patches for me?
>>
>> Hi.
>>
>> I can do it for you. But if I see correctly, Joseph M. had some concerns
>> and he should probably make final patch approval.
> 
> If I understand correctly, Joseph wanted an error message to still be
> printed if other diagnostics are also printed, the same as how
> -Wno- works. The patches that I sent yesterday do
> exactly that.
> 
> -Alex
> 

Hi.

What's Joseph telling is that right now the patch can't be approved (and 
installed).
It's due to fact that we are close to the release in stage4:
https://www.gnu.org/software/gcc/develop.html#stage4

So that a proper review will happen in couple of weeks.

Thanks for understanding,
Martin

Re: [PATCH] Fix -Wenum-compare-switch warning in i386.c.

[Martin Liška]

On 3/19/19 3:37 PM, Jeff Law wrote:
> On 3/8/19 5:34 AM, Martin Liška wrote:
>> A small patch that deals with:
>> gcc/config/i386/i386.c:39427:11:Semantic Issue: comparison of two values 
>> with different enumeration types in switch statement ('enum 
>> built_in_function' and 'ix86_builtins'): -Wenum-compare-switch
>>
>> Is it fine to install it?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-03-08  Martin Liska  
>>
>>  * config/i386/i386.c (ix86_builtin_reciprocal): Cast
>>  DECL_FUNCTION_CODE into ix86_builtins enum before
>>  the switch statement.
>> ---
>>  gcc/config/i386/i386.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>>
> Seems trivial, but I'd still defer to gcc-10 unless there's a compelling
> need to address it right now.

I'm fine with application of the patch in next stage1.

Thanks,
Martin

> 
> jeff
> 

Re: [PATCH] rs6000: Unaligned stfiwx on older CPUs (PR89746)

[Segher Boessenkool]

On Tue, Mar 19, 2019 at 04:26:18PM +, Segher Boessenkool wrote:
> The "classic" PowerPCs (6xx/7xx) are not STRICT_ALIGNMENT, but their
> floating point units are.  This is not normally a problem, the ABIs
> make everything FP aligned.  The RTL patterns converting FP to integer
> however get a potentially unaligned destination, and we do not want to
> do an stfiwx on that on such older CPUs.
> 
> This fixes it.  It does not change anything for TARGET_MFCRF targets
> (POWER4 and later).  It also won't change anything for strict-alignment
> targets, or CPUs without hardware FP of course, or CPUs that do not
> implement stfiwx (older 4xx/5xx/8xx).
> 
> It does not change the correcponding fixuns* pattern, because that can
> not be enabled on any CPU that cannot handle unaligned FP well.
> 
> Tested on powerpc64-linux {-m32,-m64}, committing to trunk.  Will do
> backports later, too.
> 
> 
> Segher
> 
> 
> 2018-03-19  Segher Boessenkool  
> 
>   PR target/89746
>   * config/rs6000/rs6000.md (fix_truncsi2_stfiwx): If we have a
>   non-TARGET_MFCRF target, and the dest is memory but not 32-bit aligned,
>   go via a stack temporary.

The actual patch I committed is


Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 269801)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -5639,13 +5639,13 @@
 tmp = gen_reg_rtx (DImode);
 
   emit_insn (gen_fctiwz_ (tmp, src));
-  if (MEM_P (dest))
+  if (MEM_P (dest) && (TARGET_MFCRF || MEM_ALIGN (dest) >= 32))
 {
   dest = rs6000_force_indexed_or_indirect_mem (dest);
   emit_insn (gen_stfiwx (dest, tmp));
   DONE;
 }
-  else if (TARGET_POWERPC64 && (TARGET_MFPGPR || TARGET_DIRECT_MOVE))
+  else if (TARGET_POWERPC64 && (TARGET_MFPGPR || TARGET_DIRECT_MOVE) && !MEM_P 
(dest))
 {
   dest = gen_lowpart (DImode, dest);
   emit_move_insn (dest, tmp);

Re: [PATCH 0/2] Future warnings not treated as errors

[Alex Henrie]

On Tue, Mar 19, 2019 at 3:19 AM Martin Liška  wrote:
>
> On 3/19/19 3:46 AM, Alex Henrie wrote:
> > Hello,
> >
> > I have received permission to contribute to GCC, but I do not have
> > commit access. Could one of you please merge these patches for me?
>
> Hi.
>
> I can do it for you. But if I see correctly, Joseph M. had some concerns
> and he should probably make final patch approval.

If I understand correctly, Joseph wanted an error message to still be
printed if other diagnostics are also printed, the same as how
-Wno- works. The patches that I sent yesterday do
exactly that.

-Alex

[PATCH] rs6000: Unaligned stfiwx on older CPUs (PR89746)

[Segher Boessenkool]

The "classic" PowerPCs (6xx/7xx) are not STRICT_ALIGNMENT, but their
floating point units are.  This is not normally a problem, the ABIs
make everything FP aligned.  The RTL patterns converting FP to integer
however get a potentially unaligned destination, and we do not want to
do an stfiwx on that on such older CPUs.

This fixes it.  It does not change anything for TARGET_MFCRF targets
(POWER4 and later).  It also won't change anything for strict-alignment
targets, or CPUs without hardware FP of course, or CPUs that do not
implement stfiwx (older 4xx/5xx/8xx).

It does not change the correcponding fixuns* pattern, because that can
not be enabled on any CPU that cannot handle unaligned FP well.

Tested on powerpc64-linux {-m32,-m64}, committing to trunk.  Will do
backports later, too.


Segher


2018-03-19  Segher Boessenkool  

PR target/89746
* config/rs6000/rs6000.md (fix_truncsi2_stfiwx): If we have a
non-TARGET_MFCRF target, and the dest is memory but not 32-bit aligned,
go via a stack temporary.

---
 gcc/config/rs6000/rs6000.md | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index aedba2c..e8885b6 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -5668,13 +5668,13 @@ (define_insn_and_split "fix_truncsi2_stfiwx"
 tmp = gen_reg_rtx (DImode);
 
   emit_insn (gen_fctiwz_ (tmp, src));
-  if (MEM_P (dest))
+  if (MEM_P (dest) && (TARGET_MFCRF || MEM_ALIGN (dest) >= 32))
 {
   dest = rs6000_force_indexed_or_indirect_mem (dest);
   emit_insn (gen_stfiwx (dest, tmp));
   DONE;
 }
-  else if (TARGET_POWERPC64 && TARGET_DIRECT_MOVE)
+  else if (TARGET_POWERPC64 && TARGET_DIRECT_MOVE && !MEM_P (dest))
 {
   dest = gen_lowpart (DImode, dest);
   emit_move_insn (dest, tmp);
-- 
1.8.3.1

Re: [PATCH] PR ada/89583, GNAT.Sockets.Bind_Socket fails with IPv4 address

[Simon Wright]

Ping?

> On 5 Mar 2019, at 20:43, Simon Wright  wrote:
> 
> Arno,
> 
> OK.
> 
> --S
> 
>> On 5 Mar 2019, at 20:38, Arnaud Charlet  wrote:
>> 
>> Simon,
>> 
>> Thanks for the patch.
>> We already have a fix pending for that in our tree that we will merge.
>> 
>> Arno
>> 
>>> On 4 Mar 2019, at 20:48, Simon Wright  wrote:
>>> 
>>> With GCC9, GNAT.Sockets includes support for IPv6. Sockaddr is an 
>>> Unchecked_Union, which now includes IPv6 fields, bringing the total 
>>> possible size to 28 bytes. The code in Bind_Socket currently calculates the 
>>> length of the struct sockaddr to be passed to bind(2) as this size, which 
>>> (at any rate on Darwin x86_64) results in failure (EINVAL).
>>> 
>>> This patch provides the required length explicitly from the socket's family.
>>> 
>>> Tested by rebuilding the compiler with --disable-bootstrap and re-running 
>>> the reproducer.
>>> 
>>> gcc/ada/Changelog:
>>> 
>>>  2019-03-04 Simon Wright 
>>> 
>>>  PR ada/89583
>>>  * libgnat/g-socket.adb (Bind_Socket): Calculate Len (the significant 
>>> length of
>>>the Sockaddr) using the Family of the Address parameter.
>>> 
>>> 
>>> 
>> 
> 

Re: C++ PATCH for c++/89612 - ICE with member friend template with noexcept

[Marek Polacek]

On Thu, Mar 14, 2019 at 04:22:41PM -0400, Jason Merrill wrote:
> On 3/7/19 4:52 PM, Marek Polacek wrote:
> > This was one of those PRs where the more you poke, the more ICEs turn up.
> > This patch fixes the ones I could find.  The original problem was that
> > maybe_instantiate_noexcept got a TEMPLATE_DECL created for the member
> > friend template in do_friend.  Its noexcept-specification was deferred,
> > so we went to the block with push_access_scope, but that crashes on a
> > TEMPLATE_DECL.  One approach could be to somehow not defer noexcept-specs
> > for friend templates, I guess, but I didn't want to do that.
> 
> How does it make sense to instantiate the noexcept-specifier of a template?
> We should only get there for fully-instantiated function decls.

Hmm, but duplicate_decls calls check_redeclaration_exception_specification even
for DECL_FUNCTION_TEMPLATE_Ps.

It's likely I got this wrong, but I thought template friends are somewhat
special, and they are wrapped in a TEMPLATE_DECL even when it's not an
instantiation.  If I have this

template 
class C {
  int n;

  template 
  friend int foo(C);
};

template 
int foo(C c)
{
  return c.n;
}

void
g ()
{
  C c;
  foo<0>(c);
}

then we call instantiate_class_template for C, which calls 
tsubst_friend_function
on the TEMPLATE_DECL foo, but that remains a TEMPLATE_DECL, unlike for member
template functions.  Here's a related patch:
https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01918.html

Note the crash happens in tsubst_friend_function.  I wouldn't know when to
check the noexcept-specifier of such a TEMPLATE_DECL for a template friend
if not there.

> > Lastly, I found an invalid testcase that was breaking because a template 
> > code
> > leaked to constexpr functions.  This I fixed similarly to the recent 
> > explicit
> > PR fix (r269131).
> 
> This spot should probably also use build_converted_constant_expr.

Ok, I'll address this.

Marek

Re: [RFC] D support for S/390

[Robin Dapp]

> This would mean that StructFlags and ClassFlags will also both have a
> wrong value as well.

Yes, can confirm that m_flags = 0 (instead of 1) for a struct containing
a pointer.

> If there's a compiler/library discrepancy, the compiler should be
> adjusted to write out the value at the correct size.
> 
> I think the following below should do it.

The diff fixes the problems for me.  I first encountered the algn = 0 in
an associative array and couldn't immediately link it to structs.  In
order to make pinpointing easier in the future I wrote a simple test
case for this that I'm going to post with the next diff.

Regards
 Robin

Re: [PATCH] PR libgcc/60790: Avoid IFUNC resolver access to uninitialized data

[Florian Weimer]

* Florian Weimer:

> * Jeff Law:
>
>> On 03/29/2018 08:00 AM, Florian Weimer wrote:
>>> This patch performs lazy initialization of the relevant CPUID feature
>>> register value.  It will needlessly invoke the CPUID determination code
>>> on architectures which lack CPUID support or support for the feature
>>> register, but I think it's not worth to avoid the complexity for that.
>>> 
>>> I verified manually that the CMPXCHG16B implementation is still selected
>>> for a 128-bit load after the change.  I don't know how to write an
>>> automated test for that.
>>> 
>>> Thanks,
>>> Florian
>>> 
>>> pr60790.patch
>>> 
>>> 
>>> Index: libatomic/ChangeLog
>>> ===
>>> --- libatomic/ChangeLog (revision 258952)
>>> +++ libatomic/ChangeLog (working copy)
>>> @@ -1,3 +1,18 @@
>>> +2018-03-29  Florian Weimer  
>>> +
>>> +   PR libgcc/60790
>>> +   x86: Do not assume ELF constructors run before IFUNC resolvers.
>>> +   * config/x86/host-config.h (libat_feat1_ecx, libat_feat1_edx):
>>> +   Remove declarations.
>>> +   (__libat_feat1, __libat_feat1_init): Declare.
>>> +   (FEAT1_REGISTER): Define.
>>> +   (load_feat1): New function.
>>> +   (IFUNC_COND_1): Adjust.
>>> +   * config/x86/init.c (libat_feat1_ecx, libat_feat1_edx)
>>> +   (init_cpuid): Remove definitions.
>>> +   (__libat_feat1): New variable.
>>> +   (__libat_feat1_init): New function.
>> OK.
>
> Here is the backport to gcc-8-branch.

Ping?  

Re: [PATCH 0/2] Future warnings not treated as errors

[Joseph Myers]

On Tue, 19 Mar 2019, Martin Liška wrote:

> On 3/19/19 3:46 AM, Alex Henrie wrote:
> > Hello,
> > 
> > I have received permission to contribute to GCC, but I do not have
> > commit access. Could one of you please merge these patches for me?
> 
> Hi.
> 
> I can do it for you. But if I see correctly, Joseph M. had some concerns
> and he should probably make final patch approval.

I've noted to look at this patch during development stage 1.

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

Re: [PATCH] [MinGW] Set __USE_MINGW_ACCESS for C++ as well

[JonY]

On 3/18/19 10:31 PM, JonY wrote:
> On 3/3/19 10:41 AM, Johannes Pfau wrote:
>> We set __USE_MINGW_ACCESS for windows hosts to use MinGWs wrapper
>> for the access function. The wrapper ensures that access behaves
>> in the expected way (e.g. for special files, such as nul).
>> However, we now compile most sources with the C++ compiler and
>> the __USE_MINGW_ACCESS in CFLAGS is not used there. This causes
>> GCCs build against newer msvcrt versions with incompatible
>> access implementations to fail. This patch adds the flag to the
>> CXXFLAGS for all bootstrap stages. Bootstrapped on
>> x86_64-mingw64-seh.
>>
>> config/ChangeLog:
>>
>> 2019-03-02  Johannes Pfau  
>>
>>  * mh-mingw: Also set __USE_MINGW_ACCESS flag for C++ code.
>>
>> ---
>>  config/mh-mingw | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/config/mh-mingw b/config/mh-mingw
>> index bc1d27477d0..a795096f038 100644
>> --- a/config/mh-mingw
>> +++ b/config/mh-mingw
>> @@ -2,6 +2,11 @@
>>  # Vista (see PR33281 for details).
>>  BOOT_CFLAGS += -D__USE_MINGW_ACCESS -Wno-pedantic-ms-format
>>  CFLAGS += -D__USE_MINGW_ACCESS
>> +STAGE1_CXXFLAGS += -D__USE_MINGW_ACCESS
>> +STAGE2_CXXFLAGS += -D__USE_MINGW_ACCESS
>> +STAGE3_CXXFLAGS += -D__USE_MINGW_ACCESS
>> +STAGE4_CXXFLAGS += -D__USE_MINGW_ACCESS
>> +
>>  # Increase stack limit to a figure based on the Linux default, with 4MB 
>> added
>>  # as GCC turns out to need that much more to pass all the limits-* tests.
>>  LDFLAGS += -Wl,--stack,12582912
>>
> 
> Patch looks good, will apply soon.
> 

Forgot to reply that this was checked in as r269784.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Fix set of even probabilities (PR middle-end/89737).

[Jakub Jelinek]

On Tue, Mar 19, 2019 at 09:02:27AM +0100, Martin Liška wrote:
> 2019-03-18  Martin Liska  
> 
>   PR middle-end/89737
>   * predict.c (combine_predictions_for_bb): Empty likely_edges and
>   unlikely_edges if there's an edge that belongs to both these sets.

I admit I don't know this code at all, just wonder if if you find
an intersection between likely_edges and unlikely_edges, couldn't you just
remove the intersection from both, rather than all other edges too?

Jakub

Re: [PATCH] Fix set of even probabilities (PR middle-end/89737).

[Jan Hubicka]

> On 3/19/19 2:02 AM, Martin Liška wrote:
> > Hi.
> > 
> > When calling set_even_probabilities, the function assumes that an edge
> > can't live in both sets ({un,}likely_edges). When such situation happens,
> > clear just the sets.
> > 
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > 
> > Ready to be installed?
> > Thanks,
> > Martin
> > 
> > gcc/ChangeLog:
> > 
> > 2019-03-18  Martin Liska  
> > 
> > PR middle-end/89737
> > * predict.c (combine_predictions_for_bb): Empty likely_edges and
> > unlikely_edges if there's an edge that belongs to both these sets.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2019-03-18  Martin Liska  
> > 
> > PR middle-end/89737
> > * gcc.dg/pr89737.c: New test.
> But is having the edge in both sets a valid state?  You know this code
> better than I, so I'll go with your recommendation here.

It is the situation you add both hot and cold attribute which I guess
can happen.
I think it is OK to ignore both hint then, so patch is OK.

Honza
> 
> jeff

Re: [PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)

[Jeff Law]

On 3/14/19 4:43 AM, Richard Biener wrote:
> On Wed, 13 Mar 2019, Richard Biener wrote:
> 
>> On Wed, 13 Mar 2019, Bin.Cheng wrote:
>>
>>> On Wed, Mar 13, 2019 at 3:58 AM Richard Biener  wrote:


 This makes an attempt at fixing the most annoying parts of the GIMPLE
 FE unit testing - the lack of proper CFG preservation and hoops you
 need to jump through to make the CFG and SSA builders happy.

 Due to this the __GIMPLE specifiers takes two new flags, "cfg"
 for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA.
 When there is a CFG you need to start basic-block boundaries with

 __BB(index):

 where 'index' is the basic-block index.  That implicitely defines
 a label __BBindex for use in goto stmts and friends (but doesn't
 actually add a GIMPLE_LABEL).

 The parsing code isn't defensive right now so you need to watch
 out to not use index 0 or 1 (entry and exit block) which are
 only implicitely present.

 As a proof of concept I added one BB annotation - loop_header(num)
 where "num" is the loop number.  This means you can now also
 have loop structures preserved (to some extent - the loop tree is
 not explicitely represented nor are loop fathers, both are re-built
 via fixup).

 I've not yet adjusted -gimple dumping.

 I've adjusted all testcases I could find.

 The CFG build inside the frontend is a bit awkward (spread out...),
 likewise some functionality is asking for split-out.

 This is mainly a RFC for whether you think this is forward looking
 enough.  Future enhancements would include EH and abnormal edges
 via stmt annotations:
>>> Thanks very much for doing this.  Do we have a centralized
>>> wiki/document on the formal definition?  It would be very helpful even
>>> it's still evolving, otherwise we would need to dig into messages for
>>> fragmented clues.
>>
>> Earlier this week I transformed the old 
>> https://gcc.gnu.org/wiki/GimpleFrontEnd page into one documenting
>> the current state but this doesn't yet include any kind of documentation.
>>
>> I suppose special syntax documentation should go into our texinfo
>> docs, I guess into a new section in the internals manual.  Currently
>> we only have documentation for the -fgimple switch.
>>
>> Note that I view -gimple dumping as the thing that should give you
>> a clue about expected syntax.  I hope to spend a little more time
>> on the patch to make it ready for GCC9.
> 
> I've settled on another change, forcing explicit gotos for fallthru
> edges to the next block.
> 
> Otherwise I've cleaned up the patch and removed no longer necessary
> support for IFN_PHI from the middle-end.  During cleanup I introduced
> a gimple_parser class to hold the extra parsing state and I added
> some extra error handling.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing is still in progress.
> 
> I will commit this to make testcase extraction to GIMPLE more feasible
> for GCC 9+ (we could even backport the GIMPLE FE changes to GCC 8 if 
> needed I guess).  At least it will give me motivation to do further
> required changes when I run into the next big LTO testcase running
> into a loop optimizer issue... (non-loop passes should work fine
> already).
If it makes building testcases easier, then I'm all for it ;-)

jeff

Re: [PATCH] Fix mips subreg handling (PR target/89378)

[Jeff Law]

On 3/14/19 7:52 AM, Jakub Jelinek wrote:
> Hi!
> 
> On the gcc.dg/vect/pr88598-3.c testcase, gen_vec_extract... is called with
> operands[1] that is already a subreg - (subreg:V4SF (reg:V4SI 201 [ _31 ]) 0)
> and wraps it in another SUBREG, which is invalid in RTL.
> 
> In all the following 4 spots I've verified that the operand's mode is some
> 128-bit vector mode and the mode we want to use is another 128-bit vector
> mode, so we are just VCEing the operand to another same sized mode, so we can
> call gen_lowpart to handle everything for us (there are no worries about big
> vs. little endian etc.).
> 
> Paul Hua has kindly bootstrapped/regtested this on mips (mentioned in the
> PR), ok for trunk?
> 
> 2019-03-14  Jakub Jelinek  
> 
>   PR target/89378
>   * config/mips/mips.c (mips_expand_vec_cond_expr): Use gen_lowpart
>   instead of gen_rtx_SUBREG.
>   * config/mips/mips-msa.md (vec_extract): Likewise.
I thought this had been OK'd, but I don't see it in the tree.  So I'll
go ahead and explicitly OK it.

jeff

Re: [PATCH] Fix set of even probabilities (PR middle-end/89737).

[Jeff Law]

On 3/19/19 2:02 AM, Martin Liška wrote:
> Hi.
> 
> When calling set_even_probabilities, the function assumes that an edge
> can't live in both sets ({un,}likely_edges). When such situation happens,
> clear just the sets.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-03-18  Martin Liska  
> 
>   PR middle-end/89737
>   * predict.c (combine_predictions_for_bb): Empty likely_edges and
>   unlikely_edges if there's an edge that belongs to both these sets.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-03-18  Martin Liska  
> 
>   PR middle-end/89737
>   * gcc.dg/pr89737.c: New test.
But is having the edge in both sets a valid state?  You know this code
better than I, so I'll go with your recommendation here.

jeff

Re: [PATCH] Fix -Wenum-compare-switch warning in i386.c.

[Jeff Law]

On 3/8/19 5:34 AM, Martin Liška wrote:
> A small patch that deals with:
> gcc/config/i386/i386.c:39427:11:Semantic Issue: comparison of two values with 
> different enumeration types in switch statement ('enum built_in_function' and 
> 'ix86_builtins'): -Wenum-compare-switch
> 
> Is it fine to install it?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-03-08  Martin Liska  
> 
>   * config/i386/i386.c (ix86_builtin_reciprocal): Cast
>   DECL_FUNCTION_CODE into ix86_builtins enum before
>   the switch statement.
> ---
>  gcc/config/i386/i386.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
Seems trivial, but I'd still defer to gcc-10 unless there's a compelling
need to address it right now.

jeff

Re: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)

[Jeff Law]

On 3/11/19 8:27 PM, Martin Sebor wrote:
> The -Wstringop-truncation handling for strncpy/stpncpy neglects
> to consider that character arrays tracked by the strlen pass
> are not necessarily nul-terminated.  It unconditionally adds
> one when computing the size of each sequence to account for
> the nul.  This leads to false positive warnings when checking
> the validity of indices/pointers computed by the built-ins.
> 
> The attached patch corrects this by adding one for the nul
> only when the character array is known to be nul-terminated.
> 
> Since GCC 7 does not issue the warning this is a 8/9 regression
> that I would like to fix in both releases.  Is the patch okay
> for trunk/gcc-8-branch?
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-89644.diff
> 
> PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic on 
> strncpy
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/89644
>   * tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
>   arrays in determining sequence sizes in strncpy and stpncpy.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/89644
>   * gcc.dg/Wstringop-truncation-8.c: New test.
OK for the trunk as well as the affected release branches.

jeff

[PATCH, rs6000] PR89732: New test pr87532-mc.c fails on compiler not defaulting to VSX

[Kelvin Nilsen]

A recently added test was observed to fail when compiled without the -mvsx 
option.  This patch adds -mvsx to the dg-options directive.

Was boostrapped and regression tested on powerpc-linux (P7 big-endian, both 
-m32 and -m64).

Was preapproved by seg...@gcc.gnu.org and has been merged with trunk.

gcc/testsuite/ChangeLog:

2019-03-19  Kelvin Nilsen  

PR target/89736
* gcc.target/powerpc/pr87532-mc.c: Modify dejagnu directives to
restrict this test to vsx targets.

Index: gcc/testsuite/gcc.target/powerpc/pr87532-mc.c
===
--- gcc/testsuite/gcc.target/powerpc/pr87532-mc.c   (revision 269782)
+++ gcc/testsuite/gcc.target/powerpc/pr87532-mc.c   (working copy)
@@ -1,8 +1,8 @@
 /* { dg-do run { target int128 } } */
-/* { dg-require-effective-target vmx_hw } */
-/* { dg-options "-maltivec -O2" } */
+/* { dg-require-effective-target vsx_hw } */
+/* { dg-options "-mvsx -O2" } */
 
-/* This test should run the same on any target that supports altivec/dfp
+/* This test should run the same on any target that supports vsx
instructions.  Intentionally not specifying cpu in order to test
all code generation paths.  */
 

libgo patch committed: Fix AIX support

[Ian Lance Taylor]

This patch by Clément Chigot fixes the AIX support after the update to
Go 1.12.  Bootstrapped and ran Go tests on x86_64-pc-linux-gnu.
Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 269780)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-87945b620b2100d33e27f33e6276a4e4e5890659
+069afe85f38c099660c5d81950d65248ed4fc516
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/archive/tar/stat_actime1.go
===
--- libgo/go/archive/tar/stat_actime1.go(revision 269619)
+++ libgo/go/archive/tar/stat_actime1.go(working copy)
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build hurd linux dragonfly openbsd solaris
+// +build aix hurd linux dragonfly openbsd solaris
 
 package tar
 
Index: libgo/go/cmd/go/internal/lockedfile/internal/filelock/filelock_fcntl.go
===
--- libgo/go/cmd/go/internal/lockedfile/internal/filelock/filelock_fcntl.go 
(revision 269619)
+++ libgo/go/cmd/go/internal/lockedfile/internal/filelock/filelock_fcntl.go 
(working copy)
@@ -35,7 +35,9 @@ const (
writeLock lockType = syscall.F_WRLCK
 )
 
-type inode = uint64 // type of syscall.Stat_t.Ino
+// type of syscall.Stat_t.Ino for 64 bits architectures.
+// For 32 bits architecture, it's easier to cast it instead.
+type inode = uint64
 
 type inodeLock struct {
owner File
@@ -59,7 +61,7 @@ func lock(f File, lt lockType) (err erro
if err != nil {
return err
}
-   ino := fi.Sys().(*syscall.Stat_t).Ino
+   ino := uint64(fi.Sys().(*syscall.Stat_t).Ino)
 
mu.Lock()
if i, dup := inodes[f]; dup && i != ino {
Index: libgo/go/internal/syscall/unix/at_largefile.go
===
--- libgo/go/internal/syscall/unix/at_largefile.go  (revision 269619)
+++ libgo/go/internal/syscall/unix/at_largefile.go  (working copy)
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build aix hurd linux solaris,386 solaris,sparc
+// +build hurd linux solaris,386 solaris,sparc
 
 package unix
 
Index: libgo/go/internal/syscall/unix/at_regfile.go
===
--- libgo/go/internal/syscall/unix/at_regfile.go(revision 269619)
+++ libgo/go/internal/syscall/unix/at_regfile.go(working copy)
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build !aix
 // +build !hurd
 // +build !linux
 // +build !solaris !386
Index: libgo/go/internal/syscall/unix/ioctl_aix.go
===
--- libgo/go/internal/syscall/unix/ioctl_aix.go (revision 269619)
+++ libgo/go/internal/syscall/unix/ioctl_aix.go (working copy)
@@ -9,17 +9,12 @@ import (
"unsafe"
 )
 
-//go:cgo_import_dynamic libc_ioctl ioctl "libc.a/shr_64.o"
-//go:linkname libc_ioctl libc_ioctl
-var libc_ioctl uintptr
-
-// Implemented in syscall/syscall_aix.go.
-func syscall6(trap, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, 
err syscall.Errno)
+//extern __go_ioctl_ptr
+func ioctl(int32, int32, unsafe.Pointer) int32
 
 func Ioctl(fd int, cmd int, args uintptr) (err error) {
-   _, _, e1 := syscall6(uintptr(unsafe.Pointer(_ioctl)), 3, 
uintptr(fd), uintptr(cmd), uintptr(args), 0, 0, 0)
-   if e1 != 0 {
-   err = e1
+   if ioctl(int32(fd), int32(cmd), unsafe.Pointer(args)) < 0 {
+   return syscall.GetErrno()
}
return
 }
Index: libgo/go/net/interface_aix.go
===
--- libgo/go/net/interface_aix.go   (revision 269619)
+++ libgo/go/net/interface_aix.go   (working copy)
@@ -32,6 +32,8 @@ const _RTAX_NETMASK = 2
 const _RTAX_IFA = 5
 const _RTAX_MAX = 8
 
+const _SIOCGIFMTU = -0x3fd796aa
+
 func getIfList() ([]byte, error) {
needed, err := syscall.Getkerninfo(_KINFO_RT_IFLIST, 0, 0, 0)
if err != nil {
@@ -62,7 +64,7 @@ func interfaceTable(ifindex int) ([]Inte
}
if ifm.Type == syscall.RTM_IFINFO {
if ifindex == 0 || ifindex == int(ifm.Index) {
-   sdl := 
(*rawSockaddrDatalink)(unsafe.Pointer([syscall.SizeofIfMsghdr]))
+   sdl := 
(*rawSockaddrDatalink)(unsafe.Pointer([unsafe.Sizeof(syscall.IfMsgHdr)]))
 
ifi := {Index: int(ifm.Index), Flags: 
linkFlags(ifm.Flags)}
ifi.Name = 

[PING] [PATCHv2] Fix not 8-byte aligned ldrd/strd on ARMv5 (PR 89544)

[Bernd Edlinger]

Hi,

I'd like to ping for this patch:
https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00438.html

Thanks
Bernd.


On 3/10/19 10:42 AM, Bernd Edlinger wrote:
> Hi,
> 
> This patch is an update to the previous patch, which takes into account that
> the middle-end is not supposed to use the unaligned DI value directly which
> was passed in an unaligned stack slot due to the AAPCS parameter passing 
> rules.
> 
> The patch works by changing use_register_for_decl to return false if the
> incoming RTL is not sufficiently aligned on a STRICT_ALIGNMENT target,
> as if the address of the parameter was taken (which is TREE_ADDRESSABLE).
> So not taking the address of the parameter is a necessary condition
> for the wrong-code in PR 89544.
> 
> It works together with this check in assign_parm_adjust_stack_rtl:
>   /* If we can't trust the parm stack slot to be aligned enough for its
>  ultimate type, don't use that slot after entry.  We'll make another
>  stack slot, if we need one.  */
>   if (stack_parm
>   && ((STRICT_ALIGNMENT
>&& GET_MODE_ALIGNMENT (data->nominal_mode) > MEM_ALIGN 
> (stack_parm))
> ...
> stack_param = NULL
> 
> This makes assign_parms use assign_parm_setup_stack instead of
> assign_parm_setup_reg, and the latter does assign a suitably aligned
> stack slot, because stack_param == NULL by now, and uses emit_block_move
> which avoids the issue with the back-end.
> 
> Additionally, to prevent unnecessary performance regressions,
> assign_parm_find_stack_rtl is enhanced to make use of a possible larger
> alignment if the parameter was passed in an aligned stack slot without
> the ABI requiring it.
> 
> 
> Bootstrapped and reg-tested with all languages on arm-linux-gnueabihf.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

Re: [RFC] D support for S/390

[Iain Buclaw]

On Tue, 19 Mar 2019 at 13:07, Robin Dapp  wrote:
>
> Hi,
>
> > Alignment is written to TypeInfo, I don't think it should ever be
> > zero.  That would mean that it isn't being generated by the compiler,
> > or read by the library correctly, so something else is amiss.
>
> it took me a while to see that in libphobos/libdruntime/object.d
>
>  override @property size_t talign() nothrow pure const {return m_align;}
>
> returns a size_t but m_align is a uint.  The type info seems to be
> stored in memory by the compiler and writes a GCC "sizetype".  When
> using it, we only emit a 4-byte read which loads the first half of the
> stored 8 bytes.  This will work on a little-endian machine but fail on
> big endian.
>
> I'd hope it is safe to change m_align's type to size_t since, as far as
> I can tell, the compiler will always write 8 bytes and the memory layout
> will not be changed by that.
>

So, when the initializer value type is larger than the field, it
truncates rather than rounds?

This would mean that StructFlags and ClassFlags will also both have a
wrong value as well.

> Does this [1] look reasonable?
>

If there's a compiler/library discrepancy, the compiler should be
adjusted to write out the value at the correct size.

I think the following below should do it.

-- 
Iain

--- a/gcc/d/typeinfo.cc
+++ b/gcc/d/typeinfo.cc
@@ -830,7 +830,7 @@ public:
flags |= ClassFlags::noPointers;

 Lhaspointers:
-   this->layout_field (size_int (flags));
+   this->layout_field (build_integer_cst (flags, d_uint_type));

/* void *deallocator;  */
tree ddtor = (cd->aggDelete)
@@ -886,7 +886,7 @@ public:
if (cd->isCOMinterface ())
  flags |= ClassFlags::isCOMclass;

-   this->layout_field (size_int (flags));
+   this->layout_field (build_integer_cst (flags, d_uint_type));

/* void *deallocator;
   OffsetTypeInfo[] m_offTi;  (not implemented)
@@ -1019,7 +1019,7 @@ public:
 StructFlags::Type m_flags = 0;
 if (ti->hasPointers ())
   m_flags |= StructFlags::hasPointers;
-this->layout_field (size_int (m_flags));
+this->layout_field (build_integer_cst (m_flags, d_uint_type));

 /* void function(void*) xdtor;  */
 tree dtor = (sd->dtor) ? build_address (get_symbol_decl (sd->dtor))
@@ -1033,7 +1033,7 @@ public:
   this->layout_field (null_pointer_node);

 /* uint m_align;  */
-this->layout_field (size_int (ti->alignsize ()));
+this->layout_field (build_integer_cst (ti->alignsize (), d_uint_type));

 if (global.params.is64bit)
   {
@@ -1488,9 +1488,8 @@ create_typeinfo (Type *type, Module *mod)
  make_internal_typeinfo (tk, ident,
  array_type_node, array_type_node,
  ptr_type_node, ptr_type_node,
- ptr_type_node, ptr_type_node,
- size_type_node, ptr_type_node,
- ptr_type_node, size_type_node,
+ ptr_type_node, ptr_type_node, d_uint_type,
+ ptr_type_node, ptr_type_node, d_uint_type,
  ptr_type_node, argtype, argtype, NULL);
}
  t->vtinfo = TypeInfoStructDeclaration::create (t);

Re: [RFC] D support for S/390

[Richard Biener]

On March 19, 2019 1:07:26 PM GMT+01:00, Robin Dapp  wrote:
>Hi,
>
>> Alignment is written to TypeInfo, I don't think it should ever be
>> zero.  That would mean that it isn't being generated by the compiler,
>> or read by the library correctly, so something else is amiss.
>
>it took me a while to see that in libphobos/libdruntime/object.d
>
>override @property size_t talign() nothrow pure const {return m_align;}
>
>returns a size_t but m_align is a uint.  The type info seems to be
>stored in memory by the compiler and writes a GCC "sizetype".  

It should probably use sth specified by the C ABI instead, like using 
size_type_node which maps to the targets size_t rather than the internal 
sizetype.

When
>using it, we only emit a 4-byte read which loads the first half of the
>stored 8 bytes.  This will work on a little-endian machine but fail on
>big endian.
>
>I'd hope it is safe to change m_align's type to size_t since, as far as
>I can tell, the compiler will always write 8 bytes and the memory
>layout
>will not be changed by that.
>
>Does this [1] look reasonable?
>
>Regards
> Robin
>
>---
>
>[1]
>diff --git a/gcc/d/typeinfo.cc b/gcc/d/typeinfo.cc
>index dac66acdcd4..1bd94c33e82 100644
>--- a/gcc/d/typeinfo.cc
>+++ b/gcc/d/typeinfo.cc
>@@ -954,7 +954,7 @@ public:
>StructFlags m_flags;
>void function(void*) xdtor;
>void function(void*) xpostblit;
>-   uint m_align;
>+   size_t m_align;
>version (X86_64)
>TypeInfo m_arg1;
>TypeInfo m_arg2;
>@@ -1032,7 +1032,7 @@ public:
> else
>   this->layout_field (null_pointer_node);
>
>-/* uint m_align;  */
>+/* size_t m_align;  */
> this->layout_field (size_int (ti->alignsize ()));
>
> if (global.params.is64bit)
>diff --git a/libphobos/libdruntime/object.d
>b/libphobos/libdruntime/object.d
>index 38bd0ae1f6b..bb821bf7040 100644
>--- a/libphobos/libdruntime/object.d
>+++ b/libphobos/libdruntime/object.d
>@@ -1226,7 +1226,7 @@ class TypeInfo_Struct : TypeInfo
> }
> void function(void*)xpostblit;
>
>-uint m_align;
>+size_t m_align;
>
>override @property immutable(void)* rtInfo() const { return m_RTInfo; }

Re: [RFC] D support for S/390

[Robin Dapp]

Hi,

> Alignment is written to TypeInfo, I don't think it should ever be
> zero.  That would mean that it isn't being generated by the compiler,
> or read by the library correctly, so something else is amiss.

it took me a while to see that in libphobos/libdruntime/object.d

 override @property size_t talign() nothrow pure const {return m_align;}

returns a size_t but m_align is a uint.  The type info seems to be
stored in memory by the compiler and writes a GCC "sizetype".  When
using it, we only emit a 4-byte read which loads the first half of the
stored 8 bytes.  This will work on a little-endian machine but fail on
big endian.

I'd hope it is safe to change m_align's type to size_t since, as far as
I can tell, the compiler will always write 8 bytes and the memory layout
will not be changed by that.

Does this [1] look reasonable?

Regards
 Robin

---

[1]
diff --git a/gcc/d/typeinfo.cc b/gcc/d/typeinfo.cc
index dac66acdcd4..1bd94c33e82 100644
--- a/gcc/d/typeinfo.cc
+++ b/gcc/d/typeinfo.cc
@@ -954,7 +954,7 @@ public:
StructFlags m_flags;
void function(void*) xdtor;
void function(void*) xpostblit;
-   uint m_align;
+   size_t m_align;
version (X86_64)
TypeInfo m_arg1;
TypeInfo m_arg2;
@@ -1032,7 +1032,7 @@ public:
 else
   this->layout_field (null_pointer_node);

-/* uint m_align;  */
+/* size_t m_align;  */
 this->layout_field (size_int (ti->alignsize ()));

 if (global.params.is64bit)
diff --git a/libphobos/libdruntime/object.d b/libphobos/libdruntime/object.d
index 38bd0ae1f6b..bb821bf7040 100644
--- a/libphobos/libdruntime/object.d
+++ b/libphobos/libdruntime/object.d
@@ -1226,7 +1226,7 @@ class TypeInfo_Struct : TypeInfo
 }
 void function(void*)xpostblit;

-uint m_align;
+size_t m_align;

 override @property immutable(void)* rtInfo() const { return m_RTInfo; }

Re: Fix a case in which the vector cost model was ignored

[Bernhard Reutner-Fischer]

On Tue, 19 Mar 2019 08:47:49 +
Richard Sandiford  wrote:

> > ... this identical condition, AFAICS?
> > So this second conditions else arm should be dead, shouldn't it?  
> 
> Yeah, that's what:
> 
>   /* ??? The "if" arm is written to handle all cases; see below for what
>  we would do for !LOOP_VINFO_FULLY_MASKED_P.  */

Ok, i somehow managed not to correlate this comment to the inner if's
else. It's been late. Sorry for the noise!

thanks,
> 
> was trying to say.  Like I mentioned in the covering note, in principle
> the approach of calculating the minimum number of vector iterations
> should work for all cases, and we might want to consider doing that
> for stage 1.  I wanted to show what the !LOOP_VINFO_FULLY_MASKED_P
> code would look like if we did that.
> 
> I'd wondered about putting the inner else in an #if 0 or comment instead,
> but this way makes it easier to experiment with.
> 
> Thanks,
> Richard

Re: [PATCH] Fix PR71598, aliasing between enums and compatible types

[Richard Biener]

On Tue, 19 Mar 2019, Michael Matz wrote:

> Hi,
> 
> On Tue, 19 Mar 2019, Richard Biener wrote:
> 
> > It doesn't really.  Consider the following IMHO valid testcase:
> > 
> > enum X { A, B };
> > enum Y { C, D };
> > void foo (int *p)
> > {
> >  *(enum X *)p = A;
> >  *(enum Y *)p = D;
> >  return *(enum X *)p;
> > }
> > 
> > int main()
> > {
> >   int storage;
> >   if (foo () != A)
> 
> (You want to require 'B' here, not 'A')

Eh, yes.  Actually D I guess, or simply 1.

> > abort ();
> > }
> 
> If you want the above testcase to be valid then yes, I agree, you have to 
> give all enums the same alias set as the underlying type.  Meh, I don't 
> like it :-/

I think the C standard says the testcase should not abort.

The patch still needs approval though.  I'll make sure to add a testcase
like the above.

Richard.

[PATCH] Disable -gsplit-dwarf with -flto (PR88389)

[Richard Biener]

The following ignores -gsplit-dwarf when doing LTO since its model
seems to be fundamentally incompatible, see my last comment in the
now suspended PR.

Bootstrap & regtest running on x86_64-unknown-linux-gnu, I'll apply
this to trunk and GCC 8 branch.

Richard.

2019-03-19  Richard Biener  

PR debug/88389
* opts.c (finish_options): Disable -gsplit-dwarf when doing LTO.

Index: gcc/opts.c
===
--- gcc/opts.c  (revision 269793)
+++ gcc/opts.c  (working copy)
@@ -1077,6 +1077,14 @@ finish_options (struct gcc_options *opts
  "linker plugin");
  opts->x_flag_fat_lto_objects = 1;
}
+
+  /* -gsplit-dwarf isn't compatible with LTO, see PR88389.  */
+  if (opts->x_dwarf_split_debug_info)
+   {
+ inform (loc, "%<-gsplit-dwarf%> is not supported with LTO,"
+ " disabling");
+ opts->x_dwarf_split_debug_info = 0;
+   }
 }
 
   /* We initialize opts->x_flag_split_stack to -1 so that targets can set a

Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink

[James Hilliard]

On Tue, Mar 19, 2019 at 3:56 AM Jakub Jelinek  wrote:
>
> On Mon, Mar 18, 2019 at 06:35:12PM -0600, James Hilliard wrote:
> > On Mon, Mar 18, 2019 at 5:46 PM Jeff Law  wrote:
> > >
> > > On 3/18/19 5:07 PM, James Hilliard wrote:
> > > > On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek  wrote:
> > > >>
> > > >> On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote:
> > >  Thanks, but I'm saying that if you look at the code you can see that
> > >  st is clearly initialized, by the call to lstat.  I would like to see
> > >  an explanation for why you are seeing that warning before changing 
> > >  the
> > >  code to disable it.  Initializing st should not be necessary here.
> > >  For example, perhaps lstat is a macro when compiling libsanitizer; if
> > >  that is the underlying problem, then we should fix the macro, not 
> > >  this
> > >  code.
> > > >>> Yeah, I'm not sure why the compiler thinks lstat isn't initializing 
> > > >>> st.
> > > >>> What should I do to debug this further?
> > > >>
> > > >> Guess you should start by telling us which OS it is on (I can't 
> > > >> reproduce
> > > >> this warning on x86_64-linux nor i686-linux with glibc 2.28), looking 
> > > >> at
> > > >> preprocessed source to see what exactly lstat does (e.g. if it is some 
> > > >> macro
> > > >> or inline function and what exactly it is doing).
> > > > I am cross compiling with buildroot master branch using ubuntu 18.10.
> > > > I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain.
> > > > The build and target systems are both x86_64.
> > > Add "-save-temps" to the command line.  That will create a .i file, send
> > > the .i file along with the command line.
> > I added --save-temps to the cflags for the gcc package build.
> > Here's the command line log:
> > https://gist.githubusercontent.com/jameshilliard/015d972c26d1ec7bbbd1ad2d57d5dd3b/raw/7c848cdfba0967a681401de24ed6f4c86315d2d9/cli.log
> > Here's the elf.i fille:
> > https://gist.githubusercontent.com/jameshilliard/2a391d7292f3ba7412fe80e166c0f0e3/raw/63eb16ba9eff0b3ee668b16257b0948b2ca894a0/elf.i
>
> I still can't reproduce it, even with gcc 8.3.0:
> /d/gcc-8.3.0/objdir/gcc/xgcc -B /d/gcc-8.3.0/objdir/gcc/ -S -g3 -Og elf.i -W 
> -Wall -Wwrite-strings -Wmissing-format-attribute -Wcast-qual -Werror 
> -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -fPIC 
> -Wno-implicit-fallthrough
> doesn't print anything.
Try downloading buildroot(git commit
0af24710da70b04947229333c9450c86499b3108) and use this defconfig which
should reproduce the problem:
https://gist.github.com/jameshilliard/80e61f58e05e1325e0dbc6e30a11c851/raw/8e7d67cd1cc48f94b50e21f1ec407f29fe1dfe37/defconfig
>
> Jakub

Re: [PATCH] ARM cmpsi2_addneg fix follow-up (PR target/89506)

[Ramana Radhakrishnan]

On 04/03/2019 09:04, Jakub Jelinek wrote:
> On Fri, Mar 01, 2019 at 03:41:33PM +, Wilco Dijkstra wrote:
>> > and regtest revealed two code size
>> > regressions because of that.  Is -1 vs. 1 the only case of immediate
>> > valid for both "I" and "L" constraints where the former is longer than the
>> > latter?
>> 
>> Yes -1 is the only case which can result in larger code on Thumb-2, so -1 
>> should
>> be disallowed by the I constraint (or even better, the underlying query). 
>> That way
>> it will work correctly for all add/sub patterns, not just this one.
> 
> So, over the weekend I've bootstrapped/regtested on armv7hl-linux-gnueabi
> following two possible follow-ups which handle the -1 and 1 cases right
> (prefer the instruction with #1 for thumb2), 0 and INT_MIN (use subs) and
> for others use subs if both constraints match, otherwise adds.
> 
> The first one uses constraints and no C code in the output, I believe it is
> actually more expensive for compile time, because if one just reads what
> constrain_operands needs to do for another constraint, it is quite a lot.
> I've tried to at least not introduce new constraints for this, there is no
> constraint for number 1 (or for number -1).
> The Pu constraint is thumb2 only for numbers 1..8, and the alternative uses
> I constraint for the negation of it, i.e. -8..-1, only -1 from this is
> valid for I.  If that matches, we emit adds with #1, otherwise just prefer
> subs over adds.
> 
> The other swaps the alternatives similarly to the above, but for the special
> case of desirable adds with #1 uses C code instead of another alternative.
> 
> Ok for trunk (which one)?
> 
>      Jakub


Option #2 is better (the C code variant)- for something like this adding 
one more constraint is a bit painful.

Ok and watch out for any regressions as usual.

Ramana

Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink

[Jakub Jelinek]

On Mon, Mar 18, 2019 at 06:35:12PM -0600, James Hilliard wrote:
> On Mon, Mar 18, 2019 at 5:46 PM Jeff Law  wrote:
> >
> > On 3/18/19 5:07 PM, James Hilliard wrote:
> > > On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek  wrote:
> > >>
> > >> On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote:
> >  Thanks, but I'm saying that if you look at the code you can see that
> >  st is clearly initialized, by the call to lstat.  I would like to see
> >  an explanation for why you are seeing that warning before changing the
> >  code to disable it.  Initializing st should not be necessary here.
> >  For example, perhaps lstat is a macro when compiling libsanitizer; if
> >  that is the underlying problem, then we should fix the macro, not this
> >  code.
> > >>> Yeah, I'm not sure why the compiler thinks lstat isn't initializing st.
> > >>> What should I do to debug this further?
> > >>
> > >> Guess you should start by telling us which OS it is on (I can't reproduce
> > >> this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at
> > >> preprocessed source to see what exactly lstat does (e.g. if it is some 
> > >> macro
> > >> or inline function and what exactly it is doing).
> > > I am cross compiling with buildroot master branch using ubuntu 18.10.
> > > I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain.
> > > The build and target systems are both x86_64.
> > Add "-save-temps" to the command line.  That will create a .i file, send
> > the .i file along with the command line.
> I added --save-temps to the cflags for the gcc package build.
> Here's the command line log:
> https://gist.githubusercontent.com/jameshilliard/015d972c26d1ec7bbbd1ad2d57d5dd3b/raw/7c848cdfba0967a681401de24ed6f4c86315d2d9/cli.log
> Here's the elf.i fille:
> https://gist.githubusercontent.com/jameshilliard/2a391d7292f3ba7412fe80e166c0f0e3/raw/63eb16ba9eff0b3ee668b16257b0948b2ca894a0/elf.i

I still can't reproduce it, even with gcc 8.3.0:
/d/gcc-8.3.0/objdir/gcc/xgcc -B /d/gcc-8.3.0/objdir/gcc/ -S -g3 -Og elf.i -W 
-Wall -Wwrite-strings -Wmissing-format-attribute -Wcast-qual -Werror 
-Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -fPIC 
-Wno-implicit-fallthrough
doesn't print anything.

Jakub

[PATCH] Update libstdc++ API Evolution documentation

[Jonathan Wakely]

* doc/xml/manual/allocator.xml: Link to table documenting evolution
of extension allocators.
* doc/xml/manual/evolution.xml: Use angle brackets for header names.
Document new headers in 7.2, 8.1 and 9.1 releases.
* doc/xml/manual/using.xml: Adjust link target for new_allocator.
* doc/html/*: Regenerate.

Committed to trunk.

commit 2afabb199008ad0196de9d2c5c775efcffcb7782
Author: Jonathan Wakely 
Date:   Tue Mar 19 09:24:12 2019 +

Update libstdc++ API Evolution documentation

* doc/xml/manual/allocator.xml: Link to table documenting evolution
of extension allocators.
* doc/xml/manual/evolution.xml: Use angle brackets for header names.
Document new headers in 7.2, 8.1 and 9.1 releases.
* doc/xml/manual/using.xml: Adjust link target for new_allocator.
* doc/html/*: Regenerate.

diff --git a/libstdc++-v3/doc/xml/manual/allocator.xml 
b/libstdc++-v3/doc/xml/manual/allocator.xml
index 93d5c4a30f4..8d49b919ff6 100644
--- a/libstdc++-v3/doc/xml/manual/allocator.xml
+++ b/libstdc++-v3/doc/xml/manual/allocator.xml
@@ -138,7 +138,6 @@
 
 
   Interface 
Design
-
 

  The only allocator interface that
@@ -162,7 +161,6 @@
   
 
   Selecting Default 
Allocation Policy
-
 

  It's difficult to pick an allocation strategy that will provide
@@ -228,7 +226,6 @@
   
 
   Disabling Memory 
Caching
-
 
 
   In use, allocator may allocate and
@@ -328,8 +325,8 @@
 names have changed, but in all cases, functionality is
 equivalent. Starting with gcc-3.4, all extension allocators are
 standard style. Before this point, SGI style was the norm. Because of
-this, the number of template arguments also changed. Here's a simple
-chart to track the changes.
+this, the number of template arguments also changed.
+ tracks the changes.
   
 
   
@@ -468,7 +465,7 @@

 A high-performance fixed-size allocator with
 exponentially-increasing allocations. It has its own
-chapter 
+chapter
  in the documentation.

  
diff --git a/libstdc++-v3/doc/xml/manual/evolution.xml 
b/libstdc++-v3/doc/xml/manual/evolution.xml
index 3288e2f6cf2..e24418fefc0 100644
--- a/libstdc++-v3/doc/xml/manual/evolution.xml
+++ b/libstdc++-v3/doc/xml/manual/evolution.xml
@@ -13,7 +13,6 @@
 
 
 
-
 
 A list of user-visible changes, in chronological order
 
@@ -33,9 +32,9 @@ is added that notifies on inclusion 
(-Wno-deprecated
 deactivates the warning.)
 
 
-Deprecated include backward/strstream added.
+Deprecated include backward/strstream added.
 
-Removal of include builtinbuf.h, 
indstream.h, parsestream.h, PlotFile.h, SFile.h, stdiostream.h, and stream.h.
+Removal of include builtinbuf.h, indstream.h, parsestream.h, PlotFile.h, SFile.h, stdiostream.h, and stream.h.
 
 
 
@@ -51,7 +50,7 @@ deactivates the warning.)
 Extensions from SGI/HP moved from namespace std
 to namespace __gnu_cxx. As part of this, the following
 new includes are
-added: ext/algorithm, ext/functional, ext/iterator, ext/memory, and ext/numeric.
+added: ext/algorithm, 
ext/functional, ext/iterator, ext/memory, and ext/numeric.
 
 
 
@@ -59,11 +58,11 @@ Extensions to basic_filebuf introduced: 
__gnu_cxx::enc_filebu
 
 
 
-Extensions to tree data structures added in ext/rb_tree.
+Extensions to tree data structures added in ext/rb_tree.
 
 
 
-Removal of ext/tree, moved to 
backward/tree.h.
+Removal of ext/tree, moved to 
backward/tree.h.
 
 
 
@@ -74,7 +73,7 @@ Removal of ext/tree, 
moved to 
 Symbol versioning introduced for shared library.
 
-Removal of include backward/strstream.h.
+Removal of include backward/strstream.h.
 
 Allocator changes. Change __malloc_alloc to 
malloc_allocator and __new_alloc to 
new_allocator. 
 
@@ -109,7 +108,7 @@ Removal of ext/tree, moved to 
 
- Extensions for generic characters and char_traits added in 
ext/pod_char_traits.h.
+ Extensions for generic characters and char_traits added in 
ext/pod_char_traits.h.
 
 
 
@@ -123,7 +122,7 @@ Support for char_traits beyond builtin types.
 
 Conformant allocator class and usage in containers. As
 part of this, the following extensions are
-added: ext/bitmap_allocator.h, 
ext/debug_allocator.h, ext/mt_allocator.h, ext/malloc_allocator.h,ext/new_allocator.h, ext/pool_allocator.h.
+added: ext/bitmap_allocator.h, 
ext/debug_allocator.h, 
ext/mt_allocator.h, ext/malloc_allocator.h,ext/new_allocator.h, ext/pool_allocator.h.
 
 
 
@@ -186,37 +185,37 @@ _Alloc_traits have been removed.
   
   
 __gnu_cxx::new_allocatorT
-ext/new_allocator.h
+ext/new_allocator.h
 std::__new_alloc
-memory
+memory
   
   
 __gnu_cxx::malloc_allocatorT
-ext/malloc_allocator.h
+ext/malloc_allocator.h
 
std::__malloc_alloc_templateint
-memory
+memory
   
   
 __gnu_cxx::debug_allocatorT
-ext/debug_allocator.h
+ext/debug_allocator.h
 

Re: [PATCH] Fix PR71598, aliasing between enums and compatible types

[Michael Matz]

Hi,

On Tue, 19 Mar 2019, Richard Biener wrote:

> It doesn't really.  Consider the following IMHO valid testcase:
> 
> enum X { A, B };
> enum Y { C, D };
> void foo (int *p)
> {
>  *(enum X *)p = A;
>  *(enum Y *)p = D;
>  return *(enum X *)p;
> }
> 
> int main()
> {
>   int storage;
>   if (foo () != A)

(You want to require 'B' here, not 'A')

> abort ();
> }

If you want the above testcase to be valid then yes, I agree, you have to 
give all enums the same alias set as the underlying type.  Meh, I don't 
like it :-/


Ciao,
Michael.

Re: [PATCH 0/2] Future warnings not treated as errors

[Martin Liška]

On 3/19/19 3:46 AM, Alex Henrie wrote:
> Hello,
> 
> I have received permission to contribute to GCC, but I do not have
> commit access. Could one of you please merge these patches for me?

Hi.

I can do it for you. But if I see correctly, Joseph M. had some concerns
and he should probably make final patch approval.

Martin

> 
> -Alex
> 
> Alex Henrie (1):
>   PR c/65403 - Add tests for -Wno-error=
> 
> Manuel López-Ibáñez (1):
>   PR c/65403 - Ignore -Wno-error=
> 
>  gcc/opts-common.c  |  2 ++
>  gcc/opts-global.c  | 10 +++---
>  gcc/opts.c |  5 -
>  gcc/opts.h |  2 ++
>  gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
>  gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
>  6 files changed, 40 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c
> 

Re: Fix a case in which the vector cost model was ignored

[Richard Sandiford]

Bernhard Reutner-Fischer  writes:
> On 18 March 2019 10:58:53 CET, Richard Sandiford  
> wrote:
>>This patch fixes a case in which we vectorised something with a
>>fully-predicated loop even after the cost model had rejected it.
>>E.g. the loop in the testcase has the costs:
>>
>>  Vector inside of loop cost: 27
>>  Vector prologue cost: 0
>>  Vector epilogue cost: 0
>>  Scalar iteration cost: 7
>>  Scalar outside cost: 6
>>  Vector outside cost: 0
>>  prologue iterations: 0
>>  epilogue iterations: 0
>>
>>and we can see that the loop executes at most three times, but we
>>decided to vectorise it anyway.
>>
>>(The costs here are equal for three iterations, but the same thing
>>happens even when the vector code is strictly more expensive.)
>>
>>The problem is the handling of "/VF" in:
>>
>>  /* Calculate number of iterations required to make the vector version
>> profitable, relative to the loop bodies only.  The following condition
>> must hold true:
>> SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC
>> where
>> SIC = scalar iteration cost, VIC = vector iteration cost,
>> VOC = vector outside cost, VF = vectorization factor,
>> PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations
>> SOC = scalar outside cost for run time cost model check.  */
>>
>>We treat the "/VF" as truncating, but for fully-predicated loops, it's
>>closer to a ceil division, since fractional iterations are handled by a
>>full iteration with some predicate bits set to false.
>>
>>The easiest fix seemed to be to calculate the minimum number of vector
>>iterations first, then use that to calculate the minimum number of
>>scalar
>>iterations.
>>
>>Calculating the minimum number of vector iterations might make sense
>>for
>>unpredicated loops too, since calculating the scalar niters directly
>>doesn't take into account the fact that the VIC multiple has to be an
>>integer.  But the handling of PL_ITERS and EP_ITERS for unpredicated
>>loops is a bit hand-wavy anyway, so maybe vagueness here cancels out
>>vagueness there?
>>
>>Either way, changing this for unpredicated loops would be much too
>>invasive for stage 4, so the patch keeps it specific to
>>fully-predicated
>>loops (i.e. SVE) for now.  There's no functional change for other
>>targets.
>>
>>Tested on aarch64-linux-gnu with and without SVE, and on
>>x86_64-linux-gnu.
>>This is a regression introduced by the original cost model patches for
>>fully-predicated loops, so OK for GCC 9?
>>
>>Thanks,
>>Richard
>>
>>
>>2019-03-18  Richard Sandiford  
>>
>>gcc/
>>  * tree-vect-loop.c (vect_estimate_min_profitable_iters): Fix the
>>  calculation of the minimum number of scalar iterations for
>>  fully-predicated loops.
>>
>>gcc/testsuite/
>>  * gcc.target/aarch64/sve/cost_model_1.c: New test.
>>
>>Index: gcc/tree-vect-loop.c
>>===
>>--- gcc/tree-vect-loop.c  2019-03-08 18:15:33.668751875 +
>>+++ gcc/tree-vect-loop.c  2019-03-18 09:55:03.257194326 +
>>@@ -3600,14 +3600,89 @@ vect_estimate_min_profitable_iters (loop
>>  /* Calculate number of iterations required to make the vector version
>> profitable, relative to the loop bodies only.  The following condition
>>  must hold true:
>>- SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC
>>+ SIC * niters + SOC > VIC * ((niters - NPEEL) / VF) + VOC
>>  where
>>  SIC = scalar iteration cost, VIC = vector iteration cost,
>>  VOC = vector outside cost, VF = vectorization factor,
>>- PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations
>>+ NPEEL = prologue iterations + epilogue iterations,
>>  SOC = scalar outside cost for run time cost model check.  */
>> 
>>-  if ((scalar_single_iter_cost * assumed_vf) > (int) vec_inside_cost)
>>+  int saving_per_viter = (scalar_single_iter_cost * assumed_vf
>>+   - vec_inside_cost);
>>+  if (saving_per_viter <= 0)
>>+{
>>+  if (LOOP_VINFO_LOOP (loop_vinfo)->force_vectorize)
>>+ warning_at (vect_location.get_location_t (), OPT_Wopenmp_simd,
>>+ "vectorization did not happen for a simd loop");
>>+
>>+  if (dump_enabled_p ())
>>+dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>>+  "cost model: the vector iteration cost = %d "
>>+  "divided by the scalar iteration cost = %d "
>>+  "is greater or equal to the vectorization factor = %d"
>>+ ".\n",
>>+  vec_inside_cost, scalar_single_iter_cost, assumed_vf);
>>+  *ret_min_profitable_niters = -1;
>>+  *ret_min_profitable_estimate = -1;
>>+  return;
>>+}
>>+
>>+  /* ??? The "if" arm is written to handle all cases; see below for
>>what
>>+ we would do for !LOOP_VINFO_FULLY_MASKED_P.  */
>>+  if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo))
>>+{
>
> The condition above seems to 

[PATCH] elf.c: initialize st_mode member

[mingli.yu]

From: Mingli Yu 

Initialize st_mode member to fix the below
build failure when -Og included in compiler flag.
| 
./../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:
 In function 'elf_is_symlink':
| 
../../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21:
 error: 'st.st_mode' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
   return S_ISLNK (st.st_mode);

Signed-off-by: Mingli Yu 
---
 libbacktrace/elf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index f4863f0..561bdc2 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -766,6 +766,7 @@ static int
 elf_is_symlink (const char *filename)
 {
   struct stat st;
+  st.st_mode = 0;
 
   if (lstat (filename, ) < 0)
 return 0;
-- 
2.7.4

Re: [PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)

[Richard Biener]

On Mon, Mar 18, 2019 at 10:31 PM Jeff Law  wrote:
>
> On 3/18/19 1:03 PM, Martin Sebor wrote:
> > I the -Warray-bounds enhancement committed at the beginning
> > of the GCC 9 window I tried to correctly handle offsets in
> > MEM_REFs in the [max, min] kind of a range after converting
> > from sizetype to ptrdiff_type, but I did it wrong.  The bug
> > results in false positives in some unusual use cases that
> > I didn't consider at the time.
> >
> > The attached patch removes this incorrect handling and uses
> > the conservative anti-range handling for these cases instead.
> >
> > Martin
> >
> > PS Is there some technical reason why pointer offsets are
> > represented as the unsigned sizetype when they can be signed?
> I'm not aware of a conscious decision to treat them as unsigned or a
> particular target need to do so.  It's most likely a historical accident.

That historical accident included treating those unsigned types as
sign-extending ...

But yes, changing sizetype to ssizetype wherever we use it in
offset context would be a cleanup I guess.  But IIRC Bin tried
this and the fallout is (as usual) non-trivial to fix...

Richard.

>
> >
> > gcc-89720.diff
> >
> > PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range 
> > with max < min
> >
> > gcc/ChangeLog:
> >
> >   PR tree-optimization/89720
> >   * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min
> >   more conservatively, the same as anti-range.
> >
> > gcc/testsuite/ChangeLog:
> >
> >   PR tree-optimization/89720
> >   * gcc.dg/Warray-bounds-42.c: New test.
> OK
> jeff

Re: [PATCH] Fix PR71598, aliasing between enums and compatible types

[Richard Biener]

On Mon, 18 Mar 2019, Michael Matz wrote:

> Hi,
> 
> On Mon, 18 Mar 2019, Richard Biener wrote:
> 
> > > Or, because an enum with these properties could be modelled as a struct 
> > > containing one member of basetype you could also change 
> > > record_component_aliases(), though that doesn't allow language specific 
> > > behaviour differences.
> > 
> > No, you can't.  I believe that enum X and int being compatible means
> > that accessing an enum X object via an lvalue of effective type int
> > is OK _and_ accessing an int object via an lvalue of effective type
> > enum X is OK.
> 
> I agree.  But the objects type still remains whatever it was: either an 
> enum or an int; type equivalence/identity isn't affected by compatiblity.
> 
> > That commutativity doesn't hold for struct X { int i; }
> > vs. int i and those types are not compatible.
> 
> True, but that's not the important aspect that the aliasing subsetting 
> expresses: if A subsets B or B subsets A, all accesses between an A and a 
> B conflict.  If A is the "struct S1 {int}" and B is "int" that's exactly 
> right, an access to the struct and one to an int (_access_, where you 
> don't necessarily know the declared type) conflict.  But if we have a C, 
> an "struct S2 {int}" the direction of the subsetting starts to matter:
> A conflict B, and C conflict B, but !(A conflict C).  This is exactly the 
> situation with A and C being different enums as well.
> 
> The non-transitivity of the 'conflicts' relation is expressed by having a 
> direction in the alias subset relation: everything that is (or contains) 
> and A conflicts with everything that is (or contains) an int, everything 
> that is (or contains) a C conflicts with int, but nothing that is (or 
> contains) an A conflicts with anything that is (or contains) a C (if not 
> for other reasons).
> 
> > At least unless Joseph corrects me here and that "type X is compatible
> > with type Y" doesn't imply "type Y is compatible with type X"
> > (that's not transitivity).
> 
> That's not transitivity but symmetry, and that of course holds for 
> "compatible".  X compat Y, and Z compat Y --> X compat Z would be 
> transitivity and doesn't hold.
> 
> > Now, we can't currently model precisely this way of non-transitivity
> > of C's notion of "compatibility".
> > 
> >  enum X { A };
> >  enum Y { B };
> >  void *ptr = malloc (4);
> >  *(enum X *)ptr = A; // dynamic type is now X
> >  foo (*(enum Y *)ptr); // undefined, X and Y are not compatible(?)
> >  foo (*(int *)ptr); // OK, X and int are compatible
> >  *(int *)ptr = *(enum X *); // dynamic type is now int
> >  foo (*(enum Y *)ptr); // OK(?), Y and int are compatible
> 
> I'm pretty sure this can be expressed, in the way I suggested, making X 
> subset int, Y subset int (or superset?  As said, I'm always confused), and 
> those above would work.

It doesn't really.  Consider the following IMHO valid testcase:

enum X { A, B };
enum Y { C, D };
void foo (int *p)
{
 *(enum X *)p = A;
 *(enum Y *)p = D;
 return *(enum X *)p;
}

int main()
{
  int storage;
  if (foo () != A)
abort ();
}

where in C a store doesn't change the dynamic type of an object
with a declared type so the dynamic type stays 'int' always and
thus both stores happen via compatible types.  But the middle-end
just sees stores via type X and Y which are not C-compatible.
The middle-end doesn't know anything about those stores not updating
the dynamic type which means any valid lvalue access according to
6.5/7 may not change the alias-set.

So as I already said, our implementation cannot express this
non-transitive compatibility in a safe manner.

Richard.

[PATCH] Fix set of even probabilities (PR middle-end/89737).

[Martin Liška]

Hi.

When calling set_even_probabilities, the function assumes that an edge
can't live in both sets ({un,}likely_edges). When such situation happens,
clear just the sets.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-03-18  Martin Liska  

PR middle-end/89737
* predict.c (combine_predictions_for_bb): Empty likely_edges and
unlikely_edges if there's an edge that belongs to both these sets.

gcc/testsuite/ChangeLog:

2019-03-18  Martin Liska  

PR middle-end/89737
* gcc.dg/pr89737.c: New test.
---
 gcc/predict.c  | 17 ++---
 gcc/testsuite/gcc.dg/pr89737.c | 17 +
 2 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr89737.c


diff --git a/gcc/predict.c b/gcc/predict.c
index 43ee91a5b13..60a19d7edd1 100644
--- a/gcc/predict.c
+++ b/gcc/predict.c
@@ -1229,12 +1229,23 @@ combine_predictions_for_bb (basic_block bb, bool dry_run)
 	if (pred->ep_probability <= PROB_VERY_UNLIKELY
 		|| pred->ep_predictor == PRED_COLD_LABEL)
 	  unlikely_edges.add (pred->ep_edge);
-	if (pred->ep_probability >= PROB_VERY_LIKELY
-		|| pred->ep_predictor == PRED_BUILTIN_EXPECT
-		|| pred->ep_predictor == PRED_HOT_LABEL)
+	else if (pred->ep_probability >= PROB_VERY_LIKELY
+		 || pred->ep_predictor == PRED_BUILTIN_EXPECT
+		 || pred->ep_predictor == PRED_HOT_LABEL)
 	  likely_edges.add (pred);
 	  }
 
+  /* It can happen that an edge is both in likely_edges and unlikely_edges.
+	 Clear both sets in that situation.  */
+  for (hash_set::iterator it = likely_edges.begin ();
+	   it != likely_edges.end (); ++it)
+	if (unlikely_edges.contains ((*it)->ep_edge))
+	  {
+	likely_edges.empty ();
+	unlikely_edges.empty ();
+	break;
+	  }
+
   if (!dry_run)
 	set_even_probabilities (bb, _edges, _edges);
   clear_bb_predictions (bb);
diff --git a/gcc/testsuite/gcc.dg/pr89737.c b/gcc/testsuite/gcc.dg/pr89737.c
new file mode 100644
index 000..cd3dc81769e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr89737.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+int a, b;
+
+void c() {
+  &
+  void *e = &, *g = &
+f:
+  __attribute__((hot)) h : __attribute__((cold)) for (; a;) goto *g;
+d:
+  for (; b;)
+goto *e;
+}
+
+/* { dg-final { scan-tree-dump-times "predicted to even probabilities" 4 "profile_estimate"} } */
+

Re: Fix two ICEs with C++ destructors and LTO

[Richard Biener]

On Mon, 18 Mar 2019, Jan Hubicka wrote:

> Hi,
> these two PRs are about C++ destructors that are not virtual but have
> virtual alias. The devirtualization machinery walks from alias to symbol
> and is then confused by not knowing the class symbol belongs to.
> 
> I think it would make more sense to avoid these walks but that require
> more of surgery, so for GCC9 I think it is best to stop freeing contexts
> for desctructors. This does not save any stremaing because THIS pointer
> of the destructor has the same type.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Richard.

> Honza
> 
>   PR lto/87809
>   PR lto/89335
>   * tree.c (free_lang_data_in_decl): Do not free context of C++
>   destrutors.
> 
>   * g++.dg/lto/pr87089_0.C: New testcase.
>   * g++.dg/lto/pr87089_1.C: New testcase.
>   * g++.dg/lto/pr89335_0.C: New testcase.
> Index: tree.c
> ===
> --- tree.c(revision 269704)
> +++ tree.c(working copy)
> @@ -5772,10 +5772,16 @@ free_lang_data_in_decl (tree decl, struc
>   not do well with TREE_CHAIN pointers linking them.
>  
>   Also do not drop containing types for virtual methods and tables because
> - these are needed by devirtualization.  */
> + these are needed by devirtualization.
> + C++ destructors are special because C++ frontends sometimes produces
> + virtual destructor as an alias of non-virtual destructor.  In
> + devirutalization code we always walk through aliases and we need
> + context to be preserved too.  See PR89335  */
>if (TREE_CODE (decl) != FIELD_DECL
>&& ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
> -  || !DECL_VIRTUAL_P (decl)))
> +  || (!DECL_VIRTUAL_P (decl)
> +   && (TREE_CODE (decl) != FUNCTION_DECL
> +   || !DECL_CXX_DESTRUCTOR_P (decl)
>  DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl));
>  }
>  
> Index: testsuite/g++.dg/lto/pr87089_0.C
> ===
> --- testsuite/g++.dg/lto/pr87089_0.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr87089_0.C  (working copy)
> @@ -0,0 +1,21 @@
> +// { dg-lto-do link }
> +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
> +namespace itpp {
> +template  void b(a *c) { c[0].~a(); }
> +class CFix;
> +template  class d {
> +  void e(const char *);
> +  CFix *data;
> +};
> +class CFix {
> +public:
> +  virtual ~CFix();
> +};
> +template <> void d::e(const char *) { b(data); }
> +} // namespace itpp
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> Index: testsuite/g++.dg/lto/pr87089_1.C
> ===
> --- testsuite/g++.dg/lto/pr87089_1.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr87089_1.C  (working copy)
> @@ -0,0 +1,12 @@
> +namespace itpp {
> +enum a { b };
> +class CFix {
> +public:
> +  virtual ~CFix();
> +};
> +template  class c : CFix {
> +  ~c() {}
> +};
> +template class c<>;
> +} // namespace itpp
> +
> Index: testsuite/g++.dg/lto/pr89335_0.C
> ===
> --- testsuite/g++.dg/lto/pr89335_0.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr89335_0.C  (working copy)
> @@ -0,0 +1,16 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-O2 -flto -Wsuggest-final-methods}} }
> +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
> +class Container
> +{
> +public:
> +  virtual ~Container ();
> +};
> +class List : public Container // { dg-lto-message "final would enable 
> devirtualization" }
> +{
> +};
> +static List cache[256];
> +int main (void)
> +{
> +  return 0;
> +}
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH] Fix up handling of "+rm" with TREE_ADDRESSABLE types (PR target/89752)

[Richard Biener]

On Mon, 18 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> For input arguments, we do:
>   /* If we can't make copies, we can only accept memory.  */
>   if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link
> {
>   if (allows_mem)
> allows_reg = 0;
>   else
> {
>   error ("impossible constraint in %");
>   error ("non-memory input %d must stay in memory", i);
>   return GS_ERROR;
> }
> }
> The following patch does the same thing for output operands as well.
> For the cases where !allows_mem, that will just result in one extra error
> explaining what's going on (previously one would get the
> impossible constraint in % error during vregs pass, now it gets
> during gimplification + the extra error too), and if allows_mem,
> it will for the + case make sure we get "=rm" (x) ... : "rm" (x) instead
> of "=rm" (x) ... : "0" (x) that LRA ICEs on.
> 
> While the LRA ICE needs to be fixed in any case (one can still reproduce it
> with
>   __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
> ), this patch opens the possibility to accept "+rm" (a0) and reject
> "=rm" (a0) ... "0" (a0) if reload can't prove it is the same address.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-03-18  Jakub Jelinek  
> 
>   PR target/89752
>   * gimplify.c (gimplify_asm_expr): For output argument with
>   TREE_ADDRESSABLE type, clear allows_reg if it allows memory, otherwise
>   diagnose error.
> 
>   * g++.dg/ext/asm15.C: Check for particular diagnostic wording.
>   * g++.dg/ext/asm16.C: Likewise.
>   * g++.dg/ext/asm17.C: New test.
> 
> --- gcc/gimplify.c.jj 2019-03-07 20:45:39.168938360 +0100
> +++ gcc/gimplify.c2019-03-18 16:18:16.515466234 +0100
> @@ -6155,6 +6155,19 @@ gimplify_asm_expr (tree *expr_p, gimple_
> is_inout = false;
>   }
>  
> +  /* If we can't make copies, we can only accept memory.  */
> +  if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link
> + {
> +   if (allows_mem)
> + allows_reg = 0;
> +   else
> + {
> +   error ("impossible constraint in %");
> +   error ("non-memory output %d must stay in memory", i);
> +   return GS_ERROR;
> + }
> + }
> +
>if (!allows_reg && allows_mem)
>   mark_addressable (TREE_VALUE (link));
>  
> --- gcc/testsuite/g++.dg/ext/asm15.C.jj   2018-05-06 23:13:33.252652046 
> +0200
> +++ gcc/testsuite/g++.dg/ext/asm15.C  2019-03-18 18:00:48.907456236 +0100
> @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s; };
>  void
>  foo (S )
>  {
> -  __asm volatile ("" : "+r" (s) : : "memory");   // { dg-error "" }
> +  __asm volatile ("" : "+r" (s) : : "memory");   // { dg-error 
> "impossible constraint" }
> + // { dg-error "must stay in 
> memory" "" { target *-*-* } .-1 }
>  }
> --- gcc/testsuite/g++.dg/ext/asm16.C.jj   2018-05-06 23:13:33.252652046 
> +0200
> +++ gcc/testsuite/g++.dg/ext/asm16.C  2019-03-18 18:00:35.978664187 +0100
> @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s[64]; } s;
>  void
>  foo ()
>  {
> -  __asm volatile ("" : "=r" (s) : : "memory");   // { dg-error "" }
> +  __asm volatile ("" : "=r" (s) : : "memory");   // { dg-error 
> "impossible constraint" }
> + // { dg-error "must stay in 
> memory" "" { target *-*-* } .-1 }
>  }
> --- gcc/testsuite/g++.dg/ext/asm17.C.jj   2019-03-18 17:57:44.409424473 
> +0100
> +++ gcc/testsuite/g++.dg/ext/asm17.C  2019-03-18 17:58:32.414651932 +0100
> @@ -0,0 +1,11 @@
> +// PR target/89752
> +// { dg-do compile }
> +
> +struct A { A (); ~A (); short c; };
> +
> +void
> +foo ()
> +{
> +  A a0, a1;
> +  __asm volatile ("" : "+rm" (a0), "+rm" (a1));
> +}
> 
>   Jakub
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Patch ping^2 (Re: [PATCH] ARM cmpsi2_addneg fix follow-up (PR target/89506))

[Jakub Jelinek]

On Tue, Mar 12, 2019 at 12:43:29PM +0100, Jakub Jelinek wrote:
> On Mon, Mar 04, 2019 at 10:04:01AM +0100, Jakub Jelinek wrote:
> > The first one uses constraints and no C code in the output, I believe it is
> > actually more expensive for compile time, because if one just reads what
> > constrain_operands needs to do for another constraint, it is quite a lot.
> > I've tried to at least not introduce new constraints for this, there is no
> > constraint for number 1 (or for number -1).
> > The Pu constraint is thumb2 only for numbers 1..8, and the alternative uses
> > I constraint for the negation of it, i.e. -8..-1, only -1 from this is
> > valid for I.  If that matches, we emit adds with #1, otherwise just prefer
> > subs over adds.
> > 
> > The other swaps the alternatives similarly to the above, but for the special
> > case of desirable adds with #1 uses C code instead of another alternative.
> > 
> > Ok for trunk (which one)?
> 
> I'd like to ping these patches:
> 
> > 2019-03-04  Jakub Jelinek  
> > 
> > PR target/89506
> > * config/arm/arm.md (cmpsi2_addneg): Swap the alternatives, add
> > another alternative with I constraint for operands[2] and Pu
> > for operands[3] and emit adds in that case, don't use C code to
> > emit the instruction.
> 
> or:
> 
> > 2019-03-04  Jakub Jelinek  
> > 
> > PR target/89506
> > * config/arm/arm.md (cmpsi2_addneg): Swap the alternatives and use
> > subs for the first alternative except when operands[3] is 1.

Ping.

Jakub

Re: [PATCH] Fix i?86 -mfpmath=sse ceil inline expansion (PR target/89726)

[Uros Bizjak]

On Fri, Mar 15, 2019 at 9:46 PM Jakub Jelinek  wrote:
>
> Hi!
>
> As the enhanced testcase shows, ix86_expand_floorceildf_32 doesn't emit
> correct ceil when honoring signed zeros, because it performs copysign,
> then comparison and then based on the comparison an optional addition of 1.
> The comment claims that it is essential to use x2 -= -1.0 instead of
> x2 += 1.0, but in reality we emit x2 += 1.0 anyway and it makes no
> difference on the sign of result if it is zero, as (unless rounding to
> negative infinity) both -1.0 - (-1.0) and -1.0 + 1.0 evaluate to +0.0
> rather than -0.0 we need in this case.
>
> I have no better ideas than to use another copysign, but it should be just
> one extra instruction, as we've already previously done copysign from the
> same source and thus hve already computed the x & signmask and so this patch
> just adds a {,v}por of that previously computed x & signmask into the result
> again.  In this case we aren't really copying it always to positive, but
> all we want is handle the single problematic case when we compute positive 0
> and need negative 0, for all others the oring won't really change the sign.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or does anybody have some smarter idea?
>
> Note, not sure if floor isn't incorrect in the round to negative infinity
> case (but that would be preexisting issue).
>
> 2019-03-15  Jakub Jelinek  
>
> PR target/89726
> * config/i386/i386.c (ix86_expand_floorceildf_32): In ceil
> compensation use x2 += 1 instead of x2 -= -1 and when honoring
> signed zeros, do another copysign after the compensation.
>
> * gcc.target/i386/fpprec-1.c (x): Add 6 new constants.
> (expect_round, expect_rint, expect_floor, expect_ceil, expect_trunc):
> Add expected results for them.

LGTM.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2019-03-14 23:44:27.862560139 +0100
> +++ gcc/config/i386/i386.c  2019-03-15 15:07:54.607453430 +0100
> @@ -45563,8 +45563,10 @@ ix86_expand_floorceildf_32 (rtx operand0
>x2 -= 1;
>   Compensate.  Ceil:
>  if (x2 < x)
> -  x2 -= -1;
> -return x2;
> +  x2 += 1;
> +   if (HONOR_SIGNED_ZEROS (mode))
> + x2 = copysign (x2, x);
> +   return x2;
> */
>machine_mode mode = GET_MODE (operand0);
>rtx xa, TWO52, tmp, one, res, mask;
> @@ -45590,17 +45592,16 @@ ix86_expand_floorceildf_32 (rtx operand0
>/* xa = copysign (xa, operand1) */
>ix86_sse_copysign_to_positive (xa, xa, res, mask);
>
> -  /* generate 1.0 or -1.0 */
> -  one = force_reg (mode,
> -  const_double_from_real_value (do_floor
> -? dconst1 : dconstm1, mode));
> +  /* generate 1.0 */
> +  one = force_reg (mode, const_double_from_real_value (dconst1, mode));
>
>/* Compensate: xa = xa - (xa > operand1 ? 1 : 0) */
>tmp = ix86_expand_sse_compare_mask (UNGT, xa, res, !do_floor);
>emit_insn (gen_rtx_SET (tmp, gen_rtx_AND (mode, one, tmp)));
> -  /* We always need to subtract here to preserve signed zero.  */
> -  tmp = expand_simple_binop (mode, MINUS,
> +  tmp = expand_simple_binop (mode, do_floor ? MINUS : PLUS,
>  xa, tmp, NULL_RTX, 0, OPTAB_DIRECT);
> +  if (!do_floor && HONOR_SIGNED_ZEROS (mode))
> +ix86_sse_copysign_to_positive (tmp, tmp, res, mask);
>emit_move_insn (res, tmp);
>
>emit_label (label);
> --- gcc/testsuite/gcc.target/i386/fpprec-1.c.jj 2010-05-21 11:46:22.0 
> +0200
> +++ gcc/testsuite/gcc.target/i386/fpprec-1.c2019-03-15 15:01:51.430345113 
> +0100
> @@ -11,6 +11,9 @@ double x[] = { __builtin_nan(""), __buil
> 0x1.1p-1, 0x1.fp-2,
> 0x1.1p+0, 0x1.fp-1,
> 0x1.80001p+0, 0x1.7p+0,
> +   -0x1.1p-1, -0x1.fp-2,
> +   -0x1.1p+0, -0x1.fp-1,
> +   -0x1.80001p+0, -0x1.7p+0,
> -0.0, 0.0, -0.5, 0.5, -1.0, 1.0, -1.5, 1.5, -2.0, 2.0,
> -2.5, 2.5 };
>  #define NUM (sizeof(x)/sizeof(double))
> @@ -19,6 +22,7 @@ double expect_round[] = { __builtin_nan(
> -0x1.fp+1023, 0x1.fp+1023,
> -0.0, 0.0,
> 1.0, 0.0, 1.0, 1.0, 2.0, 1.0,
> +   -1.0, -0.0, -1.0, -1.0, -2.0, -1.0,
> -0.0, 0.0, -1.0, 1.0, -1.0, 1.0, -2.0, 2.0, -2.0, 2.0,
> -3.0, 3.0 };
>
> @@ -26,6 +30,7 @@ double expect_rint[] = { __builtin_nan("
>  -0x1.fp+1023, 0x1.fp+1023,
>  -0.0, 0.0,
>  1.0, 0.0, 1.0, 1.0, 2.0, 1.0,
> +-1.0, -0.0, -1.0, -1.0, -2.0, -1.0,
>  -0.0, 0.0, -0.0, 0.0, -1.0, 1.0, -2.0, 2.0, -2.0, 2.0,
>  -2.0, 2.0 };
>
> @@ -33,6 +38,7 @@ double expect_floor[] = { __builtin_nan(
>  -0x1.fp+1023, 0x1.fp+1023,
>  -1.0, 0.0,
>  0.0,