Re: [PATCH v3] Use builtin sort instead of shell sort

[Andreas Schwab]

On Apr 20 2019, Émeric Dupont  wrote:

> The following patch fixes this issue by using the [Make $(sort list)][1]
> function instead to remove duplicates from the list of headers. There is
> no functional change, the value assigned to the shell variable is the
> same.

Is it?

> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index d186d71c91e..3196e774a26 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3538,7 +3538,7 @@ install-plugin: installdirs lang.install-plugin 
> s-header-vars install-gengtype
>  # We keep the directory structure for files in config or c-family and .def
>  # files. All other files are flattened to a single directory.
>  $(mkinstalldirs) $(DESTDIR)$(plugin_includedir)
> -headers=`echo $(PLUGIN_HEADERS) $$(cd $(srcdir); echo *.h *.def) | tr ' ' 
> '\012' | sort -u`; \
> +headers=$(sort $(PLUGIN_HEADERS) $$(cd $(srcdir); echo *.h *.def)); \

This sorts the words $(PLUGIN_HEADERS) '$(cd' $(srcdir); echo '*.h'
'*.def)' and produces a command line that doesn't make sense.

Andreas.

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

Re: [PATCH, LRA]: Revert the revert of removal of usless move insns.

[Uros Bizjak]

On 4/20/19, Vladimir Makarov  wrote:
>
> On 11/21/18 2:33 PM, Uros Bizjak wrote:
>> Hello!
>>
>> Before the recent patch to post-reload mode switching, vzeroupper
>> insertion depended on the existence of the return copy instructions
>> pair in functions that return a value. The first instruction in the
>> pair represents a move to a function return hard register, and the
>> second was a USE of the function return hard register. Sometimes a nop
>> move was generated (e.g. %eax->%eax) for the first instruction of the
>> return copy instructions pair and the patch [1] teached LRA  to remove
>> these useless instructions on the fly.
>>
>> The removal caused optimize mode switching to trigger the assert,
>> since the first instruction of a return pair was not found. The
>> relevant part of the patch was later reverted. With the recent
>> optimize mode switching patch, this is no longer necessary for
>> vzeroupper insertion pass, so attached patch reverts the revert.
>>
>> 2018-11-21  Uros Bizjak  
>>
>>  Revert the revert:
>>  2013-10-26  Vladimir Makarov  
>>
>>  Revert:
>>  2013-10-25  Vladimir Makarov  
>>
>>  * lra-spills.c (lra_final_code_change): Remove useless move insns.
>>
>> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>>
>> OK for mainline?
> Sure, Uros. I support the patch.  But I think it would be wise to
> postpone its committing after releasing GCC-9.  Simply it is hard to
> predict the patch effect to other targets and I would avoid any risk at
> this stage.

Actually, the "revert of the revert" patch was already committed to
mainline some time ago.

To clear the possible misunderstanding, let me summarise the issue:

- the original patch that remove useless move insn caused a breakage
in vzeroupper pass.
- the original patch was reverted due to the above breakage
- the vzeroupper pass was later adjusted to tolerate removed useless
move instructions, and this cleared the way to revert the revert. Now
LRA removes useless move insns.

An orthogonal issue (PR90178) was discovered, showing that some passes
also depend on the presence of useless move insn.

The bisection stumbled on the "revert of the revert" patch that
(obviously) re-introduced the issue. I'm not in the position to decide
if useless move insn can be removed or if these later passes should be
fixed, I can only say that the vzeroupper pass is now agnostic to the
presence of useless move insns.

Uros.

Re: [PATCH] PR fortran/90166 -- check F2018:C1547

