Re: [Patch,AVR]: Fix PR49903
Hans-Peter Nilsson schrieb: On Thu, 11 Aug 2011, Georg-Johann Lay wrote: This is an optimization in machine dependent reorg to remove redundant comparisons like in cc0 = compare (Reg, Num) if (cc0 == 0) goto L1 cc0 = compare (Reg, Num) if (cc0 0) goto L2 The second comparison is redundant an can be removed. Code like this can be seen in binary decision switch/case expansion. A glance at AVR makes me think this should already be handled by the NOTICE_UPDATE_CC machinery. Any analysis why this doesn't happen? With the same test-case (at -Os) I don't see redundant compares for cris-elf, for example. One reason is that branch insns set cc0 to clobber where it is actually none. I could not depict the rationale for this from the avr BE, presumably it's because of text peepholes that change branches or jump-over-one-insn skip optimizations. Second reason is that avr has no GT/GTU and therefore reorg transforms cc0 = compare (Reg, Num) if (cc0 0) goto L2 to cc0 = compare (Reg, Num-1) if (cc0 = 0) goto L2 so that the comparisons are no more the same. Of cource, reorg could ommit the last optimization if there is a similar comparison beforehand. But the hard part is not doing/skipping the optimization, the annoyance is detecting the right 4-insn instruction sequences. I also thought about extending genrecog et al. which currently can handle 3 types of things (RECOG, SPLIT, PEEPHOLE2) to a fourth one like INSN_SEQ so that one could write down the sequence as RTL instead of as brain-dead C (brain-dead in the way it must be written down, not in what it does) and use insn-recog, insn-extract etc. to analyse such sequences. This might also be helpful in other backends when doing similar optimizations or writing hand-coded schedulers or when scanning for specific sequences to work around core errata. Johann brgds, H-P
Re: [Patch,AVR]: Fix PR49903
On Sat, 13 Aug 2011, Georg-Johann Lay wrote: Hans-Peter Nilsson schrieb: A glance at AVR makes me think this should already be handled by the NOTICE_UPDATE_CC machinery. Any analysis why this doesn't happen? With the same test-case (at -Os) I don't see redundant compares for cris-elf, for example. One reason is that branch insns set cc0 to clobber where it is actually none. I could not depict the rationale for this from the avr BE, presumably it's because of text peepholes that change branches or jump-over-one-insn skip optimizations. Or gas relaxation of branches ...nope, not in current binutils. (And with gcc keeping track of instruction lengths, that's an obsolete feature for code generated by gcc.) Second reason is that avr has no GT/GTU and therefore reorg transforms Sidenote: please don't refer to it as reorg. Reorg is the delay-slot handler beast living in reorg.c and resource.c (but whose days are numbered, thankfully). ITYM machine-dependent reorg; md_reorg, avr_reorg. cc0 = compare (Reg, Num) if (cc0 0) goto L2 to cc0 = compare (Reg, Num-1) if (cc0 = 0) goto L2 so that the comparisons are no more the same. Sorry, I missed that (those). It just looked like you added a lot of code for something that already should be handled. Of cource, reorg could ommit the last optimization if there is a similar comparison beforehand. But the hard part is not doing/skipping the optimization, the annoyance is detecting the right 4-insn instruction sequences. I also thought about extending genrecog et al. which currently can handle 3 types of things (RECOG, SPLIT, PEEPHOLE2) to a fourth one like INSN_SEQ so that one could write down the sequence as RTL instead of as brain-dead C (brain-dead in the way it must be written down, not in what it does) and use insn-recog, insn-extract etc. to analyse such sequences. I'm not sure what you mean here, except it sounds like yet more code. If you meant to keep the compare and branch together, then there's an option to keep it: define cbranchM as a define_insn, not as define_expand. brgds, H-P
Make genattrtab.c create unique symbol_refs
rtx_equal_p uses:
case SYMBOL_REF:
return XSTR (x, 0) == XSTR (y, 0);
to check whether two symbol_refs are equal. This means that genattrtab,
which uses rtx_equal_p, must create unique strings for them.
attr_rtx has code to create unique strings, and I think it originally
handled symbol_refs. However, it still assumes that only s rtxes are
of interest, whereas symbol_ref is now s00 (decl and flags).
Fixed with the patch below. The attr_string patch ensures that we
preserve #line information for the unique strings. (We'll effectively
pick the first occurence, which should be OK for most cases.)
As explained in the next patch, this doesn't actually bring any
benefit on its own, but it is needed by that patch.
Tested on x86_64-linux-gnu and mips64-linux-gnu. OK to install?
Richard
gcc/
* genattrtab.c (attr_rtx_1): Hash SYMBOL_REFs.
(attr_string): Use copy_md_ptr_loc.
Index: gcc/genattrtab.c
===
--- gcc/genattrtab.c2011-07-23 10:03:35.0 +0100
+++ gcc/genattrtab.c2011-07-23 10:34:20.0 +0100
@@ -433,8 +433,9 @@ attr_rtx_1 (enum rtx_code code, va_list
XEXP (rt_val, 1) = arg1;
}
}
- else if (GET_RTX_LENGTH (code) == 1
- GET_RTX_FORMAT (code)[0] == 's')
+ else if (code == SYMBOL_REF
+ || (GET_RTX_LENGTH (code) == 1
+ GET_RTX_FORMAT (code)[0] == 's'))
{
char *arg0 = va_arg (p, char *);
@@ -452,6 +453,11 @@ attr_rtx_1 (enum rtx_code code, va_list
rtl_obstack = hash_obstack;
rt_val = rtx_alloc (code);
XSTR (rt_val, 0) = arg0;
+ if (code == SYMBOL_REF)
+ {
+ X0EXP (rt_val, 1) = NULL_RTX;
+ X0EXP (rt_val, 2) = NULL_RTX;
+ }
}
}
else if (GET_RTX_LENGTH (code) == 2
@@ -610,6 +616,7 @@ attr_string (const char *str, int len)
memcpy (new_str, str, len);
new_str[len] = '\0';
attr_hash_add_string (hashcode, new_str);
+ copy_md_ptr_loc (new_str, str);
return new_str; /* Return the new string. */
}
Allow match_test to be used for .md attribute tests
Like the recent MEM_ATTRS changes, this patch is supposed to help the
transition to const_ints with modes. However, like those changes,
I think it makes sense on its own.
At the moment, the canonical way of testing a C condition in an
.md attribute is to use:
(ne (symbol_ref TEST) (const_int 0))
The patch below allows the predicate construct:
(match_test TEST)
to be used instead, which is both shorter and IMO easier to read.
The other advantage for me is that it gets rid of some fake const_ints.
Looking at genattrtab.c, I'm not sure whether the use of symbol_ref
in these kinds of attribute test was a deliberate feature, or whether
it's something that worked by accident (on the back of the support for
symbol_ref attribute values, which are definitely a deliberate feature).
There's code to create unique rtxes for comparisons of two symbol_ref
attribute values:
case LE: case LT: case GT: case GE:
case LEU: case LTU: case GTU: case GEU:
case NE: case EQ:
if (GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
GET_CODE (XEXP (exp, 1)) == SYMBOL_REF)
exp = attr_rtx (GET_CODE (exp),
attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 1), 0)));
but no corresponding code for (foo (symbol_ref ...) (const_int 0)).
This means that two separate occurences of:
(ne (symbol_ref TEST) (const_int 0))
in the .md file will appear to genattrtab as two separate conditions
whose relationship is unknown.
As well as the usual bootstrap and regression testing, I tried applying:
Index: gcc/genattrtab.c
===
--- gcc/genattrtab.c2011-08-13 09:11:33.0 +0100
+++ gcc/genattrtab.c2011-08-13 09:43:19.0 +0100
@@ -857,6 +857,25 @@ check_attr_test (rtx exp, int is_const,
exp = attr_rtx (GET_CODE (exp),
attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 1), 0)));
+ if (GET_CODE (exp) == NE
+ GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
+ CONST_INT_P (XEXP (exp, 1))
+ XWINT (XEXP (exp, 1), 0) == 0)
+ exp = attr_rtx (NE,
+ attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
+ false_rtx);
+ if (GET_CODE (exp) == EQ
+ GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
+ CONST_INT_P (XEXP (exp, 1))
+ XWINT (XEXP (exp, 1), 0) == 0)
+ {
+ exp = attr_rtx (NE,
+ attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
+ false_rtx);
+ ATTR_IND_SIMPLIFIED_P (exp) = 1;
+ exp = attr_rtx (NOT, exp);
+ break;
+ }
/* These cases can't be simplified. */
ATTR_IND_SIMPLIFIED_P (exp) = 1;
break;
to an otherwise unpatched compiler. This simplified the insn-attrtab.c
output, because genattrtab was able to merge some case statements with
identical bodies. I used this as the old version of insn-attrtab.c.
I then applied the patch at the end of this message, the i386 patch that
I'm about to post, and a MIPS patch that I'll post if the patch below
is OK. I also applied:
Index: gcc/genattrtab.c
===
--- gcc/genattrtab.c2011-08-13 09:43:46.0 +0100
+++ gcc/genattrtab.c2011-08-13 09:44:02.0 +0100
@@ -3598,9 +3598,9 @@ write_test_expr (rtx exp, unsigned int a
break;
case MATCH_TEST:
+ printf (();
print_c_condition (XSTR (exp, 0));
- if (flags FLG_BITWISE)
- printf ( != 0);
+ printf () != (0));
break;
/* A random C expression. */
in order to make the new output look more like the original. I used the
insn-attrtab.c generated by this new genattrtab as the new version.
After removing #line directives, the old and new versions were the same,
apart from cases where I'd split a single symbol_ref condition into two
match_tests.
Bootstrapped regression-tested on x86_64-linux-gnu with the next patch.
Also regression-tested on mips64-linux-gnu with the corresponding .md patch
to config/mips. (I'll post and commit that if these changes are OK.
It'd just be noise otherwise.) OK to install?
If the patch is OK, I'd like to make corresponding changes to each port's
.md files. Are they simple enough to count as obvious, or should I get
persmission for each one?
Richard
gcc/
* doc/md.texi: Describe the use of match_tests in attribute tests.
* rtl.def (MATCH_TEST): Update commentary.
* genattrtab.c (attr_copy_rtx, check_attr_test, clear_struct_flag)
(write_test_expr, walk_attr_value): Handle MATCH_TEST.
Index: gcc/doc/md.texi
===
--- gcc/doc/md.texi 2011-08-13 09:42:09.0 +0100
+++ gcc/doc/md.texi 2011-08-13
[x86] Use match_test for .md attributes
Following on from the two patches I've just posted, this one makes config/i386/*.md use match_test for .md attributes. Tested as described here: http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01182.html OK to install? Richard gcc/ * config/i386/i386.md: Use (match_test ...) for attribute tests. * config/i386/mmx.md: Likewise. * config/i386/sse.md: Likewise. Index: gcc/config/i386/i386.md === --- gcc/config/i386/i386.md 2011-08-13 11:06:03.0 +0100 +++ gcc/config/i386/i386.md 2011-08-13 11:06:08.0 +0100 @@ -478,18 +478,16 @@ (define_attr prefix_0f ;; Set when REX opcode prefix is used. (define_attr prefix_rex - (cond [(eq (symbol_ref TARGET_64BIT) (const_int 0)) + (cond [(not (match_test TARGET_64BIT)) (const_int 0) (and (eq_attr mode DI) (and (eq_attr type !push,pop,call,callv,leave,ibr) (eq_attr unit !mmx))) (const_int 1) (and (eq_attr mode QI) - (ne (symbol_ref x86_extended_QIreg_mentioned_p (insn)) - (const_int 0))) + (match_test x86_extended_QIreg_mentioned_p (insn))) (const_int 1) -(ne (symbol_ref x86_extended_reg_mentioned_p (insn)) -(const_int 0)) +(match_test x86_extended_reg_mentioned_p (insn)) (const_int 1) (and (eq_attr type imovx) (match_operand:QI 1 ext_QIreg_operand )) @@ -539,7 +537,7 @@ (define_attr modrm (eq_attr unit i387) (const_int 0) (and (eq_attr type incdec) - (and (eq (symbol_ref TARGET_64BIT) (const_int 0)) + (and (not (match_test TARGET_64BIT)) (ior (match_operand:SI 1 register_operand ) (match_operand:HI 1 register_operand (const_int 0) @@ -585,7 +583,7 @@ (define_attr length (attr length_address))) (ior (eq_attr prefix vex) (and (eq_attr prefix maybe_vex) - (ne (symbol_ref TARGET_AVX) (const_int 0 + (match_test TARGET_AVX))) (plus (attr length_vex) (plus (attr length_immediate) (plus (attr modrm) @@ -1911,16 +1909,13 @@ (define_insn *movti_internal_rex64 (set (attr mode) (cond [(eq_attr alternative 2,3) (if_then_else - (ne (symbol_ref optimize_function_for_size_p (cfun)) - (const_int 0)) + (match_test optimize_function_for_size_p (cfun)) (const_string V4SF) (const_string TI)) (eq_attr alternative 4) (if_then_else - (ior (ne (symbol_ref TARGET_SSE_TYPELESS_STORES) - (const_int 0)) - (ne (symbol_ref optimize_function_for_size_p (cfun)) - (const_int 0))) + (ior (match_test TARGET_SSE_TYPELESS_STORES) + (match_test optimize_function_for_size_p (cfun))) (const_string V4SF) (const_string TI))] (const_string DI)))]) @@ -1969,13 +1964,11 @@ (define_insn *movti_internal_sse [(set_attr type sselog1,ssemov,ssemov) (set_attr prefix maybe_vex) (set (attr mode) - (cond [(ior (eq (symbol_ref TARGET_SSE2) (const_int 0)) - (ne (symbol_ref optimize_function_for_size_p (cfun)) - (const_int 0))) + (cond [(ior (not (match_test TARGET_SSE2)) + (match_test optimize_function_for_size_p (cfun))) (const_string V4SF) (and (eq_attr alternative 2) - (ne (symbol_ref TARGET_SSE_TYPELESS_STORES) - (const_int 0))) + (match_test TARGET_SSE_TYPELESS_STORES)) (const_string V4SF)] (const_string TI)))]) @@ -2289,11 +2282,11 @@ (define_insn *movsi_internal (const_string DI) (eq_attr alternative 6,7) (if_then_else - (eq (symbol_ref TARGET_SSE2) (const_int 0)) + (not (match_test TARGET_SSE2)) (const_string V4SF) (const_string TI)) (and (eq_attr alternative 8,9,10,11) -(eq (symbol_ref TARGET_SSE2) (const_int 0))) +(not (match_test TARGET_SSE2))) (const_string SF) ] (const_string SI)))]) @@ -2317,20 +2310,16 @@ (define_insn *movhi_internal } } [(set (attr type) - (cond [(ne (symbol_ref optimize_function_for_size_p (cfun)) - (const_int 0)) + (cond [(match_test optimize_function_for_size_p (cfun)) (const_string imov) (and (eq_attr alternative 0) -(ior (eq (symbol_ref
Re: Allow match_test to be used for .md attribute tests
On Sat, 13 Aug 2011, Richard Sandiford wrote: If the patch is OK, I'd like to make corresponding changes to each port's .md files. Are they simple enough to count as obvious, or should I get persmission for each one? Preapproved for CRIS and MMIX. brgds, H-P
Re: [PATCH 2/2] document ISL requirement for GCC installation
On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers jos...@codesourcery.com wrote: I advise either removing the option for CLooG to use bundled ISL, or making the bundled version the recommended version for GCC. Having too many ways to configure things is bad. I would prefer using the ISL bundled with CLooG and not have to provide a way to configure GCC with ISL. Sebastian
Re: [PATCH 2/2] document ISL requirement for GCC installation
On Sat, Aug 13, 2011 at 11:02:40AM -0500, Sebastian Pop wrote: On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers jos...@codesourcery.com wrote: I advise either removing the option for CLooG to use bundled ISL, or making the bundled version the recommended version for GCC. Having too many ways to configure things is bad. I would prefer using the ISL bundled with CLooG and not have to provide a way to configure GCC with ISL. Sebastian, On a related issue, only isl git... http://repo.or.cz/w/isl.git seems to have these newer changes. These changes need to be synchronized with cloog.org git in order to test your proposed patches. Jack Sebastian
Re: [PATCH 2/2] document ISL requirement for GCC installation
On 08/13/2011 06:02 PM, Sebastian Pop wrote: On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers jos...@codesourcery.com wrote: I advise either removing the option for CLooG to use bundled ISL, or making the bundled version the recommended version for GCC. Having too many ways to configure things is bad. I would prefer using the ISL bundled with CLooG and not have to provide a way to configure GCC with ISL. which would be exactly the way no distribution would use it. So please just don't bundle ISL with CLoog. Matthias
Re: [PATCH 2/2] document ISL requirement for GCC installation
On Sat, Aug 13, 2011 at 11:26, Matthias Klose d...@debian.org wrote: On 08/13/2011 06:02 PM, Sebastian Pop wrote: On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers jos...@codesourcery.com wrote: I advise either removing the option for CLooG to use bundled ISL, or making the bundled version the recommended version for GCC. Having too many ways to configure things is bad. I would prefer using the ISL bundled with CLooG and not have to provide a way to configure GCC with ISL. which would be exactly the way no distribution would use it. So please just don't bundle ISL with CLoog. Sven also pointed out that it would be a mistake to use the ISL bundled with CLooG. So, I will prepare a patch for GCC to use a configure option --with-isl. Sebastian
Small C++ PATCH to reorganize handling of constexpr refs
While looking at 48370, I noticed that this bit of code in
initialize_reference really ought to be in grok_reference_init instead,
since it only applied to top-level references.
Tested x86_64-pc-linux-gnu, applied to trunk.
commit 82efe7318e5cd58632043cc92624b0d31d9ad0d4
Author: Jason Merrill ja...@redhat.com
Date: Fri Aug 12 17:09:20 2011 -0400
* decl.c (grok_reference_init): Handle constexpr here.
* call.c (initialize_reference): Not here.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index e8fb68d..d2700cb 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8820,12 +8820,6 @@ initialize_reference (tree type, tree expr, tree decl, tree *cleanup,
(build_pointer_type (base_conv_type), expr,
complain));
expr = build_nop (type, expr);
- if (DECL_DECLARED_CONSTEXPR_P (decl))
- {
- expr = cxx_constant_value (expr);
- DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
- = reduced_constant_expression_p (expr);
- }
}
}
else
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 1db0748..c125f05 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4597,6 +4597,12 @@ grok_reference_init (tree decl, tree type, tree init, tree *cleanup)
explicitly); we need to allow the temporary to be initialized
first. */
tmp = initialize_reference (type, init, decl, cleanup, tf_warning_or_error);
+ if (DECL_DECLARED_CONSTEXPR_P (decl))
+{
+ tmp = cxx_constant_value (tmp);
+ DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
+ = reduced_constant_expression_p (tmp);
+}
if (tmp == error_mark_node)
return NULL_TREE;
RE: [wwwdocs] Announce new ARM/embedded-4_6-branch branch
Ping. BR, Terry -Original Message- From: Terry Guo [mailto:terry@arm.com] Sent: Thursday, August 11, 2011 3:13 PM To: 'Gerald Pfeifer' Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw; Matthew Gretton-Dann Subject: RE: [wwwdocs] Announce new ARM/embedded-4_6-branch branch Hi Gerald, Thanks for your review. Here I attached the updated one. Is it ok to commit? BR, Terry -Original Message- From: Gerald Pfeifer [mailto:ger...@pfeifer.com] Sent: Monday, August 08, 2011 5:43 PM To: Terry Guo Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw; Matthew Gretton-Dann Subject: Re: [wwwdocs] Announce new ARM/embedded-4_6-branch branch Hi Terry, On Mon, 8 Aug 2011, Terry Guo wrote: This new branch intends to provide fixes and enhancements for GCC 4.6 when used with ARM embedded cores. The attached patch adds documentation for this branch. thanks for thinking to document this! This looks good modulo two notes. + ddThis branch provides bug-fixes, minor enhancements and stability + fixes for GCC-4.6 when used with ARM's embedded cores, such as the + Cortex-R and Cortex-M processors. Most patches will be back-ports + of changes already made on trunk. Patches should be marked with the + tag code[arm-embedded]/code in the subject line. The branch is + maintained by ARM./dd Would you mind making ths GCC 4.6 (without the dash), and perhaps noting which version of GCC 4.6 this is based on (or whether it tracks the general GCC 4.6 branch)? As for maintainership, we usually have individuals, not companies there. Could you name one or two (or three)? You can always adjust the list an any time later on, without any form of approval. Gerald
