[patch][fix PR other/31566] @missing_file gives bad error message
Hi all,
I don't know if this is the right time to submit such patches.
But this patch attempts to fix
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31566
I have successfully bootstrapped and tested on x86_64-pc-linux-gnu
testcases:
file:
-Wall
test.c:
void foo()
{
int a,b;
a = b + 1;
}
@test2:
void foo()
{
int a,b;
a = b + 1;
}
case 1: file and test.c are present
./gcc @file test.c -c
test.c: In function ‘foo’:
test.c:3:7: warning: variable ‘a’ set but not used [-Wunused-but-set-variable]
int a,b;
^
test.c:4:5: warning: ‘b’ is used uninitialized in this function
[-Wuninitialized]
a = b + 1;
~~^~~
case 2: file1 is not present and test.c,file are present
./gcc @file @file1 test.c -c
gcc: error: file1: No such file or directory
case 3: file is present and test1.c is not present
./gcc @file test1.c -c
gcc: error: test1.c: No such file or directory
gcc: fatal error: no input files
compilation terminated.
case 4: file1 and test1.c both are not present
./gcc @file1 test1.c -c
gcc: error: file1: No such file or directory
gcc: error: test1.c: No such file or directory
gcc: fatal error: no input files
compilation terminated.
case 5: @test2.c is present
./gcc @test2.c -c
-> compiled successfully without any error/warning
case 6: both file and @test2.c are present
./gcc @file @test2.c -c
@test2.c: In function ‘foo’:
@test2.c:3:7: warning: variable ‘a’ set but not used [-Wunused-but-set-variable]
int a,b;
^
@test2.c:4:5: warning: ‘b’ is used uninitialized in this function
[-Wuninitialized]
a = b + 1;
~~^~~
case 7: @file1 is not present and @test2.c is present
./gcc @file1 @test2.c -c
gcc: error: file1: No such file or directory
case 8: both @file1 and @test3.c are not present
gcc: error: file1: No such file or directory
gcc: error: test3.c: No such file or directory
gcc: fatal error: no input files
compilation terminated.
-- in this case @test3.c is treated as a file with arguments (test3.c)
Thanks,
Prasad
diff --git a/gcc/gcc.c b/gcc/gcc.c
index d3e8c88..fd2b182 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4417,7 +4417,12 @@ process_command (unsigned int decoded_options_count,
fname = xstrdup (arg);
if (strcmp (fname, "-") != 0 && access (fname, F_OK) < 0)
- perror_with_name (fname);
+ {
+ if (fname[0] == '@' && access (fname + 1, F_OK) < 0)
+ perror_with_name (fname + 1);
+ else
+ perror_with_name (fname);
+ }
else
add_infile (arg, spec_lang);
changelog
Description: Binary data
Re: [PATCH][1/2] Fix PR77768
On 2016.09.28 at 13:33 +0200, Richard Biener wrote: > > I am testing the following patch to avoid useless VRP range allocations > when we just ask for varying on stmts we don't know how to handle. > I think it should fix the PR where we end up assigning to the > static const vr_const_varying returned by get_value_range in the early VRP > context. > > Eventually the VRP lattice needs to become sparse (or just growable, I > have a patch for that as well). Not until early VRP gets some more > capabilities though, it would be a waste otherwise. > > Bootstrap / regtest in progress on x86_64-unknown-linux-gnu. You probably noticed this yourself, but the patch causes conftests to spin during stageprofile. For example libiberty: configure:6516: checking for working fork configure:6538: /home/trippels/gcc_build_dir_/./prev-gcc/xgcc -B/home/trippels/gcc_build_dir_/./prev-gcc/ -B/usr/local/powerpc64le-unknown-linux-gnu/bin/ -B/usr/local/powerpc64le-unknown-linux-gnu/bin/ -B/usr/local/powerpc64le-unknown-linux-gnu/lib/ -isystem /usr/local/powerpc64le-unknown-linux-gnu/include -isystem /usr/local/powerpc64le-unknown-linux-gnu/sys-include-o conftest -mcpu=power8 -O3 -pipe -gtoggle -fprofile-generate -static-libstdc++ -static-libgcc conftest.c >&5 configure:6538: $? = 0 configure:6538: ./conftest <> -- Markus
Re: Change license of filenames.h to LGPL
> From: Ozkan Sezer > Date: Thu, 29 Sep 2016 00:09:19 +0300 > Cc: Alexandre Oliva , gcc-patches@gcc.gnu.org > > On 9/28/16, Eli Zaretskii wrote: > >> From: Alexandre Oliva > >> Cc: gcc-patches@gcc.gnu.org > >> Date: Wed, 28 Sep 2016 16:03:02 -0300 > >> > >> Does that work for everyone involved? > > > > Except that no one will reimburse me for the time I wasted talking to > > several people, with eventually null result... > > > > FWIW, you have my thanks for at least helping my case. It's worth a lot to me, and you are welcome. I just hoped to actually do what you requested, not just talk about it. Now I'm in a situation where, after being authorized to make the change by whom I consider the legal custodian of that file's license, I face people who, while not being opposed to the change, don't actually want to do that. That doesn't sound right to me.
libgo patch committed: Copy runtime.go and runtime1.go from Go 1.7 to libgo
This patch copies runtime.go and runtime1.go from the Go 1.7 runtime
library to libgo, replacing some miscellaneous functions currently
written in C. Some transitional support routines are added to
stubs.go. A few other minor files come along for the ride.
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 240588)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-c79a35411c1065c71add196fdeca6e5207a79248
+e51657a576367c7a498c94baf985b79066fc082a
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/go/runtime/cputicks.go
===
--- libgo/go/runtime/cputicks.go(revision 0)
+++ libgo/go/runtime/cputicks.go(working copy)
@@ -0,0 +1,9 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+// careful: cputicks is not guaranteed to be monotonic! In particular, we have
+// noticed drift between cpus on certain os/arch combinations. See issue 8976.
+func cputicks() int64
Index: libgo/go/runtime/env_posix.go
===
--- libgo/go/runtime/env_posix.go (revision 0)
+++ libgo/go/runtime/env_posix.go (working copy)
@@ -0,0 +1,20 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris windows
+
+package runtime
+
+func gogetenv(key string) string {
+ env := environ()
+ if env == nil {
+ throw("getenv before env init")
+ }
+ for _, s := range environ() {
+ if len(s) > len(key) && s[len(key)] == '=' && s[:len(key)] ==
key {
+ return s[len(key)+1:]
+ }
+ }
+ return ""
+}
Index: libgo/go/runtime/export_test.go
===
--- libgo/go/runtime/export_test.go (revision 240334)
+++ libgo/go/runtime/export_test.go (working copy)
@@ -6,10 +6,6 @@
package runtime
-import (
- "unsafe"
-)
-
//var Fadd64 = fadd64
//var Fsub64 = fsub64
//var Fmul64 = fmul64
@@ -103,20 +99,6 @@ var HashLoad = &hashLoad
//type Uintreg uintreg
-//extern __go_open
-func open(path *byte, mode int32, perm int32) int32
-
-func Open(path *byte, mode int32, perm int32) int32 {
- return open(path, mode, perm)
-}
-
-//extern close
-func close(int32) int32
-
-func Close(fd int32) int32 {
- return close(fd)
-}
-
/*
func RunSchedLocalQueueTest() {
_p_ := new(p)
@@ -224,25 +206,13 @@ var IfaceHash = ifaceHash
var MemclrBytes = memclrBytes
*/
-//extern read
-func read(fd int32, buf unsafe.Pointer, size int32) int32
+var Open = open
+var Close = closefd
+var Read = read
+var Write = write
-func Read(fd int32, buf unsafe.Pointer, size int32) int32 {
- return read(fd, buf, size)
-}
-
-//extern write
-func write(fd int32, buf unsafe.Pointer, size int32) int32
-
-func Write(fd uintptr, buf unsafe.Pointer, size int32) int32 {
- return write(int32(fd), buf, size)
-}
-
-func envs() []string
-func setenvs([]string)
-
-var Envs = envs
-var SetEnvs = setenvs
+func Envs() []string { return envs }
+func SetEnvs(e []string) { envs = e }
//var BigEndian = sys.BigEndian
@@ -287,7 +257,10 @@ var ForceGCPeriod = &forcegcperiod
// SetTracebackEnv is like runtime/debug.SetTraceback, but it raises
// the "environment" traceback level, so later calls to
// debug.SetTraceback (e.g., from testing timeouts) can't lower it.
-func SetTracebackEnv(level string)
+func SetTracebackEnv(level string) {
+ setTraceback(level)
+ traceback_env = traceback_cache
+}
/*
var ReadUnaligned32 = readUnaligned32
Index: libgo/go/runtime/os_linux.go
===
--- libgo/go/runtime/os_linux.go(revision 0)
+++ libgo/go/runtime/os_linux.go(working copy)
@@ -0,0 +1,56 @@
+// Copyright 2009 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+import (
+ "runtime/internal/sys"
+ "unsafe"
+)
+
+const (
+ _AT_NULL = 0 // End of vector
+ _AT_PAGESZ = 6 // System physical page size
+ _AT_RANDOM = 25 // introduced in 2.6.29
+)
+
+func sysargs(argc int32, argv **byte) {
+ n := argc + 1
+
+ // skip over argv, envp to get to auxv
+ for argv_index(argv, n) != nil {
+ n++
+ }
+
+ // skip NULL separator
+
Re: Implement -Wimplicit-fallthrough (version 9)
> "Jakub" == Jakub Jelinek writes:
>> default:
>> {
>> complaint (&symfile_complaints,
>> _("Storage class %d not recognized during scan"),
>> sclass);
>> }
>> /* FALLTHROUGH */
>>
>> /* C_FCN is .bf and .ef symbols. I think it is sufficient
>> to handle only the C_FUN and C_EXT. */
>> case C_FCN:
Jakub> Is complaint a noreturn call?
Nope.
Jakub> but right now we only look for such comments immediately before a
Jakub> case/default keyword or user label; if there is another comment
Jakub> in between, it is ignored. This is something we are considering
Jakub> to change, exactly because often the /* FALLTHRU */ comment
Jakub> appears after some case and then there is unrelated comment
Jakub> before the next case about what that case handles.
Make sense. Thanks.
Tom
Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP
Committed as svn r240601 and patch 2/2 as r240602. -- Alan Modra Australia Development Lab, IBM
Re: Patch ping
On Wed, Sep 28, 2016 at 11:46:59PM +0200, Bernd Schmidt wrote: > On 09/28/2016 11:40 PM, Jakub Jelinek wrote: > >On Wed, Sep 28, 2016 at 11:17:55PM +0200, Bernd Schmidt wrote: > >>On 09/28/2016 09:47 PM, Jakub Jelinek wrote: > >>>And here are the 0 < var to var > 0 changes. Thoughts on those? > >> > >>I kind of meant it the other way round, so yeah, please install. > > > >Oops, sorry, shall I revert what I've committed then? > > No, I think it looks fine too, although I can't figure out why that one > block of code was moved. The intent was that each of the non-__*_chk builtins is followed by its __*_chk counterpart; without the patch that was almost the case except for that one exception. Jakub
Re: Patch ping
On 09/28/2016 11:40 PM, Jakub Jelinek wrote: On Wed, Sep 28, 2016 at 11:17:55PM +0200, Bernd Schmidt wrote: On 09/28/2016 09:47 PM, Jakub Jelinek wrote: And here are the 0 < var to var > 0 changes. Thoughts on those? I kind of meant it the other way round, so yeah, please install. Oops, sorry, shall I revert what I've committed then? No, I think it looks fine too, although I can't figure out why that one block of code was moved. Bernd
Re: Patch ping
On Wed, Sep 28, 2016 at 11:17:55PM +0200, Bernd Schmidt wrote: > On 09/28/2016 09:47 PM, Jakub Jelinek wrote: > >And here are the 0 < var to var > 0 changes. Thoughts on those? > > I kind of meant it the other way round, so yeah, please install. Oops, sorry, shall I revert what I've committed then? Jakub
[PATCH, i386]: Fix PR 77756, _-get_cpuid() returns wrong values for level 7 (extended features)
2016-09-28 Uros Bizjak
PR target/77756
* config/i386/cpuid.h (__get_cpuid): Handle CPUID level >= 7.
testsuite/ChangeLog:
2016-09-28 Uros Bizjak
PR target/77756
* gcc.target/i386/pr77756.c: New test.
Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline, will be backported to release branches.
Uros.
Index: config/i386/cpuid.h
===
--- config/i386/cpuid.h (revision 240579)
+++ config/i386/cpuid.h (working copy)
@@ -244,6 +244,16 @@ __get_cpuid (unsigned int __level,
if (__get_cpuid_max (__ext, 0) < __level)
return 0;
- __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
+ if (__ext)
+__cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
+ else
+{
+ if (__level >= 13)
+ __cpuid_count (__level, 1, *__eax, *__ebx, *__ecx, *__edx);
+ else if (__level >= 7)
+ __cpuid_count (__level, 0, *__eax, *__ebx, *__ecx, *__edx);
+ else
+ __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
+}
return 1;
}
Index: testsuite/gcc.target/i386/pr77756.c
===
--- testsuite/gcc.target/i386/pr77756.c (nonexistent)
+++ testsuite/gcc.target/i386/pr77756.c (working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+
+#include "cpuid.h"
+
+int
+main ()
+{
+ __builtin_cpu_init ();
+
+ if (__builtin_cpu_supports ("avx2"))
+{
+ unsigned int eax, ebx, ecx, edx;
+
+ if (!__get_cpuid (7, &eax, &ebx, &ecx, &edx))
+ __builtin_abort ();
+
+ if (!(ebx & bit_AVX2))
+ __builtin_abort ();
+}
+
+ return 0;
+}
Re: Patch ping
On 09/28/2016 09:47 PM, Jakub Jelinek wrote: And here are the 0 < var to var > 0 changes. Thoughts on those? I kind of meant it the other way round, so yeah, please install. Bernd
Re: Change license of filenames.h to LGPL
On 9/28/16, Eli Zaretskii wrote: >> From: Alexandre Oliva >> Cc: gcc-patches@gcc.gnu.org >> Date: Wed, 28 Sep 2016 16:03:02 -0300 >> >> Does that work for everyone involved? > > Except that no one will reimburse me for the time I wasted talking to > several people, with eventually null result... > FWIW, you have my thanks for at least helping my case.
Re: Patch ping
Hi, I too personally always prefer to write the code as the variable at the left side and the constant at the right side of the comparison, because that is how I would also say it naturally in an English or German sentence. Like for instance "my son is more than 7 years old". I think nobody would ever say it the other way round. Maybe, except when it is a mathematical "a < b < c" relation, I would write it in C as "a < b && b < c", even if b is the variable part and a and c the constants. Hope that does not add more confusion... Having that said, the patch looks good to me. Thanks Bernd.
Re: Change license of filenames.h to LGPL
> From: Alexandre Oliva > Cc: gcc-patches@gcc.gnu.org > Date: Wed, 28 Sep 2016 16:03:02 -0300 > > Does that work for everyone involved? Except that no one will reimburse me for the time I wasted talking to several people, with eventually null result...
Re: [PATCH] fix profiledbootstrap with -Werror=format-length (77753)
On 09/27/2016 10:38 PM, Jeff Law wrote: On 09/27/2016 01:30 PM, Martin Sebor wrote: The attached one line patch increases a local buffer size to avoid an apparently justified (though in reality likely a false positive) -Wformat-length warning in varasm.c. The warning has been reported to break profiledbootstrap on powerp64le (though not ordinary bootstrap). An arguably better solution would be to explicitly constrain the range of the integer variable to that of valid priority values. Unfortunately, as pointed out in bug 77721, due to the limitation of the current VRP implementation, this isn't always reliable. Martin gcc-77753.diff PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to -Werror=format-length in varasm.c:1573 gcc/ChangeLog: 2016-09-27 Martin Sebor PR bootstrap/77753 * varasm.c (assemble_addr_to_section): Increase local buffer size. OK. WRT 77721, could you add that test to the testsuite, but xfailed? Sure. Done in r240595. Martin
Re: Patch ping
On Wed, Sep 28, 2016 at 09:28:14PM +0200, Bernd Schmidt wrote:
> On 09/28/2016 09:24 PM, Jakub Jelinek wrote:
> >I'd like to ping the
> >
> >http://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html
> >
> >patch, containing various fixes for gimple-ssa-sprintf.c.
> >If the 0 < var to var > 0 changes are deemed too controversial, I can
> >separate them from the other changes.
>
> I'd like to see them separated because they are obvious and good, so please
> install them first.
And here are the 0 < var to var > 0 changes. Thoughts on those?
2016-09-28 Jakub Jelinek
* gimple-ssa-sprintf.c (pass_sprintf_length::gate): Use x > 0 instead
of 0 < x.
(format_floating, format_string, format_directive,
get_destination_size, pass_sprintf_length::handle_gimple_call):
Likewise.
--- gcc/gimple-ssa-sprintf.c.jj 2016-09-21 08:54:15.0 +0200
+++ gcc/gimple-ssa-sprintf.c2016-09-21 15:09:02.209261013 +0200
@@ -130,8 +130,8 @@ pass_sprintf_length::gate (function *)
not optimizing and the pass is being invoked early, or when
optimizing and the pass is being invoked during optimization
(i.e., "late"). */
- return ((0 < warn_format_length || flag_printf_return_value)
- && (0 < optimize) == fold_return_value);
+ return ((warn_format_length > 0 || flag_printf_return_value)
+ && (optimize > 0) == fold_return_value);
}
/* The result of a call to a formatted function. */
@@ -1188,7 +1188,7 @@ format_floating (const conversion_spec &
case 'a':
{
/* The minimum output is "0x.p+0". */
- res.range.min = 6 + (0 < prec ? prec : 0);
+ res.range.min = 6 + (prec > 0 ? prec : 0);
/* Compute the maximum just once. */
static const int a_max[] = {
@@ -1249,7 +1249,7 @@ format_floating (const conversion_spec &
gcc_unreachable ();
}
- if (0 < width)
+ if (width > 0)
{
if (res.range.min < (unsigned)width)
res.range.min = width;
@@ -1440,7 +1440,7 @@ get_string_length (tree str)
static fmtresult
format_string (const conversion_spec &spec, tree arg)
{
- unsigned width = spec.have_width && 0 < spec.width ? spec.width : 0;
+ unsigned width = spec.have_width && spec.width > 0 ? spec.width : 0;
int prec = spec.have_precision ? spec.precision : -1;
if (spec.star_width)
@@ -1756,7 +1756,7 @@ format_directive (const pass_sprintf_len
}
else
{
- if (!res->warned && 0 < fmtres.range.min && navail < fmtres.range.min)
+ if (!res->warned && fmtres.range.min > 0 && navail < fmtres.range.min)
{
const char* fmtstr
= (info.bounded
@@ -2332,7 +2332,7 @@ get_destination_size (tree dest)
a member array as opposed to the whole enclosing object), otherwise
use type-zero object size to determine the size of the enclosing
object (the function fails without optimization in this type). */
- int ost = 0 < optimize;
+ int ost = optimize > 0;
unsigned HOST_WIDE_INT size;
if (compute_builtin_object_size (dest, ost, &size))
return size;
@@ -2648,7 +2640,8 @@ pass_sprintf_length::handle_gimple_call
attempt to substitute the computed result for the return value of
the call. Avoid this optimization when -frounding-math is in effect
and the format string contains a floating point directive. */
- if (0 < optimize && flag_printf_return_value
+ if (optimize > 0
+ && flag_printf_return_value
&& (!flag_rounding_math || !res.floating))
try_substitute_return_value (gsi, info, res);
}
Jakub
Re: Patch ping
On Wed, Sep 28, 2016 at 09:28:14PM +0200, Bernd Schmidt wrote:
> On 09/28/2016 09:24 PM, Jakub Jelinek wrote:
> >I'd like to ping the
> >
> >http://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html
> >
> >patch, containing various fixes for gimple-ssa-sprintf.c.
> >If the 0 < var to var > 0 changes are deemed too controversial, I can
> >separate them from the other changes.
>
> I'd like to see them separated because they are obvious and good, so please
> install them first.
Ok, so here is the separated patch I've installed:
2016-09-28 Jakub Jelinek
* gimple-ssa-sprintf.c: Fix comment formatting.
(format_integer): Use is_gimple_assign.
(pass_sprintf_length::handle_gimple_call): Use gimple_call_builtin_p
and gimple_call_fndecl. Reorder case BUILT_IN_SPRINTF_CHK. Fix up
BUILT_IN_SNPRINTF_CHK comment. Replace "to to" with "to" in comment.
(pass_sprintf_length::execute): Use is_gimple_call.
--- gcc/gimple-ssa-sprintf.c.jj 2016-09-21 08:54:15.0 +0200
+++ gcc/gimple-ssa-sprintf.c2016-09-21 15:09:02.209261013 +0200
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.
The pass handles all forms standard sprintf format directives,
including character, integer, floating point, pointer, and strings,
- with the standard C flags, widths, and precisions. For integers
+ with the standard C flags, widths, and precisions. For integers
and strings it computes the length of output itself. For floating
point it uses MPFR to fornmat known constants with up and down
rounding and uses the resulting range of output lengths. For
@@ -464,7 +464,7 @@ struct conversion_spec
/* Format conversion function that given a conversion specification
and an argument returns the formatting result. */
- fmtresult (*fmtfunc) (const conversion_spec &, tree);
+ fmtresult (*fmtfunc) (const conversion_spec &, tree);
/* Return True when a the format flag CHR has been used. */
bool get_flag (char chr) const
@@ -1032,10 +1032,10 @@ format_integer (const conversion_spec &s
{
/* The argument here may be the result of promoting the actual
argument to int. Try to determine the type of the actual
-argument before promotion and narrow down its range that
+argument before promotion and narrow down its range that
way. */
gimple *def = SSA_NAME_DEF_STMT (arg);
- if (gimple_code (def) == GIMPLE_ASSIGN)
+ if (is_gimple_assign (def))
{
tree_code code = gimple_assign_rhs_code (def);
if (code == NOP_EXPR)
@@ -2449,18 +2449,10 @@ pass_sprintf_length::handle_gimple_call
call_info info = call_info ();
info.callstmt = gsi_stmt (gsi);
- info.func = gimple_call_fn (info.callstmt);
- if (!info.func)
-return;
-
- if (TREE_CODE (info.func) == ADDR_EXPR)
-info.func = TREE_OPERAND (info.func, 0);
-
- if (TREE_CODE (info.func) != FUNCTION_DECL
- || !DECL_BUILT_IN(info.func)
- || DECL_BUILT_IN_CLASS (info.func) != BUILT_IN_NORMAL)
+ if (!gimple_call_builtin_p (info.callstmt, BUILT_IN_NORMAL))
return;
+ info.func = gimple_call_fndecl (info.callstmt);
info.fncode = DECL_FUNCTION_CODE (info.func);
/* The size of the destination as in snprintf(dest, size, ...). */
@@ -2487,6 +2479,14 @@ pass_sprintf_length::handle_gimple_call
info.argidx = 2;
break;
+case BUILT_IN_SPRINTF_CHK:
+ // Signature:
+ // __builtin___sprintf_chk (dst, ost, objsize, format, ...)
+ idx_objsize = 2;
+ idx_format = 3;
+ info.argidx = 4;
+ break;
+
case BUILT_IN_SNPRINTF:
// Signature:
// __builtin_snprintf (dst, size, format, ...)
@@ -2498,7 +2498,7 @@ pass_sprintf_length::handle_gimple_call
case BUILT_IN_SNPRINTF_CHK:
// Signature:
- // __builtin___sprintf_chk (dst, size, ost, objsize, format, ...)
+ // __builtin___snprintf_chk (dst, size, ost, objsize, format, ...)
idx_dstsize = 1;
idx_objsize = 3;
idx_format = 4;
@@ -2506,14 +2506,6 @@ pass_sprintf_length::handle_gimple_call
info.bounded = true;
break;
-case BUILT_IN_SPRINTF_CHK:
- // Signature:
- // __builtin___sprintf_chk (dst, ost, objsize, format, ...)
- idx_objsize = 2;
- idx_format = 3;
- info.argidx = 4;
- break;
-
case BUILT_IN_VSNPRINTF:
// Signature:
// __builtin_vsprintf (dst, size, format, va)
@@ -2556,7 +2548,7 @@ pass_sprintf_length::handle_gimple_call
if (idx_dstsize == HOST_WIDE_INT_M1U)
{
- // For non-bounded functions like sprintf, to to determine
+ // For non-bounded functions like sprintf, to determine
// the size of the destination from the object or pointer
// passed to it as the first argument.
dstsize = get_destination_size (gimple_call_arg (info.callstmt, 0));
@@ -2667,7 +2660,7 @@ pass_sprintf_l
Re: [PATCH] Backport of 25 fortran patches
On 09/28/2016 12:12 PM, Steve Kargl wrote: The attached patch and ChangeLog entries are for the backporting of 25 patches from trunk to the 6-branch. The bugzilla PR's contained in the patch are fortran/41922 fortran/60774 fortran/61318 fortran/68566 fortran/69514 fortran/69867 fortran/69962 fortran/70006 fortran/71067 fortran/71730 fortran/71799 fortran/71859 fortran/71862 fortran/77260 fortran/77351 fortran/77372 fortran/77380 fortran/77391 fortran/77420 fortran/77429 fortran/77460 fortran/77506 fortran/77507 fortran/77612 fortran/77694 The patch has been bootstrapped and regression tested on x86_64-*-freebsd. Ok to commit? Well, though it is preferred to backport one at a time to help with backtracking, I also don't think we should be too pedantic either. Thanks for all the work. OK Jerry
Re: debug container mutex association
On 28/09/16 21:30 +0200, François Dumont wrote:
On 27/09/2016 12:32, Jonathan Wakely wrote:
Index: include/debug/safe_base.h
===
--- include/debug/safe_base.h(revision 240509)
+++ include/debug/safe_base.h(working copy)
@@ -121,11 +121,11 @@
void
_M_detach();
+ public:
/** Likewise, but not thread-safe. */
void
_M_detach_single() throw ();
- public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
Would this be a smaller change, that doesn't make the member
accessible to all code?
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -50,6 +50,7 @@ namespace __gnu_debug
class _Safe_iterator_base
{
friend class _Safe_sequence_base;
+template friend class _Safe_sequence;
public:
/** The sequence this iterator references; may be NULL to indicate
.
I am not a great fan of friend class. As long as it was friend
declaration between iterator and sequence base types it was ok. Now
that we need to make a template class friend I consider that it is too
much friendship and prefer to make the necessary method public.
OK.
But if you think otherwise just tell me and I will use your approach.
Making it public is fine. Please commit your original patch, thanks!
Re: Change license of filenames.h to LGPL
On 9/28/16, Alexandre Oliva wrote: > On Sep 27, 2016, Ozkan Sezer wrote: > >> FYI: What I originally wanted was an authorization _for me_ to use >> filenames.h in LGPL projects with LGPL license notice; the version >> I use is modified (not refer to any external code other than libc, >> i.e. only macros and inlines) and doesn't include hashtab.h either; >> therefore I believe that my request is fulfilled and is not subject >> to the concerns raised by you guys. > > It would probably be wise for you to amend the modified copy you'll > distribute with the changes proposd by Eli, and to add a link to this > thread in the archives, should anyone be surprised by the different > license. I believe the following is enough? https://sf.net/p/libtimidity/libtimidity/ci/master/tree/src/filenames.h > As for the copy in GCC, that has additional code, we can then keep it > under the stronger copyleft defenses. > > Does that work for everyone involved? For me, yes. -- O.S.
Shared mutex pool
Hi
Here is the patch to share a mutex pool between debug mode and
shared_ptr implementation. It saves 392 bytes on generated .so and will
make sure that fixing false sharing will impact both usages.
I preferred to leave implementation in shared_ptr.cc to avoid
introducing another translation unit.
* src/c++11/shared_ptr.cc (mask, invalid, get_mutex): Move
declaration...
* src/c++11/mutex_pool.h: ... here. New.
* src/c++11/debug.cc: Use latter.
Tested under Linux x86_64, normal and debug modes.
Ok to commit ?
François
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 6b0db11..d79e43b 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -40,6 +40,8 @@
#include // for __cxa_demangle
+#include "mutex_pool.h"
+
using namespace std;
namespace
@@ -50,15 +52,13 @@ namespace
__gnu_cxx::__mutex&
get_safe_base_mutex(void* address)
{
-const size_t mask = 0xf;
-static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
-
// Use arbitrarily __gnu_debug::vector as the container giving
// alignment of debug containers.
const auto alignbits = __builtin_ctz(alignof(__gnu_debug::vector));
-const size_t index
- = (reinterpret_cast(address) >> alignbits) & mask;
-return safe_base_mutex[index];
+const unsigned char index
+ = (reinterpret_cast(address) >> alignbits)
+ & __gnu_internal::mask;
+return __gnu_internal::get_mutex(index);
}
void
diff --git a/libstdc++-v3/src/c++11/mutex_pool.h b/libstdc++-v3/src/c++11/mutex_pool.h
new file mode 100644
index 000..a352467
--- /dev/null
+++ b/libstdc++-v3/src/c++11/mutex_pool.h
@@ -0,0 +1,34 @@
+// Mutex pool used to limit contention -*- C++ -*-
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+//
Re: [PATCH] fix typos behind incorrect destination buffer length in -Wformat-length warning (77762)
On 09/27/2016 10:43 PM, Jeff Law wrote: On 09/27/2016 04:50 PM, Martin Sebor wrote: The attached patch corrects a couple of typos in argument numbers in the handling of __builtin__vsnprintf_chk calls in the gimple- ssa-sprintf pass, and another couple of typos in the test for this that were masking this failure. As an aside, the patch also fixes the off-by-one line test failures introduced in r240503. If there is a way to make the line numbers relative (as suggested in https://gcc.gnu.org/ml/gcc-patches/2016-09/msg02070.html) I'm happy to update the -Wformat-length tests to make use of them (and document it on the Wiki) if someone can point me at an example (or documentation). I couldn't find examples of dg-warning directives that use the feature. Thanks Martin gcc-77762.diff PR c/77762 - Incorrect destination buffer length in -Wformat-length warning gcc/testsuite/ChangeLog: 2016-09-27 Martin Sebor PR c/77762 * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test_vsnprintf_chk_s): Call __builtin___vsnprintf_chk, not __builtin___snprintf_chk. (test_sprintf_p_const): Adjust line numbers to avoid failures introduced in r240503. gcc/ChangeLog: 2016-09-27 Martin Sebor PR c/77762 * gimple-ssa-sprintf.c (pass_sprintf_length::handle_gimple_call): Fix typos. OK. The relative line number capability was just added: https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01617.html Thanks. I added it to the Wiki: https://gcc.gnu.org/wiki/HowToPrepareATestcas Martin
Re: debug container mutex association
On 27/09/2016 12:32, Jonathan Wakely wrote:
Index: include/debug/safe_base.h
===
--- include/debug/safe_base.h(revision 240509)
+++ include/debug/safe_base.h(working copy)
@@ -121,11 +121,11 @@
void
_M_detach();
+ public:
/** Likewise, but not thread-safe. */
void
_M_detach_single() throw ();
- public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
Would this be a smaller change, that doesn't make the member
accessible to all code?
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -50,6 +50,7 @@ namespace __gnu_debug
class _Safe_iterator_base
{
friend class _Safe_sequence_base;
+template friend class _Safe_sequence;
public:
/** The sequence this iterator references; may be NULL to indicate
.
I am not a great fan of friend class. As long as it was friend
declaration between iterator and sequence base types it was ok. Now that
we need to make a template class friend I consider that it is too much
friendship and prefer to make the necessary method public.
But if you think otherwise just tell me and I will use your approach.
François
Re: Patch ping
On 09/28/2016 09:24 PM, Jakub Jelinek wrote: I'd like to ping the http://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html patch, containing various fixes for gimple-ssa-sprintf.c. If the 0 < var to var > 0 changes are deemed too controversial, I can separate them from the other changes. I'd like to see them separated because they are obvious and good, so please install them first. Bernd
Patch ping
Hi! I'd like to ping the http://gcc.gnu.org/ml/gcc-patches/2016-09/msg01436.html patch, containing various fixes for gimple-ssa-sprintf.c. If the 0 < var to var > 0 changes are deemed too controversial, I can separate them from the other changes. Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On 09/28/2016 02:15 PM, Michael Matz wrote: P.S.: Initially I even wanted to argue that the mere existence of _any_ comment before a case label would disable the warning. I don't have the numbers but I bet even that version would have found the very same bugs that the picky version has. Sounds like a pretty good idea to me for a default setting. If we really want to have multiple levels of the warning. I agree that it's likely to find the majority of problems, and it no longer depends on language and spelling of the comment. Bernd
[PATCH] Backport of 25 fortran patches
The attached patch and ChangeLog entries are for the backporting of 25 patches from trunk to the 6-branch. The bugzilla PR's contained in the patch are fortran/41922 fortran/60774 fortran/61318 fortran/68566 fortran/69514 fortran/69867 fortran/69962 fortran/70006 fortran/71067 fortran/71730 fortran/71799 fortran/71859 fortran/71862 fortran/77260 fortran/77351 fortran/77372 fortran/77380 fortran/77391 fortran/77420 fortran/77429 fortran/77460 fortran/77506 fortran/77507 fortran/77612 fortran/77694 The patch has been bootstrapped and regression tested on x86_64-*-freebsd. Ok to commit? -- Steve 2016-09-28 Steven G. Kargl PR fortran/41922 * target-memory.c (expr_to_char): Pass in locus and use it in error messages. (gfc_merge_initializers): Ditto. * target-memory.h: Update prototype for gfc_merge_initializers (). * trans-common.c (get_init_field): Use the correct locus. PR fortran/60774 * parse.c (next_free,next_fixed): Issue error for statement label without a statement. PR fortran/61318 * interface.c (compare_parameter): Use better locus for error message. PR fortran/68566 * check.c (gfc_check_reshape): Check for constant expression. PR fortran/69514 * array.c (gfc_match_array_constructor): If type-spec is present, walk the array constructor performing possible conversions for numeric types. PR fortran/69867 * decl.c (build_struct): Ensure that pointers point to something. PR fortran/69962 * decl.c (gfc_set_constant_character_len): if expr is not constant issue an error instead of an ICE. PR fortran/70006 * io.c (gfc_resolve_dt): Use correct locus. * resolve.c (resolve_branch): Ditto. PR fortran/71067 * decl.c (match_data_constant): On error, set 'result' to NULL. PR fortran/71730 * decl.c (char_len_param_value): Check return value of gfc_reduce_init_expr(). PR fortran/71799 * resolve.c(gfc_resolve_iterator): Failure of type conversion need not ICE. PR fortran/71859 * check.c(numeric_check): Prevent ICE. Issue error for invalid subroutine as an actual argument when numeric argument is expected. PR fortran/71862 * class.c: Remove assert. Iterate over component only if non-null. PR fortran/77260 * gcc/fortran/trans-decl.c (generate_local_decl): Suppress warning for unused variable if symbol is entry point. PR fortran/77351 * frontend-passes.c (remove_trim,combine_array_constructor): Check for NULL pointer. PR fortran/77372 simplify.c (simplify_ieee_selected_real_kind): Check for NULL pointers. PR fortran/77380 * dependency.c (gfc_check_dependency): Do not assert with -fcoarray=lib. PR fortran/77391 * resolve.c (deferred_requirements): New function to check F2008:C402. (resolve_fl_variable,resolve_fl_parameter): Use it. PR fortran/77420 * trans-common.c: Handle array elements in equivalence when the lower and upper bounds of array spec are NULL. PR fortran/77429 * dependency.c (gfc_check_dependency): Convert gcc_assert() to a conditional and possible call to gfc_internal_error(). PR fortran/77460 * simplify.c (simplify_transformation_to_scalar): On error, result may be NULL, simply return. PR fortran/77506 * array.c (gfc_match_array_constructor): CHARACTER(len=*) cannot appear in an array constructor. PR fortran/77507 * intrinsic.c (add_functions): Use correct keyword. PR fortran/77612 * decl.c (char_len_param_value): Check parent namespace for seen_implicit_none. PR fortran/77694 * frontend-passes.c (optimize_binop_array_assignment): Check pointer for NULL. 2016-09-28 Steven G. Kargl PR fortran/77507 * ieee/ieee_arithmetic.F90 (IEEE_VALUE_4,IEEE_VALUE_8,IEEE_VALULE_10, IEEE_VALUE_16): Use correct keyword. 2016-09-28 Steven G. Kargl PR fortran/41922 * gfortran.dg/equiv_constraint_5.f90: Adjust the error message. * gfortran.dg/equiv_constraint_7.f90: Ditto. * gfortran.dg/pr41922.f90: New test. PR fortran/60774 * gfortran.dg/empty_label.f: Adjust test for new error message. * gfortran.dg/empty_label.f90: Ditto. * gfortran.dg/empty_label_typedecl.f90: Ditto. * gfortran.dg/label_3.f90: Deleted (redundant with empty_label.f90). * gfortran.dg/warnings_are_errors_1.f90: Remove invalid statement label. PR fortran/61318 * gfortran.dg/pr61318.f90: New test. PR fortran/68566 * gfortran.dg/pr68566.f90: new test. PR fortran/69514 * gfortra
Re: Implement -Wimplicit-fallthrough (version 9)
On Wed, Sep 28, 2016 at 09:29:01AM -0600, Tom Tromey wrote:
> > "Michael" == Michael Matz writes:
>
> Michael> Not accepting
> Michael> /* And here we intentionally fall through because ... */
> Michael> and forcing users to replace this by:
> Michael> /* fallthrough */
> Michael> is not robust either. It's actually actively lowering robustness of
> code,
> Michael> it creates work for programmers that will be regarded as pointless
> (and
> Michael> rightly so) and will merely lead to everybody disabling the warning
> (see
> Michael> our generated files)
>
> We can't control what programmers might do. My point is that accepting
> too much is actively bad -- it hides errors. If this somehow makes some
> programmer fall down a slippery slope, well, that's their error, not
> gcc's.
>
> TBH I think it would be better not to parse comments at all. Heuristics
> are generally bad and this case and ensuing discussion is a great
> demonstration of that.
>
> The other day I built gdb with -Wimplicit-fallthrough. I was surprised
> to find that gcc rejected this:
>
> default:
> {
> complaint (&symfile_complaints,
> _("Storage class %d not recognized during scan"),
> sclass);
> }
> /* FALLTHROUGH */
>
> /* C_FCN is .bf and .ef symbols. I think it is sufficient
>to handle only the C_FUN and C_EXT. */
> case C_FCN:
>
> Presumably without the comment heuristic, this would be accepted.
Is complaint a noreturn call? If not, then it would certainly warn, unless
there is [[fallthrough]]; or __attribute__((fallthrough)); etc. (or the
comment). For the comment, /* FALLTHROUGH */ is the recognized spelling of
the comment, but right now we only look for such comments immediately before
a case/default keyword or user label; if there is another comment in
between, it is ignored. This is something we are considering to change,
exactly because often the /* FALLTHRU */ comment appears after some case and
then there is unrelated comment before the next case about what that case
handles.
Jakub
Re: [PATCH] objc: update documetation and add test-case of constructor/destructor attr.
On Aug 10, 2016, at 2:11 AM, Martin Liška wrote: > > Following patch clarifies usage of ctor and dtor attributes for Objective C. > Patch survives (on x86_64-linux-gnu): > > make -k check-objc RUNTESTFLAGS="execute.exp" > > Ready for trunk? Ok.
Re: Change license of filenames.h to LGPL
On Sep 27, 2016, Ozkan Sezer wrote: > FYI: What I originally wanted was an authorization _for me_ to use > filenames.h in LGPL projects with LGPL license notice; the version > I use is modified (not refer to any external code other than libc, > i.e. only macros and inlines) and doesn't include hashtab.h either; > therefore I believe that my request is fulfilled and is not subject > to the concerns raised by you guys. It would probably be wise for you to amend the modified copy you'll distribute with the changes proposd by Eli, and to add a link to this thread in the archives, should anyone be surprised by the different license. As for the copy in GCC, that has additional code, we can then keep it under the stronger copyleft defenses. Does that work for everyone involved? -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH, Fortran] Extension: COTAN and degree-valued trig intrinsics with -fdec-math
On Mon, Sep 26, 2016 at 10:01:27AM -0400, Fritz Reese wrote: > > Attached is a patch extending the GNU Fortran front-end to support > some additional math intrinsics, enabled with a new compile flag > -fdec-math. The flag adds the COTAN intrinsic (cotangent), as well as > degree versions of all trigonometric intrinsics (SIND, TAND, ACOSD, > etc...). This extension allows for further compatibility with legacy > code that depends on the compiler to support such intrinsic functions. > I plan to review this patch over the weekend. Two things to consider. 1) The documentation should note that these intrinsics are for compatibility with legacy code and should strongly discourage their use in new code. 2) In regards to Joseph and Tobias' comments, the documentation should give a hint to the quality of implementation. Argument reduction can be a real pain and without a formal numerical analysis, I can imagine large ULP errors near zeros and infinities. I haven't looked at the implementation yet, but will suggest that REAL(4) should probably be simply written in terms of REAL(8), e.g., function sind(x) result(retval) real(4) retval real(4), intent(in) :: x retval = dsind(real(x, 8)) end function sind Yes, the layer of indirection and computations in REAL(8) will be slower, but you should have much improved accuracy. -- Steve
Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
On 09/24/2016 10:39 AM, Andreas Schwab wrote: I'm still seeing these failures on m68k: FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 358) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1222) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1272) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for warnings, line 1383) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors) Excess errors: /daten/aranym/gcc/gcc-20160924/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c:1408:3: warning: specified size 4294967295 exceeds the size 2 of the destination object [-Wformat-length=] I'm pretty sure the first two are due to the same problem as bug 77735. I plan to look into fixing it today or later this week. The last two were caused by bug v77762 and should be fixed by now. Martin
Re: C++ PATCH for c++/68703 (dependent vector length)
Hi, Jason This patch added testcases g++.dg/ext/vector32.C and vector32a.C, but there is no gcc/testsuite/ChangeLog entry. The testcases are failing on AIX with a strange ICE: src/gcc/testsuite/g++.dg/ext/vector32.C: In function 'int main()': src/gcc/testsuite/g++.dg/ext/vector32.C:18:1: internal compiler error: in get, at cgraph.h:395 Do you want me to open a new PR for this? Thanks, David
[PATCH, i386 libgcc]: Remove obsolete workaround for PR 44174
Hello!
Attached patch removes obsolete register allocator workaround from
gcc-4.6 era. The problem was fixed in gcc-4.7+.
2015-09-28 Uros Bizjak
* config/i386/cpuinfo.c (__get_cpuid_output): Remove.
(__cpu_indicator_init): Call __get_cpuid, not __get_cpuid_output.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
Index: config/i386/cpuinfo.c
===
--- config/i386/cpuinfo.c (revision 240579)
+++ config/i386/cpuinfo.c (working copy)
@@ -381,20 +381,6 @@ get_available_features (unsigned int ecx, unsigned
__cpu_model.__cpu_features[0] = features;
}
-/* A noinline function calling __get_cpuid. Having many calls to
- cpuid in one function in 32-bit mode causes GCC to complain:
- "can't find a register in class CLOBBERED_REGS". This is
- related to PR rtl-optimization 44174. */
-
-static int __attribute__ ((noinline))
-__get_cpuid_output (unsigned int __level,
- unsigned int *__eax, unsigned int *__ebx,
- unsigned int *__ecx, unsigned int *__edx)
-{
- return __get_cpuid (__level, __eax, __ebx, __ecx, __edx);
-}
-
-
/* A constructor function that is sets __cpu_model and __cpu_features with
the right values. This needs to run only once. This constructor is
given the highest priority and it should run before constructors without
@@ -406,7 +392,7 @@ __cpu_indicator_init (void)
{
unsigned int eax, ebx, ecx, edx;
- int max_level = 5;
+ int max_level;
unsigned int vendor;
unsigned int model, family, brand_id;
unsigned int extended_model, extended_family;
@@ -416,7 +402,7 @@ __cpu_indicator_init (void)
return 0;
/* Assume cpuid insn present. Run in level 0 to get vendor id. */
- if (!__get_cpuid_output (0, &eax, &ebx, &ecx, &edx))
+ if (!__get_cpuid (0, &eax, &ebx, &ecx, &edx))
{
__cpu_model.__cpu_vendor = VENDOR_OTHER;
return -1;
@@ -431,7 +417,7 @@ __cpu_indicator_init (void)
return -1;
}
- if (!__get_cpuid_output (1, &eax, &ebx, &ecx, &edx))
+ if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
{
__cpu_model.__cpu_vendor = VENDOR_OTHER;
return -1;
Re: [patch, libgfortran] PR77707 formatted direct access: nextrec off by one
On Tue, Sep 27, 2016 at 10:15:40PM -0700, Jerry DeLisle wrote: > I plan to commit the attached patch in the next few days. Fairly simple. > > Regression tested on x86-64. > Looks ok to me. -- Steve
[PATCH] include/std/chrono (system_clock): Fix typo in comment.
Committed to trunk. commit 6246cb3120548a1dfc62bf7aa538f3e5538123c9 Author: Jonathan Wakely Date: Wed Sep 28 19:03:43 2016 +0100 * include/std/chrono (system_clock): Fix typo in comment. diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono index f29d8e1..11e7fa2 100644 --- a/libstdc++-v3/include/std/chrono +++ b/libstdc++-v3/include/std/chrono @@ -784,7 +784,7 @@ _GLIBCXX_END_NAMESPACE_VERSION // Clocks. // Why nanosecond resolution as the default? -// Why have std::system_clock always count in the higest +// Why have std::system_clock always count in the highest // resolution (ie nanoseconds), even if on some OSes the low 3 // or 9 decimal digits will be always zero? This allows later // implementations to change the system_clock::now()
libgo patch committed: Pass -fcompiling-runtime when testing the runtime package
My earlier patch to inline runtime.getcallerpc and runtime.getcallersp when compiling the runtime package was incomplete, because it only took effect when passing the -fcompiling-runtime option, and that option is not passed when testing the runtime package. This patch by Than McIntosh fixes that by passing the option. 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 240560) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -4046a883070c1f5f58de336f7378f3bca69ea2b6 +c79a35411c1065c71add196fdeca6e5207a79248 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/Makefile.am === --- libgo/Makefile.am (revision 240560) +++ libgo/Makefile.am (working copy) @@ -1291,6 +1291,7 @@ runtime.inc: s-runtime-inc; @true s-runtime-inc: runtime-go.lo $(SHELL) $(srcdir)/mvifdiff.sh runtime.inc.tmp runtime.inc $(STAMP) $@ +runtime_check_GOCFLAGS = -fgo-compiling-runtime runtime/check: $(CHECK_DEPS) @$(CHECK) .PHONY: runtime/check Index: libgo/Makefile.in === --- libgo/Makefile.in (revision 240560) +++ libgo/Makefile.in (working copy) @@ -1249,6 +1249,7 @@ CHECK_DEPS = $(toolexeclibgo_DATA) $(too @LIBGO_IS_SOLARIS_FALSE@matchargs_os = extra_go_files_runtime = runtime_sysinfo.go runtime_go_lo_GOCFLAGS = -fgo-c-header=runtime.inc.tmp -fgo-compiling-runtime +runtime_check_GOCFLAGS = -fgo-compiling-runtime @LIBGO_IS_BSD_TRUE@golang_org_x_net_route_lo = \ @LIBGO_IS_BSD_TRUE@golang_org/x/net/route/route.lo
[PATCH] Check for overflow in filesystem::last_write_time
A system_clock::time_point can't be used to represent all valid values
of a 64-bit time_t because nearly half the bits are used up for the
nanoseconds part. This makes filesystem::last_write_time() check the
time_t values and report an error if it can't be converted to the
time_point type. This means we don't get undefined behaviour for files
with mtime after the 24th century :-)
Thanks to Eric Fiselier for point it out.
* include/experimental/bits/fs_fwd.h (file_time_type): Simplify
definition.
* src/filesystem/ops.cc (file_time): Take error_code parameter and
check for overflow.
(do_copy_file, last_write_time): Pass error_code in file_time calls.
* testsuite/experimental/filesystem/operations/last_write_time.cc:
New.
* testsuite/util/testsuite_fs.h (scoped_file): Define RAII helper.
Tested x86_64-linux, committed to trunk. Backports for this (and some
other filesystem changes) to follow soon.
commit 3a59a5aabe310ca9e659dcf73829b3f89454f3ab
Author: Jonathan Wakely
Date: Wed Sep 28 13:51:09 2016 +0100
Check for overflow in filesystem::last_write_time
* include/experimental/bits/fs_fwd.h (file_time_type): Simplify
definition.
* src/filesystem/ops.cc (file_time): Take error_code parameter and
check for overflow.
(do_copy_file, last_write_time): Pass error_code in file_time calls.
* testsuite/experimental/filesystem/operations/last_write_time.cc:
New.
* testsuite/util/testsuite_fs.h (scoped_file): Define RAII helper.
diff --git a/libstdc++-v3/include/experimental/bits/fs_fwd.h
b/libstdc++-v3/include/experimental/bits/fs_fwd.h
index 57aa4d3..b9cc041 100644
--- a/libstdc++-v3/include/experimental/bits/fs_fwd.h
+++ b/libstdc++-v3/include/experimental/bits/fs_fwd.h
@@ -253,7 +253,7 @@ _GLIBCXX_END_NAMESPACE_CXX11
operator^=(directory_options& __x, directory_options __y) noexcept
{ return __x = __x ^ __y; }
- typedef chrono::time_point file_time_type;
+ using file_time_type = std::chrono::system_clock::time_point;
// operational functions
diff --git a/libstdc++-v3/src/filesystem/ops.cc
b/libstdc++-v3/src/filesystem/ops.cc
index 0ecb8b9..659cfbb 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -288,16 +288,24 @@ namespace
}
inline fs::file_time_type
- file_time(const stat_type& st) noexcept
+ file_time(const stat_type& st, std::error_code& ec) noexcept
{
using namespace std::chrono;
-return fs::file_time_type{
#ifdef _GLIBCXX_USE_ST_MTIM
- seconds{st.st_mtim.tv_sec} + nanoseconds{st.st_mtim.tv_nsec}
+time_t s = st.st_mtim.tv_sec;
+nanoseconds ns{st.st_mtim.tv_nsec};
#else
- seconds{st.st_mtime}
+time_t s = st.st_mtime;
+nanoseconds ns{};
#endif
-};
+
+if (s >= (nanoseconds::max().count() / 1e9))
+ {
+ ec = std::make_error_code(std::errc::value_too_large); // EOVERFLOW
+ return fs::file_time_type::min();
+ }
+ec.clear();
+return fs::file_time_type{seconds{s} + ns};
}
// Returns true if the file descriptor was successfully closed,
@@ -373,11 +381,11 @@ namespace
}
else if (is_set(option, opts::update_existing))
{
- if (file_time(*from_st) <= file_time(*to_st))
- {
- ec.clear();
- return false;
- }
+ const auto from_mtime = file_time(*from_st, ec);
+ if (ec)
+ return false;
+ if ((from_mtime <= file_time(*to_st, ec)) || ec)
+ return false;
}
else if (!is_set(option, opts::overwrite_existing))
{
@@ -1036,7 +1044,7 @@ fs::last_write_time(const path& p)
fs::file_time_type
fs::last_write_time(const path& p, error_code& ec) noexcept
{
- return do_stat(p, ec, [](const auto& st) { return file_time(st); },
+ return do_stat(p, ec, [&ec](const auto& st) { return file_time(st, ec); },
file_time_type::min());
}
diff --git
a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
new file mode 100644
index 000..b1aea20
--- /dev/null
+++
b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
@@ -0,0 +1,111 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// Yo
Re: [PATCH v3] Optimize strchr to strlen
On 28 September 2016 at 16:45, Jason Merrill wrote: > I think this broke g++.dg/ext/builtin10.C. > > Jason It also broke: gc gcc.c-torture/execute/builtins/strchr.c execution, -O1 c.c-torture/execute/builtins/strchr.c (execution) on arm* and aarch64* Christophe
Re: [Patch] Remove all uses of TARGET_FLT_EVAL_METHOD_NON_DEFAULT and poison it
On Wed, 28 Sep 2016, James Greenhalgh wrote: > On Thu, Sep 22, 2016 at 05:55:21PM +, Joseph Myers wrote: > > On Thu, 22 Sep 2016, James Greenhalgh wrote: > > > > > The relaxation isn't portable, and keeping it in place is tricky, so this > > > patch removes it, and poisons TARGET_FLT_EVAL_METHOD_NON_DEFAULT in > > > system.h to prevent future use. > > > > > > Bootstrapped and tested on x86_64 with > > > --enable-languages=all,ada,go,obj-c++ > > > with no issues. > > > > > > OK? > > > > OK. > > Hi Joseph, > > Is this OK sufficient for me to commit the patch, or must I wait for input > from the other CCed front-end maintainers? It is sufficient for you to commit the patch. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Fix PR55152
On Wed, 28 Sep 2016, Richard Biener wrote:
> Index: gcc/testsuite/gcc.dg/pr55152.c
> ===
> --- gcc/testsuite/gcc.dg/pr55152.c(revision 0)
> +++ gcc/testsuite/gcc.dg/pr55152.c(working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -ffinite-math-only -fstrict-overflow
> -fdump-tree-optimized" } */
> +
> +double g (double a)
> +{
> + return (a>=-a)?a:-a;
You should need -fno-signed-zeros for this (that is, for the
transformation to MAX_EXPR), not -ffinite-math-only. For a == -0, that
function should return -0, but abs would produce +0.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o
On Wed, 28 Sep 2016, Alan Modra wrote: > > I'd expect libraries such as libstdc++ and libgcc (generally, all compiler > > and libc libraries) to be set up in such a way that they will work with > > all long double choices in user code (via mangling and headers mapping > > access to long double library functions to the right versions for the > > chosen type) - and so need to be compiled without these attribute tags to > > avoid the linker complaining when someone links them with user code built > > with a non-default choice of long double. Certainly for glibc I'd think > > using the option globally to build everything is the right choice (well, > > except for libnldbl.a, where -mlong-double-64 attributes are logically > > correct). > > Yes, and this is why the linker only warns rather than errors on > mismatching .gnu.attributes tags. But for a library that is aware of long double variants, it shouldn't even warn. And given that we don't build multiple copies of GCC's libraries, they should be aware of the variants (via mangling them differently, ensuring versions of the relevant functions for each long double type are present, etc.) and so using them should not result in warnings. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH][v4] GIMPLE store merging pass
On 09/28/2016 10:54 AM, Kyrill Tkachov wrote: > +fstore-merging > +Common Var(flag_store_merging) Optimization > +Use the tree store merging pass. > + Did you purposely leave off "Report" for this option? I noticed the option didn't show up in the "options enabled:" section of the .s file when -fverbose-asm is specified. -Pat
Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos
On 09/28/2016 06:36 PM, Jeff Law wrote: A "p" prefix for pseudos might still be a good idea, but there's still the issue of a real "p0" register name causing confusion. So how do you think we should deal with distinguishing between the different registers that may appear in a dump file? I think the main problem we were trying to solve is making sure we can make future-proof dumps. So that would argue for allowing h0, v0, p0 syntax in the reader, but not printing them out that way by default. Also, if we don't already, allow hard regs to be specified by name in the reader, and maybe even require it for virtuals. Bernd
Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos
On 09/28/2016 10:30 AM, Bernd Schmidt wrote: On 09/28/2016 06:23 PM, Jeff Law wrote: (reg/i:SI h0 ax) (reg/i:SF h21 xmm0) (Replying to myself, in the hope of better demonstrating the idea) The following patch implements this idea for RTL dumps, so that all REGNO values in dumps get a one character prefix: 'h' for hard registers, 'v' for virtual registers, and 'p' for non-virtual pseudos (making it easier for both humans and parsers to grok the meaning of a REGNO). I think you nailed it. h, v & p prefixing for each of the register types, but leaving the actual register number as-is in the dump file. I'm actually no longer quite so sure this buys us much: a port might have an actual register named "h0", leading to confusion. Virtual and hard registers also already have their real name printed after the number. A "p" prefix for pseudos might still be a good idea, but there's still the issue of a real "p0" register name causing confusion. So how do you think we should deal with distinguishing between the different registers that may appear in a dump file? The case I'm worried about is the register meanings in a testsuite dump file changing over time if/when new hard registers are added to the port or we introduce new virtual registers. FOr hard regs and virtuals we can probably map backwards using their names. So given a register in a dump, if we can't reverse map it back to a hard reg or a virtual, then we assume its a pseudo? jeff
Re: [PATCH][RTL ifcvt] Transform (X == CST) ? -CST : Y into (X == CST) ? -X : Y when conditional negation is available
Hi Jeff,
On 28/09/16 17:07, Jeff Law wrote:
On 09/28/2016 02:48 AM, Kyrill Tkachov wrote:
Hi all,
This patch tries to avoid materialising an immediate
when comparison has shown that it is already loaded.
This is performed during ifcvt and only when the target has the
conditional negate
or inverse optabs, which for now is only aarch64.
Thus for the code:
int
foo (int a, int b)
{
return a == 5 ? -5 : b;
}
we currently emit on aarch64:
foo:
mov w2, -5
cmp w0, 5
cselw0, w1, w2, ne
ret
but with this patch we emit:
foo:
cmp w0, 5
csneg w0, w1, w0, ne
ret
Bootstrapped and tested on arm, aarch64, x86_64.
Ok for trunk?
Thanks,
Kyrill
P.S. The simpler form of the transformation: X == CST ? CST : Y --> X ==
CST ? X : Y
could also be beneficial IMO but I found that it can cause bad codegen
in combine
due to extending the live range of X. I'm still working on a way to work
around that,
but that is a separate piece of work anyway.
FWIW, the simpler form of the transformation is already done just prior to
leaving SSA form in tree-ssa-uncprop.c. So there may not be a ton of
opportunities for the simpler form in the RTL optimizers.
Indeed there weren't that many places, but they were some. They helped in
avoiding materialising the floating point 0.0
in particular in some cases.
My only concern here is this transformation isn't significantly different than
the simpler form, which is apparently causing some kind of combine problem.
I'd like to understand the combine problem better before giving final approval
to this patch.
Sure, I'm attaching the ifcvt patch that does the aforementioned simple
transform.
The testcase it regresses is:
int
foo (float a, float b, float c, int d)
{
return a > 5 && a < 10 ? 6 : 0;
}
As far as I got is:
The relevant pre-combine (aarch64) RTL without the patch is:
(insn 21 19 45 2 (set (reg:SI 87)
(zero_extend:SI (subreg:QI (reg:SI 86) 0))) fbad.c:4 90
{*zero_extendqisi2_aarch64}
(expr_list:REG_DEAD (reg:SI 86)
(nil)))
(insn 47 45 48 2 (set (reg:CC 66 cc)
(compare:CC (reg:SI 87)
(const_int 0 [0]))) fbad.c:4 392 {cmpsi}
(expr_list:REG_DEAD (reg:SI 87)
(nil)))
(insn 48 47 30 2 (set (reg:SI 76 [ ])
(if_then_else:SI (ne (reg:CC 66 cc)
(const_int 0 [0]))
(reg:SI 89)
(const_int 0 [0]))) fbad.c:4 444 {*cmovsi_insn}
(expr_list:REG_DEAD (reg:SI 89)
(expr_list:REG_DEAD (reg:CC 66 cc)
(nil
and with the patch it is:
(insn 21 19 45 2 (set (reg:SI 87)
(zero_extend:SI (subreg:QI (reg:SI 86) 0))) fbad.c:4 90
{*zero_extendqisi2_aarch64}
(expr_list:REG_DEAD (reg:SI 86)
(nil)))
(insn 46 45 47 2 (set (reg:CC 66 cc)
(compare:CC (reg:SI 87)
(const_int 0 [0]))) fbad.c:4 392 {cmpsi}
(nil))
(insn 47 46 30 2 (set (reg:SI 76 [ ])
(if_then_else:SI (eq (reg:CC 66 cc)
(const_int 0 [0]))
(reg:SI 87)
(reg:SI 89))) fbad.c:4 444 {*cmovsi_insn}
(expr_list:REG_DEAD (reg:SI 89)
(expr_list:REG_DEAD (reg:SI 87)
(expr_list:REG_DEAD (reg:CC 66 cc)
(nil)
Combine is trying to combine 21 -> 47 in the first case (21 -> 46 in the
second) and without the
transformation ends up looking into the uses of CC as well, the IF_THEN_ELSE in
insn 48.
But I think it only does that if reg 87 is dead in insn 47.
In the end what happens is it fails to merge the comparison and the use of the
comparison
and ends up emitting extra code.
So before:
fmovs2, 5.0e+0
fmovs1, 1.0e+1
movw0, 6
fcmpes0, s2
fccmpes0, s1, 0, gt
cselw0, w0, wzr, mi //conditional select
ret
after:
foo:
fmovs2, 5.0e+0
fmovs1, 1.0e+1
movw1, 6
fcmpes0, s2
fccmpes0, s1, 0, gt
csetw0, mi // conditional set
cmpw0, 0
cselw0, w0, w1, eq
ret
I think it has to do with the logic to set and use added_sets_2 in try_combine
but I got lost trying to debug it, so I sort of attributed it to "extending
live ranges
of registers is bad"
Kyrill
Jeff
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 24542f008485e6c28e068030fa301f2ce040efc1..18cda590f3c17d0f7a48ba3e3f09bd715f60891b 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1810,13 +1810,38 @@ noce_try_cmove (struct noce_if_info *if_info)
&& (CONSTANT_P (if_info->b) || register_operand (if_info->b, VOIDmode)))
{
start_sequence ();
+ rtx cond = if_info->cond;
- code = GET_CODE (if_info->cond);
- target = noce_emit_cmove (if_info, if_info->x, code,
-XEXP (if_info->cond, 0),
-XEXP (if_info->cond, 1),
-if_info->a, if_info->b);
+ code = GET_CODE (cond);
+
+ rtx a = if_info->a;
+ rtx b = if_info->b;
+ /* Transform x == CST ? CST : y into x == CST ? x : y to avoid
+ materializing CST for the c
Re: [PATCH 4/5] shrink-wrap: Shrink-wrapping for separate components
On 09/28/2016 03:04 AM, Segher Boessenkool wrote:
+static void
+place_prologue_for_one_component (unsigned int which, basic_block head)
+{
+ /* The block we are currently dealing with. */
+ basic_block bb = head;
+ /* Is this the first time we visit this block, i.e. have we just gone
+ down the tree. */
+ bool first_visit = true;
+
+ /* Walk the dominator tree, visit one block per iteration of this loop.
+ Each basic block is visited twice: once before visiting any children
+ of the block, and once after visiting all of them (leaf nodes are
+ visited only once). As an optimization, we do not visit subtrees
+ that can no longer influence the prologue placement. */
+ for (;;)
Is there some reason you wrote this as a loop rather than recursion?
IMHO it makes this function (and spread_components) more difficult to
reason about than it needs to be.
It would recurse a few thousand frames deep on not terribly big testcases
(i.e. I've seen it happen). This uses a lot of stack space on some ABIs,
and is very slow as well (this is the slowest function here by far).
Unlimited recursion is bad.
I'm surprised the recursion was that deep. Such is life. Thanks for
clarifying. I won't object to the iterative version. :-)
+/* Place code for prologues and epilogues for COMPONENTS where we can put
+ that code at the end of basic blocks. */
+static void
+emit_common_tails_for_components (sbitmap components)
[ Snip. ]
+
+ /* Put the code at the end of the BB, but before any final jump. */
+ if (!bitmap_empty_p (epi))
So what if the final jump uses hard registers in one way or another? I
don't immediately see anything that verifies it is safe to transpose the
epilogue and the final jump.
Whoops. Thanks for catching this.
I missed it the first time though the code too.
Conceptually want the epilogue to occur on the outgoing edge(s). But
you want to actually transpose the epilogue and the control flow insn so
that you can insert the epilogue in one place.
The same problem happens with prologues, too.
Yea. I guess if a had a jump with an embedded side effect (such as movb
or addb on the PA), then transposing the control flow insn with the
prologue would be problematical as well.
A cc0 target can not use separate shrink-wrapping *anyway* if any of the
components would clobber cc0, so that is no problem here.
True, but I'd be more comfortable if we filtered out cc0 targets explicitly.
I think you need to handle the former more cleanly. The latter I'd
be comfortable filtering out in try_shrink_wrapping_separate.
I'm thinking to not do the common tail optimisation if BB_END is a
JUMP_INSN but not simplejump_p (or a return or a sibling call). Do
you see any problem with that?
Seems reasonable.
Jeff
Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos
On 09/21/2016 01:01 PM, David Malcolm wrote: Presumably we could use "v" rather than "p" as the prefix for the first 5 pseudos (up to LAST_VIRTUAL_REGISTER), doing any adjustment at load time, rather than at dump time. So the above example would look like: (reg/f:DI v82 virtual-stack-vars) i.e. the 82 for x86_64's virtual-stack-vars would be prefixed with a 'v', and the loader would adjust it to be the current target's value for VIRTUAL_STACK_VARS_REGNUM. Do you like the idea of prefixing regnums of hardregs with 'h'? (so that all regnos get a one-char prefix) e.g. (reg/i:SI h0 ax) (reg/i:SF h21 xmm0) (Replying to myself, in the hope of better demonstrating the idea) The following patch implements this idea for RTL dumps, so that all REGNO values in dumps get a one character prefix: 'h' for hard registers, 'v' for virtual registers, and 'p' for non-virtual pseudos (making it easier for both humans and parsers to grok the meaning of a REGNO). I think you nailed it. h, v & p prefixing for each of the register types, but leaving the actual register number as-is in the dump file. Successfully bootstrapped on x86_64-pc-linux-gnu. There are various regression test failures involving scan-rtl-dump due to regexes not matching e.g. PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set \\(reg/i:TI 0 ax\\)" PASS -> FAIL : gcc.target/i386/pr20020-1.c scan-rtl-dump expand "\\(set \\(reg:TI [0-9]* \\[ \\]\\)" If the approach is OK, I can do an updated patch that also fixes up the relevant tests (adding the appropriate prefixes); this would touch multiple targets. Yea. This obviously highlights some of the long term issues with making changes into the dump format. As I said in our meeting yesterday, I do understand the desire to nail down the format :-) But I also want to use the opportunity we have to make the dumps easier for your work to read & interpret. jeff
Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos
On 09/28/2016 06:23 PM, Jeff Law wrote: (reg/i:SI h0 ax) (reg/i:SF h21 xmm0) (Replying to myself, in the hope of better demonstrating the idea) The following patch implements this idea for RTL dumps, so that all REGNO values in dumps get a one character prefix: 'h' for hard registers, 'v' for virtual registers, and 'p' for non-virtual pseudos (making it easier for both humans and parsers to grok the meaning of a REGNO). I think you nailed it. h, v & p prefixing for each of the register types, but leaving the actual register number as-is in the dump file. I'm actually no longer quite so sure this buys us much: a port might have an actual register named "h0", leading to confusion. Virtual and hard registers also already have their real name printed after the number. A "p" prefix for pseudos might still be a good idea, but there's still the issue of a real "p0" register name causing confusion. Bernd
Re: [PATCH] Introduce selftest::locate_file
On 09/21/2016 06:59 PM, David Malcolm wrote:
On Mon, 2016-09-19 at 11:31 -0600, Jeff Law wrote:
On 09/16/2016 03:19 PM, David Malcolm wrote:
When possible I don't think we want the tests to be target
specific.
Hmm, I'm probably about to argue for Bernd's work... The 71779
testcase
is a great example -- it uses high/lo_sum. Not all targets
support
that
-- as long as we don't try to recognize those insns we're likely
OK,
but
that seems fragile long term. Having an idealized target means
we
can
ignore much of these issues.
An alternative would be to pick a specific target for each test.
It's an alternative, but not one I particularly like since those
tests
won't be consistently run. With an abstracted target like Bernd
suggests we ought to be able to make most tests work with the
abstracted
target and minimize the number of truely target specific tests.
So I'm real curious, what happens if you run this RTL selftest
under
valgrind? I have the sneaking suspicion that we'll start doing
some
uninitialized memory reads.
I'm seeing various leaks (an htab within linemap_init for all of
the
line_table fixtures), but no uninitialized reads.
Wow. I must say I'm surprised. Good news though.
+
+ /* Dump taken from comment 2 of PR 71779, of
+ "...the relevant memory access coming out of expand"
+ with basic block IDs added, and prev/next insns set to
+ 0 at ends. */
+ const char *input_dump
+= (";; MEM[(struct isl_obj *)&obj1] =
&isl_obj_map_vtable;\n"
+ "(insn 1045 0 1046 2 (set (reg:SI 480)\n"
+ "(high:SI (symbol_ref:SI
(\"isl_obj_map_vtable\")
[flags 0xc0] )))
y.c:12702 -1\n"
+ " (nil))\n"
+ "(insn 1046 1045 1047 2 (set (reg/f:SI 479)\n"
+ "(lo_sum:SI (reg:SI 480)\n"
+ "(symbol_ref:SI (\"isl_obj_map_vtable\")
[flags
0xc0] ))) y.c:12702
-1\n"
+ " (expr_list:REG_EQUAL (symbol_ref:SI
(\"isl_obj_map_vtable\") [flags 0xc0] )\n"
+ "(nil)))\n"
+ "(insn 1047 1046 1048 2 (set (reg:DI 481)\n"
+ "(subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1\n"
+ " (nil))\n"
+ "(insn 1048 1047 1049 2 (set (zero_extract:DI (reg/v:DI
191
[ obj1D.17368 ])\n"
+ "(const_int 32 [0x20])\n"
+ "(const_int 0 [0]))\n"
+ "(reg:DI 481)) y.c:12702 -1\n"
+ " (nil))\n"
So looking at this just makes my head hurt. Escaping, lots of
quotes,
unnecessary things in the dump, etc. The question I would have
is
why
not have a file with the dump and read the file?
(nods)
Seems like I need to add a mechanism for telling the selftests
which
directory to load the tests relative to.
What about putting them inside the appropriate gcc.target
directories?
We could have one for the "generic" target assuming we build
something
like that, the others could live in their target specific directory.
jeff
Having selftests that read RTL dumps load them from files rather than
embedding them as strings in the source requires a way to locate the
relevant files.
This patch adds a selftest::locate_file function for locating such
files, relative to "$(SRCDIR)/gcc/testsuite/selftests". This is
done via a new argument to -fself-test, which supplies the current
value of "$(SRCDIR)/gcc" to cc1.
I chose "$(SRCDIR)/gcc/testsuite/selftests", so as to be below
gcc/testsuite, but not below any of the existing DejaGnu subdirectories,
to avoid selftest-specific files from being picked up by .exp globbing
patterns. We could add target-specific directories below that dir if
necessary.
I've rewritten the rest of the patch kit to use this to load from .rtl
dump files within that directory, rather than embedding the dumps as
string literals in the C source.
The patch also exposes a selftests::path_to_src_gcc, which could be
used by a selftest to e.g. load a DejaGnu file, so that if need be
we could share .rtl input files between both -fself-test tests and
DejaGnu-based tests for the .rtl frontend.
(Depends on the approved-when-needed
"[PATCH 2/9] Add selftest::read_file"
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00476.html ).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk once the dependencies are in?
gcc/ChangeLog:
* Makefile.in (s-selftest) Add $(srcdir) as an argument of
-fself-test.
(selftest-gdb): Likewise.
(selftest-valgrind): Likewise.
* common.opt (fself-test): Rename to...
(fself-test=): ...this, documenting the meaning of the argument.
* selftest-run-tests.c: Include "options.h".
(selftest::run_tests): Initialize selftest::path_to_src_gcc from
flag_self_test.
* selftest.c (selftest::path_to_src_gcc): New global.
(selftest::locate_file): New function.
(selftest::test_locate_file): New function.
(selftest::selftest_c_tests): Call test_locate_file.
* selftest.h (selftest::locate_file): New decl.
(selfte
Re: [Patch] Remove all uses of TARGET_FLT_EVAL_METHOD_NON_DEFAULT and poison it
On Thu, Sep 22, 2016 at 05:55:21PM +, Joseph Myers wrote: > On Thu, 22 Sep 2016, James Greenhalgh wrote: > > > The relaxation isn't portable, and keeping it in place is tricky, so this > > patch removes it, and poisons TARGET_FLT_EVAL_METHOD_NON_DEFAULT in > > system.h to prevent future use. > > > > Bootstrapped and tested on x86_64 with --enable-languages=all,ada,go,obj-c++ > > with no issues. > > > > OK? > > OK. Hi Joseph, Is this OK sufficient for me to commit the patch, or must I wait for input from the other CCed front-end maintainers? Thanks, James
Re: [PATCH GCC 6/9]Simplify control flow graph for vectorized loop
On 09/21/2016 02:52 AM, Bin.Cheng wrote: On Wed, Sep 14, 2016 at 5:43 PM, Jeff Law wrote: On 09/14/2016 07:21 AM, Richard Biener wrote: On Tue, Sep 6, 2016 at 8:52 PM, Bin Cheng wrote: Hi, This is the main patch improving control flow graph for vectorized loop. It generally rewrites loop peeling stuff in vectorizer. As described in patch, for a typical loop to be vectorized like: preheader: LOOP: header_bb: loop_body if (exit_loop_cond) goto exit_bb elsegoto header_bb exit_bb: This patch peels prolog and epilog from the loop, adds guards skipping PROLOG and EPILOG for various conditions. As a result, the changed CFG would look like: guard_bb_1: if (prefer_scalar_loop) goto merge_bb_1 elsegoto guard_bb_2 guard_bb_2: if (skip_prolog) goto merge_bb_2 else goto prolog_preheader prolog_preheader: PROLOG: prolog_header_bb: prolog_body if (exit_prolog_cond) goto prolog_exit_bb else goto prolog_header_bb prolog_exit_bb: merge_bb_2: vector_preheader: VECTOR LOOP: vector_header_bb: vector_body if (exit_vector_cond) goto vector_exit_bb else goto vector_header_bb vector_exit_bb: guard_bb_3: if (skip_epilog) goto merge_bb_3 else goto epilog_preheader merge_bb_1: epilog_preheader: EPILOG: epilog_header_bb: epilog_body if (exit_epilog_cond) goto merge_bb_3 else goto epilog_header_bb merge_bb_3: Note this patch peels prolog and epilog only if it's necessary, as well as adds different guard_conditions/branches. Also the first guard/branch could be further improved by merging it with loop versioning. Before this patch, up to 4 branch instructions need to be executed before the vectorized loop is reached in the worst case, while the number is reduced to 2 with this patch. The patch also does better in compile time analysis to avoid unnecessary peeling/branching. From implementation's point of view, vectorizer needs to update induction variables and iteration bounds along with control flow changes. Unfortunately, it also becomes much harder to follow because slpeel_* functions updates SSA by itself, rather than using update_ssa interface. This patch tries to factor out SSA/IV/Niter_bound changes from CFG changes. This should make the implementation easier to read, and I think it maybe a step forward to replace slpeel_* functions with generic GIMPLE loop copy interfaces as Richard suggested. I've skimmed over the patch and it looks reasonable to me. THanks. I was maybe 15% of the way through the main patch. Nothing that gave me cause for concern, but I wasn't ready to ACK it myself yet. Hi Jeff, Any update on this one? Well, it might conflict with the epilogue vectorization patch set? I considered Richi's message an ACK for the patch. Sorry if I wasn't clear about that. While this patch may conflict with the epilogue vectorization patch set, but the epilogue vectorization work seems to have stalled, so let's have yours go in now. Jeff
Re: [Patch AArch64] Add floatdihf2 and floatunsdihf2 patterns
On Wed, Sep 21, 2016 at 10:42:03AM +0100, James Greenhalgh wrote:
> On Tue, Sep 13, 2016 at 10:31:28AM +0100, James Greenhalgh wrote:
> > On Tue, Sep 06, 2016 at 10:19:50AM +0100, James Greenhalgh wrote:
> > > This patch adds patterns for conversion from 64-bit integer to 16-bit
> > > floating-point values under AArch64 targets which don't have support for
> > > the ARMv8.2-A 16-bit floating point extensions.
> > >
> > > We implement these by first saturating to a SImode (we know that any
> > > values >= 65504 will round to infinity after conversion to HFmode), then
> > > converting to a DFmode (unsigned conversions could go to SFmode, but there
> > > is no performance benefit to this). Then converting to HFmode.
> > >
> > > Having added these patterns, the expansion path in "expand_float" will
> > > now try to use them for conversions from SImode to HFmode as there is no
> > > floatsihf2 pattern. expand_float first tries widening the integer size and
> > > looking for a match, so it will try SImode -> DImode. But our DI mode
> > > pattern is going to then saturate us back to SImode which is wasteful.
> > >
> > > Better, would be for us to provide float(uns)sihf2 patterns directly.
> > > So that's what this patch does.
> > >
> > > The testcase add in this patch would fail on trunk for AArch64. There is
> > > no libgcc routine to make the conversion, and we don't provide appropriate
> > > patterns in the backend, so we get a link-time error.
> > >
> > > Bootstrapped and tested on aarch64-none-linux-gnu
> > >
> > > OK for trunk?
> >
> > Ping.
>
> Ping^2
Ping^3
There was an off-list question as to whether the mid-end could catch this,
rather than requiring the target to do so. My objection to that is that it
would involve teaching the midend about saturating narrowing operations,
which if the target doesn't provide them natively require branching.
I'd rather push targets that want DImode to HFmode (and don't provide a
DImode to TFmode to go through first) to use libgcc/soft-fp than try to add
a special generic expander for DImode to HFmode conversions.
Note that even if we did have a generic expander for these types, we would
still need some version of this patch, as we want to override the behaviour
where the ARMv8.2-A 16-bit floating-point types are available.
Thanks,
James
> > > 2016-09-06 James Greenhalgh
> > >
> > > * config/aarch64/aarch64.md (sihf2): Convert to expand.
> > > (dihf2): Likewise.
> > > (aarch64_fp16_hf2): New.
> > >
> > > 2016-09-06 James Greenhalgh
> > >
> > > * gcc.target/aarch64/floatdihf2_1.c: New.
> > >
> >
> > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > > index 6afaf90..1882a72 100644
> > > --- a/gcc/config/aarch64/aarch64.md
> > > +++ b/gcc/config/aarch64/aarch64.md
> > > @@ -4630,7 +4630,14 @@
> > >[(set_attr "type" "f_cvti2f")]
> > > )
> > >
> > > -(define_insn "hf2"
> > > +;; If we do not have ARMv8.2-A 16-bit floating point extensions, the
> > > +;; midend will arrange for an SImode conversion to HFmode to first go
> > > +;; through DFmode, then to HFmode. But first it will try converting
> > > +;; to DImode then down, which would match our DImode pattern below and
> > > +;; give very poor code-generation. So, we must provide our own emulation
> > > +;; of the mid-end logic.
> > > +
> > > +(define_insn "aarch64_fp16_hf2"
> > >[(set (match_operand:HF 0 "register_operand" "=w")
> > > (FLOATUORS:HF (match_operand:GPI 1 "register_operand" "r")))]
> > >"TARGET_FP_F16INST"
> > > @@ -4638,6 +4645,53 @@
> > >[(set_attr "type" "f_cvti2f")]
> > > )
> > >
> > > +(define_expand "sihf2"
> > > + [(set (match_operand:HF 0 "register_operand")
> > > + (FLOATUORS:HF (match_operand:SI 1 "register_operand")))]
> > > + "TARGET_FLOAT"
> > > +{
> > > + if (TARGET_FP_F16INST)
> > > +emit_insn (gen_aarch64_fp16_sihf2 (operands[0], operands[1]));
> > > + else
> > > +{
> > > + rtx convert_target = gen_reg_rtx (DFmode);
> > > + emit_insn (gen_sidf2 (convert_target, operands[1]));
> > > + emit_insn (gen_truncdfhf2 (operands[0], convert_target));
> > > +}
> > > + DONE;
> > > +}
> > > +)
> > > +
> > > +;; For DImode there is no wide enough floating-point mode that we
> > > +;; can convert through natively (TFmode would work, but requires a
> > > library
> > > +;; call). However, we know that any value >= 65504 will be rounded
> > > +;; to infinity on conversion. This is well within the range of SImode,
> > > so
> > > +;; we can:
> > > +;; Saturate to SImode.
> > > +;; Convert from that to DFmode
> > > +;; Convert from that to HFmode (phew!).
> > > +;; Note that the saturation to SImode requires the SIMD extensions. If
> > > +;; we ever need to provide this pattern where the SIMD extensions are not
> > > +;; available, we would need a different approach.
> > > +
> > > +(define_expand "dihf2"
> > > + [(set (match_operand:HF 0 "register_operand")
> > > + (FLOATUORS:HF (matc
Re: [PATCH] Use CONSTRUCTOR_NELTS macro some more
On 09/22/2016 02:07 PM, Jakub Jelinek wrote: Hi! I've noticed lots of vec_safe_length (CONSTRUCTOR_ELTS (...)) uses in the sources, which IMHO are less readable than the much more often used CONSTRUCTOR_NELTS (...) macro that does the same thing. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2016-09-22 Jakub Jelinek * hsa-gen.c (hsa_op_immed::hsa_op_immed Use CONSTRUCTOR_NELTS (...) instead of vec_safe_length (CONSTRUCTOR_ELTS (...)). (gen_hsa_ctor_assignment): Likewise. * print-tree.c (print_node): Likewise. * tree-dump.c (dequeue_and_dump): Likewise. * tree-sra.c (sra_modify_constructor_assign): Likewise. * expr.c (store_constructor): Likewise. * fold-const.c (operand_equal_p): Likewise. * tree-pretty-print.c (dump_generic_node): Likewise. * hsa-brig.c (hsa_op_immed::emit_to_buffer): Likewise. * ipa-icf-gimple.c (func_checker::compare_operand): Likewise. cp/ * typeck2.c (process_init_constructor_record): Use CONSTRUCTOR_NELTS (...) instead of vec_safe_length (CONSTRUCTOR_ELTS (...)). * decl.c (reshape_init_r): Likewise. (check_initializer): Likewise. ada/ * gcc-interface/decl.c (gnat_to_gnu_entity): Use CONSTRUCTOR_NELTS (...) instead of vec_safe_length (CONSTRUCTOR_ELTS (...)). OK. jeff
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
On 09/28/16 16:41, Jason Merrill wrote: > On Tue, Sep 27, 2016 at 11:10 AM, Bernd Edlinger > wrote: >> On 09/27/16 16:42, Jason Merrill wrote: >>> On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger >>> wrote: On 09/27/16 16:10, Florian Weimer wrote: > * Bernd Edlinger: > >>> “0 << 0” is used in a similar context, to create a zero constant for a >>> multi-bit subfield of an integer. >>> >>> This example comes from GDB, in bfd/elf64-alpha.c: >>> >>> | insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0); >>> >> >> Of course that is not a boolean context, and will not get a warning. >> >> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)". >> >> Maybe 1 and 0 come from macro expansion > > But what's the intent of treating 1 << 0 and 0 << 0 differently in the > patch, then? I am not sure if it was a good idea. I saw, we had code of the form bool flag = 1 << 2; another value LOOKUP_PROTECT is 1 << 0, and bool flag = 1 << 0; would at least not overflow the allowed value range of a boolean. >>> >>> Assigning a bit mask to a bool variable is still probably not what was >>> intended, even if it doesn't change the value. >> >> That works for me too. >> I can simply remove that exception. > > Sounds good. > Great. Is that an "OK with that change"? Bernd.
Re: [PATCH][RTL ifcvt] Transform (X == CST) ? -CST : Y into (X == CST) ? -X : Y when conditional negation is available
On 09/28/2016 02:48 AM, Kyrill Tkachov wrote:
Hi all,
This patch tries to avoid materialising an immediate
when comparison has shown that it is already loaded.
This is performed during ifcvt and only when the target has the
conditional negate
or inverse optabs, which for now is only aarch64.
Thus for the code:
int
foo (int a, int b)
{
return a == 5 ? -5 : b;
}
we currently emit on aarch64:
foo:
mov w2, -5
cmp w0, 5
cselw0, w1, w2, ne
ret
but with this patch we emit:
foo:
cmp w0, 5
csneg w0, w1, w0, ne
ret
Bootstrapped and tested on arm, aarch64, x86_64.
Ok for trunk?
Thanks,
Kyrill
P.S. The simpler form of the transformation: X == CST ? CST : Y --> X ==
CST ? X : Y
could also be beneficial IMO but I found that it can cause bad codegen
in combine
due to extending the live range of X. I'm still working on a way to work
around that,
but that is a separate piece of work anyway.
FWIW, the simpler form of the transformation is already done just prior
to leaving SSA form in tree-ssa-uncprop.c. So there may not be a ton of
opportunities for the simpler form in the RTL optimizers.
My only concern here is this transformation isn't significantly
different than the simpler form, which is apparently causing some kind
of combine problem.
I'd like to understand the combine problem better before giving final
approval to this patch.
Jeff
Re: [PATCH][v4] GIMPLE store merging pass
On 28/09/16 16:59, Bill Schmidt wrote: On Sep 28, 2016, at 10:54 AM, Kyrill Tkachov wrote: Hi all, This is v4 of the pass. It addresses feedback by Bernhard, including typo fixes and skipping of debug statements. Also, I've extended it to handle the case from PR 23684 and included that testcase in the patch. Merging now triggers more often. I've also added purging of dead EH edges that was missing from the previous versions. Bootstrapped and tested on aarch64-none-linux-gnu, x86_64-unknown-linux-gnu, arm-none-linux-gnueabihf. Also tested on aarch64 big-endian. I saw no regressions on my x86_64 machine on SPEC2006. I think the changes in individual benchmarks were in the noise though I think the x86_64 expanders could be improved to split expensive movabsq instructions into two movl ones (I think). Bill, could you or someone else with access to Power benchmarking try this patch out on some benchmarks that you usually use? The new pass in this patch is on by default and can be turned off by -fno-store-merging if needed. Jakub indicated that his last attempt at this work caused regressions on powerpc so I'd like to see if this patch is okay in that regard. Hi Kyrill, Thanks for the heads-up! I will have someone on my team look at this as soon as possible. Thanks! Kyrill Much obliged, Bill Thanks, Kyrill 2016-09-28 Kyrylo Tkachov PR middle-end/22141 * Makefile.in (OBJS): Add gimple-ssa-store-merging.o. * common.opt (fstore-merging): New Optimization option. * opts.c (default_options_table): Add entry for OPT_ftree_store_merging. * params.def (PARAM_STORE_MERGING_ALLOW_UNALIGNED): Define. * passes.def: Insert pass_tree_store_merging. * tree-pass.h (make_pass_store_merging): Declare extern prototype. * gimple-ssa-store-merging.c: New file. * doc/invoke.texi (Optimization Options): Document -fstore-merging. 2016-09-28 Kyrylo Tkachov Jakub Jelinek Andrew Pinski PR middle-end/22141 PR rtl-optimization/23684 * gcc.c-torture/execute/pr22141-1.c: New test. * gcc.c-torture/execute/pr22141-2.c: Likewise. * gcc.target/aarch64/ldp_stp_1.c: Adjust for -fstore-merging. * gcc.target/aarch64/ldp_stp_4.c: Likewise. * gcc.dg/store_merging_1.c: New test. * gcc.dg/store_merging_2.c: Likewise. * gcc.dg/store_merging_3.c: Likewise. * gcc.dg/store_merging_4.c: Likewise. * gcc.dg/store_merging_5.c: Likewise. * gcc.dg/store_merging_6.c: Likewise. * gcc.dg/store_merging_7.c: Likewise. * gcc.target/i386/pr22141.c: Likewise. * gcc.target/i386/pr34012.c: Add -fno-store-merging to dg-options.
Re: [PATCH][v4] GIMPLE store merging pass
On Sep 28, 2016, at 10:54 AM, Kyrill Tkachov wrote: > > Hi all, > > This is v4 of the pass. It addresses feedback by Bernhard, including typo > fixes and > skipping of debug statements. > Also, I've extended it to handle the case from PR 23684 and included that > testcase > in the patch. Merging now triggers more often. > I've also added purging of dead EH edges that was missing from the previous > versions. > > Bootstrapped and tested on aarch64-none-linux-gnu, x86_64-unknown-linux-gnu, > arm-none-linux-gnueabihf. > Also tested on aarch64 big-endian. > > I saw no regressions on my x86_64 machine on SPEC2006. I think the changes in > individual benchmarks were > in the noise though I think the x86_64 expanders could be improved to split > expensive movabsq instructions > into two movl ones (I think). > > Bill, could you or someone else with access to Power benchmarking try this > patch out on some benchmarks > that you usually use? The new pass in this patch is on by default and can be > turned off by -fno-store-merging > if needed. Jakub indicated that his last attempt at this work caused > regressions on powerpc so I'd like to > see if this patch is okay in that regard. Hi Kyrill, Thanks for the heads-up! I will have someone on my team look at this as soon as possible. Much obliged, Bill > > Thanks, > Kyrill > > 2016-09-28 Kyrylo Tkachov > >PR middle-end/22141 >* Makefile.in (OBJS): Add gimple-ssa-store-merging.o. >* common.opt (fstore-merging): New Optimization option. >* opts.c (default_options_table): Add entry for >OPT_ftree_store_merging. >* params.def (PARAM_STORE_MERGING_ALLOW_UNALIGNED): Define. >* passes.def: Insert pass_tree_store_merging. >* tree-pass.h (make_pass_store_merging): Declare extern >prototype. >* gimple-ssa-store-merging.c: New file. >* doc/invoke.texi (Optimization Options): Document >-fstore-merging. > > 2016-09-28 Kyrylo Tkachov >Jakub Jelinek >Andrew Pinski > >PR middle-end/22141 >PR rtl-optimization/23684 >* gcc.c-torture/execute/pr22141-1.c: New test. >* gcc.c-torture/execute/pr22141-2.c: Likewise. >* gcc.target/aarch64/ldp_stp_1.c: Adjust for -fstore-merging. >* gcc.target/aarch64/ldp_stp_4.c: Likewise. >* gcc.dg/store_merging_1.c: New test. >* gcc.dg/store_merging_2.c: Likewise. >* gcc.dg/store_merging_3.c: Likewise. >* gcc.dg/store_merging_4.c: Likewise. >* gcc.dg/store_merging_5.c: Likewise. >* gcc.dg/store_merging_6.c: Likewise. >* gcc.dg/store_merging_7.c: Likewise. >* gcc.target/i386/pr22141.c: Likewise. >* gcc.target/i386/pr34012.c: Add -fno-store-merging to dg-options. >
Re: Change license of filenames.h to LGPL
Florian Weimer writes: > Sorry, I don't understand. Surely anything released under the LGPL by > the FSF can be upgraded to the current GPLv3? First upgrade to the > latest LGPL, then switch over to the GPLv3? > > (I assume that the FSF releases their works under the “any later > version” regime.) That's not what that means. The license terms cannot be changed, and remain "version X or later", even if the user chooses to apply the terms of some later version. The "or later" allows the users alternatives for when the FSF fixes a "license bug" in a newer version; it avoids needing to update all the licenses. It also future-proofs the older code, ensuring it's license-compatible with newer code.
[PATCH][v4] GIMPLE store merging pass
Hi all,
This is v4 of the pass. It addresses feedback by Bernhard, including typo fixes
and
skipping of debug statements.
Also, I've extended it to handle the case from PR 23684 and included that
testcase
in the patch. Merging now triggers more often.
I've also added purging of dead EH edges that was missing from the previous
versions.
Bootstrapped and tested on aarch64-none-linux-gnu, x86_64-unknown-linux-gnu,
arm-none-linux-gnueabihf.
Also tested on aarch64 big-endian.
I saw no regressions on my x86_64 machine on SPEC2006. I think the changes in
individual benchmarks were
in the noise though I think the x86_64 expanders could be improved to split
expensive movabsq instructions
into two movl ones (I think).
Bill, could you or someone else with access to Power benchmarking try this
patch out on some benchmarks
that you usually use? The new pass in this patch is on by default and can be
turned off by -fno-store-merging
if needed. Jakub indicated that his last attempt at this work caused
regressions on powerpc so I'd like to
see if this patch is okay in that regard.
Thanks,
Kyrill
2016-09-28 Kyrylo Tkachov
PR middle-end/22141
* Makefile.in (OBJS): Add gimple-ssa-store-merging.o.
* common.opt (fstore-merging): New Optimization option.
* opts.c (default_options_table): Add entry for
OPT_ftree_store_merging.
* params.def (PARAM_STORE_MERGING_ALLOW_UNALIGNED): Define.
* passes.def: Insert pass_tree_store_merging.
* tree-pass.h (make_pass_store_merging): Declare extern
prototype.
* gimple-ssa-store-merging.c: New file.
* doc/invoke.texi (Optimization Options): Document
-fstore-merging.
2016-09-28 Kyrylo Tkachov
Jakub Jelinek
Andrew Pinski
PR middle-end/22141
PR rtl-optimization/23684
* gcc.c-torture/execute/pr22141-1.c: New test.
* gcc.c-torture/execute/pr22141-2.c: Likewise.
* gcc.target/aarch64/ldp_stp_1.c: Adjust for -fstore-merging.
* gcc.target/aarch64/ldp_stp_4.c: Likewise.
* gcc.dg/store_merging_1.c: New test.
* gcc.dg/store_merging_2.c: Likewise.
* gcc.dg/store_merging_3.c: Likewise.
* gcc.dg/store_merging_4.c: Likewise.
* gcc.dg/store_merging_5.c: Likewise.
* gcc.dg/store_merging_6.c: Likewise.
* gcc.dg/store_merging_7.c: Likewise.
* gcc.target/i386/pr22141.c: Likewise.
* gcc.target/i386/pr34012.c: Add -fno-store-merging to dg-options.
commit 74e3e742e446f6a892c8ee755f640756efc816cd
Author: Kyrylo Tkachov
Date: Tue Jul 12 12:30:47 2016 +0100
GIMPLE store merging pass
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 9e08fbf..d83b2f1 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1300,6 +1300,7 @@ OBJS = \
gimple-ssa-isolate-paths.o \
gimple-ssa-nonnull-compare.o \
gimple-ssa-split-paths.o \
+ gimple-ssa-store-merging.o \
gimple-ssa-strength-reduction.o \
gimple-ssa-sprintf.o \
gimple-streamer-in.o \
diff --git a/gcc/common.opt b/gcc/common.opt
index 168735e..4d73852 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1460,6 +1460,10 @@ fstrict-volatile-bitfields
Common Report Var(flag_strict_volatile_bitfields) Init(-1) Optimization
Force bitfield accesses to match their type width.
+fstore-merging
+Common Var(flag_store_merging) Optimization
+Use the tree store merging pass.
+
fguess-branch-probability
Common Report Var(flag_guess_branch_prob) Optimization
Enable guessing of branch probabilities.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6767462..83acd3b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -403,7 +403,7 @@ Objective-C and Objective-C++ Dialects}.
-fsingle-precision-constant -fsplit-ivs-in-unroller @gol
-fsplit-paths @gol
-fsplit-wide-types -fssa-backprop -fssa-phiopt @gol
--fstdarg-opt -fstrict-aliasing @gol
+-fstdarg-opt -fstore-merging -fstrict-aliasing @gol
-fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
-ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol
-ftree-coalesce-vars -ftree-copy-prop -ftree-dce -ftree-dominator-opts @gol
@@ -414,8 +414,8 @@ Objective-C and Objective-C++ Dialects}.
-ftree-loop-vectorize @gol
-ftree-parallelize-loops=@var{n} -ftree-pre -ftree-partial-pre -ftree-pta @gol
-ftree-reassoc -ftree-sink -ftree-slsr -ftree-sra @gol
--ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol
--ftree-vectorize -ftree-vrp -funconstrained-commons @gol
+-ftree-switch-conversion -ftree-tail-merge @gol
+-ftree-ter -ftree-vectorize -ftree-vrp -funconstrained-commons @gol
-funit-at-a-time -funroll-all-loops -funroll-loops @gol
-funsafe-math-optimizations -funswitch-loops @gol
-fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol
@@ -6593,6 +6593,7 @@ compilation time.
-fsplit-wide-types @gol
-fssa-backprop @gol
-fssa-phiopt @gol
+-fstore-merging @gol
-ftree-bit-ccp @gol
-ftree-ccp @gol
-ftree-ch @gol
@@ -7927,6 +7928,13 @@ Perform scalar replacement of aggregates. This pass replaces structure
reference
[PATCH PR77718]
This patch that Bernd put in PR77718 seems to be fine. Bootstrap and
regtest done on powerpc64le, no new failures. Ok for trunk?
2016-09-28 Bernd Schmidt
* builtins.c (expand_builtin_memcmp): don't swap args unless
result is only being compared with zero.
Index: builtins.c
===
--- builtins.c (revision 240511)
+++ builtins.c (working copy)
@@ -3707,11 +3707,13 @@ expand_builtin_memcmp (tree exp, rtx tar
by_pieces_constfn constfn = NULL;
- const char *src_str = c_getstr (arg1);
- if (src_str == NULL)
-src_str = c_getstr (arg2);
- else
-std::swap (arg1_rtx, arg2_rtx);
+ const char *src_str = c_getstr (arg2);
+ if (result_eq && src_str == NULL)
+{
+ src_str = c_getstr (arg1);
+ if (src_str != NULL)
+ std::swap (arg1_rtx, arg2_rtx);
+}
/* If SRC is a string constant and block move would be done
by pieces, we can avoid loading the string from memory
--
Aaron Sawdey, Ph.D. acsaw...@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
Re: [PATCH] Avoid store forwarding issue in vectorizing strided SLP loads
On 09/28/2016 05:41 AM, Richard Biener wrote:
Currently strided SLP vectorization creates vector constructors composed
of vector elements. This is a constructor form that is not handled
specially by the expander but it gets expanded via piecewise stores
to scratch memory and a load of that scratch memory.
Ugh. Yup, obviously bad, even without store forwarding.
does not work on any CPU I know of). The following patch simply avoids
the issue by making the vectorizer create integer loads, composing
a vector of that integers and then punning that to the desired vector
type. Thus (V4SF){V2SF, V2SF} becomes (V4SF)(V2DI){DI, DI} and
every body is happy. Especially x264 gets a 5-10% improvement
(dependent on vector size and x86 sub-architecture).
Seems reasonable to me -- there's not a lot of difference (conceptually)
to how we've used SImode constants to construct DFmode constants in the
past.
Handling the vector-vector constructors on the expander side would
require either similar punning or making vec_init parametric on
the element mode plus supporting vector elements in all targets
(which in the end probably will simply pun them similarly).
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
Any comments?
Thanks,
Richard.
2016-09-28 Richard Biener
* tree-vect-stmts.c (vectorizable_load): Avoid emitting vector
constructors with vector elements.
Seems quite reasonable.
jeff
Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities
> Le 28 sept. 2016 à 17:44, Jason Merrill a écrit : > > On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat > wrote: >> A check in dwarf2out_imported_module_or_decl prevents valid strict >> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted >> in dwarf2out_imported_module_or_decl_1. >> >> The latter already protects the emission of newer DWARF tags with >> appropriate checks, so the one in the former is redundant and >> pessimistic. This function is already called from places like >> process_scope_var, which are not protected anyway. >> >> This patch removes the check in dwarf2out_imported_module_or_decl so >> that tags like DW_TAG_imported_declaration are emitted even in strict >> DWARF2 mode. >> >> Bootstrapped and regtested on x86_64-linux, no regression. > > This check was added for bug 41405, a Darwin bootstrap problem. > Dominique, can you verify that the compiler still bootstraps with this > change? OK if so. OK, but not before tomorrow afternoon. Dominique > Jason
Re: [PATCH v3] Optimize strchr to strlen
OK. On Wed, Sep 28, 2016 at 11:43 AM, Wilco Dijkstra wrote: > Jason Merrill wrote: >> I think this broke g++.dg/ext/builtin10.C. > > That's odd. It appears if you add a fold in gimple-fold.c, it no longer calls > the > folding code in builtins.c. No idea what the idea behind that is (especially > since > there are other builtins that appear in both files), but this simple patch > fixes it: > > If strchr can't be folded in gimple-fold.c, break so folding code in > builtins.c is > also called. > > OK for commit? > > 2016-09-28 Wilco Dijkstra > > * gimple-fold.c (gimple_fold_builtin): After failing to fold > strchr, also try the generic folding. > -- > diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c > index > ddf4cf0ae68ef6708377fdb1a2b45575d90da799..b6802e81fd1a7fd0b309cb9aa0f984f7bacb6596 > 100644 > --- a/gcc/gimple-fold.c > +++ b/gcc/gimple-fold.c > @@ -2948,7 +2948,10 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi) > case BUILT_IN_STRNCAT: >return gimple_fold_builtin_strncat (gsi); > case BUILT_IN_STRCHR: > - return gimple_fold_builtin_strchr (gsi); > + if (gimple_fold_builtin_strchr (gsi)) > + return true; > + /* Perform additional folding in builtin.c. */ > + break; > case BUILT_IN_FPUTS: >return gimple_fold_builtin_fputs (gsi, gimple_call_arg (stmt, 0), > gimple_call_arg (stmt, 1), false); >
Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities
On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat wrote: > A check in dwarf2out_imported_module_or_decl prevents valid strict > DWARF2 constructs such as DW_TAG_imported_declaration from being emitted > in dwarf2out_imported_module_or_decl_1. > > The latter already protects the emission of newer DWARF tags with > appropriate checks, so the one in the former is redundant and > pessimistic. This function is already called from places like > process_scope_var, which are not protected anyway. > > This patch removes the check in dwarf2out_imported_module_or_decl so > that tags like DW_TAG_imported_declaration are emitted even in strict > DWARF2 mode. > > Bootstrapped and regtested on x86_64-linux, no regression. This check was added for bug 41405, a Darwin bootstrap problem. Dominique, can you verify that the compiler still bootstraps with this change? OK if so. Jason
Re: [PATCH v3] Optimize strchr to strlen
Jason Merrill wrote: > I think this broke g++.dg/ext/builtin10.C. That's odd. It appears if you add a fold in gimple-fold.c, it no longer calls the folding code in builtins.c. No idea what the idea behind that is (especially since there are other builtins that appear in both files), but this simple patch fixes it: If strchr can't be folded in gimple-fold.c, break so folding code in builtins.c is also called. OK for commit? 2016-09-28 Wilco Dijkstra * gimple-fold.c (gimple_fold_builtin): After failing to fold strchr, also try the generic folding. -- diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index ddf4cf0ae68ef6708377fdb1a2b45575d90da799..b6802e81fd1a7fd0b309cb9aa0f984f7bacb6596 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -2948,7 +2948,10 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi) case BUILT_IN_STRNCAT: return gimple_fold_builtin_strncat (gsi); case BUILT_IN_STRCHR: - return gimple_fold_builtin_strchr (gsi); + if (gimple_fold_builtin_strchr (gsi)) + return true; + /* Perform additional folding in builtin.c. */ + break; case BUILT_IN_FPUTS: return gimple_fold_builtin_fputs (gsi, gimple_call_arg (stmt, 0), gimple_call_arg (stmt, 1), false);
Re: Change license of filenames.h to LGPL
> From: Florian Weimer > Cc: DJ Delorie , gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Wed, 28 Sep 2016 16:43:53 +0200 > > * Eli Zaretskii: > > > If my arithmetics is correct, about 70% of its files is LGPL, the > > rest GPL. Which doesn't keep many GNU projects under GPL from using > > Gnulib. > > Sorry, I don't understand. Surely anything released under the LGPL by > the FSF can be upgraded to the current GPLv3? First upgrade to the > latest LGPL, then switch over to the GPLv3? > > (I assume that the FSF releases their works under the “any later > version” regime.) The above was in response to DJ's questions up-thread: > > Because Ozkan wants to use it in an otherwise LGPL package. > > But then the implementation would need relicensing as well, wouldn't > it? > > Having both under different licenses is just confusing. Did I misunderstand the question?
Re: Change license of filenames.h to LGPL
> Sorry, I don't understand. Surely anything released under the LGPL by > the FSF can be upgraded to the current GPLv3? First upgrade to the > latest LGPL, then switch over to the GPLv3? That seems correct to me.
Re: Implement -Wimplicit-fallthrough (version 9)
> "Michael" == Michael Matz writes:
Michael> Not accepting
Michael> /* And here we intentionally fall through because ... */
Michael> and forcing users to replace this by:
Michael> /* fallthrough */
Michael> is not robust either. It's actually actively lowering robustness of
code,
Michael> it creates work for programmers that will be regarded as pointless
(and
Michael> rightly so) and will merely lead to everybody disabling the warning
(see
Michael> our generated files)
We can't control what programmers might do. My point is that accepting
too much is actively bad -- it hides errors. If this somehow makes some
programmer fall down a slippery slope, well, that's their error, not
gcc's.
TBH I think it would be better not to parse comments at all. Heuristics
are generally bad and this case and ensuing discussion is a great
demonstration of that.
The other day I built gdb with -Wimplicit-fallthrough. I was surprised
to find that gcc rejected this:
default:
{
complaint (&symfile_complaints,
_("Storage class %d not recognized during scan"),
sclass);
}
/* FALLTHROUGH */
/* C_FCN is .bf and .ef symbols. I think it is sufficient
to handle only the C_FUN and C_EXT. */
case C_FCN:
Presumably without the comment heuristic, this would be accepted.
Michael> The point of warnings is to make code robust under the condition of
not
Michael> being a pain by giving zillions of false positives.
My experience so far is that it's not so bad. gdb actually had comments
in most spots, they just required a quick pass to clean them up:
https://sourceware.org/ml/gdb-patches/2016-09/msg00375.html
And, code bases in more dire straights can just disable the warning after all.
Tom
Re: [gomp4] more tile parsing
ENOPATCH
2016-09-28 Nathan Sidwell
c/
* c-parser.c (c_parser_omp_for_tiling): Accept tiling constructs.
cp/
* parser.c (cp_parser_omp_for_loop): Deal with tile clause. Don't
emit a parse error about missing for after already emitting
one. Use more conventional for idiom for unbounded loop.
* semantics.c (finish_omp_for): Deal with tile clause.
testsuite/
* c-c++-common/goacc/tile-2.c: New.
* c-c++-common/goacc/tile.c: Fix.
Index: c/c-parser.c
===
--- c/c-parser.c (revision 240524)
+++ c/c-parser.c (working copy)
@@ -14920,11 +14920,17 @@ c_parser_omp_for_loop (location_t loc, c
bool fail = false, open_brace_parsed = false;
int i, collapse = 1, ordered = 0, count, nbraces = 0;
location_t for_loc;
+ bool tiling = false;
vec *for_block = make_tree_vector ();
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE)
+ {
+ tiling = true;
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (cl));
+ }
else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED
&& OMP_CLAUSE_ORDERED_EXPR (cl))
{
@@ -14954,7 +14960,7 @@ c_parser_omp_for_loop (location_t loc, c
pc = &OMP_CLAUSE_CHAIN (*pc);
}
- gcc_assert (collapse >= 1 && ordered >= 0);
+ gcc_assert (tiling || (collapse >= 1 && ordered >= 0));
count = ordered ? ordered : collapse;
declv = make_tree_vec (count);
Index: cp/parser.c
===
--- cp/parser.c (revision 240524)
+++ cp/parser.c (working copy)
@@ -33660,10 +33660,16 @@ cp_parser_omp_for_loop (cp_parser *parse
int i, collapse = 1, ordered = 0, count, nbraces = 0;
vec *for_block = make_tree_vector ();
auto_vec orig_inits;
+ bool tiling = false;
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_TILE)
+ {
+ tiling = true;
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (cl));
+ }
else if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_ORDERED
&& OMP_CLAUSE_ORDERED_EXPR (cl))
{
@@ -33693,7 +33699,7 @@ cp_parser_omp_for_loop (cp_parser *parse
pc = &OMP_CLAUSE_CHAIN (*pc);
}
- gcc_assert (collapse >= 1 && ordered >= 0);
+ gcc_assert (tiling || (collapse >= 1 && ordered >= 0));
count = ordered ? ordered : collapse;
declv = make_tree_vec (count);
@@ -33712,13 +33718,15 @@ cp_parser_omp_for_loop (cp_parser *parse
if (code != CILK_FOR
&& !cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR))
{
- cp_parser_error (parser, "for statement expected");
+ if (!collapse_err)
+ cp_parser_error (parser, "for statement expected");
return NULL;
}
if (code == CILK_FOR
&& !cp_lexer_next_token_is_keyword (parser->lexer, RID_CILK_FOR))
{
- cp_parser_error (parser, "_Cilk_for statement expected");
+ if (!collapse_err)
+ cp_parser_error (parser, "_Cilk_for statement expected");
return NULL;
}
loc = cp_lexer_consume_token (parser->lexer)->location;
@@ -33878,7 +33886,7 @@ cp_parser_omp_for_loop (cp_parser *parse
nested. Hopefully the final version clarifies this.
For now handle (multiple) {'s and empty statements. */
cp_parser_parse_tentatively (parser);
- do
+ for (;;)
{
if (cp_lexer_next_token_is_keyword (parser->lexer, RID_FOR))
break;
@@ -33893,14 +33901,13 @@ cp_parser_omp_for_loop (cp_parser *parse
else
{
loc = cp_lexer_peek_token (parser->lexer)->location;
- error_at (loc, "not enough collapsed for loops");
+ error_at (loc, "not enough for loops to collapse");
collapse_err = true;
cp_parser_abort_tentative_parse (parser);
declv = NULL_TREE;
break;
}
}
- while (1);
if (declv)
{
Index: cp/semantics.c
===
--- cp/semantics.c (revision 240524)
+++ cp/semantics.c (working copy)
@@ -7986,11 +7986,19 @@ finish_omp_for (location_t locus, enum t
gcc_assert (TREE_VEC_LENGTH (declv) == TREE_VEC_LENGTH (incrv));
if (TREE_VEC_LENGTH (declv) > 1)
{
- tree c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE);
+ tree c;
+
+ c = find_omp_clause (clauses, OMP_CLAUSE_TILE);
if (c)
- collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));
- if (collapse != TREE_VEC_LENGTH (declv))
- ordered = TREE_VEC_LENGTH (declv);
+ collapse = list_length (OMP_CLAUSE_TILE_LIST (c));
+ else
+ {
+ c = find_omp_clause (clauses, OMP_CLAUSE_COLLAPSE);
+ if (c)
+ collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c));
+ if (collapse != TREE_VEC_LENGTH (declv))
+ ordered = TREE_VEC_LENGTH (declv)
[gomp4] more tile parsing
In my recent patch for tile clauses, I noticed some tile tests didn't appear to fail, even though the loops they tiled were ill formed. I figured I'd find out why in the fullness of time. Didn't take long for time to be full. The omp_for parsing routines had no knowledge of tile, so did not checking. This patch rectifies that. I did notice a couple of nits in the c++ parser, also fixed here 1) we'd complain about a missing inner loop twice, which is a bit repetative. Fixed by only emitting the missing for token error, if we didn't already emit an error. 2) 'do ... while (1)' is an odd way to write a non-terminating loop. Changed to 'for (;;) ...' I guess the loop condition was originally something other than '1'. applied to gomp4 nathan
Re: [PATCH v3] Optimize strchr to strlen
I think this broke g++.dg/ext/builtin10.C. Jason
Re: Change license of filenames.h to LGPL
* Eli Zaretskii: > If my arithmetics is correct, about 70% of its files is LGPL, the > rest GPL. Which doesn't keep many GNU projects under GPL from using > Gnulib. Sorry, I don't understand. Surely anything released under the LGPL by the FSF can be upgraded to the current GPLv3? First upgrade to the latest LGPL, then switch over to the GPLv3? (I assume that the FSF releases their works under the “any later version” regime.)
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: gcc-patches@gcc.gnu.org, e...@gnu.org, f...@deneb.enyo.de > Date: Tue, 27 Sep 2016 15:45:28 -0400 > > > I wonder if us relicensing our modified copy would apply to your old > copy. I mean, are we sure RMS knows you're also relicensing an old > copy, and that the current copy is being relicensed only to avoid future > issues. If we're only doing it to document the decision, the fact that > hashtab.h and filename_cmp.c are still GPL mostly negates the > effectiveness of our change anyway. > > (i.e. it seems like you can get what you need whether we relicense ours > or not, and relicensing ours doesn't have much actual effect). I see no reason why setting the record straight about license compatibility should be an issue for us. Better late than never, right?
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
On Tue, Sep 27, 2016 at 11:10 AM, Bernd Edlinger wrote: > On 09/27/16 16:42, Jason Merrill wrote: >> On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger >> wrote: >>> On 09/27/16 16:10, Florian Weimer wrote: * Bernd Edlinger: >> “0 << 0” is used in a similar context, to create a zero constant for a >> multi-bit subfield of an integer. >> >> This example comes from GDB, in bfd/elf64-alpha.c: >> >> | insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0); >> > > Of course that is not a boolean context, and will not get a warning. > > Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)". > > Maybe 1 and 0 come from macro expansion But what's the intent of treating 1 << 0 and 0 << 0 differently in the patch, then? >>> >>> I am not sure if it was a good idea. >>> >>> I saw, we had code of the form >>> bool flag = 1 << 2; >>> >>> another value LOOKUP_PROTECT is 1 << 0, and >>> bool flag = 1 << 0; >>> >>> would at least not overflow the allowed value range of a boolean. >> >> Assigning a bit mask to a bool variable is still probably not what was >> intended, even if it doesn't change the value. > > That works for me too. > I can simply remove that exception. Sounds good. Jason
Re: Change license of filenames.h to LGPL
> From: DJ Delorie > Cc: f...@deneb.enyo.de, gcc-patches@gcc.gnu.org, seze...@gmail.com > Date: Tue, 27 Sep 2016 15:23:46 -0400 > > > Why would it need to > > change? It's perfectly okay to link GPL code with LGPL code, we do > > this all the time with libgcc, no? Or am I missing something? > > libgcc has an exception that covers most of those cases; be careful when > comparing those to your (his) use case. OK, then take Gnulib as a better example. If my arithmetics is correct, about 70% of its files is LGPL, the rest GPL. Which doesn't keep many GNU projects under GPL from using Gnulib.
Re: [C++ PATCH] Fix constexpr switch handling (PR c++/77467, take 2)
OK, thanks. Jason
RE: [PATCH] [ARC] New CPU C-define handler.
> This looks like a good improvement to me. > > Thanks, > Andrew > Committed r240577 Thank you for your review, Claudiu
Re: [PATCH] Bits from Early LTO debug merge -- move stuff from late to early finish
OK.
[PATCH] Fix PR55152
The following adds a pattern to optimize MAX (a, -a) to abs (a).
Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
Richard.
2016-09-28 Richard Biener
PR middle-end/55152
* match.pd: Add max(a,-a) -> abs(a) pattern.
* gcc.dg/pr55152.c: New testcase.
Index: gcc/match.pd
===
--- gcc/match.pd(revision 240565)
+++ gcc/match.pd(working copy)
@@ -1271,6 +1284,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(simplify
(max:c (min:c @0 @1) @1)
@1)
+/* max(a,-a) -> abs(a). */
+(simplify
+ (max:c @0 (negate @0))
+ (if (TREE_CODE (type) != COMPLEX_TYPE
+ && (! ANY_INTEGRAL_TYPE_P (type)
+ || TYPE_OVERFLOW_UNDEFINED (type)))
+ (abs @0)))
(simplify
(min @0 @1)
(switch
Index: gcc/testsuite/gcc.dg/pr55152.c
===
--- gcc/testsuite/gcc.dg/pr55152.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr55152.c (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O -ffinite-math-only -fstrict-overflow
-fdump-tree-optimized" } */
+
+double g (double a)
+{
+ return (a>=-a)?a:-a;
+}
+int f(int a)
+{
+ return (a>=-a)?a:-a;
+}
+
+/* { dg-final { scan-tree-dump-times "ABS_EXPR" 2 "optimized" } } */
[PATCH] Fix (part of) PR77399
This fixes the original request in PR77399, better handling of
typedef int v4si __attribute__((vector_size(16)));
typedef float v4sf __attribute__((vector_size(16)));
v4sf vec_cast(v4si f)
{
return (v4sf){f[0], f[1], f[2], f[3]};
}
which nicely fits into the existing simplify_vector_constructor code.
Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
Richard.
2016-09-28 Richard Biener
PR tree-optimization/77399
* tree-ssa-forwprop.c (simplify_vector_constructor): Handle
float <-> int conversions.
* gcc.dg/tree-ssa/forwprop-35.c: New testcase.
Index: gcc/tree-ssa-forwprop.c
===
*** gcc/tree-ssa-forwprop.c (revision 240565)
--- gcc/tree-ssa-forwprop.c (working copy)
*** simplify_vector_constructor (gimple_stmt
*** 1953,1959
gimple *def_stmt;
tree op, op2, orig, type, elem_type;
unsigned elem_size, nelts, i;
! enum tree_code code;
constructor_elt *elt;
unsigned char *sel;
bool maybe_ident;
--- 1953,1959
gimple *def_stmt;
tree op, op2, orig, type, elem_type;
unsigned elem_size, nelts, i;
! enum tree_code code, conv_code;
constructor_elt *elt;
unsigned char *sel;
bool maybe_ident;
*** simplify_vector_constructor (gimple_stmt
*** 1970,1975
--- 1970,1976
sel = XALLOCAVEC (unsigned char, nelts);
orig = NULL;
+ conv_code = ERROR_MARK;
maybe_ident = true;
FOR_EACH_VEC_SAFE_ELT (CONSTRUCTOR_ELTS (op), i, elt)
{
*** simplify_vector_constructor (gimple_stmt
*** 1984,1989
--- 1985,2008
if (!def_stmt)
return false;
code = gimple_assign_rhs_code (def_stmt);
+ if (code == FLOAT_EXPR
+ || code == FIX_TRUNC_EXPR)
+ {
+ op1 = gimple_assign_rhs1 (def_stmt);
+ if (conv_code == ERROR_MARK)
+ {
+ if (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (elt->value)))
+ != GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op1
+ return false;
+ conv_code = code;
+ }
+ else if (conv_code != code)
+ return false;
+ if (TREE_CODE (op1) != SSA_NAME)
+ return false;
+ def_stmt = SSA_NAME_DEF_STMT (op1);
+ code = gimple_assign_rhs_code (def_stmt);
+ }
if (code != BIT_FIELD_REF)
return false;
op1 = gimple_assign_rhs1 (def_stmt);
*** simplify_vector_constructor (gimple_stmt
*** 1997,2003
{
if (TREE_CODE (ref) != SSA_NAME)
return false;
! if (!useless_type_conversion_p (type, TREE_TYPE (ref)))
return false;
orig = ref;
}
--- 2016,2024
{
if (TREE_CODE (ref) != SSA_NAME)
return false;
! if (! VECTOR_TYPE_P (TREE_TYPE (ref))
! || ! useless_type_conversion_p (TREE_TYPE (op1),
! TREE_TYPE (TREE_TYPE (ref
return false;
orig = ref;
}
*** simplify_vector_constructor (gimple_stmt
*** 2010,2016
return false;
if (maybe_ident)
! gimple_assign_set_rhs_from_tree (gsi, orig);
else
{
tree mask_type, *mask_elts;
--- 2031,2043
return false;
if (maybe_ident)
! {
! if (conv_code == ERROR_MARK)
! gimple_assign_set_rhs_from_tree (gsi, orig);
! else
! gimple_assign_set_rhs_with_ops (gsi, conv_code, orig,
! NULL_TREE, NULL_TREE);
! }
else
{
tree mask_type, *mask_elts;
*** simplify_vector_constructor (gimple_stmt
*** 2028,2034
for (i = 0; i < nelts; i++)
mask_elts[i] = build_int_cst (TREE_TYPE (mask_type), sel[i]);
op2 = build_vector (mask_type, mask_elts);
! gimple_assign_set_rhs_with_ops (gsi, VEC_PERM_EXPR, orig, orig, op2);
}
update_stmt (gsi_stmt (*gsi));
return true;
--- 2055,2072
for (i = 0; i < nelts; i++)
mask_elts[i] = build_int_cst (TREE_TYPE (mask_type), sel[i]);
op2 = build_vector (mask_type, mask_elts);
! if (conv_code == ERROR_MARK)
! gimple_assign_set_rhs_with_ops (gsi, VEC_PERM_EXPR, orig, orig, op2);
! else
! {
! gimple *perm
! = gimple_build_assign (make_ssa_name (TREE_TYPE (orig)),
! VEC_PERM_EXPR, orig, orig, op2);
! orig = gimple_assign_lhs (perm);
! gsi_insert_before (gsi, perm, GSI_SAME_STMT);
! gimple_assign_set_rhs_with_ops (gsi, conv_code, orig,
! NULL_TREE, NULL_TREE);
! }
}
update_stmt (gsi_stmt (*gsi));
return true;
Index: gcc/testsuite/gcc.dg/tree-ssa/forwprop-35.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/forw
[PATCH] Fix PR77407
The following patch implements patterns to catch x / abs (x)
and x / -x, taking advantage of undefinedness at x == 0 as
opposed to the PR having testcases with explicit != 0 checks.
Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
Richard.
2016-09-28 Richard Biener
PR middle-end/77407
* match.pd: Add X / abs (X) -> X < 0 ? -1 : 1 and
X / -X -> -1 simplifications.
* gcc.dg/pr77407.c: New testcase.
Index: gcc/match.pd
===
--- gcc/match.pd(revision 240565)
+++ gcc/match.pd(working copy)
@@ -147,12 +147,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(op @0 integer_onep)
(non_lvalue @0)))
-/* X / -1 is -X. */
(for div (trunc_div ceil_div floor_div round_div exact_div)
+ /* X / -1 is -X. */
(simplify
(div @0 integer_minus_onep@1)
(if (!TYPE_UNSIGNED (type))
-(negate @0
+(negate @0)))
+ /* X / abs (X) is X < 0 ? -1 : 1. */
+ (simplify
+ (div @0 (abs @0))
+ (if ((INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
+ && TYPE_OVERFLOW_UNDEFINED (type))
+(cond (lt @0 { build_zero_cst (type); })
+ { build_minus_one_cst (type); } { build_one_cst (type); })))
+ /* X / -X is -1. */
+ (simplify
+ (div @0 (negate @0))
+ (if ((INTEGRAL_TYPE_P (type) || VECTOR_INTEGER_TYPE_P (type))
+ && TYPE_OVERFLOW_UNDEFINED (type))
+{ build_minus_one_cst (type); })))
/* For unsigned integral types, FLOOR_DIV_EXPR is the same as
TRUNC_DIV_EXPR. Rewrite into the latter in this case. */
Index: gcc/testsuite/gcc.dg/pr77407.c
===
--- gcc/testsuite/gcc.dg/pr77407.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr77407.c (working copy)
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fstrict-overflow -fdump-tree-gimple" } */
+
+int foo (int c)
+{
+ if (c != 0)
+c /= __builtin_abs (c);
+ return c;
+}
+
+int bar (int c)
+{
+ if (c != 0)
+c /= -c;
+ return c;
+}
+
+/* { dg-final { scan-tree-dump-times "/" 0 "gimple" } } */
RE: [PATCH] [ARC] Add simple shift/rotate ops.
> > This looks good to me. > > Thanks, > Andrew > Committed r240576. Thank you for your review, Claudiu
Re: [Fortran, Patch] First patch for coarray FAILED IMAGES (TS 18508)
* PING *
2016-09-21 19:03 GMT+01:00 Alessandro Fanfarillo :
> Thanks Andre.
>
> 2016-09-19 9:55 GMT-06:00 Andre Vehreschild :
>> Hi Alessandro,
>
>> The if in resolve.c at 8837: resolve_failed_image (... is intentional? It is
>> doing nothing. So do you plan to add more code, or will there never be
>> anything. If the later I recommend to just put a comment there and remove the
>> empty if.
>
> I added the if statement during the development and I forgot to remove it.
>
>>
>> There still is no test when -fcoarray=single is used. This shouldn't be so
>> hard, should it?
>
> Done.
>
> Built and regtested on x86_64-pc-linux-gnu.
>
>>
>> Regards,
>> Andre
>>
>> On Mon, 19 Sep 2016 08:30:12 -0700
>> Alessandro Fanfarillo wrote:
>>
>>> * PING *
>>>
>>> On Sep 7, 2016 3:01 PM, "Alessandro Fanfarillo"
>>> wrote:
>>>
>>> > Dear all,
>>> > the attached patch supports failed images also when -fcoarray=single is
>>> > used.
>>> >
>>> > Built and regtested on x86_64-pc-linux-gnu.
>>> >
>>> > Cheers,
>>> > Alessandro
>>> >
>>> > 2016-08-09 5:22 GMT-06:00 Paul Richard Thomas <
>>> > paul.richard.tho...@gmail.com>:
>>> > > Hi Sandro,
>>> > >
>>> > > As far as I can see, this is OK barring a couple of minor wrinkles and
>>> > > a question:
>>> > >
>>> > > For coarray_failed_images_err.f90 and coarray_image_status_err.f90 you
>>> > > have used the option -fdump-tree-original without making use of the
>>> > > tree dump.
>>> > >
>>> > > Mikael asked you to provide an executable test with -fcoarray=single.
>>> > > Is this not possible for some reason?
>>> > >
>>> > > Otherwise, this is OK for trunk.
>>> > >
>>> > > Thanks for the patch.
>>> > >
>>> > > Paul
>>> > >
>>> > > On 4 August 2016 at 05:07, Alessandro Fanfarillo
>>> > > wrote:
>>> > >> * PING *
>>> > >>
>>> > >> 2016-07-21 13:05 GMT-06:00 Alessandro Fanfarillo <
>>> > fanfarillo@gmail.com>:
>>> > >>> Dear Mikael and all,
>>> > >>>
>>> > >>> in attachment the new patch, built and regtested on
>>> > x86_64-pc-linux-gnu.
>>> > >>>
>>> > >>> Cheers,
>>> > >>> Alessandro
>>> > >>>
>>> > >>> 2016-07-20 13:17 GMT-06:00 Mikael Morin :
>>> > Le 20/07/2016 à 11:39, Andre Vehreschild a écrit :
>>> > >
>>> > > Hi Mikael,
>>> > >
>>> > >
>>> > >>> + if(st == ST_FAIL_IMAGE)
>>> > >>> +new_st.op = EXEC_FAIL_IMAGE;
>>> > >>> + else
>>> > >>> +gcc_unreachable();
>>> > >>
>>> > >> You can use
>>> > >> gcc_assert (st == ST_FAIL_IMAGE);
>>> > >> foo...;
>>> > >> instead of
>>> > >> if (st == ST_FAIL_IMAGE)
>>> > >> foo...;
>>> > >> else
>>> > >> gcc_unreachable ();
>>> > >
>>> > >
>>> > > Be careful, this is not 100% identical in the general case. For
>>> > > older
>>> > > gcc version (gcc < 4008) gcc_assert() is mapped to nothing, esp. not
>>> > to
>>> > > an abort(), so the behavior can change. But in this case everything
>>> > is
>>> > > fine, because the patch is most likely not backported.
>>> > >
>>> > Didn't know about this. The difference seems to be very subtle.
>>> > I don't mind much anyway. The original version can stay if preferred,
>>> > this
>>> > was just a suggestion.
>>> >
>>> > By the way, if the function is inlined in its single caller, the
>>> > assert or
>>> > unreachable statement can be removed, which avoids choosing between
>>> > them.
>>> > That's another suggestion.
>>> >
>>> >
>>> > >>> +
>>> > >>> + return MATCH_YES;
>>> > >>> +
>>> > >>> + syntax:
>>> > >>> + gfc_syntax_error (st);
>>> > >>> +
>>> > >>> + return MATCH_ERROR;
>>> > >>> +}
>>> > >>> +
>>> > >>> +match
>>> > >>> +gfc_match_fail_image (void)
>>> > >>> +{
>>> > >>> + /* if (!gfc_notify_std (GFC_STD_F2008_TS, "FAIL IMAGE statement
>>> > >>> at %C")) */
>>> > >>> + /* return MATCH_ERROR; */
>>> > >>> +
>>> > >>
>>> > >> Can this be uncommented?
>>> > >>
>>> > >>> + return fail_image_statement (ST_FAIL_IMAGE);
>>> > >>> +}
>>> > >>>
>>> > >>> /* Match LOCK/UNLOCK statement. Syntax:
>>> > >>> LOCK ( lock-variable [ , lock-stat-list ] )
>>> > >>> diff --git a/gcc/fortran/trans-intrinsic.c
>>> > >>> b/gcc/fortran/trans-intrinsic.c index 1aaf4e2..b2f5596 100644
>>> > >>> --- a/gcc/fortran/trans-intrinsic.c
>>> > >>> +++ b/gcc/fortran/trans-intrinsic.c
>>> > >>> @@ -1647,6 +1647,24 @@ trans_this_image (gfc_se * se, gfc_expr
>>> > >>> *expr) m, lbound));
>>> > >>> }
>>> > >>>
>>> > >>> +static void
>>> > >>> +gfc_conv_intrinsic_image_status (gfc_se *se, gfc_expr *expr)
>>> > >>> +{
>>> > >>> + unsigned int num_args;
>>> > >>> + tree *args,tmp;
>>> > >>> +
>>> > >>> + num_args = gfc_intrinsic_argument_list_length (expr);
>>> > >>> + args = XALLOCAVEC (tree, num_args);
>>> > >>> +
>>> > >>>
[PATCH 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]
falign-functions=N is too simplistic. Ingo Molnar ran some tests and it seems that on latest x86 CPUs, 64-byte alignment of functions runs fastest (he tried many other possibilites): this way, after a call CPU can fetch a lot of insns in the first cacheline fill. However, developers are less than thrilled by the idea of a slam-dunk 64-byte aligning everything. Too much waste: On 05/20/2015 02:47 AM, Linus Torvalds wrote: > At the same time, I have to admit that I abhor a 64-byte function > alignment, when we have a fair number of functions that are (much) > smaller than that. > > Is there some way to get gcc to take the size of the function into > account? Because aligning a 16-byte or 32-byte function on a 64-byte > alignment is just criminally nasty and wasteful. This change makes it possible to align functions to 64-byte boundaries *if* this does not introduce huge amount of padding. Example syntax is -falign-functions=64,9: "align to 64 by skipping up to 9 bytes (not inclusive)". IOW: "after a call insn, CPU will always be able to fetch at least 9 bytes of insns". x86 had a tweak: -falign-functions=N with N > 8 was adding secondary alignment. For example, falign-functions=10 was emitting this before every function: .p2align 4,,9 .p2align 3 This tweak was removed by the previous patch. Now it is reinstated by the logic that if falign-functions=N[,M] is specified and N > 8, then default value of N2 is 8, not 1. But now this can be suppressed by falign-functions=N,M,1 - which wasn't possible before. In general, optional N2,M2 pair can be used to generate any secondary alignment user wants. Testing: Tested that with -falign-functions=N (tried 8, 15, 16, 17...) the alignment directives are the same before and after the patch. Tested that -falign-functions=N,N (two equal parameters) works exactly like -falign-functions=N. No change from past behavior: Tested that "-falign-functions" uses an arch-dependent alignment. Tested that "-O2" uses an arch-dependent alignment. Tested that "-O2 -falign-functions=N" uses explicitly given alignment. 2016-09-27 Denys Vlasenko * common.opt (-falign-functions=): Accept a string instead of integer. (-falign-jumps=): Likewise. (-falign-labels=): Likewise. (-falign-loops=): Likewise. * flags.h (struct target_flag_state): Revamp how alignment data is stored: for each of four alignment types, store two pairs of log/maxskip values. * toplev.c (read_uint): New function. (read_log_maxskip): New function. (parse_N_M): New function. (init_alignments): Set align_FOO[0/1].log/maxskip from specified falign-FOO=N[,M[,N[,M]]] options. * varasm.c (assemble_start_function): Call two ASM_OUTPUT_MAX_SKIP_ALIGN macros, first for N,M and second time for N2,M2 from falign-functions=N,M,N2,M2. This generates 0, 1, or 2 align directives. * config/aarch64/aarch64-protos.h (struct tune_params): Change foo_align members from integers to strings. * config/i386/i386.c (struct ptt): Likewise. * config/aarch64/aarch64.c (_tunings): Change foo_align field values from integers to strings. * config/i386/i386.c (processor_target_table): Likewise. * config/arm/arm.c (arm_override_options_after_change_1): Fix if() condition to detect that -falign-functions is specified. * config/i386/i386.c (ix86_default_align): Likewise. * common/config/i386/i386-common.c (ix86_handle_option): Remove conversion of malign_foo's to falign_foo's. The warning is still emitted. * doc/invoke.texi: Update option documentation. * testsuite/gcc.target/i386/falign-functions.c: New file. Index: gcc/common/config/i386/i386-common.c === --- gcc/common/config/i386/i386-common.c(revision 239860) +++ gcc/common/config/i386/i386-common.c(working copy) @@ -998,36 +998,17 @@ ix86_handle_option (struct gcc_options *opts, } return true; - - /* Comes from final.c -- no real reason to change it. */ -#define MAX_CODE_ALIGN 16 - case OPT_malign_loops_: warning_at (loc, 0, "-malign-loops is obsolete, use -falign-loops"); - if (value > MAX_CODE_ALIGN) - error_at (loc, "-malign-loops=%d is not between 0 and %d", - value, MAX_CODE_ALIGN); - else - opts->x_align_loops = 1 << value; return true; case OPT_malign_jumps_: warning_at (loc, 0, "-malign-jumps is obsolete, use -falign-jumps"); - if (value > MAX_CODE_ALIGN) - error_at (loc, "-malign-jumps=%d is not between 0 and %d", - value, MAX_CODE_ALIGN); - else - opts->x_align_jumps = 1 << value; return true; case OPT_malign_functions_: warning_at (loc, 0, "-malign-functions is obsolete, use -falign-functions"); - if (value > MAX_CODE_ALIGN) - error_at
[PATCH 0/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]] version 4
These patches are for this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66240 "RFE: extend -falign-xyz syntax" This is version 4 of the patch set. Changes since version 3: * Improved documentation in invoke.texi * Fixed x86-specific calculation of default N2 value: previous version was doing it incorrectly for cross-compile
[PATCH 1/2] Temporary remove "at least 8 byte alignment" code from x86
This change drops forced alignment to 8 if requested alignment is higher
than 8: before the patch, -falign-functions=9 was generating
.p2align 4,,8
.p2align 3
which means: "align to 16 if the skip is 8 bytes or less; else align to 8".
After this change, ".p2align 3" is not emitted.
This behavior will be implemented differently by the next patch.
The new SUBALIGN_LOG define will be used by the next patch.
2016-09-27 Denys Vlasenko
* config/i386/freebsd.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Remove "If N
is large, do at least 8 byte alignment" code. Add SUBALIGN_LOG
define.
* config/i386/gnu-user.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/iamcu.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/openbsdelf.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
* config/i386/x86-64.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Likewise.
Index: gcc/config/i386/freebsd.h
===
--- gcc/config/i386/freebsd.h (revision 239390)
+++ gcc/config/i386/freebsd.h (working copy)
@@ -92,25 +92,19 @@ along with GCC; see the file COPYING3. If not see
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
#endif
Index: gcc/config/i386/gnu-user.h
===
--- gcc/config/i386/gnu-user.h (revision 239390)
+++ gcc/config/i386/gnu-user.h (working copy)
@@ -94,24 +94,18 @@ along with GCC; see the file COPYING3. If not see
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
#endif
Index: gcc/config/i386/iamcu.h
===
--- gcc/config/i386/iamcu.h (revision 239390)
+++ gcc/config/i386/iamcu.h (working copy)
@@ -62,23 +62,17 @@ see the files COPYING3 and COPYING.RUNTIME respect
/* A C statement to output to the stdio stream FILE an assembler
command to advance the location counter to a multiple of 1<= (1<<(LOG))) \
+fprintf ((FILE), "\t.p2align %d\n", (LOG));\
+ else \
fprintf ((FILE), "\t.p2align %d,,%d\n", (LOG), (MAX_SKIP)); \
- /* Make sure that we have at least 8 byte alignment if > 8 byte \
- alignment is preferred. */ \
- if ((LOG) > 3 \
- && (1 << (LOG)) > ((MAX_SKIP) + 1) \
- && (MAX_SKIP) >= 7) \
- fputs ("\t.p2align 3\n", (FILE)); \
- }
\
} \
} while (0)
Index: gcc/config/i386/openbsdelf.h
===
--- gcc/config/i386/openbsdelf.h(revision 239390)
+++ gcc/config/i386/openbsdelf.h(working copy)
@@ -63,24 +63,18 @@ along with GCC; see the file COPYING3. If not see
/*
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 28/09/16 12:40 +, Joseph Myers wrote: On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: > On Tue, 27 Sep 2016, Jonathan Wakely wrote: > > > This adds the new 3D std::hypot() functions. This implementation seems > > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > > hypot(hypot(x, y), z), and should be a bit more accurate at very large > > or very small values due to reducing the arguments by the largest one. > > Improvements welcome though, as this is not my forte. > > Should I take it from this implementation that C++ is not concerned by > certain considerations that would arise for C: spurious underflow > exceptions from the scaling when some arguments much larger than others; > spurious "invalid" exceptions from the comparisons when any argument is > (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C > Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 As a further issue: even if you ignore exceptions and Annex F issues, and don't have NaN arguments, if you have an Inf argument it will be the largest, and so your code will do Inf/Inf, and so return NaN, if any argument is Inf (when obviously the correct answer in that case is Inf). I've opened https://gcc.gnu.org/PR6 but I don't plan to work on it.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Wed, 28 Sep 2016, Jonathan Wakely wrote: > On 27/09/16 23:28 +, Joseph Myers wrote: > > On Tue, 27 Sep 2016, Jonathan Wakely wrote: > > > > > This adds the new 3D std::hypot() functions. This implementation seems > > > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > > > hypot(hypot(x, y), z), and should be a bit more accurate at very large > > > or very small values due to reducing the arguments by the largest one. > > > Improvements welcome though, as this is not my forte. > > > > Should I take it from this implementation that C++ is not concerned by > > certain considerations that would arise for C: spurious underflow > > exceptions from the scaling when some arguments much larger than others; > > spurious "invalid" exceptions from the comparisons when any argument is > > (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C > > Annex F has Inf returned, not NaN)? > > The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 As a further issue: even if you ignore exceptions and Annex F issues, and don't have NaN arguments, if you have an Inf argument it will be the largest, and so your code will do Inf/Inf, and so return NaN, if any argument is Inf (when obviously the correct answer in that case is Inf). -- Joseph S. Myers jos...@codesourcery.com
Re: Implement -Wimplicit-fallthrough (version 9)
Hi, On Tue, 27 Sep 2016, Tom Tromey wrote: > The point of the warning is to make code more robust. But accepting any > comment like "Don't fall through" is not more robust, but rather an > error waiting to happen; as IIUC the user has no way to detect this > case. > > I think it's better for the comment-scanning feature to be very picky > (or even just not exist at all) -- that way you know exactly what you > are getting. Lint was traditionally picky IIRC. And, this is a warning > that isn't default and can also be disabled, so it's not as if users > have no recourse. Not accepting /* And here we intentionally fall through because ... */ and forcing users to replace this by: /* fallthrough */ is not robust either. It's actually actively lowering robustness of code, it creates work for programmers that will be regarded as pointless (and rightly so) and will merely lead to everybody disabling the warning (see our generated files) at which point we could just as well not have implemented it (which would be a shame because I think it's genuinely useful). The point of warnings is to make code robust under the condition of not being a pain by giving zillions of false positives. In this specific case the chance of giving false positives by being picky in how to disable the warning is very high. On the other hand the chance of unintentionally disabling the warning by a negative comment like "Don't fall through here" is low, because presumably the one adding that comment (and hence thinking about that part of the code) also in fact put in the "break;" afterwards. The argument with lint being picky would apply only if GCC would have added this warning maybe 20 years ago, not now where nearly nobody even knows what lint is, which lead to a large existing code base not having comments that would be accepted by lint but comments that do specify the intent of falling through. Ciao, Michael. P.S.: Initially I even wanted to argue that the mere existence of _any_ comment before a case label would disable the warning. I don't have the numbers but I bet even that version would have found the very same bugs that the picky version has.
Re: [BUILDROBOT] tic6x-uclinux: undefined reference to `gnu_libc_printf_pointer_format(tree_node*, char const**)' (was: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905))
On Tue, Sep 27, 2016 at 12:08:46AM +0200, Jan-Benedict Glaw wrote: > Hi Martin, > > On Thu, 2016-09-08 13:03:12 -0600, Martin Sebor wrote: > > Attached is another update to the patch to address the last round > > of comments and suggestions, most notably to: > [...] > > with the currently committed version, the tic6x-uclinux target fails, > see ie. > http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=630308 : [ snip ] The same happens for bfin-uclinux: g++ -no-pie -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -DHAVE_CONFIG_H -static-libstdc++ -static-libgcc -o cc1 c/c-lang.o c-family/stub-objc.o attribs.o c/c-errors.o c/c-decl.o c/c-typeck.o c/c-convert.o c/c-aux-info.o c/c-objc-common.o c/c-parser.o c/c-array-notation.o c/c-fold.o c-family/c-common.o c-family/c-cppbuiltin.o c-family/c-dump.o c-family/c-format.o c-family/c-gimplify.o c-family/c-indentation.o c-family/c-lex.o c-family/c-omp.o c-family/c-opts.o c-family/c-pch.o c-family/c-ppoutput.o c-family/c-pragma.o c-family/c-pretty-print.o c-family/c-semantics.o c-family/c-ada-spec.o c-family/c-cilkplus.o c-family/array-notation-common.o c-family/cilk.o c-family/c-ubsan.o default-c.o \ cc1-checksum.o libbackend.a main.o libcommon-target.a libcommon.a ../libcpp/libcpp.a ../libdecnumber/libdecnumber.a libcommon.a ../libcpp/libcpp.a ../libbacktrace/.libs/libbacktrace.a ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a -lmpc -lmpfr -lgmp -rdynamic -ldl -L./../zlib -lz bfin.o:(.data.rel+0x778): undefined reference to `gnu_libc_printf_pointer_format(tree_node*, char const**)' (that symbol would be defined in linux.o, but that file is not built for uclinux). Segher
[PATCH] Avoid store forwarding issue in vectorizing strided SLP loads
Currently strided SLP vectorization creates vector constructors composed
of vector elements. This is a constructor form that is not handled
specially by the expander but it gets expanded via piecewise stores
to scratch memory and a load of that scratch memory. This is obviously
bad on any modern CPU which can do store forwarding (which in this case
does not work on any CPU I know of). The following patch simply avoids
the issue by making the vectorizer create integer loads, composing
a vector of that integers and then punning that to the desired vector
type. Thus (V4SF){V2SF, V2SF} becomes (V4SF)(V2DI){DI, DI} and
every body is happy. Especially x264 gets a 5-10% improvement
(dependent on vector size and x86 sub-architecture).
Handling the vector-vector constructors on the expander side would
require either similar punning or making vec_init parametric on
the element mode plus supporting vector elements in all targets
(which in the end probably will simply pun them similarly).
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
Any comments?
Thanks,
Richard.
2016-09-28 Richard Biener
* tree-vect-stmts.c (vectorizable_load): Avoid emitting vector
constructors with vector elements.
Index: gcc/tree-vect-stmts.c
===
--- gcc/tree-vect-stmts.c (revision 240565)
+++ gcc/tree-vect-stmts.c (working copy)
@@ -6862,17 +6925,40 @@ vectorizable_load (gimple *stmt, gimple_
int nloads = nunits;
int lnel = 1;
tree ltype = TREE_TYPE (vectype);
+ tree lvectype = vectype;
auto_vec dr_chain;
if (memory_access_type == VMAT_STRIDED_SLP)
{
- nloads = nunits / group_size;
if (group_size < nunits)
{
- lnel = group_size;
- ltype = build_vector_type (TREE_TYPE (vectype), group_size);
+ /* Avoid emitting a constructor of vector elements by performing
+the loads using an integer type of the same size,
+constructing a vector of those and then re-interpreting it
+as the original vector type. This works around the fact
+that the vec_init optab was only designed for scalar
+element modes and thus expansion goes through memory.
+This avoids a huge runtime penalty due to the general
+inability to perform store forwarding from smaller stores
+to a larger load. */
+ unsigned lsize
+ = group_size * TYPE_PRECISION (TREE_TYPE (vectype));
+ enum machine_mode elmode = mode_for_size (lsize, MODE_INT, 0);
+ enum machine_mode vmode = mode_for_vector (elmode,
+nunits / group_size);
+ /* If we can't construct such a vector fall back to
+element loads of the original vector type. */
+ if (VECTOR_MODE_P (vmode)
+ && optab_handler (vec_init_optab, vmode) != CODE_FOR_nothing)
+ {
+ nloads = nunits / group_size;
+ lnel = group_size;
+ ltype = build_nonstandard_integer_type (lsize, 1);
+ lvectype = build_vector_type (ltype, nloads);
+ }
}
else
{
+ nloads = 1;
lnel = nunits;
ltype = vectype;
}
@@ -6925,9 +7011,17 @@ vectorizable_load (gimple *stmt, gimple_
}
if (nloads > 1)
{
- tree vec_inv = build_constructor (vectype, v);
- new_temp = vect_init_vector (stmt, vec_inv, vectype, gsi);
+ tree vec_inv = build_constructor (lvectype, v);
+ new_temp = vect_init_vector (stmt, vec_inv, lvectype, gsi);
new_stmt = SSA_NAME_DEF_STMT (new_temp);
+ if (lvectype != vectype)
+ {
+ new_stmt = gimple_build_assign (make_ssa_name (vectype),
+ VIEW_CONVERT_EXPR,
+ build1 (VIEW_CONVERT_EXPR,
+ vectype, new_temp));
+ vect_finish_stmt_generation (stmt, new_stmt, gsi);
+ }
}
if (slp)
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 28/09/16 13:31 +0200, Marc Glisse wrote: On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 It doesn't mention "without undue overflow or underflow" as the C standard does for hypot(x, y). Handling those conditions would of course be nice, but I'm not capable of doing so, and I'd rather have a poor implementation than none. p0030r0 had "In addition to the two-argument versions of certain math functions in and its float and long double overloads, C++ adds three-argument versions of these functions that follow similar semantics as their two-argument counterparts." One could interpret that as an intent to follow the C semantics. On the other hand, the paper mentions speed and convenience as the main selling points. Maybe LWG could clarify the intent? I expect the answer will be that it's QoI.
[PATCH][2/2] Fix PR77768
The following patch makes VN not choke on code that stores to readonly
memory it knows the constant value of. I took the liberty to clean up
the surrounding code a bit as well.
Bootstrap / regtest in progress on x86_64-unknown-linux-gnu.
Richard.
2016-09-28 Richard Biener
PR tree-optimization/77768
* tree-ssa-sccvn.c (visit_reference_op_store): Properly deal
with stores to a place we know has a constant value.
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 240566)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -3575,7 +3575,7 @@ visit_reference_op_store (tree lhs, tree
{
bool changed = false;
vn_reference_t vnresult = NULL;
- tree result, assign;
+ tree assign;
bool resultsame = false;
tree vuse = gimple_vuse (stmt);
tree vdef = gimple_vdef (stmt);
@@ -3599,9 +3599,11 @@ visit_reference_op_store (tree lhs, tree
Otherwise, the vdefs for the store are used when inserting into
the table, since the store generates a new memory state. */
- result = vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false);
- if (result)
+ vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false);
+ if (vnresult
+ && vnresult->result)
{
+ tree result = vnresult->result;
if (TREE_CODE (result) == SSA_NAME)
result = SSA_VAL (result);
resultsame = expressions_equal_p (result, op);
@@ -3616,22 +3618,21 @@ visit_reference_op_store (tree lhs, tree
}
}
- if ((!result || !resultsame)
+ if (!resultsame)
+{
/* Only perform the following when being called from PRE
which embeds tail merging. */
- && default_vn_walk_kind == VN_WALK)
-{
- assign = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, op);
- vn_reference_lookup (assign, vuse, VN_NOWALK, &vnresult, false);
- if (vnresult)
+ if (default_vn_walk_kind == VN_WALK)
{
- VN_INFO (vdef)->use_processed = true;
- return set_ssa_val_to (vdef, vnresult->result_vdef);
+ assign = build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, op);
+ vn_reference_lookup (assign, vuse, VN_NOWALK, &vnresult, false);
+ if (vnresult)
+ {
+ VN_INFO (vdef)->use_processed = true;
+ return set_ssa_val_to (vdef, vnresult->result_vdef);
+ }
}
-}
- if (!result || !resultsame)
-{
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "No store match\n");
@@ -3644,9 +3645,7 @@ visit_reference_op_store (tree lhs, tree
/* Have to set value numbers before insert, since insert is
going to valueize the references in-place. */
if (vdef)
- {
- changed |= set_ssa_val_to (vdef, vdef);
- }
+ changed |= set_ssa_val_to (vdef, vdef);
/* Do not insert structure copies into the tables. */
if (is_gimple_min_invariant (op)
[PATCH][1/2] Fix PR77768
I am testing the following patch to avoid useless VRP range allocations
when we just ask for varying on stmts we don't know how to handle.
I think it should fix the PR where we end up assigning to the
static const vr_const_varying returned by get_value_range in the early VRP
context.
Eventually the VRP lattice needs to become sparse (or just growable, I
have a patch for that as well). Not until early VRP gets some more
capabilities though, it would be a waste otherwise.
Bootstrap / regtest in progress on x86_64-unknown-linux-gnu.
Richard.
2016-09-28 Richard Biener
* tree-vrp.c (set_defs_to_varying): New helper.
(vrp_initialize): Call it.
(vrp_visit_stmt): Likewise.
(evrp_dom_walker::before_dom_children): Likewise.
Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c (revision 240568)
+++ gcc/tree-vrp.c (working copy)
@@ -343,6 +343,24 @@ set_value_range_to_varying (value_range
}
+/* Set value-ranges of all SSA names defined by STMT to varying. */
+
+static void
+set_defs_to_varying (gimple *stmt)
+{
+ ssa_op_iter i;
+ tree def;
+ FOR_EACH_SSA_TREE_OPERAND (def, stmt, i, SSA_OP_DEF)
+{
+ unsigned ver = SSA_NAME_VERSION (def);
+ /* Avoid needlessly allocating value-ranges. */
+ if (num_vr_values >= ver
+ && vr_value[ver])
+ set_value_range_to_varying (vr_value[ver]);
+}
+}
+
+
/* Set value range VR to {T, MIN, MAX, EQUIV}. */
static void
@@ -7022,10 +7040,7 @@ vrp_initialize ()
prop_set_simulate_again (stmt, true);
else if (!stmt_interesting_for_vrp (stmt))
{
- ssa_op_iter i;
- tree def;
- FOR_EACH_SSA_TREE_OPERAND (def, stmt, i, SSA_OP_DEF)
- set_value_range_to_varying (get_value_range (def));
+ set_defs_to_varying (stmt);
prop_set_simulate_again (stmt, false);
}
else
@@ -7901,8 +7916,6 @@ vrp_visit_stmt (gimple *stmt, edge *take
{
value_range vr = VR_INITIALIZER;
tree lhs = gimple_get_lhs (stmt);
- tree def;
- ssa_op_iter iter;
extract_range_from_stmt (stmt, taken_edge_p, output_p, &vr);
if (*output_p)
@@ -7997,8 +8010,7 @@ vrp_visit_stmt (gimple *stmt, edge *take
/* All other statements produce nothing of interest for VRP, so mark
their outputs varying and prevent further simulation. */
- FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
-set_value_range_to_varying (get_value_range (def));
+ set_defs_to_varying (stmt);
return (*taken_edge_p) ? SSA_PROP_INTERESTING : SSA_PROP_VARYING;
}
@@ -10726,12 +10738,7 @@ evrp_dom_walker::before_dom_children (ba
&& (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
update_value_range (output, &vr);
else
- {
- tree def;
- ssa_op_iter iter;
- FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
- set_value_range_to_varying (get_value_range (def));
- }
+ set_defs_to_varying (stmt);
/* Try folding stmts with the VR discovered. */
bool did_replace
@@ -10780,12 +10787,7 @@ evrp_dom_walker::before_dom_children (ba
}
}
else
- {
- tree def;
- ssa_op_iter iter;
- FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
- set_value_range_to_varying (get_value_range (def));
- }
+ set_defs_to_varying (stmt);
}
bb->flags |= BB_VISITED;
return NULL;
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 It doesn't mention "without undue overflow or underflow" as the C standard does for hypot(x, y). Handling those conditions would of course be nice, but I'm not capable of doing so, and I'd rather have a poor implementation than none. p0030r0 had "In addition to the two-argument versions of certain math functions in and its float and long double overloads, C++ adds three-argument versions of these functions that follow similar semantics as their two-argument counterparts." One could interpret that as an intent to follow the C semantics. On the other hand, the paper mentions speed and convenience as the main selling points. Maybe LWG could clarify the intent? -- Marc Glisse
Re: internal fn pretty printing
On 09/27/16 09:30, Bernd Schmidt wrote:
On 09/27/2016 12:58 PM, Nathan Sidwell wrote:
In working on some new code I got sufficiently frustrated to implement
pretty printing on internal function discriminators, as I think one of
you suggested a while back. With this patch we get:
.data_dep.2 = UNIQUE (OACC_FORK, .data_dep.2, -1);
rather than an obscure raw integer for the first argument.
I realized I could simplify the comma separator logic, by checking i for
non-zeroness before emitting the arg. Committed as obvious.
nathan
2016-09-28 Nathan Sidwell
* gimple-pretty-print.c (dump_gimple_call_args): Simplify "' "
printing.
Index: gimple-pretty-print.c
===
--- gimple-pretty-print.c (revision 240552)
+++ gimple-pretty-print.c (working copy)
@@ -649,26 +649,21 @@ dump_gimple_call_args (pretty_printer *b
{
i++;
pp_string (buffer, enums[v]);
- if (i < gimple_call_num_args (gs))
- pp_string (buffer, ", ");
}
}
}
for (; i < gimple_call_num_args (gs); i++)
{
- dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);
- if (i < gimple_call_num_args (gs) - 1)
+ if (i)
pp_string (buffer, ", ");
+ dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);
}
if (gimple_call_va_arg_pack_p (gs))
{
- if (gimple_call_num_args (gs) > 0)
-{
- pp_comma (buffer);
- pp_space (buffer);
-}
+ if (i)
+ pp_string (buffer, ", ");
pp_string (buffer, "__builtin_va_arg_pack ()");
}
[PATCH] [ARC COMMITTED] MAINTAINERS (Reviewers): Add myself.
From: claziss 2016-09-28 Claudiu Zissulescu * MAINTAINERS (Reviewers): Add myself. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240569 138bc75d-0d04-0410-961f-82ee72b054a4 --- ChangeLog | 4 MAINTAINERS | 1 + 2 files changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index a51606c..e3dc6b7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2016-09-28 Claudiu Zissulescu + + * MAINTAINERS (Reviewers): Add myself. + 2016-09-26 Anton Kolesov * configure.ac: Disable "sim" directory for arc*-*-*. diff --git a/MAINTAINERS b/MAINTAINERS index 87157c9..644d02e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -276,6 +276,7 @@ check in changes outside of the parts of the compiler they maintain. Reviewers aarch64 port James Greenhalgh +arc port Claudiu Zissulescu arm port Kyrylo Tkachov C front endMarek Polacek dataflow Paolo Bonzini -- 1.9.1
Re: [PATCH] libstdc++/77686 use may_alias for std::function storage
On 28/09/16 13:14 +0200, Richard Biener wrote: On Wed, Sep 28, 2016 at 12:57 PM, Jonathan Wakely wrote: std::function::swap does swap(_M_functor, x._M_functor) which exchanges the underlying bytes of the two _Any_data PODs using that type's implicit assignment operator. However, unlike using placement new to construct an object in the storage, simply memcpying the bytes doesn't change the effective type of the storage, so alias analysis decides it can't point to whatever we've tried to store in there. To clarify -- the implicit assingment operator for PODs (including unions) simply expands to an aggregate assignment which is subject to TBAA rules and thus in this case instantiates an effective type of _Any_data. Using memcpy would have worked as memcpy _does_ transfer the effective type of the storage. It wasn't points-to deciding things here but TBAA given automatic storage X with effective type T read via an lvalue of type _Any_data. This attribute tells the middle-end to assume anything those bytes could contain any type of object, which is exactly what we want. It tells the middle-end that accessing storage via a _pointer_ to such marked type may access storage with a dynamic type that is not compatible with the type. Details ;) But important ones, thanks.
Re: [PATCH] libstdc++/77686 use may_alias for std::function storage
On Wed, Sep 28, 2016 at 12:57 PM, Jonathan Wakely wrote: > std::function::swap does swap(_M_functor, x._M_functor) which > exchanges the underlying bytes of the two _Any_data PODs using that > type's implicit assignment operator. However, unlike using placement > new to construct an object in the storage, simply memcpying the bytes > doesn't change the effective type of the storage, so alias analysis > decides it can't point to whatever we've tried to store in there. To clarify -- the implicit assingment operator for PODs (including unions) simply expands to an aggregate assignment which is subject to TBAA rules and thus in this case instantiates an effective type of _Any_data. Using memcpy would have worked as memcpy _does_ transfer the effective type of the storage. It wasn't points-to deciding things here but TBAA given automatic storage X with effective type T read via an lvalue of type _Any_data. > This attribute tells the middle-end to assume anything those bytes > could contain any type of object, which is exactly what we want. It tells the middle-end that accessing storage via a _pointer_ to such marked type may access storage with a dynamic type that is not compatible with the type. Details ;) Btw, I think the patch is correct. Richard. > PR libstdc++/77686 > * include/std/functional (_Any_data): Add may_alias attribute. > > Tested powerpc64le-linux, committing to trunk and gcc-6-branch. > > std::any doesn't have the same problem, because I defined an _Op_xfer > operation that uses placement new to copy the object into the > destination, so the dynamic type is changed correctly. I haven't > checked whether optional and variant might have similar problems in > any implicit copy/move operations working on POD storage. > >