[Dominique d'Humières]

Hi Steve,

The changes in gfortran.dg/submodule_22.f08 look weird:
(1) is the error in the CONTAINS of a SUBMODULE invalid?
From

* decl.c (in_module_or_interface): New function to check that the
current state is in a module, submodule, or interface.

it should not, should it?

(2) left over?
+
+found outside of a module

TIA

Dominique

Re: [PATCH] PR fortran/90166 -- check F2018:C1547

[Steve Kargl]

On Sat, Apr 20, 2019 at 05:38:34PM +0200, Dominique d'Humières wrote:
> 
> The changes in gfortran.dg/submodule_22.f08 look weird:
> (1) is the error in the CONTAINS of a SUBMODULE invalid?
> From
> 
>   * decl.c (in_module_or_interface): New function to check that the
>   current state is in a module, submodule, or interface.
> 
> it should not, should it?
> 
> (2) left over?
> +
> +found outside of a module
> 

It's a sequence of run-on errors.  The first statement in
the original code is rejected with a syntax error.  When
that happenrs gfc_current_state() is not COMP_MODULE, 
COMP_SUBMODULE, or COMP_INTERFENCE.  The next line has 
the MODULE prefix, and the new check finds that it 
occurs outside of MODULE, SUBMODULE, and INTERFERENCE,
so a new error occurs.  The remaining errors are then 
found to be bogus assignments.  My conclusion, if the
first error is fixed, then the run-on errors don't
happen.

If you rather fix the problems with '! dg-options "-fmax-errors=1"'
I'm fine with that.

-- 
Steve

[PATCH, PR d/89293] Committed core.atomic should have fallback when there's no libatomic

[Iain Buclaw]

Hi,

This patch adds an implementation of core.atomic for when there's no
libatomic support linked in, nor provided by the compiler.

The main part of which fakes the purity of system
mutex_lock/mutex_unlock in order to satisfy the using of it in the
pure/nothrow/safe atomic functions.

Regression tested on x86_64-linux-gnu, where all libatomic
configurables in libphobos were set to false to force testing the new
code.

Committed to trunk as r270470.

-- 
Iain
---
libphobos/ChangeLog:

2019-04-20  Iain Buclaw  

PR d/89293
* libdruntime/core/atomic.d (casImpl): Remove static assert for
GNU_Have_Atomics, add static path to handle missing atomic support.
(atomicLoad): Likewise.
(atomicStore): Likewise.
(atomicFence):  Likewise.
(atomicMutexHandle, AtomicMutex): Declare types.
(_getAtomicMutex): New function.
(getAtomicMutex): Declare.

---
diff --git a/libphobos/libdruntime/core/atomic.d b/libphobos/libdruntime/core/atomic.d
index 0b39cddb6c9..1d0a2ea8b48 100644
--- a/libphobos/libdruntime/core/atomic.d
+++ b/libphobos/libdruntime/core/atomic.d
@@ -1353,36 +1353,62 @@ else version (GNU)
 
 private bool casImpl(T,V1,V2)( shared(T)* here, V1 ifThis, V2 writeThis ) pure nothrow @nogc @trusted
 {
-static assert(GNU_Have_Atomics, "cas() not supported on this architecture");
 bool res = void;
 
-static if (T.sizeof == byte.sizeof)
+static if (GNU_Have_Atomics || GNU_Have_LibAtomic)
 {
-res = __atomic_compare_exchange_1(here, cast(void*) &ifThis, *cast(ubyte*) &writeThis,
-  false, MemoryOrder.seq, MemoryOrder.seq);
-}
-else static if (T.sizeof == short.sizeof)
-{
-res = __atomic_compare_exchange_2(here, cast(void*) &ifThis, *cast(ushort*) &writeThis,
-  false, MemoryOrder.seq, MemoryOrder.seq);
-}
-else static if (T.sizeof == int.sizeof)
-{
-res = __atomic_compare_exchange_4(here, cast(void*) &ifThis, *cast(uint*) &writeThis,
-  false, MemoryOrder.seq, MemoryOrder.seq);
-}
-else static if (T.sizeof == long.sizeof && GNU_Have_64Bit_Atomics)
-{
-res = __atomic_compare_exchange_8(here, cast(void*) &ifThis, *cast(ulong*) &writeThis,
-  false, MemoryOrder.seq, MemoryOrder.seq);
+static if (T.sizeof == byte.sizeof)
+{
+res = __atomic_compare_exchange_1(here, cast(void*) &ifThis, *cast(ubyte*) &writeThis,
+  false, MemoryOrder.seq, MemoryOrder.seq);
+}
+else static if (T.sizeof == short.sizeof)
+{
+res = __atomic_compare_exchange_2(here, cast(void*) &ifThis, *cast(ushort*) &writeThis,
+  false, MemoryOrder.seq, MemoryOrder.seq);
+}
+else static if (T.sizeof == int.sizeof)
+{
+res = __atomic_compare_exchange_4(here, cast(void*) &ifThis, *cast(uint*) &writeThis,
+  false, MemoryOrder.seq, MemoryOrder.seq);
+}
+else static if (T.sizeof == long.sizeof && GNU_Have_64Bit_Atomics)
+{
+res = __atomic_compare_exchange_8(here, cast(void*) &ifThis, *cast(ulong*) &writeThis,
+  false, MemoryOrder.seq, MemoryOrder.seq);
+}
+else static if (GNU_Have_LibAtomic)
+{
+res = __atomic_compare_exchange(T.sizeof, here, cast(void*) &ifThis, cast(void*) &writeThis,
+MemoryOrder.seq, MemoryOrder.seq);
+}
+else
+static assert(0, "Invalid template type specified.");
 }
-else static if (GNU_Have_LibAtomic)
+else
 {
-res = __atomic_compare_exchange(T.sizeof, here, cast(void*) &ifThis, cast(void*) &writeThis,
-MemoryOrder.seq, MemoryOrder.seq);
+static if (T.sizeof == byte.sizeof)
+alias U = byte;
+else static if (T.sizeof == short.sizeof)
+alias U = short;
+else static if (T.sizeof == int.sizeof)
+alias U = int;
+else static if (T.sizeof == long.sizeof)
+alias U = long;
+else
+static assert(0, "Invalid template type specified.");
+
+getAtomicMutex.lock();
+scope(exit) getAtomicMutex.unlock();
+
+if (*cast(U*)here == *cast(U*)&ifThis)
+{
+*here = writeThis;
+res = true;
+}
+else
+res

Re: [PATCH] PR fortran/90166 -- check F2018:C1547

[Paul Richard Thomas]

It looks good to me, modulo Dominique's query. OK for trunk (note from
Richi - still aiming for zero P1 bugs so you're good to go).

Thanks

Paul


On Fri, 19 Apr 2019 at 22:44, Steve Kargl
 wrote:
>
> The attached patch fixes PR fortran/91066.  The original
> code was feeding a nonsense string of tokens to the
> assembler causing it to toss its cookies.  It turns out
> that gfortran was not enforcing the constraint C1547
> from Fortran 2018.  The attached patch now performs
> that check.  Regression tested on x86_64-*-freebsd.
> OK to commit?
>
> 2019-04-19  Steven G. Kargl  
>
> PR fortran/90166
> * decl.c (in_module_or_interface): New function to check that the
> current state is in a module, submodule, or interface.
> (gfc_match_prefix): Use it.
>
> PR fortran/90166
> * gfortran.dg/submodule_22.f08: Add additional dg-error comments.
>
> --
> Steve



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

New French PO file for 'gcc' (version 9.1-b20190414)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the French team of translators.  The file is available at:

https://translationproject.org/latest/gcc/fr.po

(This file, 'gcc-9.1-b20190414.fr.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/gcc/

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/gcc.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.

Re: [PATCH] PR fortran/90166 -- check F2018:C1547

[Steve Kargl]

On Sat, Apr 20, 2019 at 09:57:34AM -0700, Steve Kargl wrote:
> On Sat, Apr 20, 2019 at 05:38:34PM +0200, Dominique d'Humières wrote:
> > 
> > The changes in gfortran.dg/submodule_22.f08 look weird:
> > (1) is the error in the CONTAINS of a SUBMODULE invalid?
> > From
> > 
> > * decl.c (in_module_or_interface): New function to check that the
> > current state is in a module, submodule, or interface.
> > 
> > it should not, should it?
> > 
> > (2) left over?
> > +
> > +found outside of a module
> > 
> 
> It's a sequence of run-on errors.  The first statement in
> the original code is rejected with a syntax error.  When
> that happenrs gfc_current_state() is not COMP_MODULE, 
> COMP_SUBMODULE, or COMP_INTERFENCE.  The next line has 
> the MODULE prefix, and the new check finds that it 
> occurs outside of MODULE, SUBMODULE, and INTERFERENCE,
> so a new error occurs.  The remaining errors are then 
> found to be bogus assignments.  My conclusion, if the
> first error is fixed, then the run-on errors don't
> happen.
> 
> If you rather fix the problems with '! dg-options "-fmax-errors=1"'
> I'm fine with that.
> 

Just to follow up.  If you use a debugger, one finds

(gdb) b decl.c:6130
(gdb) c
Continuing.
/safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/submodule_22.f08:41:23:

   41 | submodule (mtop:submod:subsubmod) subsubsubmod ! { dg-error "Syntax 
error in SUBMODULE statement" }
  |   1
Error: Syntax error in SUBMODULE statement at (1)

This is the original error.  Note it is a syntax error.  gfortran
does nothing with this statement

(gdb) c

we reach the point where the new error will be issued.

(gdb) p gfc_state_stack->state
$2 = COMP_CONTAINS

This is the CONTAINS in the submodule.

(gdb) p gfc_state_stack->previous->state
$3 = COMP_PROGRAM

This state fine here, because the syntax rejects the submodule
statement.  So, the code looks like (from memory...)

  [program main implicitly included here]
  contains
  module subroutine foo
  x = 2
  y = 3
  end submodule

The module prefix cannot appear in the subroutine statement. The
new error rejects it.  So, now you have 2 assignments in after a
contains statement.  The program now looks like

  contains
  x = 2
  y = 3 
  end submodule

Well, you cannot do an assignment, so two additional error messages
are emitted.  So, now we come to a program of the form


  [program main implicitly included here]
  end submodule

gfortran is expecting an END [PROGRAM] statement.

Is this clear or do you want me to withdrawal the patch?

-- 
Steve

Re: [PATCH] PR fortran/90166 -- check F2018:C1547

[Dominique d'Humières]

OK I missed the previous error. However I am still puzzled by (2):

+
+found outside of a module

Dominique

> Le 20 avr. 2019 à 21:18, Steve Kargl  a 
> écrit :
> 
> On Sat, Apr 20, 2019 at 09:57:34AM -0700, Steve Kargl wrote:
>> On Sat, Apr 20, 2019 at 05:38:34PM +0200, Dominique d'Humières wrote:
>>> 
>>> The changes in gfortran.dg/submodule_22.f08 look weird:
>>> (1) is the error in the CONTAINS of a SUBMODULE invalid?
>>> From
>>> 
>>> * decl.c (in_module_or_interface): New function to check that the
>>> current state is in a module, submodule, or interface.
>>> 
>>> it should not, should it?
>>> 
>>> (2) left over?
>>> +
>>> +found outside of a module
>>> 
>> 
>> It's a sequence of run-on errors.  The first statement in
>> the original code is rejected with a syntax error.  When
>> that happenrs gfc_current_state() is not COMP_MODULE, 
>> COMP_SUBMODULE, or COMP_INTERFENCE.  The next line has 
>> the MODULE prefix, and the new check finds that it 
>> occurs outside of MODULE, SUBMODULE, and INTERFERENCE,
>> so a new error occurs.  The remaining errors are then 
>> found to be bogus assignments.  My conclusion, if the
>> first error is fixed, then the run-on errors don't
>> happen.
>> 
>> If you rather fix the problems with '! dg-options "-fmax-errors=1"'
>> I'm fine with that.
>> 
> 
> Just to follow up.  If you use a debugger, one finds
> 
> (gdb) b decl.c:6130
> (gdb) c
> Continuing.
> /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/submodule_22.f08:41:23:
> 
>   41 | submodule (mtop:submod:subsubmod) subsubsubmod ! { dg-error "Syntax 
> error in SUBMODULE statement" }
>  |   1
> Error: Syntax error in SUBMODULE statement at (1)
> 
> This is the original error.  Note it is a syntax error.  gfortran
> does nothing with this statement
> 
> (gdb) c
> 
> we reach the point where the new error will be issued.
> 
> (gdb) p gfc_state_stack->state
> $2 = COMP_CONTAINS
> 
> This is the CONTAINS in the submodule.
> 
> (gdb) p gfc_state_stack->previous->state
> $3 = COMP_PROGRAM
> 
> This state fine here, because the syntax rejects the submodule
> statement.  So, the code looks like (from memory...)
> 
>  [program main implicitly included here]
>  contains
>  module subroutine foo
>  x = 2
>  y = 3
>  end submodule
> 
> The module prefix cannot appear in the subroutine statement. The
> new error rejects it.  So, now you have 2 assignments in after a
> contains statement.  The program now looks like
> 
>  contains
>  x = 2
>  y = 3 
>  end submodule
> 
> Well, you cannot do an assignment, so two additional error messages
> are emitted.  So, now we come to a program of the form
> 
> 
>  [program main implicitly included here]
>  end submodule
> 
> gfortran is expecting an END [PROGRAM] statement.
> 
> Is this clear or do you want me to withdrawal the patch?
> 
> -- 
> Steve

Re: [PATCH 14/14] Add D Phobos config, makefiles, and testsuite.

[Thomas Schwinge]

Hi!

On Tue, 18 Sep 2018 02:39:46 +0200, Iain Buclaw  wrote:
> This patch adds the configure and make files used for building D
> runtime and Phobos.  As well as running all unittests and the
> testsuite.

With a x86_64-pc-linux-gnu build, I've noticed breakage in '-m32'
multilib testing, made apparent by message: "[...]:
/lib/i386-linux-gnu/libgcc_s.so.1: version `GCC_7.0.0' not found
(required by [...])".  (That is, the system 'libgcc_s.so.1' being
dynamically linked instead of the just built one.)  This is because of
incomplete 'gccdir' setup in the '*.exp' file.  In such a multilibbed
configuration, there are 'build-gcc/gcc/libgcc.*' and
'build-gcc/gcc/32/libgcc.*' (for example); for '-m32' multilib testing,
paths need to be set up to point to the latter instead of the former.  It
seems as if some of this '*.exp' stuff has been copied from libffi (?);
the attached patch copies the missing pieces from there, too.  I've been
tempted to commit this "as obvious", but then thought I'll still get some
review/approval first.  If approving this patch, please respond with
"Reviewed-by: NAME " so that your effort will be recorded in the
commit log, see .


Grüße
 Thomas


From 5eb9e491a66c231df573d1915ba8c5f533e03d2c Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Sun, 14 Apr 2019 12:48:22 +0200
Subject: [PATCH] Fix libphobos testsuite libgcc multilib search path

---
 libphobos/testsuite/lib/libphobos.exp | 38 +--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/libphobos/testsuite/lib/libphobos.exp b/libphobos/testsuite/lib/libphobos.exp
index 6d113bc51723..0e4e2ebe085d 100644
--- a/libphobos/testsuite/lib/libphobos.exp
+++ b/libphobos/testsuite/lib/libphobos.exp
@@ -98,6 +98,15 @@ proc libphobos_init { args } {
 global gluefile wrap_flags
 global ld_library_path
 global DEFAULT_DFLAGS
+global GCC_UNDER_TEST
+
+if ![info exists GCC_UNDER_TEST] then {
+	if [info exists TOOL_EXECUTABLE] {
+	set GCC_UNDER_TEST $TOOL_EXECUTABLE
+	} else {
+	set GCC_UNDER_TEST "[find_gcc]"
+	}
+}
 
 # If a testcase doesn't have special options, use these.
 if ![info exists DEFAULT_DFLAGS] then {
@@ -142,13 +151,14 @@ proc libphobos_init { args } {
 	set exeext $env(EXEEXT)
 }
 
-# Compute what needs to be added to the existing LD_LIBRARY_PATH.
+# Compute what needs to be put into LD_LIBRARY_PATH
 set ld_library_path ""
 
+# Locate libgcc.a so we don't need to account for different values of
+# SHLIB_EXT on different platforms
 set gccdir [lookfor_file $tool_root_dir gcc/libgcc.a]
 if {$gccdir != ""} {
 	set gccdir [file dirname $gccdir]
-	append ld_library_path ":${gccdir}"
 }
 
 if { [file exists "${blddir}/libdruntime/.libs/libgdruntime.${shlib_ext}"] } {
@@ -159,6 +169,30 @@ proc libphobos_init { args } {
 	append ld_library_path ":${blddir}/src/.libs"
 }
 
+# Compute what needs to be added to the existing LD_LIBRARY_PATH.
+if {$gccdir != ""} {
+	# Add AIX pthread directory first.
+	if { [llength [glob -nocomplain ${gccdir}/pthread/libgcc_s*.a]] >= 1 } {
+	append ld_library_path ":${gccdir}/pthread"
+	}
+	append ld_library_path ":${gccdir}"
+	set compiler [lindex $GCC_UNDER_TEST 0]
+
+	if { [is_remote host] == 0 && [which $compiler] != 0 } {
+	  foreach i "[exec $compiler --print-multi-lib]" {
+	set mldir ""
+	regexp -- "\[a-z0-9=_/\.-\]*;" $i mldir
+	set mldir [string trimright $mldir "\;@"]
+	if { "$mldir" == "." } {
+	  continue
+	}
+	if { [llength [glob -nocomplain ${gccdir}/${mldir}/libgcc_s*.so.*]] >= 1 } {
+	  append ld_library_path ":${gccdir}/${mldir}"
+	}
+	  }
+	}
+}
+
 set_ld_library_path_env_vars
 
 libphobos_maybe_build_wrapper "${objdir}/testglue.o"
-- 
2.17.1



signature.asc
Description: PGP signature

Re: [PATCH 14/14] Add D Phobos config, makefiles, and testsuite.

[Iain Buclaw]

On Sat, 20 Apr 2019 at 22:30, Thomas Schwinge  wrote:
>
> Hi!
>
> On Tue, 18 Sep 2018 02:39:46 +0200, Iain Buclaw  
> wrote:
> > This patch adds the configure and make files used for building D
> > runtime and Phobos.  As well as running all unittests and the
> > testsuite.
>
> With a x86_64-pc-linux-gnu build, I've noticed breakage in '-m32'
> multilib testing, made apparent by message: "[...]:
> /lib/i386-linux-gnu/libgcc_s.so.1: version `GCC_7.0.0' not found
> (required by [...])".  (That is, the system 'libgcc_s.so.1' being
> dynamically linked instead of the just built one.)  This is because of
> incomplete 'gccdir' setup in the '*.exp' file.  In such a multilibbed
> configuration, there are 'build-gcc/gcc/libgcc.*' and
> 'build-gcc/gcc/32/libgcc.*' (for example); for '-m32' multilib testing,
> paths need to be set up to point to the latter instead of the former.  It
> seems as if some of this '*.exp' stuff has been copied from libffi (?);
> the attached patch copies the missing pieces from there, too.  I've been
> tempted to commit this "as obvious", but then thought I'll still get some
> review/approval first.  If approving this patch, please respond with
> "Reviewed-by: NAME " so that your effort will be recorded in the
> commit log, see .
>

Seems reasonable to me.

-- 
Iain

Re: [PATCH, LRA]: Revert the revert of removal of usless move insns.

[Vladimir Makarov]

On 4/20/19 4:55 AM, Uros Bizjak wrote:

On 4/20/19, Vladimir Makarov  wrote:

On 11/21/18 2:33 PM, Uros Bizjak wrote:

Hello!

Before the recent patch to post-reload mode switching, vzeroupper
insertion depended on the existence of the return copy instructions
pair in functions that return a value. The first instruction in the
pair represents a move to a function return hard register, and the
second was a USE of the function return hard register. Sometimes a nop
move was generated (e.g. %eax->%eax) for the first instruction of the
return copy instructions pair and the patch [1] teached LRA  to remove
these useless instructions on the fly.

The removal caused optimize mode switching to trigger the assert,
since the first instruction of a return pair was not found. The
relevant part of the patch was later reverted. With the recent
optimize mode switching patch, this is no longer necessary for
vzeroupper insertion pass, so attached patch reverts the revert.

2018-11-21  Uros Bizjak  

  Revert the revert:
  2013-10-26  Vladimir Makarov  

  Revert:
  2013-10-25  Vladimir Makarov  

  * lra-spills.c (lra_final_code_change): Remove useless move insns.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline?

Sure, Uros. I support the patch.  But I think it would be wise to
postpone its committing after releasing GCC-9.  Simply it is hard to
predict the patch effect to other targets and I would avoid any risk at
this stage.

Actually, the "revert of the revert" patch was already committed to
mainline some time ago.


Sorry for confusion, Uros. I did not check the date of your original 
posting.  Insn removal was added to RA just to avoid wasting CPU cycles 
on such insn processing afterwards.  Such insns are removed anyway later 
in the pass pipeline.  The CPU time savings are tiny but the removal 
creates too many problems including new one PR90178.  I should have 
avoided to put this code in the first place.


I think we should remove this code forever. It is not convenient for me 
to do this now because I am traveling.  If somebody wants to remove the 
code, i am approving this in advance.




To clear the possible misunderstanding, let me summarise the issue:

- the original patch that remove useless move insn caused a breakage
in vzeroupper pass.
- the original patch was reverted due to the above breakage
- the vzeroupper pass was later adjusted to tolerate removed useless
move instructions, and this cleared the way to revert the revert. Now
LRA removes useless move insns.

An orthogonal issue (PR90178) was discovered, showing that some passes
also depend on the presence of useless move insn.

The bisection stumbled on the "revert of the revert" patch that
(obviously) re-introduced the issue. I'm not in the position to decide
if useless move insn can be removed or if these later passes should be
fixed, I can only say that the vzeroupper pass is now agnostic to the
presence of useless move insns.

Uros.

Re: [PATCH] Improve implementation of parallel equal()

[Thomas Rodgers]

Jonathan Wakely writes:

> On 16/04/19 12:39 -0700, Thomas Rodgers wrote:
>>
>>  * include/pstl/algorithm_impl.h
>>  (__internal::__brick_equal): use "4 iterator" version of
>>  std::equal().
>>  (__internal::__brick_equal): use simd for random access
>>  iterators on unsequenced execution policies.
>>  (__internal::__pattern_equal): add "4 iterator" version
>>  (__internal::__pattern_equal): dispatch to simd __brick_equal
>>  for vector-only execution policies.
>>  (__internal::__pattern_equal): disptach to __parallel_or for
>
> s/disptach/dispatch/ in the changelog
>
>
>
>>--- a/libstdc++-v3/include/pstl/glue_algorithm_impl.h
>>+++ b/libstdc++-v3/include/pstl/glue_algorithm_impl.h
>>@@ -757,7 +757,7 @@ 
>>__pstl::__internal::__enable_if_execution_policy<_ExecutionPolicy, bool>
>> equal(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, 
>> _ForwardIterator1 __last1, _ForwardIterator2 __first2,
>>   _ForwardIterator2 __last2)
>> {
>>-return equal(std::forward<_ExecutionPolicy>(__exec), __first1, __last1, 
>>__first2,
>>+return equal(std::forward<_ExecutionPolicy>(__exec), __first1, __last1, 
>>__first2, __last2,
>
> N.B. I don't think this should be an unqualified call, but that can be
> fixed in a later patch.
>
> OK for trunk, thanks.

Tested x86_64-linux-gnu, committed to trunk.

Re: [PATCH] Cleanup algorithm implementations

[Thomas Rodgers]

Jonathan Wakely writes:

> On 19/04/19 11:59 -0700, Thomas Rodgers wrote:
>>  * include/pstl/glue_algorithm_impl.h (stable_sort): Forward
>>execution policy.
>>  (mismatch): Forward execution policy.
>>  (equal): Qualify call to std::equal().
>>  (partial_sort): Forward execution policy.
>>  (inplace_merge): Forward execution policy.
>
> OK for trunk, thanks.

Tested x86_64-linux-gnu, committed to trunk.

Re: [PATCH] Delegate PSTL configuration to pstl/pstl_config.h

[Thomas Rodgers]

Jonathan Wakely writes:

> On 18/04/19 17:02 -0700, Thomas Rodgers wrote:
>>
>>   * include/bits/c++config: Remove explicit PSTL configuration
>>   macros and use definitions from .
>
> OK for trunk, thanks.

Tested x86_64-linux-gnu, committed to trunk.

[patch, testsuite] make g++.dg/ipa/pr89009.C require fpic effective target

[Sandra Loosemore]

I've checked in the attached patch as obvious, to clean up some test 
failures on nios2-elf.  (The nios2 PSABI only defines PIC relocations 
for GNU/Linux targets so gcc rejects -fpic when compiling for bare-metal 
target.)


-Sandra
2019-04-20  Sandra Loosemore  

	gcc/testsuite/
	* g++.dg/ipa/pr89009.C: Add dg-require-effective-target fpic.
Index: gcc/testsuite/g++.dg/ipa/pr89009.C
===
--- gcc/testsuite/g++.dg/ipa/pr89009.C	(revision 270475)
+++ gcc/testsuite/g++.dg/ipa/pr89009.C	(working copy)
@@ -1,5 +1,6 @@
 /* PR ipa/89009 */
 /* { dg-do run } */
+/* { dg-require-effective-target fpic } */
 /* { dg-options "-fpic -O2 -fno-inline" } */
 /* { dg-require-visibility "" } */