[patch, libgfortran] PR56743 Namelist bug with comment and no blank
The attached patch works by adding the comment character to the set of
characters treated as separators. This works because a namelist comment
abstractly can be though of as equivalent to an end of line character. For
namelist the '!' character is already handled in the 'eat_separators' helper
function, so the patch s really trivial.
The patch does not address character data that is not quote delimited. Besides
the fact that using character data in namelists without quotes leaves a very bad
taste in my mouth, I have not had time to look into it further in that area.
Regression tested on x86-64. I will dejanuize the test case attached.
OK for trunk?
Jerry
2015-04-17 Jerry DeLisle
PR libgfortran/56743
* io/list_read.c (CASE_SEPARATORS): Add case for '!'.
(is_separator): Add condition for '!'.
Index: io/list_read.c
===
--- io/list_read.c (revision 222194)
+++ io/list_read.c (working copy)
@@ -53,12 +53,12 @@ typedef unsigned char uchar;
case '5': case '6': case '7': case '8': case '9'
#define CASE_SEPARATORS case ' ': case ',': case '/': case '\n': case '\t': \
- case '\r': case ';'
+ case '\r': case ';': case '!'
/* This macro assumes that we're operating on a variable. */
#define is_separator(c) (c == '/' || c == ',' || c == '\n' || c == ' ' \
- || c == '\t' || c == '\r' || c == ';')
+ || c == '\t' || c == '\r' || c == ';' || c == '!')
/* Maximum repeat count. Less than ten times the maximum signed int32. */
! { dg-do run }
!
! PR fortran/56743
!
! Contributed by Kai Gallmeister
!
! Note that Fortran 2008 (Section 10.11.3.6) requires that there is
! a value separator between the value and the "!". Thus, all examples
! in this file are invalid; they should either by accepted as vendor
! extension or lead to a run-time error (iostat /=0).
!
! For the c1 and c2 character example, please note that the Fortran
! standard (F2008, 10.11.3.3) requires delimiters; accepting
! a single word (in spirit of list-directed I/O) would be possible
! as vendor extension. But the current run-time failure is fine as well.
!
implicit none
integer :: i = -1
real :: r1 = -2
real :: r2 = -3
real :: r3 = -4
real :: r4 = -5
real :: r5 = -6
complex :: c = (-7,-7)
logical :: ll = .false.
character :: c1 = 'X'
character(3) :: c2 = 'YYY'
character(3) :: c3 = 'ZZZ'
namelist /nml/ i, r1,r2,r3,r4,r5,c,ll,c1,c2,c3
write (*, nml=nml)
open (99, file='nml.dat', status="replace")
write(99,*) "&nml"
write(99,*) " i=42!11" ! BUG: wrong result: Unmodified, no error
write(99,*) " r1=43!11"! BUG: wrong result: Unmodified, no error
write(99,*) " r2=43.!11" ! BUG: wrong result: Unmodified, no error
write(99,*) " r3=inf!11" ! OK: run-time error (Cannot match namelist object)
write(99,*) " r4=NaN(0x33)!11" ! OK: run-time error (Cannot match namelist object)
write(99,*) " r5=3.e5!11" ! BUG: wrong result: Unmodified, no error
write(99,*) " c=(4,2)!11" ! OK: value accepted as vendor extension
write(99,*) " ll=.true.!11"! OK: value accepted as vendor extension
write(99,*) " c1='a'!11" ! OK: run-time error (Cannot match namelist object)
write(99,*) " c2='bc'!11" ! OK: run-time error (Cannot match namelist object)
write(99,*) " c3='ax'!11" ! OK: value accepted as vendor extension
write(99,*) "/"
rewind(99)
read (99, nml=nml)
write (*, nml=nml)
close (99)
if (r1 /= 43) call abort ()
if (r2 /= 43) call abort ()
if (r3 /= r3 .or. r3 <= huge(r3)) call abort ()
if (r4 == r4) call abort ()
if (r5 /= 30) call abort ()
if (c /= cmplx(4,2)) call abort ()
if (.not. ll) call abort ()
if (c1 /= "a") call abort ()
if (c2 /= "bc") call abort ()
if (c3 /= "ax") call abort ()
end
Re: [PATCH] -Warray-bounds TLC
On Sat, 2015-04-18 at 00:15 +0200, Marc Glisse wrote:
> >
> > extern void bad (const char *__assertion) __attribute__ ((__noreturn__));
> > struct link_map { long int l_ns; };
> > extern struct link_namespaces
> > {
> >unsigned int _ns_nloaded;
> > } _dl_ns[1];
> > void _dl_close_worker (struct link_map *map)
> > {
> > long int nsid = map->l_ns;
> > struct link_namespaces *ns = &_dl_ns[nsid];
> > (nsid != 0) ? (void) (0) : bad ("nsid != 0");
> > --ns->_ns_nloaded;
> > }
>
> It looks close enough to me. The actual access to _dl_ns[nsid] only ever
> happens for an index that is out of range. The last line of the function
> can never make sense (unreachable or undefined behavior), it is good that
> the compiler tells you about it.
I guess, but it left me very confused because the compiler didn't point
me at the last line, it pointed me at the '*ns = &_dl_ns[nsid]' line.
If there was a lot of stuff in between that line, the line with the call
to the noreturn function, and the ns->ns_loaded line (like there is in
the real glibc), it is very hard to understand what the compiler is
trying to tell me when it only points out the first line as where the
error is.
Steve Ellcey
Re: [PATCH] -Warray-bounds TLC
On Fri, 17 Apr 2015, Steve Ellcey wrote:
As a follow-up, I got the same error with dl-close.c from glibc and
assumed it was the same type of code but when I looked at it and cut it
down I got this code and error. This seems more like a real GCC error
(in that it should not be warning). Note that I only get the error when
bad is declared as 'noreturn'.
Steve Ellcey
sell...@imgtec.com
extern void bad (const char *__assertion) __attribute__ ((__noreturn__));
struct link_map { long int l_ns; };
extern struct link_namespaces
{
unsigned int _ns_nloaded;
} _dl_ns[1];
void _dl_close_worker (struct link_map *map)
{
long int nsid = map->l_ns;
struct link_namespaces *ns = &_dl_ns[nsid];
(nsid != 0) ? (void) (0) : bad ("nsid != 0");
--ns->_ns_nloaded;
}
It looks close enough to me. The actual access to _dl_ns[nsid] only ever
happens for an index that is out of range. The last line of the function
can never make sense (unreachable or undefined behavior), it is good that
the compiler tells you about it.
--
Marc Glisse
libgo patch committed: in runtime.Caller don't return ok as true if PC == 0
PR 65798 says that in some cases runtime.Caller can return with ok == true when PC == 0. It's not clear to me quite how that can happen, but it's easy to avoid with this patch. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and GCC 5 branch. Ian diff -r 400258017261 libgo/runtime/go-caller.c --- a/libgo/runtime/go-caller.c Fri Apr 17 14:27:33 2015 -0700 +++ b/libgo/runtime/go-caller.c Fri Apr 17 14:55:44 2015 -0700 @@ -166,7 +166,7 @@ runtime_memclr (&ret, sizeof ret); n = runtime_callers (skip + 1, &loc, 1, false); - if (n < 1) + if (n < 1 || loc.pc == 0) return ret; ret.pc = loc.pc; ret.file = loc.filename;
Re: [PATCH, stage1] Better error recovery for merge-conflict markers
On Fri, 2015-03-20 at 17:50 +, Joseph Myers wrote:
> On Fri, 20 Mar 2015, David Malcolm wrote:
>
> > I believe that the presense of these markers in source code is almost
> > always a bug (are there any GCC frontends in which the markers are
> > parsable as something valid?)
>
> Well, obviously they are valid inside #if 0, strings (where you have a
> test, though not one at start of line "\
> <<<") and comments (where you don't have a test). They are also valid
> when stringized:
>
> #define str(s) #s
> const char *s = str(
> <<<
> );
>
> must be accepted. They are also valid in the expansion of a macro that
> doesn't get expanded.
>
> #define foo \
> <<<
>
> That is, in general, the invalidity only occurs when preprocessing tokens
> are converted to tokens.
>
> In C++ (C++11 and later), >>> can also close a sequence of nested
> template argument lists, thanks to the rule about replacing >> by > > in
> that context. And of course it's OK, if odd, to put that at the start of
> a line. So in that case the preprocessing tokens do get converted to
> tokens, and that token sequence (interpreted as >> >> >> > and then
> contextually adjusted to > > > > > > >) is valid.
Thanks.
It seems that the libcpp approach from my original attempt is untenable,
so I've rewritten the patch.
Attached is v2 of the patch, which instead does things in the C and C++
frontends.
Specifically, it adds special-case detection of patch conflict markers
to c_parser_error and cp_parser_error. It only affects places where we
were going to emit an error anyway, and checks to see if we have a patch
conflict marker. If we do, it simply changes the error message to
better describe the problem, eliminating the:
expected identifier or ‘(’ before ‘<<’ token
gobbledydook in favor of:
source file contains patch conflict marker
Hence this should not break any existing code, it should merely give
better error messages.
I added testcases to cover the situations you described in your mail.
Successfully bootstrapped®rtested on x86_64-unknown-linux-gnu
(Fedora 20), with:
27 new "PASS" results in gcc.sum
82 new "PASS" results in g++.sum
for the new test cases (relative to a control build of r221492).
OK for trunk? (for GCC 6)
gcc/c-family/ChangeLog:
* c-common.h (conflict_marker_get_final_tok_kind): New prototype.
* c-lex.c (conflict_marker_get_final_tok_kind): New function.
gcc/c/ChangeLog:
* c-parser.c (struct c_parser): Expand array "tokens_buf" from 2
to 4.
(c_parser_peek_nth_token): New function.
(c_parser_peek_conflict_marker): New function.
(c_parser_error): Detect patch conflict markers and report them as
such.
gcc/cp/ChangeLog:
* parser.c (cp_lexer_peek_conflict_marker): New function.
(cp_parser_error): Detect patch conflict markers and report them
as such.
gcc/testsuite/ChangeLog:
* c-c++-common/patch-conflict-markers-1.c: New testcase.
* c-c++-common/patch-conflict-markers-2.c: Likewise.
* c-c++-common/patch-conflict-markers-3.c: Likewise.
* c-c++-common/patch-conflict-markers-4.c: Likewise.
* c-c++-common/patch-conflict-markers-5.c: Likewise.
* c-c++-common/patch-conflict-markers-6.c: Likewise.
* c-c++-common/patch-conflict-markers-7.c: Likewise.
* c-c++-common/patch-conflict-markers-8.c: Likewise.
* c-c++-common/patch-conflict-markers-9.c: Likewise.
* g++.dg/patch-conflict-markers-1.C: Likewise.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5b2c5ab..383a4c7 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1064,6 +1064,10 @@ extern void c_genericize (tree);
extern int c_gimplify_expr (tree *, gimple_seq *, gimple_seq *);
extern tree c_build_bind_expr (location_t, tree, tree);
+/* In c-lex.c. */
+extern enum cpp_ttype
+conflict_marker_get_final_tok_kind (enum cpp_ttype tok1_kind);
+
/* In c-pch.c */
extern void pch_init (void);
extern void pch_cpp_save_state (void);
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index bb55be8..387f20e 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -1275,3 +1275,29 @@ lex_charconst (const cpp_token *token)
return value;
}
+
+/* Helper function for c_parser_peek_conflict_marker
+ and cp_lexer_peek_conflict_marker.
+ Given a possible patch conflict marker token of kind TOK1_KIND
+ consisting of a pair of characters, get the token kind for the
+ standalone final character. */
+
+enum cpp_ttype
+conflict_marker_get_final_tok_kind (enum cpp_ttype tok1_kind)
+{
+ switch (tok1_kind)
+{
+default: gcc_unreachable ();
+case CPP_LSHIFT:
+ /* "<<" and '<' */
+ return CPP_LESS;
+
+case CPP_EQ_EQ:
+ /* "==" and '=' */
+ return CPP_EQ;
+
+case CPP_RSHIFT:
+ /* ">>" and '>' */
+ return CPP_GREATER;
+}
+}
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser
Re: [PATCH] PR target/65780: [5/6 Regression] Uninitialized common handling in executables
On Fri, Apr 17, 2015 at 02:38:01PM -0700, H.J. Lu wrote: > Please add PR target/65780 line to the ChangeLog entry. Ok for trunk and 5 branch with that change, thanks. > * config/i386/i386.c (ix86_binds_local_p): Define only if > TARGET_MACHO and TARGET_DLLIMPORT_DECL_ATTRIBUTES are false. > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index da69186..d870ab8 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -51737,6 +51737,7 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, > tree *stmts) >return 2; > } > > +#if !TARGET_MACHO && !TARGET_DLLIMPORT_DECL_ATTRIBUTES > /* For i386, common symbol is local only for non-PIE binaries. For > x86-64, common symbol is local only for non-PIE binaries or linker > supports copy reloc in PIE binaries. */ > @@ -51749,6 +51750,7 @@ ix86_binds_local_p (const_tree exp) > || (TARGET_64BIT > && HAVE_LD_PIE_COPYRELOC != 0))); > } > +#endif > > /* Initialize the GCC target structure. */ > #undef TARGET_RETURN_IN_MEMORY Jakub
Re: [PATCH] PR target/65780: [5/6 Regression] Uninitialized common handling in executables
On Fri, Apr 17, 2015 at 9:12 AM, Jeff Law wrote:
> On 04/16/2015 12:57 PM, H.J. Lu wrote:
>>
>> Uninitialized common symbol behavior in executables is target and linker
>> dependent. default_binds_local_p_3 is made public and updated to take an
>> argument to indicate if common symbol may be local. If common symbol
>> may be local, default_binds_local_p_3 will treat non-external variable
>> as defined locally. default_binds_local_p_2 is changed to treat common
>> symbol as local for non-PIE binaries.
>>
>> For i386, common symbol is local only for non-PIE binaries. For x86-64,
>> common symbol is local only for non-PIE binaries or linker supports copy
>> reloc in PIE binaries. If a target treats common symbol as local only
>> for non-PIE binaries, it can define TARGET_BINDS_LOCAL_P as
>> default_binds_local_p_2.
>>
>> Tested on Linux/x86-64 using -m32 with binutils master and 2.24. OK for
>> trunk and 5 branch?
>>
>>
>> H.J.
>> ---
>> gcc/
>>
>> PR target/65780
>> * output.h (default_binds_local_p_3): New.
>> * varasm.c (default_binds_local_p_3): Make it public. Take an
>> argument to indicate if common symbol may be local. If common
>> symbol may be local, treat non-external variable as defined
>> locally.
>> (default_binds_local_p_2): Pass !flag_pic to
>> default_binds_local_p_3.
>> (default_binds_local_p_1): Pass false to default_binds_local_p_3.
>> * config/i386/i386.c (ix86_binds_local_p): New.
>> (TARGET_BINDS_LOCAL_P): Replace default_binds_local_p_2 with
>> ix86_binds_local_p.
>>
>> gcc/testsuite/
>>
>> PR target/65780
>> * gcc.dg/pr65780-1.c: New test.
>> * gcc.dg/pr65780-2.c: Likewise.
>> * gcc.target/i386/pr32219-9.c: Likewise.
>> * gcc.target/i386/pr32219-1.c (xxx): Make it initialized common
>> symbol.
>> * gcc.target/i386/pr64317.c (c): Initialize.
>
> I approved the slightly different version of this attached to pr 65780,
> c#35.
>
This patch is needed for MacOS and Windows. OK for trunk
and 5 branch?
--
H.J.
* config/i386/i386.c (ix86_binds_local_p): Define only if
TARGET_MACHO and TARGET_DLLIMPORT_DECL_ATTRIBUTES are false.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index da69186..d870ab8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -51737,6 +51737,7 @@ ix86_initialize_bounds (tree var, tree lb, tree ub, tree *stmts)
return 2;
}
+#if !TARGET_MACHO && !TARGET_DLLIMPORT_DECL_ATTRIBUTES
/* For i386, common symbol is local only for non-PIE binaries. For
x86-64, common symbol is local only for non-PIE binaries or linker
supports copy reloc in PIE binaries. */
@@ -51749,6 +51750,7 @@ ix86_binds_local_p (const_tree exp)
|| (TARGET_64BIT
&& HAVE_LD_PIE_COPYRELOC != 0)));
}
+#endif
/* Initialize the GCC target structure. */
#undef TARGET_RETURN_IN_MEMORY
libgo patch committed: Skip runtime functions with no name in runtime/pprof
GCC PR 65797 causes some of the runtime functions to be compiled with
no name in the debug info. This in turn causes the runtime/pprof test
to fail as reported in GCC PR 64683.
There are no good choices when a function has no name in the debug
info, but this patch assumes that if we see such a function while
reading the runtime functions, we assume that it is also a runtime
function.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and GCC 5 branch.
Ian
diff -r df28882adbdf libgo/go/runtime/pprof/pprof.go
--- a/libgo/go/runtime/pprof/pprof.go Fri Apr 17 12:28:57 2015 -0700
+++ b/libgo/go/runtime/pprof/pprof.go Fri Apr 17 14:24:43 2015 -0700
@@ -351,6 +351,10 @@
if !show && !strings.Contains(name, ".") &&
strings.HasPrefix(name, "__go_") {
continue
}
+ if !show && name == "" {
+ // This can happen due to
http://gcc.gnu.org/PR65797.
+ continue
+ }
show = true
fmt.Fprintf(w, "#\t%#x\t%s+%#x\t%s:%d\n", pc, name,
pc-f.Entry(), file, line)
}
Re: libgo patch committed: Adjust libbacktrace PC value in runtime_callers callback
On Fri, Apr 17, 2015 at 1:03 PM, wrote: > On 04/17/2015 01:29 PM, Ian Lance Taylor wrote: >> >> The libbacktrace library returns a PC that was (usually) decremented >> to be part of the call instruction. The Go code that uses >> runtime.Callers does not expect this, and Go code that adjusts the PC >> value, such as libgo/go/runtime/pprof/pprof.go, can get fooled by it. >> This leads to GCC PRs 64999 and 65180. > > There are a couple of bugs in libsanitizer with symptoms similar to > those noted in 65180: 65749 and the related 65479. I've been looking > into these but don't have a patch yet. What I've found is that using > the decremented PC leads to the wrong line numbers on some targets > (e.g, powerpc) while using the correct PC causes the same problem on > others (such as x86_64) as a result of differences in the DWARF line > programs emitted on these targets. I don't know what your options are for libsanitizer. For libgo I am somewhat constrained by not wanting to vary too far from the master library. That causes problems because the master library thinks that a single PC value can represent a function/file/line, which is not correct in the presence of inlined functions. If libsanitizer is also using a single PC value, then there isn't much you can do. But if libsanitizer can work with the full function/file/line information returned by backtrace_full, then I think you should always have the correct information even if the precisely correct PC is difficult to know. Ian
Re: [PATCH] -Warray-bounds TLC
On Apr 17, 2015, at 1:52 PM, Steve Ellcey wrote:
> struct link_namespaces *ns = &_dl_ns[nsid];
> (nsid != 0) ? (void) (0) : bad ("nsid != 0”);
Without disagreeing with the fact this looks like a bug, ideally, the two lines
above would be switched:
c++98:
If both the pointer operand and the result point to elements
of the same array object, or one past the last element of the array
object, the evaluation shall not produce an overflow; otherwise, the
behavior is undefined.
and c99 tc3:
8 When an expression that has integer type is added to or subtracted from a
pointer, the
result has the type of the pointer operand. If the pointer operand points
to an element of
an array object, and the array is large enough, the result points to an
element offset from
the original element such that the difference of the subscripts of the
resulting and original
array elements equals the integer expression. In other words, if the
expression P points to
the i-th element of an array object, the expressions (P)+N (equivalently,
N+(P)) and
(P)-N (where N has the value n) point to, respectively, the i+n-th and
i-n-th elements of
the array object, provided they exist. Moreover, if the expression P points
to the last
element of an array object, the expression (P)+1 points one past the last
element of the
array object, and if the expression Q points one past the last element of
an array object,
the expression (Q)-1 points to the last element of the array object. If
both the pointer
operand and the result point to elements of the same array object, or one
past the last
element of the array object, the evaluation shall not produce an overflow;
otherwise, the
behavior is undefined.
Re: [PATCH 1/2] PR c++/61636
On 2015-04-17 20:58, Jason Merrill wrote: On 04/09/2015 11:31 PM, Adam Butcher wrote: + /* For generic lambdas, resolve default captured 'this' now. */ This isn't quite right. We don't want to capture 'this' any time we see a member function call, as overload resolution might choose a static member function that doesn't need 'this'. The special handling we want is for the case where the call depends on a generic lambda parameter, in which case we capture 'this' because the call "names [this] in a potentially-evaluated expression (3.2) where the enclosing full-expression depends on a generic lambda parameter declared within the reaching scope of the lambda-expression." Good point. I'll look into it. So for a nullary member call we will always capture 'this', but for N-ary, we only capture if we find one of the lambda's parameters (or a parameter from an enclosing generic lambda?) in the call's arguments right? Cheers, Adam
Re: [PATCH] -Warray-bounds TLC
As a follow-up, I got the same error with dl-close.c from glibc and
assumed it was the same type of code but when I looked at it and cut it
down I got this code and error. This seems more like a real GCC error
(in that it should not be warning). Note that I only get the error when
bad is declared as 'noreturn'.
Steve Ellcey
sell...@imgtec.com
extern void bad (const char *__assertion) __attribute__ ((__noreturn__));
struct link_map { long int l_ns; };
extern struct link_namespaces
{
unsigned int _ns_nloaded;
} _dl_ns[1];
void _dl_close_worker (struct link_map *map)
{
long int nsid = map->l_ns;
struct link_namespaces *ns = &_dl_ns[nsid];
(nsid != 0) ? (void) (0) : bad ("nsid != 0");
--ns->_ns_nloaded;
}
% inst*/bin/*-gcc -O2 -Wall -Werror x.c
x.c: In function '_dl_close_worker':
x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds]
struct link_namespaces *ns = &_dl_ns[nsid];
^
x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds]
cc1: all warnings being treated as errors
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, 2015-04-17 at 20:27 +0200, Jakub Jelinek wrote: > On Fri, Apr 17, 2015 at 01:06:22PM -0500, Bill Schmidt wrote: > > Yep, thanks -- I just finished testing that, and it fixes the problem > > with no regressions. Thanks for the help. > > > > Is this ok to commit? > > If David is ok with it, it is fine with me too. But, please commit to both > gcc-5-branch and the trunk, your last commit only went to the branch and not > to the trunk. Yes, absolutely. I was about to push it to trunk when I found out it was inadequate, so I held off. I'll push the combined correct patch there as well. Thank you again for your help! Bill > > Jakub >
Re: libgo patch committed: Adjust libbacktrace PC value in runtime_callers callback
On 04/17/2015 01:29 PM, Ian Lance Taylor wrote: The libbacktrace library returns a PC that was (usually) decremented to be part of the call instruction. The Go code that uses runtime.Callers does not expect this, and Go code that adjusts the PC value, such as libgo/go/runtime/pprof/pprof.go, can get fooled by it. This leads to GCC PRs 64999 and 65180. There are a couple of bugs in libsanitizer with symptoms similar to those noted in 65180: 65749 and the related 65479. I've been looking into these but don't have a patch yet. What I've found is that using the decremented PC leads to the wrong line numbers on some targets (e.g, powerpc) while using the correct PC causes the same problem on others (such as x86_64) as a result of differences in the DWARF line programs emitted on these targets. Martin This patch from Lynn Boger adjusts the PC value so that the Go code makes the correct adjustments. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and GCC 5 branch. Ian
Re: [PATCH 1/2] PR c++/61636
On 04/09/2015 11:31 PM, Adam Butcher wrote: + /* For generic lambdas, resolve default captured 'this' now. */ + if (processing_template_decl + && is_dummy_object (instance) + && current_class_type + && CLASSTYPE_LAMBDA_EXPR (current_class_type)) + if (tree callop = lambda_function (current_class_type)) + if (DECL_TEMPLATE_INFO (callop) + && (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (callop)) + == callop) + && TREE_TYPE (instance) != current_class_type + && DERIVED_FROM_P (TREE_TYPE (instance), + current_nonlambda_class_type ())) + TREE_OPERAND (postfix_expression, 0) + = instance + = maybe_resolve_dummy (instance, true); This isn't quite right. We don't want to capture 'this' any time we see a member function call, as overload resolution might choose a static member function that doesn't need 'this'. The special handling we want is for the case where the call depends on a generic lambda parameter, in which case we capture 'this' because the call "names [this] in a potentially-evaluated expression (3.2) where the enclosing full-expression depends on a generic lambda parameter declared within the reaching scope of the lambda-expression." Jason
Re: [PATCH] -Warray-bounds TLC
On Fri, 2015-04-17 at 21:08 +0200, Marc Glisse wrote:
> On Fri, 17 Apr 2015, Richard Biener wrote:
>
> >> The difference in behavior between bar and baz seems odd.
> >
> > Yeah, I suppose VRP gets conservative in a way that's not helpful for
> > consistency of this warning. ~[0,0] and ~[-2,-2] likely meet as VARYING
> > and the warning code doesn't look at equivalences.
>
> Visiting statement:
> i_10 = ASSERT_EXPR ;
> Intersecting
>~[-2, -2] EQUIVALENCES: { i_2(D) i_9 } (2 elements)
> and
>~[0, 0] EQUIVALENCES: { i_2(D) } (1 elements)
> to
>~[-2, -2] EQUIVALENCES: { i_2(D) i_9 } (2 elements)
> Found new range for i_10: ~[-2, -2]
>
> It has to pick one of the 2 anti-ranges, which one it picks is pretty
> arbitrary. It probably warns if you swap the tests for 0 and -2.
You are right, If I swap the tests then I do get the warning.
Steve Ellcey
sell...@imgtec.com
Re: [PATCH][expmed] Properly account for the cost and latency of shift+add ops when synthesizing mults
On 04/14/2015 02:11 AM, Kyrill Tkachov wrote:
Of course the effect on codegen of this patch depends a lot on the rtx
costs in the backend.
On aarch64 with -mcpu=cortex-a57 tuning I see the cost limit being
exceeded in more cases and the
expansion code choosing instead to do a move-immediate and a mul
instruction.
No regressions on SPEC2006 on a Cortex-A57.
For example, for code:
long f0 (int x, int y)
{
return (long)x * 6L;
}
int f1(int x)
{
return x * 10;
}
int f2(int x)
{
return x * 100;
}
int f3(int x)
{
return x * 20;
}
int f4(int x)
{
return x * 25;
}
int f5(int x)
{
return x * 11;
}
Please turn this into a test for the testsuite. It's fine if this the
test is specific to AArch64. You may need to break it into 6 individual
tests since what you want to check for in each one may be significantly
different. For example, f0, f4 and f5 you'd probably check for the
constant load & multiply instructions. Not sure how to best test for
what you want in f1-f3.
Bootstrapped and tested on arm, aarch64, x86_64-linux.
Ok for trunk?
Thanks,
Kyrill
2015-04-14 Kyrylo Tkachov
* expmed.c: (synth_mult): Only assume overlapping
shift with previous steps in alg_sub_t_m2 case.
OK with a test for the testsuite.
jeff
RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
On Fri, 17 Apr 2015, Petar Jovanovic wrote: > This issue will not trigger a linker error (I believe it treats the > symbol referred by the relocation as a local symbol). This is a follow > up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think > this brings back the need to run the test. I can look into separating > sections with -Wl,-T.. (that may also require extending > mips_option_groups in mips/mips.exp), if running executable is OK. If the assembler or the linker knowingly (or under conditions where it can be determined) creates a broken executable, then it is a separate bug that needs to be filed against binutils. Due to the unusual constraints of absolute MIPS jump instructions any associated symbol references have to result in external relocations, to be resolved in the final static link only. If this is not the case or such a relocation resolves successfully despite a range overflow, then this has to be fixed in binutils. Can you double-check if this is the case? I see this in a dump from crtbegin.o: Disassembly of section .init: <.init>: 0: 04110001bal 8 <.init+0x8> 4: nop 8: 0c4bjal 12c 8: R_MIPS_26.text c: nop -- if `.text+0x12c' is out of range for JAL (R_MIPS_26) from `.init+0xc', that is the 4 most significant bits of both final addresses are not the same (the range calculation for this instruction/relocation is relative to the delay slot), then the static link is supposed to fail. I think this can be easily verified and if needed, converted to an LD test case. As to a GCC test case I agree with Catherine that a run-time test case will be fine, and actually inevitable if the linker fails to fail. Maciej
libgo patch committed: Adjust libbacktrace PC value in runtime_callers callback
The libbacktrace library returns a PC that was (usually) decremented to be part of the call instruction. The Go code that uses runtime.Callers does not expect this, and Go code that adjusts the PC value, such as libgo/go/runtime/pprof/pprof.go, can get fooled by it. This leads to GCC PRs 64999 and 65180. This patch from Lynn Boger adjusts the PC value so that the Go code makes the correct adjustments. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and GCC 5 branch. Ian diff -r f9a377ea92a9 libgo/runtime/go-callers.c --- a/libgo/runtime/go-callers.cFri Apr 17 11:17:24 2015 -0700 +++ b/libgo/runtime/go-callers.cFri Apr 17 11:46:32 2015 -0700 @@ -83,7 +83,20 @@ } loc = &arg->locbuf[arg->index]; - loc->pc = pc; + + /* On the call to backtrace_full the pc value was most likely + decremented if there was a normal call, since the pc referred to + the instruction where the call returned and not the call itself. + This was done so that the line number referred to the call + instruction. To make sure the actual pc from the call stack is + used, it is incremented here. + + In the case of a signal, the pc was not decremented by + backtrace_full but still incremented here. That doesn't really + hurt anything since the line number is right and the pc refers to + the same instruction. */ + + loc->pc = pc + 1; /* The libbacktrace library says that these strings might disappear, but with the current implementation they won't. We can't easily
Re: [PATCH] gfortran.dg/pr32627.f03 prints nul byte
On 04/17/2015 11:51 AM, mse...@redhat.com wrote: Ping. Is this patch okay for trunk? Yes, both are OK for the trunk. Sorry for the delay, we're really just getting started working through stuff that was queued up for stage1. jeff
Refactoring shared code in jump threading/dom
DOM and jump threading both want to manipulate the SSA_NAME equivalency tables. Right now DOM passes its table into the threader and the threader manipulates the table directly (knowing its really just a vector stack). This results in duplicated code and a general lack of encapsulation. This patch creates a class to hold the equivalency stack and the obvious methods needed by DOM and threading. This should not change the code we generate. Bootstrapped and regression tested on x86_64-linux-gnu. Installed on the trunk. The next step on the path to resolving 47679 is to make a similar change for the expression table. That's far more interesting because to fix 47679 we need to be able to add and remove entries in the expression table within the threader and it seems the natural way to do that is to encapsulate the table in a class with reasonable methods and pass that into the threader instead of having the threader know intimate details about the hash table. Jeff commit 5c9aacc9ad269f59480dc87bfae60c3eb73f7cd4 Author: law Date: Fri Apr 17 19:24:17 2015 + PR tree-optimization/47679 * Makefile.in (OBJS); Add tree-ssa-scopedtables.o. * tree-ssa-scopedtables.c: New file. * tree-ssa-scopedtables.h: New file. * tree-ssa-dom.c: Include tree-ssa-scopedtables.h. (const_and_copies): Change name/type. (record_const_or_copy): Move into tree-ssa-scopedtables.c (record_const_or_copy_1): Similarly. (restore_vars_to_original_value): Similarly. (pass_dominator::execute): Create and destroy const_and_copies table. (thread_across_edge): Update passing of const_and_copies. (record_temporary_equivalence): Use method calls rather than manipulating const_and_copies directly. (record_equality, cprop_into_successor_phis): Similarly. (dom_opt_dom_walker::before_dom_children): Similarly. (dom_opt_dom_walker::after_dom_children): Similarly. (eliminate_redundant_computations): Similarly. * tree-ssa-threadedge.c (remove_temporary_equivalences): Delete. (record_temporary_equivalence): Likewise. (invalidate_equivalences): Likewise. (record_temporary_equivalences_from_phis): Update due to type change of const_and_copies. Use method calls rather than manipulating the stack directly. (record_temporary_equivalences_from_stmts_at_dest): Likewise. (thread_through_normal_block, thread_across_edge): Likewise. (thread_across_edge): Likewise. * tree-ssa-threadedge.h (thread_across_edge): Update prototype. * tree-vrp.c: Include tree-ssa-scopedtables.h. Change type of equiv_stack. (identify_jump_threads): Update due to type change of equiv_stack. (finalize_jump_threads): Delete the equiv_stack when complete. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@222195 138bc75d-0d04-0410-961f-82ee72b054a4 diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4fbb70b..e0a4cbf 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,37 @@ +2015-04-17 Jeff Law + + PR tree-optimization/47679 + * Makefile.in (OBJS); Add tree-ssa-scopedtables.o. + * tree-ssa-scopedtables.c: New file. + * tree-ssa-scopedtables.h: New file. + * tree-ssa-dom.c: Include tree-ssa-scopedtables.h. + (const_and_copies): Change name/type. + (record_const_or_copy): Move into tree-ssa-scopedtables.c + (record_const_or_copy_1): Similarly. + (restore_vars_to_original_value): Similarly. + (pass_dominator::execute): Create and destroy const_and_copies table. + (thread_across_edge): Update passing of const_and_copies. + (record_temporary_equivalence): Use method calls rather than + manipulating const_and_copies directly. + (record_equality, cprop_into_successor_phis): Similarly. + (dom_opt_dom_walker::before_dom_children): Similarly. + (dom_opt_dom_walker::after_dom_children): Similarly. + (eliminate_redundant_computations): Similarly. + * tree-ssa-threadedge.c (remove_temporary_equivalences): Delete. + (record_temporary_equivalence): Likewise. + (invalidate_equivalences): Likewise. + (record_temporary_equivalences_from_phis): Update due to type + change of const_and_copies. Use method calls rather than + manipulating the stack directly. + (record_temporary_equivalences_from_stmts_at_dest): Likewise. + (thread_through_normal_block, thread_across_edge): Likewise. + (thread_across_edge): Likewise. + * tree-ssa-threadedge.h (thread_across_edge): Update prototype. + * tree-vrp.c: Include tree-ssa-scopedtables.h. Change type + of equiv_stack. + (identify_jump_threads): Update due to type change of equiv_stack. + (finalize_jump_threads): Delete the equiv_stack when complete. + 2015-04-17 Uros Bizjak * config/i386/i
Re: [PATCH] -Warray-bounds TLC
On Fri, 17 Apr 2015, Richard Biener wrote:
The difference in behavior between bar and baz seems odd.
Yeah, I suppose VRP gets conservative in a way that's not helpful for
consistency of this warning. ~[0,0] and ~[-2,-2] likely meet as VARYING
and the warning code doesn't look at equivalences.
Visiting statement:
i_10 = ASSERT_EXPR ;
Intersecting
~[-2, -2] EQUIVALENCES: { i_2(D) i_9 } (2 elements)
and
~[0, 0] EQUIVALENCES: { i_2(D) } (1 elements)
to
~[-2, -2] EQUIVALENCES: { i_2(D) i_9 } (2 elements)
Found new range for i_10: ~[-2, -2]
It has to pick one of the 2 anti-ranges, which one it picks is pretty
arbitrary. It probably warns if you swap the tests for 0 and -2.
--
Marc Glisse
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 2:27 PM, Jakub Jelinek wrote: > On Fri, Apr 17, 2015 at 01:06:22PM -0500, Bill Schmidt wrote: >> Yep, thanks -- I just finished testing that, and it fixes the problem >> with no regressions. Thanks for the help. >> >> Is this ok to commit? > > If David is ok with it, it is fine with me too. But, please commit to both > gcc-5-branch and the trunk, your last commit only went to the branch and not > to the trunk. Fine with me too. And thanks a lot to Jakub for helping design the correct solution. Thanks, David
Re: [PATCH] -Warray-bounds TLC
On April 17, 2015 8:01:42 PM GMT+02:00, Steve Ellcey wrote:
>On Thu, 2015-04-16 at 13:55 +0200, Richard Biener wrote:
>> The following applies the patch produced earlier this year, applying
>> TLC to array bound warnings and catching a few more cases.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>>
>> Richard.
>>
>> 2015-04-16 Richard Biener
>>
>> PR tree-optimization/64277
>> * tree-vrp.c (check_array_ref): Fix anti-range handling,
>> simplify upper bound handling.
>> (search_for_addr_array): Simplify.
>> (check_array_bounds): Handle ADDR_EXPRs here.
>> (check_all_array_refs): Simplify.
>
>This caused an interesting build failure of glibc when using the latest
>GCC. Here is a cut down case from elf/dl-open.c:
>
> extern void foo(void);
> struct s { int n; } v[1];
> int bar (int i)
> {
> if ((i != 0 && i != -1 && i != -2) && (v[i].n == 0))
> foo ();
> return 0;
> }
> int baz (int i)
> {
> if ((i != 0 && i != -2) && (v[i].n == 0))
> foo ();
> return 0;
> }
>
>When compiled with -O2 -Wall -Werror, I now get an error about v[i]
>being out-of-bounds in bar. But I do not get this error in baz (where
>we don't check for -1. In reality, in glibc, we know that i can only
>be
>0, -1, or -2. GCC of course doesn't know that. Does this
>error/warning
>seem right?
Yes. Once you reach the array reference it will be out of bounds.
The difference in behavior between bar and baz seems odd.
>
Yeah, I suppose VRP gets conservative in a way that's not helpful for
consistency of this warning. ~[0,0] and ~[-2,-2] likely meet as VARYING and
the warning code doesn't look at equivalences.
Richard.
>Steve Ellcey
>sell...@imgtec.com
RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> -Original Message- > From: Petar Jovanovic [mailto:petar.jovano...@rt-rk.com] > Sent: Friday, April 17, 2015 2:23 PM > To: Petar Jovanovic > Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org; Moore, > Catherine > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > Resending the previous message in a plain text format. > > > Original Message > > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > > Date: Thursday, April 16, 2015 10:38 PM CEST > > From: "Moore, Catherine" > > To: "Maciej W. Rozycki" , Petar > > Jovanovic > > CC: 'Matthew Fortune' , > > "gcc-patches@gcc.gnu.org" > > References: > > <003e01d04179$ccc38bc0$664aa340$@rt- > rk.com><6D39441BF12EF246A7ABCE6654 > > > b0235320fca...@lemail01.le.imgtec.org><000201d04723$f69b1350$e3d13 > 9f0$ > > @rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com> > > Hi Petar, > > It looks like I answered a little too quickly the first time around. I'm > > sorry. > > In any case, Maciej is correct. I think a link-time test should do it. > > Thanks, > > Catherine > > > > > You'd need `objdump' for doing that and there is no ready-to-use > > > solution within the GCC test suite available, you'd have to cook > > > something up yourself, perhaps starting with `[find_binutils_prog > objdump]'. > > > > > > Another solution might be using an auxiliary linker script (`-Wl,-T,...' > > > or maybe just an implicit linker script will do; see the LD manual > > > for > > > details) to place the sections the jump is made between apart > > > manually at addresses appropriate for JAL to fail. They span does > > > not have to be large -- when placed correctly, even `JAL .' can jump > > > to the wrong place, which is what LD is supposed to catch as a > > > relocation error -- so a test executable successfully linked with > > > your correction in place won't be large, it doesn't have to take more than > 2 virtual pages. > > > > > > The downside of the latter solution are possible portability issues > > > with the linker script. You won't have to run your executable AFAICT > > > from glibc BZ > > > #17601 as you'll get a link error if a jump target is out of range. > > > So you could make it a mere link test, no need to run it. > > > > > > Maciej > > > > > > Hi Maciej, Catherine, > > This issue will not trigger a linker error (I believe it treats the symbol > referred by the relocation as a local symbol). This is a follow up to GLIBC > BZ > #17601, the problem is seen only at runtime. So, I think this brings back the > need to run the test. I can look into separating sections with -Wl,-T.. (that > may also require extending mips_option_groups in mips/mips.exp), if > running executable is OK. > Hi Petar, Running the executable is fine, but didn't you say that your test case takes hours to execute? If so, that's not acceptable. Is it possible to construct a test case that will run more quickly? Thanks, Catherine
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 01:06:22PM -0500, Bill Schmidt wrote: > Yep, thanks -- I just finished testing that, and it fixes the problem > with no regressions. Thanks for the help. > > Is this ok to commit? If David is ok with it, it is fine with me too. But, please commit to both gcc-5-branch and the trunk, your last commit only went to the branch and not to the trunk. Jakub
RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
Resending the previous message in a plain text format. > Original Message > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro > Date: Thursday, April 16, 2015 10:38 PM CEST > From: "Moore, Catherine" > To: "Maciej W. Rozycki" , Petar > Jovanovic > CC: 'Matthew Fortune' , > "gcc-patches@gcc.gnu.org" > References: > <003e01d04179$ccc38bc0$664aa340$@rt-rk.com><6d39441bf12ef246a7abce6654b0235320fca...@lemail01.le.imgtec.org><000201d04723$f69b1350$e3d139f0$@rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com> > Hi Petar, > It looks like I answered a little too quickly the first time around. I'm > sorry. > In any case, Maciej is correct. I think a link-time test should do it. > Thanks, > Catherine > > > You'd need `objdump' for doing that and there is no ready-to-use solution > > within the GCC test suite available, you'd have to cook something up > > yourself, perhaps starting with `[find_binutils_prog objdump]'. > > > > Another solution might be using an auxiliary linker script (`-Wl,-T,...' > > or maybe just an implicit linker script will do; see the LD manual for > > details) to place the sections the jump is made between apart manually at > > addresses appropriate for JAL to fail. They span does not have to be large > > -- > > when placed correctly, even `JAL .' can jump to the wrong place, which is > > what LD is supposed to catch as a relocation error -- so a test executable > > successfully linked with your correction in place won't be large, it doesn't > > have to take more than 2 virtual pages. > > > > The downside of the latter solution are possible portability issues with the > > linker script. You won't have to run your executable AFAICT from glibc BZ > > #17601 as you'll get a link error if a jump target is out of range. So you > > could > > make it a mere link test, no need to run it. > > > > Maciej > > Hi Maciej, Catherine, This issue will not trigger a linker error (I believe it treats the symbol referred by the relocation as a local symbol). This is a follow up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think this brings back the need to run the test. I can look into separating sections with -Wl,-T.. (that may also require extending mips_option_groups in mips/mips.exp), if running executable is OK. Regards, Petar
Re: Go patch committed: Fix PR 65755 on GCC 5 branch
On Tue, Apr 14, 2015 at 11:48 AM, Ian Lance Taylor wrote:
> This patch to the GCC 5 branch fixes PR 65755. This is a conservative
> patch for the branch. I will shortly apply a more complete, less
> conservative, patch to trunk. This patch simply adds the receiver
> type when producing the pkgpath or the reflection string for a type
> defined within a method. It also emits the pkgpath in the reflection
> string even when not using the -fgo-pkgpath option. For this patch
> bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
This is the patch to fix the same problem on mainline. This patch
changes the libraries to consistently use the type reflection string
to determine whether two types are the same. This simplifies the code
and permits us to change the PkgPath to be consistently the same as
the gc compiler. Also put the receiver type in the type reflection
string, as for the GCC 5 branch patch. Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 468b832d9163 go/types.cc
--- a/go/types.cc Fri Apr 10 17:49:24 2015 -0700
+++ b/go/types.cc Fri Apr 17 11:13:56 2015 -0700
@@ -2253,22 +2253,7 @@
const std::string& pkgpath(package == NULL
? gogo->pkgpath()
: package->pkgpath());
- n.assign(pkgpath);
- unsigned int index;
- const Named_object* in_function = name->in_function(&index);
- if (in_function != NULL)
- {
- n.append(1, '.');
- n.append(Gogo::unpack_hidden_name(in_function->name()));
- if (index > 0)
- {
- char buf[30];
- snprintf(buf, sizeof buf, "%u", index);
- n.append(1, '.');
- n.append(buf);
- }
- }
- s = Expression::make_string(n, bloc);
+ s = Expression::make_string(pkgpath, bloc);
vals->push_back(Expression::make_unary(OPERATOR_AND, s, bloc));
}
}
@@ -9102,22 +9087,17 @@
}
if (!this->is_builtin())
{
- // We handle -fgo-prefix and -fgo-pkgpath differently here for
- // compatibility with how the compiler worked before
- // -fgo-pkgpath was introduced. When -fgo-pkgpath is specified,
- // we use it to make a unique reflection string, so that the
- // type canonicalization in the reflect package will work. In
- // order to be compatible with the gc compiler, we put tabs into
- // the package path, so that the reflect methods can discard it.
+ // When -fgo-pkgpath or -fgo-prefix is specified, we use it to
+ // make a unique reflection string, so that the type
+ // canonicalization in the reflect package will work. In order
+ // to be compatible with the gc compiler, we put tabs into the
+ // package path, so that the reflect methods can discard it.
const Package* package = this->named_object_->package();
- if (gogo->pkgpath_from_option())
- {
- ret->push_back('\t');
- ret->append(package != NULL
- ? package->pkgpath_symbol()
- : gogo->pkgpath_symbol());
- ret->push_back('\t');
- }
+ ret->push_back('\t');
+ ret->append(package != NULL
+ ? package->pkgpath_symbol()
+ : gogo->pkgpath_symbol());
+ ret->push_back('\t');
ret->append(package != NULL
? package->package_name()
: gogo->package_name());
@@ -9126,6 +9106,14 @@
if (this->in_function_ != NULL)
{
ret->push_back('\t');
+ const Typed_identifier* rcvr =
+ this->in_function_->func_value()->type()->receiver();
+ if (rcvr != NULL)
+ {
+ Named_type* rcvr_type = rcvr->type()->deref()->named_type();
+ ret->append(Gogo::unpack_hidden_name(rcvr_type->name()));
+ ret->push_back('.');
+ }
ret->append(Gogo::unpack_hidden_name(this->in_function_->name()));
ret->push_back('$');
if (this->in_function_index_ > 0)
diff -r 468b832d9163 libgo/go/reflect/type.go
--- a/libgo/go/reflect/type.go Fri Apr 10 17:49:24 2015 -0700
+++ b/libgo/go/reflect/type.go Fri Apr 17 11:13:56 2015 -0700
@@ -1940,13 +1940,7 @@
if t == nil {
return nil
}
- u := t.uncommon()
- var s string
- if u == nil || u.PkgPath() == "" {
- s = t.rawString()
- } else {
- s = u.PkgPath() + "." + u.Name()
- }
+ s := t.rawString()
canonicalTypeLock.RLock()
if r, ok := canonicalType[s]; ok {
canonicalTypeLock.RUnlock()
diff -r 468b832d9163 libgo/runtime/go-typedesc-equal.c
--- a/libgo/runtime/go-typedesc-equal.c Fri Apr 10 17:49:24 2015 -0700
+++ b/libgo/runtime/go-typedesc-equal.c Fri Apr 17 11:13:56 2015 -0700
@@ -24,16 +24,5 @@
return 0;
if (td1->__code != td2
Re: [PATCH, 5.1, rs6000] Fix PR65787
Yep, thanks -- I just finished testing that, and it fixes the problem
with no regressions. Thanks for the help.
Is this ok to commit?
Thanks,
Bill
On Fri, 2015-04-17 at 19:46 +0200, Jakub Jelinek wrote:
> On Fri, Apr 17, 2015 at 06:39:59PM +0200, Jakub Jelinek wrote:
> > The " && special_op != SH_NONE" test from the second if can go then,
> > because it is never true. And I'd really think that you shouldn't change
> > just the fmt[i] == 'E' handling, but also the fmt[i] == 'e' || fmt[i] == 'u'
> > handling a few lines earlier (both the added
> > "if (special_op == SH_NONE) continue;" there and
> > removal of " && special_op != SH_NONE".
>
> In particular, this is what I had in mind.
>
> 2015-04-17 Bill Schmidt
>
> PR target/65787
> * config/rs6000/rs6000.c (rtx_is_swappable_p): Remove previous
> fix; ensure that a subsequent SH_NONE operand does not overwrite
> an existing *special value.
>
> --- gcc/config/rs6000/rs6000.c.jj 2015-04-17 19:09:59.0 +0200
> +++ gcc/config/rs6000/rs6000.c2015-04-17 19:28:43.264784372 +0200
> @@ -34204,17 +34204,6 @@ rtx_is_swappable_p (rtx op, unsigned int
>else
> return 0;
>
> -case PARALLEL:
> - /* A vec_extract operation may be wrapped in a PARALLEL with a
> - clobber, so account for that possibility. */
> - if (XVECLEN (op, 0) != 2)
> - return 0;
> -
> - if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
> - return 0;
> -
> - return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
> -
> case UNSPEC:
>{
> /* Various operations are unsafe for this optimization, at least
> @@ -34296,10 +34285,11 @@ rtx_is_swappable_p (rtx op, unsigned int
>{
> unsigned int special_op = SH_NONE;
> ok &= rtx_is_swappable_p (XEXP (op, i), &special_op);
> + if (special_op == SH_NONE)
> + continue;
> /* Ensure we never have two kinds of special handling
> for the same insn. */
> - if (*special != SH_NONE && special_op != SH_NONE
> - && *special != special_op)
> + if (*special != SH_NONE && *special != special_op)
> return 0;
> *special = special_op;
>}
> @@ -34308,10 +34298,11 @@ rtx_is_swappable_p (rtx op, unsigned int
> {
> unsigned int special_op = SH_NONE;
> ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
> + if (special_op == SH_NONE)
> + continue;
> /* Ensure we never have two kinds of special handling
>for the same insn. */
> - if (*special != SH_NONE && special_op != SH_NONE
> - && *special != special_op)
> + if (*special != SH_NONE && *special != special_op)
> return 0;
> *special = special_op;
> }
>
>
> Jakub
>
Re: [PATCH] -Warray-bounds TLC
On Thu, 2015-04-16 at 13:55 +0200, Richard Biener wrote:
> The following applies the patch produced earlier this year, applying
> TLC to array bound warnings and catching a few more cases.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Richard.
>
> 2015-04-16 Richard Biener
>
> PR tree-optimization/64277
> * tree-vrp.c (check_array_ref): Fix anti-range handling,
> simplify upper bound handling.
> (search_for_addr_array): Simplify.
> (check_array_bounds): Handle ADDR_EXPRs here.
> (check_all_array_refs): Simplify.
This caused an interesting build failure of glibc when using the latest
GCC. Here is a cut down case from elf/dl-open.c:
extern void foo(void);
struct s { int n; } v[1];
int bar (int i)
{
if ((i != 0 && i != -1 && i != -2) && (v[i].n == 0))
foo ();
return 0;
}
int baz (int i)
{
if ((i != 0 && i != -2) && (v[i].n == 0))
foo ();
return 0;
}
When compiled with -O2 -Wall -Werror, I now get an error about v[i]
being out-of-bounds in bar. But I do not get this error in baz (where
we don't check for -1. In reality, in glibc, we know that i can only be
0, -1, or -2. GCC of course doesn't know that. Does this error/warning
seem right? The difference in behavior between bar and baz seems odd.
Steve Ellcey
sell...@imgtec.com
[PATCH, i386]: Kill LEGITIMIZE_RELOAD_ADDRESS
Hello!
This is just dead code for LRA-enabled target.
2015-04-17 Uros Bizjak
* config/i386/i386.h (LEGITIMIZE_RELOAD_ADDRESS): Remove.
* config/i386/i386.c (ix86_legitimize_reload_address): Ditto.
* config/i386/i386-protos.h (ix86_legitimize_reload_address): Ditto.
Bootstrapped, regression tested on x86_64-linux-gnu {,-m32} and
committed to mainline SVN.
Uros.
Index: config/i386/i386-protos.h
===
--- config/i386/i386-protos.h (revision 222181)
+++ config/i386/i386-protos.h (working copy)
@@ -67,8 +67,6 @@
extern bool constant_address_p (rtx);
extern bool legitimate_pic_operand_p (rtx);
extern bool legitimate_pic_address_disp_p (rtx);
-extern bool ix86_legitimize_reload_address (rtx, machine_mode,
- int, int, int);
extern void print_reg (rtx, int, FILE*);
extern void ix86_print_operand (FILE *, rtx, int);
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 222181)
+++ config/i386/i386.c (working copy)
@@ -13320,62 +13320,6 @@
return false;
}
-/* Our implementation of LEGITIMIZE_RELOAD_ADDRESS. Returns a value to
- replace the input X, or the original X if no replacement is called for.
- The output parameter *WIN is 1 if the calling macro should goto WIN,
- 0 if it should not. */
-
-bool
-ix86_legitimize_reload_address (rtx x, machine_mode, int opnum, int type,
- int)
-{
- /* Reload can generate:
-
- (plus:DI (plus:DI (unspec:DI [(const_int 0 [0])] UNSPEC_TP)
- (reg:DI 97))
- (reg:DI 2 cx))
-
- This RTX is rejected from ix86_legitimate_address_p due to
- non-strictness of base register 97. Following this rejection,
- reload pushes all three components into separate registers,
- creating invalid memory address RTX.
-
- Following code reloads only the invalid part of the
- memory address RTX. */
-
- if (GET_CODE (x) == PLUS
- && REG_P (XEXP (x, 1))
- && GET_CODE (XEXP (x, 0)) == PLUS
- && REG_P (XEXP (XEXP (x, 0), 1)))
-{
- rtx base, index;
- bool something_reloaded = false;
-
- base = XEXP (XEXP (x, 0), 1);
- if (!REG_OK_FOR_BASE_STRICT_P (base))
- {
- push_reload (base, NULL_RTX, &XEXP (XEXP (x, 0), 1), NULL,
- BASE_REG_CLASS, GET_MODE (x), VOIDmode, 0, 0,
- opnum, (enum reload_type) type);
- something_reloaded = true;
- }
-
- index = XEXP (x, 1);
- if (!REG_OK_FOR_INDEX_STRICT_P (index))
- {
- push_reload (index, NULL_RTX, &XEXP (x, 1), NULL,
- INDEX_REG_CLASS, GET_MODE (x), VOIDmode, 0, 0,
- opnum, (enum reload_type) type);
- something_reloaded = true;
- }
-
- gcc_assert (something_reloaded);
- return true;
-}
-
- return false;
-}
-
/* Determine if op is suitable RTX for an address register.
Return naked register if a register or a register subreg is
found, otherwise return NULL_RTX. */
Index: config/i386/i386.h
===
--- config/i386/i386.h (revision 222181)
+++ config/i386/i386.h (working copy)
@@ -1830,17 +1830,6 @@
#define CONSTANT_ADDRESS_P(X) constant_address_p (X)
-/* Try a machine-dependent way of reloading an illegitimate address
- operand. If we find one, push the reload and jump to WIN. This
- macro is used in only one place: `find_reloads_address' in reload.c. */
-
-#define LEGITIMIZE_RELOAD_ADDRESS(X, MODE, OPNUM, TYPE, INDL, WIN) \
-do { \
- if (ix86_legitimize_reload_address ((X), (MODE), (OPNUM),\
- (int)(TYPE), (INDL))) \
-goto WIN; \
-} while (0)
-
/* If defined, a C expression to determine the base term of address X.
This macro is used in only one place: `find_base_term' in alias.c.
Re: [PATCH] gfortran.dg/pr32627.f03 prints nul byte
Ping. Is this patch okay for trunk? On 04/09/2015 03:16 PM, Martin Sebor wrote: Attached is an updated patch that fixes the substr_6.f90 test that also prints a nul character to stdout. I don't think there are any others. Besides interfering with the debugging of the log corruption I mentioned, printing nuls or control characters in tests is also problematic for tools designed to post-process log files (e.g., grep) that are intended to work with text files (i.e., files not containing nuls). Control characters can cause the tools to fail in non-C locales (such as UTF-8). On 04/09/2015 12:54 PM, Martin Sebor wrote: We've been debugging a problem where nul (and other control) characters have been randomly appearing in powerpc parallel build logs. In the process, I noticed that some of the nuls are readily reproducible. One such case is due to the pr32627.f03 test which verifies that Fortran programs can initialize character arrays from C strings. The test declares an array as big as the C string (including the terminating nul) and prints its value to stdout. This then causes the nul to appear in the log files. The attached patch changes the declaration of the Fortran array to match the number of non-nul characters. Tested on powerpc64. Martin
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 06:39:59PM +0200, Jakub Jelinek wrote:
> The " && special_op != SH_NONE" test from the second if can go then,
> because it is never true. And I'd really think that you shouldn't change
> just the fmt[i] == 'E' handling, but also the fmt[i] == 'e' || fmt[i] == 'u'
> handling a few lines earlier (both the added
> "if (special_op == SH_NONE) continue;" there and
> removal of " && special_op != SH_NONE".
In particular, this is what I had in mind.
2015-04-17 Bill Schmidt
PR target/65787
* config/rs6000/rs6000.c (rtx_is_swappable_p): Remove previous
fix; ensure that a subsequent SH_NONE operand does not overwrite
an existing *special value.
--- gcc/config/rs6000/rs6000.c.jj 2015-04-17 19:09:59.0 +0200
+++ gcc/config/rs6000/rs6000.c 2015-04-17 19:28:43.264784372 +0200
@@ -34204,17 +34204,6 @@ rtx_is_swappable_p (rtx op, unsigned int
else
return 0;
-case PARALLEL:
- /* A vec_extract operation may be wrapped in a PARALLEL with a
-clobber, so account for that possibility. */
- if (XVECLEN (op, 0) != 2)
- return 0;
-
- if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
- return 0;
-
- return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
-
case UNSPEC:
{
/* Various operations are unsafe for this optimization, at least
@@ -34296,10 +34285,11 @@ rtx_is_swappable_p (rtx op, unsigned int
{
unsigned int special_op = SH_NONE;
ok &= rtx_is_swappable_p (XEXP (op, i), &special_op);
+ if (special_op == SH_NONE)
+ continue;
/* Ensure we never have two kinds of special handling
for the same insn. */
- if (*special != SH_NONE && special_op != SH_NONE
- && *special != special_op)
+ if (*special != SH_NONE && *special != special_op)
return 0;
*special = special_op;
}
@@ -34308,10 +34298,11 @@ rtx_is_swappable_p (rtx op, unsigned int
{
unsigned int special_op = SH_NONE;
ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
+ if (special_op == SH_NONE)
+ continue;
/* Ensure we never have two kinds of special handling
for the same insn. */
- if (*special != SH_NONE && special_op != SH_NONE
- && *special != special_op)
+ if (*special != SH_NONE && *special != special_op)
return 0;
*special = special_op;
}
Jakub
Re: [patch, c, ping] Fix PR c/48956: diagnostics for conversions involving complex types (reviewed)
On 04/17/2015 08:10 PM, Jeff Law wrote: > Have you received confirmation from the FSF WRT your copyright > assignment was accepted? > > jeff > Yes, it's ID is [gnu.org #972407]. Should I forward the PDF to you? -- Regards, Mikhail Maltsev
Re: [PATCH] Optionally sanitize globals in user-defined sections
Yury Gribov writes:
> +
> +static bool
> +section_sanitized_p (const char *sec)
> +{
> + if (!sanitized_sections)
> +return false;
> + size_t len = strlen (sec);
> + const char *p = sanitized_sections;
> + while ((p = strstr (p, sec)))
> +{
> + if ((p == sanitized_sections || p[-1] == ',')
> + && (p[len] == 0 || p[len] == ','))
> + return true;
No wildcard support? That may be a long option in some cases.
-Andi
--
a...@linux.intel.com -- Speaking for myself only
Re: [PATCH][expr.c] PR 65358 Avoid clobbering partial argument during sibcall
On 03/19/2015 08:39 AM, Kyrill Tkachov wrote:
Hi all,
This patch fixes PR 65358. For details look at the excellent write-up
by Honggyu in bugzilla. The problem is that we're trying to pass a struct
partially on the stack and partially in regs during a tail-call
optimisation
but the struct we're passing is also a partial incoming arg though the
split
between stack and regs is different from its outgoing usage.
The emit_push_insn code ends up doing a block move for the on-stack part
but
ends up overwriting the part that needs to be loaded into regs.
My first thought was to just load the regs part first and then do the stack
part but that doesn't work as multiple comments in that function indicate
(the block move being expanded to movmem or other functions being one of
the
reasons).
My proposed solution is to detect when the overlap happens, find the
overlapping region and load it before the stack pushing into pseudos and
after the stack pushing is done move the overlapping values from the
pseudos
into the hard argument regs that they're supposed to go.
That way this new functionality should only ever be triggered when there's
the overlap in this PR (causing wrong-code) and shouldn't affect codegen
anywhere else.
Bootstrapped and tested on arm-none-linux-gnueabihf, aarch64-none-linux-gnu
and x86_64-linux-gnu.
According to the PR this appears at least as far back 4.6 so this isn't a
regression on the release branches, but it is a wrong-code bug.
I'll let Honggyu upstream the testcase separately
(https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00984.html)
I'll be testing this on the 4.8 and 4.9 branches.
Thoughts on this approach?
Thanks,
Kyrill
2015-03-19 Kyrylo Tkachov
PR middle-end/65358
* expr.c (memory_load_overlap): New function.
(emit_push_insn): When pushing partial args to the stack would
clobber the register part load the overlapping part into a pseudo
and put it into the hard reg after pushing.
expr.patch
commit 490c5f2074d76a2927afaea99e4dd0bacccb413c
Author: Kyrylo Tkachov
Date: Wed Mar 18 13:42:37 2015 +
[expr.c] PR 65358 Avoid clobbering partial argument during sibcall
diff --git a/gcc/expr.c b/gcc/expr.c
index dc13a14..d3b9156 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -4121,6 +4121,25 @@ emit_single_push_insn (machine_mode mode, rtx x, tree
type)
}
#endif
+/* Add SIZE to X and check whether it's greater than Y.
+ If it is, return the constant amount by which it's greater or smaller.
+ If the two are not statically comparable (for example, X and Y contain
+ different registers) return -1. This is used in expand_push_insn to
+ figure out if reading SIZE bytes from location X will end up reading from
+ location Y. */
+
+static int
+memory_load_overlap (rtx x, rtx y, HOST_WIDE_INT size)
+{
+ rtx tmp = plus_constant (Pmode, x, size);
+ rtx sub = simplify_gen_binary (MINUS, Pmode, tmp, y);
+
+ if (!CONST_INT_P (sub))
+return -1;
+
+ return INTVAL (sub);
+}
Hmmm, so what happens if the difference is < 0? I'd be a bit worried
about that case for the PA (for example).
So how about asserting that the INTVAL is >= 0 prior to returning so
that we catch that case if it ever occurs?
OK for the trunk with the added assert. Please commit the testcase from
Honggyu at the same time you commit the patch.
Let's let it simmer for a while on the trunk before considering it to be
backported.
jeff
Re: [patch, c, ping] Fix PR c/48956: diagnostics for conversions involving complex types (reviewed)
On 04/16/2015 08:01 PM, Mikhail Maltsev wrote: I would like to ping the following patch: https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01925.html Review: https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02672.html I fixed minor issues mentioned in the review and updated the changelog message. Rebased on current trunk (r222157), bootstrapped and regtested on x86_64-unknown-linux-gnu. If it is OK for trunk, please assist with applying (I don't have write access), and I will then create a new PR in bugzilla for the remaining cases mentioned in review. Have you received confirmation from the FSF WRT your copyright assignment was accepted? jeff
Re: [PATCH] Work around PR bootstrap/62077
On April 17, 2015 2:37:08 PM GMT+02:00, Jakub Jelinek wrote:
>Hi!
>
>As discussed in the PR, during LTO bootstrap we have some hard to debug
>issues where different gc checking values between stage1 and stage2
>result
>in different GC collections and occassionally we generate different
>code for
>that. The stated workaround is --enable-stage1-checking=release,
>I've narrowed it down to just the gc checking that should if possible
>for
>lto bootstraps match between stage1 and later checking, and
>--enable-checking=yes,types that we want to use by default for stage1
>minus gc checking is
>--enable-checking=release,misc,gimple,rtlflag,tree,types
>
>This patch arranges to do that by default, i.e. for e.g.
>--disable-checking --with-build-config=bootstrap-lto
>--with-build-config=bootstrap-lto-noplugin
>(the latter only on the release branches). When --enable-checking is
>used
>explicitly, gc matches between the stages, as we use then
>--enable-checking=$enable_checking,types
>and for explicit --enable-stage1-checking of course we should honor
>whatever
>the user asked for.
>
>Bootstrapped/regtested on x86_64-linux on the GCC 5 branch
>with
>--with-build-config=bootstrap-lto
>and tested with just running configure and inspecting config.status on
>both
>GCC 5 branch and trunk for
>default
>--with-build-config=bootstrap-lto
>--disable-checking
>--disable-checking --with-build-config=bootstrap-lto
>--enable-checking=release
>--enable-checking=release --with-build-config=bootstrap-lto
>--enable-checking=yes
>--enable-checking=yes --with-build-config=bootstrap-lto
>--enable-checking=misc
>--enable-checking=misc --with-build-config=bootstrap-lto
>
>Ok for trunk/5.1?
OK.
Thanks,
Richard.
>2015-04-17 Jakub Jelinek
>
> PR bootstrap/62077
> * configure.ac (--enable-stage1-checking): Default to
> release,misc,gimple,rtlflag,tree,types if --disable-checking
> or --enable-checking is not specified and DEV-PHASE is not
> experimental.
> * configure: Regenerated.
>
>--- configure.ac.jj2015-04-12 21:48:10.891076088 +0200
>+++ configure.ac 2015-04-17 13:48:00.591972993 +0200
>@@ -3482,7 +3482,19 @@ AC_ARG_ENABLE(stage1-checking,
> [choose additional checking for stage1 of the compiler])],
> [stage1_checking=--enable-checking=${enable_stage1_checking}],
>[if test "x$enable_checking" = xno || test "x$enable_checking" = x;
>then
>- stage1_checking=--enable-checking=yes,types
>+ # For --disable-checking or implicit --enable-checking=release,
>avoid
>+ # setting --enable-checking=gc in the default stage1 checking for
>LTO
>+ # bootstraps. See PR62077.
>+
>stage1_checking=--enable-checking=release,misc,gimple,rtlflag,tree,types
>+ case $BUILD_CONFIG in
>+*lto*)
>+ if test "x$enable_checking" = x && \
>+ test -d ${srcdir}/gcc && \
>+ test x"`cat ${srcdir}/gcc/DEV-PHASE`" = xexperimental; then
>+ stage1_checking=--enable-checking=yes,types
>+ fi;;
>+*) stage1_checking=--enable-checking=yes,types;;
>+ esac
> else
> stage1_checking=--enable-checking=$enable_checking,types
> fi])
>--- configure.jj 2015-04-12 21:48:53.0 +0200
>+++ configure 2015-04-17 13:48:14.674745554 +0200
>@@ -14761,7 +14761,19 @@ if test "${enable_stage1_checking+set}"
>enableval=$enable_stage1_checking;
>stage1_checking=--enable-checking=${enable_stage1_checking}
> else
>if test "x$enable_checking" = xno || test "x$enable_checking" = x; then
>- stage1_checking=--enable-checking=yes,types
>+ # For --disable-checking or implicit --enable-checking=release,
>avoid
>+ # setting --enable-checking=gc in the default stage1 checking for
>LTO
>+ # bootstraps. See PR62077.
>+
>stage1_checking=--enable-checking=release,misc,gimple,rtlflag,tree,types
>+ case $BUILD_CONFIG in
>+*lto*)
>+ if test "x$enable_checking" = x && \
>+ test -d ${srcdir}/gcc && \
>+ test x"`cat ${srcdir}/gcc/DEV-PHASE`" = xexperimental; then
>+ stage1_checking=--enable-checking=yes,types
>+ fi;;
>+*) stage1_checking=--enable-checking=yes,types;;
>+ esac
> else
> stage1_checking=--enable-checking=$enable_checking,types
> fi
>
> Jakub
Re: [PATCH] Add new target h8300-*-linux
On 03/05/2015 09:50 AM, Yoshinori Sato wrote:
Add h8300-*-linux target for h8300 linux kernel and userland.
h8300-*-elf is some difference of standard elf.
h8300-*-linux is compatible of standard elf rules.
Thanks.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index cfacea1..fc5101c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-03-06 Yoshinori Sato
+
+ * config.gcc: Add h8300-*-linux
+ * config/h8300/h8300.c (h8300_option_override):
+ Exclusive -mh vs -ms/-msx
+ (h8300_file_start): Target priority -msx > -ms > -mh
+ * config/h8300/linux.h: New file.
+ * config/h8300/t-linux: Likewise.
Mostly OK. Two minor issues/questions that need to be addressed, then
this ought to be able to be committed to the trunk.
+ if (TARGET_H8300H && (TARGET_H8300S || TARGET_H8300SX))
+{
+ target_flags ^= MASK_H8300H;
+}
I'm a bit concerned by this. Why did you need to make this change?
+#undef LINK_SPEC
+#define LINK_SPEC "%{mh:%{!mn:-m h8300helf_linux}} %{ms:%{!mn:-m
h8300self_linux}}"
Presumably you don't need to support normal mode or the older H8/300
processor. Does that allow you to simplify LINK_SPEC at all?
I'm going to assume the sfp-machine.h contents are correct.
You did file a copyright form with the FSF, right (I believe I asked
before, but I don't recall the result).
Jeff
Re: [patch,avr,installed] ad PR65296: Adjust specs to new avr-libc layout as of #44574
2015-04-17 18:32 GMT+03:00 Georg-Johann Lay : > Am 04/17/2015 um 04:43 PM schrieb Denis Chertykov: >> >> 2015-04-17 17:02 GMT+03:00 Georg-Johann Lay : >>> >>> ...I went ahead and installed as >>> >>> http://gcc.gnu.org/r222179 >>> >>> It will be backported to 5.2 as soon as 5.1 is open for patches again >>> (assuming RM won't approve this one for 5.1). >> >> >> IMHO AVR port is not locked for patches. >> It's not a primary target. > > > hmm, the usual text is that the complete branch is frozen: I asked this question a few years ago. The answe was that I can change the port in any time. Maybe something has changed.
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 11:32:44AM -0500, Bill Schmidt wrote:
> 2015-04-17 Bill Schmidt
>
> PR target/65787
> * config/rs6000/rs6000.c (rtx_is_swappable_p): Remove previous
> fix; ensure that a subsequent SH_NONE operand does not overwrite
> an existing *special value.
>
>
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c(revision 222182)
> +++ gcc/config/rs6000/rs6000.c(working copy)
> @@ -34204,17 +34204,6 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
>else
> return 0;
>
> -case PARALLEL:
> - /* A vec_extract operation may be wrapped in a PARALLEL with a
> - clobber, so account for that possibility. */
> - if (XVECLEN (op, 0) != 2)
> - return 0;
> -
> - if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
> - return 0;
> -
> - return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
> -
> case UNSPEC:
>{
> /* Various operations are unsafe for this optimization, at least
> @@ -34308,6 +34297,8 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
> {
> unsigned int special_op = SH_NONE;
> ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
> + if (special_op == SH_NONE)
> + continue;
> /* Ensure we never have two kinds of special handling
>for the same insn. */
> if (*special != SH_NONE && special_op != SH_NONE
The " && special_op != SH_NONE" test from the second if can go then,
because it is never true. And I'd really think that you shouldn't change
just the fmt[i] == 'E' handling, but also the fmt[i] == 'e' || fmt[i] == 'u'
handling a few lines earlier (both the added
"if (special_op == SH_NONE) continue;" there and
removal of " && special_op != SH_NONE".
Jakub
Re: [PATCH, 5.1, rs6000] Fix PR65787
Hi,
On Fri, 2015-04-17 at 10:02 -0500, Bill Schmidt wrote:
> On Fri, 2015-04-17 at 16:49 +0200, Jakub Jelinek wrote:
> > You have actually mailed the original patch again, not the revised one.
>
> > That said, PARALLEL seems to be already handled by rtx_is_swappable_p,
> > so if it isn't handled correctly, perhaps there is a bug in that function.
> >
Quite right. I've fixed this as you suggested in the attached patch.
Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.
Is this version ok?
Sorry for apparently not knowing my own code as well as I should... :/
Thanks,
Bill
2015-04-17 Bill Schmidt
PR target/65787
* config/rs6000/rs6000.c (rtx_is_swappable_p): Remove previous
fix; ensure that a subsequent SH_NONE operand does not overwrite
an existing *special value.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 222182)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -34204,17 +34204,6 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
else
return 0;
-case PARALLEL:
- /* A vec_extract operation may be wrapped in a PARALLEL with a
-clobber, so account for that possibility. */
- if (XVECLEN (op, 0) != 2)
- return 0;
-
- if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
- return 0;
-
- return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
-
case UNSPEC:
{
/* Various operations are unsafe for this optimization, at least
@@ -34308,6 +34297,8 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
{
unsigned int special_op = SH_NONE;
ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
+ if (special_op == SH_NONE)
+ continue;
/* Ensure we never have two kinds of special handling
for the same insn. */
if (*special != SH_NONE && special_op != SH_NONE
Re: unused comparison warning (bug 62182)
On 04/16/2015 02:41 AM, Marek Polacek wrote: On Thu, Apr 16, 2015 at 12:51:33AM +0200, Arnaud Bienner wrote: Hi, I've submitted a patch to bug 62182 [1], and I would like to have some feedback about it (this is still WIP as noted in the bug). As it is my first patch to gcc, I'm not sure what is the best way to discuss/review patches (here or in bugzilla). Anyway, please let me know your thoughts :) Thanks for working on this. Several comments: - Do you have a copyright assignment on file? (Not sure if it's needed for this particular patch.) Probably not needed for this patch, but definitely useful if Arnaud might have more patches later -- in which case it's good to start that process now. Jeff
Re: [PATCH] fix PR target/65535
On 04/16/2015 02:38 PM, Andreas Tobler wrote: Hi all, the below is an attempt to warn a user when she/he builds a cross compiler for *-*-freebsd* without giving a major version number. Ok for trunk? Thanks, Andreas 2015-04-16 Andreas Tobler * config.gcc: Exit with a comment when we do not have a major version number for the FreeBSD target. OK for the trunk. jeff
Re: [PATCH] PR target/65780: [5/6 Regression] Uninitialized common handling in executables
On 04/16/2015 12:57 PM, H.J. Lu wrote: Uninitialized common symbol behavior in executables is target and linker dependent. default_binds_local_p_3 is made public and updated to take an argument to indicate if common symbol may be local. If common symbol may be local, default_binds_local_p_3 will treat non-external variable as defined locally. default_binds_local_p_2 is changed to treat common symbol as local for non-PIE binaries. For i386, common symbol is local only for non-PIE binaries. For x86-64, common symbol is local only for non-PIE binaries or linker supports copy reloc in PIE binaries. If a target treats common symbol as local only for non-PIE binaries, it can define TARGET_BINDS_LOCAL_P as default_binds_local_p_2. Tested on Linux/x86-64 using -m32 with binutils master and 2.24. OK for trunk and 5 branch? H.J. --- gcc/ PR target/65780 * output.h (default_binds_local_p_3): New. * varasm.c (default_binds_local_p_3): Make it public. Take an argument to indicate if common symbol may be local. If common symbol may be local, treat non-external variable as defined locally. (default_binds_local_p_2): Pass !flag_pic to default_binds_local_p_3. (default_binds_local_p_1): Pass false to default_binds_local_p_3. * config/i386/i386.c (ix86_binds_local_p): New. (TARGET_BINDS_LOCAL_P): Replace default_binds_local_p_2 with ix86_binds_local_p. gcc/testsuite/ PR target/65780 * gcc.dg/pr65780-1.c: New test. * gcc.dg/pr65780-2.c: Likewise. * gcc.target/i386/pr32219-9.c: Likewise. * gcc.target/i386/pr32219-1.c (xxx): Make it initialized common symbol. * gcc.target/i386/pr64317.c (c): Initialize. I approved the slightly different version of this attached to pr 65780, c#35. Jeff
Re: [RFC stage 1] Proposed new warning: -Wmisleading-indentation
On 17/04/15 16:42, Tom Tromey wrote: "Dave" == David Malcolm writes: Dave> However within libcpp and gcc, in linemap's expanded_location and in Dave> diagnostic messages, the "column" numbers are actually 1-based counts of Dave> *characters*, so the "column" numbers emitted in diagnostics for the Dave> start of the first token in each line are actually: FWIW this is actually in violation of the GNU coding standards. There's a bug open for it. However, I was always afraid to change this in cpp, since presumably it would break existing programs that read gcc's output. It's a bad situation because the standard can't be changed, either, as other programs (e.g., bison and I think Emacs) do follow it faithfully. Which programs rely on precise column numbers given by GCC? They cannot be very old neither unused to pain. This "bug" actually breaks going to an error location in Emacs's compilation mode, which is annoying when compiling GCC's code. Dave> (i) a consistent value for tabs in terms of spaces, or There's already -ftabstop for this, but it isn't per-file. And it doesn't actually do anything: https://gcc.gnu.org/PR52899 Perhaps it should be fixed to do what was intended and influence how cpp transforms tabs as columns? Also, there is https://gcc.gnu.org/PR49973, which means that non-ascii characters may mess up "visual" indentation. Cheers, Manuel.
[PATCH][AArch64] PR/64134: Make aarch64_expand_vector_init use 'ins' more often
From https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64134, testcase
#define vector __attribute__((vector_size(16)))
float a; float b;
vector float fb(void) { return (vector float){ 0,0,b,a};}
currently produces (correct, but suboptimal):
fb:
fmovs0, wzr
adrpx1, b
adrpx0, a
sub sp, sp, #16
ldr w1, [x1, #:lo12:b]
ldr w0, [x0, #:lo12:a]
stp s0, s0, [sp]
stp w1, w0, [sp, 8]
ldr q0, [sp]
add sp, sp, 16
ret
with this patch:
fb:
adrpx1, b
moviv0.4s, 0
adrpx0, a
ldr s2, [x1, #:lo12:b]
ldr s1, [x0, #:lo12:a]
ins v0.s[2], v2.s[0]
ins v0.s[3], v1.s[0]
ret
The reason is that aarch64_expand_vector_init presently loads a constant and
then overwrites with 'ins' only if exactly one element of the vector is
variable; otherwise, it dumps the entire vector out to the stack (later changed
to STP) and then loads the whole vector in. This patch changes behaviour to load
constants and then 'ins' if at most half the elements are variable rather than
only one.
AFAICT this code path is only used for initialization of GCC vector extension
vectors, and there is already a special cases for all elements being the same
(e.g. the _dup_ instrinsics). So it doesn't feel worth introducing a 'cost
model'-type approach for this one use case (such would probably have to be based
on an assumption about success of STP pattern later anyway). Instead this is a
(relatively) simple heuristic improvement.
There is a possibility of using ld1 rather than ldr+ins, which *may* generate
further improvement (probably requiring adrp+add due to limited addressing modes
of ld1, however); this patch does not tackle that.
Tested on aarch64-none-elf.
gcc/ChangeLog:
PR target/64134
config/aarch64/aarch64.c (aarch64_expand_vector_init): Load constant
and overwrite variable parts if <= 1/2 the elements are variable.
gcc/testsuite/ChangeLog:
PR target/64134
gcc.target/aarch64/vec_init_1.c: New test.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index cba3c1a4d42c7d543e0ed96a7b41fcd9c925f245..767b986e907fbede46b117a271d179b58406afca 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8769,22 +8769,19 @@ aarch64_expand_vector_init (rtx target, rtx vals)
machine_mode mode = GET_MODE (target);
machine_mode inner_mode = GET_MODE_INNER (mode);
int n_elts = GET_MODE_NUNITS (mode);
- int n_var = 0, one_var = -1;
+ int n_var = 0;
+ rtx any_const = NULL_RTX;
bool all_same = true;
- rtx x, mem;
- int i;
- x = XVECEXP (vals, 0, 0);
- if (!CONST_INT_P (x) && !CONST_DOUBLE_P (x))
-n_var = 1, one_var = 0;
-
- for (i = 1; i < n_elts; ++i)
+ for (int i = 0; i < n_elts; ++i)
{
- x = XVECEXP (vals, 0, i);
+ rtx x = XVECEXP (vals, 0, i);
if (!CONST_INT_P (x) && !CONST_DOUBLE_P (x))
- ++n_var, one_var = i;
+ ++n_var;
+ else
+ any_const = x;
- if (!rtx_equal_p (x, XVECEXP (vals, 0, 0)))
+ if (i > 0 && !rtx_equal_p (x, XVECEXP (vals, 0, 0)))
all_same = false;
}
@@ -8801,36 +8798,60 @@ aarch64_expand_vector_init (rtx target, rtx vals)
/* Splat a single non-constant element if we can. */
if (all_same)
{
- x = copy_to_mode_reg (inner_mode, XVECEXP (vals, 0, 0));
+ rtx x = copy_to_mode_reg (inner_mode, XVECEXP (vals, 0, 0));
aarch64_emit_move (target, gen_rtx_VEC_DUPLICATE (mode, x));
return;
}
- /* One field is non-constant. Load constant then overwrite varying
- field. This is more efficient than using the stack. */
- if (n_var == 1)
+ /* Half the fields (or less) are non-constant. Load constant then overwrite
+ varying fields. Hope that this is more efficient than using the stack. */
+ if (n_var <= n_elts/2)
{
rtx copy = copy_rtx (vals);
- rtx index = GEN_INT (one_var);
- enum insn_code icode;
- /* Load constant part of vector, substitute neighboring value for
- varying element. */
- XVECEXP (copy, 0, one_var) = XVECEXP (vals, 0, one_var ^ 1);
+ /* Load constant part of vector. We really don't care what goes into the
+ parts we will overwrite, but we're more likely to be able to load the
+ constant efficiently if it has fewer, larger, repeating parts
+ (see aarch64_simd_valid_immediate). */
+ for (int i = 0; i < n_elts; i++)
+ {
+ rtx x = XVECEXP (vals, 0, i);
+ if (CONST_INT_P (x) || CONST_DOUBLE_P (x))
+ continue;
+ rtx subst = any_const;
+ for (int bit = n_elts / 2; bit > 0; bit /= 2)
+ {
+ /* Look in the copied vector, as more elements are const. */
+ rtx test = XVECEXP (copy, 0, i ^ bit);
+ if (CONST_INT_P (test) || CONST_DOUBLE_P (test))
+ {
+ subst = test;
+ break;
+ }
+ }
+ XVECEXP (copy, 0, i) = subst;
+ }
Re: Patch ping
On 17/04/15 16:46, Richard Earnshaw wrote: > On 06/02/14 12:12, Jakub Jelinek wrote: >> Hi! >> >> I'd like to ping a few outstanding patches: >> >> - PR59575 P1 ARM dwarf2cfi ICEs fix >> http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01997.html >> > > Wasn't this already approved (with comment fix)? > > https://gcc.gnu.org/ml/gcc-patches/2014-02/msg00392.html > Never mind. Old mail popped back to the top of my list due to Jeff's other response. R. > R. > >> - PR59992 P1 var-tracking improvement >> http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01962.html >> >> - PR60030 P1 ubsan expansion fix >> http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00167.html >> >> Jakub >> >
Re: Patch ping
On 06/02/14 12:12, Jakub Jelinek wrote: > Hi! > > I'd like to ping a few outstanding patches: > > - PR59575 P1 ARM dwarf2cfi ICEs fix > http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01997.html > Wasn't this already approved (with comment fix)? https://gcc.gnu.org/ml/gcc-patches/2014-02/msg00392.html R. > - PR59992 P1 var-tracking improvement > http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01962.html > > - PR60030 P1 ubsan expansion fix > http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00167.html > > Jakub >
[PATCH 3/3][AArch64] Idiomatic 64x1 comparisons in arm_neon.h
This also makes the existing intrinsics tests apply to the new patterns.
Tested on aarch64-none-elf.
gcc/ChangeLog:
* config/aarch64/arm_neon.h (vceq_s64, vceq_u64, vceqz_s64, vceqz_u64,
vcge_s64, vcge_u64, vcgez_s64, vcgt_s64, vcgt_u64, vcgtz_s64, vcle_s64,
vcle_u64, vclez_s64, vclt_s64, vclt_u64, vcltz_s64, vtst_s64,
vtst_u64): Rewrite using gcc vector extensions.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/singleton_intrinsics_1.c: Generalize regex to
allow cmlt or sshr.
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index 319cd8c1a0a441831a037e9c063badce7565f97c..02cdc7852d92e30e38c9c62ed09137b0d96cf6a6 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -12367,7 +12367,7 @@ vceq_s32 (int32x2_t __a, int32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vceq_s64 (int64x1_t __a, int64x1_t __b)
{
- return (uint64x1_t) {__a[0] == __b[0] ? -1ll : 0ll};
+ return (uint64x1_t) (__a == __b);
}
__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
@@ -12391,7 +12391,7 @@ vceq_u32 (uint32x2_t __a, uint32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vceq_u64 (uint64x1_t __a, uint64x1_t __b)
{
- return (uint64x1_t) {__a[0] == __b[0] ? -1ll : 0ll};
+ return (__a == __b);
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -12527,7 +12527,7 @@ vceqz_s32 (int32x2_t __a)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vceqz_s64 (int64x1_t __a)
{
- return (uint64x1_t) {__a[0] == 0ll ? -1ll : 0ll};
+ return (uint64x1_t) (__a == __AARCH64_INT64_C (0));
}
__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
@@ -12551,7 +12551,7 @@ vceqz_u32 (uint32x2_t __a)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vceqz_u64 (uint64x1_t __a)
{
- return (uint64x1_t) {__a[0] == 0ll ? -1ll : 0ll};
+ return (__a == __AARCH64_UINT64_C (0));
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -12681,7 +12681,7 @@ vcge_s32 (int32x2_t __a, int32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcge_s64 (int64x1_t __a, int64x1_t __b)
{
- return (uint64x1_t) {__a[0] >= __b[0] ? -1ll : 0ll};
+ return (uint64x1_t) (__a >= __b);
}
__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
@@ -12705,7 +12705,7 @@ vcge_u32 (uint32x2_t __a, uint32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcge_u64 (uint64x1_t __a, uint64x1_t __b)
{
- return (uint64x1_t) {__a[0] >= __b[0] ? -1ll : 0ll};
+ return (__a >= __b);
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -12829,7 +12829,7 @@ vcgez_s32 (int32x2_t __a)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcgez_s64 (int64x1_t __a)
{
- return (uint64x1_t) {__a[0] >= 0ll ? -1ll : 0ll};
+ return (uint64x1_t) (__a >= __AARCH64_INT64_C (0));
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -12923,7 +12923,7 @@ vcgt_s32 (int32x2_t __a, int32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcgt_s64 (int64x1_t __a, int64x1_t __b)
{
- return (uint64x1_t) (__a[0] > __b[0] ? -1ll : 0ll);
+ return (uint64x1_t) (__a > __b);
}
__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
@@ -12947,7 +12947,7 @@ vcgt_u32 (uint32x2_t __a, uint32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcgt_u64 (uint64x1_t __a, uint64x1_t __b)
{
- return (uint64x1_t) (__a[0] > __b[0] ? -1ll : 0ll);
+ return (__a > __b);
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -13071,7 +13071,7 @@ vcgtz_s32 (int32x2_t __a)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcgtz_s64 (int64x1_t __a)
{
- return (uint64x1_t) {__a[0] > 0ll ? -1ll : 0ll};
+ return (uint64x1_t) (__a > __AARCH64_INT64_C (0));
}
__extension__ static __inline uint32x4_t __attribute__ ((__always_inline__))
@@ -13165,7 +13165,7 @@ vcle_s32 (int32x2_t __a, int32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcle_s64 (int64x1_t __a, int64x1_t __b)
{
- return (uint64x1_t) {__a[0] <= __b[0] ? -1ll : 0ll};
+ return (uint64x1_t) (__a <= __b);
}
__extension__ static __inline uint8x8_t __attribute__ ((__always_inline__))
@@ -13189,7 +13189,7 @@ vcle_u32 (uint32x2_t __a, uint32x2_t __b)
__extension__ static __inline uint64x1_t __attribute__ ((__always_inline__))
vcle_u64 (uint64x1_t __a, uint64x1_t __b)
{
- return (uint64x1_t) {__a[0] <= __b[0] ? -1ll : 0ll};
+ return (__a <= __b);
}
__extension__ static __inline uint32x4_t __attribute__ ((__alwa
[PATCH 2/3][AArch64] Add vcond(u?)didi pattern
This just adds the necessary patterns used for comparisons of DImode vectors.
Used as part of arm_neon.h, in next/final patch.
Tested on aarch64-none-elf.
gcc/ChangeLog:
* config/aarch64/aarch64-simd.md (aarch64_vcond_internal,
vcond, vcondu,): Add DImode variant.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 52a1c3ba792adcaeaec9be4d8ada0f81bfa4714a..591740f5809d95f6f5502feda8599fd2958327bd 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -2031,13 +2031,13 @@
})
(define_expand "aarch64_vcond_internal"
- [(set (match_operand:VDQ_I 0 "register_operand")
- (if_then_else:VDQ_I
+ [(set (match_operand:VSDQ_I_DI 0 "register_operand")
+ (if_then_else:VSDQ_I_DI
(match_operator 3 "comparison_operator"
- [(match_operand:VDQ_I 4 "register_operand")
- (match_operand:VDQ_I 5 "nonmemory_operand")])
- (match_operand:VDQ_I 1 "nonmemory_operand")
- (match_operand:VDQ_I 2 "nonmemory_operand")))]
+ [(match_operand:VSDQ_I_DI 4 "register_operand")
+ (match_operand:VSDQ_I_DI 5 "nonmemory_operand")])
+ (match_operand:VSDQ_I_DI 1 "nonmemory_operand")
+ (match_operand:VSDQ_I_DI 2 "nonmemory_operand")))]
"TARGET_SIMD"
{
rtx op1 = operands[1];
@@ -2339,13 +2339,13 @@
})
(define_expand "vcond"
- [(set (match_operand:VALL 0 "register_operand")
- (if_then_else:VALL
+ [(set (match_operand:VALLDI 0 "register_operand")
+ (if_then_else:VALLDI
(match_operator 3 "comparison_operator"
- [(match_operand:VALL 4 "register_operand")
- (match_operand:VALL 5 "nonmemory_operand")])
- (match_operand:VALL 1 "nonmemory_operand")
- (match_operand:VALL 2 "nonmemory_operand")))]
+ [(match_operand:VALLDI 4 "register_operand")
+ (match_operand:VALLDI 5 "nonmemory_operand")])
+ (match_operand:VALLDI 1 "nonmemory_operand")
+ (match_operand:VALLDI 2 "nonmemory_operand")))]
"TARGET_SIMD"
{
emit_insn (gen_aarch64_vcond_internal (operands[0], operands[1],
@@ -2372,13 +2372,13 @@
})
(define_expand "vcondu"
- [(set (match_operand:VDQ_I 0 "register_operand")
- (if_then_else:VDQ_I
+ [(set (match_operand:VSDQ_I_DI 0 "register_operand")
+ (if_then_else:VSDQ_I_DI
(match_operator 3 "comparison_operator"
- [(match_operand:VDQ_I 4 "register_operand")
- (match_operand:VDQ_I 5 "nonmemory_operand")])
- (match_operand:VDQ_I 1 "nonmemory_operand")
- (match_operand:VDQ_I 2 "nonmemory_operand")))]
+ [(match_operand:VSDQ_I_DI 4 "register_operand")
+ (match_operand:VSDQ_I_DI 5 "nonmemory_operand")])
+ (match_operand:VSDQ_I_DI 1 "nonmemory_operand")
+ (match_operand:VSDQ_I_DI 2 "nonmemory_operand")))]
"TARGET_SIMD"
{
emit_insn (gen_aarch64_vcond_internal (operands[0], operands[1],
[PATCH 1/3] optabs.c: Make vector_compare_rtx cope with VOIDmode constants (e.g. const0_rtx)
As per introduction, this allows vector_compare_rtx to work on DImode vectors.
Bootstrapped + check-gcc on x86-unknown-linux-gnu.
gcc/ChangeLog:
* optabs.c (vector_compare_rtx): Handle RTL operands having VOIDmode.
diff --git a/gcc/optabs.c b/gcc/optabs.c
index f8d584eeeb11a2c19d8c8d887a0ff18aed5f56b4..135c88938f8bc03eed4dc7f1b5adcb0bb0606b1e 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -6530,18 +6530,28 @@ vector_compare_rtx (enum tree_code tcode, tree t_op0, tree t_op1,
{
struct expand_operand ops[2];
rtx rtx_op0, rtx_op1;
+ machine_mode m0, m1;
enum rtx_code rcode = get_rtx_code (tcode, unsignedp);
gcc_assert (TREE_CODE_CLASS (tcode) == tcc_comparison);
- /* Expand operands. */
+ /* Expand operands. For vector types with scalar modes, e.g. where int64x1_t
+ has mode DImode, this can produce a constant RTX of mode VOIDmode; in such
+ cases, use the original mode. */
rtx_op0 = expand_expr (t_op0, NULL_RTX, TYPE_MODE (TREE_TYPE (t_op0)),
EXPAND_STACK_PARM);
+ m0 = GET_MODE (rtx_op0);
+ if (m0 == VOIDmode)
+m0 = TYPE_MODE (TREE_TYPE (t_op0));
+
rtx_op1 = expand_expr (t_op1, NULL_RTX, TYPE_MODE (TREE_TYPE (t_op1)),
EXPAND_STACK_PARM);
+ m1 = GET_MODE (rtx_op1);
+ if (m1 == VOIDmode)
+m1 = TYPE_MODE (TREE_TYPE (t_op1));
- create_input_operand (&ops[0], rtx_op0, GET_MODE (rtx_op0));
- create_input_operand (&ops[1], rtx_op1, GET_MODE (rtx_op1));
+ create_input_operand (&ops[0], rtx_op0, m0);
+ create_input_operand (&ops[1], rtx_op1, m1);
if (!maybe_legitimize_operands (icode, 4, 2, ops))
gcc_unreachable ();
return gen_rtx_fmt_ee (rcode, VOIDmode, ops[0].value, ops[1].value);
[PATCH 0/3][AArch64] DImode vector compares
Hi,
Comparing 64x1 vector types (defined by hand or from arm_neon.h) using GCC
vector extensions currently generates very poor assembly code, for example
"uint64x1_t foo (uint64x1_t a, uint64x1_t b) { return a >= b; }" generates (at -O3):
fmov x0, d0 // 22 movdi_aarch64/12 [length = 4]
fmov x1, d1 // 23 movdi_aarch64/12 [length = 4]
cmp x0, x1 // 10 cmpdi/1 [length = 4]
csinv x0, xzr, xzr, cc // 17 cmovdi_insn/3 [length = 4]
fmov d0, x0 // 24 *movdi_aarch64/11 [length = 4]
ret // 27 simple_return [length = 4]
Meaning that arm_neon.h instead has to use rather awkward forms like "return
(uint64x1_t) {__a[0] >= __b[0] ? -1ll : 0ll};" to produce the desired assembly
cmhs d0, d0, d1
ret
This series adds vcond(u?)didi patterns for AArch64, to generate appropriate RTL
from direct comparisons of 64x1 vectors (which are of DImode). However, as
things stand, adding a vconddidi pattern causes an ICE in vector_compare_rtx
(maybe_legitimize_operands), because a DImode constant-zero (vector or
otherwise) is expanded as const0_rtx, which has mode VOIDmode. I tried quite a
bit to generate an RTL const_vector, or even just something with mode DImode,
but without success, hence the first patch fixes vector_compare_rtx to use the
mode from the tree if necessary. (DImode vectors are specifically allowed by
stor-layout.c, but no other platform defines vconddidi.)
Re: Patch ping
On 04/17/2015 01:59 AM, Jakub Jelinek wrote: Hi! I'd like to ping PR target/65689 - P2 - http://gcc.gnu.org/ml/gcc-patches/2015-04/msg00358.html patch (perhaps with the code[?] == ' ' -> ISSPACE (code[?]) changes or strstr, as discussed in the following thread). At this point of course for trunk only, and perhaps after a while for 5.2. For the comment in compute_maybe_allows, "This should be conservative" could be interpreted as setting both bits or setting neither bit. The code clearly does the former and with the background from reading the patch thread I know why, but someone reading the code may not get it without having to either look in the archives or follow how it gets used. Consider updating the comment. I'd tend to prefer strstr; I don't think this is performance sensitive code. OK for the trunk with the comment fixed and your call on how to handle the whitespace issues. jeff
Re: [patch,avr,installed] ad PR65296: Adjust specs to new avr-libc layout as of #44574
Am 04/17/2015 um 04:43 PM schrieb Denis Chertykov: 2015-04-17 17:02 GMT+03:00 Georg-Johann Lay : ...I went ahead and installed as http://gcc.gnu.org/r222179 It will be backported to 5.2 as soon as 5.1 is open for patches again (assuming RM won't approve this one for 5.1). IMHO AVR port is not locked for patches. It's not a primary target. hmm, the usual text is that the complete branch is frozen: the branches/gcc-5-branch has been created last night and GCC 5.1-rc1 built and announced. The branch is now frozen for blocking regressions and documentation fixes only, all changes to the branch require a RM approval now. https://gcc.gnu.org/ml/gcc/2015-04/msg00145.html Johann
ping: [gcc patch] libcc1: '@' GDB array operator
Hi, ping: [gcc patch] libcc1: '@' GDB array operator https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01451.html Message-ID: <20150327163646.ga16...@host1.jankratochvil.net> Jan
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, 2015-04-17 at 16:49 +0200, Jakub Jelinek wrote:
> On Fri, Apr 17, 2015 at 08:28:02AM -0500, Bill Schmidt wrote:
> > On Fri, 2015-04-17 at 07:27 -0500, Bill Schmidt wrote:
> > > Note that Jakub requested a small change in the bugzilla commentary,
> > > which I've implemented. I'm doing a regstrap now.
> > >
> > > Bill
> > >
> >
> > Here's the revised and tested patch. OK for trunk and gcc-5-branch?
>
> You have actually mailed the original patch again, not the revised one.
Yes, sorry, I had just noticed that. I forgot to download the revised
patch to the system I send my mail from. This is what I get for
multitasking during a meeting...
>
> That said, PARALLEL seems to be already handled by rtx_is_swappable_p,
> so if it isn't handled correctly, perhaps there is a bug in that function.
>
> for (i = 0; i < GET_RTX_LENGTH (code); ++i)
> if (fmt[i] == 'e' || fmt[i] == 'u')
> {
> unsigned int special_op = SH_NONE;
> ok &= rtx_is_swappable_p (XEXP (op, i), &special_op);
> /* Ensure we never have two kinds of special handling
>for the same insn. */
> if (*special != SH_NONE && special_op != SH_NONE
> && *special != special_op)
> return 0;
> *special = special_op;
> }
> else if (fmt[i] == 'E')
> for (j = 0; j < XVECLEN (op, i); ++j)
> {
> unsigned int special_op = SH_NONE;
> ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
> /* Ensure we never have two kinds of special handling
> for the same insn. */
> if (*special != SH_NONE && special_op != SH_NONE
> && *special != special_op)
> return 0;
> *special = special_op;
> }
>
> If the comments are correct, then supposedly if say on the PARALLEL with
> a SET that is SH_EXTRACT and a CLOBBER that is SH_NONE, both returning 1,
> the outcome should by return 1 (that is the case), but *special should be
> SH_EXTRACT, rather than the last case wins right now (SH_NONE).
> If so, then perhaps after each of the ok &= rtx_is_swappable_p ...
> line should be
> if (special_op == SH_NONE)
> continue;
> so that we never store SH_NONE (leave that just to the initial store), and
> then just
> if (*special != SH_NONE && *special != special_op)
> return 0;
Hm, yes, there is definitely a problem here. Let me look at reworking
this. Thanks!
Bill
>
> Jakub
>
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 08:28:02AM -0500, Bill Schmidt wrote:
> On Fri, 2015-04-17 at 07:27 -0500, Bill Schmidt wrote:
> > Note that Jakub requested a small change in the bugzilla commentary,
> > which I've implemented. I'm doing a regstrap now.
> >
> > Bill
> >
>
> Here's the revised and tested patch. OK for trunk and gcc-5-branch?
You have actually mailed the original patch again, not the revised one.
That said, PARALLEL seems to be already handled by rtx_is_swappable_p,
so if it isn't handled correctly, perhaps there is a bug in that function.
for (i = 0; i < GET_RTX_LENGTH (code); ++i)
if (fmt[i] == 'e' || fmt[i] == 'u')
{
unsigned int special_op = SH_NONE;
ok &= rtx_is_swappable_p (XEXP (op, i), &special_op);
/* Ensure we never have two kinds of special handling
for the same insn. */
if (*special != SH_NONE && special_op != SH_NONE
&& *special != special_op)
return 0;
*special = special_op;
}
else if (fmt[i] == 'E')
for (j = 0; j < XVECLEN (op, i); ++j)
{
unsigned int special_op = SH_NONE;
ok &= rtx_is_swappable_p (XVECEXP (op, i, j), &special_op);
/* Ensure we never have two kinds of special handling
for the same insn. */
if (*special != SH_NONE && special_op != SH_NONE
&& *special != special_op)
return 0;
*special = special_op;
}
If the comments are correct, then supposedly if say on the PARALLEL with
a SET that is SH_EXTRACT and a CLOBBER that is SH_NONE, both returning 1,
the outcome should by return 1 (that is the case), but *special should be
SH_EXTRACT, rather than the last case wins right now (SH_NONE).
If so, then perhaps after each of the ok &= rtx_is_swappable_p ...
line should be
if (special_op == SH_NONE)
continue;
so that we never store SH_NONE (leave that just to the initial store), and
then just
if (*special != SH_NONE && *special != special_op)
return 0;
Jakub
Re: [patch,avr,installed] ad PR65296: Adjust specs to new avr-libc layout as of #44574
2015-04-17 17:02 GMT+03:00 Georg-Johann Lay :
> ...I went ahead and installed as
>
> http://gcc.gnu.org/r222179
>
> It will be backported to 5.2 as soon as 5.1 is open for patches again
> (assuming RM won't approve this one for 5.1).
IMHO AVR port is not locked for patches.
It's not a primary target.
>
>
> As far as I can tell, all works fine now, even with install-paths containing
> spaces and LTO.
>
>
> Johann
>
>
> 2015-04-17 Sivanupandi Pitchumani
>
> PR target/65296
> * config/avr/gen-avr-mmcu-specs.c (*avrlibc_startfile): Adjust
> to new AVR-LibC file layout (bug #44574).
> (*avrlibc_devicelib): Same.
> * config/avr/avr-mcus.def: Adjust comments.
> * config/avr/avr.opt (nodevicelib): Adjust help.
>
>
>
> Index: config/avr/gen-avr-mmcu-specs.c
> ===
> --- config/avr/gen-avr-mmcu-specs.c (revision 222178)
> +++ config/avr/gen-avr-mmcu-specs.c (revision 222179)
> @@ -171,11 +171,11 @@ bool is_arch = NULL == mcu->macro;
>if (is_device)
> {
>fprintf (f, "*avrlibc_startfile:\n");
> - fprintf (f, "\tdev/%s/crt1.o%%s", mcu->name);
> + fprintf (f, "\tcrt%s.o%%s", mcu->name);
>fprintf (f, "\n\n");
>
>fprintf (f, "*avrlibc_devicelib:\n");
> - fprintf (f, "\t%%{!nodevicelib:dev/%s/libdev.a%%s}", mcu->name);
> + fprintf (f, "\t%%{!nodevicelib:-l%s}", mcu->name);
>fprintf (f, "\n\n");
> }
> #endif // WITH_AVRLIBC
> Index: config/avr/avr-mcus.def
> ===
> --- config/avr/avr-mcus.def (revision 222178)
> +++ config/avr/avr-mcus.def (revision 222179)
> @@ -44,8 +44,8 @@ Before including this file, define a mac
> used by DRIVER_SELF_SPECS and gen-avr-mmcu-specs.c for
> - the name of the device specific specs file
> in -specs=device-specs/spec-
> - - the name of the startup file dev//crt1.o
> - - the name of the device library dev//libdev.a
> + - the name of the startup file crt.o
> + - to link the device library by means of -l
>
> ARCH Specifies the multilib variant together with
> AVR_SHORT_SP
>
> Index: config/avr/avr.opt
> ===
> --- config/avr/avr.opt (revision 222178)
> +++ config/avr/avr.opt (revision 222179)
> @@ -97,4 +97,4 @@ Allow to use truncation instead of round
>
> nodevicelib
> Driver Target Report RejectNegative
> -Do not link against the device-specific library libdev.a
> +Do not link against the device-specific library lib.a
>
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, Apr 17, 2015 at 9:28 AM, Bill Schmidt wrote: > On Fri, 2015-04-17 at 07:27 -0500, Bill Schmidt wrote: >> Note that Jakub requested a small change in the bugzilla commentary, >> which I've implemented. I'm doing a regstrap now. >> >> Bill >> > > Here's the revised and tested patch. OK for trunk and gcc-5-branch? > > Thanks, > Bill > > > [gcc] > > 2015-04-16 Bill Schmidt > > PR target/65787 > * config/rs6000/rs6000.c (rtx_is_swappable_p): Handle case where > vec_extract operation is wrapped in a PARALLEL with a CLOBBER. > (adjust_extract): Likewise. > > [gcc/testsuite] > > 2015-04-16 Bill Schmidt > > PR target/65787 > * gcc.target/powerpc/pr65787.c: New. The revised patch is okay. Thanks, David
Re: [RFC stage 1] Proposed new warning: -Wmisleading-indentation
> "Dave" == David Malcolm writes: Dave> However within libcpp and gcc, in linemap's expanded_location and in Dave> diagnostic messages, the "column" numbers are actually 1-based counts of Dave> *characters*, so the "column" numbers emitted in diagnostics for the Dave> start of the first token in each line are actually: FWIW this is actually in violation of the GNU coding standards. There's a bug open for it. However, I was always afraid to change this in cpp, since presumably it would break existing programs that read gcc's output. It's a bad situation because the standard can't be changed, either, as other programs (e.g., bison and I think Emacs) do follow it faithfully. Dave> (i) a consistent value for tabs in terms of spaces, or There's already -ftabstop for this, but it isn't per-file. Tom
Re: [C++ PATCH] Reject trailing return type for operator auto()
OK. Jason
Re: [PATCH] Improve debug info generation for TLS + const (PR debug/65771)
OK. Jason
[patch,avr,installed] ad PR65296: Adjust specs to new avr-libc layout as of #44574
...I went ahead and installed as
http://gcc.gnu.org/r222179
It will be backported to 5.2 as soon as 5.1 is open for patches again (assuming
RM won't approve this one for 5.1).
As far as I can tell, all works fine now, even with install-paths containing
spaces and LTO.
Johann
2015-04-17 Sivanupandi Pitchumani
PR target/65296
* config/avr/gen-avr-mmcu-specs.c (*avrlibc_startfile): Adjust
to new AVR-LibC file layout (bug #44574).
(*avrlibc_devicelib): Same.
* config/avr/avr-mcus.def: Adjust comments.
* config/avr/avr.opt (nodevicelib): Adjust help.
Index: config/avr/gen-avr-mmcu-specs.c
===
--- config/avr/gen-avr-mmcu-specs.c (revision 222178)
+++ config/avr/gen-avr-mmcu-specs.c (revision 222179)
@@ -171,11 +171,11 @@ bool is_arch = NULL == mcu->macro;
if (is_device)
{
fprintf (f, "*avrlibc_startfile:\n");
- fprintf (f, "\tdev/%s/crt1.o%%s", mcu->name);
+ fprintf (f, "\tcrt%s.o%%s", mcu->name);
fprintf (f, "\n\n");
fprintf (f, "*avrlibc_devicelib:\n");
- fprintf (f, "\t%%{!nodevicelib:dev/%s/libdev.a%%s}", mcu->name);
+ fprintf (f, "\t%%{!nodevicelib:-l%s}", mcu->name);
fprintf (f, "\n\n");
}
#endif // WITH_AVRLIBC
Index: config/avr/avr-mcus.def
===
--- config/avr/avr-mcus.def (revision 222178)
+++ config/avr/avr-mcus.def (revision 222179)
@@ -44,8 +44,8 @@ Before including this file, define a mac
used by DRIVER_SELF_SPECS and gen-avr-mmcu-specs.c for
- the name of the device specific specs file
in -specs=device-specs/spec-
- - the name of the startup file dev//crt1.o
- - the name of the device library dev//libdev.a
+ - the name of the startup file crt.o
+ - to link the device library by means of -l
ARCH Specifies the multilib variant together with AVR_SHORT_SP
Index: config/avr/avr.opt
===
--- config/avr/avr.opt (revision 222178)
+++ config/avr/avr.opt (revision 222179)
@@ -97,4 +97,4 @@ Allow to use truncation instead of round
nodevicelib
Driver Target Report RejectNegative
-Do not link against the device-specific library libdev.a
+Do not link against the device-specific library lib.a
Re: [PATCH, 5.1, rs6000] Fix PR65787
On Fri, 2015-04-17 at 07:27 -0500, Bill Schmidt wrote:
> Note that Jakub requested a small change in the bugzilla commentary,
> which I've implemented. I'm doing a regstrap now.
>
> Bill
>
Here's the revised and tested patch. OK for trunk and gcc-5-branch?
Thanks,
Bill
[gcc]
2015-04-16 Bill Schmidt
PR target/65787
* config/rs6000/rs6000.c (rtx_is_swappable_p): Handle case where
vec_extract operation is wrapped in a PARALLEL with a CLOBBER.
(adjust_extract): Likewise.
[gcc/testsuite]
2015-04-16 Bill Schmidt
PR target/65787
* gcc.target/powerpc/pr65787.c: New.
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 222158)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -34204,6 +34204,20 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
else
return 0;
+case PARALLEL: {
+ /* A vec_extract operation may be wrapped in a PARALLEL with a
+clobber, so account for that possibility. */
+ unsigned int len = XVECLEN (op, 0);
+
+ if (len != 2)
+ return 0;
+
+ if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
+ return 0;
+
+ return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
+}
+
case UNSPEC:
{
/* Various operations are unsafe for this optimization, at least
@@ -34603,7 +34617,10 @@ permute_store (rtx_insn *insn)
static void
adjust_extract (rtx_insn *insn)
{
- rtx src = SET_SRC (PATTERN (insn));
+ rtx pattern = PATTERN (insn);
+ if (GET_CODE (pattern) == PARALLEL)
+pattern = XVECEXP (pattern, 0, 0);
+ rtx src = SET_SRC (pattern);
/* The vec_select may be wrapped in a vec_duplicate for a splat, so
account for that. */
rtx sel = GET_CODE (src) == VEC_DUPLICATE ? XEXP (src, 0) : src;
Index: gcc/testsuite/gcc.target/powerpc/pr65787.c
===
--- gcc/testsuite/gcc.target/powerpc/pr65787.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr65787.c (working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { powerpc64le-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O3" } */
+/* { dg-final { scan-assembler "xxsldwi \[0-9\]*,\[0-9\]*,\[0-9\]*,3" } } */
+/* { dg-final { scan-assembler-not "xxpermdi" } } */
+
+/* This test verifies that a vector extract operand properly has its
+ lane changed by the swap optimization. Element 2 of LE corresponds
+ to element 1 of BE. When doublewords are swapped, this becomes
+ element 3 of BE, so we need to shift the vector left by 3 words
+ to be able to extract the correct value from BE element zero. */
+
+typedef float v4f32 __attribute__ ((__vector_size__ (16)));
+
+void foo (float);
+extern v4f32 x, y;
+
+int main() {
+ v4f32 z = x + y;
+ foo (z[2]);
+}
Re: acc_on_device for device_type_host_nonshm (was: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks)
On Tue, Apr 14, 2015 at 05:43:26PM +0200, Thomas Schwinge wrote:
> On Tue, 14 Apr 2015 15:15:02 +0100, Julian Brown
> wrote:
> > On Wed, 8 Apr 2015 17:58:56 +0300
> > Ilya Verbin wrote:
> > > I see several regressions:
> > > FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/acc_on_device-1.c
> > > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution test
> > > FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/if-1.c
> > > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution test
> >
> > I think there may be multiple issues here. The attached patch addresses
> > one -- acc_device_type not distinguishing between "offloaded" and host
> > code with the host_nonshm plugin.
>
> (You mean acc_on_device?)
>
> > --- libgomp/oacc-init.c (revision 221922)
> > +++ libgomp/oacc-init.c (working copy)
> > @@ -548,7 +549,14 @@ ialias (acc_set_device_num)
> > int
> > acc_on_device (acc_device_t dev)
> > {
> > - if (acc_get_device_type () == acc_device_host_nonshm)
> > + struct goacc_thread *thr = goacc_thread ();
> > +
> > + /* We only want to appear to be the "host_nonshm" plugin from "offloaded"
> > + code -- i.e. within a parallel region. Test a flag set by the
> > + openacc_parallel hook of the host_nonshm plugin to determine that. */
> > + if (acc_get_device_type () == acc_device_host_nonshm
> > + && thr && thr->target_tls
> > + && ((struct nonshm_thread *)thr->target_tls)->nonshm_exec)
> > return dev == acc_device_host_nonshm || dev == acc_device_not_host;
> >
> >/* Just rely on the compiler builtin. */
>
> Really, acc_on_device is implemented as a compiler builtin (which is just
> disabled for a few libgomp test cases, in order to test the acc_on_device
> library function in libgomp), and I never understood why the "fallback"
> implementation in libgomp (cited above) should be doing anything
> different from the GCC builtin. Is the "problem" actually, that some
The question is if the builtin expansion isn't wrong, at least as long as
the host_nonshm device is meant to be supported. The
#ifdef ACCEL_COMPILER
case is easier, at least as long as ACCEL_COMPILER compiled code is not
meant to be able to offload to other devices (or host again), but the
non-ACCEL_COMPILER case means the code is either on the host, or
host_nonshm, or e.g. with Intel MIC you could have some shared library be
compiled by the host compiler, but then actuall linked into the MIC
offloaded path. In all those cases, I think it is just the library that
can determine the return value.
E.g. OpenMP omp_is_initial_device function is also only implemented in the
library, perhaps at some point I could expand it for #ifdef ACCEL_COMPILER
as builtin, but not for the host code, at least not due to the host-nonshm
plugin.
Jakub
[Obvious][AArch64] arm_neon.h: Remove unnecessary forward declaration of vdup_n_f32
Committed r222177 after testing on aarch64-none-linux-gnu and aarch64-none-elf.
gcc/ChangeLog:
config/aarch64/arm_neon.h (vdup_n_f32): Remove forward declaration
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index 71ef027..e9cc825 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -5665,8 +5665,6 @@ vaddlvq_u32 (uint32x4_t a)
/* vcvt_high_f32_f16 not supported */
-static float32x2_t vdup_n_f32 (float32_t);
-
#define vcvt_n_f32_s32(a, b)\
__extension__ \
({ \
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 05:36:30AM -0700, H.J. Lu wrote: > This patch works for me. OK for trunk? > > gcc/testsuite/ > > PR target/65612 > * g++.dg/ext/mv18.C: New test. > * g++.dg/ext/mv19.C: Likewise. > * g++.dg/ext/mv20.C: Likewise. > * g++.dg/ext/mv21.C: Likewise. > * g++.dg/ext/mv22.C: Likewise. > * g++.dg/ext/mv23.C: Likewise. > > libgcc/ > > PR target/65612 > * config.host (tmake_file): Add t-slibgcc-libgcc for Linux/x86. > * config/i386/cpuinfo.c (__cpu_model): Initialize. > (__cpu_indicator_init@GCC_4.8.0): New. > (__cpu_model@GCC_4.8.0): Likewise. > * config/i386/t-linux (HOST_LIBGCC2_CFLAGS): Add > -DUSE_ELF_SYMVER. LGTM. Jakub
[PATCH] Work around PR bootstrap/62077
Hi!
As discussed in the PR, during LTO bootstrap we have some hard to debug
issues where different gc checking values between stage1 and stage2 result
in different GC collections and occassionally we generate different code for
that. The stated workaround is --enable-stage1-checking=release,
I've narrowed it down to just the gc checking that should if possible for
lto bootstraps match between stage1 and later checking, and
--enable-checking=yes,types that we want to use by default for stage1
minus gc checking is --enable-checking=release,misc,gimple,rtlflag,tree,types
This patch arranges to do that by default, i.e. for e.g.
--disable-checking --with-build-config=bootstrap-lto
--with-build-config=bootstrap-lto-noplugin
(the latter only on the release branches). When --enable-checking is used
explicitly, gc matches between the stages, as we use then
--enable-checking=$enable_checking,types
and for explicit --enable-stage1-checking of course we should honor whatever
the user asked for.
Bootstrapped/regtested on x86_64-linux on the GCC 5 branch
with
--with-build-config=bootstrap-lto
and tested with just running configure and inspecting config.status on both
GCC 5 branch and trunk for
default
--with-build-config=bootstrap-lto
--disable-checking
--disable-checking --with-build-config=bootstrap-lto
--enable-checking=release
--enable-checking=release --with-build-config=bootstrap-lto
--enable-checking=yes
--enable-checking=yes --with-build-config=bootstrap-lto
--enable-checking=misc
--enable-checking=misc --with-build-config=bootstrap-lto
Ok for trunk/5.1?
2015-04-17 Jakub Jelinek
PR bootstrap/62077
* configure.ac (--enable-stage1-checking): Default to
release,misc,gimple,rtlflag,tree,types if --disable-checking
or --enable-checking is not specified and DEV-PHASE is not
experimental.
* configure: Regenerated.
--- configure.ac.jj 2015-04-12 21:48:10.891076088 +0200
+++ configure.ac2015-04-17 13:48:00.591972993 +0200
@@ -3482,7 +3482,19 @@ AC_ARG_ENABLE(stage1-checking,
[choose additional checking for stage1 of the compiler])],
[stage1_checking=--enable-checking=${enable_stage1_checking}],
[if test "x$enable_checking" = xno || test "x$enable_checking" = x; then
- stage1_checking=--enable-checking=yes,types
+ # For --disable-checking or implicit --enable-checking=release, avoid
+ # setting --enable-checking=gc in the default stage1 checking for LTO
+ # bootstraps. See PR62077.
+ stage1_checking=--enable-checking=release,misc,gimple,rtlflag,tree,types
+ case $BUILD_CONFIG in
+*lto*)
+ if test "x$enable_checking" = x && \
+test -d ${srcdir}/gcc && \
+test x"`cat ${srcdir}/gcc/DEV-PHASE`" = xexperimental; then
+ stage1_checking=--enable-checking=yes,types
+ fi;;
+*) stage1_checking=--enable-checking=yes,types;;
+ esac
else
stage1_checking=--enable-checking=$enable_checking,types
fi])
--- configure.jj2015-04-12 21:48:53.0 +0200
+++ configure 2015-04-17 13:48:14.674745554 +0200
@@ -14761,7 +14761,19 @@ if test "${enable_stage1_checking+set}"
enableval=$enable_stage1_checking;
stage1_checking=--enable-checking=${enable_stage1_checking}
else
if test "x$enable_checking" = xno || test "x$enable_checking" = x; then
- stage1_checking=--enable-checking=yes,types
+ # For --disable-checking or implicit --enable-checking=release, avoid
+ # setting --enable-checking=gc in the default stage1 checking for LTO
+ # bootstraps. See PR62077.
+ stage1_checking=--enable-checking=release,misc,gimple,rtlflag,tree,types
+ case $BUILD_CONFIG in
+*lto*)
+ if test "x$enable_checking" = x && \
+test -d ${srcdir}/gcc && \
+test x"`cat ${srcdir}/gcc/DEV-PHASE`" = xexperimental; then
+ stage1_checking=--enable-checking=yes,types
+ fi;;
+*) stage1_checking=--enable-checking=yes,types;;
+ esac
else
stage1_checking=--enable-checking=$enable_checking,types
fi
Jakub
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 4:59 AM, Jakub Jelinek wrote: > On Fri, Apr 17, 2015 at 04:48:48AM -0700, H.J. Lu wrote: >> > I don't like it. Nonshared libgcc is libgcc.a, period. No sense in >> > creating yet another library for that. >> > So, IMHO beyond making the __cpu* entrypoints compat symbols only (@ >> > instead >> > of @@ symbol versions) the right fix is simply tweak init_gcc_spec, so that >> > static_name is always linked in, in the switch combinations that it isn't >> > right now of course after shared_name rather than before that. >> > I thought we've fixed that years ago... >> > >> >> We never pass -lgcc to linker when building C++ DSO: >> >> /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/collect2 -plugin >> /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/liblto_plugin.so >> -plugin-opt=/usr/libexec/gcc/x86_64-redhat-linux/4.9.2/lto-wrapper >> -plugin-opt=-fresolution=/tmp/ccZC7iqy.res >> -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc >> -plugin-opt=-pass-through=-lgcc_s --build-id --no-add-needed >> --eh-frame-hdr --hash-style=gnu -m elf_x86_64 -shared >> /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crti.o >> /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtbeginS.o >> -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2 >> -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64 >> -L/lib/../lib64 -L/usr/lib/../lib64 >> -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../.. x.o -lstdc++ -lm >> -lgcc_s -lc -lgcc_s /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtendS.o >> /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crtn.o >> [hjl@gnu-32 tmp]$ >> >> That is why libgcc_nonshared.a is needed. > > See what I wrote. I think it is a bug that we don't do that, in your case > we should pass -lgcc_s -lgcc -lc -lgcc_s -lgcc. > Or, if you don't want to change that, as the multi-versioning change is > i386/x86_64 only change, just ensure that those targets have > t-slibgcc-libgcc in libgcc/config.host and thus behave like most other linux > targets where -lgcc is linked in always after -lgcc_s. > > Jakub This patch works for me. OK for trunk? gcc/testsuite/ PR target/65612 * g++.dg/ext/mv18.C: New test. * g++.dg/ext/mv19.C: Likewise. * g++.dg/ext/mv20.C: Likewise. * g++.dg/ext/mv21.C: Likewise. * g++.dg/ext/mv22.C: Likewise. * g++.dg/ext/mv23.C: Likewise. libgcc/ PR target/65612 * config.host (tmake_file): Add t-slibgcc-libgcc for Linux/x86. * config/i386/cpuinfo.c (__cpu_model): Initialize. (__cpu_indicator_init@GCC_4.8.0): New. (__cpu_model@GCC_4.8.0): Likewise. * config/i386/t-linux (HOST_LIBGCC2_CFLAGS): Add -DUSE_ELF_SYMVER. Thanks. -- H.J. 0001-Hide-__cpu_indicator_init-__cpu_model-from-linker.patch Description: Binary data
Re: [PATCH, 5.1, rs6000] Fix PR65787
Note that Jakub requested a small change in the bugzilla commentary,
which I've implemented. I'm doing a regstrap now.
Bill
On Thu, 2015-04-16 at 16:46 -0500, Bill Schmidt wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65787 identifies an issue
> where the powerpc64le vector swap optimization miscompiles some code.
> The code for handling vector extract operations did not expect to find
> those operations wrapped in a PARALLEL with a CLOBBER, but this test
> shows that this can now happen. This patch adds that possibility to the
> handling for vector extract. I've added a small test case to verify
> that we now catch this.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions. Is this ok for trunk and gcc-5-branch?
>
> After this is complete I will investigate whether we need to backport
> this to 4.8 and 4.9 also.
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2015-04-16 Bill Schmidt
>
> PR target/65787
> * config/rs6000/rs6000.c (rtx_is_swappable_p): Handle case where
> vec_extract operation is wrapped in a PARALLEL with a CLOBBER.
> (adjust_extract): Likewise.
>
> [gcc/testsuite]
>
> 2015-04-16 Bill Schmidt
>
> PR target/65787
> * gcc.target/powerpc/pr65787.c: New.
>
>
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c(revision 222158)
> +++ gcc/config/rs6000/rs6000.c(working copy)
> @@ -34204,6 +34204,20 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
>else
> return 0;
>
> +case PARALLEL: {
> + /* A vec_extract operation may be wrapped in a PARALLEL with a
> + clobber, so account for that possibility. */
> + unsigned int len = XVECLEN (op, 0);
> +
> + if (len != 2)
> + return 0;
> +
> + if (GET_CODE (XVECEXP (op, 0, 1)) != CLOBBER)
> + return 0;
> +
> + return rtx_is_swappable_p (XVECEXP (op, 0, 0), special);
> +}
> +
> case UNSPEC:
>{
> /* Various operations are unsafe for this optimization, at least
> @@ -34603,7 +34617,10 @@ permute_store (rtx_insn *insn)
> static void
> adjust_extract (rtx_insn *insn)
> {
> - rtx src = SET_SRC (PATTERN (insn));
> + rtx pattern = PATTERN (insn);
> + if (GET_CODE (pattern) == PARALLEL)
> +pattern = XVECEXP (pattern, 0, 0);
> + rtx src = SET_SRC (pattern);
>/* The vec_select may be wrapped in a vec_duplicate for a splat, so
> account for that. */
>rtx sel = GET_CODE (src) == VEC_DUPLICATE ? XEXP (src, 0) : src;
> Index: gcc/testsuite/gcc.target/powerpc/pr65787.c
> ===
> --- gcc/testsuite/gcc.target/powerpc/pr65787.c(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr65787.c(working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile { target { powerpc64le-*-* } } } */
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> "-mcpu=power8" } } */
> +/* { dg-options "-mcpu=power8 -O3" } */
> +/* { dg-final { scan-assembler "xxsldwi \[0-9\]*,\[0-9\]*,\[0-9\]*,3" } } */
> +/* { dg-final { scan-assembler-not "xxpermdi" } } */
> +
> +/* This test verifies that a vector extract operand properly has its
> + lane changed by the swap optimization. Element 2 of LE corresponds
> + to element 1 of BE. When doublewords are swapped, this becomes
> + element 3 of BE, so we need to shift the vector left by 3 words
> + to be able to extract the correct value from BE element zero. */
> +
> +typedef float v4f32 __attribute__ ((__vector_size__ (16)));
> +
> +void foo (float);
> +extern v4f32 x, y;
> +
> +int main() {
> + v4f32 z = x + y;
> + foo (z[2]);
> +}
>
>
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Apr 17, 2015, at 1:05 AM, Uros Bizjak wrote: > On Thu, Apr 16, 2015 at 6:28 PM, Mike Stump wrote: >> On Apr 14, 2015, at 8:07 AM, H.J. Lu wrote: I can confirm that the most current patch bootstraps on x86_64-apple-darwin14 and that all of the new tests show up as unsupported in the test suite. Jack >>> >>> I am re-posting this patch. OK for trunk? >> >> If Jack is happy, I’m happy. :-) That leaves the x86 people to comment on >> it. > > What about Solaris? To be clear, I didn’t want to give the impression that my note was approval for the entire patch. If I said, darwin parts ok, I think that would have been clearer...
Re: [PATCH] Do not discard the constructors of empty structs [PR c++/64527]
On Thu, Apr 16, 2015 at 3:54 PM, Jason Merrill wrote: > OK. Thanks, committed as revision 222176. > > Jason
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 04:48:48AM -0700, H.J. Lu wrote: > > I don't like it. Nonshared libgcc is libgcc.a, period. No sense in > > creating yet another library for that. > > So, IMHO beyond making the __cpu* entrypoints compat symbols only (@ instead > > of @@ symbol versions) the right fix is simply tweak init_gcc_spec, so that > > static_name is always linked in, in the switch combinations that it isn't > > right now of course after shared_name rather than before that. > > I thought we've fixed that years ago... > > > > We never pass -lgcc to linker when building C++ DSO: > > /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/collect2 -plugin > /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/liblto_plugin.so > -plugin-opt=/usr/libexec/gcc/x86_64-redhat-linux/4.9.2/lto-wrapper > -plugin-opt=-fresolution=/tmp/ccZC7iqy.res > -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc > -plugin-opt=-pass-through=-lgcc_s --build-id --no-add-needed > --eh-frame-hdr --hash-style=gnu -m elf_x86_64 -shared > /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crti.o > /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtbeginS.o > -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2 > -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64 > -L/lib/../lib64 -L/usr/lib/../lib64 > -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../.. x.o -lstdc++ -lm > -lgcc_s -lc -lgcc_s /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtendS.o > /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crtn.o > [hjl@gnu-32 tmp]$ > > That is why libgcc_nonshared.a is needed. See what I wrote. I think it is a bug that we don't do that, in your case we should pass -lgcc_s -lgcc -lc -lgcc_s -lgcc. Or, if you don't want to change that, as the multi-versioning change is i386/x86_64 only change, just ensure that those targets have t-slibgcc-libgcc in libgcc/config.host and thus behave like most other linux targets where -lgcc is linked in always after -lgcc_s. Jakub
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 4:37 AM, Jakub Jelinek wrote: > On Fri, Apr 17, 2015 at 01:04:20PM +0200, Uros Bizjak wrote: >> On Fri, Apr 17, 2015 at 12:36 PM, H.J. Lu wrote: >> >> > I can confirm that the most current patch bootstraps on >> > x86_64-apple-darwin14 and that all of the new tests show up as >> > unsupported in the test suite. >> > Jack >> >> I am re-posting this patch. OK for trunk? >> >>> >> >>> If Jack is happy, I’m happy. :-) That leaves the x86 people to comment >> >>> on it. >> >> >> >> What about Solaris? >> >> >> >> Uros. >> > >> > There are >> >> [...] >> >> Assuming Jakub is OK with the patch, let's go ahead with it. >> >> OK for mainline. > > I don't like it. Nonshared libgcc is libgcc.a, period. No sense in > creating yet another library for that. > So, IMHO beyond making the __cpu* entrypoints compat symbols only (@ instead > of @@ symbol versions) the right fix is simply tweak init_gcc_spec, so that > static_name is always linked in, in the switch combinations that it isn't > right now of course after shared_name rather than before that. > I thought we've fixed that years ago... > We never pass -lgcc to linker when building C++ DSO: /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/collect2 -plugin /usr/libexec/gcc/x86_64-redhat-linux/4.9.2/liblto_plugin.so -plugin-opt=/usr/libexec/gcc/x86_64-redhat-linux/4.9.2/lto-wrapper -plugin-opt=-fresolution=/tmp/ccZC7iqy.res -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc_s --build-id --no-add-needed --eh-frame-hdr --hash-style=gnu -m elf_x86_64 -shared /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtbeginS.o -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2 -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../.. x.o -lstdc++ -lm -lgcc_s -lc -lgcc_s /usr/lib/gcc/x86_64-redhat-linux/4.9.2/crtendS.o /usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../lib64/crtn.o [hjl@gnu-32 tmp]$ That is why libgcc_nonshared.a is needed. -- H.J.
Fwd: [PATCH, CHKP] Don't require IPA_REF_CHKP reference for nodes other than instrumentation thunks
Hi,
This patch is to resolve missing IPA_REF_CHKP issues. When node has
instrumented version it usually has no body (either originally or was
tranfromed into instrumentation thunk). But in some cases we don't
instrument function and instrumentation clone becomes a thunk instead.
In this case we still have IPA_REF_CHKP reference from the original
node to its clone. But several passes (e.g. inline, expand) remove
all node's references causing IPA_REF_CHKP check in verify_node fail.
Initially I was going to always rebuild IPA_REF_CHKP for functions
having instrumentation clones. But later realized this reference is
used to keep instrumentation clones reachable which is not required
when instrumentation clone has no body (and therefore will not be
emitted anyway). Thus I just relaxed the check in verify_node.
Bootstrapped and tested on x86_64-unknown-linux-gnu. Will apply to
trunk and later to gcc_5 if no objections appear.
Thanks,
Ilya
--
gcc/
2015-04-16 Ilya Enkovich
* cgraph.c (cgraph_node::verify_node): Require
IPA_CHKP_REF for instrumentation thunks only.
gcc/testsuite/
2015-04-16 Ilya Enkovich
* gcc.target/i386/mpx/chkp-reference-1.c: New.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 85531c8..cbf9cfc 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3012,7 +3012,7 @@ cgraph_node::verify_node (void)
ref_found = true;
}
- if (!ref_found)
+ if (!ref_found && thunk.thunk_p && thunk.add_pointer_bounds_args)
{
error ("Analyzed node has no reference to instrumented version");
error_found = true;
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
new file mode 100644
index 000..38b0ee2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx" } */
+
+#include
+
+static int
+test1 ()
+{
+ jmp_buf buf;
+ int state;
+
+ state = __builtin_setjmp (buf);
+
+ return state;
+}
+
+void test2 (int(*)());
+
+void
+test3 (void)
+{
+test2 (test1);
+}
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 01:04:20PM +0200, Uros Bizjak wrote: > On Fri, Apr 17, 2015 at 12:36 PM, H.J. Lu wrote: > > > I can confirm that the most current patch bootstraps on > > x86_64-apple-darwin14 and that all of the new tests show up as > > unsupported in the test suite. > > Jack > > I am re-posting this patch. OK for trunk? > >>> > >>> If Jack is happy, I’m happy. :-) That leaves the x86 people to comment > >>> on it. > >> > >> What about Solaris? > >> > >> Uros. > > > > There are > > [...] > > Assuming Jakub is OK with the patch, let's go ahead with it. > > OK for mainline. I don't like it. Nonshared libgcc is libgcc.a, period. No sense in creating yet another library for that. So, IMHO beyond making the __cpu* entrypoints compat symbols only (@ instead of @@ symbol versions) the right fix is simply tweak init_gcc_spec, so that static_name is always linked in, in the switch combinations that it isn't right now of course after shared_name rather than before that. I thought we've fixed that years ago... Jakub
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
2015-04-17 10:46 GMT+03:00 Senthil Kumar Selvaraj : > > On Thu, Apr 16, 2015 at 06:47:26PM +0200, Georg-Johann Lay wrote: > > Am 04/16/2015 um 11:28 AM schrieb Senthil Kumar Selvaraj: > > >On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote: > > >>Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj: > > >>>This patch fixes PR 65657. > > >> > > >>The following artifact appears to be PR63633. > > >> > > > > > >I did see that one - unfortunately, that fix won't help here. IIUC, you > > >check if input/output operand hard regs are in the clobber list, > > >and if yes, you generate pseudos to save and restore clobbered hard > > >regs. > > > > > >In this case, the reg is actually clobbered by a different insn (one > > > > Arrgh, yes... > > > > >that loads the next argument to the function). So unless I blindly > > >generate pseudos for > > >all regs in the clobber list, the clobbering will still happen. > > > > > >FWIW, I did try saving/restoring all clobbered regs, and it did fix the > > >problem - just that it appeared like a (worse) hack to me. Aren't we > > >manually replicating what RA/reload should be doing? > > > > As it appears, we'll have to do it by hand. The attaches patch is just a > > sketch that indicates how the problem could be approached. Notice the new > > assertions in the split expanders; they will throw ICE until the fix is > > actually installed. > > > > The critical insn are generated in movMM expander and are changed to have no > > clobbers (SCRATCHes actually). An a later pass, when appropriate life info > > can be made available, run yet another avr pass that > > > > 1a) Save-restore needed hard regs around the insn. > > > > 2a) Kick out hard regs overlapping the clobbers, e.g. in set_src, into new > > pseudos. Maybe that could happen due to some hard regs progagation, or we > > can use a new predicate similar combine_pseudo_register_operand. > > > > 3) Replace scratch -> hard regs for all scratch_operands. > > > > 2b) Restore respective hard regs from their pseudos. > > > > 1b) Restore respective hard regs from their pseudos. > > > > > > And maybe we can get better code by allocating things like address register > > by hand and get better code then. > > > > When I implemented some of the libgcc insns I tried to express the operand > > by means of constraints, e.h. for (reg:HI 22) and let register allocator do > > the job. > > > > The resulting code was functional but *horrific*. > > > > The register allocator is not yet ready to generate efficient code in such > > demanding situations... > > > > > > > >What do you think? > > > > > > > IMO sooner or later we'll need such an infrastructure; maybe also for > > non-mov insn that are implemented by transparent libcalls like divmod, mul, > > etc. > > I'm curious how other embedded targets handle this though - arm, for > example? Surely they should have some libcalls/builtins which need > specific hard regs? I should check that out. > > > > > >>>When cfgexpand.c expands a function call, it first figures out the > > >>>registers in which the arguments will go, followed by expansion of the > > >>>arguments themselves (right to left). It later emits mov insns to set > > >>>the precomputed registers with the expanded RTXes. > > >>> > > >>>If one of the arguments is a __memx char pointer dereference, the mov > > >>>eventually expands to gen_xload_A (for certain cases), which > > >>>clobbers R22, R21 and Z. This causes problems if one of those > > >>>clobbered registers was precomputed to hold another argument. > > >>> > > >>>In general, call expansion does not appear to take clobbers into account > > >>>- > > > > We had been warned that using hard regs is evil... But without that > > technique the code quality would decrease way too much. > > Oh ok - do you know some place where this is documented or was > discussed? https://gcc.gnu.org/ml/gcc/2014-10/msg00207.html
[PATCH, CHKP] Don't require IPA_REF_CHKP reference for nodes other than instrumentation thunks
Hi,
This patch is to resolve missing IPA_REF_CHKP issues. When node has
instrumented version it usually has no body (either originally or was
tranfromed into instrumentation thunk). But in some cases we don't instrument
function and instrumentation clone becomes a thunk instead. In this case we
still have IPA_REF_CHKP reference from the original node to its clone. But
several passes (e.g. inline, expand) remove all node's references causing
IPA_REF_CHKP check in verify_node fail. Initially I was going to always
rebuild IPA_REF_CHKP for functions having instrumentation clones. But later
realized this reference is used to keep instrumentation clones reachable which
is not required when instrumentation clone has no body (and therefore will not
be emitted anyway). Thus I just relaxed the check in verify_node.
Bootstrapped and tested on x86_64-unknown-linux-gnu. Will apply to trunk and
later to gcc_5 if no objections appear.
Thanks,
Ilya
--
gcc/
2015-04-16 Ilya Enkovich
* cgraph.c (cgraph_node::verify_node): Require
IPA_CHKP_REF for instrumentation thunks only.
gcc/testsuite/
2015-04-16 Ilya Enkovich
* gcc.target/i386/mpx/chkp-reference-1.c: New.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 85531c8..cbf9cfc 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3012,7 +3012,7 @@ cgraph_node::verify_node (void)
ref_found = true;
}
- if (!ref_found)
+ if (!ref_found && thunk.thunk_p && thunk.add_pointer_bounds_args)
{
error ("Analyzed node has no reference to instrumented version");
error_found = true;
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
new file mode 100644
index 000..38b0ee2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-reference-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx" } */
+
+#include
+
+static int
+test1 ()
+{
+ jmp_buf buf;
+ int state;
+
+ state = __builtin_setjmp (buf);
+
+ return state;
+}
+
+void test2 (int(*)());
+
+void
+test3 (void)
+{
+test2 (test1);
+}
Re: [AArch64][PR65375] Fix RTX cost for vector SET
On 17/04/15 12:19, Kugan wrote:
Hi James,
Here is an attempt along this line. Is this what you have in mind?
Trying to keep functionality as before so that we can tune the
parameters later. Not fully tested yet.
Hi Kugan,
I'm not doing a full review here, just have a comment inline.
Thanks,
Kyrill
Thanks,
Kugan
cost.txt
diff --git a/gcc/config/aarch64/aarch64-cost-tables.h
b/gcc/config/aarch64/aarch64-cost-tables.h
index ae2b547..ed9432e 100644
--- a/gcc/config/aarch64/aarch64-cost-tables.h
+++ b/gcc/config/aarch64/aarch64-cost-tables.h
@@ -121,7 +121,9 @@ const struct cpu_cost_table thunderx_extra_costs =
},
/* Vector */
{
-COSTS_N_INSNS (1) /* Alu. */
+COSTS_N_INSNS (1), /* Alu. */
+COSTS_N_INSNS (1), /* Load. */
+COSTS_N_INSNS (1) /* Store. */
}
What about the other CPU vector costs?
Also, tune_params would need updating.
};
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index cba3c1a..c2d4a53 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5499,16 +5499,6 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
above this default. */
*cost = COSTS_N_INSNS (1);
- /* TODO: The cost infrastructure currently does not handle
- vector operations. Assume that all vector operations
- are equally expensive. */
- if (VECTOR_MODE_P (mode))
-{
- if (speed)
- *cost += extra_cost->vect.alu;
- return true;
-}
-
switch (code)
{
case SET:
@@ -5523,7 +5513,9 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
if (speed)
{
rtx address = XEXP (op0, 0);
- if (GET_MODE_CLASS (mode) == MODE_INT)
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.store;
+ else if (GET_MODE_CLASS (mode) == MODE_INT)
*cost += extra_cost->ldst.store;
else if (mode == SFmode)
*cost += extra_cost->ldst.storef;
@@ -5544,10 +5536,17 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
/* Fall through. */
case REG:
+ if (VECTOR_MODE_P (GET_MODE (op0)) && REG_P (op1))
+ {
+ /* The cost is 1 per vector-register copied. */
+ int n_minus_1 = (GET_MODE_SIZE (GET_MODE (op0)) - 1)
+ / GET_MODE_SIZE (V4SImode);
+ *cost = COSTS_N_INSNS (n_minus_1 + 1);
+ }
/* const0_rtx is in general free, but we will use an
instruction to set a register to 0. */
- if (REG_P (op1) || op1 == const0_rtx)
-{
+ else if (REG_P (op1) || op1 == const0_rtx)
+ {
/* The cost is 1 per register copied. */
int n_minus_1 = (GET_MODE_SIZE (GET_MODE (op0)) - 1)
/ UNITS_PER_WORD;
@@ -5570,6 +5569,7 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
&& (GET_MODE_BITSIZE (GET_MODE (XEXP (op1, 0)))
>= INTVAL (XEXP (op0, 1
op1 = XEXP (op1, 0);
+ gcc_assert (!VECTOR_MODE_P (mode));
We shouldn't assert in rtx costs. If some control flow logic gets buggy,
at worst we'd return a wrong rtx cost and make a suboptimal decision.
This shouldn't ever lead to a crash, IMO.
if (CONST_INT_P (op1))
{
@@ -5621,8 +5621,10 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
case CONST_DOUBLE:
if (speed)
{
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.alu;
/* mov[df,sf]_aarch64. */
- if (aarch64_float_const_representable_p (x))
+ else if (aarch64_float_const_representable_p (x))
/* FMOV (scalar immediate). */
*cost += extra_cost->fp[mode == DFmode].fpconst;
else if (!aarch64_float_const_zero_rtx_p (x))
@@ -5650,7 +5652,9 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
approximation for the additional cost of the addressing
mode. */
rtx address = XEXP (x, 0);
- if (GET_MODE_CLASS (mode) == MODE_INT)
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.load;
+ else if (GET_MODE_CLASS (mode) == MODE_INT)
*cost += extra_cost->ldst.load;
else if (mode == SFmode)
*cost += extra_cost->ldst.loadf;
@@ -5705,7 +5709,12 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
case CLRSB:
case CLZ:
if (speed)
-*cost += extra_cost->alu.clz;
+ {
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.alu;
+ else
+ *cost += extra_cost->alu.clz;
+ }
return false;
@@ -5790,6 +5799,13 @@ aarch64_rtx_costs (rtx x, int code, int outer ATTRIBUTE_UNUSED,
return false;
Re: [AArch64][PR65375] Fix RTX cost for vector SET
>> My point is that adding your patch while keeping the logic at the top
>> which claims to catch ALL vector operations makes for less readable
>> code.
>>
>> At the very least you'll need to update this comment:
>>
>> /* TODO: The cost infrastructure currently does not handle
>> vector operations. Assume that all vector operations
>> are equally expensive. */
>>
>> to make it clear that this doesn't catch vector set operations.
>>
>> But fixing the comment doesn't improve the messy code so I'd certainly
>> prefer to see one of the other approaches which have been discussed.
>
> I see your point. Let me work on this based on your suggestions above.
Hi James,
Here is an attempt along this line. Is this what you have in mind?
Trying to keep functionality as before so that we can tune the
parameters later. Not fully tested yet.
Thanks,
Kugan
diff --git a/gcc/config/aarch64/aarch64-cost-tables.h
b/gcc/config/aarch64/aarch64-cost-tables.h
index ae2b547..ed9432e 100644
--- a/gcc/config/aarch64/aarch64-cost-tables.h
+++ b/gcc/config/aarch64/aarch64-cost-tables.h
@@ -121,7 +121,9 @@ const struct cpu_cost_table thunderx_extra_costs =
},
/* Vector */
{
-COSTS_N_INSNS (1) /* Alu. */
+COSTS_N_INSNS (1), /* Alu. */
+COSTS_N_INSNS (1), /* Load. */
+COSTS_N_INSNS (1) /* Store. */
}
};
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index cba3c1a..c2d4a53 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5499,16 +5499,6 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
above this default. */
*cost = COSTS_N_INSNS (1);
- /* TODO: The cost infrastructure currently does not handle
- vector operations. Assume that all vector operations
- are equally expensive. */
- if (VECTOR_MODE_P (mode))
-{
- if (speed)
- *cost += extra_cost->vect.alu;
- return true;
-}
-
switch (code)
{
case SET:
@@ -5523,7 +5513,9 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
if (speed)
{
rtx address = XEXP (op0, 0);
- if (GET_MODE_CLASS (mode) == MODE_INT)
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.store;
+ else if (GET_MODE_CLASS (mode) == MODE_INT)
*cost += extra_cost->ldst.store;
else if (mode == SFmode)
*cost += extra_cost->ldst.storef;
@@ -5544,10 +5536,17 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
/* Fall through. */
case REG:
+ if (VECTOR_MODE_P (GET_MODE (op0)) && REG_P (op1))
+ {
+ /* The cost is 1 per vector-register copied. */
+ int n_minus_1 = (GET_MODE_SIZE (GET_MODE (op0)) - 1)
+ / GET_MODE_SIZE (V4SImode);
+ *cost = COSTS_N_INSNS (n_minus_1 + 1);
+ }
/* const0_rtx is in general free, but we will use an
instruction to set a register to 0. */
- if (REG_P (op1) || op1 == const0_rtx)
-{
+ else if (REG_P (op1) || op1 == const0_rtx)
+ {
/* The cost is 1 per register copied. */
int n_minus_1 = (GET_MODE_SIZE (GET_MODE (op0)) - 1)
/ UNITS_PER_WORD;
@@ -5570,6 +5569,7 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
&& (GET_MODE_BITSIZE (GET_MODE (XEXP (op1, 0)))
>= INTVAL (XEXP (op0, 1
op1 = XEXP (op1, 0);
+ gcc_assert (!VECTOR_MODE_P (mode));
if (CONST_INT_P (op1))
{
@@ -5621,8 +5621,10 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
case CONST_DOUBLE:
if (speed)
{
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.alu;
/* mov[df,sf]_aarch64. */
- if (aarch64_float_const_representable_p (x))
+ else if (aarch64_float_const_representable_p (x))
/* FMOV (scalar immediate). */
*cost += extra_cost->fp[mode == DFmode].fpconst;
else if (!aarch64_float_const_zero_rtx_p (x))
@@ -5650,7 +5652,9 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
approximation for the additional cost of the addressing
mode. */
rtx address = XEXP (x, 0);
- if (GET_MODE_CLASS (mode) == MODE_INT)
+ if (VECTOR_MODE_P (mode))
+ *cost += extra_cost->vect.load;
+ else if (GET_MODE_CLASS (mode) == MODE_INT)
*cost += extra_cost->ldst.load;
else if (mode == SFmode)
*cost += extra_cost->ldst.loadf;
@@ -5705,7 +5709,12 @@ aarch64_rtx_costs (rtx x, int code, int outer
ATTRIBUTE_UNUSED,
case CLRSB:
case CLZ:
if (speed)
-*cost += extra_cost->alu.clz;
+ {
+ if (VECTOR_MODE_P (mode))
+ *c
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 1:04 PM, Uros Bizjak wrote: > On Fri, Apr 17, 2015 at 12:36 PM, H.J. Lu wrote: > >> I can confirm that the most current patch bootstraps on >> x86_64-apple-darwin14 and that all of the new tests show up as >> unsupported in the test suite. >> Jack > > I am re-posting this patch. OK for trunk? If Jack is happy, I’m happy. :-) That leaves the x86 people to comment on it. >>> >>> What about Solaris? >>> >>> Uros. >> >> There are > > [...] > > Assuming Jakub is OK with the patch, let's go ahead with it. > > OK for mainline. Ehm, the approval is for x86 part, you still need approval from libgcc maintainer. Uros.
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 12:36 PM, H.J. Lu wrote: > I can confirm that the most current patch bootstraps on > x86_64-apple-darwin14 and that all of the new tests show up as > unsupported in the test suite. > Jack I am re-posting this patch. OK for trunk? >>> >>> If Jack is happy, I’m happy. :-) That leaves the x86 people to comment on >>> it. >> >> What about Solaris? >> >> Uros. > > There are [...] Assuming Jakub is OK with the patch, let's go ahead with it. OK for mainline. Thanks, Uros.
Re: [PATCH] Fix PR65549, avoid force_decl_die in late compilation
On Fri, Apr 17, 2015 at 12:32:03PM +0200, Richard Biener wrote: > So Jakub says that using comp_unit_die () for the context of the stub > DIE is wrong and he is of course right. The following adjusted patch > uses the correct context, but only if we already have a DIE for it, > otherwise we drop the DW_TAG_GNU_call_site ref on the floor. I'd mention that this last line doesn't match what the code does, because in reality it just means that we drop the DW_AT_abstract_origin of DW_TAG_GNU_call_site. That really means just that the debugger won't know what function is called at that point, as if it were e.g. an indirect jump with unknown target that is not preserved in any register across the call), but e.g. all the guarantees that we cover all the calls still holds. Jakub
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Fri, Apr 17, 2015 at 1:05 AM, Uros Bizjak wrote:
> On Thu, Apr 16, 2015 at 6:28 PM, Mike Stump wrote:
>> On Apr 14, 2015, at 8:07 AM, H.J. Lu wrote:
I can confirm that the most current patch bootstraps on
x86_64-apple-darwin14 and that all of the new tests show up as
unsupported in the test suite.
Jack
>>>
>>> I am re-posting this patch. OK for trunk?
>>
>> If Jack is happy, I’m happy. :-) That leaves the x86 people to comment on
>> it.
>
> What about Solaris?
>
> Uros.
There are
diff --git a/libgcc/config/i386/cpuinfo.c b/libgcc/config/i386/cpuinfo.c
index eaf2f10..f6f91dd 100644
--- a/libgcc/config/i386/cpuinfo.c
+++ b/libgcc/config/i386/cpuinfo.c
@@ -109,7 +109,7 @@ struct __processor_model
unsigned int __cpu_type;
unsigned int __cpu_subtype;
unsigned int __cpu_features[1];
-} __cpu_model;
+} __cpu_model = { };
/* Get the specific type of AMD CPU. */
@@ -424,3 +424,8 @@ __cpu_indicator_init (void)
return 0;
}
+
+#if defined SHARED && defined USE_ELF_SYMVER
+__asm__ (".symver __cpu_indicator_init, __cpu_indicator_init@GCC_4.8.0");
+__asm__ (".symver __cpu_model, __cpu_model@GCC_4.8.0");
+#endif
diff --git a/libgcc/config/i386/t-linux b/libgcc/config/i386/t-linux
index 4f47f7b..11bb46e 100644
--- a/libgcc/config/i386/t-linux
+++ b/libgcc/config/i386/t-linux
@@ -3,4 +3,4 @@
# t-slibgcc-elf-ver and t-linux
SHLIB_MAPFILES = libgcc-std.ver $(srcdir)/config/i386/libgcc-glibc.ver
-HOST_LIBGCC2_CFLAGS += -mlong-double-80
+HOST_LIBGCC2_CFLAGS += -mlong-double-80 -DUSE_ELF_SYMVER
USE_ELF_SYMVER is only defined for Linux. This patch won't
break Solaris.
--
H.J.
Re: [PATCH] Fix PR65549, avoid force_decl_die in late compilation
On Fri, 17 Apr 2015, Richard Biener wrote:
>
> For PR65549 the issue is that the force_decl_die DW_TAG_GNU_call_site
> resolve_addr does can end up creating DIEs for types we won't emit
> (it re-populates the limbo DIE list for the testcase). For the
> particular testcase this happens because the context of the function
> called (a lambda type) wasn't needed (it's created/used in another
> LTRANS unit).
>
> The DW_TAG_GNU_call_site support doesn't actually need a full DIE
> tree for the callee decl (which is external in the case in question):
>
> static void
> resolve_addr (dw_die_ref die)
> {
> ...
> FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
> switch (AT_class (a))
> {
> ...
> case dw_val_class_addr:
> ...
> if (die->die_tag == DW_TAG_GNU_call_site
> && a->dw_attr == DW_AT_abstract_origin)
> {
> tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
> dw_die_ref tdie = lookup_decl_die (tdecl);
> if (tdie == NULL
> && DECL_EXTERNAL (tdecl)
> && DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
> {
> force_decl_die (tdecl);
> tdie = lookup_decl_die (tdecl);
>
> instead it is enough to create a DIE with enough information for
> the debugger to associate it with the callee - namely a declaration
> with just a name. This is what the patch does.
>
> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu
> (I'm also double-checking LTO bootstrap).
>
> I've manually verified gdb is still happy with the DW_TAG_GNU_call_site
> info (not sure if we have a guality test for the feature).
>
> Ok for trunk and affected branches? (I've also run into this when
> doing the LTO early debug work this week which is where the fix
> originates).
So Jakub says that using comp_unit_die () for the context of the stub
DIE is wrong and he is of course right. The following adjusted patch
uses the correct context, but only if we already have a DIE for it,
otherwise we drop the DW_TAG_GNU_call_site ref on the floor.
With DINFO_LEVEL_TERSE we already don't emit a DW_AT_type attribute
for the subprogram DIE so we should be safe not doing that here
either (it's type is another source of the same issue).
Re-testing in progress.
Ok?
Thanks,
Richard.
2015-04-17 Richard Biener
PR debug/65549
* dwarf2out.c (lookup_context_die): New function.
(resolve_addr): Avoid forcing a full DIE for the
target of a DW_TAG_GNU_call_site during late compilation.
Instead create a stub DIE without a type if we have a
context DIE present.
* g++.dg/lto/pr65549_0.C: New testcase.
Index: gcc/dwarf2out.c
===
*** gcc/dwarf2out.c (revision 222165)
--- gcc/dwarf2out.c (working copy)
*** is_naming_typedef_decl (const_tree decl)
*** 20617,20622
--- 20617,20644
!= TYPE_NAME (TREE_TYPE (decl;
}
+ /* Looks up the DIE for a context. */
+
+ static inline dw_die_ref
+ lookup_context_die (tree context)
+ {
+ if (context)
+ {
+ /* Find die that represents this context. */
+ if (TYPE_P (context))
+ {
+ context = TYPE_MAIN_VARIANT (context);
+ dw_die_ref ctx = lookup_type_die (context);
+ if (!ctx)
+ return NULL;
+ return strip_naming_typedef (context, ctx);
+ }
+ else
+ return lookup_decl_die (context);
+ }
+ return comp_unit_die ();
+ }
+
/* Returns the DIE for a context. */
static inline dw_die_ref
*** resolve_addr (dw_die_ref die)
*** 23946,23957
{
tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
dw_die_ref tdie = lookup_decl_die (tdecl);
if (tdie == NULL
&& DECL_EXTERNAL (tdecl)
! && DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
{
! force_decl_die (tdecl);
! tdie = lookup_decl_die (tdecl);
}
if (tdie)
{
--- 23968,23989
{
tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
dw_die_ref tdie = lookup_decl_die (tdecl);
+ dw_die_ref cdie;
if (tdie == NULL
&& DECL_EXTERNAL (tdecl)
! && DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE
! && (cdie = lookup_context_die (DECL_CONTEXT (tdecl
{
! /* Creating a full DIE for tdecl is overly expensive and
! at this point even wrong when in the LTO phase
! as it can end up generating new type DIEs we didn't
! output and thus optimize_external_refs will crash. */
! tdie = new_die (DW_TAG_subprogram, cdie, NULL_TREE);
! add_AT_flag (tdie, DW_AT_external, 1);
! add_AT_flag (tdie, DW_AT
[PING][PATCH][3/3][PR65460] Mark offloaded functions as parallelized
On 20-03-15 12:38, Tom de Vries wrote: On 19-03-15 12:05, Tom de Vries wrote: On 18-03-15 18:22, Tom de Vries wrote: Hi, this patch fixes PR65460. The patch marks offloaded functions as parallelized, which means the parloops pass no longer attempts to modify that function. Updated patch to postpone mark_parallelized_function until the corresponding cgraph_node is available, to ensure it works with the updated mark_parallelized_function from patch 2/3. Updated to eliminate mark_parallelized_function. Bootstrapped and reg-tested on x86_64. OK for stage4? ping. OK for stage1? Thanks, - Tom
Re: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks (was: Merge current set of OpenACC changes from gomp-4_0-branch) (PR65742)
On Tue, 14 Apr 2015 15:15:02 +0100
Julian Brown wrote:
> On Wed, 8 Apr 2015 17:58:56 +0300
> Ilya Verbin wrote:
>
> > On Wed, Apr 08, 2015 at 15:31:42 +0100, Julian Brown wrote:
> > > This version is mostly the same as the last posted version but
> > > has a tweak in GOACC_parallel to account for the new splay tree
> > > arrangement for target functions:
> > >
> > > - tgt_fn = (void (*)) tgt_fn_key->tgt->tgt_start;
> > > + tgt_fn = (void (*)) tgt_fn_key->tgt_offset;
> > >
> > > Have there been any other changes I might have missed?
> >
> > No.
> >
> > > It passes libgomp testing on NVPTX. OK?
> >
> > Have you tested it with disabled offloading?
> >
> > I see several regressions:
> > FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/acc_on_device-1.c
> > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution test
> > FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/if-1.c
> > -DACC_DEVICE_TYPE_host_nonshm=1 -DACC_MEM_SHARED=0 execution test
>
> I think there may be multiple issues here. The attached patch
> addresses one -- acc_device_type not distinguishing between
> "offloaded" and host code with the host_nonshm plugin.
The patch appears to fix the original issue after all: I've re-run
tests with host==target and the failures no longer appear. Also the
same has been noted by Dominique d'Humieres in PR65742.
> The other problem is that it appears that the ACC_DEVICE_TYPE
> environment variable is not getting set properly on the target for
> (any of) the OpenACC tests: this means a lot of the time the "wrong"
> plugin is being tested, and means that the above tests (and several
> others) still fail. That will apparently need some more engineering
> (on our part).
Fixing this turns out to require more DejaGNU-fu than I have: AFAICT,
setting a per-test environment variable from an .exp file can't easily
be done at present. The potentially useful-looking
{dg-}set-target-env-var doesn't look quite suitable for this purpose,
and besides which doesn't actually seem to be implemented for host !=
target anyway.
(At least, if this fragment of gcc-dg.exp is anything to go by:
if { [info exists set_target_env_var] \
&& [llength $set_target_env_var] != 0 } {
if { [is_remote target] } {
return [list "unsupported" ""]
} ...
).
So: OK for trunk?
Thanks,
Julian
> ChangeLog
>
> libgomp/
> * oacc-init.c (acc_on_device): Check whether we're in an offloaded
> region for host_nonshm plugin.
> * plugin/plugin-host.c (GOMP_OFFLOAD_openacc_parallel): Set
> nonshm_exec flag in thread-local data.
> (GOMP_OFFLOAD_openacc_create_thread_data): Allocate thread-local
> data for host_nonshm plugin.
> (+GOMP_OFFLOAD_openacc_destroy_thread_data): Free thread-local
> data for host_nonshm plugin.
> * plugin/plugin-host.h: New.
Re: unused comparison warning (bug 62182)
Thanks for your quick feedback :) 2015-04-16 10:41 GMT+02:00 Marek Polacek : > - Do you have a copyright assignment on file? (Not sure if it's needed for > this particular patch.) No I don't. Do you think I need one for this patch? > - We'll need testcases. You should e.g. ensure that the warning > works with -Wunused-comparison and doesn't show up with > -Wno-unused-comparison, > that casting to void suppresses the warning, etc. You can look into > gcc/testsuite/gcc.dg. Sure. I haven't look yet at gcc tests. I first wanted to have some feedback about this, to be sure it was worth to spend some time to implement this (i.e. this is a feature you are interested in), and that what I did wasn't completely wrong. For now, I just tested my changes manually, checking that the warning will not be Wunused-value but Wunused-comparison when the expression was an "==" equality expression. I will do the changes you mentioned, and submit a new patch. > - New options need documenting in invoke.texi. Only mentioning the new > option is not enough. See e.g. "@item -Wlogical-not-parentheses" paragraph > for an example. > - As this is a C/C++ FE warning, please move it from common.opt to > c-family/c.opt. > - Could you detail how this patch has been tested? > - Please adhere to the GNU coding style (though we can sort this out in later > reviews). > > Thanks, > > Marek
[PATCH] Fix PR65549, avoid force_decl_die in late compilation
For PR65549 the issue is that the force_decl_die DW_TAG_GNU_call_site
resolve_addr does can end up creating DIEs for types we won't emit
(it re-populates the limbo DIE list for the testcase). For the
particular testcase this happens because the context of the function
called (a lambda type) wasn't needed (it's created/used in another
LTRANS unit).
The DW_TAG_GNU_call_site support doesn't actually need a full DIE
tree for the callee decl (which is external in the case in question):
static void
resolve_addr (dw_die_ref die)
{
...
FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
switch (AT_class (a))
{
...
case dw_val_class_addr:
...
if (die->die_tag == DW_TAG_GNU_call_site
&& a->dw_attr == DW_AT_abstract_origin)
{
tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
dw_die_ref tdie = lookup_decl_die (tdecl);
if (tdie == NULL
&& DECL_EXTERNAL (tdecl)
&& DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
{
force_decl_die (tdecl);
tdie = lookup_decl_die (tdecl);
instead it is enough to create a DIE with enough information for
the debugger to associate it with the callee - namely a declaration
with just a name. This is what the patch does.
Bootstrap and regtest in progress on x86_64-unknown-linux-gnu
(I'm also double-checking LTO bootstrap).
I've manually verified gdb is still happy with the DW_TAG_GNU_call_site
info (not sure if we have a guality test for the feature).
Ok for trunk and affected branches? (I've also run into this when
doing the LTO early debug work this week which is where the fix
originates).
Thanks,
Richard.
2015-04-17 Richard Biener
PR debug/65549
* dwarf2out.c (resolve_addr): Avoid forcing a full DIE for the
target of a DW_TAG_GNU_call_site during late compilation.
Instead create a stub DIE without a type.
* g++.dg/lto/pr65549_0.C: New testcase.
Index: gcc/dwarf2out.c
===
*** gcc/dwarf2out.c (revision 222165)
--- gcc/dwarf2out.c (working copy)
*** resolve_addr (dw_die_ref die)
*** 23950,23957
&& DECL_EXTERNAL (tdecl)
&& DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
{
! force_decl_die (tdecl);
! tdie = lookup_decl_die (tdecl);
}
if (tdie)
{
--- 23950,23966
&& DECL_EXTERNAL (tdecl)
&& DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
{
! /* Creating a full DIE for tdecl is overly expensive and
! at this point even wrong when in the LTO phase
! as it can end up generating new type DIEs we didn't
! output and thus optimize_external_refs will crash. */
! tdie = new_die (DW_TAG_subprogram, comp_unit_die (), NULL_TREE);
! add_AT_flag (tdie, DW_AT_external, 1);
! add_AT_flag (tdie, DW_AT_declaration, 1);
! /* GDB is happy with either of the following but add both. */
! add_linkage_attr (tdie, tdecl);
! add_name_and_src_coords_attributes (tdie, tdecl);
! equate_decl_number_to_die (tdecl, tdie);
}
if (tdie)
{
Index: gcc/testsuite/g++.dg/lto/pr65549_0.C
===
*** gcc/testsuite/g++.dg/lto/pr65549_0.C(revision 0)
--- gcc/testsuite/g++.dg/lto/pr65549_0.C(working copy)
***
*** 0
--- 1,144
+ // { dg-lto-do link }
+ // { dg-lto-options { { -std=gnu++14 -flto -g } { -std=gnu++14 -flto -g -O2
-fno-inline -flto-partition=max } } }
+ // { dg-extra-ld-options "-r -nostdlib" }
+
+ namespace std {
+ inline namespace __cxx11 {}
+ template struct integral_constant {
+ static constexpr _Tp value = 0;
+ };
+ template struct __and_;
+ struct is_member_object_pointer : integral_constant {};
+ template
+ struct is_member_function_pointer : integral_constant {};
+ template struct remove_reference { typedef int type; };
+ template class C;
+ template struct __result_of_impl;
+ template
+ struct __result_of_impl {
+ typedef decltype(0) type;
+ };
+ template
+ struct C<_Functor(_ArgTypes...)>
+ : __result_of_impl::type>::value,
+_Functor> {};
+ template using result_of_t = typename C<_Tp>::type;
+ template void forward();
+ template _Tp move(_Tp) {}
+ namespace __cxx11 {
+ class basic_string typedef string;
+ }
+ template struct allocator_traits { typedef decltype(0) pointer; };
+ }
+ struct F : std::allocator_traits {};
+ namespace std {
+ namespace __cxx11 {
+ class basic_string {
+ public:
+ struct _Alloc_hider : F {
+ _Alloc_hider(pointer);
+ } _M_dataplus;
+ basic_string(int) : _M_dataplus(0) {}
+ ~basic_string();
+
Patch ping
Hi! I'd like to ping PR target/65689 - P2 - http://gcc.gnu.org/ml/gcc-patches/2015-04/msg00358.html patch (perhaps with the code[?] == ' ' -> ISSPACE (code[?]) changes or strstr, as discussed in the following thread). At this point of course for trunk only, and perhaps after a while for 5.2. Jakub
Re: [PATCH, PR target/65103, 2/3] Propagate address constants into loops for i386
On 15 Apr 14:07, Ilya Enkovich wrote:
> 2015-04-14 8:22 GMT+03:00 Jeff Law :
> > On 03/15/2015 02:30 PM, Richard Sandiford wrote:
> >>
> >> Ilya Enkovich writes:
> >>>
> >>> This patch allows propagation of loop invariants for i386 if propagated
> >>> value is a constant to be used in address operand. Bootstrapped and
> >>> tested on x86_64-unknown-linux-gnu. OK for trunk or stage 1?
> >>
> >>
> >> Is it necessary for this to be a target hook? The concept doesn't seem
> >> particularly target-specific. We should only propagate into the address
> >> if the new cost is no greater than the old cost, but if the address
> >> meets that condition and if propagating at this point in the pipeline is
> >> a win on x86, then wouldn't it be a win for other targets too?
> >
> > I agree with Richard here. I can't see a strong reason why this should be a
> > target hook.
> >
> > Perhaps part of the issue here is the address costing metrics may not have
> > enough context to make good decisions. In which case what context do they
> > need?
>
> At this point I don't insist on a target hook. The main reasoning was
> to not affect other targets. If we extend propagation for non constant
> values different aspects may appear. E.g. possible register pressure
> changes may significantly affect ia32. I just wanted to have an
> instrument to play with a propagation on x86 not affecting other
> targets. I don't have an opportunity to test possible performance
> implications on non-x86 targets. Don't expect (significant)
> regressions there but who knows...
>
> I'll remove the hook from this patch. Will probably introduce it later
> if some target specific cases are found.
>
> Thanks,
> Ilya
>
> >
> > Jeff
Here is a version with no hook. Bootstrapped and tested on
x86_64-unknown-linux-gnu. Is it OK for trunk?
Thanks,
Ilya
--
gcc/
2015-04-17 Ilya Enkovich
PR target/65103
* fwprop.c (forward_propagate_into): Propagate loop
invariants if a target says so.
gcc/testsuite/
2015-04-17 Ilya Enkovich
PR target/65103
* gcc.target/i386/pr65103-2.c: New.
diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index fc64ec9..82ebd01 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1365,8 +1365,18 @@ forward_propagate_into (df_ref use)
if (DF_REF_IS_ARTIFICIAL (def))
return false;
- /* Do not propagate loop invariant definitions inside the loop. */
- if (DF_REF_BB (def)->loop_father != DF_REF_BB (use)->loop_father)
+ def_insn = DF_REF_INSN (def);
+ if (multiple_sets (def_insn))
+return false;
+ def_set = single_set (def_insn);
+ if (!def_set)
+return false;
+
+ /* Do not propagate loop invariant definitions inside the loop.
+ Allow address constant propagation. */
+ if (DF_REF_BB (def)->loop_father != DF_REF_BB (use)->loop_father
+ && (DF_REF_TYPE (use) == DF_REF_REG_USE
+ || GET_CODE (SET_SRC (def_set)) != CONST))
return false;
/* Check if the use is still present in the insn! */
@@ -1379,13 +1389,6 @@ forward_propagate_into (df_ref use)
if (!reg_mentioned_p (DF_REF_REG (use), parent))
return false;
- def_insn = DF_REF_INSN (def);
- if (multiple_sets (def_insn))
-return false;
- def_set = single_set (def_insn);
- if (!def_set)
-return false;
-
/* Only try one kind of propagation. If two are possible, we'll
do it on the following iterations. */
if (forward_propagate_and_simplify (use, def_insn, def_set)
diff --git a/gcc/testsuite/gcc.target/i386/pr65103-2.c
b/gcc/testsuite/gcc.target/i386/pr65103-2.c
new file mode 100644
index 000..b7a32f7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65103-2.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target ia32 } } */
+/* { dg-require-effective-target pie } */
+/* { dg-options "-O2 -fPIE" } */
+/* { dg-final { scan-assembler-not "GOTOFF," } } */
+
+typedef struct S
+{
+ int a;
+ int b;
+} S;
+struct S gs;
+
+extern int compute ( struct S * );
+
+int test( void )
+{
+int t = -1;
+while (t)
+ {
+ gs.a++;
+ t = compute (&gs);
+ }
+return 0;
+}
Re: [PATCH] Make wider use of "v" constraint in i386.md
On Thu, Mar 19, 2015 at 10:24 AM, Ilya Tocar wrote: > Hi, > > There were some discussion about "x" constraints being too conservative > for some patterns in i386.md. > Patch below fixes it. This is probably stage1 material. > > ChangeLog: > > gcc/ > > 2015-03-19 Ilya Tocar > > * config/i386/i386.h (EXT_SSE_REG_P): New. > * config/i386/i386.md (*cmpi_mixed): Use "v" > constraint. > (*cmpi_sse): Ditto. > (*movxi_internal_avx512f): Ditto. > (define_split): Check for xmm16+, when splitting scalar float_extend. > (*extendsfdf2_mixed): Use "v" constraint. > (*extendsfdf2_sse): Ditto. > (define_split): Check for xmm16+, when splitting scalar > float_truncate. > (*truncdfsf_fast_sse): Use "v" constraint. > (fix_trunc_sse): Ditto. > (*float2_sse): Ditto. > (define_peephole2): Check for xmm16+, when converting scalar > float_truncate. > (define_peephole2): Check for xmm16+, when converting scalar > float_extend. > (*fop__comm_mixed): Use "v" constraint. > (*fop__comm_sse): Ditto. > (*fop__1_mixed): Ditto. > (*sqrt2_sse): Ditto. > (*ieee_s3): Ditto. I wonder if there are also changes needed in mmx.md. There are a couple of patterns that operate on xmm registers, so they should be reviewed if they need to change their constraint to "v" to accept extended xmm register set. Uros.
Re: PATCH] PR target/65612: Multiversioning doesn't work with DSO nor PIE
On Thu, Apr 16, 2015 at 6:28 PM, Mike Stump wrote: > On Apr 14, 2015, at 8:07 AM, H.J. Lu wrote: >>> I can confirm that the most current patch bootstraps on >>> x86_64-apple-darwin14 and that all of the new tests show up as >>> unsupported in the test suite. >>> Jack >> >> I am re-posting this patch. OK for trunk? > > If Jack is happy, I’m happy. :-) That leaves the x86 people to comment on > it. What about Solaris? Uros.
Re: [patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call
On Thu, Apr 16, 2015 at 06:47:26PM +0200, Georg-Johann Lay wrote:
> Am 04/16/2015 um 11:28 AM schrieb Senthil Kumar Selvaraj:
> >On Thu, Apr 16, 2015 at 11:02:05AM +0200, Georg-Johann Lay wrote:
> >>Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj:
> >>>This patch fixes PR 65657.
> >>
> >>The following artifact appears to be PR63633.
> >>
> >
> >I did see that one - unfortunately, that fix won't help here. IIUC, you
> >check if input/output operand hard regs are in the clobber list,
> >and if yes, you generate pseudos to save and restore clobbered hard
> >regs.
> >
> >In this case, the reg is actually clobbered by a different insn (one
>
> Arrgh, yes...
>
> >that loads the next argument to the function). So unless I blindly generate
> >pseudos for
> >all regs in the clobber list, the clobbering will still happen.
> >
> >FWIW, I did try saving/restoring all clobbered regs, and it did fix the
> >problem - just that it appeared like a (worse) hack to me. Aren't we
> >manually replicating what RA/reload should be doing?
>
> As it appears, we'll have to do it by hand. The attaches patch is just a
> sketch that indicates how the problem could be approached. Notice the new
> assertions in the split expanders; they will throw ICE until the fix is
> actually installed.
>
> The critical insn are generated in movMM expander and are changed to have no
> clobbers (SCRATCHes actually). An a later pass, when appropriate life info
> can be made available, run yet another avr pass that
>
> 1a) Save-restore needed hard regs around the insn.
>
> 2a) Kick out hard regs overlapping the clobbers, e.g. in set_src, into new
> pseudos. Maybe that could happen due to some hard regs progagation, or we
> can use a new predicate similar combine_pseudo_register_operand.
>
> 3) Replace scratch -> hard regs for all scratch_operands.
>
> 2b) Restore respective hard regs from their pseudos.
>
> 1b) Restore respective hard regs from their pseudos.
>
>
> And maybe we can get better code by allocating things like address register
> by hand and get better code then.
>
> When I implemented some of the libgcc insns I tried to express the operand
> by means of constraints, e.h. for (reg:HI 22) and let register allocator do
> the job.
>
> The resulting code was functional but *horrific*.
>
> The register allocator is not yet ready to generate efficient code in such
> demanding situations...
>
> >
> >What do you think?
> >
>
> IMO sooner or later we'll need such an infrastructure; maybe also for
> non-mov insn that are implemented by transparent libcalls like divmod, mul,
> etc.
I'm curious how other embedded targets handle this though - arm, for
example? Surely they should have some libcalls/builtins which need
specific hard regs? I should check that out.
>
> >>>When cfgexpand.c expands a function call, it first figures out the
> >>>registers in which the arguments will go, followed by expansion of the
> >>>arguments themselves (right to left). It later emits mov insns to set
> >>>the precomputed registers with the expanded RTXes.
> >>>
> >>>If one of the arguments is a __memx char pointer dereference, the mov
> >>>eventually expands to gen_xload_A (for certain cases), which
> >>>clobbers R22, R21 and Z. This causes problems if one of those
> >>>clobbered registers was precomputed to hold another argument.
> >>>
> >>>In general, call expansion does not appear to take clobbers into account -
>
> We had been warned that using hard regs is evil... But without that
> technique the code quality would decrease way too much.
Oh ok - do you know some place where this is documented or was
discussed?
>
> >>>when it checks for argument overlap, the RTX (args[i].value) is only a MEM
> >>>in QImode for the memx deref - the clobber shows up when it eventually
> >>>calls emit_move_insn, at which point, it is too late.
>
> Such situations could only be handled by a target hook which allowed to
> expand specific trees by hand... Such a hook could cater for insn that must
> use hard registers.
Yes, I guess so :(
>
> >>>This does not happen for a int pointer dereference - turns out that
> >>>precompute_register_parameters does a copy_to_mode_reg if the
> >>>cost of args[i].value is more than COSTS_N_INSNS(1) i.e., it creates a
> >>>pseudo and later assigns the pseudo to the arg register. This is done
> >>>before any moves to arg registers is done, so other arguments are not
> >>>overwritten.
> >>>
> >>>Doing the same thing - providing a better cost estimate for a MEM rtx in
> >>>the non-default address space, makes this problem go away, and that is
> >>>what this patch does. Regression testing does not show any new failures.
>
> Can you tell something about overall code quality? If it is not
> significantly worse then I'd propose to apply your rtx-costs solution soon.
> The full fix will take more time to work it out.
>
For this piece of code -
void foo ( char c, unsigned int d);
void readx (const char __memx *x)
{
Re: [PATCH] Optionally sanitize globals in user-defined sections
On Fri, Apr 17, 2015 at 10:37:50AM +0300, Yury Gribov wrote: > commit 97c141d9be45b29fb7e281dc2b7cd904e93c2813 > Author: Yury Gribov > Date: Mon Feb 2 14:33:17 2015 +0300 > > 2015-04-17 Yury Gribov > > gcc/ > * asan.c (set_sanitized_sections): New function. > (section_sanitized_p): Ditto. > (asan_protect_global): Optionally sanitize user-defined > sections. > * asan.h (set_sanitized_sections): Declare new function. > * common.opt (fsanitize-sections): New option. > * doc/invoke.texi (-fsanitize-sections): Document new option. > * opts-global.c (handle_common_deferred_options): Handle new > option. > > gcc/testsuite/ > * c-c++-common/asan/user-section-1.c: New test. Ok for trunk. Jakub
Re: [PATCH] Optionally sanitize globals in user-defined sections
On 04/17/2015 10:33 AM, Yury Gribov wrote:
Hi all,
This patch adds an optional support for sanitizing user-defined
sections. Usually this is a bad idea because ASan changes relative
position of variables in section thus breaking user assumptions. But
this is a desired feature for kernel which (ab)uses sections for various
reasons (cache optimizations, etc.).
Bootstrapped and reg-tested on x64. Ok for trunk?
The patch attached.
commit 97c141d9be45b29fb7e281dc2b7cd904e93c2813
Author: Yury Gribov
Date: Mon Feb 2 14:33:17 2015 +0300
2015-04-17 Yury Gribov
gcc/
* asan.c (set_sanitized_sections): New function.
(section_sanitized_p): Ditto.
(asan_protect_global): Optionally sanitize user-defined
sections.
* asan.h (set_sanitized_sections): Declare new function.
* common.opt (fsanitize-sections): New option.
* doc/invoke.texi (-fsanitize-sections): Document new option.
* opts-global.c (handle_common_deferred_options): Handle new
option.
gcc/testsuite/
* c-c++-common/asan/user-section-1.c: New test.
diff --git a/gcc/asan.c b/gcc/asan.c
index 9e4a629..6706af7 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -272,6 +272,7 @@ along with GCC; see the file COPYING3. If not see
static unsigned HOST_WIDE_INT asan_shadow_offset_value;
static bool asan_shadow_offset_computed;
+static const char *sanitized_sections;
/* Sets shadow offset to value in string VAL. */
@@ -294,6 +295,33 @@ set_asan_shadow_offset (const char *val)
return true;
}
+/* Set list of user-defined sections that need to be sanitized. */
+
+void
+set_sanitized_sections (const char *secs)
+{
+ sanitized_sections = secs;
+}
+
+/* Checks whether section SEC should be sanitized. */
+
+static bool
+section_sanitized_p (const char *sec)
+{
+ if (!sanitized_sections)
+return false;
+ size_t len = strlen (sec);
+ const char *p = sanitized_sections;
+ while ((p = strstr (p, sec)))
+{
+ if ((p == sanitized_sections || p[-1] == ',')
+ && (p[len] == 0 || p[len] == ','))
+ return true;
+ ++p;
+}
+ return false;
+}
+
/* Returns Asan shadow offset. */
static unsigned HOST_WIDE_INT
@@ -1374,7 +1402,8 @@ asan_protect_global (tree decl)
to be an array of such vars, putting padding in there
breaks this assumption. */
|| (DECL_SECTION_NAME (decl) != NULL
- && !symtab_node::get (decl)->implicit_section)
+ && !symtab_node::get (decl)->implicit_section
+ && !section_sanitized_p (DECL_SECTION_NAME (decl)))
|| DECL_SIZE (decl) == 0
|| ASAN_RED_ZONE_SIZE * BITS_PER_UNIT > MAX_OFILE_ALIGNMENT
|| !valid_constant_size_p (DECL_SIZE_UNIT (decl))
diff --git a/gcc/asan.h b/gcc/asan.h
index 51fd9cc..10ffaca 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -79,6 +79,8 @@ asan_red_zone_size (unsigned int size)
extern bool set_asan_shadow_offset (const char *);
+extern void set_sanitized_sections (const char *);
+
/* Return TRUE if builtin with given FCODE will be intercepted by
libasan. */
diff --git a/gcc/common.opt b/gcc/common.opt
index b49ac46..380848c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -897,6 +897,11 @@ fasan-shadow-offset=
Common Joined RejectNegative Var(common_deferred_options) Defer
-fasan-shadow-offset= Use custom shadow memory offset.
+fsanitize-sections=
+Common Joined RejectNegative Var(common_deferred_options) Defer
+-fsanitize-sections= Sanitize global variables
+in user-defined sections.
+
fsanitize-recover=
Common Report Joined
After diagnosing undefined behavior attempt to continue execution
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bb17385..f5f79b8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -301,7 +301,8 @@ Objective-C and Objective-C++ Dialects}.
@xref{Debugging Options,,Options for Debugging Your Program or GCC}.
@gccoptlist{-d@var{letters} -dumpspecs -dumpmachine -dumpversion @gol
-fsanitize=@var{style} -fsanitize-recover -fsanitize-recover=@var{style} @gol
--fasan-shadow-offset=@var{number} -fsanitize-undefined-trap-on-error @gol
+-fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1,s2,...} @gol
+-fsanitize-undefined-trap-on-error @gol
-fcheck-pointer-bounds -fchkp-check-incomplete-type @gol
-fchkp-first-field-has-own-bounds -fchkp-narrow-bounds @gol
-fchkp-narrow-to-innermost-array -fchkp-optimize @gol
@@ -5803,6 +5804,10 @@ This option forces GCC to use custom shadow offset in AddressSanitizer checks.
It is useful for experimenting with different shadow memory layouts in
Kernel AddressSanitizer.
+@item -fsanitize-sections=@var{s1,s2,...}
+@opindex fsanitize-sections
+Sanitize global variables in selected user-defined sections.
+
@item -fsanitize-recover@r{[}=@var{opts}@r{]}
@opindex fsanitize-recover
@opindex fno-sanitize-recover
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index b61bdcf..1035b8d 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -458,6 +458,10 @@ handle_common_deferre
[PATCH] Optionally sanitize globals in user-defined sections
Hi all, This patch adds an optional support for sanitizing user-defined sections. Usually this is a bad idea because ASan changes relative position of variables in section thus breaking user assumptions. But this is a desired feature for kernel which (ab)uses sections for various reasons (cache optimizations, etc.). Bootstrapped and reg-tested on x64. Ok for trunk? Best regards, Yury
Re: [PATCH] remove need for store_values_directly
On Fri, Apr 17, 2015 at 6:38 AM, wrote:
> From: Trevor Saunders
>
> Hi,
>
> Last stage 1 I introduced a second form of hash_table that stored elements of
> value_type in addition to the old form that stored elements of type value_type
> *. That lead to a fair bit of code dupplication in hash_table, but it
> simplified the transition by allowing it to take place one hash table at a
> time. Now I'm switching the rest of the hash_table users to use the new
> setup,
> and removing supporot for the old one.
>
> this was bootstrapped and regtested on x86_64-unknown-linux-gnu, and I ran
> make
> all-gcc for the following crosses to check the hash tables they use were
> correctly converted
> arm-linux-androideabi
> i686-apple-darwin
> i686-solaris2.11
> i686-w64-mingw32
> ia64-linux
> mips64-linux
> nvptx-elf
> ppc64-linux
>
> Is this ok?
Ok.
Thanks,
Richard.
> Trev
>
> gcc/
>
> * hash-table.h: Remove version of hash_table that stored value_type *.
> * asan.c, attribs.c, bitmap.c, cfg.c, cgraph.h, config/arm/arm.c,
> config/i386/winnt.c, config/ia64/ia64.c, config/mips/mips.c,
> config/sol2.c, coverage.c, cselib.c, dse.c, dwarf2cfi.c,
> dwarf2out.c, except.c, gcse.c, genmatch.c, ggc-common.c,
> gimple-ssa-strength-reduction.c, gimplify.c, haifa-sched.c,
> hard-reg-set.h, hash-map.h, hash-set.h, ipa-devirt.c, ipa-icf.h,
> ipa-profile.c, ira-color.c, ira-costs.c, loop-invariant.c,
> loop-iv.c, loop-unroll.c, lto-streamer.h, plugin.c, postreload-gcse.c,
> reginfo.c, statistics.c, store-motion.c, trans-mem.c, tree-cfg.c,
> tree-eh.c, tree-hasher.h, tree-into-ssa.c, tree-parloops.c,
> tree-sra.c, tree-ssa-coalesce.c, tree-ssa-dom.c, tree-ssa-live.c,
> tree-ssa-loop-im.c, tree-ssa-loop-ivopts.c, tree-ssa-phiopt.c,
> tree-ssa-pre.c, tree-ssa-reassoc.c, tree-ssa-sccvn.c,
> tree-ssa-structalias.c, tree-ssa-tail-merge.c,
> tree-ssa-threadupdate.c, tree-vectorizer.c, tree-vectorizer.h,
> valtrack.h, var-tracking.c, vtable-verify.c, vtable-verify.h: Adjust.
>
>
> libcc1/
>
> * plugin.cc: Adjust for hash_table changes.
>
> java/
>
> * jcf-io.c: Adjust for hash_table changes.
>
> lto/
>
> * lto.c: Adjust for hash_table changes.
>
> objc/
>
> * objc-act.c: Adjust for hash_table changes.
>
> diff --git a/gcc/asan.c b/gcc/asan.c
> index 9e4a629..7b70ee2 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -407,11 +407,11 @@ asan_mem_ref_get_end (const asan_mem_ref *ref, tree len)
> struct asan_mem_ref_hasher
>: typed_noop_remove
> {
> - typedef asan_mem_ref value_type;
> - typedef asan_mem_ref compare_type;
> + typedef asan_mem_ref *value_type;
> + typedef asan_mem_ref *compare_type;
>
> - static inline hashval_t hash (const value_type *);
> - static inline bool equal (const value_type *, const compare_type *);
> + static inline hashval_t hash (const asan_mem_ref *);
> + static inline bool equal (const asan_mem_ref *, const asan_mem_ref *);
> };
>
> /* Hash a memory reference. */
> diff --git a/gcc/attribs.c b/gcc/attribs.c
> index c18bff2..7b7e2a9 100644
> --- a/gcc/attribs.c
> +++ b/gcc/attribs.c
> @@ -67,21 +67,21 @@ substring_hash (const char *str, int l)
>
> struct attribute_hasher : typed_noop_remove
> {
> - typedef attribute_spec value_type;
> - typedef substring compare_type;
> - static inline hashval_t hash (const value_type *);
> - static inline bool equal (const value_type *, const compare_type *);
> + typedef attribute_spec *value_type;
> + typedef substring *compare_type;
> + static inline hashval_t hash (const attribute_spec *);
> + static inline bool equal (const attribute_spec *, const substring *);
> };
>
> inline hashval_t
> -attribute_hasher::hash (const value_type *spec)
> +attribute_hasher::hash (const attribute_spec *spec)
> {
>const int l = strlen (spec->name);
>return substring_hash (spec->name, l);
> }
>
> inline bool
> -attribute_hasher::equal (const value_type *spec, const compare_type *str)
> +attribute_hasher::equal (const attribute_spec *spec, const substring *str)
> {
>return (strncmp (spec->name, str->str, str->length) == 0
> && !spec->name[str->length]);
> diff --git a/gcc/bitmap.c b/gcc/bitmap.c
> index d43a39f..71d5b11 100644
> --- a/gcc/bitmap.c
> +++ b/gcc/bitmap.c
> @@ -61,20 +61,20 @@ struct loc
>
> struct bitmap_desc_hasher : typed_noop_remove
> {
> - typedef bitmap_descriptor_d value_type;
> - typedef loc compare_type;
> - static inline hashval_t hash (const value_type *);
> - static inline bool equal (const value_type *, const compare_type *);
> + typedef bitmap_descriptor_d *value_type;
> + typedef loc *compare_type;
> + static inline hashval_t hash (const bitmap_descriptor_d *);
> + static inline bool equal (const bitmap_descriptor_d *, const loc *);
> };
>
> inline hashval_t
> -bitmap_desc_hasher::hash (const value_type *d)
> +bitmap_desc_hasher:
Re: [PATCH] Fix dwarf2out ICE (PR debug/65771)
On Thu, Apr 16, 2015 at 11:11 PM, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, on the following testcase we ICE, because for
> # DEBUG D#2 => b
> # DEBUG D#1 => a[D#2].t
> # DEBUG c => D#1
> during expansion we get the a[D#2].t added as MEM_EXPR of a MEM, and because
> we can't mem_loc_descriptor that MEM (I'll post separately a trunk only
> patch that fixes that in this case, but generally not all MEMs can be
> represented in debug info), we try harder and try to use MEM_EXPR in
> loc_list_from_tree, but that one ICEs on DEBUG_EXPR_DECL. There is nothing
> we can do for those at this point, debug_exprs are only useful to
> var-tracking, so returning NULL is the only thing we can do for those.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and 5.1?
Ok.
Thanks,
Richard.
> 2015-04-16 Jakub Jelinek
>
> PR debug/65771
> * dwarf2out.c (loc_list_from_tree): Return NULL
> for DEBUG_EXPR_DECL.
>
> * gcc.dg/debug/pr65771.c: New test.
>
> --- gcc/dwarf2out.c.jj 2015-04-16 16:51:52.0 +0200
> +++ gcc/dwarf2out.c 2015-04-16 16:57:28.866134980 +0200
> @@ -14642,6 +14642,7 @@ loc_list_from_tree (tree loc, int want_a
>
> case TARGET_MEM_REF:
> case SSA_NAME:
> +case DEBUG_EXPR_DECL:
>return NULL;
>
> case COMPOUND_EXPR:
> --- gcc/testsuite/gcc.dg/debug/pr65771.c.jj 2015-04-16 17:00:23.811328842
> +0200
> +++ gcc/testsuite/gcc.dg/debug/pr65771.c2015-04-16 17:00:13.0
> +0200
> @@ -0,0 +1,15 @@
> +/* PR debug/65771 */
> +/* { dg-do link } */
> +/* { dg-require-effective-target tls } */
> +
> +struct S { int s; int t; };
> +__thread struct S a[10];
> +int b;
> +
> +int
> +main ()
> +{
> + int c = a[b].t;
> + (void) c;
> + return 0;
> +}
>
> Jakub
