Re: libgo patch committed: Update to 1.11 release

[Andreas Schwab]

On Sep 24 2018, Ian Lance Taylor  wrote:

> * Makefile.am (mostlyclean-local): Run chmod on check-go-dir to
> make sure it is writable.
> (check-go-tools): Likewise.

$ make check-gotools
make[1]: Entering directory `/usr/local/gcc/gcc-20180926/Build/gotools'
chmod -R u+w check-go-dir
chmod: cannot access `check-go-dir': No such file or directory
make[1]: *** [check-go-tool] Error 1
make[1]: Leaving directory `/usr/local/gcc/gcc-20180926/Build/gotools'
make: *** [check-gotools] Error 2

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: libgo patch committed: Update to 1.11 release

[Uros Bizjak]

> I've committed a patch to update libgo to the 1.11 release.  As usual
> for these updates, the patch is too large to attach to this e-mail
> message.  I've attached some of the more relevant directories.  This
> update required some minor patches to the gotools directory and the Go
> testsuite, also included here.  Bootstrapped and ran Go testsuite on
> x86_64-pc-linux-gnu.  Committed to mainline.

Fails to build on alpha-linux-gnu:

/space/homedirs/uros/gcc-svn/trunk/libgo/go/syscall/setuidgid_linux.go:11:16:
error: reference to undefined name ‘SYS_GETEUID’
11 |  sys_GETEUID = SYS_GETEUID
   |^

This is because /usr/include/asm/unistd.h says:

/*
 * Ignore legacy syscalls that we don't use.
 */
#define __IGNORE_alarm
#define __IGNORE_creat
#define __IGNORE_getegid
#define __IGNORE_geteuid
#define __IGNORE_getgid
#define __IGNORE_getpid
#define __IGNORE_getppid
#define __IGNORE_getuid
#define __IGNORE_pause
#define __IGNORE_time
#define __IGNORE_utime
#define __IGNORE_umount2

These legacy syscalls are undefined for alpha-linux-gnu.

Uros.

Re: [PATCH] Fix unwind info in -mindirect-branch=thunk thunks (PR target/87414)

[Uros Bizjak]

On Tue, Sep 25, 2018 at 9:31 AM, Jakub Jelinek  wrote:
> Hi!
>
> The indirect branch thunks we emit look like:
> __x86_indirect_thunk_rax:
> .cfi_startproc
> call.LIND1
> .LIND0:
> pause
> lfence
> jmp .LIND0
> .LIND1:
> mov %rax, (%rsp)
> ret
> .cfi_endproc
>
> The problem is that the CFI is incorrect.  On the first entry it is correct,
> the default is CFA %rsp+8 and rip at cfa-8, before call .LIND1 is executed,
> the return address (to whatever called the thunk) is at %rsp+8 like on entry
> to any other function.  But the call insn pushes a word to the stack, which
> we don't want to show up as another caller, and worse is in the mov insn
> overwritten with something else.
>
> The following patch adds .cfi_def_cfa_offset 16 right after the .LIND1
> label, so that the CFI is correct until the end of the thunk.  It works
> properly also with -fno-dwarf2-cfi-asm.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-09-25  Jakub Jelinek  
>
> PR target/87414
> * config/i386/i386.c: Include debug.h and dwarf2out.h.
> (output_indirect_thunk): Emit DW_CFA_def_cfa_offset after the
> call.

LGTM.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2018-09-24 10:36:54.559016673 +0200
> +++ gcc/config/i386/i386.c  2018-09-24 22:09:48.218211652 +0200
> @@ -89,6 +89,8 @@ along with GCC; see the file COPYING3.
>  #include "ipa-fnsummary.h"
>  #include "wide-int-bitmask.h"
>  #include "tree-vector-builder.h"
> +#include "debug.h"
> +#include "dwarf2out.h"
>
>  /* This file should be included last.  */
>  #include "target-def.h"
> @@ -10464,6 +10466,23 @@ output_indirect_thunk (unsigned int regn
>
>ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, indirectlabel2);
>
> +  /* The above call insn pushed a word to stack.  Adjust CFI info.  */
> +  if (flag_asynchronous_unwind_tables && dwarf2out_do_frame ())
> +{
> +  if (! dwarf2out_do_cfi_asm ())
> +   {
> + dw_cfi_ref xcfi = ggc_cleared_alloc ();
> + xcfi->dw_cfi_opc = DW_CFA_advance_loc4;
> + xcfi->dw_cfi_oprnd1.dw_cfi_addr = ggc_strdup (indirectlabel2);
> + vec_safe_push (cfun->fde->dw_fde_cfi, xcfi);
> +   }
> +  dw_cfi_ref xcfi = ggc_cleared_alloc ();
> +  xcfi->dw_cfi_opc = DW_CFA_def_cfa_offset;
> +  xcfi->dw_cfi_oprnd1.dw_cfi_offset = 2 * UNITS_PER_WORD;
> +  vec_safe_push (cfun->fde->dw_fde_cfi, xcfi);
> +  dwarf2out_emit_cfi (xcfi);
> +}
> +
>if (regno != INVALID_REGNUM)
>  {
>/* MOV.  */
>
> Jakub

Re: [PATCH][GCC][AArch64] Add support for SVE stack clash probing [patch (2/7)]

[Tamar Christina]

Hi Richard,

I've added a new loop that should also exit early as described in my previous 
email.

An example sequence is:

.cfi_startproc
mov x15, sp
cntbx16, all, mul #11
add x16, x16, 304
.cfi_def_cfa_register 15
cmp x16, 61440
b.lt.SVLPEND0
.SVLPSPL0:
sub sp, sp, 61440
str xzr, [sp, 0]
subsx16, x16, 61440
b.hs.SVLPSPL0
add x16, x16, 61440
.SVLPEND0:
sub sp, sp, x16
.cfi_escape 0xf,0xc,0x8f,0,0x92,0x2e,0,0x8,0x58,0x1e,0x23,0xb0,0x2,0x22

for a 64KB guard size, and for a 4KB guard size

.cfi_startproc
mov x15, sp
cntbx16, all, mul #11
add x16, x16, 304
.cfi_def_cfa_register 15
cmp x16, 3072
b.lt.SVLPEND0
.SVLPSPL0:
sub sp, sp, 3072
str xzr, [sp, 0]
subsx16, x16, 3072
b.hs.SVLPSPL0
add x16, x16, 3072
.SVLPEND0:
sub sp, sp, x16
.cfi_escape 0xf,0xc,0x8f,0,0x92,0x2e,0,0x8,0x58,0x1e,0x23,0xb0,0x2,0x22


This has about the same semantics as alloca, except we prioritize the common 
case
where no probe is required.  We also change the amount we adjust the stack and
the probing interval to be the nearest value to `guard size - abi buffer` that
fits in the 12-bit shifted immediate used by cmp.

While this would mean we probe a bit more often than we require, in practice the
amount of SVE vectors you'd need to spill is significant. Even more so to enter 
the
loop more than once.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues in sve testsuite.
Target was tested with stack clash on and off by default.

Ok for trunk?

Thanks,
Tamar

gcc/
2018-09-26  Tamar Christina  

PR target/86486
* config/aarch64/aarch64-protos.h 
(aarch64_output_probe_sve_stack_clash): New.
* config/aarch64/aarch64.c (aarch64_output_probe_sve_stack_clash,
aarch64_uimm12_nearest_value): New.
(aarch64_allocate_and_probe_stack_space): Add SVE specific section.
* config/aarch64/aarch64.md (probe_sve_stack_clash): New.

gcc/testsuite/
2018-09-26  Tamar Christina  

PR target/86486
* gcc.target/aarch64/stack-check-prologue-16.c: New test
* gcc.target/aarch64/stack-check-cfa-3.c: New test.


The 09/20/2018 10:23, Tamar Christina wrote:
> Hi Richard,
> 
> The 09/11/2018 16:20, Richard Sandiford wrote:
> > Tamar Christina  writes:
> > >> > +
> > >> > +  /* No probe leave.  */
> > >> > +  ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, loop_end_lab);
> > >> > +  return "";
> > >> 
> > >> With the CFA stuff and constant load, I think this works out as:
> > >> 
> > >> -
> > >> # 12 insns
> > >>  mov r15, base
> > >>  mov adjustment, N
> > >> 1:
> > >>  cmp adjustment, guard_size
> > >>  b.lt2f
> > >>  sub base, base, guard_size
> > >>  str xzr, [base, limit]
> > >>  sub adjustment, adjustment, guard_size
> > >>  b   1b
> > >> 2:
> > >>  sub base, base, adjustment
> > >>  cmp adjustment, limit
> > >>  b.le3f
> > >>  str xzr, [base, limit]
> > >> 3:
> > >> -
> > >> 
> > >> What do you think about something like:
> > >> 
> > >> -
> > >> # 10 insns
> > >>  mov adjustment, N
> > >>  sub r15, base, adjustment
> > >>  subsadjustment, adjustment, min_probe_threshold
> > >>  b.lo2f
> > >> 1:
> > >>  add base, x15, adjustment
> > >>  str xzr, [base, 0]
> > >>  subsadjustment, adjustment, 16
> > >>  and adjustment, adjustment, ~(guard_size-1)
> > >>  b.hs1b
> > >> 2:
> > >>  mov base, r15
> > >> -
> > >> 
> > >> or (with different trade-offs):
> > >> 
> > >> -
> > >> # 11 insns
> > >>  mov adjustment, N
> > >>  sub r15, base, adjustment
> > >>  subsadjustment, adjustment, min_probe_threshold
> > >>  b.lo2f
> > >>  # Might be 0, leading to a double probe
> > >>  and r14, adjustment, guard_size-1
> > >> 1:
> > >>  add base, x15, adjustment
> > >>  str xzr, [base, 0]
> > >>  subsadjustment, adjustment, r14
> > >>  mov r14, guard_size
> > >>  b.hs1b
> > >> 2:
> > >>  mov base, r15
> > >> -
> > >> 
> > >> or (longer, but with a simpler loop):
> > >> 
> > >> -
> > >> # 12 insns
> > >>  mov adjustment, N
> > >>  sub r15, base, adjustment
> > >>  subsadjustment, adjustment, min_probe_threshold
> > >>  b.lo2f
> > >>  str xzr, [base, -16]!
> > >>  sub adjustment, adjustment, 32
> > >>  and adjustment, adjustment, -(guard_size-1)
> > >> 1:
> > >>  add base, x15, adjustment
> > >>  str xzr, [base, 0]
> > >>  subsadjustment

[SVE ACLE] Allow overloaded @ md patterns + small fixes

[Richard Sandiford]

The first patch allows "@foo_" patterns to have the same "@foo_"
even if they have a different number of operands.  There are no current
users of this on trunk, so I won't commit it there until we've had a bit
more time to try it out.

Tested on aarch64-linux-gnu and applied to aarch64/sve-acle-branch.

Richard


[SVE ACLE] Support multiple operand counts for .md @ patterns

This patch extends the support for "@..." pattern names so that
the patterns can have different numbers of operands.  This allows
things like binary and ternary operations to be handled in a
consistent way, a bit like optabs.  The generators assert that
the number of operands passed is correct for the underlying
instruction.

2018-09-25  Richard Sandiford  

gcc/
	* doc/md.texi: Document that @ patterns can have different
	numbers of operands.
	* genemit.c (handle_overloaded_gen): Handle this case.
	* genopinit.c (handle_overloaded_gen): Likewise.


diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 0558cdfc8e6..8fbfe2f9afd 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -10980,4 +10980,13 @@ name and same types of iterator.  For example:
 would produce a single set of functions that handles both
 @code{INTEGER_MODES} and @code{FLOAT_MODES}.
 
+It is also possible for these @samp{@@} patterns to have different
+numbers of operands from each other.  For example, patterns with
+a binary rtl code might take three operands (one output and two inputs)
+while patterns with a ternary rtl code might take four operands (one
+output and three inputs).  This combination would produce separate
+@samp{maybe_gen_@var{name}} and @samp{gen_@var{name}} functions for
+each operand count, but it would still produce a single
+@samp{maybe_code_for_@var{name}} and a single @samp{code_for_@var{name}}.
+
 @end ifset
diff --git a/gcc/genemit.c b/gcc/genemit.c
index 675cf04ff4b..fc662c8ca27 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -800,42 +800,45 @@ handle_overloaded_code_for (overloaded_name *oname)
 static void
 handle_overloaded_gen (overloaded_name *oname)
 {
+  unsigned HOST_WIDE_INT seen = 0;
   /* All patterns must have the same number of operands.  */
-  pattern_stats stats;
-  get_pattern_stats (&stats, XVEC (oname->first_instance->insn, 1));
   for (overloaded_instance *instance = oname->first_instance->next;
instance; instance = instance->next)
 {
-  pattern_stats stats2;
-  get_pattern_stats (&stats2, XVEC (instance->insn, 1));
-  if (stats.num_generator_args != stats2.num_generator_args)
-	fatal_at (get_file_location (instance->insn),
-		  "inconsistent number of operands for '%s'; "
-		  "this instance has %d, but previous instances had %d",
-		  oname->name, stats2.num_generator_args,
-		  stats.num_generator_args);
+  pattern_stats stats;
+  get_pattern_stats (&stats, XVEC (instance->insn, 1));
+  unsigned HOST_WIDE_INT mask
+	= HOST_WIDE_INT_1U << stats.num_generator_args;
+  if (seen & mask)
+	continue;
+
+  seen |= mask;
+
+  /* Print the function prototype.  */
+  printf ("\nrtx\nmaybe_gen_%s (", oname->name);
+  print_overload_arguments (oname);
+  for (int i = 0; i < stats.num_generator_args; ++i)
+	printf (", rtx x%d", i);
+  printf (")\n{\n");
+
+  /* Use maybe_code_for_*, instead of duplicating the selection
+	 logic here.  */
+  printf ("  insn_code code = maybe_code_for_%s (", oname->name);
+  for (unsigned int i = 0; i < oname->arg_types.length (); ++i)
+	printf ("%sarg%d", i == 0 ? "" : ", ", i);
+  printf (");\n"
+	  "  if (code != CODE_FOR_nothing)\n"
+	  "{\n"
+	  "  gcc_assert (insn_data[code].n_generator_args == %d);\n"
+	  "  return GEN_FCN (code) (", stats.num_generator_args);
+  for (int i = 0; i < stats.num_generator_args; ++i)
+	printf ("%sx%d", i == 0 ? "" : ", ", i);
+  printf (");\n"
+	  "}\n"
+	  "  else\n"
+	  "return NULL_RTX;\n"
+	  "}\n");
 }
-
-  /* Print the function prototype.  */
-  printf ("\nrtx\nmaybe_gen_%s (", oname->name);
-  print_overload_arguments (oname);
-  for (int i = 0; i < stats.num_generator_args; ++i)
-printf (", rtx x%d", i);
-  printf (")\n{\n");
-
-  /* Use maybe_code_for_*, instead of duplicating the selection logic here.  */
-  printf ("  insn_code code = maybe_code_for_%s (", oname->name);
-  for (unsigned int i = 0; i < oname->arg_types.length (); ++i)
-printf ("%sarg%d", i == 0 ? "" : ", ", i);
-  printf (");\n"
-	  "  if (code != CODE_FOR_nothing)\n"
-	  "return GEN_FCN (code) (");
-  for (int i = 0; i < stats.num_generator_args; ++i)
-printf ("%sx%d", i == 0 ? "" : ", ", i);
-  printf (");\n"
-	  "  else\n"
-	  "return NULL_RTX;\n"
-	  "}\n");
 }
 
 int
diff --git a/gcc/genopinit.c b/gcc/genopinit.c
index 79835cc5d88..5f11125da85 100644
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -134,31 +134,43 @@ handle_overloaded_code_for (FILE *file, overloaded_name *oname)
 static void
 handle_overloaded_gen (FILE *

Re: libgo patch committed: Update to 1.11 release

[Rainer Orth]

Hi Ian,

> On Tue, Sep 25, 2018 at 6:26 AM, Rainer Orth
>  wrote:
>>
>>> /vol/gcc/src/hg/trunk/local/libgo/go/runtime/traceback_gccgo.go:151:14: 
>>> error: reference to undefined name 'nanotime'
>>> 151 |   waitfor = (nanotime() - gp.waitsince) / 60e9
>>> |  ^
>>>
>>>   and many many more instances.  Again, I found an implementation in
>>>   upstream src/runtime/os3_solaris.go, but that isn't usable since it
>>>   uses syscall directly.
>>>
>>> So I'm currently stuck on the missing nanotime.
>>
>> I think I found it: if I enable building go/runtime/stubs3.go by
>> removing the !solaris build tag, the runtime_nanotime implementation in
>> runtime/go-nanotime.c is used.  At least the builds are going along
>> fine, it seems.  No test results yet.
>
> Thanks for the report.  I've committed this patch to fix the build on
> Solaris.  There are some test failures, though.  I haven't looked into
> them.

great, thanks for fixing this.  I do know about the failures; they are
(slowly) being looked at...

Rainer

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

[PATCH 8/8][GCC][AArch64] stack-clash: Add LR assert to layout_frame.

[Tamar Christina]

Hi All,

Since stack clash depends on the LR being saved for non-leaf functions this
patch adds an assert such that if this changes we would notice this.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

This patch has been pre-approved by AArch64 maintainer here
  https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00555.html
and will be committed with the rest.

Thanks,
Tamar

gcc/ChangeLog:

2018-09-26  Tamar Christina  

* config/aarch64/aarch64.c (aarch64_layout_frame): Add assert.

-- 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 283f3372798c84ef74356128acf2a5be7b4ce1ad..d4b13d48d852a70848fc7c51fd867e776efb5e55 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4050,6 +4050,11 @@ aarch64_layout_frame (void)
   offset = 2 * UNITS_PER_WORD;
 }
 
+  /* With stack-clash, LR must be saved in non-leaf functions.  */
+  gcc_assert (crtl->is_leaf
+	  || (cfun->machine->frame.reg_offset[R30_REGNUM]
+		  != SLOT_NOT_REQUIRED));
+
   /* Now assign stack slots for them.  */
   for (regno = R0_REGNUM; regno <= R30_REGNUM; regno++)
 if (cfun->machine->frame.reg_offset[regno] == SLOT_REQUIRED)

Re: [patch] Fix AArch64 ILP ICE

[Andreas Schwab]

On Sep 25 2018, Andrew Stubbs  wrote:

> Ensure that the address really is the correct mode for an address.
>
> 2018-09-25  Andrew Stubbs  
>
>   gcc/
>   * builtins.c (get_builtin_sync_mem): Force address mode conversion.

This has survived bootstrap so far.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: libgo patch committed: Update to 1.11 release

[Rainer Orth]

Hi Andreas,

> On Sep 24 2018, Ian Lance Taylor  wrote:
>
>> * Makefile.am (mostlyclean-local): Run chmod on check-go-dir to
>> make sure it is writable.
>> (check-go-tools): Likewise.
>
> $ make check-gotools
> make[1]: Entering directory `/usr/local/gcc/gcc-20180926/Build/gotools'
> chmod -R u+w check-go-dir
> chmod: cannot access `check-go-dir': No such file or directory
> make[1]: *** [check-go-tool] Error 1
> make[1]: Leaving directory `/usr/local/gcc/gcc-20180926/Build/gotools'
> make: *** [check-gotools] Error 2

I noticed the same when finding that gotools.{sum,log} hadn't been
generated.

The following patch fixed it for me.

Rainer

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


2018-09-26  Rainer Orth  

* Makefile.am (mostlyclean-local): Ignore chmod errors.
(check-go-tool): Likewise.
* Makefile.in: Regenerate.

# HG changeset patch
# Parent  b78b7013b47fb4c190e7b1e2a21aaa7e2dee7674
Ignore chmod errors during mostlyclean-local, check-go-tool

diff --git a/gotools/Makefile.am b/gotools/Makefile.am
--- a/gotools/Makefile.am
+++ b/gotools/Makefile.am
@@ -123,7 +123,7 @@ MOSTLYCLEANFILES = \
 	*.sent
 
 mostlyclean-local:
-	chmod -R u+w check-go-dir
+	-chmod -R u+w check-go-dir
 	rm -rf check-go-dir check-runtime-dir cgo-test-dir carchive-test-dir
 
 if NATIVE
