Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

2019-07-10 Thread Martin Sebor

On 7/2/19 2:59 PM, Jeff Law wrote:

On 6/30/19 3:50 PM, Martin Sebor wrote:

On 6/26/19 6:11 PM, Jeff Law wrote:

[ Another big snip ]



Is there a strong need for an overloaded operator?  Our guidelines
generally discourage operator overloads.  This one seems on the border
to me.  Others may have different ideas where the line is.  If we really
don't need the overloaded operator, then we can avoid the controversy
completely.


The copy assignment operator is necessary for the type to be stored
in a hash_map.  Otherwise, the implicitly generated assignment will
be used instead which is unsafe in vec and results in memory
corription (I opened bug 90904 for this).  After working around this
bug by providing the assignment operator I then ran into the hash_map
bug 90959 that also results in memory corruption.  I spent a few fun
hours tracking down the GCC miscompilations that they led to.

OK.  THanks for explaining why we need the copy assignment operator.




The golden rule in C++ is that every type should either be safe to
copy and assign with the expected semantics or made not assignable
and not copyable by declaring the copy ctor and copy assignment
operator private (or deleted in more modern C++).  It would be very
helpful to detect this kind of problem (classes that aren't safely
assignable and copyable because they acquire/release resources in
ctors/dtors).  Not just to us in GCC but I'm sure to others as well.
It's something I've been hoping to implement for a while now.

You've got my blessing to do that :-)



So how do you deal with the case where one operand of a
{MIN,MAX,COND}_EXPR is the address of a local, but the other is not?  It
seems like we'll end up returning true from this function in that case
and potentially trigger changing the return value to NULL.  Or am I
missing something?


I only briefly considered MIN/MAX but because in C and C++ the less
than and greater than expressions are only defined for pointers into
the same array/object or subobject and I didn't ponder it too hard.
(In C they're undefined otherwise, in C++ the result is unspecified.)

But you bring it up because you think the address should be returned
regardless, even if the expression is invalid (or if it's COND_EXPR
with mixed local/nonlocal addresses) so I've changed it to do that
for all these explicitly handled codes and added tests to verify it.

THanks.

So in the thread for strlen/sprintf the issue around unbounded walks up
through PHI argument's SSA_NAME_DEF_STMT was raised.

I think we've got two options here.

One would be to hold this patch until we sort out what the bounds should
look like, then adjust this patch to do something similar.

Or go forward with this patch, then come back and adjust the walker once
we have settled on solution in the other thread.

I'm OK with either.  Your choice.


I committed the patch as is until the question of which algorithms
to put the limit on is answered (as discussed in the thread Re:
sprintf/strlen integration).

Martin


Re: [PATCH] integrate sprintf pass into strlen (PR 83431)

2019-07-10 Thread Martin Sebor

On 7/2/19 4:38 PM, Jeff Law wrote:

On 7/1/19 7:47 PM, Martin Sebor wrote:

Attached is a more complete solution that fully resolves the bug
report by avoiding a warning in cases like:

   char a[32], b[8];

   void f (void)
   {
 if (strlen (a) < sizeof b - 2)
   snprintf (b, sizeof b, "b=%s", a); // no -Wformat-truncation
   }

It does that by having get_range_strlen_dynamic use the EVRP
information for non-constant strlen calls: EVRP has recorded
that the result is less sizeof b - 2 and so the function
returns this limited range of lengths to snprintf which can
then avoid warning.  It also improves the checking and can
find latent bugs it missed before (like the off-by-one in
print-rtl.c).

Besides improving the accuracy of the -Wformat-overflow and
truncation warnings this can also result in better code.
So far this only benefits snprintf but there may be other
opportunities to string functions as well (e.g., strcmp or
memcmp).

Jeff, I looked into your question/suggestion for me last week
when we spoke, to introduce some sort of a recursion limit for
get_range_strlen_dynamic.  It's easily doable but before we go
down that path I did some testing to see how bad it can get and
to compare it with get_range_strlen.  Here are the results for
a few packages.  The dept is the maximum recursion depth, success
and fail are the numbers of successful and failed calls to
the function:

   binutils-gdb:
   depth   success fail
 get_range_strlen:   319  830221621
 get_range_strlen_dynamic:41  1503  161
   gcc:
 get_range_strlen:46  721111365
 get_range_strlen_dynamic:23 10272   12
   glibc:
 get_range_strlen:76  284011422
 get_range_strlen_dynamic:51  1186   46
   elfutils:
 get_range_strlen:33  1198 2516
 get_range_strlen_dynamic:31   685   36
   kernel:
 get_range_strlen:25  529911698
 get_range_strlen_dynamic:31  9911  122

Except the first case of get_range_strlen (I haven't looked into
it yet), it doesn't seem too bad, and with just one exception it's
better than get_range_strlen.  Let me know if you think it's worth
adding a parameter (I assume we'd use it for both functions) and
what to set it to.

On 6/11/19 5:26 PM, Martin Sebor wrote:

The sprintf and strlen passes both work with strings but
run independently of one another and don't share state.  As
a result, lengths of strings dynamically created by functions
that are available to the strlen pass are not available to
sprintf.  Conversely, lengths of strings formatted by
the sprintf functions are not made available to the strlen
pass.  The result is less efficient code, poor diagnostics,
and ultimately less than optimal user experience.

The attached patch is the first step toward rectifying this
design problem.  It integrates the two passes into one and
exposes the string length data managed by the strlen pass to
the sprintf "module."  (It does not expose any sprintf data
to the strlen pass yet.)

The sprintf pass invocations in passes.def have been replaced
with those of strlen.  The first "early" invocation is only
effective for the sprintf module to enable warnings without
optimization.  The second invocation is "late" and enables
both warnings and the sprintf and strlen optimizations unless
explicitly disabled via -fno-optimize-strlen.

Since the strlen optimization is the second invocation of
the pass tests that scan the strlen dump have been adjusted
to look for the "strlen2" dump file.

The changes in the patch are mostly mechanical.  The one new
"feature" worth calling out is the get_range_strlen_dynamic
function.  It's analogous to get_range_strlen in gimple-fold.c
except that it makes use of the strlen "dynamically" obtained
string length info.  In cases when the info is not available
the former calls the latter.

The other new functions in tree-ssa-strlen.c are
check_and_optimize_call and handle_integral_assign: I added
them only to keep the check_and_optimize_stmt function from
getting excessively long and hard to follow.  Otherwise,
the code in the functions is unchanged.

There are a number of further enhancements to consider as
the next steps:
   *  enhance the strlen module to determine string length range
  information from integer variable to which it was previously
  assigned (necessary to fully resolve pr83431 and pr90625),
   *  make the sprintf/snprintf string length results directly
  available to the strlen module,
   *  enhance the sprintf module to optimize snprintf(0, 0, fmt)
  calls with fmt strings consisting solely of plain characters
  and %s directives to series of strlen calls on the arguments,
   *  and more.

Martin



gcc-83431.diff

PR tree-optimization/83431 - -Wformat-truncation may incorrectly report 
truncation

gcc/ChangeLog:

PR c++/83431
* 

[wwwdocs] gcc-4.7/changes.html - adjust link to "Collecting User-Mode Dumps"

2019-07-10 Thread Gerald Pfeifer
Applied.

Gerald

Index: changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.154
diff -u -r1.154 changes.html
--- changes.html11 Dec 2018 04:03:15 -  1.154
+++ changes.html10 Jul 2019 08:55:18 -
@@ -635,7 +635,7 @@
   depends on the user environment settings; see the ulimit -c
   setting for POSIX shells, limit coredumpsize for C shells,
   and the https://docs.microsoft.com/en-us/windows/desktop/wer/collecting-user-mode-dumps;
+  
href="https://docs.microsoft.com/en-us/windows/win32/wer/collecting-user-mode-dumps;
   >WER user-mode dumps settings on Windows.
 The https://gcc.gnu.org/onlinedocs/gcc-4.7.1/gfortran/Debugging-Options.html#index-g_t_0040code_007bfno_002dbacktrace_007d-183;


Re: [PATCH] integrate sprintf pass into strlen (PR 83431)

2019-07-10 Thread Martin Sebor

On 7/2/19 3:59 AM, Richard Biener wrote:

On Tue, Jul 2, 2019 at 3:48 AM Martin Sebor  wrote:


Attached is a more complete solution that fully resolves the bug
report by avoiding a warning in cases like:

char a[32], b[8];

void f (void)
{
  if (strlen (a) < sizeof b - 2)
snprintf (b, sizeof b, "b=%s", a); // no -Wformat-truncation
}

It does that by having get_range_strlen_dynamic use the EVRP
information for non-constant strlen calls: EVRP has recorded
that the result is less sizeof b - 2 and so the function
returns this limited range of lengths to snprintf which can
then avoid warning.  It also improves the checking and can
find latent bugs it missed before (like the off-by-one in
print-rtl.c).

Besides improving the accuracy of the -Wformat-overflow and
truncation warnings this can also result in better code.
So far this only benefits snprintf but there may be other
opportunities to string functions as well (e.g., strcmp or
memcmp).

Jeff, I looked into your question/suggestion for me last week
when we spoke, to introduce some sort of a recursion limit for
get_range_strlen_dynamic.  It's easily doable but before we go
down that path I did some testing to see how bad it can get and
to compare it with get_range_strlen.  Here are the results for
a few packages.  The dept is the maximum recursion depth, success
and fail are the numbers of successful and failed calls to
the function:

binutils-gdb:
depth   success fail
  get_range_strlen:   319  830221621
  get_range_strlen_dynamic:41  1503  161
gcc:
  get_range_strlen:46  721111365
  get_range_strlen_dynamic:23 10272   12
glibc:
  get_range_strlen:76  284011422
  get_range_strlen_dynamic:51  1186   46
elfutils:
  get_range_strlen:33  1198 2516
  get_range_strlen_dynamic:31   685   36
kernel:
  get_range_strlen:25  529911698
  get_range_strlen_dynamic:31  9911  122

Except the first case of get_range_strlen (I haven't looked into
it yet), it doesn't seem too bad, and with just one exception it's
better than get_range_strlen.  Let me know if you think it's worth
adding a parameter (I assume we'd use it for both functions) and
what to set it to.


I think we want to avoid adding code to GCC that might turn out
quadratic for artificial testcases. History tells us that eventually
somebody will find a real-world testcase that hits it.  I would
suggest to not place a limit on the depth but on the number
of SSA_NAME_DEF_STMTs visited.  Not sure to what this
would turn out but I guess using a limit around 500 would work?


None of my builds comes even close that number so setting the limit
wouldn't have any adverse impact on any of them.  But there are
quite a number of other algorithms in GCC that recursively chase
SSE_NAME_DEF_STMTs, including in gimple-fold.c.  It would be
interesting to see data for those.


For the data above what's the biggest depth / highest number
of stmts we visit when we are able to compute a useful limit
and what is it when including failure?  The individual number
of successes/fails are not too interesting.  Maybe even provide
a histogram for the successful cases depth/stmt count?


I changed the code to count SSA_NAMEs and PHI arguments (and do
other things).  The most I see for those are these in GDB/Binutils:

binutils/objcopy.c:5694:1: note: successful get_range_strlen 
(‘output_target’): depth:211, ssa_names:9, phi_args:207, 
result:{0,18446744073709551615,18446744073709551615}
binutils/objcopy.c:5694:1: note: successful get_range_strlen 
(‘input_target’): depth:104, ssa_names:4, phi_args:101, 
result:{0,18446744073709551615,18446744073709551615}
binutils/readelf.c:1152:1: note: successful get_range_strlen (‘rtype’): 
depth:189, ssa_names:73, phi_args:118, result:{10,18446744073709551615,25}
binutils/readelf.c:1152:1: note: successful get_range_strlen (‘rtype’): 
depth:201, ssa_names:77, phi_args:129, result:{10,18446744073709551615,25}


The first two work very hard only to find the string length and
array size are unbounded.  The latter two determine the string
is between 10 and 24 characters (i.e., it's stored in char[25]).

These are the only four with a three-digit depth.  There are
463 two-digit depths (ssa_names/phi args), and 9,642 single
digit ones in the GDB/Binutils build.

In GCC there are 92 two-digit depths and 17,448 single-digit.

Failures are usually very quick (with just one exception all
under 10 SSA_NAMEs and PHI args.)


Otherwise I like the merging - can you annotate the passes.def
entries with a comment indicating what the pass parameter
designates?  Like /* do optimize */?


Sure.



Since we have a parallelization GSoC project running not
adding more global state to passes would be nice as well,
strlen has a lot of it that could 

[PATCH], PowerPC, Patch #6, revision 2, Create pc-relative addressing insns

2019-07-10 Thread Michael Meissner
Here is the revision of patch #6.  Changes from the previous version of the
patch:

1) I provided separate get_TOC_alias_set and get_pc_relative_alias_set
functions, and changed all of the places that had been changed to call
get_data_alias_set to revert to calling get_TOC_alias_set.

2) I added an assert in create_TOC_reference to guard against being called with
pc-relative addressing.

3) In the other places that check for TOC handling, I added tests to skip the
TOC handling if we are generating pc-relative code.

4) I added code in rs6000_emit_move to handle the local SYMBOL_REFs when
pc-relative moves are handled, as well as the constants that were created.

5) I simplified some of the code that set up prefixed addressing since we
already created a valid address when the constant was forced to memory.  I
realized that I didn't have to check whether the SYMBOL_REF was within the
constant pool with pc-relative addressing, I just had to check if it was a
local label and medium code model.

I have bootstrapped this on a little endian power8 system and there were no
regressions in the test suite.  Can I check this into the trunk?

2019-07-10  Michael Meissner  

* config/rs6000/rs6000-protos.h (get_pc_relative_alias_set): New
declaration.
* config/rs6000/rs6000.c (create_TOC_reference): Add gcc_assert.
(toc_relative_expr_p): Add pc-relative check.
(rs6000_legitimize_address): Add pc-relative check in TOC code.
(rs6000_emit_move): Add support for loading up the addresses and
constants with pc-relative addressing.
(TOC_alias_set): Rename static variable from 'set'.
(get_TOC_alias_set): Use TOC_alias_set.
(pc_relative_alias_set): New static variable.
(get_pc_relative_alias_set): New function.
* config/rs6000/rs6000.md (cmp_internal2): Add support for
pc-relative addressing.

Index: gcc/config/rs6000/rs6000-protos.h
===
--- gcc/config/rs6000/rs6000-protos.h   (revision 273361)
+++ gcc/config/rs6000/rs6000-protos.h   (working copy)
@@ -190,6 +190,7 @@ extern void output_function_profiler (FI
 extern void output_profile_hook  (int);
 extern int rs6000_trampoline_size (void);
 extern alias_set_type get_TOC_alias_set (void);
+extern alias_set_type get_pc_relative_alias_set (void);
 extern void rs6000_emit_prologue (void);
 extern void rs6000_emit_load_toc_table (int);
 extern unsigned int rs6000_dbx_register_number (unsigned int, unsigned int);
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 273363)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -7740,6 +7740,8 @@ create_TOC_reference (rtx symbol, rtx la
 {
   rtx tocrel, tocreg, hi;
 
+  gcc_assert (TARGET_TOC && !TARGET_PCREL);
+
   if (TARGET_DEBUG_ADDR)
 {
   if (SYMBOL_REF_P (symbol))
@@ -7785,7 +7787,7 @@ bool
 toc_relative_expr_p (const_rtx op, bool strict, const_rtx *tocrel_base_ret,
 const_rtx *tocrel_offset_ret)
 {
-  if (!TARGET_TOC)
+  if (!TARGET_TOC || TARGET_PCREL)
 return false;
 
   if (TARGET_CMODEL != CMODEL_SMALL)
@@ -8137,7 +8139,7 @@ rs6000_legitimize_address (rtx x, rtx ol
emit_insn (gen_macho_high (reg, x));
   return gen_rtx_LO_SUM (Pmode, reg, x);
 }
-  else if (TARGET_TOC
+  else if (TARGET_TOC && !TARGET_PCREL
   && SYMBOL_REF_P (x)
   && constant_pool_expr_p (x)
   && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (x), Pmode))
@@ -9865,10 +9867,18 @@ rs6000_emit_move (rtx dest, rtx source,
   /* If this is a SYMBOL_REF that refers to a constant pool entry,
 and we have put it in the TOC, we just need to make a TOC-relative
 reference to it.  */
-  if (TARGET_TOC
+  if (TARGET_TOC && !TARGET_PCREL
  && SYMBOL_REF_P (operands[1])
  && use_toc_relative_ref (operands[1], mode))
operands[1] = create_TOC_reference (operands[1], operands[0]);
+
+  /* If this is a SYMBOL_REF that we refer to via pc-relative addressing,
+we don't have to do any special for it.  */
+  else if (TARGET_PCREL && SYMBOL_REF_P (operands[1])
+  && TARGET_CMODEL == CMODEL_MEDIUM
+  && SYMBOL_REF_LOCAL_P (operands[1]))
+   ;
+
   else if (mode == Pmode
   && CONSTANT_P (operands[1])
   && GET_CODE (operands[1]) != HIGH
@@ -9919,7 +9929,7 @@ rs6000_emit_move (rtx dest, rtx source,
 
  operands[1] = force_const_mem (mode, operands[1]);
 
- if (TARGET_TOC
+ if (TARGET_TOC && !TARGET_PCREL
  && SYMBOL_REF_P (XEXP (operands[1], 0))
  && use_toc_relative_ref (XEXP (operands[1], 0), mode))
{
@@ -9928,6 +9938,11 @@ rs6000_emit_move (rtx dest, rtx source,
  operands[1] = gen_const_mem (mode, tocref);
  set_mem_alias_set (operands[1], 

[patch] Small improvements to coverage info (4/n)

2019-07-10 Thread Eric Botcazou
Hi,

the returns are a little problematic for coverage info because they are 
commonalized in GIMPLE, i.e. turned into gotos to a representative return.
This means that you need to clear the location of the representative return, 
see lower_gimple_return:

  /* Remove the line number from the representative return statement.
 It now fills in for many such returns.  Failure to remove this
 will result in incorrect results for coverage analysis.  */
  gimple_set_location (tmp_rs.stmt, UNKNOWN_LOCATION);

otherwise the coverage info is blatantly wrong.  But this in turn means that 
such representative returns are vulneable to inheriting random source location 
information in the debug info generated by the compiler.

I have attached an Ada testcase that demonstrates the problem at -O0:

.loc 1 14 10 discriminator 2
movl$0, %r13d
.L12:
movl%r13d, %eax
jmp .L23

.L12 is what's left at the RTL level of a representative return in GIMPLE and 
it inherits the location directive, which gives wrong coverage info (that's 
not visible with gcov because the instrumentation done by -fprofile-arcs 
papers over the issue, but for example with callgrind).

The proposed fix is attached: it sets the current location to the function's 
end locus when expanding such a representative return to RTL.  That's a bit on 
the kludgy side, but doing something in GIMPLE, e.g. in lower_gimple_return, 
leads to various regressions in terms of quality of diagnostics.

Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline?


2019-07-10  Eric Botcazou  

* cfgexpand.c (expand_gimple_stmt_1) : If the statement
doesn't have a location, set the current location to the function's end.

-- 
Eric BotcazouIndex: cfgexpand.c
===
--- cfgexpand.c	(revision 273294)
+++ cfgexpand.c	(working copy)
@@ -3712,6 +3712,12 @@ expand_gimple_stmt_1 (gimple *stmt)
   {
 	op0 = gimple_return_retval (as_a  (stmt));
 
+	/* If a return doesn't have a location, it very likely represents
+	   multiple user returns so we cannot let it inherit the location
+	   of the last statement of the previous basic block in RTL.  */
+	if (!gimple_has_location (stmt))
+	  set_curr_insn_location (cfun->function_end_locus);
+
 	if (op0 && op0 != error_mark_node)
 	  {
 	tree result = DECL_RESULT (current_function_decl);
pragma Unsuppress (All_Checks);
pragma Check_Float_Overflow;
   
package body Ops is
   
   function Both_Ok (A, B : Capsum) return Boolean is
   begin
  if A.X + A. Y <= A.Cap and then B.X + B.Y <= B.Cap then -- # test
	 N_Ok := N_Ok + 1; -- # ok
	 return True;  -- # ok
  else
	 N_Ko := N_Ko + 1; -- # ko
	 return False; -- # ko
  end if;
   exception
  when Constraint_Error =>
	 N_Ov := N_Ov + 1; -- # ov
	 return False; -- # ov
   end;
end;
package Ops is
   
   type Constrained_Float is new Float range Float'Range;
   
   type Capsum is record
  X, Y, Cap : Constrained_Float;
   end record;
   
   Cs_Ok : constant Capsum := (X => 1.0, Y => 1.0, Cap => 3.0);
   Cs_Ko : constant Capsum := (X => 1.0, Y => 2.0, Cap => 1.0);
   Cs_Ov : constant Capsum := (X => Constrained_Float'Last,
			   Y => Constrained_Float'Last,
			   Cap => 3.0);
   
   N_Ok, N_Ko, N_Ov : Integer := 0;
   
   function Both_Ok (A, B : Capsum) return Boolean;
   
end;


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Gaius Mulley
Matthias Klose  writes:

> I don't know about a best solution, but I contributed changes to
> DRUNTIME_LIBRARIES_ZLIB in libphobos/m4/druntime/libraries.m4 and
> gcc/doc/install.texi (--with-target-system-zlib).
>
> Or you look at libffi/libgo, with libgo always linking the convenience libffi
> library.

many thanks for the pointers - will read the code,


regards,
Gaius


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Gaius Mulley
Matthias Klose  writes:

>>>  - The internal tools in the gcclibdir are installed twice, with
>>>both vanilla names and prefixed/suffixed names.
>>> The installed tree:
>>>
>>> ./usr/bin
>>> ./usr/bin/x86_64-linux-gnu-gm2-9
>>> ./usr/bin/x86_64-linux-gnu-gm2m-9
>>> ./usr/lib/gcc/x86_64-linux-gnu
>>> ./usr/lib/gcc/x86_64-linux-gnu/9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/cc1gm2
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2l
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lcc
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lgen
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lorder
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-cc1gm2-9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2l-9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lcc-9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lgen-9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lorder-9
>>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2m-9
>> 
>> With a fresh build, configured with
>> 
>>  --program-suffix=-9
>>  --program-prefix=x86_64-linux-gnu-
>> 
>> the latter set of internal binaries is installed, while I would expect just 
>> the
>> un-pre/post-fixed tool names.
>
> fixed by the attached patch.

Hi Matthias,

thank you - I think we were both working on the same code at the same
time!  I've checked in some fixes to both the master gm2/Make-lang.in
(and 9.1.0 branch).  I've culled gm2m from /usr/bin and relegated it to
libexecsubdir,




regards,
Gaius


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Gaius Mulley
Matthias Klose  writes:

> In both gcc/gm2 and libgm2, there are explicit calls to make, which probably
> should bre replaced by $(MAKE).

thanks - now fixed in 9.1.0 and master


regards,
Gaius


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Matthias Klose
On 10.07.19 22:07, Gaius Mulley wrote:
> Matthias Klose  writes:
> 
>> On 09.07.19 23:30, Matthias Klose wrote:
>>> On 09.07.19 21:48, Gaius Mulley wrote:
 Matthias Klose  writes:

>>  - libpth.{a,so} is installed in the system libdir, which
>>conflicts with the installation of the libpth packages
>>on most distros.
>
> found out that a system provided libpth can be used.  Otoh if you build 
> the
> in-tree libpth, it shouldn't be installed, but built as a convenience 
> library,
> like libgo using libffi, or libgphobos using zlib.

 Hi Matthias,

 as far as I know Redhat doesn't support libpth-dev - therefore it was
 decided to include libpth in the gm2 tree and autodetect build/install
 it as necessary.
>>>
>>> That's ok, but then please don't install it as a system library.  that's 
>>> what
>>> convenience libraries are for (a libpth.a built with -fPIC, which you can 
>>> link
>>> against).
>>
>> I still think installing libpth is wrong, however currently all multilib
>> variants install into $libdir, and not into the system multilib dir, e.g. 
>> lib64,
>> lib32, ...  So the last multilib wins, and you end up with the wrong arch in
>> your $libdir.
> 
> Hi Matthias,
> 
> ah thanks for the explanation.  The multilib build system is still new
> to me.  So regarding the libpth issue, what happens if the native os
> does not provide libpth?  From your experience with building multilib
> and shared library packages on multiple platforms - what is the best
> solution :-)

I don't know about a best solution, but I contributed changes to
DRUNTIME_LIBRARIES_ZLIB in libphobos/m4/druntime/libraries.m4 and
gcc/doc/install.texi (--with-target-system-zlib).

Or you look at libffi/libgo, with libgo always linking the convenience libffi
library.

Matthias


Re: PR90723

2019-07-10 Thread Richard Sandiford
Prathamesh Kulkarni  writes:
> Hi,
> For following test-case:
>
> typedef double v4df __attribute__ ((vector_size (32)));
> void foo(v4df);
>
> int
> main ()
> {
>   volatile v4df x1;
>   x1 = (v4df) { 10.0, 20.0, 30.0, 40.0 };
>   foo (x1);
>   return 0;
> }
>
> Compiling with -msve-vector-bits=256, the compiler goes into infinite
> recursion and eventually segfaults due to stack overflow.
>
> This happens during expansion of:
>   x1.0_1 ={v} x1;
>
> aarch64_expand_sve_mem_move calls aarch64_emit_sve_pred_move with
> dest = (reg:VNx2DF 93) and
> src = (mem/u/c:VNx2DF
>(plus:DI (reg/f:DI 94) (const_int 32 [0x20])) [0  S32 A128])
>
> aarch64_emit_sve_pred_move calls expand_insn with above ops.
> Eventually we hit EXPAND_INPUT case in maybe_legitimize_operand for
> src (opno == 2)
>
> Since the operand is marked with volatile, and volatile_ok is set to false,
> insn_operand_matches return false and we call:
>   op->value = copy_to_mode_reg (mode, op->value);
>   break;
>
> copy_to_mode_reg however, creates a fresh register and calls emit_move_insn:
> rtx temp = gen_reg_rtx (mode);
> if (x != temp)
> emit_move_insn (temp, x);
>
> and we again end up in aarch64_emit_sve_pred_move, with dest assigned
> the new register and src remaining unchanged, and thus the cycle continues.
>
> As suggested by Richard, the attached patch temporarily sets volatile_ok to 
> true
> using RAII class volatile_ok_temp in aarch64_emit_sve_pred_move which
> avoids the recursion.
>
> Bootstrap + tested on x86_64-unknown-linux-gnu, aarch64-linux-gnu.
> Cross-testing with SVE in progress.
> OK to commit ?
>
> Thanks,
> Prathamesh
>
> 2019-07-09  Prathamesh Kulkarni  
>
>   PR target/90723
>   * recog.h (volatile_ok_temp): New class.
>   * config/aarch64/aarch64.c (aarch64_emit_sve_pred_move): Set
>   volatile_ok temporarily to true using volatile_ok_temp.
>   * expr.c (emit_block_move_via_cpymem): Likewise.
>   * optabs.c (maybe_legitimize_operand): Likewise. 

I'm the last person who should be suggesting names, but how about
temporary_volatile_ok?  Putting "temp" after the name IMO makes it
sound too much like it's describing the RAII object rather than the
value of volatile_ok itself.

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 5a923ca006b..50afba1a2b6 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3457,6 +3457,7 @@ aarch64_emit_sve_pred_move (rtx dest, rtx pred, rtx src)
>create_output_operand ([0], dest, mode);
>create_input_operand ([1], pred, GET_MODE(pred));
>create_input_operand ([2], src, mode);
> +  volatile_ok_temp v(true);

Missing space before "(".  Same for later uses.

> [...]
> diff --git a/gcc/recog.h b/gcc/recog.h
> index 75cbbdc10ad..8a8eaf7e0c3 100644
> --- a/gcc/recog.h
> +++ b/gcc/recog.h
> @@ -186,6 +186,22 @@ skip_alternative (const char *p)
>  /* Nonzero means volatile operands are recognized.  */
>  extern int volatile_ok;
>  
> +/* RAII class for temporarily setting volatile_ok.  */
> +
> +class volatile_ok_temp
> +{
> +public:
> +  volatile_ok_temp(int value): save_volatile_ok (volatile_ok)

Missing space before "(" and before ":".

> +  {
> +volatile_ok = value;
> +  }
> +
> +  ~volatile_ok_temp() { volatile_ok = save_volatile_ok; }

Missing space before "(".

> +
> +private:
> +  int save_volatile_ok;

For extra safety we should probably have a private copy constructor
(declaration only, no implementation).  We're still C++03 and so can't
use "= delete".

Thanks,
Richard


> +};
> +
>  /* Set by constrain_operands to the number of the alternative that
> matched.  */
>  extern int which_alternative;


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Gaius Mulley
Matthias Klose  writes:

> On 09.07.19 23:30, Matthias Klose wrote:
>> On 09.07.19 21:48, Gaius Mulley wrote:
>>> Matthias Klose  writes:
>>>
>  - libpth.{a,so} is installed in the system libdir, which
>conflicts with the installation of the libpth packages
>on most distros.

 found out that a system provided libpth can be used.  Otoh if you build the
 in-tree libpth, it shouldn't be installed, but built as a convenience 
 library,
 like libgo using libffi, or libgphobos using zlib.
>>>
>>> Hi Matthias,
>>>
>>> as far as I know Redhat doesn't support libpth-dev - therefore it was
>>> decided to include libpth in the gm2 tree and autodetect build/install
>>> it as necessary.
>> 
>> That's ok, but then please don't install it as a system library.  that's what
>> convenience libraries are for (a libpth.a built with -fPIC, which you can 
>> link
>> against).
>
> I still think installing libpth is wrong, however currently all multilib
> variants install into $libdir, and not into the system multilib dir, e.g. 
> lib64,
> lib32, ...  So the last multilib wins, and you end up with the wrong arch in
> your $libdir.

Hi Matthias,

ah thanks for the explanation.  The multilib build system is still new
to me.  So regarding the libpth issue, what happens if the native os
does not provide libpth?  From your experience with building multilib
and shared library packages on multiple platforms - what is the best
solution :-)


regards,
Gaius


[Darwin, PPC, committed] Collate the system library spec into one expression.

2019-07-10 Thread Iain Sandoe
There's no need to redefine this dependent on the target header (that only
works in the case that we have self-hosting which is less and less likely
for the older system versions).

Actually, what we need is for the correct library set to be used based on the
SDK(s) that can target the chosen system.  This is a collated version that
applies the constraints based on the system version being compiled for.

tested on powerpc-darwin9, 
applied to mainline,
thanks
Iain

gcc/

2019-07-10  Iain Sandoe  

* config/rs6000/darwin.h (LIB_SPEC): Collate this spec here.
* config/rs6000/darwin7.h (LIB_SPEC): Remove.
* config/rs6000/darwin8.h (LIB_SPEC): Remove.
(DEF_MIN_OSX_VERSION): New.

diff --git a/gcc/config/rs6000/darwin.h b/gcc/config/rs6000/darwin.h
index b0b5047..272cd45 100644
--- a/gcc/config/rs6000/darwin.h
+++ b/gcc/config/rs6000/darwin.h
@@ -86,6 +86,24 @@ extern int darwin_emit_picsym_stub;
 
 #define RS6000_DEFAULT_LONG_DOUBLE_SIZE 128
 
+/* Machine dependent libraries.
+   Include libmx when targeting Darwin 7.0 and above, but before libSystem,
+   since the functions are actually in libSystem but for 7.x compatibility
+   we want them to be looked for in libmx first.
+   Include libSystemStubs when compiling against 10.3 - 10.5 SDKs (we assume
+   this is the case when targetting these) - but not for 64-bit long double.
+   Don't do either for m64, the library is either a dummy or non-existent.
+*/
+
+#undef LIB_SPEC
+#define LIB_SPEC \
+"%{!static:\
+  %{!m64:%{!mlong-double-64:   \
+%{pg:%:version-compare(>< 10.3 10.5 mmacosx-version-min= 
-lSystemStubs_profile)} \
+%{!pg:%:version-compare(>< 10.3 10.5 mmacosx-version-min= -lSystemStubs)} \
+ %:version-compare(>< 10.3 10.4 mmacosx-version-min= -lmx)}}   \
+  -lSystem \
+}"
 
 /* We want -fPIC by default, unless we're using -static to compile for
the kernel or some such.  The "-faltivec" option should have been
diff --git a/gcc/config/rs6000/darwin7.h b/gcc/config/rs6000/darwin7.h
index d299074..6a3adc0 100644
--- a/gcc/config/rs6000/darwin7.h
+++ b/gcc/config/rs6000/darwin7.h
@@ -17,21 +17,11 @@ You should have received a copy of the GNU General Public 
License
 along with GCC; see the file COPYING3.  If not see
 .  */
 
-/* Machine dependent libraries.  Include libmx when compiling for
-   Darwin 7.0 and above, but before libSystem, since the functions are
-   actually in libSystem but for 7.x compatibility we want them to be
-   looked for in libmx first.  Include libmx by default because otherwise
-   libstdc++ isn't usable.  */
-
-#undef LIB_SPEC
-#define LIB_SPEC "%{!static:\
-  %:version-compare(!< 10.3 mmacosx-version-min= -lmx)\
-  -lSystem}"
-
 /* This generation of tools (specifically the archive tool) did not
export weak symbols from the TOC. */
 #undef TARGET_WEAK_NOT_IN_ARCHIVE_TOC
 #define TARGET_WEAK_NOT_IN_ARCHIVE_TOC 1
 
+/* Default to the last version, with most support for C++.  */
 #undef DEF_MIN_OSX_VERSION
 #define DEF_MIN_OSX_VERSION "10.3.9"
diff --git a/gcc/config/rs6000/darwin8.h b/gcc/config/rs6000/darwin8.h
index ca4ede2..ec5a8af 100644
--- a/gcc/config/rs6000/darwin8.h
+++ b/gcc/config/rs6000/darwin8.h
@@ -17,15 +17,5 @@ You should have received a copy of the GNU General Public 
License
 along with GCC; see the file COPYING3.  If not see
 .  */
 
-/* Machine dependent libraries.  Include libmx when compiling on
-   Darwin 7.0 and above, but before libSystem, since the functions are
-   actually in libSystem but for 7.x compatibility we want them to be
-   looked for in libmx first---but only do this if 7.x compatibility
-   is a concern, which it's not in 64-bit mode.  Include
-   libSystemStubs when compiling on (not necessarily for) 8.0 and
-   above and not 64-bit long double.  */
-
-#undef LIB_SPEC
-#define LIB_SPEC "%{!static:\
-  %{!mlong-double-64:%{pg:-lSystemStubs_profile;:-lSystemStubs}} \
-  %{!m64:%:version-compare(>< 10.3 10.4 mmacosx-version-min= -lmx)} -lSystem}"
+#undef DEF_MIN_OSX_VERSION
+#define DEF_MIN_OSX_VERSION "10.4"



Re: [PATCH] Add hints for slim dumping if fallthrough bb of jump isn't next bb

2019-07-10 Thread Richard Sandiford
"Kewen.Lin"  writes:
> Hi all,
>
> 6: NOTE_INSN_BASIC_BLOCK 2
>   
>12: r135:CC=cmp(r122:DI,0)
>13: pc={(r135:CC!=0)?L52:pc}
>   REG_DEAD r135:CC
>   REG_BR_PROB 1041558836
>31: L31:
>17: NOTE_INSN_BASIC_BLOCK 3
>
> The above RTL sequence is from pass doloop dumping with -fdump-rtl-all-slim, I
> misunderstood that: the fall through BB of BB 2 is BB 3, since BB 3 is placed
> just next to BB 2.  Then I found the contradiction that BB 3 will have some
> uninitialized regs if it's true.
>
> I can get the exact information with "-blocks" dumping and even detailed one
> with "-details".  But I'm thinking whether it's worth to giving some
> information on "-slim" dump (or more exactly without "-blocks") to avoid some
> confusion especially for new comers like me.

Yeah, agree the output is overly confusing.

> This patch is to add one line to hint what's the fallthrough BB if it's the
> one closely sitting, for example:
>
> 6: NOTE_INSN_BASIC_BLOCK 2
> 
>12: r135:CC=cmp(r122:DI,0)
>13: pc={(r135:CC!=0)?L52:pc}
>   REG_DEAD r135:CC
>   REG_BR_PROB 1041558836
> ;;  pc falls through to BB 10 
>31: L31:
>17: NOTE_INSN_BASIC_BLOCK 3
>
> Bootstrapped and regression test passed on powerpc64le-unknown-linux-gnu.
>
> Is it a reasonable patch? If yes, is it ok for trunk?

It looks really useful, but IMO we should either emit the hint for all
end-of-block insns with a surprising fallthrough edge, or -- if we
really do want to keep it specific to conditional jumps -- we should
replace rhs "pc" in the jump insn with something like "bb ".
Personally the former seems better to me (and should also be simpler).

>
>
> Thanks,
> Kewen
>
> -
>
> gcc/ChangeLog
>
> 2019-07-08  Kewen Lin  
>
>   * gcc/cfgrtl.c (print_rtl_with_bb): Check and call 
>   hint_if_pc_fall_through_not_next for jump insn with two successors.
>   (hint_if_pc_fall_through_not_next): New function.
>
> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index a1ca5992c41..928b9b0f691 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -2164,7 +2164,26 @@ rtl_dump_bb (FILE *outf, basic_block bb, int indent, 
> dump_flags_t flags)
>  }
>
>  }
> -
> +
> +/* For dumping without specifying basic blocks option, when we see PC is one 
> of
> +   jump targets, it's easy to misunderstand the next basic block is the
> +   fallthrough one, but it's not so true sometimes.  This function is to dump
> +   hints for the case where basic block of next insn isn't the fall through
> +   target.  */
> +
> +static void
> +hint_if_pc_fall_through_not_next (FILE *outf, basic_block bb)
> +{
> +  gcc_assert (outf);
> +  gcc_assert (EDGE_COUNT (bb->succs) >= 2);
> +  const rtx_insn *einsn = BB_END (bb);
> +  const rtx_insn *ninsn = NEXT_INSN (einsn);
> +  edge e = FALLTHRU_EDGE (bb);
> +  basic_block dest = e->dest;
> +  if (BB_HEAD (dest) != ninsn)
> +fprintf (outf, ";;  pc falls through to BB %d\n", dest->index);

Very minor, but I think the output would be more readable if the comment
were indented by the same amount as the register notes (like REG_DEAD above).
In lisp tradition I guess the comment marker would then be ";" rather
than ";;".

Thanks,
Richard

> +}
> +
>  /* Like dump_function_to_file, but for RTL.  Print out dataflow information
> for the start of each basic block.  FLAGS are the TDF_* masks documented
> in dumpfile.h.  */
> @@ -2255,6 +2274,14 @@ print_rtl_with_bb (FILE *outf, const rtx_insn 
> *rtx_first, dump_flags_t flags)
>   putc ('\n', outf);
> }
> }
> + else if (GET_CODE (tmp_rtx) == JUMP_INSN
> +  && GET_CODE (PATTERN (tmp_rtx)) == SET)
> +   {
> + bb = BLOCK_FOR_INSN (tmp_rtx);
> + const_rtx src = SET_SRC (PATTERN (tmp_rtx));
> + if (bb != NULL && GET_CODE (src) == IF_THEN_ELSE)
> +   hint_if_pc_fall_through_not_next (outf, bb);
> +   }
> }
>
>free (start);


