Re: VEC re-write [patch 01/25]
On Sat, 17 Nov 2012, Diego Novillo wrote: I have now committed all 25 parts of this patch as rev 193595. Please CC me on any problems that you think may be related to this rewrite. That seems to have trigged some bug in gcc-4.4-era. See PR55381. There are a lot of suspicious warnings from vec.h. It smells a bit like a host gcc bug, but I'll have to find a newer version where it builds to confirm. (If so, hopefully it's as simple as upping the minimum host gcc version or blacklisting gcc-4.4.x.) brgds, H-P
Re: VEC re-write [patch 01/25]
On Sun, 18 Nov 2012, Diego Novillo wrote: My cris-elf builds worked fine, but config-list.mk only builds stage 1, it does not build libgfortran. Can you give me instructions on how to build your target on my x86 workstation? Better see the PR for cc1 command-line and preprocessed C-file. brgds, H-P
Re: VEC re-write [patch 01/25]
On Sun, 18 Nov 2012, Andreas Tobler wrote: Is there a minimum gcc to bootstrap current trunk? I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6 succeeds. A gcc-4.1.2 has worked for me in the past, before this recent vec.h change. I think that's the minimum version reportedly working. brgds, H-P
Re: VEC re-write [patch 01/25]
On Sun, 18 Nov 2012, Andreas Tobler wrote: On 18.11.12 20:11, Hans-Peter Nilsson wrote: On Sun, 18 Nov 2012, Andreas Tobler wrote: Is there a minimum gcc to bootstrap current trunk? I currently fail to bootstrap trunk with a 4.2.1 gcc, but a 4.6 succeeds. A gcc-4.1.2 has worked for me in the past, before this recent vec.h change. I think that's the minimum version reportedly working. Up to 193594 a 4.2.1 did it fine on most of my platforms. But with 193595 it seems that I have to use a more recent gcc to bootstrap. Yes, that's the vec.h-change revision. I hope we can revert that behavior (the *behavior*, not the patch). brgds, H-P
Re: PATCH: Add --with-build-config=bootstrap-asan support
On Sun, 18 Nov 2012, Paolo Bonzini wrote: Il 18/11/2012 16:59, Hans-Peter Nilsson ha scritto: Nice, but I agree with the other poster that this'd IMHO be better as --enable-checking=asan (or actually, --enable-checking=all,asan). Yeah, that's a good thing to support too. However, you still need a toplevel build configuration to enable asan post-stage1. Configuring with --enable-checking=all,asan would require the bootstrap compiler to support asan, too. We seem to be able to add various flags to compilations *after* the first stage (that are not valid at the first stage, for the bootstrapping/host compiler) so I still don't get why this should be conceptually different. But whatever. brgds, H-P
Re: [PATCH, generic] New RTL primitive: `define_subst'
On Wed, 14 Nov 2012, Jakub Jelinek wrote: Which brings us to the question of what to do with the patch for 4.8. It's true that you made the deadline for stage1 closure. But there will be no users of this feature, so it begs the question of why we should apply it now. Have you a convincing reason? RM's do you have an opinion here? I'm not against it going in now, it shouldn't be too risky and will allow people to start experimenting with it sooner. Another point in favor of applying it within 4.8 is that it makes for easier backport of those bug fixes that are simplified by use of define_subst. brgds, H-P
Re: [COMMITTED] Move libsanitizer configure logic to subdirectory
On Tue, 13 Nov 2012, Richard Henderson wrote: As discussed elsewhere. Tested on x86_64-linux. +2012-11-13 Richard Henderson r...@redhat.com + + * configure.ac: Move libsanitizer logic to subdirectory. + * configure: Regenerate. + Thanks and sorry for copypasting the wrong pattern in the first place. +++ b/libsanitizer/configure.tgt @@ -0,0 +1,28 @@ +# -*- shell-script -*- +# Copyright (C) 2012 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. Heh, version 2... But licensing changes are never obvious, right? brgds, H-P
Re: [Committed] Add testcase
On Fri, 9 Nov 2012, Andrew Pinski wrote: Committed the testcase as obvious after a quick test to make sure it works. Note someone might need to mark the testcase as only executable on targets which have 32bit ints. Someone like you? There are plenty of greppable effective-target attributes and gcc.c-torture handles dg- marking fine these days. ChangeLog: * gcc.c-torture/execute/20121108-1.c: New testcase. Please avoid printing things, just call abort or exit. brgds, H-P
Regression with [C++11] PR54413 Option for turning off compiler extensions for numeric literals.
From: Ed Smith-Rowland 3dw...@verizon.net Date: Fri, 9 Nov 2012 05:55:16 +0100 libcpp 2012-11-09 Ed Smith-Rowland 3dw...@verizon.net PR c++/54413 * include/cpplib.h (cpp_interpret_float_suffix): Add cpp_reader* arg. (cpp_interpret_int_suffix): Add cpp_reader* arg. * init.c (cpp_create_reader): Iitialize new flags. * expr.c (interpret_float_suffix): Use new flags. (cpp_interpret_float_suffix): Add cpp_reader* arg. (interpret_int_suffix): Use new flags. (cpp_interpret_int_suffix): Add cpp_reader* arg. (cpp_classify_number): Adjust calls to interpret_x_suffix. gcc/c-family 2012-11-09 Ed Smith-Rowland 3dw...@verizon.net PR c++/54413 * c-opts.c (c_common_handle_option): Set new flags. * c.opt: Describe new flags. gcc/cp 2012-11-09 Ed Smith-Rowland 3dw...@verizon.net PR c++/54413 * decl.c (grokfndecl): Adjust calls to interpret_x_suffix. One of these caused PR55325. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
From: Dodji Seketeli do...@redhat.com Date: Tue, 13 Nov 2012 16:04:12 +0100 I guess when the issue of the missing files is resolved, we can enable building libsanitizer on Darwin proper. Here is the patchlet I am proposing so far http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00993.html. That's the direction I suggested, thanks for resolving this. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
From: Richard Henderson r...@redhat.com Date: Tue, 13 Nov 2012 21:38:40 +0100 On 11/13/2012 05:24 AM, Jakub Jelinek wrote: Yes. And it shouldn't be just based on target CPU, but also based on target OS, I don't think libsanitizer supports anything but linux (glibc + maybe android) right now, with some smaller or bigger tweaks it could support darwin (but see the reports that it doesn't build there right now) or mingw/cygwin? (but there is a PR that it doesn't build there). So IMHO it should be a whitelist of supported targets with *) case adding noconfigdirs=$noconfigdirs target-libsanitizer, rather than blacklist of few unsupported ones. Can you please prepare a patch? See how libatomic and libitm are structured. The logic for what targets are supported belongs inside the library directory, and not at top-level. Right. And, I think it's worth to repeat ;) that IMHO best to there simply check that -faddress-sanitizer can compile error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on the target). No target lists needed. brgds, H-P
Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
From: Richard Henderson r...@redhat.com Date: Wed, 14 Nov 2012 02:34:32 +0100 On 11/13/2012 05:20 PM, Hans-Peter Nilsson wrote: Right. And, I think it's worth to repeat ;) that IMHO best to there simply check that -faddress-sanitizer can compile error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on the target). No target lists needed. We can't do that, since the compiler hasn't been built at top-level configure time. Obviously. Yah, I was referring to configure-time in libsanitizer ...but that's obviously not what you meant; I now see you referred to toplevel configure.ac including various subdir/configure.tgt at (toplevel) configure-time. Delaying bail-out to target-library configure-time would probably require things like a dummy buildsubdir/Makefile. I don't see at a glance other libraries doing that so might not be worth pursuing. Bah. brgds, H-P
Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.
While the fallout(*) from the libsanitizer commit is handled,
it's obvious it should have a noconfigdirs= section in
toplevel/configure.ac like the other target libs. Here's what I
committed after observing that a cris-elf build passed, where it
previously failed building libsanitizer which wrongly tries to
compile using -fPIC despite checking in its configure.ac IIUC.
But, I'd like to update the target contents there to something a
bit more generic.
Maybe disable libsanitizer everywhere except for (referring to
the three common target-related file-name parts in libsanitizer)
mac, win and linux? Or disable everywhere except x86_64 /
i386? That's the only port that defines the required
TARGET_ASAN_SHADOW_OFFSET. Maybe the library configure.ac
should check whether using -fsanitizer is error-free and disable
the libsanitizer build automatically?
toplevel:
* configure.ac: Add section for noconfigdirs for libsanitizer.
Disable for cris-*-* and mmix-*-*.
* configure: Regenerate.
Index: configure.ac
===
--- configure.ac(revision 193455)
+++ configure.ac(working copy)
@@ -547,6 +547,13 @@ case ${target} in
;;
esac
+# Disable libsanitizer for some systems.
+case ${target} in
+ cris-*-* | crisv32-*-* | mmix-*-*)
+noconfigdirs=$noconfigdirs target-libsanitizer
+;;
+esac
+
# Disable libssp for some systems.
case ${target} in
avr-*-*)
Index: configure
===
--- configure (revision 193455)
+++ configure (working copy)
@@ -3205,6 +3205,13 @@ case ${target} in
;;
esac
+# Disable libsanitizer for some systems.
+case ${target} in
+ cris-*-* | crisv32-*-* | mmix-*-*)
+noconfigdirs=$noconfigdirs target-libsanitizer
+;;
+esac
+
# Disable libssp for some systems.
case ${target} in
avr-*-*)
*) -fPIC passed when building libsanitizer for targets that
don't support it, lack of multilib setup for libsanitizer,
libsanitizer not installed in version-specific subdir...
Basically, IMHO it should just copy the generic
libgfortran/configure.ac and be done with it. Right now it has
the smallest configure.ac of the target-libraries.
brgds, H-P
Committed: Fix PR55257: g++.dg/debug/dwarf2/non-virtual-thunk.C and heads-up target maintainers
...those of you who don't have the equivalent of the patch
below in your ports, that is. You'll likely only notice through
a slightly reduced debugging experience and
g++.dg/debug/dwarf2/non-virtual-thunk.C failing. Yes, the docs
should mention these functions need to be called IMHO obviously,
but not by me not now maybe later. Tested cris-elf, committed.
gcc:
PR target/55257
* config/cris/cris.c (cris_asm_output_mi_thunk): Call
final_start_function and final_end_function.
Index: gcc/config/cris/cris.c
===
--- gcc/config/cris/cris.c (revision 192677)
+++ gcc/config/cris/cris.c (working copy)
@@ -2698,6 +2698,9 @@ cris_asm_output_mi_thunk (FILE *stream,
HOST_WIDE_INT vcall_offset ATTRIBUTE_UNUSED,
tree funcdecl)
{
+ /* Make sure unwind info is emitted for the thunk if needed. */
+ final_start_function (emit_barrier (), stream, 1);
+
if (delta 0)
fprintf (stream, \tadd%s HOST_WIDE_INT_PRINT_DEC ,$%s\n,
ADDITIVE_SIZE_MODIFIER (delta), delta,
@@ -2735,6 +2738,8 @@ cris_asm_output_mi_thunk (FILE *stream,
if (TARGET_V32)
fprintf (stream, \tnop\n);
}
+
+ final_end_function ();
}
/* Boilerplate emitted at start of file.
brgds, H-P
[RFA:] fix PR55030, wrong code from __builtin_setjmp_receiver
The problem exposed in PR55030 (repeatable on x86_64-linux with -m32 at
r192676) is that the fake-frame-pointer frame is replaced with the
actual-frame-pointer bp in cse1, around the critical insn in
__builtin_setjmp_receiver that restores their defined offset. The
patch in PR55030/r192676 removed a clobber of the frame-pointer, and
cse1 felt free to replace the former register with the latter, as in
the following expansion of __builtin_setjmp_receiver (in builtins.c:
expand_builtin_setjmp_receiver) from a reduced test-case, see the PR.
(You might find the setup/restore code having an unexpected order;
there are two __builtin_setjmps in series and the setup of the second
one is storing the wrong value for the frame-pointer.)
The pre-transformation dump in cse1 says:
(code_label/s 13 51 14 3 2 [2 uses])
(note 14 13 15 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 15 14 78 3 (use (reg/f:SI 6 bp))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
-1
(nil))
(insn 78 15 17 3 (set (reg/f:SI 20 frame)
(reg/f:SI 6 bp))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
-1
(nil))
(insn 17 78 56 3 (unspec_volatile [
(const_int 0 [0])
] UNSPECV_BLOCKAGE)
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
643 {blockage}
(nil))
(insn 56 17 57 3 (set (reg/f:SI 74)
(symbol_ref:SI (chk_fail_buf) [flags 0x40] var_decl 0x77512688
chk_fail_buf))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:37
65 {*movsi_internal}
(nil))
(insn 57 56 58 3 (set (mem:SI (reg/f:SI 74) [5 S4 A8])
(reg/f:SI 20 frame))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:37
65 {*movsi_internal}
(nil))
Before r192676 and after the reversion in r192701, there was/is that
weird extra (clobber (reg/f:SI 6 bp) inserted before insn 17. But
without that, we notice post-cse1-transformation, a change in insn 57:
(code_label/s 13 51 14 3 2 [3 uses])
(note 14 13 15 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 15 14 78 3 (use (reg/f:SI 6 bp))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
-1
(nil))
(insn 78 15 17 3 (set (reg/f:SI 20 frame)
(reg/f:SI 6 bp))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
-1
(nil))
(insn 17 78 56 3 (unspec_volatile [
(const_int 0 [0])
] UNSPECV_BLOCKAGE)
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:31
643 {blockage}
(nil))
(insn 56 17 57 3 (set (reg/f:SI 74)
(symbol_ref:SI (chk_fail_buf) [flags 0x40] var_decl 0x77512688
chk_fail_buf))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:37
65 {*movsi_internal}
(nil))
(insn 57 56 58 3 (set (mem:SI (reg/f:SI 74) [5 S4 A8])
(reg/f:SI 6 bp))
/home/hp/gcctop/tmp/base/gcc/gcc/testsuite/gcc.c-torture/execute/builtins/pr55030-chk.c:37
65 {*movsi_internal}
(nil))
It seemed wrong for this change to be stopped *only* by that (clobber bp).
Stepping through the code for insn 78 and 57, I was looking for
related special-casing of frame- and other similar registers in cse.c
and was a bit confused when I found none (not counting hash_rtx_cb).
(I admit lack of special-casing is a good sign, though. :) Then I was
blinded by the obvious; the next insn, insn 17, is a blockage insn.
This is a target-specific blockage insn, but with the general form
found in all targets defining it. The default blockage is an empty
asm-volatile, which is what cse_insn recognized. The blockage insn is
there to prevent scheduling of the critical insns and register
values. It's almost obvious that an unspec_volatile should have that
effect too; at least that's intended by the code in
expand_builtin_setjmp_receiver. Luckily (or unluckily regarding the
presence of the bug) *this* cse code is not run post-frame-layout
(post-reload-cse does not use this code), or it seems people would
soon notice register values used from the wrong side of the blockage,
considering its critical use at the restore of the stack-pointer.
As mentioned in the previous patch,
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01172.html, clobbering
the frame-pointer (and then using it) does not seem the logical
alternative to the patch below; the blockage insn should just do its job.
I updated comments and documentation so it's more apparent that
register uses should also not be moved across the blockage; see the
patched comments.
Tested native x86_64-unknown-linux-gnu w./wo. -m32 and before/after
192677. Ok to commit?
gcc:
PR middle-end/55030
* builtins.c (expand_builtin_setjmp_receiver): Update comment
regarding purpose of blockage.
* emit-rtl.c [!HAVE_blockage] (gen_blockage): Similarly for
Re: patch fixing a test for PR55151
On Wed, 7 Nov 2012, Vladimir Makarov wrote:
On 12-11-07 5:27 PM, H.J. Lu wrote:
You should check !ia32 target:
/* { dg-do compile { target { ! { ia32 } } } } */
Thanks, H.J. I've just fixed it.
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 193316)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2012-11-07 Vladimir Makarov vmaka...@redhat.com
+
+ PR rtl-optimization/55151
+ * gcc.dg/pr55151.c: Use ia32 instead of x86_64.
+
Lots of constraints there that might not be suitable for all
machines. Unless it's expected to pass (almost) everywhere, it
should move to gcc.target/i386.
If it *is* expected to pass everywhere,
at least add /* { dg-require-effective-target fpic } */
or similar (can be done in the target expression).
brgds, H-P
Re: [RFA:] PR55186 - gcc.dg/const-uniq-1.c fails due to vector not in the constant pool
From: Eric Botcazou ebotca...@adacore.com Date: Mon, 5 Nov 2012 15:29:11 +0100 But, for cris-elf (and reasonably the same for other targets) there might not be such a constant-pool entry in the first place: the vectors are too short to rule out piecewise initialization as optimal for size (counting the vectors once per use). Let's increase them. Twice might just break-even for cris-elf so let's make them four times as long. Sanity-checked for x86_64 w/w.o -m32 for a gcc-version that includes the optimization for which the test-case tests (CCing author). I don't know cris at all, but 4 times sounds quite a lot compared to the other architectures (I presume that the test passes on most of them). That's right, because I *want* the size to be excessive compared to the (expected max) in-line-limits of any target. What I wrote above regarding numbers is incorrect; i misthought. Counting just the size of the vector and emitted instructions, it wouldn't make sense to just increase the vector, as the size for the best-case instructions for initializing one item in the 32-bit-int: moveq 1,$r2 move.d $r2,[$r10+] is four bytes; the size the corresponding constant-vector item. The overhead for forming the address in a register is of course smaller than the overhead for calling memcpy, as the former is a subset of the latter. :) What happens is that I increase the vector-length much above what I see as the expected target-specific max-limit for the in-lined memcpy/memset/whatever; from 32 to 128 for 32-bit targets. Yes, outliers are expected to have to adjust the test-case anyway; this outlier is just the first one, I'm vainly hoping there will be no others. :) I could also have just changed the numbers in the vector from those expected to be quickly formed (for CRIS, 0..31) to e.g. random numbers, but then I'd have to deal with 32-bit vs. 16-bit-issues ...and also, to be absolutely correct I'd have to deal with issues where the size of the emitted in-lined code isn't fit for -Os compared to memcpy. IIRC when -Os, gcc middle-end *doesn't* emit rtl and add the sizes of the insns (the insn attribute-size numbers) before deciding on emitting insns or in-lined memcpy. I *guess* the 32 bytes set [for CRIS] by MOVE_RATIO is the only target-number that matters at present. Since this minor can of worms wasn't the point of this test-case, I opted for simply increasing the size of the vector, keeping it simple. No objections by me though. I take that as approval. Committed as posted. brgds, H-P
Re: [PATCH, generic] New RTL primitive: `define_subst'
On Sun, 4 Nov 2012, Kirill Yukhin wrote: Hi, But... I don't really understand it, so here's some feedback on the documentation: Regarding the language, a definite article is Patch with fixed doc is attached. Changelog is the same Is it OK? The structure is much improved and quite satisfactory, thank you! I can actually understand it, even through a mild fever. :) This feature looks quite nice: the example covers exactly where I hope to use it: explicit condition-code settings i.e. to move a target from the deprecated cc0 machinery without source pattern explosion. (Patterns generally need to be expressed in three forms: clobbering cc0, setting cc0, and special cases for insns with variants that don't touch cc0, so two substs.) Come to think of it, maybe subst iterator is somewhat a misnomer; perhaps better use just subst in place of subst iterator in the documentation. Language issues with definite article remain, but I'll leave further nitpicking until maintainers with actual approval powers have reviewed the feature and its implementation, from a higher level. brgds, H-P
[RFA:] PR55186 - gcc.dg/const-uniq-1.c fails due to vector not in the constant pool
Due to weird circumstances detailed in the PR, this test briefly
passed (it has always failed before), so technically I'm fixing
a regression. :)
The test checks that a certain label is mentioned twice; being
mentioned once infers that there are two identical initializer
vectors in the constant-pool.
But, for cris-elf (and reasonably the same for other targets)
there might not be such a constant-pool entry in the first
place: the vectors are too short to rule out piecewise
initialization as optimal for size (counting the vectors once
per use). Let's increase them. Twice might just break-even for
cris-elf so let's make them four times as long. Sanity-checked
for x86_64 w/w.o -m32 for a gcc-version that includes the
optimization for which the test-case tests (CCing author).
Ok to commit?
gcc/testsuite:
PR testsuite/55186
* gcc/testsuite/gcc.dg/const-uniq-1.c (a): Increase length four times.
Index: gcc/testsuite/gcc.dg/const-uniq-1.c
===
--- gcc/testsuite/gcc.dg/const-uniq-1.c (revision 193117)
+++ gcc/testsuite/gcc.dg/const-uniq-1.c (working copy)
@@ -5,13 +5,18 @@
int lookup1 (int i)
{
- int a[] = { 0, 1, 2, 3, 4, 5, 6, 7 };
+ /* We use vectors long enough that piece-wise initialization is not
+ reasonably preferable even for size (when including the constant
+ vectors for initialization) for any target. */
+ int a[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
return a[i];
}
int lookup2 (int i)
{
- int a[] = { 0, 1, 2, 3, 4, 5, 6, 7 };
+ int a[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
return a[i+1];
}
brgds, H-P
Re: [PATCH] Fix CDDCE miscompilation (PR tree-optimization/55018)
From: Steven Bosscher stevenb@gmail.com Date: Sun, 28 Oct 2012 19:33:29 +0100 On Mon, Oct 22, 2012 at 11:09 PM, Jakub Jelinek wrote: On Mon, Oct 22, 2012 at 10:51:43PM +0200, Steven Bosscher wrote: Wouldn't it be way cheaper to just export dfs_find_deadend from cfganal.c and call it in calc_dfs_tree on each unconnected bb? I.e. (untested with the exception of the testcase): 2012-10-22 Jakub Jelinek ja...@redhat.com PR tree-optimization/55018 * cfganal.c (dfs_find_deadend): No longer static. * basic-block.h (dfs_find_deadend): New prototype. * dominance.c (calc_dfs_tree): If saw_unconnected, traverse from dfs_find_deadend of unconnected b instead of b directly. * gcc.dg/torture/pr55018.c: New test. Attached patch was bootstrappedtested on gcc/ PR tree-optimization/55018 * basic-block.h (dfs_find_deadend): New prototype. * cfganal.c (dfs_find_deadend): No longer static. Use bitmap instead of sbitmap for visited. (flow_dfs_compute_reverse_execute): Use dfs_find_deadend here, too. * dominance.c (calc_dfs_tree): If saw_unconnected, traverse from dfs_find_deadend of unconnected b instead of b directly. It seems this caused PR55168, ICE. brgds, H-P
Re: [PATCH, generic] New RTL primitive: `define_subst'
(CC list trimmed.)
On Wed, 31 Oct 2012, Kirill Yukhin wrote:
Hi,
This patch introduces a new RTL expression called define_subst and
required by it define_subst_attr.
The new feature allows to make MD-files more compact - it defines a
rule by which a parser could generate modified versions of
RTL-templates. For example, i386.md contains RTL-templates for
ordinary arithmetic, as well as templates for arithmetic with
zero-extension. With define_subst one could specify one rule (write
one define_subst) and keep only RTL-templates for ordinary arithmetic
- the versions with zero-extensions will be generated automatically.
The rule in this case would describe how zero-extended pattern could
be generated with given pattern without zero-extension.
There are some more examples in existing RTL templates where
define_subst could be useful, and it would be extremely useful for
future x86 ISA extensions.
I'd be glad to have a feedback on this feature and ask any questions.
Sounds great, (I think) this is something I've longed to see
ever since I saw the iterator machinery (thanks, Richard S!) and
wanted to do structural replacements just as easily.
But... I don't really understand it, so here's some feedback on
the documentation: Regarding the language, a definite article is
missing in about half the places where one is needed, at least
it seems so to me; not a native English speaker. There are
other similar grammar issues. I suggest having a native speaker
proofread it. (I'm just guessing that being with Intel gives
the benefit that there's someone that could do that for you.)
I think it'd make a big difference if you put the example first
and the detailed explanation *after that*, around Most of
expressions (or as it should be: Most expressions).
Start with a pattern where the feature is used as intended in
the .md and show the expansion only *after* that; it seems
you're doing it the other way round (or I'm confused). Try to
avoid using other types of iterator expansions in the same
example. To do that, it's ok to simplify the example: it
doesn't have to be exactly as actually used; it likely won't
stay that way for long anyway.
Specifically, I don't get the If @code{define_subst} could not
be applied because the pattern does not match the
@code{define_insn}/@code{define_expand} original template, then
the second copy is deleted. I think an example showing such an
expansion (partial and failed) would do wonders.
Thanks for your work, I hope something like this gets in for 4.8.
brgds, H-P
Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls
On Wed, 24 Oct 2012, Dodji Seketeli wrote: Jakub Jelinek ja...@redhat.com writes: On Tue, Oct 23, 2012 at 03:11:29PM +0200, Dodji Seketeli wrote: * asan.c (insert_if_then_before_iter) (instrument_mem_region_access) (maybe_instrument_builtin_call, maybe_instrument_call): New static Why not just write it: * asan.c (insert_if_then_before_iter, instrument_mem_region_access, maybe_instrument_builtin_call, maybe_instrument_call): New static ? It's emacs that formats it like that automatically. I am not sure how to teach him otherwise. I have fixed this as you want by doing it by hand. JFTR, the emacs-formatted-version *is* the correct one, at least when going by the GNU coding standard. Maybe something about being eager to balance parentheses as early as possible. :) But, I'll keep formatting it like that myself until someone updates the GCC coding standard. Hint, like. brgds, H-P
Re: [RFA:] Fix frame-pointer-clobbering in builtins.c:expand_builtin_setjmp_receiver
On Tue, 23 Oct 2012, Dominique Dhumieres wrote: This patch (r192676) is probably causing FAIL: gcc.c-torture/execute/builtins/memcpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/memmove-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/mempcpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/memset-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/snprintf-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/stpcpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/stpncpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/strcat-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/strcpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/strncat-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/strncpy-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/vsnprintf-chk.c execution, -Os FAIL: gcc.c-torture/execute/builtins/vsprintf-chk.c execution, -Os on i?86 (see http://gcc.gnu.org/ml/gcc-testresults/2012-10/msg02350.html ). Confirmed, now PR55030. I'll revert that patch pending further investigation. Feel free to open PR's whenever something like this happens. brgds, H-P
Committed: fix gcc.dg/webizer.c (the trivial part :)
Committed as obvious. Without this, mmix-knuth-mmixware exited with
2256, likely a return-address left over in the return-value-register.
I used __builtin_exit just to avoid declarations or includes; the
default implicit declaration apparently incompatible with this use.
I saw nothing in the related conversation thread that this was
anything but an oversight. Of course this might have caused an
unintended failure mistaken for a valid test-case, but Occam says
that's overthinking.
testsuite:
* gcc.dg/webizer.c (main): Add missing exit call.
Index: gcc.dg/webizer.c
===
--- gcc.dg/webizer.c(revision 192651)
+++ gcc.dg/webizer.c(working copy)
@@ -32,4 +32,5 @@ configure2()
main()
{
configure2();
+ __builtin_exit (0);
}
brgds, H-P
Re: Committed: fix gcc.dg/webizer.c (the trivial part :)
On Sun, 21 Oct 2012, Andreas Schwab wrote: Hans-Peter Nilsson h...@bitrange.com writes: I used __builtin_exit just to avoid declarations or includes; A return 0 would have worked just as well. It's not the preferred method. Fail: abort(). Pass: exit(0). Don't remember where I read it, though. Brgds, H-P
Committed, libgcc MMIX: implement static marking of program and data memory
With a simulator that doesn't just allocate zeros on any access, it's necessary to tell the simulator the bounds of defined memory, both for static and dynamically allocated memory. This patch implements static code and data allocatation; zero'd data and constants may not be otherwise loaded. A patch to newlib is about to be committed to mark dynamically allocated memory. The attached mmix-sim.ch (a literate programming / ctangle change file; the equivalent of a patch) implements such marking and memory checking. Put it into the untarred mmix-20110831.tar.gz (may work with other versions) before compiling the mmix simulator. Beware that the distribution terms of the mmix simulator requires (in my layman interpretation) that the resulting program is not distributed as any part of the original mmixware package. There was some fallout from this new checking, but I believe all needed patches have been at least submitted, though not all approved and committed. libgcc: * config/mmix/crti.S: Mark program and data addresses using PRELD. Remove typo'd and unnecessary alignment-LOC for .data. Remove no-longer-needed LDBU insns. Index: crti.S === --- crti.S (revision 192353) +++ crti.S (working copy) @@ -35,20 +35,25 @@ see the files COPYING3 and COPYING.RUNTI % respectively, so the compiler can switch between them pretending they're % segments. -% This little treasure is here so the 32 lowest address bits of user data -% will not be zero. Because of truncation, that would cause testcase -% gcc.c-torture/execute/980701-1.c to incorrectly fail. +% This little treasure (some contents) is required so the 32 lowest +% address bits of user data will not be zero. Because of truncation, +% that would cause testcase gcc.c-torture/execute/980701-1.c to +% incorrectly fail. .data ! mmixal:= 8H LOC Data_Segment .p2align 3 - LOC @+(8-@)@7 - OCTA 2009 +dstart OCTA 2009 .text ! mmixal:= 9H LOC 8B; LOC #100 .global Main % The __Stack_start symbol is provided by the link script. stackppOCTA __Stack_start +crtstxtOCTA _init % Assumed to be the lowest executed address. + OCTA __etext% Assumed to be beyond the highest executed address. + +crtsdatOCTA dstart % Assumed to be the lowest accessed address. + OCTA _end % Assumed to be beyond the highest accessed address. % Main is the magic symbol the simulator jumps to. We want to go % on to main. @@ -56,16 +61,47 @@ stackpp OCTA __Stack_start Main SETL$255,32 PUT rG,$255 +% Make sure we have valid memory for addresses in .text and .data (and +% .bss, but we include this in .data), for the benefit of mmo-using +% simulators that require validation of addresses for which contents +% is not present. Due to its implicit-zero nature, zeros in contents +% may be left out in the mmo format, but we don't know the boundaries +% of those zero-chunks; for mmo files from binutils, they correspond +% to the beginning and end of sections in objects before linking. We +% validate the contents by executing PRELD (0; one byte) on each +% 2048-byte-boundary of our .text .data, and we assume this size +% matches the magic lowest-denominator chunk-size for all +% validation-requiring simulators. The effect of the PRELD (any size) +% is assumed to be the same as initial loading of the contents, as +% long as the PRELD happens before the first PUSHJ/PUSHGO. If it +% happens after that, we'll need to distinguish between +% access-for-execution and read/write access. + + GETA$255,crtstxt + LDOU$2,$255,0 + ANDNL $2,#7ff % Align the start at a 2048-boundary. + LDOU$3,$255,8 + SETL$4,2048 +0H PRELD 0,$2,0 + ADDU$2,$2,$4 + CMP $255,$2,$3 + BN $255,0B + + GETA$255,crtsdat + LDOU$2,$255,0 + ANDNL $2,#7ff + LDOU$3,$255,8 +0H PRELD 0,$2,0 + ADDU$2,$2,$4 + CMP $255,$2,$3 + BN $255,0B + % Initialize the stack pointer. It is supposedly made a global % zero-initialized (allowed to change) register in crtn.S; we use the % explicit number. GETA$255,stackpp LDOU$254,$255,0 -% Make sure we get more than one mem, to simplify counting cycles. - LDBU$255,$1,0 - LDBU$255,$1,1 - PUSHJ $2,_init #ifdef __MMIX_ABI_GNU__ brgds, H-PCopyright 2012 Hans-Peter Nilsson. This file may be freely copied and distributed, provided that no changes whatsoever are made. Ah, just kidding: you may change it as you like, except that this paragraph, including the above attribution, must be kept unmodified and the distribution terms not limited. Add your own attribution below if you change anything, so people don't blame me. Special permission is granted for the copyright owner
Ping: [RFA:] Fix frame-pointer-clobbering in builtins.c:expand_builtin_setjmp_receiver
CC:ing middle-end maintainers this time. I was a bit surprised
when Eric Botcazou wrote in his review, quoted below, that he's
not one of you. Maybe approve that too?
On Mon, 15 Oct 2012, Hans-Peter Nilsson wrote:
On Fri, 12 Oct 2012, Eric Botcazou wrote:
(insn 168 49 51 3 (set (reg/f:DI 253 $253)
(plus:DI (reg/f:DI 253 $253)
(const_int 24 [0x18])))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21
-1 (nil))
(insn 51 168 52 3 (clobber (reg/f:DI 253 $253))
...
Note that insn 168 deleted, which seems a logical optimization. The
bug is to emit the clobber, not that the restoring insn is removed.
Had that worked in the past for MMIX?
Yes, for svn revision 106027 (20051030) 4.1.0-era (!)
http://gcc.gnu.org/ml/gcc-testresults/2005-10/msg01340.html
where the test must have passed, as
gcc.c-torture/execute/built-in-setjmp.c is at least four years
older than that.
If so, what changed recently?
By these days I didn't mean recent, just not eons ago. :)
I see in a gcc-test-results posting from Mike Stein (whom I'd
like to thank for test-results posting over the years), matching
FAILs for svn revision 126095 (20070628) 4.3.0-era
http://gcc.gnu.org/ml/gcc-testresults/2007-06/msg01287.html.
Sorry, I have nothing in between those reports, my bad. Though
I see no point narrowing down the failing revision further here
IMO; as mentioned the bug is not that the restoring insn is
removed.
Agreed. However, I'd suggest rescuing the comment for the ELIMINABLE_REGS
block from expand_nl_goto_receiver as it still sounds valid to me.
Oops, my bad; I see I removed all the good comments. Fixed.
* stmt.c (expand_nl_goto_receiver): Remove almost-copy of
expand_builtin_setjmp_receiver.
(expand_label): Adjust, call expand_builtin_setjmp_receiver
with NULL for the label parameter.
* builtins.c (expand_builtin_setjmp_receiver): Don't clobber
the frame-pointer. Adjust comments.
[HAVE_builtin_setjmp_receiver]: Emit builtin_setjmp_receiver
only if LABEL is non-NULL.
I cannot formally approve, but this looks good to me modulo:
+ If RECEIVER_LABEL is NULL, instead the port-specific parts of a
+ nonlocal goto handler are emitted. */
The port-specific parts wording is a bit confusing I think. I'd just
write:
If RECEIVER_LABEL is NULL, instead contruct a nonlocal goto handler.
Sure. Thanks for the review. Updated patch below. As nothing
was changed from the previous post but comments as per the
review (mostly moving / reviving, fixing one grammo), already
covered by the changelog quoted above, the previous testing is
still valid.
Ok for trunk, approvers?
Index: gcc/builtins.c
===
--- gcc/builtins.c(revision 192353)
+++ gcc/builtins.c(working copy)
@@ -885,14 +885,15 @@ expand_builtin_setjmp_setup (rtx buf_add
}
/* Construct the trailing part of a __builtin_setjmp call. This is
- also called directly by the SJLJ exception handling code. */
+ also called directly by the SJLJ exception handling code.
+ If RECEIVER_LABEL is NULL, instead contruct a nonlocal goto handler. */
void
expand_builtin_setjmp_receiver (rtx receiver_label ATTRIBUTE_UNUSED)
{
rtx chain;
- /* Clobber the FP when we get here, so we have to make sure it's
+ /* Mark the FP as used when we get here, so we have to make sure it's
marked as used by this function. */
emit_use (hard_frame_pointer_rtx);
@@ -907,17 +908,28 @@ expand_builtin_setjmp_receiver (rtx rece
#ifdef HAVE_nonlocal_goto
if (! HAVE_nonlocal_goto)
#endif
-{
- emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
- /* This might change the hard frame pointer in ways that aren't
- apparent to early optimization passes, so force a clobber. */
- emit_clobber (hard_frame_pointer_rtx);
-}
+/* First adjust our frame pointer to its actual value. It was
+ previously set to the start of the virtual area corresponding to
+ the stacked variables when we branched here and now needs to be
+ adjusted to the actual hardware fp value.
+
+ Assignments to virtual registers are converted by
+ instantiate_virtual_regs into the corresponding assignment
+ to the underlying register (fp in this case) that makes
+ the original assignment true.
+ So the following insn will actually be decrementing fp by
+ STARTING_FRAME_OFFSET. */
+emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
#if !HARD_FRAME_POINTER_IS_ARG_POINTER
if (fixed_regs[ARG_POINTER_REGNUM])
{
#ifdef ELIMINABLE_REGS
+ /* If the argument pointer can be eliminated in favor of the
+ frame pointer, we don't need to restore it. We assume here
+ that if such an elimination is present, it can always
Committed: skip testsuite/23_containers/bitset/45713.cc for mmix-*-*.
For mmix-knuth-mmixware, MAX_FIXED_MODE_SIZE is the default,
GET_MODE_BITSIZE (DImode), which of course isn't larger than the
size-type, the same size on this 64-bit target. I don't think making
it larger (i.e. TImode) would help: that seems instead likely to
introduce awkward spurious non-host_integerp ()-related code
differences between hosts with/without a 128-bit integer type.
The minor benefit would be to be able to handle objects larger than
1/8 of the (architecturall) address space. Besides, of course,
supporting test-cases like the one below. Committed.
* testsuite/23_containers/bitset/45713.cc: Skip for mmix-*-*.
Tweak sizetype-related comment.
Index: libstdc++-v3/testsuite/23_containers/bitset/45713.cc
===
--- libstdc++-v3/testsuite/23_containers/bitset/45713.cc(revision
192646)
+++ libstdc++-v3/testsuite/23_containers/bitset/45713.cc(working copy)
@@ -16,9 +16,9 @@
// http://www.gnu.org/licenses/.
// The testcase requires bitsizetype to be wider than sizetype,
-// otherwise types/vars with 0x2000 bytes or larger can't be used.
-// See http://gcc.gnu.org/PR54897
-// { dg-do compile { target { ! { avr*-*-* cris*-*-* h8300*-*-* mcore*-*-*
moxie*-*-* } } } }
+// otherwise types/vars with (e.g. for 32-bit sizetype) 0x2000
+// bytes or larger can't be used. See http://gcc.gnu.org/PR54897
+// { dg-do compile { target { ! { avr*-*-* cris*-*-* h8300*-*-* mcore*-*-*
moxie*-*-* mmix-*-* } } } }
#include bitset
brgds, H-P
breakage with [v3] (almost) finish scoped_allocator
From: Jonathan Wakely jwakely@gmail.com Date: Fri, 19 Oct 2012 18:16:51 +0200 This adds support for piecewise construction of std::pair by scoped_allocator_adaptor. The only thing missing from scoped_allocator_adaptor now is that my definition of OUTERMOST isn't recursive so doesn't work for nested scoped_allocator_adaptors. That's a suitably obscure use case that I'm not going to rush to fix it today. * include/std/scoped_allocator (__outermost_alloc_traits): Define. (scoped_allocator_adaptor::destroy): Use it. (scoped_allocator_adaptor::construct): Likewise. Overload for piecewise construction of std::pair objects. * testsuite/20_util/scoped_allocator/2.cc: New. Tested x86_64-linux, committed to trunk. Looks like _U is one of those identifiers that should be avoided: Grep yields: src/newlib/libc/include/ctype.h:#define _U 01 And also: #define _L 02 #define _N 04 #define _S 010 #define _P 020 #define _C 040 #define _X 0100 #define _B 0200 Trunk is broken for cris-elf (a newlib target) as follows, coinciding with the above commit (one in 192612:192621). /tmp/hpautotest-gcc0/cris-elf/gccobj/./gcc/xgcc -shared-libgcc -B/tmp/hpautotest-gcc0/cris-elf/gccobj/./gcc -nostdinc++ -L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/src -L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/src/.libs -nostdinc -B/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc0/gcc/newlib/libc/include -B/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc0/gcc/libgloss/cris -B/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/sys-include-x c++-header -nostdinc++ -g -O2 -I/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/cris-elf -I/tmp/hpautotest-gcc0/cris-elf/ gccobj/cris-elf/libstdc++-v3/include -I/tmp/hpautotest-gcc0/gcc/libstdc++-v3/libsupc++ -O2 -g -std=gnu++0x /tmp/hpautotest-gcc0/gcc/libstdc++-v3/include/precompiled/stdc++.h \ -o cris-elf/bits/stdc++.h.gch/O2ggnu++0x.gch In file included from /tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/cctype:44:0, from /tmp/hpautotest-gcc0/gcc/libstdc++-v3/include/precompiled/stdc++.h:36: /tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/scoped_allocator:368:53: error: expected nested-name-specifier before numeric constant templatetypename _T1, typename _T2, typename _U, typename _V ^ /tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/scoped_allocator:368:53: error: expected '' before numeric constant /tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/scoped_allocator:370:33: error: expected identifier before numeric constant construct(pair_T1, _T2* __p, _U __u, _V __v) ^ (many similar error lines) What's the preferred replacement? brgds, H-P
Committed, MMIX: fix */builtin-apply-2.c for an instrumented setup.
When running the test-suite for these tests on an instrumented mmix simulator (plus gcc and newlib patches to match) that, instead of silently allocating zeros, barfs on accesses beyond the defined stack, these tests fail because of that check. There are no stack parameters to copy, so there's just an unused and undefined (well, zeroed) chunk of memory copied. Fixes in that setup: FAIL: gcc.dg/builtin-apply2.c execution test .. FAIL: gcc.dg/torture/stackalign/builtin-apply-2.c -O0 execution test Committed. gcc/testsuite: * gcc.dg/torture/stackalign/builtin-apply-2.c, gcc.dg/builtin-apply2.c: Correct STACK_ARGUMENTS_SIZE for MMIX. Index: gcc/testsuite/gcc.dg/torture/stackalign/builtin-apply-2.c === --- gcc/testsuite/gcc.dg/torture/stackalign/builtin-apply-2.c (revision 192353) +++ gcc/testsuite/gcc.dg/torture/stackalign/builtin-apply-2.c (working copy) @@ -16,6 +16,9 @@ E, F and G are passed on stack. So the size of the stack argument data is 20. */ #define STACK_ARGUMENTS_SIZE 20 +#elif defined __MMIX__ +/* No parameters on stack for bar. */ +#define STACK_ARGUMENTS_SIZE 0 #else #define STACK_ARGUMENTS_SIZE 64 #endif Index: gcc/testsuite/gcc.dg/builtin-apply2.c === --- gcc/testsuite/gcc.dg/builtin-apply2.c (revision 192353) +++ gcc/testsuite/gcc.dg/builtin-apply2.c (working copy) @@ -17,6 +17,9 @@ E, F and G are passed on stack. So the size of the stack argument data is 20. */ #define STACK_ARGUMENTS_SIZE 20 +#elif defined __MMIX__ +/* No parameters on stack for bar. */ +#define STACK_ARGUMENTS_SIZE 0 #else #define STACK_ARGUMENTS_SIZE 64 #endif brgds, H-P
Re: [RFA:] Fix frame-pointer-clobbering in builtins.c:expand_builtin_setjmp_receiver
On Fri, 12 Oct 2012, Eric Botcazou wrote:
(insn 168 49 51 3 (set (reg/f:DI 253 $253)
(plus:DI (reg/f:DI 253 $253)
(const_int 24 [0x18])))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21
-1 (nil))
(insn 51 168 52 3 (clobber (reg/f:DI 253 $253))
...
Note that insn 168 deleted, which seems a logical optimization. The
bug is to emit the clobber, not that the restoring insn is removed.
Had that worked in the past for MMIX?
Yes, for svn revision 106027 (20051030) 4.1.0-era (!)
http://gcc.gnu.org/ml/gcc-testresults/2005-10/msg01340.html
where the test must have passed, as
gcc.c-torture/execute/built-in-setjmp.c is at least four years
older than that.
If so, what changed recently?
By these days I didn't mean recent, just not eons ago. :)
I see in a gcc-test-results posting from Mike Stein (whom I'd
like to thank for test-results posting over the years), matching
FAILs for svn revision 126095 (20070628) 4.3.0-era
http://gcc.gnu.org/ml/gcc-testresults/2007-06/msg01287.html.
Sorry, I have nothing in between those reports, my bad. Though
I see no point narrowing down the failing revision further here
IMO; as mentioned the bug is not that the restoring insn is
removed.
Agreed. However, I'd suggest rescuing the comment for the ELIMINABLE_REGS
block from expand_nl_goto_receiver as it still sounds valid to me.
Oops, my bad; I see I removed all the good comments. Fixed.
* stmt.c (expand_nl_goto_receiver): Remove almost-copy of
expand_builtin_setjmp_receiver.
(expand_label): Adjust, call expand_builtin_setjmp_receiver
with NULL for the label parameter.
* builtins.c (expand_builtin_setjmp_receiver): Don't clobber
the frame-pointer. Adjust comments.
[HAVE_builtin_setjmp_receiver]: Emit builtin_setjmp_receiver
only if LABEL is non-NULL.
I cannot formally approve, but this looks good to me modulo:
+ If RECEIVER_LABEL is NULL, instead the port-specific parts of a
+ nonlocal goto handler are emitted. */
The port-specific parts wording is a bit confusing I think. I'd just write:
If RECEIVER_LABEL is NULL, instead contruct a nonlocal goto handler.
Sure. Thanks for the review. Updated patch below. As nothing
was changed from the previous post but comments as per the
review (mostly moving / reviving, fixing one grammo), already
covered by the changelog quoted above, the previous testing is
still valid.
Ok for trunk, approvers?
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 192353)
+++ gcc/builtins.c (working copy)
@@ -885,14 +885,15 @@ expand_builtin_setjmp_setup (rtx buf_add
}
/* Construct the trailing part of a __builtin_setjmp call. This is
- also called directly by the SJLJ exception handling code. */
+ also called directly by the SJLJ exception handling code.
+ If RECEIVER_LABEL is NULL, instead contruct a nonlocal goto handler. */
void
expand_builtin_setjmp_receiver (rtx receiver_label ATTRIBUTE_UNUSED)
{
rtx chain;
- /* Clobber the FP when we get here, so we have to make sure it's
+ /* Mark the FP as used when we get here, so we have to make sure it's
marked as used by this function. */
emit_use (hard_frame_pointer_rtx);
@@ -907,17 +908,28 @@ expand_builtin_setjmp_receiver (rtx rece
#ifdef HAVE_nonlocal_goto
if (! HAVE_nonlocal_goto)
#endif
-{
- emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
- /* This might change the hard frame pointer in ways that aren't
-apparent to early optimization passes, so force a clobber. */
- emit_clobber (hard_frame_pointer_rtx);
-}
+/* First adjust our frame pointer to its actual value. It was
+ previously set to the start of the virtual area corresponding to
+ the stacked variables when we branched here and now needs to be
+ adjusted to the actual hardware fp value.
+
+ Assignments to virtual registers are converted by
+ instantiate_virtual_regs into the corresponding assignment
+ to the underlying register (fp in this case) that makes
+ the original assignment true.
+ So the following insn will actually be decrementing fp by
+ STARTING_FRAME_OFFSET. */
+emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
#if !HARD_FRAME_POINTER_IS_ARG_POINTER
if (fixed_regs[ARG_POINTER_REGNUM])
{
#ifdef ELIMINABLE_REGS
+ /* If the argument pointer can be eliminated in favor of the
+frame pointer, we don't need to restore it. We assume here
+that if such an elimination is present, it can always be used.
+This is the case on all known machines; if we don't make this
+assumption, we do unnecessary saving on many machines. */
size_t i;
static const struct elims {const int from, to;} elim_regs[] =
ELIMINABLE_REGS;
@@ -938,7 +950,7 @@
Committed, MMIX: fix INCOMING_REGNO / OUTGOING_REGNO for return-value
Back then, I must've missed that INCOMING_REGNO / OUTGOING_REGNO are
used to map return-value-register/s too. Fixes:
FAIL: gcc.dg/builtin-apply4.c execution test
...
FAIL: gcc.dg/builtin-return-1.c execution test
...
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O0 execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O1 execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O2 execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O3 -fomit-frame-pointer
execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O3 -g execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -Os execution test
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution tes\
t
FAIL: gcc.dg/torture/stackalign/builtin-apply-4.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O0 execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O1 execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O2 execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O3 -fomit-frame-pointer
execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O3 -g execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -Os execution test
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution te\
st
FAIL: gcc.dg/torture/stackalign/builtin-return-1.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects execution test
Committed.
* config/mmix/mmix.c (mmix_opposite_regno): Handle the
return-value register too.
--- gcc/config/mmix/mmix.c.prev 2012-10-09 02:00:51.0 +0200
+++ gcc/config/mmix/mmix.c 2012-10-14 00:59:54.0 +0200
@@ -392,15 +392,33 @@ mmix_conditional_register_usage (void)
/* INCOMING_REGNO and OUTGOING_REGNO worker function.
Those two macros must only be applied to function argument
- registers. FIXME: for their current use in gcc, it'd be better
- with an explicit specific additional FUNCTION_INCOMING_ARG_REGNO_P
- a'la TARGET_FUNCTION_ARG / TARGET_FUNCTION_INCOMING_ARG instead of
+ registers and the function return value register for the opposite
+ use. FIXME: for their current use in gcc, it'd be better with an
+ explicit specific additional FUNCTION_INCOMING_ARG_REGNO_P a'la
+ TARGET_FUNCTION_ARG / TARGET_FUNCTION_INCOMING_ARG instead of
forcing the target to commit to a fixed mapping and for any
- unspecified register use. */
+ unspecified register use. Particularly when thinking about the
+ return-value, it is better to imagine INCOMING_REGNO and
+ OUTGOING_REGNO as named CALLEE_TO_CALLER_REGNO and INNER_REGNO as
+ named CALLER_TO_CALLEE_REGNO because the direction. The incoming
+ and outgoing is from the perspective of the parameter-registers,
+ but the same macro is (must be, lacking an alternative like
+ suggested above) used to map the return-value-register from the
+ same perspective. To make directions even more confusing, the macro
+ MMIX_OUTGOING_RETURN_VALUE_REGNUM holds the number of the register
+ in which to return a value, i.e. INCOMING_REGNO for the return-value-
+ register as received from a called function; the return-value on the
+ way out. */
int
mmix_opposite_regno (int regno, int incoming)
{
+ if (incoming regno == MMIX_OUTGOING_RETURN_VALUE_REGNUM)
+return MMIX_RETURN_VALUE_REGNUM;
+
+ if (!incoming regno == MMIX_RETURN_VALUE_REGNUM)
+return MMIX_OUTGOING_RETURN_VALUE_REGNUM;
+
if (!mmix_function_arg_regno_p (regno, incoming))
return regno;
brgds, H-P
Committed, PR54897 (was: [C++ PATCH] -Wsizeof-pointer-memaccess warning (take 2))
From: Hans-Peter Nilsson h...@axis.com
Date: Thu, 11 Oct 2012 02:13:32 +0200
There's now an excess error:
x/libstdc++-v3/testsuite/23_containers/bitset/45713.cc:24:55:
error: size of array 'test' is not an integral constant-expression
int test[sizeof(std::bitset0x) != 1 ? 1 : -1];
Or is the error message in error and just missing before?
Please have a look.
As per discussion in PR54897, I committed the following, after
correcting the target syntax in the cutnpasted patch there.
(The target parser is picky about matching *-*-*, which I guess is
appropriate since it has to handle effective-targets as well.)
PR testsuite/54897
* testsuite/23_containers/bitset/45713.cc: Skip for avr*-*-*
cris*-*-* h8300*-*-* mcore*-*-* moxie*-*-*.
Index: libstdc++-v3/testsuite/23_containers/bitset/45713.cc
===
--- libstdc++-v3/testsuite/23_containers/bitset/45713.cc(revision
192353)
+++ libstdc++-v3/testsuite/23_containers/bitset/45713.cc(working copy)
@@ -1,4 +1,4 @@
-// Copyright (C) 2010 Free Software Foundation, Inc.
+// Copyright (C) 2010, 2012 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -15,7 +15,10 @@
// with this library; see the file COPYING3. If not see
// http://www.gnu.org/licenses/.
-// { dg-do compile }
+// The testcase requires bitsizetype to be wider than sizetype,
+// otherwise types/vars with 0x2000 bytes or larger can't be used.
+// See http://gcc.gnu.org/PR54897
+// { dg-do compile { target { ! { avr*-*-* cris*-*-* h8300*-*-* mcore*-*-*
moxie*-*-* } } } }
#include bitset
brgds, H-P
[RFA:] Fix frame-pointer-clobbering in builtins.c:expand_builtin_setjmp_receiver
The md.texi entry for nonlocal_goto_receiver says A typical reason
why you might need this pattern is if some value, such as a pointer to
a global table, must be restored when the frame pointer is restored.
Note that a nonlocal goto only occurs within a unit-of-translation, so
a global table pointer that is shared by all functions of a given
module need not be restored. One use would be to restore a
hardware-register-value saved in the current frame; the frame where
__builtin_setjmp is called (i.e. not a global context). This is what
the MMIX port does, saving the register-stack-pointer-register for use
when unwinding the register stack. I can imagine other similar
register-restoring needs that require something saved in the current
frame, but current ports with that pattern (or setjmp_receiver) but
without a nonlocal_goto pattern (see the HAVE_nonlocal_goto condition
in the patch) don't. (The AVR port performs what appears to be a
cargo-cult song and dance; the bug below copied into the port, but
the port will not otherwise be affected by the bug or this patch, as it
has a nonlocal_goto pattern.)
But, in the builtins.c:expand_builtin_setjmp_receiver, the
frame-pointer is *clobbered* for a mysterious and fuddy reason:
/* This might change the hard frame pointer in ways that aren't
apparent to early optimization passes, so force a clobber. */
emit_clobber (hard_frame_pointer_rtx);
That comment might have been true eons ago, but these days clobbering
the frame-pointer means that its value is void and any restoring insns
emitted before the clobber are deleted *because* of the clobber. For
example, in built-in-setjmp.c at -O2. Before .191r.ud_dce (i.e. in
the .190r.init-regs dump):
(code_label/s 47 115 48 3 3 [2 uses])
(note 48 47 49 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 49 48 168 3 (use (reg/f:DI 253 $253))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
(insn 168 49 51 3 (set (reg/f:DI 253 $253)
(plus:DI (reg/f:DI 253 $253)
(const_int 24 [0x18])))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
(insn 51 168 52 3 (clobber (reg/f:DI 253 $253))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
(insn 52 51 54 3 (parallel [
(unspec_volatile [
(reg/f:DI 253 $253)
] 1)
(clobber (scratch:DI))
(clobber (reg:DI 259 rJ))
])
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 63
{*nonlocal_goto_receiver_expanded}
(expr_list:REG_UNUSED (reg:DI 259 rJ)
(nil)))
(insn 54 52 136 3 (asm_input/v () (null):0)
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
After:
(code_label/s 47 115 48 3 3 [2 uses])
(note 48 47 49 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 49 48 51 3 (use (reg/f:DI 253 $253))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
(insn 51 49 52 3 (clobber (reg/f:DI 253 $253))
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
(insn 52 51 54 3 (parallel [
(unspec_volatile [
(reg/f:DI 253 $253)
] 1)
(clobber (scratch:DI))
(clobber (reg:DI 259 rJ))
])
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 63
{*nonlocal_goto_receiver_expanded}
(expr_list:REG_UNUSED (reg:DI 259 rJ)
(nil)))
(insn 54 52 136 3 (asm_input/v () (null):0)
/tmp/mmiximp2/gcc/gcc/testsuite/gcc.c-torture/execute/built-in-setjmp.c:21 -1
(nil))
Note that insn 168 deleted, which seems a logical optimization. The
bug is to emit the clobber, not that the restoring insn is removed.
While grepping around for other emitters of nonlocal_goto_receiver I
noticed that builtins.c:expand_builtin_setjmp_receiver is identical to
stmt.c:expand_nl_goto_receiver save for two things: the frame-pointer
clobbering(!) and that expand_builtin_setjmp_receiver instead prefers to
emit setjmp_receiver. I don't see how the frame-pointer-clobbering
would be needed as part of emitting setjmp_receiver.
I suggest eliminating the bug and one copy of the apparently bug-prone
code. I kept the function in builtins.c for obvious reasons (if not
obvious, consider the name: expand *builtin* setjmp_receiver) with the
setjmp-ness expressed through the label parameter, which is non-NULL
for pre-existing calls. Note also the fixed clobber-comment,
obviously incorrect in the stmt.c almost-copy, and at least on the
wrong line in expand_builtin_setjmp_receiver.
Tested mmix-knuth-mmixware, x86_64-unknown-linux-gnu (for good measure)
and rl78-elf (a SJLJ target; fixed-up with a patch from the maintainer
for the current breakage in PR54882, but without fortran), no regressions.
This fixes the following FAILs for mmix-knuth-mmixware:
Running
Re: libbacktrace patch committed: Trace through shared libraries
On Tue, 9 Oct 2012, Ian Lance Taylor wrote:
It's an incorrect warning from an old version of GCC. Fixed like so.
Bootstrapped and ran libbacktrace testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Another two in libbackend/elf.c, committed as obvious after
build passed the point-of-failure.
* elf.c (elf_add_syminfo_data): Add casts to avoid warning.
Index: libbacktrace/elf.c
===
--- libbacktrace/elf.c (revision 192306)
+++ libbacktrace/elf.c (working copy)
@@ -410,7 +410,7 @@ elf_add_syminfo_data (struct backtrace_s
{
struct elf_syminfo_data **pp;
- for (pp = (struct elf_syminfo_data **) state-syminfo_data;
+ for (pp = (struct elf_syminfo_data **) (void *) state-syminfo_data;
*pp != NULL;
pp = (*pp)-next)
;
@@ -422,7 +422,7 @@ elf_add_syminfo_data (struct backtrace_s
{
struct elf_syminfo_data **pp;
- pp = (struct elf_syminfo_data **) state-syminfo_data;
+ pp = (struct elf_syminfo_data **) (void *) state-syminfo_data;
while (1)
{
brgds, H-P
Re: [C++ PATCH] -Wsizeof-pointer-memaccess warning (take 2)
From: Jakub Jelinek ja...@redhat.com Date: Tue, 2 Oct 2012 14:56:29 +0200 2012-10-02 Jakub Jelinek ja...@redhat.com cp/ * cp-tree.h (SIZEOF_EXPR_TYPE_P): Define. * tree.c (cp_tree_equal): Handle SIZEOF_EXPR with SIZEOF_EXPR_TYPE_P. ...etc. Looks like this caused a regression; PR54897, where the error message seems to indicate an ABI change. There's now an excess error: x/libstdc++-v3/testsuite/23_containers/bitset/45713.cc:24:55: error: size of array 'test' is not an integral constant-expression int test[sizeof(std::bitset0x) != 1 ? 1 : -1]; Or is the error message in error and just missing before? Please have a look. brgds, H-P
Re: PATCH to acinclude.m4 to fix gas version detection
On Thu, 4 Oct 2012, Jason Merrill wrote: Recent versions of binutils seem to have started putting ' around the version number in bfd/configure.in, which was confusing gcc configure. This patch allows us to detect the version number again. OK for trunk? I committed my already-approved patch as r192342 instead (it had a sanity-check for the extracted version and covered ld as well). brgds, H-P
Re: libbacktrace patch committed: Trace through shared libraries
On Tue, 9 Oct 2012, Ian Lance Taylor wrote: Patch bootstrapped and ran libbacktrace and Go testsuites on x86_64-unknown-linux-gnu. Committed to mainline. Trying x86_64 host gcc 4.1.2 (--target=rl78-elf but not the issue) at r192285: /bin/sh ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I/tmp/rl78b/gcc/libbacktrace -I /tmp/rl78b/gcc/li\ bbacktrace/../include -I /tmp/rl78b/gcc/libbacktrace/../libgcc -I ../libgcc -I ../gcc/include -I ../../gcc/include -fu\ nwind-tables -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-\ attribute -Wcast-qual -Werror -g -O2 -c -o dwarf.lo /tmp/rl78b/gcc/libbacktrace/dwarf.c libtool: compile: gcc -DHAVE_CONFIG_H -I. -I/tmp/rl78b/gcc/libbacktrace -I /tmp/rl78b/gcc/libbacktrace/../include -I /\ tmp/rl78b/gcc/libbacktrace/../libgcc -I ../libgcc -I ../gcc/include -I ../../gcc/include -funwind-tables -W -Wall -Wwri\ te-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -Werr\ or -g -O2 -c /tmp/rl78b/gcc/libbacktrace/dwarf.c -o dwarf.o cc1: warnings being treated as errors /tmp/rl78b/gcc/libbacktrace/dwarf.c: In function 'dwarf_fileline': /tmp/rl78b/gcc/libbacktrace/dwarf.c:2766: warning: dereferencing type-punned pointer will break strict-aliasing rules /tmp/rl78b/gcc/libbacktrace/dwarf.c: In function 'backtrace_dwarf_add': /tmp/rl78b/gcc/libbacktrace/dwarf.c:2887: warning: dereferencing type-punned pointer will break strict-aliasing rules /tmp/rl78b/gcc/libbacktrace/dwarf.c:2899: warning: dereferencing type-punned pointer will break strict-aliasing rules make[3]: *** [dwarf.lo] Error 1 brgds, H-P
Build failure with [PATCH] PR 53528 c++/ C++11 Generalized Attribute support
From: Dodji Seketeli do...@redhat.com Date: Mon, 8 Oct 2012 14:12:04 +0200 Jason Merrill ja...@redhat.com writes: OK. Thanks. Committed to trunk at revision r192199. This caused a build failure, see PR54860. brgds, H-P
Breakage with [v3] patch, configuring GCC --disable-libgomp causes libstdc++ abi check to fail.
MIME-Version: 1.0 From: Benjamin De Kosnik b...@redhat.com Date: Fri, 28 Sep 2012 22:02:46 +0200 There's no reason for this unintentional ABI breakage with this configure flag. 2012-09-28 Benjamin Kosnik b...@redhat.com * acinclude.m4 (GLIBCXX_ENABLE_PARALLEL): Remove ENABLE_PARALLEL. * include/Makefile.am: Same. * src/c++98/Makefile.am: Same. * src/Makefile.am: Same. * Makefile.in: Regenerated. * aclocal.m4: Same. * configure: Same. * doc/Makefile.in: Same. * include/Makefile.in: Same. * libsupc++/Makefile.in: Same. * po/Makefile.in: Same. * python/Makefile.in: Same. * src/Makefile.in: Same. * testsuite/Makefile.in: Same. * src/c++11/Makefile.in: Same. * src/c++98/Makefile.in: Same. Something went wrong and the target compiler is now wrongly assumed to unconditionally understand -pthread. For cris-elf: libtool: compile: /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -shared-libgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc -nostdinc++ -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libstdc++-v3/src -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libstdc++-v3/src/.libs -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -I/tmp/hpautotest-gcc1/gcc/libstdc++-v3/../libgcc -I/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libstdc++-v3/include/cris-elf -I/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libstdc++-v3/include -I/tmp/hpautotest-gcc1/gcc/libstdc++-v3/libsupc++ -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=parallel_settings.lo -g -O2 -fopenmp -D_GLIBCXX_PARALLEL -I/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libstdc++-v3/../libgomp -c /tmp/hpautotest-gcc1/gcc/libstdc++-v3/src/c++98/parallel_settings.cc -o parallel_settings.o xgcc: error: unrecognized command line option '-pthread' brgds, H-P
Re: [patch] PR54645 move location_adhoc_data map into GC
On Fri, 21 Sep 2012, Dehao Chen wrote: This patch moves location_adhoc_data into GC, and also rebuild the hash table when reading in the PCH. After the patch, PCH can work as expected. Bootstrapped and passed gcc regression tests on x8664_linux. If you have a moment to consider improvements for the test-instructions at http://gcc.gnu.org/contribute.html#testing to try and avoid this situation (introducing regressions), that'd be nice; IIUC it wasn't clear enough that the make check must be at the top tree? It seems to say so but maybe that's somehow in a blind spot. OK for trunk? Thanks, Dehao libcpp/ChangeLog: 2012-09-21 Dehao Chen de...@google.com PR middle-end/54645 * include/line-map.h (location_adhoc_data): Move location_adhoc_data into GC. (location_adhoc_data_map): Likewise. (line_maps): Likewise. (rebuild_location_adhoc_htab): New Function. * line-map.c (+rebuild_location_adhoc_htab): new Funcion. (get_combined_adhoc_loc): Move location_adhoc_data into GC. (location_adhoc_data_fini): Likewise. (linemap_init): Likewise. (location_adhoc_data_init): Remove Function. gcc/ChangeLog: 2012-09-21 Dehao Chen de...@google.com PR middle-end/54645 * c-family/c-pch.c (c_common_read_pch): Rebuild the location_adhoc_data map when read in the pch. I can't say anything insightful about this patch other than the nitpick below (i.e. I see nothing wrong with it) but I'd encourage a proper review of it to resolve the PCH regressions. There's no PCH section in MAINTAINERS, but next-of-kin global maintainers CC:ed. I have just a nitpicking remark: please break the overlong lines. I noticed the ones with htab_create calls as my viewer helpfully broke the lines at 80 columns at the last comma. brgds, H-P
Re: [Patch] PR54635: Add addr_space_t argument to mode_dependent_address_p
On Thu, 20 Sep 2012, Georg-Johann Lay wrote: mode_dependent_address_p is not sensitive to the address space of the passed address. Thus, add an addr_space_t parameter to the hook. Borderline obvious. :) (J/k; the added functionality seems obvious but the implementation may have non-obvious issues, though I saw none.) One more question: This patch changes a target hook. Is there a place in the internals that gathers changes by naming *all* the changed/extended/removed/poisoned interfaces? So that developers can read the list of functions/macros/hooks to learn if something interesting happened as they update the GCC sources? Someone once suggested an internal-changes web-page or file for the benefit of out-of-tree ports and patches, but I don't know what happened to that. I'd look in the wiki. Diff'ing the internals texi is tedious and too much noise as it would be helpful to get a briefing. Johann PR 54635 Better spell that PR other/54635 in the ChangeLog; if you don't get the syntax right it won't automatically show up in bugzilla. * doc/tm.texi.in (TARGET_MODE_DEPENDENT_ADDRESS_P): Document new parameter addrspace. ... brgds, H-P
Re: [Patch] PR54635: Add addr_space_t argument to mode_dependent_address_p
On Thu, 20 Sep 2012, Georg-Johann Lay wrote: Hans-Peter Nilsson wrote: Georg-Johann Lay wrote: mode_dependent_address_p is not sensitive to the address space of the passed address. Thus, add an addr_space_t parameter to the hook. Borderline obvious. :) (J/k; the added functionality seems obvious but the implementation may have non-obvious issues, though I saw none.) What specifically? Specifically none; nothing. (Not a typo for one. :) Being unsure about the component, I chose other. Is middle-end better? Since this is the back-end API, i think your other fits just as good or better. Just IMHO. AFAIK PR12345, PR 12345 and PR component/12345 entries are recognized and connected to bugzilla PRs? If you're sure it gets through to bugzilla, then I certainly don't care. brgds, H-P
Re: [committed] Fix g++.dg/debug/dwarf2/nested-3.C to handle hppa assembler comment
From: John David Anglin d...@hiauly1.hia.nrc.ca
Date: Sun, 16 Sep 2012 19:13:19 +0200
This adjusts the regexp to work on hppa. Tested on hppa2.0w-hp-hpux11.11
and hppa64-hp-hpux11.11. Committed to trunk.
2012-09-16 John David Anglin dave.ang...@nrc-cnrc.gc.ca
PR debug/54460
* g++.dg/debug/dwarf2/nested-3.C: Add hppa assembler comment character
to scary regexp.
(Changing \[^#/!|@\]*\[#/!|@\]
into \[^#;/!|@\]*\[#;/!|@\])
The pa/pa.h ASM_COMMENT_START is ;.
This caused a regression for cris-elf which is apparently one of
the few targets to use the *default* value of ;# for
ASM_COMMENT_START. A very portable sequence, by the way, too
bad almost all other targets define their own. FWIW, the port
most of you'd want to look up has #.
I committed the following, adding a + after the set of matched
comment characters to match any repetition and combination of
them. The sibling test-case, nested-2.C, had a similar flaw.
* g++.dg/debug/dwarf2/nested-3.C: Match a sequence
of asm-comment characters instead of a single one.
* g++.dg/debug/dwarf2/nested-2.C: Similar.
--- g++.dg/debug/dwarf2/nested-3.C (revision 191314)
+++ g++.dg/debug/dwarf2/nested-3.C (working copy)
@@ -59,4 +59,4 @@
//
// Hence the scary regexp:
//
-// { dg-final { scan-assembler \[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\)
DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*\thread\[\^\n\r]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\\(DIE
\\(0x(\[0-9a-f\]+)\\)
DW_TAG_class_type\\)(\[\n\r\]+\[^\n\r\]*)+\Executor\[^\n\r\]+\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*DW_AT_signature\[^#;/!|@\]*\[#;/!|@\]
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_subprogram\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\CurrentExecutor\[^\n\r\]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*end
of children of DIE 0x\\3\[\n\r]+\[^\n\r\]*end of children of DIE
0x\\1\[\n\r]+ } }
+// { dg-final { scan-assembler \[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\)
DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*\thread\[\^\n\r]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\\(DIE
\\(0x(\[0-9a-f\]+)\\)
DW_TAG_class_type\\)(\[\n\r\]+\[^\n\r\]*)+\Executor\[^\n\r\]+\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*DW_AT_signature\[^#;/!|@\]*\[#;/!|@\]+
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_subprogram\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\CurrentExecutor\[^\n\r\]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*end
of children of DIE 0x\\3\[\n\r]+\[^\n\r\]*end of children of DIE
0x\\1\[\n\r]+ } }
--- g++.dg/debug/dwarf2/nested-2.C (revision 191455)
+++ g++.dg/debug/dwarf2/nested-2.C (working copy)
@@ -32,6 +32,6 @@ We want to express that the DIE of S::T
Hence the slightly odd regexp:
- { dg-final { scan-assembler
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_structure_type\\)\[\n\r\]+\[^\n\r\]*\S0\\[
\t\]+\(\[@|#;!\]|//?\)\[
\t\]+DW_AT_name\[\n\r\]+\(.*\)?\\(DIE\[^\n\r\]*DW_TAG_structure_type\\)\[\n\r\]+\[^\n\r\]*\Tint0\\[
\t\]+\(.*\)?\\(DIE\[^\n\r\]*DW_TAG_template_type_param\\)\[\n\r\]+\[^\n\r\]*\[\n\r\]+\[^\n\r\]*\[\n\r\]+\[^\n\r\]*\(\[@|#;!\]|//?\)\[
\t\]+end of children of DIE\[^\n\r\]*\[\n\r\]+\[^\n\r\]*end of children of
DIE\[^\n\r\]* } }
+ { dg-final { scan-assembler
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_structure_type\\)\[\n\r\]+\[^\n\r\]*\S0\\[
\t\]+\(\[@|#;!\]+|//?\)\[
\t\]+DW_AT_name\[\n\r\]+\(.*\)?\\(DIE\[^\n\r\]*DW_TAG_structure_type\\)\[\n\r\]+\[^\n\r\]*\Tint0\\[
\t\]+\(.*\)?\\(DIE\[^\n\r\]*DW_TAG_template_type_param\\)\[\n\r\]+\[^\n\r\]*\[\n\r\]+\[^\n\r\]*\[\n\r\]+\[^\n\r\]*\(\[@|#;!\]+|//?\)\[
\t\]+end of children of DIE\[^\n\r\]*\[\n\r\]+\[^\n\r\]*end of children of
DIE\[^\n\r\]* } }
*/
brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Wed, 12 Sep 2012, Hans-Peter Nilsson wrote: On Wed, 12 Sep 2012, Nathan Froyd wrote: - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. ISTR without this change, there were lots of assembly changes like: I'll try with your original patch and see it I can spot something. Nope, I see no differences in the generated code before/after the patch-patch below (applied to your original patch, except edited as if using --no-prefix, to fit with my other patches). Case closed: I don't think gen* mishandled neither construct. --- patch.nathanorig.adjusted 2012-09-12 12:33:34.0 +0200 +++ patch3 2012-09-14 14:42:31.0 +0200 @@ -364,7 +364,7 @@ diff --git a/gcc/config/mmix/predicates. index b5773b8..7fa3bf1 100644 --- gcc/config/mmix/predicates.md +++ gcc/config/mmix/predicates.md -@@ -149,7 +149,13 @@ +@@ -149,7 +149,14 @@ ;; True if this is a register or an int 0..255. (define_predicate mmix_reg_or_8bit_operand @@ -372,9 +372,10 @@ index b5773b8..7fa3bf1 100644 - (match_operand 0 register_operand) - (and (match_code const_int) - (match_test CONST_OK_FOR_LETTER_P (INTVAL (op), 'I') -+ (if_then_else (match_code const_int) -+(match_test satisfies_constraint_I (op)) -+(match_operand 0 register_operand))) ++ (ior ++ (match_operand 0 register_operand) ++ (and (match_code const_int) ++ (match_test satisfies_constraint_I (op) + +;; True if this is a memory address, possibly strictly. + brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Wed, 12 Sep 2012, Nathan Froyd wrote: - Original Message - Nathan, again thanks. There are a few minor tweaks compared to your version: Thanks for fixing this up! - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. ISTR without this change, there were lots of assembly changes like: set rx, 6 cmp rz, ry, rx instead of the previous and better: cmp rz, ry, 6 (apologies if the assembly syntax isn't correct; hopefully the intent is clear) Yes. My only guess is a typo in your previous edit, as the two constructs certainly should be equivalent in *what's* being recognized. If they aren't, we have a bug in the gen* machinery. If my construct is wrong, then that's a separate bug-fix, ehm. Seems worth it to double-check, not just for the sake of MMIX. but if you double-checked that the assembly didn't change after your changes, maybe something else that you tweaked fixed this. I have no clue; nothing in the patch below stands out - the missing I-constraint that may explain this (modulo that H wasn't recognizable either) was on an or operation, not a compare. But maybe one was close enough that it mattered. What revision was your baseline? My baseline was r190260 but configure-patches as posted (required to build, affecting crtstuff at most) and with: I'll try with your original patch and see it I can spot something. Index: gcc/config/mmix/predicates.md === --- gcc/config/mmix/predicates.md (revision 190682) +++ gcc/config/mmix/predicates.md (working copy) @@ -118,7 +118,7 @@ (define_predicate mmix_symbolic_or_addr return 1; /* Fall through. */ default: - return address_operand (op, mode); + return mmix_extra_constraint (op, 'U', reload_in_progress || reload_completed); } }) Index: gcc/config/mmix/mmix.md === --- gcc/config/mmix/mmix.md (revision 190682) +++ gcc/config/mmix/mmix.md (working copy) @@ -274,7 +275,7 @@ (define_insn anddi3 (define_insn iordi3 [(set (match_operand:DI 0 register_operand =r,r) (ior:DI (match_operand:DI 1 register_operand %r,0) - (match_operand:DI 2 mmix_reg_or_constant_operand rH,LS)))] + (match_operand:DI 2 mmix_reg_or_constant_operand rI,LS)))] @ OR %0,%1,%2 - Replacing undefined-constraint-H with I instead of removing it. It was either renamed early or a genuine typo. Good catch. Hard not to see it; the gen* machinery complains about undefined constraints. :) Exactly! :) You don't congratulate the machine, but the machinist - and sometimes the inventor of the machine. (Thank you, Zack!) brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Fri, 31 Aug 2012, Hans-Peter Nilsson wrote: On Fri, 3 Aug 2012, Hans-Peter Nilsson wrote: Sorry, but I need more time to get test-results for a recent trunk revision to a state where testing patches makes sense. I'll test the patch and commit that or a variant. Thanks for your work. Nathan, again thanks. There are a few minor tweaks compared to your version: - U is a define_address_constraint, not plain define_constraint. - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. - Replacing undefined-constraint-H with I instead of removing it. It was either renamed early or a genuine typo. Good catch. - Carrying over old comments at R constraint. Marking supposedly redundant constraints with FIXME:s. - Removing the now-unused MMIX_REG_OK_STRICT macros. - Using mmix_address_operand instead of address_operand in mmix_symbolic_or_address_operand. - Missing CL entry for mmix_emit_sp_add. You thought nobody reads them? ;) The redundant constraints and attempts to match const_double for integer scalars is an artefact from olden times when large integers (64 bits!) had to be modelled as const_doubles. A clean-up for later. Tested cross to mmix-knuth-mmixware and verified that the newlib libc and libm, libgcc and libstdc++ were the same after this patch as before (modulo a patch to fix H-I and the equivalent of the new mmix_address_operand in mmix_symbolic_or_address_operand). Committed. * config/mmix/mmix.h (MMIX_REG_OK_STRICT): Delete. (REG_CLASS_FROM_LETTER, CONST_OK_FOR_LETTER_P): Delete. (CONST_DOUBLE_OK_FOR_LETTER_P, EXTRA_CONSTRAINT): Delete. * config/mmix/mmix-protos.h (mmix_intval): Declare. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. * config/mmix/constraints.md: New file. * config/mmix/mmix.md: Include it. (iordi3): Fix typo; use I instead of undefined H constraint. (*call_real): Update comment about not using the p constraint. * config/mmix/predicates.md (mmix_reg_or_8bit_operand): Use satisfies_constraint_I. (mmix_address_operand): New predicate. (mmix_symbolic_or_address_operand): Use it instead of address_operand. * config/mmix/mmix.c: #include tm-constrs.h. (mmix_intval): Delete declaration. Make non-static. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. (mmix_legitimate_address_p): Use satisfies_constraint_I. (mmix_print_operand_address): Likewise. (mmix_emit_sp_add): Adjust to use insn_const_int_ok_for_constraint when matching L constraint. Index: gcc/config/mmix/mmix.h === --- gcc/config/mmix/mmix.h (revision 190682) +++ gcc/config/mmix/mmix.h (working copy) @@ -72,12 +72,6 @@ along with GCC; see the file COPYING3. untouched by the epilogue. */ #define MMIX_EH_RETURN_STACKADJ_REGNUM MMIX_STATIC_CHAIN_REGNUM -#ifdef REG_OK_STRICT -# define MMIX_REG_OK_STRICT 1 -#else -# define MMIX_REG_OK_STRICT 0 -#endif - #define MMIX_FUNCTION_ARG_SIZE(MODE, TYPE) \ ((MODE) != BLKmode ? GET_MODE_SIZE (MODE) : int_size_in_bytes (TYPE)) @@ -439,11 +433,6 @@ enum reg_class #define INDEX_REG_CLASS GENERAL_REGS -#define REG_CLASS_FROM_LETTER(CHAR)\ - ((CHAR) == 'x' ? SYSTEM_REGS \ - : (CHAR) == 'y' ? REMAINDER_REG \ - : (CHAR) == 'z' ? HIMULT_REG : NO_REGS) - #define REGNO_OK_FOR_BASE_P(REGNO) \ ((REGNO) = MMIX_LAST_GENERAL_REGISTER\ || (REGNO) == MMIX_ARG_POINTER_REGNUM\ @@ -460,16 +449,6 @@ enum reg_class #define CLASS_MAX_NREGS(CLASS, MODE) HARD_REGNO_NREGS (CLASS, MODE) -#define CONST_OK_FOR_LETTER_P(VALUE, C)\ - mmix_const_ok_for_letter_p (VALUE, C) - -#define EXTRA_CONSTRAINT(VALUE, C) \ - mmix_extra_constraint (VALUE, C, MMIX_REG_OK_STRICT) - -/* Do we need anything serious here? Yes, any FLOT constant. */ -#define CONST_DOUBLE_OK_FOR_LETTER_P(VALUE, C) \ - mmix_const_double_ok_for_letter_p (VALUE, C) - /* Node: Frame Layout */ Index: gcc/config/mmix/predicates.md === --- gcc/config/mmix/predicates.md (revision 190682) +++ gcc/config/mmix/predicates.md (working copy) @@ -118,7 +118,7 @@ (define_predicate mmix_symbolic_or_addr return 1; /* Fall through. */ default: - return address_operand (op, mode); + return mmix_address_operand (op, mode); } }) @@ -152,4 +152,12 @@ (define_predicate mmix_reg_or_8bit_oper (ior (match_operand 0 register_operand) (and (match_code const_int) - (match_test CONST_OK_FOR_LETTER_P (INTVAL (op
Re: [PATCH] Add -fmem-report-wpa
On Thu, 6 Sep 2012, Andi Kleen wrote: From: Andi Kleen a...@linux.intel.com Passed bootstrap and testsuite on x86_64. +++ b/gcc/lto/lto.c @@ -2016,6 +2016,8 @@ do_whole_program_analysis (void) /* Show the LTO report before launching LTRANS. */ if (flag_lto_report) print_lto_report (); + if (mem_report_wpa) +dump_mem_report (); } Broke build for cris-elf: g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -W -Wall -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -Ilto -I/tmp/hpautotest-gcc0/gcc/gcc -I/tmp/hpautotest-gcc0/gcc/gcc/lto -I/tmp/hpautotest-gcc0/gcc/gcc/../include -I/tmp/hpautotest-gcc0/gcc/gcc/../libcpp/include -I/tmp/hpautotest-gcc0/cris-elf/gccobj/./gmp -I/tmp/hpautotest-gcc0/gcc/gmp -I/tmp/hpautotest-gcc0/cris-elf/gccobj/./mpfr -I/tmp/hpautotest-gcc0/gcc/mpfr -I/tmp/hpautotest-gcc0/gcc/mpc/src -I/tmp/hpautotest-gcc0/gcc/gcc/../libdecnumber -I/tmp/hpautotest-gcc0/gcc/gcc/../libdecnumber/dpd -I../libdecnumber/tmp/hpautotest-gcc0/gcc/gcc/lto/lto.c -o lto/lto.o /tmp/hpautotest-gcc0/gcc/gcc/lto/lto.c: In function 'void do_whole_program_analysis()': /tmp/hpautotest-gcc0/gcc/gcc/lto/lto.c:2020: error: 'dump_mem_report' was not declared in this scope make[2]: *** [lto/lto.o] Error 1 brgds, H-P
Re: [middle-end] Add machine_mode to address_cost target hook
Oleg Endo wrote: On Thu, 2012-09-06 at 14:41 +0200, Georg-Johann Lay wrote: The change is definitely in the right direction, but I wonder how it helps to fix code bloats of 300%-400% as in PR52543? I'm not familiar with the AVR parts. BTW, There was a small change in lower-subreg which required some adaptations in targets: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00425.html See also gcc/config/sh/sh.c (sh_rtx_costs): case SET: ... Not sure whether it helps in your case. Those kinds of changes are not required anymore (for most targets), the fallback rtx_cost was amended to more sanely take the mode-size into account, and to handle SET. Whether that is related to PR52543, I don't know. brgds, H-P
Re: Fold VEC_PERM_EXPR a little more
On Sat, 1 Sep 2012, Marc Glisse wrote: gcc/ * fold-const.c (fold_ternary_loc): Constant-propagate after removing dead operands. gcc/testsuite/ * gcc.dg/fold-perm.c: Improve test. (adding a line and a parameter to the function containing the test-code) JFTR: generally speaking, editing existing tests is frowned upon. Adding a new separate test is almost always better. Not sure there's reason for an exception this time, but I see you're the author of the original test and it's 2 weeks since it was added. brgds, H-P
ping [RFA:] fix configury version checks for in-tree binutils
Ping with CC to build maintainers. (Further pings, if any are needed, will be with URL only.) On Sun, 26 Aug 2012, Hans-Peter Nilsson wrote: Found while investigating PR54373. A combined tree (in-tree binutils) using binutils post-2.22 is semi-broken at the moment: the version of the assembler and linker can't be found. The configury doesn't expect the single-quote that has appeared; i.e. VERSION=2.22.0 vs. VERSION='2.23.51' You see this in the build log as lots of: checking assembler flags... checking assembler for .balign and .p2align... /x/combined/gcc/configure: line 21848: test: -ge: unary operator expected no checking assembler for .p2align with maximum skip... /x/combined/gcc/configure: line 21884: test: -ge: unary operator expected no obviously causing all optional features in the assembler and linker to be turned off. (This could have been the cause of not seeing PR54373 in a combined tree, but that's actually due to a combination of two other bugs.) Here's a patch to fix in-tree binutils and to sanity-check that the version is extracted to avoid semi-silent failures like the above. An alternative would be to scrap the special-case and just go with out-of-tree feature tests; it seems few enough people use it, that this has gone undetected for a while. I don't know how important this is to canadian-cross support though (where you can't run the assembler and linker to test features). I tested this by building a combined tree for mmix-knuth-mmixware with top-of-tree binutils as well as binutils-2.22 from CVS (see quotes above) past the point of failure. I also built gcc with out-of-tree binutils past the point-of-failure, as well as versions of the patch in-tree and out-tree with just the error check, to verify that it works (is not executed, for out-of-tree binutils). I don't need to tell build maintainers how much of a problem it is matching a single-quote character portably; I just went for any non-numeric/identifier character. I also don't need to tell build maintainers that ? and + are unportables in sed regexps. The changequote calls in the ld version check in configure.ac, are necessary to avoid autoconf terminating with a too-deep-recursion message for the AC_MSG_ERROR call, which can't be helped by quoting the message (duh). Can be alternatively fixed by re/moving the other/outer pair of changequotes visible in the context of this patch and to add corresponding [] where needed. Too much fun for me. N.B. combining CVS binutils-2.20 in-tree is broken due to mismatching libtool versions. I did not test CVS binutils-2.21. Ok to commit? I'd suggest applying this to 4.7 as well. Ok there after test? gcc: * acinclude.m4 (_gcc_COMPUTE_GAS_VERSION): Allow a single character to quote the VERSION= contents. Sanity-check contents. * configure.ac (what linker to use ld version extraction): Ditto. * configure: Regenerate. Index: gcc/acinclude.m4 === --- gcc/acinclude.m4 (revision 190682) +++ gcc/acinclude.m4 (working copy) @@ -393,11 +393,15 @@ for f in $gcc_cv_as_bfd_srcdir/configure $gcc_cv_as_gas_srcdir/configure \ $gcc_cv_as_gas_srcdir/configure.in \ $gcc_cv_as_gas_srcdir/Makefile.in ; do - gcc_cv_gas_version=`sed -n -e 's/^[[ ]]*\(VERSION=[[0-9]]*\.[[0-9]]*.*\)/\1/p' $f` + gcc_cv_gas_version=`sed -n -e 's/^[[ ]]*VERSION=[[^0-9A-Za-z_]]*\([[0-9]]*\.[[0-9]]*.*\)/VERSION=\1/p' $f` if test x$gcc_cv_gas_version != x; then break fi done +case $gcc_cv_gas_version in + VERSION=[[0-9]]*) ;; + *) AC_MSG_ERROR([[cannot find version of in-tree assembler]]);; +esac gcc_cv_gas_major_version=`expr $gcc_cv_gas_version : VERSION=\([[0-9]]*\)` gcc_cv_gas_minor_version=`expr $gcc_cv_gas_version : VERSION=[[0-9]]*\.\([[0-9]]*\)` gcc_cv_gas_patch_version=`expr $gcc_cv_gas_version : VERSION=[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)` Index: gcc/configure.ac === --- gcc/configure.ac (revision 190682) +++ gcc/configure.ac (working copy) @@ -2030,11 +2030,17 @@ if test $gcc_cv_ld = ../ld/ld-new$buil for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in do changequote(,)dnl - gcc_cv_gld_version=`sed -n -e 's/^[ ]*\(VERSION=[0-9]*\.[0-9]*.*\)/\1/p' $f` + gcc_cv_gld_version=`sed -n -e 's/^[ ]*VERSION=[^0-9A-Za-z_]*\([0-9]*\.[0-9]*.*\)/VERSION=\1/p' $f` if test x$gcc_cv_gld_version != x; then break fi done + case $gcc_cv_gld_version in + VERSION=[0-9]*) ;; +changequote([,])dnl + *) AC_MSG_ERROR([[cannot find version of in-tree linker]]) ;; +changequote(,)dnl + esac
Re: [PATCH,mmix] convert to constraints.md
On Fri, 3 Aug 2012, Hans-Peter Nilsson wrote: On Thu, 2 Aug 2012, Nathan Froyd wrote: H-P, if you'd like to test beforehand, that'd be great. ...yes, I'd like to test this before, please. Thanks for your work. (Let's set a timeout at the end of August; ok to commit Sep 1 have I not got back to you.) This is that call. Sorry, but I need more time to get test-results for a recent trunk revision to a state where testing patches makes sense. I'll test the patch and commit that or a variant. Thanks for your work. If you have general patches depending on this, e.g. that gets rid of the old constraint methods, don't let this hold you back. brgds, H-P
Re: out-of-line and arch-specific random_device
From: Ulrich Drepper drep...@gmail.com
Date: Tue, 28 Aug 2012 05:57:08 +0200
This patch (commit r190787) broke build for non-_GLIBCXX_USE_RANDOM_TR1
targets. (See libstdc++-v3/configure.ac and its crossconfig.m4 for a
list.)
Index: libstdc++-v3/include/bits/random.h
===
--- libstdc++-v3/include/bits/random.h(revision 190713)
+++ libstdc++-v3/include/bits/random.h(working copy)
@@ -1575,40 +1575,20 @@
#ifdef _GLIBCXX_USE_RANDOM_TR1
explicit
-random_device(const std::string __token = /dev/urandom)
+random_device(const std::string __token = default)
{
- if ((__token != /dev/urandom __token != /dev/random)
- || !(_M_file = std::fopen(__token.c_str(), rb)))
- std::__throw_runtime_error(__N(random_device::
-random_device(const std::string)));
+ _M_init(__token);
}
~random_device()
-{ std::fclose(_M_file); }
+{ _M_fini(); }
#else
explicit
random_device(const std::string __token = mt19937)
-: _M_mt(_M_strtoul(__token)) { }
+{ return _M_init_pretr1(__token); }
make[4]: Entering directory
`/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include'
mkdir -p ./cris-elf/bits/stdc++.h.gch
/tmp/hpautotest-gcc0/cris-elf/gccobj/./gcc/xgcc -shared-libgcc
-B/tmp/hpautotest-gcc0/cris-elf/gccobj/./gcc -nostdinc++
-L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/src
-L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/src/.libs
-nostdinc -B/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/newlib/ -isystem
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem
/tmp/hpautotest-gcc0/gcc/newlib/libc/include
-B/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libgloss/cris
-L/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libgloss/libnosys
-L/tmp/hpautotest-gcc0/gcc/libgloss/cris
-B/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/bin/
-B/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/lib/ -isystem
/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/include -isystem
/tmp/hpautotest-gcc0/cris-elf/pre/cris-elf/sys-include-x c++-header
-nostdinc++ -g -O2
-I/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/cris-elf
-I/tmp/hpautotest-gcc0/cris-elf/
gccobj/cris-elf/libstdc++-v3/include
-I/tmp/hpautotest-gcc0/gcc/libstdc++-v3/libsupc++ -O2 -g -std=gnu++0x
/tmp/hpautotest-gcc0/gcc/libstdc++-v3/include/precompiled/stdc++.h \
-o cris-elf/bits/stdc++.h.gch/O2ggnu++0x.gch
In file included from
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/random:50:0,
from
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/bits/stl_algo.h:67,
from
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/algorithm:63,
from
/tmp/hpautotest-gcc0/gcc/libstdc++-v3/include/precompiled/stdc++.h:65:
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/bits/random.h:
In constructor 'std::random_device::random_device(const string)':
/tmp/hpautotest-gcc0/cris-elf/gccobj/cris-elf/libstdc++-v3/include/bits/random.h:1590:36:
error: returning a value from a constructor
{ return _M_init_pretr1(__token); }
^
make[4]: *** [cris-elf/bits/stdc++.h.gch/O2ggnu++0x.gch] Error 1
brgds, H-P
Re: [google/gcc-4_7, trunk] Fix problem with -fdebug-types-section and template instantiations, take 2
On Tue, 28 Aug 2012, Cary Coutant wrote: 2012-08-28 Cary Coutant ccout...@google.com * gcc/dwarf2out.c (clone_tree_partial): Remove. (copy_decls_walk): Don't copy children of a declaration into a type unit. Can't say anything about the patch but watch out with the changelog entry: put it in the gcc/ChangeLog but without the gcc/ prefix. brgds, H-P
Re: [PATCH] rs6000: Add a builtin to read the time base register on PowerPC
On Wed, 29 Aug 2012, Segher Boessenkool wrote:
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-get-timebase.c
@@ -0,0 +1,22 @@
+/* { dg-do run { target { powerpc*-*-* } } } */
+
+/* Test if __builtin_ppc_get_timebase() is compatible with the current
+ processor and if it's changing between reads. A read failure might
indicate
+ a Power ISA or binutils change. */
+
+#include inttypes.h
+
+int
+main(void)
+{
+ uint64_t t1, t2, t3;
+
+ t1 = __builtin_ppc_get_timebase ();
+ t2 = __builtin_ppc_get_timebase ();
+ t3 = __builtin_ppc_get_timebase ();
+
+ if (t1 != t2 t1 != t3 t2 != t3)
+return 0;
+
+ return 1;
+}
On some systems the timebase runs at a rather low frequency, say 20MHz.
This test will spuriously fail there. Waste a million CPU cycles before
reading TB the second time?
Waste said million cycles portably by calling sched_yield()?
(Available only on POSIX systems. :)
brgds, H-P
Re: [PATCH] rs6000: Add a builtin to read the time base register on PowerPC
On Wed, 29 Aug 2012, Michael Meissner wrote: On Wed, Aug 29, 2012 at 01:56:05PM -0400, Hans-Peter Nilsson wrote: On Wed, 29 Aug 2012, Segher Boessenkool wrote: On some systems the timebase runs at a rather low frequency, say 20MHz. This test will spuriously fail there. Waste a million CPU cycles before reading TB the second time? Waste said million cycles portably by calling sched_yield()? (Available only on POSIX systems. :) Well only for a test environment. You don't want to call sched_yield in the normal case, since the apps that do this many millions of times need this to be as a fast as possible. Surely, but IMHO what goes for the normal case is not a valid reading of waste...millions of cycles. ;) Point being, for simulator environments, you may not want the loop that was suggested later. On the other hand, that might not be an observable period, either. brgds, H-P
Re: [MIPS, committed] Add missing COSTS_N_INSNS call.
On Wed, 29 Aug 2012, Richard Sandiford wrote:
Richard Sandiford rdsandif...@googlemail.com writes:
I'm testing a patch to make the testsuite work out the default
-m{no,}synci, which ought to be enough. The usual rules should
then kick in and force -mno-synci where necessary. Hopefully.
Here's the patch.
Index: gcc/testsuite/gcc.target/mips/mips.exp
===
--- gcc/testsuite/gcc.target/mips/mips.exp2012-08-27 17:27:13.0
+0100
+++ gcc/testsuite/gcc.target/mips/mips.exp2012-08-29 19:50:50.141982450
+0100
@@ -767,6 +767,12 @@ proc mips-dg-init {} {
-mno-smartmips,
#endif
+ #ifdef __mips_synci
JFTR, I came up with something very similar locally, but without
new builtin defines and with the invalid assumption of
configuring with --with-synci=yes, hence #if (__mips == 32 ||
__mips == 64) __mips_isa_rev == 2 !defined(__mips16)
brgds, H-P
Re: [MIPS, committed] Add missing COSTS_N_INSNS call.
On Tue, 28 Aug 2012, Richard Sandiford wrote: Hans-Peter Nilsson h...@bitrange.com writes: On Sun, 26 Aug 2012, Richard Sandiford wrote: I'm preparing a patch to turn gcc.target/mips into a torture-like testsuite. While on the subject of gcc.target/mips and its extensions, it also doesn't handle a build configured with --with-synci=yes. (Well, not on the 4.7 branch at least.) What goes wrong? I don't remember details, but IIRC some synci-related tests go wrong for mipsisa32r2el-linux-gnu due to -msynci being the default. Don't worry, I've fixed it in the local import. :) I though the above would entice you to try it, but I guess I need to report better for that to happen. Maybe later. brgds, H-P
Re: [MIPS, committed] Add missing COSTS_N_INSNS call.
On Sun, 26 Aug 2012, Richard Sandiford wrote: I'm preparing a patch to turn gcc.target/mips into a torture-like testsuite. While on the subject of gcc.target/mips and its extensions, it also doesn't handle a build configured with --with-synci=yes. (Well, not on the 4.7 branch at least.) brgds, H-P
[RFA:] fix configury version checks for in-tree binutils
Found while investigating PR54373. A combined tree (in-tree binutils) using binutils post-2.22 is semi-broken at the moment: the version of the assembler and linker can't be found. The configury doesn't expect the single-quote that has appeared; i.e. VERSION=2.22.0 vs. VERSION='2.23.51' You see this in the build log as lots of: checking assembler flags... checking assembler for .balign and .p2align... /x/combined/gcc/configure: line 21848: test: -ge: unary operator expected no checking assembler for .p2align with maximum skip... /x/combined/gcc/configure: line 21884: test: -ge: unary operator expected no obviously causing all optional features in the assembler and linker to be turned off. (This could have been the cause of not seeing PR54373 in a combined tree, but that's actually due to a combination of two other bugs.) Here's a patch to fix in-tree binutils and to sanity-check that the version is extracted to avoid semi-silent failures like the above. An alternative would be to scrap the special-case and just go with out-of-tree feature tests; it seems few enough people use it, that this has gone undetected for a while. I don't know how important this is to canadian-cross support though (where you can't run the assembler and linker to test features). I tested this by building a combined tree for mmix-knuth-mmixware with top-of-tree binutils as well as binutils-2.22 from CVS (see quotes above) past the point of failure. I also built gcc with out-of-tree binutils past the point-of-failure, as well as versions of the patch in-tree and out-tree with just the error check, to verify that it works (is not executed, for out-of-tree binutils). I don't need to tell build maintainers how much of a problem it is matching a single-quote character portably; I just went for any non-numeric/identifier character. I also don't need to tell build maintainers that ? and + are unportables in sed regexps. The changequote calls in the ld version check in configure.ac, are necessary to avoid autoconf terminating with a too-deep-recursion message for the AC_MSG_ERROR call, which can't be helped by quoting the message (duh). Can be alternatively fixed by re/moving the other/outer pair of changequotes visible in the context of this patch and to add corresponding [] where needed. Too much fun for me. N.B. combining CVS binutils-2.20 in-tree is broken due to mismatching libtool versions. I did not test CVS binutils-2.21. Ok to commit? I'd suggest applying this to 4.7 as well. Ok there after test? gcc: * acinclude.m4 (_gcc_COMPUTE_GAS_VERSION): Allow a single character to quote the VERSION= contents. Sanity-check contents. * configure.ac (what linker to use ld version extraction): Ditto. * configure: Regenerate. Index: gcc/acinclude.m4 === --- gcc/acinclude.m4(revision 190682) +++ gcc/acinclude.m4(working copy) @@ -393,11 +393,15 @@ for f in $gcc_cv_as_bfd_srcdir/configure $gcc_cv_as_gas_srcdir/configure \ $gcc_cv_as_gas_srcdir/configure.in \ $gcc_cv_as_gas_srcdir/Makefile.in ; do - gcc_cv_gas_version=`sed -n -e 's/^[[ ]]*\(VERSION=[[0-9]]*\.[[0-9]]*.*\)/\1/p' $f` + gcc_cv_gas_version=`sed -n -e 's/^[[ ]]*VERSION=[[^0-9A-Za-z_]]*\([[0-9]]*\.[[0-9]]*.*\)/VERSION=\1/p' $f` if test x$gcc_cv_gas_version != x; then break fi done +case $gcc_cv_gas_version in + VERSION=[[0-9]]*) ;; + *) AC_MSG_ERROR([[cannot find version of in-tree assembler]]);; +esac gcc_cv_gas_major_version=`expr $gcc_cv_gas_version : VERSION=\([[0-9]]*\)` gcc_cv_gas_minor_version=`expr $gcc_cv_gas_version : VERSION=[[0-9]]*\.\([[0-9]]*\)` gcc_cv_gas_patch_version=`expr $gcc_cv_gas_version : VERSION=[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)` Index: gcc/configure.ac === --- gcc/configure.ac(revision 190682) +++ gcc/configure.ac(working copy) @@ -2030,11 +2030,17 @@ if test $gcc_cv_ld = ../ld/ld-new$buil for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in do changequote(,)dnl - gcc_cv_gld_version=`sed -n -e 's/^[ ]*\(VERSION=[0-9]*\.[0-9]*.*\)/\1/p' $f` + gcc_cv_gld_version=`sed -n -e 's/^[ ]*VERSION=[^0-9A-Za-z_]*\([0-9]*\.[0-9]*.*\)/VERSION=\1/p' $f` if test x$gcc_cv_gld_version != x; then break fi done + case $gcc_cv_gld_version in + VERSION=[0-9]*) ;; +changequote([,])dnl + *) AC_MSG_ERROR([[cannot find version of in-tree linker]]) ;; +changequote(,)dnl + esac gcc_cv_gld_major_version=`expr $gcc_cv_gld_version : VERSION=\([0-9]*\)` gcc_cv_gld_minor_version=`expr $gcc_cv_gld_version : VERSION=[0-9]*\.\([0-9]*\)` changequote([,])dnl brgds, H-P
Committed, btest-gcc.sh: make fortran tests optional
Some targets, like mmix-knuth-mmixware, has fortran turned off. This patch has seen a large amount of testing, both for targets with fortran on and off. A similar patch in the past (http://gcc.gnu.org/ml/gcc-patches/2005-04/msg00766.html) made the libstdc++ tests optional, so I'm calling it obvious. Committed. contrib/regression: * btest-gcc.sh (TESTLOGS): Make gfortran.sum optional. Index: btest-gcc.sh === --- btest-gcc.sh(revision 190670) +++ btest-gcc.sh(working copy) @@ -118,5 +118,4 @@ H_REAL_TARGET=`$SOURCE/config.sub $H_TAR TESTLOGS=gcc/testsuite/gcc/gcc.sum gcc/testsuite/g++/g++.sum -gcc/testsuite/gfortran/gfortran.sum gcc/testsuite/objc/objc.sum @@ -145,4 +145,8 @@ echo error $RESULT || exit 1 make $dashj -k check +if [ -f gcc/testsuite/gfortran/gfortran.sum ] ; then + TESTLOGS=$TESTLOGS gcc/testsuite/gfortran/gfortran.sum +fi + if [ -f $BUILD/$H_TARGET/libstdc++-v3/testsuite/libstdc++.sum ] ; then TESTLOGS=$TESTLOGS $H_TARGET/libstdc++-v3/testsuite/libstdc++.sum brgds, H-P
To-be-committed: fix configure case when assembler COMDAT group support is lacking
If the out-of-tree target assembler supports neither:
.section .text,axG,@progbits,.foo,comdat
(tsk tsk no leading space, though that's a red herring)
nor:
.section .text,axG,%progbits,.foo,comdat
then gcc/configure.ac drops through into the system-specific case seen
in the patch, where for *-*-solaris2*, gcc_cv_as_comdat_group_group is
set. (If one of the above syntaxes work, then
gcc_cv_as_comdat_group_group is forced to no.) For other systems,
it is not set. In subsequent code (setting HAVE_COMDAT_GROUP), a test
of that variable for such systems is seen in the build log as an error
message from test:
...
checking assembler for section merging support... yes
checking assembler for COMDAT group support (GNU as)... no
checking assembler for COMDAT group support (GNU as, %type)... no
/mnt/2/tmp/hp/testgcc/tmp/st3/gcc/gcc/configure: line 23125: test: =: unary
operator expected
checking assembler for line table discriminator support... yes
...
I'll commit this in a while as obvious. The idiom for testing that
the variable is set (N.B. manual settings are supposed to work) is
copied from the closest variant, also the least objectionable one
among the extant ones. There are lots of variants in there. :/
gcc:
* configure.ac (gcc_cv_as_comdat_group_group): Default to no.
* configure: Regenerate.
Index: gcc/configure.ac
===
--- gcc/configure.ac(revision 190682)
+++ gcc/configure.ac(working copy)
@@ -2630,6 +2630,9 @@ else
if test $gcc_cv_as_comdat_group_percent = yes; then
gcc_cv_as_comdat_group_group=no
else
+ if test -z ${gcc_cv_as_comdat_group_group+set}; then
+ gcc_cv_as_comdat_group_group=no
+ fi
case ${target} in
# Sun as uses a completely different syntax.
*-*-solaris2*)
To-be-committed: correct out-of-tree .hidden support for mmix-knuth-mmixware
GCC configure should say that the linker does *not* support .hidden.
For static-only linking (the only mode supported on this simulator-only
system) this shouldn't really matter. But, different code is actually
used (not just the attribute-hidden decorations), so different bugs are
exposed. Like PR54373. There's a difference for in-tree and separate
build/out-of-tree binutils and this makes the out-of-tree-binutils case
align to the in-tree one, where the linker is found to emit something
that doesn't match the ELF test; the `grep 'EMUL = .*elf' ../ld/Makefile`
fails. I'll hold this until the non-configury bugs in PR54373 are fixed.
gcc:
* configure.ac (out-of-tree linker .hidden support) Set to no
for mmix-knuth-mmixware.
* configure: Regenerate.
Index: gcc/configure.ac
===
--- gcc/configure.ac(revision 190682)
+++ gcc/configure.ac(working copy)
@@ -2300,6 +2300,12 @@ else
if test x$ld_is_gold = xyes; then
:
elif echo $ld_ver | grep GNU /dev/null; then
+case ${target} in
+ mmix-knuth-mmixware)
+# The linker emits by default mmo, not ELF, so no is appropriate.
+ gcc_cv_ld_hidden=no
+ ;;
+esac
if test 0$ld_date -lt 20020404; then
if test -n $ld_date; then
# If there was date string, but was earlier than 2002-04-04, fail
brgds, H-P
Re: [PATCH v2, middle-end]: Introduce TARGET_REJECT_COMBINED_INSN target hook
On Sat, 25 Aug 2012, Uros Bizjak wrote: Hello! v2 patch differences: - moves hook description text to target.def - fixes error path to clear clobbers, as expected by recog_for_combine callers 2012-08-25 Uros Bizjak ubiz...@gmail.com * target.def (reject_combined_insn): New target hook. * doc/tm.texi.in (TARGET_REJECT_COMBINED_INSN): New hook. * doc/tm.texi: Regenerated. * combine.c (recog_for_combine): Call targetm.reject_combined_insn to allow targets to reject combined insn. Maybe mention that the default is to allow all combinations for which a pattern match? And that the reason to disallow them can be that they're known to result in suboptimal code? Or other reasons. I don't think it should be mentioned that it can be used to stop invalid code to be generated; that'd just be encouraging covering up bugs in reload. (Using it to simplify the port may be valid.) brgds, H-P
Re: [PATCH v2, middle-end]: Introduce TARGET_REJECT_COMBINED_INSN target hook
On Sat, 25 Aug 2012, Uros Bizjak wrote:
On Sat, Aug 25, 2012 at 7:22 PM, Hans-Peter Nilsson h...@bitrange.com wrote:
Maybe mention that the default is to allow all combinations for
which a pattern match? And that the reason to disallow them can
be that they're known to result in suboptimal code? Or other
reasons.
Something like this perhaps:
/* Returns true if the combined insn should be rejected
for some reason. */
DEFHOOK
(reject_combined_insn,
Take an instruction in @var{insn} and return @code{true} if the instruction\
should be rejected as a combination of two or more instructions. The\
default is to accept all instructions.,
bool, (rtx insn),
hook_bool_rtx_false)
Better.
brgds, H-P
Committed: old patch to btest-gcc.sh: Add libmudflap.sum, if it exists.
I had a nagging feeling there was a patch I didn't commit. Patch approval link, having link to the patch: http://gcc.gnu.org/ml/gcc-patches/2009-03/msg01509.html (modulo obvious adjustments to copyright line) brgds, H-P
Re: [bootstrap] Tentative fix for PR 54281
From: Diego Novillo dnovi...@google.com Date: Thu, 16 Aug 2012 21:12:56 +0200 On 12-08-16 15:00 , Diego Novillo wrote: This is the patch I'm currently testing. I need someone with a very old toolchain (4.1 or earlier) to also give this a try (the original problem does not occur in g++ more recent than 4.1). + PR bootstrap/54281 + * intl.h: Prevent libintl.h from being included when + ENABLE_NLS is not set. Regtested on a x86_64 F 8 system with gcc-4.1.2-33, no regressions at r190450 (with --disable-nls) compared to results at r190381 (without). brgds, H-P
Re: CXX conversion: min g++ version pre-requisite?
On Fri, 17 Aug 2012, Gary Funck wrote: Paul Hargrove noted the following build failure on an older x86-32 Linux (Redhat 8.0) system. wow. old. The g++ version is: g++ (GCC) 3.4.0 Currently, install.texi states: @heading Tools/packages necessary for building GCC @table @asis @item ISO C90 compiler Necessary to bootstrap GCC, although versions of GCC prior to 3.4 also allow bootstrapping with a traditional (KR) C compiler. This appears to be out-of-date with respect to new C++ stage 1 build requirement. Or rather, nobody until now built with such an old version to find all problems that need to be worked around if possible, or the new (lowest version to support) oldest version to give up on, where the maintenance yield is too low... People had problems finding systems with gcc as low as 4.1.x, for which problems have recently been solved. I wouldn't be surprised if a detailed bug report would help making 3.4.x supported at least for native bootstrap... Maybe person or persons with such access could help finding the right set of casts, which for gen* programs might be cold enough to minimize the maintenance burden from uglification. brgds, H-P
Re: [RFA:] fix PR54261, reverse operator emitted for compare_and_swap-libfunc targets
From: Hans-Peter Nilsson h...@axis.com
Date: Wed, 15 Aug 2012 02:20:37 +0200
I looked around and it seems only cris{v32,}-axis-linux-gnu is
affected. Still, besides that target, for a 4.7/r189762 import
and c/c++ testing, boot+regtest in progress for x86_64-linux and
cross-test for cris-axis-elf. The test-case is spot-checked to
pass for a target not implementing any atomics whatsoever,
i.e. mmix-knuth-mmixware.
(clean test-results)
Ok for trunk, assuming clean test-results? Maybe 4.7 too, it
being somewhat trivial?
gcc:
PR middle-end/54261
* optabs.c (expand_atomic_fetch_op): Save and restore code when
retrying after failed attempt.
gcc/testsuite:
PR middle-end/54261
* gcc.dg/torture/pr54261-1.c: New test.
Approved on IRC by dnovillo, committed. A backport to 4.7 is
trivial but AFAIK not affecting any FSF target so I'll drop
that; affected private ports have to have maintainers skilled
enough to find this post.
brgds, H-P
Re: combine permutations in gimple
On Wed, 15 Aug 2012, Richard Guenther wrote: On Wed, Aug 15, 2012 at 1:56 PM, Ramana Radhakrishnan ramana.radhakrish...@linaro.org wrote: Of-course, the problem here is this change of semantics with the hook TARGET_VEC_PERM_CONST_OK. Targets were expanding to generic permutes with constants in the *absence* of being able to deal with them with the specialized permutes. fwprop will now leave us at a point where each target has to now grow more knowledge with respect to how best to expand a generic constant permute with a sequence of permutes rather than just using the generic permute. Generating a sequence of permutes from a single constant permute will be a harder problem than (say) dealing with immediate expansions so you are pushing more complexity into the target but in the short term target maintainers should definitely have a heads up that folks using vector permute intrinsics could actually have performance regressions on their targets. It's of course the same with the user input containing such a non-optimal handled constant permute. So I'm less convinced that it's too much burden on the target side. OTOH if there is a generic kind of shuffles that targets do not implement directly but can be simulated by two that are directly implemented pushing the logic to the expander (and adjusting the target hook semantic) would be ok. There's a wonderful knapsack-class problem in there, something for next year's GSoC? (Given a constant permutation, synthesize it with a set of simpler operations with total cost constant_shuffle, where the simpler operation and costs are target-specific (query for presence by TARGET_VEC_PERM_CONST_OK and cost hooks; decompose to simpler operation by generic heuristics). A similar but simpler problem is to synthesize a constant vector instead of loading it from memory (though a load may be cheap enough for most SIMD targets that this may be uninteresting to generalize). brgds, H-P
Re: add strnlen to libiberty (was Re: Assembly output optimisations)
On Mon, 6 Aug 2012, Ian Lance Taylor wrote: On Mon, Aug 6, 2012 at 10:44 PM, Dimitrios Apostolou ji...@gmx.net wrote: What else is missing to make this patch appropriate for libiberty? Should I change the prolog in strnlen.c, since I only copied it intact from gnulib? We generally try to avoid straight GPL source code without runtime exception in libiberty, and I don't see a reason to bend that rule for this trivial function. Just don't forget that libiberty isn't a target library anymore. To wit, the (GCC) run-time exception is moot for that code, AFAIK. Maybe enough reason to abandon that rule so its code can be truly and freely shared between GNU projects. brgds, H-P
Re: Assembly output optimisations (was: PR 51094 - fprint_w() in output_addr_const() reinstated)
On Tue, 7 Aug 2012, Dimitrios Apostolou wrote: Thanks Andreas, hp, Mike, for your comments. Mike I'd appreciate if you elaborated on how to speed-up sprint_uw_rev(), I don't think I understood what you have in mind. I just commented on comments and just above the nit-level; formatting and contents. Still, I believe such nits affects reviewer interest. Formatting: two spaces after punctuation at end of sentence, see other comments. You got it right in most places. Some of the comments you changed were actually wrong *before*, some of those you actually corrected. I can't quote the non-inlined patch, so I'll copy-paste some lines, adding quote-style: +/* Return a statically allocated string with the decimal representation of + VALUE. String IS NOT null-terminated. */ Write as: +/* Return a statically allocated string with the decimal representation of + VALUE. String IS NOT null-terminated. */ Avoid comments that look like commented out code. Add a word or two to avoid that. + /* Emit the .loc directive understood by GNU as. Equivalent: */ + /* printf (\t.loc %u %u 0 is_stmt %u discriminator %u, + file_num, line, is_stmt, discriminator); */ Write as: + /* Emit the .loc directive understood by GNU as. The following is + equivalent to printf (\t.loc %u %u 0 is_stmt %u discriminator %u, + file_num, line, file_num, line, is_stmt, discriminator); */ Most of the new comments in default_elf_asm_output_ascii are wrongly formatted. Most are missing end-of-sentence punctuation and two spaces. Some look odd; complete the sentences or add ellipsis to continue in the next comment. There's also wrong terminology both in comments and code, in several places: a '\0' is NIL, not NULL. Better write as \0 for clarity in comments. + /* If short enough */ + if (((unsigned long) (next_null - s) ELF_STRING_LIMIT) + /* and if it starts with NULL and it is only a +single NULL (empty string) */ + ((*s != '\0') || (s == next_null))) + /* then output as .string */ + s = default_elf_asm_output_limited_string (f, s); + else + /* long string or many NULLs, output as .ascii */ Write as (note also the s/next_null/next_nil/): + /* If short enough... */ + if (((unsigned long) (next_nil - s) ELF_STRING_LIMIT) + /* ... and if it starts with \0 and it is only +single \0 (an empty string) ... */ + ((*s != '\0') || (s == next_null))) + /* ... then output as .string. */ + s = default_elf_asm_output_limited_string (f, s); + else + /* A long string or many \0, output as .ascii. */ Contents: I missed a note on relative speedups. You need to guard the changes, or else a valid future cleanup could be to simplify the code, using library functions, possibly slowing it down. There's just the word fast added in a comment, reminding of self-adhesive go-fast-stripes for cars and computer cases. :) /* Write a signed HOST_WIDE_INT as decimal to a file, fast. */ But *how* much faster? Something like: /* Write a signed HOST_WIDE_INT as decimal to a file. At some time, this was X times faster than fprintf (%lld, l); */ Right, not the most important review bits. Still, you asked for elaboration. Oh right, I see that was just for Mike's comments. Well, anyway, since my comments still apply. :P brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Thu, 2 Aug 2012, Nathan Froyd wrote: As $SUBJECT says. There's not too much interesting here. I did a fairly literal-minded conversion, so it's possible there's smarter ways to do some things. Doesn't look too bad though, but ... Compiled with cross to mmix-knuth-mmixware and spot-checked by comparing libgcc object files. I have no idea how to set up a simulator environment. Find the MMIXware simulator (mmix), install it. Following the links from our further readings page will take you to http://www-cs-faculty.stanford.edu/~knuth/mmix-news.html or http://mmix.cs.hm.edu/src/index.html which both have links to the simulator. At time of this reading, the links point to identical tarballs, for example is http://mmix.cs.hm.edu/src/mmix-20110831.tar.gz. A dejagnu baseboard was in dejagnu-1.4.4 already, use RUNTESTFLAGS=--target_board=mmixware-sim, which should not be a surprise to you, have you ever tested using a simulator before. Though if nothing changed in GCC in the last month (ha-ha), expect a lot of timeouts, and lots of LTO FAILs. Also, better wrap the executable in a script that does e.g. ulimit -v 30, as the simulator allocates memory on access (kind of like stack on modern systems), making debugging a bit harder than expected and letting the oomkiller run around out of control otherwise. (On my virtual todo-list to have MMIXware patches for controlled memory allocation; right after looking into the LTO FAILs.) H-P, if you'd like to test beforehand, that'd be great. ...yes, I'd like to test this before, please. Thanks for your work. (Let's set a timeout at the end of August; ok to commit Sep 1 have I not got back to you.) (Now, from where did I get that H constraint?) brgds, H-P
Re: [PATCH v2] Target-specific limits on vector alignment
From: Ulrich Weigand uweig...@de.ibm.com Date: Fri, 27 Jul 2012 17:24:08 +0200 Richard (Earnshaw) has asked me to take over working on this patch now. I've now made the change requested above and removed the size argument. The target is now simply asked to return the required alignment for the given vector type. I've also added a check for the case where the target provides both an alignment and a mode for a vector type, but the mode actually requires bigger alignment than the type. This is simply rejected (the target can fix this by reporting a different type alignment or changing the mode alignment). Sounds great, IMHO. brgds, H-P
Re: [PATCH/MIPS] Emit stack executable note
On Wed, 25 Jul 2012, Andrew Pinski wrote: Hi, The Linux kernel already supports non-executable stack since around February 2010. This patch has GCC emit the notes that are associated with non-executable stack. What does the kernel do when the note isn't present? OK? Bootstrapped and tested on mips64-linux-gnu with kernel already supporting the non-executable stack with no regressions. Thanks, Andrew Pinski ChangeLog: * config/mips/linux-common.h (TARGET_ASM_FILE_END): Define. I can't quote your non-inline patch so I'll copy-paste it. + +/* For MIPS, the default is to assume *no* executable stack, so output +an executable-stack-note only when needed. */ +#define TARGET_ASM_FILE_END file_end_indicate_exec_stack The comment is wrong, your patch is always emitting the note. Besides IIRC, *glibc* defaults to an executable stack for MIPS, so you need to emit it if (!trampolines_created) or something like that. See the CRIS cris_file_end from which the comment seems copy-pasted, but the opposite condition. :) If the kernel and glibc are in contradiction regarding the default, you do need to use a slightly different expression, checking too whether you're creating an executable, if you really mean to emit the note only when needed. brgds, H-P
Re: [patch] Move lowering of switches to bit tests to GIMPLE
On Tue, 24 Jul 2012, Richard Henderson wrote: On 07/21/2012 06:10 AM, Oleg Endo wrote: I think on SH the cost test in lshift_cheap_p with gen_rtx_ASHIFT (word_mode, const1_rtx, reg), speed_p); will always 'fail', because of sh.c (shiftcosts): /* There is no pattern for constant first operand. */ if (CONST_INT_P (XEXP (x, 0))) return MAX_COST; On SH3 / SH4* / SH2A there is a dynamic shift that does reg reg or reg reg, which is not that expensive actually. However, the constant 1 must be loaded into a register first. I'm currently trying to brush up the shift code in SH a little bit and could add some things to handle the const reg case. Having to load the 1 into a register is something common to all targets. IIUC, no, not for testing a single bit and/or several bits from bit 0 up to the numbered bit. See cris *btst and m68k (several unnamed patterns with about the same contents, setting cc0, calling output_btst) r188179 m68k.md:666 (evil patterns?) I shouldn't think that SH should need to do anything special. Indeed, that conditional that you quote above looks very wrong. I believe you should simply recurse (or let the rtx_cost driver recurse) to find the cost of arguments to the shift. r~
Re: [RFC] Target-specific limits on vector alignment
From: Ulrich Weigand uweig...@de.ibm.com Date: Tue, 24 Jul 2012 19:38:15 +0200 I've implemented this as a separate hook, rather than using the existing hooks because there's a strong likelihood of breaking some existing ABIs if I did it another way. There are a couple of tests that will need some re-working before this can be committed to deal with the fall-out of making this change; I'll prepare those changes if this patch is deemed generally acceptable. That hook changes the alignment requirement for vector types. However, those will still be *increased* in finalize_type_size to the alignment of an underlying vector mode (if any), if that is greater. Fortunately, this does not occur in the ARM case since vector mode alignment is bounded by BIGGEST_ALIGNMENT, and the ARM implementation of the new hook never returns anything smaller. However, for the general case this would re-introduce the possibility that vector type alignment differs based on the presence or absence of vector modes. I don't follow all your arguments right now; I'll have to revisit this in abut three weeks, but what's true for ARM should be true for MIPS in my case (32-bit o32 with vendor-specific 16-byte vectors): BIGGEST_ALIGNMENT is the same 64 bits (non-aligned for 16-byte vectors) AFAICS. Nice to know they have the same issue and I have something to compare. :) brgds, H-P
Re: Commit: ARM: Document -munaligned-access
On Fri, 20 Jul 2012, Ryan Mansfield wrote: On 12-07-19 05:33 PM, Hans-Peter Nilsson wrote: Index: changes.html +some source codes generates code that accesses memory on unaligned +adresses. This will require the kernel of those systems to enable adresses - addresses. Thanks, fixed. brgds, H-P
Committed: executable-stack note for CRIS
Tested crisv32-linux (much as you can do without actually
installing a new /lib/ld.so.1 with the changed defaults).
Yep, this is actually a change of the default in glibc (port to
be submitted), but changing it this way is safe: with old
libraries lacking a note, they default to not having an
executable stack, as should be. You pragmatically only need an
executable stack for the gcc test-suite, AFAIK. :) Committed.
Tested cris-elf and crisv32-elf too with some other patches FWIW.
Emit executable-stack note correctly for CRIS targets.
* config/cris/cris.c (cris_file_end): New function.
(TARGET_ASM_FILE_END): Define.
Index: config/cris/cris.c
===
--- config/cris/cris.c (revision 189754)
+++ config/cris/cris.c (revision 189755)
@@ -153,6 +153,7 @@ static void cris_trampoline_init (rtx, t
static rtx cris_function_value(const_tree, const_tree, bool);
static rtx cris_libcall_value (enum machine_mode, const_rtx);
static bool cris_function_value_regno_p (const unsigned int);
+static void cris_file_end (void);
/* This is the parsed result of the -max-stack-stackframe= option. If
it (still) is zero, then there was no such option given. */
@@ -199,6 +200,8 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_
#undef TARGET_ASM_FILE_START
#define TARGET_ASM_FILE_START cris_file_start
+#undef TARGET_ASM_FILE_END
+#define TARGET_ASM_FILE_END cris_file_end
#undef TARGET_INIT_LIBFUNCS
#define TARGET_INIT_LIBFUNCS cris_init_libfuncs
@@ -2747,6 +2750,17 @@ cris_file_start (void)
default_file_start ();
}
+/* Output that goes at the end of the file, similarly. */
+
+static void
+cris_file_end (void)
+{
+ /* For CRIS, the default is to assume *no* executable stack, so output
+ an executable-stack-note only when needed. */
+ if (TARGET_LINUX trampolines_created)
+file_end_indicate_exec_stack ();
+}
+
/* Rename the function calls for integer multiply and divide. */
static void
cris_init_libfuncs (void)
brgds, H-P
Committed: testsuite updates for recent changes in atomics for cris*-linux*
I changed the defaults for cris*-linux*, but didn't fix the
test-suite to go with that, so with unaligned accesses working,
the test-suite still checked that they trapped, doh. Here's the
update, checked trunk cris-elf and crisv32-elf (because of
difference in atomics) and crisv32-linux* on the local 4.7
import - sorry, no actual trunk results. Some new tests
piggy-backed to check that unaligned atomics don't fall over
when exposed to actual unaligned accesses, but the results of
the operations aren't properly checked. A buglet fixed; a local
var shadowed the intended global var. Committed.
gcc/testsuite:
Handle recent changes in default atomics for cris*-*-linux*.
* gcc.target/cris/torture/sync-mis-xchg-i-1ml.c,
gcc.target/cris/torture/sync-mis-xchg-i-2ml.c,
gcc.target/cris/torture/sync-mis-xchg-i-3ml.c,
gcc.target/cris/torture/sync-mis-xchg-s-1ml.c,
gcc.target/cris/torture/sync-mis-op-i-1ml.c,
gcc.target/cris/torture/sync-mis-op-i-2ml.c,
gcc.target/cris/torture/sync-mis-op-i-3ml.c,
gcc.target/cris/torture/sync-mis-op-s-1ml.c: New tests.
* gcc.target/cris/torture/sync-mis-op-i-2a.c: Make sure
-mno-unaligned-atomic-may-use-library is in effect for
cris*-*-linux*.
* gcc.target/cris/torture/sync-mis-xchg-i-1.c,
gcc.target/cris/torture/sync-mis-xchg-i-2.c,
gcc.target/cris/torture/sync-mis-xchg-i-3.c,
gcc.target/cris/torture/sync-mis-xchg-i-2a.c,
gcc.target/cris/torture/sync-mis-xchg-s-1.c,
gcc.target/cris/torture/sync-mis-op-i-1.c,
gcc.target/cris/torture/sync-mis-op-i-2.c,
gcc.target/cris/torture/sync-mis-op-i-1a.c,
gcc.target/cris/torture/sync-mis-op-i-3.c,
gcc.target/cris/torture/sync-mis-op-i-3a.c,
gcc.target/cris/torture/sync-mis-op-s-1a.c,
gcc.target/cris/torture/sync-mis-xchg-i-1a.c,
gcc.target/cris/torture/sync-mis-xchg-i-3a.c,
gcc.target/cris/torture/sync-mis-xchg-s-1a.c: Similar.
* gcc.target/cris/torture/sync-mis-op-s-1.c: Ditto.
(main): Remove local variable x.
[mis_ok]: Check that atomics don't fail.
Index: gcc.target/cris/torture/sync-mis-op-i-2a.c
===
--- gcc.target/cris/torture/sync-mis-op-i-2a.c (revision 189754)
+++ gcc.target/cris/torture/sync-mis-op-i-2a.c (working copy)
@@ -2,4 +2,5 @@
/* { dg-additional-sources ../sync-1.c } */
/* { dg-options -Dop -Dtype=int -Dmisalignment=2 -DTRAP_USING_ABORT
-mno-trap-using-break8 } */
/* { dg-additional-options -mtrap-unaligned-atomic { target cris-*-elf } } */
+/* { dg-additional-options -mno-unaligned-atomic-may-use-library { target
cris*-*-linux* } } */
#include sync-mis-op-s-1.c
Index: gcc.target/cris/torture/sync-mis-xchg-i-1ml.c
===
--- gcc.target/cris/torture/sync-mis-xchg-i-1ml.c (revision 0)
+++ gcc.target/cris/torture/sync-mis-xchg-i-1ml.c (revision 0)
@@ -0,0 +1,4 @@
+/* { dg-do run { target *-*-linux* } } */
+/* { dg-additional-sources ../sync-1.c } */
+/* { dg-options -Dxchg -Dtype=int -Dmis_ok } */
+#include sync-mis-op-s-1.c
Index: gcc.target/cris/torture/sync-mis-xchg-i-2ml.c
===
--- gcc.target/cris/torture/sync-mis-xchg-i-2ml.c (revision 0)
+++ gcc.target/cris/torture/sync-mis-xchg-i-2ml.c (revision 0)
@@ -0,0 +1,4 @@
+/* { dg-do run { target *-*-linux* } } */
+/* { dg-additional-sources ../sync-1.c } */
+/* { dg-options -Dxchg -Dtype=int -Dmisalignment=2 -Dmis_ok } */
+#include sync-mis-op-s-1.c
Index: gcc.target/cris/torture/sync-mis-xchg-i-3ml.c
===
--- gcc.target/cris/torture/sync-mis-xchg-i-3ml.c (revision 0)
+++ gcc.target/cris/torture/sync-mis-xchg-i-3ml.c (revision 0)
@@ -0,0 +1,4 @@
+/* { dg-do run { target *-*-linux* } } */
+/* { dg-additional-sources ../sync-1.c } */
+/* { dg-options -Dxchg -Dtype=int -Dmisalignment=3 -Dmis_ok } */
+#include sync-mis-op-s-1.c
Index: gcc.target/cris/torture/sync-mis-xchg-i-1.c
===
--- gcc.target/cris/torture/sync-mis-xchg-i-1.c (revision 189754)
+++ gcc.target/cris/torture/sync-mis-xchg-i-1.c (working copy)
@@ -1,4 +1,4 @@
/* { dg-do run { target *-*-linux* } } */
/* { dg-additional-sources ../sync-1.c } */
-/* { dg-options -Dxchg -Dtype=int } */
+/* { dg-options -Dxchg -Dtype=int -mno-unaligned-atomic-may-use-library } */
#include sync-mis-op-s-1.c
Index: gcc.target/cris/torture/sync-mis-xchg-i-2.c
===
--- gcc.target/cris/torture/sync-mis-xchg-i-2.c (revision 189754)
+++ gcc.target/cris/torture/sync-mis-xchg-i-2.c (working copy)
@@ -1,4 +1,4 @@
/* { dg-do run { target *-*-linux* } } */
/* { dg-additional-sources ../sync-1.c } */
Committed, CRIS: remove unused variables from cris_asm_output_ident
Looks like these were left from Steven's ident-cleanup.
Steven, for future reference, you might want to use
contrib/warn_summary on the build-logs. ;)
Incidentally, the magic option to increase diff context for svn
is -x -UN as in -x -U5 below, just to show there's not much
else in that function.
Committed after a cris-elf test-run.
gcc:
* config/cris/cris.c (cris_asm_output_ident): Remove unused
local variables section_asm_op, size, buf.
Index: gcc/config/cris/cris.c
===
--- gcc/config/cris/cris.c (revision 189727)
+++ gcc/config/cris/cris.c (working copy)
@@ -2518,14 +2518,10 @@ cris_legitimate_pic_operand (rtx x)
If the front-end is done, we must be being called from toplev.c.
In that case, do nothing. */
void
cris_asm_output_ident (const char *string)
{
- const char *section_asm_op;
- int size;
- char *buf;
-
if (cgraph_state != CGRAPH_STATE_PARSING)
return;
default_asm_output_ident_directive (string);
}
brgds, H-P
Yet another gcc.c-torture/execute/20101011-1.c DO_TEST 0
The #elif defined ... #define DO_TEST 0 exceptions are accumulating. Maybe the DO_TEST = 1 case should be the exception... Committed. gcc/testsuite: * gcc.c-torture/execute/20101011-1.c (DO_TEST): Define as 0 for CRIS. Index: gcc.c-torture/execute/20101011-1.c === --- gcc.c-torture/execute/20101011-1.c (revision 189727) +++ gcc.c-torture/execute/20101011-1.c (working copy) @@ -36,6 +36,9 @@ /* Attempting to trap division-by-zero in this way isn't likely to work on bare-metal m68k systems. */ # define DO_TEST 0 +#elif defined (__CRIS__) + /* No SIGFPE for CRIS integer division. */ +# define DO_TEST 0 #else # define DO_TEST 1 #endif brgds, H-P
Re: [RESEND-2][PATCH] Allow printing of escaped curly braces in assembler directives with operands
On Wed, 18 Jul 2012, Siddhesh Poyarekar wrote:
Hi,
Resending. I did not get any responses the last two times and I too
forgot about it. Can someone please review this?
This is *not* an approver-review.
An assembler directive with an operand is filtered through
output_asm_insn (or asm_fprintf for gcc internal asm() directives) to
expand the operand values in the assembler as well as to choose
dialects if present. This patch is concerned primarily with the
dialects, since their syntax prevent inclusion of assembler strings
with curly braces, causing them to be interpreted as dialects.
The attached patch allows printing of curly braces in assembler by
preceding them with a \\.
(the single character \ to wit. The \\ is the sequence you have
to write in C.)
Sounds like a good idea, but I think you shouldn't limit that to
{} but rather introduce \ to escape and cause any next
character to be emitted as-is, in particular |.
So to print the following code into assembler:
.pushsection .foo; .asciz div { width : 50%% | height=10px }; .long
42; .popsection
The following code needs to be used with this patch:
void f()
{
asm ( .pushsection \.foo\; .asciz \div \\{ width : 50%% |
height=10px \\} \; .long %c0; .popsection : : i(42) ); }
The other option to \\ (since it doesn't look as clean) was to use %
as an escape character, but I was not sure if that is a better looking
option or a worse looking one. I don't mind resubmitting the patch to
use %{ and %} to print curly braces if that is acceptable.
I'm on the fence regarding this, but leaning towards maybe
that's a better way forward, as it can't collide with any
current use of \\ (seeing as %{ %} and %| currently fall
down into a gcc_unreachable). If you change your patch to this,
make sure you include %| to emit |.
(Hm, why is there a gcc_unreachable? It should be an error as
user asm statements can reach it!)
It is still possible to print curly braces in asm string literals
without operands since they do not undergo any transformation.
The patch does not introduce any regressions. I have tested this with
x86_64 and i686 and it works well with both of them.
That's a target (yes, counting it as one target) that defines
ASSEMBLER_DIALECT wherein {}| has the predefined meaning you
try to escape. I suggest testing a non-ASSEMBLER_DIALECT target
as well. And check that no target uses \\ for something else
through ASM_FPRINTF_EXTENSIONS... ...so it seems like %{ etc.
would be safer.
gcc/ChangeLog:
2012-03-27 Siddhesh Poyarekar siddh...@redhat.com
* final.c (output_asm_insn, asm_fprintf): Print curly braces if
preceded by an escaped backslash (\\).
testsuite/ChangeLog:
2012-03-27 Siddhesh Poyarekar siddh...@redhat.com
* gcc.dg/asm-braces.c: New test case.
Thanks and keep pinging.
brgds, H-P
Re: Commit: ARM: Document -munaligned-access
From: nick clifton ni...@redhat.com Date: Fri, 20 Jul 2012 09:18:52 +0200 approved - please apply. Thanks! I see the last sentence could do with better punctuation, so I added the obvious comma. Installed as follows. Index: changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v retrieving revision 1.121 diff -p -u -r1.121 changes.html --- changes.html15 Jul 2012 15:04:38 - 1.121 +++ changes.html20 Jul 2012 13:35:48 - @@ -43,6 +43,19 @@ /li +liOn ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A, +ARMv7-R, or ARMv7-M, the new option +code-munaligned-access/code is active by default, which for +some source codes generates code that accesses memory on unaligned +adresses. This will require the kernel of those systems to enable +such accesses (controlled by CP15 register codec1/code, refer +to ARM documentation). Alternatively or for compatibility with +kernels where unaligned accesses are not supported, all code has +to be compiled with code-mno-unaligned-access/code. +Linux/ARM in official releases has automatically and +unconditionally supported unaligned accesses as emitted by GCC due +to this option being active, since Linux version 2.6.28./li + liSupport on ARM for the legacy floating-point accelerator (FPA) and the mixed-endian floating-point format that it used has been obsoleted. The ports that still use this format have been obsoleted as well. brgds, H-P
Re: Commit: ARM: Document -munaligned-access
From: Nick Clifton ni...@redhat.com
Date: Wed, 18 Jul 2012 08:51:16 +0200
2012-07-18 Nick Clifton ni...@redhat.com
* doc/invoke.texi (ARM Options): Document -munaligned-access.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 189603)
+++ gcc/doc/invoke.texi (working copy)
@@ -497,7 +497,8 @@
-mcaller-super-interworking -mcallee-super-interworking @gol
-mtp=@var{name} -mtls-dialect=@var{dialect} @gol
-mword-relocations @gol
--mfix-cortex-m3-ldrd}
+-mfix-cortex-m3-ldrd @gol
+-munaligned-access}
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
@@ -11049,6 +11050,23 @@
generating these instructions. This option is enabled by default when
@option{-mcpu=cortex-m3} is specified.
+@item -munaligned-access
+@itemx -mno-unaligned-access
+@opindex munaligned-access
+@opindex mno-unaligned-access
+Enables (or disables) reading and writing of 16- and 32- bit values
+from addresses that are not 16- or 32- bit aligned. By default
+unaligned access is disabled for all pre-ARMv6 and all ARMv6-M
+architectures, and enabled for all other architectures.
*cough*
Sounds like a call for a note in changes.html, to warn people
that they have to turn on the alignment feature in their startup
code (for whatever OS) for their ARMv6-or-later targets, if they
have not already done so.
Maybe something like:
Index: changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.113
diff -p -u -r1.113 changes.html
--- changes.html5 Jun 2012 11:03:53 - 1.113
+++ changes.html15 Jun 2012 02:04:46 -
@@ -43,6 +43,19 @@
/li
+liOn ARM, when compiling for ARMv6 (but not ARMv6-M), ARMv7-A,
+ARMv7-R, or ARMv7-M, the new option
+code-munaligned-access/code is active by default, which for
+some source codes generates code that accesses memory on unaligned
+adresses. This will require the kernel of those systems to enable
+such accesses (controlled by CP15 register codec1/code, refer
+to ARM documentation). Alternatively or for compatibility with
+kernels where unaligned accesses are not supported, all code has
+to be compiled with code-mno-unaligned-access/code.
+Linux/ARM in official releases has automatically and
+unconditionally supported unaligned accesses as emitted by GCC due
+to this option being active since Linux version 2.6.28./li
+
liSupport on ARM for the legacy floating-point accelerator (FPA) and
the mixed-endian floating-point format that it used has been obsoleted.
The ports that still use this format have been obsoleted as well.
http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00282.html
Sorry, but I couldn't miss the opportunity to ping this now that
the ARM maintainers have this issue in their L1-cache. A simple
yes or no would do; I don't see what could possibly need any
contemplation regarding this note.
brgds, H-P
Re: CRIS atomics revisited 4/4: give up on alignment of atomic data, RFC for is_lock_free hook
From: Andrew MacLeod amacl...@redhat.com
Date: Tue, 17 Jul 2012 14:24:48 +0200
On 07/15/2012 11:49 PM, Hans-Peter Nilsson wrote:
Well, give up by default that is, and fix it up in a helper
function in glibc to hold a global byte-sized atomic lock for
the duration. (Sorry!) Yes, this means that
fold_builtin_atomic_always_lock_free is wrong. It knows about
alignment in general but doesn't handle the case where the
default alignment of the underlying type is too small for atomic
accesses, and should probably be augmented by a target hook,
alternatively, change the allow_libcall argument in the call to
can_compare_and_swap_p to false. I guess I should open a PR for
this and add a test-case. Later.
I set it up to eventually handle alignment issues, but didn't really
know any details of what to do when, or how. Input was rather lacking
at the time and there was a time crunch, so I just punted on it in 4.7
at the tree level and targets did their own thing.
Most (all but mine :) seem happy to provide naturally-aligned
types and promising to never access non-naturally-aligned data
so not much they need to do... Not sure if the i386 ABI (or
e.g. xchg and similar insns in the CPU) promises or requires
that all data (ints) are aligned or if something is nominally
required.
The library idea was
new enough that the standards guys couldnt/wouldn't give me a straight
answer since it hadn't really been thought through I don't think. My
apologies if I misrepresent anyone there :-)
The basic problem you are dealing with is a 4 byte object that is not
aligned properly, so the lock-free instructions don't work on it., but
'always_lock_free' says yes, that type is lock free. Then in the
library,
(The library here meaning the atomicity implementation, not
e.g. libstdc++. I assume you don't mean libatomic, which seems
to be intended just as a fallback for targets and sizes
unsupported by core gcc, so they're guaranteed to not be
lock-free.)
its implemented via a lock? Is that approximately it? or is
there an additional issue?
That's pretty much it... except it'd be better letting the
target decorate the intended atomic type, e.g. with alignment
attributes. See suggestion last. Decoration is in place for
_Atomic_word but I can't tell how _Atomic_word relates to the
issues here. Is _Atomic_word just obsolete?
The original intention was that __atomic{is,always}_lock_free takes the
size and a pointer to the object, and the alignment of the pointer
object would be checked at compile time to see if it is always
compatible with size.
The alignment of the pointed-to *object* can't be checked at
compile-time. As GCC is pretty bad at alignment, taking on the
task of trying to make a difference for different *types* seems
non-trivial. All you can hope for is telling apart the size of
the objects.
Alignment of objects *can* be checked at run-time from within
the current __atomic_is_lock_free API, and apparently that's the
intention in the core gcc implementation, but the last few
details aren't there or are wrong.
(so a char pointer to a 4 byte object would fail
that test). which would means 'always_lock_free' should return false,
and in turn a call to 'is_lock_free' would generate a library call and
check the alignment at runtime of the object to determine what to do.
For the record, is_lock_free in libstdc++ calls
__atomic_is_lock_free, which is per-instance at the API level.
I don't see usage of __atomic_always_lock_free in libstdc++.
(I see it in libatomic/glfree.c:libat_is_free, but dominated by
a check for natural alignment of the pointer instance with
failure yielding false, bravo.)
Not using __atomic_always_lock_free is probably wrong, it seems
it should call __atomic_always_lock_free if I understand
/path/to/trunk/libstdc++-v3/doc/html/ext/lwg-closed.html#1146
correctly (the link to www.open-std.org there is dead). Can
this be confirmed? Still wouldn't work of course - the target
can't doesn't have a say in the __atomic_always_lock_free
implementation.
The library implementation of all the other __atomic operations would
check 'is_lock_free' to see whether they need to use a lock, or whether
they can use available lock-free sequences/syscalls.
(You must mean libstdc++ here which is slightly confusing with
the use of __atomic_ which for me implies the gcc
implementation.)
Some confusion around the standard surfaced and the intention from the
standard point of view appeared to be that 'is_lock_free' should be true
or false ALL the time for a given type.
And there's confusion all around telling apart atomic access
from *lock-free* atomic access. :-) Witness
__GCC_ATOMIC_INT_LOCK_FREE and the md.texi note: For the
purposes of C++11 @code{std::atomic::is_lock_free}, it is
assumed that these library calls do @emph{not} use any kind of
interruptable locking.
Come to think of it, what is interruptable locking?
(More STFW
Re: CRIS atomics revisited 4/4: give up on alignment of atomic data, RFC for is_lock_free hook
From: Andrew MacLeod amacl...@redhat.com Date: Tue, 17 Jul 2012 14:24:48 +0200 Any PR's you open related this this, copy me on them and I'll try to get them addressed. I could separate the issues I saw into PRs 54003-6. That's all, hopefully ...at least for now. :) BTW, your @gcc.gnu.org account doesn't have a bugzilla account (did not match anything says bugzilla), contrary to most/all other accounts. Maybe deliberate or known, maybe something to look into. brgds, H-P
Re: CRIS atomics revisited 4/4: give up on alignment of atomic data
From: Hans-Peter Nilsson h...@axis.com
Date: Mon, 16 Jul 2012 05:49:00 +0200
gcc:
* config/cris/sync.md (atomic_fetch_atomic_op_namemode)
(cris_atomic_fetch_atomic_op_namemode_1)
(atomic_compare_and_swapmode)
(cris_atomic_compare_and_swapmode_1): Make
conditional on TARGET_ATOMICS_MAY_CALL_LIBFUNCS for
sizes larger than byte.
A sync goof (the VC kind): the committed and sent patch, but
not the changelog, was missing the first hunk, now committed:
Index: config/cris/sync.md
===
--- config/cris/sync.md (revision 189504)
+++ config/cris/sync.md (working copy)
@@ -101,7 +101,7 @@ (define_expand atomic_fetch_atomic_op_
(match_operand:BWD 2 atomic_op_op_pred)
(match_operand 3)
(atomic_op:BWD (match_dup 0) (match_dup 1))]
-
+ MODEmode == QImode || !TARGET_ATOMICS_MAY_CALL_LIBFUNCS
{
enum memmodel mmodel = (enum memmodel) INTVAL (operands[3]);
brgds, H-P
CRIS atomics revisited 0/4: summary
These were spotted while debugging usage of atomics within glibc. The kind of changes are microoptimizations, nanooptimizations, a buglet and a major issue. Micro: the load-store-conditional sequence for compare-and-swap I originally committed was an earlier version improved later. Nanooptimizations: choosing better-fitting operands for the atomic operator insn. Buglet: a post-increment could have sneaked into the (non-atomic) arithmetic operator operand; better make it nonmemory_operand altogether. I also threw in use of the now generic need_atomic_barrier_p, let's call that a microoptimization. The major issue, giving up on alignment of atomic data by default, is last. Tested together and some separately, no regressions for cris-elf nor crisv32-elf. Committed separately. brgds, H-P
CRIS atomics revisited 1/4: use need_atomic_barrier_p.
Use the new need_atomic_barrier_p. gcc: * config/cris/sync.md (atomic_fetch_atomic_op_namemode) (atomic_compare_and_swapmode): Gate expand_mem_thread_fence calls on result of call to need_atomic_barrier_p. Index: config/cris/sync.md === --- config/cris/sync.md (revision 189499) +++ config/cris/sync.md (working copy) @@ -93,11 +93,15 @@ (define_expand atomic_fetch_atomic_op_ if (MODEmode != QImode TARGET_TRAP_UNALIGNED_ATOMIC) cris_emit_trap_for_misalignment (operands[1]); - expand_mem_thread_fence (mmodel); + if (need_atomic_barrier_p (mmodel, true)) +expand_mem_thread_fence (mmodel); + emit_insn (gen_cris_atomic_fetch_atomic_op_namemode_1 (operands[0], operands[1], operands[2])); - expand_mem_thread_fence (mmodel); + if (need_atomic_barrier_p (mmodel, false)) +expand_mem_thread_fence (mmodel); + DONE; }) @@ -196,13 +200,17 @@ (define_expand atomic_compare_and_swap if (MODEmode != QImode TARGET_TRAP_UNALIGNED_ATOMIC) cris_emit_trap_for_misalignment (operands[2]); - expand_mem_thread_fence (mmodel); + if (need_atomic_barrier_p (mmodel, true)) +expand_mem_thread_fence (mmodel); + emit_insn (gen_cris_atomic_compare_and_swapmode_1 (operands[0], operands[1], operands[2], operands[3], operands[4])); - expand_mem_thread_fence (mmodel); + if (need_atomic_barrier_p (mmodel, false)) +expand_mem_thread_fence (mmodel); + DONE; }) brgds, H-P
CRIS atomics revisited 2/4: don't allow a memory operand (with possible side-effects)
Buglet in atomic_compare_and_swapmode, allowing (in theory) a volatile or post-increment memory operand. Simplest and safest fixed by excluding all memory operands. gcc: * config/cris/sync.md (atomic_compare_and_swapmode): Change predicate to nonmemory_operand for operand 3. Add FIXME. (cris_atomic_compare_and_swapmode_1): Change predicates and constraints for operand 3 to exclude memory. Index: config/cris/sync.md === --- config/cris/sync.md (revision 189500) +++ config/cris/sync.md (working copy) @@ -184,11 +184,12 @@ (define_insn cris_atomic_fetch_atomic_ ;; can_compare_and_swap_p call in omp-low.c, 4.8 era). We'd slightly ;; prefer atomic_exchangemode over this, but having both would be ;; redundant. +;; FIXME: handle memory without side-effects for operand[3]. (define_expand atomic_compare_and_swapmode [(match_operand:SI 0 register_operand) (match_operand:BWD 1 register_operand) (match_operand:BWD 2 memory_operand) - (match_operand:BWD 3 general_operand) + (match_operand:BWD 3 nonmemory_operand) (match_operand:BWD 4 register_operand) (match_operand 5) (match_operand 6) @@ -218,7 +219,7 @@ (define_insn cris_atomic_compare_and_sw [(set (match_operand:SI 0 register_operand =r) (unspec_volatile:SI [(match_operand:BWD 2 memory_operand +Q) - (match_operand:BWD 3 general_operand g)] + (match_operand:BWD 3 nonmemory_operand ri)] CRIS_UNSPEC_ATOMIC_SWAP_BOOL)) (set (match_operand:BWD 1 register_operand =r) (match_dup 2)) (set (match_dup 2) brgds, H-P
CRIS atomics revisited 3/4: pattern improvements
Microoptimizations for the atomic patterns themselves. Constant
operands are so common that it seems wasteful not to handle the
most common cases and avoid wasting a register.
gcc/testsuite:
* gcc.target/cris/20011127-1.c: Adjust to %P being a
valid register operand output modifier.
gcc:
* config/cris/cris.c (cris_print_operand) case 'P', 'q': New cases.
* config/cris/sync.md (atomic_op_op_cnstr): New code_attr.
(atomic_op_op_pred): Ditto.
(atomic_op_mnem_pre_op2): Renamed from atomic_op_mnem_pre; to
reflect the change to include %2 in expansion. All callers changed.
(qm3): New mode_attr.
(atomic_fetch_atomic_op_namemode): Use atomic_op_op_pred
as predicate for operand 2.
(cris_atomic_fetch_atomic_op_namemode_1): Update FIXME. Use
atomic_op_op_pred atomic_op_op_cnstr for predicate and
constraint for operand 2.
(atomic_compare_and_swapmode): Add FIXME. Change predicate to
nonmemory_operand for operand 3.
(cris_atomic_compare_and_swapmode_1): Change operand 3 to
exclude memory. Improve emitted sync code for v10 and v32. Use
qm3 instead of m for size designator for cmp.
Index: config/cris/cris.c
===
--- config/cris/cris.c (revision 189499)
+++ config/cris/cris.c (working copy)
@@ -981,6 +981,53 @@ cris_print_operand (FILE *file, rtx x, i
fprintf (file, INTVAL (operand) 0 ? adds.w : addq);
return;
+case 'P':
+ /* For const_int operands, print the additive mnemonic and the
+modified operand (byte-sized operands don't save anything):
+ N=MIN_INT..-65536: add.d N
+ -65535..-64: subu.w -N
+ -63..-1: subq -N
+ 0..63: addq N
+ 64..65535: addu.w N
+ 65536..MAX_INT: add.d N.
+(Emitted mnemonics are capitalized to simplify testing.)
+For anything else (N.B: only register is valid), print add.d. */
+ if (REG_P (operand))
+ {
+ fprintf (file, Add.d );
+
+ /* Deal with printing the operand by dropping through to the
+normal path. */
+ break;
+ }
+ else
+ {
+ int val;
+ gcc_assert (CONST_INT_P (operand));
+
+ val = INTVAL (operand);
+ if (!IN_RANGE (val, -65535, 65535))
+ fprintf (file, Add.d %d, val);
+ else if (val = -64)
+ fprintf (file, Subu.w %d, -val);
+ else if (val = -1)
+ fprintf (file, Subq %d, -val);
+ else if (val = 63)
+ fprintf (file, Addq %d, val);
+ else if (val = 65535)
+ fprintf (file, Addu.w %d, val);
+ return;
+ }
+ break;
+
+case 'q':
+ /* If the operand is an integer -31..31, print q else .d. */
+ if (CONST_INT_P (operand) IN_RANGE (INTVAL (operand), -31, 31))
+ fprintf (file, q);
+ else
+ fprintf (file, .d);
+ return;
+
case 'd':
/* If this is a GOT symbol, force it to be emitted as :GOT and
:GOTPLT regardless of -fpic (i.e. not as :GOT16, :GOTPLT16).
Index: config/cris/sync.md
===
--- config/cris/sync.md (revision 189501)
+++ config/cris/sync.md (working copy)
@@ -73,17 +73,32 @@ (define_code_iterator atomic_op [plus mi
(define_code_attr atomic_op_name
[(plus add) (minus sub) (and and) (ior or) (xor xor) (mult nand)])
+;; The operator nonatomic-operand can be memory, constant or register
+;; for all but xor. We can't use memory or addressing modes with
+;; side-effects though, so just use registers and literal constants.
+(define_code_attr atomic_op_op_cnstr
+ [(plus ri) (minus ri) (and ri) (ior ri) (xor r) (mult ri)])
+
+(define_code_attr atomic_op_op_pred
+ [(plus nonmemory_operand) (minus nonmemory_operand)
+ (and nonmemory_operand) (ior nonmemory_operand)
+ (xor register_operand) (mult nonmemory_operand)])
+
;; Pairs of these are used to insert the not after the and for nand.
-(define_code_attr atomic_op_mnem_pre ;; Upper-case only to sinplify testing.
- [(plus Add.d) (minus Sub.d) (and And.d) (ior Or.d) (xor Xor)
- (mult aNd.d)])
+(define_code_attr atomic_op_mnem_pre_op2 ;; Upper-case only to simplify
testing.
+ [(plus %P2) (minus Sub.d %2) (and And%q2 %2) (ior Or%q2 %2) (xor Xor
%2)
+ (mult aNd%q2 %2)])
+
(define_code_attr atomic_op_mnem_post_op3
[(plus ) (minus ) (and ) (ior ) (xor ) (mult not %3\;)])
+;; For SImode, emit q for operands -31..31.
+(define_mode_attr qm3 [(SI %q3) (HI .w) (QI .b)])
+
(define_expand atomic_fetch_atomic_op_namemode
[(match_operand:BWD 0 register_operand)
(match_operand:BWD 1 memory_operand)
- (match_operand:BWD 2 register_operand)
+ (match_operand:BWD 2 atomic_op_op_pred)
(match_operand 3)
(atomic_op:BWD (match_dup 0) (match_dup 1))]
@@ -109,8 +124,9 @@ (define_insn
CRIS atomics revisited 4/4: give up on alignment of atomic data, RFC for is_lock_free hook
Well, give up by default that is, and fix it up in a helper function in glibc to hold a global byte-sized atomic lock for the duration. (Sorry!) Yes, this means that fold_builtin_atomic_always_lock_free is wrong. It knows about alignment in general but doesn't handle the case where the default alignment of the underlying type is too small for atomic accesses, and should probably be augmented by a target hook, alternatively, change the allow_libcall argument in the call to can_compare_and_swap_p to false. I guess I should open a PR for this and add a test-case. Later. Too many library API writers don't cater for the possibility that atomic (lockless) data may need to have certain properties that may not be matched by the basic underlying data type, specifically alignment, and fixing the failing instances by hand is...challenging. About half the cases have the atomic data defined in the proximity of the atomic operations, and are easily locally fixable. The other half are of increasing complexity; may have the data defined elsewhere, where the need for atomicity is surprising and fixing it would be a kludge. (But with a proper API could be easily handled, e.g. if a data-type defined specific for the purpose was used; one different than the underlying type or other common derivated type used in the library.) So, we'll change things for cris*-linux. By default, call a helper function. Users can change the default at the caller site where atomic alignment is known good or where there is interest in fixing it up when failure is seen; executing a trap insn was the old default. Regarding changing fold_builtin_atomic_always_lock_free or adding a hook, I posit that a better default is to assume atomic data has to be naturally aligned on *all* existing GCC targets to accomplish lockless operations, at least for those that don't just punt to a system call, so maybe fold_builtin_atomic_always_lock_free should be changed to check that, by default. Now, it just assumes that the default type-alignment is ok and that only a smaller alignment is not always atomic. People with counter-examples are asked to please explain how the counter-example handles data straddling a page boundary. Yes, it can be done, but how does the kernel-equivalent accomplish atomicity; are the pages locked while the instruction (assumed to cause an exception), is emulated, or is kernel re-entrance impossible or what? The default remains the same for non-*-linux-* cris-* and crisv32-* subtargets, since the code compiled for those targets is expected to have a different focus, one where fixing non-aligned data definitions is feasible and desirable. I deliberately make it optional and use weasel-wording whether the library functions are actually called or an atomic insn sequence emitted when suitable; when GCC knows the alignment of the data (for example, for local static data or through deliberate attributes) it should be allowed to emit the atomic instruction sequence even without alignment checks. Right now (or maybe it was just the 4.7 branch), GCC's handling of alignment is so poor that the emitted alignment checks (those that conditionally execute a trap insn) aren't eliminated for atomic data with explicit large-enough attribute declarations. From what I (with limited tree-foo) could see, IIRC basically everything about alignment for the specific data is discarded and the underlying default type alignment is reported. (Right, a PR is in order; I know I've entered a PR for the related __alignof__.) gcc: * config/cris/cris.c (cris_init_libfuncs): Handle initialization of library functions for basic atomic compare-and-swap. * config/cris/cris.h (TARGET_ATOMICS_MAY_CALL_LIBFUNCS): New macro. * config/cris/cris.opt (munaligned-atomic-may-use-library): New option. * config/cris/sync.md (atomic_fetch_atomic_op_namemode) (cris_atomic_fetch_atomic_op_namemode_1) (atomic_compare_and_swapmode) (cris_atomic_compare_and_swapmode_1): Make conditional on TARGET_ATOMICS_MAY_CALL_LIBFUNCS for sizes larger than byte. gcc/testsuite: * gcc.target/cris/sync-2i.c, gcc.target/cris/sync-2s.c, gcc.target/cris/sync-3i.c, gcc.target/cris/sync-3s.c, gcc.target/cris/sync-4i.c, gcc.target/cris/sync-4s.c, gcc.target/cris/sync-1-v10.c, gcc.target/cris/sync-1-v32.c: For cris*-*-linux*, also pass -mno-unaligned-atomic-may-use-library. * gcc.target/cris/sync-xchg-1.c: New test. diff --git gcc/config/cris/cris.c gcc/config/cris/cris.c index 22b254f..e4c11fd 100644 --- gcc/config/cris/cris.c +++ gcc/config/cris/cris.c @@ -3130,6 +3176,16 @@ cris_init_libfuncs (void) set_optab_libfunc (udiv_optab, SImode, __Udiv); set_optab_libfunc (smod_optab, SImode, __Mod); set_optab_libfunc (umod_optab, SImode, __Umod); + + /* Atomic data being unaligned is unfortunately a reality. + Deal with it. */ + if
Fixing gcc.c-torture/compile/pr44707.c for CRIS v32 1/2.
Buglet in cris_preferred_reload_class, incidental, apparently
without effect at least regarding failing test-cases. A class
disjunct from the input was returned as preferred. It could
arguably be gcc_asserted as a sanity-check by the caller that
the returned class is a subset of the original class. ...and I
guess I'll add such a gcc_assert *inside*
cris_preferred_reload_class. Later. No regressions, cris-elf
and crisv32-elf. Committed.
gcc:
* config/cris/cris.c (cris_preferred_reload_class):
Don't return GENERAL_REGS as preferred to MOF_SRP_REGS.
Index: gcc/config/cris/cris.c
===
--- gcc/config/cris/cris.c (revision 189470)
+++ gcc/config/cris/cris.c (working copy)
@@ -1503,6 +1550,7 @@ cris_preferred_reload_class (rtx x ATTRI
{
if (rclass != ACR_REGS
rclass != MOF_REGS
+ rclass != MOF_SRP_REGS
rclass != SRP_REGS
rclass != CC0_REGS
rclass != SPECIAL_REGS)
brgds, H-P
Fixing gcc.c-torture/compile/pr44707.c for CRIS v32 2/2: RFC: CONSTANT_ADDRESS_P and its default are evil!
I think CONSTANT_ADDRESS_P can and should be eliminated,
replaced by something like
CONSTANT_P (x) targetm.legitimate_address_p (QImode, x, false)
(or QImode replaced by the known used mode) in the code
currently calling it.
It should, because the default definition is redundant and evil;
easy to miss for targets where (mem (const x)) is not valid for
any arbitrary generic x (symbol_ref, label_ref or const_int,
including offsetted ones; (plus x (const_int N)).
This is the case for CRIS v32, for which only (mem reg) and (mem
(post_inc reg)) are valid. Like ia64 it has no offsettable
addressing mode. For example, the constraint in
gcc.c-torture/compile/pr44707.c of nro can only match for the
r part.
If your target fails gcc.c-torture/compile/pr44707.c, this might
be the reason.
No regressions for cris-elf nor crisv32-elf; fixes
gcc.c-torture/compile/pr44707.c for the latter. Committed.
* config/cris/cris-protos.h (cris_legitimate_address_p): Declare.
* config/cris/cris.h (CONSTANT_ADDRESS_P): Define in terms of
CONSTANT_P and cris_legitimate_address_p.
* config/cris/cris.c (cris_legitimate_address_p): Make non-static.
Index: config/cris/cris.c
===
--- config/cris/cris.c (revision 189506)
+++ config/cris/cris.c (working copy)
@@ -127,8 +127,6 @@ static void cris_init_libfuncs (void);
static reg_class_t cris_preferred_reload_class (rtx, reg_class_t);
-static bool cris_legitimate_address_p (enum machine_mode, rtx, bool);
-
static int cris_register_move_cost (enum machine_mode, reg_class_t,
reg_class_t);
static int cris_memory_move_cost (enum machine_mode, reg_class_t, bool);
static bool cris_rtx_costs (rtx, int, int, int, int *, bool);
@@ -1414,7 +1412,7 @@ cris_biap_index_p (const_rtx x, bool str
here (but is thankfully a general_operand in itself). A local PIC
symbol is valid for the plain symbol + offset case. */
-static bool
+bool
cris_legitimate_address_p (enum machine_mode mode, rtx x, bool strict)
{
const_rtx x1, x2;
Index: config/cris/cris.h
===
--- config/cris/cris.h (revision 189504)
+++ config/cris/cris.h (working copy)
@@ -778,6 +778,9 @@ struct cum_args {int regs;};
#define HAVE_POST_INCREMENT 1
+#define CONSTANT_ADDRESS_P(X) \
+ (CONSTANT_P (X) cris_legitimate_address_p (QImode, X, false))
+
/* Must be a compile-time constant, so we go with the highest value
among all CRIS variants. */
#define MAX_REGS_PER_ADDRESS 2
Index: config/cris/cris-protos.h
===
--- config/cris/cris-protos.h (revision 189499)
+++ config/cris/cris-protos.h (working copy)
@@ -40,6 +40,7 @@ extern bool cris_base_p (const_rtx, bool
extern bool cris_base_or_autoincr_p (const_rtx, bool);
extern bool cris_bdap_index_p (const_rtx, bool);
extern bool cris_biap_index_p (const_rtx, bool);
+extern bool cris_legitimate_address_p (enum machine_mode, rtx, bool);
extern bool cris_store_multiple_op_p (rtx);
extern bool cris_movem_load_rest_p (rtx, int);
extern void cris_asm_output_symbol_ref (FILE *, rtx);
brgds, H-P
Re: Fix PR53908
From: Richard Sandiford rsand...@nildram.co.uk
Date: Thu, 12 Jul 2012 21:18:54 +0200
if (CALL_P (insn))
{
if (RTL_CONST_OR_PURE_CALL_P (insn))
/* Pure functions can read from memory. Const functions can
read from arguments that the ABI has forced onto the stack.
Neither sort of read can be volatile. */
memrefs_in_across |= MEMREF_NORMAL;
else
{
memrefs_in_across |= MEMREF_VOLATILE;
mem_sets_in_across |= MEMREF_VOLATILE;
}
}
OK with that change if you agree.
(Steven or Bernd, please ACK/NAK, for quick resolution.)
I've tested this version for trunk; no regressions for revision
189439 on x86_64-unknown-linux-gnu (N.B. the test-case doesn't
trigger on trunk). I've also tested Steven's earlier version,
same results, if you change your mind. :)
It'll be another 6:21 hours(!) before results from the 4.7
branch.
brgds, H-P
Re: Fix PR53908
From: Richard Guenther richard.guent...@gmail.com Date: Fri, 13 Jul 2012 09:37:13 +0200 On Fri, Jul 13, 2012 at 9:21 AM, Steven Bosscher stevenb@gmail.com wrote: On Fri, Jul 13, 2012 at 8:47 AM, Hans-Peter Nilsson hans-peter.nils...@axis.com wrote: From: Richard Sandiford rsand...@nildram.co.uk Date: Thu, 12 Jul 2012 21:18:54 +0200 OK with that change if you agree. (Steven or Bernd, please ACK/NAK, for quick resolution.) I can't ACK, but I think Richard S.'s fix is correct. (There was a delegation from a maintainer so yes.) I agree, so, ok for trunk if it passes bootstrap testing. Ok for 4.7 too? brgds, H-P
Re: Fix PR53908
From: Richard Guenther richard.guent...@gmail.com
Date: Fri, 13 Jul 2012 10:08:05 +0200
On Fri, Jul 13, 2012 at 9:59 AM, Hans-Peter Nilsson
hans-peter.nils...@axis.com wrote:
Ok for 4.7 too?
Of course.
Committed to trunk as follows, including the test-case which
doesn't fail on trunk but does on 4.6 and 4.7.
Will commit to 4.7 branch too. I'll leave 4.6 to those
interested enough to invest time to do the testing.
gcc:
2012-07-13 Richard Sandiford rdsandif...@googlemail.com
Steven Bosscher ste...@gcc.gnu.org
Bernd Schmidt ber...@codesourcery.com
PR rtl-optimization/53908
* df-problems.c (can_move_insns_across): When doing
memory-reference book-keeping, handle call insns.
gcc/testsuite:
2012-07-13 Hans-Peter Nilsson h...@axis.com
PR rtl-optimization/53908
* gcc.dg/torture/pr53908.c: New test.
Index: gcc/df-problems.c
===
--- gcc/df-problems.c (revision 189440)
+++ gcc/df-problems.c (working copy)
@@ -4068,6 +4068,19 @@ can_move_insns_across (rtx from, rtx to,
for (insn = across_to; ; insn = next)
{
+ if (CALL_P (insn))
+ {
+ if (RTL_CONST_OR_PURE_CALL_P (insn))
+ /* Pure functions can read from memory. Const functions can
+ read from arguments that the ABI has forced onto the stack.
+ Neither sort of read can be volatile. */
+ memrefs_in_across |= MEMREF_NORMAL;
+ else
+ {
+ memrefs_in_across |= MEMREF_VOLATILE;
+ mem_sets_in_across |= MEMREF_VOLATILE;
+ }
+ }
if (NONDEBUG_INSN_P (insn))
{
memrefs_in_across |= for_each_rtx (PATTERN (insn), find_memory,
--- /dev/null Tue Oct 29 15:57:07 2002
+++ gcc/testsuite/gcc.dg/torture/pr53908.c Thu Jul 12 19:22:59 2012
@@ -0,0 +1,288 @@
+/* { dg-do run } */
+/* SEGV at comment below. */
+typedef unsigned int size_t;
+typedef enum har {
+ he_fatal = (-199),
+ he_not_initialized,
+ he_bad_input,
+ he_memory_too_small,
+ he_bad_action,
+ he_duplicate,
+ he_bad_nonce,
+ he_stale_nonce,
+ he_bad_credentials,
+ he_bad_user,
+ he_no_such_user,
+ he_bad_passwd,
+ he_unknown_auth_scheme,
+ he_not_found,
+ he_failed_digest_file_check,
+ he_failed_digest_file_save,
+ he_process_not_privileged,
+ he_other,
+ he_end_of_range,
+ ha_no_error = 0,
+ ha_no_value = 1
+} har;
+typedef enum realm_type
+{
+ axis_realm = 0,
+ ws_realm
+} realm_type;
+
+__attribute__((__noclone__, __noinline__))
+har has_www_auth(char *, size_t, realm_type, har);
+
+__attribute__((__noclone__, __noinline__))
+har has_auth_user(const char *, const char *, realm_type, char *, size_t);
+
+__attribute__((__noclone__, __noinline__))
+char *ha_get_string_value(void);
+
+typedef struct
+{
+ unsigned int track_id;
+ char* user;
+ char* realm;
+ char* authent;
+ int internal_realm;
+} request;
+enum user_response {
+ file_not_found_user_response = -3,
+ access_denied_user_response = -2,
+ no_user_response = -1,
+ ok_user_response = 0
+};
+struct realm_group {
+ char *name;
+ int id;
+ struct realm_group *next;
+};
+struct realm {
+ char *name;
+ char *space;
+ struct realm_group *groups;
+ struct realm *next;
+};
+struct user_info {
+ char *name;
+ int no_groups;
+ int groups[128];
+ struct user_info *next;
+};
+static struct user_info *find_user(const char *user_name);
+static int is_member_of_groups(const struct user_info *user_item,
+const struct realm_group *groups);
+int authent_author(request *req);
+struct realm *realms = ((void *)0);
+struct user_info *users = ((void *)0);
+static struct user_info*
+find_user(const char *user_name)
+{
+ struct user_info *user_item;
+ user_item = users;
+ while (user_item != ((void *)0)) {
+/* SEGV due to NULL access here on user_name. See also comment below. */
+if ((__builtin_strcmp(user_item-name, user_name) == 0))
+ break;
+user_item = user_item-next;
+ }
+ return user_item;
+}
+static int
+is_member_of_groups(const struct user_info *user_item,
+const struct realm_group *groups)
+{
+ const struct realm_group *group_item;
+ int i;
+ group_item = groups;
+ while (group_item != ((void *)0)) {
+for (i = 0; i user_item-no_groups; i++)
+ if (user_item-groups[i] == group_item-id)
+ return 0;
+group_item = group_item-next;
+ }
+ return -1;
+}
+char *foo (void) __attribute__((__noclone__, __noinline__));
+char* g_strdup (const char *str) __attribute__((__malloc__, __noclone__,
__noinline__));
+int g_strcmp0 (const char *str1, const char *str2);
+static int
+is_basic(char **user)
+{
+ char *passwd_ptr;
+ char *authent = foo();
+ passwd_ptr = __builtin_strchr(authent, ':');
+ if (passwd_ptr != ((void *)0)) {
+*user = g_strdup(authent);
+return 0;
+ }
+ return -1;
+}
+static int
+is_digest(char **user
Re: [PATCH] [LM32] Fix lm32-elf-gcc build error by remove unnecessary constant legitimate check.
On Thu, 12 Jul 2012, Jia Liu wrote: May you give me more comment? I wanna fix it. Sorry, I'll leave that to the lm32 maintainer, I just wasnted to clear a misunderstanding regarding TARGET_LEGITIMATE_CONSTANT_P. brgds, H-P
Re: Fix gcc.dg/lower-subreg-1.c failure, revisited.
From: Hans-Peter Nilsson h...@axis.com Date: Sun, 8 Jul 2012 01:02:34 +0200 PR rtl-optimization/53176 * rtlanal.c (rtx_cost): Adjust default cost for X with a UNITS_PER_WORD factor for all X according to the size of its mode, not just for SUBREGs with untieable modes. Handle SET. Use factor * factor for MULT, DIV, UDIV, MOD, UMOD. Approved (IRL by RTH), committed. brgds, H-P