@@ -229,7 +229,7 @@ ECHO_ENV = PATH=`echo $(abs_builddir):$$
 
 # check-go-tool runs `go test cmd/go` in our environment.
 check-go-tool: go$(EXEEXT) $(noinst_PROGRAMS) check-head check-gccgo check-gcc
-	chmod -R u+w check-go-dir
+	-chmod -R u+w check-go-dir
 	rm -rf check-go-dir cmd_go-testlog
 	$(MKDIR_P) check-go-dir/src/cmd/go
 	cp $(cmdsrcdir)/go/*.go check-go-dir/src/cmd/go/

Re: [PATCH 2/4] Remove unused functions and fields.

[Martin Liška]

On 9/25/18 3:34 PM, Jeff Law wrote:
> On 9/25/18 6:19 AM, Richard Biener wrote:
>> On Tue, Sep 25, 2018 at 9:09 AM Martin Liška  wrote:
>>>
>>> On 9/24/18 4:42 PM, Jeff Law wrote:
 On 9/22/18 1:08 PM, marxin wrote:
>
> gcc/ChangeLog:
>
> 2018-09-24  Martin Liska  
>
>  * alias.c (set_dest_equal_p): Remove unused function.
>  * config/i386/i386.c (def_builtin_pure2): Likewise.
>  * diagnostic-show-locus.c (class layout): Remove
>  unused field.
>  (layout::layout): Likewise here.
>  * dump-context.h (class temp_dump_context): Likewise.
>  * dwarf2out.c (add_AT_fde_ref): Remove unused function.
>  (add_AT_loclistsptr): Likewise.
>  (add_AT_offset): Likewise.
>  (get_AT_hi_pc): Likewise.
>  (is_comdat_die): Likewise.
>  (type_is_enum): Likewise.
>  (ceiling): Likewise.
>  (add_AT_vms_delta): Likewise.
>  (is_class_die): Likewise.
>  * edit-context.c (class line_event): Remove unused field.
>  * graphite-sese-to-poly.c (tree_int_to_gmp): Remove
>  unused function.
>  * ipa-cp.c (ipa_get_vr_lat): Likewise.
>  * lra-constraints.c (ok_for_index_p_nonstrict): Likewise.
>  (ok_for_base_p_nonstrict): Likewise.
>  * tree-chrec.c (is_not_constant_evolution): Likewise.
>  (chrec_fold_poly_cst): Likewise.
>  * tree-if-conv.c (has_pred_critical_p): Likewise.
>  * tree-ssa-coalesce.c (print_exprs): Likewise.
>  * tree-ssa-pre.c (bitmap_set_contains_expr): Likewise.
>  * tree-ssa-uninit.c (is_and_or_or_p): Likewise.
>  * tree-vrp.c (value_ranges_intersect_p): Likewise.
>  (value_range_nonnegative_p): Likewise.
>
> gcc/cp/ChangeLog:
>
> 2018-09-24  Martin Liska  
>
>  * name-lookup.c (namespace_scope_ht_size): Remove
>  unused function.
>  * parser.c (cp_lexer_next_token_is_not_keyword): Likewise.
>
> gcc/fortran/ChangeLog:
>
> 2018-09-24  Martin Liska  
>
>  * trans.c (remove_suffix): Remove
>  unused function.
>
> gcc/go/ChangeLog:
>
> 2018-09-24  Martin Liska  
>
>  * gofrontend/escape.cc (Gogo::analyze_escape): Remove
>  usage of a parameter.
>  (Gogo::assign_connectivity): Likewise.
>  (class Escape_analysis_tag): Likewise.
>  (Gogo::tag_function): Likewise.
>  * gofrontend/expressions.cc (Call_expression::do_type): Likewise.
>  * gofrontend/gogo.h (class Gogo): Likewise.
>  * gofrontend/types.cc (class Call_multiple_result_type): Likewise.
>  (Type::make_call_multiple_result_type): Likewise.
>  * gofrontend/types.h (class Type): Likewise.
>  * gofrontend/wb.cc (class Check_escape): Likewise.
>  (Gogo::add_write_barriers): Likewise.
 Note that some of these may be used inside conditional code.  For
 example the add_AT_vms_delta is used by ia64-vms.  You could argue that
 ia64-vms is a zombie and should be removed, but the general concern WRT
 conditionally compiled code holds.
>>>
>>> Well, please let me install the patch as is and if somebody will need the
>>> functionality, I would be happy to put it back.
>>
>> You could use contrib/config-list.mk to build all supported target configs.  
>> You
>> shouldn't break builds for valid configs deliberately
> Right.  On a suitably large box it only takes a few hours.   Fire it off
> at the end of the day and you've got results in the morning.
> 
> So how about this, if it passes config-list.mk then it's OK.   Obviously
> it's OK to retain any functions that are flagged as necessary by
> config-list.mk.
> 
> Alternately find + grep is your friend, but I find config-list.mk is better.
> 
> jeff
> 

Ok, next time I would be more careful about testing of other targets.

Thanks,
Martin

Re: [PATCH, AArch64 00/11] LSE atomics out-of-line

[Florian Weimer]

* rth:

> Therefore, I've created small out-of-line helpers that are directly
> linked into every library or executable that requires them.  There
> will be two direct branches, both of which will be well-predicted.

This seems reasonable to me, considering the trade-offs.

If the indirect function call overhead is deemed too large, the only
other feasible option I see from a distribution point of view is to drop
support for the previous architecture version without LSE.

Thanks,
Florian

Re: [PATCH, AArch64 08/11] aarch64: Add out-of-line functions for LSE atomics

[Florian Weimer]

* rth:

> diff --git a/libgcc/config/aarch64/lse.c b/libgcc/config/aarch64/lse.c
> new file mode 100644
> index 000..20f4bde741f
> --- /dev/null
> +++ b/libgcc/config/aarch64/lse.c

> +static void __attribute__((constructor))
> +init_have_atomics(void)
> +{
> +  unsigned long hwcap = getauxval(AT_HWCAP);
> +  __aa64_have_atomics = (hwcap & HWCAP_ATOMICS) != 0;
> +}

Is there an expectation that it is possible to use the atomics in IFUNC
resolvers?  Then this needs an explanation why it is safe to run with
the other kind of atomics until the initialization of
__aa64_have_atomics has happened.

(GNU style requires a space before a parenthesis, at least in a function
call or function declarator.)

Thanks,
Florian

[Ada] Introduce -gnatd_A to set Opt.Disable_ALI_File

[Pierre-Marie de Rodat]

This will allow us to remove the import of flag_compare_debug in
lib-writ.adb in a second stage.

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

2018-09-26  Arnaud Charlet  

gcc/ada/

* gnat1drv.adb (Adjust_Global_Switches): -gnatd_A sets
Opt.Disable_ALI_File.
* debug.adb: Update debug flags documentation.--- gcc/ada/debug.adb
+++ gcc/ada/debug.adb
@@ -172,7 +172,7 @@ package body Debug is
--  d_y
--  d_z
 
-   --  d_A
+   --  d_A  Stop generation of ALI file
--  d_B
--  d_C
--  d_D
@@ -843,6 +843,8 @@ package body Debug is
--   a call to routine Ada.Synchronous_Task_Control.Suspend_Until_True
--   or Ada.Synchronous_Barriers.Wait_For_Release.
 
+   --  d_A  Do not generate ALI files by setting Opt.Disable_ALI_File.
+
--  d_L  Output trace information on elaboration checking. This debug switch
--   causes output to be generated showing each call or instantiation as
--   it is checked, and the progress of the recursive trace through

--- gcc/ada/gnat1drv.adb
+++ gcc/ada/gnat1drv.adb
@@ -161,6 +161,12 @@ procedure Gnat1drv is
  Modify_Tree_For_C := True;
   end if;
 
+  --  -gnatd_A disables generation of ALI files.
+
+  if Debug_Flag_Underscore_AA then
+ Disable_ALI_File := True;
+  end if;
+
   --  Set all flags required when generating C code
 
   if Generate_C_Code then

[Ada] Set Current_Error_Node directly

[Pierre-Marie de Rodat]

This changes gigi to set Current_Error_Node directly, which should
result in a more robust error handling.

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

2018-09-26  Eric Botcazou  

gcc/ada/

* gcc-interface/gigi.h (error_gnat_node): Delete.
* gcc-interface/trans.c (error_gnat_node): Likewise.
(gigi): Replace it with Current_Error_Node.
(gnat_to_gnu): Likewise.
* gcc-interface/utils.c (rest_of_subprog_body_compilation):
Likewise.
* gcc-interface/misc.c (internal_error_function): Do not set it.--- gcc/ada/gcc-interface/gigi.h
+++ gcc/ada/gcc-interface/gigi.h
@@ -214,9 +214,6 @@ extern void destroy_gnat_decl (void);
 /* Highest number in the front-end node table.  */
 extern int max_gnat_nodes;
 
-/* Current node being treated, in case abort called.  */
-extern Node_Id error_gnat_node;
-
 /* True when gigi is being called on an analyzed but unexpanded
tree, and the only purpose of the call is to properly annotate
types with representation information.  */

--- gcc/ada/gcc-interface/misc.c
+++ gcc/ada/gcc-interface/misc.c
@@ -347,7 +347,6 @@ internal_error_function (diagnostic_context *context, const char *msgid,
   sp_loc.Bounds = &temp_loc;
   sp_loc.Array = loc;
 
-  Current_Error_Node = error_gnat_node;
   Compiler_Abort (sp, sp_loc, true);
 }
 

--- gcc/ada/gcc-interface/trans.c
+++ gcc/ada/gcc-interface/trans.c
@@ -86,9 +86,6 @@ struct List_Header *List_Headers_Ptr;
 /* Highest number in the front-end node table.  */
 int max_gnat_nodes;
 
-/* Current node being treated, in case abort called.  */
-Node_Id error_gnat_node;
-
 /* True when gigi is being called on an analyzed but unexpanded
tree, and the only purpose of the call is to properly annotate
types with representation information.  */
@@ -719,7 +716,7 @@ gigi (Node_Id gnat_root,
   destroy_gnat_utils ();
 
   /* We cannot track the location of errors past this point.  */
-  error_gnat_node = Empty;
+  Current_Error_Node = Empty;
 }
 
 /* Return a subprogram decl corresponding to __gnat_rcheck_xx for the given
@@ -5910,7 +5907,7 @@ gnat_to_gnu (Node_Id gnat_node)
   bool sync = false;
 
   /* Save node number for error message and set location information.  */
-  error_gnat_node = gnat_node;
+  Current_Error_Node = gnat_node;
   Sloc_to_locus (Sloc (gnat_node), &input_location);
 
   /* If we are only annotating types and this node is a statement, return

--- gcc/ada/gcc-interface/utils.c
+++ gcc/ada/gcc-interface/utils.c
@@ -3389,7 +3389,7 @@ void
 rest_of_subprog_body_compilation (tree subprog_decl)
 {
   /* We cannot track the location of errors past this point.  */
-  error_gnat_node = Empty;
+  Current_Error_Node = Empty;
 
   /* If we're only annotating types, don't actually compile this function.  */
   if (type_annotate_only)

[Ada] Fix assertion failure on record subtype with -gnatRj

[Pierre-Marie de Rodat]

The JSON output of the -gnatR machinery was choking on record subtypes
and the change fixes this oversight.

The following package must now compile properly with -gnatRj:

package P is

  type Rec (D : Integer) is record
  C : Integer;

  case D is
 when 1 =>
S : String (1 .. 20);
 when 2 =>
B : Boolean;
 when others =>
Ch1 : Character;
F   : Float;
Ch2 : Character;
  end case;

   end record;

   subtype Rec1 is Rec (1);

end P;

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

2018-09-26  Eric Botcazou  

gcc/ada/

* repinfo.adb (List_Record_Layout): Be prepared for JSON output.
(List_Record_Info): Use the flat representation for record
subtypes in the JSON format.--- gcc/ada/repinfo.adb
+++ gcc/ada/repinfo.adb
@@ -1358,7 +1358,8 @@ package body Repinfo is
  Starting_First_Bit : Uint := Uint_0;
  Prefix : String := "")
   is
- Comp : Entity_Id;
+ Comp  : Entity_Id;
+ First : Boolean := True;
 
   begin
  Comp := First_Component_Or_Discriminant (Ent);
@@ -1413,6 +1414,15 @@ package body Repinfo is
   goto Continue;
end if;
 
+   if List_Representation_Info_To_JSON then
+  if First then
+ Write_Eol;
+ First := False;
+  else
+ Write_Line (",");
+  end if;
+   end if;
+
List_Component_Layout (Comp,
  Starting_Position, Starting_First_Bit, Prefix);
 end;
@@ -1678,7 +1688,11 @@ package body Repinfo is
  Write_Line (",");
  Write_Str ("  ""record"": [");
 
- List_Structural_Record_Layout (Ent, Ent);
+ if Is_Base_Type (Ent) then
+List_Structural_Record_Layout (Ent, Ent);
+ else
+List_Record_Layout (Ent);
+ end if;
 
  Write_Eol;
  Write_Str ("  ]");

[Ada] Regression in partial compilation of RCI units

[Pierre-Marie de Rodat]

This patch fixes an issue whereby the compilation of partial sources
(packages without bodies that require them) would not occur when said
sources were remote call interfaces. This is required because such
interfaces may have bodies that only exist on the server side or vice
versa

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

2018-09-26  Justin Squirek  

gcc/ada/

* lib-writ.adb, lib-writ.ads (Write_With_Lines): Add
documentation and an extra conditional check for RCI units so
that generated ali files will list the spec only instead of a
body when a body is not found.--- gcc/ada/lib-writ.adb
+++ gcc/ada/lib-writ.adb
@@ -960,9 +960,14 @@ package body Lib.Writ is
 
   --  In GNATprove mode we must write the spec of a unit which
   --  requires a body if that body is not found. This will
-  --  allow partial analysis on incomplete sources.
-
-  if GNATprove_Mode then
+  --  allow partial analysis on incomplete sources. Also, in
+  --  the case of a unit that is a remote call interface, the
+  --  bodies of packages may not exist but still may form a
+  --  valid program - so we handle that here as well.
+
+  if GNATprove_Mode
+or else Is_Remote_Call_Interface (Cunit_Entity (Unum))
+  then
  Body_Fname :=
Get_File_Name
  (Uname=> Get_Body_Name (Uname),

--- gcc/ada/lib-writ.ads
+++ gcc/ada/lib-writ.ads
@@ -624,18 +624,19 @@ package Lib.Writ is
--Z unit-name [source-name lib-name] [E] [EA] [ED] [AD]
 
--  One W line is present for each unit that is mentioned in an explicit
-   --  non-limited with clause by the current unit. One Y line is present
+   --  nonlimited with clause by the current unit. One Y line is present
--  for each unit that is mentioned in an explicit limited with clause
--  by the current unit. One Z line is present for each unit that is
--  only implicitly withed by the current unit. The first parameter is
--  the unit name in internal format. The second parameter is the file
-   --  name of the body unit on which the current compliation depends -
-   --  except when in GNATprove mode. In GNATprove mode, when packages
-   --  which require a body have no associated source file, the file name
-   --  of the spec is used instead to allow partial analysis of incomplete
-   --  sources. The third parameter is the file name of the library
-   --  information file that contains the results of compiling this unit.
-   --  The optional modifiers are used as follows:
+   --  name of the body unit on which the current compilation unit depends,
+   --  except when in GNATprove mode or when the unit is a remote call
+   --  interface. In these cases, when packages that require a body have
+   --  no associated source file, the file name of the spec is used instead
+   --  to allow partial analysis of incomplete sources. The third parameter
+   --  is the file name of the library information file that contains the
+   --  results of compiling this unit. The optional modifiers are used as
+   --  follows:
 
--E   pragma Elaborate applies to this unit
 

[Ada] Crash on expression functions within quantified expressions

[Pierre-Marie de Rodat]

This patch fixes an issue whereby using a call to an expression function
as the domain of iteration for a loop would trigger a crash due to the
function not being frozen appropriately.

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

2018-09-26  Justin Squirek  

gcc/ada/

* sem_ch5.adb (Analyze_Iterator_Specification): Add conditional
to freeze called functions within iterator specifications during
full analysis.
(Preanalyze_Range): Minor typo fix.

gcc/testsuite/

* gnat.dg/expr_func8.adb: New testcase.--- gcc/ada/sem_ch5.adb
+++ gcc/ada/sem_ch5.adb
@@ -2203,6 +2203,19 @@ package body Sem_Ch5 is
 
   Preanalyze_Range (Iter_Name);
 
+  --  If the domain of iteration is a function call, make sure the function
+  --  itself is frozen. This is an issue if this is a local expression
+  --  function.
+
+  if Nkind (Iter_Name) = N_Function_Call
+and then Is_Entity_Name (Name (Iter_Name))
+and then Full_Analysis
+and then (In_Assertion_Expr = 0
+   or else Assertions_Enabled)
+  then
+ Freeze_Before (N, Entity (Name (Iter_Name)));
+  end if;
+
   --  Set the kind of the loop variable, which is not visible within the
   --  iterator name.
 
@@ -4136,10 +4149,10 @@ package body Sem_Ch5 is
   Full_Analysis := False;
   Expander_Mode_Save_And_Set (False);
 
-  --  In addition to the above we must ecplicity suppress the
-  --  generation of freeze nodes which might otherwise be generated
-  --  during resolution of the range (e.g. if given by an attribute
-  --  that will freeze its prefix).
+  --  In addition to the above we must explicitly suppress the generation
+  --  of freeze nodes that might otherwise be generated during resolution
+  --  of the range (e.g. if given by an attribute that will freeze its
+  --  prefix).
 
   Set_Must_Not_Freeze (R_Copy);
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/expr_func8.adb
@@ -0,0 +1,37 @@
+--  { dg-do compile }
+--  { dg-options "-gnata" }
+
+procedure Expr_Func8 is
+
+   type Node_Set is array (Positive range <>) of Integer;
+
+   function Nodes return Node_Set is
+ ((1,2,3,4,5,6,7,8,9));
+
+   X1 : Boolean := (for all N of Nodes => N = N);
+
+   function Predecessors (N : Integer) return Node_Set Is
+  (Nodes (1 .. N - 1));
+   function Successors (N : Integer) return Node_Set Is
+  (Nodes (N + 1 .. Nodes'Last));
+
+   pragma Assert
+ (for all N of Nodes =>
+   (for some S of Successors (N) => S = N));
+
+   X2 : Boolean :=
+ (for all N of Nodes =>
+   (for some S of Successors (N) => S = N));
+
+   X3 : Boolean :=
+ (for all N of Nodes =>
+   (for some S of Successors (N) => S = N)) with Ghost;
+
+   pragma Assert
+  (for all N of Nodes =>
+  (for all P of Predecessors (N) =>
+  (for some S of Successors (P) => S = N)));
+
+begin
+   null;
+end;

[Ada] Missing predicate check on return value

[Pierre-Marie de Rodat]

The semantics of the return statement includes an implicit conversion of
the value to the return type of the funcction. This conversion, as
elsewhere, entails a predicate check if the return type has a predicate
aspect.

We do not apply the check to a case expression because in the context of
a return statement it will be expanded into a series of return
statements, each of which will receive a predicate check.

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

2018-09-26  Ed Schonberg  

gcc/ada/

* sem_ch6.adb (Analyze_Function_Return): If the return type has
a dynamic_predicate, apply a Predicate_Check to the expression,
given that it is implicitly converted to the return type.
Exclude case expressions from the check, because in this context
the expression is expanded into individual return statements.

gcc/testsuite/

* gnat.dg/predicate3.adb, gnat.dg/predicate3_pkg.ads: New
testcase.--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -1060,6 +1060,16 @@ package body Sem_Ch6 is
 
  Apply_Constraint_Check (Expr, R_Type);
 
+ --  The return value is converted to the return type of the function,
+ --  which implies a predicate check if the return type is predicated.
+ --  We do not apply the check to a case expression because it will
+ --  be expanded into a series of return statements, each of which
+ --  will receive a predicate check.
+
+ if Nkind (Expr) /= N_Case_Expression then
+Apply_Predicate_Check (Expr, R_Type);
+ end if;
+
  --  Ada 2005 (AI-318-02): When the result type is an anonymous access
  --  type, apply an implicit conversion of the expression to that type
  --  to force appropriate static and run-time accessibility checks.

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate3.adb
@@ -0,0 +1,39 @@
+--  { dg-do run }
+--  { dg-options "-gnata" }
+
+with Ada.Assertions, Ada.Text_IO;
+use  Ada.Assertions, Ada.Text_IO;
+
+with Predicate3_Pkg;
+use  Predicate3_Pkg;
+
+procedure Predicate3 is
+   Got_Assertion : Boolean := False;
+begin
+
+   begin
+  Put_Line (Good (C)'Image);
+   exception
+  when Assertion_Error =>
+ Got_Assertion := True;
+   end;
+
+   if not Got_Assertion then
+  raise Program_Error;
+   end if;
+
+   Got_Assertion := False;
+   declare
+  X: Priv;
+   begin
+  X := Wrong;
+   exception
+  when Assertion_Error =>
+ Got_Assertion := True;
+   end;
+
+   if not Got_Assertion then
+  raise Program_Error;
+   end if;
+
+end Predicate3;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate3_pkg.ads
@@ -0,0 +1,22 @@
+package Predicate3_Pkg is
+ 
+   type Priv is private;
+   C: constant Priv;
+ 
+   function Test (X: Priv) return Boolean;
+   subtype Subt is Priv with Dynamic_Predicate => (Test (Subt));
+ 
+   function Wrong return Subt;
+   function Good (X: Subt) return Boolean;
+ 
+private
+ 
+   type Priv is new Integer;
+   C: constant Priv := -1;
+ 
+   function Test (X: Priv) return Boolean is (X > 0);
+ 
+   function Wrong return Subt is (-1);
+   function Good (X: Subt) return Boolean is (True);
+ 
+end Predicate3_Pkg;

Re: [PATCH] PR86957

[Martin Liška]

On 9/24/18 9:21 PM, Indu Bhagat wrote:
> Done. Attached is updated patch.

Thanks for it, I tested that right now.
You have ACK, so please install the patch. Please do not forget
to install ChangeLog entry and I would include PR entry:
https://www.gnu.org/software/gcc/contribute.html

example can be seen here:
https://github.com/gcc-mirror/gcc/blob/089d1a5f4939282881c108e0d04d026bdd82f6c6/ChangeLog#L178

Martin

> 
> Patch is tested on x86_64
> 
> Thanks
> 
> 
> On 09/24/2018 09:37 AM, Martin Sebor wrote:
>> I would suggest to use the term "remove" or "delete" instead of
>> the informal "wipe out" when referring to removing files or their
>> contents.
>>
>> Martin 
> 

[Ada] Inlining of renamed subprogram instances in package body

[Pierre-Marie de Rodat]

This fixes a small discrepancy in the handling of renamed subprograms
declared in a package body, between those originally a regular
subprogram and those an instance of a generic subprogram, the latter
being slightly hindered.

The difference comes from the setting of the Is_Public flag, which was
more conservative in the latter case because instantiations of generic
subprograms are done in compiler-generated local packages.

It is eliminated by allowing Has_Referencer to recurse into nested
packages, but only if they are themselves not instances of generic
packages.

The compiler must now fully inline Doit_I into Doit at -O2 in:

package P is

  generic procedure Doit_G;

  procedure Doit;
end P;

package body P is

  N : Natural := 0;

  procedure Doit_G is
  begin
N := 1;
  end Doit_G;

  procedure Doit_I is new Doit_G;

  procedure Doit renames Doit_I;

end P;

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

2018-09-26  Eric Botcazou  

gcc/ada/

* sem_ch7.adb (Has_Referencer): Remove Top_Level parameter and
add In_Nested_Instance and
Has_Outer_Referencer_Of_Non_Subprograms parameters.  Rename
Has_Non_Subprograms_Referencer variable into
Has_Referencer_Of_Non_Subprograms and initialize it with the new
third parameter.  Adjust recursive calls and to the renaming.
Replace test on Top_Level with test on In_Nested_Instance to
decide whether to clear the Is_Public flag on entities.
(Hide_Public_Entities): Adjust call to Has_Referencer.--- gcc/ada/sem_ch7.adb
+++ gcc/ada/sem_ch7.adb
@@ -259,11 +259,12 @@ package body Sem_Ch7 is
 
   procedure Hide_Public_Entities (Decls : List_Id) is
  function Has_Referencer
-   (Decls : List_Id;
-Top_Level : Boolean := False) return Boolean;
+   (Decls   : List_Id;
+In_Nested_Instance  : Boolean;
+Has_Outer_Referencer_Of_Non_Subprograms : Boolean) return Boolean;
  --  A "referencer" is a construct which may reference a previous
  --  declaration. Examine all declarations in list Decls in reverse
- --  and determine whether once such referencer exists. All entities
+ --  and determine whether one such referencer exists. All entities
  --  in the range Last (Decls) .. Referencer are hidden from external
  --  visibility.
 
@@ -286,14 +287,16 @@ package body Sem_Ch7 is
  
 
  function Has_Referencer
-   (Decls : List_Id;
-Top_Level : Boolean := False) return Boolean
+   (Decls   : List_Id;
+In_Nested_Instance  : Boolean;
+Has_Outer_Referencer_Of_Non_Subprograms : Boolean) return Boolean
  is
 Decl: Node_Id;
 Decl_Id : Entity_Id;
 Spec: Node_Id;
 
-Has_Non_Subprograms_Referencer : Boolean := False;
+Has_Referencer_Of_Non_Subprograms : Boolean :=
+   Has_Outer_Referencer_Of_Non_Subprograms;
 --  Set if an inlined subprogram body was detected as a referencer.
 --  In this case, we do not return True immediately but keep hiding
 --  subprograms from external visibility.
@@ -319,13 +322,23 @@ package body Sem_Ch7 is
 
elsif Nkind (Decl) = N_Package_Declaration then
   Spec := Specification (Decl);
+  Decl_Id := Defining_Entity (Spec);
 
   --  Inspect the declarations of a non-generic package to try
   --  and hide more entities from external visibility.
 
-  if not Is_Generic_Unit (Defining_Entity (Spec)) then
- if Has_Referencer (Private_Declarations (Spec))
-   or else Has_Referencer (Visible_Declarations (Spec))
+  if not Is_Generic_Unit (Decl_Id) then
+ if Has_Referencer (Private_Declarations (Spec),
+In_Nested_Instance
+  or else
+Is_Generic_Instance (Decl_Id),
+Has_Referencer_Of_Non_Subprograms)
+   or else
+Has_Referencer (Visible_Declarations (Spec),
+In_Nested_Instance
+  or else
+Is_Generic_Instance (Decl_Id),
+Has_Referencer_Of_Non_Subprograms)
  then
 return True;
  end if;
@@ -354,7 +367,12 @@ package body Sem_Ch7 is
   --  Inspect the declarations of a non-generic package body to
   --  try and hide more entities fro

[Ada] Preparation for new description of interface thunks

[Pierre-Marie de Rodat]

This adjusts and exposes a couple of functions of the front-end used for the
generation of interface thunks so as to make them callable from gigi.  This
also propagates the debug info setting from the targets to the thunks so as
to make stepping into primitives work better in the debugger.

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

2018-09-26  Eric Botcazou  

gcc/ada/

* exp_disp.adb (Expand_Interface_Conversion): Use Present test.
(Expand_Interface_Thunk): Propagate debug info setting from
target.
* exp_util.ads (Find_Interface_Tag): Adjust comment.
* exp_util.adb (Find_Interface_Tag): Remove assertions of
success.
* sem_util.adb (Is_Variable_Size_Record): Only look at
components and robustify the implementation.
* fe.h (Find_Interface_Tag): Declare.
(Is_Variable_Size_Record): Likewise.--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -1454,7 +1454,7 @@ package body Exp_Disp is
   end if;
 
   Iface_Tag := Find_Interface_Tag (Operand_Typ, Iface_Typ);
-  pragma Assert (Iface_Tag /= Empty);
+  pragma Assert (Present (Iface_Tag));
 
   --  Keep separate access types to interfaces because one internal
   --  function is used to handle the null value (see following comments)
@@ -2046,6 +2046,7 @@ package body Exp_Disp is
   Set_Ekind (Thunk_Id, Ekind (Prim));
   Set_Is_Thunk (Thunk_Id);
   Set_Convention (Thunk_Id, Convention (Prim));
+  Set_Needs_Debug_Info (Thunk_Id, Needs_Debug_Info (Target));
   Set_Thunk_Entity (Thunk_Id, Target);
 
   --  Procedure case

--- gcc/ada/exp_util.adb
+++ gcc/ada/exp_util.adb
@@ -5529,7 +5529,6 @@ package body Exp_Util is
  then
 --  Skip the tag associated with the primary table
 
-pragma Assert (Etype (First_Tag_Component (Typ)) = RTE (RE_Tag));
 AI_Tag := Next_Tag_Component (First_Tag_Component (Typ));
 pragma Assert (Present (AI_Tag));
 
@@ -5590,14 +5589,12 @@ package body Exp_Util is
   --  primary dispatch table.
 
   if Is_Ancestor (Iface, Typ, Use_Full_View => True) then
- pragma Assert (Etype (First_Tag_Component (Typ)) = RTE (RE_Tag));
  return First_Tag_Component (Typ);
 
   --  Otherwise we need to search for its associated tag component
 
   else
  Find_Tag (Typ);
- pragma Assert (Found);
  return AI_Tag;
   end if;
end Find_Interface_Tag;

--- gcc/ada/exp_util.ads
+++ gcc/ada/exp_util.ads
@@ -585,8 +585,9 @@ package Exp_Util is
function Find_Interface_Tag
  (T : Entity_Id;
   Iface : Entity_Id) return Entity_Id;
-   --  Ada 2005 (AI-251): Given a type T implementing the interface Iface,
-   --  return the record component containing the tag of Iface.
+   --  Ada 2005 (AI-251): Given a type T and an interface Iface, return the
+   --  record component containing the tag of Iface if T implements Iface or
+   --  Empty if it does not.
 
function Find_Prim_Op (T : Entity_Id; Name : Name_Id) return Entity_Id;
--  Find the first primitive operation of a tagged type T with name Name.

--- gcc/ada/fe.h
+++ gcc/ada/fe.h
@@ -159,8 +159,10 @@ extern void Get_External_Name	(Entity_Id, Boolean, String_Pointer);
 /* exp_util: */
 
 #define Is_Fully_Repped_Tagged_Type exp_util__is_fully_repped_tagged_type
+#define Find_Interface_Tag exp_util__find_interface_tag
 
 extern Boolean Is_Fully_Repped_Tagged_Type  (Entity_Id);
+extern Entity_Id Find_Interface_Tag		(Entity_Id, Entity_Id);
 
 /* lib: */
 
@@ -269,12 +271,14 @@ extern Boolean Is_OK_Static_Subtype	(Entity_Id);
 #define Defining_Entity			sem_util__defining_entity
 #define First_Actual			sem_util__first_actual
 #define Next_Actual			sem_util__next_actual
+#define Is_Variable_Size_Record 	sem_util__is_variable_size_record
 #define Requires_Transient_Scope	sem_util__requires_transient_scope
 
 extern Entity_Id Defining_Entity	(Node_Id);
 extern Node_Id First_Actual		(Node_Id);
 extern Node_Id Next_Actual		(Node_Id);
-extern Boolean Requires_Transient_Scope (Entity_Id);
+extern Boolean Is_Variable_Size_Record 	(Entity_Id Id);
+extern Boolean Requires_Transient_Scope	(Entity_Id);
 
 /* sinfo: */
 

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -17714,9 +17714,9 @@ package body Sem_Util is
begin
   pragma Assert (Is_Record_Type (E));
 
-  Comp := First_Entity (E);
+  Comp := First_Component (E);
   while Present (Comp) loop
- Comp_Typ := Etype (Comp);
+ Comp_Typ := Underlying_Type (Etype (Comp));
 
  --  Recursive call if the record type has discriminants
 
@@ -17732,7 +17732,7 @@ package body Sem_Util is
 return True;
  end if;
 
- Next_Entity (Comp);
+ Next_Component (Comp);
   end loop;
 
   return False;

[Ada] Missing error on non-limited derived type with limited component

[Pierre-Marie de Rodat]

This patch fixes a missing error on a type extension with limited
components, when the parent type is a derived limited interface. This
may allow the unit to improperly compile, but may lead to bind-time
errors when compiling a client of that unit.

Compiling p.adb must yield:

 keys.ads:8:06: extension of nonlimited type cannot have limited components
 keys.ads:8:06: limitedness is not inherited from limited interface
 keys.ads:8:06: add "limited" to type indication


with Keys;
procedure P is
begin
  null;
end;

with GNAT.Semaphores;
package Keys is

  type Ref0 is limited interface;
  type Ref2 is limited interface and Ref0;

  type Object is new Ref2 with record
 Lock : aliased GNAT.Semaphores.Binary_Semaphore
   (True, GNAT.Semaphores.Default_Ceiling);
  end record;

end;

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

2018-09-26  Ed Schonberg  

gcc/ada/

* sem_ch3.adb (Is_Onown_Limited): A derived type whose parent P
is a derived limited record is not itself limited if P is a
derived limited interface.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -1928,9 +1928,12 @@ package body Sem_Ch3 is
 return True;
 
  --  Else the type may have a limited interface progenitor, but a
- --  limited record parent.
+ --  limited record parent that is not an interface.
 
- elsif R /= P and then Is_Limited_Record (P) then
+ elsif R /= P
+and then Is_Limited_Record (P)
+and then not Is_Interface (P)
+ then
 return True;
 
  else

[Ada] New unit GNAT.Sets

[Pierre-Marie de Rodat]

This patch implements unit GNAT.Sets which currently offers a general purpose
membership set. The patch also streamlines GNAT.Dynamic_HTables and GNAT.Lists
to use parts of the same API, types, and exceptions as those used by GNAT.Sets.

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

2018-09-26  Hristian Kirtchev  

gcc/ada/

* gcc-interface/Make-lang.in: Add unit GNAT.Sets to the list of
front end sources.
* impunit.adb: Add unit GNAT.Sets to the list of predefined
units.
* Makefile.rtl: Add unit GNAT.Sets to the list of non-tasking
units.
* libgnat/g-sets.adb: New unit.
* libgnat/g-sets.ads: New unit.
* libgnat/g-dynhta.adb (Minimum_Size): Decrease to 8 in order to
allow for small sets.  Update all occurrences of Table_Locked to
Iterated.
(Ensure_Unlocked): Query the number of iterators.
(Find_Node): Use the supplied equality.
(Is_Empty): New routine.
(Lock): Update the number of iterators.
(Prepend_Or_Replace): Use the supplied equality.
(Size): Update the return type.
(Unlock): Update the number of iterators.
* libgnat/g-dynhta.ads: Update all occurrences of Table_Locked
to Iterated.  Rename formal subprogram Equivalent_Keys to "=".
(Bucket_Range_Type, Pair_Count_Type): Remove types.
(Not_Created, Table_Locked, Iterator_Exhausted): Remove
exceptions.
(Hash_Table): Update to store the number of iterators rather
than locks.
(Is_Empty): New routine.
(Size): Update the return type.
* libgnat/g-lists.adb: Update all occurrences of List_Locked to
Iterated.
(Ensure_Unlocked): Query the number of iterators.
(Length): Remove.
(Lock): Update the number of iterators.
(Size): New routine.
(Unlock): Update the number of iterators.
* libgnat/g-lists.ads: Update all occurrences of List_Locked to
Iterated.
(Element_Count_Type): Remove type.
(Not_Created, Table_Locked, Iterator_Exhausted): Remove
exceptions.
(Linked_List): Update type to store the number of iterators
rather than locks.
(Length): Remove.
(Size): New routine.
* libgnat/gnat.ads (Bucket_Range_Type): New type.
(Iterated, Iterator_Exhausted, and Not_Created): New exceptions.

gcc/testsuite/

* gnat.dg/sets1.adb: New testcase.
* gnat.dg/dynhash.adb, gnat.dg/linkedlist.adb: Update testcases
to new API.--- gcc/ada/Makefile.rtl
+++ gcc/ada/Makefile.rtl
@@ -445,6 +445,7 @@ GNATRTL_NONTASKING_OBJS= \
   g-sehash$(objext) \
   g-sercom$(objext) \
   g-sestin$(objext) \
+  g-sets$(objext) \
   g-sha1$(objext) \
   g-sha224$(objext) \
   g-sha256$(objext) \

--- gcc/ada/gcc-interface/Make-lang.in
+++ gcc/ada/gcc-interface/Make-lang.in
@@ -320,6 +320,7 @@ GNAT_ADA_OBJS =	\
  ada/libgnat/g-hesora.o	\
  ada/libgnat/g-htable.o	\
  ada/libgnat/g-lists.o \
+ ada/libgnat/g-sets.o \
  ada/libgnat/g-spchge.o	\
  ada/libgnat/g-speche.o	\
  ada/libgnat/g-u3spch.o	\

--- gcc/ada/impunit.adb
+++ gcc/ada/impunit.adb
@@ -298,6 +298,7 @@ package body Impunit is
 ("g-semaph", F),  -- GNAT.Semaphores
 ("g-sercom", F),  -- GNAT.Serial_Communications
 ("g-sestin", F),  -- GNAT.Secondary_Stack_Info
+("g-sets  ", F),  -- GNAT.Sets
 ("g-sha1  ", F),  -- GNAT.SHA1
 ("g-sha224", F),  -- GNAT.SHA224
 ("g-sha256", F),  -- GNAT.SHA256

--- gcc/ada/libgnat/g-dynhta.adb
+++ gcc/ada/libgnat/g-dynhta.adb
@@ -369,7 +369,7 @@ package body GNAT.Dynamic_HTables is

 
package body Dynamic_HTable is
-  Minimum_Size : constant Bucket_Range_Type := 32;
+  Minimum_Size : constant Bucket_Range_Type := 8;
   --  Minimum size of the buckets
 
   Safe_Compression_Size : constant Bucket_Range_Type :=
@@ -401,8 +401,8 @@ package body GNAT.Dynamic_HTables is
 
   procedure Ensure_Unlocked (T : Instance);
   pragma Inline (Ensure_Unlocked);
-  --  Verify that hash table T is unlocked. Raise Table_Locked if this is
-  --  not the case.
+  --  Verify that hash table T is unlocked. Raise Iterated if this is not
+  --  the case.
 
   function Find_Bucket
 (Bkts : Bucket_Table_Ptr;
@@ -472,9 +472,10 @@ package body GNAT.Dynamic_HTables is
   -- Create --
   
 
-  function Create (Initial_Size : Bucket_Range_Type) return Instance is
+  function Create (Initial_Size : Positive) return Instance is
  Size : constant Bucket_Range_Type :=
-   Bucket_Range_Type'Max (Initial_Size, Minimum_Size);
+   Bucket_Range_Type'Max
+ (Bucket_Range_Type (Initial_Size), Minimum_Size);
  --  Ensure that the buckets meet a minimum size
 
  T : constant Instance := new Hash_Table;
@@ -661,8 +662,8 @@ package body GNAT.Dynamic_HTab

[Ada] Illegal formal objects associated with anonymous acc-to-subp args

[Pierre-Marie de Rodat]

The compiler was incorrectly accepting generic instantiations with
formal objects of named access-to-subprogram types associated with an
actual of an anonymous access-to-subprogram type.
Analyze_Object_Declaration tests for objects initialized anonymous
access-to-subprogram values, and wraps a conversion around the argument,
which normally will result in error checks during resolution in
Valid_Conversion, but the conversion was only created when the
initialization expression Comes_From_Source, which prevented the
conversion wrapping from happening for constant declarations resulting
from generic expansion. The test for Comes_From_Source was removed.

The following test must report the error output given further below for
the three constructs marked as errors when compiled with this command:

gcc -c -gnatj70 bad_anon_access_instance.adb

procedure Bad_Anon_Access_Instance (Anon_Acc : access procedure) is

   type Ref is access procedure;

   Ref_1 : Ref := Anon_Acc;   -- ERROR (flagged by GNAT)

   Ref_2 : constant Ref := Anon_Acc;  -- ERROR (flagged by GNAT)

   generic
  Formal_Ref : Ref;
   package Gen is
   end Gen;

   package Inst
 is new Gen (Formal_Ref => Anon_Acc); -- ERROR (but not flagged by GNAT)

begin
   null;
end Bad_Anon_Access_Instance;

-
Error output:
-

bad_anon_access_instance.adb:4:19: illegal attempt to store anonymous
   access to subprogram, value has
   deeper accessibility than any
   master (RM 3.10.2 (13)), use named
   access type for "Anon_Acc" instead
   of access parameter
bad_anon_access_instance.adb:6:28: illegal attempt to store anonymous
   access to subprogram, value has
   deeper accessibility than any
   master (RM 3.10.2 (13)), use named
   access type for "Anon_Acc" instead
   of access parameter
bad_anon_access_instance.adb:14:32: illegal attempt to store
anonymous access to subprogram,
value has deeper accessibility
than any master (RM 3.10.2 (13)),
use named access type for
"Anon_Acc" instead of access
parameter

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

2018-09-26  Gary Dismukes  

gcc/ada/

* sem_ch3.adb (Analyze_Object_Declaration): Remove test for
Comes_From_Source, which prevented implicit conversions from
being applied to anonymous access-to-subprogram formals in
constant declartions that arise from instance associations for
generic formal objects.  Add RM and AARM references to comment.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -4286,12 +4286,11 @@ package body Sem_Ch3 is
  else
 
 --  If the expression is a formal that is a "subprogram pointer"
---  this is illegal in accessibility terms. Add an explicit
---  conversion to force the corresponding check, as is done for
---  assignments.
+--  this is illegal in accessibility terms (see RM 3.10.2 (13.1/2)
+--  and AARM 3.10.2 (13.b/2)). Add an explicit conversion to force
+--  the corresponding check, as is done for assignments.
 
-if Comes_From_Source (N)
-  and then Is_Entity_Name (E)
+if Is_Entity_Name (E)
   and then Present (Entity (E))
   and then Is_Formal (Entity (E))
   and then

[Ada] Pair miscount in Dynamic_HTable.Put

[Pierre-Marie de Rodat]

This patch corrects the logic of GNAT.Dynamic_HTables.Dynamic_HTable.Put to
update the number of key-value pairs in the hash table only when the put is
adding a new pair, rather than updating the value of an existing pair.

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

2018-09-26  Hristian Kirtchev  

gcc/ada/

* libgnat/g-dynhta.adb (Prepend_Or_Replace): Update the number
of key-value pairs in the hash table only when adding a brand
new pair.

gcc/testsuite/

* gnat.dg/dynhash1.adb: New testcase.--- gcc/ada/libgnat/g-dynhta.adb
+++ gcc/ada/libgnat/g-dynhta.adb
@@ -544,6 +544,9 @@ package body GNAT.Dynamic_HTables is
 Detach (Nod);
 Free   (Nod);
 
+--  The number of key-value pairs is updated when the hash table
+--  contains a valid node which represents the pair.
+
 T.Pairs := T.Pairs - 1;
 
 --  Compress the hash table if the load factor drops below
@@ -1121,6 +1124,11 @@ package body GNAT.Dynamic_HTables is
 Nod := new Node'(Key, Value, null, null);
 
 Prepend (Nod, Head);
+
+--  The number of key-value pairs must be updated for a prepend,
+--  never for a replace.
+
+T.Pairs := T.Pairs + 1;
  end Prepend_Or_Replace;
 
  --  Local variables
@@ -1148,8 +1156,6 @@ package body GNAT.Dynamic_HTables is
 
  Prepend_Or_Replace (Head);
 
- T.Pairs := T.Pairs + 1;
-
  --  Expand the hash table if the ratio of pairs to buckets goes over
  --  Expansion_Threshold.
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/dynhash1.adb
@@ -0,0 +1,53 @@
+with Ada.Text_IO;  use Ada.Text_IO;
+with GNAT; use GNAT;
+with GNAT.Dynamic_HTables; use GNAT.Dynamic_HTables;
+
+procedure Dynhash1 is
+   function Hash (Key : Integer) return Bucket_Range_Type is
+   begin
+  return Bucket_Range_Type (Key);
+   end Hash;
+
+   package Integer_Hash_Tables is new Dynamic_HTable
+ (Key_Type  => Integer,
+  Value_Type=> Integer,
+  No_Value  => 0,
+  Expansion_Threshold   => 1.3,
+  Expansion_Factor  => 2,
+  Compression_Threshold => 0.3,
+  Compression_Factor=> 2,
+  "="   => "=",
+  Hash  => Hash);
+   use Integer_Hash_Tables;
+
+   Siz : Natural;
+   T   : Instance;
+
+begin
+   T := Create (8);
+
+   Put (T, 1, 1);
+   Put (T, 1, 2);
+   Put (T, 1, 3);
+
+   Siz := Size (T);
+
+   if Siz /= 1 then
+  Put_Line ("ERROR: Put: wrong size");
+  Put_Line ("expected: 1");
+  Put_Line ("got :" & Siz'Img);
+   end if;
+
+   Delete (T, 1);
+   Delete (T, 1);
+
+   Siz := Size (T);
+
+   if Siz /= 0 then
+  Put_Line ("ERROR: Delete: wrong size");
+  Put_Line ("expected: 0");
+  Put_Line ("got :" & Siz'Img);
+   end if;
+
+   Destroy (T);
+end Dynhash1;

[Ada] Mimic the C++ ABI when passing class-wide conversion actuals

[Pierre-Marie de Rodat]

This patch does not affect the behavior of Ada-only code but improves
consistency with the code generated by the C++ compiler.

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

2018-09-26  Javier Miranda  

gcc/ada/

* sem_res.adb (Resolve_Actuals): If the formal is a class-wide
type conversion then do not skip resolving and expanding the
conversion; required to displace the pointer to the object and
reference the dispatch table associated with the target
interface type.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -3809,6 +3809,7 @@ package body Sem_Res is
 if Ekind (F) /= E_In_Parameter
   and then Nkind (A) = N_Type_Conversion
   and then not Is_Class_Wide_Type (Etype (Expression (A)))
+  and then not Is_Interface (Etype (A))
 then
if Ekind (F) = E_In_Out_Parameter
  and then Is_Array_Type (Etype (F))

[Ada] Spurious error on private extension with predicate

[Pierre-Marie de Rodat]

This patch fixes a spurious error involving a private extension whose
full view includes a dynamic predicate, when the parent type is itself
private at the point of the predicate check.  The conversion is known to
be legal so no extra conversion checks are required.

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

2018-09-26  Ed Schonberg  

gcc/ada/

* exp_util.adb (Make_Predicate_Call): Use OK_Convert_To when
applying a predicate check to prevent spurious errors when
private ancestors are involved.

gcc/testsuite/

* gnat.dg/predicate2-containers.ads,
gnat.dg/predicate2-project-name_values.ads,
gnat.dg/predicate2-project-registry-attribute.ads,
gnat.dg/predicate2-project-registry.ads,
gnat.dg/predicate2-project-typ-set.ads,
gnat.dg/predicate2-project-typ.ads,
gnat.dg/predicate2-project.ads,
gnat.dg/predicate2-source_reference.ads, gnat.dg/predicate2.ads,
gnat.dg/predicate2_main.adb: New testcase.--- gcc/ada/exp_util.adb
+++ gcc/ada/exp_util.adb
@@ -9313,14 +9313,16 @@ package body Exp_Util is
 
   --  If the type is tagged, the expression may be class-wide, in which
   --  case it has to be converted to its root type, given that the
-  --  generated predicate function is not dispatching.
+  --  generated predicate function is not dispatching. The conversion
+  --  is type-safe and does not need validation, which matters when
+  --  private extensions are involved.
 
   if Is_Tagged_Type (Typ) then
  Call :=
Make_Function_Call (Loc,
  Name   => New_Occurrence_Of (Func_Id, Loc),
  Parameter_Associations =>
-   New_List (Convert_To (Typ, Relocate_Node (Expr;
+   New_List (OK_Convert_To (Typ, Relocate_Node (Expr;
   else
  Call :=
Make_Function_Call (Loc,

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-containers.ads
@@ -0,0 +1,13 @@
+
+with Ada.Containers.Indefinite_Vectors;
+
+package Predicate2.Containers is
+
+   subtype Count_Type is Ada.Containers.Count_Type;
+
+   package Value_Type_List is
+ new Ada.Containers.Indefinite_Vectors (Positive, Value_Type);
+
+   subtype Value_List is Value_Type_List.Vector;
+
+end Predicate2.Containers;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-project-name_values.ads
@@ -0,0 +1,37 @@
+
+
+with Predicate2.Containers;
+with Predicate2.Project.Registry.Attribute;
+with Predicate2.Source_Reference;
+
+private with Ada.Strings.Unbounded;
+
+package Predicate2.Project.Name_Values is
+
+   use type Containers.Count_Type;
+   use all type Registry.Attribute.Value_Kind;
+
+   type Object is new Source_Reference.Object with private;
+
+   Undefined : constant Object;
+
+   subtype Value_Kind is Registry.Attribute.Value_Kind;
+
+   function Kind (Self : Object'Class) return Registry.Attribute.Value_Kind
+ with Pre => Object (Self) /= Undefined;
+   --  Returns the Kind for the Name/Values pair object
+
+private
+
+   use Ada.Strings.Unbounded;
+
+   type Object is new Source_Reference.Object with record
+  Kind   : Registry.Attribute.Value_Kind := List;
+  Name   : Unbounded_String;
+  Values : Containers.Value_List;
+   end record;
+
+   Undefined : constant Object :=
+ Object'(Source_Reference.Object with others => <>);
+
+end Predicate2.Project.Name_Values;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-project-registry-attribute.ads
@@ -0,0 +1,7 @@
+
+
+package Predicate2.Project.Registry.Attribute is
+
+   type Value_Kind is (Single, List);
+
+end Predicate2.Project.Registry.Attribute;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-project-registry.ads
@@ -0,0 +1,3 @@
+
+package Predicate2.Project.Registry is
+end Predicate2.Project.Registry;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-project-typ-set.ads
@@ -0,0 +1,13 @@
+
+with Ada.Containers.Indefinite_Ordered_Maps;
+
+package Predicate2.Project.Typ.Set is
+
+   --  The type names must not be case-sensitive
+
+   package Set is new Ada.Containers.Indefinite_Ordered_Maps
+ (Name_Type, Object, "<");
+
+   subtype Object is Set.Map;
+
+end Predicate2.Project.Typ.Set;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate2-project-typ.ads
@@ -0,0 +1,24 @@
+
+with Predicate2.Project.Name_Values;
+
+private with Predicate2.Project.Registry.Attribute;
+
+package Predicate2.Project.Typ is
+
+   type Object is new Name_Values.Object with private;
+
+   Undefined : constant Object;
+
+private
+
+   use all type Predicate2.Project.Registry.Attribute.Value_Kind;
+
+   --  BUG HERE: removing the Dynamic_Predicate below will allow
+   --  compilation of the unit.
+
+   type Object is new Name_Values.Object with null record
+with Dynamic_Predicate => Object.Kind = List;
+
+   U

[Ada] Spurious dependency on secondary stack

[Pierre-Marie de Rodat]

This patch reimplements the handling of the secondary stack when the
iteration scheme of a loop statement requires this support.

Prior to this modification, an iterator loop over a container was
assumed to require unconditional secondary stack management. This is
however not always true because of user-defined iterator types, where
routines First and Next return an iterator that does require the
secondary stack.


-- Source --


--  gnat.adc

pragma Restrictions (No_Secondary_Stack);

--  test.ads

package Test is
   type Test_Type is private
   with
  Default_Initial_Condition,
  Iterable => (First   => First_Element,
   Next=> Next_Element,
   Has_Element => Has_Element,
   Element => Element);

   type Cursor_Type is private;

   function First_Element (T : Test_Type) return Cursor_Type;

   function Next_Element (T : Test_Type; C : Cursor_Type) return Cursor_Type;

   function Has_Element (T : Test_Type; C : Cursor_Type) return Boolean;

   function Element (T : Test_Type; C : Cursor_Type) return Natural;

private
   type Cursor_Type is new Natural;

   type Test_Type is record
  null;
   end record;

   function First_Element (T : Test_Type) return Cursor_Type
   is (0);

   function Next_Element (T : Test_Type; C : Cursor_Type) return Cursor_Type
   is (0);

   function Has_Element (T : Test_Type; C : Cursor_Type) return Boolean
   is (False);

   function Element (T : Test_Type; C : Cursor_Type) return Natural
   is (0);
end Test;

--  main.adb

with Test; use Test;

procedure Main is
   F : Boolean;
   M : Test_Type;

begin
   for Elem of M loop
  null;
   end loop;

   F := (for all C of M => C = 1);
   F := (for all C in M => True);
end Main;

-
-- Compilation --
-

$ gnatmake -q --RTS=zfp -nostdlib main.adb

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

2018-09-26  Hristian Kirtchev  

gcc/ada/

* exp_ch4.adb (Expand_N_Allocator): Ensure that the use of the
secondary stack does not clash with restriction
No_Secondary_Stack.
* exp_ch6.adb (Expand_N_Extended_Return_Statement): Ensure that
the use of the secondary stack does not clash with restriction
No_Secondary_Stack.
* sem_ch5.adb (Analyze_Loop_Statement): Wrap the loop in a block
prior to analysis in order to either provide a local scope for
an iterator, or ensure that the secondary stack is properly
managed.
(Check_Call): Account for the case where the tree may be
unanalyzed or contain prior errors.
(Has_Call_Using_Secondary_Stack): Renamed to Has_Sec_Stack_Call.
Update all uses of the subprogram.
(Prepare_Loop_Statement): New routine.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -4417,6 +4417,7 @@ package body Exp_Ch4 is
 Set_Storage_Pool (N, Pool);
 
 if Is_RTE (Pool, RE_SS_Pool) then
+   Check_Restriction (No_Secondary_Stack, N);
Set_Procedure_To_Call (N, RTE (RE_SS_Allocate));
 
 --  In the case of an allocator for a simple storage pool, locate

--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -5267,8 +5267,9 @@ package body Exp_Ch6 is
 Set_Comes_From_Source (Pool_Allocator, True);
  end if;
 
- --  The allocator is returned on the secondary stack.
+ --  The allocator is returned on the secondary stack
 
+ Check_Restriction (No_Secondary_Stack, N);
  Set_Storage_Pool (SS_Allocator, RTE (RE_SS_Pool));
  Set_Procedure_To_Call
(SS_Allocator, RTE (RE_SS_Allocate));

--- gcc/ada/sem_ch5.adb
+++ gcc/ada/sem_ch5.adb
@@ -83,7 +83,7 @@ package body Sem_Ch5 is
--  messages. This variable is recursively saved on entry to processing the
--  construct, and restored on exit.
 
-   function Has_Call_Using_Secondary_Stack (N : Node_Id) return Boolean;
+   function Has_Sec_Stack_Call (N : Node_Id) return Boolean;
--  N is the node for an arbitrary construct. This function searches the
--  construct N to see if any expressions within it contain function
--  calls that use the secondary stack, returning True if any such call
@@ -2850,7 +2850,7 @@ package body Sem_Ch5 is
 --  proper trace of the value, useful in optimizations that get rid
 --  of junk range checks.
 
-if not Has_Call_Using_Secondary_Stack (Analyzed_Bound) then
+if not Has_Sec_Stack_Call (Analyzed_Bound) then
Analyze_And_Resolve (Original_Bound, Typ);
 
--  Ensure that the bound is valid. This check should not be
@@ -3360,18 +3360,23 @@ package body Sem_Ch5 is
 
procedure Analyze_Loop_Statement (N : Node_Id) is
 
+  --  The following exception is raised by routine Prepare_Loop_Statement
+  -

[Ada] Spurious error on interface conversion under ZFP

[Pierre-Marie de Rodat]

The frontend reports an error under ZFP when performing the type
conversion of a tagged object to one of its covered interface types.

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

2018-09-26  Javier Miranda  

gcc/ada/

* exp_disp.adb (Expand_Interface_Conversion): No displacement of
the pointer needed when the type of the operand is an interface
type that maches the target type and we are compiling under
configurable runtime. Adding also documentation explaining why
this cannot be done when compiling with the full runtime.
* exp_intr.adb: Update comment.

gcc/testsuite/

* gnat.dg/interface8.adb, gnat.dg/interface8.ads: New testcase.--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -1339,11 +1339,39 @@ package body Exp_Disp is
 Opnd := Designated_Type (Opnd);
  end if;
 
+ Opnd := Underlying_Record_Type (Opnd);
+
  if not Is_Interface (Opnd)
and then Is_Ancestor (Iface_Typ, Opnd, Use_Full_View => True)
  then
 return;
  end if;
+
+ --  When the type of the operand and the target interface type match,
+ --  it is generally safe to skip generating code to displace the
+ --  pointer to the object to reference the secondary dispatch table
+ --  associated with the target interface type. The exception to this
+ --  general rule is when the underlying object of the type conversion
+ --  is an object built by means of a dispatching constructor (since in
+ --  such case the expansion of the constructor call is a direct call
+ --  to an object primitive, i.e. without thunks, and the expansion of
+ --  the constructor call adds an explicit conversion to the target
+ --  interface type to force the displacement of the pointer to the
+ --  object to reference the corresponding secondary dispatch table
+ --  (cf. Make_DT and Expand_Dispatching_Constructor_Call)).
+
+ --  At this stage we cannot identify whether the underlying object is
+ --  a BIP object and hence we cannot skip generating the code to try
+ --  displacing the pointer to the object. However, under configurable
+ --  runtime it is safe to skip generating code to displace the pointer
+ --  to the object, because generic dispatching constructors are not
+ --  supported.
+
+ if Opnd = Iface_Typ
+   and then not RTE_Available (RE_Displace)
+ then
+return;
+ end if;
   end;
 
   --  Evaluate if we can statically displace the pointer to the object

--- gcc/ada/exp_intr.adb
+++ gcc/ada/exp_intr.adb
@@ -402,7 +402,10 @@ package body Exp_Intr is
   end if;
 
   --  Rewrite and analyze the call to the instance as a class-wide
-  --  conversion of the call to the actual constructor.
+  --  conversion of the call to the actual constructor. When the result
+  --  type is a class-wide interface type this conversion is required to
+  --  force the displacement of the pointer to the object to reference the
+  --  corresponding dispatch table.
 
   Rewrite (N, Convert_To (Result_Typ, Cnstr_Call));
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface8.adb
@@ -0,0 +1,9 @@
+--  { dg-do compile }
+
+package body Interface8 is
+   function Get_Iface (This : Child) return not null access Iface'Class
+   is
+   begin
+  return This.Interface_1;
+   end;
+end;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface8.ads
@@ -0,0 +1,11 @@
+package Interface8 is
+   type Iface is interface;
+
+   type Root is abstract tagged null record;
+
+   type Child is new Root and Iface with record
+  Interface_1 : access Iface'Class;
+   end record;
+
+   function Get_Iface (This : Child) return not null access Iface'Class;
+end;

[Ada] Do not issue by default info messages for inlining in GNATprove

[Pierre-Marie de Rodat]

Info messages about lack of inlining for analysis in GNATprove may be
confusing to users. They are now only issued when GNATprove is called
with switch --info, which it passes on to gnat2why with switch -gnatd_f.

There is no effect on compilation.

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

2018-09-26  Yannick Moy  

gcc/ada/

* debug.adb: Add use for -gnatd_f switch.
* inline.adb (Cannot_Inline): Only issue info message for
failure to inline in GNATprove mode when switch -gnatd_f is
used.--- gcc/ada/debug.adb
+++ gcc/ada/debug.adb
@@ -150,7 +150,7 @@ package body Debug is
--  d_c
--  d_d
--  d_e  Ignore entry calls and requeue statements for elaboration
-   --  d_f
+   --  d_f  Issue info messages related to GNATprove usage
--  d_g
--  d_h
--  d_i  Ignore activations and calls to instances for elaboration
@@ -831,6 +831,11 @@ package body Debug is
--   control, conditional entry calls, timed entry calls, and requeue
--   statements in both the static and dynamic elaboration models.
 
+   --  d_f  Issue info messages related to GNATprove usage to help users
+   --   understand analysis results. By default these are not issued as
+   --   beginners find them confusing. Set automatically by GNATprove when
+   --   switch --info is used.
+
--  d_i  The compiler ignores calls and task activations when they target a
--   subprogram or task type defined in an external instance for both
--   the static and dynamic elaboration models.

--- gcc/ada/inline.adb
+++ gcc/ada/inline.adb
@@ -1607,13 +1607,16 @@ package body Inline is
  then
 null;
 
- --  In GNATprove mode, issue a warning, and indicate that the
- --  subprogram is not always inlined by setting flag Is_Inlined_Always
- --  to False.
+ --  In GNATprove mode, issue a warning when -gnatd_f is set, and
+ --  indicate that the subprogram is not always inlined by setting
+ --  flag Is_Inlined_Always to False.
 
  elsif GNATprove_Mode then
 Set_Is_Inlined_Always (Subp, False);
-Error_Msg_NE (Msg & "p?", N, Subp);
+
+if Debug_Flag_Underscore_F then
+   Error_Msg_NE (Msg & "p?", N, Subp);
+end if;
 
  elsif Has_Pragma_Inline_Always (Subp) then
 
@@ -1634,12 +1637,16 @@ package body Inline is
 
  Error_Msg_NE (Msg (Msg'First .. Msg'Last - 1), N, Subp);
 
-  --  In GNATprove mode, issue a warning, and indicate that the subprogram
-  --  is not always inlined by setting flag Is_Inlined_Always to False.
+  --  In GNATprove mode, issue a warning when -gnatd_f is set, and
+  --  indicate that the subprogram is not always inlined by setting
+  --  flag Is_Inlined_Always to False.
 
   elsif GNATprove_Mode then
  Set_Is_Inlined_Always (Subp, False);
- Error_Msg_NE (Msg & "p?", N, Subp);
+
+ if Debug_Flag_Underscore_F then
+Error_Msg_NE (Msg & "p?", N, Subp);
+ end if;
 
   else
 

[Ada] Spurious elaboration issue due to inlining

[Pierre-Marie de Rodat]

This patch ensures that the full compilation context is captured prior
to package or subprogram instantiation/inlining and restored after the
action takes place.

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

2018-09-26  Hristian Kirtchev  

gcc/ada/

* sem_ch12.adb (Instantiate_Package_Body): Capture and restore
the full compilation context.
(Instantiate_Subprogram_Body): Capture and restore the full
compilation context.

gcc/testsuite/

* gnat.dg/elab7.adb, gnat.dg/elab7_pkg1.adb,
gnat.dg/elab7_pkg1.ads, gnat.dg/elab7_pkg2.adb,
gnat.dg/elab7_pkg2.ads: New testcase.--- gcc/ada/sem_ch12.adb
+++ gcc/ada/sem_ch12.adb
@@ -11202,10 +11202,6 @@ package body Sem_Ch12 is
   Gen_Decl: constant Node_Id:= Unit_Declaration_Node (Gen_Unit);
   Loc : constant Source_Ptr := Sloc (Inst_Node);
 
-  Saved_ISMP: constant Boolean :=
-   Ignore_SPARK_Mode_Pragmas_In_Instance;
-  Saved_Style_Check : constant Boolean := Style_Check;
-
   procedure Check_Initialized_Types;
   --  In a generic package body, an entity of a generic private type may
   --  appear uninitialized. This is suspicious, unless the actual is a
@@ -11276,20 +11272,30 @@ package body Sem_Ch12 is
 
   --  Local variables
 
-  Saved_GM  : constant Ghost_Mode_Type := Ghost_Mode;
-  Saved_IGR : constant Node_Id := Ignored_Ghost_Region;
-  Saved_SM  : constant SPARK_Mode_Type := SPARK_Mode;
-  Saved_SMP : constant Node_Id := SPARK_Mode_Pragma;
-  --  Save the Ghost and SPARK mode-related data to restore on exit
+  --  The following constants capture the context prior to instantiating
+  --  the package body.
 
-  Act_Body : Node_Id;
-  Act_Body_Id  : Entity_Id;
-  Act_Body_Name: Node_Id;
-  Gen_Body : Node_Id;
-  Gen_Body_Id  : Node_Id;
-  Par_Ent  : Entity_Id := Empty;
-  Par_Vis  : Boolean   := False;
-  Parent_Installed : Boolean := False;
+  Saved_CS   : constant Config_Switches_Type := Save_Config_Switches;
+  Saved_GM   : constant Ghost_Mode_Type  := Ghost_Mode;
+  Saved_IGR  : constant Node_Id  := Ignored_Ghost_Region;
+  Saved_ISMP : constant Boolean  :=
+ Ignore_SPARK_Mode_Pragmas_In_Instance;
+  Saved_LSST : constant Suppress_Stack_Entry_Ptr :=
+ Local_Suppress_Stack_Top;
+  Saved_SC   : constant Boolean  := Style_Check;
+  Saved_SM   : constant SPARK_Mode_Type  := SPARK_Mode;
+  Saved_SMP  : constant Node_Id  := SPARK_Mode_Pragma;
+  Saved_SS   : constant Suppress_Record  := Scope_Suppress;
+  Saved_Warn : constant Warning_Record   := Save_Warnings;
+
+  Act_Body  : Node_Id;
+  Act_Body_Id   : Entity_Id;
+  Act_Body_Name : Node_Id;
+  Gen_Body  : Node_Id;
+  Gen_Body_Id   : Node_Id;
+  Par_Ent   : Entity_Id := Empty;
+  Par_Installed : Boolean := False;
+  Par_Vis   : Boolean   := False;
 
   Vis_Prims_List : Elist_Id := No_Elist;
   --  List of primitives made temporarily visible in the instantiation
@@ -11452,13 +11458,13 @@ package body Sem_Ch12 is
 Par_Ent := Entity (Prefix (Gen_Id));
 Par_Vis := Is_Immediately_Visible (Par_Ent);
 Install_Parent (Par_Ent, In_Body => True);
-Parent_Installed := True;
+Par_Installed := True;
 
  elsif Is_Child_Unit (Gen_Unit) then
 Par_Ent := Scope (Gen_Unit);
 Par_Vis := Is_Immediately_Visible (Par_Ent);
 Install_Parent (Par_Ent, In_Body => True);
-Parent_Installed := True;
+Par_Installed := True;
  end if;
 
  --  If the instantiation is a library unit, and this is the main unit,
@@ -11527,7 +11533,7 @@ package body Sem_Ch12 is
  --  Remove the parent instances if they have been placed on the scope
  --  stack to compile the body.
 
- if Parent_Installed then
+ if Par_Installed then
 Remove_Parent (In_Body => True);
 
 --  Restore the previous visibility of the parent
@@ -11599,13 +11605,21 @@ package body Sem_Ch12 is
  end if;
   end if;
 
-  Expander_Mode_Restore;
-
<>
+
+  --  Restore the context that was in effect prior to instantiating the
+  --  package body.
+
   Ignore_SPARK_Mode_Pragmas_In_Instance := Saved_ISMP;
-  Restore_Ghost_Region (Saved_GM, Saved_IGR);
-  Restore_SPARK_Mode   (Saved_SM, Saved_SMP);
-  Style_Check := Saved_Style_Check;
+  Local_Suppress_Stack_Top  := Saved_LSST;
+  Scope_Suppress:= Saved_SS;
+  Style_Check   := Saved_SC;
+
+  Expander_Mode_Restore;
+  Restore_Config_Switches (Sa

[Ada] Missing front-end code for constraint checks on fixed point exprs

[Pierre-Marie de Rodat]

This patch ensures that the front-end generates constraint checks for
some operations that previously depended on gigi for the corresponding
check. The patch also resets the Do_Range_Check flag so that it never
appears in the tree presented to gigi.

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

2018-09-26  Ed Schonberg  

gcc/ada/

* checks.adb (Apply_Type_Conversion_Checks): Do not generate a
range check on the expression of the conversion if it involves a
fixed-point type, as such conversions are handled specially
during expansion.
* exp_ch4.adb (Expand_N_Type_Conversion): In a conversion from
Fixed to Integer, use the base type of the expression to ensure
that the caller will generate the proper constraint check when
needed.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -3550,8 +3550,21 @@ package body Checks is
   Apply_Float_Conversion_Check (Expr, Target_Type);
 
else
-  Apply_Scalar_Range_Check
-(Expr, Target_Type, Fixed_Int => Conv_OK);
+  --  Conversions involving fixed-point types are expanded
+  --  separately, and do not need a Range_Check flag, except
+  --  in SPARK_Mode, where the explicit constraint check will
+  --  not be generated.
+
+  if SPARK_Mode = On
+or else (not Is_Fixed_Point_Type (Expr_Type)
+  and then not Is_Fixed_Point_Type (Target_Type))
+  then
+ Apply_Scalar_Range_Check
+   (Expr, Target_Type, Fixed_Int => Conv_OK);
+
+  else
+ Set_Do_Range_Check (Expression (N), False);
+  end if;
 
   --  If the target type has predicates, we need to indicate
   --  the need for a check, even if Determine_Range finds that

--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -11694,6 +11694,11 @@ package body Exp_Ch4 is
 elsif Is_Integer_Type (Etype (N)) then
Expand_Convert_Fixed_To_Integer (N);
 
+   --  The result of the conversion might need a range check,
+   --   so do not assume that the result is in bounds.
+
+   Set_Etype (N, Base_Type (Target_Type));
+
 else
pragma Assert (Is_Floating_Point_Type (Etype (N)));
Expand_Convert_Fixed_To_Float (N);

[Ada] Issue info message on inlined subprograms in GNATprove mode

[Pierre-Marie de Rodat]

Issue a positive message that inlining was performed in GNATprove mode,
when corresponding debug switch -gnatd_f is set.

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

2018-09-26  Yannick Moy  

gcc/ada/

* errout.ads: Update comment for insertion character '?'.
* inline.adb: Use simple insertion character '?' for GNATprove
info messages.
* sem_res.adb (Resolve_Call): Issue an info message on inlining
in GNATprove mode.--- gcc/ada/errout.ads
+++ gcc/ada/errout.ads
@@ -305,7 +305,9 @@ package Errout is
--  Note: this usage is obsolete, use ?? ?*? ?$? ?x? ?X? to specify
--  the string to be added when Warn_Doc_Switch is set to True. If this
--  switch is True, then for simple ? messages it has no effect. This
-   --  simple form is to ease transition and will be removed later.
+   --  simple form is to ease transition and may be removed later except
+   --  for GNATprove-specific messages (info and warnings) which are not
+   --  subject to the same GNAT warning switches.
 
--Insertion character ?? (Two question marks: default warning)
--  Like ?, but if the flag Warn_Doc_Switch is True, adds the string

--- gcc/ada/inline.adb
+++ gcc/ada/inline.adb
@@ -1615,7 +1615,7 @@ package body Inline is
 Set_Is_Inlined_Always (Subp, False);
 
 if Debug_Flag_Underscore_F then
-   Error_Msg_NE (Msg & "p?", N, Subp);
+   Error_Msg_NE (Msg, N, Subp);
 end if;
 
  elsif Has_Pragma_Inline_Always (Subp) then
@@ -1645,7 +1645,7 @@ package body Inline is
  Set_Is_Inlined_Always (Subp, False);
 
  if Debug_Flag_Underscore_F then
-Error_Msg_NE (Msg & "p?", N, Subp);
+Error_Msg_NE (Msg, N, Subp);
  end if;
 
   else

--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -6842,9 +6842,15 @@ package body Sem_Res is
 ("cannot inline & (possible check on input parameters)?",
  N, Nam_UA);
 
-   --  Otherwise, inline the call
+   --  Otherwise, inline the call, issuing an info message when
+   --  -gnatd_f is set.
 
else
+  if Debug_Flag_Underscore_F then
+ Error_Msg_NE
+   ("info: analyzing call to & in context?", N, Nam_UA);
+  end if;
+
   Expand_Inlined_Call (N, Nam_UA, Nam);
end if;
 end if;

Re: [PATCH] Come up with --param asan-stack-small-redzone (PR sanitizer/81715).

[Martin Liška]

On 9/25/18 5:53 PM, Jakub Jelinek wrote:
> On Tue, Sep 25, 2018 at 05:26:44PM +0200, Martin Liška wrote:
>> The only missing piece is how to implement asan_emit_redzone_payload more 
>> smart.
>> It means doing memory stores with 8,4,2,1 sizes in order to reduce # of 
>> insns.
>> Do we have somewhere a similar code?
> 
> Yeah, that is a very important optimization.  I wasn't using DImode because
> at least on x86_64 64-bit constants are quite expensive and on several other
> targets even more so, so SImode was a compromise to get size of the prologue
> under control and not very slow.  What I think we want is figure out ranges

Ah, some time ago, I remember you mentioned the 64-bit constants are expensive
(even on x86_64). Btw. it's what clang used for the red zone instrumentation.

> of shadow bytes we want to initialize and the values we want to store there,
> perhaps take also into account strict alignment vs. non-strict alignment,
> and perform kind of store merging for it.  Given that 2 shadow bytes would
> be only used for the very small variables (<=4 bytes in size, so <= 0.5
> bytes of shadow), we'd just need a way to remember the 2 shadow bytes across
> handling adjacent vars and store it together.

Agree, it's implemented in next version of patch.

> 
> I think we want to introduce some define for minimum red zone size and use
> it instead of the granularity (granularity is 8 bytes, but minimum red zone
> size if we count into it also the very small variable size is 16 bytes).
> 
>> --- a/gcc/asan.h
>> +++ b/gcc/asan.h
>> @@ -102,6 +102,26 @@ asan_red_zone_size (unsigned int size)
>>return c ? 2 * ASAN_RED_ZONE_SIZE - c : ASAN_RED_ZONE_SIZE;
>>  }
>>  
>> +/* Return how much a stack variable occupy on a stack
>> +   including a space for redzone.  */
>> +
>> +static inline unsigned int
>> +asan_var_and_redzone_size (unsigned int size)
> 
> The argument needs to be UHWI, otherwise you do a wrong thing for
> say 4GB + 4 bytes long variable.  Ditto the result.
> 
>> +{
>> +  if (size <= 4)
>> +return 16;
>> +  else if (size <= 16)
>> +return 32;
>> +  else if (size <= 128)
>> +return 32 + size;
>> +  else if (size <= 512)
>> +return 64 + size;
>> +  else if (size <= 4096)
>> +return 128 + size;
>> +  else
>> +return 256 + size;
> 
> I'd prefer size + const instead of const + size operand order.
> 
>> @@ -1125,13 +1125,13 @@ expand_stack_vars (bool (*pred) (size_t), struct 
>> stack_vars_data *data)
>>&& stack_vars[i].size.is_constant ())
>>  {
>>prev_offset = align_base (prev_offset,
>> -MAX (alignb, ASAN_RED_ZONE_SIZE),
>> +MAX (alignb, ASAN_SHADOW_GRANULARITY),
> 
> Use that ASAN_MIN_RED_ZONE_SIZE (16) here.
> 
>>  !FRAME_GROWS_DOWNWARD);
>>tree repr_decl = NULL_TREE;
>> +  poly_uint64 size =  asan_var_and_redzone_size 
>> (stack_vars[i].size.to_constant ());
> 
> Too long line.  Two spaces instead of one.  Why poly_uint64?
> Plus, perhaps if data->asan_vec is empty (i.e. when assigning the topmost
> automatic variable in a frame), we should ensure that size is at least
> 2 * ASAN_RED_ZONE_SIZE (or just 1 * ASAN_RED_ZONE_SIZE). 
> 
>>offset
>> -= alloc_stack_frame_space (stack_vars[i].size
>> -   + ASAN_RED_ZONE_SIZE,
>> -   MAX (alignb, ASAN_RED_ZONE_SIZE));
>> += alloc_stack_frame_space (size,
>> +   MAX (alignb, 
>> ASAN_SHADOW_GRANULARITY));
> 
> Again, too long line and we want 16 instead of 8 here too.
>>  
>>data->asan_vec.safe_push (prev_offset);
>>/* Allocating a constant amount of space from a constant
>> @@ -2254,7 +2254,7 @@ expand_used_vars (void)
>>   & ~(data.asan_alignb - HOST_WIDE_INT_1)) - sz;
>>/* Allocating a constant amount of space from a constant
>>   starting offset must give a constant result.  */
>> -  offset = (alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE)
>> +  offset = (alloc_stack_frame_space (redzonesz, ASAN_SHADOW_GRANULARITY)
> 
> and here too.
> 
>   Jakub
> 

The rest is also implemented as requested. I'm testing Linux kernel now, will 
send
stats to the PR created for it.

Patch survives testing on x86_64-linux-gnu.

Martin
>From acbea3a2127a5eb19e23a202010847e098cf8ce8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 25 Sep 2018 10:54:37 +0200
Subject: [PATCH] Make red zone size more flexible for stack variables (PR
 sanitizer/81715).

gcc/ChangeLog:

2018-09-26  Martin Liska  

	PR sanitizer/81715
	* asan.c (asan_shadow_cst): Remove.
	(asan_emit_redzone_payload): New.
	(asan_emit_stack_protection): Make it more
	flexible to support arbitrary size of red zones.
	* asan.h (ASAN_MIN_RED_ZONE_SIZE): New.
	(asan_var_and_redzone_size): Likewise.
	* cfgexpand

[Ada] Spurious ineffective use_clause warning

[Pierre-Marie de Rodat]

This patch fixes an issue whereby user-defined subprograms used as
generic actuals with corresponding formals containing other formal types
led to spurious ineffective use_clause warnings.

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

2018-09-26  Justin Squirek  

gcc/ada/

* sem_ch8.adb (Analyze_Subprogram_Renaming): Add extra condition
to check for unmarked subprogram references coming from
renamings.

gcc/testsuite/

* gnat.dg/warn16.adb: New testcase.--- gcc/ada/sem_ch8.adb
+++ gcc/ada/sem_ch8.adb
@@ -3692,8 +3692,16 @@ package body Sem_Ch8 is
   --  and mark any use_package_clauses that affect the visibility of the
   --  implicit generic actual.
 
+  --  Also, we may be looking at an internal renaming of a user-defined
+  --  subprogram created for a generic formal subprogram association,
+  --  which will also have to be marked here. This can occur when the
+  --  corresponding formal subprogram contains references to other generic
+  --  formals.
+
   if Is_Generic_Actual_Subprogram (New_S)
-and then (Is_Intrinsic_Subprogram (New_S) or else From_Default (N))
+and then (Is_Intrinsic_Subprogram (New_S)
+   or else From_Default (N)
+   or else Nkind (N) = N_Subprogram_Renaming_Declaration)
   then
  Mark_Use_Clauses (New_S);
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/warn16.adb
@@ -0,0 +1,38 @@
+--  { dg-do compile }
+--  { dg-options "-gnatwa" }
+
+procedure Warn16 is
+
+   package Define is
+  type Key_Type is record
+ Value : Integer := 0;
+  end record;
+
+  function "=" (Left  : in Key_Type;
+Right : in Key_Type)
+return Boolean;
+   end;
+   package body Define is
+  function "=" (Left  : in Key_Type;
+Right : in Key_Type)
+return Boolean is
+  begin
+ return Left.Value = Right.Value;
+  end;
+   end;
+
+   generic
+  type Key_Type is private;
+  with function "=" (Left  : in Key_Type;
+ Right : in Key_Type)
+ return Boolean;
+   package Oper is end;
+
+   use type Define.Key_Type; -- !!!
+
+   package Inst is new Oper (Key_Type => Define.Key_Type,
+ "="  => "=");
+   pragma Unreferenced (Inst);
+begin
+   null;
+end;

[Ada] Fix inheritance of representation items defined as aspects

[Pierre-Marie de Rodat]

When a representation item is defined by a pragma or attribute
definition clause, the entity it applies to is that of the Name of the
representation item. But when it is defined by an aspect definition, the
entity is directly denoted by the Entity attribute of the represenation
item. The circuitry that inherits representation items for derived types
or subtypes must account for these two possible cases.

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

2018-09-26  Thomas Quinot  

gcc/ada/

* sem_ch13.adb (Inherit_Aspects_At_Freeze_Point): For a
representation item that is an N_Aspect_Definition, retrieve the
entity it applies to using the Entity attribute.

gcc/testsuite/

* gnat.dg/sso13.adb: New testcase.--- gcc/ada/sem_ch13.adb
+++ gcc/ada/sem_ch13.adb
@@ -11446,6 +11446,26 @@ package body Sem_Ch13 is
   --  specification node whose correponding pragma (if any) is present in
   --  the Rep Item chain of the entity it has been specified to.
 
+  function Rep_Item_Entity (Rep_Item : Node_Id) return Entity_Id;
+  --  Return the entity for which Rep_Item is specified
+
+  -
+  -- Rep_Item_Entity --
+  -
+
+  function Rep_Item_Entity (Rep_Item : Node_Id) return Entity_Id is
+  begin
+ if Nkind (Rep_Item) = N_Aspect_Specification then
+return Entity (Rep_Item);
+
+ else
+pragma Assert (Nkind_In (Rep_Item,
+ N_Pragma,
+ N_Attribute_Definition_Clause));
+return Entity (Name (Rep_Item));
+ end if;
+  end Rep_Item_Entity;
+
   --
   -- Is_Pragma_Or_Corr_Pragma_Present_In_Rep_Item --
   --
@@ -11650,8 +11670,8 @@ package body Sem_Ch13 is
  and then Has_Rep_Item (Typ, Name_Bit_Order)
then
   Set_Reverse_Bit_Order (Bas_Typ,
-Reverse_Bit_Order (Entity (Name
-  (Get_Rep_Item (Typ, Name_Bit_Order);
+Reverse_Bit_Order (Rep_Item_Entity
+  (Get_Rep_Item (Typ, Name_Bit_Order;
end if;
 end if;
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/sso13.adb
@@ -0,0 +1,24 @@
+--  { dg-do compile }
+
+with System;
+
+procedure SSO13 is
+   type Pulse_Buffer_Type is abstract tagged null record
+with Bit_Order => System.High_Order_First,
+ Scalar_Storage_order =>System.High_order_First;  --  { dg-warning "scalar storage order specified but no component clause" }
+   type Pulse_Train_Type is abstract new Pulse_Buffer_Type with null record;
+   type WO_Pulse_Train_Type is new Pulse_Train_Type with null record;
+   type WO_Confirmation_Pulse_Train_Type is new WO_Pulse_Train_Type with record
+  null;
+   end record;
+
+   type Update_Dwell_Type is abstract tagged null record
+ with Bit_Order => System.High_Order_First,
+  Scalar_Storage_order =>System.High_order_First;  --  { dg-warning "scalar storage order specified but no component clause" }
+   type Confirmation_Dwell_Type is new Update_Dwell_Type with
+   record
+  Pulses : aliased WO_Pulse_Train_Type; -- (Location of Error #1)
+   end record;
+begin
+   null;
+end;

Re: libgo patch committed: Update to 1.11 release

[Andreas Schwab]

All execution tests are now failing with "fatal error: impossible call
to aeshashbody".

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Alexander Monakov]

On Fri, 21 Sep 2018, Qing Zhao wrote:
> +2018-09-20  Qing Zhao  
> +
> + * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> + * common.opt (-finline-only-static): New option.
> + * doc/invoke.texi: Document -finline-only-static.
> + * ipa-inline.c (can_inline_edge_p): Control inlining based on
> + function's linkage. 

Note, I am not a reviewer.

In my opinion, there's a problem with the patch that it looks like an ad-hoc,
incomplete solution. You said you need this change to help with building
livepatching-capable kernels, but it's not clear what exactly the issue with
inlining non-static functions is. Can you describe how the workflow looks like
so code duplication due to inlining static functions is not an issue, but
inlining non-static functions is a problem? Does using existing
-fno-inline-functions flag achieve something useful for your usecase?

Please always make it clear what problem the patch is intended to solve and help
reviewers see the connection between the problem and your solution. Look how the
"XY problem" effect applies partially in this situation.

https://en.wikipedia.org/wiki/XY_problem
http://xyproblem.info/

Alexander

Re: [PATCH v4] [aarch64] Add HiSilicon tsv110 CPU support

[Kyrill Tkachov]

Hi Shaokun,

On 25/09/18 14:40, Zhangshaokun wrote:

Hi ARM maintainers,

Any plan to support CTR_EL0.DIC and CTR_EL0.IDC in GCC?
I saw it has been supported in linux mainline(on Mar 7),
Patch link:
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565090.html
Kernel link:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/cpufeature.c?h=v4.19-rc5
 +205


Do you mean implementing the data cache clearing elision in __clear_cache as 
discussed in June [1]?
I am not aware of any plans to implement that support yet as we'd need hardware 
to test this properly on.

If you can implement and test it and post it to the list I'm sure the 
maintainers would be happy to review such patches though.

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00307.html

Thanks,
Shaokun

On 2018/9/20 22:22, James Greenhalgh wrote:

On Wed, Sep 19, 2018 at 04:53:52AM -0500, Shaokun Zhang wrote:

This patch adds HiSilicon's an mcpu: tsv110, which supports v8_4A.
It has been tested on aarch64 and no regressions from this patch.

This patch is OK for Trunk.

Do you need someone to commit it on your behalf?

Thanks,
James


---
  gcc/ChangeLog|   9 +++
  gcc/config/aarch64/aarch64-cores.def |   3 +
  gcc/config/aarch64/aarch64-cost-tables.h | 104 +++
  gcc/config/aarch64/aarch64-tune.md   |   2 +-
  gcc/config/aarch64/aarch64.c |  82 
  gcc/doc/invoke.texi  |   2 +-
  6 files changed, 200 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 69e2e14..a040daa 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2018-09-19  Shaokun Zhang  
+Bo Zhou  
+
+   * config/aarch64/aarch64-cores.def (tsv110): New CPU.
+   * config/aarch64/aarch64-tune.md: Regenerated.
+   * doc/invoke.texi (AArch64 Options/-mtune): Add "tsv110".
+   * config/aarch64/aarch64.c (tsv110_tunings): New tuning table.
+   * config/aarch64/aarch64-cost-tables.h: Add "tsv110" extra costs.
+
  2018-09-18  Marek Polacek  
  
  	P1064R0 - Allowing Virtual Function Calls in Constant Expressions
  


.

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Paolo Carlini]

Hi,

On 9/26/18 1:12 PM, Alexander Monakov wrote:

On Fri, 21 Sep 2018, Qing Zhao wrote:

+2018-09-20  Qing Zhao  
+
+   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
+   * common.opt (-finline-only-static): New option.
+   * doc/invoke.texi: Document -finline-only-static.
+   * ipa-inline.c (can_inline_edge_p): Control inlining based on
+   function's linkage.

Note, I am not a reviewer.

In my opinion, there's a problem with the patch that it looks like an ad-hoc,
incomplete solution.


It is not. Actually, I don't understand why we are raising this sort of 
issue now, after so many messages, like, for example Jeff's, which 
should have fully clarified the rationale.


Paolo.

Re: libgo patch committed: Update to 1.11 release

[Ian Lance Taylor]

On Wed, Sep 26, 2018 at 3:54 AM, Andreas Schwab  wrote:
>
> All execution tests are now failing with "fatal error: impossible call
> to aeshashbody".

Which target?

Ian

Re: libgo patch committed: Update to 1.11 release

[Ian Lance Taylor]

On Wed, Sep 26, 2018 at 1:38 AM, Rainer Orth
 wrote:
>
>> On Sep 24 2018, Ian Lance Taylor  wrote:
>>
>>> * Makefile.am (mostlyclean-local): Run chmod on check-go-dir to
>>> make sure it is writable.
>>> (check-go-tools): Likewise.
>>
>> $ make check-gotools
>> make[1]: Entering directory `/usr/local/gcc/gcc-20180926/Build/gotools'
>> chmod -R u+w check-go-dir
>> chmod: cannot access `check-go-dir': No such file or directory
>> make[1]: *** [check-go-tool] Error 1
>> make[1]: Leaving directory `/usr/local/gcc/gcc-20180926/Build/gotools'
>> make: *** [check-gotools] Error 2
>
> I noticed the same when finding that gotools.{sum,log} hadn't been
> generated.
>
> The following patch fixed it for me.
>
> Rainer
>
> --
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University
>
>
> 2018-09-26  Rainer Orth  
>
> * Makefile.am (mostlyclean-local): Ignore chmod errors.
> (check-go-tool): Likewise.
> * Makefile.in: Regenerate.

