Re: [cxx-conversion] New Hash Table (issue6244048)
On Thu, May 24, 2012 at 09:43:42AM -0700, Lawrence Crowl wrote:
Add a type-safe hash table, typed_htab. Uses of this table replace
uses of libiberty's htab_t. The benefits include less boiler-plate
code, full type safety, and improved performance.
You haven't looked at the most important problem of that approach -
code bloat. hashtab.o .text is currently ~ 4KB on x86_64, in your version
only very small part out of it is shared. In a cc1plus binary I count
roughly 179 htab_create{,_ggc} calls, ignoring the first parameter (initial
size) that is roughly 139 unique hash/eq/del fn combinations, thus you'll
end up with over hundred copies of most of the hashtab code, in many of the
cases performance critical and thus its I-cache friendliness is quite
important.
Static functions are also not acceptable as template arguments, so
this patch externalizes the functions. To avoid potential name
conflicts, the function names have been prefixed.
Ugh. I guess the C++ way around this would be to put the functions into
anonymous namespace.
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+return htab-size;
+ }
(and various other places) - formatting is wrong, missing space between (.
+void
+typed_htab Element, Hash, Equal, Remove, Allocator
+::dispose ()
Do we want to format methods this way?
Jakub
Re: PATCH: PR bootstrap/53472: contrib/compare-debug should strip out .comment section
2012-05-24 H.J. Lu hongjiu...@intel.com
PR bootstrap/53472
* contrib/compare-debug (remove_comment): New function.
Remove any .comment sections if the first cmp failed.
Ok.
Paolo
diff --git a/contrib/compare-debug b/contrib/compare-debug
index 010d17f..fb8986d 100755
--- a/contrib/compare-debug
+++ b/contrib/compare-debug
@@ -73,11 +73,35 @@ Darwin)
;;
esac
+remove_comment ()
+{
+ file=$1
+ opts=
+ for s in `objdump --section-headers $file | awk '{ print $2 }'`; do
+ case $s in
+ .comment*)
+ opts=$opts --remove-section $s
+ ;;
+ esac
+ done
+ [ -n $opts ] objcopy $opts $file
+}
+
if cmp $1.$suf1 $2.$suf2; then
status=0
else
status=1
+ # Remove any .comment sections.
+ if (objcopy -v) 21 | grep ' --remove-section' /dev/null \
+ (objdump --help) 21 | grep ' --\[*section-\]*headers'
/dev/null; then
+ remove_comment $1.$suf1
+ remove_comment $2.$suf2
+ if cmp $1.$suf1 $2.$suf2; then
+ status=0
+ fi
+ fi
+
# Assembler-generated CFI will add an .eh_frame section for -g not
# present in -g0. Try to cope with it by checking that an .eh_frame
# section is present in either object file, and then stripping it
Re: [patch] Fix PR ada/52362
Paolo, The fix is mainly in the Ada tools, but there is a hitch: when you pass a response file to the linker by means of the -Wl,@ option, it is intercepted by collect2. And collect2 creates another response file for the linker proper only if HAVE_GNU_LD is set to 1. Now HAVE_GNU_LD is set to 1 only if you pass the --with-ld= or --with-gnu-ld switches at configure time; in particular, for systems where the use of the GNU linker is the only choice and thus hardcoded in config.gcc (e.g. Linux), you can end up with HAVE_GNU_LD set to 0, breaking the response file mechanism. Hence the configure.ac hunk. Tested on i586-suse-linux and i686-pc-mingw32, OK for mainline and 4.7 branch? 2012-05-18 Eric Botcazou ebotca...@adacore.com PR ada/52362 * config.gcc (i[34567]86-*-mingw* | x86_64-*-mingw*): Set gas and gnu_ld variables to yes. * configure.ac (HAVE_GNU_LD): Move to after config.gcc inclusion. (HAVE_GNU_AS): Likewise. * config.in: Regenerate. * configure: Likewise. What do you think about the above change? Thanks in advance. -- Eric Botcazou
Re: [patch] Fix PR ada/52362
Il 18/05/2012 23:05, Eric Botcazou ha scritto: Hi, this is a regression present on mainline and 4.7 branch or, more precisely, if you use recent GNU linkers with LTO plugin. During the link phase, gnatlink can decide to use a response file to pass a very long line to the linker. It was actually using a linker script with the GNU linker, but this doesn't work in LTO mode if you use the plugin. The fix is mainly in the Ada tools, but there is a hitch: when you pass a response file to the linker by means of the -Wl,@ option, it is intercepted by collect2. And collect2 creates another response file for the linker proper only if HAVE_GNU_LD is set to 1. Now HAVE_GNU_LD is set to 1 only if you pass the --with-ld= or --with-gnu-ld switches at configure time; in particular, for systems where the use of the GNU linker is the only choice and thus hardcoded in config.gcc (e.g. Linux), you can end up with HAVE_GNU_LD set to 0, breaking the response file mechanism. Hence the configure.ac hunk. Tested on i586-suse-linux and i686-pc-mingw32, OK for mainline and 4.7 branch? Ok, thanks! Paolo
Re: use longcall to abort from !pic trampoline setup on ppc-vxworks
Hello David, On May 25, 2012, at 03:25 , David Edelsohn wrote: libgcc/ * config/rs6000/vxworks/tramp.S (trampoline_setup): Use a longcall sequence in the non pic case on VxWorks. + addis 11, 0,JUMP_TARGET(abort)@ha Why do you use the addis mnemonic instead of lis mnemonic? No specific reason. It might just have been the compiler output for a generated longcall years ago. Yes, lis X,Y is an alias for addis X,0,Y, but the simplified mnemonic clearer about the actual operation and intention. Sure. The patch is okay with that change. Committing, thanks :-)
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
Tom Tromey tro...@redhat.com writes:
Dodji == Dodji Seketeli do...@redhat.com writes:
I didn't see a gcc-patches CC...?
Woops, indeed. Sorry. I have just bounced the original messages to the
list now.
Dodji When building the compiler on altivec, it appears that an unused local
Dodji typedef is lurking in a function of libcpp, breaking bootstrap since
now
Dodji -Wunused-local-typedefs is activated by -Wall.
Dodji -typedef char check_count[(N == 2 || N == 4) * 2 - 1];
I think this is a static assert.
You should probably try s/typedef/extern/ instead.
And you are right. I was probably too hasty on this one. Here are two
patchlets for trunk then. The first one to revert my wrong commit, and
the second one to fix the issue as you suggest. My tree bootstrap with
these.
Dominique, maybe you could test it on your side?
Thanks, and sorry for the inconvenience.
From d65626ffe5ea39eddb0ed2bb9c73f3ab3073fc09 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli do...@redhat.com
Date: Fri, 25 May 2012 09:29:33 +0200
Subject: [PATCH 1/2] Revert PR bootstrap/53459 - unused local typedef when
building on altivec
This reverts commit r187853
---
libcpp/ChangeLog |5 -
libcpp/lex.c |1 +
2 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/libcpp/lex.c b/libcpp/lex.c
index 2ebbee9..c4dd603 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -590,6 +590,7 @@ search_line_fast (const uchar *s, const uchar *end
ATTRIBUTE_UNUSED)
{
#define N (sizeof(vc) / sizeof(long))
+typedef char check_count[(N == 2 || N == 4) * 2 - 1];
union {
vc v;
unsigned long l[N];
--
1.7.6.5
From 690564ffc05a9b8ed8629860878133b0864e3ea4 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli do...@redhat.com
Date: Fri, 25 May 2012 09:36:17 +0200
Subject: [PATCH 2/2] PR bootstrap/53459 - unused local typedef when building
on altivec
PR bootstrap/53459
* lex.c (search_line_fast): Avoid unused local typedefs to simulate
a static assertion.
---
libcpp/ChangeLog |6 ++
libcpp/lex.c |4 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/libcpp/lex.c b/libcpp/lex.c
index c4dd603..d82e335 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -589,8 +589,8 @@ search_line_fast (const uchar *s, const uchar *end
ATTRIBUTE_UNUSED)
{
#define N (sizeof(vc) / sizeof(long))
-
-typedef char check_count[(N == 2 || N == 4) * 2 - 1];
+/* The below is a static assertion. */
+extern char check_count[(N == 2 || N == 4) * 2 - 1];
union {
vc v;
unsigned long l[N];
--
1.7.6.5
--
Dodji
[Dodji Seketeli] [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
I initially forgot to add the gcc-patches list in CC, and tried
unsuccessfully to bounce the message to the list. I am thus forwarding
it now.
---BeginMessage---
Hello,
When building the compiler on altivec, it appears that an unused local
typedef is lurking in a function of libcpp, breaking bootstrap since now
-Wunused-local-typedefs is activated by -Wall.
The reason why I haven't seen this on x86_64-unknown-linux-gnu is that
the culprit function here is preceded by the directive:
#elif (GCC_VERSION = 4005) defined(__ALTIVEC__)
Bootstrapped and tested against trunk on x86_64-unknown-linux-gnu and by
Dominique on altivec, as he stated in the audit trail.
I am applying this to trunk as obvious.
libcpp/
PR bootstrap/53459
* lex.c (search_line_fast): Remove unused typedef check_count.
---
libcpp/ChangeLog |5 +
libcpp/lex.c |1 -
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/libcpp/lex.c b/libcpp/lex.c
index c4dd603..2ebbee9 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -590,7 +590,6 @@ search_line_fast (const uchar *s, const uchar *end
ATTRIBUTE_UNUSED)
{
#define N (sizeof(vc) / sizeof(long))
-typedef char check_count[(N == 2 || N == 4) * 2 - 1];
union {
vc v;
unsigned long l[N];
--
Dodji
---End Message---
--
Dodji
Re: [Patch]: Fix very large frame bug on i386
On May 22, 2012, at 7:17 PM, Richard Henderson wrote:
On 05/15/12 02:10, Tristan Gingold wrote:
2012-05-15 Tristan Gingold ging...@adacore.com
* config/i386/i386.c (struct ix86_frame): Remove unused frame field.
(ix86_compute_frame_layout): Fix type of stack_alignment_needed
and preferred_alignment.
Ok with a test case. Even if it's a scan-assember sort of test.
Thanks, now committed with the following testcase:
/* { dg-do compile } */
/* { dg-require-effective-target lp64 } */
/* { dg-final { scan-assembler -429496 } } */
extern void dump (int *buf, int a);
void func (int a)
{
int bigbuf[1 30];
dump (bigbuf, a);
}
Tristan.
Re: [Patch]: Fix typo in common/config/ia64/ia64-common.c
On May 22, 2012, at 7:17 PM, Richard Henderson wrote: On 05/15/12 01:52, Tristan Gingold wrote: 2012-05-14 Tristan Gingold ging...@adacore.com * common/config/ia64/ia64-common.c (ia64_except_unwind_info): Fix typo. Ok. Thanks, committed. Tristan.
ping: fix mem+32760 ICE on powerpc - introduce predicates weaker than mode_dependent_address_p
Hello, Ping for the non-back-end parts of http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01668.html David approved the rs6000 parts already (with adjustments to comments, http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00368.html) Thanks much in advance for your feedback, Olivier On Apr 26, 2012, at 11:30 , Olivier Hainque wrote: ... a number of places in the compiler use the mode_dependent_address_p predicate to actually check for weaker necesssary conditions. Typically, a few places need to check that a MEM access remains valid when just narrowing the mode, while mode_dependent_address_p tells if any mode change is valid. While this is of course generally safe, this has been causing endless troubles to the powerpc back-end which has apparently unique particularities related to altivec modes. ... The attached patch is a proposal to fix this, slightly generalized compared to the original one. The general idea is to allow for weaker predicates at the places where we need them. This is achieved by the introduction of a TARGET_MAY_NARROW_ACCESS target hook, which defaults to !mode_dependent_address_p and is redefined for powerpc. The patch uses this hook directly instead of the former predicate in a couple of places where this was the intent already, as well as new valid_access_mode_change_p function to direct to one or the or the other depending on provided original and destination modes. This provides a better match for actual internal needs, allows to get rid of the powerpc back-end twists (no need to lie in mode_dependent_address_p any more) and cures the observed internal compiler error. ... 2012-04-26 Olivier Hainque hain...@adacore.com * target.def (TARGET_MAY_NARROW_ACCESS_TO): New hook. * doc/tm.texi[.in] (TARGET_MAY_NARROW_ACCESS_TO): Document. * targhooks.c (default_may_narrow_access_to): Default implementation. * targhooks.h (default_may_narrow_access_to): Declare. * config/rs6000/rs6000.c (rs6000_may_narrow_access_to): Specific implementation. (rs6000_mode_dependent_address): Stop lying for + const_int. (rs6000_offsettable_memref_p): Adjust comments accordingly. * expr.c (convert_move): Use may_narrow_access_to instead of mode_dependent_address_p where appropriate. * recog.c (offsettable_address_addr_space_p): Likewise. (valid_access_mode_change_p): New function. * recog.h (valid_access_mode_change_p): Declare. * simplify-rtx.c (simplify_subreg): Use it instead of mode_dependent_address_p where appropriate. testsuite/ * gcc.dg/offsetmem.c: New test. narrow.dif
[Patch ARM 4_7-branch ] Backport fix for flto failure with neon intrinsics.
Hi, We hit an ICE using lto with neon intrinsics that could be traced back to an invalid call to convert during expand with LTO . Fixed by backporting Richi's fix from mainline - checking he had no objections to this on IRC , tested cross on arm-linux-gnueabi with qemu for no regressions. Applied to the FSF 4.7 branch. regards, Ramana 2012-05-24 Ramana Radhakrishnan ramana.radhakrish...@linaro.org Backport from mainline 2012-03-12 Richard Guenther rguent...@suse.de * config/arm/arm.c (neon_dereference_pointer): Do not call covert during RTL expansion. Index: gcc/config/arm/arm.c === --- gcc/config/arm/arm.c(revision 187800) +++ gcc/config/arm/arm.c(working copy) @@ -20604,9 +20604,8 @@ array_type = build_array_type (elem_type, build_index_type (upper_bound)); /* Dereference EXP using that type. */ - exp = convert (build_pointer_type (array_type), exp); return fold_build2 (MEM_REF, array_type, exp, - build_int_cst (TREE_TYPE (exp), 0)); + build_int_cst (build_pointer_type (array_type), 0)); } /* Expand a Neon builtin. */
Re: divide 64-bit by constant for 32-bit target machines
Hi, I have replaced expand_mult to expand_widening_mult and removed all direct references to DImode, SImode modes in the expand_mult_highpart_optab funtion. The attached patch was tested on arm-7l, mips-32r2 (74k), i686 without new regressions. Richard, do you think it is ready now? thanks, Dinar. On Tue, May 22, 2012 at 7:45 PM, Richard Henderson r...@redhat.com wrote: On 05/22/12 07:05, Dinar Temirbulatov wrote: + if ((size - 1 BITS_PER_WORD + BITS_PER_WORD == 32 mode == DImode) Do note that this patch will not go in with hard-coded SImode and DImode references. Which, really, you do not even need. GET_MODE_BITSIZE (mode) == 2*BITS_PER_WORD is what you wanted for testing for double-word-ness, and word_mode is a good substitute for SImode here. + /* Splitting the 64-bit constant for the higher and the lower parts. */ + y0 = gen_rtx_CONST_INT (DImode, dUINT32_MAX); + y1 = gen_rtx_CONST_INT (DImode, d32); Use gen_int_mode. + x1 = convert_to_mode (DImode, x1, 1); + x0 = convert_to_mode (DImode, x0, 1); + + /* Splitting the 64-bit constant for the higher and the lower parts. */ + y0 = gen_rtx_CONST_INT (DImode, dUINT32_MAX); + y1 = gen_rtx_CONST_INT (DImode, d32); + + z2 = gen_reg_rtx (DImode); + u0 = gen_reg_rtx (DImode); + + /* Unsigned multiplication of the higher multiplier part + and the higher constant part. */ + z2 = expand_mult(DImode, x1, y1, z2, 1); + /* Unsigned multiplication of the lower multiplier part + and the higher constant part. */ + u0 = expand_mult(DImode, x0, y1, u0, 1); I'm fairly sure you really want to be using expand_widening_mult here, rather than using convert_to_mode first. While combine may be able to re-generate a widening multiply out of this sequence, there's no sense making it work too hard. r~ 2012-05-25 Dinar Temirbulatov dtemirbula...@gmail.com Alexey Kravets mr.kayr...@gmail.com * config/arm/arm.c (arm_rtx_costs_1): Add cost estimate for the integer double-word division operation. * config/mips/mips.c (mips_rtx_costs): Extend cost estimate for the integer double-word division operation for 32-bit targets. * gcc/expmed.c (expand_mult_highpart_optab): Allow to generate the higher multipilcation product for unsigned double-word integers using 32-bit wide registers. 22.patch Description: Binary data
[patch] Fix PR lto/52178 (continued)
This is a follow-up to PR lto/52178: there are more type mismatch in component
reference issues related to variably_modified_type_p when you try to build
the gnattools with -flto:
1. Instances of a record type with fixed size at top level aren't merged if
it has a field with self-referential size because variably_modified_type_p
returns true for the latter. I think the fix is straightforward: return
false for the special PLACEHOLDER_EXPR marker that is set in this case by
free_lang_data_in_one_sizepos.
2. Twin variably_modified_type_p qualified union types that belong to two
different functions may appear in the same COMPONENT_REF expression,
leading to a type mismatch. This occurs after a variably_modified_type_p
qualified union in the first function is remapped during the versioning
done by ipa-split to create the second function, and remapping overwrites
the type of a FIELD_DECL of the original type.
Proposed patch attached, bootstrapped/regtested on x86_64-suse-linux and LTO
bootstrapped on the same platform. #1 is a regression from 4.6 and #2 has
been exposed by ipa-split (although it's probably older) so I'd like to put
this on the 4.7 branch as well.
2012-05-25 Eric Botcazou ebotca...@adacore.com
PR lto/52178
* tree-inline.c (remap_gimple_op_r): Fix handling of FIELD_DECL.
* tree.c (RETURN_TRUE_IF_VAR): Do not return true for PLACEHOLDER_EXPR.
--
Eric Botcazou
Index: tree.c
===
--- tree.c (revision 187868)
+++ tree.c (working copy)
@@ -8477,8 +8477,11 @@ variably_modified_type_p (tree type, tre
a variable in FN. */
#define RETURN_TRUE_IF_VAR(T) \
do { tree _t = (T); \
-if (_t _t != error_mark_node TREE_CODE (_t) != INTEGER_CST \
- (!fn || walk_tree (_t, find_var_from_fn, fn, NULL))) \
+if (_t != NULL_TREE \
+ _t != error_mark_node \
+ TREE_CODE (_t) != INTEGER_CST\
+ TREE_CODE (_t) != PLACEHOLDER_EXPR\
+ (!fn || walk_tree (_t, find_var_from_fn, fn, NULL))) \
return true; } while (0)
if (type == error_mark_node)
Index: tree-inline.c
===
--- tree-inline.c (revision 187868)
+++ tree-inline.c (working copy)
@@ -818,6 +818,15 @@ remap_gimple_op_r (tree *tp, int *walk_s
|| decl_function_context (*tp) == id-src_fn))
/* These may need to be remapped for EH handling. */
*tp = remap_decl (*tp, id);
+ else if (TREE_CODE (*tp) == FIELD_DECL)
+{
+ /* If the enclosing record type is variably_modified_type_p, the field
+ has already been remapped. Otherwise, it need not be. */
+ tree *n = (tree *) pointer_map_contains (id-decl_map, *tp);
+ if (n)
+ *tp = *n;
+ *walk_subtrees = 0;
+}
else if (TYPE_P (*tp))
/* Types may need remapping as well. */
*tp = remap_type (*tp, id);
Re: [patch] Fix PR lto/52178 (continued)
On Fri, May 25, 2012 at 12:27 PM, Eric Botcazou ebotca...@adacore.com wrote: This is a follow-up to PR lto/52178: there are more type mismatch in component reference issues related to variably_modified_type_p when you try to build the gnattools with -flto: 1. Instances of a record type with fixed size at top level aren't merged if it has a field with self-referential size because variably_modified_type_p returns true for the latter. I think the fix is straightforward: return false for the special PLACEHOLDER_EXPR marker that is set in this case by free_lang_data_in_one_sizepos. 2. Twin variably_modified_type_p qualified union types that belong to two different functions may appear in the same COMPONENT_REF expression, leading to a type mismatch. This occurs after a variably_modified_type_p qualified union in the first function is remapped during the versioning done by ipa-split to create the second function, and remapping overwrites the type of a FIELD_DECL of the original type. Proposed patch attached, bootstrapped/regtested on x86_64-suse-linux and LTO bootstrapped on the same platform. #1 is a regression from 4.6 and #2 has been exposed by ipa-split (although it's probably older) so I'd like to put this on the 4.7 branch as well. Ok. Please make sure to verify LTO bootstrap on the branch still works after this. Thanks, Richard. 2012-05-25 Eric Botcazou ebotca...@adacore.com PR lto/52178 * tree-inline.c (remap_gimple_op_r): Fix handling of FIELD_DECL. * tree.c (RETURN_TRUE_IF_VAR): Do not return true for PLACEHOLDER_EXPR. -- Eric Botcazou
[AARCH64] [PATCH 0/3] AArch64 Port
ARM is pleased to announce a port of GCC to its AArch64 architecture. Please note that while the compiler has been used to build a large body of software, it cannot yet be considered complete. We expect there to be a number of as yet undiscovered bugs and other issues that will need to be resolved before the code can be integrated into the main compiler. Nevertheless, we believe that the code is now in a state where it is worth starting the process of a public review so that we can address the outstanding issues in time for the GCC-4.8 release. Perhaps one of the more contentious design issues that we have made is to produce a compiler that is separate from the existing ARM compiler. We spent quite a bit of time debating this internally, but concluded that keeping the sources separate was the correct answer: - There is no function-call-level compatibility between the AArch64 and AArch32 states; interworking can only occur at the exception level boundary (eg between user and kernel states). - The A64 instruction set differs from ARM in several significant ways - There are more registers - There is no generalized conditional execution - Register naming is substantially different - There is at best only a similarity between the A64 and the A32 and T32 mnemonics. In particular, although Neon provides similar functionality, the instructions are very different. - The floating-point register architecture in AArch64 differs from that in AArch32. - The majority of the existing ARM-specific command-line options do not apply to AArch64; conflating the two would be confusing to users - Very little code in the port back-end would be directly shared; and in addition to the maintenance overhead, sharing the code would create a performance overhead at run time. - By starting from scratch we've been able to fully utilize some of the new constructs that GCC now contains. Perhaps the only area where there is potentially significant opportunity for sharing is in the pipeline descriptions. However, we believe that in this case it should be feasible to make use of the same description in both back-ends. There are still a number of components that require some further work. Issues that we are aware of (at a coarse level of granularity) include: - Improving the arm_neon.h implementation and auto-vectorization - Support for languages beyond C, C++ and Fortran - Atomic optab support - Further memory and TLS models - Some of the extended compilation modes, such as profile support, libmudflap, stack-protector. Some of these will be much easier to complete if we can openly discuss the issues with community experts. The sources can be found in SVN in svn://gcc.gnu.org/gcc/branches/ARM/aarch64-branch. This branch will track trunk until such time as the code is merged into trunk. Patches to this branch should be tagged [AARCH64] and must be approved by ARM personnel.
[AARCH64] [PATCH 3/3] AArch64 Port
This patch adds an implementation of integer iterators.
Index: gcc/ChangeLog.aarch64
* read-rtl.c (rtx_list): New data structure.
(int_iterator_mapping): New data structure.
(int_iterator_data): New. List of int iterator details.
(num_int_iterator_data): New.
(ints): New group list.
(find_int): New. Find an int iterator in a list.
(dummy_uses_int_iterator): Dummy handle.
(dummy_apply_int_iterator): Dummy handle.
(uses_int_iterator_p): New.
(apply_iterator_to_rtx): Handle case for rtx field specifier 'i'.
(initialize_iterators): Initialize int iterators data struts.
(find_int_iterator): New. Find an Int iterators from a hash-table.
(add_int_iterator: Add int iterator to database.
(read_rtx): Parse and read int iterators mapping and attributes.
Initialize int iterators group's hash-table. Memory management.
(read_rtx_code): Handle case for rtl field specifier 'i'.Index: gcc/read-rtl.c
===
--- gcc/read-rtl.c (revision 187870)
+++ gcc/read-rtl.c (working copy)
@@ -94,6 +94,26 @@
#define BELLWETHER_CODE(CODE) \
((CODE) NUM_RTX_CODE ? CODE : bellwether_codes[CODE - NUM_RTX_CODE])
+/* One element in the (rtx, opno) pair list. */
+struct rtx_list {
+ /* rtx. */
+ rtx x;
+ /* Position of the operand to replace. */
+ int opno;
+};
+
+/* A structure to track which rtx uses which int iterator. */
+struct int_iterator_mapping {
+ /* Iterator. */
+ struct mapping *iterator;
+ /* list of rtx using ITERATOR. */
+ struct rtx_list *rtxs;
+ int num_rtx;
+};
+
+static struct int_iterator_mapping *int_iterator_data;
+static int num_int_iterator_data;
+
static int find_mode (const char *);
static bool uses_mode_iterator_p (rtx, int);
static void apply_mode_iterator (rtx, int);
@@ -121,8 +141,8 @@
static rtx read_nested_rtx (struct map_value **);
static rtx read_rtx_variadic (struct map_value **, rtx);
-/* The mode and code iterator structures. */
-static struct iterator_group modes, codes;
+/* The mode, code and int iterator structures. */
+static struct iterator_group modes, codes, ints;
/* Index I is the value of BELLWETHER_CODE (I + NUM_RTX_CODE). */
static enum rtx_code *bellwether_codes;
@@ -179,6 +199,59 @@
PUT_CODE (x, (enum rtx_code) code);
}
+/* Since GCC does not construct a table of valid constants,
+ we have to accept any int as valid. No cross-checking can
+ be done. */
+static int
+find_int (const char *name)
+{
+ char *endptr;
+ int ret;
+
+ if (ISDIGIT (*name))
+{
+ ret = strtol (name, endptr, 0);
+ gcc_assert (*endptr == '\0');
+ return ret;
+}
+ else
+fatal_with_file_and_line (unknown int `%s', name);
+}
+
+static bool
+dummy_uses_int_iterator (rtx x ATTRIBUTE_UNUSED, int index ATTRIBUTE_UNUSED)
+{
+ return false;
+}
+
+static void
+dummy_apply_int_iterator (rtx x ATTRIBUTE_UNUSED, int code ATTRIBUTE_UNUSED)
+{
+ /* Do nothing. */
+}
+
+/* Stand-alone int iterator usage-checking function. */
+static bool
+uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
+{
+ int i;
+ for (i=0; i num_int_iterator_data; i++)
+if (int_iterator_data[i].iterator-group == iterator-group
+ int_iterator_data[i].iterator-index == iterator-index)
+ {
+ /* Found an existing entry. Check if X is in its list. */
+ struct int_iterator_mapping it = int_iterator_data[i];
+ int j;
+
+ for (j=0; j it.num_rtx; j++)
+ {
+ if (it.rtxs[j].x == x it.rtxs[j].opno == opno)
+ return true;
+ }
+ }
+ return false;
+}
+
/* Map a code or mode attribute string P to the underlying string for
ITERATOR and VALUE. */
@@ -341,7 +414,9 @@
x = rtx_alloc (bellwether_code);
memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
- /* Change the mode or code itself. */
+ /* Change the mode or code itself.
+ For int iterators, apply_iterator () does nothing. This is
+ because we want to apply int iterators to operands below. */
group = iterator-group;
if (group-uses_iterator_p (x, iterator-index + group-num_builtins))
group-apply_iterator (x, value);
@@ -379,6 +454,10 @@
unknown_mode_attr);
}
break;
+ case 'i':
+ if (uses_int_iterator_p (original, iterator, i))
+ XINT (x, i) = value;
+ break;
default:
break;
@@ -419,6 +498,10 @@
return true;
break;
+ case 'i':
+ if (uses_int_iterator_p (x, iterator, i))
+ return true;
+
default:
break;
}
@@ -480,6 +563,7 @@
iterator = (struct mapping *) *slot;
for (elem = mtd-queue; elem != 0; elem = XEXP (elem, 1))
+ {
if (uses_iterator_p (XEXP (elem, 0), iterator))
{
/* For each iterator we expand, we set UNKNOWN_MODE_ATTR to NULL.
@@ -509,6 +593,7 @@
XEXP (elem, 0) = x;
}
}
+ }
return 1;
}
@@ -553,7 +638,7 @@
return value-next;
}
-/* Do one-time initialization
Re: [AARCH64] [PATCH 1/3] AArch64 Port
On Fri, May 25, 2012 at 12:19:01PM +0100, Marcus Shawcroft wrote: ... * config/aarch64/aarch64-simd.md: New file. ... * config/aarch64/iterators.md: New file. With the above in the same ChangeLog entry * config/aarch64/aarch64-simd.md (reduc_smax_v4sf, reduc_smin_v4sf): Factor into... (reduc_sfmaxminv_v4sf): ...this. (reduc_smax_mode, reduc_smin_mode): Factor into... (reduc_sfmaxminv_mode): ...this. (reduc_smax_mode, reduc_smin_mode, reduc_umax_mode, reduc_umin_mode): Factor into... (reduc_maxminv_mode): ...this. (reduc_smax_v2si, reduc_smin_v2si, reduc_umax_v2si): Factor into... (reduc_maxminv_v2si): ...this. * config/aarch64/iterators.md (MAXMINV): New. (FMAXMINV): New. (maxminv): New. (fmaxminv): New. these lines don't make any sense. Jakub
Re: [libgo] Fix IRIX bootstrap failure
Hi Ian, Thanks. I committed the patch to mainline and 4.7 branch. Thanks. Testsuite results are still terrible since many tests timeout (with timeout doubled to 600s to account for the slow 250 MHz MIPS R10k CPUs on my test machine), and others not even hitting the timeout at all: PID USERNAME PRI NICE SIZE RES STATE TIME WCPUCPU COMMAND 35782082 ro 200 784M 3696K ready 357:50 29.59% 59.18% goprint.ex 33666480 ro 200 784M 5568K ready 437:14 29.56% 59.11% select5.ex 35957493 ro 200 784M 5264K run/2 206:40 29.43% 58.86% 64bit.exe 35654428 ro 200 784M 5072K ready 432:59 29.41% 58.82% index.exe They all seem to loop in a SIGSEGV handling loop. I don't know what is happening here. I plan to have a look sometime, but unfortunately I don't have much time for GCC work these days. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [AARCH64] [PATCH 1/3] AArch64 Port
On 25/05/12 12:27, Jakub Jelinek wrote: On Fri, May 25, 2012 at 12:19:01PM +0100, Marcus Shawcroft wrote: ... * config/aarch64/aarch64-simd.md: New file. ... * config/aarch64/iterators.md: New file. With the above in the same ChangeLog entry * config/aarch64/aarch64-simd.md (reduc_smax_v4sf, reduc_smin_v4sf): Factor into... (reduc_sfmaxminv_v4sf): ...this. (reduc_smax_mode, reduc_smin_mode): Factor into... (reduc_sfmaxminv_mode): ...this. (reduc_smax_mode, reduc_smin_mode, reduc_umax_mode, reduc_umin_mode): Factor into... (reduc_maxminv_mode): ...this. (reduc_smax_v2si, reduc_smin_v2si, reduc_umax_v2si): Factor into... (reduc_maxminv_v2si): ...this. * config/aarch64/iterators.md (MAXMINV): New. (FMAXMINV): New. (maxminv): New. (fmaxminv): New. these lines don't make any sense. Thanks for pointing this out. I will correct the entry. /Marcus
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 1:25 AM, Jakub Jelinek ja...@redhat.com wrote: Static functions are also not acceptable as template arguments, so this patch externalizes the functions. To avoid potential name conflicts, the function names have been prefixed. Ugh. I guess the C++ way around this would be to put the functions into anonymous namespace. Anonymous namespaces have the problem that they may interfere with bootstrap diff-compare. -- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
On Thu, May 24, 2012 at 4:55 PM, Diego Novillo dnovi...@google.com wrote: On 12-05-24 17:52 , Lawrence Crowl wrote: That said, I'll let y'all decide how much to put in any one piece. I favour the approach we are taking now. Each patch to cxx-conversion is a small incremental step. When we merge into trunk we'll merge the final product of each change, of course. But with the minimalistic approach, we can discuss and understand each change better. I agree. Lawrence satisfactorily answered my main concern with the patch he posted (the template argument); so I am board modulo the change on class name. -- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi,
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+return htab-size;
+ }
(and various other places) - formatting is wrong, missing space between (.
And it doesn't start at the first column, and type isn't on a separate
line. I realize that this is a member method, hence indenting and C GNU
coding standards conflict, but the latter do have some nice properties
(for instance that 'grep ^func_name *.c' finds only the definition, not
all the calls to func_name).
I think we need a discussion about style, now that actually people are
working on this.
+void
+typed_htab Element, Hash, Equal, Remove, Allocator
+::dispose ()
Do we want to format methods this way?
Don't know, but at least it starts at the first column, and grepping for
^:: will give only C++ methods, 'grep -B1 ^::' even including class. So
it's no too horrible, despite the syntactic C++ noise.
Ciao,
Michael.
Re: RFC: PATCH to correct the scopes of various types
On Wed, May 2, 2012 at 1:08 PM, Jason Merrill ja...@redhat.com wrote: I've been working on a patch to split out the DWARF debug info for COMDAT functions into COMDAT CUs to go in the same group. One problem I ran into was that modified versions of function-local types (pointers, cv-qualified variants, etc) were ending up at global scope. Separately, Tom Tromey asked me why GCC was using a declaration/definition pair for namespace-scope types rather than just defining them in the namespace. This patch fixes both of those issues; it also fixes an issue with typedef scoping that I haven't been able to reproduce more recently. Tested x86_64-pc-linux-gnu. Does this make sense to you? This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53470 -- H.J.
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
Alternatively you could put the static assertion with the comment into the l field, i.e. /* Statically assert that N is 2 or 4. */ unsigned long l[(N == 2 || N == 4) ? N : -1]; I certainly prefer this alternative (the use of extern for that purpose being extremely confusing for a fortraner;-). However, if my test is valid, if N is not equal to 2 or 4, one gets the following error error: size of array 'l' is negative Would not it be better to have an explicit test of N being 2 or 4, and an error of the kind the vector size has to be the size of 2 or 4 long if not? I'll test the above change tonight: allow for ~14h for a clean bootstrap and Dominique
[PATCH] PR debug/53453 ensure debug notes linked on darwin
The attached patch solves PR53453. The upcoming dsymutil release will now
issue
a warning when no debug notes are emitted for an object file on linkage. This
causes
several thousand failures in the FSF gcc testsuite. The origin of this problem
is
that the darwin linker requires the presence of both AT_name and AT_comp_dir
attributes
for each object file otherwise their debug notes will be omitted on linkage.
The
patch adds a new TARGET_FORCE_AT_COMP_DIR target hook for darwin so that
AT_comp_dir
will be present for all object files. Bootstrap and regression tested on
x86_64-apple-darwin12.
http://gcc.gnu.org/ml/gcc-testresults/2012-05/msg02331.html
Okay for gcc trunk and later gcc-4_7-branch and gcc-4_6-branch?
Jack
ps Note that this is a pre-existing problem on darwin and that the newer
dsymutil simply
has alerted us to the issue.
2012-05-24 Jack Howarth howa...@bromo.med.uc.edu
PR debug/53453
* doc/tm.texi: Update.
* doc/tm.texi.in (SDB and DWARF) TARGET_FORCE_AT_COMP_DIR: Add @hook.
* target-def (force_at_comp_dir): New hook.
* config/darwin.h (TARGET_FORCE_AT_COMP_DIR): Define.
* dwarf2out.c (dwarf2out_finish): Check targetm.force_at_comp_dir.
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in (revision 187856)
+++ gcc/doc/tm.texi.in (working copy)
@@ -9392,6 +9392,8 @@ tables, and hence is desirable if it wor
@hook TARGET_WANT_DEBUG_PUB_SECTIONS
+@hook TARGET_FORCE_AT_COMP_DIR
+
@hook TARGET_DELAY_SCHED2
@hook TARGET_DELAY_VARTRACK
Index: gcc/target.def
===
--- gcc/target.def (revision 187856)
+++ gcc/target.def (working copy)
@@ -2749,6 +2749,13 @@ DEFHOOKPOD
bool, false)
DEFHOOKPOD
+(force_at_comp_dir,
+ True if the @code{AT_comp_dir} attribute should be emitted for each \
+ compilation unit. This attribute is required for the darwin linker \
+ to emit debug information.,
+ bool, false)
+
+DEFHOOKPOD
(delay_sched2, True if sched2 is not to be run at its normal place. \
This usually means it will be run as part of machine-specific reorg.,
bool, false)
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 187856)
+++ gcc/dwarf2out.c (working copy)
@@ -21949,7 +21949,7 @@ dwarf2out_finish (const char *filename)
/* Add the name for the main input file now. We delayed this from
dwarf2out_init to avoid complications with PCH. */
add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
- if (!IS_ABSOLUTE_PATH (filename))
+ if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
add_comp_dir_attribute (comp_unit_die ());
else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
{
Index: gcc/config/darwin.h
===
--- gcc/config/darwin.h (revision 187856)
+++ gcc/config/darwin.h (working copy)
@@ -414,6 +414,8 @@ extern GTY(()) int darwin_ms_struct;
#define TARGET_WANT_DEBUG_PUB_SECTIONS true
+#define TARGET_FORCE_AT_COMP_DIR true
+
/* When generating stabs debugging, use N_BINCL entries. */
#define DBX_USE_BINCL
[PATCH] omit -lcrt1.10.6.o and pass -no_new_main with -pg on darwin = 10.8
The attached patch limits the linkage of -lcrt1.10.6.o to darwin10 and
darwin11
since its usage is deprecated in the 10.8sdk. The patch also solves
radr://11491405,
-pg broken for -mmacosx-version-min=10.8...
19-May-2012 11:10 PM Jack Howarth:
Summary: The default -mmacosx-version-min=10.8 under Xcode 4.4 breaks a number
of FSF gcc tests which rely on -pg. such as...
FAIL: gcc.dg/nest.c execution test
FAIL: gcc.dg/nested-func-4.c execution test
Steps to Reproduce:
The attached compressed archive for xcodebug_pg_10.8.tar.bz2 contains the files
from compiling these test cases with...
gcc-fsf-4.7
/sw/src/fink.build/gcc47-4.7.1-1000/gcc-4.7-20120518/gcc/testsuite/gcc.dg/nest.c
--save-temps -v -O2 -pg -lm -m32 -o ./nest.exe
and
gcc-fsf-4.7
/sw/src/fink.build/gcc47-4.7.1-1000/gcc-4.7-20120518/gcc/testsuite/gcc.dg/nested-func-4.c
-v --save-temps -pg -lm -m32 -o ./nested-func-4.exe
as well as shell scripts for bad_runs.sh and bad_runs2.sh to execute each of
the miscompiled binaries against the bundled FSF gcc shared library for
libgcc_s.1.dylib.
Expected Results:
The nest.exe and nested-func-4.exe test cases should execute without errors as
they do when built with the -mmacosx-version-min=10.7 flag.
Actual Results:
In both cases, these test case fail with the error...
dyld: no valid entry point
The patch implements the recommended approach from the darwin linker developer
of passing -no_new_main to the linker
when targeting 10.8 or later which tells the linker to look for start instead
of _main. Otherwise the linker sets up
_main as the entry point and _moninit is never called, resulting in a runtime
error. Bootstrap and regression tested
on x86_64-apple-darwin12.
http://gcc.gnu.org/ml/gcc-testresults/2012-05/msg02331.html
Okay for gcc trunk and later gcc-4_7-branch and gcc-4_6-branch?
Jack
2012-05-24 Jack Howarth howa...@bromo.med.uc.edu
* config/darwin.h (STARTFILE_SPEC): Use -no_new_main with -lgcrt1.o
on Darwin = 12.
(DARWIN_CRT1_SPEC): Use -lcrt1.10.6.o when Darwin = 10 and 12.
--- gcc/config/darwin.h (revision 187856)
+++ gcc/config/darwin.h (working copy)
@@ -356,7 +356,9 @@ extern GTY(()) int darwin_ms_struct;
%{!Zbundle:%{pg:%{static:-lgcrt0.o} \
%{!static:%{object:-lgcrt0.o} \
%{!object:%{preload:-lgcrt0.o} \
- %{!preload:-lgcrt1.o %(darwin_crt2)\
+ %{!preload:-lgcrt1.o \
+ %:version-compare(= 10.8
mmacosx-version-min= -no_new_main) \
+ %(darwin_crt2)\
%{!pg:%{static:-lcrt0.o} \
%{!static:%{object:-lcrt0.o} \
%{!object:%{preload:-lcrt0.o} \
@@ -379,7 +381,7 @@ extern GTY(()) int darwin_ms_struct;
#define DARWIN_CRT1_SPEC \
%:version-compare(! 10.5 mmacosx-version-min= -lcrt1.o)\
%:version-compare( 10.5 10.6 mmacosx-version-min= -lcrt1.10.5.o) \
- %:version-compare(= 10.6 mmacosx-version-min= -lcrt1.10.6.o) \
+ %:version-compare( 10.6 10.8 mmacosx-version-min= -lcrt1.10.6.o) \
%{fgnu-tm: -lcrttms.o}
/* Default Darwin ASM_SPEC, very simple. */
Re: [PATCH] PR debug/53453 ensure debug notes linked on darwin
On Fri, May 25, 2012 at 09:03:02AM -0400, Jack Howarth wrote:
DEFHOOKPOD
+(force_at_comp_dir,
+ True if the @code{AT_comp_dir} attribute should be emitted for each \
It is DW_AT_comp_dir, so at least the comment should get it right.
Jakub
Re: [AARCH64] [PATCH 3/3] AArch64 Port
On Fri, May 25, 2012 at 1:19 PM, Marcus Shawcroft
marcus.shawcr...@arm.com wrote:
This patch adds an implementation of integer iterators.
Index: gcc/ChangeLog.aarch64
* read-rtl.c (rtx_list): New data structure.
(int_iterator_mapping): New data structure.
(int_iterator_data): New. List of int iterator details.
(num_int_iterator_data): New.
(ints): New group list.
(find_int): New. Find an int iterator in a list.
(dummy_uses_int_iterator): Dummy handle.
(dummy_apply_int_iterator): Dummy handle.
(uses_int_iterator_p): New.
(apply_iterator_to_rtx): Handle case for rtx field specifier 'i'.
(initialize_iterators): Initialize int iterators data struts.
(find_int_iterator): New. Find an Int iterators from a hash-table.
(add_int_iterator: Add int iterator to database.
(read_rtx): Parse and read int iterators mapping and attributes.
Initialize int iterators group's hash-table. Memory management.
(read_rtx_code): Handle case for rtl field specifier 'i'.
Hello,
Can you please use diff -up (or svn diff -x -up) when you post a
patch? With -p it is easier to see what functions you have modified.
Can you please add documentation for this new iterator (I suppose in
doc/md.texi??).
@@ -480,6 +563,7 @@
iterator = (struct mapping *) *slot;
for (elem = mtd-queue; elem != 0; elem = XEXP (elem, 1))
+ {
if (uses_iterator_p (XEXP (elem, 0), iterator))
{
/* For each iterator we expand, we set UNKNOWN_MODE_ATTR to NULL.
@@ -509,6 +593,7 @@
XEXP (elem, 0) = x;
}
}
+ }
return 1;
}
Not sure what this change is for. In any case, indentation does not
conform to GCC coding conventions (I don't think everyone is
particularly fond of them, but we should follow them nonetheless ;-)
Ciao!
Steven
[PATCH][revised] PR debug/53453 ensure debug notes linked on darwin
The attached patch solves PR53453. The upcoming dsymutil release will now
issue
a warning when no debug notes are emitted for an object file on linkage. This
causes
several thousand failures in the FSF gcc testsuite. The origin of this problem
is
that the darwin linker requires the presence of both AT_name and AT_comp_dir
attributes
for each object file otherwise their debug notes will be omitted on linkage.
The
patch adds a new TARGET_FORCE_AT_COMP_DIR target hook for darwin so that
AT_comp_dir
will be present for all object files. Bootstrap and regression tested on
x86_64-apple-darwin12.
http://gcc.gnu.org/ml/gcc-testresults/2012-05/msg02331.html
Okay for gcc trunk and later gcc-4_7-branch and gcc-4_6-branch?
Jack
ps Note that this is a pre-existing problem on darwin and that the newer
dsymutil simply
has alerted us to the issue.
pps Added correction of s/AT_comp_dir/DW_AT_comp_dir/g to gcc/target.def change.
2012-05-24 Jack Howarth howa...@bromo.med.uc.edu
PR debug/53453
* doc/tm.texi: Update.
* doc/tm.texi.in (SDB and DWARF) TARGET_FORCE_AT_COMP_DIR: Add @hook.
* target-def (force_at_comp_dir): New hook.
* config/darwin.h (TARGET_FORCE_AT_COMP_DIR): Define.
* dwarf2out.c (dwarf2out_finish): Check targetm.force_at_comp_dir.
Index: gcc/doc/tm.texi.in
===
--- gcc/doc/tm.texi.in (revision 187856)
+++ gcc/doc/tm.texi.in (working copy)
@@ -9392,6 +9392,8 @@ tables, and hence is desirable if it wor
@hook TARGET_WANT_DEBUG_PUB_SECTIONS
+@hook TARGET_FORCE_AT_COMP_DIR
+
@hook TARGET_DELAY_SCHED2
@hook TARGET_DELAY_VARTRACK
Index: gcc/target.def
===
--- gcc/target.def (revision 187856)
+++ gcc/target.def (working copy)
@@ -2749,6 +2749,13 @@ DEFHOOKPOD
bool, false)
DEFHOOKPOD
+(force_at_comp_dir,
+ True if the @code{DW_AT_comp_dir} attribute should be emitted for each \
+ compilation unit. This attribute is required for the darwin linker \
+ to emit debug information.,
+ bool, false)
+
+DEFHOOKPOD
(delay_sched2, True if sched2 is not to be run at its normal place. \
This usually means it will be run as part of machine-specific reorg.,
bool, false)
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 187856)
+++ gcc/dwarf2out.c (working copy)
@@ -21949,7 +21949,7 @@ dwarf2out_finish (const char *filename)
/* Add the name for the main input file now. We delayed this from
dwarf2out_init to avoid complications with PCH. */
add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
- if (!IS_ABSOLUTE_PATH (filename))
+ if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
add_comp_dir_attribute (comp_unit_die ());
else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
{
Index: gcc/config/darwin.h
===
--- gcc/config/darwin.h (revision 187856)
+++ gcc/config/darwin.h (working copy)
@@ -414,6 +414,8 @@ extern GTY(()) int darwin_ms_struct;
#define TARGET_WANT_DEBUG_PUB_SECTIONS true
+#define TARGET_FORCE_AT_COMP_DIR true
+
/* When generating stabs debugging, use N_BINCL entries. */
#define DBX_USE_BINCL
Re: [AARCH64] [PATCH 1/3] AArch64 Port
On Fri, May 25, 2012 at 1:19 PM, Marcus Shawcroft marcus.shawcr...@arm.com wrote: * config/aarch64/aarch64.md: New file. Index: gcc/config/aarch64/aarch64.md === --- gcc/config/aarch64/aarch64.md (revision 0) +++ gcc/config/aarch64/aarch64.md (revision 0) @@ -0,0 +1,2935 @@ (...) +(define_constants + [ +(UNSPEC_NOP0) +(UNSPEC_TLS1) Can you use define_c_enum unspec here instead? +(define_constants + [ +(UNSPECV_EH_RETURN 0) +(UNSPECV_SYNC_COMPARE_AND_SWAP 1) And define_c_enum unspecv here? With define_c_enum your constants will be printed as names in the rtl dumps. Ciao! Steven
PR middle-end/53008 (trans-mem): output clone if function accessed indirectly
This is a patch from Patrick, based on an earlier patch by Dave
Boutcher. Thanks folks.
In the failing testcase below we have a transaction_safe function being
accessed indirectly, but for -O1 and above, the corresponding clone is
not generated because we think it is unused. Fixed by forcing the clone
output if it is accessed indirectly.
Tested on x86-64 Linux.
OK? Would this be acceptable for the 4.7 branch as well?
PR middle-end/53008
* trans-mem.c (ipa_tm_create_version_alias): Output new_node if
accessed indirectly.
(ipa_tm_create_version): Same.
Index: testsuite/gcc.dg/tm/pr53008.c
===
--- testsuite/gcc.dg/tm/pr53008.c (revision 0)
+++ testsuite/gcc.dg/tm/pr53008.c (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options -fgnu-tm -O } */
+
+void __attribute__((transaction_safe)) (*fn)(void);
+
+static void __attribute__((transaction_safe))
+foo(void)
+{
+}
+
+void set_fn(void)
+{
+ fn = foo;
+}
Index: trans-mem.c
===
--- trans-mem.c (revision 187729)
+++ trans-mem.c (working copy)
@@ -4328,7 +4328,8 @@ ipa_tm_create_version_alias (struct cgra
record_tm_clone_pair (old_decl, new_decl);
- if (info-old_node-symbol.force_output)
+ if (info-old_node-symbol.force_output
+ || ipa_ref_list_first_referring (info-old_node-symbol.ref_list))
ipa_tm_mark_force_output_node (new_node);
return false;
}
@@ -4381,7 +4382,8 @@ ipa_tm_create_version (struct cgraph_nod
record_tm_clone_pair (old_decl, new_decl);
cgraph_call_function_insertion_hooks (new_node);
- if (old_node-symbol.force_output)
+ if (old_node-symbol.force_output
+ || ipa_ref_list_first_referring (old_node-symbol.ref_list))
ipa_tm_mark_force_output_node (new_node);
/* Do the same thing, but for any aliases of the original node. */
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+ return htab-size;
+ }
(and various other places) - formatting is wrong, missing space between (.
And it doesn't start at the first column, and type isn't on a separate
line. I realize that this is a member method, hence indenting and C GNU
coding standards conflict, but the latter do have some nice properties
(for instance that 'grep ^func_name *.c' finds only the definition, not
all the calls to func_name).
I think we need a discussion about style, now that actually people are
working on this.
Note also the almost 2 decades of C++ style practice in our libstdc++
implementation.
-- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi, On Fri, 25 May 2012, Gabriel Dos Reis wrote: (and various other places) - formatting is wrong, missing space between (. And it doesn't start at the first column, and type isn't on a separate line. I realize that this is a member method, hence indenting and C GNU coding standards conflict, but the latter do have some nice properties (for instance that 'grep ^func_name *.c' finds only the definition, not all the calls to func_name). I think we need a discussion about style, now that actually people are working on this. Note also the almost 2 decades of C++ style practice in our libstdc++ implementation. That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Ciao, Michael.
Re: PATCH: PR target/53383: Allow -mpreferred-stack-boundary=3 on x86-64
On Sun, May 20, 2012 at 7:47 AM, H.J. Lu hongjiu...@intel.com wrote:
Hi,
This patch allows -mpreferred-stack-boundary=3 on x86-64 when SSE is
disabled. Since this option changes ABI, I also added a warning for
-mpreferred-stack-boundary=3. OK for trunk?
Thanks.
H.J.
PR target/53383
* doc/invoke.texi: Add a warning for -mpreferred-stack-boundary=3.
* config/i386/i386.c (ix86_option_override_internal): Allow
-mpreferred-stack-boundary=3 for 64-bit if SSE is disenabled.
* config/i386/i386.h (MIN_STACK_BOUNDARY): Set to 64 for 64-bit
if SSE is disenabled.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index eca542c..338d387 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3660,7 +3660,7 @@ ix86_option_override_internal (bool main_args_p)
ix86_preferred_stack_boundary = PREFERRED_STACK_BOUNDARY_DEFAULT;
if (global_options_set.x_ix86_preferred_stack_boundary_arg)
{
- int min = (TARGET_64BIT ? 4 : 2);
+ int min = (TARGET_64BIT ? (TARGET_SSE ? 4 : 3) : 2);
int max = (TARGET_SEH ? 4 : 12);
if (ix86_preferred_stack_boundary_arg min
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index ddb3645..f7f13d2 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -708,7 +708,7 @@ enum target_cpu_default
#define MAIN_STACK_BOUNDARY (TARGET_64BIT ? 128 : 32)
/* Minimum stack boundary. */
-#define MIN_STACK_BOUNDARY (TARGET_64BIT ? 128 : 32)
+#define MIN_STACK_BOUNDARY (TARGET_64BIT ? (TARGET_SSE ? 128 : 64) : 32)
/* Boundary (in *bits*) on which the stack pointer prefers to be
aligned; the compiler cannot rely on having this alignment. */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4c5c79f..daa1f3a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13521,6 +13521,12 @@ Attempt to keep the stack boundary aligned to a 2
raised to @var{num}
byte boundary. If @option{-mpreferred-stack-boundary} is not specified,
the default is 4 (16 bytes or 128 bits).
+@strong{Warning:} When generating code for the x86-64 architecture with
+SSE extensions disabled, @option{-mpreferred-stack-boundary=3} can be
+used to keep the stack boundary aligned to 8 byte boundary. You must
+build all modules with @option{-mpreferred-stack-boundary=3}, including
+any libraries. This includes the system libraries and startup modules.
+
@item -mincoming-stack-boundary=@var{num}
@opindex mincoming-stack-boundary
Assume the incoming stack is aligned to a 2 raised to @var{num} byte
I applied the above patch to GCC 4.7 and the following patch to
Linux kernel 3.4.0. Kernel boots and runs correctly. Is the patch
OK for trunk?
Thanks.
--
H.J.
---
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 94e91e4..cd4a4f7 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -49,6 +49,9 @@ else
KBUILD_AFLAGS += -m64
KBUILD_CFLAGS += -m64
+ # Use -mpreferred-stack-boundary=3 if supported.
+ KBUILD_CFLAGS += $(call cc-option,-mno-sse -mpreferred-stack-boundary=3)
+
# FIXME - should be integrated in Makefile.cpu (Makefile_32.cpu)
cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 3:51 PM, Michael Matz m...@suse.de wrote: Hi, On Fri, 25 May 2012, Gabriel Dos Reis wrote: (and various other places) - formatting is wrong, missing space between (. And it doesn't start at the first column, and type isn't on a separate line. I realize that this is a member method, hence indenting and C GNU coding standards conflict, but the latter do have some nice properties (for instance that 'grep ^func_name *.c' finds only the definition, not all the calls to func_name). I think we need a discussion about style, now that actually people are working on this. Note also the almost 2 decades of C++ style practice in our libstdc++ implementation. That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Neither is the GNU C style, but we use it anyway. IMHO it'd be very strange to use one style in gcc itself, and another in libstdc++. Ciao! Steven
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 03:51:25PM +0200, Michael Matz wrote: On Fri, 25 May 2012, Gabriel Dos Reis wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. Jakub
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi, On Fri, 25 May 2012, Steven Bosscher wrote: Note also the almost 2 decades of C++ style practice in our libstdc++ implementation. That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Neither is the GNU C style, but we use it anyway. IMHO it is. IMHO it'd be very strange to use one style in gcc itself, and another in libstdc++. We already have a style for GCC, which is different from libstdc++. And mixing two styles _within_ one project source base (for the moment thinking about GCC and libstdc++ as two different projects) would be even worse. Ciao, Michael.
Re: [RFA PATCH, i386]: Fix gcc.target/i386/pr53249.c on Sun targets
Hi Uros, Currently gcc fails to compile following test from the testsuite [1]: FAIL: gcc.target/i386/pr53249.c (test for excess errors) We are trying to compile X32 specific test, but the special pattern that was introduced to handle certain sun assembler limitation, is not prepared to handle X32. The patch avoids this special pattern for X32 target. X32 is currently supported only with gnu tools, and if/when sun tools will support X32 target, they will certainly fix the limitation which allows TP loads to AX register only. I'll give the patch a whirl in my regular bootstraps this weekend. The Sun ld limitation is fixed in recent versions (at least Solaris 11, perhaps also backported to S10) and I do have a patch to detect this and turn off TARGET_SUN_TLS appropriately. Unfortunately, there's still a corner case in 64-bit TLS-IE handling that isn't handled correctly, but I haven't yet found to time to fully investigate this. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [cxx-conversion] New Hash Table (issue6244048)
On 12-05-25 09:56 , Jakub Jelinek wrote: On Fri, May 25, 2012 at 03:51:25PM +0200, Michael Matz wrote: On Fri, 25 May 2012, Gabriel Dos Reis wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. I don't care how ugly coding conventions look. I've dealt with many and they all have their ugly spots. I mostly care about consistency. I don't think we should deviate much from the established GNU standards (which are hideous, btw). Mostly because we are all used to them and it is going to be simpler to write new code that is similar to what is already there. Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). Lawrence, have you had a chance to update them with your latest edits? Diego.
Re: [AARCH64] [PATCH 1/3] AArch64 Port
General question: I suppose you expect to submit patches soon for
other toolchain components (such as binutils, GDB, glibc) and the
Linux kernel, if you haven't done so yet?
Index: config.guess
===
--- config.guess (revision 187870)
+++ config.guess (working copy)
@@ -4,7 +4,7 @@
# 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
# 2011 Free Software Foundation, Inc.
-timestamp='2011-06-03'
+timestamp='2012-05-25'
Instead of patching config.sub or config.guess, you should copy the
latest version, unmodified, from upstream config.git.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 187870)
+++ gcc/doc/extend.texi (working copy)
@@ -935,7 +935,8 @@
Not all targets support additional floating point types. @code{__float80}
and @code{__float128} types are supported on i386, x86_64 and ia64 targets.
-The @code{__float128} type is supported on hppa HP-UX targets.
+The @code{__float128} type is supported on hppa HP-UX targets and ARM AArch64
+targets.
I don't see any good reason to support it on AArch64, since it's the
same as long double there. (It's on PA HP-UX as a workaround for
libquadmath requiring the type rather than being able to with with a
type called either long double or __float128 - libquadmath being
used on PA HP-UX as a workaround for the system libm lacking much long
double support. But that shouldn't be an issue for new targets such
as AArch64 GNU/Linux. And my understanding from N1582 is that the C
bindings for IEEE 754-2008, being worked on for a five-part ISO/IEC
TS, are expected to use names such as _Float128, not __float128, as
standard names for supported IEEE floating-point types.)
+@opindex mbig-endian
+Generate big-endian code. This is the default when GCC is configured for an
+@samp{aarch64*be-*-*} target.
In general, throughout Texinfo changes, two spaces after . at the
end of a sentence.
+@item -march=@var{name}
+@opindex march
+Specify the name of the target architecture, optionally suffixed by one or
+more feature modifiers. This option has the form
+@samp{-march=arch[+[no]feature]}, where the only value for @samp{arch}
+is @samp{armv8}, and the possible values for @samp{feature} are
+@samp{crypto}, @samp{fp}, @samp{simd}.
It's unfortunate that you've chosen this complicated syntax that means
the generic support for enumerated option arguments cannot be used
(and so --help information cannot list supported CPUs and features).
A simpler syntax where -march takes just an architecture name and
features have separate options would seem better, and more in line
with most other architectures supported by GCC.
There are several Texinfo problems above. Instead of feature you
should use @var{feature}, and since the '[' and ']' are not literal
text they should be inside @r{} - the proper way of writing
@samp{-march=arch[+[no]feature]} would be
@option{-march=@var{arch}@r{[}+@r{[}no@r{]}@var{feature}@r{]}}.
Also, could you document what the feature names mean?
+@item -mcpu=@var{name}
+@opindex mcpu
+Specify the name of the target processor, optionally suffixed by one or more
+feature modifiers. This option has the form
@samp{-cpu=cpu[+[no]feature]},
+where the possible values for @samp{cpu} are @samp{generic}, @samp{large},
+and the possible values for @samp{feature} are @samp{crypto}, @samp{fp},
+@samp{simd}.
Same comments apply.
+This option is very similar to the -mcpu= option, except that instead of
@option{-mcpu=}. And does -mtune= take feature names or just plain CPU
names?
+ if (mvn == 0)
+ {
+ if (widthc != 'd')
+ sprintf (templ,movi\t%%0.%d%c, %%1, lsl %d ,(64/width),
+ widthc, shift);
+ else
+ sprintf (templ,movi\t%%d0, %%1);
+ }
+ else
+ sprintf (templ,mvni\t%%0.%d%c, %%1, lsl %d,(64/width),
+ widthc, shift);
Presumably you have some logic for why the 40-byte buffer size is
enough, but could you use snprintf with sizeof (templ) specified in
the call to protect against any mistakes in that logic? Also, spaces
after commas and around the / in the division, and the second line
in the function call should be lined up immediately after the opening
'(', not further right. (Check for and fix all these issues elsewhere
in the port as well; I've just pointed out a representative instance
of them.)
+__extension__ static __inline int8x8_t __attribute__ ((__always_inline__))
+vneg_s8 (int8x8_t a)
+{
+ int8x8_t result;
+ __asm__ (neg %0.8b,%1.8b
+ : =w(result)
+ : w(a)
+ : /* No clobbers */);
+ return result;
+}
I presume you've determined for each intrinsic represented as an asm
that it cannot usefully be described
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 8:57 AM, Michael Matz m...@suse.de wrote: Hi, On Fri, 25 May 2012, Steven Bosscher wrote: Note also the almost 2 decades of C++ style practice in our libstdc++ implementation. That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Neither is the GNU C style, but we use it anyway. IMHO it is. IMHO it'd be very strange to use one style in gcc itself, and another in libstdc++. We already have a style for GCC, which is different from libstdc++. And mixing two styles _within_ one project source base (for the moment thinking about GCC and libstdc++ as two different projects) would be even worse. The existing style for GCC made sense when gcc was originally designed with a deliberate flavour of writing Lisp in C. However, after 25 years, the codebase has a distinctively different flavour -- one that isn't Lispy anymore. While adopting a new implementation language, it makes sense to revisit and revise the style. It also makse sense to look at existing C++ style within the GCC project. That isn't imposing by any stretch of imagination or hyperbole. It would be dismaying if we just kept the old lispy style just because grand'pa did it that way in C. -- Gaby
Re: [PATCH] Add powerpc64-linux configuration options
On Thu, May 24, 2012 at 07:11:53PM -0400, David Edelsohn wrote: On Wed, May 23, 2012 at 6:36 PM, Michael Meissner meiss...@linux.vnet.ibm.com wrote: On powerpc64-linux systems that run on IBM servers, the 32-bit software emulation library is not built with the Red Hat and SUSE distributions, but the FSF sources still list it as a multilib. This patch adds a configuration option (--disable-ppc64-swfloat) to disable building this library. We've also seen some performance issues where building the multilibs with the -mstrict-align option causes slow downs. I vaguelly recall adding this option in the 1995 time frame, because the 603, 604 systems of the day did not handle unaligned accesses in little endian mode, and maybe some of the other embedded chips did not handle unaligned accesses at all. This patch also adds an option (--disable-ppc64-strict-align) to disable building the multilibs with -mstrict-align. I thought the use of -mstrict-align for multilibs was because of some re-use by embedded distros. That somehow one could build apps for embedded PowerPC processors without a full embedded cross-toolchain -- just using the regular toolchain with an embedded target command line option and the base libraries already worked. I am not suggesting that (ab)use is appropriate, but I don't want to break embedded environments. As I said, I believe the original motivation was that unaligned accesses did not work in little endian mode, or for floating point but given it was 17 years ago (and 4 or 5 different employers in my case), I don't remember the details as much. At least in the past, there are some differences between eabi programming and Linux (2 small data pointers vs. 1, some alignment differences). The first patch does this by keeping with the status quo and then if you want to suppress the two features, adding options. I could flip the sense of the options, and make it default to no software floating point and no strict align. Current SLES (Sles 10 and 11) and Red Hat (RHEL 5 and 6) right now disable software floating emulation by applying a patch in their rpm spec files to delete it. In fact having the trunk still supporting software emulation causes some problems as we build toolchains that use the host libraries. The IBM Advace Toolchain also eliminates the emulation libraries for the same reason. I can't tell whether yellowdog linux compiles with the emulation library multilib or not. At the end of the day, any of the 3 proposals (counting the first patch and swapping the defaults as a 3rd proposal) are fine with me. It is simplest to do the 2nd patch, but if we wanted to preserve the ability to do it, I would suggest the first patch with the defaults set not to build the library or use -mstrict-align would be best. Alan does have a point that the MULTILIB_EXTRA_OPTS is broken for the default libraries. If you configure the compiler with a default for 64-bit, it is mostly ok skipping the pic option or strict align. I agree that strict-align does not make sense for PPC64. I believe that is what you are accomplishing with the second version of your patch, correct? Yes, the second patch just does not build the library or use the -mstrict-align option. It is much simpler, but there may be somebody out there that depends on the functionality. I really don't know, one way or the other. -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Re: [AARCH64] [PATCH 2/3] AArch64 Port
On Fri, 25 May 2012, Marcus Shawcroft wrote:
Index: gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-4.x
===
--- gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-4.x (revision 0)
+++ gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-4.x (revision 0)
@@ -0,0 +1,5 @@
+if { [istarget aarch64_be-*-*] } then {
+ return 1
+}
+
+return 0
This isn't a suitable way of enabling a test only for one endianness,
since a test may be run with -mbig-endian or -mlittle-endian with a
compiler defaulting to the other endianness. You need to test an
effective-target keyword instead.
Index: gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-3.x
===
--- gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-3.x (revision 0)
+++ gcc/testsuite/gcc.target/aarch64/aapcs64/func-ret-3.x (revision 0)
@@ -0,0 +1,5 @@
+if { [istarget aarch64_be-*-*] } then {
+ return 1
+}
+
+return 0
Likewise.
+/* { dg-final { scan-assembler-times \\tsubhn2 v 6} } */
+/* { dg-final { scan-assembler-times \\taddhn2\\tv 6} } */
Scanning for lots of different instructions in a testcase testing many
intrinsics doesn't do a good job of testing that each intrinsic does
the right thing, or showing where the problem is when the test starts
to fail. I advise splitting such tests up into lots of little tests,
each testing just one intrinsic.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 9:15 AM, Diego Novillo dnovi...@google.com wrote: I don't care how ugly coding conventions look. I've dealt with many and they all have their ugly spots. I mostly care about consistency. Agreed that consistency is very important. (At at single time in a week, I deal with 3 different styles with 4 projects.) There are some nice properties with the existing GNU style; but they don't scale well with advanced scoping constructs (e.g. namespaces, classes, etc.) and we need to address those. Conversely the libstdc++ style good bits that we should look at. -- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 10:15:54AM -0400, Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). That page is quite inconsistent. E.g. it first talks about retaining space before ( (which I really hope will be retained, it makes code more readable), but in the example it doesn't bother with it. And the long line example is horribly ugly, ending line with ( ? That ( should surely go on the next line after the few spaces. It should list also examples of when the first parameter isn't too long, but there are too many parameters and so some wrapping is needed, etc. Jakub
Re: [PATCH] pr51020 Fix invalid options validation for ARM target
Hi guys,
This issue has not been fixed in latest Linaro release (4.6.2012.04) yet.
Can you suggest whether this patch is a correct fix for the pr51020.
If so, should it be applied to the corresponding gcc branch?
On Tue, May 01, 2012 at 11:21:28PM +0400, Alexey Kravets wrote:
On Mon, Apr 30, 2012 at 02:15:03PM +0100, Richard Earnshaw wrote:
On 28/04/12 11:04, Alexey Kravets wrote:
Hi guys,
Please, take a look at this patch. It fixes the invalid star symbol
processing in validate_switches function reported in GCC bugzilla:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51020
With this patch invalid options are no longer accepted by the
compiler (see new testcase for more details).
It showed no regressions on GCC-4.6.3 on ARM.
ChangeLog:
* gcc/gcc.c(validate_switches): Reset starred flag.
* gcc/testsuite/gcc.target/arm/pr51020.c: New test.
Why have you created an arm-specific test for something that, at least
on initial examination, is a generic issue?
Yes, that is my bad. I have updated the test to be generic and rerun
testsuite on x86_64 machine (with no regressions). The updated patch is
attached.
R.
--
Alexey Kravets
mr.kayr...@gmail.com
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 75f522e..e3c8609 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -7023,11 +7023,12 @@ validate_switches (const char *start)
size_t len;
int i;
bool suffix = false;
- bool starred = false;
+ bool starred;
#define SKIP_WHITE() do { while (*p == ' ' || *p == '\t') p++; } while (0)
next_member:
+ starred = false;
SKIP_WHITE ();
if (*p == '!')
diff --git a/gcc/testsuite/gcc.dg/pr51020.c b/gcc/testsuite/gcc.dg/pr51020.c
new file mode 100644
index 000..ac77dc1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr51020.c
@@ -0,0 +1,5 @@
+/* PR 51020 */
+/* { dg-do compile } */
+/* { dg-options --- } */
+int main() {return 0;}
+/* { dg-error unrecognized.*'---' { target *-*-* } 0 } */
--
1.7.5.4
--
Alexey Kravets
a.krav...@samsung.com
local ext. 3964
signature.asc
Description: Digital signature
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, 25 May 2012, Diego Novillo wrote: I don't care how ugly coding conventions look. I've dealt with many and they all have their ugly spots. I mostly care about consistency. I don't think we should deviate much from the established GNU standards (which are hideous, btw). Mostly because we are all used to them and it is going to be simpler to write new code that is similar to what is already there. In addition, many people work on multiple components of the GNU toolchain and other pieces of the GNU system, and it's desirable to keep consistency with binutils, GDB, glibc etc. (The GNU Coding Standards do change over time ... for example, they now allow ranges of years in copyright notices, which I think it would be good for GCC and binutils to adopt as GDB and glibc have done, and, following GCC's lead, they no longer recommand `' quoting in diagnostics.) -- Joseph S. Myers jos...@codesourcery.com
Re: [cxx-conversion] New Hash Table (issue6244048)
On 05/25/2012 04:15 PM, Diego Novillo wrote: On 12-05-25 09:56 , Jakub Jelinek wrote: On Fri, May 25, 2012 at 03:51:25PM +0200, Michael Matz wrote: On Fri, 25 May 2012, Gabriel Dos Reis wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. I don't care how ugly coding conventions look. I've dealt with many and they all have their ugly spots. I mostly care about consistency. I don't think we should deviate much from the established GNU standards (which are hideous, btw). Mostly because we are all used to them and it is going to be simpler to write new code that is similar to what is already there. Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). Can emacs handle the indentation rules automatically? Bernd
unwind.h installation causes rebuilds
Hi,
I've noticed that libitm is always rebuild with a non-bootstrap tree even
with merely a sequence of two makes. The reason turns out to be that
installation of unwind.h from libgcc, which is always done with a simple
make:
# make
# make -d
...
dest=../.././gcc/include/tmp$$-unwind.h; \
cp unwind.h $dest; \
chmod a+r $dest; \
sh ../../../gcc/libgcc/../move-if-change $dest
../.././gcc/include/unwind.h
(this is still fine, because it uses move-if-change)
...
make install-leaf DESTDIR=../.././gcc \
slibdir= libsubdir= MULTIOSDIR=.
...
/bin/sh ../../../gcc/libgcc/../mkinstalldirs ../.././gcc/include
/usr/bin/install -c -m 644 unwind.h ../.././gcc/include
This one updates the mtime of gcc/include/unwind.h, and is the problematic
one, ultimately resulting in:
Prerequisite `/matz/gcc/svn/real-trunk/dev/gcc/include/unwind.h' is
newer than target `aatree.lo'.
And all of libitm (our only c++ library) is rebuilt as all objects therein
have a dependency on unwind.h.
There are multiple variants to fix:
a) use install -p (or -C) generally for all INSTALLs in all makefiles
b) use a) for only libgcc Makefile
c) don't repeatedly install the same files on a remake even though nothing
was compiled newly (perhaps some stamp files could be used to make
leaf-install a no-op if nothing new was built?)
I've checked that hardcoding -p for INSTALL in the top-level Makefile
solves this problem (see patch), but perhaps you prefer one of the others
(or can think of something else entirely).
Ciao,
Michael.
Index: configure.ac
===
--- configure.ac(revision 187708)
+++ configure.ac(working copy)
@@ -93,8 +93,8 @@ srcpwd=`cd ${srcdir} ; ${PWDCMD-pwd}`
# We pass INSTALL explicitly to sub-makes. Make sure that it is not
# a relative path.
-if test $INSTALL = ${srcdir}/install-sh -c; then
- INSTALL=${srcpwd}/install-sh -c
+if test $INSTALL = ${srcdir}/install-sh -c -p; then
+ INSTALL=${srcpwd}/install-sh -c -p
fi
# Set srcdir to . if that's what it is.
Index: config/proginstall.m4
===
--- config/proginstall.m4 (revision 187708)
+++ config/proginstall.m4 (working copy)
@@ -54,12 +54,12 @@ case $as_dir/ in
echo one conftest.one
echo two conftest.two
mkdir conftest.dir
- if $as_dir/$ac_prog$ac_exec_ext -c conftest.one conftest.two
`pwd`/conftest.dir
+ if $as_dir/$ac_prog$ac_exec_ext -c -p conftest.one conftest.two
`pwd`/conftest.dir
test -s conftest.one test -s conftest.two
test -s conftest.dir/conftest.one
test -s conftest.dir/conftest.two
then
- ac_cv_path_install=$as_dir/$ac_prog$ac_exec_ext -c
+ ac_cv_path_install=$as_dir/$ac_prog$ac_exec_ext -c -p
break 3
fi
fi
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi,
On Fri, May 25, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+return htab-size;
+ }
(and various other places) - formatting is wrong, missing space between (.
And it doesn't start at the first column, and type isn't on a separate
line.
I don't think I'm going to follow this discussion in much detail and, more
importantly, I don't know how representative may own contributions are as
'libstdc++ style', but I *always* have the return type on a separate line in my
own patches. It's true however that we don't normally put a space between name
and open round bracket, but hey, I don't think we (the libstdc++ people) are so
in love with that: myself would approve right away a big mechanical patch
adding spaces everywhere, to be clear.
Paolo
Re: [PATCH] ARM/NEON: vld1q_dup_s64 builtin
On 21.05.2012 11:16, Christophe Lyon wrote:
I tried applying your patch but ran into trouble with patch not liking
this . My suspicion is mailer munging white spaces in some form -
Could you send the patch as an attachment please rather than inline in
your mail ?
regards,
Ramana
Here it is, as an attachment. Note however that this patch is against GCC-4.6.3.
Thanks for testing.
Christophe.
Hi,
I have attached the version for GCC trunk.
Christophe.
2012-05-25 Christophe Lyon christophe.l...@st.com
* gcc/config/arm/neon.md (neon_vld1_dup): Restrict to VQ
operands.
(neon_vld1_dupv2di): New, fixes vld1q_dup_s64.
* gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c: New test.
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 4568dea..0a4d00b 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -4397,20 +4397,32 @@
)
(define_insn neon_vld1_dupmode
- [(set (match_operand:VQX 0 s_register_operand =w)
-(unspec:VQX [(match_operand:V_elem 1 neon_struct_operand Um)]
+ [(set (match_operand:VQ 0 s_register_operand =w)
+(unspec:VQ [(match_operand:V_elem 1 neon_struct_operand Um)]
UNSPEC_VLD1_DUP))]
TARGET_NEON
{
- if (GET_MODE_NUNITS (MODEmode) 2)
-return vld1.V_sz_elem\t{%e0[], %f0[]}, %A1;
- else
-return vld1.V_sz_elem\t%h0, %A1;
+ return vld1.V_sz_elem\t{%e0[], %f0[]}, %A1;
}
[(set (attr neon_type)
- (if_then_else (gt (const_string V_mode_nunits) (const_string 1))
-(const_string neon_vld2_2_regs_vld1_vld2_all_lanes)
-(const_string neon_vld1_1_2_regs)))]
+ (const_string neon_vld2_2_regs_vld1_vld2_all_lanes))]
+)
+
+(define_insn_and_split neon_vld1_dupv2di
+ [(set (match_operand:V2DI 0 s_register_operand =w)
+(vec_duplicate:V2DI (match_operand:DI 1 neon_struct_operand Um)))]
+ TARGET_NEON
+ #
+reload_completed
+ [(const_int 0)]
+ {
+rtx tmprtx = gen_lowpart (DImode, operands[0]);
+emit_insn (gen_neon_vld1_dupdi (tmprtx, operands[1]));
+emit_move_insn (gen_highpart (DImode, operands[0]), tmprtx );
+DONE;
+}
+ [(set_attr length 8)
+ (set (attr neon_type) (const_string
neon_vld2_2_regs_vld1_vld2_all_lanes))]
)
(define_expand vec_store_lanesmodemode
diff --git a/gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c
b/gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c
new file mode 100644
index 000..b5793bf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c
@@ -0,0 +1,24 @@
+/* Test the `vld1q_s64' ARM Neon intrinsic. */
+
+/* { dg-do run } */
+/* { dg-require-effective-target arm_neon_hw } */
+/* { dg-options -O0 } */
+/* { dg-add-options arm_neon } */
+
+#include arm_neon.h
+#include stdlib.h
+
+int main (void)
+{
+ int64x1_t input[2] = {(int64x1_t)0x0123456776543210LL,
+ (int64x1_t)0x89abcdeffedcba90LL};
+ int64x1_t output[2] = {0, 0};
+ int64x2_t var = vld1q_dup_s64(input);
+
+ vst1q_s64(output, var);
+ if (output[0] != (int64x1_t)0x0123456776543210LL)
+abort();
+ if (output[1] != (int64x1_t)0x0123456776543210LL)
+abort();
+ return 0;
+}
Re: unwind.h installation causes rebuilds
Hello Michael, On May 25, 2012, at 16:30 , Michael Matz wrote: I've noticed that libitm is always rebuild with a non-bootstrap tree even with merely a sequence of two makes. The reason turns out to be that installation of unwind.h from libgcc, which is always done with a simple make: This should have been fixed by rev 187839 after the thread initiated at http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01596.html Could you please tell if that works for you ? Thanks in advance, Olivier
[gimplefe] Fix parsing of assign_stmt (issue6247046)
Fix parser initialization and parsing of gimple_assign.
This patch fixes the parser to use the new line map table
initialization code. It was not setting the allocation function
pointers properly.
Additionally, we were only expecting binary RHS for gimple_assign.
This was causing failures in the small test file I added yesterday.
Sandeep, now that we have an initial harness for testing, could you
please add a set of .gimple files that test the *whole* gimple
grammar? (or at least a large chunk of it).
An initial starting point would be to use -fdump-tree-gimple-raw on
the compilation of several C/C++ source files. This will give you an
initial set of test cases to put in the testsuite to exercise the
recognizer.
Tested on x86_64.
2012-05-25 Diego Novillo dnovi...@google.com
gimple/ChangeLog
* parser.c (gimple_register_var_decl_in_symtab): Tidy.
(gl_peek_token): New.
(gl_consume_token): Call it.
(gl_tree_code_for_token): Add FIXME note.
(gl_gimple_code_for_token): Likewise.
(gl_dump): Surround the current token in '[[[' ']]]'.
(gl_consume_expected_token): Only emit an error if no other errors
have been emitted.
(gp_parse_expect_subcode): Return the recognized code. Return
the read token in new argument *TOKEN_P.
Update all callers.
(gp_parse_expect_lhs): Call gl_peek_token.
(gp_parse_expect_rhs_op): Rename from gp_parse_expect_rhs1.
Call gl_peek_token.
(gp_parse_expect_rhs2): Remove.
(gp_parse_assign_stmt): Re-write to support all gimple RHS
classes.
(gp_parse_cond_stmt): Tidy.
Emit an error if OPCODE is anything but a GIMPLE_BINARY_RHS.
(gp_parse_goto_stmt): Tidy.
(gp_parse_label_stmt): Tidy.
(gp_parse_switch_stmt): Tidy.
(gp_parse_stmt): Do not emit an error if one has been emitted
already.
(realloc_for_line_map): New.
(gp_init): Use it.
Update code to initialize the line table.
(gimple_main): Tidy.
testsuite/ChangeLog.gimplefe:
* gimple.dg/20120523-1.gimple: Add expected error.
* gimple.dg/20120525-1.gimple: New.
diff --git a/gcc/gimple/parser.c b/gcc/gimple/parser.c
index 4b29333..3f3eb96 100644
--- a/gcc/gimple/parser.c
+++ b/gcc/gimple/parser.c
@@ -152,10 +152,10 @@ gl_token_as_text (const gimple_token *token)
static void
gimple_register_var_decl_in_symtab (const gimple_token *name_token)
{
- const char *name = gl_token_as_text(name_token);
- tree id = get_identifier(name);
- tree decl = build_decl(name_token-location, VAR_DECL, get_identifier(name),
void_type_node);
-
+ const char *name = gl_token_as_text (name_token);
+ tree id = get_identifier (name);
+ tree decl = build_decl (name_token-location, VAR_DECL,
+ get_identifier(name), void_type_node);
gimple_symtab_register_decl (decl,id);
}
@@ -168,15 +168,25 @@ gl_at_eof (gimple_lexer *lexer)
}
-/* Consume the next token from LEXER. */
+/* Peek into the next token from LEXER. */
static gimple_token *
-gl_consume_token (gimple_lexer *lexer)
+gl_peek_token (gimple_lexer *lexer)
{
if (gl_at_eof (lexer))
return gl_eof_token;
+ return VEC_index (gimple_token, lexer-tokens, lexer-cur_token_ix);
+}
- return VEC_index (gimple_token, lexer-tokens, lexer-cur_token_ix++);
+
+/* Consume the next token from LEXER. */
+
+static gimple_token *
+gl_consume_token (gimple_lexer *lexer)
+{
+ gimple_token *tok = gl_peek_token (lexer);
+ lexer-cur_token_ix++;
+ return tok;
}
@@ -189,6 +199,7 @@ gl_tree_code_for_token (const gimple_token *token)
size_t code;
const char *s;
+ /* FIXME. Expensive linear scan, convert into a string-code map. */
s = gl_token_as_text (token);
for (code = ERROR_MARK; code LAST_AND_UNUSED_TREE_CODE; code++)
if (strcasecmp (s, tree_code_name[code]) == 0)
@@ -207,6 +218,7 @@ gl_gimple_code_for_token (const gimple_token *token)
size_t code;
const char *s;
+ /* FIXME. Expensive linear scan, convert into a string-code map. */
s = gl_token_as_text (token);
for (code = GIMPLE_ERROR_MARK; code LAST_AND_UNUSED_GIMPLE_CODE; code++)
if (strcasecmp (s, gimple_code_name[code]) == 0)
@@ -215,6 +227,7 @@ gl_gimple_code_for_token (const gimple_token *token)
return (enum gimple_code) code;
}
+
/* Return true if TOKEN is the start of a declaration. */
static bool
@@ -272,7 +285,15 @@ gl_dump (FILE *file, gimple_lexer *lexer)
lexer-cur_token_ix);
for (i = 0; VEC_iterate (gimple_token, lexer-tokens, i, token); i++)
-gl_dump_token (file, token);
+{
+ if (i == lexer-cur_token_ix)
+ fprintf (file, [[[ );
+ gl_dump_token (file, token);
+ if (i == lexer-cur_token_ix)
+ fprintf (file, ]]]);
+}
+
+ fprintf (file, \n);
}
@@ -293,7 +314,7 @@ static const gimple_token *
gl_consume_expected_token (gimple_lexer *lexer, enum cpp_ttype
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi, On Fri, 25 May 2012, Diego Novillo wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. I don't care how ugly coding conventions look. Perhaps people who don't care about uglyness shouldn't then be defining the style? Half-way :-/ Ciao, Michael.
Re: [cxx-conversion] New Hash Table (issue6244048)
On 12-05-25 11:06 , Michael Matz wrote: Hi, On Fri, 25 May 2012, Diego Novillo wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. I don't care how ugly coding conventions look. Perhaps people who don't care about uglyness shouldn't then be defining the style? Half-way :-/ Who says I am? I *explicitly* said that Lawrence, Ian and Gaby are working on it. I do *not* care about C++ style, I care about all the other benefits it provides us. I will use whatever style y'all agree to. Diego.
[PATCH, i386]: Fix PR 53474, Solaris/x86 bootstrap with Sun as broken: j.e
Hello!
2012-05-25 Uros Bizjak ubiz...@gmail.com
PR target/53474
* config/i386/i386.c (ix86_print_operand) case 'O': Print '.' here.
case 'C', case 'c', case 'F', case 'f': Print '.' only for C and c.
Tested on i386-pc-solaris2.10 by Rainer, and on x86_64-pc-linux-gnu
{,-m32}. Committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 187882)
+++ config/i386/i386.c (working copy)
@@ -13931,8 +13931,8 @@
C -- print opcode suffix for set/cmov insn.
c -- like C, but print reversed condition
F,f -- likewise, but for floating-point.
- O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, print the opcode suffix for
- the size of the current operand, otherwise nothing.
+ O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to w., l. or q.,
+ otherwise nothing
R -- print the prefix for register names.
z -- print the opcode suffix for the size of the current operand.
Z -- likewise, with special suffixes for x87 instructions.
@@ -14061,6 +14061,8 @@
(invalid operand size for operand code 'O');
return;
}
+
+ putc ('.', file);
#endif
return;
@@ -14320,20 +14322,21 @@
}
return;
+ case 'F':
+ case 'f':
+#ifdef HAVE_AS_IX86_CMOV_SUN_SYNTAX
+ if (ASSEMBLER_DIALECT == ASM_ATT)
+ putc ('.', file);
+#endif
+
case 'C':
case 'c':
- case 'F':
- case 'f':
if (!COMPARISON_P (x))
{
output_operand_lossage (operand is not a condition code,
invalid operand code '%c', code);
return;
}
-#ifdef HAVE_AS_IX86_CMOV_SUN_SYNTAX
- if (ASSEMBLER_DIALECT == ASM_ATT)
- putc ('.', file);
-#endif
put_condition_code (GET_CODE (x), GET_MODE (XEXP (x, 0)),
code == 'c' || code == 'f',
code == 'F' || code == 'f',
Re: unwind.h installation causes rebuilds
Hi, On Fri, 25 May 2012, Olivier Hainque wrote: I've noticed that libitm is always rebuild with a non-bootstrap tree even with merely a sequence of two makes. The reason turns out to be that installation of unwind.h from libgcc, which is always done with a simple make: This should have been fixed by rev 187839 after the thread initiated at http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01596.html Could you please tell if that works for you ? Super, yes, that works. Though I still wonder if the whole copy-over-to-gcc business shouldn't be dependend on anything newly built. I can't see the use in copying over the same unchanged files to a place where they already are. And generally installing with -p also can't hurt, can it? Ciao, Michael.
Re: unwind.h installation causes rebuilds
On May 25, 2012, at 17:17 , Michael Matz wrote: This should have been fixed by rev 187839 ... Super, yes, that works. Great :) Though I still wonder if the whole copy-over-to-gcc business shouldn't be dependend on anything newly built. I can't see the use in copying over the same unchanged files to a place where they already are. And generally installing with -p also can't hurt, can it? I suppose not but don't know the full story here.
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 11:07 AM, Diego Novillo dnovi...@google.com wrote: On 12-05-25 11:06 , Michael Matz wrote: Hi, On Fri, 25 May 2012, Diego Novillo wrote: That's one of my fears, namely that those used to the libstdc++ style impose that on the compiler source base. Because IMHO the libstdc++ style isn't very appealing. Seconded. I don't care how ugly coding conventions look. Perhaps people who don't care about uglyness shouldn't then be defining the style? Half-way :-/ Who says I am? I *explicitly* said that Lawrence, Ian and Gaby are working on it. Didn't Ian already create a style for writing gold in c++?
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 9:33 AM, Paolo Carlini pcarl...@gmail.com wrote:
Hi,
On Fri, May 25, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+ return htab-size;
+ }
(and various other places) - formatting is wrong, missing space between (.
And it doesn't start at the first column, and type isn't on a separate
line.
I don't think I'm going to follow this discussion in much detail and, more
importantly, I don't know how representative may own contributions are as
'libstdc++ style', but I *always* have the return type on a separate line in
my own patches. It's true however that we don't normally put a space between
name and open round bracket, but hey, I don't think we (the libstdc++ people)
are so in love with that: myself would approve right away a big mechanical
patch adding spaces everywhere, to be clear.
no, you don't want to do that :-)
-- Gaby
Re: PR middle-end/53008 (trans-mem): output clone if function accessed indirectly
On 05/25/2012 06:25 AM, Aldy Hernandez wrote: OK? Would this be acceptable for the 4.7 branch as well? curr PR middle-end/53008 * trans-mem.c (ipa_tm_create_version_alias): Output new_node if accessed indirectly. (ipa_tm_create_version): Same. Ok everywhere. r~
Re: [PATCH, c++]: Fix PR 53441, [4.8 Regression] obj-c++.dg/ivar-invalid-type-1.mm ICE
Uros Bizjak ubiz...@gmail.com a écrit:
As shown in the PR, ivar-invalid-type-1 ICEs in constructor_name_p,
due to accessor on NULL type argument.
The one-liner patch fixes the ICE by adding a guard that checks that
current_class_type is non-NULL before calling constructor_name_p.
2012-05-21 Uros Bizjak ubiz...@gmail.com
PR obj-c++/53441
* decl.c (grokdeclarator): Check that current_class_type is non-NULL
before calling constructor_name_p.
Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}.
OK for mainline SVN?
I cannot approve or deny this patch, but it looks OK to me. Let's CC
Jason.
[...]
Index: decl.c
===
--- decl.c(revision 187736)
+++ decl.c(working copy)
@@ -9803,7 +9803,8 @@ grokdeclarator (const cp_declarator *declarator,
clones. */
DECL_ABSTRACT (decl) = 1;
}
- else if (constructor_name_p (unqualified_id, current_class_type))
+ else if (current_class_type
+ constructor_name_p (unqualified_id, current_class_type))
permerror (input_location, ISO C++ forbids nested type %qD with same
name
as enclosing class,
unqualified_id);
--
Dodji
Re: [PATCH, c++]: Fix PR 53441, [4.8 Regression] obj-c++.dg/ivar-invalid-type-1.mm ICE
OK. Jason
Re: [C++ Patch] PR 32054
On 05/24/2012 10:57 PM, Paolo Carlini wrote: + error_at (decl_spec_token_start-location, + a storage class is not allowed); Let's give more explanation here so that the user knows that the problem is a storage class on an anonymous union/struct in class scope. Jason
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
On 05/25/2012 08:46 AM, Dominique Dhumieres wrote: I certainly prefer this alternative (the use of extern for that purpose being extremely confusing for a fortraner;-). However, if my test is valid, if N is not equal to 2 or 4, one gets the following error error: size of array 'l' is negative Would not it be better to have an explicit test of N being 2 or 4, and an error of the kind the vector size has to be the size of 2 or 4 long if not? There's no way to tell the compiler to emit that kind of error (in C; that's what C++11 static_assert is for). Jason
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 11:44 AM, Paweł Sikora pl...@agmk.net wrote: so, why you just don't use the hash table implementation from libstdc++? we have agreed on C++03 as a bootstrap compiler. There is unfortunately no hash table in C++03. The page about not defining a new template was just when we were gathering all kinds of ideas. It is clearly too restrictive to be appropriate. -- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 12:50 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Fri, May 25, 2012 at 11:44 AM, Paweł Sikora pl...@agmk.net wrote: so, why you just don't use the hash table implementation from libstdc++? we have agreed on C++03 as a bootstrap compiler. There is unfortunately no hash table in C++03. That's a shame. C++11 makes life wonderful.
Re: [cxx-conversion] New Hash Table (issue6244048)
On Friday 25 of May 2012 11:50:13 Gabriel Dos Reis wrote: On Fri, May 25, 2012 at 11:44 AM, Paweł Sikora pl...@agmk.net wrote: so, why you just don't use the hash table implementation from libstdc++? we have agreed on C++03 as a bootstrap compiler. There is unfortunately no hash table in C++03. can't you use implementation from tr1 for c++98/03 mode?
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Diego Novillo dnovi...@google.com wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). Lawrence, have you had a chance to update them with your latest edits? Not yet; they're on the desk in front of me. I'll do that ASAP. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Paweł Sikora pl...@agmk.net wrote: On Friday 25 of May 2012 10:15:54 Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). on this page we can read: The following features of the C++ language should in general be avoided in GCC code. (...) Defining new templates (use of existing templates, e.g., from the standard library, is fine). That rule came from Ian's approach with gold. It works well for a new project. It does not work well in gcc. so, why you just don't use the hash table implementation from libstdc++? Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, so we would have to instrument the library. Second, the GTY parser doesn't handle C++, so we would have to redo the GTY parser. Third, PCH requires gc, which requires GTY, so we cannot easily get rid of gc. Fourth, any use of libstdc++ will make different tradeoffs in size and performance, so any switch would require both a change in form and a change in substance. We would simultaneously have to show benefit on two axes, which is a predictor of trouble. So, instead we chose to take a more incremental approach, exploiting C++ with the existing algorithms and requiring mostly mechanical changes in client code. Both the hash table and vector patches take that approach. -- Lawrence Crowl
Re: Turn check macros into functions. (issue6188088)
Lawrence == Lawrence Crowl cr...@google.com writes: Lawrence Should I add that to my patch to gdbinit.in? I think it would be helpful. Tom
Re: Turn check macros into functions. (issue6188088)
Mike == Mike Stump mikest...@comcast.net writes:
Mike Yeah, I kinda think the gdb people are wimping out by not just
Mike implementing __extension__ and ({}), which, I think get us most of the
Mike way there. Shh, don't tell them I said that.
We eagerly await your patch.
Tom
libgo patch committed: Fix cast error
This patch to libgo fixes a cast error in the new file print.c that
shows up on 32-bit systems. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline and 4.7 branch.
Ian
diff -r ae91b408310f libgo/runtime/print.c
--- a/libgo/runtime/print.c Thu May 24 14:06:08 2012 -0700
+++ b/libgo/runtime/print.c Fri May 25 11:20:33 2012 -0700
@@ -136,7 +136,7 @@
runtime_printpc(void *p __attribute__ ((unused)))
{
runtime_prints(PC=);
- runtime_printhex((uint64)runtime_getcallerpc(p));
+ runtime_printhex((uint64)(uintptr)runtime_getcallerpc(p));
}
void
Re: [cxx-conversion] Convert vec.[ch] to C++ [1/3] (issue6233044)
Diego == Diego Novillo dnovi...@google.com writes:
Diego +struct GTY(()) vec_prefix
Diego +{
Diego + unsigned num;
Diego + unsigned alloc;
Diego +};
Diego +
Diego +/* Vector type, user visible. */
Diego +templatetypename T
Diego +struct GTY(()) vec_t
Diego +{
Diego + vec_prefix prefix;
Diego + T GTY((length (%h.prefix.num))) vec[1];
Diego +};
I'm curious why you didn't use inheritance here.
templatetypename T
struct GTY(()) vec_t : public vec_prefix
{
...
};
Tom
Re: [cxx-conversion] Convert vec.[ch] to C++ [1/3] (issue6233044)
On Fri, May 25, 2012 at 1:27 PM, Tom Tromey tro...@redhat.com wrote:
Diego == Diego Novillo dnovi...@google.com writes:
Diego +struct GTY(()) vec_prefix
Diego +{
Diego + unsigned num;
Diego + unsigned alloc;
Diego +};
Diego +
Diego +/* Vector type, user visible. */
Diego +templatetypename T
Diego +struct GTY(()) vec_t
Diego +{
Diego + vec_prefix prefix;
Diego + T GTY((length (%h.prefix.num))) vec[1];
Diego +};
I'm curious why you didn't use inheritance here.
templatetypename T
struct GTY(()) vec_t : public vec_prefix
{
...
};
Very good question.
I thought about it, and then offered myself ah, so that vec_tint is still
a POD type, allowing some of the usual C practices, whereas
inheritance would have ruled that out as C++03 portable code.
-- Gaby
Re: [cxx-conversion] New Hash Table (issue6244048)
On May 25, 2012, at 10:50 AM, Lawrence Crowl wrote: Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, If portability to other C++ compilers wasn't a concern, we could extend out g++ to make supporting GTY better, so that we can simplify and refine the GTY stuff. I fear we need some light weight reflection, might make a nice language feature for a future C++ standard, if done well.
Re: [cxx-conversion] New Hash Table (issue6244048)
On May 25, 2012, at 7:29 AM, Bernd Schmidt wrote: Can emacs handle the indentation rules automatically? I hope that any style picked, will have a no local mods required for emacs as the corner stone. One thing I hate about .md files, is they don't (last I checked) fulfill this requirement.
Re: [PATCH] omit -lcrt1.10.6.o and pass -no_new_main with -pg on darwin = 10.8
On May 25, 2012, at 6:06 AM, Jack Howarth wrote: The attached patch limits the linkage of -lcrt1.10.6.o to darwin10 and darwin11 since its usage is deprecated in the 10.8sdk. Ok.
Re: [PATCH] PR debug/53453 ensure debug notes linked on darwin
On May 25, 2012, at 6:03 AM, Jack Howarth wrote: The attached patch solves PR53453. Darwin bits are Ok.
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
On 05/25/2012 05:27 PM, Jason Merrill wrote: On 05/25/2012 08:46 AM, Dominique Dhumieres wrote: I certainly prefer this alternative (the use of extern for that purpose being extremely confusing for a fortraner;-). However, if my test is valid, if N is not equal to 2 or 4, one gets the following error error: size of array 'l' is negative Would not it be better to have an explicit test of N being 2 or 4, and an error of the kind the vector size has to be the size of 2 or 4 long if not? There's no way to tell the compiler to emit that kind of error (in C; that's what C++11 static_assert is for). You can simply rename the 'l' array to the error you want to output, so you get an error like: error: size of array 'the_vector_size_has_to_be_the_size_of_2_or_4_long' is negative or whatever you want to say as error. -- Pedro Alves
Re: [AARCH64] [PATCH 1/3] AArch64 Port
On 25/05/12 15:18, Joseph S. Myers wrote:
General question: I suppose you expect to submit patches soon for
other toolchain components (such as binutils, GDB, glibc) and the
Linux kernel, if you haven't done so yet?
There is work going on to get other components ready for community
review. I expect to see patches submitted over the next few months.
+The @code{__float128} type is supported on hppa HP-UX targets and ARM AArch64
+targets.
I don't see any good reason to support it on AArch64, since it's the
same as long double there. (It's on PA HP-UX as a workaround for
libquadmath requiring the type rather than being able to with with a
type called either long double or __float128 - libquadmath being
Ok, thanks for the explanation, we will drop __float128.
+@item -march=@var{name}
+@opindex march
+Specify the name of the target architecture, optionally suffixed by one or
+more feature modifiers. This option has the form
+@samp{-march=arch[+[no]feature]}, where the only value for @samp{arch}
+is @samp{armv8}, and the possible values for @samp{feature} are
+@samp{crypto}, @samp{fp}, @samp{simd}.
It's unfortunate that you've chosen this complicated syntax that means
the generic support for enumerated option arguments cannot be used
(and so --help information cannot list supported CPUs and features).
A simpler syntax where -march takes just an architecture name and
features have separate options would seem better, and more in line
with most other architectures supported by GCC.
I will respond to this comment separately.
+__extension__ static __inline int8x8_t __attribute__ ((__always_inline__))
+vneg_s8 (int8x8_t a)
+{
+ int8x8_t result;
+ __asm__ (neg %0.8b,%1.8b
+ : =w(result)
+ : w(a)
+ : /* No clobbers */);
+ return result;
+}
I presume you've determined for each intrinsic represented as an asm
that it cannot usefully be described using generic GNU C, GIMPLE or
RTL - as an example, for this one, what are its semantics that generic
internal representations cannot handle usefully?
We initially put together a simple asm based implementation of each
intrinsic. Subsequently we have replaced many of the intrinsics with
GNU C implementations. The vneg_* patterns are an oversight which will
be fixed.
We have a long list of intrinsics which we want to move into RTL, only
some of these have been moved so far. However, that said, point noted
that we can usefully exploit TARGET_FOLD_BUILTIN in preference to RTL.
Wht are you using t-softfp-sfdf and t-softfp-excl when
aarch64/t-softfp appears to override them completely?
Agreed, these are not needed.
Are you really planning to support non-ELF for AArch64? If no, leave
out the conditionals.
We have no plans for non-ELF, the conditionals will be removed.
The remainder of your comments will be addressed with patches shortly.
Thank you for the feedback!
/Marcus
Re: [PATCH] Add powerpc64-linux configuration options
On Fri, May 25, 2012 at 10:22 AM, Michael Meissner meiss...@linux.vnet.ibm.com wrote: Yes, the second patch just does not build the library or use the -mstrict-align option. It is much simpler, but there may be somebody out there that depends on the functionality. I really don't know, one way or the other. Mike, Please apply the second version of your patch corresponding to * config/rs6000/t-linux64: Delete the 32-bit multilib that uses software floating point emulation. No longer build the multilibs with -mstrict-align. Thanks, David
Re: [PATCH][revised] PR debug/53453 ensure debug notes linked on darwin
OK. Jason
Re: [AARCH64] [PATCH 1/3] AArch64 Port
On Fri, 25 May 2012, Marcus Shawcroft wrote: We have a long list of intrinsics which we want to move into RTL, only some of these have been moved so far. However, that said, point noted that we can usefully exploit TARGET_FOLD_BUILTIN in preference to RTL. I think the order of preference is roughly: * Generic GNU C code. For example, I saw you had addition intrinsics that just used + on the vector types. This is the best approach for anything that can be represented like that in GNU C. This includes any cases where the generic GNU C way of doing something is with a generic __builtin (for example, if the operation can naturally be defined in terms of __builtin_shuffle then you should do that). * Architecture-specific built-in functions that fold to produce GENERIC / GIMPLE codes with architecture-independent semantics. (But if several architectures have the same vector operation, it may be better to define an architecture-independent built-in function for it - though that then needs generic fallback support for architectures without the operation.) * Architecture-specific built-in functions that produce RTL with architecture-independent semantics. * Architecture-specific built-in functions that produce UNSPEC RTL. * Inline asm. And, inline functions will generally be preferred over macros, but it's possible there are cases (e.g. if the intrinsic is defined to take an lvalue argument and modify it) that can't be directly represented in C inline functions. -- Joseph S. Myers jos...@codesourcery.com
Re: [cxx-conversion] Convert vec.[ch] to C++ [1/3] (issue6233044)
On 12-05-25 14:27 , Tom Tromey wrote:
Diego == Diego Novillodnovi...@google.com writes:
Diego +struct GTY(()) vec_prefix
Diego +{
Diego + unsigned num;
Diego + unsigned alloc;
Diego +};
Diego +
Diego +/* Vector type, user visible. */
Diego +templatetypename T
Diego +struct GTY(()) vec_t
Diego +{
Diego + vec_prefix prefix;
Diego + T GTY((length (%h.prefix.num))) vec[1];
Diego +};
I'm curious why you didn't use inheritance here.
templatetypename T
struct GTY(()) vec_t : public vec_prefix
{
...
};
Minimalism. I was almost obsessed by it with the first patch. It
would've required more changes if I didn't make it a POD type.
I will make it into a more standard C++ type in subsequent patches.
Diego.
Re: [patch] Fix PR lto/52178 (continued)
Ok. Please make sure to verify LTO bootstrap on the branch still works after this. Thanks. I already verified it with --enable-checking=yes,rtl. -- Eric Botcazou
libgcc patch committed: Fix for -fsplit-stack when using gold
When using the gold linker, the linker will adjust the split-stack
prologue to call __morestack_non_split if it sees that the function
calls a different function compiled without -fsplit-stack. The
__morestack_non_split function has an optimization I committed
2011-12-20 to avoid splitting the stack if it has a lot of space.
Unfortunately, I forgot that if the function calling
__morestack_non_split is a varargs function, then it expects %ebp to
have a particular value when __morestack_non_split returns. There is no
reasonable way for __morestack_non_split to set %ebp to the right value
without actually splitting the stack. This patch changes
__morestack_non_split to do a stack split when called by a varargs
function. It detects a call from a varargs function by looking at the
instruction it is going to return to, to see if it uses %ebp.
Bootstrapped and ran Go testsuite and split-stack tests on
x86_64-unknown-linux-gnu, both 32-bit and 64-bit. Added a new test to
check this case in the future. With an unpatched libgcc, the test will
succeed with GNU ld, fail with gold.
Committed to mainline and 4.7 branch.
Ian
libgcc/:
2012-05-25 Ian Lance Taylor i...@google.com
* config/i386/morestack.S (__morestack_non_split): Check whether
caller is varargs and needs %bp to hold the stack frame on return.
gcc/testsuite/:
2012-05-25 Ian Lance Taylor i...@google.com
* gcc.dg/split-6.c: New test.
Index: libgcc/config/i386/morestack.S
===
--- libgcc/config/i386/morestack.S (revision 187020)
+++ libgcc/config/i386/morestack.S (working copy)
@@ -1,5 +1,5 @@
# x86/x86_64 support for -fsplit-stack.
-# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
+# Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
# Contributed by Ian Lance Taylor i...@google.com.
# This file is part of GCC.
@@ -138,6 +138,24 @@ __morestack_non_split:
je 1f # see above.
addl $2,%eax
1: inc %eax
+
+ # If the instruction that we return to is
+ # leal 20(%ebp),{%eax,%ecx,%edx}
+ # then we have been called by a varargs function that expects
+ # %ebp to hold a real value. That can only work if we do the
+ # full stack split routine. FIXME: This is fragile.
+ cmpb $0x8d,(%eax)
+ jne 3f
+ cmpb $0x14,2(%eax)
+ jne 3f
+ cmpb $0x45,1(%eax)
+ je 2f
+ cmpb $0x4d,1(%eax)
+ je 2f
+ cmpb $0x55,1(%eax)
+ je 2f
+
+3:
movl %eax,4(%esp) # Update return address.
popl %eax # Restore %eax and stack.
@@ -175,18 +193,32 @@ __morestack_non_split:
#else
cmpl %fs:0x40,%eax
#endif
- popq %rax # Restore register.
-
- .cfi_adjust_cfa_offset -8 # Adjust for popped register.
jb 2f # Get more space if we need it.
# This breaks call/return prediction, as described above.
- incq (%rsp) # Increment the return address.
+ incq 8(%rsp) # Increment the return address.
+
+ # If the instruction that we return to is
+ # leaq 24(%rbp), %r11n
+ # then we have been called by a varargs function that expects
+ # %ebp to hold a real value. That can only work if we do the
+ # full stack split routine. FIXME: This is fragile.
+ movq 8(%rsp),%rax
+ cmpl $0x185d8d4c,(%rax)
+ je 2f
+
+ popq %rax # Restore register.
+
+ .cfi_adjust_cfa_offset -8 # Adjust for popped register.
ret# Return to caller.
2:
+ popq %rax # Restore register.
+
+ .cfi_adjust_cfa_offset -8 # Adjust for popped register.
+
addq $0x5000+BACKOFF,%r10 # Increment space we request.
# Fall through into morestack.
Index: gcc/testsuite/gcc.dg/split-6.c
===
--- gcc/testsuite/gcc.dg/split-6.c (revision 0)
+++ gcc/testsuite/gcc.dg/split-6.c (revision 0)
@@ -0,0 +1,53 @@
+/* { dg-do run } */
+/* { dg-require-effective-target split_stack } */
+/* { dg-options -fsplit-stack } */
+
+/* This test is like split-3.c, but tests with a smaller stack frame,
+ since that uses a different prologue. */
+
+#include stdarg.h
+#include stdlib.h
+
+/* Use a noinline function to ensure that the buffer is not removed
+ from the stack. */
+static void use_buffer (char *buf) __attribute__ ((noinline));
+static void
+use_buffer (char *buf)
+{
+ buf[0] = '\0';
+}
+
+/* When using gold, the call to abort will force a stack split. */
+
+static void
+down (int i, ...)
+{
+ char buf[1];
+ va_list ap;
+
+ va_start (ap, i);
+ if (va_arg (ap, int) != 1
+ || va_arg (ap, int) != 2
+ || va_arg (ap, int) != 3
+ || va_arg (ap, int) != 4
+ || va_arg (ap, int) != 5
+ || va_arg (ap, int) != 6
+ || va_arg (ap, int) != 7
+ || va_arg (ap, int) != 8
+ || va_arg (ap, int) != 9
+ || va_arg (ap, int) != 10)
+abort ();
+
+ if (i 0)
+{
+ use_buffer (buf);
+ down (i - 1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+}
+}
+
+int
+main (void)
+{
+ down (1000, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ return 0;
+}
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Michael Matz m...@suse.de wrote:
On Fri, 25 May 2012, Jakub Jelinek wrote:
+ /* Return the current size of this hash table. */
+
+ size_t size()
+ {
+return htab-size;
+ }
(and various other places) - formatting is wrong, missing space
between (.
Yes, an omission that's a consequence of dealing with several
coding conventions.
And it doesn't start at the first column, and type isn't on a
separate line. I realize that this is a member method, hence
indenting and C GNU coding standards conflict, but the latter do
have some nice properties (for instance that 'grep ^func_name *.c'
finds only the definition, not all the calls to func_name).
We can address this issue by requiring all function definitions
to appear outside the class definition. The code is more verbose,
but livable.
I think we need a discussion about style, now that actually people
are working on this.
Yes.
+void
+typed_htab Element, Hash, Equal, Remove, Allocator
+::dispose ()
Do we want to format methods this way?
Don't know, but at least it starts at the first column, and
grepping for ^:: will give only C++ methods, 'grep -B1 ^::' even
including class. So it's no too horrible, despite the syntactic
C++ noise.
For template classes, the specification of an out-of-line member
function is going to be long, so a single-line specification is
likely to overflow often. Whatever we chose, it should be reasonably
extended to multiple lines.
--
Lawrence Crowl
Go patch committed: Don't create a closure if not needed
This patch to the Go frontend changes it to not produce a closure for an
embedded function if one is not needed. Earlier, when Go permitted
comparisons of function types, it was necessary to always create a
closure for an embedded function, so that each embedded function would
be reliably distinct. Go 1 removed function comparisons, for this and
other reasons. Now, if an embedded function does not require a closure,
because it does not refer to any variables defined in enclosing
functions, we don't need to create a closure. Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.7
branch.
Ian
diff -r b7d2ae6bea1b go/expressions.cc
--- a/go/expressions.cc Fri May 25 11:21:08 2012 -0700
+++ b/go/expressions.cc Fri May 25 14:11:54 2012 -0700
@@ -1312,30 +1312,18 @@
TREE_CODE(TREE_OPERAND(fnaddr, 0)) == FUNCTION_DECL);
TREE_ADDRESSABLE(TREE_OPERAND(fnaddr, 0)) = 1;
- // For a normal non-nested function call, that is all we have to do.
- if (!this-function_-is_function()
- || this-function_-func_value()-enclosing() == NULL)
-{
- go_assert(this-closure_ == NULL);
- return fnaddr;
-}
-
- // For a nested function call, we have to always allocate a
- // trampoline. If we don't always allocate, then closures will not
- // be reliably distinct.
- Expression* closure = this-closure_;
- tree closure_tree;
- if (closure == NULL)
-closure_tree = null_pointer_node;
- else
-{
- // Get the value of the closure. This will be a pointer to
- // space allocated on the heap.
- closure_tree = closure-get_tree(context);
- if (closure_tree == error_mark_node)
- return error_mark_node;
- go_assert(POINTER_TYPE_P(TREE_TYPE(closure_tree)));
-}
+ // If there is no closure, that is all have to do.
+ if (this-closure_ == NULL)
+return fnaddr;
+
+ go_assert(this-function_-func_value()-enclosing() != NULL);
+
+ // Get the value of the closure. This will be a pointer to space
+ // allocated on the heap.
+ tree closure_tree = this-closure_-get_tree(context);
+ if (closure_tree == error_mark_node)
+return error_mark_node;
+ go_assert(POINTER_TYPE_P(TREE_TYPE(closure_tree)));
// Now we need to build some code on the heap. This code will load
// the static chain pointer with the closure and then jump to the
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Jakub Jelinek ja...@redhat.com wrote: On Fri, May 25, 2012 at 10:15:54AM -0400, Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). That page is quite inconsistent. E.g. it first talks about retaining space before ( (which I really hope will be retained, it makes code more readable), but in the example it doesn't bother with it. And the long line example is horribly ugly, ending line with ( ? That ( should surely go on the next line after the few spaces. It should list also examples of when the first parameter isn't too long, but there are too many parameters and so some wrapping is needed, etc. We will clean the page. There are technical guidelines that we can, and I think should, apply to the coding convention. In particular, I believe any coding convention should be consistent with the grammar of the language. Both the GNU style and the C++ Standard style break that guideline in different respects. For example, consider the following GNU code. int *p = *foo (3); Here, the convention implies that foo binds more tightly to the * than it does to the (. This implication is false. Now consider the C++ Standard equivalent. int* p = *foo(3); It fixes the binding of foo, but implies that * is syntactically closer to int than it is to p. That implication is false. Now consider a form that is consistent with the actual grammar of the language: int *p = *foo(3); Here there is no implication that foo binds more closely to either the * or the (, so the form is consistent with the grammar. We can go further and have the spacing reinforce the grammar, but lines will get longer. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/24/12, Jakub Jelinek ja...@redhat.com wrote:
On Thu, May 24, 2012 at 09:43:42AM -0700, Lawrence Crowl wrote:
Add a type-safe hash table, typed_htab. Uses of this table
replace uses of libiberty's htab_t. The benefits include less
boiler-plate code, full type safety, and improved performance.
You haven't looked at the most important problem of that approach
- code bloat.
Are you claiming that the size of the binary is more important than
run-time performance, type safety, and source code size?
hashtab.o .text is currently ~ 4KB on x86_64, in your version
only very small part out of it is shared. In a cc1plus binary
I count roughly 179 htab_create{,_ggc} calls, ignoring the first
parameter (initial size) that is roughly 139 unique hash/eq/del
fn combinations, thus you'll end up with over hundred copies of
most of the hashtab code,
First, the new hash table code is smaller because it avoid supporting
special cases that we are not using in practice. What was formerly
conditional is now unconditional, leading to tighter binaries for
a given function.
As you say, there will be more functions in the source, leading to
an increased executable size. Setting aside run-time performance,
why is that size important?
in many of the cases performance critical and thus its I-cache
friendliness is quite important.
It is precisely the performance critical code that needs the template
approach. The approach avoids two indirect calls, which go across
translation units. The optimizer has no ability to inline the often
trivial actions within those indirect calls. This problem is most
severe in the traverse functions, because the optimize cannot see
enough to do anything useful with the loop. The inliner also has no
ability to elide comparisons for null function pointers. In short,
with the libiberty code, the operations must span a large number
of completely useless operations. Those operations disappear with
the template approach. The result is that the working set in the
cache is much smaller.
That said, if you think code size is important for tables that
are not performance critical, I can provide a version of the code
that simply reuses libiberty and gets the type safety without
the performance. I don't have a good intuition for which of those
tables are performance-critical, so please give me an idea of which
they are.
--
Lawrence Crowl
libgo patch committed: More efficient trampoline allocation
This patch to libgo makes the implementation of function trampolines,
used for nested functions that refer to variables in an enclosing
function scope, much more efficient. Previously we were allocating a
separate page for each trampoline. This patch packs them on a single
page as much as possible. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Commited to mainline and 4.7 branch.
Ian
diff -r 9dad4e6b17e9 libgo/runtime/go-trampoline.c
--- a/libgo/runtime/go-trampoline.c Fri May 25 14:12:42 2012 -0700
+++ b/libgo/runtime/go-trampoline.c Fri May 25 14:45:55 2012 -0700
@@ -14,40 +14,100 @@
#include sys/mman.h
#endif
-#include go-alloc.h
+#include runtime.h
+#include arch.h
+#include malloc.h
#include go-assert.h
-/* In order to build a trampoline we need space which is both writable
- and executable. We currently just allocate a whole page. This
- needs to be more system dependent. */
+/* Trampolines need to run in memory that is both writable and
+ executable. In order to implement them, we grab a page of memory
+ and mprotect it. We fill in the page with trampolines as they are
+ required. When we run out of space, we drop the pointer to the
+ page and allocate a new one. The page will be freed by the garbage
+ collector when there are no more variables of type func pointing to
+ it. */
+
+/* A lock to control access to the page of closures. */
+
+static Lock trampoline_lock;
+
+/* The page of closures. */
+
+static unsigned char *trampoline_page;
+
+/* The size of trampoline_page. */
+
+static uintptr_t trampoline_page_size;
+
+/* The number of bytes we have used on trampoline_page. */
+
+static uintptr_t trampoline_page_used;
+
+/* Allocate a trampoline of SIZE bytes that will use the closure in
+ CLOSURE. */
void *
__go_allocate_trampoline (uintptr_t size, void *closure)
{
- unsigned int page_size;
- void *ret;
- size_t off;
+ uintptr_t ptr_size;
+ uintptr_t full_size;
+ unsigned char *ret;
- page_size = getpagesize ();
- __go_assert (page_size = size);
- ret = __go_alloc (2 * page_size - 1);
- ret = (void *) (((uintptr_t) ret + page_size - 1)
- ~ ((uintptr_t) page_size - 1));
+ /* Because the garbage collector only looks at aligned addresses, we
+ need to store the closure at an aligned address to ensure that it
+ sees it. */
+ ptr_size = sizeof (void *);
+ full_size = (((size + ptr_size - 1) / ptr_size) * ptr_size);
+ full_size += ptr_size;
- /* Because the garbage collector only looks at correct address
- offsets, we need to ensure that it will see the closure
- address. */
- off = ((size + sizeof (void *) - 1) / sizeof (void *)) * sizeof (void *);
- __go_assert (size + off + sizeof (void *) = page_size);
- __builtin_memcpy (ret + off, closure, sizeof (void *));
+ runtime_lock (trampoline_lock);
+
+ if (full_size trampoline_page_size - trampoline_page_used)
+trampoline_page = NULL;
+
+ if (trampoline_page == NULL)
+{
+ uintptr_t page_size;
+ unsigned char *page;
+
+ page_size = getpagesize ();
+ __go_assert (page_size = full_size);
+ page = (unsigned char *) runtime_mallocgc (2 * page_size - 1, 0, 0, 0);
+ page = (unsigned char *) (((uintptr_t) page + page_size - 1)
+ ~ (page_size - 1));
#ifdef HAVE_SYS_MMAN_H
- {
-int i;
-i = mprotect (ret, size, PROT_READ | PROT_WRITE | PROT_EXEC);
-__go_assert (i == 0);
- }
+ {
+ int i;
+
+ i = mprotect (page, page_size, PROT_READ | PROT_WRITE | PROT_EXEC);
+ __go_assert (i == 0);
+ }
#endif
- return ret;
+ trampoline_page = page;
+ trampoline_page_size = page_size;
+ trampoline_page_used = 0;
+}
+
+ ret = trampoline_page + trampoline_page_used;
+ trampoline_page_used += full_size;
+
+ runtime_unlock (trampoline_lock);
+
+ __builtin_memcpy (ret + full_size - ptr_size, closure, ptr_size);
+
+ return (void *) ret;
}
+
+/* Scan the trampoline page when running the garbage collector. This
+ just makes sure that the garbage collector sees the pointer in
+ trampoline_page, so that the page itself is not freed if there are
+ no other references to it. */
+
+void
+runtime_trampoline_scan (void (*scan) (byte *, int64))
+{
+ if (trampoline_page != NULL)
+scan ((byte *) trampoline_page, sizeof trampoline_page);
+}
diff -r 9dad4e6b17e9 libgo/runtime/mgc0.c
--- a/libgo/runtime/mgc0.c Fri May 25 14:12:42 2012 -0700
+++ b/libgo/runtime/mgc0.c Fri May 25 14:45:55 2012 -0700
@@ -703,6 +703,7 @@
scan((byte*)runtime_allm, sizeof runtime_allm);
runtime_MProf_Mark(scan);
runtime_time_scan(scan);
+ runtime_trampoline_scan(scan);
// mark stacks
for(gp=runtime_allg; gp!=nil; gp=gp-alllink) {
diff -r 9dad4e6b17e9 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h Fri May 25 14:12:42 2012 -0700
+++ b/libgo/runtime/runtime.h Fri May 25 14:45:55 2012 -0700
@@ -490,6 +490,7 @@
void runtime_setprof(bool);
void runtime_time_scan(void (*)(byte*, int64));
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/25/12, Mike Stump mikest...@comcast.net wrote: On May 25, 2012, at 10:50 AM, Lawrence Crowl wrote: Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, If portability to other C++ compilers wasn't a concern, we could extend out g++ to make supporting GTY better, so that we can simplify and refine the GTY stuff. Personally, I would rather see if we can take advantage of C++ features to reduce garbage and then use the Boehm collector. There is too much manual management with GTY, and I'd rather the compiler leverage mainstream practice rather than depart from it. I fear we need some light weight reflection, might make a nice language feature for a future C++ standard, if done well. There has been some talk of such a language feature. As yet, the only concrete proposal does not have wide support. I suspect it will take several more proposals before we hit upon one that will get wide support. If you have any good ideas, now would be a really excellent time to speak up! -- Lawrence Crowl
[google/gcc-4_6] Fix -gfission ICEs with pubnames and .debug_addr table (issue6254054)
This patch is for the google/gcc-4_6 branch.
It fixes a problem where we were still trying to output entries
in the .debug_addr table for location lists that were removed,
and fixes a problem where we were getting an ICE while trying
to output a pubname for a member function of a struct.
Tested with the GCC testsuite and validate-failures.py, and
also with an internal Google build with -gfission enabled.
2012-05-25 Sterling Augustine saugust...@google.com
Cary Coutant ccout...@google.com
* gcc/dwarf2out.c (remove_loc_list_addr_table_entries): New function.
(is_class_die): Return TRUE for DW_TAG_structure_type.
(resolve_addr): Remove address table entries when replacing a
location list.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 187887)
+++ gcc/dwarf2out.c (working copy)
@@ -7864,6 +7864,19 @@ remove_addr_table_entry (unsigned int i)
attr-dw_attr = (enum dwarf_attribute) 0;
}
+/* Given a location list, remove all addresses it refers to from the
+ address_table. */
+
+static void
+remove_loc_list_addr_table_entries (dw_loc_list_ref loc)
+{
+ dw_loc_descr_ref descr;
+
+ for (descr = loc-expr; descr; descr = descr-dw_loc_next)
+if (descr-dw_loc_oprnd1.val_index != -1U)
+ remove_addr_table_entry (descr-dw_loc_oprnd1.val_index);
+}
+
/* Add an address constant attribute value to a DIE. */
static inline void
@@ -10033,7 +10046,8 @@ is_namespace_die (dw_die_ref c)
static inline bool
is_class_die (dw_die_ref c)
{
- return c c-die_tag == DW_TAG_class_type;
+ return c (c-die_tag == DW_TAG_class_type
+ || c-die_tag == DW_TAG_structure_type);
}
static char *
@@ -23788,15 +23802,13 @@ resolve_addr (dw_die_ref die)
if (!resolve_addr_in_expr ((*curr)-expr))
{
dw_loc_list_ref next = (*curr)-dw_loc_next;
- dw_loc_descr_ref l = (*curr)-expr;
if (next (*curr)-ll_symbol)
{
gcc_assert (!next-ll_symbol);
next-ll_symbol = (*curr)-ll_symbol;
}
- if (l-dw_loc_oprnd1.val_index != -1U)
- remove_addr_table_entry (l-dw_loc_oprnd1.val_index);
+ remove_loc_list_addr_table_entries (*curr);
*curr = next;
}
else
--
This patch is available for review at http://codereview.appspot.com/6254054
Re: [C++ Patch] PR 32054
Hi,
On 05/25/2012 06:25 PM, Jason Merrill wrote:
On 05/24/2012 10:57 PM, Paolo Carlini wrote:
+error_at (decl_spec_token_start-location,
+ a storage class is not allowed);
Let's give more explanation here so that the user knows that the
problem is a storage class on an anonymous union/struct in class scope.
Good, then I propose the below, using the form anonymous aggregate,
because - I didn't know - we are already using it in error messages
elsewhere, in decl.c too.
Thanks,
Paolo.
Index: testsuite/g++.dg/other/anon-union3.C
===
--- testsuite/g++.dg/other/anon-union3.C(revision 0)
+++ testsuite/g++.dg/other/anon-union3.C(revision 0)
@@ -0,0 +1,25 @@
+// PR c++/32054
+
+class C
+{
+ auto union // { dg-error storage class { target c++98 } }
+{
+ int a;
+};// { dg-error multiple|specified { target c++11 } }
+ register union // { dg-error storage class }
+{
+ int b;
+};
+ static union// { dg-error storage class }
+{
+ int c;
+};
+ extern union// { dg-error storage class }
+{
+ int d;
+};
+ mutable union // { dg-error storage class }
+{
+ int e;
+};
+};
Index: cp/parser.c
===
--- cp/parser.c (revision 187868)
+++ cp/parser.c (working copy)
@@ -18910,6 +18910,12 @@ cp_parser_member_declaration (cp_parser* parser)
particular type), as opposed to a nested class. */
else if (ANON_AGGR_TYPE_P (type))
{
+ /* C++11 9.5/6. */
+ if (decl_specifiers.storage_class != sc_none)
+ error_at (decl_spec_token_start-location,
+ a storage class on an anonymous aggregate
+ in class scope is not allowed);
+
/* Remove constructors and such from TYPE, now that we
know it is an anonymous aggregate. */
fixup_anonymous_aggr (type);
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 4:16 PM, Lawrence Crowl cr...@google.com wrote: On 5/25/12, Jakub Jelinek ja...@redhat.com wrote: On Fri, May 25, 2012 at 10:15:54AM -0400, Diego Novillo wrote: Lawrence, Ian and Gaby have been working on the proposed coding guidelines for C++ (http://gcc.gnu.org/wiki/CppConventions). That page is quite inconsistent. E.g. it first talks about retaining space before ( (which I really hope will be retained, it makes code more readable), but in the example it doesn't bother with it. And the long line example is horribly ugly, ending line with ( ? That ( should surely go on the next line after the few spaces. It should list also examples of when the first parameter isn't too long, but there are too many parameters and so some wrapping is needed, etc. We will clean the page. There are technical guidelines that we can, and I think should, apply to the coding convention. In particular, I believe any coding convention should be consistent with the grammar of the language. The problem with this is that it assumes that the grammar is a logical expression of the ideas and ideas of the language. When that is the case, that is fine. For C++ in particular, that is not the case. We have gone to extra length to fit ideas and ideals into something that has grown barnacles and then patch over those deficiencies by telling/teaching people to write in certain ways. The examples you gave below are great and in some sense illustrate what the C++ would have liked to happen (the ideas and ideals) as opposed to the gritty and unsavory details of the horror that is the C++ grammar. While we should pay attention, I think it would be a mistake to set coding conventions that mirror the horrors of the C++ grammar. Both the GNU style and the C++ Standard style break that guideline in different respects. For example, consider the following GNU code. int *p = *foo (3); Here, the convention implies that foo binds more tightly to the * than it does to the (. This implication is false. Now consider the C++ Standard equivalent. int* p = *foo(3); It fixes the binding of foo, but implies that * is syntactically closer to int than it is to p. That implication is false. Now consider a form that is consistent with the actual grammar of the language: int *p = *foo(3); Here there is no implication that foo binds more closely to either the * or the (, so the form is consistent with the grammar. C++ standard thinks of * binding to int because it puts emphasis on types -- and one of selling points of moving to C++ is that by exploiting the static typing we would eliminate bugs and runtime checks that are there only there because we could not get enough static assurances and be sufficiently expressive and elegant with the current implementation language. Consequently, I think we should retain the binding suggested by the C++ standard. I realize that the existing GNU C convention says the opposite -- but then it is written for C. We can go further and have the spacing reinforce the grammar, but lines will get longer. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 4:52 PM, Lawrence Crowl cr...@google.com wrote: Personally, I would rather see if we can take advantage of C++ features to reduce garbage and then use the Boehm collector. There is too much manual management with GTY, and I'd rather the compiler leverage mainstream practice rather than depart from it. I could not agree more. -- Gaby
Merge from gcc-4_7-branch to gccgo branch
I've merged gcc-4_7-branch revision 187900 to the gccgo branch. Ian
Re: [C++ Patch] PR 32054
OK. Jason
[patch] Fixes to make check_makefile_deps.sh work again
Hello, The following patch was necessary to make check_makefile_deps.sh work on powerpc64-unknown-linux-gnu. Is this OK? Ciao! Steven Index: contrib/ChangeLog === --- contrib/ChangeLog (revision 187901) +++ contrib/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2012-05-25 Steven Bosscher steven at gcc dot gnu dot org + + * check_makefile_deps.sh: Add ecrti.o and ecrtn.o to the list of + objects to skip unconditionaly. Check for c-common.o in c-family/. + 2012-05-25 H.J. Lu hongjiu dot lu at intel dot com PR bootstrap/53472 Index: contrib/check_makefile_deps.sh === --- contrib/check_makefile_deps.sh (revision 187901) +++ contrib/check_makefile_deps.sh (working copy) @@ -19,7 +19,7 @@ # Skip some objects unconditionally; make sure each name in this list is # surrounded by spaces. -skip= crtbegin.o crtbeginS.o crtbeginT.o crtend.o crtendS.o crtfastmath.o crtprec64.o crtprec80.o crtprec32.o +skip= crtbegin.o crtbeginS.o crtbeginT.o crtend.o crtendS.o crtfastmath.o crtprec64.o crtprec80.o crtprec32.o ecrti.o ecrtn.o # Files which show up as dependencies other than through unconditional #include. # This is an egrep pattern. @@ -34,7 +34,7 @@ set -e st=0 -if test -f c-common.o; then :; else +if test -f c-family/c-common.o; then :; else echo $0: rerun in an up to date build-tree/gcc directory 2 exit 1 fi
