[PATCH] middle-end: Parity folding optimizations.

2020-06-11 Thread Roger Sayle
 

This patch implements several constant folding optimizations

for __builtin_parity and friends.  We canonicalize popcount(x)&1

as parity(x) in gimple, and potentially convert back again when

we expand to RTL.  parity(~x) is simplified to parity(x), which

is true for all integer modes with an even number of bits.

But probably most usefully, parity(x)^parity(y) can be simplified

to a parity(x^y), requiring only a single libcall or popcount.

This idiom is seen in PR target/44481 and elsewhere in the Linux

kernel.

 

This patch has been tested with "make bootstrap" and "make -k check" on

x86_64-pc-linux-gnu with no regressions.  If approved, I'd very much

appreciate it if someone could commit this change for me.

 

 

2020-06-12  Roger Sayle  

 

* match.pd (popcount(x)&1 -> parity(x)): New simplification.

(parity(~x) -> parity(x)): New simplification.

(parity(x&1) -> x&1): New simplification.

(parity(x)^parity(y) -> parity(x^y)): New simplification.

 

2020-06-12  Roger Sayle  

 

* gcc.dg/fold-parity-1.c: New test.

* gcc.dg/fold-parity-2.c: Likewise.

* gcc.dg/fold-parity-3.c: Likewise.

* gcc.dg/fold-parity-4.c: Likewise.

 

 

Thanks in advance,

Roger

--

Roger Sayle

NextMove Software

Cambridge, UK

 

diff --git a/gcc/match.pd b/gcc/match.pd
index 2b8c37e..ddb0650 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5949,6 +5949,32 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (cmp (popcount @0) integer_zerop)
   (rep @0 { build_zero_cst (TREE_TYPE (@0)); }
 
+/* Canonicalize POPCOUNT(x)&1 as PARITY(X).  */
+(for popcount (BUILT_IN_POPCOUNT BUILT_IN_POPCOUNTL BUILT_IN_POPCOUNTLL
+   BUILT_IN_POPCOUNTIMAX)
+ parity (BUILT_IN_PARITY BUILT_IN_PARITYL BUILT_IN_PARITYLL
+BUILT_IN_PARITYIMAX)
+  (simplify
+(bit_and (popcount @0) integer_onep)
+(parity @0)))
+
+/* PARITY simplifications.  */
+(for parity (BUILT_IN_PARITY BUILT_IN_PARITYL BUILT_IN_PARITYLL
+BUILT_IN_PARITYIMAX)
+  /* parity(~X) is parity(X).  */
+  (simplify
+(parity (bit_not @0))
+(parity @0))
+  /* parity(X&1) is nop_expr(X&1).  */
+  (simplify
+(parity @0)
+(if (tree_nonzero_bits (@0) == 1)
+  (convert @0)))
+  /* parity(X)^parity(Y) is parity(X^Y).  */
+  (simplify
+(bit_xor (parity:s @0) (parity:s @1))
+(parity (bit_xor @0 @1
+
 #if GIMPLE
 /* 64- and 32-bits branchless implementations of popcount are detected:
 
/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-original" } */

int foo(unsigned int x)
{
  return __builtin_popcount(x) & 1;
}

int fool(unsigned long x)
{
  return __builtin_popcountl(x) & 1;
}

int fooll(unsigned long long x)
{
  return __builtin_popcountll(x) & 1;
}

/* { dg-final { scan-tree-dump-times "__builtin_popcount" 0 "original" } } */
/* { dg-final { scan-tree-dump-times "__builtin_parity" 3 "original" } } */

/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

int foo(unsigned int x)
{
  return __builtin_parity(~x);
}

int fool(unsigned long x)
{
  return __builtin_parityl(~x);
}

int fooll(unsigned long long x)
{
  return __builtin_parityll(~x);
}

/* { dg-final { scan-tree-dump-times "~" 0 "optimized" } } */

/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

int foo(unsigned int x)
{
  return __builtin_parity(x&1);
}

int fool(unsigned long x)
{
  return __builtin_parityl(x&1);
}

int fooll(unsigned long long x)
{
  return __builtin_parityll(x&1);
}

/* { dg-final { scan-tree-dump-times "__builtin_parity" 0 "optimized" } } */

/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-optimized" } */

int foo(unsigned int x, unsigned int y)
{
  return __builtin_parity(x) ^ __builtin_parity(y);
}

int fool(unsigned long x, unsigned long y)
{
  return __builtin_parityl(x) ^ __builtin_parityl(y);
}

int fooll(unsigned long long x, unsigned long long y)
{
  return __builtin_parityll(x) ^ __builtin_parityll(y);
}

/* { dg-final { scan-tree-dump-times "__builtin_parity" 3 "optimized" } } */



[PATCH, RS6000 PR target/94954] Fix wrong codegen for vec_pack_to_short_fp32() builtin

2020-06-11 Thread will schmidt via Gcc-patches


Hi,
  Fix codegen implementation for the builtin vec_pack_to_short_fp32.

  Regtests are underway against powerpc64 (power7be,power8le,power9le).
  (this is a power9 builtin, so should be a noop for most of those).
  OK for trunk and backports?

  Thanks
  -Will


[gcc]
  target pr/94954

* config/rs6000/altivec.h (vec_pack_to_short_fp32):  Update.
* config/rs6000/altivec.md (UNSPEC_CONVERT_4F32_8F16):  New unspec.
(convert_4f32_8f16):  New define_expand.
* config/rs6000/rs6000-builtin.def (convert_4f32_8f16): New builtin 
define
and overload.
* config/rs6000/rs6000-call.c (P9V_BUILTIN_VEC_CONVERT_4F32_8F16): New
overloaded builtin entry.
* config/rs6000/vsx.md (UNSPEC_VSX_XVCVSPHP): New unspec.
(vsx_xvcvsphp): New define_insn.

[testsuite]
* testsuite/gcc.target/powerpc/builtins-1-p9-runnable.c: Update.

diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index 0a7e8ab..ab10025 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -431,11 +431,11 @@
 /* Vector additions added in ISA 3.0.  */
 #define vec_first_match_index __builtin_vec_first_match_index
 #define vec_first_match_or_eos_index __builtin_vec_first_match_or_eos_index
 #define vec_first_mismatch_index __builtin_vec_first_mismatch_index
 #define vec_first_mismatch_or_eos_index 
__builtin_vec_first_mismatch_or_eos_index
-#define vec_pack_to_short_fp32 __builtin_vec_convert_4f32_8i16
+#define vec_pack_to_short_fp32 __builtin_vec_convert_4f32_8f16
 #define vec_parity_lsbb __builtin_vec_vparity_lsbb
 #define vec_vctz __builtin_vec_vctz
 #define vec_cnttz __builtin_vec_vctz
 #define vec_vctzb __builtin_vec_vctzb
 #define vec_vctzd __builtin_vec_vctzd
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 159f24e..855e7cc 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -78,10 +78,11 @@
UNSPEC_VUNPACK_HI_SIGN_DIRECT
UNSPEC_VUNPACK_LO_SIGN_DIRECT
UNSPEC_VUPKHPX
UNSPEC_VUPKLPX
UNSPEC_CONVERT_4F32_8I16
+   UNSPEC_CONVERT_4F32_8F16
UNSPEC_DST
UNSPEC_DSTT
UNSPEC_DSTST
UNSPEC_DSTSTT
UNSPEC_LVSL
@@ -3215,10 +3216,32 @@
   emit_insn (gen_altivec_vctuxs (rtx_tmp_lo, operands[2], const0_rtx));
   emit_insn (gen_altivec_vpkswss (operands[0], rtx_tmp_hi, rtx_tmp_lo));
   DONE;
 })
 
+
+;; Convert two vector F32 to packed vector f16.
+(define_expand "convert_4f32_8f16"
+  [(set (match_operand:V8HI 0 "register_operand" "=v")
+   (unspec:V8HI [(match_operand:V4SF 1 "register_operand" "v")
+ (match_operand:V4SF 2 "register_operand" "v")]
+UNSPEC_CONVERT_4F32_8F16))]
+  "TARGET_P9_VECTOR"
+{
+  rtx rtx_tmp_hi = gen_reg_rtx (V4SImode);
+  rtx rtx_tmp_lo = gen_reg_rtx (V4SImode);
+
+  emit_insn (gen_vsx_xvcvsphp (rtx_tmp_hi, operands[1] ));
+  emit_insn (gen_vsx_xvcvsphp (rtx_tmp_lo, operands[2] ));
+  if (BYTES_BIG_ENDIAN)
+emit_insn (gen_altivec_vpkuwum (operands[0], rtx_tmp_lo, rtx_tmp_hi));
+  else
+emit_insn (gen_altivec_vpkuwum (operands[0], rtx_tmp_hi, rtx_tmp_lo));
+  DONE;
+})
+
+
 ;; Generate
 ;;xxlxor/vxor SCRATCH0,SCRATCH0,SCRATCH0
 ;;vsubu?m SCRATCH2,SCRATCH1,%1
 ;;vmaxs? %0,%1,SCRATCH2"
 (define_expand "abs2"
diff --git a/gcc/config/rs6000/rs6000-builtin.def 
b/gcc/config/rs6000/rs6000-builtin.def
index 8b1ddb0..47e9137 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2208,10 +2208,11 @@ BU_P8V_OVERLOAD_3 (VPERMXOR,   "vpermxor")
 
 /* ISA 3.0 vector overloaded 2-argument functions. */
 BU_P9V_AV_2 (VSLV, "vslv", CONST, vslv)
 BU_P9V_AV_2 (VSRV, "vsrv", CONST, vsrv)
 BU_P9V_AV_2 (CONVERT_4F32_8I16, "convert_4f32_8i16", CONST, convert_4f32_8i16)
+BU_P9V_AV_2 (CONVERT_4F32_8F16, "convert_4f32_8f16", CONST, convert_4f32_8f16)
 
 BU_P9V_AV_2 (VFIRSTMATCHINDEX_V16QI, "first_match_index_v16qi",
 CONST, first_match_index_v16qi)
 BU_P9V_AV_2 (VFIRSTMATCHINDEX_V8HI, "first_match_index_v8hi",
 CONST, first_match_index_v8hi)
@@ -2238,10 +2239,11 @@ BU_P9V_AV_2 (VFIRSTMISMATCHOREOSINDEX_V4SI, 
"first_mismatch_or_eos_index_v4si",
 
 /* ISA 3.0 vector overloaded 2-argument functions. */
 BU_P9V_OVERLOAD_2 (VSLV,   "vslv")
 BU_P9V_OVERLOAD_2 (VSRV,   "vsrv")
 BU_P9V_OVERLOAD_2 (CONVERT_4F32_8I16, "convert_4f32_8i16")
+BU_P9V_OVERLOAD_2 (CONVERT_4F32_8F16, "convert_4f32_8f16")
 
 /* 2 argument vector functions added in ISA 3.0 (power9). */
 BU_P9V_AV_2 (VADUB,"vadub",CONST,  vaduv16qi3)
 BU_P9V_AV_2 (VADUH,"vaduh",CONST,  vaduv8hi3)
 BU_P9V_AV_2 (VADUW,"vaduw",CONST,  vaduv4si3)
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 0ac8054..9708d7e 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ 

Re: [PATCH] Optimize multiplication for V8QI,V16QI,V32QI under TARGET_AVX512BW [target/95488]

2020-06-11 Thread Jeff Law via Gcc-patches
On Fri, 2020-06-05 at 13:46 +0800, Hongtao Liu via Gcc-patches wrote:
> Hi:
> 
> +/* Optimize vector MUL generation for V8QI, V16QI and V32QI
> +   under TARGET_AVX512BW. i.e. for v16qi a * b, it has
> +
> +   vpmovzxbw ymm2, xmm0
> +   vpmovzxbw ymm3, xmm1
> +   vpmullw   ymm4, ymm2, ymm3
> +   vpmovwb   xmm0, ymm4
> +
> +   it would take less instructions than ix86_expand_vecop_qihi.
> +   Return true if success.  */
> 
>   Bootstrap is ok, regression test on i386/x86-64 backend is ok.
> 
> gcc/ChangeLog:
> PR target/95488
> * config/i386/i386-expand.c (ix86_expand_vecmul_qihi): New
> function.
> * config/i386/i386-protos.h (ix86_expand_vecmul_qihi): Declare.
> * config/i386/sse.md (mul3): Drop mask_name since
> there's no real vector char multiplication instruction with
> mask. Also optimize it under TARGET_AVX512BW.
> (mulv8qi3): New expander.
> 
> gcc/testsuite/ChangeLog:
> * gcc.target/i386/avx512bw-pr95488-1.c: New test.
> * gcc.target/i386/avx512bw-pr95488-2.c: Ditto.
> * gcc.target/i386/avx512vl-pr95488-1.c: Ditto.
> * gcc.target/i386/avx512vl-pr95488-2.c: Ditto.
> 
OK
jeff



Re: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA

2020-06-11 Thread Jeff Law via Gcc-patches
On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote:
> Currently patchable area is at the wrong place.  It is placed immediately
> after function label, before both .cfi_startproc and ENDBR.  This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> changes ENDBR insertion pass to also insert patchable area instruction.
> TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> patchable area before .cfi_startproc and ENDBR.
> 
> OK for master?
> 
> Thanks.
> 
> H.J.
> ---
> gcc/
> 
>   PR target/93492
>   * config/i386/i386-features.c (rest_of_insert_endbranch):
>   Renamed to ...
>   (rest_of_insert_endbr_and_patchable_area): Change return type
>   to void. Add need_endbr and patchable_area_size arguments.
>   Don't call timevar_push nor timevar_pop.  Replace
>   endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
>   UNSPECV_PATCHABLE_AREA for patchable area.
>   (pass_data_insert_endbranch): Renamed to ...
>   (pass_data_insert_endbr_and_patchable_area): This.  Change
>   pass name to endbr_and_patchable_area.
>   (pass_insert_endbranch): Renamed to ...
>   (pass_insert_endbr_and_patchable_area): This.  Add need_endbr
>   and patchable_area_size;.
>   (pass_insert_endbr_and_patchable_area::gate): Set and check
>   need_endbr and patchable_area_size.
>   (pass_insert_endbr_and_patchable_area::execute): Call
>   timevar_push and timevar_pop.  Pass need_endbr and
>   patchable_area_size to rest_of_insert_endbr_and_patchable_area.
>   (make_pass_insert_endbranch): Renamed to ...
>   (make_pass_insert_endbr_and_patchable_area): This.
>   * config/i386/i386-passes.def: Replace pass_insert_endbranch
>   with pass_insert_endbr_and_patchable_area.
>   * config/i386/i386-protos.h (ix86_output_patchable_area): New.
>   (make_pass_insert_endbranch): Renamed to ...
>   (make_pass_insert_endbr_and_patchable_area): This.
>   * config/i386/i386.c (ix86_asm_output_function_label): Set
>   function_label_emitted to true.
>   (ix86_print_patchable_function_entry): New function.
>   (ix86_output_patchable_area): Likewise.
>   (x86_function_profiler): Replace endbr_queued_at_entrance with
>   insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
>   Call ix86_output_patchable_area to generate patchable area if
>   needed.
>   (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
>   * i386.h (queued_insn_type): New.
>   (machine_function): Add function_label_emitted.  Replace
>   endbr_queued_at_entrance with insn_queued_at_entrance.
>   * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
>   (patchable_area): New.
> 
> gcc/testsuite/
> 
>   PR target/93492
>   * gcc.target/i386/pr93492-1.c: New test.
>   * gcc.target/i386/pr93492-2.c: Likewise.
>   * gcc.target/i386/pr93492-3.c: Likewise.
>   * gcc.target/i386/pr93492-4.c: Likewise.
>   * gcc.target/i386/pr93492-5.c: Likewise.
OK
jeff
> 



Re: collect2.exe errors not pruned

2020-06-11 Thread Jeff Law via Gcc-patches
On Wed, 2020-06-10 at 23:41 -0300, Alexandre Oliva wrote:
> On May 26, 2020, Alexandre Oliva  wrote:
> 
> > On May 19, 2020, Joseph Myers  wrote:
> > > Allowing a missing executable name is reasonable enough, but I was 
> > > actually thinking that the messages should print "gcc" or whatever 
> > > command 
> > > the user ran in place of "collect2".
> > Should we make the regexps '\[^\n\]*', as in so many other pruned
> > messages?
> 
> Like this.  Regstrapped on x86_64-linux-gnu.  Ok to install?
> 
> 
> match any program name when pruning collect messages
> 
> From: Alexandre Oliva 
> 
> When collect* programs have an executable suffix, they may include it
> in their outputs.  Match them when pruning gcc output, making room for
> other program names to print them.
> 
> 
> for  gcc/testsuite/ChangeLog
> 
>   * lib/prune.exp (prune_gcc_output): Match any executable name
>   in collect messages.
OK
jeff
> 



Re: [PATCH] Fix few -Wformat-diag warnings.

2020-06-11 Thread Jeff Law via Gcc-patches
On Thu, 2020-06-11 at 10:43 +0200, Martin Liška wrote:
> Ready for master?
> 
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
>   * cgraphunit.c (process_symver_attribute): Wrap weakref keyword.
>   * dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new
>   line.
>   * lto-wrapper.c (merge_and_complain): Wrap option names.
OK.  And I think we should consider other changes of similar nature as pre-
approved.  Just run them through the usual regression testing.

Jeff
> 



Re: [PATCH 2/7 V3] rs6000: lenload/lenstore optab support

2020-06-11 Thread Kewen.Lin via Gcc-patches
Hi Segher,

on 2020/6/12 上午6:55, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Jun 10, 2020 at 08:39:19PM +0800, Kewen.Lin wrote:
>> +;; Define optab for vector access with length vectorization exploitation.
>> +(define_expand "lenload"
>> +  [(match_operand:VEC_A 0 "vlogical_operand")
>> +   (match_operand:VEC_A 1 "memory_operand")
>> +   (match_operand:QI 2 "int_reg_operand")]
> 
> Why this?  gpc_reg_operand will just work, no?  (Even just
> register_operand should, but let's not go there today ;-) )
> 

Good question!  The existing lxvl requires register_operand,
yeah, gpr_reg_operand looks fine too.  I was thinking this
operand for length would only exist in GPR, int_reg_operand
looks more reasonable here?

> Okay for trunk with that change, or with some explanation.  Thanks!
> 

Thanks!

BR,
Kewen

> 
> Segher
> 


RE: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning

2020-06-11 Thread Yangfei (Felix)
Hi,

> -Original Message-
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Friday, June 12, 2020 2:29 AM
> To: Yangfei (Felix) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR95570] vect: ICE: Segmentation fault in
> vect_loop_versioning
> 
> "Yangfei (Felix)"  writes:
> > From 30a0196b0afd45bae9291cfab3fee4ad6b90cbcb Mon Sep 17 00:00:00
> 2001
> > From: Fei Yang 
> > Date: Thu, 11 Jun 2020 19:33:22 +0800
> > Subject: [PATCH] vect: Fix an ICE in vect_loop_versioning [PR95570]
> >
> > In the test case for PR95570, the only data reference in the loop is a
> > gather-statter access.  Scalar evolution analysis for this data
> > reference failed, so DR_STEP is NULL_TREE.  This leads to the segmentation
> fault.
> > We should filter out scatter-gather access in
> vect_enhance_data_refs_alignment.
> 
> Looks good, just a couple of very minor details…
> 
> > 2020-06-11  Felix Yang  
> >
> > gcc/
> > PR tree-optimization/95570
> > * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New
> function.
> > (vect_verify_datarefs_alignment): Call it to filter out data 
> > references
> > in the loop whose alignment is irrelevant.
> > (vect_get_peeling_costs_all_drs): Likewise.
> > (vect_peeling_supportable): Likewise.
> > (vect_enhance_data_refs_alignment): Likewise.
> >
> > gcc/testsuite/
> >
> > PR tree-optimization/95570
> > * gcc.dg/vect/pr95570.c: New test.
> > ---
> >  gcc/testsuite/gcc.dg/vect/pr95570.c | 11 
> >  gcc/tree-vect-data-refs.c   | 83 -
> >  2 files changed, 45 insertions(+), 49 deletions(-)  create mode
> > 100644 gcc/testsuite/gcc.dg/vect/pr95570.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/vect/pr95570.c
> > b/gcc/testsuite/gcc.dg/vect/pr95570.c
> > new file mode 100644
> > index 000..b9362614004
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/pr95570.c
> > @@ -0,0 +1,11 @@
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-march=armv8.2-a+sve
> > +-msve-vector-bits=256 -mstrict-align -fwrapv" { target aarch64*-*-* }
> > +} */
> > +
> > +int x[8][32];
> > +
> > +void
> > +foo (int start)
> > +{
> > +  for (int i = start; i < start + 16; i++)
> > +x[start][i] = i;
> > +}
> > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> > index 39d5a1b554c..98f6fb76ff9 100644
> > --- a/gcc/tree-vect-data-refs.c
> > +++ b/gcc/tree-vect-data-refs.c
> > @@ -1129,6 +1129,35 @@ vect_update_misalignment_for_peel
> (dr_vec_info *dr_info,
> >SET_DR_MISALIGNMENT (dr_info, DR_MISALIGNMENT_UNKNOWN);  }
> >
> > +/* Return TRUE if alignment is relevant for DR_INFO.  */
> 
> Just “Return true …“ for new code.  TRUE is a hold-over from C days.

OK.

> > +static bool
> > +vect_relevant_for_alignment_p (dr_vec_info *dr_info) {
> > +  stmt_vec_info stmt_info = dr_info->stmt;
> > +
> > +  if (!STMT_VINFO_RELEVANT_P (stmt_info))
> > +return false;
> > +
> > +  /* For interleaving, only the alignment of the first access
> > + matters.  */  if (STMT_VINFO_GROUPED_ACCESS (stmt_info)
> > +  && DR_GROUP_FIRST_ELEMENT (stmt_info) != stmt_info)
> > +return false;
> > +
> > +  /* For scatter-gather or invariant accesses, alignment is irrelevant
> > + for them.  */
> 
> Maybe:
> 
>   /* Scatter-gather and invariant accesses continue to address individual
>  scalars, so vector-level alignment is irrelevant.  */
 
Much better : - )   Updated accordingly.
Also bootstrapped and tested on x86_64-linux-gnu.

Thanks,
Felix

gcc/

+2020-06-12  Felix Yang  
+
+   PR tree-optimization/95570
+   * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function.
+   (vect_verify_datarefs_alignment): Call it to filter out data references
+   in the loop whose alignment is irrelevant.
+   (vect_get_peeling_costs_all_drs): Likewise.
+   (vect_peeling_supportable): Likewise.
+   (vect_enhance_data_refs_alignment): Likewise.

gcc/testsuite/

+2020-06-12  Felix Yang  
+
+   PR tree-optimization/95570
+   * gcc.dg/vect/pr95570.c: New test.



pr95570-v3.diff
Description: pr95570-v3.diff


Re: [PATCH] guard against calls with fewer arguments than the function type implies (PR 95581)

2020-06-11 Thread Segher Boessenkool
On Tue, Jun 09, 2020 at 09:51:12AM -0600, Martin Sebor wrote:
> >I think the backend declaration is wrong, the function only takes
> >a void * argument and returns a long.
> 
> Thanks.  In his comment on the bug Segher (CC'd) points to
> the internals manual that documents the function:
> 
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/target.def;h=07059a87caf7cc0237d583124b476ee45ea41ed5;hb=HEAD#l1744
> 
> (By the way, thanks for the pointer!)
> 
> If I read it right, ihe function f in
> the TARGET_VECTORIZE_BUILTIN_MASK_FOR_LOAD documentation is
> __builtin_altivec_mask_for_load.  Although the manual isn't
> unequivocal about this but it does suggest the address addr
> the function is given as an argument should be the first
> (and presumably only) argument.  This matches the call in
> the IL where the first argument is a pointer, but not
> the function's signature.
> 
> I think the middle end needs to be able to rely on built-in
> function types when processing calls: i.e., the types and
> numbers of actual arguments in the calls need to match those
> of the built-in function type.  Otherwise it would have to
> validate every call against the function signature and avoid
> treating it as a built-in if it doesn't match.  There are
> parts of the middle end that do that for library built-ins
> (because they can be declared in a program with mismatched
> signatures) but as we have seen it's error-prone.  I don't
> think it would be helpful to try to extend this approach to
> internal built-ins.
> 
> So I agree that the real problem is the declaration of
> the built-in.

The problem is that this altivec builtin cannot implement the generic
builtin directly.  It should *not* be changed, we *do* need to keep
this builtin as-is.  We just cannot forward the generic builtin to it
like this.

Why did this work before, and not anymore?  That sounds like a missing
or broken test?

Thanks,


Segher


Re: [PATCH] Port libgccjit to Windows.

2020-06-11 Thread JonY via Gcc-patches
On 6/11/20 10:02 PM, Nicolas Bértolo wrote:
> Hi,
> 
> On 6/7/20 11:12 PM, JonY wrote:
>> Ideally, libtool is used so we get libgccjit-0.dll, unfortunately it is
>> not. So the only way to ABI version the dll would be to use Unix style
>> soname to mark when an ABI has changed.
> 
> I tried generating the library as libgccjit-0.dll and naming its import 
> library
> libgccjit.dll.a. It worked. I understand you prefer the libgccjit-0.dll 
> filename
> from this comment about libtool. A patch implementing this change is attached.
> 
> Thanks for your feedback.
> 
> Nicolas.
> 

Thanks for the patch, it looks good to me. I will push this soon if no
one else objects.




signature.asc
Description: OpenPGP digital signature


Re: [PATCH v1 1/2][PPC64] [PR88877]

2020-06-11 Thread Segher Boessenkool
On Tue, Jun 09, 2020 at 02:29:13PM -0600, Jeff Law wrote:
> On Sun, 2020-05-24 at 11:22 -0500, Segher Boessenkool wrote:
> > OTOH, you don't need to name Tuple at all...  It should not *have* a
> > constructor, since you declared it as class...  But you can just use
> > std::tuple here?
> > 
> > > (emit_library_call): Added default arg unsigned_p.
> > > (emit_library_call_value): Added default arg unsigned_p.
> > 
> > Yeah, eww.  Default arguments have all the problems you had before,
> > except now it is hidden and much more surprising.
> > 
> > Those functions really should take rtx_mode_t arguments?
> > 
> > Thanks again for working on this,
> ISTM that using std::tuple would be better than defining our own types.

Yeah.  But as Jakub an Iain said, not using a container type (but a more
concrete type, instead) is much better anyway :-)

> I'd rather see the argument be explicit rather than using default arguments 
> too. 
> While I have ack'd some patches with default arguments, I still don't like 
> 'em.

Default arguments have their place (but it's not here :-) )

> I do like the approach of getting the infrastructure in place without changing
> behavior, then having the behavior fix as a distinct change.

With Git, commits are easy and cheap, and massaging a patch series into
shape is easy and cheap as well.  If you develop using Git in the first
place (and you should!), you should naturally end up with many patches
in your series, and the preparatory patches first (after you reshuffle
things a bit, if you are like me and your foresight is severly limited).

So you have this separate *anyway* (or should have).  Since it helps
reviewing a lot, and also later bisecting, it is good to keep it.


Segher


Re: [PATCH 2/7 V3] rs6000: lenload/lenstore optab support

2020-06-11 Thread Segher Boessenkool
Hi!

On Wed, Jun 10, 2020 at 08:39:19PM +0800, Kewen.Lin wrote:
> +;; Define optab for vector access with length vectorization exploitation.
> +(define_expand "lenload"
> +  [(match_operand:VEC_A 0 "vlogical_operand")
> +   (match_operand:VEC_A 1 "memory_operand")
> +   (match_operand:QI 2 "int_reg_operand")]

Why this?  gpc_reg_operand will just work, no?  (Even just
register_operand should, but let's not go there today ;-) )

Okay for trunk with that change, or with some explanation.  Thanks!


Segher


Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]

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

On 6/11/20 5:28 PM, Marek Polacek wrote:

On Thu, Jun 11, 2020 at 03:51:29PM -0400, Jason Merrill wrote:

On 6/9/20 2:17 PM, Marek Polacek wrote:

Another part of 95369 is that we accept designated initializers with
non-aggregate types.  That seems to be wrong since they're part of
aggregate initialization.  clang/icc also reject it.

(Un)fortunately there are multiple contexts where we can use designated
initializers: function-like casts, member list initializers, NTTP, etc.
So I've adjusted multiple places in the compiler in order to to detect
this case and to provide a nice diagnostic, instead of an ugly raft of
errors.


Would it work to handle this only in add_list_candidates?


'fraid not -- we don't call add_list_candidates at all when compiling
desig16.C.


Hmm, why not?  What is turning the CONSTRUCTOR into an argument vec?

Jason



Re: [PATCH 1/2] c++: Improve access checking inside templates [PR41437]

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

On 6/5/20 5:16 PM, Patrick Palka wrote:

This patch generalizes our existing functionality for deferring access
checking of typedefs when parsing a function or class template to now
defer all kinds of access checks until template instantiation time,
including member function and member object accesses.

Since all access checks eventually go through enforce_access, the main
component of this patch is new handling inside enforce_access to defer
the current access check if we're inside a template.  The bulk of the
rest of the patch consists of removing now-unneeded code pertaining to
suppressing access checks inside templates or pertaining to
typedef-specific access handling.  Renamings and other changes with no
functional impact have been split off into the followup patch.


Great!


Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested by
building parts of boost, cmcstl2 and other libraries.



-  && !dependent_type_p (qualifying_type))
+  && !dependent_type_p (scope))


This needs a comment.  And it occurs to me that if we're only going to 
check access if scope is non-dependent, we can check that much earlier 
and avoid the need to guard DECL_NONSTATIC_MEMBER_P.


Jason



Re: [PATCH] Port libgccjit to Windows.

2020-06-11 Thread Nicolas Bértolo via Gcc-patches
Hi,

On 6/7/20 11:12 PM, JonY wrote:
> Ideally, libtool is used so we get libgccjit-0.dll, unfortunately it is
> not. So the only way to ABI version the dll would be to use Unix style
> soname to mark when an ABI has changed.

I tried generating the library as libgccjit-0.dll and naming its import library
libgccjit.dll.a. It worked. I understand you prefer the libgccjit-0.dll filename
from this comment about libtool. A patch implementing this change is attached.

Thanks for your feedback.

Nicolas.


0001-Rename-libgccjit.dll-to-libgccjit-0.dll.patch
Description: Binary data


Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]

2020-06-11 Thread Marek Polacek via Gcc-patches
On Thu, Jun 11, 2020 at 03:51:29PM -0400, Jason Merrill wrote:
> On 6/9/20 2:17 PM, Marek Polacek wrote:
> > Another part of 95369 is that we accept designated initializers with
> > non-aggregate types.  That seems to be wrong since they're part of
> > aggregate initialization.  clang/icc also reject it.
> > 
> > (Un)fortunately there are multiple contexts where we can use designated
> > initializers: function-like casts, member list initializers, NTTP, etc.
> > So I've adjusted multiple places in the compiler in order to to detect
> > this case and to provide a nice diagnostic, instead of an ugly raft of
> > errors.
> 
> Would it work to handle this only in add_list_candidates?

'fraid not -- we don't call add_list_candidates at all when compiling
desig16.C.

Marek



Re: [PATCH] avoid false positives due to compute_objsize (PR 95353)

2020-06-11 Thread Rainer Orth
Hi Martin,

> The compute_objsize() function started out as a thin wrapper around
> compute_builtin_object_size(), but over time developed its own
> features to compensate for the other function's limitations (such
> as its inability to work with ranges).  The interaction of these
> features and the limitations has started to become increasingly
> problematic as the former function is used in more contexts.
>
> A complete "fix" for all the problems (as well as some other
> limitations) that I'm working on will be more extensive and won't
> be appropriate for backports.  Until then, the attached patch
> cleans up the extensions compute_objsize() has accumulated over
> the years to avoid a class of false positives.
>
> To make the warnings issued based on the results of the function
> easier to understand and fix, the patch also adds an informative
> message to many instances of -Wstringop-overflow to point to
> the object to which the warning refers.  This is especially
> helpful when the object is referenced by a series of pointer
> operations.
>
> Tested by boostrapping on x86_64-linux and building Binutils/GDB,
> Glibc, and the Linux kernel with no new warnings.
>
> Besides applying it to trunk I'm looking to backport the fix to
> GCC 10.

it seems you were over-eager in removing xfail's from
gcc.dg/builtin-stringop-chk-5.c: on both Solaris/SPARC and x86 (32-bit
only) I see

+FAIL: gcc.dg/builtin-stringop-chk-5.c (test for excess errors)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 136)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 139)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 142)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 145)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 148)
+FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for 
warnings, line 151)