This is OK.  Thanks for writing it.  Please go ahead and commit.

Ian

Re: [PATCH, AArch64 00/11] LSE atomics out-of-line

[Michael Matz]

Hi,

On Wed, 26 Sep 2018, Florian Weimer wrote:

> > Therefore, I've created small out-of-line helpers that are directly
> > linked into every library or executable that requires them.  There
> > will be two direct branches, both of which will be well-predicted.
> 
> This seems reasonable to me, considering the trade-offs.
> 
> If the indirect function call overhead is deemed too large,

With IFUNCs there's the concern that it's not really a feasible 
solution for all cases: you'd have to specialize each and every function 
containing atomic accesses.  That's difficult to do by hand and 
potentially explodes size when done automatically.

> the only other feasible option I see from a distribution point of view 
> is to drop support for the previous architecture version without LSE.

Agreed.  So thanks rth for that :)


Ciao,
Michael.

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jason Merrill]

On Fri, Sep 21, 2018 at 11:12 AM, Qing Zhao  wrote:
> Hi, this is the 4th version of the patch.
>
> mainly address Martin’s comments on some spelling issues.
>
> I have tested the patch on both x86 and aarch64, no issue.
>
> Okay for commit?
>
> thanks.
>
> Qing.
>
> gcc/ChangeLog
>
> +2018-09-20  Qing Zhao  
> +
> +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> +   * common.opt (-finline-only-static): New option.
> +   * doc/invoke.texi: Document -finline-only-static.
> +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
> +   function's linkage.