Re: [PATCH] i386: Add AVX512 unaligned intrinsics

2019-07-10 Thread Uros Bizjak
On Wed, Jul 10, 2019 at 9:11 PM Sunil Pandey  wrote:
>
> Thanks Uros. I incorporated suggested changes in attached patch.
>
> --Sunil Pandey
>
> i386: Add AVX512 unaligned intrinsics
>
> __m512i _mm512_loadu_epi32( void * sa);
> __m512i _mm512_loadu_epi64( void * sa);
> void _mm512_storeu_epi32(void * d, __m512i a);
> void _mm256_storeu_epi32(void * d, __m256i a);
> void _mm_storeu_epi32(void * d, __m128i a);
> void _mm512_storeu_epi64(void * d, __m512i a);
> void _mm256_storeu_epi64(void * d, __m256i a);
> void _mm_storeu_epi64(void * d, __m128i a);
>
> Tested on x86-64.
>
> gcc/
>
> PR target/90980
> * config/i386/avx512fintrin.h (_mm512_loadu_epi32): New.
> (_mm512_loadu_epi64): Likewise.
> (_mm512_storeu_epi32): Likewise.
> (_mm512_storeu_epi64): Likewise.
> * config/i386/avx512vlintrin.h (_mm_storeu_epi32): New.
> (_mm256_storeu_epi32): Likewise.
> (_mm_storeu_epi64): Likewise.
> (_mm256_storeu_epi64): Likewise.
>
> gcc/testsuite/
>
> PR target/90980
> * gcc.target/i386/pr90980-1.c: New test.
> * gcc.target/i386/pr90980-2.c: Likewise.
> * gcc.target/i386/pr90980-3.c: Likewise.

Looks good, but please put new intrinsics nearby existing intrinsics,
so we will have e.g.:

_mm512_loadu_epi32
_mm512_mask_loadu_epi32
_mm512_maskz_loadu_epi32

and in similar way for other loads and stores.

Uros.

>
> On Tue, Jul 9, 2019 at 11:39 PM Uros Bizjak  wrote:
> >
> > On Tue, Jul 9, 2019 at 11:44 PM Sunil Pandey  wrote:
> > >
> > > __m512i _mm512_loadu_epi32( void * sa);
> > > __m512i _mm512_loadu_epi64( void * sa);
> > > void _mm512_storeu_epi32(void * d, __m512i a);
> > > void _mm256_storeu_epi32(void * d, __m256i a);
> > > void _mm_storeu_epi32(void * d, __m128i a);
> > > void _mm512_storeu_epi64(void * d, __m512i a);
> > > void _mm256_storeu_epi64(void * d, __m256i a);
> > > void _mm_storeu_epi64(void * d, __m128i a);
> > >
> > > Tested on x86-64.
> > >
> > > OK for trunk?
> > >
> > > --Sunil Pandey
> > >
> > >
> > > gcc/
> > >
> > > PR target/90980
> > > * config/i386/avx512fintrin.h (__v16si_u): New data type
> > > (__v8di_u): Likewise
> > > (_mm512_loadu_epi32): New.
> > > (_mm512_loadu_epi64): Likewise.
> > > (_mm512_storeu_epi32): Likewise.
> > > (_mm512_storeu_epi64): Likewise.
> > > * config/i386/avx512vlintrin.h (_mm_storeu_epi32): New.
> > > (_mm256_storeu_epi32): Likewise.
> > > (_mm_storeu_epi64): Likewise.
> > > (_mm256_storeu_epi64): Likewise.
> > >
> > > gcc/testsuite/
> > >
> > > PR target/90980
> > > * gcc.target/i386/avx512f-vmovdqu32-3.c: New test.
> > > * gcc.target/i386/avx512f-vmovdqu64-3.c: Likewise.
> > > * gcc.target/i386/pr90980-1.c: Likewise.
> > > * gcc.target/i386/pr90980-2.c: Likewise.
> >
> > +/* Internal data types for implementing unaligned version of intrinsics.  
> > */
> > +typedef int __v16si_u __attribute__ ((__vector_size__ (64),
> > +  __aligned__ (1)));
> > +typedef long long __v8di_u __attribute__ ((__vector_size__ (64),
> > +   __aligned__ (1)));
> >
> > You should define only one generic __m512i_u type, something like:
> >
> > typedef long long __m512i_u __attribute__ ((__vector_size__ (64),
> > __may_alias__, __aligned__ (1)));
> >
> > Please see avxintrin.h how __m256i_u is defined and used.
> >
> > Uros.


Re: [PATCH] i386: Add AVX512 unaligned intrinsics

2019-07-10 Thread Sunil Pandey
Thanks Uros. I incorporated suggested changes in attached patch.

--Sunil Pandey

i386: Add AVX512 unaligned intrinsics

__m512i _mm512_loadu_epi32( void * sa);
__m512i _mm512_loadu_epi64( void * sa);
void _mm512_storeu_epi32(void * d, __m512i a);
void _mm256_storeu_epi32(void * d, __m256i a);
void _mm_storeu_epi32(void * d, __m128i a);
void _mm512_storeu_epi64(void * d, __m512i a);
void _mm256_storeu_epi64(void * d, __m256i a);
void _mm_storeu_epi64(void * d, __m128i a);

Tested on x86-64.

gcc/

PR target/90980
* config/i386/avx512fintrin.h (_mm512_loadu_epi32): New.
(_mm512_loadu_epi64): Likewise.
(_mm512_storeu_epi32): Likewise.
(_mm512_storeu_epi64): Likewise.
* config/i386/avx512vlintrin.h (_mm_storeu_epi32): New.
(_mm256_storeu_epi32): Likewise.
(_mm_storeu_epi64): Likewise.
(_mm256_storeu_epi64): Likewise.

gcc/testsuite/

PR target/90980
* gcc.target/i386/pr90980-1.c: New test.
* gcc.target/i386/pr90980-2.c: Likewise.
* gcc.target/i386/pr90980-3.c: Likewise.

On Tue, Jul 9, 2019 at 11:39 PM Uros Bizjak  wrote:
>
> On Tue, Jul 9, 2019 at 11:44 PM Sunil Pandey  wrote:
> >
> > __m512i _mm512_loadu_epi32( void * sa);
> > __m512i _mm512_loadu_epi64( void * sa);
> > void _mm512_storeu_epi32(void * d, __m512i a);
> > void _mm256_storeu_epi32(void * d, __m256i a);
> > void _mm_storeu_epi32(void * d, __m128i a);
> > void _mm512_storeu_epi64(void * d, __m512i a);
> > void _mm256_storeu_epi64(void * d, __m256i a);
> > void _mm_storeu_epi64(void * d, __m128i a);
> >
> > Tested on x86-64.
> >
> > OK for trunk?
> >
> > --Sunil Pandey
> >
> >
> > gcc/
> >
> > PR target/90980
> > * config/i386/avx512fintrin.h (__v16si_u): New data type
> > (__v8di_u): Likewise
> > (_mm512_loadu_epi32): New.
> > (_mm512_loadu_epi64): Likewise.
> > (_mm512_storeu_epi32): Likewise.
> > (_mm512_storeu_epi64): Likewise.
> > * config/i386/avx512vlintrin.h (_mm_storeu_epi32): New.
> > (_mm256_storeu_epi32): Likewise.
> > (_mm_storeu_epi64): Likewise.
> > (_mm256_storeu_epi64): Likewise.
> >
> > gcc/testsuite/
> >
> > PR target/90980
> > * gcc.target/i386/avx512f-vmovdqu32-3.c: New test.
> > * gcc.target/i386/avx512f-vmovdqu64-3.c: Likewise.
> > * gcc.target/i386/pr90980-1.c: Likewise.
> > * gcc.target/i386/pr90980-2.c: Likewise.
>
> +/* Internal data types for implementing unaligned version of intrinsics.  */
> +typedef int __v16si_u __attribute__ ((__vector_size__ (64),
> +  __aligned__ (1)));
> +typedef long long __v8di_u __attribute__ ((__vector_size__ (64),
> +   __aligned__ (1)));
>
> You should define only one generic __m512i_u type, something like:
>
> typedef long long __m512i_u __attribute__ ((__vector_size__ (64),
> __may_alias__, __aligned__ (1)));
>
> Please see avxintrin.h how __m256i_u is defined and used.
>
> Uros.


0001-i386-Add-AVX512-unaligned-intrinsics.patch
Description: Binary data


[PATCH, GCC, AArch64] Enable Transactional Memory Extension

2019-07-10 Thread Sudakshina Das
Hi

This patch enables the new Transactional Memory Extension announced 
recently as part of Arm's new architecture technologies.
We introduce a new optional extension "tme" to enable this. The 
following instructions are part of the extension:
* tstart 
* ttest 
* tcommit
* tcancel #
The documentation for the above can be found here:
https://developer.arm.com/docs/ddi0602/latest/base-instructions-alphabetic-order

We have also added ACLE intrinsics for the instructions above according to:
https://developer.arm.com/docs/101028/latest/transactional-memory-extension-tme-intrinsics

Builds and regression tested on aarch64-none-linux-gnu and added new 
tests for the new instructions.

Is this okay for trunk?

Thanks
Sudi

*** gcc/ChangeLog ***

2019-xx-xx  Sudakshina Das  

* config/aarch64/aarch64-builtins.c (enum aarch64_builtins): Add
AARCH64_TME_BUILTIN_TSTART, AARCH64_TME_BUILTIN_TCOMMIT,
AARCH64_TME_BUILTIN_TTEST and AARCH64_TME_BUILTIN_TCANCEL.
(aarch64_init_tme_builtins): New.
(aarch64_init_builtins): Call aarch64_init_tme_builtins.
(aarch64_expand_builtin_tme): New.
(aarch64_expand_builtin): Handle TME builtins.
* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Define
__ARM_FEATURE_TME when enabled.
* config/aarch64/aarch64-option-extensions.def: Add "tme".
* config/aarch64/aarch64.h (AARCH64_FL_TME, AARCH64_ISA_TME): New.
(TARGET_TME): New.
* config/aarch64/aarch64.md (define_c_enum "unspec"): Add UNSPEC_TTEST.
(define_c_enum "unspecv"): Add UNSPECV_TSTART, UNSPECV_TCOMMIT and
UNSPECV_TCANCEL.
(tstart, ttest, tcommit, tcancel): New instructions.
* config/aarch64/arm_acle.h (__tstart, __tcommit): New.
(__tcancel, __ttest): New.
(_TMFAILURE_REASON, _TMFAILURE_RTRY, _TMFAILURE_CNCL): New macro.
(_TMFAILURE_MEM, _TMFAILURE_IMP, _TMFAILURE_ERR): Likewise.
(_TMFAILURE_SIZE, _TMFAILURE_NEST, _TMFAILURE_DBG): Likewise.
(_TMFAILURE_INT, _TMFAILURE_TRIVIAL): Likewise.
* config/arm/types.md: Add new tme type attr.
* doc/invoke.texi: Document "tme".

*** gcc/testsuite/ChangeLog ***

2019-xx-xx  Sudakshina Das  

* gcc.target/aarch64/acle/tme.c: New test.
* gcc.target/aarch64/pragma_cpp_predefs_2.c: New test.
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 549a6c249243372eacb5d29923b5d1abce4ac79a..16c1d42ea2be0f477692be592e30ba8ce27f05a7 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -438,6 +438,11 @@ enum aarch64_builtins
   /* Special cased Armv8.3-A Complex FMA by Lane quad Builtins.  */
   AARCH64_SIMD_FCMLA_LANEQ_BUILTIN_BASE,
   AARCH64_SIMD_FCMLA_LANEQ_BUILTINS
+  /* TME builtins.  */
+  AARCH64_TME_BUILTIN_TSTART,
+  AARCH64_TME_BUILTIN_TCOMMIT,
+  AARCH64_TME_BUILTIN_TTEST,
+  AARCH64_TME_BUILTIN_TCANCEL,
   AARCH64_BUILTIN_MAX
 };
 
@@ -1067,6 +1072,35 @@ aarch64_init_pauth_hint_builtins (void)
 			NULL_TREE);
 }
 
+/* Initialize the transactional memory extension (TME) builtins.  */
+static void
+aarch64_init_tme_builtins (void)
+{
+  tree ftype_uint64_void
+= build_function_type_list (uint64_type_node, NULL);
+  tree ftype_void_void
+= build_function_type_list (void_type_node, NULL);
+  tree ftype_void_uint64
+= build_function_type_list (void_type_node, uint64_type_node, NULL);
+
+  aarch64_builtin_decls[AARCH64_TME_BUILTIN_TSTART]
+= add_builtin_function ("__builtin_aarch64_tstart", ftype_uint64_void,
+			AARCH64_TME_BUILTIN_TSTART, BUILT_IN_MD,
+			NULL, NULL_TREE);
+  aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
+= add_builtin_function ("__builtin_aarch64_ttest", ftype_uint64_void,
+			AARCH64_TME_BUILTIN_TTEST, BUILT_IN_MD,
+			NULL, NULL_TREE);
+  aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
+= add_builtin_function ("__builtin_aarch64_tcommit", ftype_void_void,
+			AARCH64_TME_BUILTIN_TCOMMIT, BUILT_IN_MD,
+			NULL, NULL_TREE);
+  aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
+= add_builtin_function ("__builtin_aarch64_tcancel", ftype_void_uint64,
+			AARCH64_TME_BUILTIN_TCANCEL, BUILT_IN_MD,
+			NULL, NULL_TREE);
+}
+
 void
 aarch64_init_builtins (void)
 {
@@ -1104,6 +1138,9 @@ aarch64_init_builtins (void)
  register them.  */
   if (!TARGET_ILP32)
 aarch64_init_pauth_hint_builtins ();
+
+  if (TARGET_TME)
+aarch64_init_tme_builtins ();
 }
 
 tree
@@ -1507,6 +1544,47 @@ aarch64_expand_fcmla_builtin (tree exp, rtx target, int fcode)
   return target;
 }
 