Excess errors:
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:136:3:
 warning: 'memset' writing 4 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:139:3:
 warning: 'memset' writing 4 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:142:3:
 warning: 'memset' writing 3 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:145:3:
 warning: 'memset' writing 3 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:148:3:
 warning: 'memset' writing 2 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]
/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:151:3:
 warning: 'memset' writing 2 bytes into a region of size 1 overflows the 
destination [-Wstringop-overflow=]

Rainer

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


Re: [PATCH] underline null argument in -Wnonnull (PR c++/86568)

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

On 6/5/20 3:41 PM, Martin Sebor wrote:

+  location_t loc
+= EXPR_HAS_LOCATION (param) ? EXPR_LOCATION (param) : pctx->loc;


This could be EXPR_LOC_OR_LOC (param, pctx->loc)


+ location_t loc = (EXPR_HAS_LOCATION (ptr)
+   ? EXPR_LOCATION (ptr) : EXPR_LOCATION (exp));


And similarly here.


+  if (param_num == 0)
 {
+  warned = warning_at (loc, OPT_Wnonnull,
+  "%qs pointer null", "this");
+  if (pctx->fndecl)
+   inform (DECL_SOURCE_LOCATION (pctx->fndecl),
+   "in a call to non-static member function %qD",
+   pctx->fndecl);
 }
+  else
+{
+  warned = warning_at (loc, OPT_Wnonnull,
+  "argument %u null where non-null expected",
+  (unsigned) param_num);
+  if (pctx->fndecl)
+   inform (DECL_SOURCE_LOCATION (pctx->fndecl),
+   "in a call to function %qD declared %qs",
+   pctx->fndecl, "nonnull");
+}


You need auto_diagnostic_group somewhere.

The c-common.c and tree.c changes are OK with these adjustments.  I'll 
leave the optimizer bits to someone else.


Jason



Can we use safe-ctype.h in a library?

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi,

I am currently looking at making libgfortran use the
TOUPPER et al functions from ctype-safe.h.  Adding this
in a gfortran header results in lots of