I would suggest "internal" rather than "static" in general.  So

+N_("function has external linkage when the user requests only"
+   " inlining functions with internal linkage"))

+Inline functions only if they have internal linkage.

+@item -finline-only-internal
+@opindex finline-only-internal
+By default, GCC inlines functions without considering their linkage.
+This flag guides the inliner to only inline functions with internal linkage.
+This option has any effect only when inlining itself is turned on by the
+-finline-functions or -finline-small-functions options.

This should also mention whether it applies to functions explicitly
declared inline; I assume it does not.

Jason

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Richard Biener]

On Wed, 26 Sep 2018, Jason Merrill wrote:

> On Fri, Sep 21, 2018 at 11:12 AM, Qing Zhao  wrote:
> > Hi, this is the 4th version of the patch.
> >
> > mainly address Martin’s comments on some spelling issues.
> >
> > I have tested the patch on both x86 and aarch64, no issue.
> >
> > Okay for commit?
> >
> > thanks.
> >
> > Qing.
> >
> > gcc/ChangeLog
> >
> > +2018-09-20  Qing Zhao  
> > +
> > +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
> > +   * common.opt (-finline-only-static): New option.
> > +   * doc/invoke.texi: Document -finline-only-static.
> > +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
> > +   function's linkage.
> 
> I would suggest "internal" rather than "static" in general.  So
> 
> +N_("function has external linkage when the user requests only"
> +   " inlining functions with internal linkage"))
> 
> +Inline functions only if they have internal linkage.
> 
> +@item -finline-only-internal
> +@opindex finline-only-internal
> +By default, GCC inlines functions without considering their linkage.
> +This flag guides the inliner to only inline functions with internal linkage.
> +This option has any effect only when inlining itself is turned on by the
> +-finline-functions or -finline-small-functions options.
> 
> This should also mention whether it applies to functions explicitly
> declared inline; I assume it does not.

IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
what 'internal' would mean in this context.

But then the implementation looks at callee->externally_visible which
matches hidden visibility... externally_visible is probably not
the very best thing to look at depending on what we intend to do.

What about 'static' functions with their address taken?

Note you shouldn't look at individual cgraph_node fields but use
some of the accessors with more well-constrained semantics.
Why are you not simply checking !TREE_PUBLIC?

Honza might be able to suggest one.

Richard.

[PATCH] Fix PR87443, bogus/missing DW_AT_abstract_origins

[Richard Biener]

This fixes mentioned PR (I should stop looking at generated debug
info...) where it notes that concrete inline instance DW_TAG_lexical_block
miss DW_AT_abstract_origin attributes pointing to the abstract instance
and that inline instances of DW_TAG_lexical_block have 
DW_AT_abstract_origins pointing to other inline or concrete instances
rather than the abstract instance.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I even added a testcase.  Yay.

OK?

Thanks,
Richard.

2018-09-26  Richard Biener  

PR debug/87443
* dwarf2out.c (gen_lexical_block_die): Do not equate inline
or concrete instance DIE to the tree.  Create abstract origin
attributes also for concrete instances.

* gcc.dg/debug/dwarf2/inline5.c: New testcase.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 264594)
+++ gcc/dwarf2out.c (working copy)
@@ -24091,18 +24091,16 @@ gen_lexical_block_die (tree stmt, dw_die
 }
   else if (BLOCK_ABSTRACT_ORIGIN (stmt))
 {
-  /* If this is an inlined instance, create a new lexical die for
-anything below to attach DW_AT_abstract_origin to.  */
+  /* If this is an inlined or conrecte instance, create a new lexical
+die for anything below to attach DW_AT_abstract_origin to.  */
   if (old_die)
-   {
- stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
- equate_block_to_die (stmt, stmt_die);
- old_die = NULL;
-   }
+   stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
 
   tree origin = block_ultimate_origin (stmt);
-  if (origin != NULL_TREE && origin != stmt)
+  if (origin != NULL_TREE && (origin != stmt || old_die))
add_abstract_origin_attribute (stmt_die, origin);
+
+  old_die = NULL;
 }
 
   if (old_die)