+/* Function to expand an expression EXP which calls one of the Transactional
+   Memory Extension (TME) builtins FCODE with the result going to TARGET.  */
+static rtx
+aarch64_expand_builtin_tme (int fcode, tree exp, rtx target)
+{
+  switch (fcode)
+{
+case AARCH64_TME_BUILTIN_TSTART:
+  target = gen_reg_rtx 

Re: [PATCH], PowerPC, Patch #6, Create pc-relative addressing insns

2019-07-10 Thread Segher Boessenkool
On Wed, Jul 10, 2019 at 01:56:07PM -0400, Michael Meissner wrote:
> On Wed, Jul 10, 2019 at 12:11:49PM -0500, Segher Boessenkool wrote:
> > On Tue, Jul 09, 2019 at 06:30:00PM -0400, Michael Meissner wrote:
> > > @@ -8760,12 +8762,34 @@ rs6000_cannot_force_const_mem (machine_m
> > >  static bool
> > >  use_toc_relative_ref (rtx sym, machine_mode mode)
> > >  {
> > > -  return ((constant_pool_expr_p (sym)
> > > -&& ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> > > -get_pool_mode (sym)))
> > > -   || (TARGET_CMODEL == CMODEL_MEDIUM
> > > -   && SYMBOL_REF_LOCAL_P (sym)
> > > -   && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
> > > +  if (!SYMBOL_REF_P (sym) || TARGET_PCREL || !TARGET_TOC)
> > > +return false;
> > 
> > Why the SYMBOL_REF test?  The original didn't have any.  But its comment
> > says
> >   /* Return true iff the given SYMBOL_REF refers to a constant pool entry
> >  that we have put in the TOC, or for cmodel=medium, if the SYMBOL_REF
> >  can be addressed relative to the toc pointer.  */
> > so perhaps you want an assert instead.
> 
> The only two callers had a test for SYMBOL_REF_P.  By moving it into the
> function, it made the call to the function one line instead of two.  But I can
> go back to the previous method.

It makes more sense with the param as a symbol always (the function
comment says it is, too, and the "sym" name suggests it is).  So yes,
please go back to that.  Reducing number of lines of code isn't a goal;
reducing complexity is, reducing astonishment.

> > > +
> > > +  if (constant_pool_expr_p (sym)
> > > +  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> > > +   get_pool_mode (sym)))
> > > +return true;
> > > +
> > > +  return (TARGET_CMODEL == CMODEL_MEDIUM && SYMBOL_REF_LOCAL_P (sym)
> > > +   && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT);
> > 
> > Please don't put disparate things on one line, it was fine before.
> 
> I'm not sure what you mean, I was just trying to break up a long if statement.

"TARGET_CMODEL == CMODEL_MEDIUM" and "SYMBOL_REF_LOCAL_P (sym)" are quite
different things.

> > > -static GTY(()) alias_set_type set = -1;
> > > +/* Return the alias set for constants stored in either the TOC or via
> > > +   pc-relative addressing.  */
> > > +static GTY(()) alias_set_type data_alias_set = -1;
> > 
> > Please just make a separate alias set for pcrel.  The new name isn't as
> > bad as just "set" :-), but "data"?  That's not great either.  Conflating
> > toc and pcrel isn't a good thing.
> > 
> > (Variables don't "return" anything btw).
> > 
> > >  alias_set_type
> > > -get_TOC_alias_set (void)
> > > +get_data_alias_set (void)
> > 
> > This name is much worse than the old one.  Just make separate functions
> > for TOC and for pcrel?  Or is there any reason you want to share them?
> 
> Well in theory, an object file that contains some functions wtih pc-relative
> and some with TOC (due to #pragma target or attribute), using the same data 
> set
> would flag that they are related, but I imagine in practice the two uses don't
> mix.  It was more of a hangover from trying to have one function to create 
> both
> addressing forms.

I think it would make things quite a bit simpler if you split them up.
Could you please try that?


Segher


Re: [PATCH], PowerPC, Patch #7, Split up SIGNED_34BIT and SIGNED_16BIT macros

2019-07-10 Thread Segher Boessenkool
On Tue, Jul 09, 2019 at 07:46:26PM -0400, Michael Meissner wrote:
> This patch splits up the macros SIGNED_16BIT_OFFSET_P and 
> SIGNED_34BIT_OFFSET_P
> into two separate macros as you asked for previously in private mail.  The 
> main
> macros:
> 
>   SIGNED_16BIT_OFFSET_P
>   SIGNED_34BIT_OFFSET_P
> 
> only take one argument, and that is the offset that is being tested.  The new
> macros:
> 
>   SIGNED_16BIT_OFFSET_EXTRA_P
>   SIGNED_34BIT_OFFSET_EXTRA_P
> 
> Retain the two arguments that the current macros have.  It is useful when the
> functions that are validating addresses that might be split (such as the two
> doubles in __ibm128) to verify that all addresses in the range of offset to
> offset + extra are valid 16 or 34-bit offsets.  I have changed the existing
> uses of these macros.

This is okay for trunk.  Thanks Mike.


Segher


>   * config/rs6000/predicates.md (cint34_operand): Update
>   SIGNED_34BIT_OFFSET_P call.
>   (pcrel_address): Update SIGNED_34BIT_OFFSET_P call.
>   (pcrel_external_address): Update SIGNED_34BIT_OFFSET_P call.
>   * config/rs6000/rs6000.c (rs6000_prefixed_address): Update
>   SIGNED_16BIT_OFFSET_P and SIGNED_34BIT_OFFSET_P calls.
>   * config/rs6000/rs6000.h (SIGNED_16BIT_OFFSET_P): Remove EXTRA
>   argument.
>   (SIGNED_34BIT_OFFSET_P): Remove EXTRA argument.
>   (SIGNED_16BIT_OFFSET_EXTRA_P): New macro, like
>   SIGNED_16BIT_OFFSET_P with an EXTRA argument.
>   (SIGNED_34BIT_OFFSET_EXTRA_P): New macro, like
>   SIGNED_34BIT_OFFSET_P with an EXTRA argument.


Re: [wwwdocs] Introducing C++ DR status table

2019-07-10 Thread Marek Polacek
On Wed, Jul 10, 2019 at 02:20:24PM -0400, Nathan Sidwell wrote:
> On 7/8/19 1:43 PM, Marek Polacek wrote:
> > For a long time I wished we had a table documenting the status of C++ defect
> > reports in the C++ FE, like clang has [1].  I finally got around to tackling
> > this bad boy and created 
> > and will now commit the attached patch.
> > 
> > The table was created by an awk script which processed
> >  and then I made
> > a lot of manual changes.  I also searched through 3000 C++ PRs and linked
> > various PRs and changed the status of a few DRs.  For this table to be
> > useful it needs to be much more complete, but this is a good start.
> > 
> > I'm not going to force anyone's hand to go over that list and update random 
> > DRs
> > (though *any* help would be greatly appreciated; even a list of DR #s that 
> > you
> > know are fixed would be useful), but please, when you fix a DR, reflect that
> > in the table (or let me know and I'll update it for ya).  I want to see more
> > green!
> 
> thanks for putting this together.  Is it worth including the NAD entries? Or
> perhaps not marking them as unknown status?

Good point.  I've been meaning to do a pass over NADs and change their status
to N/A or something.  But I guess it'd be prudent to still check whether the
compiler does the right thing, which is often non-trivial.  I think we should
include them in the list so that there aren't any numbers missing.

Thanks for taking a look!

Marek


Re: [wwwdocs] Introducing C++ DR status table

2019-07-10 Thread Nathan Sidwell

On 7/8/19 1:43 PM, Marek Polacek wrote:

For a long time I wished we had a table documenting the status of C++ defect
reports in the C++ FE, like clang has [1].  I finally got around to tackling
this bad boy and created 
and will now commit the attached patch.

The table was created by an awk script which processed
 and then I made
a lot of manual changes.  I also searched through 3000 C++ PRs and linked
various PRs and changed the status of a few DRs.  For this table to be
useful it needs to be much more complete, but this is a good start.

I'm not going to force anyone's hand to go over that list and update random DRs
(though *any* help would be greatly appreciated; even a list of DR #s that you
know are fixed would be useful), but please, when you fix a DR, reflect that
in the table (or let me know and I'll update it for ya).  I want to see more
green!


thanks for putting this together.  Is it worth including the NAD 
entries? Or perhaps not marking them as unknown status?


nathan

--
Nathan Sidwell


Go patch committed: Finalize methods when importing types

2019-07-10 Thread Ian Lance Taylor
This patch to the Go frontend by Than McIntosh changes the compiler to
be more aggressive about finalizing methods on imported types, to
avoid problems with interface types that are imported but remain
unreachable until a later stage in the compilation.

The normal pattern prior to this change was that the import process
would leave imported interface types alone, and rely on
Gogo::finalize_methods to locate and finalize all interface types at a
later point.  This way of doing things was not working in all cases
due to the fact that we can import an interface type that is only
reachable from the body of an inlinable function, meaning that we
can't "find" the type during the methods finalize phase.

The importer's Import::read_types() now makes a pass over all imported
types to finalize methods on any newly imported type, which takes care
of the issue.

New test case for this problem in https://golang.org/cl/185517.

This fixes https://golang.org/issue/33013.

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

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 273359)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-170ecdf6b2eab8aac2b8c852fa95d3c36d6bf604
+ec754ff4617d564d3dc377121ea9ac5e55f6535a
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc   (revision 273307)
+++ gcc/go/gofrontend/gogo.cc   (working copy)
@@ -3422,24 +3422,6 @@ Gogo::create_function_descriptors()
   this->traverse();
 }
 
-// Look for interface types to finalize methods of inherited
-// interfaces.
-
-class Finalize_methods : public Traverse
-{
- public:
-  Finalize_methods(Gogo* gogo)
-: Traverse(traverse_types),
-  gogo_(gogo)
-  { }
-
-  int
-  type(Type*);
-
- private:
-  Gogo* gogo_;
-};
-
 // Finalize the methods of an interface type.
 
 int
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 273307)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -3556,6 +3556,24 @@ class Traverse
   Expressions_seen* expressions_seen_;
 };
 
+// This class looks for interface types to finalize methods of inherited
+// interfaces.
+
+class Finalize_methods : public Traverse
+{
+ public:
+  Finalize_methods(Gogo* gogo)
+: Traverse(traverse_types),
+  gogo_(gogo)
+  { }
+
+  int
+  type(Type*);
+
+ private:
+  Gogo* gogo_;
+};
+
 // A class which makes it easier to insert new statements before the
 // current statement during a traversal.
 
Index: gcc/go/gofrontend/import.cc
===
--- gcc/go/gofrontend/import.cc (revision 273307)
+++ gcc/go/gofrontend/import.cc (working copy)
@@ -290,10 +290,16 @@ Import::Import(Stream* stream, Location
   : gogo_(NULL), stream_(stream), location_(location), package_(NULL),
 add_to_globals_(false), packages_(), type_data_(), type_pos_(0),
 type_offsets_(), builtin_types_((- SMALLEST_BUILTIN_CODE) + 1),
-types_(), version_(EXPORT_FORMAT_UNKNOWN)
+types_(), finalizer_(NULL), version_(EXPORT_FORMAT_UNKNOWN)
 {
 }
 
+Import::~Import()
+{
+  if (this->finalizer_ != NULL)
+delete this->finalizer_;
+}
+
 // Import the data in the associated stream.
 
 Package*
@@ -672,9 +678,40 @@ Import::read_types()
this->gogo_->add_named_type(nt);
 }
 
+  // Finalize methods for any imported types. This is done after most of
+  // read_types() is complete so as to avoid method finalization of a type
+  // whose methods refer to types that are only partially read in.
+  // See issue #33013 for more on why this is needed.
+  this->finalize_methods();
+
   return true;
 }
 
+void
+Import::finalize_methods()
+{
+  if (this->finalizer_ == NULL)
+this->finalizer_ = new Finalize_methods(gogo_);
+  Unordered_set(Type*) real_for_named;
+  for (size_t i = 1; i < this->types_.size(); i++)
+{
+  Type* type = this->types_[i];
+  if (type != NULL && type->named_type() != NULL)
+{
+  this->finalizer_->type(type);
+  real_for_named.insert(type->named_type()->real_type());
+}
+}
+  for (size_t i = 1; i < this->types_.size(); i++)
+{
+  Type* type = this->types_[i];
+  if (type != NULL
+  && type->named_type() == NULL
+  && real_for_named.find(type) == real_for_named.end())
+this->finalizer_->type(type);
+}
+}
+
 // Import a constant.
 
 void
Index: gcc/go/gofrontend/import.h
===
--- gcc/go/gofrontend/import.h  (revision 273307)
+++ gcc/go/gofrontend/import.h  (working copy)
@@ -20,6 +20,7 @@ class Expression;
 class Import_function_body;
 class Temporary_statement;

Go patch committed: Add break label in 1,2 case select statement lowering

2019-07-10 Thread Ian Lance Taylor
This patch by Cherry Zhang adds a break label when lowering a select
statement with one or two cases.  The earlier change
https://golang.org/cl/184998
(https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00322.html) added
optimizations for one- and two-case select statements.  But it didn't
handle break statement in the select case correctly.  Specifically, it
didn't add the label definition, so it could result in a dangling
goto.  This patch fixes this, by adding the label definition.  A test
case is https://golang.org/cl/185520.  Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 273307)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-7a8e10be0ddb8909ce25a264d03b24cee4df60cc
+170ecdf6b2eab8aac2b8c852fa95d3c36d6bf604
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 273307)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -5855,6 +5855,10 @@ Select_statement::lower_one_case(Block*
 Statement::make_block_statement(scase.statements(), scase.location());
   b->add_statement(bs);
 
+  Statement* label =
+Statement::make_unnamed_label_statement(this->break_label());
+  b->add_statement(label);
+
   this->is_lowered_ = true;
   return Statement::make_block_statement(b, loc);
 }
@@ -5958,6 +5962,10 @@ Select_statement::lower_two_case(Block*
 Statement::make_if_statement(call, bchan, defcase.statements(), loc);
   b->add_statement(ifs);
 
+  Statement* label =
+Statement::make_unnamed_label_statement(this->break_label());
+  b->add_statement(label);
+
   this->is_lowered_ = true;
   return Statement::make_block_statement(b, loc);
 }


Re: [PATCH], PowerPC, Patch #6, Create pc-relative addressing insns

2019-07-10 Thread Michael Meissner
On Wed, Jul 10, 2019 at 12:11:49PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Tue, Jul 09, 2019 at 06:30:00PM -0400, Michael Meissner wrote:
> > @@ -8760,12 +8762,34 @@ rs6000_cannot_force_const_mem (machine_m
> >  static bool
> >  use_toc_relative_ref (rtx sym, machine_mode mode)
> >  {
> > -  return ((constant_pool_expr_p (sym)
> > -  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> > -  get_pool_mode (sym)))
> > - || (TARGET_CMODEL == CMODEL_MEDIUM
> > - && SYMBOL_REF_LOCAL_P (sym)
> > - && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
> > +  if (!SYMBOL_REF_P (sym) || TARGET_PCREL || !TARGET_TOC)
> > +return false;
> 
> Why the SYMBOL_REF test?  The original didn't have any.  But its comment
> says
>   /* Return true iff the given SYMBOL_REF refers to a constant pool entry
>  that we have put in the TOC, or for cmodel=medium, if the SYMBOL_REF
>  can be addressed relative to the toc pointer.  */
> so perhaps you want an assert instead.

The only two callers had a test for SYMBOL_REF_P.  By moving it into the
function, it made the call to the function one line instead of two.  But I can
go back to the previous method.

> > +
> > +  if (constant_pool_expr_p (sym)
> > +  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> > + get_pool_mode (sym)))
> > +return true;
> > +
> > +  return (TARGET_CMODEL == CMODEL_MEDIUM && SYMBOL_REF_LOCAL_P (sym)
> > + && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT);
> 
> Please don't put disparate things on one line, it was fine before.

I'm not sure what you mean, I was just trying to break up a long if statement.

> > +/* Return true iff the given SYMBOL_REF refers to a constant pool entry 
> > that we
> > +   have put in the pc-relative constant area, or for cmodel=medium, if the
> > +   SYMBOL_REF can be addressed via pc-relative instructions.  */
> > +
> > +static bool
> > +use_pc_relative_ref (rtx sym)
> > +{
> > +  if (!SYMBOL_REF_P (sym) || !TARGET_PCREL)
> > +return false;
> 
> Same here, assert please.  Or nothing, it will ICE not much later anyway.
> But don't silently return if something violates the call contract.

Again, this was meant to simplify the call.

> > -static GTY(()) alias_set_type set = -1;
> > +/* Return the alias set for constants stored in either the TOC or via
> > +   pc-relative addressing.  */
> > +static GTY(()) alias_set_type data_alias_set = -1;
> 
> Please just make a separate alias set for pcrel.  The new name isn't as
> bad as just "set" :-), but "data"?  That's not great either.  Conflating
> toc and pcrel isn't a good thing.
> 
> (Variables don't "return" anything btw).
> 
> >  
> >  alias_set_type
> > -get_TOC_alias_set (void)
> > +get_data_alias_set (void)
> 
> This name is much worse than the old one.  Just make separate functions
> for TOC and for pcrel?  Or is there any reason you want to share them?

Well in theory, an object file that contains some functions wtih pc-relative
and some with TOC (due to #pragma target or attribute), using the same data set
would flag that they are related, but I imagine in practice the two uses don't
mix.  It was more of a hangover from trying to have one function to create both
addressing forms.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797



Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Matthias Klose
On 09.07.19 23:30, Matthias Klose wrote:
> On 09.07.19 21:48, Gaius Mulley wrote:
>> Matthias Klose  writes:
>>
  - libpth.{a,so} is installed in the system libdir, which
conflicts with the installation of the libpth packages
on most distros.
>>>
>>> found out that a system provided libpth can be used.  Otoh if you build the
>>> in-tree libpth, it shouldn't be installed, but built as a convenience 
>>> library,
>>> like libgo using libffi, or libgphobos using zlib.
>>
>> Hi Matthias,
>>
>> as far as I know Redhat doesn't support libpth-dev - therefore it was
>> decided to include libpth in the gm2 tree and autodetect build/install
>> it as necessary.
> 
> That's ok, but then please don't install it as a system library.  that's what
> convenience libraries are for (a libpth.a built with -fPIC, which you can link
> against).

I still think installing libpth is wrong, however currently all multilib
variants install into $libdir, and not into the system multilib dir, e.g. lib64,
lib32, ...  So the last multilib wins, and you end up with the wrong arch in
your $libdir.

Matthias




Re: [PATCH], PowerPC, Patch #6, Create pc-relative addressing insns

2019-07-10 Thread Segher Boessenkool
Hi Mike,

On Tue, Jul 09, 2019 at 06:30:00PM -0400, Michael Meissner wrote:
> @@ -8760,12 +8762,34 @@ rs6000_cannot_force_const_mem (machine_m
>  static bool
>  use_toc_relative_ref (rtx sym, machine_mode mode)
>  {
> -  return ((constant_pool_expr_p (sym)
> -&& ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> -get_pool_mode (sym)))
> -   || (TARGET_CMODEL == CMODEL_MEDIUM
> -   && SYMBOL_REF_LOCAL_P (sym)
> -   && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
> +  if (!SYMBOL_REF_P (sym) || TARGET_PCREL || !TARGET_TOC)
> +return false;

Why the SYMBOL_REF test?  The original didn't have any.  But its comment
says
  /* Return true iff the given SYMBOL_REF refers to a constant pool entry
 that we have put in the TOC, or for cmodel=medium, if the SYMBOL_REF
 can be addressed relative to the toc pointer.  */
so perhaps you want an assert instead.

> +
> +  if (constant_pool_expr_p (sym)
> +  && ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> +   get_pool_mode (sym)))
> +return true;
> +
> +  return (TARGET_CMODEL == CMODEL_MEDIUM && SYMBOL_REF_LOCAL_P (sym)
> +   && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT);

Please don't put disparate things on one line, it was fine before.

> +/* Return true iff the given SYMBOL_REF refers to a constant pool entry that 
> we
> +   have put in the pc-relative constant area, or for cmodel=medium, if the
> +   SYMBOL_REF can be addressed via pc-relative instructions.  */
> +
> +static bool
> +use_pc_relative_ref (rtx sym)
> +{
> +  if (!SYMBOL_REF_P (sym) || !TARGET_PCREL)
> +return false;

Same here, assert please.  Or nothing, it will ICE not much later anyway.
But don't silently return if something violates the call contract.

> -static GTY(()) alias_set_type set = -1;
> +/* Return the alias set for constants stored in either the TOC or via
> +   pc-relative addressing.  */
> +static GTY(()) alias_set_type data_alias_set = -1;

Please just make a separate alias set for pcrel.  The new name isn't as
bad as just "set" :-), but "data"?  That's not great either.  Conflating
toc and pcrel isn't a good thing.

(Variables don't "return" anything btw).

>  
>  alias_set_type
> -get_TOC_alias_set (void)
> +get_data_alias_set (void)

This name is much worse than the old one.  Just make separate functions
for TOC and for pcrel?  Or is there any reason you want to share them?


Segher


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Matthias Klose
On 09.07.19 23:35, Matthias Klose wrote:
> On 08.07.19 23:19, Matthias Klose wrote:
>> On 14.06.19 15:09, Gaius Mulley wrote:
>>>
>>> Hello,
>>>
>>> here is version two of the patches which introduce Modula-2 into the
>>> GCC trunk.  The patches include:
>>>
>>>   (*)  a patch to allow all front ends to register a lang spec function.
>>>(included are patches for all front ends to provide an empty
>>> callback function).
>>>   (*)  patch diffs to allow the Modula-2 front end driver to be
>>>built using GCC Makefile and friends.
>>>
>>> The compressed tarball includes:
>>>
>>>   (*)  gcc/m2  (compiler driver and lang-spec stuff for Modula-2).
>>>Including the need for registering lang spec functions.
>>>   (*)  gcc/testsuite/gm2  (a Modula-2 dejagnu test to ensure that
>>>the gm2 driver is built and can understands --version).
>>>
>>> These patches have been re-written after taking on board the comments
>>> found in this thread:
>>>
>>>https://gcc.gnu.org/ml/gcc-patches/2013-11/msg02620.html
>>>
>>> it is a revised patch set from:
>>>
>>>https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00220.html
>>>
>>> I've run make bootstrap and run the regression tests on trunk and no
>>> extra failures occur for all languages touched in the ChangeLog.
>>>
>>> I'm currently tracking gcc trunk and gcc-9 with gm2 (which works well
>>> with amd64/arm64/i386) - these patches are currently simply for the
>>> driver to minimise the patch size.  There are also > 1800 tests in a
>>> dejagnu testsuite for gm2 which can be included at some future time.
>>
>> I had a look at the GCC 9 version of the patches, with a build including a 
>> make
>> install. Some comments:
> 
> [...]
> 
>>  - The internal tools in the gcclibdir are installed twice, with
>>both vanilla names and prefixed/suffixed names.
>> The installed tree:
>>
>> ./usr/bin
>> ./usr/bin/x86_64-linux-gnu-gm2-9
>> ./usr/bin/x86_64-linux-gnu-gm2m-9
>> ./usr/lib/gcc/x86_64-linux-gnu
>> ./usr/lib/gcc/x86_64-linux-gnu/9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/cc1gm2
>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2l
>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lcc
>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lgen
>> ./usr/lib/gcc/x86_64-linux-gnu/9/gm2lorder
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-cc1gm2-9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2l-9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lcc-9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lgen-9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lorder-9
>> ./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2m-9
> 
> With a fresh build, configured with
> 
>  --program-suffix=-9
>  --program-prefix=x86_64-linux-gnu-
> 
> the latter set of internal binaries is installed, while I would expect just 
> the
> un-pre/post-fixed tool names.

fixed by the attached patch.



diff --git a/gcc-versionno/gcc/gm2/Make-lang.in b/gcc-versionno/gcc/gm2/Make-lang.in
index 6fc4ee84..81f5cbbc 100644
--- a/gcc-versionno/gcc/gm2/Make-lang.in
+++ b/gcc-versionno/gcc/gm2/Make-lang.in
@@ -373,12 +373,11 @@ gm2.install-common: installdirs $(GM2_LINK_TOOLS_BOOT) \
   chmod a+x $(DESTDIR)$(bindir)/$(GM2_INSTALL_NAME)$(exeext); \
   for tool in stage1/gm2/cc1gm2$(exeext) \
   $(GM2_LINK_TOOLS_BOOT) stage1/gm2/gm2m$(exeext) ; do \
-toolbase=`basename $$tool` ; \
-tool_transformed_name=`echo $$toolbase|sed '$(program_transform_name)'`; \
+tool_name=`basename $$tool` ; \
 if [ -f $$tool ]; then \
-  rm -f $(DESTDIR)$(libexecsubdir)/$$tool_transformed_name; \
-  $(INSTALL_PROGRAM) $$tool $(DESTDIR)$(libexecsubdir)/$$tool_transformed_name; \
-  chmod a+x $(DESTDIR)$(libexecsubdir)/$$tool_transformed_name; \
+  rm -f $(DESTDIR)$(libexecsubdir)/$$tool_name; \
+  $(INSTALL_PROGRAM) $$tool $(DESTDIR)$(libexecsubdir)/$$tool_name; \
+  chmod a+x $(DESTDIR)$(libexecsubdir)/$$tool_name; \
 else \
   echo "odd cannot find $$tool" ; \
 fi ; \


Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)

2019-07-10 Thread Segher Boessenkool
On Wed, Jul 10, 2019 at 05:23:28PM +0100, Jozef Lawrynowicz wrote:
> On Tue, 9 Jul 2019 16:36:46 -0500
> Segher Boessenkool  wrote:
> > But it is data, not a constant, so it does not allow optimising based
> > on its potentially constant value?  Where "potentially" in this case
> > means "always" :-/
> 
> I can think of two alternatives so far which should get around this
> optimization issue you point out.
> 
> 1. a regular target macro defined in tm.texi such as
> TARGET_CASE_INSENSITIVE_REGISTER_NAMES, defined to 0 by default. Overriden
> with 1 for MSP430 

That is how most other things work, yeah.

For this particular case the impact of using DEFHOOKPOD instead of a
target macro would be minimal, of course, register name compare are not
executed so very often; but we cannot convert all the other target
macros this way.


Segher


Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)

2019-07-10 Thread Jozef Lawrynowicz
On Tue, 9 Jul 2019 16:36:46 -0500
Segher Boessenkool  wrote:

> On Tue, Jul 09, 2019 at 10:16:31PM +0100, Jozef Lawrynowicz wrote:
> > On Mon, 8 Jul 2019 16:42:15 -0500
> > Segher Boessenkool  wrote:
> >   
> > > > Ok, yes a DEFHOOKPOD or similar sounds like a good idea, I'll look into 
> > > > this
> > > > alternative.
> > > 
> > > What is that, like target macros?  But with some indirection?  
> > 
> > Yes its for target macros, it looks like the "POD" in DEFHOOKPOD stands for
> > "piece-of-data", i.e. the hook represents a variable rather than function.  
> 
> But it is data, not a constant, so it does not allow optimising based
> on its potentially constant value?  Where "potentially" in this case
> means "always" :-/
> 
> 
> Segher

I can think of two alternatives so far which should get around this
optimization issue you point out.

1. a regular target macro defined in tm.texi such as
TARGET_CASE_INSENSITIVE_REGISTER_NAMES, defined to 0 by default. Overriden
with 1 for MSP430 

> for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> if (reg_names[i][0]
> +#if TARGET_CASE_INSENSITIVE_REGISTER_NAMES
> +   && ! strcasecmp (asmspec, strip_reg_name (reg_names[i])))
> +#else
> && ! strcmp (asmspec, strip_reg_name (reg_names[i])))
> +#endif
>   return i;

2. a function-like target macro TARGET_COMPARE_REGISTER_NAMES in tm.texi which
defines the function to use for register name comparisons. Defined to
"strcmp" by default and overriden with strcasecmp for MSP430.

> for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> if (reg_names[i][0]
> -   && ! strcmp (asmspec, strip_reg_name (reg_names[i])))
> +   && ! TARGET_COMPARE_REGISTER_NAMES (asmspec, strip_reg_name 
> (reg_names[i])))
>  return i;

Alternatively a DEFHOOK could be used for above so we'd have
targetm.compare_register_names (asmspec, ...) which would essentially be a
wrapper round strcmp or strcasecmp. But I'm unsure if GCC would end up
inlining the str{,case}cmp call from the target hook - maybe if the file is
built with lto.. So we may end up with sub-optimal code again with this.

I think (1) is more immediately clear as to what the macro is doing, although
it is less concise than (2) as 3 of these #if..else blocks would be required.
Nevertheless (1) would still be my preferred solution.
Any thoughts?

Thanks,
Jozef


Re: [PATCH PR Fortran/89286] Intrinsic sign and GNU Extension

2019-07-10 Thread Bernhard Reutner-Fischer
On 10 July 2019 17:52:40 CEST, Steve Kargl  
wrote:
>On Wed, Jul 10, 2019 at 02:50:47PM +0100, Mark Eggleston wrote:
>> The attached patch treats the intrinsic SIGN in the same way as MOD
>and 
>> DIM as it has the same arguments.
>> 
>> Tested using make -j 8 check-fortran on x86_64
>> 
>> Conditional compilation using #ifdef __GFC_REAL_16__ has been
>employed 
>> where appropriate in the test cases so should be OK on platforms
>without 
>> REAL(16).
>> 
>> Change logs:
>> 
>> gcc/fortran
>> 
>>      Mark Eggleston  
>> 
>>      PR fortran/89286
>>      * check.c (gfc_check_sign): Deleted.

ChangeLog has to be in present tense per convention.

>>      * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p
>>      instead of gfc_check_sign for "sign".
>>      * intrinsic.h: Remove declaration of gfc_check_sign.
>>      * iresolve.c (gfc_resolve_sign): Check for largest kind of the
>actual
>>      arguments and convert the smaller. Set return kind to be the
>largest.
>>      * simplify.c (gfc_simplify_sign): Use the largest kind of the
>actual
>>      arguments for return
>>      * intrinsic.texi: Add GNU extension notes for return value to
>SIGN.
>> 
>> gcc/testsuite
>> 
>>      Mark Eggleston 
>> 
>>      PR fortran/89240
>>      * gfortran.dg/sign_gnu_extension_1.f90: New test.
>>      * gfortran.dg/sign_gnu_extension_2.f90: New test.
>>      * gfortran.dg/pr78619.f90: Check for "must have" instead of
>"must be".
>> 
>> If OK please can someone commit as I do not have the privileges.
>> 
>
>We really need to get you commit access to the tree.
>
>I also am not a fan of this type of change.  Having spent the
>last few days working on fixing BOZ to conform to F2018, I'm
>finding all sorts of undocumented "extensions".  Personally,
>I think gfortran should be moving towards the standard by
>deprecating of these types of extensions.

At least make them explicit under explicit extension or at least -legacy or 
whatever its called.

thanks,



Re: Fix folding of vector EQ/NE

2019-07-10 Thread Richard Biener
On July 10, 2019 12:52:13 PM GMT+02:00, Richard Sandiford 
 wrote:
>For vector1 != vector2, we returned false if any elements were equal,
>rather than if all elements were equal.
>
>Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu.
>OK for trunk, gcc 9 and gcc 8?

Ok. 

Richard. 

>Richard
>
>
>2019-07-10  Richard Sandiford  
>
>gcc/
>   * fold-const.c (fold_relational_const): Fix folding of
>   vector-to-scalar NE_EXPRs.
>   (test_vector_folding): Add more tests.
>
>Index: gcc/fold-const.c
>===
>--- gcc/fold-const.c   2019-06-18 09:35:52.785887115 +0100
>+++ gcc/fold-const.c   2019-07-10 11:49:25.907674865 +0100
>@@ -14026,13 +14026,13 @@ fold_relational_const (enum tree_code co
>   {
> tree elem0 = VECTOR_CST_ELT (op0, i);
> tree elem1 = VECTOR_CST_ELT (op1, i);
>-tree tmp = fold_relational_const (code, type, elem0, elem1);
>+tree tmp = fold_relational_const (EQ_EXPR, type, elem0, elem1);
> if (tmp == NULL_TREE)
>   return NULL_TREE;
> if (integer_zerop (tmp))
>-  return constant_boolean_node (false, type);
>+  return constant_boolean_node (code == NE_EXPR, type);
>   }
>-return constant_boolean_node (true, type);
>+return constant_boolean_node (code == EQ_EXPR, type);
>   }
>   tree_vector_builder elts;
>   if (!elts.new_binary_operation (type, op0, op1, false))
>@@ -14803,6 +14803,7 @@ test_vector_folding ()
>   tree type = build_vector_type (inner_type, 4);
>   tree zero = build_zero_cst (type);
>   tree one = build_one_cst (type);
>+  tree index = build_index_vector (type, 0, 1);
> 
>   /* Verify equality tests that return a scalar boolean result.  */
>   tree res_type = boolean_type_node;
>@@ -14810,6 +14811,13 @@ test_vector_folding ()
>ASSERT_TRUE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type, zero,
>zero)));
>ASSERT_TRUE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, zero,
>one)));
>ASSERT_FALSE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, one,
>one)));
>+  ASSERT_TRUE (integer_nonzerop (fold_build2 (NE_EXPR, res_type,
>index, one)));
>+  ASSERT_FALSE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type,
>+ index, one)));
>+  ASSERT_FALSE (integer_nonzerop (fold_build2 (NE_EXPR, res_type,
>+index, index)));
>+  ASSERT_TRUE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type,
>+index, index)));
> }
> 
> /* Verify folding of VEC_DUPLICATE_EXPRs.  */