Gcc/trunk-bin/x86_64-pc-linux-gnu/./libgfortran/.libs/libgfortran.so: 
undefined reference to `_sch_istable'


errors.

So, can we use this, or should we just define this ourselves with
something like

#define TOUPPER(a) toupper ((unsigned char) (a))

?

Regards

Thomas


[PATCH] coroutines: Copy attributes to the outlined functions [PR95518]

2020-06-11 Thread Iain Sandoe
Hi

We had omitted the copying of function attributes (including the
'used' status).  Mark the outlined functions as artificial, since
they are; some diagnostic processing tests this.

tested on Linux and Darwin,
OK for master?
10.2?
thanks
Iain

gcc/cp/ChangeLog:

PR c++/95518
* coroutines.cc (act_des_fn): Copy function attributes from
the user’s function decl onto the outlined helpers.

gcc/testsuite/ChangeLog:

PR c++/95518
* g++.dg/coroutines/pr95518.C: New test.
---
 gcc/cp/coroutines.cc  |  5 +
 gcc/testsuite/g++.dg/coroutines/pr95518.C | 27 +++
 2 files changed, 32 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 93f1e5ca30d..4f7356e94e5 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3530,12 +3530,17 @@ act_des_fn (tree orig, tree fn_type, tree 
coro_frame_ptr, const char* name)
   tree fn_name = get_fn_local_identifier (orig, name);
   tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type);
   DECL_CONTEXT (fn) = DECL_CONTEXT (orig);
+  DECL_ARTIFICIAL (fn) = true;
   DECL_INITIAL (fn) = error_mark_node;
   tree id = get_identifier ("frame_ptr");
   tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
   DECL_CONTEXT (fp) = fn;
   DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr);
   DECL_ARGUMENTS (fn) = fp;
+  /* Copy used-ness from the original function.  */
+  TREE_USED (fn) = TREE_USED (orig);
+  /* Apply attributes from the original fn.  */
+  DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig));
   return fn;
 }
 
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C 
b/gcc/testsuite/g++.dg/coroutines/pr95518.C
new file mode 100644
index 000..2d7dd049e1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C
@@ -0,0 +1,27 @@
+//  { dg-additional-options "-O -Wunused-function" }
+
+#if __has_include ()
+#include 
+using namespace std;
+#elif defined (__clang__) && __has_include ()
+#include 
+namespace std { using namespace experimental; }
+#endif
+
+struct dummy
+{
+struct promise_type
+{
+dummy get_return_object() const noexcept { return {}; }
+std::suspend_never initial_suspend() const noexcept { return {}; }
+std::suspend_never final_suspend() const noexcept { return {}; }
+void return_void() const noexcept {}
+void unhandled_exception() const noexcept {}
+};
+int i; // work around #95516
+};
+
+[[maybe_unused]] static dummy foo()
+{ 
+co_return;
+}
-- 
2.24.1



Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]

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

On 6/9/20 2:17 PM, Marek Polacek wrote:

Another part of 95369 is that we accept designated initializers with
non-aggregate types.  That seems to be wrong since they're part of
aggregate initialization.  clang/icc also reject it.

(Un)fortunately there are multiple contexts where we can use designated
initializers: function-like casts, member list initializers, NTTP, etc.
So I've adjusted multiple places in the compiler in order to to detect
this case and to provide a nice diagnostic, instead of an ugly raft of
errors.


Would it work to handle this only in add_list_candidates?


Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

PR c++/95369
* call.c (implicit_conversion): Return NULL if a designated
initializer is used with a non-aggregate.
(implicit_conversion_error): Give an error for the case above.
* decl.c (check_initializer): Likewise.
* init.c (build_aggr_init): Likewise.
* semantics.c (finish_compound_literal): Likewise.

gcc/testsuite/ChangeLog:

PR c++/95369
* g++.dg/cpp2a/desig11.C: Adjust dg-error.
* g++.dg/cpp2a/desig16.C: New test.
---
  gcc/cp/call.c| 11 +++
  gcc/cp/decl.c|  9 +
  gcc/cp/init.c| 10 ++
  gcc/cp/semantics.c   |  7 +++
  gcc/testsuite/g++.dg/cpp2a/desig11.C |  2 +-
  gcc/testsuite/g++.dg/cpp2a/desig16.C | 28 
  6 files changed, 66 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig16.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 3c97b9846e2..346fb850f84 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -2020,6 +2020,12 @@ implicit_conversion (tree to, tree from, tree expr, bool 
c_cast_p,
if (is_std_init_list (to) && !CONSTRUCTOR_IS_DESIGNATED_INIT (expr))
return build_list_conv (to, expr, flags, complain);
  
+  /* Designated initializers can only be used to initialize an aggregate

+because they're part of aggregate initialization.  Return NULL here,
+implicit_conversion_error will issue an actual error.  */
+  if (CONSTRUCTOR_IS_DESIGNATED_INIT (expr) && !CP_AGGREGATE_TYPE_P (to))
+   return NULL;
+
/* As an extension, allow list-initialization of _Complex.  */
if (TREE_CODE (to) == COMPLEX_TYPE
  && !CONSTRUCTOR_IS_DESIGNATED_INIT (expr))
@@ -4301,6 +4307,11 @@ implicit_conversion_error (location_t loc, tree type, 
tree expr)
  instantiate_type (type, expr, complain);
else if (invalid_nonstatic_memfn_p (loc, expr, complain))
  /* We gave an error.  */;
+  else if (BRACE_ENCLOSED_INITIALIZER_P (expr)
+  && CONSTRUCTOR_IS_DESIGNATED_INIT (expr)
+  && !CP_AGGREGATE_TYPE_P (type))
+error_at (loc, "designated initializers cannot be used with a "
+ "non-aggregate type %qT", type);
else
  {
range_label_for_type_mismatch label (TREE_TYPE (expr), type);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index b8bd09b37e6..577643a1523 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6668,6 +6668,15 @@ check_initializer (tree decl, tree init, int flags, 
vec **cleanups)
  return NULL_TREE;
}
}
+  else if (CONSTRUCTOR_IS_DESIGNATED_INIT (init)
+  && !CP_AGGREGATE_TYPE_P (type))
+   {
+ error_at (cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (decl)),
+   "designated initializers cannot be used with a "
+   "non-aggregate type %qT", type);
+ TREE_TYPE (decl) = error_mark_node;
+ return NULL_TREE;
+   }
  }
  
if (TREE_CODE (decl) == CONST_DECL)

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index ef4b3c4dc3c..de261cfe7a6 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1802,6 +1802,16 @@ build_aggr_init (tree exp, tree init, int flags, 
tsubst_flags_t complain)
TREE_TYPE (init) = itype;
return stmt_expr;
  }
+  else if (init
+  && BRACE_ENCLOSED_INITIALIZER_P (init)
+  && CONSTRUCTOR_IS_DESIGNATED_INIT (init)
+  && !CP_AGGREGATE_TYPE_P (type))
+{
+  if (complain & tf_error)
+   error_at (init_loc, "designated initializers cannot be used with a "
+ "non-aggregate type %qT", type);
+  return error_mark_node;
+}
  
if (init && init != void_type_node

&& TREE_CODE (init) != TREE_LIST
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 64587c791c6..f25c4ec7110 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2933,6 +2933,13 @@ finish_compound_literal (tree type, tree 
compound_literal,
  
if (TYPE_NON_AGGREGATE_CLASS (type))

  {
+  if (CONSTRUCTOR_IS_DESIGNATED_INIT (compound_literal))
+   {
+ if (complain & tf_error)
+   error ("designated initializers cannot be used with a "
+  "non-aggregate type %qT", 

[PATCH] coroutines: Update handling and failure for g-r-o-o-a-f [PR95505]

2020-06-11 Thread Iain Sandoe
Hi,

The reason that the code fails is that (by a somewhat complex
implied route), when a user adds a 'get-return-on-alloc-fail’
to their coroutine promise, this implies the use of some content
from ; we should not ICE because the user forgot that tho.

Jonathan and I were discussing whether there’s a better way than
including  in  to make this less likely to happen.

The issue is that neither  nor the user’s code overtly
use ; one has to know that the standard makes use of it
by implication… anyway that’s not the bug, but the background.

tested on Linux, and Darwin.
OK for master?
10.2?
thanks
Iain



The actual issue is that (in the testcase) std::nothrow is not
available.  So update the handling of the get-return-on-alloc-fail
to include the possibility that std::nothrow might not be
available.

gcc/cp/ChangeLog:

* coroutines.cc (morph_fn_to_coro): Update handling of
get-return-object-on-allocation-fail and diagnose missing
std::nothrow.

gcc/testsuite/ChangeLog:

* g++.dg/coroutines/pr95505.C: New test.
---
 gcc/cp/coroutines.cc  | 43 +++
 gcc/testsuite/g++.dg/coroutines/pr95505.C | 26 ++
 2 files changed, 47 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95505.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index f0b7e71633d..93f1e5ca30d 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3913,30 +3913,25 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
   tree grooaf = NULL_TREE;
   tree dummy_promise = build_dummy_object (get_coroutine_promise_type (orig));
 
-  /* We don't require this, so lookup_promise_method can return NULL...  */
+  /* We don't require this, so lookup_promise_method can return NULL,
+ but, if the lookup succeeds, then the function must be usable.*/
   if (grooaf_meth && BASELINK_P (grooaf_meth))
-{
-  /* ... but, if the lookup succeeds, then the function must be
-usable.
-build_new_method_call () wants a valid pointer to (an empty)  args
-list in this case.  */
-  vec *args = make_tree_vector ();
-  grooaf = build_new_method_call (dummy_promise, grooaf_meth, ,
- NULL_TREE, LOOKUP_NORMAL, NULL,
- tf_warning_or_error);
-  release_tree_vector (args);
-}
+grooaf = build_new_method_call (dummy_promise, grooaf_meth, NULL,
+   NULL_TREE, LOOKUP_NORMAL, NULL,
+   tf_warning_or_error);
 
   /* Allocate the frame, this has several possibilities:
  [dcl.fct.def.coroutine] / 9 (part 1)
  The allocation function’s name is looked up in the scope of the promise
  type.  It's not a failure for it to be absent see part 4, below.  */
+
   tree nwname = ovl_op_identifier (false, NEW_EXPR);
-  tree fns = lookup_promise_method (orig, nwname, fn_start,
-   /*musthave=*/false);
   tree new_fn = NULL_TREE;
-  if (fns && BASELINK_P (fns))
+  /* We don't require this, so lookup_promise_method can return NULL...  */
+  if (TYPE_HAS_NEW_OPERATOR (promise_type))
 {
+  tree fns = lookup_promise_method (orig, nwname, fn_start,
+   /*musthave=*/true);
   /* [dcl.fct.def.coroutine] / 9 (part 2)
If the lookup finds an allocation function in the scope of the promise
type, overload resolution is performed on a function call created by
@@ -3973,14 +3968,13 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
  LOOKUP_NORMAL, , tf_none);
   release_tree_vector (args);
 
-  if (!new_fn || new_fn == error_mark_node)
+  if (new_fn == error_mark_node)
{
  /* [dcl.fct.def.coroutine] / 9 (part 3)
If no viable function is found, overload resolution is performed
again on a function call created by passing just the amount of
space required as an argument of type std::size_t.  */
- args = make_tree_vector ();
- vec_safe_push (args, resizeable); /* Space needed.  */
+ args = make_tree_vector_single (resizeable); /* Space needed.  */
  new_fn = build_new_method_call (dummy_promise, fns, ,
  NULL_TREE, LOOKUP_NORMAL, ,
  tf_none);
@@ -3989,7 +3983,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
 
  /* However, if the initial lookup succeeded, then one of these two
options must be available.  */
-if (!new_fn || new_fn == error_mark_node)
+if (new_fn == error_mark_node)
   {
error_at (fn_start, "%qE is provided by %qT but is not usable with"
  " the function signature %qD", nwname, promise_type, orig);
@@ -3999,7 +3993,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 

Re: [PATCH] c++: constrained class template friend [PR93467]

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

On 6/9/20 2:18 PM, Patrick Palka wrote:

This fixes two issues in our handling of constrained class template
friend declarations.

The first issue is that we fail to set the constraints on the injected
class template declaration during tsubst_friend_class.

The second issue is that the template parameter levels within the parsed
constraints of a class template friend declaration are shifted if the
enclosing class is a template, and this shift leads to spurious
constraint mismatch errors in associate_classtype_constraints if the
friend declaration refers to an already declared class template.

Passes 'make check-c++', and also verified by building the testsuite of
cmcstl2.  Does this look OK to commit to master and to the 10.2 branch
after a full bootstrap and regtest?


OK.


gcc/cp/ChangeLog:

PR c++/93467
* constraint.cc (associate_classtype_constraints): If there is a
discrepancy between the current template depth and the template
depth of the original declaration, then adjust the template
parameter depth within the current constraints appropriately.
* pt.c (tsubst_friend_class): Substitute into and set the
constraints on injected declaration.

gcc/testsuite/ChangeLog:

PR c++/93467
* g++.dg/cpp2a/concepts-friend6.C: New test.
* g++.dg/cpp2a/concepts-friend7.C: New test.
---
  gcc/cp/constraint.cc  | 13 +
  gcc/cp/pt.c   | 10 ++
  gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C | 19 +++
  gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C | 19 +++
  4 files changed, 61 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 92ff283013e..d0da2300ba9 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1075,6 +1075,19 @@ associate_classtype_constraints (tree type)
 original declaration.  */
if (tree orig_ci = get_constraints (decl))
  {
+ if (int extra_levels = (TMPL_PARMS_DEPTH (current_template_parms)
+ - TMPL_ARGS_DEPTH (TYPE_TI_ARGS (type
+   {
+ /* If there is a discrepancy between the current template depth
+and the template depth of the original declaration, then we
+must be redeclaring a class template as part of a friend
+declaration within another class template.  Before matching
+constraints, we need to reduce the template parameter level
+within the current constraints via substitution.  */
+ tree outer_gtargs = template_parms_to_args 
(current_template_parms);
+ TREE_VEC_LENGTH (outer_gtargs) = extra_levels;
+ ci = tsubst_constraint_info (ci, outer_gtargs, tf_none, 
NULL_TREE);
+   }
if (!equivalent_constraints (ci, orig_ci))
  {
  error ("%qT does not match original declaration", type);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 142392224c6..7fcb3e9f1c5 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11211,6 +11211,16 @@ tsubst_friend_class (tree friend_tmpl, tree args)
  DECL_ANTICIPATED (tmpl)
= DECL_ANTICIPATED (DECL_TEMPLATE_RESULT (tmpl)) = true;
  
+	  /* Substitute into and set the constraints on the new declaration.  */

+ if (tree ci = get_constraints (friend_tmpl))
+   {
+ ++processing_template_decl;
+ ci = tsubst_constraint_info (ci, args, tf_warning_or_error,
+  DECL_FRIEND_CONTEXT (friend_tmpl));
+ --processing_template_decl;
+ set_constraints (tmpl, ci);
+   }
+
  /* Inject this template into the enclosing namspace scope.  */
  tmpl = pushdecl_namespace_level (tmpl, true);
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C
new file mode 100644
index 000..11e8313f0ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C
@@ -0,0 +1,19 @@
+// PR c++/93467
+// { dg-do compile { target c++20 } }
+
+template requires B
+  class C;
+
+template
+class S1
+{
+  template requires B
+friend class ::C;
+};
+
+template
+class S2
+{
+  template requires (!B)
+friend class ::C; // { dg-error "does not match original declaration" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C
new file mode 100644
index 000..1c6893584b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C
@@ -0,0 +1,19 @@
+// PR c++/93467
+// { dg-do compile { target c++20 } }
+
+template concept True = true;
+
+template
+struct S1 {
+template friend struct S2; // friend declaration for S2
+};
+
+S1 s; // 

Re: [RFA] Fix various regressions and kernel build failure due to adjust-alignment issue

2020-06-11 Thread Hans-Peter Nilsson
On Tue, 9 Jun 2020, Jeff Law via Gcc-patches wrote:

> On Tue, 2020-06-09 at 17:26 +0200, Jakub Jelinek wrote:
> > On Tue, Jun 09, 2020 at 09:18:25AM -0600, Jeff Law wrote:
> > > On Tue, 2020-06-09 at 16:59 +0200, Jakub Jelinek wrote:
> > > > On Tue, Jun 09, 2020 at 08:54:47AM -0600, Jeff Law via Gcc-patches 
> > > > wrote:
> > > > > > While technically OK the issue is that it does not solve the x86 
> > > > > > issue
> > > > > > which with incoming stack alignment < 8 bytes does not perform
> > > > > > dynamic re-alignment to align 'long long' variables appropriately.
> > > > > > Currently the x86 backend thinks it can fixup by lowering alignment
> > > > > > via LOCAL_DECL_ALIGNMENT but this is a latent wrong-code issue
> > > > > > because the larger alignment is present in the IL for a long 
> > > > > > (previously
> > > > > > RTL expansion, now adjust-aligment) time and your patch makes that
> > > > > > wrong info last forever (or alternatively cause dynamic stack 
> > > > > > alignment
> > > > > > which we do _not_ want to perform here).
> > > > > I've never looked at the dynamic realignment stuff -- is there a good 
> > > > > reason why
> > > > > we don't dynamically realign when we've got a local with a requested 
> > > > > alignment?
> > > > > Isn't that a huge oversight in the whole concept of dynamic 
> > > > > realignment?
> > > >
> > > > It is quite expensive and long long/double uses are pervasive, so we 
> > > > don't
> > > > want to realign just because of that.  If we do dynamic realignment for
> > > > other reasons, there is no reason not to align the long long/double
> > > Right, but if there's an explicit alignment from the user, don't we need 
> > > to honor
> > > that?
> >
> > Sure.  Do we ignore that?  For user alignment we have DECL_USER_ALIGN bit...
> I suspect that's what's going on with the kernel build failure.

Sounds like you're about to tackle PR84877.
Great! :)

brgds, H-P


Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]

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

On 6/11/20 3:32 PM, Jason Merrill wrote:

On 6/10/20 5:11 PM, Marek Polacek wrote:

Another indication that perhaps this warning is emitted too early.  We
crash because same_type_p gets a null type: we have an enumerator
without a fixed underlying type and finish_enum_value_list hasn't yet
run.  So check if the type is null before calling same_type_p.


Hmm, I wonder why we use NULL_TREE for the type of uninitialized 
enumerators in a template; why not give them integer_type_node temporarily?


But this patch is OK for 10.2.


(This is a regression and this fix is suitable for backporting.  Delaying
the warning would not be suitable to backport.)

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?

gcc/cp/ChangeLog:

PR c++/95560
* name-lookup.c (check_local_shadow): Check if types are
non-null before calling same_type_p.

gcc/testsuite/ChangeLog:

PR c++/95560
* g++.dg/warn/Wshadow-local-3.C: New test.
---
  gcc/cp/name-lookup.c    | 4 +++-
  gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++
  2 files changed, 10 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2ff85f1cf5e..159c98a67cd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
    enum opt_code warning_code;
    if (warn_shadow)
  warning_code = OPT_Wshadow;
-  else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+  else if ((TREE_TYPE (old)
+    && TREE_TYPE (decl)
+    && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
 || TREE_CODE (decl) == TYPE_DECL
 || TREE_CODE (old) == TYPE_DECL
 || (!dependent_type_p (TREE_TYPE (decl))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C

new file mode 100644
index 000..fd743eca347
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
@@ -0,0 +1,7 @@
+// PR c++/95560
+// { dg-do compile { target c++11 } }
+
+template  void fn1() {
+  bool ready;
+  enum class State { ready };
+}

base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469







Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]

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

On 6/11/20 3:34 PM, Jason Merrill wrote:

On 6/11/20 3:32 PM, Jason Merrill wrote:

On 6/10/20 5:11 PM, Marek Polacek wrote:

Another indication that perhaps this warning is emitted too early.  We
crash because same_type_p gets a null type: we have an enumerator
without a fixed underlying type and finish_enum_value_list hasn't yet
run.  So check if the type is null before calling same_type_p.


Hmm, I wonder why we use NULL_TREE for the type of uninitialized 
enumerators in a template; why not give them integer_type_node 
temporarily?


But this patch is OK for 10.2.


And 9.



(This is a regression and this fix is suitable for backporting.  
Delaying

the warning would not be suitable to backport.)

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?

gcc/cp/ChangeLog:

PR c++/95560
* name-lookup.c (check_local_shadow): Check if types are
non-null before calling same_type_p.

gcc/testsuite/ChangeLog:

PR c++/95560
* g++.dg/warn/Wshadow-local-3.C: New test.
---
  gcc/cp/name-lookup.c    | 4 +++-
  gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++
  2 files changed, 10 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2ff85f1cf5e..159c98a67cd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
    enum opt_code warning_code;
    if (warn_shadow)
  warning_code = OPT_Wshadow;
-  else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+  else if ((TREE_TYPE (old)
+    && TREE_TYPE (decl)
+    && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
 || TREE_CODE (decl) == TYPE_DECL
 || TREE_CODE (old) == TYPE_DECL
 || (!dependent_type_p (TREE_TYPE (decl))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C

new file mode 100644
index 000..fd743eca347
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
@@ -0,0 +1,7 @@
+// PR c++/95560
+// { dg-do compile { target c++11 } }
+
+template  void fn1() {
+  bool ready;
+  enum class State { ready };
+}

base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469









Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]

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

On 6/10/20 5:11 PM, Marek Polacek wrote:

Another indication that perhaps this warning is emitted too early.  We
crash because same_type_p gets a null type: we have an enumerator
without a fixed underlying type and finish_enum_value_list hasn't yet
run.  So check if the type is null before calling same_type_p.


Hmm, I wonder why we use NULL_TREE for the type of uninitialized 
enumerators in a template; why not give them integer_type_node temporarily?



(This is a regression and this fix is suitable for backporting.  Delaying
the warning would not be suitable to backport.)

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?

gcc/cp/ChangeLog:

PR c++/95560
* name-lookup.c (check_local_shadow): Check if types are
non-null before calling same_type_p.

gcc/testsuite/ChangeLog:

PR c++/95560
* g++.dg/warn/Wshadow-local-3.C: New test.
---
  gcc/cp/name-lookup.c| 4 +++-
  gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++
  2 files changed, 10 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2ff85f1cf5e..159c98a67cd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
enum opt_code warning_code;
if (warn_shadow)
warning_code = OPT_Wshadow;
-  else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+  else if ((TREE_TYPE (old)
+   && TREE_TYPE (decl)
+   && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
   || TREE_CODE (decl) == TYPE_DECL
   || TREE_CODE (old) == TYPE_DECL
   || (!dependent_type_p (TREE_TYPE (decl))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
new file mode 100644
index 000..fd743eca347
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
@@ -0,0 +1,7 @@
+// PR c++/95560
+// { dg-do compile { target c++11 } }
+
+template  void fn1() {
+  bool ready;
+  enum class State { ready };
+}

base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469





[PATCH] middle-end: Optimize (A)^(B) to (A^B) in simplify_rtx.

2020-06-11 Thread Roger Sayle
 

My apologies in advance for a middle-end patch without a test case.   The
patch below

implements a simple/safe missing transformation in the RTL optimizers, that
transforms

(A)^(B) into the equivalent (A^B), when C doesn't side-effect, such as
a constant.

 

I originally identified this opportunity in gfortran, where:

 

integer function foo(i) result(res)

  integer(kind=16), intent(in) :: i

  res = poppar(i)

end function

 

currently on x86_64 with -O2 -march=corei7 gfortran produces:

foo_:   popcntq (%rdi), %rdx

popcntq 8(%rdi), %rax

andl$1, %edx

andl$1, %eax

xorl%edx, %eax

ret

 

But with this patch, now produces:

foo_:   popcntq (%rdi), %rdx

popcntq 8(%rdi), %rax

xorl%edx, %eax

andl$1, %eax

ret

 

The equivalent C/C++ testcase is:

 

unsigned int foo(unsigned int x, unsigned  int y)

{

  return __builtin_parityll(x) ^ __builtin_parityll(y);

}

 

where GCC currently generates:

foo:movl%esi, %eax

movl%edi, %edi

popcntq %rdi, %rdi

popcntq %rax, %rax

andl$1, %edi

andl$1, %eax

xorl%edi, %eax

ret

 

and with this patch, it instead generates:

foo:movl%esi, %eax

movl%edi, %edi

popcntq %rdi, %rdi

popcntq %rax, %rax

xorl%edi, %eax

andl$1, %eax

ret

 

 

The trouble is I'm just about to submit a patch to improve constant folding
of parity in the

middle-end's match.pd, which will generate different RTL code sequences for
the above

two examples.  Hopefully, folks agree it's better to have a RTL optimization
that's difficult

to test, than not perform this simplification at all.  The
semantics/correctness of this

transformation are tested by the run-time tests in
gfortran.dg/popcnt_poppar_2.F90

 

This patch has been tested with "make bootstrap" and "make -k check" on

x86_64-pc-linux-gnu with no regressions.  If approved, I'd very much
appreciate it if

someone could commit this change for me.

 

 

2020-06-11  Roger Sayle  

 

* simplify-rtx.c (simplify_binary_operation_1): Simplify

(X & C) ^ (Y & C) to (X ^ Y) & C, when C is simple (i.e. a
constant).

 

 

Thanks very much in advance,

Roger

--

Roger Sayle

NextMove Software

Cambridge, UK

 

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 28c2dc6..ccf5f6d 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -3128,6 +3128,17 @@ simplify_binary_operation_1 (enum rtx_code code, 
machine_mode mode,
 mode);
   }
 
+  /* Convert (xor (and A C) (and B C)) into (and (xor A B) C).  */
+  if (GET_CODE (op0) == AND 
+ && GET_CODE (op1) == AND
+ && rtx_equal_p (XEXP (op0, 1), XEXP (op1, 1))
+ && ! side_effects_p (XEXP (op0, 1)))
+   return simplify_gen_binary (AND, mode,
+   simplify_gen_binary (XOR, mode,
+XEXP (op0, 0),
+XEXP (op1, 0)),
+   XEXP (op0, 1));
+
   /* Convert (xor (and A B) B) to (and (not A) B).  The latter may
 correspond to a machine insn or result in further simplifications
 if B is a constant.  */


PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075

2020-06-11 Thread Harald Anlauf
Found by Steve.  Committed as obvious.

Thanks,
Harald


PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075

When reporting a duplicate access specification of an operator, refer to
the proper symbol.

2020-06-11  Harald Anlauf 

gcc/fortran/
PR fortran/95611
* decl.c (access_attr_decl): Use correct symbol in error message.

Co-Authored-By: Steven G. Kargl  


diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 1c1626d3fa4..c8a98537e87 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -9073,7 +9073,7 @@ access_attr_decl (gfc_statement st)
  else
{
  gfc_error ("Access specification of the .%s. operator at %C "
-"has already been specified", sym->name);
+"has already been specified", uop->name);
  goto done;
}

diff --git a/gcc/testsuite/gfortran.dg/pr95611.f90 
b/gcc/testsuite/gfortran.dg/pr95611.f90
new file mode 100644
index 000..b7a54514ca3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95611.f90
@@ -0,0 +1,7 @@
+! { dg-do compile }
+! PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075
+
+module m
+  public operator (.a.)
+  public operator (.a.) ! { dg-error "has already been specified" }
+end



Re: [PATCH] rs6000: skip debug info statements

2020-06-11 Thread Segher Boessenkool
Hi!

On Thu, Jun 11, 2020 at 01:49:57PM +0200, Martin Liška wrote:
> Since stmt_vec_info is not generated for debug info statements, these needs 
> to
> be skipped in rs6000_density_test.

>   PR target/95627
>   * config/rs6000/rs6000.c (rs6000_density_test): Skip debug
>   statements.

> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 0bbd06ad1de..00daf979856 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data)
>for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
>   {
> gimple *stmt = gsi_stmt (gsi);
> +   if (is_gimple_debug (stmt))
> +   continue;

Yes, this is fine for trunk (with the indent fixed), thanks!

Or you could use gsi_next_nondebug?  That is a bit neater.  Either way
is okay.

Thanks!


Segher


Re: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494

2020-06-11 Thread Harald Anlauf
Hi Thomas,

> one remark: Instead of
>
> +   && !(strcmp(gfc_current_intrinsic, "associated") == 0
> + || strcmp(gfc_current_intrinsic, "null") == 0
> + || strcmp(gfc_current_intrinsic, "present") == 0))
>
> could you maybe test sym->id ?
>
> OK with that change.

done.  See attached for the actual commit.

Thanks for the hint and the swift review!

Harald
diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 0afb96c0414..148a3269815 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1431,8 +1431,8 @@ gfc_check_x_yd (gfc_expr *x, gfc_expr *y)
   return true;
 }

-static bool
-invalid_null_arg (gfc_expr *x)
+bool
+gfc_invalid_null_arg (gfc_expr *x)
 {
   if (x->expr_type == EXPR_NULL)
 {
@@ -1451,7 +1451,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
   int i;
   bool t;

-  if (invalid_null_arg (pointer))
+  if (gfc_invalid_null_arg (pointer))
 return false;

   attr1 = gfc_expr_attr (pointer);
@@ -1477,7 +1477,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
   if (target == NULL)
 return true;

-  if (invalid_null_arg (target))
+  if (gfc_invalid_null_arg (target))
 return false;

   if (target->expr_type == EXPR_VARIABLE || target->expr_type == EXPR_FUNCTION)
@@ -3374,7 +3374,7 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_expr *status)
 bool
 gfc_check_kind (gfc_expr *x)
 {
-  if (invalid_null_arg (x))
+  if (gfc_invalid_null_arg (x))
 return false;

   if (gfc_bt_struct (x->ts.type) || x->ts.type == BT_CLASS)
@@ -3453,6 +3453,9 @@ gfc_check_len_lentrim (gfc_expr *s, gfc_expr *kind)
   if (!type_check (s, 0, BT_CHARACTER))
 return false;

+  if (gfc_invalid_null_arg (s))
+return false;
+
   if (!kind_check (kind, 1, BT_INTEGER))
 return false;
   if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic "
@@ -4138,10 +4141,10 @@ gfc_check_transf_bit_intrins (gfc_actual_arglist *ap)
 bool
 gfc_check_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask)
 {
-  if (invalid_null_arg (tsource))
+  if (gfc_invalid_null_arg (tsource))
 return false;

-  if (invalid_null_arg (fsource))
+  if (gfc_invalid_null_arg (fsource))
 return false;

   if (!same_type_check (tsource, 0, fsource, 1))
@@ -5061,7 +5064,7 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind)
 {
   gfc_array_ref *ar;

-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   if (source->rank == 0 || source->expr_type != EXPR_VARIABLE)
@@ -5146,7 +5149,7 @@ gfc_check_size (gfc_expr *array, gfc_expr *dim, gfc_expr *kind)
 bool
 gfc_check_sizeof (gfc_expr *arg)
 {
-  if (invalid_null_arg (arg))
+  if (gfc_invalid_null_arg (arg))
 return false;

   if (arg->ts.type == BT_PROCEDURE)
@@ -5634,7 +5637,7 @@ gfc_check_sngl (gfc_expr *a)
 bool
 gfc_check_spread (gfc_expr *source, gfc_expr *dim, gfc_expr *ncopies)
 {
-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   if (source->rank >= GFC_MAX_DIMENSIONS)
@@ -6167,7 +6170,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size)
   size_t source_size;
   size_t result_size;

-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   /* SOURCE shall be a scalar or array of any type.  */
@@ -6186,7 +6189,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size)
   if (mold->ts.type == BT_BOZ && illegal_boz_arg (mold))
 return false;

-  if (invalid_null_arg (mold))
+  if (gfc_invalid_null_arg (mold))
 return false;

   /* MOLD shall be a scalar or array of any type.  */
@@ -6412,6 +6415,9 @@ gfc_check_trim (gfc_expr *x)
   if (!type_check (x, 0, BT_CHARACTER))
 return false;

+  if (gfc_invalid_null_arg (x))
+return false;
+
   if (!scalar_check (x, 0))
 return false;

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 0ef7b1b0eff..6d76efb5298 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3553,6 +3553,7 @@ bool gfc_calculate_transfer_sizes (gfc_expr*, gfc_expr*, gfc_expr*,
 bool gfc_boz2int (gfc_expr *, int);
 bool gfc_boz2real (gfc_expr *, int);
 bool gfc_invalid_boz (const char *, locus *);
+bool gfc_invalid_null_arg (gfc_expr *);


 /* class.c */
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index 17f5efc6566..60d91f658bd 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -4442,6 +4442,18 @@ check_arglist (gfc_actual_arglist **ap, gfc_intrinsic_sym *sym,
 	  return false;
 	}

+  /* F2018, p. 328: An argument to an intrinsic procedure other than
+	 ASSOCIATED, NULL, or PRESENT shall be a data object.  An EXPR_NULL
+	 is not a data object.  */
+  if (actual->expr->expr_type == EXPR_NULL
+	  && (!(sym->id == GFC_ISYM_ASSOCIATED
+		|| sym->id == GFC_ISYM_NULL
+		|| sym->id == GFC_ISYM_PRESENT)))
+	{
+	  gfc_invalid_null_arg (actual->expr);
+	  return false;
+	}
+
   /* If the formal argument is INTENT([IN]OUT), 

Re: [PATCH] c++: ICE with IMPLICIT_CONV_EXPR in array subscript [PR95508]

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

On 6/10/20 5:11 PM, Marek Polacek wrote:

Since r10-7096 convert_like, when called in a template, creates an
IMPLICIT_CONV_EXPR when we're converting to/from array type.

In this test, we have e[f], and we're converting f (of type class A) to
int, so convert_like in build_new_op_1 created the IMPLICIT_CONV_EXPR
that got into cp_build_array_ref which calls maybe_constant_value.  My
patch above failed to adjust this spot to call fold_non_dependent_expr
instead, which can handle codes like I_C_E in a template.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

gcc/cp/ChangeLog:

PR c++/95508
* typeck.c (cp_build_array_ref): Call fold_non_dependent_expr instead
of maybe_constant_value.

gcc/testsuite/ChangeLog:

PR c++/95508
* g++.dg/template/conv16.C: New test.
---
  gcc/cp/typeck.c|  2 +-
  gcc/testsuite/g++.dg/template/conv16.C | 17 +
  2 files changed, 18 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/template/conv16.C

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index f01ae656254..07144d4c1fc 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3565,7 +3565,7 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
 pointer arithmetic.)  */
idx = cp_perform_integral_promotions (idx, complain);
  
-  idx = maybe_constant_value (idx);

+  idx = fold_non_dependent_expr (idx, complain);


Hmm, if idx isn't constant this will leave us with non-template trees in 
the ARRAY_REF.  I think what we want is a function that will fold a 
non-dependent expr to a constant or return the argument. 
maybe_fold_non_dependent_expr?


Jason



Re: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning

2020-06-11 Thread Richard Sandiford
"Yangfei (Felix)"  writes:
> From 30a0196b0afd45bae9291cfab3fee4ad6b90cbcb Mon Sep 17 00:00:00 2001
> From: Fei Yang 
> Date: Thu, 11 Jun 2020 19:33:22 +0800
> Subject: [PATCH] vect: Fix an ICE in vect_loop_versioning [PR95570]
>
> In the test case for PR95570, the only data reference in the loop is a
> gather-statter access.  Scalar evolution analysis for this data reference
> failed, so DR_STEP is NULL_TREE.  This leads to the segmentation fault.
> We should filter out scatter-gather access in 
> vect_enhance_data_refs_alignment.

Looks good, just a couple of very minor details…

> 2020-06-11  Felix Yang  
>
> gcc/
> PR tree-optimization/95570
> * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function.
> (vect_verify_datarefs_alignment): Call it to filter out data 
> references
> in the loop whose alignment is irrelevant.
> (vect_get_peeling_costs_all_drs): Likewise.
> (vect_peeling_supportable): Likewise.
> (vect_enhance_data_refs_alignment): Likewise.
>
> gcc/testsuite/
>
> PR tree-optimization/95570
> * gcc.dg/vect/pr95570.c: New test.
> ---
>  gcc/testsuite/gcc.dg/vect/pr95570.c | 11 
>  gcc/tree-vect-data-refs.c   | 83 -
>  2 files changed, 45 insertions(+), 49 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/pr95570.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr95570.c 
> b/gcc/testsuite/gcc.dg/vect/pr95570.c
> new file mode 100644
> index 000..b9362614004
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr95570.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-march=armv8.2-a+sve -msve-vector-bits=256 
> -mstrict-align -fwrapv" { target aarch64*-*-* } } */
> +
> +int x[8][32];
> +
> +void
> +foo (int start)
> +{
> +  for (int i = start; i < start + 16; i++)
> +x[start][i] = i;
> +}
> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> index 39d5a1b554c..98f6fb76ff9 100644
> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -1129,6 +1129,35 @@ vect_update_misalignment_for_peel (dr_vec_info 
> *dr_info,
>SET_DR_MISALIGNMENT (dr_info, DR_MISALIGNMENT_UNKNOWN);
>  }
>  
> +/* Return TRUE if alignment is relevant for DR_INFO.  */

Just “Return true …“ for new code.  TRUE is a hold-over from C days.

> +static bool
> +vect_relevant_for_alignment_p (dr_vec_info *dr_info)
> +{
> +  stmt_vec_info stmt_info = dr_info->stmt;
> +
> +  if (!STMT_VINFO_RELEVANT_P (stmt_info))
> +return false;
> +
> +  /* For interleaving, only the alignment of the first access matters.  */
> +  if (STMT_VINFO_GROUPED_ACCESS (stmt_info)
> +  && DR_GROUP_FIRST_ELEMENT (stmt_info) != stmt_info)
> +return false;
> +
> +  /* For scatter-gather or invariant accesses, alignment is irrelevant
> + for them.  */

Maybe:

  /* Scatter-gather and invariant accesses continue to address individual
 scalars, so vector-level alignment is irrelevant.  */

Thanks,
Richard


Re: [PATCH/RFC] How to fix PR95440

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

On 6/10/20 4:43 PM, Iain Sandoe wrote:

Hi Jason,

Jason Merrill  wrote:


On Tue, Jun 9, 2020 at 5:04 AM Iain Sandoe  wrote:




 /* Don't bother reversing an operator with two identical parameters.  
*/
-  else if (args->length () == 2 && (flags & LOOKUP_REVERSED))
+  else if (args && args->length () == 2 && (flags & LOOKUP_REVERSED))

The usual pattern is to use vec_safe_length here.  Similarly, in 
build_new_method_call_1 I think

!user_args->is_empty()

should be

vec_safe_is_empty (user_args)

Those changes are OK.


Thanks,

What I applied (after regtest on x86_64-linux/darwin and powerpc64-linux) is 
below.

Given that this fixes an IDE-on-valid filed against 10.1, I’d like to backport 
it for 10.2,
is that OK?


Yes.


thanks
Iain

coroutines: Make call argument handling more robust [PR95440]

build_new_method_call is supposed to be able to handle a null
arguments list pointer (when the method has no parms).  There
were a couple of places where uses of the argument list pointer
were not defended against NULL.

gcc/cp/ChangeLog:

PR c++/95440
* call.c (add_candidates): Use vec_safe_length() for
testing the arguments list.
(build_new_method_call_1): Use vec_safe_is_empty() when
checking for an empty args list.

gcc/testsuite/ChangeLog:

PR c++/95440
* g++.dg/coroutines/pr95440.C: New test.
---
  gcc/cp/call.c |  4 +--
  gcc/testsuite/g++.dg/coroutines/pr95440.C | 39 +++
  2 files changed, 41 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95440.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 3c97b9846e2..b99959f76f9 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5862,7 +5862,7 @@ add_candidates (tree fns, tree first_arg, const vec *args,
}
  
/* Don't bother reversing an operator with two identical parameters.  */

-  else if (args->length () == 2 && (flags & LOOKUP_REVERSED))
+  else if (vec_safe_length (args) == 2 && (flags & LOOKUP_REVERSED))
{
  tree parmlist = TYPE_ARG_TYPES (TREE_TYPE (fn));
  if (same_type_p (TREE_VALUE (parmlist),
@@ -10263,7 +10263,7 @@ build_new_method_call_1 (tree instance, tree fns, 
vec **args,
  && !(flags & LOOKUP_ONLYCONVERTING)
  && cxx_dialect >= cxx20
  && CP_AGGREGATE_TYPE_P (basetype)
- && !user_args->is_empty ())
+ && !vec_safe_is_empty (user_args))
{
  /* Create a CONSTRUCTOR from ARGS, e.g. {1, 2} from <1, 2>.  */
  tree list = build_tree_list_vec (user_args);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr95440.C 
b/gcc/testsuite/g++.dg/coroutines/pr95440.C
new file mode 100644
index 000..8542880d1ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr95440.C
@@ -0,0 +1,39 @@
+#if __has_include()
+#include 
+#else
+#include 
+namespace std { using namespace experimental; }
+#endif
+#if 0
+struct suspend_n {
+  const int x;
+  constexpr suspend_n (int x) : x (x) {}
+  constexpr static bool await_ready() { return false; }
+  constexpr static void await_suspend(std::coroutine_handle<>) {}
+  constexpr static void await_resume() {}
+};
+#endif
+struct task
+{
+  struct promise_type
+  {
+auto get_return_object() const { return task{}; }
+#if 0
+//static constexpr suspend_n initial_suspend()  { return {2}; }
+#endif
+static constexpr std::suspend_always initial_suspend()  { return {}; }
+static constexpr std::suspend_never final_suspend() { return {}; }
+static constexpr void return_void() {}
+static constexpr void unhandled_exception() {}
+  };
+};
+
+task
+test_task ()
+{
+  co_await std::suspend_always{};
+}
+
+auto t = test_task();
+
+int main() {}





[committed] libstdc++: Fix istream::ignore discarding too many chars (PR 94749)

2020-06-11 Thread Jonathan Wakely via Gcc-patches
The current code assumes that if the next character in the stream is
equal to the delimiter then we stopped because we saw that delimiter,
and so discards it.  But in the testcase for the PR we stop because we
reached the maximum number of characters, and it's coincidence that the
next character equals the delimiter. We should not discard the next
character in that case.

The fix is to check that we haven't discarded __n characters already,
instead of checking whether the next character equals __delim. Because
we've already checked for EOF, if we haven't discarded __n yet then we
know we stopped because we saw the delimiter. On the other hand, if the
next character is the delimiter we don't know if that's why we stopped.

PR libstdc++/94749
* include/bits/istream.tcc (basic_istream::ignore(streamsize, CharT)):
Only discard an extra character if we didn't already reach the
maximum number.
* src/c++98/istream.cc (istream::ignore(streamsiz, char))
(wistream::ignore(streamsize, wchar_t)): Likewise.
* testsuite/27_io/basic_istream/ignore/char/94749.cc: New test.
* testsuite/27_io/basic_istream/ignore/wchar_t/94749.cc: New test.

Tested powerpc64le-linux, committed to master.


commit b32eea9c0c25a03e77170675abc4e4bcab6d2b3b
Author: Jonathan Wakely 
Date:   Thu Jun 11 18:41:37 2020 +0100

libstdc++: Fix istream::ignore discarding too many chars (PR 94749)

The current code assumes that if the next character in the stream is
equal to the delimiter then we stopped because we saw that delimiter,
and so discards it.  But in the testcase for the PR we stop because we
reached the maximum number of characters, and it's coincidence that the
next character equals the delimiter. We should not discard the next
character in that case.

The fix is to check that we haven't discarded __n characters already,
instead of checking whether the next character equals __delim. Because
we've already checked for EOF, if we haven't discarded __n yet then we
know we stopped because we saw the delimiter. On the other hand, if the
next character is the delimiter we don't know if that's why we stopped.

PR libstdc++/94749
* include/bits/istream.tcc (basic_istream::ignore(streamsize, 
CharT)):
Only discard an extra character if we didn't already reach the
maximum number.
* src/c++98/istream.cc (istream::ignore(streamsiz, char))
(wistream::ignore(streamsize, wchar_t)): Likewise.
* testsuite/27_io/basic_istream/ignore/char/94749.cc: New test.
* testsuite/27_io/basic_istream/ignore/wchar_t/94749.cc: New test.

diff --git a/libstdc++-v3/include/bits/istream.tcc 
b/libstdc++-v3/include/bits/istream.tcc
index c82da56b9f9..d36374c707f 100644
--- a/libstdc++-v3/include/bits/istream.tcc
+++ b/libstdc++-v3/include/bits/istream.tcc
@@ -601,11 +601,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   if (traits_type::eq_int_type(__c, __eof))
 __err |= ios_base::eofbit;
- else if (traits_type::eq_int_type(__c, __delim))
+ else if (_M_gcount < __n) // implies __c == __delim
{
- if (_M_gcount
- < __gnu_cxx::__numeric_traits::__max)
-   ++_M_gcount;
+ ++_M_gcount;
  __sb->sbumpc();
}
 }
diff --git a/libstdc++-v3/src/c++98/istream.cc 
b/libstdc++-v3/src/c++98/istream.cc
index 79d829e23b4..d6fee1a0d66 100644
--- a/libstdc++-v3/src/c++98/istream.cc
+++ b/libstdc++-v3/src/c++98/istream.cc
@@ -171,11 +171,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
  if (traits_type::eq_int_type(__c, __eof))
__err |= ios_base::eofbit;
- else if (traits_type::eq_int_type(__c, __delim))
+ else if (_M_gcount < __n) // implies __c == __delim
{
- if (_M_gcount
- < __gnu_cxx::__numeric_traits::__max)
-   ++_M_gcount;
+ ++_M_gcount;
  __sb->sbumpc();
}
}
@@ -413,11 +411,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
  if (traits_type::eq_int_type(__c, __eof))
__err |= ios_base::eofbit;
- else if (traits_type::eq_int_type(__c, __delim))
+ else if (_M_gcount < __n) // implies __c == __delim
{
- if (_M_gcount
- < __gnu_cxx::__numeric_traits::__max)
-   ++_M_gcount;
+ ++_M_gcount;
  __sb->sbumpc();
}
}
diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/94749.cc 
b/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/94749.cc
new file mode 100644
index 000..03b5286b00e
--- /dev/null
+++ 

Re: [PATCH] Fix few -Wformat-diag warnings.

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

On 6/11/20 2:43 AM, Martin Liška wrote:

Ready for master?


I've been meaning to do some of this to resolve pr94982.  Thanks
a lot for taking the lead on it!  I'll try to get to it soon!

Martin



Thanks,
Martin

gcc/ChangeLog:

 * cgraphunit.c (process_symver_attribute): Wrap weakref keyword.
 * dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new
 line.
 * lto-wrapper.c (merge_and_complain): Wrap option names.
---
  gcc/cgraphunit.c  |  2 +-
  gcc/dbgcnt.c  |  2 +-
  gcc/lto-wrapper.c | 12 ++--
  3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 01b3f82a4b2..ea9a34bda6f 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -762,7 +762,7 @@ process_symver_attribute (symtab_node *n)
    if (n->weakref)
  {
    error_at (DECL_SOURCE_LOCATION (n->decl),
-    "weakref cannot be versioned");
+    "% cannot be versioned");
    return;
  }
    if (!TREE_PUBLIC (n->decl))
diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c
index b0dd893ed49..ae98a281d63 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -126,7 +126,7 @@ dbg_cnt_set_limit_by_index (enum debug_counter 
index, const char *name,

    if (t1.first <= t2.second)
  {
    error ("Interval overlap of %<-fdbg-cnt=%s%>: [%u, %u] and "
- "[%u, %u]\n", name, t2.first, t2.second, t1.first, t1.second);
+ "[%u, %u]", name, t2.first, t2.second, t1.first, t1.second);
    return false;
  }
  }
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index d565b0861f5..8fbca7cabc4 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -508,24 +508,24 @@ merge_and_complain (struct cl_decoded_option 
**decoded_options,

    break;
  else if (i < *decoded_options_count && j == fdecoded_options_count)
    {
-    warning (0, "Extra option to -Xassembler: %s,"
- " dropping all -Xassembler and -Wa options.",
+    warning (0, "Extra option to %<-Xassembler%>: %s,"
+ " dropping all %<-Xassembler%> and %<-Wa%> options.",
   (*decoded_options)[i].arg);
  xassembler_options_error = true;
  break;
    }
  else if (i == *decoded_options_count && j < fdecoded_options_count)
    {
-    warning (0, "Extra option to -Xassembler: %s,"
- " dropping all -Xassembler and -Wa options.",
+    warning (0, "Extra option to %<-Xassembler%>: %s,"
+ " dropping all %<-Xassembler%> and %<-Wa%> options.",
   fdecoded_options[j].arg);
  xassembler_options_error = true;
  break;
    }
  else if (strcmp ((*decoded_options)[i].arg, fdecoded_options[j].arg))
    {
-    warning (0, "Options to Xassembler do not match: %s, %s,"
- " dropping all -Xassembler and -Wa options.",
+    warning (0, "Options to %<-Xassembler%> do not match: %s, %s,"
+ " dropping all %<-Xassembler%> and %<-Wa%> options.",
   (*decoded_options)[i].arg, fdecoded_options[j].arg);
  xassembler_options_error = true;
  break;




Re: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi Harald,

one remark: Instead of

+ && !(strcmp(gfc_current_intrinsic, "associated") == 0
+   || strcmp(gfc_current_intrinsic, "null") == 0
+   || strcmp(gfc_current_intrinsic, "present") == 0))

could you maybe test sym->id ?

OK with that change.

Best regards, and thanks for the patch

Thomas


Re: [PATCH] diagnostics: Add options to control the column units [PR49973] [PR86904]

2020-06-11 Thread Lewis Hyatt via Gcc-patches
On Wed, Jun 10, 2020 at 12:11:00PM -0400, David Malcolm wrote:
> Thanks for the patch; sorry about the delay in reviewing it.
> 
> Some high-level review points
> 
> - I like the patch overall
> 
> - This will deserve an item in the release notes
> 
> - I don't like adding "global_tabstop" (I don't like global
> variables).  Is there nowhere else we can handle this? I believe
> there's a cluster of functions in the callgraph that make use of
> it; can we simply pass around the tabstop value instead?  "tabstop"
> seems to have several meanings.  If I'm reading the patch correctly
>   * "tabstop > 0" means to expand tabs so that column numbers are a
> multiple of tabstop
>   * "tabstop == 0" means "don't expand tabs"
>   * "tabstop < 0" in some places means: use the global_tabstop value
> Is it possible to eliminate global_tabstop value?  Or is there some
> deep reason I'm missing?
> 
> I'll do a more thorough review once that's addressed/resolved (since
> eliminating global_tabstop might touch a few places).
>

Thanks for the feedback! The attached updated patch addresses these
concerns. Regarding tabstop, I have removed the new static variable
global_tabstop in charset.c. FWIW, the usage of "tabstop" arguments in the
various new APIs did previously work a bit more consistently than you
described. In all cases "tabstop <= 0" meant to use the default value,
otherwise it specified the tabstop to use (with tabstop=1 naturally
restoring the old behavior of changing tabs to a single space). In order
for libcpp to provide this feature (callers can pass tabstop <= 0 to get a
default, and the default can in turn by configured when processing the
-ftabstop option), it does need to remember the default, and this has to
be a file-level static variable because the routines need to work
independent of any cpp_reader instance. (Some frontends don't use
libcpp to read their input, for instance.) Anyway, I see the point that
this file-level static, being accessible with cpp_set_tabstop() and
cpp_get_tabstop(), is effectively just a global variable, so I have
removed this feature, which just means that all callers need to pass the
tabstop they want to use. I am now rather using the diagnostic_context
object to remember the value passed to -ftabstop. The only place this
involves global variables is now in c-family/c-indentation.c, where if I
understood correctly, the only diagnostic_context available is global_dc,
so I am getting the tabstop value from there. Please let me know if
there's a better way to handle that? Prior to my patch, the tabstop was
obtained from a different global variable (extern cpp_options *cpp_opts),
so at least conservation of total globals is maintained. :)

Compared to the previous version, this one is a bit longer, since 25 or
so call sites had to be modified to know the value of -ftabstop. Most of
the churn is in diagnostic-show-locus.c, because there are a fair number of
static helper functions and helper classes there, which just needed to
receive the diagnostic_context object from their callers. I could
have made this simpler by letting the tabstop argument default to
something like 8 in all functions that require it... this would remove the
need to pass it in all the selftests that are indifferent to it. I figured
it would be better to force this argument to be passed, though, or else in
the future it may be easy to forget to pass it where it is needed. 

> Thanks for adding docs; some nits on them:
> 
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> 
> [...snip...]
> 
> > +@item -fdiagnostics-column-unit=@var{UNIT}
> > +@opindex fdiagnostics-column-unit
> > +Select the units for the column number.  This affects traditional 
> > diagnostics
> > +(in the absence of @option{-fno-show-column}), as well as JSON format
> > +diagnostics if requested.
> > +
> > +The default @var{UNIT}, @samp{display}, considers the number of display 
> > columns
> > +occupied by each character.  This may be larger than the number of bytes
> > +occupied, in the case of tab characters, or it may be smaller, in the case 
> > of
> > +multibyte characters.  For example, the UTF-8 character ``@U{03C0}'' 
> > occupies
> > +two bytes and one display column, while the character ``@U{1F642}'' 
> > occupies
> > +four bytes and two display columns.
> 
> This is imprecise.  A unicode code point occupies some number of display 
> columns,
> and its *UTF-8 encoding* occupies some number of bytes.
> 
> [and my inner pedant is now thinking: what about combining diacritics? 
> But I don't think we can ever issue a diagnostic on a diacritic; I
> *think* we only ever care about the per-glyph level]
> 
> > +Setting @var{UNIT} to @samp{byte} changes the column number to the
> raw byte
> > +count in all cases, as was traditionally output by GCC prior to version 
> > 11.1.0.
> > +
> > +@item -fdiagnostics-column-origin=@var{ORIGIN}
> > +@opindex fdiagnostics-column-origin
> > +Select the origin for column numbers, i.e. the 

Re: [patch] Fix ICE in verify_sra_access_forest

2020-06-11 Thread Eric Botcazou
> I am not very good at reasoning about reverse storage order stuff but
> can it ever happen that this reverse is not the same as racc->reverse?
> 
> In order for that to happen, we'd have to have an assignment (folded
> memcpy?) between two aggregates in the original code that, at the same
> offset within the aggregates, have single field structures with
> different storage orders.  If that has undefined behavior (even for
> folded memcpy), I guess I am fine with the patch (but I cannot approve
> it).  If not, I'd add a condition that racc->reverse is the same as
> TYPE_REVERSE_STORAGE_ORDER(lacc->type) to the if guarding all of this.

You cannot do an assignment between aggregates with opposite storage order, 
that's rejected by the compiler (both in C and Ada); generally speaking, such 
types are not compatible, even if they have the same underlying structure.
We also block SRA when there is a VIEW_CONVERT_EXPR messing with the storage 
order, see calls to storage_order_barrier_p.  And folding a memcpy between 
them should be invalid too (see e.g. the ongoing discussion with Richard in an 
earlier thread).  So my understanding is that you don't need the condition.

-- 
Eric Botcazou


[PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494

2020-06-11 Thread Harald Anlauf
> Gesendet: Montag, 08. Juni 2020 um 22:25 Uhr
> Von: "Harald Anlauf" 
> An: "fortran" , "gcc-patches" 
> Betreff: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at 
> fortran/trans-decl.c:494

OK, now with a brown bag over my head, here comes the patch instead of
just the testcase.

Thanks to Thomas for pointing that out in private.

Harald
diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 0afb96c0414..148a3269815 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -1431,8 +1431,8 @@ gfc_check_x_yd (gfc_expr *x, gfc_expr *y)
   return true;
 }

-static bool
-invalid_null_arg (gfc_expr *x)
+bool
+gfc_invalid_null_arg (gfc_expr *x)
 {
   if (x->expr_type == EXPR_NULL)
 {
@@ -1451,7 +1451,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
   int i;
   bool t;

-  if (invalid_null_arg (pointer))
+  if (gfc_invalid_null_arg (pointer))
 return false;

   attr1 = gfc_expr_attr (pointer);
@@ -1477,7 +1477,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
   if (target == NULL)
 return true;

-  if (invalid_null_arg (target))
+  if (gfc_invalid_null_arg (target))
 return false;

   if (target->expr_type == EXPR_VARIABLE || target->expr_type == EXPR_FUNCTION)
@@ -3374,7 +3374,7 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_expr *status)
 bool
 gfc_check_kind (gfc_expr *x)
 {
-  if (invalid_null_arg (x))
+  if (gfc_invalid_null_arg (x))
 return false;

   if (gfc_bt_struct (x->ts.type) || x->ts.type == BT_CLASS)
@@ -3453,6 +3453,9 @@ gfc_check_len_lentrim (gfc_expr *s, gfc_expr *kind)
   if (!type_check (s, 0, BT_CHARACTER))
 return false;

+  if (gfc_invalid_null_arg (s))
+return false;
+
   if (!kind_check (kind, 1, BT_INTEGER))
 return false;
   if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic "
@@ -4138,10 +4141,10 @@ gfc_check_transf_bit_intrins (gfc_actual_arglist *ap)
 bool
 gfc_check_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask)
 {
-  if (invalid_null_arg (tsource))
+  if (gfc_invalid_null_arg (tsource))
 return false;

-  if (invalid_null_arg (fsource))
+  if (gfc_invalid_null_arg (fsource))
 return false;

   if (!same_type_check (tsource, 0, fsource, 1))
@@ -5061,7 +5064,7 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind)
 {
   gfc_array_ref *ar;

-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   if (source->rank == 0 || source->expr_type != EXPR_VARIABLE)
@@ -5146,7 +5149,7 @@ gfc_check_size (gfc_expr *array, gfc_expr *dim, gfc_expr *kind)
 bool
 gfc_check_sizeof (gfc_expr *arg)
 {
-  if (invalid_null_arg (arg))
+  if (gfc_invalid_null_arg (arg))
 return false;

   if (arg->ts.type == BT_PROCEDURE)
@@ -5634,7 +5637,7 @@ gfc_check_sngl (gfc_expr *a)
 bool
 gfc_check_spread (gfc_expr *source, gfc_expr *dim, gfc_expr *ncopies)
 {
-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   if (source->rank >= GFC_MAX_DIMENSIONS)
@@ -6167,7 +6170,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size)
   size_t source_size;
   size_t result_size;

-  if (invalid_null_arg (source))
+  if (gfc_invalid_null_arg (source))
 return false;

   /* SOURCE shall be a scalar or array of any type.  */
@@ -6186,7 +6189,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size)
   if (mold->ts.type == BT_BOZ && illegal_boz_arg (mold))
 return false;

-  if (invalid_null_arg (mold))
+  if (gfc_invalid_null_arg (mold))
 return false;

   /* MOLD shall be a scalar or array of any type.  */
@@ -6412,6 +6415,9 @@ gfc_check_trim (gfc_expr *x)
   if (!type_check (x, 0, BT_CHARACTER))
 return false;

+  if (gfc_invalid_null_arg (x))
+return false;
+
   if (!scalar_check (x, 0))
 return false;

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 0ef7b1b0eff..6d76efb5298 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3553,6 +3553,7 @@ bool gfc_calculate_transfer_sizes (gfc_expr*, gfc_expr*, gfc_expr*,
 bool gfc_boz2int (gfc_expr *, int);
 bool gfc_boz2real (gfc_expr *, int);
 bool gfc_invalid_boz (const char *, locus *);
+bool gfc_invalid_null_arg (gfc_expr *);


 /* class.c */
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index 17f5efc6566..95150c8b6ce 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -4442,6 +4442,18 @@ check_arglist (gfc_actual_arglist **ap, gfc_intrinsic_sym *sym,
 	  return false;
 	}

+  /* F2018, p. 328: An argument to an intrinsic procedure other than
+	 ASSOCIATED, NULL, or PRESENT shall be a data object.  An EXPR_NULL
+	 is not a data object.  */
+  if (actual->expr->expr_type == EXPR_NULL
+	  && !(strcmp(gfc_current_intrinsic, "associated") == 0
+		|| strcmp(gfc_current_intrinsic, "null") == 0
+		|| strcmp(gfc_current_intrinsic, "present") == 0))
+	{
+	  gfc_invalid_null_arg (actual->expr);
+	  return false;
+	}
+
   /* If the formal 

Re: BRIG FE testsuite: Fix all dump-scans (Was: Re: drop -aux{dir, base}, revamp -dump{dir, base})

2020-06-11 Thread Martin Jambor
Hi Mike,

On Tue, Jun 09 2020, Mike Stump wrote:
> I think I'd prefer the fix on the other side, if reasonable.  I'd give
> them some time to see about a fix there before selecting this patch.

given Alexandre's email, are you OK with the patch?  It essentially
manually keeps the input name "rootname" in sync with the output file
"rootname" which is the new basis for dump names.  As Alexandre noted,
the proper fix is probably to teach the testsuite to expect dump names
based on outputs by default but I do not want to leave the majority of
BRIG testcases broken until that happens.

Thanks,

Martin


>
> On Jun 9, 2020, at 5:42 AM, Martin Jambor  wrote:

[...]

>> 
>> 
>> I looked into the issue yesterday and decided the simplest fix is
>> probably the following.  I am going to use my BRIG maintainer hat to
>> commit the patch in a day or two unless someone thinks it is a bad idea.
>> Tested by running make check-brig on an x86_64-linux.
>> 
>> Martin
>> 
>> 
>> 
>> Since Alexandre's revamp of dump files handling in
>> r11-627-g1dedc12d186, BRIG FE has been receiving slightly different
>> -dumpbase (e.g. smoke_test.brig instead of smoke_test.hsail.brig when
>> compiling file smoke_test.hsail.brig) and the testsuite then could not
>> find the generated dump files it wanted to scan.  I have not really
>> looked into why that changed, the easiest fix seems to me to remove
>> the hsail part already when generating the binary brig file from the
>> textual HSAIL representation.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 2020-06-09  Martin Jambor  
>> 
>>  * lib/brig.exp (brig_target_compile): Strip hsail extension when
>>  gnerating the name of the binary brig file.
>> ---
>> gcc/testsuite/lib/brig.exp | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/gcc/testsuite/lib/brig.exp b/gcc/testsuite/lib/brig.exp
>> index fbfb1da947a..de47f13e42c 100644
>> --- a/gcc/testsuite/lib/brig.exp
>> +++ b/gcc/testsuite/lib/brig.exp
>> @@ -29,7 +29,7 @@ proc brig_target_compile { source dest type options } {
>>  # We cannot assume all inputs are .hsail as the dg machinery
>>  # calls this for a some c files to check linker plugin support or
>>  # similar.
>> -set brig_source ${tmpdir}/[file tail ${source}].brig
>> +set brig_source ${tmpdir}/[file rootname [file tail ${source}]].brig
>>  exec HSAILasm $source -o ${brig_source}
>>  set source ${brig_source}
>>  # Change the testname the .brig.
>> -- 
>> 2.26.2
>> 


Re: [patch] Fix ICE in verify_sra_access_forest

2020-06-11 Thread Martin Jambor
Hi,

On Thu, Jun 11 2020, Eric Botcazou wrote:
> Hi,
>
> this fixes an issue with reverse storage order in SRA, which is caught by the 
> built-in verifier:
>
> ===GNAT BUG DETECTED==+
> | 11.0.0 20200610 (experimental) (x86_64-suse-linux) GCC error:|
> | in verify_sra_access_forest, at tree-sra.c:2359  
>
>  gcc_assert (reverse == access->reverse);
>
> The problem is that propagate_subaccesses_from_rhs changes the type of an 
> access from aggregate to scalar and, as discussed elsewhere, this must be 
> done 
> with extra care in the presence of reverse storage order.
>
> Tested on x86-64/Linux, OK for the mainline?
>
>
> 2020-06-11  Eric Botcazou  
>
>   * tree-sra.c (propagate_subaccesses_from_rhs): When a non-scalar
>   access on the LHS is replaced with a scalar access, propagate the
>   TYPE_REVERSE_STORAGE_ORDER flag of the type of the original access.
>
>
> 2020-06-11  Eric Botcazou  
>
>   * gnat.dg/opt85.ad[sb]: New test.
>
> -- 
> Eric Botcazou
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index 4793b48f32c..fcba7fbdd31 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -2758,6 +2758,9 @@ propagate_subaccesses_from_rhs (struct access *lacc, 
> struct access *racc)
>   }
>if (!lacc->first_child && !racc->first_child)
>   {
> +   /* We are about to change the access type from aggregate to scalar,
> +  so we need to put the reverse flag onto the access, if any.  */
> +   const bool reverse = TYPE_REVERSE_STORAGE_ORDER (lacc->type);

I am not very good at reasoning about reverse storage order stuff but
can it ever happen that this reverse is not the same as racc->reverse?

In order for that to happen, we'd have to have an assignment (folded
memcpy?) between two aggregates in the original code that, at the same
offset within the aggregates, have single field structures with
different storage orders.  If that has undefined behavior (even for
folded memcpy), I guess I am fine with the patch (but I cannot approve
it).  If not, I'd add a condition that racc->reverse is the same as
TYPE_REVERSE_STORAGE_ORDER(lacc->type) to the if guarding all of this.

I tried to come up with a (C) testcase for this but just could not
trigger even this condition reasonably quickly.

Thanks for looking into this,

Martin



Re: [PATCH, PR fortran/95503] [9/10/11 Regression] ICE in gfc_is_simply_contiguous, at fortran/expr.c:5844

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi Harald,


The following patch fixes an almost obvious ICE in invalid.

Regtested on x86_64-pc-linux-gnu.

OK for master, and backports to 9/10?


OK.  Thanks for the patch!

Regards

Thomas


Re: [Patch, fortran] PR fortran/95331 - Unlimited polymorphic arrays have wrong bounds

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi Jose,

Proposed patch to PR95331 - Unlimited polymorphic arrays have wrong bounds.

Patch tested only on x86_64-pc-linux-gnu.


reviewed, ChangeLog reformatted, committed as
r11-1235-g2ee70f5d161edd99a7af97d166b251bcf83cd91b .

Thanks a lot for the patch!

Do you have interest in getting write access?

Regards

Thomas

PR95331 - Unlimited polymorphic arrays have wrong bounds.

When iterating over a class array use the bounds provided by the
transformed descriptor (in sym->backend_decl) instead of the original
bounds of the array (in the descriptor passed in the class _data)
which are passed in se->expr.

The patch partially depends on the patch for PR52351 and PR85868, but
does not seems to break anything by itself.

gcc/fortran/ChangeLog:

2020-06-11  José Rui Faustino de Sousa  

PR fortran/95331
* trans-array.c (gfc_conv_array_ref): For class array dummy
arguments use the transformed descriptor in sym->backend_decl
instead of the original descriptor.

gcc/testsuite/ChangeLog:

2020-06-11  José Rui Faustino de Sousa  

PR fortran/95331
* gfortran.dg/PR95331.f90: New test.


Re: [PATCH] rs6000: skip debug info statements

2020-06-11 Thread Jakub Jelinek via Gcc-patches
On Thu, Jun 11, 2020 at 01:49:57PM +0200, Martin Liška wrote:
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 0bbd06ad1de..00daf979856 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data)
>for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
>   {
> gimple *stmt = gsi_stmt (gsi);
> +   if (is_gimple_debug (stmt))
> +   continue;

Formatting, continue; indented too much.

Jakub



Re: [PATCH] rs6000: skip debug info statements

2020-06-11 Thread Richard Biener via Gcc-patches
On June 11, 2020 1:49:57 PM GMT+02:00, "Martin Liška"  wrote:
>Hi.
>
>Since stmt_vec_info is not generated for debug info statements, these
>needs to
>be skipped in rs6000_density_test.
>
>Patch can bootstrap on ppc64le-linux-gnu and survives regression tests.
>
>Ready to be installed?

OK. 

Richard. 

>Thanks,
>Martin
>
>gcc/ChangeLog:
>
>   PR target/95627
>   * config/rs6000/rs6000.c (rs6000_density_test): Skip debug
>   statements.
>---
>  gcc/config/rs6000/rs6000.c | 3 +++
>  1 file changed, 3 insertions(+)
>
>diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>index 0bbd06ad1de..00daf979856 100644
>--- a/gcc/config/rs6000/rs6000.c
>+++ b/gcc/config/rs6000/rs6000.c
>@@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data)
>   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
>   {
> gimple *stmt = gsi_stmt (gsi);
>+if (is_gimple_debug (stmt))
>+continue;
>+
> stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (stmt);
>  
> if (!STMT_VINFO_RELEVANT_P (stmt_info)



Re: [Patch, fortran] PR fortran/52351, 85868 Wrong array section bounds when passing to an intent-in pointer dummy

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi Jose,

Proposed patch to PRs 52351, 85868 Wrong array section bounds when 
passing to an intent-in pointer dummy.


First, thanks for working on this and for this patch.

Regarding the patch, there are a few style issues which I fixed
for the commit.  If you could try to adhere to a few more of them,
I'd be obliged :-)

Regarding the ChangeLog: That format is rather rigid, see
https://gcc.gnu.org/codingconventions.html#ChangeLogs . Also,
a script run on the server checks all the ChangeLogs for correct
formats.  I usually get this right on my second or third attempt :-)

Regarding the patch itself: There were a few whitespace issues that
could be corrected easily (git diff shows them up bright red).

Reviewed, regression-tested and committed as
r11-1230-g2ff0f48819c8a7ed5d7c03e2bfc02e5907e2ff1a .

Thanks a lot for fixing this!

Regards

Thomas

Here is the ChangeLog of what I committed:

Wrong array section bounds when passing to an intent-in pointer dummy.

Add code to allow for the creation a new descriptor for array
sections with the correct one based indexing.

Rework the generated descriptors indexing (hopefully) fixing the
wrong offsets generated.

gcc/fortran/ChangeLog:

2020-06-11  José Rui Faustino de Sousa  

PR fortran/52351
PR fortran/85868
* trans-array.c (gfc_conv_expr_descriptor): Enable the
creation of a new descriptor with the correct one based
indexing for array sections.  Rework array descriptor
indexing offset calculation.

gcc/testsuite/ChangeLog:

2020-06-11  José Rui Faustino de Sousa  

PR fortran/52351
PR fortran/85868
* gfortran.dg/coarray_lib_comm_1.f90: Adjust match test for
the newly generated descriptor.
* gfortran.dg/PR85868A.f90: New test.
* gfortran.dg/PR85868B.f90: New test.


Re: [PR95416] outputs.exp: skip lto tests when not using linker plugin

2020-06-11 Thread Rainer Orth
Hi Alexandre,

> On Jun  9, 2020, Rainer Orth  wrote:
>
>> this is wrong unfortunately: braces are the Tcl equivalent of single
>> quotes so you're setting skip_lto to the string inside.
>
> Aah, thanks.  So when $skip_lto is expanded in the ifs, the whole thing
> is evaluated, and that's why it works anyway?

but it didn't work: when I tried the patch on i386-pc-solaris2.11 with
as/ld (i.e. no linker plugin}, the lto tests of outputs.exp were still
run, and check_linker_plugin_available wasn't even run at all, what
quite confused me at first.  As I'd mentioned in the PR, you can easily
test an equivalent configuration with gld when configuring gcc with
--disable-lto-plugin.

>> While this worked for me on i386-pc-solaris2.11 (both with ld, thus
>> without the lto plugin, and with gld and the plugin), I wonder if
>> there's not a better way than just skipping the lto tests,
>
> My initial focus was on relieving the pain by removing the symptoms, so
> I can then work on the improvements with lower urgency.

Fully understood: as I'd mentioned, that's mine as well.  We already
accumulate all too many testsuite failures on master...

> Here's what I'd like to check in for now:
>
> [PR95416] outputs.exp: skip lto tests when not using linker plugin
>
> From: Alexandre Oliva 
>
> When the linker plugin is not available, dump outputs in lto runs are
> different from those outputs.exp tests expect, so skip them for now.
>
> Regstrapped on x86_64-linux-gnu.  Ok to install?
>
>
> for  gcc/testsuite/ChangeLog
>
>   * outputs.exp (skip_lto): Set when missing the linker plugin.

Ok, thanks.

Rainer

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


[PATCH PR94274] fold phi whose incoming args are defined from binary

2020-06-11 Thread Zhanghaijian (A)
Hi,

This is a experimental fix for pr94274.
For if/else structure, when the expressions is binary operation and have a 
common subexpression, and the opcode is the same, we can
fold the merging phi node in tree_ssa_phiopt_worker (ssa-phiopt). It can be 
optimized to do csel first and then do binary operations.
This can eliminate one or more instructions. And this patch is effective for 
500.perlbench_r in spec2017.
Bootstrap and tested on aarch64/x86_64 Linux platform. No new regression 
witnessed.

Any suggestion?  

Thanks,
Haijian Zhang


pr94274-v1.diff
Description: pr94274-v1.diff


Re: [Patch, fortran] PR fortran/94022 - Array slices of assumed-size arrays

2020-06-11 Thread Thomas Koenig via Gcc-patches

Hi Jose,



Proposed patch to Bug 94022 - Array slices of assumed-size arrays.

Patch tested only on x86_64-pc-linux-gnu.


Reviewed, regression-tested and commited as
r11-1228-g6a07010b774cb5a0b1790b857e69d3d8534eebd2 .

Thanks for the patch!

Regards

Thomas


Re: [PATCH] git_update_version: add --current argument.

2020-06-11 Thread Jakub Jelinek via Gcc-patches
On Thu, Jun 11, 2020 at 01:47:30PM +0200, Martin Liška wrote:
> The argument can be useful to update arbitrary branch, the changes
> are added to git index and user is supposed to make a commit.
> 
> I'm going to install it if there are no objections.
> 
> Martin
> 
> contrib/ChangeLog:
> 
>   * gcc-changelog/git_update_version.py: Add --curent argument.

LGTM, thanks.

Jakub



(patch] Optimize assignment to volatile aggregate variable

2020-06-11 Thread Eric Botcazou
Hi,

gimplify_modify_expr_rhs has an optimization whereby the assignment to an 
aggregate variable from a read-only object with a DECL_INITIAL is optimized 
into the direct assignment of the DECL_INITIAL, provided that no temporary is 
created in the process.

The optimization is blocked if the read-only object is volatile, which is OK 
as per the semantics of volatile, but also if the target variable is volatile, 
on the grounds that the modified assignment might end up being done on a per 
field basis, which is also OK.  But this latter restriction is enforced a 
priori and there are cases where the modified assignment would be OK, for 
example if there is only one field or the DECL_INITIAL is equivalent to the 
empty CONSTRUCTOR, i.e. all zeros.

So, in the latter case, the patch changes gimplify_modify_expr_rhs to ask 
gimplify_init_constructor whether the assignment would be done as a block,
which is easy because gimplify_init_constructor knows that it must create a 
temporary if the LHS is volatile and this would not be the case, so it's just 
a matter of completing the NOTIFY_TEMP_CREATION mechanism for this case.

Tested on x86-64/Linux, OK for the mainline?


2020-06-11  Eric Botcazou  

* gimplify.c (gimplify_init_constructor) : Declare new
ENSURE_SINGLE_ACCESS constant and move variables down.  If it is true
and all elements are zero, then always clear.  Return GS_ERROR if a
temporary would be created for it and NOTIFY_TEMP_CREATION is set.
(gimplify_modify_expr_rhs) : If the target is volatile but
the type is aggregate non-addressable, ask gimplify_init_constructor
whether it can generate a single access to the target.


2020-06-11  Eric Botcazou  

* gnat.dg/aggr30.adb[sb]: New test.

-- 
Eric Botcazouwith Interfaces;

package Aggr30 is

   type Data_Type is array (1 .. 4) of Interfaces.Unsigned_8;

   type Padding_Type is array (5 .. 4096) of Interfaces.Unsigned_8;

   type Rec is record
  Data: Data_Type;
  Padding : Padding_Type;
   end record;

   procedure Init;

   procedure Init_Volatile;

private

   Instance : Rec;

   Instance_Volatile : Rec;
   pragma Volatile (Instance_Volatile);

end Aggr30;
-- { dg-do compile }
-- { dg-options "-fdump-tree-gimple" }

package body Aggr30 is

   Null_Constant : constant Rec := (Data => (others => 0),
Padding => (others => 0));
   procedure Init is
   begin
  Instance := Null_Constant;
   end;

   procedure Init_Volatile is
   begin
  Instance_Volatile := Null_Constant;
   end;

end Aggr30;

-- { dg-final { scan-tree-dump-times "= {}" 2 "gimple"} }
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index e14932fafaf..ff2be7b4fc4 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4865,10 +4865,6 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 case QUAL_UNION_TYPE:
 case ARRAY_TYPE:
   {
-	struct gimplify_init_ctor_preeval_data preeval_data;
-	HOST_WIDE_INT num_ctor_elements, num_nonzero_elements;
-	HOST_WIDE_INT num_unique_nonzero_elements;
-	bool cleared, complete_p, valid_const_initializer;
 	/* Use readonly data for initializers of this or smaller size
 	   regardless of the num_nonzero_elements / num_unique_nonzero_elements
 	   ratio.  */
@@ -4876,6 +4872,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	/* If num_nonzero_elements / num_unique_nonzero_elements ratio
 	   is smaller than this, use readonly data.  */
 	const int unique_nonzero_ratio = 8;
+	/* True if a single access of the object must be ensured.  This is the
+	   case if the target is volatile, the type is non-addressable and more
+	   than one field need to be assigned.  */
+	const bool ensure_single_access
+	  = TREE_THIS_VOLATILE (object)
+	&& !TREE_ADDRESSABLE (type)
+	&& vec_safe_length (elts) > 1;
+	struct gimplify_init_ctor_preeval_data preeval_data;
+	HOST_WIDE_INT num_ctor_elements, num_nonzero_elements;
+	HOST_WIDE_INT num_unique_nonzero_elements;
+	bool cleared, complete_p, valid_const_initializer;
 
 	/* Aggregate types must lower constructors to initialization of
 	   individual elements.  The exception is that a CONSTRUCTOR node
@@ -4914,6 +4921,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  {
 	if (notify_temp_creation)
 	  return GS_ERROR;
+
 	DECL_INITIAL (object) = ctor;
 	TREE_STATIC (object) = 1;
 	if (!DECL_NAME (object))
@@ -4961,6 +4969,10 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  /* If there are "lots" of zeros, it's more efficient to clear
 	 the memory and then set the nonzero elements.  */
 	  cleared = true;
+	else if (ensure_single_access && num_nonzero_elements == 0)
+	  /* If a single access to the target must be ensured and all elements
+	 are zero, then it's optimal to clear whatever their number.  */
+	  cleared = true;
 	else

[PATCH] rs6000: skip debug info statements

2020-06-11 Thread Martin Liška

Hi.

Since stmt_vec_info is not generated for debug info statements, these needs to
be skipped in rs6000_density_test.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

PR target/95627
* config/rs6000/rs6000.c (rs6000_density_test): Skip debug
statements.
---
 gcc/config/rs6000/rs6000.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0bbd06ad1de..00daf979856 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data)
   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
{
  gimple *stmt = gsi_stmt (gsi);
+ if (is_gimple_debug (stmt))
+ continue;
+
  stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (stmt);
 
 	  if (!STMT_VINFO_RELEVANT_P (stmt_info)

--
2.26.2



[PATCH] git_update_version: add --current argument.

2020-06-11 Thread Martin Liška

The argument can be useful to update arbitrary branch, the changes
are added to git index and user is supposed to make a commit.

I'm going to install it if there are no objections.

Martin

contrib/ChangeLog:

* gcc-changelog/git_update_version.py: Add --curent argument.
---
 contrib/gcc-changelog/git_update_version.py | 106 +++-
 1 file changed, 59 insertions(+), 47 deletions(-)

diff --git a/contrib/gcc-changelog/git_update_version.py 
b/contrib/gcc-changelog/git_update_version.py
index 6b6ccf68a5e..733a1a0f14a 100755
--- a/contrib/gcc-changelog/git_update_version.py
+++ b/contrib/gcc-changelog/git_update_version.py
@@ -66,60 +66,72 @@ parser.add_argument('-d', '--dry-mode',
 help='Generate patch for ChangeLog entries and do it'
  ' even if DATESTAMP is unchanged; folder argument'
  ' is expected')
+parser.add_argument('-c', '--current', action='store_true',
+help='Modify current branch (--push argument is ignored)')
 args = parser.parse_args()
 
 repo = Repo(args.git_path)

 origin = repo.remotes['origin']
 
-for ref in origin.refs:

-assert ref.name.startswith('origin/')
-name = ref.name[len('origin/'):]
-if name in active_refs:
-if name in repo.branches:
-branch = repo.branches[name]
+
+def update_current_branch():
+commit = repo.head.commit
+commit_count = 1
+while commit:
+if (commit.author.email == 'gccad...@gcc.gnu.org'
+and commit.message.strip() == 'Daily bump.'):
+break
+commit = commit.parents[0]
+commit_count += 1
+
+print('%d revisions since last Daily bump' % commit_count)
+datestamp_path = os.path.join(args.git_path, 'gcc/DATESTAMP')
+if (read_timestamp(datestamp_path) != current_timestamp
+or args.dry_mode or args.current):
+commits = parse_git_revisions(args.git_path, '%s..HEAD'
+  % commit.hexsha)
+for git_commit in reversed(commits):
+prepend_to_changelog_files(repo, args.git_path, git_commit,
+   not args.dry_mode)
+if args.dry_mode:
+diff = repo.git.diff('HEAD')
+patch = os.path.join(args.dry_mode,
+ branch.name.split('/')[-1] + '.patch')
+with open(patch, 'w+') as f:
+f.write(diff)
+print('branch diff written to %s' % patch)
+repo.git.checkout(force=True)
 else:
-branch = repo.create_head(name, ref).set_tracking_branch(ref)
-print('=== Working on: %s ===' % branch, flush=True)
-origin.pull(rebase=True)
-branch.checkout()
-print('branch pulled and checked out')
-assert not repo.index.diff(None)
-commit = branch.commit
-commit_count = 1
-while commit:
-if (commit.author.email == 'gccad...@gcc.gnu.org'
-and commit.message.strip() == 'Daily bump.'):
-break
-commit = commit.parents[0]
-commit_count += 1
-
-print('%d revisions since last Daily bump' % commit_count)
-datestamp_path = os.path.join(args.git_path, 'gcc/DATESTAMP')
-if (read_timestamp(datestamp_path) != current_timestamp
-or args.dry_mode):
-commits = parse_git_revisions(args.git_path, '%s..HEAD'
-  % commit.hexsha)
-for git_commit in reversed(commits):
-prepend_to_changelog_files(repo, args.git_path, git_commit,
-   not args.dry_mode)
-if args.dry_mode:
-diff = repo.git.diff('HEAD')
-patch = os.path.join(args.dry_mode,
- branch.name.split('/')[-1] + '.patch')
-with open(patch, 'w+') as f:
-f.write(diff)
-print('branch diff written to %s' % patch)
-repo.git.checkout(force=True)
-else:
-# update timestamp
-print('DATESTAMP will be changed:')
-with open(datestamp_path, 'w+') as f:
-f.write(current_timestamp)
-repo.git.add(datestamp_path)
+# update timestamp
+print('DATESTAMP will be changed:')
+with open(datestamp_path, 'w+') as f:
+f.write(current_timestamp)
+repo.git.add(datestamp_path)
+if not args.current:
 repo.index.commit('Daily bump.')
 if args.push:
 repo.git.push('origin', branch)
 print('branch is pushed')
-else:
-print('DATESTAMP unchanged')
-print('branch is done\n', flush=True)
+else:
+print('DATESTAMP unchanged')
+
+
+if args.current:
+   

RE: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning

2020-06-11 Thread Yangfei (Felix)
Hi,

> -Original Message-
> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
> Sent: Thursday, June 11, 2020 12:23 AM
> To: Yangfei (Felix) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR95570] vect: ICE: Segmentation fault in
> vect_loop_versioning
> 
> "Yangfei (Felix)"  writes:
> > Hi,
> >
> > PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95570
> >
> > Here, we are doing loop versioning for alignment. The only dr here is a
> gather-statter operation: x[start][i].
> > Scalar evolution analysis for this dr failed, so DR_STEP is NULL_TREE, which
> leads to the segfault.
> > But scatter-gather operation should be filtered out in
> vect_enhance_data_refs_alignment.
> > There are similar issues in vect_verify_datarefs_alignment,
> vect_get_peeling_costs_all_drs and vect_peeling_supportable.
> > Proposed patch adds back the necessary tests.  Bootstrapped and tested
> on aarch64-linux-gnu & x86_64-linux-gnu.
> >
> > Test coverage:
> > Existing tests [1] and newly added test ensures coverage for all the changes
> except for the changes in vect_peeling_supportable.
> > Currently I don't have a test to cover the changes in
> vect_peeling_supportable.  Should we keep them?
> 
> Rather than add several instances of the new test, I think it would make
> sense to split the (hopefully) correct conditions in
> vect_enhance_data_refs_alignment out into a subroutine and use it in the
> other sites.  Doing that for vect_peeling_supportable would then be
> justifiable as a clean-up.

OK.

> How about something like vect_relevant_for_alignment_p as the name of
> the subroutine?

Nice name.   Does the v2 patch look better?
Bootstrapped and tested on aarch64-linux-gnu.
Newly added test fail without the fix and pass otherwise.

gcc/

+2020-06-11  Felix Yang  
+
+   PR tree-optimization/95570
+   * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function.
+   (vect_verify_datarefs_alignment): Call it to filter out data references
+   in the loop whose alignment is irrelevant.
+   (vect_get_peeling_costs_all_drs): Likewise.
+   (vect_peeling_supportable): Likewise.
+   (vect_enhance_data_refs_alignment): Likewise.

gcc/testsuite/

+2020-06-11  Felix Yang  
+
+   PR tree-optimization/95570
+   * gcc.dg/vect/pr95570.c: New test.

Thanks,
Felix


pr95570-v2.diff
Description: pr95570-v2.diff


[patch] Fix ICE in verify_sra_access_forest

2020-06-11 Thread Eric Botcazou
Hi,

this fixes an issue with reverse storage order in SRA, which is caught by the 
built-in verifier:

===GNAT BUG DETECTED==+
| 11.0.0 20200610 (experimental) (x86_64-suse-linux) GCC error:|
| in verify_sra_access_forest, at tree-sra.c:2359  

 gcc_assert (reverse == access->reverse);

The problem is that propagate_subaccesses_from_rhs changes the type of an 
access from aggregate to scalar and, as discussed elsewhere, this must be done 
with extra care in the presence of reverse storage order.

Tested on x86-64/Linux, OK for the mainline?


2020-06-11  Eric Botcazou  

* tree-sra.c (propagate_subaccesses_from_rhs): When a non-scalar
access on the LHS is replaced with a scalar access, propagate the
TYPE_REVERSE_STORAGE_ORDER flag of the type of the original access.


2020-06-11  Eric Botcazou  

* gnat.dg/opt85.ad[sb]: New test.

-- 
Eric Botcazoudiff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 4793b48f32c..fcba7fbdd31 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -2758,6 +2758,9 @@ propagate_subaccesses_from_rhs (struct access *lacc, struct access *racc)
 	}
   if (!lacc->first_child && !racc->first_child)
 	{
+	  /* We are about to change the access type from aggregate to scalar,
+	 so we need to put the reverse flag onto the access, if any.  */
+	  const bool reverse = TYPE_REVERSE_STORAGE_ORDER (lacc->type);
 	  tree t = lacc->base;
 
 	  lacc->type = racc->type;
@@ -2772,9 +2775,12 @@ propagate_subaccesses_from_rhs (struct access *lacc, struct access *racc)
 	  lacc->expr = build_ref_for_model (EXPR_LOCATION (lacc->base),
 		lacc->base, lacc->offset,
 		racc, NULL, false);
+	  if (TREE_CODE (lacc->expr) == MEM_REF)
+		REF_REVERSE_STORAGE_ORDER (lacc->expr) = reverse;
 	  lacc->grp_no_warning = true;
 	  lacc->grp_same_access_path = false;
 	}
+	  lacc->reverse = reverse;
 	}
   return ret;
 }
with Ada.Finalization;
with Interfaces;
with System;

package Opt85 is

   type Data_Type is record
  Value : Interfaces.Integer_16;
   end record;
   for Data_Type use record
  Value at 0 range 0 .. 15;
   end record;
   for Data_Type'Alignment use 1;
   for Data_Type'Size use 2 * System.Storage_Unit;
   for Data_Type'Bit_Order use System.High_Order_First;
   for Data_Type'Scalar_Storage_Order use System.High_Order_First;

   type Header_Type is array (1 .. 1) of Boolean;

   type Record_Type is new Ada.Finalization.Controlled with record
  Header : Header_Type;
  Data   : Data_Type;
   end record;

   function Create (Value : Integer) return Record_Type;

end Opt85;
-- { dg-do compile }
-- { dg-options "-O" }

package body Opt85 is

   function Conversion_Of (Value : Integer) return Data_Type is
   begin
  return (Value => Interfaces.Integer_16 (Value));
   end;

   function Create (Value : Integer) return Record_Type is
  Rec : constant Record_Type :=
(Ada.Finalization.Controlled with
 Header => (others => False),
 Data   => Conversion_Of (Value));
   begin
  return Rec;
   end;

end Opt85;


[PATCH][RFC] __builtin_shuffle sometimes should produce zip1 rather than TBL (PR82199)

2020-06-11 Thread Dmitrij Pochepko
The following patch enables vector permutations optimization by using another 
vector element size when applicable.
It allows usage of simpler instructions in applicable cases.

example:
#define vector __attribute__((vector_size(16) ))

vector float f(vector float a, vector float b)
{
  return __builtin_shuffle  (a, b, (vector int){0, 1, 4,5});
}

was compiled into:
...
adrpx0, .LC0
ldr q2, [x0, #:lo12:.LC0]
tbl v0.16b, {v0.16b - v1.16b}, v2.16b
...

and after patch:
...
zip1v0.2d, v0.2d, v1.2d
...

bootstrapped and tested on aarch64-linux-gnu with no regressions


This patch was initially introduced by Andrew Pinksi  with 
me being involved later.

(I have no write access to repo)

Thanks,
Dmitrij

gcc/ChangeLog:

2020-06-11  Andrew Pinski   

PR gcc/82199

* gcc/config/aarch64/aarch64.c (aarch64_evpc_reencode): New function

gcc/testsuite/ChangeLog:

2020-06-11  Andrew Pinski   

PR gcc/82199

* gcc.target/aarch64/vdup_n_3.c: New test
* gcc.target/aarch64/vzip_1.c: New test
* gcc.target/aarch64/vzip_2.c: New test
* gcc.target/aarch64/vzip_3.c: New test
* gcc.target/aarch64/vzip_4.c: New test

Co-Authored-By: Dmitrij Pochepko



Thanks,
Dmitrij
>From 3c9f3fe834811386223755fc58e2ab4a612eefcf Mon Sep 17 00:00:00 2001
From: Dmitrij Pochepko 
Date: Thu, 11 Jun 2020 14:13:35 +0300
Subject: [PATCH] __builtin_shuffle sometimes should produce zip1 rather than
 TBL (PR82199)

The following patch enables vector permutations optimization by using another vector element size when applicable.
It allows usage of simpler instructions in applicable cases.

example:

vector float f(vector float a, vector float b)
{
  return __builtin_shuffle  (a, b, (vector int){0, 1, 4,5});
}

was compiled into:
...
	adrp	x0, .LC0
	ldr	q2, [x0, #:lo12:.LC0]
	tbl	v0.16b, {v0.16b - v1.16b}, v2.16b
...

and after patch:
...
	zip1	v0.2d, v0.2d, v1.2d
...

bootstrapped and tested on aarch64-linux-gnu with no regressions

gcc/ChangeLog:

2020-06-11	Andrew Pinski	

	PR gcc/82199

	* gcc/config/aarch64/aarch64.c (aarch64_evpc_reencode): New function

gcc/testsuite/ChangeLog:

2020-06-11  Andrew Pinski   

	PR gcc/82199

	* gcc.target/aarch64/vdup_n_3.c: New test
	* gcc.target/aarch64/vzip_1.c: New test
	* gcc.target/aarch64/vzip_2.c: New test
	* gcc.target/aarch64/vzip_3.c: New test
	* gcc.target/aarch64/vzip_4.c: New test

Co-Authored-By:	Dmitrij Pochepko	
---
 gcc/config/aarch64/aarch64.c| 81 +
 gcc/testsuite/gcc.target/aarch64/vdup_n_3.c | 16 ++
 gcc/testsuite/gcc.target/aarch64/vzip_1.c   | 11 
 gcc/testsuite/gcc.target/aarch64/vzip_2.c   | 12 +
 gcc/testsuite/gcc.target/aarch64/vzip_3.c   | 12 +
 gcc/testsuite/gcc.target/aarch64/vzip_4.c   | 12 +
 6 files changed, 144 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/vdup_n_3.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_1.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_3.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_4.c

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 973c65a..ab7b39e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -19889,6 +19889,8 @@ struct expand_vec_perm_d
   bool testing_p;
 };
 
+static bool aarch64_expand_vec_perm_const_1 (struct expand_vec_perm_d *d);
+
 /* Generate a variable permutation.  */
 
 static void
@@ -20074,6 +20076,83 @@ aarch64_evpc_trn (struct expand_vec_perm_d *d)
   return true;
 }
 
+/* Try to re-encode the PERM constant so it use the bigger size up.
+   This rewrites constants such as {0, 1, 4, 5}/V4SF to {0, 2}/V2DI.
+   We retry with this new constant with the full suite of patterns.  */
+static bool
+aarch64_evpc_reencode (struct expand_vec_perm_d *d)
+{
+  expand_vec_perm_d newd;
+  unsigned HOST_WIDE_INT nelt;
+
+  if (d->vec_flags != VEC_ADVSIMD)
+return false;
+
+  unsigned int encoded_nelts = d->perm.encoding ().encoded_nelts ();
+  for (unsigned int i = 0; i < encoded_nelts; ++i)
+if (!d->perm[i].is_constant ())
+  return false;
+
+  /* to_constant is safe since this routine is specific to Advanced SIMD
+ vectors.  */
+  nelt = d->perm.length ().to_constant ();
+
+  /* Get the new mode.  Always twice the size of the inner
+ and half the elements.  */
+  machine_mode new_mode;
+  switch (d->vmode)
+{
+/* 128bit vectors.  */
+case E_V4SFmode:
+case E_V4SImode:
+  new_mode = V2DImode;
+  break;
+case E_V8BFmode:
+case E_V8HFmode:
+case E_V8HImode:
+  new_mode = V4SImode;
+  break;
+case E_V16QImode:
+  new_mode = V8HImode;
+  break;
+/* 64bit vectors.  */
+case E_V4BFmode:
+case E_V4HFmode:
+case E_V4HImode:
+  new_mode = V2SImode;
+  break;
+case E_V8QImode:
+  

Re: [PATCH 4/4 V2] vect: Factor out and rename some functions/macros

2020-06-11 Thread Richard Sandiford
"Kewen.Lin"  writes:
> @@ -9195,12 +9222,13 @@ optimize_mask_stores (class loop *loop)
>  }
>  
>  /* Decide whether it is possible to use a zero-based induction variable
> -   when vectorizing LOOP_VINFO with a fully-masked loop.  If it is,
> -   return the value that the induction variable must be able to hold
> -   in order to ensure that the loop ends with an all-false mask.
> -   Return -1 otherwise.  */
> +   when vectorizing LOOP_VINFO with partial vectors.  If it is, return
> +   the value that the induction variable must be able to hold in order
> +   to ensure that the loop ends with an all-false rgroup control like
> +   mask.  Return -1 otherwise.  */

Maybe: “…in order to ensure that the rgroups eventually have no active
vector elements”.

>  static void
> -check_load_store_masking (loop_vec_info loop_vinfo, tree vectype,
> -   vec_load_store_type vls_type, int group_size,
> -   vect_memory_access_type memory_access_type,
> -   gather_scatter_info *gs_info, tree scalar_mask)
> +check_load_store_for_partial_vectors (
> +  loop_vec_info loop_vinfo, tree vectype, vec_load_store_type vls_type,
> +  int group_size, vect_memory_access_type memory_access_type,
> +  gather_scatter_info *gs_info, tree scalar_mask)

Think it's more usual in GCC to put the "(" on the same line as the
arguments in this situation.

OK with those changes, thanks.

Richard


[Ada] Put_Image attribute

2020-06-11 Thread Pierre-Marie de Rodat
Work around bug in Put_Image of types in Remote_Types packages.  Use the
switch -gnatd_z to control enabling of Put_Image.  Put_Image is still
disabled by default for all types.

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

2020-06-11  Bob Duff  

gcc/ada/

* exp_put_image.adb (Build_Record_Put_Image_Procedure): Remove
special processing of protected types, because those are handled
by Build_Protected_Put_Image_Call.
(Enable_Put_Image): Use the switch -gnatd_z to control enabling
of Put_Image. Disable Put_Image for types in Remote_Types
packages.
* debug.adb: Document -gnatd_z switch.
* exp_imgv.adb, libgnat/a-stteou.ads, opt.ads: Minor cleanups.--- gcc/ada/debug.adb
+++ gcc/ada/debug.adb
@@ -170,7 +170,7 @@ package body Debug is
--  d_w
--  d_x
--  d_y
-   --  d_z
+   --  d_z  Enable Put_Image
 
--  d_A  Stop generation of ALI file
--  d_B
@@ -993,6 +993,9 @@ package body Debug is
--   a call to routine Ada.Synchronous_Task_Control.Suspend_Until_True
--   or Ada.Synchronous_Barriers.Wait_For_Release.
 
+   --  d_z  The Put_Image attribute is a work in progress, and is disabled by
+   --   default. This enables it.
+
--  d_A  Do not generate ALI files by setting Opt.Disable_ALI_File.
 
--  d_F  The compiler encodes the full path from an invocation construct to

--- gcc/ada/exp_imgv.adb
+++ gcc/ada/exp_imgv.adb
@@ -747,7 +747,7 @@ package body Exp_Imgv is
 
--btyp (Value_xx (X))
 
-   --  where btyp is he base type of the prefix
+   --  where btyp is the base type of the prefix
 
--For types whose root type is Character
--  xx = Character

--- gcc/ada/exp_put_image.adb
+++ gcc/ada/exp_put_image.adb
@@ -24,6 +24,7 @@
 --
 
 with Atree;use Atree;
+with Debug;use Debug;
 with Einfo;use Einfo;
 with Exp_Tss;  use Exp_Tss;
 with Lib;  use Lib;
@@ -323,9 +324,14 @@ package body Exp_Put_Image is
  --
  -- Put_Wide_Wide_String (Sink, U_Type'Wide_Wide_Image (Item));
  --
- --  This is a bit of a cheat; we should probably do it the other way
- --  around (define '[[Wide_]Wide_]Image in terms of 'Put_Image). But
- --  this is expedient for now. We can't do this:
+ --  It would be more elegant to do it the other way around (define
+ --  '[[Wide_]Wide_]Image in terms of 'Put_Image). But this is easier
+ --  to implement, because we already have support for
+ --  'Wide_Wide_Image. Furthermore, we don't want to remove the
+ --  existing support for '[[Wide_]Wide_]Image, because we don't
+ --  currently plan to support 'Put_Image on restricted runtimes.
+
+ --  We can't do this:
  --
  -- Put_UTF_8 (Sink, U_Type'Image (Item));
  --
@@ -689,22 +695,12 @@ package body Exp_Put_Image is
 
   Stms : constant List_Id := New_List;
   Rdef : Node_Id;
-  Typt : Entity_Id;
-  Type_Decl : Node_Id;
+  Type_Decl : constant Node_Id :=
+Declaration_Node (Base_Type (Underlying_Type (Typ)));
 
--  Start of processing for Build_Record_Put_Image_Procedure
 
begin
-  --  For the protected type case, use corresponding record
-
-  if Is_Protected_Type (Typ) then
- Typt := Corresponding_Record_Type (Typ);
-  else
- Typt := Typ;
-  end if;
-
-  Type_Decl := Declaration_Node (Base_Type (Underlying_Type (Typt)));
-
   Append_To (Stms,
 Make_Procedure_Call_Statement (Loc,
   Name => New_Occurrence_Of (RTE (RE_Record_Before), Loc),
@@ -813,7 +809,7 @@ package body Exp_Put_Image is
 
function Enable_Put_Image (T : Entity_Id) return Boolean is
begin
-  if True then -- True to disable for all types.
+  if not Debug_Flag_Underscore_Z then -- True to disable for all types
  return False;
   end if;
 
@@ -832,6 +828,15 @@ package body Exp_Put_Image is
   --  scalar types are expanded inline. We certainly want to be able to use
   --  Integer'Put_Image, for example.
 
+  --  ???Work around a bug: Put_Image does not work for Remote_Types.
+  --  We check the containing package, rather than the type itself, because
+  --  we want to include types in the private part of a Remote_Types
+  --  package.
+
+  if Is_Remote_Types (Scope (T)) then
+ return False;
+  end if;
+
   --  ???Disable Put_Image on type Sink declared in
   --  Ada.Strings.Text_Output. Note that we can't call Is_RTU on
   --  Ada_Strings_Text_Output, because it's not known yet (we might be

--- gcc/ada/libgnat/a-stteou.ads
+++ gcc/ada/libgnat/a-stteou.ads
@@ -133,7 +133,7 @@ package Ada.Strings.Text_Output is
  (UTF_Encoding.Wide_Wide_Strings.Decode (UTF_8_Lines)) = UTF_8_Lines;
 
subtype UTF_8 is UTF_8_Lines with
- Predicate => (for all 

[Ada] Fix missing insertion of explicit dereference in instance

2020-06-11 Thread Pierre-Marie de Rodat
This adjusts the new Resolve_Implicit_Dereference procedure to the cases
where nodes in an instance do not have the proper view of a type that
was declared as private (a well-known limitation of the current
implementation of generic instantiations for types that are only
implicitly referenced in the generic code).

No functional changes.

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

2020-06-11  Eric Botcazou  

gcc/ada/

* sem_res.adb (Resolve_Implicit_Dereference): In an instance,
reset the type of the prefix if it is private before building
the dereference.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -8740,6 +8740,17 @@ package body Sem_Res is
   Desig_Typ : Entity_Id;
 
begin
+  --  In an instance the proper view may not always be correct for
+  --  private types, see e.g. Sem_Type.Covers for similar handling.
+
+  if Is_Private_Type (Etype (P))
+and then Present (Full_View (Etype (P)))
+and then Is_Access_Type (Full_View (Etype (P)))
+and then In_Instance
+  then
+ Set_Etype (P, Full_View (Etype (P)));
+  end if;
+
   if Is_Access_Type (Etype (P)) then
  Desig_Typ := Implicitly_Designated_Type (Etype (P));
  Insert_Explicit_Dereference (P);



[Ada] Fix assertion failure on entry call through unchecked conversion

2020-06-11 Thread Pierre-Marie de Rodat
The Safe_Unchecked_Type_Conversion predicate was invoking the
Has_Discriminant predicate without checking that the Etype really
Is_Type, which is not the case for Standard_Void_Type "returned" by
entry calls.

No functional changes.

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

2020-06-11  Eric Botcazou  

gcc/ada/

* exp_util.adb (Safe_Unchecked_Type_Conversion): Add missing
Is_Type guard before calling Has_Discriminants on Etype.--- gcc/ada/exp_util.adb
+++ gcc/ada/exp_util.adb
@@ -12551,13 +12551,10 @@ package body Exp_Util is
   elsif Nkind (Pexp) = N_Selected_Component
 and then Prefix (Pexp) = Exp
   then
- if No (Etype (Pexp)) then
-return True;
- else
-return
-  not Has_Discriminants (Etype (Pexp))
-or else Is_Constrained (Etype (Pexp));
- end if;
+ return No (Etype (Pexp))
+   or else not Is_Type (Etype (Pexp))
+   or else not Has_Discriminants (Etype (Pexp))
+   or else Is_Constrained (Etype (Pexp));
   end if;
 
   --  Set the output type, this comes from Etype if it is set, otherwise we



[Ada] Refine type of a routine parameter from Node_Id to Entity_Id

2020-06-11 Thread Pierre-Marie de Rodat
Routine Get_Value is only called with its Compon parameter equal to
entity ids of components. This is now reflected in the type of this
parameter.  Code cleanup only; semantics is unaffected.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Get_Value): Refine type of the Compon parameter.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -3336,7 +3336,7 @@ package body Sem_Aggr is
   --  of the ancestor.
 
   function Get_Value
-(Compon : Node_Id;
+(Compon : Entity_Id;
  From   : List_Id;
  Consider_Others_Choice : Boolean := False) return Node_Id;
   --  Given a record component stored in parameter Compon, this function
@@ -3614,7 +3614,7 @@ package body Sem_Aggr is
   ---
 
   function Get_Value
-(Compon : Node_Id;
+(Compon : Entity_Id;
  From   : List_Id;
  Consider_Others_Choice : Boolean := False) return Node_Id
   is



[Ada] Allow specifying volatility refinement aspects for types

2020-06-11 Thread Pierre-Marie de Rodat
Previously, the four aspects Async_Readers, Async_Writers,
Effective_Reads, and Effective_Writes could only be specified for
volatile variables and for state abstractions. Allow specifying these
aspects for volatile types.

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

2020-06-11  Steve Baird  

gcc/ada/

* contracts.adb (Add_Contract_Item): Support specifying
volatility refinement aspects for types.
(Analyze_Contracts): Add call to Analyze_Type_Contract in the
case of a contract for a type.
(Freeze_Contracts): Add call to Analyze_Type_Contract in the
case of a contract for a type.
(Check_Type_Or_Object_External_Properties): A new procedure
which performs the work that needs to be done for both object
declarations and types.
(Analyze_Object_Contract): Add a call to
Check_Type_Or_Object_External_Properties and remove the code in
this procedure which did much of the work that is now performed
by that call.
(Analyze_Type_Contract): Implement this new routine as nothing
more than a call to Check_Type_Or_Object_External_Properties.
* contracts.ads: Update comment for Add_Contract_To_Item because
types can have contracts.  Follow (questionable) precedent and
declare new routine Analyze_Type_Contract as visible (following
example of Analyze_Object_Contract), despite the fact that it is
never called from outside of the package where it is declared.
* einfo.adb (Contract, Set_Contract): Id argument can be a type;
support this case.
(Write_Field34_Name): Field name is "contract" for a type.
* einfo.ads: Update comment describing Contract attribute.
* sem_ch3.adb (Build_Derived_Numeric_Type): Is_Volatile should
return same answer for all subtypes of a given type. Thus, when
building the base type for something like type Volatile_1_To_10
is range 1 .. 10 with Volatile; that basetype should be marked
as being volatile.
(Access_Type_Declaration): Add SPARK-specific legality check
that the designated type of an access type shall be compatible
with respect to volatility with the access type.
* sem_ch12.adb (Check_Shared_Variable_Control_Aspects): Add
SPARK-specific legality check that an actual type parameter in
an instantiation shall be compatible with respect to volatility
with the corresponding formal type.
* sem_ch13.adb (Analyze_Aspect_Specifications): Perform checks
for aspect specs for the 4 volatility refinement aspects that
were already being performed for all language-defined aspects.
* sem_prag.adb (Analyze_External_Property_In_Decl_Part,
Analyze_Pragma): External properties (other than No_Caching) may
be specified for a type, including a generic formal type.
* sem_util.ads: Declare new subprograms - Async_Readers_Enabled,
Async_Writers_Enabled, Effective_Reads, Effective_Writes, and
Check_Volatility_Compatibility.
* sem_util.adb (Async_Readers_Enabled, Async_Writers_Enabled,
Effective_Reads, Effective_Writes): Initial implementation of
new functions for querying aspect values.
(Check_Volatility_Compatibility): New procedure intended for use
in checking all SPARK legality rules of the form "<> shall be
compatible with respect to volatility with <>".
(Has_Enabled_Property): Update comment because Item_Id can be a
type.  Change name of nested Variable_Has_Enabled_Property
function to Type_Or_Variable_Has_Enabled_Property; add a
parameter to that function because recursion may be needed,
e.g., in the case of a derived typ).  Cope with the case where
the argument to Has_Enabled_Property is a type.

patch.diff.gz
Description: application/gzip


[Ada] Update SPARK RM rule numbers after removing a redundant rule

2020-06-11 Thread Pierre-Marie de Rodat
SPARK RM 7.1.3(8) has been deleted, but there were actually plenty of
mistakes in references to rules 7.1.3(X). This patch fixes them in both
comments and error messages.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_ch4.adb, sem_ch6.adb, sem_res.adb, sem_util.ads: Fix
references to SPARK RM 7.1.3 rule numbers.--- gcc/ada/sem_ch4.adb
+++ gcc/ada/sem_ch4.adb
@@ -665,7 +665,7 @@ package body Sem_Ch4 is
--  that outside of spec expressions, otherwise the declaration
--  cannot be inserted and analyzed. In such a case, GNATprove
--  later rejects the allocator as it is not used here in
-   --  a non-interfering context (SPARK 4.8(2) and 7.1.3(12)).
+   --  a non-interfering context (SPARK 4.8(2) and 7.1.3(10)).
 
if Expander_Active
  or else (GNATprove_Mode and then not In_Spec_Expression)

--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -11889,7 +11889,7 @@ package body Sem_Ch6 is
 
 --  A procedure cannot have an effectively volatile formal
 --  parameter of mode IN because it behaves as a constant
---  (SPARK RM 7.1.3(6)). -- ??? maybe 7.1.3(4)
+--  (SPARK RM 7.1.3(4)).
 
 elsif Ekind (Scope (Formal)) = E_Procedure
   and then Ekind (Formal) = E_In_Parameter

--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -3328,7 +3328,7 @@ package body Sem_Res is
 
   procedure Flag_Effectively_Volatile_Objects (Expr : Node_Id);
   --  Emit an error concerning the illegal usage of an effectively volatile
-  --  object in interfering context (SPARK RM 7.1.3(12)).
+  --  object in interfering context (SPARK RM 7.1.3(10)).
 
   procedure Insert_Default;
   --  If the actual is missing in a call, insert in the actuals list
@@ -3613,7 +3613,7 @@ package body Sem_Res is
then
   Error_Msg_N
 ("volatile object cannot appear in this context (SPARK "
- & "RM 7.1.3(11))", N);
+ & "RM 7.1.3(10))", N);
   return Skip;
end if;
 end if;
@@ -4739,7 +4739,7 @@ package body Sem_Res is
 
--  An effectively volatile object may act as an actual when the
--  corresponding formal is of a non-scalar effectively volatile
-   --  type (SPARK RM 7.1.3(11)).
+   --  type (SPARK RM 7.1.3(10)).
 
if not Is_Scalar_Type (Etype (F))
  and then Is_Effectively_Volatile (Etype (F))
@@ -4748,7 +4748,7 @@ package body Sem_Res is
 
--  An effectively volatile object may act as an actual in a
--  call to an instance of Unchecked_Conversion.
-   --  (SPARK RM 7.1.3(11)).
+   --  (SPARK RM 7.1.3(10)).
 
elsif Is_Unchecked_Conversion_Instance (Nam) then
   null;
@@ -4758,7 +4758,7 @@ package body Sem_Res is
elsif Is_Effectively_Volatile_Object (A) then
   Error_Msg_N
 ("volatile object cannot act as actual in a call (SPARK "
- & "RM 7.1.3(11))", A);
+ & "RM 7.1.3(10))", A);
 
--  Otherwise the actual denotes an expression. Inspect the
--  expression and flag each effectively volatile object with
@@ -7544,7 +7544,7 @@ package body Sem_Res is
 
 --  An effectively volatile object subject to enabled properties
 --  Async_Writers or Effective_Reads must appear in non-interfering
---  context (SPARK RM 7.1.3(12)).
+--  context (SPARK RM 7.1.3(10)).
 
 if Is_Object (E)
   and then Is_Effectively_Volatile (E)
@@ -7554,7 +7554,7 @@ package body Sem_Res is
 then
SPARK_Msg_N
  ("volatile object cannot appear in this context "
-  & "(SPARK RM 7.1.3(12))", N);
+  & "(SPARK RM 7.1.3(10))", N);
 end if;
 
 --  Check for possible elaboration issues with respect to reads of

--- gcc/ada/sem_util.ads
+++ gcc/ada/sem_util.ads
@@ -1919,7 +1919,7 @@ package Sem_Util is
  (Context : Node_Id;
   Obj_Ref : Node_Id) return Boolean;
--  Determine whether node Context denotes a "non-interfering context" (as
-   --  defined in SPARK RM 7.1.3(12)) where volatile reference Obj_Ref can
+   --  defined in SPARK RM 7.1.3(10)) where volatile reference Obj_Ref can
--  safely reside.
 
function Is_Package_Contract_Annotation (Item : Node_Id) return Boolean;



[Ada] Fix wrong access to large bit-packed arrays with reverse SSO

2020-06-11 Thread Pierre-Marie de Rodat
Large bit-packed arrays, i.e. whose size is greater than 64 bits,
are implemented under the hood by means of arrays of storage units,
the front-end generating the required mask-and-shifts operations to
go back and forth between the two representations.

These operations depend on the endianness of the bit-packed arrays,
which may be different from that of the target if the attribute
Scalar_Storage_Order is specified for the bit-packed arrays.

But, even though the component type of arrays of storage units is
endian neutral, the Scalar_Storage_Order attribute of the bit-packed
arrays must be propagated onto them, because it is required if the
bit-packed arrays are components of an outer composite type that do
not start or do not end on a storage unit boundary.

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

2020-06-11  Eric Botcazou  

gcc/ada/

* exp_pakd.ads: Add paragraph about scalar storage order.
* exp_pakd.adb (Install_PAT): Do not set the scalar storage
order of the PAT here but...
(Set_PB_Type): ...here instead and...
(Create_Packed_Array_Impl_Type): ...here as well.
* rtsfind.ads (RE_Id): Add RE_Rev_Packed_Bytes{1,2,4}.
(RE_Unit_Table): Likewise.
* libgnat/s-unstyp.ads (Rev_Packed_Bytes1): New derived type.
(Rev_Packed_Bytes2): Likewise.
(Rev_Packed_Bytes4): Likewise.--- gcc/ada/exp_pakd.adb
+++ gcc/ada/exp_pakd.adb
@@ -501,8 +501,9 @@ package body Exp_Pakd is
   --  packed array type. It creates the type and installs it as required.
 
   procedure Set_PB_Type;
-  --  Sets PB_Type to Packed_Bytes{1,2,4} as required by the alignment
-  --  requirements (see documentation in the spec of this package).
+  --  Set PB_Type to [Rev_]Packed_Bytes{1,2,4} as required by the alignment
+  --  and the scalar storage order requirements (see documentation in the
+  --  spec of this package).
 
   -
   -- Install_PAT --
@@ -580,14 +581,6 @@ package body Exp_Pakd is
  Set_Is_Volatile_Full_Access (PAT, Is_Volatile_Full_Access  (Typ));
  Set_Treat_As_Volatile   (PAT, Treat_As_Volatile(Typ));
 
- --  For a non-bit-packed array, propagate reverse storage order
- --  flag from original base type to packed array base type.
-
- if not Is_Bit_Packed_Array (Typ) then
-Set_Reverse_Storage_Order
-  (Etype (PAT), Reverse_Storage_Order (Base_Type (Typ)));
- end if;
-
  --  We definitely do not want to delay freezing for packed array
  --  types. This is of particular importance for the itypes that are
  --  generated for record components depending on discriminants where
@@ -616,16 +609,36 @@ package body Exp_Pakd is
or else Alignment (Typ) = 1
or else Component_Alignment (Typ) = Calign_Storage_Unit
  then
-PB_Type := RTE (RE_Packed_Bytes1);
+if Reverse_Storage_Order (Typ) then
+   PB_Type := RTE (RE_Rev_Packed_Bytes1);
+else
+   PB_Type := RTE (RE_Packed_Bytes1);
+end if;
 
  elsif Csize mod 4 /= 0
or else Alignment (Typ) = 2
  then
-PB_Type := RTE (RE_Packed_Bytes2);
+if Reverse_Storage_Order (Typ) then
+   PB_Type := RTE (RE_Rev_Packed_Bytes2);
+else
+   PB_Type := RTE (RE_Packed_Bytes2);
+end if;
 
  else
-PB_Type := RTE (RE_Packed_Bytes4);
+if Reverse_Storage_Order (Typ) then
+   PB_Type := RTE (RE_Rev_Packed_Bytes4);
+else
+   PB_Type := RTE (RE_Packed_Bytes4);
+end if;
  end if;
+
+ --  The Rev_Packed_Bytes{1,2,4} types cannot be directly declared with
+ --  the reverse scalar storage order in System.Unsigned_Types because
+ --  their component type is aliased and the combination would then be
+ --  flagged as illegal by the compiler. Moreover changing the compiler
+ --  would not address the bootstrap path issue with earlier versions.
+
+ Set_Reverse_Storage_Order (PB_Type, Reverse_Storage_Order (Typ));
   end Set_PB_Type;
 
--  Start of processing for Create_Packed_Array_Impl_Type
@@ -797,6 +810,10 @@ package body Exp_Pakd is
  end;
 
  Install_PAT;
+
+ --  Propagate the reverse storage order flag to the base type
+
+ Set_Reverse_Storage_Order (Etype (PAT), Reverse_Storage_Order (Typ));
  return;
 
   --  Case of bit-packing required for unconstrained array. We create

--- gcc/ada/exp_pakd.ads
+++ gcc/ada/exp_pakd.ads
@@ -86,6 +86,15 @@ package Exp_Pakd is
--Packed_Bytes{1,2,4} type is made on the basis of alignment needs as
--described above for the unconstrained case.
 
+   --  When the packed array (sub)type is specified to have the reverse scalar
+   --  storage order, the 

[Ada] Put_Image attribute

2020-06-11 Thread Pierre-Marie de Rodat
Work around the fact that Put_Image doesn't work for private types whose
full type is real.  Make Put_Image_Unknown print out the name of the
type.  Put_Image is still disabled by default for all types.

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

2020-06-11  Bob Duff  

gcc/ada/

* exp_put_image.adb (Build_Elementary_Put_Image_Call): If the
underlying type is real, call Put_Image_Unknown.
(Build_Unknown_Put_Image_Call): Pass the type name to
Put_Image_Unknown.
* libgnat/s-putima.ads, libgnat/s-putima.adb
(Put_Image_Unknown): Add Type_Name parameter.  Remove
overly-detailed documentation of what it does; better to leave
it open.--- gcc/ada/exp_put_image.adb
+++ gcc/ada/exp_put_image.adb
@@ -27,6 +27,7 @@ with Atree;use Atree;
 with Debug;use Debug;
 with Einfo;use Einfo;
 with Exp_Tss;  use Exp_Tss;
+with Exp_Util;
 with Lib;  use Lib;
 with Namet;use Namet;
 with Nlists;   use Nlists;
@@ -340,26 +341,34 @@ package body Exp_Put_Image is
  --
  --  Note that this is putting a leading space for reals.
 
+ --  ???Work around the fact that Put_Image doesn't work for private
+ --  types whose full type is real.
+
+ if Is_Real_Type (U_Type) then
+return Build_Unknown_Put_Image_Call (N);
+ end if;
+
  declare
 Image : constant Node_Id :=
   Make_Attribute_Reference (Loc,
 Prefix => New_Occurrence_Of (U_Type, Loc),
 Attribute_Name => Name_Wide_Wide_Image,
 Expressions => New_List (Relocate_Node (Item)));
- begin
-return
+Put_Call : constant Node_Id :=
   Make_Procedure_Call_Statement (Loc,
 Name =>
   New_Occurrence_Of (RTE (RE_Put_Wide_Wide_String), Loc),
 Parameter_Associations => New_List
   (Relocate_Node (Sink), Image));
+ begin
+return Put_Call;
  end;
   end if;
 
   --  Unchecked-convert parameter to the required type (i.e. the type of
   --  the corresponding parameter), and call the appropriate routine.
   --  We could use a normal type conversion for scalars, but the
-  --  "unchecked" is needed for access types.
+  --  "unchecked" is needed for access and private types.
 
   declare
  Libent : constant Entity_Id := RTE (Lib_RE);
@@ -800,7 +809,10 @@ package body Exp_Put_Image is
 Make_Procedure_Call_Statement (Loc,
   Name => New_Occurrence_Of (Libent, Loc),
   Parameter_Associations => New_List (
-Relocate_Node (Sink)));
+Relocate_Node (Sink),
+Make_String_Literal (Loc,
+  Exp_Util.Fully_Qualified_Name_String (
+Entity (Prefix (N)), Append_NUL => False;
end Build_Unknown_Put_Image_Call;
 
--

--- gcc/ada/libgnat/s-putima.adb
+++ gcc/ada/libgnat/s-putima.adb
@@ -212,9 +212,11 @@ package body System.Put_Images is
   Put_7bit (S, ')');
end Record_After;
 
-   procedure Put_Image_Unknown (S : in out Sink'Class) is
+   procedure Put_Image_Unknown (S : in out Sink'Class; Type_Name : String) is
begin
-  Put_UTF_8 (S, "{unknown image}");
+  Put_UTF_8 (S, "{");
+  Put_String (S, Type_Name);
+  Put_UTF_8 (S, " object}");
end Put_Image_Unknown;
 
 end System.Put_Images;

--- gcc/ada/libgnat/s-putima.ads
+++ gcc/ada/libgnat/s-putima.ads
@@ -86,8 +86,8 @@ package System.Put_Images is
procedure Record_Between (S : in out Sink'Class);
procedure Record_After (S : in out Sink'Class);
 
-   procedure Put_Image_Unknown (S : in out Sink'Class);
+   procedure Put_Image_Unknown (S : in out Sink'Class; Type_Name : String);
--  For Put_Image of types that don't have the attribute, such as type
-   --  Sink. Prints a canned string.
+   --  Sink.
 
 end System.Put_Images;



[Ada] Consolidate handling of implicit dereferences into semantic analysis

2020-06-11 Thread Pierre-Marie de Rodat
This consolidates the handling of almost all the implicit dereferences
allowed in Ada 95 into the semantic analysis phase of the compiler, and
more precisely in the Resolve routine of the front-end.

This both means that the generic code handling them in the expander
is removed, and that various constructs generated by it are directly
built with explicit dereferences when needed.

No functional changes, except for swapped error messages in case of
problematic dereferences in requeue statements.

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

2020-06-11  Eric Botcazou  

gcc/ada/

* checks.adb (Build_Discriminant_Checks): Build an explicit
dereference when the type is an access type.
* exp_atag.adb (Build_CW_Membership): Add explicit dereferences.
(Build_Get_Access_Level): Likewise.
(Build_Get_Alignment): Likewise.
(Build_Inherit_Prims): Likewise.
(Build_Get_Transportable): Likewise.
(Build_Set_Size_Function): Likewise.
* exp_ch3.adb (Build_Offset_To_Top_Function): Likewise.
* exp_ch4.adb (Expand_Allocator_Expression): Likewise.
(Expand_N_Indexed_Component ): Remove code dealing with implicit
dereferences.
(Expand_N_Selected_Component): Likewise.
(Expand_N_Slice): Likewise.
* exp_ch9.adb (Add_Formal_Renamings): Add explicit dereference.
(Expand_Accept_Declarations): Likewise.
(Build_Simple_Entry_Call): Remove code dealing with implicit
dereferences.
(Expand_N_Requeue_Statement): Likewise.
* exp_disp.adb (Expand_Dispatching_Call): Build an explicit
dereference when the controlling type is an access type.
* exp_spark.adb (Expand_SPARK_N_Selected_Component): Delete.
(Expand_SPARK_N_Slice_Or_Indexed_Component): Likewise.
(Expand_SPARK): Do not call them.
* sem_ch4.adb (Process_Implicit_Dereference_Prefix): Delete.
(Process_Indexed_Component): Call Implicitly_Designated_Type
to get the designated type for an implicit dereference.
(Analyze_Overloaded_Selected_Component): Do not insert an
explicit dereference here.
(Analyze_Selected_Component): Likewise.
(Analyze_Slice): Call Implicitly_Designated_Type to get the
designated type for an implicit dereference.
* sem_ch8.adb (Has_Components): New predicate extracted from...
(Is_Appropriate_For_Record): ...this.  Delete.
(Is_Appropriate_For_Entry_Prefix): Likewise.
(Analyze_Renamed_Entry): Deal with implicit dereferences.
(Find_Selected_Component): Do not insert an explicit dereference
here.  Call Implicitly_Designated_Type to get the designated type
for an implicit dereference.  Call Has_Components, Is_Task_Type
and Is_Protected_Type directly.  Adjust test for error.
* sem_res.adb (Resolve_Implicit_Dereference): New procedure.
(Resolve_Call): Call Resolve_Indexed_Component last.
(Resolve_Entry): Call Resolve_Implicit_Dereference on the prefix.
(Resolve_Indexed_Component): Call Implicitly_Designated_Type to
get the designated type for an implicit dereference and
Resolve_Implicit_Dereference on the prefix at the end.
(Resolve_Selected_Component): Likewise.
(Resolve_Slice): Likewise.  Do not apply access checks here.
* sem_util.ads (Implicitly_Designated_Type): Declare.
* sem_util.adb (Copy_And_Maybe_Dereference): Simplify.
(Implicitly_Designated_Type): New function.
(Object_Access_Level): Fix typo.
* sem_warn.adb (Check_Unset_Reference): Test Comes_From_Source
on the original node.

patch.diff.gz
Description: application/gzip


[Ada] Create constrained itypes for nested record aggregates

2020-06-11 Thread Pierre-Marie de Rodat
When resolving a record aggregate with a box as the value of one of its
component that is itself of a discriminated record type, this box is
replaced with an inner record aggregate. However, while a constrained
itype is created for the outer record aggregate (as described in the
comment of Resolve_Record_Aggregate, Step 4), a similar itype was never
created for the inner record aggregate.

This didn't matter for GNAT, but GNATprove assumes that all record
aggregates have constrained types (or itypes, where necessary). In
theory, GNATprove could be deduce the needed information by itself, but
this would require frontend code to be either duplicated (or preferably
reused).

This patch creates the missing constrained itypes in the frontend to
make them available to GNATprove (and to other backends, should they
need it).

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Build_Constrained_Itype): Previously a declare
block, now a separate procedure; the only change is that now
New_Assoc_List might include components and an others clause,
which we ignore (while we deal with discriminants exactly as we
did before); extend a ??? comment about how this routine is
different from the Build_Subtype
(Resolve_Record_Aggregate): Create a constrained itype not just
for the outermost record aggregate, but for its inner record
aggregates as well.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -3315,6 +3315,29 @@ package body Sem_Aggr is
   --  part of the enclosing aggregate. Assoc_List provides the discriminant
   --  associations of the current type or of some enclosing record.
 
+  procedure Build_Constrained_Itype
+(N  : Node_Id;
+ Typ: Entity_Id;
+ New_Assoc_List : List_Id);
+  --  Build a constrained itype for the newly created record aggregate N
+  --  and set it as a type of N. The itype will have Typ as its base type
+  --  and will be constrained by the values of discriminants from the
+  --  component association list New_Assoc_List.
+
+  --  ??? This code used to be pretty much a copy of Sem_Ch3.Build_Subtype,
+  --  but now those two routines behave differently for types with unknown
+  --  discriminants. They should really be exported in sem_util or some
+  --  such and used in sem_ch3 and here rather than have a copy of the
+  --  code which is a maintenance nightmare.
+
+  --  ??? Performance WARNING. The current implementation creates a new
+  --  itype for all aggregates whose base type is discriminated. This means
+  --  that for record aggregates nested inside an array aggregate we will
+  --  create a new itype for each record aggregate if the array component
+  --  type has discriminants. For large aggregates this may be a problem.
+  --  What should be done in this case is to reuse itypes as much as
+  --  possible.
+
   function Discriminant_Present (Input_Discr : Entity_Id) return Boolean;
   --  If aggregate N is a regular aggregate this routine will return True.
   --  Otherwise, if N is an extension aggregate, then Input_Discr denotes
@@ -3474,6 +3497,78 @@ package body Sem_Aggr is
  end loop;
   end Add_Discriminant_Values;
 
+  -
+  -- Build_Constrained_Itype --
+  -
+
+  procedure Build_Constrained_Itype
+(N  : Node_Id;
+ Typ: Entity_Id;
+ New_Assoc_List : List_Id)
+  is
+ Constrs : constant List_Id:= New_List;
+ Loc : constant Source_Ptr := Sloc (N);
+ Def_Id  : Entity_Id;
+ Indic   : Node_Id;
+ New_Assoc   : Node_Id;
+ Subtyp_Decl : Node_Id;
+
+  begin
+ New_Assoc := First (New_Assoc_List);
+ while Present (New_Assoc) loop
+
+--  There is exactly one choice in the component association (and
+--  it is either a discriminant, a component or the others clause).
+pragma Assert (List_Length (Choices (New_Assoc)) = 1);
+
+--  Duplicate expression for the discriminant and put it on the
+--  list of constraints for the itype declaration.
+
+if Is_Entity_Name (First (Choices (New_Assoc)))
+  and then
+Ekind (Entity (First (Choices (New_Assoc = E_Discriminant
+then
+   Append_To (Constrs, Duplicate_Subexpr (Expression (New_Assoc)));
+end if;
+
+Next (New_Assoc);
+ end loop;
+
+ if Has_Unknown_Discriminants (Typ)
+   and then Present (Underlying_Record_View (Typ))
+ then
+Indic :=
+  Make_Subtype_Indication (Loc,
+Subtype_Mark =>
+  New_Occurrence_Of (Underlying_Record_View 

[Ada] Refine type of a counter-like variable

2020-06-11 Thread Pierre-Marie de Rodat
A local variable that is used as a counter (which is clear from both its
comment and its used) will only be assigned with natural numbers. This
is now reflected in its type. Code cleanup only; semantics is
unaffected.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Resolve_Record_Aggregate): Refine type of
Others_Box.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -3283,7 +3283,7 @@ package body Sem_Aggr is
 
   Box_Node   : Node_Id := Empty;
   Is_Box_Present : Boolean := False;
-  Others_Box : Integer := 0;
+  Others_Box : Natural := 0;
   --  Ada 2005 (AI-287): Variables used in case of default initialization
   --  to provide a functionality similar to Others_Etype. Box_Present
   --  indicates that the component takes its default initialization;



[Ada] Avoid "others => <>" association in resolved record aggregates

2020-06-11 Thread Pierre-Marie de Rodat
When resolving record aggregates, frontend was creating "others => <>"
association to represent nested components, for example:

   type T1 (D1 : Boolean := False) is record
  C1 : Boolean := True;
   end record;

   type T2 is record
  C2 : T1;
   end record;

   X2 : T2 := (others => <>);

was expanded into:

   x2 : t2 := (c2 => (d1 => false, others => <>));
   

Now those components are represented explicitly:

   x2 : t2 := (c2 => (d1 => false, c1 => <>));
   

This makes no difference for the compiler (except that heavily nested
records aggregates will be resolved into bigger AST), but GNATprove will
not need to rediscover which components are covered by the "others =>
<>" association.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Add_Association): Add assertion about the formal
parameters.
(Propagate_Discriminants): Always add an explicit component
association, so that an "others => <>" association is never
needed.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -3393,6 +3393,8 @@ package body Sem_Aggr is
  --  If this is a box association the expression is missing, so use the
  --  Sloc of the aggregate itself for the new association.
 
+ pragma Assert (Present (Expr) xor Is_Box_Present);
+
  if Present (Expr) then
 Loc := Sloc (Expr);
  else
@@ -3804,8 +3806,6 @@ package body Sem_Aggr is
   is
  Loc : constant Source_Ptr := Sloc (N);
 
- Needs_Box : Boolean := False;
-
  procedure Process_Component (Comp : Entity_Id);
  --  Add one component with a box association to the inner aggregate,
  --  and recurse if component is itself composite.
@@ -3834,7 +3834,9 @@ package body Sem_Aggr is
Build_Constrained_Itype
  (New_Aggr, T, Component_Associations (New_Aggr));
 else
-   Needs_Box := True;
+   Add_Association
+ (Comp, Empty, Component_Associations (Aggr),
+  Is_Box_Present => True);
 end if;
  end Process_Component;
 
@@ -3885,14 +3887,6 @@ package body Sem_Aggr is
Next_Component (Comp);
 end loop;
  end if;
-
- if Needs_Box then
-Append_To (Component_Associations (Aggr),
-  Make_Component_Association (Loc,
-Choices => New_List (Make_Others_Choice (Loc)),
-Expression  => Empty,
-Box_Present => True));
- end if;
   end Propagate_Discriminants;
 
   ---



[Ada] Move duplicated routines for building itypes to Sem_Util

2020-06-11 Thread Pierre-Marie de Rodat
Routine Build_Constrained_Itype was created as an exact duplicate
Build_Subtype with a comment suggesting that their code should be
exported from Sem_Util and reused. Unfortunately, since then both
routines diverged and now are subtly different, so reusing is not
straightforward. However, it is still better to have them both exported
to prevent further duplication.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Build_Constrained_Itype): Move to Sem_Util.
* sem_ch3.adb (Build_Subtype, Inherit_Predicate_Flags): Move...
* sem_util.adb (Build_Subtype): Here.  Add parameters for
references to objects previously declared in enclosing scopes.
(Inherit_Predicate_Flags): And here, because it is called by
Build_Subtype.
* sem_util.ads (Build_Overriding_Spec): Reorder alphabetically.
(Build_Subtype): Moved from Sem_Ch3; comments updated.
(Build_Constrained_Itype): Moved from Sem_Aggr; comments
updated.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -3313,29 +3313,6 @@ package body Sem_Aggr is
   --  part of the enclosing aggregate. Assoc_List provides the discriminant
   --  associations of the current type or of some enclosing record.
 
-  procedure Build_Constrained_Itype
-(N  : Node_Id;
- Typ: Entity_Id;
- New_Assoc_List : List_Id);
-  --  Build a constrained itype for the newly created record aggregate N
-  --  and set it as a type of N. The itype will have Typ as its base type
-  --  and will be constrained by the values of discriminants from the
-  --  component association list New_Assoc_List.
-
-  --  ??? This code used to be pretty much a copy of Sem_Ch3.Build_Subtype,
-  --  but now those two routines behave differently for types with unknown
-  --  discriminants. They should really be exported in sem_util or some
-  --  such and used in sem_ch3 and here rather than have a copy of the
-  --  code which is a maintenance nightmare.
-
-  --  ??? Performance WARNING. The current implementation creates a new
-  --  itype for all aggregates whose base type is discriminated. This means
-  --  that for record aggregates nested inside an array aggregate we will
-  --  create a new itype for each record aggregate if the array component
-  --  type has discriminants. For large aggregates this may be a problem.
-  --  What should be done in this case is to reuse itypes as much as
-  --  possible.
-
   function Discriminant_Present (Input_Discr : Entity_Id) return Boolean;
   --  If aggregate N is a regular aggregate this routine will return True.
   --  Otherwise, if N is an extension aggregate, then Input_Discr denotes
@@ -3495,78 +3472,6 @@ package body Sem_Aggr is
  end loop;
   end Add_Discriminant_Values;
 
-  -
-  -- Build_Constrained_Itype --
-  -
-
-  procedure Build_Constrained_Itype
-(N  : Node_Id;
- Typ: Entity_Id;
- New_Assoc_List : List_Id)
-  is
- Constrs : constant List_Id:= New_List;
- Loc : constant Source_Ptr := Sloc (N);
- Def_Id  : Entity_Id;
- Indic   : Node_Id;
- New_Assoc   : Node_Id;
- Subtyp_Decl : Node_Id;
-
-  begin
- New_Assoc := First (New_Assoc_List);
- while Present (New_Assoc) loop
-
---  There is exactly one choice in the component association (and
---  it is either a discriminant, a component or the others clause).
-pragma Assert (List_Length (Choices (New_Assoc)) = 1);
-
---  Duplicate expression for the discriminant and put it on the
---  list of constraints for the itype declaration.
-
-if Is_Entity_Name (First (Choices (New_Assoc)))
-  and then
-Ekind (Entity (First (Choices (New_Assoc = E_Discriminant
-then
-   Append_To (Constrs, Duplicate_Subexpr (Expression (New_Assoc)));
-end if;
-
-Next (New_Assoc);
- end loop;
-
- if Has_Unknown_Discriminants (Typ)
-   and then Present (Underlying_Record_View (Typ))
- then
-Indic :=
-  Make_Subtype_Indication (Loc,
-Subtype_Mark =>
-  New_Occurrence_Of (Underlying_Record_View (Typ), Loc),
-Constraint   =>
-  Make_Index_Or_Discriminant_Constraint (Loc,
-Constraints => Constrs));
- else
-Indic :=
-  Make_Subtype_Indication (Loc,
-Subtype_Mark =>
-  New_Occurrence_Of (Base_Type (Typ), Loc),
-Constraint   =>
-  Make_Index_Or_Discriminant_Constraint (Loc,
- 

[Ada] Iterate with procedural version of Next routine where possible

2020-06-11 Thread Pierre-Marie de Rodat
Routine Next is implemented both as a procedure and as a function. The
procedure variant seems meant to be used when iterating, e.g.:

   Next (Decl);

because it is more readable than the corresponding functions:

   Decl := Next (Decl);

(and it is inlined anyway, so there is no performance penalty). This
patch makes use of the procedure variants where possible; uses of
function were detected with:

  $ grep " \([[:alpha:]_]\+\) := Next (\1)" *.adb

Note: procedural version is kept in sem_prag.adb, where it is used to
iterate over pragma arguments and it feels more readable there.

Semantics is unaffected.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* checks.adb, exp_ch7.adb, exp_ch9.adb, exp_smem.adb, lib.adb,
nlists.adb, sem.adb, sem_aggr.adb, sem_ch3.adb, sem_ch6.adb,
sem_ch8.adb, sem_dim.adb, sem_res.adb, sem_util.adb,
sem_warn.adb: Replace uses of Next function with procedure.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -3531,7 +3531,7 @@ package body Checks is
 
  --  Move to next subscript
 
- Sub := Next (Sub);
+ Next (Sub);
   end loop;
end Apply_Subscript_Validity_Checks;
 

--- gcc/ada/exp_ch7.adb
+++ gcc/ada/exp_ch7.adb
@@ -2852,7 +2852,7 @@ package body Exp_Ch7 is
  exit;
   end if;
 
-  Result := Next (Result);
+  Next (Result);
end loop;
 
return Result;
@@ -2895,7 +2895,7 @@ package body Exp_Ch7 is
 if No_Initialization (Decl) then
if No (Expression (Last_Init)) then
   loop
- Last_Init := Next (Last_Init);
+ Next (Last_Init);
  exit when No (Last_Init);
  exit when Nkind (Last_Init) = N_Object_Declaration
and then Nkind (Expression (Last_Init)) = N_Reference

--- gcc/ada/exp_ch9.adb
+++ gcc/ada/exp_ch9.adb
@@ -2130,7 +2130,7 @@ package body Exp_Ch9 is
 if Present (First_Formal (Iface_Op))
   and then Is_Controlling_Formal (First_Formal (Iface_Op))
 then
-   Iface_Op_Param := Next (Iface_Op_Param);
+   Next (Iface_Op_Param);
 end if;
 
 Wrapper_Param := First (Wrapper_Params);
@@ -2215,7 +2215,7 @@ package body Exp_Ch9 is
  if Is_Private_Primitive_Subprogram (Subp_Id)
and then not Has_Controlling_Result (Subp_Id)
  then
-Formal := Next (Formal);
+Next (Formal);
  end if;
 
  while Present (Formal) loop

--- gcc/ada/exp_smem.adb
+++ gcc/ada/exp_smem.adb
@@ -454,7 +454,7 @@ package body Exp_Smem is
 
   begin
  while Next (Nod) /= After loop
-Nod := Next (Nod);
+Next (Nod);
  end loop;
 
  return Nod;

--- gcc/ada/lib.adb
+++ gcc/ada/lib.adb
@@ -1335,7 +1335,7 @@ package body Lib is
and then (Nkind (Context_Item) /= N_With_Clause
   or else Limited_Present (Context_Item))
  loop
-Context_Item := Next (Context_Item);
+Next (Context_Item);
  end loop;
 
  if Present (Context_Item) then
@@ -1359,7 +1359,7 @@ package body Lib is
   Write_Eol;
end if;
 
-   Context_Item := Next (Context_Item);
+   Next (Context_Item);
 end loop;
 
 Outdent;

--- gcc/ada/nlists.adb
+++ gcc/ada/nlists.adb
@@ -243,7 +243,7 @@ package body Nlists is
 N := F;
 loop
Set_List_Link (N, To);
-   N := Next (N);
+   Next (N);
exit when No (N);
 end loop;
 
@@ -530,7 +530,7 @@ package body Nlists is
 loop
Set_List_Link (N, LC);
exit when N = L;
-   N := Next (N);
+   Next (N);
 end loop;
 
 if Present (Before) then
@@ -597,7 +597,7 @@ package body Nlists is
 loop
Set_List_Link (N, LC);
exit when N = L;
-   N := Next (N);
+   Next (N);
 end loop;
 
 if Present (After) then
@@ -699,7 +699,7 @@ package body Nlists is
   Node := First (List);
   while Present (Node) loop
  Result := Result + 1;
- Node := Next (Node);
+ Next (Node);
   end loop;
 
   return Result;
@@ -756,7 +756,7 @@ package body Nlists is
 
  while Present (E) loop
 Append (New_Copy (E), NL);
-E := Next (E);
+Next (E);
  end loop;
 
  return NL;
@@ -784,7 +784,7 @@ package body Nlists is
Append (New_Copy (E), NL);
 end if;
 
-E := Next (E);
+Next (E);
  end loop;
 
  return NL;
@@ -990,7 +990,7 @@ package body 

[Ada] Additional warnings on overlapping actuals of composite types

2020-06-11 Thread Pierre-Marie de Rodat
This patch refines the handling of warnings on overlapping actuals of
composite types when only one of them is writable. Formals of a generic
type are excluded, given that the warning will be given on any instance.
Uniform treatment of formals and actuals.

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

2020-06-11  Ed Schonberg  

gcc/ada/

* sem_warn.adb (Warn_On_Overlapping_Actuals): Simplify code,
remove inner predicate Is_Covered_Formal, preserve warning for
two overlapping composite types when only one is writable, and
for two overlapping and writable elementary types.--- gcc/ada/sem_warn.adb
+++ gcc/ada/sem_warn.adb
@@ -3643,9 +3643,6 @@ package body Sem_Warn is
-
 
procedure Warn_On_Overlapping_Actuals (Subp : Entity_Id; N : Node_Id) is
-  function Is_Covered_Formal (Formal : Node_Id) return Boolean;
-  --  Return True if Formal is covered by the rule
-
   function Refer_Same_Object
 (Act1 : Node_Id;
  Act2 : Node_Id) return Boolean;
@@ -3658,19 +3655,6 @@ package body Sem_Warn is
   --  (RM 6.4.1(6.11/3))
 
   ---
-  -- Is_Covered_Formal --
-  ---
-
-  function Is_Covered_Formal (Formal : Node_Id) return Boolean is
-  begin
- return
-   Ekind_In (Formal, E_Out_Parameter, E_In_Out_Parameter)
- and then (Is_Elementary_Type (Etype (Formal))
-or else Is_Record_Type (Etype (Formal))
-or else Is_Array_Type (Etype (Formal)));
-  end Is_Covered_Formal;
-
-  ---
   -- Refer_Same_Object --
   ---
 
@@ -3759,137 +3743,182 @@ package body Sem_Warn is
   Form1 := First_Formal (Subp);
   Act1  := First_Actual (N);
   while Present (Form1) and then Present (Act1) loop
- if Is_Covered_Formal (Form1)
-or else not Is_Elementary_Type (Etype (Act1))
+ if Is_Generic_Type (Etype (Act1)) then
+return;
+ end if;
+
+ --  One of the formals must be either (in)-out or composite.
+ --  The other must be (in)-out.
+
+ if Is_Elementary_Type (Etype (Act1))
+   and then Ekind (Form1) = E_In_Parameter
  then
+null;
+
+ else
 Form2 := First_Formal (Subp);
 Act2  := First_Actual (N);
 while Present (Form2) and then Present (Act2) loop
if Form1 /= Form2
- and then Is_Covered_Formal (Form2)
  and then Refer_Same_Object (Act1, Act2)
then
-  --  Guard against previous errors
+  if Is_Generic_Type (Etype (Act2)) then
+ return;
+  end if;
 
-  if Error_Posted (N)
-or else No (Etype (Act1))
-or else No (Etype (Act2))
-  then
- null;
+  --  First case : two writable elementary parameters
+  --  that overlap.
 
-  --  If the actual is a function call in prefix notation,
-  --  there is no real overlap.
+  if (Is_Elementary_Type (Etype (Form1))
+and then Is_Elementary_Type (Etype (Form2))
+and then Ekind (Form1) /= E_In_Parameter
+and then Ekind (Form2) /= E_In_Parameter)
 
-  elsif Nkind (Act2) = N_Function_Call then
- null;
+  --  Second case : two composite parameters that overlap,
+  --  one of which is writable.
 
-  --  If type is not by-copy, assume that aliasing is intended
+or else (Is_Composite_Type (Etype (Form1))
+ and then Is_Composite_Type (Etype (Form2))
+ and then (Ekind (Form1) /= E_In_Parameter
+   or else Ekind (Form2) /= E_In_Parameter))
 
-  elsif
-Present (Underlying_Type (Etype (Form1)))
-  and then
-(Is_By_Reference_Type (Underlying_Type (Etype (Form1)))
-  or else
-Convention (Underlying_Type (Etype (Form1))) =
-  Convention_Ada_Pass_By_Reference)
-  then
- null;
+  --  Third case : an elementary writable parameter that
+  --  overlaps a composite one.
 
-  --  Under Ada 2012 we only report warnings on overlapping
-  --  arrays and record types if switch is set.
+or else (Is_Elementary_Type (Etype (Form1))
+ and then Ekind (Form1) /= E_In_Parameter
+ and then Is_Composite_Type (Etype (Form2)))
 
-  elsif 

[Ada] Skip unnecessary iterations over constraint expressions

2020-06-11 Thread Pierre-Marie de Rodat
When looking for references to discriminants within constraint
expressions we now stop once the first such a reference is found. This
is just a tiny performance improvement; semantics is unaffected.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_ch3.adb (Build_Constrained_Array_Type,
Build_Constrained_Discriminated_Type): Skip unnecessary loop
iterations.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -13093,7 +13093,7 @@ package body Sem_Ch3 is
  Scop  : Entity_Id;
 
   begin
- --  if the original access type was not embedded in the enclosing
+ --  If the original access type was not embedded in the enclosing
  --  type definition, there is no need to produce a new access
  --  subtype. In fact every access type with an explicit constraint
  --  generates an itype whose scope is the enclosing record.
@@ -13192,6 +13192,7 @@ package body Sem_Ch3 is
Is_Discriminant (Hi_Expr)
 then
Need_To_Create_Itype := True;
+   exit;
 end if;
 
 Next_Index (Old_Index);
@@ -13248,6 +13249,7 @@ package body Sem_Ch3 is
 
 if Is_Discriminant (Expr) then
Need_To_Create_Itype := True;
+   exit;
 
 --  After expansion of discriminated task types, the value
 --  of the discriminant may be converted to a run-time type
@@ -13259,6 +13261,7 @@ package body Sem_Ch3 is
   and then Is_Discriminant (Expression (Expr))
 then
Need_To_Create_Itype := True;
+   exit;
 end if;
 
 Next_Elmt (Old_Constraint);



[Ada] Missing accessibility error on object in type conversion

2020-06-11 Thread Pierre-Marie de Rodat
This patch corrects an issue whereby the compiler would incorrectly
calculate accessibility levels of objects within type conversions -
leading to potentially missing static and dynamic errors.

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

2020-06-11  Justin Squirek  

gcc/ada/

* sem_util.adb (Expand_N_Attribute_Reference): Use original
nodes where required to avoid looking at the expanded tree.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -23175,18 +23175,20 @@ package body Sem_Util is
 
   --  Local variables
 
-  E : Entity_Id;
+  E: Entity_Id;
+  Orig_Obj : constant Node_Id := Original_Node (Obj);
+  Orig_Pre : Node_Id;
 
--  Start of processing for Object_Access_Level
 
begin
-  if Nkind (Obj) = N_Defining_Identifier
-or else Is_Entity_Name (Obj)
+  if Nkind (Orig_Obj) = N_Defining_Identifier
+or else Is_Entity_Name (Orig_Obj)
   then
- if Nkind (Obj) = N_Defining_Identifier then
-E := Obj;
+ if Nkind (Orig_Obj) = N_Defining_Identifier then
+E := Orig_Obj;
  else
-E := Entity (Obj);
+E := Entity (Orig_Obj);
  end if;
 
  if Is_Prival (E) then
@@ -23220,14 +23222,17 @@ package body Sem_Util is
 return Scope_Depth (Enclosing_Dynamic_Scope (E));
  end if;
 
-  elsif Nkind_In (Obj, N_Indexed_Component, N_Selected_Component) then
- if Is_Access_Type (Etype (Prefix (Obj))) then
-return Type_Access_Level (Etype (Prefix (Obj)));
+  elsif Nkind_In (Orig_Obj, N_Indexed_Component, N_Selected_Component) then
+ Orig_Pre := Original_Node (Prefix (Orig_Obj));
+
+ if Is_Access_Type (Etype (Orig_Pre)) then
+return Type_Access_Level (Etype (Prefix (Orig_Obj)));
  else
-return Object_Access_Level (Prefix (Obj));
+return Object_Access_Level (Prefix (Orig_Obj));
  end if;
 
-  elsif Nkind (Obj) = N_Explicit_Dereference then
+  elsif Nkind (Orig_Obj) = N_Explicit_Dereference then
+ Orig_Pre := Original_Node (Prefix (Orig_Obj));
 
  --  If the prefix is a selected access discriminant then we make a
  --  recursive call on the prefix, which will in turn check the level
@@ -23239,46 +23244,48 @@ package body Sem_Util is
  --  otherwise expansion will already have transformed the prefix into
  --  a temporary.
 
- if Nkind (Prefix (Obj)) = N_Selected_Component
-   and then Ekind (Etype (Prefix (Obj))) = E_Anonymous_Access_Type
+ if Nkind (Orig_Pre) = N_Selected_Component
+   and then Ekind (Etype (Orig_Pre)) = E_Anonymous_Access_Type
and then
- Ekind (Entity (Selector_Name (Prefix (Obj = E_Discriminant
+ Ekind (Entity (Selector_Name (Orig_Pre))) = E_Discriminant
and then
  (not Has_Implicit_Dereference
-(Entity (Selector_Name (Prefix (Obj
+(Entity (Selector_Name (Orig_Pre)))
or else Nkind (Parent (Obj)) /= N_Selected_Component)
  then
-return Object_Access_Level (Prefix (Obj));
+return Object_Access_Level (Prefix (Orig_Obj));
 
  --  Detect an interface conversion in the context of a dispatching
  --  call. Use the original form of the conversion to find the access
  --  level of the operand.
 
- elsif Is_Interface (Etype (Obj))
-   and then Is_Interface_Conversion (Prefix (Obj))
-   and then Nkind (Original_Node (Obj)) = N_Type_Conversion
+ elsif Is_Interface (Etype (Orig_Obj))
+   and then Is_Interface_Conversion (Orig_Pre)
+   and then Nkind (Orig_Obj) = N_Type_Conversion
  then
-return Object_Access_Level (Original_Node (Obj));
+return Object_Access_Level (Orig_Obj);
 
- elsif not Comes_From_Source (Obj) then
+ elsif not Comes_From_Source (Orig_Obj) then
 declare
-   Ref : constant Node_Id := Reference_To (Obj);
+   Ref : constant Node_Id := Reference_To (Orig_Obj);
 begin
if Present (Ref) then
   return Object_Access_Level (Ref);
else
-  return Type_Access_Level (Etype (Prefix (Obj)));
+  return Type_Access_Level (Etype (Prefix (Orig_Obj)));
end if;
 end;
 
  else
-return Type_Access_Level (Etype (Prefix (Obj)));
+return Type_Access_Level (Etype (Prefix (Orig_Obj)));
  end if;
 
-  elsif Nkind_In (Obj, N_Type_Conversion, N_Unchecked_Type_Conversion) then
- return Object_Access_Level (Expression (Obj));
+  elsif Nkind_In (Orig_Obj, N_Type_Conversion,
+N_Unchecked_Type_Conversion)
+  then
+ return Object_Access_Level (Expression 

[Ada] Simplify iteration over formal parameters for aliasing error

2020-06-11 Thread Pierre-Marie de Rodat
When iterating over pairs of formal parameters we now finish as soon as
we find a single problematic pair; previously we continued iteration.
This is just a simplification and a compiler performance improvement.
Semantics is not affected.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_warn.adb (Warn_On_Overlapping_Actuals): Add label to the
outer loop and use it in the exit statement.--- gcc/ada/sem_warn.adb
+++ gcc/ada/sem_warn.adb
@@ -1835,7 +1835,7 @@ package body Sem_Warn is
  elsif Nkind (Pref) = N_Explicit_Dereference then
 return True;
 
---  If prefix is itself a component reference or slice check prefix
+ --  If prefix is itself a component reference or slice check prefix
 
  elsif Nkind (Pref) = N_Slice
or else Nkind (Pref) = N_Indexed_Component
@@ -3707,7 +3707,7 @@ package body Sem_Warn is
 
   Warn_Only := True;
   Form1 := First_Formal (Subp);
-  while Present (Form1) loop
+  Set_Warn_Only : while Present (Form1) loop
  Form2 := Next_Formal (Form1);
  while Present (Form2) loop
 if Is_Elementary_Type (Etype (Form1))
@@ -3716,14 +3716,14 @@ package body Sem_Warn is
   and then Ekind (Form2) /= E_In_Parameter
 then
Warn_Only := False;
-   exit;
+   exit Set_Warn_Only;
 end if;
 
 Next_Formal (Form2);
  end loop;
 
  Next_Formal (Form1);
-  end loop;
+  end loop Set_Warn_Only;
 
   --  Exclude calls rewritten as enumeration literals
 



[Ada] Make Object Specific Dispatch tables constant

2020-06-11 Thread Pierre-Marie de Rodat
Internally generated static dispatch tables are preferably put in ROM,
so they are declared as constant where possible. However, the Object
Specific Dispatch tables were declared as variables, even though they
are initialized with static aggregates (with only literal integers) and
are never modified (either by the GNAT runtime nor by the code generated
by the GNAT compiler).

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* exp_disp.adb (Make_Secondary_DT): Internally generated OSD
tables are now constant.--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -4310,6 +4310,7 @@ package body Exp_Disp is
 Append_To (Result,
   Make_Object_Declaration (Loc,
 Defining_Identifier => OSD,
+Constant_Present=> True,
 Object_Definition   =>
   Make_Subtype_Indication (Loc,
 Subtype_Mark =>



[Ada] Remove a dubious optimization for Object Specific Data dispatching

2020-06-11 Thread Pierre-Marie de Rodat
Routines Sem_Aggr.Build_Constrained_Itype and Sem_Ch3.Build_Subtype that
create discriminated itypes were originally identical, but now they are
subtly different. This patch removes one of their two subtle
differences, namely a call to Set_Size_Known_At_Compile_Time that was
meant as a very narrow optimization (and was introduced many years ago
without a specific reason).

This call appears to only matter for aggregates of the
Ada.Tags.Object_Specific_Data type, which are part of the secondary
dispatch table machinery. Now, instead of relying on this very specific
optimization we recognize aggregates of this type as static. This is
safe, because such aggregates are only created in the Exp_Disp.Make_DT
routine and are only composed from integer literals.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* exp_disp.adb: Minor reformatting.
* exp_aggr.adb (Is_Static_Dispatch_Table_Aggregate): Recognize
aggregates of the Ada.Tags.Object_Specific_Data type as static.
* sem_aggr.adb (Check_Static_Discriminated_Subtype): Deconstruct
and do not call it from Build_Constrained_Itype.--- gcc/ada/exp_aggr.adb
+++ gcc/ada/exp_aggr.adb
@@ -7790,6 +7790,9 @@ package body Exp_Aggr is
 or else
   Typ = RTE (RE_Tag_Table)
 or else
+  (RTE_Available (RE_Object_Specific_Data)
+ and then Typ = RTE (RE_Object_Specific_Data))
+or else
   (RTE_Available (RE_Interface_Data)
  and then Typ = RTE (RE_Interface_Data))
 or else

--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -4348,7 +4348,7 @@ package body Exp_Disp is
 Attribute_Name => Name_Alignment)));
 
 --  In secondary dispatch tables the Typeinfo component contains
---  the address of the Object Specific Data (see a-tags.ads)
+--  the address of the Object Specific Data (see a-tags.ads).
 
 Append_To (DT_Aggr_List,
   Make_Attribute_Reference (Loc,

--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -226,12 +226,6 @@ package body Sem_Aggr is
--  misspelling of one of the components of the Assoc_List. This is called
--  by Resolve_Aggr_Expr after producing an invalid component error message.
 
-   procedure Check_Static_Discriminated_Subtype (T : Entity_Id; V : Node_Id);
-   --  An optimization: determine whether a discriminated subtype has a static
-   --  constraint, and contains array components whose length is also static,
-   --  either because they are constrained by the discriminant, or because the
-   --  original component bounds are static.
-
-
-- Subprograms used for ARRAY AGGREGATE Processing --
-
@@ -722,66 +716,6 @@ package body Sem_Aggr is
   end if;
end Check_Expr_OK_In_Limited_Aggregate;
 
-   
-   -- Check_Static_Discriminated_Subtype --
-   
-
-   procedure Check_Static_Discriminated_Subtype (T : Entity_Id; V : Node_Id) is
-  Disc : constant Entity_Id := First_Discriminant (T);
-  Comp : Entity_Id;
-  Ind  : Entity_Id;
-
-   begin
-  if Has_Record_Rep_Clause (T) then
- return;
-
-  elsif Present (Next_Discriminant (Disc)) then
- return;
-
-  elsif Nkind (V) /= N_Integer_Literal then
- return;
-  end if;
-
-  Comp := First_Component (T);
-  while Present (Comp) loop
- if Is_Scalar_Type (Etype (Comp)) then
-null;
-
- elsif Is_Private_Type (Etype (Comp))
-   and then Present (Full_View (Etype (Comp)))
-   and then Is_Scalar_Type (Full_View (Etype (Comp)))
- then
-null;
-
- elsif Is_Array_Type (Etype (Comp)) then
-if Is_Bit_Packed_Array (Etype (Comp)) then
-   return;
-end if;
-
-Ind := First_Index (Etype (Comp));
-while Present (Ind) loop
-   if Nkind (Ind) /= N_Range
- or else Nkind (Low_Bound (Ind))  /= N_Integer_Literal
- or else Nkind (High_Bound (Ind)) /= N_Integer_Literal
-   then
-  return;
-   end if;
-
-   Next_Index (Ind);
-end loop;
-
- else
-return;
- end if;
-
- Next_Component (Comp);
-  end loop;
-
-  --  On exit, all components have statically known sizes
-
-  Set_Size_Known_At_Compile_Time (T);
-   end Check_Static_Discriminated_Subtype;
-
-
-- Is_Others_Aggregate --
-
@@ -4509,8 +4443,6 @@ package body Sem_Aggr is
 Analyze (Subtyp_Decl, Suppress => All_Checks);
 
 Set_Etype (N, 

[Ada] Refine type for sorting case-choices tables

2020-06-11 Thread Pierre-Marie de Rodat
Tables with case-choices are sorted by Sort_Case_Table with an insertion
sort. Contrary to what comments for the Case_Table_Type says, this
routine doesn't use the table element at index 0 as a placeholder.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* sem_aggr.adb (Case_Table_Type): Change index type from Nat to
Pos.--- gcc/ada/sem_aggr.adb
+++ gcc/ada/sem_aggr.adb
@@ -85,9 +85,8 @@ package body Sem_Aggr is
   --  The node of the choice
end record;
 
-   type Case_Table_Type is array (Nat range <>) of Case_Bounds;
-   --  Table type used by Check_Case_Choices procedure. Entry zero is not
-   --  used (reserved for the sort). Real entries start at one.
+   type Case_Table_Type is array (Pos range <>) of Case_Bounds;
+   --  Table type used by Check_Case_Choices procedure
 
---
-- Local Subprograms --
@@ -1827,9 +1826,8 @@ package body Sem_Aggr is
 --  if a choice in an aggregate is a subtype indication these
 --  denote the lowest and highest values of the subtype
 
-Table : Case_Table_Type (0 .. Case_Table_Size);
---  Used to sort all the different choice values. Entry zero is
---  reserved for sorting purposes.
+Table : Case_Table_Type (1 .. Case_Table_Size);
+--  Used to sort all the different choice values
 
 Single_Choice : Boolean;
 --  Set to true every time there is a single discrete choice in a



[Ada] Remove useless code in Backend_Processing_Possible

2020-06-11 Thread Pierre-Marie de Rodat
The call to Set_Size_Known_At_Compile_Time in
Backend_Processing_Possible happened just after querying
Size_Known_At_Compile_Time several lines before. Both calls operated on
the same type entity, so the call to "Set" routine had no effect and now
is removed.

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

2020-06-11  Piotr Trojanek  

gcc/ada/

* exp_aggr.adb (Backend_Processing_Possible): Remove useless
call.--- gcc/ada/exp_aggr.adb
+++ gcc/ada/exp_aggr.adb
@@ -743,7 +743,6 @@ package body Exp_Aggr is
 
   --  Backend processing is possible
 
-  Set_Size_Known_At_Compile_Time (Etype (N), True);
   return True;
end Backend_Processing_Possible;
 



[Ada] AI12-0356 Root_Storage_Pool_With_Subpools & Preelaborable_Init

2020-06-11 Thread Pierre-Marie de Rodat
This Ada 202x AI clarifies that Root_Storage_Pool_With_Subpools and
Root_Subpool should have pragma Preelaborable_Initialization.

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

2020-06-11  Arnaud Charlet  

gcc/ada/

* libgnat/s-stposu.ads (Root_Storage_Pool_With_Subpools,
Root_Subpool): Mark with Preelaborable_Initialization.--- gcc/ada/libgnat/s-stposu.ads
+++ gcc/ada/libgnat/s-stposu.ads
@@ -42,12 +42,14 @@ package System.Storage_Pools.Subpools is
 
type Root_Storage_Pool_With_Subpools is abstract
  new Root_Storage_Pool with private;
+   pragma Preelaborable_Initialization (Root_Storage_Pool_With_Subpools);
--  The base for all implementations of Storage_Pool_With_Subpools. This
--  type is Limited_Controlled by derivation. To use subpools, an access
--  type must be associated with an implementation descending from type
--  Root_Storage_Pool_With_Subpools.
 
type Root_Subpool is abstract tagged limited private;
+   pragma Preelaborable_Initialization (Root_Subpool);
--  The base for all implementations of Subpool. Objects of this type are
--  managed by the pool_with_subpools.
 



[Ada] Fix unnesting crash with Predicate_Failure/no pred

2020-06-11 Thread Pierre-Marie de Rodat
This patch fixes a bug where if you have a Predicate_Failure aspect on a
nested type, but no Predicate, Static_Predicate, or Dynamic_Predicate,
the compiler crashes when compiled with assertions enabled.

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

2020-06-11  Bob Duff  

gcc/ada/

* sem_ch13.adb (Analyze_Aspect_Specifications): Do not set the
Has_Predicates flag when the Predicate_Failure aspect is seen.
It is legal (but pointless) to use this aspect without a
predicate.  If we set the flag, we generate a half-baked
Predicate procedure, and if that procedure is nested, it causes
unnesting to crash.--- gcc/ada/sem_ch13.adb
+++ gcc/ada/sem_ch13.adb
@@ -2540,8 +2540,6 @@ package body Sem_Ch13 is
  Expression => Relocate_Node (Expr))),
  Pragma_Name => Name_Predicate_Failure);
 
-  Set_Has_Predicates (E);
-
   --  If the type is private, indicate that its completion
   --  has a freeze node, because that is the one that will
   --  be visible at freeze time.



[Ada] Generate predicate checks for on assignments in records

2020-06-11 Thread Pierre-Marie de Rodat
When an assignment of an allocator the type of which has predicate
checks is made inside of a record, GNAT generates a call to the
predicate function. However, before this commit, GNAT wouldn't check if
the subtype mark had a predicate, which would result in the predicate
check function not being called if no temporary was created for the
designated object.

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

2020-06-11  Ghjuvan Lacambre  

gcc/ada/

* exp_ch3.adb (Build_Assignment): Generate predicate check if
subtype mark has predicate.--- gcc/ada/exp_ch3.adb
+++ gcc/ada/exp_ch3.adb
@@ -1998,6 +1998,20 @@ package body Exp_Ch3 is
 Append (Make_Predicate_Check (Typ, Exp), Res);
  end if;
 
+ if Nkind (Exp) = N_Allocator
+and then Nkind (Expression (Exp)) = N_Qualified_Expression
+ then
+declare
+   Subtype_Entity : constant Entity_Id
+  := Entity (Subtype_Mark (Expression (Exp)));
+begin
+   if Has_Predicates (Subtype_Entity) then
+  Append (Make_Predicate_Check
+ (Subtype_Entity, Expression (Expression (Exp))), Res);
+   end if;
+end;
+ end if;
+
  return Res;
 
   exception



Re: [committed] gcc-changelog: Use non-zero exit status on error

2020-06-11 Thread Martin Liška

On 6/11/20 11:47 AM, Jonathan Wakely wrote:

Is the attached patch what you're asking for?


Yes ;)

Martin


Re: [committed] gcc-changelog: Use non-zero exit status on error

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

On 11/06/20 10:54 +0200, Martin Liška wrote:

On 6/11/20 10:48 AM, Jonathan Wakely wrote:

On 11/06/20 09:54 +0200, Martin Liška wrote:

On 6/10/20 2:20 PM, Jonathan Wakely wrote:

Oops, this line was left in while I was testing it by amending
existing commits!

Here's an updated patch without that line.


I generally like the suggested idea and I have suggestion to usage of the 
GCC_FORCE_MKLOG.
Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' 
alias.


Ah yes, I forgot about that alias (I don't use it).


I use it (and I want to distinguish in between it and normal commit command) ;)


OK. I'd rather just have one command that always does the right thing,
so want the hook to always run. But I can just change my local copy.


Wouldn't it be better to only add

git config gcc-config.mklog-hook-type [always|smart-amend]

and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?


The point of GCC_FORCE_MKLOG=no was as an override to disable the hook
if you have it enabled in your git config options.

If my git config enables the hook, but I want it off for a single
commit, I can use the env var to disable it.

For your scenario, you want the hook off by default, but enabled when
the env var is set (by the gcc-commit-mklog alias).


Sure, that works for me.


Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?


No, I was actually using a modified version of the script that didn't
check the env var at all, because I always wanted it to be used.


I bet some other people (including me) want to have 2 different commands.



I'll prepare a patch that replaces the env var completely, and also
modifies the contrib/gcc-git-customization.sh script to optionally
enable the hook.


I would preserve it for the alias.


OK, I misunderstood what you were suggesting.

Is the attached patch what you're asking for?


Martin



Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
won't do anything different from 'git commit'?




commit c0e9ad58b132d01a3d7bb7eaeb981324eb55074a
Author: Jonathan Wakely 
Date:   Thu Jun 11 10:22:04 2020 +0100

contrib: Make prepare-commit-msg hook smarter for amends

With this change the prepare-commit-msg hook can compare the log of a
commit being amended with the staged changes, and not run mklog.py
unnecessarily. This is controlled by a git config option,
gcc-config.mklog-hook-type.

contrib/ChangeLog:

* prepare-commit-msg: Use the gcc-config.mklog-hook-type Git
config key instead of the GCC_FORCE_MKLOG environment variable.
Optionally disable generating a new ChangeLog template for
amended commits when the existing log is still OK.

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index cc9ba2e6ba1..969847df6f4 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -49,6 +49,19 @@ elif [ $COMMIT_SOURCE = commit ]; then
 # otherwise, assume a new commit with -C.
 if [ $SHA1 = HEAD ]; then
 	cmd="diff --cached HEAD^"
+	if [ "$(git config gcc-config.mklog-hook-type)" = "smart-amend" ]; then
+	# Check if the existing message still describes the staged changes.
+	f=$(mktemp /tmp/git-commit.XX) || exit 1
+	git log -1 --pretty=email HEAD > $f
+	printf '\n---\n\n' >> $f
+	git $cmd >> $f
+	if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then
+		# Existing commit message is still OK for amended commit.
+		rm $f
+		exit 0
+	fi
+	rm $f
+	fi
 else
 	cmd="diff --cached"
 fi


Re: [PATCH] asan: fix RTX emission for ilp32

2020-06-11 Thread Martin Liška

On 6/11/20 10:50 AM, Jakub Jelinek wrote:

On Thu, Jun 11, 2020 at 10:12:14AM +0200, Martin Liška wrote:

gcc/ChangeLog:

PR sanitizer/95634
* asan.c (asan_emit_stack_protection): Fix emission for ilp32
by using Pmode instead of ptr_mode.
---
  gcc/asan.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index e015fa3ec9b..5d123a3e8a6 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned 
int alignb,
= (1 << (use_after_return_class + 6));
  offset -= GET_MODE_SIZE (ptr_mode);
  mem = gen_rtx_MEM (ptr_mode, base);
- mem = adjust_address (mem, ptr_mode, offset);
- rtx addr = gen_reg_rtx (ptr_mode);
+ mem = adjust_address (mem, Pmode, offset);
+ rtx addr = gen_reg_rtx (Pmode);


That is not correct.  On the architectures where ptr_mode != Pmode,
when you are reading a pointer from memory, you want to use ptr_mode,
because that is how the pointer is represented in memory.
So, it needs to stay:
  mem = gen_rtx_MEM (ptr_mode, base);
  mem = adjust_address (mem, ptr_mode, offset);
  rtx addr = gen_reg_rtx (ptr_mode);
  emit_move_insn (addr, mem);
But, at this point addr is ptr_mode, but you need to convert it into Pmode.
  addr = convert_memory_address (Pmode, addr);
This one will do nothing at all on normal arches where ptr_mode == Pmode,
and perform some extension (zero/sign/whatever else the arch needs)
otherwise.


Thank you for help, I'm going to push the patch.

Martin




  mem = gen_rtx_MEM (QImode, addr);
  emit_move_insn (mem, const0_rtx);
--
2.26.2


Jakub





Re: [committed] gcc-changelog: Use non-zero exit status on error

2020-06-11 Thread Martin Liška

On 6/11/20 10:48 AM, Jonathan Wakely wrote:

On 11/06/20 09:54 +0200, Martin Liška wrote:

On 6/10/20 2:20 PM, Jonathan Wakely wrote:

Oops, this line was left in while I was testing it by amending
existing commits!

Here's an updated patch without that line.


I generally like the suggested idea and I have suggestion to usage of the 
GCC_FORCE_MKLOG.
Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' 
alias.


Ah yes, I forgot about that alias (I don't use it).


I use it (and I want to distinguish in between it and normal commit command) ;)




Wouldn't it be better to only add

git config gcc-config.mklog-hook-type [always|smart-amend]

and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?


Sure, that works for me.


Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?


No, I was actually using a modified version of the script that didn't
check the env var at all, because I always wanted it to be used.


I bet some other people (including me) want to have 2 different commands.



I'll prepare a patch that replaces the env var completely, and also
modifies the contrib/gcc-git-customization.sh script to optionally
enable the hook.


I would preserve it for the alias.

Martin



Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
won't do anything different from 'git commit'?






Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.

2020-06-11 Thread Martin Liška

On 6/9/20 3:42 PM, Richard Biener wrote:

I think we need to fix that before merging.


There's updated version of the patch that should handle the EH properly.

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

Ready to be installed?
Thanks,
Martin
>From fc5a59e8c8887c102bff06e1a537ccfc9d44e3d8 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Mon, 9 Mar 2020 13:23:03 +0100
Subject: [PATCH] Lower VEC_COND_EXPR into internal functions.

gcc/ChangeLog:

2020-03-30  Martin Liska  

	* expr.c (expand_expr_real_2): Put gcc_unreachable, we should reach
	this path.
	(do_store_flag): Likewise here.
	* internal-fn.c (vec_cond_mask_direct): New.
	(vec_cond_direct): Likewise.
	(vec_condu_direct): Likewise.
	(vec_condeq_direct): Likewise.
	(expand_vect_cond_optab_fn): Move from optabs.c.
	(expand_vec_cond_optab_fn): New alias.
	(expand_vec_condu_optab_fn): Likewise.
	(expand_vec_condeq_optab_fn): Likewise.
	(expand_vect_cond_mask_optab_fn): Moved from optabs.c.
	(expand_vec_cond_mask_optab_fn): New alias.
	(direct_vec_cond_mask_optab_supported_p): New.
	(direct_vec_cond_optab_supported_p): Likewise.
	(direct_vec_condu_optab_supported_p): Likewise.
	(direct_vec_condeq_optab_supported_p): Likewise.
	* internal-fn.def (VCOND): New new internal optab
	function.
	(VCONDU): Likewise.
	(VCONDEQ): Likewise.
	(VCOND_MASK): Likewise.
	* optabs.c (expand_vec_cond_mask_expr): Removed.
	(expand_vec_cond_expr): Likewise.
	* optabs.h (expand_vec_cond_expr): Likewise.
	(vector_compare_rtx): Likewise.
	* passes.def: Add pass_gimple_isel.
	* tree-cfg.c (verify_gimple_assign_ternary): Add new
	GIMPLE check.
	* tree-pass.h (make_pass_gimple_isel): New.
	* tree-ssa-forwprop.c (pass_forwprop::execute): Do not forward
	to already lowered VEC_COND_EXPR.
	* tree-vect-generic.c (expand_vector_divmod): Expand to SSA_NAME.
	(expand_vector_condition): Expand tcc_comparison of a VEC_COND_EXPR
	into a SSA_NAME.
	(expand_vector_condition): Add new argument.
	(expand_vector_operations): Likewise.
	(expand_vector_operations_1): Fix up EH by moving that to vector
	comparison.
	* tree-vect-isel.c: New file.

gcc/testsuite/ChangeLog:

	* g++.dg/vect/vec-cond-expr-eh.C: New test.
---
 gcc/Makefile.in  |   2 +
 gcc/expr.c   |  25 +-
 gcc/internal-fn.c|  89 +++
 gcc/internal-fn.def  |   5 +
 gcc/optabs.c | 124 +-
 gcc/optabs.h |   7 +-
 gcc/passes.def   |   1 +
 gcc/testsuite/g++.dg/vect/vec-cond-expr-eh.C |  17 ++
 gcc/tree-cfg.c   |   8 +
 gcc/tree-pass.h  |   1 +
 gcc/tree-ssa-forwprop.c  |   6 +
 gcc/tree-vect-generic.c  |  71 --
 gcc/tree-vect-isel.c | 244 +++
 13 files changed, 431 insertions(+), 169 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/vect/vec-cond-expr-eh.C
 create mode 100644 gcc/tree-vect-isel.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 4f70c189b9d..4cbb9d23606 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1631,6 +1631,7 @@ OBJS = \
 	tree-streamer-out.o \
 	tree-tailcall.o \
 	tree-vect-generic.o \
+	tree-vect-isel.o \
 	tree-vect-patterns.o \
 	tree-vect-data-refs.o \
 	tree-vect-stmts.o \
@@ -2600,6 +2601,7 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
   $(srcdir)/dwarf2cfi.c \
   $(srcdir)/dwarf2out.c \
   $(srcdir)/tree-vect-generic.c \
+  $(srcdir)/tree-vect-isel.c \
   $(srcdir)/dojump.c $(srcdir)/emit-rtl.h \
   $(srcdir)/emit-rtl.c $(srcdir)/except.h $(srcdir)/explow.c $(srcdir)/expr.c \
   $(srcdir)/expr.h \
diff --git a/gcc/expr.c b/gcc/expr.c
index ca6b1c1291e..3c68b0d754c 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9316,17 +9316,8 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
   if (temp != 0)
 	return temp;
 
-  /* For vector MIN , expand it a VEC_COND_EXPR 
-	 and similarly for MAX .  */
   if (VECTOR_TYPE_P (type))
-	{
-	  tree t0 = make_tree (type, op0);
-	  tree t1 = make_tree (type, op1);
-	  tree comparison = build2 (code == MIN_EXPR ? LE_EXPR : GE_EXPR,
-type, t0, t1);
-	  return expand_vec_cond_expr (type, comparison, t0, t1,
-   original_target);
-	}
+	gcc_unreachable ();
 
   /* At this point, a MEM target is no longer useful; we will get better
 	 code without it.  */
@@ -9915,10 +9906,6 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
 	return temp;
   }
 
-case VEC_COND_EXPR:
-  target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target);
-  return target;
-
 case VEC_DUPLICATE_EXPR:
   op0 = expand_expr (treeop0, NULL_RTX, VOIDmode, modifier);
   target = expand_vector_broadcast (mode, op0);
@@ -12249,8 +12236,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
   

Re: [PATCH] asan: fix RTX emission for ilp32

2020-06-11 Thread Jakub Jelinek via Gcc-patches
On Thu, Jun 11, 2020 at 10:12:14AM +0200, Martin Liška wrote:
> gcc/ChangeLog:
> 
>   PR sanitizer/95634
>   * asan.c (asan_emit_stack_protection): Fix emission for ilp32
>   by using Pmode instead of ptr_mode.
> ---
>  gcc/asan.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/asan.c b/gcc/asan.c
> index e015fa3ec9b..5d123a3e8a6 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, 
> unsigned int alignb,
>   = (1 << (use_after_return_class + 6));
> offset -= GET_MODE_SIZE (ptr_mode);
> mem = gen_rtx_MEM (ptr_mode, base);
> -   mem = adjust_address (mem, ptr_mode, offset);
> -   rtx addr = gen_reg_rtx (ptr_mode);
> +   mem = adjust_address (mem, Pmode, offset);
> +   rtx addr = gen_reg_rtx (Pmode);

That is not correct.  On the architectures where ptr_mode != Pmode,
when you are reading a pointer from memory, you want to use ptr_mode,
because that is how the pointer is represented in memory.
So, it needs to stay:
  mem = gen_rtx_MEM (ptr_mode, base);
  mem = adjust_address (mem, ptr_mode, offset);
  rtx addr = gen_reg_rtx (ptr_mode);
  emit_move_insn (addr, mem);
But, at this point addr is ptr_mode, but you need to convert it into Pmode.
  addr = convert_memory_address (Pmode, addr);
This one will do nothing at all on normal arches where ptr_mode == Pmode,
and perform some extension (zero/sign/whatever else the arch needs)
otherwise.

> mem = gen_rtx_MEM (QImode, addr);
> emit_move_insn (mem, const0_rtx);
> -- 
> 2.26.2

Jakub



Re: [committed] gcc-changelog: Use non-zero exit status on error

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

On 11/06/20 09:54 +0200, Martin Liška wrote:

On 6/10/20 2:20 PM, Jonathan Wakely wrote:

Oops, this line was left in while I was testing it by amending
existing commits!

Here's an updated patch without that line.


I generally like the suggested idea and I have suggestion to usage of the 
GCC_FORCE_MKLOG.
Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' 
alias.


Ah yes, I forgot about that alias (I don't use it).


Wouldn't it be better to only add

git config gcc-config.mklog-hook-type [always|smart-amend]

and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?


Sure, that works for me.


Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?


No, I was actually using a modified version of the script that didn't
check the env var at all, because I always wanted it to be used.

I'll prepare a patch that replaces the env var completely, and also
modifies the contrib/gcc-git-customization.sh script to optionally
enable the hook.

Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
won't do anything different from 'git commit'?




[PATCH] Fix few -Wformat-diag warnings.

2020-06-11 Thread Martin Liška

Ready for master?

Thanks,
Martin

gcc/ChangeLog:

* cgraphunit.c (process_symver_attribute): Wrap weakref keyword.
* dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new
line.
* lto-wrapper.c (merge_and_complain): Wrap option names.
---
 gcc/cgraphunit.c  |  2 +-
 gcc/dbgcnt.c  |  2 +-
 gcc/lto-wrapper.c | 12 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 01b3f82a4b2..ea9a34bda6f 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -762,7 +762,7 @@ process_symver_attribute (symtab_node *n)
   if (n->weakref)
 {
   error_at (DECL_SOURCE_LOCATION (n->decl),
-   "weakref cannot be versioned");
+   "% cannot be versioned");
   return;
 }
   if (!TREE_PUBLIC (n->decl))
diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c
index b0dd893ed49..ae98a281d63 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -126,7 +126,7 @@ dbg_cnt_set_limit_by_index (enum debug_counter index, const 
char *name,
   if (t1.first <= t2.second)
{
  error ("Interval overlap of %<-fdbg-cnt=%s%>: [%u, %u] and "
-"[%u, %u]\n", name, t2.first, t2.second, t1.first, t1.second);
+"[%u, %u]", name, t2.first, t2.second, t1.first, t1.second);
  return false;
}
 }
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index d565b0861f5..8fbca7cabc4 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -508,24 +508,24 @@ merge_and_complain (struct cl_decoded_option 
**decoded_options,
  break;
else if (i < *decoded_options_count && j == fdecoded_options_count)
  {
-   warning (0, "Extra option to -Xassembler: %s,"
-" dropping all -Xassembler and -Wa options.",
+   warning (0, "Extra option to %<-Xassembler%>: %s,"
+" dropping all %<-Xassembler%> and %<-Wa%> options.",
 (*decoded_options)[i].arg);
xassembler_options_error = true;
break;
  }
else if (i == *decoded_options_count && j < fdecoded_options_count)
  {
-   warning (0, "Extra option to -Xassembler: %s,"
-" dropping all -Xassembler and -Wa options.",
+   warning (0, "Extra option to %<-Xassembler%>: %s,"
+" dropping all %<-Xassembler%> and %<-Wa%> options.",
 fdecoded_options[j].arg);
xassembler_options_error = true;
break;
  }
else if (strcmp ((*decoded_options)[i].arg, fdecoded_options[j].arg))
  {
-   warning (0, "Options to Xassembler do not match: %s, %s,"
-" dropping all -Xassembler and -Wa options.",
+   warning (0, "Options to %<-Xassembler%> do not match: %s, %s,"
+" dropping all %<-Xassembler%> and %<-Wa%> options.",
 (*decoded_options)[i].arg, fdecoded_options[j].arg);
xassembler_options_error = true;
break;
--
2.26.2



[PATCH] asan: fix RTX emission for ilp32

2020-06-11 Thread Martin Liška

Hello.

There's a patch for ilp32 where we should use Pmode instead of ptr_mode.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

PR sanitizer/95634
* asan.c (asan_emit_stack_protection): Fix emission for ilp32
by using Pmode instead of ptr_mode.
---
 gcc/asan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index e015fa3ec9b..5d123a3e8a6 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned 
int alignb,
= (1 << (use_after_return_class + 6));
  offset -= GET_MODE_SIZE (ptr_mode);
  mem = gen_rtx_MEM (ptr_mode, base);
- mem = adjust_address (mem, ptr_mode, offset);
- rtx addr = gen_reg_rtx (ptr_mode);
+ mem = adjust_address (mem, Pmode, offset);
+ rtx addr = gen_reg_rtx (Pmode);
  emit_move_insn (addr, mem);
  mem = gen_rtx_MEM (QImode, addr);
  emit_move_insn (mem, const0_rtx);
--
2.26.2



Re: [committed] gcc-changelog: Use non-zero exit status on error

2020-06-11 Thread Martin Liška

On 6/11/20 9:45 AM, Martin Liška wrote:

On 6/10/20 2:15 PM, Jonathan Wakely wrote:

And this is another little patch that just avoids running 'git diff'
twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect
of generating the diff to pipe to mklog.py.


Thanks, please install the improvement.

Martin


I've pushed the patch as I'm going to replace my GCC_GIT_DIFF_FILE environment
variable with a more feasible 'git config gcc-config.diff-file'.

Pushed to master as well.
Martin
>From 2dd6d9d4029d99c5ad54a21677506bd25e3f7540 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 11 Jun 2020 10:02:26 +0200
Subject: [PATCH] prepare-commit-hook: Use gcc-config.diff-file.

contrib/ChangeLog:

	* prepare-commit-msg: Replace ENV variable with a git config
	value.
---
 contrib/prepare-commit-msg | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 57bb91747f6..d4971e65092 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -58,8 +58,9 @@ else
 fi
 
 # Save diff to a file if requested.
-if ! [ -z "$GCC_GIT_DIFF_FILE" ]; then
-tee="tee $GCC_GIT_DIFF_FILE"
+DIFF_FILE = $(git config gcc-config.diff-file)
+if ! [ -z "$DIFF_FILE" ]; then
+tee="tee $DIFF_FILE"
 else
 tee="cat"
 fi
-- 
2.26.2



Re: [committed] gcc-changelog: Use non-zero exit status on error

2020-06-11 Thread Martin Liška

On 6/10/20 2:20 PM, Jonathan Wakely wrote:

Oops, this line was left in while I was testing it by amending
existing commits!

Here's an updated patch without that line.


I generally like the suggested idea and I have suggestion to usage of the 
GCC_FORCE_MKLOG.
Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' 
alias.
Wouldn't it be better to only add

git config gcc-config.mklog-hook-type [always|smart-amend]

and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?
Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?

Thanks,
Martin


Re: [committed] gcc-changelog: Use non-zero exit status on error

2020-06-11 Thread Martin Liška

On 6/10/20 2:15 PM, Jonathan Wakely wrote:

And this is another little patch that just avoids running 'git diff'
twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect
of generating the diff to pipe to mklog.py.


Thanks, please install the improvement.

Martin


[PATCH 4/4 V2] vect: Factor out and rename some functions/macros

2020-06-11 Thread Kewen.Lin via Gcc-patches
on 2020/6/11 上午12:58, Richard Sandiford wrote:
> "Kewen.Lin"  writes:
>> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
>> index ca68d04a919..1fac5898525 100644
>> --- a/gcc/tree-vect-loop-manip.c
>> +++ b/gcc/tree-vect-loop-manip.c
>> @@ -420,8 +420,8 @@ vect_set_loop_controls_directly (class loop *loop, 
>> loop_vec_info loop_vinfo,
>>   rgroup_controls *rgc, tree niters,
>>   tree niters_skip, bool might_wrap_p)
>>  {
>> -  tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo);
>> -  tree iv_type = LOOP_VINFO_MASK_IV_TYPE (loop_vinfo);
>> +  tree compare_type = LOOP_VINFO_COMPARE_TYPE (loop_vinfo);
>> +  tree iv_type = LOOP_VINFO_IV_TYPE (loop_vinfo);
> 
> How about s/MASK/RGROUP/ instead?  COMPARE_TYPE and IV_TYPE sound a bit
> too generic, and might give the impression that they're meaningful for
> classic full-vector vectorisation too.
> 
>> @@ -748,13 +748,12 @@ vect_set_loop_condition_masked (class loop *loop, 
>> loop_vec_info loop_vinfo,
>>  }
>>  
>>  /* Like vect_set_loop_condition, but handle the case in which there
>> -   are no loop masks.  */
>> +   are no partial vectorization loops.  */
> 
> Maybe:
> 
>   … in which the vector loop handles exactly VF scalars per iteration.
> 
>> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
>> index 7ea75d6d095..b6e96f77f69 100644
>> --- a/gcc/tree-vect-loop.c
>> +++ b/gcc/tree-vect-loop.c
>> @@ -155,6 +155,7 @@ along with GCC; see the file COPYING3.  If not see
>>  static void vect_estimate_min_profitable_iters (loop_vec_info, int *, int 
>> *);
>>  static stmt_vec_info vect_is_simple_reduction (loop_vec_info, stmt_vec_info,
>> bool *, bool *);
>> +static bool known_niters_smaller_than_vf (loop_vec_info);
> 
> Please instead define the function before its first caller.
> Adding “vect_” to the beginning of the name would probably be
> more consistent.
> 
>> [...]
>> @@ -959,14 +960,41 @@ vect_get_max_nscalars_per_iter (loop_vec_info 
>> loop_vinfo)
>>return res;
>>  }
>>  
>> +/* Calculate the minimal bits necessary to represent the maximal iteration
>> +   count of loop with loop_vec_info LOOP_VINFO which is scaling with a given
>> +   factor FACTOR.  */
> 
> How about:
> 
> /* Calculate the minimum precision necessary to represent:
> 
>   MAX_NITERS * FACTOR
> 
>as an unsigned integer, where MAX_NITERS is the maximum number of
>loop header iterations for the original scalar form of LOOP_VINFO.  */
> 
>> +
>> +static unsigned
>> +min_prec_for_max_niters (loop_vec_info loop_vinfo, unsigned int factor)
> 
> Here too I think a “vect_” prefix would be more consistent.
> 
>> +{
>> +  class loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
>> +
>> +  /* Get the maximum number of iterations that is representable
>> + in the counter type.  */
>> +  tree ni_type = TREE_TYPE (LOOP_VINFO_NITERSM1 (loop_vinfo));
>> +  widest_int max_ni = wi::to_widest (TYPE_MAX_VALUE (ni_type)) + 1;
>> +
>> +  /* Get a more refined estimate for the number of iterations.  */
>> +  widest_int max_back_edges;
>> +  if (max_loop_iterations (loop, _back_edges))
>> +max_ni = wi::smin (max_ni, max_back_edges + 1);
>> +
>> +  /* Account for factor, in which each bit is replicated N times.  */
> 
> The “, in which each bit …” part no longer makes sense in this generic
> context.  Probably best just to drop the comment altogether and…
> 
>> +  max_ni *= factor;
>> +
>> +  /* Work out how many bits we need to represent the limit.  */
>> +  unsigned int min_ni_width = wi::min_precision (max_ni, UNSIGNED);
>> +
>> +  return min_ni_width;
> 
> …change this to:
> 
>   /* Work out how many bits we need to represent the limit.  */
>   return wi::min_precision (max_ni * factor, UNSIGNED);
> 
>> [...]
>> @@ -6813,8 +6820,8 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
>>  {
>>if (dump_enabled_p ())
>>  dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> - "can't use a fully-masked loop because no"
>> - " conditional operation is available.\n");
>> + "can't use a partial vectorization loop because"
>> + " no conditional operation is available.\n");
> 
> Maybe “can't operate on partial vectors because…”.  Same for the
> later changes.
> 
>> @@ -9194,12 +9202,13 @@ optimize_mask_stores (class loop *loop)
>>  }
>>  
>>  /* Decide whether it is possible to use a zero-based induction variable
>> -   when vectorizing LOOP_VINFO with a fully-masked loop.  If it is,
>> -   return the value that the induction variable must be able to hold
>> -   in order to ensure that the loop ends with an all-false mask.
>> +   when vectorizing LOOP_VINFO with a partial vectorization loop.  If
> 
> Maybe ”…with partial vectors”
> 
>> +   it is, return the value that the induction variable must be able to
>> +   hold in order to ensure that 

[PATCH][OBVIOUS] Fix -Wformat-diag in options-save.c

2020-06-11 Thread Martin Liška

The patch removes bunch of warnings:

options-save.c:12004:29: warning: unquoted identifier or keyword 
‘global_options’ in format [-Wformat-diag]
12004 | internal_error ("Error: global_options are modified in local 
context\n");

gcc/ChangeLog:

* optc-save-gen.awk: Quote error string.
---
 gcc/optc-save-gen.awk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 4a0e5ab64f3..1b010085b75 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -967,7 +967,7 @@ for (i = 0; i < n_opts; i++) {
checked_options[name]++
 
 	print "  if (ptr1->x_" name " != ptr2->x_" name ")"

-   print "internal_error (\"Error: global_options are modified in local 
context\\n\");";
+   print "internal_error (\"% are modified in local 
context\");";
 }
 
 print "}";

--
2.26.2



[PATCH] PR 83938 Reduce memory consumption in stable_sort/inplace_merge

2020-06-11 Thread François Dumont via Gcc-patches

As we are on patching algos we still have this old one.

    From the original patch I only kept the memory optimization part as 
the new performance test was not showing good result for the other part 
to change pivot value. I also kept the small change in 
get_temporary_buffer even if I don't have strong feeling about it, it 
just make sure that we'll try to allocate 1 element as a last chance 
allocation.


    Note that there is still place for an improvement. If we miss 
memory on the heap we then use a recursive implementation which then 
rely on stack memory. I would be surprise that a system which miss heap 
memory would have no problem to allocate about the same on the stack so 
we will surely end up in a stack overflow. I still have this on my todo 
even if I already made several tries with no satisfying result in terms 
of performance.


    Tested under Linux x86_64.

Commit message:

    libstdc++: Limit memory allocation in stable_sort/inplace_merge (PR 
83938)


    Reduce memory consumption in stable_sort/inplace_merge to what is used.

    Co-authored-by: François Dumont  

    libstdc++-v3/ChangeLog:

    2020-06-11  John Chang  
    François Dumont  

    PR libstdc++/83938
    * include/bits/stl_tempbuf.h (get_temporary_buffer): Change 
__len

    computation in the loop.
    * include/bits/stl_algo.h:
    (__inplace_merge): Take temporary buffer length from 
smallest range.

    (__stable_sort): Limit temporary buffer length.
    * testsuite/25_algorithms/inplace_merge/1.cc (test03): Test 
different

    pivot positions.
    * testsuite/performance/25_algorithms/stable_sort.cc: Test 
stable_sort

    under different heap memory conditions.
    * testsuite/performance/25_algorithms/inplace_merge.cc: New.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index fd6edd0d5f4..44798ffaa7f 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -2531,7 +2531,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   const _DistanceType __len2 = std::distance(__middle, __last);
 
   typedef _Temporary_buffer<_BidirectionalIterator, _ValueType> _TmpBuf;
-  _TmpBuf __buf(__first, __len1 + __len2);
+  _TmpBuf __buf(__first, std::min(__len1, __len2));
 
   if (__buf.begin() == 0)
 	std::__merge_without_buffer
@@ -2740,6 +2740,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  std::__merge_sort_with_buffer(__first, __middle, __buffer, __comp);
 	  std::__merge_sort_with_buffer(__middle, __last, __buffer, __comp);
 	}
+
   std::__merge_adaptive(__first, __middle, __last,
 			_Distance(__middle - __first),
 			_Distance(__last - __middle),
@@ -5006,8 +5007,11 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   typedef typename iterator_traits<_RandomAccessIterator>::difference_type
 	_DistanceType;
 
+  if (__first == __last)
+	return;
+
   typedef _Temporary_buffer<_RandomAccessIterator, _ValueType> _TmpBuf;
-  _TmpBuf __buf(__first, std::distance(__first, __last));
+  _TmpBuf __buf(__first, (__last - __first + 1) / 2);
 
   if (__buf.begin() == 0)
 	std::__inplace_stable_sort(__first, __last, __comp);
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index f6f17960472..d76ed7f7ea6 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -110,7 +110,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			std::nothrow));
 	  if (__tmp != 0)
 	return std::pair<_Tp*, ptrdiff_t>(__tmp, __len);
-	  __len /= 2;
+	  __len = __len == 1 ? 0 : ((__len + 1) / 2);
 	}
   return std::pair<_Tp*, ptrdiff_t>(static_cast<_Tp*>(0), 0);
 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/inplace_merge/1.cc b/libstdc++-v3/testsuite/25_algorithms/inplace_merge/1.cc
index 5859f0363d5..7f78687d0aa 100644
--- a/libstdc++-v3/testsuite/25_algorithms/inplace_merge/1.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/inplace_merge/1.cc
@@ -28,7 +28,7 @@ using std::inplace_merge;
 
 typedef test_container container;
 
-void 
+void
 test1()
 {
   int array[] = { 1 };
@@ -39,7 +39,7 @@ test1()
   inplace_merge(con2.begin(), con2.begin(), con2.end());
 }
 
-void 
+void
 test2()
 {
   int array[] = { 0, 2, 4, 1, 3, 5 };
@@ -60,30 +60,34 @@ struct S
   { return a < _s.a; }
 };
 
-void 
+void
 test3()
 {
   S s[8];
-  s[0].a = 0;
-  s[1].a = 1;
-  s[2].a = 2;
-  s[3].a = 3;
-  s[4].a = 0;
-  s[5].a = 1;
-  s[6].a = 2;
-  s[7].a = 3;
-
-  s[0].b = 0;
-  s[1].b = 1;
-  s[2].b = 2;
-  s[3].b = 3;
-  s[4].b = 4;
-  s[5].b = 5;
-  s[6].b = 6;
-  s[7].b = 7;
-
-  inplace_merge(s, s + 4, s + 8);
-  VERIFY( s[0].b == 0 && s[1].b == 4 && s[2].b == 1 && s[3].b == 5 );
+  for (int pivot_idx = 0; pivot_idx < 8; ++pivot_idx)
+{
+  int bval = 0;
+  for (int i = 0; i != pivot_idx; ++i)
+	{
+	  s[i].a = i;
+	  s[i].b = bval++;
+	}
+
+