Index: gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c
===
--- gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c (nonexistent)
+++ gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c (working copy)
@@ -0,0 +1,26 @@
+/* Verify that the concrete instance DW_TAG_lexical_block has an abstract
+   origin.  Verify that the inline instance has the abstract instance as
+   abstract origin rather than the concrete one.  */
+/* { dg-options "-O -gdwarf -dA" } */
+/* { dg-do compile } */
+/* { dg-final { scan-assembler-times "DW_TAG_inlined_subroutine" 2 } } */
+/* { dg-final { scan-assembler-times "DW_TAG_lexical_block\\)\[^#\]*# 
DW_AT_abstract_origin" 2 } } */
+/* { dg-final { scan-assembler-times "DW_TAG_lexical_block\\)\[^#\]*# \\(DIE 
\\(0x\[0-9a-f\]*\\) DW_TAG_variable" 1 } } */
+/* We do not know which is output first so look for both invalid abstract
+   origins on the lexical blocks (knowing that the abstract instance has
+   no attribute following the DW_TAG_lexical_block.  */
+/* { dg-final { scan-assembler-not "\\(DIE \\(0x(\[0-9a-f\]*)\\) 
DW_TAG_lexical_block\\)\[^#\]*# 
\[^(\].*DW_TAG_lexical_block\\)\[^#x\]*x\\1\[^#\]*# DW_AT_abstract_origin" } } 
*/
+/* { dg-final { scan-assembler-not 
"DW_TAG_lexical_block\\)\[^#x\]*x(\[0-9a-f\]*)\[^#\]*# 
DW_AT_abstract_origin.*\\(DIE \\(0x\\1\\) DW_TAG_lexical_block\\)\[^#\]*# 
DW_AT" } } */
+
+int foo (int i)
+{
+{
+  volatile int j = i + 3;
+  return j - 2;
+}
+}
+int main()
+{
+  volatile int z = foo (-1);
+  return z;
+}

Re: [PATCH][OpenACC] Update deviceptr handling during gimplification

[Cesar Philippidis]

On 09/25/2018 05:55 PM, Julian Brown wrote:
> On Tue, 7 Aug 2018 15:09:38 -0700
> Cesar Philippidis  wrote:
> 
>> I had previously posted this patch as part of a monster deviceptr
>> patch here
>> . This
>> patch breaks out the generic gimplifier changes. Essentially, with
>> this patch, the gimplifier will now transfer deviceptr data clauses
>> using GOMP_MAP_FORCE_DEVICEPTR.
>>
>> Is this patch OK for trunk? It bootstrapped / regression tested
>> cleanly for x86_64 with nvptx offloading.
> 
> This patch also appears to fix the attached test case, which had been
> associated with a different deviceptr-related patch on the og8 branch
> (the other parts of which are upstream already). Perhaps you'd like to
> incorporate this test into your patch? It was by James Norris
> originally, IIUC.

Ok, I'll do that. Thanks for updating those tests.

Cesar

[PATCH] Fix PR87440, extra lexical block in inline instances

[Richard Biener]

We do not create a DW_AT_lexical_block for the outermost block in
functions but we do for DW_AT_inlined_subroutines.  That makes
debuginfo look like if there were two of each local, the outer
one (from the abstract instance) optimized out (visible via
info locals in gdb).

The following elides the outermost block also from inline instances.
It's a bit tricky to reliably track that block given we remove unused
blocks here and there.  The trick is to have the block in the abstract
instance _not_ point to itself (given we do not output it it isn't
the abstract origin for itself).

Bootstrapped on x86_64-unkown-linux-gnu, testing in progress.

Again with some scan-assembler testcase, guality cannot do 'info locals'.

OK?

Thanks,
Richard.

2018-09-26  Richard Biener  

PR debug/87440
* dwarf2out.c (set_block_origin_self): Do not mark outermost
block as we do not output that.
(gen_inlined_subroutine_die): Elide the originally outermost
block, matching what we do for concrete instances.
(decls_for_scope): Add parameter specifying whether to recurse
to subblocks.

* gcc.dg/debug/dwarf2/inline4.c: New testcase.