patch to fix PR91102

2019-07-10 Thread Vladimir Makarov

  The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91102

  The patch was bootstrapped and tested on x86-64

Committed as rev. 273357


Index: ChangeLog
===
--- ChangeLog	(revision 273356)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2019-07-10  Vladimir Makarov  
+
+	PR target/91102
+	* lra-constraints.c (process_alt_operands): Don't match user
+	defined regs only if they are early clobbers.
+
 2019-07-10  Marc Glisse  
 
 	* wide-int.h (wi::lshift): Reject negative values for the fast path.
Index: lra-constraints.c
===
--- lra-constraints.c	(revision 273356)
+++ lra-constraints.c	(working copy)
@@ -2172,8 +2172,9 @@ process_alt_operands (int only_alternati
 		else
 		  {
 			/* Operands don't match.  If the operands are
-			   different user defined explicit hard registers,
-			   then we cannot make them match.  */
+			   different user defined explicit hard
+			   registers, then we cannot make them match
+			   when one is early clobber operand.  */
 			if ((REG_P (*curr_id->operand_loc[nop])
 			 || SUBREG_P (*curr_id->operand_loc[nop]))
 			&& (REG_P (*curr_id->operand_loc[m])
@@ -2192,9 +2193,17 @@ process_alt_operands (int only_alternati
 && REG_P (m_reg)
 && HARD_REGISTER_P (m_reg)
 && REG_USERVAR_P (m_reg))
-			  break;
+			  {
+int i;
+
+for (i = 0; i < early_clobbered_regs_num; i++)
+  if (m == early_clobbered_nops[i])
+break;
+if (i < early_clobbered_regs_num
+|| early_clobber_p)
+  break;
+			  }
 			  }
-
 			/* Both operands must allow a reload register,
 			   otherwise we cannot make them match.  */
 			if (curr_alt[m] == NO_REGS)
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog	(revision 273356)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2019-07-10  Vladimir Makarov  
+
+	PR target/91102
+	* gcc.target/aarch64/pr91102.c: New test.
+
 2019-07-10  Richard Biener  
 
 	PR tree-optimization/91126
Index: testsuite/gcc.target/aarch64/pr91102.c
===
--- testsuite/gcc.target/aarch64/pr91102.c	(nonexistent)
+++ testsuite/gcc.target/aarch64/pr91102.c	(working copy)
@@ -0,0 +1,26 @@
+/* PR target/91102 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+foo (long d, long l)
+{
+  register long e asm ("x1") = d;
+  register long f asm("x2") = l;
+  asm ("" : : "r" (e), "r" (f));
+  return 3;
+}
+
+struct T { int i; int j; };
+union S { long h; struct T t; };
+
+void
+bar (union S b)
+{
+  while (1)
+{
+  union S c = b;
+  c.t.j++;
+  b.h = foo (b.h, c.h);
+}
+}


Re: [PATCH PR Fortran/89286] Intrinsic sign and GNU Extension

2019-07-10 Thread Steve Kargl
On Wed, Jul 10, 2019 at 02:50:47PM +0100, Mark Eggleston wrote:
> The attached patch treats the intrinsic SIGN in the same way as MOD and 
> DIM as it has the same arguments.
> 
> Tested using make -j 8 check-fortran on x86_64
> 
> Conditional compilation using #ifdef __GFC_REAL_16__ has been employed 
> where appropriate in the test cases so should be OK on platforms without 
> REAL(16).
> 
> Change logs:
> 
> gcc/fortran
> 
>      Mark Eggleston  
> 
>      PR fortran/89286
>      * check.c (gfc_check_sign): Deleted.
>      * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p
>      instead of gfc_check_sign for "sign".
>      * intrinsic.h: Remove declaration of gfc_check_sign.
>      * iresolve.c (gfc_resolve_sign): Check for largest kind of the actual
>      arguments and convert the smaller. Set return kind to be the largest.
>      * simplify.c (gfc_simplify_sign): Use the largest kind of the actual
>      arguments for return
>      * intrinsic.texi: Add GNU extension notes for return value to SIGN.
> 
> gcc/testsuite
> 
>      Mark Eggleston 
> 
>      PR fortran/89240
>      * gfortran.dg/sign_gnu_extension_1.f90: New test.
>      * gfortran.dg/sign_gnu_extension_2.f90: New test.
>      * gfortran.dg/pr78619.f90: Check for "must have" instead of "must be".
> 
> If OK please can someone commit as I do not have the privileges.
> 

We really need to get you commit access to the tree.

I also am not a fan of this type of change.  Having spent the
last few days working on fixing BOZ to conform to F2018, I'm
finding all sorts of undocumented "extensions".  Personally,
I think gfortran should be moving towards the standard by
deprecating of these types of extensions.

-- 
Steve


Re: [PATCH] Restrict LOOP_ALIGN to loop headers only.

2019-07-10 Thread Richard Biener
On July 10, 2019 2:11:17 PM GMT+02:00, Michael Matz  wrote:
>Hi,
>
>On Tue, 9 Jul 2019, Richard Biener wrote:
>
>> >The basic block index is not a DFS index, so no, that's not a test
>for 
>> >backedge.
>> 
>> I think in CFG RTL mode the BB index designates the order of the BBs
>in 
>> the object file? So this is a way to identify backwards jumps?
>
>Even if it means a backwards jump (and that's not always the case, the 
>insns are emitted by following the NEXT_INSN links, without a CFG, and 
>that all happens after machine-dependend reorg, and going out of cfg 
>layout might link insn together even from high index BBs to low index
>BBs 
>(e.g. because of fall-through)), that's still not a backedge in the 
>general case.  If a heuristic is enough here it might be okay, though.
>
>OTOH, as here a CFG still exists, why not simply rely on a proper DFS 
>marking backedges?

Because proper backedges is not what we want here, see honzas example. 

So I'm second-guessing why we have different LOOP_ALIGN and when it makes sense 
to apply. 

Richard. 

>
>Ciao,
>Michael.



[C++] Don't fold __builtin_constant_p prematurely

2019-07-10 Thread Marc Glisse

Hello,

this avoids folding __builtin_constant_p to 0 early when we are not forced 
to do so. Clearly this has an effect, since it uncovered a bug in 
wi::lshift, fixed today ;-)


I wasn't sure about using |= or just =, the first one seemed more 
conservative.


Bootstrap+regtest on x86_64-pc-linux-gnu.

2019-07-11  Marc Glisse  

gcc/cp/
* constexpr.c (cxx_eval_builtin_function_call): Only set
force_folding_builtin_constant_p if manifestly_const_eval.

gcc/testsuite/
* g++.dg/pr85746.C: New file.

--
Marc GlisseIndex: gcc/cp/constexpr.c
===
--- gcc/cp/constexpr.c	(revision 273356)
+++ gcc/cp/constexpr.c	(working copy)
@@ -1248,21 +1248,21 @@ cxx_eval_builtin_function_call (const co
 		  , );
 	}
 
   if (bi_const_p)
 	/* For __builtin_constant_p, fold all expressions with constant values
 	   even if they aren't C++ constant-expressions.  */
 	args[i] = cp_fold_rvalue (args[i]);
 }
 
   bool save_ffbcp = force_folding_builtin_constant_p;
-  force_folding_builtin_constant_p = true;
+  force_folding_builtin_constant_p |= ctx->manifestly_const_eval;
   tree save_cur_fn = current_function_decl;
   /* Return name of ctx->call->fundef->decl for __builtin_FUNCTION ().  */
   if (fndecl_built_in_p (fun, BUILT_IN_FUNCTION)
   && ctx->call
   && ctx->call->fundef)
 current_function_decl = ctx->call->fundef->decl;
   new_call = fold_builtin_call_array (EXPR_LOCATION (t), TREE_TYPE (t),
   CALL_EXPR_FN (t), nargs, args);
   current_function_decl = save_cur_fn;
   force_folding_builtin_constant_p = save_ffbcp;
Index: gcc/testsuite/g++.dg/pr85746.C
===
--- gcc/testsuite/g++.dg/pr85746.C	(nonexistent)
+++ gcc/testsuite/g++.dg/pr85746.C	(working copy)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-gimple" } */
+
+int f(int a,int b){
+  // The front-end should not fold this to 0.
+  int c = __builtin_constant_p(a < b);
+  return c;
+}
+
+/* { dg-final { scan-tree-dump "__builtin_constant_p" "gimple" } } */


Re: [PATCH] Fix PR91126

2019-07-10 Thread Jeff Law
On 7/10/19 5:15 AM, Richard Biener wrote:
> 
> The following patch fixes an old (but now manifesting in a testsuite
> fail) issue with value-numbering on big-endian machines.  I've
> checked the VN results after the patch on aarch64 with -mbig-endian.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> I'll commit after this succeeded.
> 
> Richard.
> 
> 2019-07-10  Richard Biener  
> 
>   PR tree-optimization/91126
>   * tree-ssa-sccvn.c (n_walk_cb_data::push_partial_def): Adjust
>   native encoding offset for BYTES_BIG_ENDIAN.
>   (vn_reference_lookup_3): Likewise.
> 
>   * gcc.dg/torture/pr91126.c: New testcase.
FWIW, the easiest big endian system to test is ppc64 as there's a
machine in the build farm.  It gets a bootstrap & regression cycle
daily.  Including the glibc & kernel build it takes ~4hrs.

My tester will also bootstrap aarch64_be, m68k and armeb, but I think
the ppc64 tester is the most effective and you can debug natively if
something goes wrong.  Debugging gcc via the qemu stub is, umm, painful.

jeff




[PATCH] Fix PR91131

2019-07-10 Thread Richard Biener


The PR complains that we fail to properly initialize a
volatile struct with a single assignment if the initializer
is all-zeros.

Fixed as follows.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-07-10  Richard Biener  

PR middle-end/91131
* gimplify.c (gimplify_compound_literal_expr): Force a temporary
when the object is volatile and we have not cleared it even though
there are no nonzero elements.

* gcc.target/i386/pr91131.c: New testcase.

Index: gcc/gimplify.c
===
--- gcc/gimplify.c  (revision 273355)
+++ gcc/gimplify.c  (working copy)
@@ -5005,7 +5004,7 @@ gimplify_init_constructor (tree *expr_p,
   one field to assign, initialize the target from a temporary.  */
if (TREE_THIS_VOLATILE (object)
&& !TREE_ADDRESSABLE (type)
-   && num_nonzero_elements > 0
+   && (num_nonzero_elements > 0 || !cleared)
&& vec_safe_length (elts) > 1)
  {
tree temp = create_tmp_var (TYPE_MAIN_VARIANT (type));
Index: gcc/testsuite/gcc.target/i386/pr91131.c
===
--- gcc/testsuite/gcc.target/i386/pr91131.c (nonexistent)
+++ gcc/testsuite/gcc.target/i386/pr91131.c (working copy)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+struct Reg_T {
+unsigned int a : 3;
+unsigned int b : 1;
+unsigned int c : 4;
+};
+
+volatile struct Reg_T Reg_A;
+
+int
+main ()
+{
+  Reg_A = (struct Reg_T){ .a = 0, .b = 0, .c = 0 };
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times "mov\[^\r\n\]*Reg_A" 1 } } */


[PATCH PR Fortran/89286] Intrinsic sign and GNU Extension

2019-07-10 Thread Mark Eggleston
The attached patch treats the intrinsic SIGN in the same way as MOD and 
DIM as it has the same arguments.


Tested using make -j 8 check-fortran on x86_64

Conditional compilation using #ifdef __GFC_REAL_16__ has been employed 
where appropriate in the test cases so should be OK on platforms without 
REAL(16).


Change logs:

gcc/fortran

    Mark Eggleston  

    PR fortran/89286
    * check.c (gfc_check_sign): Deleted.
    * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p
    instead of gfc_check_sign for "sign".
    * intrinsic.h: Remove declaration of gfc_check_sign.
    * iresolve.c (gfc_resolve_sign): Check for largest kind of the actual
    arguments and convert the smaller. Set return kind to be the largest.
    * simplify.c (gfc_simplify_sign): Use the largest kind of the actual
    arguments for return
    * intrinsic.texi: Add GNU extension notes for return value to SIGN.

gcc/testsuite

    Mark Eggleston 

    PR fortran/89240
    * gfortran.dg/sign_gnu_extension_1.f90: New test.
    * gfortran.dg/sign_gnu_extension_2.f90: New test.
    * gfortran.dg/pr78619.f90: Check for "must have" instead of "must be".

If OK please can someone commit as I do not have the privileges.

--
https://www.codethink.co.uk/privacy.html

>From 59069e6053f8b178d6f981f02e4b33701ae78062 Mon Sep 17 00:00:00 2001
From: Mark Eggleston 
Date: Thu, 31 Jan 2019 13:36:48 +
Subject: [PATCH 01/12] Intrinsic sign and GNU extension.

The intrinsic sign has the same parameters as other intrinsics such as
dim and mod. This support is part of the GNU extension enabled by using
-std=gnu (the default).
---
 gcc/fortran/check.c|  14 ---
 gcc/fortran/intrinsic.c|   2 +-
 gcc/fortran/intrinsic.h|   1 -
 gcc/fortran/intrinsic.texi |   6 +-
 gcc/fortran/iresolve.c |  13 +++
 gcc/fortran/simplify.c |   4 +-
 gcc/testsuite/gfortran.dg/pr78619.f90  |   2 +-
 gcc/testsuite/gfortran.dg/sign-gnu-extension_1.f90 | 103 +
 gcc/testsuite/gfortran.dg/sign-gnu-extension_2.f90 |  70 ++
 9 files changed, 195 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/sign-gnu-extension_1.f90
 create mode 100644 gcc/testsuite/gfortran.dg/sign-gnu-extension_2.f90

diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 95801804022..a7c5a6ef2a1 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -4484,20 +4484,6 @@ gfc_check_shift (gfc_expr *i, gfc_expr *shift)
   return true;
 }
 
-
-bool
-gfc_check_sign (gfc_expr *a, gfc_expr *b)
-{
-  if (!int_or_real_check (a, 0))
-return false;
-
-  if (!same_type_check (a, 0, b, 1))
-return false;
-
-  return true;
-}
-
-
 bool
 gfc_check_size (gfc_expr *array, gfc_expr *dim, gfc_expr *kind)
 {
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index c21fbddd5fb..fd10c48f1cf 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -2930,7 +2930,7 @@ add_functions (void)
   make_generic ("shiftr", GFC_ISYM_SHIFTR, GFC_STD_F2008);
 
   add_sym_2 ("sign", GFC_ISYM_SIGN, CLASS_ELEMENTAL, ACTUAL_YES, BT_REAL, dr, GFC_STD_F77,
-	 gfc_check_sign, gfc_simplify_sign, gfc_resolve_sign,
+	 gfc_check_a_p, gfc_simplify_sign, gfc_resolve_sign,
 	 a, BT_REAL, dr, REQUIRED, b, BT_REAL, dr, REQUIRED);
 
   add_sym_2 ("isign", GFC_ISYM_SIGN, CLASS_ELEMENTAL, ACTUAL_YES, BT_INTEGER, di, GFC_STD_F77,
diff --git a/gcc/fortran/intrinsic.h b/gcc/fortran/intrinsic.h
index 0c60dab8390..83be8b38bdf 100644
--- a/gcc/fortran/intrinsic.h
+++ b/gcc/fortran/intrinsic.h
@@ -154,7 +154,6 @@ bool gfc_check_set_exponent (gfc_expr *, gfc_expr *);
 bool gfc_check_shape (gfc_expr *, gfc_expr *);
 bool gfc_check_shift (gfc_expr *, gfc_expr *);
 bool gfc_check_size (gfc_expr *, gfc_expr *, gfc_expr *);
-bool gfc_check_sign (gfc_expr *, gfc_expr *);
 bool gfc_check_signal (gfc_expr *, gfc_expr *);
 bool gfc_check_sizeof (gfc_expr *);
 bool gfc_check_c_associated (gfc_expr *, gfc_expr *);
diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi
index f390761dc3d..c336cfccb3c 100644
--- a/gcc/fortran/intrinsic.texi
+++ b/gcc/fortran/intrinsic.texi
@@ -12917,11 +12917,13 @@ Elemental function
 @item @emph{Arguments}:
 @multitable @columnfractions .15 .70
 @item @var{A} @tab Shall be of type @code{INTEGER} or @code{REAL}
-@item @var{B} @tab Shall be of the same type and kind as @var{A}
+@item @var{B} @tab Shall be of the same type and kind as @var{A}.  (As a GNU
+extension, arguments of different kinds are permitted.)
 @end multitable
 
 @item @emph{Return value}:
-The kind of the return value is that of @var{A} and @var{B}.
+The kind of the return value is that of @var{A} and @var{B}.  (As a GNU
+extension, kind is the largest kind of the actual arguments.)
 If @math{B\ge 0} then the result is @code{ABS(A)}, else
 it is @code{-ABS(A)}.
 

[PATCH] Fix -O2 vs. -O1 missed optimizations in VN

2019-07-10 Thread Richard Biener


The "do fancy stuff" routine failed to look at the valueized LHS
in some cases which makes it regress at -O2 compared to -O1
(because with -O1 all preceeding stmts have gone through elimination
already).

Noticed when working on aarch64 support for PR83518.

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

Richard.

2019-07-10  Richard Biener  

c/
* gimple-parser.c (c_parser_gimple_postfix_expression): Support
_Literal (int *)  for address literals.

* tree-ssa-sccvn.c (vn_reference_lookup_3): Look at valueized
LHS whenever possible.

* gcc.dg/torture/ssa-fre-5.c: New testcase.
* gcc.dg/torture/ssa-fre-6.c: Likewise.
* gcc.dg/torture/ssa-fre-7.c: Likewise.

Index: gcc/c/gimple-parser.c
===
--- gcc/c/gimple-parser.c   (revision 273353)
+++ gcc/c/gimple-parser.c   (working copy)
@@ -1606,8 +1606,10 @@ c_parser_gimple_postfix_expression (gimp
  tree val = c_parser_gimple_postfix_expression (parser).value;
  if (! val
  || val == error_mark_node
- || ! CONSTANT_CLASS_P (val)
- || (addr_p && TREE_CODE (val) != STRING_CST))
+ || (!CONSTANT_CLASS_P (val)
+ && !(addr_p
+  && (TREE_CODE (val) == STRING_CST
+  || DECL_P (val)
{
  c_parser_error (parser, "invalid _Literal");
  return expr;
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-5.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-5.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-5.c(working copy)
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fgimple -fdump-tree-fre1" } */
+
+typedef int v4si __attribute__((vector_size(16)));
+
+int __GIMPLE (ssa,startwith("fre"))
+foo ()
+{
+  int * p;
+  int i;
+  int x[4];
+  long unsigned int _1;
+  long unsigned int _2;
+  int _7;
+
+  __BB(2):
+  i_3 = 0;
+  _1 = (long unsigned int) i_3;
+  _2 = _1 * 4ul;
+  p_4 = _Literal (int *)  + _2;
+  __MEM  ((v4si *)p_4) = _Literal (v4si) { 1, 2, 3, 4 };
+  _7 = x[0];
+  return _7;
+}
+
+/* { dg-final { scan-tree-dump "return 1;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-6.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-6.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-6.c(working copy)
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fgimple -fdump-tree-fre1" } */
+
+typedef int v4si __attribute__((vector_size(16)));
+
+int __GIMPLE (ssa,startwith("fre"))
+foo ()
+{
+  int * p;
+  int i;
+  int x[4];
+  long unsigned int _1;
+  long unsigned int _2;
+  int _7;
+
+  __BB(2):
+  i_3 = 0;
+  _1 = (long unsigned int) i_3;
+  _2 = _1 * 4ul;
+  p_4 = _Literal (int *)  + _2;
+  __MEM  ((v4si *)p_4) = _Literal (v4si) {};
+  _7 = x[0];
+  return _7;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-7.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-7.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-7.c(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fgimple -fdump-tree-fre1" } */
+
+typedef int v4si __attribute__((vector_size(16)));
+
+int __GIMPLE (ssa,startwith("fre"))
+foo (int c)
+{
+  int * p;
+  int i;
+  int x[4];
+  long unsigned int _1;
+  long unsigned int _2;
+  int _7;
+  v4si _6;
+
+  __BB(2):
+  i_3 = 0;
+  _1 = (long unsigned int) i_3;
+  _2 = _1 * 4ul;
+  p_4 = _Literal (int *)  + _2;
+  _6 = _Literal (v4si) { c_5(D), c_5(D), c_5(D), c_5(D) };
+  __MEM  ((v4si *)p_4) = _6;
+  _7 = x[0];
+  return _7;
+}
+
+/* { dg-final { scan-tree-dump "return c_5\\(D\\);" "fre1" } } */
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273353)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -2488,12 +2488,22 @@ vn_reference_lookup_3 (ao_ref *ref, tree
   && gimple_assign_rhs_code (def_stmt) == CONSTRUCTOR
   && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (def_stmt)) == 0)
 {
+  tree lhs = gimple_assign_lhs (def_stmt);
   tree base2;
   poly_int64 offset2, size2, maxsize2;
   HOST_WIDE_INT offset2i, size2i;
   bool reverse;
-  base2 = get_ref_base_and_extent (gimple_assign_lhs (def_stmt),
-  , , , );
+  if (lhs_ref_ok)
+   {
+ base2 = ao_ref_base (_ref);
+ offset2 

Re: [PATCH] Deprecate -frepo option.

2019-07-10 Thread Jakub Jelinek
On Wed, Jul 10, 2019 at 02:53:35PM +0200, Martin Liška wrote:
> On 7/10/19 2:48 PM, Nathan Sidwell wrote:
> > On 7/10/19 7:28 AM, Martin Liška wrote:
> > 
> >> Great, thank you.
> >>
> >> There's a patch for deprecating of the option in GCC 9 changes.
> >>
> >> May I install the patch right now or should I wait?
> >>
> > 
> > I think there needs to be a code patch so that use of the option gives a 
> > warning.
> > 
> > nathan
> > 
> That's done by 'Deprecated' keyword put on frepo in *.opt file:
> 
> $ ./gcc/xgcc -Bgcc -frepo /tmp/main.c
> xgcc: warning: switch ‘-frepo’ is no longer supported
> 
> I've already used the same mechanism for other deprecated options e.g.:
> 
> $ ./gcc/xgcc -Bgcc -mmpx /tmp/main.c
> xgcc: warning: switch ‘-mmpx’ is no longer supported

That doesn't seem to be correct for GCC 9 and -frepo though, the option will
be still supported, but deprecated and to be removed in the next release.
I think the Deprecated *.opt keyword is misnamed.

Jakub


Re: [PATCH] Deprecate -frepo option.

2019-07-10 Thread Nathan Sidwell

On 7/10/19 8:53 AM, Martin Liška wrote:


nathan


That's done by 'Deprecated' keyword put on frepo in *.opt file:

$ ./gcc/xgcc -Bgcc -frepo /tmp/main.c
xgcc: warning: switch ‘-frepo’ is no longer supported

I've already used the same mechanism for other deprecated options e.g.:

$ ./gcc/xgcc -Bgcc -mmpx /tmp/main.c
xgcc: warning: switch ‘-mmpx’ is no longer supported


Great!  I must have missed that patch.  All good to go for gcc-9 deprecation

nathan

--
Nathan Sidwell


Re: [PATCH] Deprecate -frepo option.

2019-07-10 Thread Martin Liška
On 7/10/19 2:48 PM, Nathan Sidwell wrote:
> On 7/10/19 7:28 AM, Martin Liška wrote:
> 
>> Great, thank you.
>>
>> There's a patch for deprecating of the option in GCC 9 changes.
>>
>> May I install the patch right now or should I wait?
>>
> 
> I think there needs to be a code patch so that use of the option gives a 
> warning.
> 
> nathan
> 
That's done by 'Deprecated' keyword put on frepo in *.opt file:

$ ./gcc/xgcc -Bgcc -frepo /tmp/main.c
xgcc: warning: switch ‘-frepo’ is no longer supported

I've already used the same mechanism for other deprecated options e.g.:

$ ./gcc/xgcc -Bgcc -mmpx /tmp/main.c
xgcc: warning: switch ‘-mmpx’ is no longer supported

Martin


Re: [PATCH] Deprecate -frepo option.

2019-07-10 Thread Nathan Sidwell

On 7/10/19 7:28 AM, Martin Liška wrote:


Great, thank you.

There's a patch for deprecating of the option in GCC 9 changes.

May I install the patch right now or should I wait?



I think there needs to be a code patch so that use of the option gives a 
warning.


nathan

--
Nathan Sidwell


Re: (C++) Remove -fdeduce-init-list?

2019-07-10 Thread Nathan Sidwell

On 7/9/19 5:23 PM, Marek Polacek wrote:

On Mon, May 20, 2019 at 02:38:31PM -0400, Marek Polacek wrote:

Back in 2011, -fdeduce-init-list was marked as deprecated:
.

7.5 years later, is it time to rip it out completely from the compiler?


Seeing the -frepo deprecation, any opinions re this?


We should remove it (in the usual way of leaving it as an ignored 
undocumented option)


nathan

--
Nathan Sidwell


Re: [PATCH] Restrict LOOP_ALIGN to loop headers only.

2019-07-10 Thread Michael Matz
Hi,

On Tue, 9 Jul 2019, Richard Biener wrote:

> >The basic block index is not a DFS index, so no, that's not a test for 
> >backedge.
> 
> I think in CFG RTL mode the BB index designates the order of the BBs in 
> the object file? So this is a way to identify backwards jumps?

Even if it means a backwards jump (and that's not always the case, the 
insns are emitted by following the NEXT_INSN links, without a CFG, and 
that all happens after machine-dependend reorg, and going out of cfg 
layout might link insn together even from high index BBs to low index BBs 
(e.g. because of fall-through)), that's still not a backedge in the 
general case.  If a heuristic is enough here it might be okay, though.

OTOH, as here a CFG still exists, why not simply rely on a proper DFS 
marking backedges?


Ciao,
Michael.


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Rainer Orth
Hi Gaius,

> Many thanks for all the feedback/bugs/patches.
> I've been working through some of these.  The parallel build is now done.

thank you for the blazingly quick fixes.

>> * The mc output is far too verbose right now: this isn't of interest to
>>   anyone but gm2 developers ;-)
>
> added --quiet to all invocations of mc on master - will apply to 9.1.0
> soon.

Perfect.

>> With the missing gm2l worked around as above, my i386-pc-solaris2.11
>> testresults are way better now:
>>
>> === gm2 Summary for unix ===
>>
>> # of expected passes11186
>> # of unexpected failures24
>> # of unresolved testcases   12
>
> these results are great for solaris.  On the amd64/GNU/Linux I get 12
> failures (long.mod and long2.mod) run 6 times each.

Indeed: I'm pretty pleased by those results myself.

The remaining failures mostly fall in two categories:

Undefined   first referenced
 symbol in file
KeyBoardLEDs_SwitchScroll   
/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/i386-pc-solaris2.11/./libgm2/libcor/.libs/libcor.so
ld: fatal: symbol referencing errors
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: gm2/coroutines/pim/run/pass/testiotransfer.mod compilation,  -g 
UNRESOLVED: gm2/coroutines/pim/run/pass/testiotransfer.mod execution,  -g  
(link failed)

The definition lives in libgm2/libcor/KeyBoardLEDs.c and is Linux-only,
requiring the KGETLED/KSETLED ioctls.  An empty fallback implementation
is missing from the !linux section.

Then there's

/var/tmp//ccrqpjSdcpp:1:failed to find module name
compiler exited with status 1
FAIL: gm2/cpp/pass/cpph.mod,  -g  

FAIL: gm2/pim/fail/TestLong4.mod,  -g  

with no further indication in gm2.log what might be wrong.  Will have a
look.

Most 64-bit testcases FAIL for the same reason:

Undefined   first referenced
 symbol in file
funcString  /var/tmp//ccGNv4Zd.a(m.o)
ld: fatal: symbol referencing errors
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: gm2/calling-c/datatypes/unbounded/run/pass/m.mod compilation,  -g 
UNRESOLVED: gm2/calling-c/datatypes/unbounded/run/pass/m.mod execution,  -g  
(link failed)

funcString *is* defined in c.c, but that file is compiled in the .exp
file `under the hood' with exec ${XGCC} (not even visible in gm2.log)
and without the correct multilib flag.

[01m[K/vol/gcc/src/hg/trunk/solaris/gcc/testsuite/gm2/cpp/pass/subaddr.mod:32:3:[m[K
 [01;31m[Kerror: [m[Kcan only CAST objects of the same size
   32 | #else
  |   [01;31m[K^[m[K
compiler exited with status 1
FAIL: gm2/cpp/pass/subaddr.mod,  -g  

Haven't looked yet, but might be a generic multilib testing problem.

I also noticed that quite a number of temp files are left behind in
gcc/testsuite:

* many *_m2.s files.

* several .o files:

-rw-r--r-- 1 ro gcc 8928 Jul  9 13:45 cpp.o
-rw-r--r-- 1 ro gcc 3772 Jul  9 13:46 cvararg.o
-rw-r--r-- 1 ro gcc 5248 Jul  9 13:52 except2.o
-rw-r--r-- 1 ro gcc 4616 Jul  9 13:52 except4.o
-rw-r--r-- 1 ro gcc 4480 Jul  9 13:52 except5.o
-rw-r--r-- 1 ro gcc 6152 Jul  9 13:52 except7.o
-rw-r--r-- 1 ro gcc 5408 Jul  9 13:52 except8.o
-rw-r--r-- 1 ro gcc 2888 Jul  9 13:56 exceptiontest.o
-rw-r--r-- 1 ro gcc 1968 Jul  9 13:55 fileio.o
-rw-r--r-- 1 ro gcc 1712 Jul  9 14:19 hello.o
-rw-r--r-- 1 ro gcc 6424 Jul  9 13:45 libexcept.o
-rw-r--r-- 1 ro gcc 9780 Jul  9 13:45 mycpp.o
-rw-r--r-- 1 ro gcc 3392 Jul  9 13:56 raise.o
-rw-r--r-- 1 ro gcc 3520 Jul  9 13:57 raise2.o
-rw-r--r-- 1 ro gcc 1760 Jul  9 14:17 rangesupport.o
-rw-r--r-- 1 ro gcc 3488 Jul  9 14:14 sys.o

* a few *.x[0-5] files:

except2
except4
except5
except7
except8
exceptiontest
libexcept
raise
raise2

* 3 others:

-rw-r--r--   1 ro   gcc 4199 Jul  9 14:00 integer_m2.cpp
-rw-r--r--   1 ro   gcc   36 Jul  9 14:14 results.dat
-rw-r--r--   1 ro   gcc   26 Jul  9 13:56 test.txt

Besides, gm2.log is very full with debug output, making it hard to see
what's going on:

Stuff like

looking for 
/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/i386-pc-solaris2.11/./libpim/.libs/libiso.a

even seems to run several times for the same lib!?

Same for

about to call target_compile with 
/vol/gcc/src/hg/trunk/solaris/gcc/testsuite/../gm2/examples/callingC/hello.mod 
/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/gcc/testsuite/hello.o
 object {additional_flags= -g  } 
{compiler=/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/gcc/xgm2
 -B/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/gcc 
-I/var/gcc/gcc-10.0.0-20190708/11.5-gcc-gas-gm2-no-bootstrap-j1/i386-pc-solaris2.11/./libgm2/libpim:/vol/gcc/src/hg/trunk/solaris/gcc/testsuite/../gm2/gm2-libs
 

Re: [PATCH V4] PR88497 - Extend reassoc for vector bit_field_ref

2019-07-10 Thread Richard Biener
On Mon, 8 Jul 2019, Kewen.Lin wrote:

> Hi Richard,
> 
> Thanks a lot for your review and reply, I've updated the patch accordingly.
> 
> Main changes compared to previous one:
>   - Consider SVE (poly_int) on bit_field_ref size/offset.
>   - Filter valid candidates with sbitmap first.
>   - Support multiple modes (TODO 1).
>   - Test cases: add SSE2 support for 1..5, update run result check for 1,
> add two more cases (5,6)to verify multiple mode support (x86).
> 
> Bootstrapped and regression test passed on powerpc64le-unknown-linux-gnu
> and x86_64-linux-gnu.
> 
> Could you help to review it again?  Thanks in advance!

+  tree rhs = gimple_assign_rhs1 (oe1def);
+  tree vec = TREE_OPERAND (rhs, 0);
+  tree vec_type = TREE_TYPE (vec);
+
+  if (TREE_CODE (vec) != SSA_NAME || !VECTOR_TYPE_P (vec_type))
+   continue;
...
+  /* Ignore it if target machine can't support this VECTOR type.  */
+  if (!VECTOR_MODE_P (TYPE_MODE (vec_type)))
+   continue;

put the check below next to the one above, it's cheaper than the
poly-int stuff that currently preceeds it.

+  v_info_ptr *info_ptr = v_info_map.get (vec);
+  if (info_ptr)
+   {

there is get_or_insert which enables you to mostly merge the two cases
with a preceeding

  if (!existed)
v_info_ptr = new v_info;

also eliding the extra hash-lookup the put operation otherwise requires.

+  for (hash_map::iterator it = v_info_map.begin ();
+   it != v_info_map.end (); ++it)
+{
+  tree cand_vec = (*it).first;
+  v_info_ptr cand_info = (*it).second;
+  unsigned int num_elems = VECTOR_CST_NELTS (cand_vec).to_constant 
();

please add a quick out like

  if (cand_info->length () != num_elems)
continue;

since to have no holes and no overlap you need exactly that many.

I think you can avoid the somewhat ugly mode_to_total counter array.
Since you have sorted valid_vecs after modes simply do

  for (unsigned i = 0; i < valid_vecs.length () - 1; ++i)
{
  tree tvec = valid_vecs[i];
  enum machine_mode mode = TYPE_MODE (TREE_TYPE (tvec));

(please don't use unsigned int for mode!)

  /* Skip modes with only a single candidate.  */
  if (TYPE_MODE (TREE_TYPE (valid_vecs[i+1])) != mode)
continue;

  do
{
...
}
  while (...)

Richard.

> Kewen
> 
> 
> 
> gcc/ChangeLog
> 
> 2019-07-08  Kewen Lin  
> 
>   PR tree-optimization/88497
>   * tree-ssa-reassoc.c (reassociate_bb): Swap the positions of 
>   GIMPLE_BINARY_RHS check and gimple_visited_p check, call new 
>   function undistribute_bitref_for_vector.
>   (undistribute_bitref_for_vector): New function.
>   (cleanup_vinfo_map): Likewise.
>   (sort_by_mach_mode): Likewise.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-07-08  Kewen Lin  
> 
>   * gcc.dg/tree-ssa/pr88497-1.c: New test.
>   * gcc.dg/tree-ssa/pr88497-2.c: Likewise.
>   * gcc.dg/tree-ssa/pr88497-3.c: Likewise.
>   * gcc.dg/tree-ssa/pr88497-4.c: Likewise.
>   * gcc.dg/tree-ssa/pr88497-5.c: Likewise.
>   * gcc.dg/tree-ssa/pr88497-6.c: Likewise.
>   * gcc.dg/tree-ssa/pr88497-7.c: Likewise.
> 
> 
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-10 Thread Rainer Orth
Hi Gaius,

> Rainer Orth  writes:
>
>> here are some initial issues.  I'll reply to Matthias' mail to expand on
>> other problems he's raised.
>>
>> * First, the build broke like this:
>>
>> /vol/gcc/src/hg/trunk/solaris/gcc/gm2/mc-boot/GRTint.c:57:30: error:
>> 'time' redeclared as different kind of symbol
>>57 | typedef enum {input, output, time} VectorType;
>>   |  ^~~~
>> In file included from /usr/include/time.h:12,
>>  from /usr/include/sys/time.h:448,
>>  from /usr/include/sys/select.h:27,
>>  from /usr/include/sys/types.h:665,
>>  from /usr/include/stdlib.h:22,
>>  from 
>> /vol/gcc/src/hg/trunk/solaris/gcc/gm2/mc-boot/Glibc.h:15,
>>  from 
>> /vol/gcc/src/hg/trunk/solaris/gcc/gm2/mc-boot/GRTint.c:42:
>> /usr/include/iso/time_iso.h:96:15: note: previous declaration of 'time' was 
>> here
>>96 | extern time_t time(time_t *);
>>   |   ^~~~
>
> Hi Rainer,
>
> thanks for the bug report.  Now fixed in the git repro -
> the mc bootstrap tool now avoids 'time'.  Also fixed Make-lang.in
> to allow parallel builds.

excellent, thanks.

Rainer

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


Re: [PATCH] Deprecate -frepo option.

2019-07-10 Thread Martin Liška
On 7/9/19 11:14 PM, Jason Merrill wrote:
> On 7/9/19 1:48 PM, Nathan Sidwell wrote:
>> On 7/9/19 9:00 AM, Martin Liška wrote:
>>> On 7/9/19 1:41 PM, Nathan Sidwell wrote:
 On 7/9/19 6:39 AM, Richard Biener wrote:
> On Mon, Jul 8, 2019 at 2:04 PM Martin Liška  wrote:
>>

>>
>> Same happens also for GCC7. It does 17 iteration (#define MAX_ITERATIONS 
>> 17) and
>> apparently 17 is not enough to resolve all symbols. And it's really slow.
>
> Ouch.

 hm, 17 is a magic number.  in C++98 it was the maximum depth of template 
 instantiations that implementations needed to support.  Portable code 
 could not expect more.  So the worst case -frepo behaviour would be 17 
 iterations.

 That's not true any more, it's been 1024 since C++11.

 Has a bug been filed about this frepo problem?
>>>
>>> I create a new one:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91125
>>>
 If not, it suggest those using frepo are not compiling modern C++.

>> That said, I would recommend to remove it :)
>
> In the end it's up to the C++ FE maintainers but the above clearly
> doesn't look promising
> (not sure if it keeps re-compiling _all_ repo-triggered templates or
> just incrementally adds
> them to new object files).

> I'm not opposed to removing -frepo from GCC 10 but then I would start
> noting it is obsolete
> on the GCC 9 branch at least.

 I concur.  frepo's serial reinvocation of the compiler is not compatible 
 with modern C++ code bases.
>>>
>>> Great. Then I'm sending patch that does the functionality removal.
>>>
>>> Ready to be installed after proper testing & bootstrap?
>>
>> I'd like Jason to render an opinion, and we should mark it obsolete in the 
>> gcc 9 branch (how much runway would that give people?)
> 
> I haven't noticed any responses to my earlier question: Are there any targets 
> without COMDAT support that we still care about?
> 
> But given the observation above about the 17 limit, even if there are such 
> targets it wouldn't be very useful for modern code.  And if people want to 
> compile old code for old platforms, they might as well continue to use an old 
> compiler.
> 
> So I'm OK with deprecating with a warning for the next GCC 9 release, to see 
> if anyone complains, and removing in 10.

Great, thank you.

There's a patch for deprecating of the option in GCC 9 changes.

May I install the patch right now or should I wait?

Thanks,
Martin

> 
> Jason

diff --git a/htdocs/gcc-9/changes.html b/htdocs/gcc-9/changes.html
index bf9f6db..bec4754 100644
--- a/htdocs/gcc-9/changes.html
+++ b/htdocs/gcc-9/changes.html
@@ -57,6 +57,11 @@ You may also want to check out our
   announcement.
 
   
+  
+  The automatic template instantiation at link time (https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/C_002b_002b-Dialect-Options.html#index-frepo;>-frepo) has been deprecated and
+will be removed in a future release.
+  
+
 
 
 


PR90724 - ICE with __sync_bool_compare_and_swap with -march=armv8.2-a

2019-07-10 Thread Prathamesh Kulkarni
Hi,
For following test-case,
static long long AL[24];

int
check_ok (void)
{
  return (__sync_bool_compare_and_swap (AL+1, 0x20003ll, 0x1234567890ll));
}

Compiling with -O2 -march=armv8.2-a results in:
pr90724.c: In function ‘check_ok’:
pr90724.c:7:1: error: unrecognizable insn:
7 | }
  | ^
(insn 11 10 12 2 (set (reg:CC 66 cc)
(compare:CC (reg:DI 95)
(const_int 8589934595 [0x20003]))) "pr90724.c":6:11 -1
 (nil))

IIUC, the issue is that 0x20003 falls outside the range of
allowable immediate in cmp ? If it's replaced by a small constant then it works.

The ICE results with -march=armv8.2-a because, we enter if
(TARGET_LSE) { ... } condition
in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes into else,
which forces oldval into register if the predicate fails to match.

The attached patch checks if y (oldval) satisfies aarch64_plus_operand
predicate and if not, forces it to be in register, which resolves ICE.
Does it look OK ?

Bootstrap+testing in progress on aarch64-linux-gnu.

PS: The issue has nothing to do with SVE, which I incorrectly
mentioned in bug report.

Thanks,
Prathamesh
2019-07-10  Prathamesh Kulkarni  

PR target/90724
* config/aarch64/aarch64.c (aarch64_gen_compare_reg_maybe_ze): Force y
in reg if it fails aarch64_plus_operand predicate.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index a18fbd0f0aa..22d4726e19a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1930,6 +1930,9 @@ aarch64_gen_compare_reg_maybe_ze (RTX_CODE code, rtx x, 
rtx y,
}
 }
 
+  if (!aarch64_plus_operand (y, y_mode))
+y = force_reg (y_mode, y);
+
   return aarch64_gen_compare_reg (code, x, y);
 }
 


Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-10 Thread Ramana Radhakrishnan
On Wed, Jul 10, 2019 at 11:07 AM Richard Sandiford
 wrote:
>
> Christophe Lyon  writes:
> > On Mon, 8 Jul 2019 at 11:04, Richard Sandiford
> >  wrote:
> >>
> >> Christophe Lyon  writes:
> >> > Hi,
> >> >
> >> > This patch fixes PR 91060 where the lane ordering was no longer the
> >> > right one (GCC's vs architecture's).
> >>
> >> Sorry, we clashed :-)
> >>
> >> I'd prefer to go with the version I attached to bugzilla just now.
> >
> > Yes just saw that, thanks!
>
> The bugzilla version didn't properly adjust vec_setv2di_internal.
> Fixed with the version below, tested on armeb-eabi.
>
> Besides gcc.c-torture/execute/scal-to-vec*.c, the patch also fixes:
>
>   c-c++-common/torture/vector-compare-1.c
>   gcc.target/arm/pr69614.c
>   g++.dg/ext/vector37.C
>
> OK for trunk?

OK.

Ramana
>
> Richard
>
>
> 2019-07-10  Richard Sandiford  
>
> gcc/
> PR target/91060
> * config/arm/iterators.md (V2DI_ONLY): New mode iterator.
> * config/arm/neon.md (vec_set_internal): Add a '@' prefix.
> (vec_setv2di_internal): Reexpress as...
> (@vec_set_internal): ...this.
> * config/arm/arm.c (neon_expand_vector_init): Use gen_vec_set_internal
> rather than gen_neon_vset_lane.
>
> Index: gcc/config/arm/iterators.md
> ===
> --- gcc/config/arm/iterators.md 2019-06-18 09:35:55.377865698 +0100
> +++ gcc/config/arm/iterators.md 2019-07-10 11:01:57.990749932 +0100
> @@ -186,6 +186,9 @@ (define_mode_iterator VX [V8QI V4HI V16Q
>  ;; Modes with 8-bit elements.
>  (define_mode_iterator VE [V8QI V16QI])
>
> +;; V2DI only (for use with @ patterns).
> +(define_mode_iterator V2DI_ONLY [V2DI])
> +
>  ;; Modes with 64-bit elements only.
>  (define_mode_iterator V64 [DI V2DI])
>
> Index: gcc/config/arm/neon.md
> ===
> --- gcc/config/arm/neon.md  2019-07-01 09:37:07.220524486 +0100
> +++ gcc/config/arm/neon.md  2019-07-10 11:01:57.990749932 +0100
> @@ -319,7 +319,7 @@ (define_insn "*movmisalign_neon_lo
>"vld1.\t{%q0}, %A1"
>[(set_attr "type" "neon_load1_1reg")])
>
> -(define_insn "vec_set_internal"
> +(define_insn "@vec_set_internal"
>[(set (match_operand:VD_LANE 0 "s_register_operand" "=w,w")
>  (vec_merge:VD_LANE
>(vec_duplicate:VD_LANE
> @@ -340,7 +340,7 @@ (define_insn "vec_set_internal"
>  }
>[(set_attr "type" "neon_load1_all_lanes,neon_from_gp")])
>
> -(define_insn "vec_set_internal"
> +(define_insn "@vec_set_internal"
>[(set (match_operand:VQ2 0 "s_register_operand" "=w,w")
>  (vec_merge:VQ2
>(vec_duplicate:VQ2
> @@ -369,12 +369,12 @@ (define_insn "vec_set_internal"
>[(set_attr "type" "neon_load1_all_lanes,neon_from_gp")]
>  )
>
> -(define_insn "vec_setv2di_internal"
> -  [(set (match_operand:V2DI 0 "s_register_operand" "=w,w")
> -(vec_merge:V2DI
> -  (vec_duplicate:V2DI
> +(define_insn "@vec_set_internal"
> +  [(set (match_operand:V2DI_ONLY 0 "s_register_operand" "=w,w")
> +(vec_merge:V2DI_ONLY
> +  (vec_duplicate:V2DI_ONLY
>  (match_operand:DI 1 "nonimmediate_operand" "Um,r"))
> -  (match_operand:V2DI 3 "s_register_operand" "0,0")
> +  (match_operand:V2DI_ONLY 3 "s_register_operand" "0,0")
>(match_operand:SI 2 "immediate_operand" "i,i")))]
>"TARGET_NEON"
>  {
> Index: gcc/config/arm/arm.c
> ===
> --- gcc/config/arm/arm.c2019-07-01 09:37:07.220524486 +0100
> +++ gcc/config/arm/arm.c2019-07-10 11:01:57.990749932 +0100
> @@ -12471,7 +12471,7 @@ neon_expand_vector_init (rtx target, rtx
>if (n_var == 1)
>  {
>rtx copy = copy_rtx (vals);
> -  rtx index = GEN_INT (one_var);
> +  rtx merge_mask = GEN_INT (1 << one_var);
>
>/* Load constant part of vector, substitute neighboring value for
>  varying element.  */
> @@ -12480,38 +12480,7 @@ neon_expand_vector_init (rtx target, rtx
>
>/* Insert variable.  */
>x = copy_to_mode_reg (inner_mode, XVECEXP (vals, 0, one_var));
> -  switch (mode)
> -   {
> -   case E_V8QImode:
> - emit_insn (gen_neon_vset_lanev8qi (target, x, target, index));
> - break;
> -   case E_V16QImode:
> - emit_insn (gen_neon_vset_lanev16qi (target, x, target, index));
> - break;
> -   case E_V4HImode:
> - emit_insn (gen_neon_vset_lanev4hi (target, x, target, index));
> - break;
> -   case E_V8HImode:
> - emit_insn (gen_neon_vset_lanev8hi (target, x, target, index));
> - break;
> -   case E_V2SImode:
> - emit_insn (gen_neon_vset_lanev2si (target, x, target, index));
> - break;
> -   case E_V4SImode:
> - emit_insn (gen_neon_vset_lanev4si (target, x, target, index));
> - break;
> -   case E_V2SFmode:
> - emit_insn 

Re: [PATCH] Use -flto instead of -flto=N in DWARF producer string.

2019-07-10 Thread Martin Liška
On 7/10/19 1:15 PM, Jakub Jelinek wrote:
> On Wed, Jul 10, 2019 at 01:08:52PM +0200, Martin Liška wrote:
>> --- a/gcc/dwarf2out.c
>> +++ b/gcc/dwarf2out.c
>> @@ -24460,6 +24460,13 @@ gen_producer_string (void)
>>case OPT_fchecking_:
>>  /* Ignore these.  */
>>  continue;
>> +  case OPT_flto_:
>> +  {
>> +const char *lto_canonical = "-flto";
>> +switches.safe_push (lto_canonical);
>> +len += strlen (lto_canonical) + 1;
>> +break;
>> +  }
> 
> The indentation looks off, when case is indented by 6 columns,
> { should be by 8 (i.e. a tab) and const by 10 (i.e. a tab + 2 spaces).
> 
>   Jakub
> 

You are right, sorry for that.

Martin
>From eda41b25bf8b91412683ad542074724c872b18a4 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Wed, 10 Jul 2019 13:05:19 +0200
Subject: [PATCH] Use -flto instead of -flto=N in DWARF producer string.

gcc/ChangeLog:

2019-07-10  Martin Liska  

	* dwarf2out.c (gen_producer_string): Canonize -flto=N
	to -flto in dwarf producer string.
---
 gcc/dwarf2out.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0449c2b2912..aa7fd7eb465 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24460,6 +24460,13 @@ gen_producer_string (void)
   case OPT_fchecking_:
 	/* Ignore these.  */
 	continue;
+  case OPT_flto_:
+	{
+	  const char *lto_canonical = "-flto";
+	  switches.safe_push (lto_canonical);
+	  len += strlen (lto_canonical) + 1;
+	  break;
+	}
   default:
 if (cl_options[save_decoded_options[j].opt_index].flags
 	& CL_NO_DWARF_RECORD)
-- 
2.22.0



[PATCH] Fix PR91126

2019-07-10 Thread Richard Biener


The following patch fixes an old (but now manifesting in a testsuite
fail) issue with value-numbering on big-endian machines.  I've
checked the VN results after the patch on aarch64 with -mbig-endian.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

I'll commit after this succeeded.

Richard.

2019-07-10  Richard Biener  

PR tree-optimization/91126
* tree-ssa-sccvn.c (n_walk_cb_data::push_partial_def): Adjust
native encoding offset for BYTES_BIG_ENDIAN.
(vn_reference_lookup_3): Likewise.

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

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273353)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -1832,10 +1832,19 @@ vn_walk_cb_data::push_partial_def (const
0, MIN ((HOST_WIDE_INT)sizeof (buffer), pd.size));
  else
{
+ unsigned pad = 0;
+ if (BYTES_BIG_ENDIAN)
+   {
+ /* On big-endian the padding is at the 'front' so
+just skip the initial bytes.  */
+ fixed_size_mode mode = as_a 
+  (TYPE_MODE (TREE_TYPE (pd.rhs)));
+ pad = GET_MODE_SIZE (mode) - pd.size;
+   }
  len = native_encode_expr (pd.rhs,
buffer + MAX (0, pd.offset),
sizeof (buffer - MAX (0, 
pd.offset)),
-   MAX (0, -pd.offset));
+   MAX (0, -pd.offset) + pad);
  if (len <= 0
  || len < (pd.size - MAX (0, -pd.offset)))
{
@@ -2568,9 +2577,19 @@ vn_reference_lookup_3 (ao_ref *ref, tree
  tree rhs = gimple_assign_rhs1 (def_stmt);
  if (TREE_CODE (rhs) == SSA_NAME)
rhs = SSA_VAL (rhs);
+ unsigned pad = 0;
+ if (BYTES_BIG_ENDIAN)
+   {
+ /* On big-endian the padding is at the 'front' so
+just skip the initial bytes.  */
+ fixed_size_mode mode
+   = as_a  (TYPE_MODE (TREE_TYPE (rhs)));
+ pad = GET_MODE_SIZE (mode) - size2i / BITS_PER_UNIT;
+   }
  len = native_encode_expr (rhs,
buffer, sizeof (buffer),
-   (offseti - offset2i) / BITS_PER_UNIT);
+   ((offseti - offset2i) / BITS_PER_UNIT
++ pad));
  if (len > 0 && len * BITS_PER_UNIT >= maxsizei)
{
  tree type = vr->type;
Index: gcc/testsuite/gcc.dg/torture/pr91126.c
===
--- gcc/testsuite/gcc.dg/torture/pr91126.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr91126.c  (working copy)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+
+struct S
+{
+  __INT32_TYPE__ a : 24;
+  __INT32_TYPE__ b : 8;
+} s;
+
+int
+main()
+{
+  s.a = 0xfefefe;
+  s.b = 0xfe;
+  unsigned char c;
+  c = ((unsigned char *))[0];
+  if (c != 0xfe)
+__builtin_abort ();
+  c = ((unsigned char *))[1];
+  if (c != 0xfe)
+__builtin_abort ();
+  c = ((unsigned char *))[2];
+  if (c != 0xfe)
+__builtin_abort ();
+  c = ((unsigned char *))[3];
+  if (c != 0xfe)
+__builtin_abort ();
+  return 0;
+}


Re: [PATCH] Use -flto instead of -flto=N in DWARF producer string.

2019-07-10 Thread Jakub Jelinek
On Wed, Jul 10, 2019 at 01:08:52PM +0200, Martin Liška wrote:
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -24460,6 +24460,13 @@ gen_producer_string (void)
>case OPT_fchecking_:
>   /* Ignore these.  */
>   continue;
> +  case OPT_flto_:
> +   {
> + const char *lto_canonical = "-flto";
> + switches.safe_push (lto_canonical);
> + len += strlen (lto_canonical) + 1;
> + break;
> +   }

The indentation looks off, when case is indented by 6 columns,
{ should be by 8 (i.e. a tab) and const by 10 (i.e. a tab + 2 spaces).

Jakub


Contents of PO file 'cpplib-9.1-b20190203.zh_TW.po'

2019-07-10 Thread Translation Project Robot


cpplib-9.1-b20190203.zh_TW.po.gz
Description: Binary data
The Translation Project robot, in the
name of your translation coordinator.



New Chinese (traditional) PO file for 'cpplib' (version 9.1-b20190203)

2019-07-10 Thread Translation Project Robot
Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Chinese (traditional) team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/zh_TW.po

(This file, 'cpplib-9.1-b20190203.zh_TW.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.




[PATCH] Use -flto instead of -flto=N in DWARF producer string.

2019-07-10 Thread Martin Liška
Hi.

We're using LTO in openSUSE package builds and we use -flto=N based on number
of CPU that a builder machine has. That leads to debug info divergence and
so that I would like to canonize the option in producer string.

Ready to be installed after tests & bootstrap?
Thanks,
Martin

gcc/ChangeLog:

2019-07-10  Martin Liska  

* dwarf2out.c (gen_producer_string): Canonize -flto=N
to -flto in dwarf producer string.
---
 gcc/dwarf2out.c | 7 +++
 1 file changed, 7 insertions(+)


diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0449c2b2912..dccfe088de2 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24460,6 +24460,13 @@ gen_producer_string (void)
   case OPT_fchecking_:
 	/* Ignore these.  */
 	continue;
+  case OPT_flto_:
+	  {
+	const char *lto_canonical = "-flto";
+	switches.safe_push (lto_canonical);
+	len += strlen (lto_canonical) + 1;
+	break;
+	  }
   default:
 if (cl_options[save_decoded_options[j].opt_index].flags
 	& CL_NO_DWARF_RECORD)



Re: Fix wi::lshift

2019-07-10 Thread Richard Sandiford
Marc Glisse  writes:
> Hello,
>
> because of an other bug, __builtin_constant_p is ignored in some cases, 
> and this bug in wide_int went unnoticed. I am fixing it by making it match 
> more closely the comment above.
>
> Bootstrap+regtest together with a fix for __builtin_constant_p on 
> x86_64-pc-linux-gnu.
>
> 2019-07-11  Marc Glisse  
>
>   * wide-int.h (wi::lshift): Reject negative values for the fast path.
>
> Index: gcc/wide-int.h
> ===
> --- gcc/wide-int.h(revision 273306)
> +++ gcc/wide-int.h(working copy)
> @@ -3025,22 +3025,22 @@ wi::lshift (const T1 , const T2 )
>handle the case where the shift value is constant and the
>result is a single nonnegative HWI (meaning that we don't
>need to worry about val[1]).  This is particularly common
>for converting a byte count to a bit count.
> 
>For variable-precision integers like wide_int, handle HWI
>and sub-HWI integers inline.  */
>if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
> ? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)
>&& xi.len == 1
> -  && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)
> -   HOST_WIDE_INT_MAX >> shift))
> +  && xi.val[0] >= 0
> +  && xi.val[0] <= HOST_WIDE_INT_MAX >> shift)

Slightly prefer IN_RANGE (xi.val[0], 0, HOST_WIDE_INT_MAX >> shift)
for this, although the compiler should be smart enough to do that
transformation itself.

OK with that change, thanks.

The testing obviously shows that we don't need the UWHI cast
before the shift any more.  Maybe we didn't in 2013 either and
I was just being overcautious.

Richard


Fix folding of vector EQ/NE

2019-07-10 Thread Richard Sandiford
For vector1 != vector2, we returned false if any elements were equal,
rather than if all elements were equal.

Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu.
OK for trunk, gcc 9 and gcc 8?

Richard


2019-07-10  Richard Sandiford  

gcc/
* fold-const.c (fold_relational_const): Fix folding of
vector-to-scalar NE_EXPRs.
(test_vector_folding): Add more tests.

Index: gcc/fold-const.c
===
--- gcc/fold-const.c2019-06-18 09:35:52.785887115 +0100
+++ gcc/fold-const.c2019-07-10 11:49:25.907674865 +0100
@@ -14026,13 +14026,13 @@ fold_relational_const (enum tree_code co
{
  tree elem0 = VECTOR_CST_ELT (op0, i);
  tree elem1 = VECTOR_CST_ELT (op1, i);
- tree tmp = fold_relational_const (code, type, elem0, elem1);
+ tree tmp = fold_relational_const (EQ_EXPR, type, elem0, elem1);
  if (tmp == NULL_TREE)
return NULL_TREE;
  if (integer_zerop (tmp))
-   return constant_boolean_node (false, type);
+   return constant_boolean_node (code == NE_EXPR, type);
}
- return constant_boolean_node (true, type);
+ return constant_boolean_node (code == EQ_EXPR, type);
}
   tree_vector_builder elts;
   if (!elts.new_binary_operation (type, op0, op1, false))
@@ -14803,6 +14803,7 @@ test_vector_folding ()
   tree type = build_vector_type (inner_type, 4);
   tree zero = build_zero_cst (type);
   tree one = build_one_cst (type);
+  tree index = build_index_vector (type, 0, 1);
 
   /* Verify equality tests that return a scalar boolean result.  */
   tree res_type = boolean_type_node;
@@ -14810,6 +14811,13 @@ test_vector_folding ()
   ASSERT_TRUE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type, zero, zero)));
   ASSERT_TRUE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, zero, one)));
   ASSERT_FALSE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, one, one)));
+  ASSERT_TRUE (integer_nonzerop (fold_build2 (NE_EXPR, res_type, index, one)));
+  ASSERT_FALSE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type,
+  index, one)));
+  ASSERT_FALSE (integer_nonzerop (fold_build2 (NE_EXPR, res_type,
+ index, index)));
+  ASSERT_TRUE (integer_nonzerop (fold_build2 (EQ_EXPR, res_type,
+ index, index)));
 }
 
 /* Verify folding of VEC_DUPLICATE_EXPRs.  */


Re: [PATCH 3/3] change class-key of PODs to struct and others to class (PR 61339)

2019-07-10 Thread Richard Sandiford
Richard Biener  writes:
> On Tue, Jul 9, 2019 at 8:34 PM Martin Sebor  wrote:
>>
>> On 7/9/19 9:17 AM, Richard Sandiford wrote:
>> > Martin Sebor  writes:
>> >> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>> >> index cfc41e1ed86..625d5b17413 100644
>> >> --- a/gcc/cp/cp-tree.h
>> >> +++ b/gcc/cp/cp-tree.h
>> >> @@ -6428,7 +6428,7 @@ extern tree get_scope_of_declarator
>> >> (const cp_declarator *);
>> >>   extern void grok_special_member_properties (tree);
>> >>   extern bool grok_ctor_properties   (const_tree, const_tree);
>> >>   extern bool grok_op_properties (tree, bool);
>> >> -extern tree xref_tag(enum tag_types, 
>> >> tree, tag_scope, bool, bool * = NULL);
>> >> +extern tree xref_tag(enum tag_types, 
>> >> tree, tag_scope, bool);
>> >>   extern tree xref_tag_from_type (tree, tree, 
>> >> tag_scope);
>> >>   extern void xref_basetypes (tree, tree);
>> >>   extern tree start_enum (tree, tree, tree, 
>> >> tree, bool, bool *);
>> >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>> >> index 005f99a6e15..9accc3d141b 100644
>> >> --- a/gcc/cp/decl.c
>> >> +++ b/gcc/cp/decl.c
>> >> @@ -14119,7 +14119,7 @@ lookup_and_check_tag (enum tag_types tag_code, 
>> >> tree name,
>> >>
>> >>   static tree
>> >>   xref_tag_1 (enum tag_types tag_code, tree name,
>> >> -tag_scope scope, bool template_header_p, bool *new_p)
>> >> +tag_scope scope, bool template_header_p)
>> >>   {
>> >> enum tree_code code;
>> >> tree context = NULL_TREE;
>> >> @@ -14151,9 +14151,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >> if (t == error_mark_node)
>> >>   return error_mark_node;
>> >>
>> >> -  /* Let the caller know this is a new type.  */
>> >> -  *new_p = t == NULL_TREE;
>> >> -
>> >> if (scope != ts_current && t && current_class_type
>> >> && template_class_depth (current_class_type)
>> >> && template_header_p)
>> >> @@ -14215,7 +14212,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >>scope = ts_current;
>> >>  }
>> >>t = pushtag (name, t, scope);
>> >> -  *new_p = true;
>> >>  }
>> >>   }
>> >> else
>> >> @@ -14267,13 +14263,11 @@ xref_tag_1 (enum tag_types tag_code, tree name,
>> >>
>> >>   tree
>> >>   xref_tag (enum tag_types tag_code, tree name,
>> >> -  tag_scope scope, bool template_header_p, bool *new_p /* = NULL 
>> >> */)
>> >> +  tag_scope scope, bool template_header_p)
>> >>   {
>> >> bool dummy;
>> >> -  if (!new_p)
>> >> -new_p = 
>> >> bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
>> >> -  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p, 
>> >> new_p);
>> >> +  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p);
>> >> timevar_cond_stop (TV_NAME_LOOKUP, subtime);
>> >> return ret;
>> >>   }
>> >> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>> >> index 52af8c0c6d6..d16bf253058 100644
>> >> --- a/gcc/cp/parser.c
>> >> +++ b/gcc/cp/parser.c
>> >> @@ -28193,8 +28193,6 @@ cp_parser_template_declaration_after_parameters 
>> >> (cp_parser* parser,
>> >> member_p,
>> >>  
>> >> /*explicit_specialization_p=*/false,
>> >> _p);
>> >> -  // maybe_warn_struct_vs_class (token->location, TREE_TYPE (decl));
>> >> -
>> >> pop_deferring_access_checks ();
>> >>
>> >> /* If this is a member template declaration, let the front
>> >
>> > Looks like this might have been part of 1/3.
>>
>> Yes, this and a few other hunks didn't belong in this patch.
>> I removed them, retested the patch, and committed r273311.
>>
>> >
>> > OK otherwise.  Thanks again for doing this.
>> >
>> > (I guess a lot of these tags could be removed, but that was just as true
>> > before the patch, so it's still a strict improvement.)
>>
>> Most could be removed and my own preference would have been to
>> remove them.  The warning has a mechanism for figuring out which
>> ones can one can go and which ones are needed and I considered
>> making use of it.  In the end I decided to be conservative and
>> keep them in case someone preferred it that way.  Making
>> the change now that the cleanup is done will be slightly more
>> involved.  I suppose we could add yet another warning to find
>> them: -Wredundant-tag.
>
> Just to pick one - why is struct loop not a POD?  Because of its
> widest_int members?

Yeah.

> But then we allocate it with ggc_cleared_alloc () which
> AFAICS doesn't invoke a constructor

Yeah, here and elsewhere we have a habit of initialising non-PODs
without using the proper constructor.

> (and I hope it doesn't trigger the finalization path).

Yeah, but that's based on whether it has a trivial destructor rather
than whether it's POD.

Thanks,

Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-10 Thread Richard Sandiford
Christophe Lyon  writes:
> On Mon, 8 Jul 2019 at 11:04, Richard Sandiford
>  wrote:
>>
>> Christophe Lyon  writes:
>> > Hi,
>> >
>> > This patch fixes PR 91060 where the lane ordering was no longer the
>> > right one (GCC's vs architecture's).
>>
>> Sorry, we clashed :-)
>>
>> I'd prefer to go with the version I attached to bugzilla just now.
>
> Yes just saw that, thanks!

The bugzilla version didn't properly adjust vec_setv2di_internal.
Fixed with the version below, tested on armeb-eabi.

Besides gcc.c-torture/execute/scal-to-vec*.c, the patch also fixes:

  c-c++-common/torture/vector-compare-1.c
  gcc.target/arm/pr69614.c
  g++.dg/ext/vector37.C

OK for trunk?

Richard


2019-07-10  Richard Sandiford  

gcc/
PR target/91060
* config/arm/iterators.md (V2DI_ONLY): New mode iterator.
* config/arm/neon.md (vec_set_internal): Add a '@' prefix.
(vec_setv2di_internal): Reexpress as...
(@vec_set_internal): ...this.
* config/arm/arm.c (neon_expand_vector_init): Use gen_vec_set_internal
rather than gen_neon_vset_lane.

Index: gcc/config/arm/iterators.md
===
--- gcc/config/arm/iterators.md 2019-06-18 09:35:55.377865698 +0100
+++ gcc/config/arm/iterators.md 2019-07-10 11:01:57.990749932 +0100
@@ -186,6 +186,9 @@ (define_mode_iterator VX [V8QI V4HI V16Q
 ;; Modes with 8-bit elements.
 (define_mode_iterator VE [V8QI V16QI])
 
+;; V2DI only (for use with @ patterns).
+(define_mode_iterator V2DI_ONLY [V2DI])
+
 ;; Modes with 64-bit elements only.
 (define_mode_iterator V64 [DI V2DI])
 
Index: gcc/config/arm/neon.md
===
--- gcc/config/arm/neon.md  2019-07-01 09:37:07.220524486 +0100
+++ gcc/config/arm/neon.md  2019-07-10 11:01:57.990749932 +0100
@@ -319,7 +319,7 @@ (define_insn "*movmisalign_neon_lo
   "vld1.\t{%q0}, %A1"
   [(set_attr "type" "neon_load1_1reg")])
 
-(define_insn "vec_set_internal"
+(define_insn "@vec_set_internal"
   [(set (match_operand:VD_LANE 0 "s_register_operand" "=w,w")
 (vec_merge:VD_LANE
   (vec_duplicate:VD_LANE
@@ -340,7 +340,7 @@ (define_insn "vec_set_internal"
 }
   [(set_attr "type" "neon_load1_all_lanes,neon_from_gp")])
 
-(define_insn "vec_set_internal"
+(define_insn "@vec_set_internal"
   [(set (match_operand:VQ2 0 "s_register_operand" "=w,w")
 (vec_merge:VQ2
   (vec_duplicate:VQ2
@@ -369,12 +369,12 @@ (define_insn "vec_set_internal"
   [(set_attr "type" "neon_load1_all_lanes,neon_from_gp")]
 )
 
-(define_insn "vec_setv2di_internal"
-  [(set (match_operand:V2DI 0 "s_register_operand" "=w,w")
-(vec_merge:V2DI
-  (vec_duplicate:V2DI
+(define_insn "@vec_set_internal"
+  [(set (match_operand:V2DI_ONLY 0 "s_register_operand" "=w,w")
+(vec_merge:V2DI_ONLY
+  (vec_duplicate:V2DI_ONLY
 (match_operand:DI 1 "nonimmediate_operand" "Um,r"))
-  (match_operand:V2DI 3 "s_register_operand" "0,0")
+  (match_operand:V2DI_ONLY 3 "s_register_operand" "0,0")
   (match_operand:SI 2 "immediate_operand" "i,i")))]
   "TARGET_NEON"
 {
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c2019-07-01 09:37:07.220524486 +0100
+++ gcc/config/arm/arm.c2019-07-10 11:01:57.990749932 +0100
@@ -12471,7 +12471,7 @@ neon_expand_vector_init (rtx target, rtx
   if (n_var == 1)
 {
   rtx copy = copy_rtx (vals);
-  rtx index = GEN_INT (one_var);
+  rtx merge_mask = GEN_INT (1 << one_var);
 
   /* Load constant part of vector, substitute neighboring value for
 varying element.  */
@@ -12480,38 +12480,7 @@ neon_expand_vector_init (rtx target, rtx
 
   /* Insert variable.  */
   x = copy_to_mode_reg (inner_mode, XVECEXP (vals, 0, one_var));
-  switch (mode)
-   {
-   case E_V8QImode:
- emit_insn (gen_neon_vset_lanev8qi (target, x, target, index));
- break;
-   case E_V16QImode:
- emit_insn (gen_neon_vset_lanev16qi (target, x, target, index));
- break;
-   case E_V4HImode:
- emit_insn (gen_neon_vset_lanev4hi (target, x, target, index));
- break;
-   case E_V8HImode:
- emit_insn (gen_neon_vset_lanev8hi (target, x, target, index));
- break;
-   case E_V2SImode:
- emit_insn (gen_neon_vset_lanev2si (target, x, target, index));
- break;
-   case E_V4SImode:
- emit_insn (gen_neon_vset_lanev4si (target, x, target, index));
- break;
-   case E_V2SFmode:
- emit_insn (gen_neon_vset_lanev2sf (target, x, target, index));
- break;
-   case E_V4SFmode:
- emit_insn (gen_neon_vset_lanev4sf (target, x, target, index));
- break;
-   case E_V2DImode:
- emit_insn (gen_neon_vset_lanev2di (target, x, target, index));
- break;
-   default:
- 

Re: **ping** Re: [PATCH] Automatics in equivalence statements

2019-07-10 Thread Mark Eggleston

Apologies typo in ChangeLog.

On 08/07/2019 14:51, Mark Eggleston wrote:

**ping**

On 01/07/2019 10:35, Mark Eggleston wrote:


On 25/06/2019 14:17, Mark Eggleston wrote:


On 25/06/2019 00:17, Jeff Law wrote:

On 6/24/19 2:19 AM, Bernhard Reutner-Fischer wrote:

On Fri, 21 Jun 2019 07:10:11 -0700
Steve Kargl  wrote:


On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote:
Currently variables with the AUTOMATIC attribute can not appear 
in an
EQUIVALENCE statement. However its counterpart, STATIC, can be 
used in

an EQUIVALENCE statement.

Where there is a clear conflict in the attributes of variables 
in an
EQUIVALENCE statement an error message will be issued as is 
currently

the case.

If there is no conflict e.g. a variable with a AUTOMATIC 
attribute and a
variable(s) without attributes all variables in the EQUIVALENCE 
will

become AUTOMATIC.

Note: most of this patch was written by Jeff Law 

Please review.

ChangeLogs:

gcc/fortran

      Jeff Law  
      Mark Eggleston 

      * gfortran.h: Add check_conflict declaration.

This is wrong.  By convention a routine that is not static
has the gfc_ prefix.

Updated the code to use gfc_check_conflict instead.



Furthermore doesn't this export indicate that you're committing a
layering violation somehow?

Possibly.  I'm the original author, but my experience in our fortran
front-end is minimal.  I fully expected this patch to need some 
tweaking.


We certainly don't want to recreate all the checking that's done in
check_conflict.  We just need to defer it to a later point --
find_equivalence seemed like a good point since we've got the full
equivalence list handy and can accumulate the attributes across the
entire list, then check for conflicts.

If there's a concrete place where you think we should be doing 
this, I'm

all ears.


Any suggestions will be appreciate.
      * symbol.c (check_conflict): Remove automatic in 
equivalence conflict

      check.
      * symbol.c (save_symbol): Add check for in equivalence to 
stop the

      the save attribute being added.
      * trans-common.c (build_equiv_decl): Add is_auto parameter and
      add !is_auto to condition where TREE_STATIC (decl) is set.
      * trans-common.c (build_equiv_decl): Add local variable 
is_auto,
      set it true if an atomatic attribute is encountered in the 
variable

atomatic? I read atomic but you mean automatic.

Yes.

      list.  Call build_equiv_decl with is_auto as an additional 
parameter.

      flag_dec_format_defaults is enabled.
      * trans-common.c (accumulate_equivalence_attributes) : New 
subroutine.
      * trans-common.c (find_equivalence) : New local variable 
dummy_symbol,
      accumulated equivalence attributes from each symbol then 
check for

      conflicts.
I'm just curious why you don't gfc_copy_attr for the most part of 
accumulate_equivalence_attributes?

thanks,

Simply didn't know about it.  It could probably significantly simplify
the accumulation of attributes step.
Using gfc_copy_attr causes a great many "Duplicate DIMENSION 
attribute specified at (1)" errors. This is because there is a great 
deal of checking done instead of simply keeping track of the 
attributes used which is all that is required for determining 
whether there is a conflict in the equivalence statement.


Also, the final section of accumulate_equivalence_attributes 
involving SAVE, INTENT and ACCESS look suspect to me. I'll check and 
update the patch if necessary.


No need to check intent as there is already a conflict with DUMMY and 
INTENT can only be present for dummy variables.


Please find attached an updated patch. Change logs:

gcc/fortran

    Jeff Law  
    Mark Eggleston  

    * gfortran.h: Add gfc_check_conflict declaration.
    * symbol.c (check_conflict): Rename cfg_check_conflict and remove
    static.
    * symbol.c (cfg_check_conflict): Remove automatic in equivalence
    conflict check.

    * symbol.c (gfc_check_conflict): Remove automatic in equivalence
    conflict check.

    * symbol.c (save_symbol): Add check for in equivalence to stop the
    the save attribute being added.
    * trans-common.c (build_equiv_decl): Add is_auto parameter and
    add !is_auto to condition where TREE_STATIC (decl) is set.
    * trans-common.c (build_equiv_decl): Add local variable is_auto,
    set it true if an atomatic attribute is encountered in the variable
    list.  Call build_equiv_decl with is_auto as an additional 
parameter.

    flag_dec_format_defaults is enabled.
    * trans-common.c (accumulate_equivalence_attributes) : New 
subroutine.
    * trans-common.c (find_equivalence) : New local variable 
dummy_symbol,

    accumulated equivalence attributes from each symbol then check for
    conflicts.

gcc/testsuite

    Mark Eggleston 

    * gfortran.dg/auto_in_equiv_1.f90: New test.
    * gfortran.dg/auto_in_equiv_2.f90: New test.
    * gfortran.dg/auto_in_equiv_3.f90: New test.

If the updated patch is acceptable, please can someone with 

Re: [PATCH 3/3] change class-key of PODs to struct and others to class (PR 61339)

2019-07-10 Thread Richard Biener
On Tue, Jul 9, 2019 at 8:34 PM Martin Sebor  wrote:
>
> On 7/9/19 9:17 AM, Richard Sandiford wrote:
> > Martin Sebor  writes:
> >> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> >> index cfc41e1ed86..625d5b17413 100644
> >> --- a/gcc/cp/cp-tree.h
> >> +++ b/gcc/cp/cp-tree.h
> >> @@ -6428,7 +6428,7 @@ extern tree get_scope_of_declarator
> >> (const cp_declarator *);
> >>   extern void grok_special_member_properties (tree);
> >>   extern bool grok_ctor_properties   (const_tree, const_tree);
> >>   extern bool grok_op_properties (tree, bool);
> >> -extern tree xref_tag(enum tag_types, 
> >> tree, tag_scope, bool, bool * = NULL);
> >> +extern tree xref_tag(enum tag_types, 
> >> tree, tag_scope, bool);
> >>   extern tree xref_tag_from_type (tree, tree, 
> >> tag_scope);
> >>   extern void xref_basetypes (tree, tree);
> >>   extern tree start_enum (tree, tree, tree, 
> >> tree, bool, bool *);
> >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> >> index 005f99a6e15..9accc3d141b 100644
> >> --- a/gcc/cp/decl.c
> >> +++ b/gcc/cp/decl.c
> >> @@ -14119,7 +14119,7 @@ lookup_and_check_tag (enum tag_types tag_code, 
> >> tree name,
> >>
> >>   static tree
> >>   xref_tag_1 (enum tag_types tag_code, tree name,
> >> -tag_scope scope, bool template_header_p, bool *new_p)
> >> +tag_scope scope, bool template_header_p)
> >>   {
> >> enum tree_code code;
> >> tree context = NULL_TREE;
> >> @@ -14151,9 +14151,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
> >> if (t == error_mark_node)
> >>   return error_mark_node;
> >>
> >> -  /* Let the caller know this is a new type.  */
> >> -  *new_p = t == NULL_TREE;
> >> -
> >> if (scope != ts_current && t && current_class_type
> >> && template_class_depth (current_class_type)
> >> && template_header_p)
> >> @@ -14215,7 +14212,6 @@ xref_tag_1 (enum tag_types tag_code, tree name,
> >>scope = ts_current;
> >>  }
> >>t = pushtag (name, t, scope);
> >> -  *new_p = true;
> >>  }
> >>   }
> >> else
> >> @@ -14267,13 +14263,11 @@ xref_tag_1 (enum tag_types tag_code, tree name,
> >>
> >>   tree
> >>   xref_tag (enum tag_types tag_code, tree name,
> >> -  tag_scope scope, bool template_header_p, bool *new_p /* = NULL 
> >> */)
> >> +  tag_scope scope, bool template_header_p)
> >>   {
> >> bool dummy;
> >> -  if (!new_p)
> >> -new_p = 
> >> bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
> >> -  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p, new_p);
> >> +  tree ret = xref_tag_1 (tag_code, name, scope, template_header_p);
> >> timevar_cond_stop (TV_NAME_LOOKUP, subtime);
> >> return ret;
> >>   }
> >> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> >> index 52af8c0c6d6..d16bf253058 100644
> >> --- a/gcc/cp/parser.c
> >> +++ b/gcc/cp/parser.c
> >> @@ -28193,8 +28193,6 @@ cp_parser_template_declaration_after_parameters 
> >> (cp_parser* parser,
> >> member_p,
> >>  
> >> /*explicit_specialization_p=*/false,
> >> _p);
> >> -  // maybe_warn_struct_vs_class (token->location, TREE_TYPE (decl));
> >> -
> >> pop_deferring_access_checks ();
> >>
> >> /* If this is a member template declaration, let the front
> >
> > Looks like this might have been part of 1/3.
>
> Yes, this and a few other hunks didn't belong in this patch.
> I removed them, retested the patch, and committed r273311.
>
> >
> > OK otherwise.  Thanks again for doing this.
> >
> > (I guess a lot of these tags could be removed, but that was just as true
> > before the patch, so it's still a strict improvement.)
>
> Most could be removed and my own preference would have been to
> remove them.  The warning has a mechanism for figuring out which
> ones can one can go and which ones are needed and I considered
> making use of it.  In the end I decided to be conservative and
> keep them in case someone preferred it that way.  Making
> the change now that the cleanup is done will be slightly more
> involved.  I suppose we could add yet another warning to find
> them: -Wredundant-tag.

Just to pick one - why is struct loop not a POD?  Because of its
widest_int members?  But then we allocate it with
ggc_cleared_alloc () which AFAICS doesn't
invoke a constructor (and I hope it doesn't trigger the finalization
path).

Richard.

>
> Martin


[Ada] Do not attempt to re-arm guard page on x86_64-vx7(r2)

2019-07-10 Thread Pierre-Marie de Rodat
A change in the API prohibits accessing Tcb fields directly. The bug in
VxWorks7 (failure to re-arm the guard page) now appears to be fixed, so
this is no long necessary.

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

2019-07-10  Doug Rupp  

gcc/ada/

* init.c: Do not attempt to re-arm guard page on x86_64-vx7(r2).--- gcc/ada/init.c
+++ gcc/ada/init.c
@@ -1725,7 +1725,7 @@ __gnat_install_handler (void)
 #include 
 #endif
 
-#if ((defined (ARMEL) && (_WRS_VXWORKS_MAJOR == 6)) || defined (__x86_64__)) && !defined(__RTP__)
+#if ((defined (ARMEL) && (_WRS_VXWORKS_MAJOR == 6))) && !defined(__RTP__)
 #define VXWORKS_FORCE_GUARD_PAGE 1
 #include 
 extern size_t vxIntStackOverflowSize;



[Ada] System.Strings.Stream_Ops: do not depend on Stream_IO

2019-07-10 Thread Pierre-Marie de Rodat
Dependence was only from Ada.Streams.Stream_IO.End_Error exception which
is renaming of the Ada.IO_Exceptions.End_Error. Use
Ada.IO_Exceptions.End_Error directly.

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

2019-07-10  Dmitriy Anisimkov  

gcc/ada/

* libgnat/s-ststop.adb: Remove System.Strings.Stream_Ops
dependence on System.Streams.Stream_IO.--- gcc/ada/libgnat/s-ststop.adb
+++ gcc/ada/libgnat/s-ststop.adb
@@ -31,8 +31,8 @@
 
 pragma Compiler_Unit_Warning;
 
+with Ada.IO_Exceptions;use Ada.IO_Exceptions;
 with Ada.Streams;  use Ada.Streams;
-with Ada.Streams.Stream_IO;use Ada.Streams.Stream_IO;
 with Ada.Unchecked_Conversion;
 
 with System;   use System;



[Ada] Crash on aggregate for limited type in extended return

2019-07-10 Thread Pierre-Marie de Rodat
This patch fixes a compiler abort on an extended return statement whose
expression is an aggregate (to be built in place) for a discriminated
record with a limited component. The build-in-place mechanism creates an
access type and a renaming declaration  through which individual
components are assigned. The renamed object is constrained because it is
limited, and the renaming declaration does not need to create a local
subtype indication for it, which may lead to type mismatches in the
back-end, and is in any case redundant. This patch extends this
optimization to the case of records that are limited only because of a
limitied component, and not because they are explicit declared limited.

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

2019-07-10  Ed Schonberg  

gcc/ada/

* sem_ch8.adb (Check_Constrained_Object): A record that is
limited because of the presence of a limited component is
constrained, and no subtype indiciation needs to be created for
it, just as is the case for declared limited records.

gcc/testsuite/

* gnat.dg/limited3.adb, gnat.dg/limited3_pkg.adb,
gnat.dg/limited3_pkg.ads: New testcase.--- gcc/ada/sem_ch8.adb
+++ gcc/ada/sem_ch8.adb
@@ -802,12 +802,17 @@ package body Sem_Ch8 is
null;
 
 --  If a record is limited its size is invariant. This is the case
---  in particular with record types with an access discirminant
+--  in particular with record types with an access discriminant
 --  that are used in iterators. This is an optimization, but it
 --  also prevents typing anomalies when the prefix is further
---  expanded. Limited types with discriminants are included.
-
-elsif Is_Limited_Record (Typ)
+--  expanded. This also applies to limited types with access
+--  discriminants.
+--  Note that we cannot just use the Is_Limited_Record flag because
+--  it does not apply to records with limited components, for which
+--  this syntactic flag is not set, but whose size is also fixed.
+
+elsif (Is_Record_Type (Typ)
+and then Is_Limited_Type (Typ))
   or else
 (Ekind (Typ) = E_Limited_Private_Type
   and then Has_Discriminants (Typ)

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited3.adb
@@ -0,0 +1,11 @@
+--  { dg-do run }
+
+with Limited3_Pkg; use Limited3_Pkg;
+
+procedure Limited3 is
+   R1 : Rec := F (15);
+   R2 : Rec := F (-1);
+   R3 : Var_Rec := FS (20);
+begin
+   null;
+end Limited3;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited3_pkg.adb
@@ -0,0 +1,20 @@
+package body Limited3_Pkg is
+   function F (I : Integer) return Rec is
+   begin
+  return (D => False, I => I);
+   end;
+
+   function FS (X : Integer) return Var_Rec is
+   begin
+  return (X, (1..X => '?'), Tag => <>);
+   end FS;
+
+   function F2 (I : Integer) return Rec2 is
+   begin
+  if I > 0 then
+ return (D => False, I => I);
+  else
+ return (D => True, L => new Limited_Rec);
+ end if;
+   end;
+end Limited3_Pkg;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited3_pkg.ads
@@ -0,0 +1,30 @@
+package Limited3_Pkg is
+
+   type Limited_Rec is limited
+null record;
+
+   type Var_Rec (X : Integer) is record
+  Name : String (1 .. X);
+  Tag  : Limited_Rec;
+   end record;
+
+   type Rec (D : Boolean := True) is record
+  case D is
+ when True => L : Limited_Rec;
+ when False => I : Integer;
+  end case;
+   end record;
+
+   function F (I : Integer) return Rec;
+
+   function FS (X : Integer) return Var_Rec;
+
+   type Rec2 (D : Boolean := True) is record
+  case D is
+ when True => L : access Limited_Rec;
+ when False => I : Integer;
+  end case;
+   end record;
+
+   function F2 (I : Integer) return Rec2;
+end Limited3_Pkg;



[Ada] Spurious run-time error with 64-bit modular types

2019-07-10 Thread Pierre-Marie de Rodat
As a lexical element an integer literal has type Universal_Integer, i.e
is compatible with any integer type. This is semantically consistent and
simplifies type checking and subsequent constant folding when
applicable.  An exception is caused by 64-bit modular types, whose upper
bound is not representable in a non-static context that will use 64-bit
integers at run-time. For such cases we need to preserve the information
that the analyzed literal has that modular type. For simplicity we
preseve the information for all integer literals that result from a
modular operation.  This happens after prior analysis (or construction)
of the literal, and after type checking and resolution.

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

2019-07-10  Ed Schonberg  

gcc/ada/

* sem_ch2.adb (Analyze_Integer_Literal): Preserve the type of
the literal if prior analysis determined that its type is a
modular integer type.

gcc/testsuite/

* gnat.dg/modular5.adb: New testcase.--- gcc/ada/sem_ch2.adb
+++ gcc/ada/sem_ch2.adb
@@ -24,12 +24,14 @@
 --
 
 with Atree;use Atree;
+with Einfo;use Einfo;
 with Namet;use Namet;
 with Opt;  use Opt;
 with Restrict; use Restrict;
 with Rident;   use Rident;
 with Sem_Ch8;  use Sem_Ch8;
 with Sem_Dim;  use Sem_Dim;
+--  with Sem_Util; use Sem_Util;
 with Sinfo;use Sinfo;
 with Stand;use Stand;
 with Uintp;use Uintp;
@@ -83,7 +85,24 @@ package body Sem_Ch2 is
 
procedure Analyze_Integer_Literal (N : Node_Id) is
begin
-  Set_Etype (N, Universal_Integer);
+  --  As a lexical element, an integer literal has type Universal_Integer,
+  --  i.e., is compatible with any integer type. This is semantically
+  --  consistent and simplifies type checking and subsequent constant
+  --  folding when needed. An exception is caused by 64-bit modular types,
+  --  whose upper bound is not representable in a nonstatic context that
+  --  will use 64-bit integers at run time. For such cases, we need to
+  --  preserve the information that the analyzed literal has that modular
+  --  type. For simplicity, we preserve the information for all integer
+  --  literals that result from a modular operation. This happens after
+  --  prior analysis (or construction) of the literal, and after type
+  --  checking and resolution.
+
+  if No (Etype (N))
+or else not Is_Modular_Integer_Type (Etype (N))
+  then
+ Set_Etype (N, Universal_Integer);
+  end if;
+
   Set_Is_Static_Expression (N);
end Analyze_Integer_Literal;
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/modular5.adb
@@ -0,0 +1,26 @@
+-- { dg-do compile }
+-- { dg-options "-gnata" }
+
+procedure Modular5 is
+   type U64 is mod 2 ** 64;
+   Maybe: Boolean := 2 ** 10 < U64'Succ (U64'last - 1);
+   For_Sure : Boolean := U64'(18446744073709551615) > 2;
+   Ditto: Boolean := 18446744073709551615 > 2;
+
+   generic
+  type TG is mod <>;
+   package PG is
+ X : TG;
+  pragma Assert (for all K in 1 .. 2 => 2 ** K <= TG'Last);
+  pragma Assert (for all K in 1 .. 2 => 2 ** K <= TG'Last - 1);
+
+ Maybe: Boolean := 2 ** 10 < TG'Succ (TG'last - 1);
+ For_Sure : Boolean := TG'(18446744073709551615) > 2;
+   end PG;
+
+   package IG is new PG (U64);
+
+begin
+   pragma Assert (for all K in 1 .. 2 => 2 ** K <= U64'Last);
+   pragma Assert (for all K in 1 .. 2 => 2 ** K <= U64'Last - 1);
+end Modular5;



[Ada] Fix possible crashes in GNATprove analysis of pointers

2019-07-10 Thread Pierre-Marie de Rodat
The new analysis of SPARK pointer rules could crash on some constructs.
Now fixed.

There is no impact on compilation.

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

2019-07-10  Claire Dross  

gcc/ada/

* sem_spark.adb (Check_Expression): Allow digits constraints as
input.
(Illegal_Global_Usage): Pass in the entity.
(Is_Subpath_Expression): New function to allow different nodes
as inner parts of a path expression.
(Read_Indexes): Allow concatenation and aggregates with box
expressions.  Allow attributes Update and Loop_Entry.
(Check_Expression): Allow richer membership test.
(Check_Node): Ignore bodies of generics.
(Get_Root_Object): Allow concatenation and attributes.--- gcc/ada/sem_spark.adb
+++ gcc/ada/sem_spark.adb
@@ -640,7 +640,8 @@ package body Sem_SPARK is
procedure Check_Expression (Expr : Node_Id; Mode : Extended_Checking_Mode);
pragma Precondition (Nkind_In (Expr, N_Index_Or_Discriminant_Constraint,
 N_Range_Constraint,
-N_Subtype_Indication)
+N_Subtype_Indication,
+N_Digits_Constraint)
 or else Nkind (Expr) in N_Subexpr);
 
procedure Check_Globals (Subp : Entity_Id);
@@ -738,7 +739,7 @@ package body Sem_SPARK is
--  the debugger to look into a hash table.
pragma Unreferenced (Hp);
 
-   procedure Illegal_Global_Usage (N : Node_Or_Entity_Id);
+   procedure Illegal_Global_Usage (N : Node_Or_Entity_Id; E : Entity_Id);
pragma No_Return (Illegal_Global_Usage);
--  A procedure that is called when deep globals or aliased globals are used
--  without any global aspect.
@@ -750,6 +751,9 @@ package body Sem_SPARK is
function Is_Path_Expression (Expr : Node_Id) return Boolean;
--  Return whether Expr corresponds to a path
 
+   function Is_Subpath_Expression (Expr : Node_Id) return Boolean;
+   --  Return True if Expr can be part of a path expression
+
function Is_Prefix_Or_Almost (Pref, Expr : Node_Id) return Boolean;
--  Determine if the candidate Prefix is indeed a prefix of Expr, or almost
--  a prefix, in the sense that they could still refer to overlapping memory
@@ -1302,7 +1306,9 @@ package body Sem_SPARK is
begin
   --  Only SPARK bodies are analyzed
 
-  if No (Prag) or else Get_SPARK_Mode_From_Annotation (Prag) /= Opt.On then
+  if No (Prag)
+or else Get_SPARK_Mode_From_Annotation (Prag) /= Opt.On
+  then
  return;
   end if;
 
@@ -1312,9 +1318,8 @@ package body Sem_SPARK is
 and then Is_Anonymous_Access_Type (Etype (Spec_Id))
 and then not Is_Traversal_Function (Spec_Id)
   then
- Error_Msg_N
-   ("anonymous access type for result only allowed for traveral "
-& "functions", Spec_Id);
+ Error_Msg_N ("anonymous access type for result only allowed for "
+  & "traveral functions", Spec_Id);
  return;
   end if;
 
@@ -1568,7 +1573,7 @@ package body Sem_SPARK is
   --  Start of processing for Read_Indexes
 
   begin
- if not Is_Path_Expression (Expr) then
+ if not Is_Subpath_Expression (Expr) then
 Error_Msg_N ("name expected here for move/borrow/observe", Expr);
 return;
  end if;
@@ -1603,6 +1608,10 @@ package body Sem_SPARK is
Read_Params (Expr);
Check_Globals (Get_Called_Entity (Expr));
 
+when N_Op_Concat =>
+   Read_Expression (Left_Opnd (Expr));
+   Read_Expression (Right_Opnd (Expr));
+
 when N_Qualified_Expression
| N_Type_Conversion
| N_Unchecked_Type_Conversion
@@ -1644,7 +1653,8 @@ package body Sem_SPARK is
  --  There can be only one element for a value of deep type
  --  in order to avoid aliasing.
 
- if Is_Deep (Etype (Expression (Assoc)))
+ if not (Box_Present (Assoc))
+   and then Is_Deep (Etype (Expression (Assoc)))
and then not Is_Singleton_Choice (CL)
  then
 Error_Msg_F
@@ -1655,7 +1665,9 @@ package body Sem_SPARK is
  --  The subexpressions of an aggregate are moved as part
  --  of the implicit assignments.
 
- Move_Expression (Expression (Assoc));
+ if not Box_Present (Assoc) then
+Move_Expression (Expression (Assoc));
+ end if;
 
  Next (Assoc);
   end loop;
@@ -1689,12 +1701,28 @@ package body Sem_SPARK is
  --  The subexpressions of an aggregate are moved as part
  --  of the implicit assignments.
 
-

[Ada] Spurious error on discriminant of incomplete type

2019-07-10 Thread Pierre-Marie de Rodat
This patch corrects the conformance verification of discriminants to
provide symmetry between the analysis of incomplete and full view
discriminants. As a result, types of discriminants always resolve to the
proper view.

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

2019-07-10  Hristian Kirtchev  

gcc/ada/

* sem_ch6.adb (Check_Discriminant_Conformance): Use Find_Type to
discover the type of a full view discriminant.

gcc/testsuite/

* gnat.dg/incomplete7.adb, gnat.dg/incomplete7.ads: New testcase.--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -5960,7 +5960,7 @@ package body Sem_Ch6 is
   Access_Definition (N, Discriminant_Type (New_Discr));
 
  else
-Analyze (Discriminant_Type (New_Discr));
+Find_Type (Discriminant_Type (New_Discr));
 New_Discr_Type := Etype (Discriminant_Type (New_Discr));
 
 --  Ada 2005: if the discriminant definition carries a null

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/incomplete7.adb
@@ -0,0 +1,5 @@
+--  { dg-do compile }
+
+package body Incomplete7 is
+   procedure Foo is null;
+end Incomplete7;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/incomplete7.ads
@@ -0,0 +1,31 @@
+package Incomplete7 is
+   type Color;
+   type Color is (red, green, blue);
+
+   type Action (C : Color := Color'(red));
+   type Action (C : Color := Color'(red)) is record
+  case C is
+ when red =>
+Stop_Time : Positive;
+
+ when others =>
+Go_For_It : Integer;
+  end case;
+   end record;
+
+   type Num;
+   type Num is new Integer;
+
+   type Rec (N : Num := Num'(1));
+   type Rec (N : Num := Num'(1)) is record
+  case N is
+ when 1 =>
+One : Integer;
+
+ when others =>
+null;
+  end case;
+   end record;
+
+   procedure Foo;
+end Incomplete7;



[Ada] Fix spurious messages on global variables for SPARK pointer support

2019-07-10 Thread Pierre-Marie de Rodat
Pointer support in GNATprove leads to spurious messages about global
variables, with local variables declared in local packages and protected
components. Now fixed.

There is no impact on compilation.

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

2019-07-10  Yannick Moy  

gcc/ada/

* sem_aux.adb, sem_aux.ads (Is_Protected_Operation): New
function to determine if a subprogram is protected.
* sem_spark.adb (Setup_Protected_Components): New procedure to
add protected components to the environment.
(Check_Callable_Body): Call the new Setup_Protected_Components.
(Check_Package_Spec): Merge local environment with enclosing one
when done.--- gcc/ada/sem_aux.adb
+++ gcc/ada/sem_aux.adb
@@ -1324,6 +1324,18 @@ package body Sem_Aux is
   end if;
end Is_Limited_View;
 
+   
+   -- Is_Protected_Operation --
+   
+
+   function Is_Protected_Operation (E : Entity_Id) return Boolean is
+   begin
+  return Is_Entry (E)
+or else (Is_Subprogram (E)
+ and then Nkind (Parent (Unit_Declaration_Node (E))) =
+N_Protected_Definition);
+   end Is_Protected_Operation;
+
--
-- Nearest_Ancestor --
--

--- gcc/ada/sem_aux.ads
+++ gcc/ada/sem_aux.ads
@@ -357,6 +357,10 @@ package Sem_Aux is
--  these types). This older routine overlaps with the previous one, this
--  should be cleaned up???
 
+   function Is_Protected_Operation (E : Entity_Id) return Boolean;
+   --  Given a subprogram or entry, determines whether E is a protected entry
+   --  or subprogram.
+
function Nearest_Ancestor (Typ : Entity_Id) return Entity_Id;
--  Given a subtype Typ, this function finds out the nearest ancestor from
--  which constraints and predicates are inherited. There is no simple link

--- gcc/ada/sem_spark.adb
+++ gcc/ada/sem_spark.adb
@@ -876,6 +876,10 @@ package body Sem_SPARK is
--  Takes a subprogram as input, and sets up the environment by adding
--  formal parameters with appropriate permissions.
 
+   procedure Setup_Protected_Components (Subp : Entity_Id);
+   --  Takes a protected operation as input, and sets up the environment by
+   --  adding protected components with appropriate permissions.
+
--
-- Global Variables --
--
@@ -1336,6 +1340,13 @@ package body Sem_SPARK is
  Setup_Globals (Spec_Id);
   end if;
 
+  --  For protected operations, add protected components to the environment
+  --  with adequate permissions.
+
+  if Is_Protected_Operation (Spec_Id) then
+ Setup_Protected_Components (Spec_Id);
+  end if;
+
   --  Analyze the body of the subprogram
 
   Check_List (Declarations (Body_N));
@@ -2634,9 +2645,13 @@ package body Sem_SPARK is
 Check_List (Private_Declarations (Spec));
  end if;
 
- --  Restore the saved environment and free the current one
+ --  Restore the saved environment and free the current one. As part of
+ --  the restoration, the environment of the package spec is merged in
+ --  the enclosing environment, which may be an enclosing
+ --  package/subprogram spec or body which has access to the variables
+ --  of the package spec.
 
- Move_Env (Saved_Env, Current_Perm_Env);
+ Merge_Env (Saved_Env, Current_Perm_Env);
 
  Inside_Elaboration := Save_In_Elab;
   end if;
@@ -5418,4 +5433,37 @@ package body Sem_SPARK is
   end loop;
end Setup_Parameters;
 
+   
+   -- Setup_Protected_Components --
+   
+
+   procedure Setup_Protected_Components (Subp : Entity_Id) is
+  Typ  : constant Entity_Id := Scope (Subp);
+  Comp : Entity_Id;
+  Kind : Formal_Kind;
+
+   begin
+  Comp := First_Component_Or_Discriminant (Typ);
+
+  --  The protected object is an implicit input of protected functions, and
+  --  an implicit input-output of protected procedures and entries.
+
+  if Ekind (Subp) = E_Function then
+ Kind := E_In_Parameter;
+  else
+ Kind := E_In_Out_Parameter;
+  end if;
+
+  while Present (Comp) loop
+ Setup_Parameter_Or_Global
+   (Id => Comp,
+Typ=> Underlying_Type (Etype (Comp)),
+Kind   => Kind,
+Subp   => Subp,
+Global_Var => False,
+Expl   => Comp);
+ Next_Component_Or_Discriminant (Comp);
+  end loop;
+   end Setup_Protected_Components;
+
 end Sem_SPARK;



[Ada] Entity names are not unique

2019-07-10 Thread Pierre-Marie de Rodat
This patch updates the Unique_Name procedure in order to prefix the
string "ada___" to child units that have a nested subprogram or package,
so that they do not clash with a parent package of the same name.

This is for GNATprove only and does not affect regular compilation.

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

2019-07-10  Simon Buist  

gcc/ada/

* sem_util.ads (Child_Prefix): New constant.
* sem_util.adb (Unique_Name): Add a special prefix to child
units that have a nested subprogram or package.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -25781,6 +25781,8 @@ package body Sem_Util is
   end if;
end;
 
+elsif Is_Child_Unit (U) then
+   return Child_Prefix & Unique_Name (S) & "__" & This_Name;
 else
return Unique_Name (S) & "__" & This_Name;
 end if;

--- gcc/ada/sem_util.ads
+++ gcc/ada/sem_util.ads
@@ -2854,6 +2854,10 @@ package Sem_Util is
--  Return a unique name for entity E, which could be used to identify E
--  across compilation units.
 
+   Child_Prefix : constant String := "ada___";
+   --  Prefix for child packages when building a unique name for an entity. It
+   --  is included here to share between Unique_Name and gnatprove.
+
function Unit_Is_Visible (U : Entity_Id) return Boolean;
--  Determine whether a compilation unit is visible in the current context,
--  because there is a with_clause that makes the unit available. Used to



[Ada] Allow multiple units per file in GNATprove

2019-07-10 Thread Pierre-Marie de Rodat
For analysis tools that rely on information generated in ALI files, but
do not generate object files, the frontend did not generate the special
extension names like file~2.ali for unit 2 in the file. This is needed
to be able to analyze files with multiple units. Now fixed.

There is no impact on compilation.

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

2019-07-10  Yannick Moy  

gcc/ada/

* osint-c.adb (Set_File_Name): Always add extension for multiple
units per file mode.--- gcc/ada/osint-c.adb
+++ gcc/ada/osint-c.adb
@@ -385,6 +385,21 @@ package body Osint.C is
  end if;
   end loop;
 
+  --  If we are in multiple-units-per-file mode, then add a ~nnn extension
+  --  to the name.
+
+  if Multiple_Unit_Index /= 0 then
+ declare
+Exten : constant String := Name_Buffer (Dot_Index .. Name_Len);
+ begin
+Name_Len := Dot_Index - 1;
+Add_Char_To_Name_Buffer (Multi_Unit_Index_Character);
+Add_Nat_To_Name_Buffer (Multiple_Unit_Index);
+Dot_Index := Name_Len + 1;
+Add_Str_To_Name_Buffer (Exten);
+ end;
+  end if;
+
   --  Make sure that the output file name matches the source file name.
   --  To compare them, remove file name directories and extensions.
 
@@ -395,21 +410,6 @@ package body Osint.C is
 
  Name_Buffer (Dot_Index) := '.';
 
- --  If we are in multiple unit per file mode, then add ~nnn
- --  extension to the name before doing the comparison.
-
- if Multiple_Unit_Index /= 0 then
-declare
-   Exten : constant String := Name_Buffer (Dot_Index .. Name_Len);
-begin
-   Name_Len := Dot_Index - 1;
-   Add_Char_To_Name_Buffer (Multi_Unit_Index_Character);
-   Add_Nat_To_Name_Buffer (Multiple_Unit_Index);
-   Dot_Index := Name_Len + 1;
-   Add_Str_To_Name_Buffer (Exten);
-end;
- end if;
-
  --  Remove extension preparing to replace it
 
  declare



[Ada] Spelling mistakes in error messages

2019-07-10 Thread Pierre-Marie de Rodat
This patch updates certain error messages to eliminate spelling
mistakes. No need for a test as this is a minor cosmetic fix.

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

2019-07-10  Hristian Kirtchev  

gcc/ada/

* sem_ch3.adb (Check_Nonoverridable_Aspects): Correct the
spelling in certain error messages.
(Check_Pragma_Implemented): Correct the spelling in certain
error messages.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -3009,14 +3009,15 @@ package body Sem_Ch3 is
--  is consistent with that of the parent.
 
declare
-  Par_Discr  : constant Entity_Id :=
-Get_Reference_Discriminant (Par_Type);
-  Cur_Discr  : constant Entity_Id :=
+  Cur_Discr : constant Entity_Id :=
 Get_Reference_Discriminant (Prev);
+  Par_Discr : constant Entity_Id :=
+Get_Reference_Discriminant (Par_Type);
 
begin
   if Corresponding_Discriminant (Cur_Discr) /= Par_Discr then
- Error_Msg_N ("aspect incosistent with that of parent", N);
+ Error_Msg_N
+   ("aspect inconsistent with that of parent", N);
   end if;
 
   --  Check that specification in partial view matches the
@@ -3029,7 +3030,7 @@ package body Sem_Ch3 is
Chars (Cur_Discr)
   then
  Error_Msg_N
-   ("aspect incosistent with that of parent", N);
+   ("aspect inconsistent with that of parent", N);
   end if;
end;
 end if;
@@ -10641,9 +10642,9 @@ package body Sem_Ch3 is
 if Ekind (Contr_Typ) /= E_Protected_Type then
Error_Msg_Node_2 := Contr_Typ;
Error_Msg_NE
- ("interface subprogram & cannot be implemented by a " &
-  "primitive procedure of task type &", Subp_Alias,
-  Iface_Alias);
+ ("interface subprogram & cannot be implemented by a "
+  & "primitive procedure of task type &",
+  Subp_Alias, Iface_Alias);
 
 --  An interface subprogram whose implementation kind is By_
 --  Protected_Procedure must be implemented by a procedure.
@@ -10651,28 +10652,27 @@ package body Sem_Ch3 is
 elsif Ekind (Impl_Subp) /= E_Procedure then
Error_Msg_Node_2 := Iface_Alias;
Error_Msg_NE
- ("type & must implement abstract subprogram & with a " &
-  "procedure", Subp_Alias, Contr_Typ);
+ ("type & must implement abstract subprogram & with a "
+  & "procedure", Subp_Alias, Contr_Typ);
 
 elsif Present (Get_Rep_Pragma (Impl_Subp, Name_Implemented))
   and then Implementation_Kind (Impl_Subp) /= Impl_Kind
 then
Error_Msg_Name_1 := Impl_Kind;
Error_Msg_N
-("overriding operation& must have synchronization%",
- Subp_Alias);
+ ("overriding operation& must have synchronization%",
+  Subp_Alias);
 end if;
 
  --  If primitive has Optional synchronization, overriding operation
- --  must match if it has an explicit synchronization..
+ --  must match if it has an explicit synchronization.
 
  elsif Present (Get_Rep_Pragma (Impl_Subp, Name_Implemented))
and then Implementation_Kind (Impl_Subp) /= Impl_Kind
  then
-   Error_Msg_Name_1 := Impl_Kind;
-   Error_Msg_N
-("overriding operation& must have syncrhonization%",
- Subp_Alias);
+Error_Msg_Name_1 := Impl_Kind;
+Error_Msg_N
+  ("overriding operation& must have synchronization%", Subp_Alias);
  end if;
   end Check_Pragma_Implemented;
 



[Ada] Improve support for tuning branch probability heuristics

2019-07-10 Thread Pierre-Marie de Rodat
This adds a new GNAT.Branch_Prediction package to make it possible to
tune the branch probability heuristics more finely.  This package
contains the equivalent of __builtin_expect in C/C++ plus a couple of
specializations.

The following program gives a summary of the usage:

package Q is

  I : Integer;
  pragma Volatile (I);

end Q;

with GNAT.Branch_Prediction; use GNAT.Branch_Prediction;
with Text_IO; use Text_IO;
with Q; use Q;

procedure P is
begin
  if Unlikely (I = 0) then
Put_Line ("Zero was passed");
return;
  end if;

  if Likely (I > 0) then
Put_Line ("A positive number was passed");
  else
Put_Line ("A negative number was passed");
  end if;

  if Expect ((I rem 2) = 0, False) then
Put_Line ("An even number was passed");
  else
Put_Line ("An odd number was passed");
  end if;
end;

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

2019-07-10  Eric Botcazou  

gcc/ada/

* Makefile.rtl (GNATRTL_NONTASKING_OBJS): Add g-brapre.
* libgnat/g-brapre.ads: New package specification.
* doc/gnat_rm/the_gnat_library.rst: Document it.
* gnat_rm.texi: Regenerate.

patch.diff.gz
Description: application/gzip


[Ada] sysdep.c: correct include directives ordering

2019-07-10 Thread Pierre-Marie de Rodat
Some VxWorks headers are relying on types that are defined in
`vxWorks.h` but do not include it themselves, we move the include
directive for `vxWorks.h` at the top of the include directives.

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

2019-07-10  Corentin Gay  

gcc/ada/

* sysdep.c: Put include directive for 'vxWorks.h' before any
other VxWorks headers.--- gcc/ada/sysdep.c
+++ gcc/ada/sysdep.c
@@ -33,6 +33,7 @@
GNAT Run Time Library */
 
 #ifdef __vxworks
+#include "vxWorks.h"
 #include "ioLib.h"
 #if ! defined (VTHREADS)
 #include "dosFsLib.h"
@@ -41,7 +42,6 @@
 # include "nfsLib.h"
 #endif
 #include "selectLib.h"
-#include "vxWorks.h"
 #include "version.h"
 #if defined (__RTP__)
 #  include "vwModNum.h"



[Ada] Spurious error on case expression with limited result

2019-07-10 Thread Pierre-Marie de Rodat
This patch modifies the expansion of case expressions to prevent a
spurious error caused by the use of assignment statements to capture the
result of the case expression when the associated type is limited.

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

2019-07-10  Hristian Kirtchev  

gcc/ada/

* exp_ch4.adb (Expand_N_Case_Expression): Mark the generated
assignments to the temporary result as being OK because the
expansion of case expressions is correct by construction.
(Is_Copy_Type): Update the predicate to match the comment
within.

gcc/testsuite/

* gnat.dg/limited2.adb, gnat.dg/limited2_pack_1.adb,
gnat.dg/limited2_pack_1.ads, gnat.dg/limited2_pack_2.adb,
gnat.dg/limited2_pack_2.ads: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -5087,7 +5087,6 @@ package body Exp_Ch4 is
--
 
procedure Expand_N_Case_Expression (N : Node_Id) is
-
   function Is_Copy_Type (Typ : Entity_Id) return Boolean;
   --  Return True if we can copy objects of this type when expanding a case
   --  expression.
@@ -5106,7 +5105,7 @@ package body Exp_Ch4 is
  or else
(Minimize_Expression_With_Actions
  and then Is_Constrained (Underlying_Type (Typ))
- and then not Is_Limited_View (Underlying_Type (Typ)));
+ and then not Is_Limited_Type (Underlying_Type (Typ)));
   end Is_Copy_Type;
 
   --  Local variables
@@ -5283,6 +5282,7 @@ package body Exp_Ch4 is
  declare
 Alt_Expr : Node_Id := Expression (Alt);
 Alt_Loc  : constant Source_Ptr := Sloc (Alt_Expr);
+LHS  : Node_Id;
 Stmts: List_Id;
 
  begin
@@ -5312,9 +5312,12 @@ package body Exp_Ch4 is
 --Target := AX['Unrestricted_Access];
 
 else
+   LHS := New_Occurrence_Of (Target, Loc);
+   Set_Assignment_OK (LHS);
+
Stmts := New_List (
  Make_Assignment_Statement (Alt_Loc,
-   Name   => New_Occurrence_Of (Target, Loc),
+   Name   => LHS,
Expression => Alt_Expr));
 end if;
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited2.adb
@@ -0,0 +1,8 @@
+--  { dg-do compile }
+
+with Limited2_Pack_2;
+
+procedure Limited2 is
+begin
+   Limited2_Pack_2.Create (P => Limited2_Pack_2.C1);
+end Limited2;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited2_pack_1.adb
@@ -0,0 +1,5 @@
+package body Limited2_Pack_1 is
+   type B is record
+  F : Integer := 0;
+   end record;
+end Limited2_Pack_1;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited2_pack_1.ads
@@ -0,0 +1,8 @@
+package Limited2_Pack_1 is
+   type A is limited private;
+   type A_Ptr is access all A;
+
+private
+   type B;
+   type A is access all B;
+end Limited2_Pack_1;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited2_pack_2.adb
@@ -0,0 +1,21 @@
+with Limited2_Pack_1;
+
+package body Limited2_Pack_2 is
+   Obj_1 : Limited2_Pack_1.A;
+   Obj_2 : Limited2_Pack_1.A;
+   Obj_3 : Limited2_Pack_1.A;
+
+   procedure M (R : Limited2_Pack_1.A) is
+   begin
+  null;
+   end M;
+
+   procedure Create (P : in C) is
+   begin
+  M (R => Obj_1);
+  M (R => (case P is
+ when C1 => Obj_1,
+ when C2 => Obj_2,
+ when C3 => Obj_3));
+   end Create;
+end Limited2_Pack_2;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/limited2_pack_2.ads
@@ -0,0 +1,5 @@
+package Limited2_Pack_2 is
+   type C is (C1, C2, C3);
+
+   procedure Create (P : in C);
+end Limited2_Pack_2;



[Ada] Elaboration order v4.0 and cycle detection

2019-07-10 Thread Pierre-Marie de Rodat
This patch introduces a new cycle detection algorithm which is based on
Tarjan's "Enumeration of the Elementary Circuits of a Directed Graph"
algorithm, with several ideas borrowed from Jonson's "Finding all the
Elementary Circuits of a Directed Graph" algorithm.

No need for a test because the new algorithm improves the performance of
cycle detection only.

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

2019-07-10  Hristian Kirtchev  

gcc/ada/

* bindo.adb: Update the section on switches.
* bindo-graphs.adb
(Add_Cycle, Add_Vertex_And_Complement): Remove.
(Create): The graph no longer needs a set of recorded cycles
because the cycles are not rediscovered in permuted forms.
(Cycle_End_Vertices): New routine.
(Destroy): The graph no longer needs a set of recorded cycles
because the cycles are not rediscovered in permuted forms.
(Destroy_Library_Graph_Vertex): Move to the library level.
(Find_All_Cycles_Through_Vertex, Find_All_Cycles_With_Edge):
Remove.
(Find_Cycles_From_Successor, Find_Cycles_From_Vertex,
Find_Cycles_In_Component, Has_Elaborate_All_Edge): New routines.
(Insert_And_Sort): Remove.
(Is_Elaborate_Body_Edge): Use predicate
Is_Vertex_With_Elaborate_Body.
(Is_Recorded_Cycle): Remove.
(Is_Vertex_With_Elaborate_Body): New routine.
(Normalize_And_Add_Cycle): Remove.
(Precedence): Rename to xxx_Precedence, where xxx relates to the
input.  These versions better reflect the desired input
precedence.
(Record_Cycle): New routine.
(Remove_Vertex_And_Complement, Set_Is_Recorded_Cycle): Remove.
(Trace_xxx): Update all versions to use debug switch -d_t.
(Trace_Component): New routine.
(Trace_Eol): Removed.
(Trace_Vertex): Do not output the component as this information
is already available when the component is traced.
(Unvisit, Visit): New routine.
* bindo-graphs.ads: Add new instance LGV_Lists.  Remove instance
RC_Sets.  Update the structure of type Library_Graph_Attributes
to remove the set of recorded cycles.
(Destroy_Library_Graph_Vertex): Move to the library level.
* bindo-writers.adb (Write_Component_Vertices): Output
information about the number of vertices.
* debug.adb: Document the use of binder switch -d_t.  Update the
use of binder switch -d_T.

patch.diff.gz
Description: application/gzip


[Ada] Vxworks7r2 SR0610 coalesced some macro values

2019-07-10 Thread Pierre-Marie de Rodat
SR0600 and SR0610 cannot be differentiated by macro testing (arguably an
oversight in header file version.h) so: The case statement testing for
"file not found" is reformulated into an if/else series of statements to
avoid a problem where two cases have identical values in SR0610, but
different values in SR0600.

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

2019-07-10  Doug Rupp  

gcc/ada/

* sysdep.c (__gnat_is_file_not_found_error): Reformulate to also
work for vxworks7r2 SR0610.--- gcc/ada/sysdep.c
+++ gcc/ada/sysdep.c
@@ -300,7 +300,7 @@ __gnat_set_mode (int handle ATTRIBUTE_UNUSED, int mode ATTRIBUTE_UNUSED)
 }
 
 char *
-__gnat_ttyname (int filedes)
+__gnat_ttyname (int filedes ATTRIBUTE_UNUSED)
 {
 #if defined (__vxworks)
   return "";
@@ -896,30 +896,34 @@ __gnat_get_task_options (void)
 #endif
 
 int
-__gnat_is_file_not_found_error (int errno_val) {
-   switch (errno_val) {
-  case ENOENT:
+__gnat_is_file_not_found_error (int errno_val)
+ {
+/* WARNING: Do not rewrite this as a switch/case statement.
+ * Some of the "cases" are duplicated in some versions of
+ * Vxworks, notably VxWorks7r2 SR0610.  */
+if (errno_val == ENOENT)
+  return 1;
 #ifdef __vxworks
-  /* In the case of VxWorks, we also have to take into account various
-   * filesystem-specific variants of this error.
-   */
+/* In the case of VxWorks, we also have to take into account various
+ * filesystem-specific variants of this error.
+ */
 #if ! defined (VTHREADS) && (_WRS_VXWORKS_MAJOR < 7)
-  case S_dosFsLib_FILE_NOT_FOUND:
+else if (errno_val == S_dosFsLib_FILE_NOT_FOUND)
+  return 1;
 #endif
 #if ! defined (__RTP__) && (! defined (VTHREADS) || defined (__VXWORKSMILS__))
-  case S_nfsLib_NFSERR_NOENT:
+else if (errno_val ==  S_nfsLib_NFSERR_NOENT)
+  return 1;
 #endif
 #if defined (__RTP__)
-	/* An RTP can return an NFS file not found, and the NFS bits must
-	   first be masked on to check the errno.  */
-  case M_nfsStat | ENOENT:
+/* An RTP can return an NFS file not found, and the NFS bits must
+   first be masked on to check the errno.  */
+else if (errno_val == (M_nfsStat | ENOENT))
+  return 1;
 #endif
 #endif
- return 1;
-
-  default:
-return 0;
-   }
+else
+  return 0;
 }
 
 #if defined (__linux__)



[Ada] Add contracts to Strings libraries

2019-07-10 Thread Pierre-Marie de Rodat
This patch adds contracts to Ada.Strings libraries, in order to remove
warnings when using these libraries in SPARK.

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

2019-07-10  Joffrey Huguet  

gcc/ada/

* libgnat/a-strbou.ads, libgnat/a-strfix.ads,
libgnat/a-strunb.ads, libgnat/a-strunb__shared.ads: Add global
contracts, contract cases, preconditions and postconditions to
procedures and functions.

patch.diff.gz
Description: application/gzip


[Ada] The environ macro is broken on vxworks7r2 SR0610

2019-07-10 Thread Pierre-Marie de Rodat
In SR0610, the TCB is made private, so the task environ field used by
the environ macro is no longer visible.  Arguably this is a bug, however
a more correct approach is to use accessor functions to retrieve this
field and not use the environ macro, thus avoiding the problem.

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

2019-07-10  Doug Rupp  

gcc/ada/

* env.c (__gnat_environ): Reformulate to also work for
vxworks7r2 SR0610.--- gcc/ada/env.c
+++ gcc/ada/env.c
@@ -60,6 +60,9 @@
 #endif
 
 #if defined (__vxworks)
+  #include 
+  #include 
+
   #if defined (__RTP__)
 /* On VxWorks 6 Real-Time process mode, environ is defined in unistd.h.  */
 #include 
@@ -69,14 +72,18 @@
envLib.h on VxWorks MILS and VxWorks 653.  */
 #include 
 #include 
-  #else
-/* This should work for kernel mode on both VxWorks 5 and VxWorks 6.  */
+  #elif (_WRS_VXWORKS_MAJOR <= 6)
 #include 
-
-/* In that mode environ is a macro which reference the following symbol.
-   As the symbol is not defined in any VxWorks include files we declare
-   it as extern.  */
+/* In that mode the following symbol is not defined in any VxWorks
+   include files, prior to vxWorks 7, so we declare it as extern.  */
 extern char** ppGlobalEnviron;
+  #elif (_WRS_VXWORKS_MAJOR >= 7)
+/* This should work for kernel mode on VxWorks 7.x.  In 7.2 the tcb
+   is made private, so accessor functions must be used, in 7.0 it
+   is optional but there is no way to distinguish between 7.2
+   and 7.0 since the version.h header file was never updated.  */
+#include 
+#include 
   #endif
 #endif
 
@@ -223,7 +230,18 @@ __gnat_environ (void)
   extern char **environ;
   return environ;
 #else
-  return environ;
+  #if defined (__RTP__) || defined (VTHREADS) || (_WRS_VXWORKS_MAJOR <= 6)
+return environ;
+  #elif (_WRS_VXWORKS_MAJOR >= 7)
+char **task_environ;
+
+task_environ = envGet (taskIdSelf ());
+
+if (task_environ == NULL)
+   return ppGlobalEnviron;
+else
+   return task_environ;
+  #endif
 #endif
 }
 



[Ada] Missing implicit interface type conversion

2019-07-10 Thread Pierre-Marie de Rodat
The compiler skips adding an implicit type conversion when the interface
type is visible through a limited-with clause.

No small reproducer available.

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

2019-07-10  Javier Miranda  

gcc/ada/

* exp_ch6.adb (Is_Class_Wide_Interface_Type): New subprogram.
(Expand_Call_Helper): Handle non-limited views when we check if
any formal is a class-wide interface type.
* exp_disp.adb (Expand_Interface_Actuals): Handle non-limited
views when we look for interface type formals to force "this"
displacement.--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -2331,6 +2331,10 @@ package body Exp_Ch6 is
   function In_Unfrozen_Instance (E : Entity_Id) return Boolean;
   --  Return true if E comes from an instance that is not yet frozen
 
+  function Is_Class_Wide_Interface_Type (E : Entity_Id) return Boolean;
+  --  Return True when E is a class-wide interface type or an access to
+  --  a class-wide interface type.
+
   function Is_Direct_Deep_Call (Subp : Entity_Id) return Boolean;
   --  Determine if Subp denotes a non-dispatching call to a Deep routine
 
@@ -2585,6 +2589,32 @@ package body Exp_Ch6 is
  return False;
   end In_Unfrozen_Instance;
 
+  --
+  -- Is_Class_Wide_Interface_Type --
+  --
+
+  function Is_Class_Wide_Interface_Type (E : Entity_Id) return Boolean is
+ Typ : Entity_Id := E;
+ DDT : Entity_Id;
+
+  begin
+ if Has_Non_Limited_View (Typ) then
+Typ := Non_Limited_View (Typ);
+ end if;
+
+ if Ekind (Typ) = E_Anonymous_Access_Type then
+DDT := Directly_Designated_Type (Typ);
+
+if Has_Non_Limited_View (DDT) then
+   DDT := Non_Limited_View (DDT);
+end if;
+
+return Is_Class_Wide_Type (DDT) and then Is_Interface (DDT);
+ else
+return Is_Class_Wide_Type (Typ) and then Is_Interface (Typ);
+ end if;
+  end Is_Class_Wide_Interface_Type;
+
   -
   -- Is_Direct_Deep_Call --
   -
@@ -2919,15 +2949,7 @@ package body Exp_Ch6 is
 
  CW_Interface_Formals_Present :=
CW_Interface_Formals_Present
- or else
-   (Is_Class_Wide_Type (Etype (Formal))
- and then Is_Interface (Etype (Etype (Formal
- or else
-   (Ekind (Etype (Formal)) = E_Anonymous_Access_Type
- and then Is_Class_Wide_Type (Directly_Designated_Type
-   (Etype (Etype (Formal
- and then Is_Interface (Directly_Designated_Type
- (Etype (Etype (Formal);
+ or else Is_Class_Wide_Interface_Type (Etype (Formal));
 
  --  Create possible extra actual for constrained case. Usually, the
  --  extra actual is of the form actual'constrained, but since this

--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -1682,18 +1682,34 @@ package body Exp_Disp is
   while Present (Formal) loop
  Formal_Typ := Etype (Formal);
 
+ if Has_Non_Limited_View (Formal_Typ) then
+Formal_Typ := Non_Limited_View (Formal_Typ);
+ end if;
+
  if Ekind (Formal_Typ) = E_Record_Type_With_Private then
 Formal_Typ := Full_View (Formal_Typ);
  end if;
 
  if Is_Access_Type (Formal_Typ) then
 Formal_DDT := Directly_Designated_Type (Formal_Typ);
+
+if Has_Non_Limited_View (Formal_DDT) then
+   Formal_DDT := Non_Limited_View (Formal_DDT);
+end if;
  end if;
 
  Actual_Typ := Etype (Actual);
 
+ if Has_Non_Limited_View (Actual_Typ) then
+Actual_Typ := Non_Limited_View (Actual_Typ);
+ end if;
+
  if Is_Access_Type (Actual_Typ) then
 Actual_DDT := Directly_Designated_Type (Actual_Typ);
+
+if Has_Non_Limited_View (Actual_DDT) then
+   Actual_DDT := Non_Limited_View (Actual_DDT);
+end if;
  end if;
 
  if Is_Interface (Formal_Typ)



[Ada] Fix crashes on ownership checking in SPARK

2019-07-10 Thread Pierre-Marie de Rodat
Code that violates the conditions for ownership checking should lead to
error messages pointing to the violations instead of crashes.

There is no impact on compilation, only GNATprove.

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

2019-07-10  Yannick Moy  

gcc/ada/

* sem_spark.adb (Get_Root_Object): Replace precondition by error
message.
(Read_Indexes): Replace precondition by error message.
(Check_Callable_Body): Check only traversal function returns an
anonymous access type.
(Check_Expression): Issue error on unexpected expression as
path.
* sem_util.adb (First_Global): Fix access to global on
entry/task.--- gcc/ada/sem_spark.adb
+++ gcc/ada/sem_spark.adb
@@ -714,7 +714,6 @@ package body Sem_SPARK is
function Get_Root_Object
  (Expr  : Node_Id;
   Through_Traversal : Boolean := True) return Entity_Id;
-   pragma Precondition (Is_Path_Expression (Expr));
--  Return the root of the path expression Expr, or Empty for an allocator,
--  NULL, or a function call. Through_Traversal is True if it should follow
--  through calls to traversal functions.
@@ -1311,6 +1310,15 @@ package body Sem_SPARK is
 
   Inside_Elaboration := False;
 
+  if Ekind (Spec_Id) = E_Function
+and then Is_Anonymous_Access_Type (Etype (Spec_Id))
+and then not Is_Traversal_Function (Spec_Id)
+  then
+ Error_Msg_N ("anonymous access type for result only allowed for "
+  & "traveral functions", Spec_Id);
+ return;
+  end if;
+
   --  Save environment and put a new one in place
 
   Move_Env (Current_Perm_Env, Saved_Env);
@@ -1451,7 +1459,6 @@ package body Sem_SPARK is
   --  Call Read_Expression on every expression in the list L
 
   procedure Read_Indexes (Expr : Node_Id);
-  pragma Precondition (Is_Path_Expression (Expr));
   --  When processing a path, the index expressions and function call
   --  arguments occurring on the path should be analyzed in Read mode.
 
@@ -1562,6 +1569,11 @@ package body Sem_SPARK is
   --  Start of processing for Read_Indexes
 
   begin
+ if not Is_Path_Expression (Expr) then
+Error_Msg_N ("name expected here for move/borrow/observe", Expr);
+return;
+ end if;
+
  case N_Subexpr'(Nkind (Expr)) is
 when N_Identifier
| N_Expanded_Name
@@ -1710,7 +1722,10 @@ package body Sem_SPARK is
   --  Expressions that are not path expressions should only be analyzed in
   --  Read mode.
 
-  pragma Assert (Mode = Read);
+  if Mode /= Read then
+ Error_Msg_N ("name expected here for move/borrow/observe", Expr);
+ return;
+  end if;
 
   --  Special handling for nodes that may contain evaluated expressions in
   --  the form of constraints.
@@ -3484,6 +3499,11 @@ package body Sem_SPARK is
   Through_Traversal : Boolean := True) return Entity_Id
is
begin
+  if not Is_Path_Expression (Expr) then
+ Error_Msg_N ("name expected here for path", Expr);
+ return Empty;
+  end if;
+
   case Nkind (Expr) is
  when N_Expanded_Name
 | N_Identifier

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -8713,7 +8713,24 @@ package body Sem_Util is
   --  case, it can only be located on the body entity.
 
   if Refined then
- Body_Id := Subprogram_Body_Entity (Subp);
+ if Is_Subprogram_Or_Generic_Subprogram (Subp) then
+Body_Id := Subprogram_Body_Entity (Subp);
+
+ elsif Is_Entry (Subp)
+   or else Is_Task_Type (Subp)
+ then
+Body_Id := Corresponding_Body (Parent (Subp));
+
+ --  ??? It should be possible to retrieve the Refined_Global on the
+ --  task body associated to the task object. This is not yet possible.
+
+ elsif Is_Single_Task_Object (Subp) then
+Body_Id := Empty;
+
+ else
+Body_Id := Empty;
+ end if;
+
  if Present (Body_Id) then
 Global := Get_Pragma (Body_Id, Pragma_Refined_Global);
  end if;



[Ada] Elaboration order v4.0 and linker switches

2019-07-10 Thread Pierre-Marie de Rodat
This patch adds a missing functionality with respect to elaboration
order v3.0.  Units carry an attribute called Elab_Position which among
other things controls the sorting of linker switches by gnatbind.
Setting the proper position ensures the gnatbind will output the linker
switches in an order compatible with what gnatlink expects.

No simple test available as this requires a Windows cross-compiler.

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

2019-07-10  Hristian Kirtchev  

gcc/ada/

* bindo-elaborators.adb (Elaborate_Units): Set attribute
Elab_Position of all elaborated units.
(Set_Unit_Elaboration_Positions): New routine.--- gcc/ada/bindo-elaborators.adb
+++ gcc/ada/bindo-elaborators.adb
@@ -274,6 +274,10 @@ package body Bindo.Elaborators is
   --  Determine whether vertex Vertex of library graph G is suitable for
   --  weak elaboration.
 
+  procedure Set_Unit_Elaboration_Positions (Order : Unit_Id_Table);
+  pragma Inline (Set_Unit_Elaboration_Positions);
+  --  Set the ALI.Units positions of all elaboration units in order Order
+
   procedure Trace_Component
 (G: Library_Graph;
  Comp : Component_Id;
@@ -750,6 +754,11 @@ package body Bindo.Elaborators is
  if Status = Order_OK then
 Validate_Elaboration_Order (Order);
 
+--  Set attribute Elab_Position of table ALI.Units for all units in
+--  the elaboration order.
+
+Set_Unit_Elaboration_Positions (Order);
+
 --  Output the dependencies among units when switch -e (output
 --  complete list of elaboration order dependencies) is active.
 
@@ -1304,6 +1313,23 @@ package body Bindo.Elaborators is
  and then Is_Weakly_Elaborable_Vertex (G, Vertex);
   end Is_Suitable_Weakly_Elaborable_Vertex;
 
+  
+  -- Set_Unit_Elaboration_Positions --
+  
+
+  procedure Set_Unit_Elaboration_Positions (Order : Unit_Id_Table) is
+ U_Id : Unit_Id;
+
+  begin
+ for Position in Unit_Id_Tables.First ..
+ Unit_Id_Tables.Last (Order)
+ loop
+U_Id := Order.Table (Position);
+
+ALI.Units.Table (U_Id).Elab_Position := Position;
+ end loop;
+  end Set_Unit_Elaboration_Positions;
+
   -
   -- Trace_Component --
   -



[Ada] Spurious error on overloaded equality in postcondition

2019-07-10 Thread Pierre-Marie de Rodat
This patch fixes a spurious error in a postcondition in a nested
instantiation when the expression includes an inherited equality and
checks are enabled.

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

2019-07-10  Ed Schonberg  

gcc/ada/

* sem_res.adb (Resolve_Equality_Op): Do not replace the resolved
operator by its alias if expander is not active, because the
operand type may not be frozen yet and its inherited operations
have not yet been created.

gcc/testsuite/

* gnat.dg/equal8.adb, gnat.dg/equal8.ads,
gnat.dg/equal8_pkg.ads: New testcase.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -8471,7 +8471,14 @@ package body Sem_Res is
   Get_Next_Interp (I, It);
end loop;
 
-   if Present (Alias (Entity (N))) then
+   --  If expansion is active and this is wn inherited operation,
+   --  replace it with its ancestor. This must not be done during
+   --  preanalysis because the type nay not be frozen yet, as when
+   --  the context is a pre/post condition.
+
+   if Present (Alias (Entity (N)))
+ and then Expander_Active
+   then
   Set_Entity (N, Alias (Entity (N)));
end if;
 end;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/equal8.adb
@@ -0,0 +1,6 @@
+--  { dg-do compile }
+--  { dg-options "-gnata" }
+
+package body Equal8 is
+   procedure Foo is null;
+end Equal8;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/equal8.ads
@@ -0,0 +1,36 @@
+with Ada.Containers.Formal_Hashed_Sets;
+with Ada.Strings.Hash;
+
+-- with Dynamic_Strings; use Dynamic_Strings;
+-- with Bounded_Dynamic_Strings;
+
+with Equal8_Pkg;
+
+package Equal8 is
+
+   package Dynamic_Strings is
+  --  pragma SPARK_Mode (On);
+
+  package Bounded_Dynamic_Strings is new Equal8_Pkg
+  (Component => Character,
+   List_Index=> Positive,
+   List  => String,
+   Default_Value => ' ');
+  type Dynamic_String is new Bounded_Dynamic_Strings.Sequence;
+
+   end Dynamic_Strings;
+   use Dynamic_Strings;
+
+   subtype Subscription_Address is Dynamic_String (Capacity => 255);
+
+   function Hashed_Subscription_Address (Element : Subscription_Address)
+  return Ada.Containers.Hash_Type is
+  (Ada.Strings.Hash (Value (Element)));
+
+   package Subscription_Addresses is new Ada.Containers.Formal_Hashed_Sets
+ (Element_Type=> Subscription_Address,
+  Hash=> Hashed_Subscription_Address,
+  Equivalent_Elements => "=");
+
+   procedure Foo;
+end Equal8;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/equal8_pkg.ads
@@ -0,0 +1,58 @@
+generic
+   type Component is private;
+   type List_Index is range <>;
+   type List is array (List_Index range <>) of Component;
+   Default_Value : Component;
+ --  with function "=" (Left, Right : List) return Boolean is <>;
+
+package Equal8_Pkg is
+
+   pragma Pure;
+
+   Maximum_Length : constant List_Index := List_Index'Last;
+
+   subtype Natural_Index is List_Index'Base range 0 .. Maximum_Length;
+   type Sequence (Capacity : Natural_Index) is private;
+   --  from zero to Capacity.
+
+   function Value (This : Sequence) return List;
+   --  Returns the content of this sequence. The value returned is the
+   --  "logical" value in that only that slice which is currently assigned
+   --  is returned, as opposed to the entire physical representation.
+
+   overriding
+   function "=" (Left, Right : Sequence) return Boolean with
+ Inline;
+
+   function "=" (Left : Sequence;  Right : List) return Boolean with
+ Inline;
+
+private
+   type Sequence (Capacity : Natural_Index) is record
+  Current_Length : Natural_Index := 0;
+  Content: List (1 .. Capacity) := (others => Default_Value);
+   end record;
+
+   ---
+   -- Value --
+   ---
+
+   function Value (This : Sequence) return List is
+ (This.Content (1 .. This.Current_Length));
+
+   -
+   -- "=" --
+   -
+
+   overriding
+   function "=" (Left, Right : Sequence) return Boolean is
+ (Value (Left) = Value (Right));
+
+   -
+   -- "=" --
+   -
+
+   function "=" (Left : Sequence;  Right : List) return Boolean is
+ (Value (Left) = Right);
+end Equal8_Pkg;
+



[Ada] Use renamings in GNATprove mode for side-effects extraction

2019-07-10 Thread Pierre-Marie de Rodat
In the GNATprove mode for formal verification, prefer renamings over
declaration of a constant to extract side-effects from expressions,
whenever the constant could be of an owning type, as declaring a
constant of an owning type has an effect on ownership which is
undesirable.

There is no impact on compilation.

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

2019-07-10  Yannick Moy  

gcc/ada/

* exp_util.adb (Remove_Side_Effects): Prefer renamings for
objects of possible owning type in GNATprove mode.--- gcc/ada/exp_util.adb
+++ gcc/ada/exp_util.adb
@@ -11333,7 +11333,17 @@ package body Exp_Util is
  --  Generate:
  --Rnn : Exp_Type renames Expr;
 
- if Renaming_Req then
+ --  In GNATprove mode, we prefer to use renamings for intermediate
+ --  variables to definition of constants, due to the implicit move
+ --  operation that such a constant definition causes as part of the
+ --  support in GNATprove for ownership pointers. Hence we generate
+ --  a renaming for a reference to an object of a non-scalar type.
+
+ if Renaming_Req
+   or else (GNATprove_Mode
+ and then Is_Object_Reference (Exp)
+ and then not Is_Scalar_Type (Exp_Type))
+ then
 E :=
   Make_Object_Renaming_Declaration (Loc,
 Defining_Identifier => Def_Id,



Re: [PATCH] i386: Add AVX512 unaligned intrinsics

2019-07-10 Thread Uros Bizjak
On Tue, Jul 9, 2019 at 11:44 PM Sunil Pandey  wrote:
>
> __m512i _mm512_loadu_epi32( void * sa);
> __m512i _mm512_loadu_epi64( void * sa);
> void _mm512_storeu_epi32(void * d, __m512i a);
> void _mm256_storeu_epi32(void * d, __m256i a);
> void _mm_storeu_epi32(void * d, __m128i a);
> void _mm512_storeu_epi64(void * d, __m512i a);
> void _mm256_storeu_epi64(void * d, __m256i a);
> void _mm_storeu_epi64(void * d, __m128i a);
>
> Tested on x86-64.
>
> OK for trunk?
>
> --Sunil Pandey
>
>
> gcc/
>
> PR target/90980
> * config/i386/avx512fintrin.h (__v16si_u): New data type
> (__v8di_u): Likewise
> (_mm512_loadu_epi32): New.
> (_mm512_loadu_epi64): Likewise.
> (_mm512_storeu_epi32): Likewise.
> (_mm512_storeu_epi64): Likewise.
> * config/i386/avx512vlintrin.h (_mm_storeu_epi32): New.
> (_mm256_storeu_epi32): Likewise.
> (_mm_storeu_epi64): Likewise.
> (_mm256_storeu_epi64): Likewise.
>
> gcc/testsuite/
>
> PR target/90980
> * gcc.target/i386/avx512f-vmovdqu32-3.c: New test.
> * gcc.target/i386/avx512f-vmovdqu64-3.c: Likewise.
> * gcc.target/i386/pr90980-1.c: Likewise.
> * gcc.target/i386/pr90980-2.c: Likewise.

+/* Internal data types for implementing unaligned version of intrinsics.  */
+typedef int __v16si_u __attribute__ ((__vector_size__ (64),
+  __aligned__ (1)));
+typedef long long __v8di_u __attribute__ ((__vector_size__ (64),
+   __aligned__ (1)));

You should define only one generic __m512i_u type, something like:

typedef long long __m512i_u __attribute__ ((__vector_size__ (64),
__may_alias__, __aligned__ (1)));

Please see avxintrin.h how __m256i_u is defined and used.

Uros.