Re: [PATCH, testsuite] Fix alignment in movapd tests
On Wed, Dec 11, 2013 at 3:37 PM, Ryan Mansfield rmansfi...@qnx.com wrote: 2013-12-10 Ryan Mansfield rmansfi...@qnx.com PR testsuite/59442 * gcc.target/i386/sse2-movapd-1.c: Fix alignment attributes. * gcc.target/i386/sse2-movapd-2.c: Likewise. * gcc.target/i386/avx-vmovapd-256-1.c: Likewise. * gcc.target/i386/avx-vmovapd-256-2.c: Likewise. OK for mainline and release branches. Thanks. Could someone please apply it for me? Done. Uros.
Re: C++ edge_iterator (was: Re: [SH] PR 53976 - Add RTL pass to eliminate clrt, sett insns)
On Wed, Dec 11, 2013 at 06:47:37PM +0100, Oleg Endo wrote:
On Thu, 2013-11-21 at 00:04 +0100, Steven Bosscher wrote:
Declaring the edge_iterator inside the for() is not a good argument
against FOR_EACH_EDGE. Of course, brownie points are up for grabs for
the brave soul daring enough to make edge iterators be proper C++
iterators... ;-)
so, as a first question why do we have a special edge iterator at all? it
seems like we could just have a vec iterator and use that removing a
bunch of indirection that seems pretty useless.
So, I gave it a try -- see the attached patch.
It allows edge iteration to look more like STL container iteration:
for (basic_block::edge_iterator ei = bb-pred_edges ().begin ();
ei != bb-pred_edges ().end (); ++ei)
{
basic_block pred_bb = (*ei)-src;
...
}
personally I'm not really a fan of overloading ++ / * that way, but I
can't speak for anyone else. I'd prefer something like
for (vec_iterator i = vec.forward_iterator (); !i.done (); i.next ())
and
for (backward_vec_iterator i = vec.backward_iterator (); !i.done (); i.next ())
but that might break range base for loops?
Then the
typedef struct basic_block_def* basic_block;
is replaced with a wrapper class 'basic_block', which is just a simple
POD wrapper around a basic_block_def*. There should be no penalties
compared to passing/storing raw pointers. Because of the union with
constructor restriction of C++98 an additional wrapper class
'basic_block_in_union' is required, which doesn't have any constructors
defined.
Having 'basic_block' as a class allows putting typedefs for the edge
iterator types in there (initially I tried putting the typedefs into
struct basic_block_def, but gengtype would bail out).
namespacing like that seems a little messy, but so is vec_iterator or
such I guess.
It would also be possible to have a free standing definition / typedef
of edge_iterator, but it would conflict with the existing one and
require too many changes at once. Moreover, the iterator type actually
I bet it'll be a lot of work but changing everything seems nice so maybe
its worth just sitting down for a couple days and banging it out if it
gives nicer names?
depends on the container type, which is vecedge, ..., and the
container type is defined/selected by the basic_block class.
I don't see how this is relevent
The following
basic_block pred_bb = (*ei)-src;
can also be written as
basic_block pred_bb = ei-src;
after converting the edge typedef to a wrapper of edge_def*.
this is assuming you overload operator - on the iterator? I'm a c++ guy
not a stl guy, but that seems pretty dubious to me.
The idea of the approach is to allow co-existence of the new
edge_iterator and the old and thus be able to gradually convert code.
The wrappers around raw pointers also helo encapsulating the underlying
memory management issues. For example, it would be much easier to
replace garbage collected objects with intrusive reference counting.
I don't think there's actually a memory management issue here,
edge_iterator can only work if you allocate it on the stack since its
not marked for gty, and afaik ggc doesn't scan the stack so the
edge_iterator can't keep the vector alive. Now I think it would be nice
if these vectors moved out of gc memory, but I don't think this is
particularly helpful for that.
Trev
Comments and feedback appreciated.
Cheers,
Oleg
Index: gcc/coretypes.h
===
--- gcc/coretypes.h (revision 205801)
+++ gcc/coretypes.h (working copy)
@@ -153,8 +153,8 @@
typedef struct edge_def *edge;
typedef const struct edge_def *const_edge;
struct basic_block_def;
-typedef struct basic_block_def *basic_block;
-typedef const struct basic_block_def *const_basic_block;
+class basic_block;
+class const_basic_block;
#define obstack_chunk_alloc ((void *(*) (long)) xmalloc)
#define obstack_chunk_free ((void (*) (void *)) free)
Index: gcc/tracer.c
===
--- gcc/tracer.c (revision 205801)
+++ gcc/tracer.c (working copy)
@@ -102,7 +102,7 @@
/* A transaction is a single entry multiple exit region. It must be
duplicated in its entirety or not at all. */
- g = last_stmt (CONST_CAST_BB (bb));
+ g = last_stmt (basic_block (bb));
if (g gimple_code (g) == GIMPLE_TRANSACTION)
return true;
Index: gcc/emit-rtl.c
===
--- gcc/emit-rtl.c(revision 205801)
+++ gcc/emit-rtl.c(working copy)
@@ -4446,7 +4446,7 @@
emit_note_after (enum insn_note subtype, rtx after)
{
rtx note = make_note_raw (subtype);
- basic_block bb = BARRIER_P (after) ? NULL : BLOCK_FOR_INSN (after);
+ basic_block bb = BARRIER_P (after) ? (basic_block_def*)NULL :
(basic_block_def*)BLOCK_FOR_INSN (after);
bool on_bb_boundary_p = (bb
Re: [gofrontend-dev] Go patch committed: Implement method values in reflect package
Ian Lance Taylor i...@google.com writes: This patch to the Go frontend and libgo implements method values in the reflect package. Working with method values and reflect now works correctly, at least on x86. Can you give me a test case? I can try it on a few other architectures tomorrow. Cheers, mwh This changes the type signature for type methods in the reflect package to match the gc compiler. That in turn required changing the reflect package to mark method values with a new flag, as previously they were detected by the type signature. The MakeFunc support needed to create a function that takes a value and passes a pointer to the method, since all methods take pointers. It also needed to create a function that holds a receiver value. And the recover code needed to handle these new cases. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.8 branch. Ian
[committed] Diagnose invalid OpenMP copyprivate clause arguments (PR libgomp/59467)
Hi!
Copyprivate clause has following restriction (in 2.5, 3.0, 3.1 and 4.0):
All list items that appear in the copyprivate clause must be either
threadprivate or private in the enclosing context.
but we weren't diagnosing it and even in crayptr2.f90 testcase violated it.
Fixed thusly, regtested on x86_64-linux, committed to trunk and 4.8.
2013-12-12 Jakub Jelinek ja...@redhat.com
PR libgomp/59467
* gimplify.c (omp_check_private): Add copyprivate argument, if it
is true, don't check omp_privatize_by_reference.
(gimplify_scan_omp_clauses): For OMP_CLAUSE_COPYPRIVATE verify
decl is private in outer context. Adjust omp_check_private caller.
* gfortran.dg/gomp/pr59467.f90: New test.
* c-c++-common/gomp/pr59467.c: New test.
* testsuite/libgomp.fortran/crayptr2.f90: Add private (d) clause to
!$omp parallel.
--- gcc/gimplify.c.jj 2013-12-03 08:44:02.0 +0100
+++ gcc/gimplify.c 2013-12-11 13:40:13.098838596 +0100
@@ -5829,7 +5829,7 @@ omp_is_private (struct gimplify_omp_ctx
region's REDUCTION clause. */
static bool
-omp_check_private (struct gimplify_omp_ctx *ctx, tree decl)
+omp_check_private (struct gimplify_omp_ctx *ctx, tree decl, bool copyprivate)
{
splay_tree_node n;
@@ -5838,8 +5838,11 @@ omp_check_private (struct gimplify_omp_c
ctx = ctx-outer_context;
if (ctx == NULL)
return !(is_global_var (decl)
-/* References might be private, but might be shared too. */
-|| lang_hooks.decls.omp_privatize_by_reference (decl));
+/* References might be private, but might be shared too,
+ when checking for copyprivate, assume they might be
+ private, otherwise assume they might be shared. */
+|| (!copyprivate
+ lang_hooks.decls.omp_privatize_by_reference (decl)));
if ((ctx-region_type (ORT_TARGET | ORT_TARGET_DATA)) != 0)
continue;
@@ -6049,12 +6052,36 @@ gimplify_scan_omp_clauses (tree *list_p,
remove = true;
break;
}
+ if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_COPYPRIVATE
+ !remove
+ !omp_check_private (ctx, decl, true))
+ {
+ remove = true;
+ if (is_global_var (decl))
+ {
+ if (DECL_THREAD_LOCAL_P (decl))
+ remove = false;
+ else if (DECL_HAS_VALUE_EXPR_P (decl))
+ {
+ tree value = get_base_address (DECL_VALUE_EXPR (decl));
+
+ if (value
+ DECL_P (value)
+ DECL_THREAD_LOCAL_P (value))
+ remove = false;
+ }
+ }
+ if (remove)
+ error_at (OMP_CLAUSE_LOCATION (c),
+ copyprivate variable %qE is not threadprivate
+ or private in outer context, DECL_NAME (decl));
+ }
do_notice:
if (outer_ctx)
omp_notice_variable (outer_ctx, decl, true);
if (check_non_private
region_type == ORT_WORKSHARE
- omp_check_private (ctx, decl))
+ omp_check_private (ctx, decl, false))
{
error (%s variable %qE is private in outer context,
check_non_private, DECL_NAME (decl));
--- gcc/testsuite/gfortran.dg/gomp/pr59467.f90.jj 2013-12-11
12:56:10.651397907 +0100
+++ gcc/testsuite/gfortran.dg/gomp/pr59467.f90 2013-12-11 12:59:44.336333836
+0100
@@ -0,0 +1,24 @@
+! PR libgomp/59467
+! { dg-do compile }
+! { dg-options -fopenmp }
+ FUNCTION t()
+INTEGER :: a, b, t
+a = 0
+b = 0
+!$OMP PARALLEL REDUCTION(+:b)
+ !$OMP SINGLE ! { dg-error is not threadprivate or private in outer
context }
+!$OMP ATOMIC WRITE
+a = 6
+ !$OMP END SINGLE COPYPRIVATE (a)
+ b = a
+!$OMP END PARALLEL
+t = b
+b = 0
+!$OMP PARALLEL REDUCTION(+:b)
+ !$OMP SINGLE
+!$OMP ATOMIC WRITE
+b = 6
+ !$OMP END SINGLE COPYPRIVATE (b)
+!$OMP END PARALLEL
+t = t + b
+ END FUNCTION
--- gcc/testsuite/c-c++-common/gomp/pr59467.c.jj2013-12-11
12:55:35.115581435 +0100
+++ gcc/testsuite/c-c++-common/gomp/pr59467.c 2013-12-11 13:09:58.879187656
+0100
@@ -0,0 +1,68 @@
+/* PR libgomp/59467 */
+
+int v;
+
+void
+foo (void)
+{
+ int x = 0, y = 0;
+ #pragma omp parallel
+ {
+int z;
+#pragma omp single copyprivate (x) /* { dg-error is not threadprivate or
private in outer context } */
+{
+ #pragma omp atomic write
+ x = 6;
+}
+#pragma omp atomic read
+z = x;
+#pragma omp atomic
+y += z;
+ }
+ #pragma omp parallel
+ {
+int z;
+#pragma omp single copyprivate (v) /* { dg-error is not threadprivate or
private in outer context } */
+{
Re: _Cilk_spawn and _Cilk_sync for C++
Iyer, Balaji V balaji.v.i...@intel.com writes:
diff --git a/gcc/testsuite/g++.dg/cilk-plus/cilk-plus.exp
b/gcc/testsuite/g++.dg/cilk-plus/cilk-plus.exp
index 707d17e..36c8111 100644
--- a/gcc/testsuite/g++.dg/cilk-plus/cilk-plus.exp
+++ b/gcc/testsuite/g++.dg/cilk-plus/cilk-plus.exp
@@ -22,6 +22,14 @@ if { ![check_effective_target_cilkplus] } {
return;
}
+verbose $tool $libdir 1
+set library_var [get_multilibs]
+# Pointing the ld_library_path to the Cilk Runtime library binaries.
+set ld_library_path $[get_multilibs]/libcilkrts/.libs
+
+set ALWAYS_CFLAGS
+lappend ALWAYS_CFLAGS -L${library_var}/libcilkrts/.libs
This is broken and useless. Remove that.
Andreas.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
And now for something completely different.
GOMP_target: alignment (was: [gomp4] #pragma omp target* fixes)
Hi!
On Thu, 5 Sep 2013 18:11:05 +0200, Jakub Jelinek ja...@redhat.com wrote:
3) I figured out we need to tell the runtime library not just
address, size and kind, but also alignment (we won't need that for
the #pragma omp declare target global vars though), so that the
runtime library can properly align it. As TYPE_ALIGN/DECL_ALIGN
is in bits and is 32 bit wide, when that is in bytes and we only care
about power of twos, I've decided to encode it in the upper 5 bits
of the kind (lower 3 bits are used for OMP_CLAUSE_MAP_* kind).
Unfortunately, this scheme breaks down with OpenACC: we need an
additional bit to codify a flag for present_or_* map clauses (meaning:
only map the data (allocate/to/from/tofrom, as for OpenMP) if not already
present on the device).
With five bits available for the OpenMP case, we can describe alignments
up to 2 GiB, and I've empirically found on my development system that the
largest possible alignment is MAX_OFILE_ALIGNMENT, 256 MiB for ELF
systems, so that's fine. But with only four bits available, we get to
describe alignments up to 1 ((1 4) - 1) = 32 KiB, which is too
small -- even though it'd be fine for normal usage of __attribute__
((aligned (x))).
So it seems our options are to use a bigger datatype for the kinds array,
to split off from the kinds array a new alignments array, or to generally
switch to using an array of a struct containing hostaddr, size,
alignment, kind. The latter would require additional changes in the
child_fn.
As it's an ABI change no matter what, would you like to see this limited
to OpenACC? Changing it also for OpenMP's GOMP_target would have the
advantage to have them not diverge (especially at the generating side in
omp-low.c's lowering functions), but I'm not sure whether such an ABI
change would easily be possible now, with the OpenMP 4 support merged
into trunk -- though, it is not yet part of a regular GCC release?
--- gcc/omp-low.c.jj 2013-09-05 09:19:03.0 +0200
+++ gcc/omp-low.c 2013-09-05 17:11:14.693638660 +0200
@@ -9342,6 +9349,11 @@ lower_omp_target (gimple_stmt_iterator *
| unsigned char tkind = 0;
| switch (OMP_CLAUSE_CODE (c))
| {
| case OMP_CLAUSE_MAP:
| tkind = OMP_CLAUSE_MAP_KIND (c);
| break;
| case OMP_CLAUSE_TO:
| tkind = OMP_CLAUSE_MAP_TO;
| break;
| case OMP_CLAUSE_FROM:
| tkind = OMP_CLAUSE_MAP_FROM;
| break;
default:
gcc_unreachable ();
}
+ unsigned int talign = TYPE_ALIGN_UNIT (TREE_TYPE (ovar));
+ if (DECL_P (ovar) DECL_ALIGN_UNIT (ovar) talign)
+ talign = DECL_ALIGN_UNIT (ovar);
+ talign = ceil_log2 (talign);
+ tkind |= talign 3;
CONSTRUCTOR_APPEND_ELT (vkind, purpose,
build_int_cst (unsigned_char_type_node,
tkind));
The use of OMP_CLAUSE_MAP_* on the generating and integer numerals on the
receiving (libgomp) side is a bit unesthetic, likewise for the hard-coded
3 in the bit shift. What would be the standard GCC way of sharing a
description of the tkind layout between gcc/omp-low.c and
libgomp/target.c? Are we allowed to #include (a new header file)
libgomp/target.h from gcc/omp-low.c?
To avoid silent breakage should alignments bigger than 2 GiB be allowed
in a distant future, would a check like the following be appropriate?
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -10378,6 +10383,11 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
omp_context *ctx)
unsigned int talign = TYPE_ALIGN_UNIT (TREE_TYPE (ovar));
if (DECL_P (ovar) DECL_ALIGN_UNIT (ovar) talign)
talign = DECL_ALIGN_UNIT (ovar);
+ const unsigned int talign_max
+ = 1 ((1 (BITS_PER_UNIT - 3)) - 1);
+ if (talign talign_max)
+ sorry (can't encode alignment of %u bytes, which is bigger than
+%u bytes, talign, talign_max);
talign = ceil_log2 (talign);
tkind |= talign 3;
CONSTRUCTOR_APPEND_ELT (vkind, purpose,
Grüße,
Thomas
pgp_bUwzkKWWX.pgp
Description: PGP signature
Re: GOMP_target: alignment (was: [gomp4] #pragma omp target* fixes)
On Thu, Dec 12, 2013 at 10:53:02AM +0100, Thomas Schwinge wrote: On Thu, 5 Sep 2013 18:11:05 +0200, Jakub Jelinek ja...@redhat.com wrote: 3) I figured out we need to tell the runtime library not just address, size and kind, but also alignment (we won't need that for the #pragma omp declare target global vars though), so that the runtime library can properly align it. As TYPE_ALIGN/DECL_ALIGN is in bits and is 32 bit wide, when that is in bytes and we only care about power of twos, I've decided to encode it in the upper 5 bits of the kind (lower 3 bits are used for OMP_CLAUSE_MAP_* kind). Unfortunately, this scheme breaks down with OpenACC: we need an additional bit to codify a flag for present_or_* map clauses (meaning: only map the data (allocate/to/from/tofrom, as for OpenMP) if not already present on the device). The OpenMP behavior is always only map the data (allocate/to/from/tofrom) if not already mapped on the device. So what behavior does OpenACC have if present_or_* isn't present? Jakub
Re: GOMP_target: alignment (was: [gomp4] #pragma omp target* fixes)
Hi!
On Thu, 12 Dec 2013 11:02:30 +0100, Jakub Jelinek ja...@redhat.com wrote:
On Thu, Dec 12, 2013 at 10:53:02AM +0100, Thomas Schwinge wrote:
On Thu, 5 Sep 2013 18:11:05 +0200, Jakub Jelinek ja...@redhat.com wrote:
3) I figured out we need to tell the runtime library not just
address, size and kind, but also alignment (we won't need that for
the #pragma omp declare target global vars though), so that the
runtime library can properly align it. As TYPE_ALIGN/DECL_ALIGN
is in bits and is 32 bit wide, when that is in bytes and we only care
about power of twos, I've decided to encode it in the upper 5 bits
of the kind (lower 3 bits are used for OMP_CLAUSE_MAP_* kind).
Unfortunately, this scheme breaks down with OpenACC: we need an
additional bit to codify a flag for present_or_* map clauses (meaning:
only map the data (allocate/to/from/tofrom, as for OpenMP) if not already
present on the device).
The OpenMP behavior is always only map the data (allocate/to/from/tofrom)
if not already mapped on the device. So what behavior does OpenACC have
if present_or_* isn't present?
OpenACC has a concept of (possibly nested) data regions (for reference,
OpenACC 2.0, 2.6.2 Data Regions and Data Lifetimes), and the semantics
are as follows:
#pragma acc parallel copy(x[0:n])
for (int i = 0; i n; ++i)
x[i] += 1;
This will first allocate the x array on the device, copy the host's x
array to the device's, then execute the structured block, then copy back
the data from the device to the host, then deallocate the copy on the
device.
#pragma acc parallel present_or_copy(x[0:n])
for (int i = 0; i n; ++i)
x[i] += 1;
If the x array is not present on the device, this will proceed as for the
copy clause just described. If the data already is present, this will
directly proceed to executing the structured block, then *not* copy back
the data from the device to the host, and *not* deallocate the copy on
the device.
The reason is that often you'd first set up explicit data regions around
several OpenACC pragmas, as data movement is expensive, and the compiler
has a hard time figuring out when it might be avoided. For example:
void foo(int n, float *x)
{
#pragma acc parallel present_or_copy(x[0:n])
for (int i = 0; i n; ++i)
x[i] += 1;
}
void bar(int n, float *x1, float *x2)
{
foo(n, x1);
#pragma acc enter data copyin(x2[0:n])
foo(n, x2);
[...]
foo(n, x2);
[...]
foo(n, x2);
#pragma acc exit data copyout(x2[0:n])
// Now use x2 on the host.
}
For x1, when executing foo, the runtime will do: allocate on device,
copyin, execute, copyout, deallocate on device -- that is, the
present_or_copy clause handled as a copy clause.
For x2, the data will first manually be allocated on and copied to the
device, entering a dynamic data region, and when executing foo is already
present (so, the present_or_copy clause basically becomes a no-op), and
then manually be copied out and deallocated, terminating the data region.
Apart from the different semantics of deallocation, while I couldn't
quickly find it in the pragmas' descriptions, the description for the
acc_copyin runtime library function explicitly states that »it is a
runtime error to call this routine if the data is already present on the
device«.
Grüße,
Thomas
pgpJCZsthGML8.pgp
Description: PGP signature
Re: New tsan tests.
Anyway, let's keep the current tests as is, the patch is ok for trunk. Commited in 205925.
RE: Two build != host fixes
I have some more fixes for Ada cross-builds that Eric commented on but need a little more work - will try to re-test this evening and re-post tomorrow. It's also PR ada/55946. Would mind trying the attached patch? -- Eric Botcazou Hi Eric, your patch looks quite nice, (maybe s/host_alias= @host_alias@/host_alias = @host_alias@/) but I have to make a few more patches, to get it working: error: system.ads has restriction No_Implicit_Dynamic_Code error: but the following files violate this restriction: error: make.adb error: makeutl.adb error: prj.adb error: prj-env.adb error: prj-conf.adb error: prj-proc.adb error: prj-nmsc.adb that's the most weird point. If I get rid of that pragma, and re-build everything I get something that looks like a working gcc/gnat. The problem with SSIZE_MAX is this: it is not defined in gcc/glimits.h, and in the host!=build cross-compiler fix-includes replaces the working limits.h with that one, so my build fails. Bernd. patch-cross-build.diff Description: Binary data
Re: Two build != host fixes
your patch looks quite nice, (maybe s/host_alias= @host_alias@/host_alias = @host_alias@/) Thanks for spotting it, now fixed. but I have to make a few more patches, to get it working: error: system.ads has restriction No_Implicit_Dynamic_Code error: but the following files violate this restriction: error: make.adb error: makeutl.adb error: prj.adb error: prj-env.adb error: prj-conf.adb error: prj-proc.adb error: prj-nmsc.adb that's the most weird point. If I get rid of that pragma, and re-build everything I get something that looks like a working gcc/gnat. Nice progress! To be fair, the violation of No_Implicit_Dynamic_Code was also reported under PR ada/55946, but the fix isn't to remove the pragma, as this particular version of system.ads should be used only to build the compiler and not the tools. This issue is supposed to be addressed by the RTS_DIR thing: # Put the host RTS dir first in the PATH to hide the default runtime # files that are among the sources RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib ))) TOOLS_FLAGS_TO_PASS_CROSS= \ CC=$(CC) \ CXX=$(CXX) \ CFLAGS=$(CFLAGS) $(WARN_CFLAGS) \ LDFLAGS=$(LDFLAGS) \ ADAFLAGS=$(ADAFLAGS) \ ADA_CFLAGS=$(ADA_CFLAGS) \ INCLUDES=$(INCLUDES_FOR_SUBDIR) \ ADA_INCLUDES=-I$(RTS_DIR)../adainclude -I$(RTS_DIR) Could you post the command line for the compilation of one of the problematic units for the gnattools? -- Eric Botcazou
Re: Two build != host fixes
Hi Eric, On 12 Dec 2013, at 12:11, Bernd Edlinger wrote: I have some more fixes for Ada cross-builds that Eric commented on but need a little more work - will try to re-test this evening and re-post tomorrow. It's also PR ada/55946. Would mind trying the attached patch? -- Eric Botcazou Hi Eric, your patch looks quite nice, (maybe s/host_alias= @host_alias@/host_alias = @host_alias@/) but I have to make a few more patches, to get it working: error: system.ads has restriction No_Implicit_Dynamic_Code error: but the following files violate this restriction: error: make.adb error: makeutl.adb error: prj.adb error: prj-env.adb error: prj-conf.adb error: prj-proc.adb error: prj-nmsc.adb using your patch + the mod I made for LDFLAGS. I built x86_64-darwin12 X powerpc-darwin9 [build = x86_64-darwin12] and then a native X powerpc-darwin9 [build = x86_64-darwin12] Ada built and I can do gnatmake hello.adb on the powerpc system and produce a working exe, gcc and g++ seem to produce similar test output from recent runs. Do you have a way to run acats for an out-of-tree test? Iain
Re: Two build != host fixes
using your patch + the mod I made for LDFLAGS. In gcc-interface/Makefile.in? I wasn't sure if it was really needed. Out of curiosity, what do you set LDFLAGS to exactly? I built x86_64-darwin12 X powerpc-darwin9 [build = x86_64-darwin12] and then a native X powerpc-darwin9 [build = x86_64-darwin12] Ada built and I can do gnatmake hello.adb on the powerpc system and produce a working exe, gcc and g++ seem to produce similar test output from recent runs. That's nice. Do you have a way to run acats for an out-of-tree test? Presumably not, but you could try to run gnat.dg instead. -- Eric Botcazou
Re: Two build != host fixes
Hi Eric, On 12 Dec 2013, at 12:34, Eric Botcazou wrote: using your patch + the mod I made for LDFLAGS. In gcc-interface/Makefile.in? I wasn't sure if it was really needed. Out of curiosity, what do you set LDFLAGS to exactly? Darwin doesn't have gettext in libSystem, I build it as a convenience library, but it still needs to refer to a system framework. For this to link the gnattools I need: LDFLAGS=-L/path/to/my/convenience/lib -framework CoreFoundation I suppose that, one day, the make machinery should do something like LDFLAGS = @LDFLAGS@ to save me putting this on every GCC build line ... I built x86_64-darwin12 X powerpc-darwin9 [build = x86_64-darwin12] and then a native X powerpc-darwin9 [build = x86_64-darwin12] Ada built and I can do gnatmake hello.adb on the powerpc system and produce a working exe, gcc and g++ seem to produce similar test output from recent runs. That's nice. Do you have a way to run acats for an out-of-tree test? Presumably not, but you could try to run gnat.dg instead. OK - will give that a whirl next time i am in front of that machine.. It would be a useful thing to make an acats script variant to work with installed/out-of-tree testing. Iain
RE: Two build != host fixes
your patch looks quite nice, (maybe s/host_alias= @host_alias@/host_alias = @host_alias@/) Thanks for spotting it, now fixed. but I have to make a few more patches, to get it working: error: system.ads has restriction No_Implicit_Dynamic_Code error: but the following files violate this restriction: error: make.adb error: makeutl.adb error: prj.adb error: prj-env.adb error: prj-conf.adb error: prj-proc.adb error: prj-nmsc.adb that's the most weird point. If I get rid of that pragma, and re-build everything I get something that looks like a working gcc/gnat. Nice progress! To be fair, the violation of No_Implicit_Dynamic_Code was also reported under PR ada/55946, but the fix isn't to remove the pragma, as this particular version of system.ads should be used only to build the compiler and not the tools. This issue is supposed to be addressed by the RTS_DIR thing: # Put the host RTS dir first in the PATH to hide the default runtime # files that are among the sources RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib ))) TOOLS_FLAGS_TO_PASS_CROSS= \ CC=$(CC) \ CXX=$(CXX) \ CFLAGS=$(CFLAGS) $(WARN_CFLAGS) \ LDFLAGS=$(LDFLAGS) \ ADAFLAGS=$(ADAFLAGS) \ ADA_CFLAGS=$(ADA_CFLAGS) \ INCLUDES=$(INCLUDES_FOR_SUBDIR) \ ADA_INCLUDES=-I$(RTS_DIR)../adainclude -I$(RTS_DIR) Could you post the command line for the compilation of one of the problematic units for the gnattools? -- Eric Botcazou arm-linux-gnueabihf-gcc -c -I./ -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4.9.0/adalib/../adainclude -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4.9.0/adalib/ -I. -I/home/ed/gnu/x/gcc-4.9-20131208/gcc/ada -g -O2 -W -Wall -gnatpg -gnata -I- /home/ed/gnu/x/gcc-4.9-20131208/gcc/ada/makeutl.adb arm-linux-gnueabihf-gcc -c -I./ -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4.9.0/adalib/../adainclude -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4.9.0/adalib/ -I. -I/home/ed/gnu/x/gcc-4.9-20131208/gcc/ada -g -O2 -W -Wall -gnatpg -gnata -I- /home/ed/gnu/x/gcc-4.9-20131208/gcc/ada/prj-env.adb /home/ed/gnu/x/arm-linux-gnueabihf-linux64: where the build=host target=arm-linux-gnueabihf compiler is installed. Bernd.
Re: [C++ PATCH] Fix GC related issues in C++ FE (PR c++/58627)
On Wed, Dec 11, 2013 at 11:51:55AM -0500, Jason Merrill wrote: It's only safe to free the targs if they weren't used to instantiate any templates, so I lean toward option #1. Did you test this with strict gc? Ok, after IRC discussion and another bootstrap/regtest I've installed this variant instead: 2013-12-12 Jakub Jelinek ja...@redhat.com PR c++/58627 * call.c (add_template_candidate_real): Don't call ggc_free on targs. --- gcc/cp/class.c.jj 2013-11-28 08:18:58.0 +0100 +++ gcc/cp/class.c 2013-12-11 20:57:40.155059669 +0100 @@ -7475,8 +7475,6 @@ resolve_address_of_overloaded_function ( /* See if there's a match. */ if (same_type_p (target_fn_type, static_fn_type (instantiation))) matches = tree_cons (instantiation, fn, matches); - - ggc_free (targs); } /* Now, remove all but the most specialized of the matches. */ Jakub
Re: [C++ Patch] Fix __is_base_of vs incomplete types
I wouldn't expect it to cause problems. Jason
Re: Two build != host fixes
arm-linux-gnueabihf-gcc -c -I./ -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4. 9.0/adalib/../adainclude -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4. 9.0/adalib/ -I. -I/home/ed/gnu/x/gcc-4.9-20131208/gcc/ada -g -O2 -W -Wall -gnatpg -gnata -I- /home/ed/gnu/x/gcc-4.9-20131208/gcc/ada/makeutl.adb arm-linux-gnueabihf-gcc -c -I./ -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4. 9.0/adalib/../adainclude -I/home/ed/gnu/x/arm-linux-gnueabihf-linux64/lib/gcc/arm-linux-gnueabihf/4. 9.0/adalib/ -I. -I/home/ed/gnu/x/gcc-4.9-20131208/gcc/ada -g -O2 -W -Wall -gnatpg -gnata -I- /home/ed/gnu/x/gcc-4.9-20131208/gcc/ada/prj-env.adb /home/ed/gnu/x/arm-linux-gnueabihf-linux64: where the build=host target=arm-linux-gnueabihf compiler is installed. OK, I think that this compiler is misconfigured, could you try this Index: gcc-interface/Makefile.in === --- gcc-interface/Makefile.in (revision 205918) +++ gcc-interface/Makefile.in (working copy) @@ -1903,7 +1903,7 @@ ifeq ($(strip $(filter-out powerpc% linu endif # ARM linux, GNU eabi -ifeq ($(strip $(filter-out arm% linux-gnueabi,$(target_cpu) $(target_os))),) +ifeq ($(strip $(filter-out arm% linux-gnueabi%,$(target_cpu) $(target_os))),) LIBGNAT_TARGET_PAIRS = \ a-intnam.adsa-intnam-linux.ads \ s-inmaop.adbs-inmaop-posix.adb \ and rebuild this compiler? -- Eric Botcazou
Re: Two build != host fixes
Darwin doesn't have gettext in libSystem, I build it as a convenience library, but it still needs to refer to a system framework. For this to link the gnattools I need: LDFLAGS=-L/path/to/my/convenience/lib -framework CoreFoundation OK, I'll add $(LDFLAGS). It was actually already passed in the native case... -- Eric Botcazou
Re: [REPOST] Invalid Code when reading from unaligned zero-sized array
OK, so we want the attached patch? FWIW it passed make -k check-c check-c++ RUNTESTFLAGS=compat.exp struct-layout-1.exp on x86/Linux, x86-64/Linux, PowerPC/Linux [*], IA-64/Linux, SPARC/Solaris and SPARC64/Solaris with ALT_CC_UNDER_TEST set to the unpatched compiler. As well as on ARM/EABI w/ and w/o -mfloat-abi=hard. -- Eric Botcazou
Re: [PATCH] Enable Cilk keywords in Cilk Runtime
Iyer, Balaji V balaji.v.i...@intel.com writes: # Compiler and linker flags. GENERAL_FLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/runtime -I$(top_srcdir)/runtime/config/$(config_dir) -DIN_CILK_RUNTIME=1 -GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for +# GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for Just remove the line. # Compiler and linker flags. +# GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for Is this comment necessary? Aldy
RE: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C
-Original Message- From: Aldy Hernandez [mailto:al...@redhat.com] Sent: Thursday, December 12, 2013 10:26 AM To: Iyer, Balaji V Cc: Jakub Jelinek; 'gcc-patches@gcc.gnu.org' Subject: Re: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C On 12/11/13 10:44, Iyer, Balaji V wrote: Just out of curiosity, why can't I keep it as-is? It is giving the correct output/behavior and doesn't seem to interfere with anything else. The only extra thing I am doing is to add an extra if-statement while recursing through all the functions to check for cilk simd function and then calling the function to handle it, which the OMP will have to do anyway. The only extra thing I added was an extra if-statement. If Cilk clones are tagged differently then we need to special case Cilk clones every where we handle OMP clones. If they share an attribute, no special handling is needed. Will it be Ok if I don’t mark them as cilk simd function but just keep it as omp declare simd from the start? That should get around this issue. Thanks, Balaji V. Iyer. Aldy
patch to fix PR59470
The following patch fixes
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59470
Committed as rev. 205930 for trunk and 205929 for gcc-4.8 branch.
Jakub is doing reg testing but I am sure the results will be not worse
as the patch is quite safe IMO.
2013-12-12 Vladimir Makarov vmaka...@redhat.com
PR middle-end/59470
* lra-coalesce.c (lra_coalesce): Invalidate inheritance pseudo
values if necessary.
Index: lra-coalesce.c
===
--- lra-coalesce.c (revision 205902)
+++ lra-coalesce.c (working copy)
@@ -221,9 +221,12 @@ lra_coalesce (void)
basic_block bb;
rtx mv, set, insn, next, *sorted_moves;
int i, mv_num, sregno, dregno;
+ unsigned int regno;
int coalesced_moves;
int max_regno = max_reg_num ();
bitmap_head involved_insns_bitmap;
+ bitmap_head result_pseudo_vals_bitmap;
+ bitmap_iterator bi;
timevar_push (TV_LRA_COALESCE);
@@ -318,6 +321,34 @@ lra_coalesce (void)
}
}
}
+ /* If we have situation after inheritance pass:
+
+ r1 - ... insn originally setting p1
+ i1 - r1 setting inheritance i1 from reload r1
+ ...
+ ... - ... p2 ... dead p2
+ ..
+ p1 - i1
+ r2 - i1
+ ...- ... r2 ...
+
+ And we are coalescing p1 and p2 using p1. In this case i1 and p1
+ should have different values, otherwise they can get the same
+ hard reg and this is wrong for insn using p2 before coalescing.
+ So invalidate such inheritance pseudo values. */
+ bitmap_initialize (result_pseudo_vals_bitmap, reg_obstack);
+ EXECUTE_IF_SET_IN_BITMAP (coalesced_pseudos_bitmap, 0, regno, bi)
+bitmap_set_bit (result_pseudo_vals_bitmap,
+ lra_reg_info[first_coalesced_pseudo[regno]].val);
+ EXECUTE_IF_SET_IN_BITMAP (lra_inheritance_pseudos, 0, regno, bi)
+if (bitmap_bit_p (result_pseudo_vals_bitmap, lra_reg_info[regno].val))
+ {
+ lra_set_regno_unique_value (regno);
+ if (lra_dump_file != NULL)
+ fprintf (lra_dump_file,
+ Make unique value for inheritance r%d\n, regno);
+ }
+ bitmap_clear (result_pseudo_vals_bitmap);
bitmap_clear (used_pseudos_bitmap);
bitmap_clear (involved_insns_bitmap);
bitmap_clear (coalesced_pseudos_bitmap);
Re: [trunk]: Patch to move BITS_PER_UNIT to be available for genmodes.c
On 12/11/13 17:35, Kenneth Zadeck wrote: This patch is for the trunk, but it solves a problem that comes up for wide-int. For wide-int we need to have the BITS_PER_UNIT available earlier.So this patch sets the default value (8) in genmodes.c so that it is available by anyone who includes insn-modes.h. The generator for tm.h was modified to include insn-modes.h.The value for BITS_PER_UNIT can be overridden by any port by placing a define for it in their target modes file. This patch removes the definition of BITS_PER_UNIT from 7 platform .h files. All of those platforms initialized it to the default value so there was no need for additions to their target modes file. In addition, this target also changes the way that MAX_BITSIZE_MODE_ANY_INT is calculated.The value is heavily used on the wide-int branch to allocate buffers that are used to hold large integer values. The change in the way it is computed was motivated by the i386 port, but there may be other ports that have the same problem. The i386 port defines two very large integer modes that are only used as containers for large vectors. They are never used for large integers. The new way of computing this allows a port to say (very early) that some of these integer modes are never used to hold numbers and so smaller buffers can be used for integer calculations. Other ports that play the same game should follow suit. This patch has been bootstrapped and regression tested on x86-64. Ok to commit? OK for MicroBlaze. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: Two build != host fixes
On 12 Dec 2013, at 15:23, Eric Botcazou wrote: Darwin doesn't have gettext in libSystem, I build it as a convenience library, but it still needs to refer to a system framework. For this to link the gnattools I need: LDFLAGS=-L/path/to/my/convenience/lib -framework CoreFoundation OK, I'll add $(LDFLAGS). It was actually already passed in the native case… I ran the gnat testsuite (it didn't quite work 'out of the box' for installed - but of fiddling symlinks was enough) results as per normal (m32 only tested) With your blanket change to gnattools/Makefile, isn't it also reasonable to apply the following? Iain diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in index cd3676f..241571d 100644 --- a/gcc/ada/gcc-interface/Make-lang.in +++ b/gcc/ada/gcc-interface/Make-lang.in @@ -152,12 +152,6 @@ ifeq ($(build), $(host)) # This is a regular cross compiler. Use the native compiler to compile # the tools. -# put the host RTS dir first in the PATH to hide the default runtime -# files that are among the sources -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell gnatls -v | grep adalib ))) -endif - ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \ @@ -193,9 +187,6 @@ else else # This is a canadian cross. We should use a toolchain running on the # build platform and targeting the host platform. -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib ))) -endif ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \
Re: Two build != host fixes
With your blanket change to gnattools/Makefile, isn't it also reasonable to apply the following? diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in index cd3676f..241571d 100644 --- a/gcc/ada/gcc-interface/Make-lang.in +++ b/gcc/ada/gcc-interface/Make-lang.in @@ -152,12 +152,6 @@ ifeq ($(build), $(host)) # This is a regular cross compiler. Use the native compiler to compile # the tools. -# put the host RTS dir first in the PATH to hide the default runtime -# files that are among the sources -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell gnatls -v | grep adalib ))) -endif - ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \ @@ -193,9 +187,6 @@ else else # This is a canadian cross. We should use a toolchain running on the # build platform and targeting the host platform. -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib ))) -endif ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \ That's tempting indeed, but we still support the old --disable-libada and these bits are required to make it work. -- Eric Botcazou
Re: Two build != host fixes
On 12 Dec 2013, at 17:21, Eric Botcazou wrote: With your blanket change to gnattools/Makefile, isn't it also reasonable to apply the following? diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in index cd3676f..241571d 100644 --- a/gcc/ada/gcc-interface/Make-lang.in +++ b/gcc/ada/gcc-interface/Make-lang.in @@ -152,12 +152,6 @@ ifeq ($(build), $(host)) # This is a regular cross compiler. Use the native compiler to compile # the tools. -# put the host RTS dir first in the PATH to hide the default runtime -# files that are among the sources -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell gnatls -v | grep adalib ))) -endif - ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \ @@ -193,9 +187,6 @@ else else # This is a canadian cross. We should use a toolchain running on the # build platform and targeting the host platform. -ifneq ($(findstring ada,$(LANGUAGES)),) - RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib ))) -endif ADA_TOOLS_FLAGS_TO_PASS=\ CC=$(CC) \ CXX=$(CXX) \ That's tempting indeed, but we still support the old --disable-libada and these bits are required to make it work. ah. .. then does the second block need hoisting to bracket the two cases with host!=build? Iain
libgo patch committed: Fix MakeFunc returning float on 32 bit x86
This patch to libgo fixes using reflect.MakeFunc with functions that
return a single floating point value on 32-bit x86. Bootstrapped and
ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and
4.8 branch.
Ian
diff -r 454895d0147d libgo/go/reflect/makefunc_386.S
--- a/libgo/go/reflect/makefunc_386.S Wed Dec 11 16:58:18 2013 -0800
+++ b/libgo/go/reflect/makefunc_386.S Wed Dec 11 23:46:27 2013 -0800
@@ -25,8 +25,9 @@
struct {
esp uint32 // 0x0
eax uint32 // 0x4
- st0 uint64 // 0x8
- sr int32 // 0x10
+ st0 float64 // 0x8
+ sr bool // 0x10
+ sf bool // 0x11
}
The sr field is set by the function to a non-zero value if
the function takes a struct hidden pointer that must be
@@ -84,6 +85,10 @@
/* Set return registers. */
movl -20(%ebp), %eax
+
+ cmpb $0, -7(%ebp)
+ je 2f
+
fldl -16(%ebp)
#ifdef __SSE2__
@@ -92,7 +97,8 @@
movsd -16(%ebp), %xmm0
#endif
- movl -8(%ebp), %edx
+2:
+ movb -8(%ebp), %dl
addl $36, %esp
popl %ebx
@@ -100,7 +106,7 @@
popl %ebp
.LCFI4:
- testl %edx,%edx
+ testb %dl,%dl
jne 1f
ret
1:
diff -r 454895d0147d libgo/go/reflect/makefuncgo_386.go
--- a/libgo/go/reflect/makefuncgo_386.go Wed Dec 11 16:58:18 2013 -0800
+++ b/libgo/go/reflect/makefuncgo_386.go Wed Dec 11 23:46:27 2013 -0800
@@ -14,9 +14,10 @@
// registers that might hold result values.
type i386Regs struct {
esp uint32
- eax uint32 // Value to return in %eax.
- st0 uint64 // Value to return in %st(0).
- sr int32 // Set to non-zero if hidden struct pointer.
+ eax uint32 // Value to return in %eax.
+ st0 float64 // Value to return in %st(0).
+ sr bool// Set to true if hidden struct pointer.
+ sf bool// Set to true if returning float
}
// MakeFuncStubGo implements the 386 calling convention for MakeFunc.
@@ -57,12 +58,13 @@
in := make([]Value, 0, len(ftyp.in))
ap := uintptr(regs.esp)
- regs.sr = 0
+ regs.sr = false
+ regs.sf = false
var retPtr unsafe.Pointer
if retStruct {
retPtr = *(*unsafe.Pointer)(unsafe.Pointer(ap))
ap += ptrSize
- regs.sr = 1
+ regs.sr = true
}
for _, rt := range ftyp.in {
@@ -126,13 +128,16 @@
v := out[0]
w := v.iword()
- if v.Kind() != Ptr v.Kind() != UnsafePointer {
- w = loadIword(unsafe.Pointer(w), v.typ.size)
- }
switch v.Kind() {
- case Float32, Float64:
- regs.st0 = uint64(uintptr(w))
+ case Ptr, UnsafePointer:
+ regs.eax = uint32(uintptr(w))
+ case Float32:
+ regs.st0 = float64(*(*float32)(unsafe.Pointer(w)))
+ regs.sf = true
+ case Float64:
+ regs.st0 = *(*float64)(unsafe.Pointer(w))
+ regs.sf = true
default:
- regs.eax = uint32(uintptr(w))
+ regs.eax = uint32(uintptr(loadIword(unsafe.Pointer(w), v.typ.size)))
}
}
Re: [gofrontend-dev] Go patch committed: Implement method values in reflect package
On Thu, Dec 12, 2013 at 12:21 AM, Michael Hudson-Doyle michael.hud...@linaro.org wrote: Ian Lance Taylor i...@google.com writes: This patch to the Go frontend and libgo implements method values in the reflect package. Working with method values and reflect now works correctly, at least on x86. Can you give me a test case? I can try it on a few other architectures tomorrow. The test case is test/recover.go in the master Go sources. But I know that that test won't work on other architectures, because currently reflect.MakeFunc is only implemented for 386 and amd64. It should be possible to implement reflect.MakeFunc for all architectures in a somewhat inefficient manner by using libffi's closure API. I have not tried that. It is also of course possible to implement support for any specific processor as I did for 386 and amd64. The difficulty depends on the difficulty of the ABI. In general it's not too hard but it requires a clear understanding of ABI details and assembly language programming with full backtrace information. It's about 600 lines of code for amd64. Ian
Go patch committed: Don't compare structs with blank non-comp fields
The Go language spec was clarified to say that a struct with a blank
field of non-comparable type may not be compared. This patch implements
that restriction in the Go frontend, by removing the code that permitted
it. This change requires a couple of test cases to be updated; I've
simply copied in the current versions of those test cases from the
master testsuite. Bootstrapped and ran Go tests on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc (revision 205916)
+++ gcc/go/gofrontend/types.cc (working copy)
@@ -575,9 +575,6 @@ Type::are_compatible_for_comparison(bool
p != fields-end();
++p)
{
- if (Gogo::is_sink_name(p-field_name()))
- continue;
-
if (!p-type()-is_comparable())
{
if (reason != NULL)
Index: gcc/testsuite/go.test/test/cmp.go
===
--- gcc/testsuite/go.test/test/cmp.go (revision 205904)
+++ gcc/testsuite/go.test/test/cmp.go (working copy)
@@ -43,8 +43,8 @@ func main() {
var d string = hel // try to get different pointer
d = d + lo
- // exp/ssa/interp can't handle unsafe.Pointer.
- if os.Getenv(GOSSAINTERP) != {
+ // go.tools/ssa/interp can't handle unsafe.Pointer.
+ if os.Getenv(GOSSAINTERP) == {
if stringptr(c) == stringptr(d) {
panic(compiler too smart -- got same string)
}
@@ -296,7 +296,7 @@ func main() {
{
var x = struct {
x int
- _ []int
+ _ string
y float64
_ float64
z int
Index: gcc/testsuite/go.test/test/cmp6.go
===
--- gcc/testsuite/go.test/test/cmp6.go (revision 205904)
+++ gcc/testsuite/go.test/test/cmp6.go (working copy)
@@ -53,7 +53,7 @@ func main() {
// Comparison of structs should have a good message
use(t3 == t3) // ERROR struct|expected
- use(t4 == t4) // ok; the []int is a blank field
+ use(t4 == t4) // ERROR cannot be compared|non-comparable
// Slices, functions, and maps too.
var x []int
RE: [PATCH] Enable Cilk keywords in Cilk Runtime
-Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Aldy Hernandez Sent: Thursday, December 12, 2013 10:47 AM To: Iyer, Balaji V Cc: gcc-patches@gcc.gnu.org; Jeff Law Subject: Re: [PATCH] Enable Cilk keywords in Cilk Runtime Iyer, Balaji V balaji.v.i...@intel.com writes: # Compiler and linker flags. GENERAL_FLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/runtime -I$(top_srcdir)/runtime/config/$(config_dir) -DIN_CILK_RUNTIME=1 -GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for +# GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for Just remove the line. # Compiler and linker flags. +# GENERAL_FLAGS += -D_Cilk_spawn= -D_Cilk_sync= -D_Cilk_for=for Is this comment necessary? No, I can remove it. But I put it in there because it is a good debugging method if you don't want to use _Cilk keywords to build runtime Aldy
Re: patch for elimination to SP when it is changed in RTL (PR57293)
On 12/11/2013, 1:59 PM, Yvan Roux wrote:
On 11 December 2013 19:25, Vladimir Makarov vmaka...@redhat.com wrote:
On 12/11/2013, 5:35 AM, Yvan Roux wrote:
Hi Vladimir,
I've some regressions on ARM after this SP elimination patch, and they
are execution failures. Here is the list:
g++.dg/cilk-plus/AN/array_test_ND_tplt.cc -O3 -fcilkplus
gcc.c-torture/execute/va-arg-22.c -O2
gcc.dg/atomic/c11-atomic-exec-5.c -O0
gfortran.dg/direct_io_12.f90 -O[23]
gfortran.dg/elemental_dependency_1.f90 -O2
gfortran.dg/matmul_2.f90 -O2
gfortran.dg/matmul_6.f90 -O2
gfortran.dg/mvbits_7.f90 -O3
gfortran.dg/unlimited_polymorphic_1.f03 -O3
I reduced and looked at var-arg-22.c and the issue is that in
lra_eliminate_regs_1 (called by get_equiv_with_elimination) we
transformed sfp + 0x4c in sp + 0xfc because of a bad sp offset. What
we try to do here is to change the pseudo 195 of the insn 118 below :
(insn 118 114 112 8 (set (reg:DI 195)
(unspec:DI [
(mem:DI (plus:SI (reg/f:SI 215)
(const_int 8 [0x8])) [7 MEM[(struct A35 *)_12
+ 64B]+8 S8 A8])
] UNSPEC_UNALIGNED_LOAD)) v2.c:49 146 {unaligned_loaddi}
(expr_list:REG_EQUIV (mem/c:DI (plus:SI (reg/f:SI 192)
(const_int 8 [0x8])) [7 a35+8 S8 A32])
(nil)))
with its equivalent (x arg of lra_eliminate_regs_1):
(mem/c:DI (plus:SI (reg/f:SI 102 sfp)
(const_int 76 [0x4c])) [7 a35+8 S8 A32])
lra_eliminate_regs_1 is called with full_p = true (it is not really
clear for what it means),
It means we use full offset between the regs, otherwise we use change in the
full offset from the previous iteration (it can be changed as we reserve
stack memory for spilled pseudos and the reservation can be done several
times). As equiv value is stored as it was before any elimination, we need
always to use full offset to make elimination.
Ok thanks it's clearer.
but in the PLUS switch case, we have offset
= 0xb (given by ep-offset) and as lra_get_insn_recog_data
(insn)-sp_offset value is 0, we will indeed add 0xb to the original
0x4c offset.
0 value is suspicious because it is default. We might have not set up it
from neighbor insns.
So, here I don't get if it is the sp_offset value of the
lra_insn_recog_data element which is not well updated or if lra_
eliminate_regs_1 has to be called with update_p and not full_p (which
fixed the value in that particular case). Is it more obvious for you
?
Yvan, could you send me the reduced preprocessed case and the options for
cc1 to reproduce it.
Here is cc1 command line :
cc1 -quiet -march=armv7-a -mtune=cortex-a15 -mfloat-abi=hard
-mfpu=neon -mthumb v2.c -O2
I use a native build on a chromebook, but it's reproducible with a
cross compiler.
With the attached test case the issue is when processing insn 118.
The offset is updated two times and that is wrong. That is because
memory in init insn is shared by ira_reg_equiv and the test involves 2
equivalent substitutions. As I wrote equiv should be stored in original
form by the current patch design. Simple copying will not work as the
first substitution is not done in this case.
I need some time to think how to fix it better still I'll try to fix it
tomorrow. I expected that the patch might have some problems. The
patch code is quite big although it is just a long standing PR fix.
Therefore that was my first PR fixed on stage 3. It is good to have it
tested earlier and sorry to break some arm tests.
Go patch committed: Don't permit importing a package as init
In Go the top-level name init is special, as it names a function that
is run when the program starts. The language therefore does not permit
other uses of init at top level. This change to the Go frontend
detects and bans one of those cases: importing a package under the local
name init. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 8a7a4f1b970f go/gogo.cc
--- a/go/gogo.cc Thu Dec 12 10:36:16 2013 -0800
+++ b/go/gogo.cc Thu Dec 12 11:23:20 2013 -0800
@@ -440,6 +440,9 @@
return;
}
+ if (local_name == init)
+error_at(location, cannot import package as init);
+
if (filename == unsafe)
{
this-import_unsafe(local_name, is_local_name_exported, location);
Re: Two build != host fixes
.. then does the second block need hoisting to bracket the two cases with host!=build? This code works fine so I don't think that we really need to do anything. -- Eric Botcazou
Re: RFA (cgraph): C++ 'structor decloning patch, Mark III
Hi,
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index f368cab..3576f7d 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -899,6 +899,10 @@ c_common_post_options (const char **pfilename)
if (warn_implicit_function_declaration == -1)
warn_implicit_function_declaration = flag_isoc99;
+ /* Declone C++ 'structors if -Os. */
+ if (flag_declone_ctor_dtor == -1)
+flag_declone_ctor_dtor = optimize_size;
So only reason why this is optimize_size only is the fact that we can't rely on
inliner
to fix up the wrappers?
Perhaps we can declone at least the non-comdat ones since there should be no
negative
effects?
In longer term, it would be nice to disable frontend driven clonning and just
produce the wrappers/virtual clones to be handled by middle-end.
It is wasteful to do so early and it needs tree-inline to handle unlowered
gimple/generic.
diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index 9e9087f..21e52a1 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/gcc/ipa-inline-analysis.c
@@ -2796,6 +2796,11 @@ compute_inline_parameters (struct cgraph_node *node,
bool early)
}
estimate_function_body_sizes (node, early);
+ for (e = node-callees; e; e = e-next_callee)
+if (symtab_comdat_local_p (e-callee))
+ break;
+ node-calls_comdat_local = (e != NULL);
+
You need to also add LTO streaming bits (or perhaps just arrange the
flag to be detected at stream-in time)
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index 7fb4ab9..70fc73d 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -272,6 +272,18 @@ inline_call (struct cgraph_edge *e, bool update_original,
inline_update_overall_summary (to);
new_size = inline_summary (to)-size;
+ if (callee-calls_comdat_local)
+to-calls_comdat_local = true;
+ else if (to-calls_comdat_local symtab_comdat_local_p (callee))
+{
+ struct cgraph_edge *se = to-callees;
+ for (; se; se = se-next_callee)
+ if (symtab_comdat_local_p (se-callee))
At this level inline decisions are represented as calls with -inline_failed
flag false. You want to test se-inline_failed symtab_comdat_local_p
(se-callee)
+ else if (callee-calls_comdat_local
+ !symtab_in_same_comdat_p (e-caller, callee))
+{
+ e-inline_failed = CIF_FUNCTION_NOT_INLINABLE;
This is bit of a lie - the function is inlinable under some conditoins.
Lets add CIF_FUNCTION_CALLS_COMDAT_LOCAL for this so it will be easier
in future to diagnose possible problems once comdat locals are more
commonly used.
@@ -969,14 +972,14 @@ function_and_variable_visibility (bool whole_program)
node-unique_name = ((node-resolution == LDPR_PREVAILING_DEF_IRONLY
|| node-resolution ==
LDPR_PREVAILING_DEF_IRONLY_EXP)
TREE_PUBLIC (node-decl));
- symtab_make_decl_local (node-decl);
node-resolution = LDPR_PREVAILING_DEF_IRONLY;
- if (node-same_comdat_group)
+ if (node-same_comdat_group TREE_PUBLIC (node-decl))
/* cgraph_externally_visible_p has already checked all other nodes
in the group and they will all be made local. We need to
dissolve the group at once so that the predicate does not
segfault though. */
symtab_dissolve_same_comdat_group_list (node);
+ symtab_make_decl_local (node-decl);
OK, so you change symtab_make_decl_local to not clear COMDAT_GROUP_FLAG, so I
do not
see where the flag is cleared when the COMDAT symbol is promoted to static (in
LTO).
Perhaps symtab_dissolve_same_comdat_group_list should also care clearing the
COMDAT_GROUP
flags?
Patch looks OK with those changes,
thanks!
Honza
Re: [RFC] libgcov.c re-factoring and offline profile-tool
On Wed, Dec 11, 2013 at 10:05 PM, Teresa Johnson tejohn...@google.com wrote:
On Fri, Dec 6, 2013 at 6:23 AM, Jan Hubicka hubi...@ucw.cz wrote:
Hi, all
This is the new patch for gcov-tool (previously profile-tool).
Honza: can you comment on the new merge interface? David posted some
comments in an earlier email and we want to know what's your opinion.
Test patch has been tested with boostrap, regresssion,
profiledbootstrap and SPEC2006.
Noticeable changes from the earlier version:
1. create a new file libgcov.h and move libgcov-*.h headers to libgcov.h
So we can included multiple libgcov-*.c without adding new macros.
2. split libgcov.h specific code in gcvo-io.h to libcc/libgcov.h
Avoid multiple-page of code under IN_LIBGCOV macro -- this
improves the readability.
3. make gcov_var static, and move the definition from gcov-io.h to
gcov-io.c. Also
move some static functions accessing gcov_var to gcvo-io.c
Current code rely on GCOV_LINKAGE tricks to avoid multi-definition. I don't
see
a reason that gcov_var needs to exposed as a global.
4. expose gcov_write_strings() and gcov_sync() to gcov_tool usage
5. rename profile-tool to gcov-tool per Honza's suggestion.
Thanks,
Hi,
I did not read in deatil the gcov-tool source itself, but lets first make
the interface changes
needed.
2013-11-18 Rong Xu x...@google.com
* gcc/gcov-io.c (gcov_var): Moved from gcov-io.h and make it static.
(gcov_position): Move from gcov-io.h
(gcov_is_error): Ditto.
(gcov_rewrite): Ditto.
* gcc/gcov-io.h: Re-factoring. Move gcov_var to gcov-io.h and
move the libgcov only part of libgcc/libgcov.h.
* libgcc/libgcov.h: New common header files for libgcov-*.h
* libgcc/Makefile.in: Add dependence to libgcov.h
* libgcc/libgcov-profiler.c: Use libgcov.h
* libgcc/libgcov-driver.c: Ditto.
* libgcc/libgcov-interface.c: Ditto.
* libgcc/libgcov-driver-system.c (allocate_filename_struct): use
xmalloc instread of malloc.
* libgcc/libgcov-merge.c (void __gcov_merge_delta): Add more
parameters to merge function.
(__gcov_merge_add): Ditto.
(__gcov_merge_ior): Ditto.
(__gcov_merge_time_profile): Ditto.
(__gcov_merge_single): Ditto.
(__gcov_merge_delta): Ditto.
* libgcc/libgcov-tool.c (void gcov_tool_set_verbose): New for
gcov-tool support.
(set_fn_ctrs): Ditto.
(tag_function): Ditto.
(tag_blocks): Ditto.
(tag_arcs): Ditto.
(tag_lines): Ditto.
(tag_counters): Ditto.
(tag_summary): Ditto.
(read_gcda_finalize): Ditto.
(read_gcda_file): Ditto.
(ftw_read_file): Ditto.
(read_profile_dir_init) Ditto.:
(gcov_read_profile_dir): Ditto.
(gcov_merge): Ditto.
(find_match_gcov_inf Ditto.o):
(gcov_profile_merge): Ditto.
(__gcov_scale_add): Ditto.
(__gcov_scale_ior): Ditto.
(__gcov_scale_delta): Ditto.
(__gcov_scale_single): Ditto.
(gcov_profile_scale): Ditto.
(gcov_profile_normalize): Ditto.
(__gcov_scale2_add): Ditto.
(__gcov_scale2_ior): Ditto.
(__gcov_scale2_delta): Ditto.
(__gcov_scale2_single): Ditto.
(gcov_profile_scale2): Ditto.
* gcc/gcov-tool.c (unlink_file): Gcov-tool driver support.
(unlink_dir): Ditto.
(profile_merge): Ditto.
(print_merge_usage_message): Ditto.
(merge_usage): Ditto.
(do_merge): Ditto.
(profile_rewrite2): Ditto.
(profile_rewrite): Ditto.
(print_rewrite_usage_message): Ditto.
(rewrite_usage): Ditto.
(do_rewrite): Ditto.
(print_usage): Ditto.
(print_version): Ditto.
(process_args): Ditto.
(main): Ditto.
* gcc/Makefile.in: Build and install gcov-tool.
Index: gcc/gcov-io.c
===
--- gcc/gcov-io.c (revision 204895)
+++ gcc/gcov-io.c (working copy)
@@ -36,6 +36,37 @@ static const gcov_unsigned_t *gcov_read_words (uns
static void gcov_allocate (unsigned);
#endif
+/* Moved for gcov-io.h and make it static. */
+static struct gcov_var gcov_var;
This is more an changelog message than a comment in source file.
Just describe what gcov_var is.
I changed this so gcov_var is no longer static, but global as before.
Do you know how the size of libgcov changed with your patch?
Quick check of current mainline on compiling empty main gives:
jh@gcc10:~/trunk/build/gcc$ cat t.c
main()
{
}
jh@gcc10:~/trunk/build/gcc$ ./xgcc -B ./ -O2 -fprofile-generate -o a.out-new
--static t.c
jh@gcc10:~/trunk/build/gcc$ gcc -O2 -fprofile-generate -o a.out-old --static
t.c
jh@gcc10:~/trunk/build/gcc$ size a.out-old
textdata bss dec hex filename
6081413560 16728 628429 996cd a.out-old
jh@gcc10:~/trunk/build/gcc$ size a.out-new
textdata bss dec
Re: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C
On 12/12/13 07:56, Iyer, Balaji V wrote: Will it be Ok if I don’t mark them as cilk simd function but just keep it as omp declare simd from the start? That should get around this issue. No, because then we won't be able to distinguish between OMP and Cilk Plus clones. This is something we do here: /* To distinguish from an OpenMP simd clone, Cilk Plus functions to be cloned have a distinctive artificial label in addition to omp declare simd. */ bool cilk_clone = (flag_enable_cilkplus lookup_attribute (cilk plus elemental, DECL_ATTRIBUTES (node-decl))); /* Allocate one more than needed just in case this is an in-branch clone which will require a mask argument. */ struct cgraph_simd_clone *clone_info = simd_clone_struct_alloc (n + 1); clone_info-nargs = n; clone_info-cilk_elemental = cilk_clone;
libgo patch committed: Fix defer of unlock thread at startup
This patch to libgo fixes the handling of the deferring of the unlock
thread at program startup. The code was incorrectly freeing a stack
object. This patch also cleans up some cases of freeing a defer block
to avoid doing it when there is no Go context available. Bootstrapped
and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to
mainline.
Ian
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/go-defer.c
--- a/libgo/runtime/go-defer.c Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/go-defer.c Thu Dec 12 12:13:34 2013 -0800
@@ -28,6 +28,7 @@
n-__arg = arg;
n-__retaddr = NULL;
n-__makefunc_can_recover = 0;
+ n-__free = 1;
g-defer = n;
}
@@ -59,7 +60,7 @@
have a memory context. Don't try to free anything in that
case--the GC will release it later. */
m = runtime_m ();
- if (m != NULL m-mcache != NULL)
+ if (m != NULL m-mcache != NULL d-__free)
__go_free (d);
/* Since we are executing a defer function here, we know we are
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/go-defer.h
--- a/libgo/runtime/go-defer.h Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/go-defer.h Thu Dec 12 12:13:34 2013 -0800
@@ -40,4 +40,8 @@
function will be somewhere in libffi, so __retaddr is not
useful. */
_Bool __makefunc_can_recover;
+
+ /* Set to true if this defer stack entry should be freed when
+ done. */
+ _Bool __free;
};
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/go-panic.c
--- a/libgo/runtime/go-panic.c Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/go-panic.c Thu Dec 12 12:13:34 2013 -0800
@@ -102,7 +102,7 @@
have a memory context. Don't try to free anything in that
case--the GC will release it later. */
m = runtime_m ();
- if (m != NULL m-mcache != NULL)
+ if (m != NULL m-mcache != NULL d-__free)
__go_free (d);
}
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/go-unwind.c
--- a/libgo/runtime/go-unwind.c Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/go-unwind.c Thu Dec 12 12:13:34 2013 -0800
@@ -80,6 +80,7 @@
{
struct __go_defer_stack *d;
void (*pfn) (void *);
+ M *m;
d = g-defer;
if (d == NULL || d-__frame != frame || d-__pfn == NULL)
@@ -90,7 +91,9 @@
(*pfn) (d-__arg);
- __go_free (d);
+ m = runtime_m ();
+ if (m != NULL m-mcache != NULL d-__free)
+ __go_free (d);
if (n-__was_recovered)
{
@@ -119,13 +122,17 @@
g-defer-__frame == frame)
{
struct __go_defer_stack *d;
+ M *m;
/* This is the defer function which called recover. Simply
return to stop the stack unwind, and let the Go code continue
to execute. */
d = g-defer;
g-defer = d-__next;
- __go_free (d);
+
+ m = runtime_m ();
+ if (m != NULL m-mcache != NULL d-__free)
+ __go_free (d);
/* We are returning from this function. */
*frame = 1;
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/panic.c
--- a/libgo/runtime/panic.c Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/panic.c Thu Dec 12 12:13:34 2013 -0800
@@ -28,7 +28,8 @@
d-__pfn = nil;
if (pfn != nil)
(*pfn)(d-__arg);
- runtime_free(d);
+ if (d-__free)
+ runtime_free(d);
}
}
diff -r f3ec8ff457ec -r 9931ebbea550 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c Thu Dec 12 11:23:56 2013 -0800
+++ b/libgo/runtime/proc.c Thu Dec 12 12:13:34 2013 -0800
@@ -541,6 +541,7 @@
d.__retaddr = nil;
d.__makefunc_can_recover = 0;
d.__frame = frame;
+ d.__free = 0;
g-defer = d;
if(m != runtime_m0)
Go patch committed: Tweak untyped nil error messages
This patch to the Go frontend tweaks the existing error messages about
use of untyped nil to do a better job when nil appears as the first
argument to the builtin append function. This avoids an error about nil
not being a slice type--it's more useful to say that the problem is an
untyped nil. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 9931ebbea550 go/expressions.cc
--- a/go/expressions.cc Thu Dec 12 12:13:34 2013 -0800
+++ b/go/expressions.cc Thu Dec 12 12:40:57 2013 -0800
@@ -7310,7 +7310,11 @@
Type* slice_type = args-front()-type();
if (!slice_type-is_slice_type())
{
- error_at(args-front()-location(), argument 1 must be a slice);
+ if (slice_type-is_nil_type())
+ error_at(args-front()-location(), use of untyped nil);
+ else
+ error_at(args-front()-location(),
+ argument 1 must be a slice);
this-set_is_error();
return this;
}
@@ -8008,7 +8012,10 @@
const Expression_list* args = this-args();
if (args == NULL || args-empty())
return Type::make_error_type();
- return args-front()-type();
+ Type *ret = args-front()-type();
+ if (!ret-is_slice_type())
+ return Type::make_error_type();
+ return ret;
}
case BUILTIN_REAL:
Re: [PATCH i386] Enable -freorder-blocks-and-partition
On Wed, Dec 11, 2013 at 1:21 AM, Martin Liška marxin.li...@gmail.com wrote: Hello, I prepared a collection of systemtap graphs for GIMP. 1) just my profile-based function reordering: 550 pages 2) just -freorder-blocks-and-partitions: 646 pages 3) just -fno-reorder-blocks-and-partitions: 638 pages Please see attached data. Thanks for the data. A few observations/questions: With both 1) (your (time-based?) reordering) and 2) (-freorder-blocks-and-partitions) there are a fair amount of accesses out of the cold section. I'm not seeing so many accesses out of the Good point, I misread the description and assumed that 1) is time profiling + reorder-blcoks-and-partition. Martin, what version of GCC you used? Rong introduced bug into libgcov that made gcov streaming around fork to split summaries (so the number of runs did not match). I fixed it by 2013-11-18 Jan Hubicka j...@suse.cz * libgcov-driver.c (run_accounted): Make global level static. (gcov_exit_merge_summary): Silence warning; do not clear run_accounted here. (gcov_exit): Clear it here. * libgcov-driver.c (gcov_exit_merge_summary): Fix setting run_accounted. * libgcov-driver.c (get_gcov_dump_complete): Update comments. (all_prg, crc32): Remove static vars. (gcov_exit_compute_summary): Rewrite to return crc32; do not clear all_prg. (gcov_exit_merge_gcda): Add crc32 parameter. (gcov_exit_merge_summary): Add crc32 and all_prg parameter; do not account run if it was already accounted. (gcov_exit_dump_gcov): Add crc32 and all_prg parameters. (gcov_exit): Initialize all_prg; update. so please be sure you have this one in tree. If you do, can you please repeat the trick with locked unlikely section so we see why we get there even with -fno-reorder-blcoks-and-partition? cold section in the apps I am looking at with splitting enabled. In the case of splitting, it could either be non-representative profile data or profile data that isn't being maintained properly and lost, although I think I fixed most of those. If you have identified any of the cold split routines that are being executed in the case of 2) it would be interesting to look at the dumps. With 2) there is also a big clump towards the end which is being executed out of the cold section, which again would be interesting to investigate. I think the data towards the end comes from fact that Martin is manually quiting gimp and at that time he may do it differently (and after different delay) each time. This makes the graphs harder to read, but one should basically ignore everything after the huge gap that indicate that the app has started. Why is your function reordering in 1) accessing more out of the cold section than 3) (-fno-reorder-blocks-and-partitions)? This seems like a scale differnce here (i.e. Martin took longer to quit gimp in gimp_reorder_blocks_and_partition). In the first gap you can see several red dots aligned vertically. I did not go into manually counting the dots, but it seems that they should be about the same. Both 2) and 3) have both normal .text and .text.hot, whereas 1) only has .text. I wonder if that is contributing to the higher number of pages either of these has compared to 1), since the non-cold addresses are distributed across both sections? Looking at Martin's tree https://github.com/marxin/gcc/blob/time-profiler-patch2/gcc/varasm.c#L536 he has a hack in disables startup/hot sections, but keeps unlikely. I agree that for more comparable results we should keep all. But first lets work out why we have unlikely section accesses in again... Martin, is there any chance you can test these things on mainline rather than patched trees? Honza
Go patch committed: Check for nil pointer when slicing pointer to array
When Go code takes a slice of a pointer to an array, the compiler was
failing to check whether that point is nil, as is required by the Go 1.2
addition of reliable nil checks. This patch fixes that oversight.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.
Ian
diff -r befe27e79459 go/expressions.cc
--- a/go/expressions.cc Thu Dec 12 12:41:44 2013 -0800
+++ b/go/expressions.cc Thu Dec 12 13:00:22 2013 -0800
@@ -10259,6 +10259,14 @@
{
Expression* deref = Expression::make_unary(OPERATOR_MULT, left,
location);
+
+ // For an ordinary index into the array, the pointer will be
+ // dereferenced. For a slice it will not--the resulting slice
+ // will simply reuse the pointer, which is incorrect if that
+ // pointer is nil.
+ if (end != NULL || cap != NULL)
+ deref-issue_nil_check();
+
return Expression::make_array_index(deref, start, end, cap, location);
}
else if (type-is_string_type())
[PATCH] Obvious bugfix to x86 machine description
I stumbled over this while trying to fix one of the regressions for 4.9. This peep2 pattern in the x86 backend is obviously broken as it does not have a mode on the zero_extend in the resulting insn. As a result, if/when this peep2 matches we get an unrecognized insn. (define_peephole2 [(set (match_operand:DI 0 register_operand) (zero_extend:DI (mult:SI (match_operand:SI 1 register_operand) (match_operand:SI 2 const_int_operand] TARGET_64BIT exact_log2 (INTVAL (operands[2])) = 0 REGNO (operands[0]) == REGNO (operands[1]) peep2_regno_dead_p (0, FLAGS_REG) [(parallel [(set (match_dup 0) (zero_extend (ashift:SI (match_dup 1) (match_dup 2 (clobber (reg:CC FLAGS_REG))])] operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));) It's obvious the zero_extend should have been DImode. I searched i386.md for other occurrences but didn't find any (remaining modeless extensions were on the peep2 and define_split input side patterns). Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed as obvious. No testcase as triggering requires local hackery. * i386.md (simple LEA peephole2): Add missing mode to zero_extend for zero-extended MULT simple LEA pattern. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 6ac2802..ab5b33f 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -17464,7 +17464,7 @@ REGNO (operands[0]) == REGNO (operands[1]) peep2_regno_dead_p (0, FLAGS_REG) [(parallel [(set (match_dup 0) - (zero_extend (ashift:SI (match_dup 1) (match_dup 2 + (zero_extend:DI (ashift:SI (match_dup 1) (match_dup 2 (clobber (reg:CC FLAGS_REG))])] operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));)
[PATCH] Don't reject TER unnecessarily (PRs middle-end/58956, middle-end/59470)
Hi!
Before the PR58956 fix find_replaceable_in_bb only gave up TER if
stmt was gimple_assign_single_p that was storing something that
could alias with TERed load, but now it also punts on calls (a lot of them
apparently) and inline asm (all of them, because stmt_may_clobber_ref_p
always returns true for GIMPLE_ASM). I believe we can restore TERing
most of the cases for calls and inline asm though, because for calls
all arguments are necessarily evaluated before the call and the only
memory side effects of the call are the call itself and perhaps storing
of non-SSA_NAME return value afterwards. And for inline asm again
all input operands have to be evaluated before the inline asm, and the
only storing side-effects are again inside of the inline asm and perhaps
storing of the output operands.
Thus, this patch will only stop TERing if USE is used somewhere in
lhs of a call for calls or somewhere in output arguments of inline asm.
Bootstrapped/regtested on x86_64-linux and i686-linux (both on trunk and 4.8
branch), ok for trunk/4.8?
The reason I'd like to see this for 4.8 is to decrease the amount of code
generation changes from pre-r205709 to minimum, as it can uncover other
latent bugs than just PR59470.
2013-12-12 Jakub Jelinek ja...@redhat.com
PR middle-end/58956
PR middle-end/59470
* tree-ssa-ter.c (find_ssa_name): New helper function.
(find_replaceable_in_bb): For calls, only set same_root_var
if USE is used somewhere in gimple_call_lhs, for GIMPLE_ASM,
only set same_root_var if USE is used somewhere in output operand
trees.
* gcc.target/i386/pr59470.c: New test.
--- gcc/tree-ssa-ter.c.jj 2013-12-10 08:52:13.0 +0100
+++ gcc/tree-ssa-ter.c 2013-12-12 10:43:26.177866960 +0100
@@ -554,6 +554,20 @@ mark_replaceable (temp_expr_table_p tab,
}
+/* Helper function for find_replaceable_in_bb. Called via walk_tree to
+ find a SSA_NAME DATA somewhere in *TP. */
+
+static tree
+find_ssa_name (tree *tp, int *walk_subtrees, void *data)
+{
+ tree var = (tree) data;
+ if (*tp == var)
+return var;
+ else if (IS_TYPE_OR_DECL_P (*tp))
+*walk_subtrees = 0;
+ return NULL_TREE;
+}
+
/* This function processes basic block BB, and looks for variables which can
be replaced by their expressions. Results are stored in the table TAB. */
@@ -618,7 +632,42 @@ find_replaceable_in_bb (temp_expr_table_
gimple_assign_single_p (def_stmt)
stmt_may_clobber_ref_p (stmt,
gimple_assign_rhs1 (def_stmt)))
- same_root_var = true;
+ {
+ if (is_gimple_call (stmt))
+ {
+ /* For calls, it is not a problem if USE is among
+call's arguments or say OBJ_TYPE_REF argument,
+all those necessarily need to be evaluated before
+the call that may clobber the memory. But if
+LHS of the call refers to USE, expansion might
+evaluate it after the call, prevent TER in that
+case. */
+ if (gimple_call_lhs (stmt)
+ TREE_CODE (gimple_call_lhs (stmt)) != SSA_NAME
+ walk_tree (gimple_call_lhs_ptr (stmt),
+ find_ssa_name, use, NULL))
+ same_root_var = true;
+ }
+ else if (gimple_code (stmt) == GIMPLE_ASM)
+ {
+ /* For inline asm, allow TER of loads into input
+arguments, but disallow TER for USEs that occur
+somewhere in outputs. */
+ unsigned int i;
+ for (i = 0; i gimple_asm_noutputs (stmt); i++)
+ if (TREE_CODE (gimple_asm_output_op (stmt, i))
+ != SSA_NAME
+walk_tree (gimple_asm_output_op_ptr (stmt,
+ i),
+ find_ssa_name, use, NULL))
+ {
+ same_root_var = true;
+ break;
+ }
+ }
+ else
+ same_root_var = true;
+ }
}
/* Mark expression as replaceable unless stmt is volatile, or the
--- gcc/testsuite/gcc.target/i386/pr59470.c.jj 2013-12-12 10:31:54.746517544
+0100
+++ gcc/testsuite/gcc.target/i386/pr59470.c 2013-12-12 10:32:42.045273313
+0100
@@ -0,0 +1,17 @@
+/* PR middle-end/58956 */
+/* PR middle-end/59470 */
+/* { dg-do
Go patch committed: Better error messages for { on next line
The Go language requires that the { starting a block for an if, for, or
switch statement be on the same line as the if/for/switch. This patch
gives better error messages when a program does it wrong. Bootstrapped
and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to
mainline.
Ian
diff -r ee6b0181e864 go/parse.cc
--- a/go/parse.cc Thu Dec 12 13:02:33 2013 -0800
+++ b/go/parse.cc Thu Dec 12 13:32:41 2013 -0800
@@ -4287,6 +4287,16 @@
cond = this-expression(PRECEDENCE_NORMAL, false, false, NULL, NULL);
}
+ // Check for the easy error of a newline before starting the block.
+ if (this-peek_token()-is_op(OPERATOR_SEMICOLON))
+{
+ Location semi_loc = this-location();
+ if (this-advance_token()-is_op(OPERATOR_LCURLY))
+ error_at(semi_loc, missing %{% after if clause);
+ // Otherwise we will get an error when we call this-block
+ // below.
+}
+
this-gogo_-start_block(this-location());
Location end_loc = this-block();
Block* then_block = this-gogo_-finish_block(end_loc);
@@ -4431,7 +4441,7 @@
Location token_loc = this-location();
if (this-peek_token()-is_op(OPERATOR_SEMICOLON)
this-advance_token()-is_op(OPERATOR_LCURLY))
- error_at(token_loc, unexpected semicolon or newline before %{%);
+ error_at(token_loc, missing %{% after switch clause);
else if (this-peek_token()-is_op(OPERATOR_COLONEQ))
{
error_at(token_loc, invalid variable name);
@@ -5158,6 +5168,16 @@
}
}
+ // Check for the easy error of a newline before starting the block.
+ if (this-peek_token()-is_op(OPERATOR_SEMICOLON))
+{
+ Location semi_loc = this-location();
+ if (this-advance_token()-is_op(OPERATOR_LCURLY))
+ error_at(semi_loc, missing %{% after for clause);
+ // Otherwise we will get an error when we call this-block
+ // below.
+}
+
// Build the For_statement and note that it is the current target
// for break and continue statements.
@@ -5224,8 +5244,7 @@
*cond = NULL;
else if (this-peek_token()-is_op(OPERATOR_LCURLY))
{
- error_at(this-location(),
- unexpected semicolon or newline before %{%);
+ error_at(this-location(), missing %{% after for clause);
*cond = NULL;
*post = NULL;
return;
[PR tree-optimization/59149] fail on invalid arguments to flags_from_decl_or_type
flags_from_decl_or_type() only handles a TYPE or DECL. Make this
explicit instead.
I also added a check in the use in trans-mem.c, just in case. The
subsequent conditionals should take care of the TM case.
It would be nice if Marc Glisse could provide the testcase he mentioned
was failing. Either way, the attached patch doesn't hurt.
Tested on x86-64 Linux.
OK?
commit b5830c04f011d6885e3a09f50602b8f5f495a408
Author: Aldy Hernandez al...@redhat.com
Date: Mon Dec 9 08:10:44 2013 -0800
PR tree-optimization/59149
* calls.c (flags_from_decl_or_type): Fail on non decl or type.
* trans-mem.c (diagnose_tm_1): Do not call flags_from_decl_or_type
if no type or decl.
diff --git a/gcc/calls.c b/gcc/calls.c
index 3963bc2..2226e78 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -769,6 +769,8 @@ flags_from_decl_or_type (const_tree exp)
|| lookup_attribute (transaction_pure, TYPE_ATTRIBUTES (exp
flags |= ECF_TM_PURE;
}
+ else
+gcc_unreachable ();
if (TREE_THIS_VOLATILE (exp))
{
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index b2adc3d..1603d82 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -677,7 +677,8 @@ diagnose_tm_1 (gimple_stmt_iterator *gsi, bool
*handled_ops_p,
}
else if (direct_call_p)
{
- if (flags_from_decl_or_type (fn) ECF_TM_BUILTIN)
+ if (IS_TYPE_OR_DECL_P (fn)
+flags_from_decl_or_type (fn) ECF_TM_BUILTIN)
is_safe = true;
else if (replacement)
{
Re: [gofrontend-dev] Go patch committed: Implement method values in reflect package
Ian Lance Taylor i...@google.com writes: On Thu, Dec 12, 2013 at 12:21 AM, Michael Hudson-Doyle michael.hud...@linaro.org wrote: Ian Lance Taylor i...@google.com writes: This patch to the Go frontend and libgo implements method values in the reflect package. Working with method values and reflect now works correctly, at least on x86. Can you give me a test case? I can try it on a few other architectures tomorrow. The test case is test/recover.go in the master Go sources. But I know that that test won't work on other architectures, because currently reflect.MakeFunc is only implemented for 386 and amd64. Ah, OK. It should be possible to implement reflect.MakeFunc for all architectures in a somewhat inefficient manner by using libffi's closure API. I have not tried that. It is also of course possible to implement support for any specific processor as I did for 386 and amd64. The difficulty depends on the difficulty of the ABI. In general it's not too hard but it requires a clear understanding of ABI details and assembly language programming with full backtrace information. It's about 600 lines of code for amd64. OK, I'll add it to a list of things to look at at some point... Cheers, mwh
C++ PATCH for c++/58954 (wrong access error with member templates)
Recently I've improved fn_type_unification's handling of access checks,
but resolve_overloaded_unification didn't get the same attention.
Fortunately, it's a simple matter of switching it over to using
instantiate_template so that we have a function to check the accesses
against.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 7ac5af72354e0db079763cd65d4683689542bf3d
Author: Jason Merrill ja...@redhat.com
Date: Thu Dec 12 17:43:14 2013 -0500
PR c++/58954
* pt.c (resolve_overloaded_unification): Use instantiate_template.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 2c64a71..d566afd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16407,7 +16407,7 @@ resolve_overloaded_unification (tree tparms,
if (subargs != error_mark_node
!any_dependent_template_arguments_p (subargs))
{
- elem = tsubst (TREE_TYPE (fn), subargs, tf_none, NULL_TREE);
+ elem = TREE_TYPE (instantiate_template (fn, subargs, tf_none));
if (try_one_overload (tparms, targs, tempargs, parm,
elem, strict, sub_strict, addr_p, explain_p)
(!goodfn || !same_type_p (goodfn, elem)))
diff --git a/gcc/testsuite/g++.dg/cpp0x/access02.C b/gcc/testsuite/g++.dg/cpp0x/access02.C
new file mode 100644
index 000..74960a6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/access02.C
@@ -0,0 +1,39 @@
+// PR c++/58954
+// { dg-require-effective-target c++11 }
+
+templateclass T
+T declval();
+
+templateclass T
+struct foo_argument
+{
+ templateclass Ret, class C, class Arg
+ static Arg test(Ret (C::*)(Arg));
+
+ typedef decltype(test(T::template foo)) type;
+};
+
+templateclass T, class
+struct dependent { typedef T type; };
+
+templateclass T
+struct base
+{
+ templateclass Ignore = void
+ auto foo(int i) - decltype(declval
+typename dependentT, Ignore::type
+ ().foo_impl(i));
+};
+
+struct derived : basederived
+{
+ friend struct basederived;
+private:
+ int foo_impl(int i);
+};
+
+int main()
+{
+ foo_argumentderived::type var = 0;
+ return var;
+}
PATCH to add input_line macro to gdbinit.in
I often use input_line in breakpoint conditions when debugging the compiler, and losing the macro complicates that. Any objection to adding it to gdbinit.in? commit 5bee3eed904bb31ffcca5330f2c36e5aa1c35cb0 Author: Jason Merrill ja...@redhat.com Date: Wed Dec 11 11:29:37 2013 -0500 * gdbinit.in (input_line, input_filename): Define. diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in index aa0bf9b..79361a5 100644 --- a/gcc/gdbinit.in +++ b/gcc/gdbinit.in @@ -195,6 +195,8 @@ macro define __FILE__ gdb macro define __LINE__ 1 macro define __FUNCTION__ gdb macro define __null 0 +macro define input_line expand_location(input_location).line +macro define input_filename expand_location(input_location).file # Gracefully handle aborts in functions used from gdb. set unwindonsignal on
Go testsuite patch committed: Update to current testsuite
This patch updates the Go testsuite to a copy of the current master testsuite. This brings the testsuite, and thus the compiler, up to the final Go 1.2 release. Tested by, of course, running the testsuite, on x86_64-unknown-linux-gnu. Committed to mainline. Ian foo.patch.bz2 Description: patch
Re: PATCH to add input_line macro to gdbinit.in
On Dec 12, 2013, at 7:51 PM, Jason Merrill ja...@redhat.com wrote: I often use input_line in breakpoint conditions when debugging the compiler, and losing the macro complicates that. Any objection to adding it to gdbinit.in? That'd be wonderful.
Re: [REPOST] Invalid Code when reading from unaligned zero-sized array
On 12/11/13 12:19, Eric Botcazou wrote:
Yes we do, even for struct { struct { int a; char a[1] } }; (note the not
really trailing as there is padding after the trailing array). We do
take size limitations from a DECL (if we see one) into account to limit the
effect of this trailing-array-supporting, so it effectively only applies to
indirect accesses (and the padding example above, you can use the whole
padding if DECL_SIZE allows that).
OK, so we want the attached patch? FWIW it passed
make -k check-c check-c++ RUNTESTFLAGS=compat.exp struct-layout-1.exp
on x86/Linux, x86-64/Linux, PowerPC/Linux [*], IA-64/Linux, SPARC/Solaris and
SPARC64/Solaris with ALT_CC_UNDER_TEST set to the unpatched compiler.
[*] the failures (DFP related) are the same as with the unpatched compiler.
Does this catch C99 VLAs? I vaguely recall we have a different internal
representation of those?!? And don't those have the same problem?
[I've been meaning to research the different ways we represent trailing
arrays/VLAs, but haven't had the time yet. ]
I think this patch is a good one, but I'm not sure it's 100% complete yet.
jeff
Re: [PR tree-optimization/59149] fail on invalid arguments to flags_from_decl_or_type
On 12/12/13 16:09, Aldy Hernandez wrote: flags_from_decl_or_type() only handles a TYPE or DECL. Make this explicit instead. I also added a check in the use in trans-mem.c, just in case. The subsequent conditionals should take care of the TM case. It would be nice if Marc Glisse could provide the testcase he mentioned was failing. Either way, the attached patch doesn't hurt. Tested on x86-64 Linux. OK? OK for the trunk. jeff
Re: [PATCH] Don't reject TER unnecessarily (PRs middle-end/58956, middle-end/59470)
Jakub Jelinek ja...@redhat.com wrote:
Hi!
Before the PR58956 fix find_replaceable_in_bb only gave up TER if
stmt was gimple_assign_single_p that was storing something that
could alias with TERed load, but now it also punts on calls (a lot of
them
apparently) and inline asm (all of them, because stmt_may_clobber_ref_p
always returns true for GIMPLE_ASM). I believe we can restore TERing
most of the cases for calls and inline asm though, because for calls
all arguments are necessarily evaluated before the call and the only
memory side effects of the call are the call itself and perhaps storing
of non-SSA_NAME return value afterwards. And for inline asm again
all input operands have to be evaluated before the inline asm, and the
only storing side-effects are again inside of the inline asm and
perhaps
storing of the output operands.
Thus, this patch will only stop TERing if USE is used somewhere in
lhs of a call for calls or somewhere in output arguments of inline asm.
Can you please simply use walk_stmt_load_store_ops to get at the stmt outputs?
Thanks,
Richard.
Bootstrapped/regtested on x86_64-linux and i686-linux (both on trunk
and 4.8
branch), ok for trunk/4.8?
The reason I'd like to see this for 4.8 is to decrease the amount of
code
generation changes from pre-r205709 to minimum, as it can uncover other
latent bugs than just PR59470.
2013-12-12 Jakub Jelinek ja...@redhat.com
PR middle-end/58956
PR middle-end/59470
* tree-ssa-ter.c (find_ssa_name): New helper function.
(find_replaceable_in_bb): For calls, only set same_root_var
if USE is used somewhere in gimple_call_lhs, for GIMPLE_ASM,
only set same_root_var if USE is used somewhere in output operand
trees.
* gcc.target/i386/pr59470.c: New test.
--- gcc/tree-ssa-ter.c.jj 2013-12-10 08:52:13.0 +0100
+++ gcc/tree-ssa-ter.c 2013-12-12 10:43:26.177866960 +0100
@@ -554,6 +554,20 @@ mark_replaceable (temp_expr_table_p tab,
}
+/* Helper function for find_replaceable_in_bb. Called via walk_tree
to
+ find a SSA_NAME DATA somewhere in *TP. */
+
+static tree
+find_ssa_name (tree *tp, int *walk_subtrees, void *data)
+{
+ tree var = (tree) data;
+ if (*tp == var)
+return var;
+ else if (IS_TYPE_OR_DECL_P (*tp))
+*walk_subtrees = 0;
+ return NULL_TREE;
+}
+
/* This function processes basic block BB, and looks for variables
which can
be replaced by their expressions. Results are stored in the table TAB.
*/
@@ -618,7 +632,42 @@ find_replaceable_in_bb (temp_expr_table_
gimple_assign_single_p (def_stmt)
stmt_may_clobber_ref_p (stmt,
gimple_assign_rhs1 (def_stmt)))
- same_root_var = true;
+ {
+if (is_gimple_call (stmt))
+ {
+/* For calls, it is not a problem if USE is among
+ call's arguments or say OBJ_TYPE_REF argument,
+ all those necessarily need to be evaluated before
+ the call that may clobber the memory. But if
+ LHS of the call refers to USE, expansion might
+ evaluate it after the call, prevent TER in that
+ case. */
+if (gimple_call_lhs (stmt)
+ TREE_CODE (gimple_call_lhs (stmt)) != SSA_NAME
+ walk_tree (gimple_call_lhs_ptr (stmt),
+ find_ssa_name, use, NULL))
+ same_root_var = true;
+ }
+else if (gimple_code (stmt) == GIMPLE_ASM)
+ {
+/* For inline asm, allow TER of loads into input
+ arguments, but disallow TER for USEs that occur
+ somewhere in outputs. */
+unsigned int i;
+for (i = 0; i gimple_asm_noutputs (stmt); i++)
+ if (TREE_CODE (gimple_asm_output_op (stmt, i))
+ != SSA_NAME
+ walk_tree (gimple_asm_output_op_ptr (stmt,
+ i),
+find_ssa_name, use, NULL))
+{
+ same_root_var = true;
+ break;
+}
+ }
+else
+ same_root_var = true;
+ }
}
/* Mark expression as replaceable unless stmt is volatile, or
the
--- gcc/testsuite/gcc.target/i386/pr59470.c.jj 2013-12-12
10:31:54.746517544 +0100
+++ gcc/testsuite/gcc.target/i386/pr59470.c2013-12-12
10:32:42.045273313 +0100
@@ -0,0
Re: [PR tree-optimization/59149] fail on invalid arguments to flags_from_decl_or_type
On Thu, 12 Dec 2013, Aldy Hernandez wrote: flags_from_decl_or_type() only handles a TYPE or DECL. Make this explicit instead. I also added a check in the use in trans-mem.c, just in case. The subsequent conditionals should take care of the TM case. It would be nice if Marc Glisse could provide the testcase he mentioned was failing. Either way, the attached patch doesn't hurt. The failing testcases are already in the testsuite (I don't remember which ones, but there were quite a few IIRC). If you add only the gcc_unreachable and not the other part of your patch, you should see several failures. -- Marc Glisse
Re: [PATCH] Don't reject TER unnecessarily (PRs middle-end/58956, middle-end/59470)
On Fri, Dec 13, 2013 at 07:30:12AM +0100, Richard Biener wrote:
Jakub Jelinek ja...@redhat.com wrote:
lhs of a call for calls or somewhere in output arguments of inline asm.
Can you please simply use walk_stmt_load_store_ops to get at the stmt outputs?
No, unfortunately. The problem is that walk_stmt_load_store_{addr_,}ops first
calls and get_base_loadstore on the operand and thus effectively strips all the
handled components from it. But we need to look at any uses of SSA_NAMEs
in the whole operand, not only if it is based on *MEM_REF with SSA_NAME
operand. I.e., a change of the patch to use walk_stmt_load_store_ops
will keep the pr58956.c testcase fixed, because there is *i, but will
make pr59470.c (the new one in the patch) broken, because there the
SSA_NAME is used as ARRAY_REF index and base is some VAR_DECL.
It guess it wouldn't be hard to make similar testcase even for the call
case, though it is unclear if it would be miscompiled or not.
Jakub
Re: [PATCH, nds32] Missing target_cpu_default in TARGET_DEFAULT_TARGET_FLAGS.
2013/12/11 Monk Chiang sh.chian...@gmail.com: Hi, Recently I used --target=nds32be-elf to configure nds32 gcc, it seems that the big endian is not set as default. [...] The following is the patch to fix this issue. Tested on nds32be-elf. OK to apply? Index: common/config/nds32/nds32-common.c === --- common/config/nds32/nds32-common.c (revision 205880) +++ common/config/nds32/nds32-common.c (working copy) @@ -93,7 +93,8 @@ TARGET_CMOV : Generate conditional move instruction. */ Could you also extend the comment about adding TARGET_CPU_DEFAULT? That would be great to let other developers realize why we need it. :) #undef TARGET_DEFAULT_TARGET_FLAGS #define TARGET_DEFAULT_TARGET_FLAGS\ - (MASK_GP_DIRECT \ + (TARGET_CPU_DEFAULT \ + | MASK_GP_DIRECT\ | MASK_16_BIT \ | MASK_PERF_EXT \ | MASK_CMOV) Index: ChangeLog === --- ChangeLog (revision 205880) +++ ChangeLog (working copy) @@ -1,3 +1,8 @@ +2013-12-11 Monk Chiang sh.chian...@gmail.com In ChangeLog formatting, there should be two spaces between 'Chiang' and ''. + + * common/config/nds32/nds32-common.c (TARGET_DEFAULT_TARGET_FLAGS): + Redefine. + Suggest using 'Consider TARGET_CPU_DEFAULT settings.' 2013-12-11 Bin Cheng bin.ch...@arm.com OK with those changes. Thank you for the patch fixing that issue. :) Best regards, jasonwucj