Index: gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c
===
--- gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (nonexistent)
+++ gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (working copy)
@@ -0,0 +1,17 @@
+/* Verify that the inline instance has no extra DW_TAG_lexical_block between
+   the DW_TAG_inlined_subroutine and the DW_TAG_variable for the local.  */
+/* { dg-options "-O -gdwarf -dA" } */
+/* { dg-do compile } */
+/* { dg-final { scan-assembler 
"DW_TAG_inlined_subroutine\[^\\(\]*\\(\[^\\)\]*\\)\[^\\(\]*\\(DIE 
\\(0x\[0-9a-f\]*\\) DW_TAG_formal_parameter\[^\\(\]*\\(DIE \\(0x\[0-9a-f\]*\\) 
DW_TAG_variable" } } */
+/* { dg-final { scan-assembler-times "DW_TAG_inlined_subroutine" 2 } } */
+
+static int foo (int i)
+{
+  volatile int j = i + 3;
+  return j - 2;
+}
+int main()
+{
+  volatile int z = foo (-1);
+  return z;
+}
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 264640)
+++ gcc/dwarf2out.c (working copy)
@@ -3867,7 +3867,7 @@ static void gen_subroutine_type_die (tre
 static void gen_typedef_die (tree, dw_die_ref);
 static void gen_type_die (tree, dw_die_ref);
 static void gen_block_die (tree, dw_die_ref);
-static void decls_for_scope (tree, dw_die_ref);
+static void decls_for_scope (tree, dw_die_ref, bool = true);
 static bool is_naming_typedef_decl (const_tree);
 static inline dw_die_ref get_context_die (tree);
 static void gen_namespace_die (tree, dw_die_ref);
@@ -22389,7 +22389,13 @@ set_block_origin_self (tree stmt)
 {
   if (BLOCK_ABSTRACT_ORIGIN (stmt) == NULL_TREE)
 {
-  BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
+  /* We do not mark the outermost block as we are not outputting it.
+This is then a reliable way of determining whether we should
+do the same for an inline instance.  */
+  if (TREE_CODE (BLOCK_SUPERCONTEXT (stmt)) != FUNCTION_DECL)
+   BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
+  else
+   gcc_assert (DECL_INITIAL (BLOCK_SUPERCONTEXT (stmt)) == stmt);
 
   {
tree local_decl;
@@ -24149,7 +24155,24 @@ gen_inlined_subroutine_die (tree stmt, d
 add_high_low_attributes (stmt, subr_die);
   add_call_src_coords_attributes (stmt, subr_die);
 
-  decls_for_scope (stmt, subr_die);
+  /* The inliner creates an extra BLOCK for the parameter setup,
+ we want to merge that with the actual outermost BLOCK of the
+inlined function to avoid duplicate locals in consumers.  Note
+we specially mark that not as origin-self.
+Do that by doing the recursion to subblocks on the single subblock
+of STMT.  */
+  bool unwrap_one = false;
+  if (BLOCK_SUBBLOCKS (stmt) && !BLOCK_CHAIN (BLOCK_SUBBLOCKS (stmt)))
+   {
+ tree origin = block_ultimate_origin (BLOCK_SUBBLOCKS (stmt));
+ if (origin
+ && TREE_CODE (origin) == BLOCK
+ && !BLOCK_ABSTRACT_ORIGIN (origin))
+   unwrap_one = true;
+   }
+  decls_for_scope (stmt, subr_die, !unwrap_one);
+  if (unwrap_one)
+   decls_for_scope (BLOCK_SUBBLOCKS (stmt), subr_die);
 }
 }
 
@@ -25777,7 +25800,7 @@ process_scope_var (tree stmt, tree decl,
all of its sub-blocks.  */
 
 static void
-decls_for_scope (tree stmt, dw_die_ref context_die)
+decls_for_scope (tree stmt, dw_die_ref context_die, bool recurse)
 {
   tree decl;
   unsigned int i;
@@ -25820,10 +25843,11 @@ decls_for_scope (tree stmt, dw_die_ref c
 
   /* Output the DIEs to represent all sub-blocks (and the items declared
  therein) of this block.  */
-  for (subblocks = BLOCK_SUBBLOCKS (stmt);
-   subblocks != NULL;
-   subblocks = BLOCK_CHAIN (subblocks))
-gen_block_die (subblocks, context_die);
+  if (re

Re: [patch] Fix AArch64 ILP ICE

[Richard Biener]

On Tue, Sep 25, 2018 at 4:25 PM Andrew Stubbs  wrote:
>
> On 22/09/18 19:51, Andreas Schwab wrote:
> > That breaks aarch64 ILP32.
>
> The problem is that the mode given to expand_expr is just a "hint",
> apparently, and it's being ignored.
>
> I'm testing the attached patch for GCN. It fixes the ICE for AArch64
> just fine.
>
> OK?

OK.

> Andrew

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jason Merrill]

On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
> what 'internal' would mean in this context.

I mean internal linkage as in the C and C++ standards.

Jason

Re: [PATCH] Fix PR87443, bogus/missing DW_AT_abstract_origins

[Jason Merrill]

OK.

On Wed, Sep 26, 2018 at 9:30 AM, Richard Biener  wrote:
>
> This fixes mentioned PR (I should stop looking at generated debug
> info...) where it notes that concrete inline instance DW_TAG_lexical_block
> miss DW_AT_abstract_origin attributes pointing to the abstract instance
> and that inline instances of DW_TAG_lexical_block have
> DW_AT_abstract_origins pointing to other inline or concrete instances
> rather than the abstract instance.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> I even added a testcase.  Yay.
>
> OK?
>
> Thanks,
> Richard.
>
> 2018-09-26  Richard Biener  
>
> PR debug/87443
> * dwarf2out.c (gen_lexical_block_die): Do not equate inline
> or concrete instance DIE to the tree.  Create abstract origin
> attributes also for concrete instances.
>
> * gcc.dg/debug/dwarf2/inline5.c: New testcase.
>
> Index: gcc/dwarf2out.c
> ===
> --- gcc/dwarf2out.c (revision 264594)
> +++ gcc/dwarf2out.c (working copy)
> @@ -24091,18 +24091,16 @@ gen_lexical_block_die (tree stmt, dw_die
>  }
>else if (BLOCK_ABSTRACT_ORIGIN (stmt))
>  {
> -  /* If this is an inlined instance, create a new lexical die for
> -anything below to attach DW_AT_abstract_origin to.  */
> +  /* If this is an inlined or conrecte instance, create a new lexical
> +die for anything below to attach DW_AT_abstract_origin to.  */
>if (old_die)
> -   {
> - stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
> - equate_block_to_die (stmt, stmt_die);
> - old_die = NULL;
> -   }
> +   stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
>
>tree origin = block_ultimate_origin (stmt);
> -  if (origin != NULL_TREE && origin != stmt)
> +  if (origin != NULL_TREE && (origin != stmt || old_die))
> add_abstract_origin_attribute (stmt_die, origin);
> +
> +  old_die = NULL;
>  }
>
>if (old_die)
>
> Index: gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c
> ===
> --- gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/debug/dwarf2/inline5.c (working copy)
> @@ -0,0 +1,26 @@
> +/* Verify that the concrete instance DW_TAG_lexical_block has an abstract
> +   origin.  Verify that the inline instance has the abstract instance as
> +   abstract origin rather than the concrete one.  */
> +/* { dg-options "-O -gdwarf -dA" } */
> +/* { dg-do compile } */
> +/* { dg-final { scan-assembler-times "DW_TAG_inlined_subroutine" 2 } } */
> +/* { dg-final { scan-assembler-times "DW_TAG_lexical_block\\)\[^#\]*# 
> DW_AT_abstract_origin" 2 } } */
> +/* { dg-final { scan-assembler-times "DW_TAG_lexical_block\\)\[^#\]*# \\(DIE 
> \\(0x\[0-9a-f\]*\\) DW_TAG_variable" 1 } } */
> +/* We do not know which is output first so look for both invalid abstract
> +   origins on the lexical blocks (knowing that the abstract instance has
> +   no attribute following the DW_TAG_lexical_block.  */
> +/* { dg-final { scan-assembler-not "\\(DIE \\(0x(\[0-9a-f\]*)\\) 
> DW_TAG_lexical_block\\)\[^#\]*# 
> \[^(\].*DW_TAG_lexical_block\\)\[^#x\]*x\\1\[^#\]*# DW_AT_abstract_origin" } 
> } */
> +/* { dg-final { scan-assembler-not 
> "DW_TAG_lexical_block\\)\[^#x\]*x(\[0-9a-f\]*)\[^#\]*# 
> DW_AT_abstract_origin.*\\(DIE \\(0x\\1\\) DW_TAG_lexical_block\\)\[^#\]*# 
> DW_AT" } } */
> +
> +int foo (int i)
> +{
> +{
> +  volatile int j = i + 3;
> +  return j - 2;
> +}
> +}
> +int main()
> +{
> +  volatile int z = foo (-1);
> +  return z;
> +}

[PATCH][GCC][testsuite] Fix caching of tests for multiple variant runs and update existing target-supports tests.

[Tamar Christina]

Hi All,

Currently some target supports checks such as vect_int cache their
results in a manner that would cause them not to be rechecked when
running the same tests against a different variant in a multi variant
run.  This causes tests to be skipped or run when they shouldn't be.

there is already an existing caching mechanism in place that does the
caching correctly, but presumably these weren't used because some of these
tests originally only contained static data. e.g. only checked if the target is
aarch64*-*-* etc.

This patch changes every function that needs to do any caching at all to use
check_cached_effective_target which will cache per variant instead of globally.

For those tests that already parameterize over et_index I have created
check_cached_effective_target_indexed to handle this common case by creating a 
list
containing the property name and the current value of et_index.

These changes result in a much simpler implementation for most tests and a large
reduction in lines for target-supports.exp.

Regtested on
  aarch64-none-elf
  x86_64-pc-linux-gnu
  powerpc64-unknown-linux-gnu
  arm-none-eabi

and no testsuite errors. Difference would depend on your site.exp.
On arm we get about 4500 new testcases and on aarch64 the low 10s.
On PowerPC and x86_64 no changes as expected since the default exp for these
just test the default configuration.

What this means for new target checks is that they should always use either
check_cached_effective_target or check_cached_effective_target_indexed if the
result of the check is to be cached.

As an example the new vect_int looks like

proc check_effective_target_vect_int { } {
return [check_cached_effective_target_indexed  {
  expr {
 
}}]
}

The debug information that was once there is now all hidden in
check_cached_effective_target, (called from 
check_cached_effective_target_indexed)
and so the only thing you are required to do is give it a unique cache name and 
a condition.

The condition doesn't need to be an if statement so simple boolean expressions 
are enough here:

 [istarget i?86-*-*] || [istarget x86_64-*-*]
 || ([istarget powerpc*-*-*]
 && ![istarget powerpc-*-linux*paired*])
 || ...

The expr may not be required as check_cached_effective_target forces 
evaluation, but I have
left these here just to be sure (TCL semantics is confusing at times.).


Ok for trunk?

Thank,
Tamar

gcc/testsuite/

2018-09-26  Tamar Christina  

* lib/target-supports.exp (check_cached_effective_target_indexed): New.
(check_cached_effective_target, clear_effective_target_cache): Cleanup.
(check_compile): Support values already Boolean.
(check_alias_available, check_gc_sections_available,
check_profiling_available, check_effective_target_vect_cmdline_needed,
check_effective_target_vect_int,
check_effective_target_vect_intfloat_cvt,
check_effective_target_vect_doubleint_cvt,
check_effective_target_vect_intdouble_cvt,
check_effective_target_vect_uintfloat_cvt,
check_effective_target_vect_floatint_cvt,
check_effective_target_vect_floatuint_cvt,
check_effective_target_vect_peeling_profitable,
check_effective_target_vect_simd_clones,
check_effective_target_vect_peeling_profitable,
check_effective_target_vect_simd_clones,
check_effective_target_vect_shift,
check_effective_target_vect_bswap,
check_effective_target_vect_shift_char,
check_effective_target_vect_float,
check_effective_target_vect_double,
check_effective_target_vect_long_long,
check_effective_target_vect_no_int_min_max,
check_effective_target_vect_no_int_add,
check_effective_target_vect_no_bitwise,
check_effective_target_vect_perm,
check_effective_target_vect_perm_byte,
check_effective_target_vect_perm_short,
check_effective_target_xorsign,
check_effective_target_vect_widen_sum_hi_to_si_pattern,
check_effective_target_vect_widen_sum_hi_to_si,
check_effective_target_vect_widen_sum_qi_to_hi,
check_effective_target_vect_widen_sum_qi_to_si,
check_effective_target_vect_widen_mult_qi_to_hi,
check_effective_target_vect_widen_mult_hi_to_si,
check_effective_target_vect_widen_mult_qi_to_hi_pattern,
check_effective_target_vect_widen_mult_hi_to_si_pattern,
check_effective_target_vect_widen_mult_si_to_di_pattern,
check_effective_target_vect_widen_shift,
check_effective_target_vect_sdot_qi,
check_effective_target_vect_udot_qi,
check_effective_target_vect_sdot_hi,
check_effective_target_vect_udot_hi,
check_effective_target_vect_usad_char,
check_effective_target_vect_pack_trunc,
check_effective_target_vect_unpack,
check_effective_target_unaligned_stack,
check_effective_target_vect_no_ali

Re: [PATCH] Fix build with ISL 0.20

[Jeff Law]

On 9/25/18 1:07 PM, Alexey Neyman wrote:
> Hi,
> 
> A trivial patch that fixes the build against the latest ISL release,
> 0.20. In that release,  and  were split in two
> headers each. The  (included from  which is
> included by "graphite.h") now includes  and
> ;  and  must be included explicitly.
> 
> These headers ( and ) are present in all
> supported versions of ISL (0.15 and later).
> 
> Bootstrapped on x86_64-pc-linux-gnu.
Thanks for verifying these are in ISL 0.15 and later -- we recommend
0.18 these days, so I think this is fine.

Installed on the trunk.

jeff

Re: [PATCH, AArch64 08/11] aarch64: Add out-of-line functions for LSE atomics

[Richard Henderson]

On 9/26/18 1:59 AM, Florian Weimer wrote:
> * rth:
> 
>> diff --git a/libgcc/config/aarch64/lse.c b/libgcc/config/aarch64/lse.c
>> new file mode 100644
>> index 000..20f4bde741f
>> --- /dev/null
>> +++ b/libgcc/config/aarch64/lse.c
> 
>> +static void __attribute__((constructor))
>> +init_have_atomics(void)
>> +{
>> +  unsigned long hwcap = getauxval(AT_HWCAP);
>> +  __aa64_have_atomics = (hwcap & HWCAP_ATOMICS) != 0;
>> +}
> 
> Is there an expectation that it is possible to use the atomics in IFUNC
> resolvers?  Then this needs an explanation why it is safe to run with
> the other kind of atomics until the initialization of
> __aa64_have_atomics has happened.

Yes.  The explanation is simple, in that the !have_atomics path is also atomic.
 It will simply use the slower load/store-exclusive path.

Perhaps, despite the official ARMv8.1-Atomics name, LSE was in fact a better
choice for a name after all, as its lack does not imply a lack of atomicity.
And a comment, to be sure.

> (GNU style requires a space before a parenthesis, at least in a function
> call or function declarator.)

Yes, of course.  It's no longer automatic for my fingers and eyes.


r~

Re: [PATCH, AArch64 08/11] aarch64: Add out-of-line functions for LSE atomics

[Florian Weimer]

* Richard Henderson:

> On 9/26/18 1:59 AM, Florian Weimer wrote:
>> * rth:
>> 
>>> diff --git a/libgcc/config/aarch64/lse.c b/libgcc/config/aarch64/lse.c
>>> new file mode 100644
>>> index 000..20f4bde741f
>>> --- /dev/null
>>> +++ b/libgcc/config/aarch64/lse.c
>> 
>>> +static void __attribute__((constructor))
>>> +init_have_atomics(void)
>>> +{
>>> +  unsigned long hwcap = getauxval(AT_HWCAP);
>>> +  __aa64_have_atomics = (hwcap & HWCAP_ATOMICS) != 0;
>>> +}
>> 
>> Is there an expectation that it is possible to use the atomics in IFUNC
>> resolvers?  Then this needs an explanation why it is safe to run with
>> the other kind of atomics until the initialization of
>> __aa64_have_atomics has happened.
>
> Yes.  The explanation is simple, in that the !have_atomics path is
> also atomic.  It will simply use the slower load/store-exclusive path.
>
> Perhaps, despite the official ARMv8.1-Atomics name, LSE was in fact a
> better choice for a name after all, as its lack does not imply a lack
> of atomicity.  And a comment, to be sure.

That's not what I meant.  I'm curious if LSE and non-LSE atomics on the
same location will still result in the expected memory ordering.  If
they don't, then this requires *some* explanation why this is okay.

Thanks,
Florian

Re: [PATCH, AArch64 08/11] aarch64: Add out-of-line functions for LSE atomics

[Richard Henderson]

On 9/26/18 7:33 AM, Florian Weimer wrote:
>>> *That's not what I meant.  I'm curious if LSE and non-LSE atomics on the
>>> same location will still result in the expected memory ordering.  If
>>> they don't, then this requires *some* explanation why this is okay.
>>>
>>> Thanks,
>>> Florian

Yes, they interoperate just fine.


r~

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Qing Zhao]

Alexander,

thanks for the questions.

Yes, we had some discussion on the questions you raised during the review of 
the initial patch back to 9/11/2018.
please take a look at those discussions at:

https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00549.html 

https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00787.html 


and let me know if those discussion still does not answer your questions.

Qing

> On Sep 26, 2018, at 6:12 AM, Alexander Monakov  wrote:
> 
> On Fri, 21 Sep 2018, Qing Zhao wrote:
>> +2018-09-20  Qing Zhao  
>> +
>> +* cif-code.def (FUNCTION_EXTERN): New CIFCODE.
>> +* common.opt (-finline-only-static): New option.
>> +* doc/invoke.texi: Document -finline-only-static.
>> +* ipa-inline.c (can_inline_edge_p): Control inlining based on
>> +function's linkage. 
> 
> Note, I am not a reviewer.
> 
> In my opinion, there's a problem with the patch that it looks like an ad-hoc,
> incomplete solution. You said you need this change to help with building
> livepatching-capable kernels, but it's not clear what exactly the issue with
> inlining non-static functions is. Can you describe how the workflow looks like
> so code duplication due to inlining static functions is not an issue, but
> inlining non-static functions is a problem? Does using existing
> -fno-inline-functions flag achieve something useful for your usecase?
> 
> Please always make it clear what problem the patch is intended to solve and 
> help
> reviewers see the connection between the problem and your solution. Look how 
> the
> "XY problem" effect applies partially in this situation.
> 
> https://en.wikipedia.org/wiki/XY_problem
> http://xyproblem.info/
> 
> Alexander

Re: [PATCH] Fix build with ISL 0.20

[Richard Biener]

On Wed, Sep 26, 2018 at 4:10 PM Jeff Law  wrote:
>
> On 9/25/18 1:07 PM, Alexey Neyman wrote:
> > Hi,
> >
> > A trivial patch that fixes the build against the latest ISL release,
> > 0.20. In that release,  and  were split in two
> > headers each. The  (included from  which is
> > included by "graphite.h") now includes  and
> > ;  and  must be included explicitly.
> >
> > These headers ( and ) are present in all
> > supported versions of ISL (0.15 and later).
> >
> > Bootstrapped on x86_64-pc-linux-gnu.
> Thanks for verifying these are in ISL 0.15 and later -- we recommend
> 0.18 these days, so I think this is fine.
>
> Installed on the trunk

I think this was fixed already in August:

2018-08-01  Richard Biener  

PR bootstrap/86724
* graphite.h: Include isl/id.h and isl/space.h to allow build
with ISL 0.20.

and also backported to branches.

Richard.

>
> jeff

[Patch, Aarch64] Testsuite patch to fix one of the regressions in PR 87433, gcc.target/aarch64/ashltidisi.c

[Steve Ellcey]

The patch for PR rtl-optimization/85160 which allowed combine to convert
two instructions into two different instructions if they had a lower cost
caused a couple of regressions on aarch64.  This patch fixes one of them.

After the above patch, the gcc.target/aarch64/ashltidisi.c test generated
3 asr instructions instead of 4.  Given that the overall test now has
two fewer instructions and appears to be superior to the original generated
code, this patch just updates the test to reflect the newly generated code.

Tested on aarch64, OK for checkin?

Steve Ellcey
sell...@cavium.com


2018-09-26  Steve Ellcey  

* gcc.target/aarch64/ashltidisi.c: Expect 3 asr instructions
instead of 4.


diff --git a/gcc/testsuite/gcc.target/aarch64/ashltidisi.c 
b/gcc/testsuite/gcc.target/aarch64/ashltidisi.c
index 293a0f2..e2a0997 100644
--- a/gcc/testsuite/gcc.target/aarch64/ashltidisi.c
+++ b/gcc/testsuite/gcc.target/aarch64/ashltidisi.c
@@ -45,5 +45,5 @@ main (int argc, char **argv)
   return 0;
 }
 
-/* { dg-final { scan-assembler-times "asr" 4 } } */
+/* { dg-final { scan-assembler-times "asr" 3 } } */
 /* { dg-final { scan-assembler-not "extr\t" } } */

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jeff Law]

On 9/26/18 7:38 AM, Jason Merrill wrote:
> On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
>> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
>> what 'internal' would mean in this context.
> 
> I mean internal linkage as in the C and C++ standards.
Since this is primarily for kernel hot patching, I think we're looking
to restrict inlining to functions that have visibility limited to a
compilation unit.

Qing, can you confirm that either way?

Jeff

Re: [PATCH] Fix build with ISL 0.20

[Jeff Law]

On 9/26/18 8:43 AM, Richard Biener wrote:
> On Wed, Sep 26, 2018 at 4:10 PM Jeff Law  wrote:
>>
>> On 9/25/18 1:07 PM, Alexey Neyman wrote:
>>> Hi,
>>>
>>> A trivial patch that fixes the build against the latest ISL release,
>>> 0.20. In that release,  and  were split in two
>>> headers each. The  (included from  which is
>>> included by "graphite.h") now includes  and
>>> ;  and  must be included explicitly.
>>>
>>> These headers ( and ) are present in all
>>> supported versions of ISL (0.15 and later).
>>>
>>> Bootstrapped on x86_64-pc-linux-gnu.
>> Thanks for verifying these are in ISL 0.15 and later -- we recommend
>> 0.18 these days, so I think this is fine.
>>
>> Installed on the trunk
> 
> I think this was fixed already in August:
> 
> 2018-08-01  Richard Biener  
> 
> PR bootstrap/86724
> * graphite.h: Include isl/id.h and isl/space.h to allow build
> with ISL 0.20.
> 
> and also backported to branches.
You're right!  I'll revert.

jeff

Re: [PATCH] Fix PR87440, extra lexical block in inline instances

[Jason Merrill]

On Wed, Sep 26, 2018 at 9:35 AM, Richard Biener  wrote:
>
> We do not create a DW_AT_lexical_block for the outermost block in
> functions but we do for DW_AT_inlined_subroutines.  That makes
> debuginfo look like if there were two of each local, the outer
> one (from the abstract instance) optimized out (visible via
> info locals in gdb).
>
> The following elides the outermost block also from inline instances.
> It's a bit tricky to reliably track that block given we remove unused
> blocks here and there.  The trick is to have the block in the abstract
> instance _not_ point to itself (given we do not output it it isn't
> the abstract origin for itself).
>
> Bootstrapped on x86_64-unkown-linux-gnu, testing in progress.
>
> Again with some scan-assembler testcase, guality cannot do 'info locals'.
>
> OK?
>
> Thanks,
> Richard.
>
> 2018-09-26  Richard Biener  
>
> PR debug/87440
> * dwarf2out.c (set_block_origin_self): Do not mark outermost
> block as we do not output that.
> (gen_inlined_subroutine_die): Elide the originally outermost
> block, matching what we do for concrete instances.
> (decls_for_scope): Add parameter specifying whether to recurse
> to subblocks.
>
> * gcc.dg/debug/dwarf2/inline4.c: New testcase.
>
> Index: gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c
> ===
> --- gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/debug/dwarf2/inline4.c (working copy)
> @@ -0,0 +1,17 @@
> +/* Verify that the inline instance has no extra DW_TAG_lexical_block between
> +   the DW_TAG_inlined_subroutine and the DW_TAG_variable for the local.  */
> +/* { dg-options "-O -gdwarf -dA" } */
> +/* { dg-do compile } */
> +/* { dg-final { scan-assembler 
> "DW_TAG_inlined_subroutine\[^\\(\]*\\(\[^\\)\]*\\)\[^\\(\]*\\(DIE 
> \\(0x\[0-9a-f\]*\\) DW_TAG_formal_parameter\[^\\(\]*\\(DIE 
> \\(0x\[0-9a-f\]*\\) DW_TAG_variable" } } */
> +/* { dg-final { scan-assembler-times "DW_TAG_inlined_subroutine" 2 } } */
> +
> +static int foo (int i)
> +{
> +  volatile int j = i + 3;
> +  return j - 2;
> +}
> +int main()
> +{
> +  volatile int z = foo (-1);
> +  return z;
> +}
> Index: gcc/dwarf2out.c
> ===
> --- gcc/dwarf2out.c (revision 264640)
> +++ gcc/dwarf2out.c (working copy)
> @@ -3867,7 +3867,7 @@ static void gen_subroutine_type_die (tre
>  static void gen_typedef_die (tree, dw_die_ref);
>  static void gen_type_die (tree, dw_die_ref);
>  static void gen_block_die (tree, dw_die_ref);
> -static void decls_for_scope (tree, dw_die_ref);
> +static void decls_for_scope (tree, dw_die_ref, bool = true);
>  static bool is_naming_typedef_decl (const_tree);
>  static inline dw_die_ref get_context_die (tree);
>  static void gen_namespace_die (tree, dw_die_ref);
> @@ -22389,7 +22389,13 @@ set_block_origin_self (tree stmt)
>  {
>if (BLOCK_ABSTRACT_ORIGIN (stmt) == NULL_TREE)
>  {
> -  BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
> +  /* We do not mark the outermost block as we are not outputting it.
> +This is then a reliable way of determining whether we should
> +do the same for an inline instance.  */
> +  if (TREE_CODE (BLOCK_SUPERCONTEXT (stmt)) != FUNCTION_DECL)
> +   BLOCK_ABSTRACT_ORIGIN (stmt) = stmt;
> +  else
> +   gcc_assert (DECL_INITIAL (BLOCK_SUPERCONTEXT (stmt)) == stmt);
>
>{
> tree local_decl;
> @@ -24149,7 +24155,24 @@ gen_inlined_subroutine_die (tree stmt, d
>  add_high_low_attributes (stmt, subr_die);
>add_call_src_coords_attributes (stmt, subr_die);
>
> -  decls_for_scope (stmt, subr_die);
> +  /* The inliner creates an extra BLOCK for the parameter setup,
> + we want to merge that with the actual outermost BLOCK of the
> +inlined function to avoid duplicate locals in consumers.  Note
> +we specially mark that not as origin-self.
> +Do that by doing the recursion to subblocks on the single subblock
> +of STMT.  */
> +  bool unwrap_one = false;
> +  if (BLOCK_SUBBLOCKS (stmt) && !BLOCK_CHAIN (BLOCK_SUBBLOCKS (stmt)))
> +   {
> + tree origin = block_ultimate_origin (BLOCK_SUBBLOCKS (stmt));
> + if (origin
> + && TREE_CODE (origin) == BLOCK
> + && !BLOCK_ABSTRACT_ORIGIN (origin))

Can we look at BLOCK_SUPERCONTEXT here rather than above?

Jason

Re: libgo patch committed: Update to 1.11 release

[H.J. Lu]

On Mon, Sep 24, 2018 at 2:46 PM, Ian Lance Taylor  wrote:
> I've committed a patch to update libgo to the 1.11 release.  As usual
> for these updates, the patch is too large to attach to this e-mail
> message.  I've attached some of the more relevant directories.  This
> update required some minor patches to the gotools directory and the Go
> testsuite, also included here.  Bootstrapped and ran Go testsuite on
> x86_64-pc-linux-gnu.  Committed to mainline.
>
> Ian
>
> 2018-09-24  Ian Lance Taylor  
>
> * Makefile.am (mostlyclean-local): Run chmod on check-go-dir to
> make sure it is writable.
> (check-go-tools): Likewise.
> (check-vet): Copy internal/objabi to check-vet-dir.
> * Makefile.in: Rebuild.

When building with -mx32, I got

/export/gnu/import/git/sources/gcc/libgo/go/runtime/malloc.go:309:44:
error: integer constant overflow
309 |  arenaBaseOffset uintptr = sys.GoarchAmd64 * (1 << 47)
|^

-- 
H.J.

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jason Merrill]

On Wed, Sep 26, 2018 at 10:45 AM, Jeff Law  wrote:
> On 9/26/18 7:38 AM, Jason Merrill wrote:
>> On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
>>> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
>>> what 'internal' would mean in this context.
>>
>> I mean internal linkage as in the C and C++ standards.

> Since this is primarily for kernel hot patching, I think we're looking
> to restrict inlining to functions that have visibility limited to a
> compilation unit.

Right, which is internal linkage.

C11: "Within one translation unit, each declaration of an identifier
with internal linkage denotes the same object or function."
C++17: "When a name has internal linkage, the entity it denotes can be
referred to by names from other scopes in the same translation unit."

Or perhaps we want to say "not external linkage", i.e. !TREE_PUBLIC as
richi suggested.

Jason

Re: [PR 87339, testsuite] Fix failure of gcc.dg/warn-abs-1.c on some targets

[Christophe Lyon]

On Tue, 25 Sep 2018 at 17:50, Martin Jambor  wrote:
>
> Hi,
>
> On Mon, Sep 24 2018, Christophe Lyon wrote:
> > On Mon, 24 Sep 2018 at 20:46, Martin Jambor  wrote:
> >>
> >> Hi,
> >>
> >> the test added to check whether _Float128 types are handled correctly by
> >> the new warning about suspicious calls to abs-like functions fails on
> >> many platforms.  The patch below circumvents the problem by running on
> >> i686/x86_64 only.  I understand that the proper solution would be to
> >> come up with a deja-gnu predicate and skip-if it was not provided but I
> >> think that for one simple test that is a bit of an overkill and testing
> >> it on x86_64 will provide all the test coverage we need.
> >>
> >> Tested on x86_64-linux and aarch64-linux, testing on i686-linux
> >> pending.  OK for trunk?
> >>
> >> Thanks,
> >>
> >> Martin
> >>
> >>
> >>
> >> 2018-09-24  Martin Jambor  
> >>
> >> PR testsuite/87339
> >> * gcc.dg/warn-abs-1.c: Do not test _Float128.  Remove dg-skip-if.
> >> * gcc.target/i386/warn-abs-3.c: New test.
> >> ---
> >>  gcc/testsuite/gcc.dg/warn-abs-1.c  |  4 +---
> >>  gcc/testsuite/gcc.target/i386/warn-abs-3.c | 12 
> >>  2 files changed, 13 insertions(+), 3 deletions(-)
> >>  create mode 100644 gcc/testsuite/gcc.target/i386/warn-abs-3.c
> >>
> >> diff --git a/gcc/testsuite/gcc.dg/warn-abs-1.c 
> >> b/gcc/testsuite/gcc.dg/warn-abs-1.c
> >> index 129a3af8ac6..b494b14f4a9 100644
> >> --- a/gcc/testsuite/gcc.dg/warn-abs-1.c
> >> +++ b/gcc/testsuite/gcc.dg/warn-abs-1.c
> >> @@ -1,5 +1,4 @@
> >>  /* { dg-do compile { target float128 } } */
> >
> > Don't you want to remove the {target float128} part?
>
> I did notice it but yes, I do want to remove it.  I am therefore going
> to commit the following after re-testing on x86_64-linux and
> aarch64-linux.
>
> Thank you,
>

Hi, thanks for this fix.
Now the test runs on ARM, but fails because of two missing warnings:
gcc.dg/warn-abs-1.c  (test for warnings, line 46)
gcc.dg/warn-abs-1.c  (test for warnings, line 60)
which correspond to:
*pld = fabs (*pld);  /* { dg-warning "may cause truncation of value" } */
*pcld = cabs (*pcld);  /* { dg-warning "may cause truncation of value" } */

Christophe

> Martin
>
>
> 2018-09-25  Martin Jambor  
>
> PR testsuite/87339
> * gcc.dg/warn-abs-1.c: Do not test _Float128.  Remove dg-skip-if and
> float125 target.
> * gcc.target/i386/warn-abs-3.c: New test.
> ---
>  gcc/testsuite/gcc.dg/warn-abs-1.c  |  6 ++
>  gcc/testsuite/gcc.target/i386/warn-abs-3.c | 12 
>  2 files changed, 14 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/warn-abs-3.c
>
> diff --git a/gcc/testsuite/gcc.dg/warn-abs-1.c 
> b/gcc/testsuite/gcc.dg/warn-abs-1.c
> index 129a3af8ac6..1c487270042 100644
> --- a/gcc/testsuite/gcc.dg/warn-abs-1.c
> +++ b/gcc/testsuite/gcc.dg/warn-abs-1.c
> @@ -1,5 +1,4 @@
> -/* { dg-do compile { target float128 } } */
> -/* { dg-skip-if "incomplete long double support" { { newlib } && 
> large_long_double } }  */
> +/* { dg-do compile } */
>  /* { dg-options "-Wabsolute-value" } */
>
>  #include 
> @@ -41,12 +40,11 @@ tst_notfloat (int *pi, long *pl, complex double *pc)
>  }
>
>  void
> -tst_float_size (double *pd, long double *pld, _Float128 *pf128)
> +tst_float_size (double *pd, long double *pld)
>  {
>*pd = fabsf (*pd);   /* { dg-warning "may cause truncation of value" } */
>*pld = fabs (*pld);  /* { dg-warning "may cause truncation of value" } */
>*pld = fabs ((double) *pld);
> -  *pf128 = fabsl (*pf128); /* { dg-warning "may cause truncation of value" } 
> */
>  }
>
>  void tst_notcomplex (int *pi, long *pl, long double *pld)
> diff --git a/gcc/testsuite/gcc.target/i386/warn-abs-3.c 
> b/gcc/testsuite/gcc.target/i386/warn-abs-3.c
> new file mode 100644
> index 000..21feaff7d8a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/warn-abs-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target float128 } } */
> +/* { dg-options "-Wabsolute-value" } */
> +
> +#include 
> +#include 
> +#include 
> +
> +void
> +tst_float128_size (_Float128 *pf128)
> +{
> +  *pf128 = fabsl (*pf128); /* { dg-warning "may cause truncation of value" } 
> */
> +}
> --
> 2.18.0
>

[Patch, Aarch64] Testsuite fix to fix one of the regressions in PR 87433, gcc.dg/zero_bits_compound-1.c

[Steve Ellcey]

PR rtl-optimization/85160 which allowed combine to convert
two instructions into two different instructions if they had a lower cost
caused a couple of regressions on aarch64.  This patch fixes one of them.

After the above patch, the gcc.dg/zero_bits_compound-1.c test on 
aarch64 generates an and rtl instruction where it did not before.
A comparision of the code generated before and after the 85160 patch
shows that GCC is still generating the same number of instructions
but there are fewer dependencies between some of the instructions
so the new code would be better on systems with multiple functional
units and no worse on systems with a single functional unit.

Since the test is just verifying that an 'and' rtl instruction is
not generated and that is now happening on aarch64 but the new code
is as good as or better than the old code this test doesn't seem
to make sense for aarch64 anymore and I would like to just not
run it on aarch64.

Tested on aarch64.  OK for checkin?

Steve Ellcey
sell...@cavium.com


2018-09-26  Steve Ellcey  

PR testsuite/87433
* gcc.dg/zero_bits_compound-1.c: Do not run on aarch64*-*-*.


diff --git a/gcc/testsuite/gcc.dg/zero_bits_compound-1.c 
b/gcc/testsuite/gcc.dg/zero_bits_compound-1.c
index 63b8ac1..e715949 100644
--- a/gcc/testsuite/gcc.dg/zero_bits_compound-1.c
+++ b/gcc/testsuite/gcc.dg/zero_bits_compound-1.c
@@ -4,7 +4,7 @@
 /* Note: This test requires that char, int and long have different sizes and 
the
target has a way to do 32 -> 64 bit zero extension other than AND.  */
 
-/* { dg-do compile { target i?86-*-* x86_64-*-* s390*-*-* aarch64*-*-* } } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* s390*-*-* } } */
 /* { dg-require-effective-target lp64 } */
 /* { dg-options "-O3 -dP" } */
 

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jan Hubicka]

> On Wed, Sep 26, 2018 at 10:45 AM, Jeff Law  wrote:
> > On 9/26/18 7:38 AM, Jason Merrill wrote:
> >> On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
> >>> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
> >>> what 'internal' would mean in this context.
> >>
> >> I mean internal linkage as in the C and C++ standards.
> 
> > Since this is primarily for kernel hot patching, I think we're looking
> > to restrict inlining to functions that have visibility limited to a
> > compilation unit.
> 
> Right, which is internal linkage.
> 
> C11: "Within one translation unit, each declaration of an identifier
> with internal linkage denotes the same object or function."
> C++17: "When a name has internal linkage, the entity it denotes can be
> referred to by names from other scopes in the same translation unit."
> 
> Or perhaps we want to say "not external linkage", i.e. !TREE_PUBLIC as
> richi suggested.

I am not quite sure if we can relate visibility flags we have at this stage
to visibility in source languge in very coherent way.  They change a lot with
LTO and we may want to make this option incompatible with LTO, but even w/o
we can turn function static that previously wasn't.

For example comdat that was cloned by IPA-SRA. See can_be_local_p and
comdat_can_be_unshared_p predicates.  Similar problem happens to clones created
by ipa-cp.

I guess we want to disable localization and cloning in this case as well.
I wonder what else.

Honza

Re: [C++ Patch] PR 84940 ("[7/8/9 Regression] internal compiler error: in build_value_init_noctor, at cp/init.c:465")

[Jason Merrill]

OK.

On Tue, Sep 25, 2018 at 12:45 PM, Paolo Carlini
 wrote:
> Hi,
>
> in this error-recovery regression we ICE after a sensible diagnostic emitted
> by cp_build_unary_op, called by finish_unary_op_expr via build_x_unary_op.
> In principle we could dig deeper, but I don't think it makes sense for
> finish_unary_op_expr to go on having seen the error_mark_node returned by
> build_x_unary_op given that the purpose of its second half is only issuing
> warnings.
>
> Tested x86_64-linux.
>
> Thanks, Paolo.
>
> //
>
>

Re: libgo patch committed: Update to 1.11 release

[H.J. Lu]

On Wed, Sep 26, 2018 at 7:50 AM, H.J. Lu  wrote:
> On Mon, Sep 24, 2018 at 2:46 PM, Ian Lance Taylor  wrote:
>> I've committed a patch to update libgo to the 1.11 release.  As usual
>> for these updates, the patch is too large to attach to this e-mail
>> message.  I've attached some of the more relevant directories.  This
>> update required some minor patches to the gotools directory and the Go
>> testsuite, also included here.  Bootstrapped and ran Go testsuite on
>> x86_64-pc-linux-gnu.  Committed to mainline.
>>
>> Ian
>>
>> 2018-09-24  Ian Lance Taylor  
>>
>> * Makefile.am (mostlyclean-local): Run chmod on check-go-dir to
>> make sure it is writable.
>> (check-go-tools): Likewise.
>> (check-vet): Copy internal/objabi to check-vet-dir.
>> * Makefile.in: Rebuild.
>
> When building with -mx32, I got
>
> /export/gnu/import/git/sources/gcc/libgo/go/runtime/malloc.go:309:44:
> error: integer constant overflow
> 309 |  arenaBaseOffset uintptr = sys.GoarchAmd64 * (1 << 47)
> |^
>

This seems to work:

diff --git a/libgo/go/runtime/malloc.go b/libgo/go/runtime/malloc.go
index ac4759ffbf1..c3445387057 100644
--- a/libgo/go/runtime/malloc.go
+++ b/libgo/go/runtime/malloc.go
@@ -306,7 +306,7 @@ const (
   //
   // On other platforms, the user address space is contiguous
   // and starts at 0, so no offset is necessary.
-  arenaBaseOffset uintptr = sys.GoarchAmd64 * (1 << 47)
+  arenaBaseOffset uintptr = _64bit * sys.GoarchAmd64 * (1 << 47)

   // Max number of threads to run garbage collection.
   // 2, 3, and 4 are all plausible maximums depending


-- 
H.J.

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Alexander Monakov]

On Wed, 26 Sep 2018, Qing Zhao wrote:

> Alexander,
> 
> thanks for the questions.
> 
> Yes, we had some discussion on the questions you raised during the review of 
> the initial patch back to 9/11/2018.
> please take a look at those discussions at:
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00549.html 
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00787.html 
> 
> 
> and let me know if those discussion still does not answer your questions.

Thank you. Yes, it is still unclear to me why restricting inlining to static
functions noticeably helps in your case. Is it because you build the kernel
with LTO? Otherwise effects from inlining are limited to one compilation unit,
except for functions defined in headers. But for those, the kernel
uses 'static inline' anyway, so the patch wouldn't change anything.

If the original issue is that inlining duplicates code, wouldn't it be better
solved by a switch that instructs inlining heuristics to inline as if for -Os,
without enabling -Os for other passes?

Alexander

Re: libgo patch committed: Update to 1.11 release

[Ian Lance Taylor]

On Wed, Sep 26, 2018 at 12:57 AM, Uros Bizjak  wrote:
>> I've committed a patch to update libgo to the 1.11 release.  As usual
>> for these updates, the patch is too large to attach to this e-mail
>> message.  I've attached some of the more relevant directories.  This
>> update required some minor patches to the gotools directory and the Go
>> testsuite, also included here.  Bootstrapped and ran Go testsuite on
>> x86_64-pc-linux-gnu.  Committed to mainline.
>
> Fails to build on alpha-linux-gnu:
>
> /space/homedirs/uros/gcc-svn/trunk/libgo/go/syscall/setuidgid_linux.go:11:16:
> error: reference to undefined name ‘SYS_GETEUID’
> 11 |  sys_GETEUID = SYS_GETEUID
>|^
>
> This is because /usr/include/asm/unistd.h says:
>
> /*
>  * Ignore legacy syscalls that we don't use.
>  */
> #define __IGNORE_alarm
> #define __IGNORE_creat
> #define __IGNORE_getegid
> #define __IGNORE_geteuid
> #define __IGNORE_getgid
> #define __IGNORE_getpid
> #define __IGNORE_getppid
> #define __IGNORE_getuid
> #define __IGNORE_pause
> #define __IGNORE_time
> #define __IGNORE_utime
> #define __IGNORE_umount2
>
> These legacy syscalls are undefined for alpha-linux-gnu.

Thanks for the report.  This is only used for testing.  I've committed
this patch, which I think should fix the problem.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 264593)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e7b98cf0a380eb45791cd5c52897224a686dcdec
+944784a93cf89d3a238e5607c993ea5f18f99c12
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/syscall/syscall_linux_test.go
===
--- libgo/go/syscall/syscall_linux_test.go  (revision 264546)
+++ libgo/go/syscall/syscall_linux_test.go  (working copy)
@@ -302,6 +302,10 @@ func TestSyscallNoError(t *testing.T) {
t.Skip("skipping root only test")
}
 
+   if syscall.Sys_GETEUID == 0 {
+   t.Skip("skipping because there is no geteuid system call")
+   }
+
// Copy the test binary to a location that a non-root user can 
read/execute
// after we drop privileges
tempDir, err := ioutil.TempDir("", "TestSyscallNoError")
Index: libgo/mksysinfo.sh
===
--- libgo/mksysinfo.sh  (revision 264572)
+++ libgo/mksysinfo.sh  (working copy)
@@ -138,6 +138,12 @@ if ! grep '^const SYS_GETDENTS64 ' ${OUT
   echo "const SYS_GETDENTS64 = 0" >> ${OUT}
 fi
 
+# The syscall package wants the geteuid system call number.  It isn't
+# defined on Alpha, which only provides the getresuid system call.
+if ! grep '^const SYS_GETEUID ' ${OUT} >/dev/null 2>&1; then
+  echo "const SYS_GETEUID = 0" >> ${OUT}
+fi
+
 # Stat constants.
 grep '^const _S_' gen-sysinfo.go | \
   sed -e 's/^\(const \)_\(S_[^= ]*\)\(.*\)$/\1\2 = _\2/' >> ${OUT}

[PATCH, i386]: Do not use "u" constraint and remove FP_TOP_SSE_REGS and FP_SECOND_SSE_REGS register classes

[Uros Bizjak]

Hello!

Stack registers are fixed up by regstack pass, so there is no point to
specify "upper" FP register in the instruction patterns. This change
allows register allocator a bit more freedom, which results in a few
fxch instructions less.

The patch also removes FP_TOP_SSE_REGS and FP_SECOND_SSE_REGS mixed
classes. We don't have any insn patterns that would allow the mix of
"t"/"u" and SSE regs, so the class is neverused.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 264643)
+++ config/i386/i386.c  (working copy)
@@ -39043,10 +39043,6 @@ ix86_preferred_reload_class (rtx x, reg_class_t re
  /* Limit class to FP regs.  */
  if (FLOAT_CLASS_P (regclass))
return FLOAT_REGS;
- else if (regclass == FP_TOP_SSE_REGS)
-   return FP_TOP_REG;
- else if (regclass == FP_SECOND_SSE_REGS)
-   return FP_SECOND_REG;
}
 
   return NO_REGS;
@@ -39092,14 +39088,7 @@ ix86_preferred_output_reload_class (rtx x, reg_cla
 return MAYBE_SSE_CLASS_P (regclass) ? ALL_SSE_REGS : NO_REGS;
 
   if (IS_STACK_MODE (mode))
-{
-  if (regclass == FP_TOP_SSE_REGS)
-   return FP_TOP_REG;
-  else if (regclass == FP_SECOND_SSE_REGS)
-   return FP_SECOND_REG;
-  else
-   return FLOAT_CLASS_P (regclass) ? regclass : NO_REGS;
-}
+return FLOAT_CLASS_P (regclass) ? regclass : NO_REGS;
 
   return regclass;
 }
Index: config/i386/i386.h
===
--- config/i386/i386.h  (revision 264645)
+++ config/i386/i386.h  (working copy)
@@ -1337,8 +1337,6 @@ enum reg_class
   SSE_REGS,
   ALL_SSE_REGS,
   MMX_REGS,
-  FP_TOP_SSE_REGS,
-  FP_SECOND_SSE_REGS,
   FLOAT_SSE_REGS,
   FLOAT_INT_REGS,
   INT_SSE_REGS,
@@ -1398,8 +1396,6 @@ enum reg_class
"SSE_REGS", \
"ALL_SSE_REGS", \
"MMX_REGS", \
-   "FP_TOP_SSE_REGS",  \
-   "FP_SECOND_SSE_REGS",   \
"FLOAT_SSE_REGS",   \
"FLOAT_INT_REGS",   \
"INT_SSE_REGS", \
@@ -1438,8 +1434,6 @@ enum reg_class
 { 0x1fe0,   0x1fe000,0x0 },/* SSE_REGS */  \
 { 0x1fe0, 0xe000,   0x1f },/* ALL_SSE_REGS */  \
 { 0xe000,   0x1f,0x0 },/* MMX_REGS */  \
-{ 0x1fe00100, 0xe000,   0x1f },/* FP_TOP_SSE_REG */\
-{ 0x1fe00200, 0xe000,   0x1f },/* FP_SECOND_SSE_REG */ \
 { 0x1fe0ff00, 0xe000,   0x1f },/* FLOAT_SSE_REGS */\
 {   0x11, 0x1fe0,0x0 },/* FLOAT_INT_REGS */\
 { 0x1ff100ff, 0xffe0,   0x1f },/* INT_SSE_REGS */  \
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 264643)
+++ config/i386/i386.md (working copy)
@@ -4973,7 +4973,7 @@
 (define_insn "fix_trunc_i387_fisttp"
   [(set (match_operand:SWI248x 0 "nonimmediate_operand" "=m")
(fix:SWI248x (match_operand 1 "register_operand" "f")))
-   (clobber (match_scratch:XF 2 "=&1f"))]
+   (clobber (match_scratch:XF 2 "=&f"))]
   "X87_FLOAT_MODE_P (GET_MODE (operands[1]))
&& TARGET_FISTTP
&& !((SSE_FLOAT_MODE_P (GET_MODE (operands[1]))
@@ -5019,7 +5019,7 @@
(fix:DI (match_operand 1 "register_operand" "f")))
(use (match_operand:HI 2 "memory_operand" "m"))
(use (match_operand:HI 3 "memory_operand" "m"))
-   (clobber (match_scratch:XF 4 "=&1f"))]
+   (clobber (match_scratch:XF 4 "=&f"))]
   "X87_FLOAT_MODE_P (GET_MODE (operands[1]))
&& !TARGET_FISTTP
&& !(TARGET_64BIT && SSE_FLOAT_MODE_P (GET_MODE (operands[1])))"
@@ -15178,7 +15178,7 @@
(unspec:XF [(match_operand:XF 2 "register_operand" "0")
(match_operand:XF 3 "register_operand" "1")]
   UNSPEC_FPREM_F))
-   (set (match_operand:XF 1 "register_operand" "=u")
+   (set (match_operand:XF 1 "register_operand" "=f")
(unspec:XF [(match_dup 2) (match_dup 3)]
   UNSPEC_FPREM_U))
(set (reg:CCFP FPSR_REG)
@@ -15253,7 +15253,7 @@
(unspec:XF [(match_operand:XF 2 "register_operand" "0")
(match_operand:XF 3 "register_operand" "1")]
   UNSPEC_FPREM1_F))
-   (set (match_operand:XF 1 "register_operand" "=u")
+   (set (match_operand:XF 1 "register_operand" "=f")
(unspec:XF [(match_dup 2) (match_dup 3)]
   UNSPEC_FPREM1_U))
(set (reg:CCFP FPSR_REG)
@@ -15365,7 +15365,7 @@
   [(set (match_operand:XF 0 "register_operand" "=f")
(unspec:XF [(match_operand:XF 2 "register_operand" "0")]
   UNSPEC_SINCOS_COS))
-   (set (match_operand:XF 1 "register_operand" "=u")
+   (set (match_operand:XF 1 

Re: [PR 87339, testsuite] Fix failure of gcc.dg/warn-abs-1.c on some targets

[Martin Jambor]

Hi,

On Wed, Sep 26 2018, Christophe Lyon wrote:
> On Tue, 25 Sep 2018 at 17:50, Martin Jambor  wrote:
>>
>> Hi,
>>
>> On Mon, Sep 24 2018, Christophe Lyon wrote:
>> > On Mon, 24 Sep 2018 at 20:46, Martin Jambor  wrote:
>> >>
>> >> Hi,
>> >>
>> >> the test added to check whether _Float128 types are handled correctly by
>> >> the new warning about suspicious calls to abs-like functions fails on
>> >> many platforms.  The patch below circumvents the problem by running on
>> >> i686/x86_64 only.  I understand that the proper solution would be to
>> >> come up with a deja-gnu predicate and skip-if it was not provided but I
>> >> think that for one simple test that is a bit of an overkill and testing
>> >> it on x86_64 will provide all the test coverage we need.
>> >>
>> >> Tested on x86_64-linux and aarch64-linux, testing on i686-linux
>> >> pending.  OK for trunk?
>> >>
>> >> Thanks,
>> >>
>> >> Martin
>> >>
>> >>
>> >>
>> >> 2018-09-24  Martin Jambor  
>> >>
>> >> PR testsuite/87339
>> >> * gcc.dg/warn-abs-1.c: Do not test _Float128.  Remove dg-skip-if.
>> >> * gcc.target/i386/warn-abs-3.c: New test.
>> >> ---
>> >>  gcc/testsuite/gcc.dg/warn-abs-1.c  |  4 +---
>> >>  gcc/testsuite/gcc.target/i386/warn-abs-3.c | 12 
>> >>  2 files changed, 13 insertions(+), 3 deletions(-)
>> >>  create mode 100644 gcc/testsuite/gcc.target/i386/warn-abs-3.c
>> >>
>> >> diff --git a/gcc/testsuite/gcc.dg/warn-abs-1.c 
>> >> b/gcc/testsuite/gcc.dg/warn-abs-1.c
>> >> index 129a3af8ac6..b494b14f4a9 100644
>> >> --- a/gcc/testsuite/gcc.dg/warn-abs-1.c
>> >> +++ b/gcc/testsuite/gcc.dg/warn-abs-1.c
>> >> @@ -1,5 +1,4 @@
>> >>  /* { dg-do compile { target float128 } } */
>> >
>> > Don't you want to remove the {target float128} part?
>>
>> I did notice it but yes, I do want to remove it.  I am therefore going
>> to commit the following after re-testing on x86_64-linux and
>> aarch64-linux.
>>
>> Thank you,
>>
>
> Hi, thanks for this fix.
> Now the test runs on ARM, but fails because of two missing warnings:
> gcc.dg/warn-abs-1.c  (test for warnings, line 46)
> gcc.dg/warn-abs-1.c  (test for warnings, line 60)
> which correspond to:
> *pld = fabs (*pld);  /* { dg-warning "may cause truncation of value" } */
> *pcld = cabs (*pcld);  /* { dg-warning "may cause truncation of value" } */
>

I see, I guess the easiest is to skip the test on targets that do not
really have long double, although if someone thinks that is too
restrictive, I can also split the test again and move long double bits
to a separate test.

Can you please test whether the following helps?

Thanks and sorry for the disruptions,

Martin



diff --git a/gcc/testsuite/gcc.dg/warn-abs-1.c 
b/gcc/testsuite/gcc.dg/warn-abs-1.c
index 1c487270042..88389ec6be8 100644
--- a/gcc/testsuite/gcc.dg/warn-abs-1.c
+++ b/gcc/testsuite/gcc.dg/warn-abs-1.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-require-effective-target large_long_double } */
 /* { dg-options "-Wabsolute-value" } */
 
 #include 

[PATCH, i386]: Use fnstcw instead of fstcw to set x87 precision in crtprec.c

[Uros Bizjak]

Hello!

There is no point to handle exceptions at the program startup time.

2018-09-26  Uros Bizjak  

* config/i386/crtprec.c (set_precision): Use fnstcw instead of fstcw.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/crtprec.c
===
--- config/i386/crtprec.c   (revision 264643)
+++ config/i386/crtprec.c   (working copy)
@@ -39,7 +39,7 @@ set_precision (void)
 {
   unsigned short int cwd;
 
-  asm volatile ("fstcw\t%0" : "=m" (cwd));
+  asm volatile ("fnstcw\t%0" : "=m" (cwd));
 
   cwd &= ~X87CW_PCMASK;
   cwd |= X87CW;

Re: [PATCH] Fix build with ISL 0.20

[Alexey Neyman]

On 09/26/2018 07:46 AM, Jeff Law wrote:

On 9/26/18 8:43 AM, Richard Biener wrote:

On Wed, Sep 26, 2018 at 4:10 PM Jeff Law  wrote:

On 9/25/18 1:07 PM, Alexey Neyman wrote:

Hi,

A trivial patch that fixes the build against the latest ISL release,
0.20. In that release,  and  were split in two
headers each. The  (included from  which is
included by "graphite.h") now includes  and
;  and  must be included explicitly.

These headers ( and ) are present in all
supported versions of ISL (0.15 and later).

Bootstrapped on x86_64-pc-linux-gnu.

Thanks for verifying these are in ISL 0.15 and later -- we recommend
0.18 these days, so I think this is fine.

Installed on the trunk

I think this was fixed already in August:

2018-08-01  Richard Biener  

 PR bootstrap/86724
 * graphite.h: Include isl/id.h and isl/space.h to allow build
 with ISL 0.20.

and also backported to branches.

You're right!  I'll revert.


Indeed, sorry for the noise. I noticed that the currently released 
versions failed to build, grepped for  in trunk - which 
yielded nothing due to a typo. So I just applied the same patch and 
verified it built.


Sorry again,
Alexey.

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Qing Zhao]

> On Sep 26, 2018, at 8:24 AM, Richard Biener  wrote:
> 
> On Wed, 26 Sep 2018, Jason Merrill wrote:
> 
>> On Fri, Sep 21, 2018 at 11:12 AM, Qing Zhao  wrote:
>>> Hi, this is the 4th version of the patch.
>>> 
>>> mainly address Martin’s comments on some spelling issues.
>>> 
>>> I have tested the patch on both x86 and aarch64, no issue.
>>> 
>>> Okay for commit?
>>> 
>>> thanks.
>>> 
>>> Qing.
>>> 
>>> gcc/ChangeLog
>>> 
>>> +2018-09-20  Qing Zhao  
>>> +
>>> +   * cif-code.def (FUNCTION_EXTERN): New CIFCODE.
>>> +   * common.opt (-finline-only-static): New option.
>>> +   * doc/invoke.texi: Document -finline-only-static.
>>> +   * ipa-inline.c (can_inline_edge_p): Control inlining based on
>>> +   function's linkage.
>> 
>> I would suggest "internal" rather than "static" in general.  So
>> 
>> +N_("function has external linkage when the user requests only"
>> +   " inlining functions with internal linkage"))
>> 
>> +Inline functions only if they have internal linkage.
>> 
>> +@item -finline-only-internal
>> +@opindex finline-only-internal
>> +By default, GCC inlines functions without considering their linkage.
>> +This flag guides the inliner to only inline functions with internal linkage.
>> +This option has any effect only when inlining itself is turned on by the
>> +-finline-functions or -finline-small-functions options.
>> 
>> This should also mention whether it applies to functions explicitly
>> declared inline; I assume it does not.
> 
> IIRC he explicitely wanted 'static' not 'hidden' linkage.

Yes.  that’s the intention. It will be very helpful to compile the application 
with ONLY inlining
STATIC functions for online-patching purpose. 

>  Not sure
> what 'internal' would mean in this context.
> 
> But then the implementation looks at callee->externally_visible which
> matches hidden visibility... externally_visible is probably not
> the very best thing to look at depending on what we intend to do.

from the comments of callee->externally_visible in cgraph.h:

  /* Set when function is visible by other units.  */
  unsigned externally_visible : 1;

My understand of this “externally_visible” is:

this function is visible from other compilation units.

Is this correct?

> 
> What about 'static' functions with their address taken?
> 

such functions should still be inlined if -finline-only-static is specified. 

is there any issue with this?

> Note you shouldn't look at individual cgraph_node fields but use
> some of the accessors with more well-constrained semantics.
> Why are you not simply checking !TREE_PUBLIC?

Yes, looks like that TREE_PUBLIC(node->decl) might be better for this purpose.

> 
> Honza might be able to suggest one.

thanks.

Qing
> 
> Richard.

Re: [PATCH 07/25] [pr82089] Don't sign-extend SFV 1 in BImode

[Andrew Stubbs]

On 17/09/18 09:40, Richard Sandiford wrote:

 writes:

This is an update of the patch posted to PR82089 long ago.  We ran into the
same bug on GCN, so we need this fixed as part of this series.

2018-09-05  Andrew Stubbs  
 Tom de Vries  

PR82089

gcc/
* expmed.c (emit_cstore): Fix handling of result_mode == BImode and
STORE_FLAG_VALUE == 1.
---
  gcc/expmed.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gcc/expmed.c b/gcc/expmed.c
index 29ce10b..0b87fdc 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -5464,11 +5464,18 @@ emit_cstore (rtx target, enum insn_code icode, enum 
rtx_code code,
   If STORE_FLAG_VALUE does not have the sign bit set when
   interpreted in MODE, we can do this conversion as unsigned, which
   is usually more efficient.  */
-  if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode))
+  if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode)
+  || (result_mode == BImode && int_target_mode != BImode))


Would be better to test GET_MODE_PRECISION instead of GET_MODE_SIZE,
if that works, instead of treating BImode as a special case.


  {
-  convert_move (target, subtarget,
-   val_signbit_known_clear_p (result_mode,
-  STORE_FLAG_VALUE));
+  gcc_assert (GET_MODE_SIZE (result_mode) != 1
+ || STORE_FLAG_VALUE == 1 || STORE_FLAG_VALUE == -1);
+  bool unsignedp
+   = (GET_MODE_SIZE (result_mode) == 1
+  ? STORE_FLAG_VALUE == 1
+  : val_signbit_known_clear_p (result_mode, STORE_FLAG_VALUE));
+
+  convert_move (target, subtarget, unsignedp);
+


GET_MODE_SIZE == 1 would also trigger for QImode, which shouldn't be treated
differently from HImode etc.

The original val_signbit_known_clear_p test seems like it might be an
abstraction too far.  In practice STORE_FLAG_VALUE has to fit within
the mode of a natural (unextended) condition result, so I think we can
simply test STORE_FLAG_VALUE >= 0 for all modes to see whether the target
wants the result to be treated as signed or unsigned.


How about the attached?

I think I addressed all your comments, and it tests fine on GCN with no 
regressions.


Andrew
[pr82089] Don't sign-extend SFV 1 in BImode

This is an update of the patch posted to PR82089 long ago.  We ran into the
same bug on GCN, so we need this fixed as part of this series.

2018-09-26  Andrew Stubbs  
Tom de Vries  

	PR82089

	gcc/
	* expmed.c (emit_cstore): Fix handling of result_mode == BImode and
	STORE_FLAG_VALUE == 1.

diff --git a/gcc/expmed.c b/gcc/expmed.c
index 29ce10b..444d6a8 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -5464,11 +5464,14 @@ emit_cstore (rtx target, enum insn_code icode, enum rtx_code code,
  If STORE_FLAG_VALUE does not have the sign bit set when
  interpreted in MODE, we can do this conversion as unsigned, which
  is usually more efficient.  */
-  if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode))
+  if (GET_MODE_PRECISION (int_target_mode) > GET_MODE_PRECISION (result_mode))
 {
-  convert_move (target, subtarget,
-		val_signbit_known_clear_p (result_mode,
-	   STORE_FLAG_VALUE));
+  gcc_assert (GET_MODE_PRECISION (result_mode) != 1
+		  || STORE_FLAG_VALUE == 1 || STORE_FLAG_VALUE == -1);
+
+  bool unsignedp = (STORE_FLAG_VALUE >= 0);
+  convert_move (target, subtarget, unsignedp);
+
   op0 = target;
   result_mode = int_target_mode;
 }

Re: [Patch, fortran] PRs 70752 and 72709 - more deferred character length bugs

[Dominique d'Humières]

> Is se->string_length guaranteed to be of type gfc_array_index_type_here?
> If so, why? And if not, maybe a fold_convert is in order?

I don’t know if this related, but if I build gfortran with the patches for PRs 
70752 and 72709, 70149, and 65677 with --enable-checking=yes, compiling the 
test in pr82617 gives an ICE:

pr82617.f90:68:0:

68 |   items(i_item) = str(i0:i1-1)
   | 
internal compiler error: tree check: expected tree that contains 'decl minimal' 
structure, have 'indirect_ref' in get_array_span, at fortran/trans.c:301

Cheers,

Dominique

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Qing Zhao]

> On Sep 26, 2018, at 9:45 AM, Jeff Law  wrote:
> 
> On 9/26/18 7:38 AM, Jason Merrill wrote:
>> On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
>>> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
>>> what 'internal' would mean in this context.
>> 
>> I mean internal linkage as in the C and C++ standards.
> Since this is primarily for kernel hot patching, I think we're looking
> to restrict inlining to functions that have visibility limited to a
> compilation unit.

Yes. that’s the intention.

-finline-only-static will ONLY inline functions that are visible within the 
current compilation unit.

Qing
> 
> Qing, can you confirm that either way?
> 
> Jeff

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jan Hubicka]

> >  Not sure
> > what 'internal' would mean in this context.
> > 
> > But then the implementation looks at callee->externally_visible which
> > matches hidden visibility... externally_visible is probably not
> > the very best thing to look at depending on what we intend to do.
> 
> from the comments of callee->externally_visible in cgraph.h:
> 
>   /* Set when function is visible by other units.  */
>   unsigned externally_visible : 1;
> 
> My understand of this “externally_visible” is:
> 
> this function is visible from other compilation units.
> 
> Is this correct?

Yes, but the catch is that we may "localize" previously visible function into
invisible by clonning, see my previous email.
> > Note you shouldn't look at individual cgraph_node fields but use
> > some of the accessors with more well-constrained semantics.
> > Why are you not simply checking !TREE_PUBLIC?
> 
> Yes, looks like that TREE_PUBLIC(node->decl) might be better for this purpose.

externally_visible is better approximation, TREE_PUBLIC is false i.e. for
weakref aliases.  But we need to check places where externally visible functions
are turned to local.

What about comdats in general? What is the intended behaviour? If you prevent
inlining them completely, C++ programs will be very slow.
Also we still can analyze the body and derive some facts about them to drive
optimization (such as discovering that they are const/pure etc). Do we want
to disable this too?

Honza

Re: [patch] Fix AArch64 ILP ICE

[Andrew Stubbs]

On 26/09/18 14:38, Richard Biener wrote:

OK.


Committed, thanks.

Andrew

[PATCH] PR libstdc++/59439 optimize uses of classic ("C") std::locale

[Jonathan Wakely]

The global locale::_Impl that represents the "C" locale is never
destroyed, so there is no need to keep track of reference count updates
for that object. This greatly reduce contention between threads that
refer to the classic locale. Since the global std::locale initially uses
the classic locale, this benefits the common case for any code using the
global locale, such as construction/destruction of iostream objects.

All these updates are done inside libstdc++.so so there's no need to
worry about users' objects having inlined old versions of the code which
still update the reference count for the classic locale.

PR libstdc++/59439
   * src/c++98/locale.cc (locale::locale(const locale&)): Bypass
   reference count updates for the classic locale.
   (locale::~locale()): Likewise.  (locale::operator=(const
   locale&)): Likewise.
   * src/c++98/locale_init.cc (locale::locale()): Likewise.
   (locale::global(const locale&)): Likewise.

Tested x86_64-linux, not committed yet.

Does anybody see any problems with this change?

commit 131d4c26876a5a884fe4408deaf054e01ba90ffb
Author: Jonathan Wakely 
Date:   Wed Sep 26 16:34:42 2018 +0100

PR libstdc++/59439 optimize uses of classic ("C") std::locale

The global locale::_Impl that represents the "C" locale is never
destroyed, so there is no need to keep track of reference count updates
for that object. This greatly reduce contention between threads that
refer to the classic locale. Since the global std::locale initially uses
the classic locale, this benefits the common case for any code using the
global locale, such as construction/destruction of iostream objects.

All these updates are done inside libstdc++.so so there's no need to
worry about users' objects having inlined old versions of the code which
still update the reference count for the classic locale.

PR libstdc++/59439
* src/c++98/locale.cc (locale::locale(const locale&)): Bypass
reference count updates for the classic locale.
(locale::~locale()): Likewise.  (locale::operator=(const
locale&)): Likewise.
* src/c++98/locale_init.cc (locale::locale()): Likewise.
(locale::global(const locale&)): Likewise.

diff --git a/libstdc++-v3/src/c++98/locale.cc b/libstdc++-v3/src/c++98/locale.cc
index 148bf59658e..fe06d297039 100644
--- a/libstdc++-v3/src/c++98/locale.cc
+++ b/libstdc++-v3/src/c++98/locale.cc
@@ -77,7 +77,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   locale::locale(const locale& __other) throw()
   : _M_impl(__other._M_impl)
-  { _M_impl->_M_add_reference(); }
+  {
+if (_M_impl != _S_classic)
+  _M_impl->_M_add_reference();
+  }
 
   // This is used to initialize global and classic locales, and
   // assumes that the _Impl objects are constructed correctly.
@@ -86,7 +89,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { }
 
   locale::~locale() throw()
-  { _M_impl->_M_remove_reference(); }
+  {
+if (_M_impl != _S_classic)
+  _M_impl->_M_remove_reference();
+  }
 
   bool
   locale::operator==(const locale& __rhs) const throw()
@@ -112,8 +118,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   const locale&
   locale::operator=(const locale& __other) throw()
   {
-__other._M_impl->_M_add_reference();
-_M_impl->_M_remove_reference();
+if (__other._M_impl != _S_classic)
+  __other._M_impl->_M_add_reference();
+if (_M_impl != _S_classic)
+  _M_impl->_M_remove_reference();
 _M_impl = __other._M_impl;
 return *this;
   }
diff --git a/libstdc++-v3/src/c++98/locale_init.cc 
b/libstdc++-v3/src/c++98/locale_init.cc
index c9078c015c3..b580a9f9d58 100644
--- a/libstdc++-v3/src/c++98/locale_init.cc
+++ b/libstdc++-v3/src/c++98/locale_init.cc
@@ -257,9 +257,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 //   fall back to lock protected access to both _S_global and
 //   its reference count.
 _M_impl = _S_global;
-if (_M_impl == _S_classic)
-  _M_impl->_M_add_reference();
-else
+if (_M_impl != _S_classic)
   {
 __gnu_cxx::__scoped_lock sentry(get_locale_mutex());
 _S_global->_M_add_reference();
@@ -275,7 +273,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 {
   __gnu_cxx::__scoped_lock sentry(get_locale_mutex());
   __old = _S_global;
-  __other._M_impl->_M_add_reference();
+  if (__other._M_impl != _S_classic)
+   __other._M_impl->_M_add_reference();
   _S_global = __other._M_impl;
   const string __other_name = __other.name();
   if (__other_name != "*")
@@ -284,7 +283,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 // Reference count sanity check: one reference removed for the
 // subsition of __other locale, one added by return-by-value. Net
-// difference: zero. When the returned locale object's destrutor
+// difference: zero. When the returned locale object's destructor
 // is called, then the reference count is decremented and possib

[Patch, Aarch64] Fix testsuite regressions related to PR tree-optimization/71625

[Steve Ellcey]

A patch for PR tree-optimized/71625 caused regressions in the
gcc.target/aarch64/vclz.c and gcc.target/aarch64/vneg_s.c tests
because a couple of routines that were not getting inlined before
started getting inlined.  The inlining is not a bug, the 
generated code is now smaller so some functions that were previously
not being inlined due to being too large are now getting inlined.
Because we also generate out-of-line code the scan-assembler-times
checks are failing.  Since inlining or not inlining is not the 
point of this test I added the noinline attribute to all the test_*
functions and this fixed the test regressions.

Tested on aarch64, OK for checkin?

Steve Ellcey
sell...@cavium.com


2018-09-26  Steve Ellcey  

PR tree-optimization/71625
* /gcc.target/aarch64/vclz.c (test_vclz_s8): Add noinline attribute.
(test_vclz_s16): Ditto.
(test_vclz_s32): Ditto.
(test_vclzq_s8): Ditto.
(test_vclzq_s16): Ditto.
(test_vclzq_s32): Ditto.
(test_vclz_u8): Ditto.
(test_vclz_u16): Ditto.
(test_vclz_u32): Ditto.
(test_vclzq_u8): Ditto.
(test_vclzq_u16): Ditto.
(test_vclzq_u32): Ditto.
* gcc.target/aarch64/vneg_s.c (test_vneg_s8): Ditto.
(test_vneg_s16): Ditto.
(test_vneg_s32): Ditto.
(test_vneg_s64): Ditto.
(test_vnegd_s64): Ditto.
(test_vnegq_s8): Ditto.
(test_vnegq_s16): Ditto.
(test_vnegq_s32): Ditto.
(test_vnegq_s64): Ditto.


diff --git a/gcc/testsuite/gcc.target/aarch64/vclz.c 
b/gcc/testsuite/gcc.target/aarch64/vclz.c
index 60494a8..a36ee44 100644
--- a/gcc/testsuite/gcc.target/aarch64/vclz.c
+++ b/gcc/testsuite/gcc.target/aarch64/vclz.c
@@ -75,7 +75,7 @@ extern void abort (void);
 if (a [i] != b [i])
\
   return 1;
 
-int
+int __attribute__ ((noinline))
 test_vclz_s8 ()
 {
   int i;
@@ -107,7 +107,7 @@ test_vclz_s8 ()
 /* Double scan-assembler-times to take account of unsigned functions.  */
 /* { dg-final { scan-assembler-times "clz\\tv\[0-9\]+\.8b, v\[0-9\]+\.8b" 4 } 
} */
 
-int
+int __attribute__ ((noinline))
 test_vclz_s16 ()
 {
   int i;
@@ -138,7 +138,7 @@ test_vclz_s16 ()
 /* Double scan-assembler-times to take account of unsigned functions.  */
 /* { dg-final { scan-assembler-times "clz\\tv\[0-9\]+\.4h, v\[0-9\]+\.4h" 10} 
} */
 
-int
+int __attribute__ ((noinline))
 test_vclz_s32 ()
 {
   int i;
@@ -205,7 +205,7 @@ test_vclz_s32 ()
 /* Double scan-assembler-times to take account of unsigned functions.  */
 /* { dg-final { scan-assembler-times "clz\\tv\[0-9\]+\.2s, v\[0-9\]+\.2s" 34 } 
} */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_s8 ()
 {
   int i;
@@ -226,7 +226,7 @@ test_vclzq_s8 ()
 /* Double scan-assembler-times to take account of unsigned functions.  */
 /* { dg-final { scan-assembler-times "clz\\tv\[0-9\]+\.16b, v\[0-9\]+\.16b" 2 
} } */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_s16 ()
 {
   int i;
@@ -262,7 +262,7 @@ test_vclzq_s16 ()
 /* Double scan-assembler-times to take account of unsigned functions.  */
 /* { dg-final { scan-assembler-times "clz\\tv\[0-9\]+\.8h, v\[0-9\]+\.8h" 6 } 
} */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_s32 ()
 {
   int i;
@@ -303,7 +303,7 @@ test_vclzq_s32 ()
 
 /* Unsigned versions.  */
 
-int
+int __attribute__ ((noinline))
 test_vclz_u8 ()
 {
   int i;
@@ -331,7 +331,7 @@ test_vclz_u8 ()
 
 /* ASM scan near test for signed version.  */
 
-int
+int __attribute__ ((noinline))
 test_vclz_u16 ()
 {
   int i;
@@ -361,7 +361,7 @@ test_vclz_u16 ()
 
 /* ASM scan near test for signed version.  */
 
-int
+int __attribute__ ((noinline))
 test_vclz_u32 ()
 {
   int i;
@@ -427,7 +427,7 @@ test_vclz_u32 ()
 
 /* ASM scan near test for signed version.  */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_u8 ()
 {
   int i;
@@ -448,7 +448,7 @@ test_vclzq_u8 ()
 
 /* ASM scan near test for signed version.  */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_u16 ()
 {
   int i;
@@ -485,7 +485,7 @@ test_vclzq_u16 ()
 
 /* ASM scan near test for signed version.  */
 
-int
+int __attribute__ ((noinline))
 test_vclzq_u32 ()
 {
   int i;
diff --git a/gcc/testsuite/gcc.target/aarch64/vneg_s.c 
b/gcc/testsuite/gcc.target/aarch64/vneg_s.c
index e7f20f2..6947526 100644
--- a/gcc/testsuite/gcc.target/aarch64/vneg_s.c
+++ b/gcc/testsuite/gcc.target/aarch64/vneg_s.c
@@ -87,7 +87,7 @@ extern void abort (void);
 force_simd (res); \
   }
 
-int
+int __attribute__ ((noinline))
 test_vneg_s8 ()
 {
   int8x8_t a;
@@ -107,7 +107,7 @@ test_vneg_s8 ()
 
 /* { dg-final { scan-assembler-times "neg\\tv\[0-9\]+\.8b, v\[0-9\]+\.8b" 1 } 
} */
 
-int
+int __attribute__ ((noinline))
 test_vneg_s16 ()
 {
   int16x4_t a;
@@ -127,7 +127,7 @@ test_vneg_s16 ()
 
 /* { dg-final { scan-assembler-times "neg\\tv\[0-9\]+\.4h, v\[0-9\]+\.4h" 2 } 
} */
 
-int
+int __attribute__ ((noinline))
 test_vneg_s32 ()
 {
 

Re: [PATCH 09/25] Elide repeated RTL elements.

[Andrew Stubbs]

Ping.

On 20/09/18 11:52, Andrew Stubbs wrote:

On 19/09/18 17:38, Andrew Stubbs wrote:
Here's an updated patch incorporating the RTL front-end changes. I had 
to change from "repeated 2x" to "repeated x2" because the former is 
not a valid C token, and apparently that's important.


Here's a patch with self tests added, for both reading and writing.

It also fixes a bug when the repeat was the last item in a list.

OK?

Andrew

Re: [Patch, fortran] PRs 70752 and 72709 - more deferred character length bugs

[Paul Richard Thomas]

Hi Dominique,

Thanks for the heads up - I will check it out and fix it next week
before doing any commits.

Cheers

Paul


On 26 September 2018 at 16:52, Dominique d'Humières  wrote:
>> Is se->string_length guaranteed to be of type gfc_array_index_type_here?
>> If so, why? And if not, maybe a fold_convert is in order?
>
> I don’t know if this related, but if I build gfortran with the patches for 
> PRs 70752 and 72709, 70149, and 65677 with --enable-checking=yes, compiling 
> the test in pr82617 gives an ICE:
>
> pr82617.f90:68:0:
>
> 68 |   items(i_item) = str(i0:i1-1)
>|
> internal compiler error: tree check: expected tree that contains 'decl 
> minimal' structure, have 'indirect_ref' in get_array_span, at 
> fortran/trans.c:301
>
> Cheers,
>
> Dominique
>



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [PATCH 11/25] Simplify vec_merge according to the mask.

[Andrew Stubbs]

Ping.

On 20/09/18 16:26, Andrew Stubbs wrote:

On 17/09/18 10:05, Richard Sandiford wrote:

Would be good to have self-tests for the new transforms.

[...]

known_eq, since we require equality for correctness.  Same for the
other tests.


How about the attached? I've made the edits you requested and written 
some self-tests.



Doesn't simplify_merge_mask make the second two redundant?  I couldn't
see the difference between them and the first condition tested by
simplify_merge_mask.


Yes, I think you're right. Removed, now.

Andrew

Re: [PATCH 07/25] [pr82089] Don't sign-extend SFV 1 in BImode

[Richard Sandiford]

Andrew Stubbs  writes:
> On 17/09/18 09:40, Richard Sandiford wrote:
>>  writes:
>>> This is an update of the patch posted to PR82089 long ago.  We ran into the
>>> same bug on GCN, so we need this fixed as part of this series.
>>>
>>> 2018-09-05  Andrew Stubbs  
>>>  Tom de Vries  
>>>
>>> PR82089
>>>
>>> gcc/
>>> * expmed.c (emit_cstore): Fix handling of result_mode == BImode and
>>> STORE_FLAG_VALUE == 1.
>>> ---
>>>   gcc/expmed.c | 15 +++
>>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/gcc/expmed.c b/gcc/expmed.c
>>> index 29ce10b..0b87fdc 100644
>>> --- a/gcc/expmed.c
>>> +++ b/gcc/expmed.c
>>> @@ -5464,11 +5464,18 @@ emit_cstore (rtx target, enum insn_code icode, enum 
>>> rtx_code code,
>>>If STORE_FLAG_VALUE does not have the sign bit set when
>>>interpreted in MODE, we can do this conversion as unsigned, which
>>>is usually more efficient.  */
>>> -  if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode))
>>> +  if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode)
>>> +  || (result_mode == BImode && int_target_mode != BImode))
>> 
>> Would be better to test GET_MODE_PRECISION instead of GET_MODE_SIZE,
>> if that works, instead of treating BImode as a special case.
>> 
>>>   {
>>> -  convert_move (target, subtarget,
>>> -   val_signbit_known_clear_p (result_mode,
>>> -  STORE_FLAG_VALUE));
>>> +  gcc_assert (GET_MODE_SIZE (result_mode) != 1
>>> + || STORE_FLAG_VALUE == 1 || STORE_FLAG_VALUE == -1);
>>> +  bool unsignedp
>>> +   = (GET_MODE_SIZE (result_mode) == 1
>>> +  ? STORE_FLAG_VALUE == 1
>>> +  : val_signbit_known_clear_p (result_mode, STORE_FLAG_VALUE));
>>> +
>>> +  convert_move (target, subtarget, unsignedp);
>>> +
>> 
>> GET_MODE_SIZE == 1 would also trigger for QImode, which shouldn't be treated
>> differently from HImode etc.
>> 
>> The original val_signbit_known_clear_p test seems like it might be an
>> abstraction too far.  In practice STORE_FLAG_VALUE has to fit within
>> the mode of a natural (unextended) condition result, so I think we can
>> simply test STORE_FLAG_VALUE >= 0 for all modes to see whether the target
>> wants the result to be treated as signed or unsigned.
>
> How about the attached?
>
> I think I addressed all your comments, and it tests fine on GCN with no 
> regressions.
>
> Andrew
>
> [pr82089] Don't sign-extend SFV 1 in BImode
>
> This is an update of the patch posted to PR82089 long ago.  We ran into the
> same bug on GCN, so we need this fixed as part of this series.
>
> 2018-09-26  Andrew Stubbs  
> Tom de Vries  
>
>   PR82089
>
>   gcc/
>   * expmed.c (emit_cstore): Fix handling of result_mode == BImode and
>   STORE_FLAG_VALUE == 1.

OK, thanks.

Richard

Re: [PATCH 11/25] Simplify vec_merge according to the mask.

[Richard Sandiford]

Andrew Stubbs  writes:
> On 17/09/18 10:05, Richard Sandiford wrote:
>> Would be good to have self-tests for the new transforms.
> [...]
>> known_eq, since we require equality for correctness.  Same for the
>> other tests.
>
> How about the attached? I've made the edits you requested and written 
> some self-tests.
>
>> Doesn't simplify_merge_mask make the second two redundant?  I couldn't
>> see the difference between them and the first condition tested by
>> simplify_merge_mask.
>
> Yes, I think you're right. Removed, now.
>
> Andrew
>
> Simplify vec_merge according to the mask.
>
> This patch was part of the original patch we acquired from Honza and Martin.
>
> It simplifies nested vec_merge operations using the same mask.
>
> Self-tests are included.
>
> 2018-09-20  Andrew Stubbs  
>   Jan Hubicka  
>   Martin Jambor  
>
>   * simplify-rtx.c (simplify_merge_mask): New function.
>   (simplify_ternary_operation): Use it, also see if VEC_MERGEs with the
>   same masks are used in op1 or op2.
>   (test_vec_merge): New function.
>   (test_vector_ops): Call test_vec_merge.
>
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index f77e1aa..13b2882 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -5578,6 +5578,68 @@ simplify_cond_clz_ctz (rtx x, rtx_code cmp_code, rtx 
> true_val, rtx false_val)
>return NULL_RTX;
>  }
>  
> +/* Try to simplify nested VEC_MERGE operations by comparing the masks.  The
> +   nested operations need not use the same vector mode, but must have the 
> same
> +   number of elements.
> +
> +   X is an operand number OP of a VEC_MERGE operation with MASK.
> +   Returns NULL_RTX if no simplification is possible.  */

X isn't always operand OP, it can be nested within it.  How about:

/* Try to simplify X given that it appears within operand OP of a
   VEC_MERGE operation whose mask is MASK.  X need not use the same
   vector mode as the VEC_MERGE, but it must have the same number of
   elements.

   Return the simplified X on success, otherwise return NULL_RTX.  */

> +
> +rtx
> +simplify_merge_mask (rtx x, rtx mask, int op)
> +{
> +  gcc_assert (VECTOR_MODE_P (GET_MODE (x)));
> +  poly_uint64 nunits = GET_MODE_NUNITS (GET_MODE (x));
> +  if (GET_CODE (x) == VEC_MERGE && rtx_equal_p (XEXP (x, 2), mask))
> +{
> +  if (!side_effects_p (XEXP (x, 1 - op)))
> + return XEXP (x, op);
> +}
> +  if (side_effects_p (x))
> +return NULL_RTX;
> +  if (UNARY_P (x)
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 0)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 0))), nunits))
> +{
> +  rtx top0 = simplify_merge_mask (XEXP (x, 0), mask, op);
> +  if (top0)
> + return simplify_gen_unary (GET_CODE (x), GET_MODE (x), top0,
> +GET_MODE (XEXP (x, 0)));
> +}
> +  if (BINARY_P (x)
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 0)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 0))), nunits)
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 1)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 1))), nunits))
> +{
> +  rtx top0 = simplify_merge_mask (XEXP (x, 0), mask, op);
> +  rtx top1 = simplify_merge_mask (XEXP (x, 1), mask, op);
> +  if (top0 || top1)
> + return simplify_gen_binary (GET_CODE (x), GET_MODE (x),
> + top0 ? top0 : XEXP (x, 0),
> + top1 ? top1 : XEXP (x, 1));
> +}
> +  if (GET_RTX_CLASS (GET_CODE (x)) == RTX_TERNARY
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 0)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 0))), nunits)
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 1)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 1))), nunits)
> +  && VECTOR_MODE_P (GET_MODE (XEXP (x, 2)))
> +  && known_eq (GET_MODE_NUNITS (GET_MODE (XEXP (x, 2))), nunits))
> +{
> +  rtx top0 = simplify_merge_mask (XEXP (x, 0), mask, op);
> +  rtx top1 = simplify_merge_mask (XEXP (x, 1), mask, op);
> +  rtx top2 = simplify_merge_mask (XEXP (x, 2), mask, op);
> +  if (top0 || top1)

|| top2?

> + return simplify_gen_ternary (GET_CODE (x), GET_MODE (x),
> +  GET_MODE (XEXP (x, 0)),
> +  top0 ? top0 : XEXP (x, 0),
> +  top1 ? top1 : XEXP (x, 1),
> +  top2 ? top2 : XEXP (x, 2));
> +}
> +  return NULL_RTX;
> +}
> +
>  
>  /* Simplify CODE, an operation with result mode MODE and three operands,
> OP0, OP1, and OP2.  OP0_MODE was the mode of OP0 before it became
> @@ -5967,6 +6029,16 @@ simplify_ternary_operation (enum rtx_code code, 
> machine_mode mode,
> && !side_effects_p (op2) && !side_effects_p (op1))
>   return op0;
>  
> +  if (!side_effects_p (op2))
> + {
> +   rtx top0 = simplify_merge_mask (op0, op2, 0);
> +   rtx top1 = simplify_merge_mask (op1, op2, 1)

Re: [PATCH, AArch64 07/11] Link static libgcc after shared libgcc for -shared-libgcc

[Joseph Myers]

On Tue, 25 Sep 2018, rth7...@gmail.com wrote:

> From: Richard Henderson 
> 
> We are about to introduce symbols to libgcc.a that will
> not be present in libgcc_s.so.  Most symbols will be
> resolved from the shared library first, and only the new
> symbols will be pulled from the static library.

Configurations with this property should be using libgcc's 
config/t-slibgcc-libgcc to make libgcc_s.so into a linker script.  Unless 
you have a configuration needing this but not supporting linker scripts, I 
wouldn't expect driver changes to be needed.

The linker script looks like:

/* GNU ld script
   Use the shared library, but some functions are only in
   the static library.  */
GROUP ( libgcc_s.so.1 -lgcc )

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

Re: [PATCH, AArch64 07/11] Link static libgcc after shared libgcc for -shared-libgcc

[Richard Henderson]

On 9/26/18 9:49 AM, Joseph Myers wrote:
> On Tue, 25 Sep 2018, rth7...@gmail.com wrote:
> 
>> From: Richard Henderson 
>>
>> We are about to introduce symbols to libgcc.a that will
>> not be present in libgcc_s.so.  Most symbols will be
>> resolved from the shared library first, and only the new
>> symbols will be pulled from the static library.
> 
> Configurations with this property should be using libgcc's 
> config/t-slibgcc-libgcc to make libgcc_s.so into a linker script.  Unless 
> you have a configuration needing this but not supporting linker scripts, I 
> wouldn't expect driver changes to be needed.
> 
> The linker script looks like:
> 
> /* GNU ld script
>Use the shared library, but some functions are only in
>the static library.  */
> GROUP ( libgcc_s.so.1 -lgcc )

Ah.  I had been surprised that we didn't already support this
form of linking, and missed this config fragment.  Will fix.


r~

Re: [PATCH 11/25] Simplify vec_merge according to the mask.

[Andrew Stubbs]

On 26/09/18 17:48, Richard Sandiford wrote:

Andrew Stubbs  writes:

+  /* Nested vec_merge.  */
+  rtx nvm = gen_rtx_VEC_MERGE (mode, vm1, vm2, mask1);
+  ASSERT_EQ (vm1, simplify_merge_mask (nvm, mask1, 0));
+  ASSERT_EQ (vm2, simplify_merge_mask (nvm, mask1, 1));


Think the last two should simplify to op0 and op3, which I guess
means recursing on the "return XEXP (x, op);"


I thought about doing that, but I noticed that, for example, 
simplify_gen_unary does not recurse into its operand. Is that an 
omission, or is it expected that those operands will already have been 
simplified?


Andrew

Re: [PR 87339, testsuite] Fix failure of gcc.dg/warn-abs-1.c on some targets

[Joseph Myers]

On Wed, 26 Sep 2018, Martin Jambor wrote:

> I see, I guess the easiest is to skip the test on targets that do not
> really have long double, although if someone thinks that is too
> restrictive, I can also split the test again and move long double bits
> to a separate test.

You should be able to use

{ dg-warning "warning regex" "test name" { target { large_long_double } } }

to make the expectation of a warning conditional without making the whole 
test conditional.

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

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Qing Zhao]

> On Sep 26, 2018, at 10:06 AM, Jan Hubicka  wrote:
> 
>> On Wed, Sep 26, 2018 at 10:45 AM, Jeff Law  wrote:
>>> On 9/26/18 7:38 AM, Jason Merrill wrote:
 On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  wrote:
> IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
> what 'internal' would mean in this context.
 
 I mean internal linkage as in the C and C++ standards.
>> 
>>> Since this is primarily for kernel hot patching, I think we're looking
>>> to restrict inlining to functions that have visibility limited to a
>>> compilation unit.
>> 
>> Right, which is internal linkage.
>> 
>> C11: "Within one translation unit, each declaration of an identifier
>> with internal linkage denotes the same object or function."
>> C++17: "When a name has internal linkage, the entity it denotes can be
>> referred to by names from other scopes in the same translation unit."
>> 
>> Or perhaps we want to say "not external linkage", i.e. !TREE_PUBLIC as
>> richi suggested.
> 
> I am not quite sure if we can relate visibility flags we have at this stage
> to visibility in source languge in very coherent way.  They change a lot with
> LTO and we may want to make this option incompatible with LTO, but even w/o
> we can turn function static that previously wasn’t.

Looks like both LTO and whole_program need to be made incompatible with 
-finline-only-static. 
from my study of the function “cgraph_externally_visible_p”, comdat functions 
can ONLY be turned into
static when either “in_lto_p” or “whole_program” is true. 


> For example comdat that was cloned by IPA-SRA. See can_be_local_p and
> comdat_can_be_unshared_p predicates.  Similar problem happens to clones 
> created
> by ipa-cp.
> 
> I guess we want to disable localization and cloning in this case as well.
> I wonder what else.

Yes, I think we should make -finline-only-static incompatible with cloning and 
tree-sra too.

Qing
> 
> Honza

PING: Re: VRP: special case all pointer conversion code

[Aldy Hernandez]

PING

On 9/17/18 6:12 AM, Aldy Hernandez wrote:
It seems most of the remaining anti range code in 
extract_range_from_unary_expr for CONVERT_EXPR_P is actually dealing 
with non-nullness in practice.


Anti-range handling is mostly handled by canonicalizing anti-ranges into 
its two set constituents (~[10,20] => [MIN,9] U [21,MAX]) and dealing 
with them piece-meal.  For that matter, the only way we can reach the 
conversion code in extract_range_from_unary_expr with an anti-range is 
either with a pointer (because pointers are ignored from 
ranges_from_anti_range on purpose), or when converting integers of the 
form ~[SSA, SSA].  I verified this with a bootstrap + tests with some 
specially placed asserts, BTW.


So... if we special handle pointer conversions (both to and fro, as 
opposed to just to), we get rid of any anti-ranges with the exception of 
~[SSA, SSA] between integers.  And anti-ranges of unknown quantities 
(SSAs) will be handled automatically already (courtesy of 
extract_range_into_wide_ints).


I propose we handle pointers at the beginning, and everything else just 
falls into place, with no special code.


As commented in the code, this will pessimize conversions from (char 
*)~[0, 2] to int, because we will forget that the range can also not be 
1 or 2.  But as Jeff commented, we really only care about null or 
non-nullness.  Special handling magic pointers with constants IMO is a 
wasted effort.  For that matter, I think it was me that added this 
spaghetti a few weeks ago to make sure we handled ~[0,2].  We weren't 
even handling it a short while back :-).  Furthermore, in a bootstrap, I 
think we only triggered this twice.  And I'm not even sure we make 
further use of anything null/not-null for pointers later on.


This patch simplifies the code, and removes more special handling and 
cryptic comments related to anti-ranges.


Tested with all languages including Ada and Go.

OK for trunk?

Re: [PATCH][Middle-end][Version 4]Add a new option to control inlining only on static functions

[Jan Hubicka]

> 
> > On Sep 26, 2018, at 10:06 AM, Jan Hubicka  wrote:
> > 
> >> On Wed, Sep 26, 2018 at 10:45 AM, Jeff Law  wrote:
> >>> On 9/26/18 7:38 AM, Jason Merrill wrote:
>  On Wed, Sep 26, 2018 at 9:24 AM, Richard Biener  
>  wrote:
> > IIRC he explicitely wanted 'static' not 'hidden' linkage.  Not sure
> > what 'internal' would mean in this context.
>  
>  I mean internal linkage as in the C and C++ standards.
> >> 
> >>> Since this is primarily for kernel hot patching, I think we're looking
> >>> to restrict inlining to functions that have visibility limited to a
> >>> compilation unit.
> >> 
> >> Right, which is internal linkage.
> >> 
> >> C11: "Within one translation unit, each declaration of an identifier
> >> with internal linkage denotes the same object or function."
> >> C++17: "When a name has internal linkage, the entity it denotes can be
> >> referred to by names from other scopes in the same translation unit."
> >> 
> >> Or perhaps we want to say "not external linkage", i.e. !TREE_PUBLIC as
> >> richi suggested.
> > 
> > I am not quite sure if we can relate visibility flags we have at this stage
> > to visibility in source languge in very coherent way.  They change a lot 
> > with
> > LTO and we may want to make this option incompatible with LTO, but even w/o
> > we can turn function static that previously wasn’t.
> 
> Looks like both LTO and whole_program need to be made incompatible with 
> -finline-only-static. 
> from my study of the function “cgraph_externally_visible_p”, comdat functions 
> can ONLY be turned into
> static when either “in_lto_p” or “whole_program” is true. 

This is not quite the case, because, we can still clone them to static functions
or derive fact about their side effects (such that information if they 
write/read
memory, particular global var etc).  All this inter-procedural propagation
is guarded by get_availability predicate returning at least AVAILABLE.

If you make this to be INTERPOSABLE (which means it can be replaced by different
implementation by linker and that is probably what we want for live patching)
then also inliner, ipa-sra and other optimization will give up on these.

Honza
> 
> 
> > For example comdat that was cloned by IPA-SRA. See can_be_local_p and
> > comdat_can_be_unshared_p predicates.  Similar problem happens to clones 
> > created
> > by ipa-cp.
> > 
> > I guess we want to disable localization and cloning in this case as well.
> > I wonder what else.
> 
> Yes, I think we should make -finline-only-static incompatible with cloning 
> and tree-sra too.
> 
> Qing
> > 
> > Honza
> 

[PATCH] change predicates to return bool instead of int

[Martin Sebor]

The attached patch modifies the return types of a number
of predicates in tree.h and tree.c from int to bool.

Tested on x86_64-linux with no unexpected failures.

Martin
gcc/ChangeLog:

	* tree.c (zerop): Change return type to bool.
	(integer_zerop, integer_onep, integer_each_onep): Same.
	(integer_all_onesp, integer_minus_onep, integer_pow2p): Same.
	(integer_nonzerop, integer_truep, tree_ctz, real_zerop): Same.
	(real_onep, real_minus_onep, chain_index): Same.
	(print_type_hash_statistics, type_list_equal): Same.
	* tree.h (zerop): Same.
	(zerop, integer_zerop, integer_onep, integer_each_onep): Same.
	(integer_all_onesp, integer_minus_onep, integer_pow2p): Same.
	(integer_nonzerop, integer_truep, tree_ctz, real_zerop): Same.
	(real_onep, real_minus_onep, chain_index): Same.
	(print_type_hash_statistics, type_list_equal): Same.

Index: gcc/tree.c
===
--- gcc/tree.c	(revision 264649)
+++ gcc/tree.c	(working copy)
@@ -2475,7 +2475,7 @@ grow_tree_vec (tree v, int len MEM_STAT_DECL)
 /* Return 1 if EXPR is the constant zero, whether it is integral, float or
fixed, and scalar, complex or vector.  */
 
-int
+bool
 zerop (const_tree expr)
 {
   return (integer_zerop (expr)
@@ -2486,7 +2486,7 @@ zerop (const_tree expr)
 /* Return 1 if EXPR is the integer constant zero or a complex constant
of zero.  */
 
-int
+bool
 integer_zerop (const_tree expr)
 {
   switch (TREE_CODE (expr))
@@ -2508,7 +2508,7 @@ integer_zerop (const_tree expr)
 /* Return 1 if EXPR is the integer constant one or the corresponding
complex constant.  */
 
-int
+bool
 integer_onep (const_tree expr)
 {
   switch (TREE_CODE (expr))
@@ -2530,7 +2530,7 @@ integer_onep (const_tree expr)
 /* Return 1 if EXPR is the integer constant one.  For complex and vector,
return 1 if every piece is the integer constant one.  */
 
-int
+bool
 integer_each_onep (const_tree expr)
 {
   if (TREE_CODE (expr) == COMPLEX_CST)
@@ -2543,13 +2543,13 @@ integer_each_onep (const_tree expr)
 /* Return 1 if EXPR is an integer containing all 1's in as much precision as
it contains, or a complex or vector whose subparts are such integers.  */
 
-int
+bool
 integer_all_onesp (const_tree expr)
 {
   if (TREE_CODE (expr) == COMPLEX_CST
   && integer_all_onesp (TREE_REALPART (expr))
   && integer_all_onesp (TREE_IMAGPART (expr)))
-return 1;
+return true;
 
   else if (TREE_CODE (expr) == VECTOR_CST)
 return (VECTOR_CST_NPATTERNS (expr) == 1
@@ -2557,7 +2557,7 @@ integer_all_onesp (const_tree expr)
 	&& integer_all_onesp (VECTOR_CST_ENCODED_ELT (expr, 0)));
 
   else if (TREE_CODE (expr) != INTEGER_CST)
-return 0;
+return false;
 
   return (wi::max_value (TYPE_PRECISION (TREE_TYPE (expr)), UNSIGNED)
 	  == wi::to_wide (expr));
@@ -2565,7 +2565,7 @@ integer_all_onesp (const_tree expr)
 
 /* Return 1 if EXPR is the integer constant minus one.  */
 
-int
+bool
 integer_minus_onep (const_tree expr)
 {
   if (TREE_CODE (expr) == COMPLEX_CST)
@@ -2578,16 +2578,16 @@ integer_minus_onep (const_tree expr)
 /* Return 1 if EXPR is an integer constant that is a power of 2 (i.e., has only
one bit on).  */
 
-int
+bool
 integer_pow2p (const_tree expr)
 {
   if (TREE_CODE (expr) == COMPLEX_CST
   && integer_pow2p (TREE_REALPART (expr))
   && integer_zerop (TREE_IMAGPART (expr)))
-return 1;
+return true;
 
   if (TREE_CODE (expr) != INTEGER_CST)
-return 0;
+return false;
 
   return wi::popcount (wi::to_wide (expr)) == 1;
 }
@@ -2595,7 +2595,7 @@ integer_pow2p (const_tree expr)
 /* Return 1 if EXPR is an integer constant other than zero or a
complex constant other than zero.  */
 
-int
+bool
 integer_nonzerop (const_tree expr)
 {
   return ((TREE_CODE (expr) == INTEGER_CST
@@ -2609,7 +2609,7 @@ integer_nonzerop (const_tree expr)
return 1 if every piece is the integer constant minus one
(representing the value TRUE).  */
 
-int
+bool
 integer_truep (const_tree expr)
 {
   if (TREE_CODE (expr) == VECTOR_CST)
@@ -2619,7 +2619,7 @@ integer_truep (const_tree expr)
 
 /* Return 1 if EXPR is the fixed-point constant zero.  */
 
-int
+bool
 fixed_zerop (const_tree expr)
 {
   return (TREE_CODE (expr) == FIXED_CST
@@ -2764,7 +2764,7 @@ tree_ctz (const_tree expr)
 /* Return 1 if EXPR is the real constant zero.  Trailing zeroes matter for
decimal float constants, so don't return 1 for them.  */
 
-int
+bool
 real_zerop (const_tree expr)
 {
   switch (TREE_CODE (expr))
@@ -2794,7 +2794,7 @@ real_zerop (const_tree expr)
Trailing zeroes matter for decimal float constants, so don't return
1 for them.  */
 
-int
+bool
 real_onep (const_tree expr)
 {
   switch (TREE_CODE (expr))
@@ -2817,7 +2817,7 @@ real_onep (const_tree expr)
 /* Return 1 if EXPR is the real constant minus one.  Trailing zeroes
matter for decimal float constants, so don't return 1 for them.  */
 
-int
+bool
 real_minus_onep (const_tree expr)
 {
   switch (TREE_CODE (expr))
@

Re: [PATCH] PR libstdc++/59439 optimize uses of classic ("C") std::locale

[Florian Weimer]

* Jonathan Wakely:

> Does anybody see any problems with this change?

Can you put a debug assert into _M_add_reference and
_M_remove_reference, to check that they are never called on an
_S_classic object?  Or put the check directly into those functions?

Thanks,
Florian

[PATCH, GCC/ARM] Fix PR87374: ICE with -mslow-flash-data and -mword-relocations

[Thomas Preudhomme]

Hi,

GCC ICEs under -mslow-flash-data and -mword-relocations because there
is no way to load an address, both literal pools and MOVW/MOVT being
forbidden. This patch gives an error message when both options are
specified by the user and adds the according dg-skip-if directives for
tests that use either of these options.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-09-25  Thomas Preud'homme  

PR target/87374
* config/arm/arm.c (arm_option_check_internal): Disable the combined
use of -mslow-flash-data and -mword-relocations.

*** gcc/testsuite/ChangeLog ***

2018-09-25  Thomas Preud'homme  

PR target/87374
* gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and
-mword-relocations would be passed when compiling the test.
* gcc.target/arm/movsi_movt.c: Likewise.
* gcc.target/arm/pr81863.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
* gcc.target/arm/tls-disable-literal-pool.c: Likewise.


Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when
targeting arm-none-eabi. Modified tests get skipped as expected when
running the testsuite with -mslow-flash-data (pr81863.c) or
-mword-relocations (all the others).


Is this ok for trunk? I'd also appreciate guidance on whether this is
worth a backport. It's a simple patch but on the other hand it only
prevents some option combination, it does not fix anything so I have
mixed feelings.

Best regards,

Thomas
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df05..5beffc875c1 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2893,17 +2893,22 @@ arm_option_check_internal (struct gcc_options *opts)
   flag_pic = 0;
 }
 
-  /* We only support -mpure-code and -mslow-flash-data on M-profile targets
- with MOVT.  */
-  if ((target_pure_code || target_slow_flash_data)
-  && (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON))
+  if (target_pure_code || target_slow_flash_data)
 {
   const char *flag = (target_pure_code ? "-mpure-code" :
 	 "-mslow-flash-data");
-  error ("%s only supports non-pic code on M-profile targets with the "
-	 "MOVT instruction", flag);
-}
 
+  /* We only support -mpure-code and -mslow-flash-data on M-profile targets
+	 with MOVT.  */
+  if (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON)
+	error ("%s only supports non-pic code on M-profile targets with the "
+	   "MOVT instruction", flag);
+
+  /* Cannot load addresses: -mslow-flash-data forbids literal pool and
+	 -mword-relocations forbids relocation of MOVT/MOVW.  */
+  if (target_word_relocations)
+	error ("%s incompatible with -mword-relocations", flag);
+}
 }
 
 /* Recompute the global settings depending on target attribute options.  */
diff --git a/gcc/testsuite/gcc.target/arm/movdi_movt.c b/gcc/testsuite/gcc.target/arm/movdi_movt.c
index e2a28ccbd99..a01ffa0dc93 100644
--- a/gcc/testsuite/gcc.target/arm/movdi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movdi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned long long
diff --git a/gcc/testsuite/gcc.target/arm/movsi_movt.c b/gcc/testsuite/gcc.target/arm/movsi_movt.c
index 3cf46e2fd17..19d202ecd33 100644
--- a/gcc/testsuite/gcc.target/arm/movsi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movsi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned
diff --git a/gcc/testsuite/gcc.target/arm/pr81863.c b/gcc/testsuite/gcc.target/arm/pr81863.c
index 63b1ed66b2c..225a0c5cc2b 100644
--- a/gcc/testsuite/gcc.target/arm/pr81863.c
+++ b/gcc/testsuite/gcc.target/arm/pr81863.c
@@ -1,5 +1,6 @@
 /* testsuite/gcc.target/arm/pr48183.c */
 /* { dg-do compile } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mslow-flash-data" } } */
 /* { dg-options "-O2 -mword-relocations -march=armv7-a -marm" } */
 /* { dg-final { scan-assembler-not "\[\\t \]+movw" } } */
 
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
index 089a72b67f3..d10391a69ac 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
@@ -6,6 +6,7 @@
 /* { dg-do compile } */
 /* { dg-require-e

[PR 87415] handle 1 bit bit fields in set_value_range_with_overflow()

[Aldy Hernandez]

As I've said in the PR...

For a 1-bit signed field we are trying to subtract the following ranges:

   [0, 0] - VR_VARYING

Mathematically these are:

   [MAX, MAX] - [MIN, MAX]
   [0, 0] - [MIN, MAX]
=> [0, 0] - [-1, 0]

For ranges: [a, b] - [c, d] is [a - d, b - c], so combine_bounds() yields:

   [0, OVERFLOW]

Then we adjust the result for overflows in 
set_value_range_with_overflow() we transform the above into:


   [0, +INF]

And since +INF is 0, we get [0, 0] which is incorrect.

[0, 0] - [MIN, MAX] should have given [MIN, MAX].

1-bit signed fields are weird in that 0 - (-MIN) is still -MIN.  In any 
other world, it is an impossible result.  For instance, in an 8-bit 
signed world, 0 - (-128) is invalid.


One option would be to treat signed bit fields as TYPE_OVERFLOW wraps, 
since they seem to have wrapping properties, but I'm afraid such a heavy 
handed approach would yield latent bugs across the compiler.  What VRP 
seems to currently be doing in set_and_canonicalize_value_range(), is 
just special casing the wrapping of 1-bit fields:


  /* Wrong order for min and max, to swap them and the VR type we need
 to adjust them.  */
  if (tree_int_cst_lt (max, min))
{
  tree one, tmp;

  /* For one bit precision if max < min, then the swapped
 range covers all values, so for VR_RANGE it is varying and
 for VR_ANTI_RANGE empty range, so drop to varying as well.  */
  if (TYPE_PRECISION (TREE_TYPE (min)) == 1)
{
  set_value_range_to_varying (vr);
  return;
}
...
}

I've done the same in set_value_range_with_overflow().

BTW, I believe this is a latent bug.  Had the MINUS_EXPR code been 
handed down [0,0] - [-1,0] instead of [0,0] - VR_VARYING, we would've 
triggered it.


Tested on x86-64 Linux.

OK for trunk?
gcc/

	PR tree-optimization/87415
	* tree-vrp.c (set_value_range_with_overflow): Special case one bit
	precision fields.

diff --git a/gcc/testsuite/gcc.dg/pr87415.c b/gcc/testsuite/gcc.dg/pr87415.c
new file mode 100644
index 000..473384ac479
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87415.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct A
+{
+  int b:1;
+};
+
+int d;
+
+int main ()
+{
+  struct A e = { 0 };
+  if (!d)
+e.b = -1;
+  if (!e.b)
+__builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index f95437b3040..77fe98a62e3 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1116,6 +1116,15 @@ set_value_range_with_overflow (value_range &vr,
   const unsigned int prec = TYPE_PRECISION (type);
   vr.type = VR_RANGE;
   vr.equiv = NULL;
+
+  /* For one bit precision if max < min, then the swapped
+ range covers all values.  */
+  if (prec == 1 && wi::lt_p (wmax, wmin, sgn))
+{
+  set_value_range_to_varying (&vr);
+  return;
+}
+
   if (TYPE_OVERFLOW_WRAPS (type))
 {
   /* If overflow wraps, truncate the values and adjust the

Re: [Patch, Aarch64] Testsuite fix to fix one of the regressions in PR 87433, gcc.dg/zero_bits_compound-1.c

[Jeff Law]

On 9/26/18 8:58 AM, Steve Ellcey wrote:
> PR rtl-optimization/85160 which allowed combine to convert
> two instructions into two different instructions if they had a lower cost
> caused a couple of regressions on aarch64.  This patch fixes one of them.
> 
> After the above patch, the gcc.dg/zero_bits_compound-1.c test on 
> aarch64 generates an and rtl instruction where it did not before.
> A comparision of the code generated before and after the 85160 patch
> shows that GCC is still generating the same number of instructions
> but there are fewer dependencies between some of the instructions
> so the new code would be better on systems with multiple functional
> units and no worse on systems with a single functional unit.
> 
> Since the test is just verifying that an 'and' rtl instruction is
> not generated and that is now happening on aarch64 but the new code
> is as good as or better than the old code this test doesn't seem
> to make sense for aarch64 anymore and I would like to just not
> run it on aarch64.
> 
> Tested on aarch64.  OK for checkin?
> 
> Steve Ellcey
> sell...@cavium.com
> 
> 
> 2018-09-26  Steve Ellcey  
> 
>   PR testsuite/87433
>   * gcc.dg/zero_bits_compound-1.c: Do not run on aarch64*-*-*.
OK.
jeff