Re: [PATCH] Fix dead_debug_insert_before ICE (PR debug/49522)
On Tue, Jul 05, 2011 at 10:35:11AM +0200, Eric Botcazou wrote: There are two kinds of changes we do on the debug insns without immediate rescanning: 1) reset the debug insn 2) replace a reg use with DEBUG_EXPR of the same mode or subreg of a larger DEBUG_EXPR with the same outer mode as the reg In the attached testcase on arm a debug insn is reset, because a multi-reg register has been used there and as the debug insn location was that multi-reg register before, it is now VOIDmode after the reset - (clobber (const_int 0)). That can happen only in this case, right? Otherwise, for a single register, the debug insn would have been removed from debug-head already. If so, how simpler would it be to remove the other uses in dead_debug_reset instead? So you prefer something like this (untested) instead? Without the second loop I have no idea how to make it work in dead_debug_reset, the other dead_debug_use referencing the same insn might be earlier or later in the chain. --- gcc/df-problems.c 2011-07-04 19:17:50.757435754 +0200 +++ gcc/df-problems.c 2011-07-05 22:04:19.817464710 +0200 @@ -3117,6 +3117,25 @@ dead_debug_reset (struct dead_debug *deb else tailp = (*tailp)-next; } + + /* If any other dead_debug_use structs refer to the debug insns + that have been reset above, remove them too. */ + if (debug-to_rescan != NULL) +{ + tailp = debug-head; + while ((cur = *tailp)) + { + insn = DF_REF_INSN (cur-use); + if (bitmap_bit_p (debug-to_rescan, INSN_UID (insn)) + VAR_LOC_UNKNOWN_P (INSN_VAR_LOCATION_LOC (insn))) + { + *tailp = cur-next; + XDELETE (cur); + } + else + tailp = (*tailp)-next; + } +} } /* Add USE to DEBUG. It must be a dead reference to UREGNO in a debug Jakub
Re: [2/11] Neater tests for signbits
On 07/05/11 21:08, Richard Henderson wrote: On 07/01/2011 10:29 AM, Bernd Schmidt wrote: * cse.c (find_comparison_args): Use val_mode_signbit_set_p. * simplify-rtx.c (mode_signbit_p): Use GET_MODE_PRECISION. (val_mode_signbit_p, val_mode_signbit_set_p): New functions. (simplify_const_unary_operation, simplify_binary_operation_1, simplify_const_binary_operation, simplify_const_relational_operation): Use them. Use GET_MODE_MASK for masking and sign-extensions. * combine.c (set_nonzero_bits_and_sign_copies, simplify_set, combine_simplify_rtx, force_to_mode, reg_nonzero_bits_for_combine, simplify_shift_const_1, simplify_comparison): Likewise. * expr.c (convert_modes): Likewise. * rtlanal.c (nonzero_bits1, canonicalize_condition): Likewise. * expmed.c (emit_cstore, emit_store_flag_1, emit_store_flag): Likewise. * rtl.h (val_mode_signbit_p, val_mode_signbit_set_p): Declare. Ok, but, /* We must sign or zero-extend in this case. Start by zero-extending, then sign extend if we need to. */ - val = ((HOST_WIDE_INT) 1 width) - 1; + val = GET_MODE_MASK (oldmode); if (! unsignedp - (val ((HOST_WIDE_INT) 1 (width - 1 -val |= (HOST_WIDE_INT) (-1) width; + val_signbit_known_set_p (oldmode, val)) +val |= ~GET_MODE_MASK (oldmode); return gen_int_mode (val, mode); Shouldn't that sign-extension already be done by gen_int_mode? I think that's a different case than the simplify_rtx one in patch #3. Here, we're extending from oldmode, while gen_int_mode takes care of the extension for mode. Thanks for the reviews! Bernd
Re: [PATCH 4/6] Fix computation of precision.
On Wed, Jun 29, 2011 at 10:35 AM, Sebastian Pop seb...@gmail.com wrote: 2011-06-29 Sebastian Pop sebastian@amd.com * graphite-clast-to-gimple.c (precision_for_value): Removed. (precision_for_interval): Removed. (gcc_type_for_interval): Use mpz_sizeinbase. --- This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49649 -- H.J.
Re: [PATCH, go] Re: Should rename ELFOSABI_LINUX into ELFOSABI_GNU, and drop ELFOSABI_HURD
Hallo! On Tue, 05 Jul 2011 12:19:31 -0700, Ian Lance Taylor i...@google.com wrote: Thomas Schwinge tho...@schwinge.name writes: The only ELFOSABI_* occurrences in GCC trunk are in libgo. Ian, what do you think about the following patch (untested -- what testing does this need)? Is it even worth keeping the ELFOSABI_LINUX alias? (It can never be returned via osabiStrings.) These files are simply copied directly from the master Go library, so the change should be made there. See libgo/README and http://golang.org/doc/contribute.html. I can probably take care of it at some point if you don't want to bother. It would be great if you could do that. I already did file the individual contributor license agreement, but I think learning the whole code review process for this small change is a bit too much for the moment. Grüße, Thomas pgpqV5A6eVIqI.pgp Description: PGP signature
Re: [wwwdocs] Buildstat update for 4.6
On Mon, 4 Jul 2011, Tom G. Christensen wrote: Latest results for 4.6.x You're amazing. Thanks a lot, Tom! Gerald
[pph] Do not clobber unemitted_tinfo_decls and keyed_classes (issue4636085)
This patch removes a FIXME in pph_read_file_contents. Instead of clobbering unemitted_tinfo_decls and keyed_classes, we should add to the existing ones. No new fixes, but this helps with the next patch. Tested on x86_64. Committed to branch. Diego. * pph-streamer-in.c (pph_read_file_contents): Do not clobber unemitted_tinfo_decls and keyed_classes. Append new elements from STREAM. diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c index 0ddcc75..72536a5 100644 --- a/gcc/cp/pph-streamer-in.c +++ b/gcc/cp/pph-streamer-in.c @@ -1314,8 +1314,9 @@ pph_read_file_contents (pph_stream *stream) cpp_ident_use *bad_use; const char *cur_def; cpp_idents_used idents_used; - tree fndecl; + tree fndecl, t, file_keyed_classes; unsigned i; + VEC(tree,gc) *file_unemitted_tinfo_decls; pph_in_identifiers (stream, idents_used); @@ -1334,10 +1335,12 @@ pph_read_file_contents (pph_stream *stream) if (flag_pph_dump_tree) pph_dump_namespace (pph_logfile, global_namespace); - keyed_classes = pph_in_tree (stream); - /* FIXME pph: This call replaces the tinfo, we should merge instead. - See pph_in_tree_vec. */ - unemitted_tinfo_decls = pph_in_tree_vec (stream); + file_keyed_classes = pph_in_tree (stream); + keyed_classes = chainon (file_keyed_classes, keyed_classes); + + file_unemitted_tinfo_decls = pph_in_tree_vec (stream); + for (i = 0; VEC_iterate (tree, file_unemitted_tinfo_decls, i, t); i++) +VEC_safe_push (tree, gc, unemitted_tinfo_decls, t); /* Expand all the functions with bodies that we read from STREAM. */ for (i = 0; VEC_iterate (tree, stream-fns_to_expand, i, fndecl); i++) -- This patch is available for review at http://codereview.appspot.com/4636085
[pph] Stream and restore static_aggregates (issue4626096)
This patch is a partial fix for c1eabi1.cc. We were missing static initializers. There is another source of assembly difference in that file, so I still need to dig some more. This fixes x2nontrivinit.cc, however. I am still not happy with the way we add bindings and symbols to namespaces. We are missing several things, but I'm not quite sure what yet. We'll need to restructure pph_add_bindings_to_namespace. Tested on x86_64. Committed to branch. Diego. * pph-streamer-in.c (pph_add_bindings_to_namespace): Recurse into each namespace bindings, if needed. (pph_read_file_contents): Add static aggregates from STREAM into static_aggregates. * pph-streamer-out.c (pph_write_file_contents): Write static_aggregates. testsuite/ChangeLog.pph * g++.dg/pph/x2nontrivinit.cc: Mark fixed. diff --git a/gcc/cp/ChangeLog.pph b/gcc/cp/ChangeLog.pph index 46b18be..a70ab0e 100644 --- a/gcc/cp/ChangeLog.pph +++ b/gcc/cp/ChangeLog.pph @@ -1,3 +1,12 @@ +2011-07-05 Diego Novillo dnovi...@google.com + + * pph-streamer-in.c (pph_add_bindings_to_namespace): Recurse into + each namespace bindings, if needed. + (pph_read_file_contents): Add static aggregates from STREAM into + static_aggregates. + * pph-streamer-out.c (pph_write_file_contents): Write + static_aggregates. + 2011-07-04 Gabriel Charette gch...@google.com * pph-streamer-in.c (pph_add_bindings_to_namespace): diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c index 72536a5..0bab93b 100644 --- a/gcc/cp/pph-streamer-in.c +++ b/gcc/cp/pph-streamer-in.c @@ -1167,11 +1167,9 @@ pph_add_bindings_to_namespace (struct cp_binding_level *bl, tree ns) /* Pushing a decl into a scope clobbers its DECL_CHAIN. Preserve it. */ chain = DECL_CHAIN (t); - - /* FIXME pph: we should first check to see if it isn't already there. -If it is, we should use this function recursively to merge -the bindings in T in the corresponding namespace. */ pushdecl_into_namespace (t, ns); + if (NAMESPACE_LEVEL (t)) + pph_add_bindings_to_namespace (NAMESPACE_LEVEL (t), t); } } @@ -1314,7 +1312,7 @@ pph_read_file_contents (pph_stream *stream) cpp_ident_use *bad_use; const char *cur_def; cpp_idents_used idents_used; - tree fndecl, t, file_keyed_classes; + tree fndecl, t, file_keyed_classes, file_static_aggregates; unsigned i; VEC(tree,gc) *file_unemitted_tinfo_decls; @@ -1342,6 +1340,9 @@ pph_read_file_contents (pph_stream *stream) for (i = 0; VEC_iterate (tree, file_unemitted_tinfo_decls, i, t); i++) VEC_safe_push (tree, gc, unemitted_tinfo_decls, t); + file_static_aggregates = pph_in_tree (stream); + static_aggregates = chainon (file_static_aggregates, static_aggregates); + /* Expand all the functions with bodies that we read from STREAM. */ for (i = 0; VEC_iterate (tree, stream-fns_to_expand, i, fndecl); i++) { diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c index acc0352..abe05a2 100644 --- a/gcc/cp/pph-streamer-out.c +++ b/gcc/cp/pph-streamer-out.c @@ -1202,6 +1202,7 @@ pph_write_file_contents (pph_stream *stream, cpp_idents_used *idents_used) pph_dump_namespace (pph_logfile, global_namespace); pph_out_tree (stream, keyed_classes, false); pph_out_tree_vec (stream, unemitted_tinfo_decls, false); + pph_out_tree (stream, static_aggregates, false); } diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph index 530adb5..da45f01 100644 --- a/gcc/testsuite/ChangeLog.pph +++ b/gcc/testsuite/ChangeLog.pph @@ -1,3 +1,7 @@ +2011-07-05 Diego Novillo dnovi...@google.com + + * g++.dg/pph/x2nontrivinit.cc: Mark fixed. + 2011-07-04 Diego Novillo dnovi...@google.com * g++.dg/pph/pph.map: Sort. diff --git a/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc b/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc index b142ac2..96ecce2 100644 --- a/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc +++ b/gcc/testsuite/g++.dg/pph/x2nontrivinit.cc @@ -1,3 +1 @@ -// pph asm xdiff - #include x2nontrivinit.h -- This patch is available for review at http://codereview.appspot.com/4626096
C++ PATCH for c++/48157 (loss of explicit template args in member template signature)
We were accidentally discarding the args from a template-id when doing a partial instantiation of a qualified-id. Tested x86_64-pc-linux-gnu, applying to trunk. commit 50ab88c80a398570a84f65802c3e004e46f27eeb Author: Jason Merrill ja...@redhat.com Date: Tue Jul 5 21:54:43 2011 -0400 PR c++/48157 * pt.c (tsubst_qualified_id): Preserve TEMPLATE_ID_EXPR in partial instantiation. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index e7be08b..17ca44c 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -11287,8 +11287,12 @@ tsubst_qualified_id (tree qualified_id, tree args, expr = name; if (dependent_scope_p (scope)) -return build_qualified_name (NULL_TREE, scope, expr, - QUALIFIED_NAME_IS_TEMPLATE (qualified_id)); +{ + if (is_template) + expr = build_min_nt (TEMPLATE_ID_EXPR, expr, template_args); + return build_qualified_name (NULL_TREE, scope, expr, + QUALIFIED_NAME_IS_TEMPLATE (qualified_id)); +} if (!BASELINK_P (name) !DECL_P (expr)) { diff --git a/gcc/testsuite/g++.dg/template/template-id-4.C b/gcc/testsuite/g++.dg/template/template-id-4.C new file mode 100644 index 000..26f4809 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/template-id-4.C @@ -0,0 +1,22 @@ +// PR c++/48157 + +struct AType +{ + templateclass AA + void SomeFuncTemplate() + { } +}; + +template class T +struct TTest2 +{ + templateT struct helper; + + templateclass U + static void check(helperU::template SomeFuncTemplateint *); +}; + +int main() +{ + TTest2 void (AType::*)() ::checkAType(0); +}
Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area
On 07/05/2011 01:54 AM, Alan Modra wrote: Before that, fwprop never tries to work on hard registers. I question this claim. It seems to me that fwprop did look at paradoxical subregs of hard regs before my change. That wasn't part of the design anyway. The main purpose of fwprop's paradoxical subreg propagation is to get rid of back-to-back SI-to-QI-to-SI conversions, and working with pseudos is enough. The patch is okay as far as I'm concerned, but I'm not a maintainer of fwprop. Perhaps it could be conditionalized on SMALL_REGISTER_CLASSES (that's the source of the bug, I think: the single-register class D conflicts with an argument register) but I don't think it's worth it. Paolo
Re: [PATCH] Fix bootstrap on OpenBSD, PR48851
On Mon, 4 Jul 2011, Mike Stump wrote: On Jul 4, 2011, at 4:04 AM, Richard Guenther wrote: It happens that OpenBSD suffers from a bogus fixinclude that changes its perfectly valid NULL define from (void *)0 to 0. The fix itself appears to be very old and is completely bogus I don't agree with the completely bogus part. Why not replace it with: #undef NULL #ifdef __GNUG__ #define NULL __null #else /* G++ */ #ifndef __cplusplus #define NULL ((void *)0) #else /* C++ */ #define NULL 0 #endif /* C++ */ #endif /* G++ */ ? Because I don't know how to do that? This is C++ friendly, C friendly and modern. It should be very safe and should work just about everywhere. - it replaces (void *)0 with 0 under the assumption the former is invalid for C++ - which is true - but 0 is inappropriate for C which is much worse. A #define to 0 is, for the C language, last I checked valid. You may not like it, but welcome to C. 0 may be valid, but it doesn't work for variadic arguments. Thus, I propose to remove the fix altogether. Breaking all systems that are broken, isn't a good tradeoff. Now, looking at the PR, in this case, one could add a bypass __GNUG__ to this fix, and avoid the change on OpenBSD. This would also fix the problem. I do not think removing the fix is a good idea. Sure. As you are objecting please take the PR and do a more proper fix then. I don't really care about OpenBSD or AIX or Interix - I just tried to be helpful. So, now it's yours ;) Thanks, Richard.
Re: [PATCH] Fix tree_could_trap_p so that weak var accesses are considered trapping (PR tree-optimization/49618)
On Mon, Jul 4, 2011 at 8:09 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! Before http://gcc.gnu.org/viewcvs?root=gccview=revrev=168951 set_mem_attributes_minus_bitpos would set MEM_NOTRAP_P for decls based on whether they are DECL_WEAK or not, but now it is set only from !tree_could_trap_p. These patches adjust tree_could_trap_p to say that references to weak vars/functions may trap (for calls it was doing that already). The first version of the patch is intended for 4.7 and only handles that way weak vars/functions that aren't known to be defined somewhere (either in current CU, or in the CUs included in -flto build). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? The second version is simplified one which always treats DECL_WEAK vars as maybe trapping. Ok for 4.6? The trunk version is ok. For the 4.6 version, don't you need a CALL_EXPR case similar to the trunk version? Thanks, Richard. Jakub
Re: [PATCH] Fix tree_could_trap_p so that weak var accesses are considered trapping (PR tree-optimization/49618)
On Tue, Jul 05, 2011 at 10:33:28AM +0200, Richard Guenther wrote: On Mon, Jul 4, 2011 at 8:09 PM, Jakub Jelinek ja...@redhat.com wrote: The second version is simplified one which always treats DECL_WEAK vars as maybe trapping. Ok for 4.6? The trunk version is ok. For the 4.6 version, don't you need a CALL_EXPR case similar to the trunk version? No, as the 4.6 version for FUNCTION_DECLs checks just DECL_WEAK and nothing else, what CALL_EXPR already checks is all that is needed. The reason why I've added recursion for CALL_EXPRs for trunk is so that all the FUNCTION_DECL/VAR_DECL cgraph/varpool lookups don't need to be duplicated. Jakub
Re: [PATCH] Fix dead_debug_insert_before ICE (PR debug/49522)
There are two kinds of changes we do on the debug insns without immediate rescanning: 1) reset the debug insn 2) replace a reg use with DEBUG_EXPR of the same mode or subreg of a larger DEBUG_EXPR with the same outer mode as the reg In the attached testcase on arm a debug insn is reset, because a multi-reg register has been used there and as the debug insn location was that multi-reg register before, it is now VOIDmode after the reset - (clobber (const_int 0)). That can happen only in this case, right? Otherwise, for a single register, the debug insn would have been removed from debug-head already. If so, how simpler would it be to remove the other uses in dead_debug_reset instead? -- Eric Botcazou
Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area
Paolo Bonzini bonz...@gnu.org writes: On 07/05/2011 01:54 AM, Alan Modra wrote: Before that, fwprop never tries to work on hard registers. I question this claim. It seems to me that fwprop did look at paradoxical subregs of hard regs before my change. That wasn't part of the design anyway. The main purpose of fwprop's paradoxical subreg propagation is to get rid of back-to-back SI-to-QI-to-SI conversions, and working with pseudos is enough. OK, in that case H.J., please go ahead. The patch is okay as far as I'm concerned, but I'm not a maintainer of fwprop. You probably should be :-) Richard
Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area
On 07/05/2011 10:51 AM, Richard Sandiford wrote: The patch is okay as far as I'm concerned, but I'm not a maintainer of fwprop. You probably should be:-) I'd have no problem with that! Paolo
Re: [PATCH] Fix tree_could_trap_p so that weak var accesses are considered trapping (PR tree-optimization/49618)
On Tue, Jul 5, 2011 at 10:43 AM, Jakub Jelinek ja...@redhat.com wrote: On Tue, Jul 05, 2011 at 10:33:28AM +0200, Richard Guenther wrote: On Mon, Jul 4, 2011 at 8:09 PM, Jakub Jelinek ja...@redhat.com wrote: The second version is simplified one which always treats DECL_WEAK vars as maybe trapping. Ok for 4.6? The trunk version is ok. For the 4.6 version, don't you need a CALL_EXPR case similar to the trunk version? No, as the 4.6 version for FUNCTION_DECLs checks just DECL_WEAK and nothing else, what CALL_EXPR already checks is all that is needed. The reason why I've added recursion for CALL_EXPRs for trunk is so that all the FUNCTION_DECL/VAR_DECL cgraph/varpool lookups don't need to be duplicated. Ah, yeah. Ok for 4.6 then. Thanks, Richard. Jakub
Re: CFT: Move unwinder to toplevel libgcc
On Jul 4, 2011, at 8:09 PM, Rainer Orth wrote: Joseph S. Myers jos...@codesourcery.com writes: On Mon, 20 Jun 2011, Rainer Orth wrote: * Move all remaining unwinder-only macros to libgcc: UNW_IVMS_MODE, MD_UNW_COMPATIBLE_PERSONALITY_P, MD_FROB_UPDATE_CONTEXT. I don't see any sign of macros being poisoned in system.h. For macros used in target-independent unwinder code - at least MD_FROB_UPDATE_CONTEXT - that used to be defined in the host tm.h but now no longer should be, I think poisoning in system.h is appropriate. Done in the updated patch below. Given that the other two are ia64 only and not documented in md.texi, I don't think they need to be poisoned. Otherwise, the patch is unchanged from the original submission: [build] Move unwinder to toplevel libgcc http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01452.html Unfortunately, it hasn't seen much comment. I'm now looking for testers especially on platforms with more change and approval of those parts: * Several IA-64 targets: ia64*-*-linux* ia64*-*-hpux* ia64-hp-*vms* For ia64-hp-vms, consider your patch approved if the parts for ia64 are. In case of break, I will fix them. Tristan.
Re: [PATCH] Fix PR49518
On Tue, 5 Jul 2011, Ira Rosen wrote: Richard Guenther rguent...@suse.de wrote on 04/07/2011 03:30:59 PM: Richard Guenther rguent...@suse.de wrote on 04/07/2011 02:38:50 PM: Handling of negative steps broke one of the many asserts in the vectorizer. The following patch drops one that I can't make sense of. I think all asserts need comments - especially this one would, as I can't see why using vf is correct to test against and not nelements (and why = vf and not vf). There is an explanation 10 rows above the assert. It doesn't make sense to peel more than vf iterations (and not nelements, since for the case of multiple types it may help to align more data-refs - see the comment in the code). IIRC = is for the case of aligned access, but I am not sure about that, so maybe you are right. I don't see how it is related to negative steps. I think that the real reason for this failure is that the loads are actually irrelevant (hence, vf=4 that doesn't take char loads into account), but we don't check that when we analyze data-refs. So, in my opinion, the proper fix will add such check. The following also works for me: Index: tree-vect-data-refs.c === --- tree-vect-data-refs.c (revision 175802) +++ tree-vect-data-refs.c (working copy) @@ -1495,6 +1495,9 @@ vect_enhance_data_refs_alignment (loop_v stmt = DR_STMT (dr); stmt_info = vinfo_for_stmt (stmt); + if (!STMT_VINFO_RELEVANT (stmt_info)) + continue; + /* For interleaving, only the alignment of the first access matters. */ if (STMT_VINFO_STRIDED_ACCESS (stmt_info) does that look better or do you propose to clean the datarefs vector from those references? Well, this is certainly enough to fix the PR. I am not sure if we can just remove these data-refs from the dependence checks. After that all the alignment and access checks are at least redundant. Ok. The following is what I have tested for this PR and also for PR49628. Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk? Thanks, Richard. 2011-07-04 Richard Guenther rguent...@suse.de PR tree-optimization/49518 PR tree-optimization/49628 * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Skip irrelevant and invariant data-references. (vect_analyze_data_ref_access): For invariant loads clear the group association. * g++.dg/torture/pr49628.C: New testcase. * gcc.dg/torture/pr49518.c: Likewise. Index: gcc/testsuite/g++.dg/torture/pr49628.C === *** gcc/testsuite/g++.dg/torture/pr49628.C (revision 0) --- gcc/testsuite/g++.dg/torture/pr49628.C (revision 0) *** *** 0 --- 1,37 + /* { dg-do compile } */ + + #include vector + + template int rank, int dim class Tensor; + template int dim class Tensor1,dim { + public: + explicit Tensor (const bool initialize = true); + Tensor (const Tensor1,dim ); + Tensor1,dim operator = (const Tensor1,dim ); + double values[(dim!=0) ? (dim) : 1]; + }; + template int dim + inline Tensor1,dim Tensor1,dim::operator = (const Tensor1,dim p) + { + for (unsigned int i=0; idim; ++i) + values[i] = p.values[i]; + }; + template int dim class Quadrature { + public: + const unsigned int n_quadrature_points; + }; + class MappingQ1 + { + class InternalData { + public: + std::vectorTensor1,3 shape_derivatives; + unsigned int n_shape_functions; + }; + void compute_data (const Quadrature3 quadrature, InternalData data) + const; + }; + void MappingQ1::compute_data (const Quadrature3 q, InternalData data) const + { + const unsigned int n_q_points = q.n_quadrature_points; + data.shape_derivatives.resize(data.n_shape_functions * n_q_points); + } Index: gcc/testsuite/gcc.dg/torture/pr49518.c === *** gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0) *** *** 0 --- 1,19 + /* { dg-do compile } */ + + int a, b; + struct S { unsigned int s, t, u; } c, d = { 0, 1, 0 }; + + void + test (unsigned char z) + { + char e[] = {0, 0, 0, 0, 1}; + for (c.s = 1; c.s; c.s++) + { + b = e[c.s]; + if (a) + break; + b = z = c.u; + if (d.t) + break; + } + } Index: gcc/tree-vect-data-refs.c === --- gcc/tree-vect-data-refs.c (revision 175802) +++ gcc/tree-vect-data-refs.c (working copy) @@ -1495,12 +1495,19 @@ vect_enhance_data_refs_alignment (loop_v stmt = DR_STMT (dr); stmt_info = vinfo_for_stmt (stmt); + if (!STMT_VINFO_RELEVANT (stmt_info)) +
[testsuite]: Add require fopenmp as needed
There is a testcase that fails if no openmp is available. This patch fixed that. CCed contributor. Johann * gcc.dg/cpp/pragma-3.c: Add dg-require-effective-target fopenmp. Index: gcc.dg/cpp/pragma-3.c === --- gcc.dg/cpp/pragma-3.c (revision 175811) +++ gcc.dg/cpp/pragma-3.c (working copy) @@ -1,6 +1,7 @@ /* { dg-options -fopenmp } { dg-do preprocess } + { dg-require-effective-target fopenmp } */ void foo (void)
Re: [PATCH] Fix PR49518
Richard Guenther rguent...@suse.de wrote on 05/07/2011 12:35:24 PM: On Tue, 5 Jul 2011, Ira Rosen wrote: Richard Guenther rguent...@suse.de wrote on 04/07/2011 03:30:59 PM: Richard Guenther rguent...@suse.de wrote on 04/07/2011 02:38:50 PM: Handling of negative steps broke one of the many asserts in the vectorizer. The following patch drops one that I can't make sense of. I think all asserts need comments - especially this one would, as I can't see why using vf is correct to test against and not nelements (and why = vf and not vf). There is an explanation 10 rows above the assert. It doesn't make sense to peel more than vf iterations (and not nelements, since for the case of multiple types it may help to align more data-refs - see the comment in the code). IIRC = is for the case of aligned access, but I am not sure about that, so maybe you are right. I don't see how it is related to negative steps. I think that the real reason for this failure is that the loads are actually irrelevant (hence, vf=4 that doesn't take char loads into account), but we don't check that when we analyze data-refs. So, in my opinion, the proper fix will add such check. The following also works for me: Index: tree-vect-data-refs.c === --- tree-vect-data-refs.c (revision 175802) +++ tree-vect-data-refs.c (working copy) @@ -1495,6 +1495,9 @@ vect_enhance_data_refs_alignment (loop_v stmt = DR_STMT (dr); stmt_info = vinfo_for_stmt (stmt); + if (!STMT_VINFO_RELEVANT (stmt_info)) + continue; + /* For interleaving, only the alignment of the first access matters. */ if (STMT_VINFO_STRIDED_ACCESS (stmt_info) does that look better or do you propose to clean the datarefs vector from those references? Well, this is certainly enough to fix the PR. I am not sure if we can just remove these data-refs from the dependence checks. After that all the alignment and access checks are at least redundant. Ok. The following is what I have tested for this PR and also for PR49628. Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk? Yes. Thanks, Ira Thanks, Richard. 2011-07-04 Richard Guenther rguent...@suse.de PR tree-optimization/49518 PR tree-optimization/49628 * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Skip irrelevant and invariant data-references. (vect_analyze_data_ref_access): For invariant loads clear the group association. * g++.dg/torture/pr49628.C: New testcase. * gcc.dg/torture/pr49518.c: Likewise. Index: gcc/testsuite/g++.dg/torture/pr49628.C === *** gcc/testsuite/g++.dg/torture/pr49628.C (revision 0) --- gcc/testsuite/g++.dg/torture/pr49628.C (revision 0) *** *** 0 --- 1,37 + /* { dg-do compile } */ + + #include vector + + template int rank, int dim class Tensor; + template int dim class Tensor1,dim { + public: + explicit Tensor (const bool initialize = true); + Tensor (const Tensor1,dim ); + Tensor1,dim operator = (const Tensor1,dim ); + double values[(dim!=0) ? (dim) : 1]; + }; + template int dim + inline Tensor1,dim Tensor1,dim::operator = (const Tensor1,dim p) + { + for (unsigned int i=0; idim; ++i) + values[i] = p.values[i]; + }; + template int dim class Quadrature { + public: + const unsigned int n_quadrature_points; + }; + class MappingQ1 + { + class InternalData { + public: + std::vectorTensor1,3 shape_derivatives; + unsigned int n_shape_functions; + }; + void compute_data (const Quadrature3 quadrature, InternalData data) + const; + }; + void MappingQ1::compute_data (const Quadrature3 q, InternalData data) const + { + const unsigned int n_q_points = q.n_quadrature_points; + data.shape_derivatives.resize(data.n_shape_functions * n_q_points); + } Index: gcc/testsuite/gcc.dg/torture/pr49518.c === *** gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr49518.c (revision 0) *** *** 0 --- 1,19 + /* { dg-do compile } */ + + int a, b; + struct S { unsigned int s, t, u; } c, d = { 0, 1, 0 }; + + void + test (unsigned char z) + { + char e[] = {0, 0, 0, 0, 1}; + for (c.s = 1; c.s; c.s++) + { + b = e[c.s]; + if (a) +break; + b = z = c.u; + if (d.t) +break; + } + } Index: gcc/tree-vect-data-refs.c === --- gcc/tree-vect-data-refs.c (revision 175802) +++ gcc/tree-vect-data-refs.c (working copy) @@ -1495,12 +1495,19 @@
Re: PATCH] PR 49580
Zdenek Dvorak rakd...@kam.mff.cuni.cz wrote on 30/06/2011 15:21:43: From: Zdenek Dvorak rakd...@kam.mff.cuni.cz To: Razya Ladelsky/Haifa/IBM@IBMIL Cc: gcc-patches@gcc.gnu.org, Richard Guenther richard.guent...@gmail.com Date: 30/06/2011 15:21 Subject: Re: PATCH] PR 49580 Hi, This patch fixes the build failure of gcc spec2006 benchmark. The change is in gimple_duplicate_sese_tail(), where we need to subtract 1 from the loop's number of iterations. The stmt created to change the rhs of the loop's condition, used to be inserted right after the defining stmt of the rhs (an ssa name). This requires special handling of different cases of the defining stmt: 1.gimple_stmt 2.gimple_phi 3.default_def Instead of handling each case separately, if we insert the new stmt at the begining of the loop's preheader, we're sure that it's already been defined. Bootstrap and testsuite pass successfully (as autopar is not enabled by default). No new regressions when the testsuite is run with autopar enabled. No new regressions for the run of spec2006 with autopar enabled, and gcc benchmark now passes successfully.. OK for trunk? actually, I think a better approach would be not to have this kind of pass-specific adjustments in gimple_duplicate_sese_tail at all. The code decreasing the number of iterations in gimple_duplicate_sese_tail only works because parloops does induction variable canonicalization first; should we need it to be used anywhere else, it will break. I.e., parloops should first transform the condition so that it does the comparison with the adjusted value, and then gimple_duplicate_sese_tail could do just code copying and cfg changes, Zdenek Hi, I moved the adjustment of the loop's iterations from gimple_duplicate_sese_tail to tree-parloops.c, right before the call to gimple_duplicate_sese_tail. I repeated the bootstrap, regression and spec runs - no new regressions. OK to commit? Thanks, razya Index: gcc/tree-parloops.c === --- gcc/tree-parloops.c (revision 174166) +++ gcc/tree-parloops.c (working copy) @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h gimple phi, nphi, cond_stmt, stmt, cond_nit; gimple_stmt_iterator gsi; tree nit_1; + edge exit_1; + tree new_rhs; split_block_after_labels (loop-header); orig_header = single_succ (loop-header); @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h control = t; } } + + /* Setting the condition towards peeling the last iteration: +If the block consisting of the exit condition has the latch as +successor, then the body of the loop is executed before +the exit condition is tested. In such case, moving the +condition to the entry, causes that the loop will iterate +one less iteration (which is the wanted outcome, since we +peel out the last iteration). If the body is executed after +the condition, moving the condition to the entry requires +decrementing one iteration. */ + exit_1 = EDGE_SUCC (exit-src, EDGE_SUCC (exit-src, 0) == exit); + if (exit_1-dest == loop-latch) +new_rhs = gimple_cond_rhs (cond_stmt); + else + { +new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), + gimple_cond_rhs (cond_stmt), + build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); +if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) + { + basic_block preheader; + gimple_stmt_iterator gsi1; + + preheader = loop_preheader_edge(loop)-src; + gsi1 = gsi_after_labels (preheader); + new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, + NULL_TREE,false,GSI_CONTINUE_LINKING); + } + } + gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs)); + gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt))); + bbs = get_loop_body_in_dom_order (loop); for (n = 0; bbs[n] != loop-latch; n++) Index: gcc/tree-cfg.c === --- gcc/tree-cfg.c (revision 174166) +++ gcc/tree-cfg.c (working copy) @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U int total_freq = 0, exit_freq = 0; gcov_type total_count = 0, exit_count = 0; edge exits[2], nexits[2], e; - gimple_stmt_iterator gsi,gsi1; + gimple_stmt_iterator gsi; gimple cond_stmt; edge sorig, snew; basic_block exit_bb; - basic_block iters_bb; - tree new_rhs; gimple_stmt_iterator psi; gimple phi; tree def; @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND); cond_stmt = gimple_copy (cond_stmt); - /* If the block consisting of the exit condition has the latch as -
Re: PATCH] PR 49580
Hi, I moved the adjustment of the loop's iterations from gimple_duplicate_sese_tail to tree-parloops.c, right before the call to gimple_duplicate_sese_tail. I repeated the bootstrap, regression and spec runs - no new regressions. OK to commit? OK, Zdenek Index: gcc/tree-parloops.c === --- gcc/tree-parloops.c (revision 174166) +++ gcc/tree-parloops.c (working copy) @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h gimple phi, nphi, cond_stmt, stmt, cond_nit; gimple_stmt_iterator gsi; tree nit_1; + edge exit_1; + tree new_rhs; split_block_after_labels (loop-header); orig_header = single_succ (loop-header); @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h control = t; } } + + /* Setting the condition towards peeling the last iteration: +If the block consisting of the exit condition has the latch as +successor, then the body of the loop is executed before +the exit condition is tested. In such case, moving the +condition to the entry, causes that the loop will iterate +one less iteration (which is the wanted outcome, since we +peel out the last iteration). If the body is executed after +the condition, moving the condition to the entry requires +decrementing one iteration. */ + exit_1 = EDGE_SUCC (exit-src, EDGE_SUCC (exit-src, 0) == exit); + if (exit_1-dest == loop-latch) +new_rhs = gimple_cond_rhs (cond_stmt); + else + { +new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), +gimple_cond_rhs (cond_stmt), +build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); +if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) + { + basic_block preheader; + gimple_stmt_iterator gsi1; + + preheader = loop_preheader_edge(loop)-src; + gsi1 = gsi_after_labels (preheader); + new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, + NULL_TREE,false,GSI_CONTINUE_LINKING); + } + } + gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs)); + gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt))); + bbs = get_loop_body_in_dom_order (loop); for (n = 0; bbs[n] != loop-latch; n++) Index: gcc/tree-cfg.c === --- gcc/tree-cfg.c(revision 174166) +++ gcc/tree-cfg.c(working copy) @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U int total_freq = 0, exit_freq = 0; gcov_type total_count = 0, exit_count = 0; edge exits[2], nexits[2], e; - gimple_stmt_iterator gsi,gsi1; + gimple_stmt_iterator gsi; gimple cond_stmt; edge sorig, snew; basic_block exit_bb; - basic_block iters_bb; - tree new_rhs; gimple_stmt_iterator psi; gimple phi; tree def; @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND); cond_stmt = gimple_copy (cond_stmt); - /* If the block consisting of the exit condition has the latch as -successor, then the body of the loop is executed before -the exit condition is tested. In such case, moving the -condition to the entry, causes that the loop will iterate -one less iteration (which is the wanted outcome, since we -peel out the last iteration). If the body is executed after -the condition, moving the condition to the entry requires -decrementing one iteration. */ - if (exits[1]-dest == orig_loop-latch) -new_rhs = gimple_cond_rhs (cond_stmt); - else - { -new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), -gimple_cond_rhs (cond_stmt), -build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); - -if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) - { - iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))); - for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (gsi1)) - if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))) - break; - - new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, - NULL_TREE,false,GSI_CONTINUE_LINKING); - } - } - gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs)); - gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt))); gsi_insert_after (gsi, cond_stmt, GSI_NEW_STMT); sorig = single_succ_edge (switch_bb); 07-05-2011 Razya Ladelsky ra...@il.ibm.com * tree-cfg.c (gimple_duplicate_sese_tail): Remove handling of the loop's number of iterations. * tree-parloops.c
Re: PATCH] PR 49580
Zdenek Dvorak rakd...@kam.mff.cuni.cz wrote on 05/07/2011 13:37:41: From: Zdenek Dvorak rakd...@kam.mff.cuni.cz To: Razya Ladelsky/Haifa/IBM@IBMIL Cc: gcc-patches@gcc.gnu.org, Richard Guenther richard.guent...@gmail.com Date: 05/07/2011 13:37 Subject: Re: PATCH] PR 49580 Hi, I moved the adjustment of the loop's iterations from gimple_duplicate_sese_tail to tree-parloops.c, right before the call to gimple_duplicate_sese_tail. I repeated the bootstrap, regression and spec runs - no new regressions. OK to commit? OK, Zdenek I also want to commit this testcase, which is a reduced case of the gcc benchmark. I apologize for not submitting it together with the patch. OK? Thanks, Razya Index: gcc/tree-parloops.c === --- gcc/tree-parloops.c (revision 174166) +++ gcc/tree-parloops.c (working copy) @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h gimple phi, nphi, cond_stmt, stmt, cond_nit; gimple_stmt_iterator gsi; tree nit_1; + edge exit_1; + tree new_rhs; split_block_after_labels (loop-header); orig_header = single_succ (loop-header); @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h control = t; } } + + /* Setting the condition towards peeling the last iteration: +If the block consisting of the exit condition has the latch as +successor, then the body of the loop is executed before +the exit condition is tested. In such case, moving the +condition to the entry, causes that the loop will iterate +one less iteration (which is the wanted outcome, since we +peel out the last iteration). If the body is executed after +the condition, moving the condition to the entry requires +decrementing one iteration. */ + exit_1 = EDGE_SUCC (exit-src, EDGE_SUCC (exit-src, 0) == exit); + if (exit_1-dest == loop-latch) +new_rhs = gimple_cond_rhs (cond_stmt); + else + { +new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), +gimple_cond_rhs (cond_stmt), +build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); +if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) + { +basic_block preheader; + gimple_stmt_iterator gsi1; + + preheader = loop_preheader_edge(loop)-src; + gsi1 = gsi_after_labels (preheader); + new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, + NULL_TREE,false,GSI_CONTINUE_LINKING); + } + } + gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs)); + gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt))); + bbs = get_loop_body_in_dom_order (loop); for (n = 0; bbs[n] != loop-latch; n++) Index: gcc/tree-cfg.c === --- gcc/tree-cfg.c (revision 174166) +++ gcc/tree-cfg.c (working copy) @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U int total_freq = 0, exit_freq = 0; gcov_type total_count = 0, exit_count = 0; edge exits[2], nexits[2], e; - gimple_stmt_iterator gsi,gsi1; + gimple_stmt_iterator gsi; gimple cond_stmt; edge sorig, snew; basic_block exit_bb; - basic_block iters_bb; - tree new_rhs; gimple_stmt_iterator psi; gimple phi; tree def; @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND); cond_stmt = gimple_copy (cond_stmt); - /* If the block consisting of the exit condition has the latch as -successor, then the body of the loop is executed before -the exit condition is tested. In such case, moving the -condition to the entry, causes that the loop will iterate -one less iteration (which is the wanted outcome, since we -peel out the last iteration). If the body is executed after -the condition, moving the condition to the entry requires -decrementing one iteration. */ - if (exits[1]-dest == orig_loop-latch) -new_rhs = gimple_cond_rhs (cond_stmt); - else - { -new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), -gimple_cond_rhs (cond_stmt), -build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); - -if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) - { - iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))); - for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (gsi1)) - if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))) - break; - - new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, - NULL_TREE,false,GSI_CONTINUE_LINKING); - } -
Re: PATCH] PR 49580
On Tue, Jul 5, 2011 at 1:08 PM, Razya Ladelsky ra...@il.ibm.com wrote: Zdenek Dvorak rakd...@kam.mff.cuni.cz wrote on 05/07/2011 13:37:41: From: Zdenek Dvorak rakd...@kam.mff.cuni.cz To: Razya Ladelsky/Haifa/IBM@IBMIL Cc: gcc-patches@gcc.gnu.org, Richard Guenther richard.guent...@gmail.com Date: 05/07/2011 13:37 Subject: Re: PATCH] PR 49580 Hi, I moved the adjustment of the loop's iterations from gimple_duplicate_sese_tail to tree-parloops.c, right before the call to gimple_duplicate_sese_tail. I repeated the bootstrap, regression and spec runs - no new regressions. OK to commit? OK, Zdenek I also want to commit this testcase, which is a reduced case of the gcc benchmark. I apologize for not submitting it together with the patch. OK? Ok. Thanks, Richard. Thanks, Razya Index: gcc/tree-parloops.c === --- gcc/tree-parloops.c (revision 174166) +++ gcc/tree-parloops.c (working copy) @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h gimple phi, nphi, cond_stmt, stmt, cond_nit; gimple_stmt_iterator gsi; tree nit_1; + edge exit_1; + tree new_rhs; split_block_after_labels (loop-header); orig_header = single_succ (loop-header); @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h control = t; } } + + /* Setting the condition towards peeling the last iteration: + If the block consisting of the exit condition has the latch as + successor, then the body of the loop is executed before + the exit condition is tested. In such case, moving the + condition to the entry, causes that the loop will iterate + one less iteration (which is the wanted outcome, since we + peel out the last iteration). If the body is executed after + the condition, moving the condition to the entry requires + decrementing one iteration. */ + exit_1 = EDGE_SUCC (exit-src, EDGE_SUCC (exit-src, 0) == exit); + if (exit_1-dest == loop-latch) + new_rhs = gimple_cond_rhs (cond_stmt); + else + { + new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), + gimple_cond_rhs (cond_stmt), + build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); + if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) + { + basic_block preheader; + gimple_stmt_iterator gsi1; + + preheader = loop_preheader_edge(loop)-src; + gsi1 = gsi_after_labels (preheader); + new_rhs = force_gimple_operand_gsi (gsi1, new_rhs, true, + NULL_TREE,false,GSI_CONTINUE_LINKING); + } + } + gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs)); + gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt))); + bbs = get_loop_body_in_dom_order (loop); for (n = 0; bbs[n] != loop-latch; n++) Index: gcc/tree-cfg.c === --- gcc/tree-cfg.c (revision 174166) +++ gcc/tree-cfg.c (working copy) @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U int total_freq = 0, exit_freq = 0; gcov_type total_count = 0, exit_count = 0; edge exits[2], nexits[2], e; - gimple_stmt_iterator gsi,gsi1; + gimple_stmt_iterator gsi; gimple cond_stmt; edge sorig, snew; basic_block exit_bb; - basic_block iters_bb; - tree new_rhs; gimple_stmt_iterator psi; gimple phi; tree def; @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND); cond_stmt = gimple_copy (cond_stmt); - /* If the block consisting of the exit condition has the latch as - successor, then the body of the loop is executed before - the exit condition is tested. In such case, moving the - condition to the entry, causes that the loop will iterate - one less iteration (which is the wanted outcome, since we - peel out the last iteration). If the body is executed after - the condition, moving the condition to the entry requires - decrementing one iteration. */ - if (exits[1]-dest == orig_loop-latch) - new_rhs = gimple_cond_rhs (cond_stmt); - else - { - new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)), - gimple_cond_rhs (cond_stmt), - build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1)); - - if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) - { - iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))); - for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (gsi1)) - if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))) - break; - - new_rhs = force_gimple_operand_gsi (gsi1,
[PATCH][C][C++] Move common tree node building to c_common_nodes_and_builtins
This consolidates build_common_tree_nodes and build_common_tree_nodes_2 at a single place in c_common_nodes_and_builtins for C family languages. It is a preparation for merging those two functions and moving them to be called from toplev.c as they are middle-end inits. Bootstrapped and tested on x86_64-unknown-linux-gnu for all C family languages and Java. Ok for trunk? Thanks, Richard. 2011-07-05 Richard Guenther rguent...@suse.de c-family/ * c-common.c (c_common_nodes_and_builtins): Build all common tree nodes first. * c-decl.c (c_init_decl_processing): Defer building common tree nodes to c_common_nodes_and_builtins. cp/ * decl.c (cxx_init_decl_processing): Defer building common tree nodes to c_common_nodes_and_builtins. Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 175840) +++ gcc/c-family/c-common.c (working copy) @@ -4576,6 +4576,9 @@ c_common_nodes_and_builtins (void) tree va_list_ref_type_node; tree va_list_arg_type_node; + build_common_tree_nodes (flag_signed_char); + build_common_tree_nodes_2 (flag_short_double); + /* Define `int' and `char' first so that dbx will output them first. */ record_builtin_type (RID_INT, NULL, integer_type_node); record_builtin_type (RID_CHAR, char, char_type_node); @@ -4675,8 +4678,6 @@ c_common_nodes_and_builtins (void) pid_type_node = TREE_TYPE (identifier_global_value (get_identifier (PID_TYPE))); - build_common_tree_nodes_2 (flag_short_double); - record_builtin_type (RID_FLOAT, NULL, float_type_node); record_builtin_type (RID_DOUBLE, NULL, double_type_node); record_builtin_type (RID_MAX, long double, long_double_type_node); Index: gcc/c-decl.c === --- gcc/c-decl.c(revision 175840) +++ gcc/c-decl.c(working copy) @@ -3478,8 +3478,6 @@ c_init_decl_processing (void) using preprocessed headers. */ input_location = BUILTINS_LOCATION; - build_common_tree_nodes (flag_signed_char); - c_common_nodes_and_builtins (); /* In C, comparisons and TRUTH_* expressions have type int. */ Index: gcc/cp/decl.c === --- gcc/cp/decl.c (revision 175840) +++ gcc/cp/decl.c (working copy) @@ -3518,8 +3518,6 @@ cxx_init_decl_processing (void) tree void_ftype; tree void_ftype_ptr; - build_common_tree_nodes (flag_signed_char); - /* Create all the identifiers we need. */ initialize_predefined_identifiers (); @@ -3536,8 +3534,6 @@ cxx_init_decl_processing (void) TREE_PUBLIC (global_namespace) = 1; begin_scope (sk_namespace, global_namespace); - current_lang_name = NULL_TREE; - if (flag_visibility_ms_compat) default_visibility = VISIBILITY_HIDDEN;
[PATCH, go] Re: Should rename ELFOSABI_LINUX into ELFOSABI_GNU, and drop ELFOSABI_HURD
Hallo! On Thu, 30 Jun 2011 10:48:02 +0100, Nick Clifton ni...@redhat.com wrote: 2011-06-19 Samuel Thibault samuel.thiba...@gnu.org * elf.c (_bfd_elf_set_osabi): Use ELFOSABI_GNU name instead of ELFOSABI_LINUX alias. * elf32-hppa.c (elf32_hppa_object_p): Likewise. * elf64-hppa.c (elf32_hppa_object_p): Likewise. 2011-06-19 Samuel Thibault samuel.thiba...@gnu.org * elfedit.c (osabis): Use ELFOSABI_GNU name instead of ELFOSABI_LINUX alias and ELFOSABI_HURD. Add GNU alias. * readelf.c (get_osabi_name): Likewise. (get_symbol_binding): Likewise. (get_symbol_type): Likewise. 2011-06-19 Samuel Thibault samuel.thiba...@gnu.org * elfcpp.h (ELFOSABI): Add ELFOSABI_GNU with value of ELFOSABI_LINUX, keep ELFOSABI_LINUX as an alias. Remove ELFOSABI_HURD. 2011-06-19 Samuel Thibault samuel.thiba...@gnu.org * config/obj-elf.c (obj_elf_type): Use ELFOSABI_GNU name instead of ELFOSABI_LINUX alias. 2011-06-19 Samuel Thibault samuel.thiba...@gnu.org * common.h (ELFOSABI_GNU): Define, replaces... (ELFOSABI_LINUX): ... this, kept as an alias. (ELFOSABI_HURD): Remove. Approved - please apply. The only ELFOSABI_* occurrences in GCC trunk are in libgo. Ian, what do you think about the following patch (untested -- what testing does this need)? Is it even worth keeping the ELFOSABI_LINUX alias? (It can never be returned via osabiStrings.) (No ChangeLog.) --- libgo/go/debug/elf/elf.go |7 +++ libgo/go/debug/elf/elf_test.go |2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/libgo/go/debug/elf/elf.go b/libgo/go/debug/elf/elf.go index 5d45b24..0c313e5 100644 --- a/libgo/go/debug/elf/elf.go +++ b/libgo/go/debug/elf/elf.go @@ -119,8 +119,8 @@ const ( ELFOSABI_NONE OSABI = 0 /* UNIX System V ABI */ ELFOSABI_HPUX OSABI = 1 /* HP-UX operating system */ ELFOSABI_NETBSD OSABI = 2 /* NetBSD */ - ELFOSABI_LINUX OSABI = 3 /* GNU/Linux */ - ELFOSABI_HURD OSABI = 4 /* GNU/Hurd */ + ELFOSABI_GNUOSABI = 3 /* GNU */ + ELFOSABI_LINUX OSABI = 3 /* GNU/Linux; alias for ELFOSABI_GNU */ ELFOSABI_86OPEN OSABI = 5 /* 86Open common IA32 ABI */ ELFOSABI_SOLARISOSABI = 6 /* Solaris */ ELFOSABI_AIXOSABI = 7 /* AIX */ @@ -139,8 +139,7 @@ var osabiStrings = []intName{ {0, ELFOSABI_NONE}, {1, ELFOSABI_HPUX}, {2, ELFOSABI_NETBSD}, - {3, ELFOSABI_LINUX}, - {4, ELFOSABI_HURD}, + {3, ELFOSABI_GNU}, {5, ELFOSABI_86OPEN}, {6, ELFOSABI_SOLARIS}, {7, ELFOSABI_AIX}, diff --git a/libgo/go/debug/elf/elf_test.go b/libgo/go/debug/elf/elf_test.go index 67b961b..118e20a 100644 --- a/libgo/go/debug/elf/elf_test.go +++ b/libgo/go/debug/elf/elf_test.go @@ -15,7 +15,7 @@ type nameTest struct { } var nameTests = []nameTest{ - {ELFOSABI_LINUX, ELFOSABI_LINUX}, + {ELFOSABI_GNU, ELFOSABI_GNU}, {ET_EXEC, ET_EXEC}, {EM_860, EM_860}, {SHN_LOPROC, SHN_LOPROC}, -- tg: (1677490..) elfosabi_gnu (depends on: master) Grüße, Thomas pgpw1T65jPdnk.pgp Description: PGP signature
Re: [build] Move MD_UNWIND_SUPPORT to toplevel libgcc
Hallo! On Wed, 29 Jun 2011 10:40:10 +0200, Paolo Bonzini bonz...@gnu.org wrote: On 06/21/2011 12:04 PM, Rainer Orth wrote: For md_unwind_header on the other hand, you'd have almost as many cases as in the general case. I fear it's hard to have the configuration split over too many places. So I'd suggest to split the affected cases into Linux and non-Linux ones, with the slight duplication necessary for extra_parts and tmake_file. I agree. Thomas, are you going to do that? Like this? libgcc/ config.host: Use i386/linux-unwind.h only for *-*-linux*. --- libgcc/config.host | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libgcc/config.host b/libgcc/config.host index 326ce91..1d5b887 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -354,12 +354,18 @@ i[34567]86-*-openbsd*) i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i[34567]86-*-gnu* | i[34567]86-*-kopensolaris*-gnu) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm - md_unwind_header=i386/linux-unwind.h +case $host in +*-*-linux*) + md_unwind_header=i386/linux-unwind.h;; +esac ;; x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm - md_unwind_header=i386/linux-unwind.h +case $host in +*-*-linux*) + md_unwind_header=i386/linux-unwind.h;; +esac ;; i[34567]86-pc-msdosdjgpp*) ;; -- tg: (1677490..) linux-unwind.h (depends on: master) Manually tested as follows -- is that enough? $ (host=i686-pc-linux-gnu . libgcc/config.host echo $md_unwind_header) i386/linux-unwind.h $ (host=i686-pc-kfreebsd-gnu . libgcc/config.host echo $md_unwind_header) no-unwind.h $ (host=i686-pc-gnu . libgcc/config.host echo $md_unwind_header) no-unwind.h $ (host=x86_64-pc-linux-gnu . libgcc/config.host echo $md_unwind_header) i386/linux-unwind.h $ (host=x86_64-pc-kfreebsd-gnu . libgcc/config.host echo $md_unwind_header) no-unwind.h Grüße, Thomas pgpMovGGfxHVZ.pgp Description: PGP signature
Re: [build] Move MD_UNWIND_SUPPORT to toplevel libgcc
Hi Thomas, Like this? libgcc/ config.host: Use i386/linux-unwind.h only for *-*-linux*. --- libgcc/config.host | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libgcc/config.host b/libgcc/config.host index 326ce91..1d5b887 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -354,12 +354,18 @@ i[34567]86-*-openbsd*) i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i[34567]86-*-gnu* | i[34567]86-*-kopensolaris*-gnu) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm - md_unwind_header=i386/linux-unwind.h +case $host in +*-*-linux*) + md_unwind_header=i386/linux-unwind.h;; +esac ;; x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm - md_unwind_header=i386/linux-unwind.h +case $host in +*-*-linux*) + md_unwind_header=i386/linux-unwind.h;; +esac Instead of nested cases, I'd rather use one i[34567]86-*-linux* case and another for the rest, duplicating extra_parts and tmake_file. Same for x86_64-*-linux* vs. the rest. But that's just me. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [build] Move MD_UNWIND_SUPPORT to toplevel libgcc
On 07/05/2011 01:52 PM, Rainer Orth wrote: Instead of nested cases, I'd rather use one i[34567]86-*-linux* case and another for the rest, duplicating extra_parts and tmake_file. Same for x86_64-*-linux* vs. the rest. But that's just me. I agree. Paolo
Re: [build] Move MD_UNWIND_SUPPORT to toplevel libgcc
Hallo! On Tue, 05 Jul 2011 13:52:08 +0200, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Like this? [...] Instead of nested cases, I'd rather use one i[34567]86-*-linux* case and another for the rest, duplicating extra_parts and tmake_file. Same for x86_64-*-linux* vs. the rest. But that's just me. My idea was to keep the GNU systems' extra_parts and tmake_file stanzas together. But it's a bit wishi washi anyway in all these configuration files, so we might as well use the following patch. Manually tested as before -- more testing required? libgcc/ config.host: Use i386/linux-unwind.h only for *-*-linux*. --- libgcc/config.host | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/libgcc/config.host b/libgcc/config.host index 326ce91..c89155f 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -351,16 +351,24 @@ i[34567]86-*-openbsd2.*|i[34567]86-*openbsd3.[0123]) ;; i[34567]86-*-openbsd*) ;; -i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i[34567]86-*-gnu* | i[34567]86-*-kopensolaris*-gnu) +i[34567]86-*-linux*) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm md_unwind_header=i386/linux-unwind.h ;; -x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu) +i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i[34567]86-*-gnu* | i[34567]86-*-kopensolaris*-gnu) + extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o + tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm + ;; +x86_64-*-linux*) extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm md_unwind_header=i386/linux-unwind.h ;; +x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu) + extra_parts=$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o + tmake_file=${tmake_file} i386/t-crtpc i386/t-crtfm + ;; i[34567]86-pc-msdosdjgpp*) ;; i[34567]86-*-lynxos*) -- tg: (1677490..) linux-unwind.h (depends on: master) Grüße, Thomas pgpDr1E5I8D33.pgp Description: PGP signature
Re: [PATCH][C][C++] Move common tree node building to c_common_nodes_and_builtins
On Tue, 5 Jul 2011, Richard Guenther wrote: This consolidates build_common_tree_nodes and build_common_tree_nodes_2 at a single place in c_common_nodes_and_builtins for C family languages. It is a preparation for merging those two functions and moving them to be called from toplev.c as they are middle-end inits. Bootstrapped and tested on x86_64-unknown-linux-gnu for all C family languages and Java. Ok for trunk? The c-common.c and c-decl.c changes are OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Address lowering [1/3] Main patch
(Sorry for the late response; yesterday was a holiday here.) On Mon, 2011-07-04 at 16:21 +0200, Richard Guenther wrote: On Thu, Jun 30, 2011 at 4:39 PM, William J. Schmidt wschm...@linux.vnet.ibm.com wrote: This is the first of three patches related to lowering addressing expressions to MEM_REFs and TARGET_MEM_REFs in late gimple. This patch contains the new pass together with supporting changes in existing modules. The second patch contains an independent change to the RTL forward propagator to keep it from undoing an optimization made in the first patch. The third patch contains new test cases and changes to existing test cases. Although I've broken it up into three patches to make the review easier, it would be best to commit at least the first and third together to avoid regressions. The second can stand alone. I've done regression tests on powerpc64 and x86_64, and have asked Andreas Krebbel to test against the IBM z (390) platform. I've done performance regression testing on powerpc64. The only performance regression of note is the 2% degradation to 188.ammp due to loss of field disambiguation information. As discussed in another thread, fixing this introduces more complexity than it's worth. Are there also performance improvements? What about code size? Yes, there are performance improvements. I've been running cpu2000 on 32- and 64-bit powerpc64. Thirteen tests show measurable improvements between 1% and 9%, with 187.facerec showing the largest improvements for both 32 and 64. I don't have formal code size results, but anecdotally from code crawling, I have seen code size either neutral or slightly improved. The largest code size improvements I've seen were on 32-bit code where the commoning allowed removal of a number of sign-extend and zero-extend instructions that were otherwise not seen to be redundant. I tried to get an understanding to what kind of optimizations this patch produces based on the test of testcases you added, but I have a hard time here. Can you outline some please? The primary goal is to clean up code such as is shown in the original post of PR46556. In late 2007 there were some changes made to address canonicalization that caused the code gen to be suboptimal on powerpc64. At that time you and others suggested a pattern recognizer prior to expand as probably the best solution, similar to what IVopts is doing. By using the same mem_ref generation machinery used by IVopts, together with local CSE, the goal was to ensure base registers are properly shared so that optimal code is generated, particularly for cases of shared addressability to structures and arrays. I also observed cases where it was useful to extend the sharing across the dominator tree. Secondarily, I noticed that once this was cleaned up we still had the suboptimal code generation for the zero-offset mem refs scenario outlined in the code commentary. The extra logic to clean this up helps keep register pressure down. I've observed some spill code reduction when this is in place. Again, using expression availability from dominating blocks helps here in some cases as well. I still do not like the implementation of yet another CSE machinery given that we already have two. I think most of the need for CSE comes from the use of the affine combination framework and force_gimple_operand. In fact I'd be interested to see cases that are optimized that could not be handled by a combine-like pattern matcher? I'm somewhat puzzled by this comment. Back on 2/4, I posted a skeletal outline for this pass and asked for comments. You indicated a concern that the affine combination expansion would un-CSE a lot of expressions, and that I should keep track of local candidates during this pass to ensure that base registers, etc., would be properly shared. It seemed to me that a quick and dirty CSE of addressing expressions would be the best way to handle that. I posted another RFC on 2/24 with an early implementation of CSE constrained to a single block, and indicated shortly thereafter my intent to extend this to a dominator walk. I didn't receive any negative comments about using CSE at that time; but then I didn't get much response at all. I probably should have pushed harder to get comments at that point, but I was very new to the community and was feeling my way through the protocols; I didn't feel comfortable pushing. In any case, yes, the primary need for CSE is as a result of the affine combination framework, which creates a large amount of redundant code. I can certainly take a look at Michael's suggestion to move the pass earlier and allow pass-dominator to handle the cleanup, and add the zero-offset mem-ref optimization to that or a subsequent pass. Do you agree that this would be a preferable approach? Thanks, Richard.
PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
Ping. On Sat, Jun 25, 2011 at 9:20 AM, H.J. Lu hongjiu...@intel.com wrote: Hi, I was informed that MEM_REF only works in ptr_mode. This patch changes addr_for_mem_ref to use ptr_mode. OK for trunk? Thanks. H.J. --- 2011-06-25 H.J. Lu hongjiu...@intel.com PR middle-end/47383 * tree-ssa-address.c (addr_for_mem_ref): Use ptr_mode instead of targetm.addr_space.address_mode. diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c index e3934e1..ddc6d58 100644 --- a/gcc/tree-ssa-address.c +++ b/gcc/tree-ssa-address.c @@ -188,12 +188,12 @@ rtx addr_for_mem_ref (struct mem_address *addr, addr_space_t as, bool really_expand) { - enum machine_mode address_mode = targetm.addr_space.address_mode (as); rtx address, sym, bse, idx, st, off; struct mem_addr_template *templ; if (addr-step !integer_onep (addr-step)) - st = immed_double_int_const (tree_to_double_int (addr-step), address_mode); + st = immed_double_int_const (tree_to_double_int (addr-step), + ptr_mode); else st = NULL_RTX; @@ -201,7 +201,7 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, off = immed_double_int_const (double_int_sext (tree_to_double_int (addr-offset), TYPE_PRECISION (TREE_TYPE (addr-offset))), - address_mode); + ptr_mode); else off = NULL_RTX; @@ -220,16 +220,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, if (!templ-ref) { sym = (addr-symbol ? - gen_rtx_SYMBOL_REF (address_mode, ggc_strdup (test_symbol)) + gen_rtx_SYMBOL_REF (ptr_mode, ggc_strdup (test_symbol)) : NULL_RTX); bse = (addr-base ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 1) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 1) : NULL_RTX); idx = (addr-index ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 2) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 2) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, + gen_addr_rtx (ptr_mode, sym, bse, idx, st? const0_rtx : NULL_RTX, off? const0_rtx : NULL_RTX, templ-ref, @@ -247,16 +247,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, /* Otherwise really expand the expressions. */ sym = (addr-symbol - ? expand_expr (addr-symbol, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-symbol, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); bse = (addr-base - ? expand_expr (addr-base, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-base, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); idx = (addr-index - ? expand_expr (addr-index, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-index, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, st, off, address, NULL, NULL); + gen_addr_rtx (ptr_mode, sym, bse, idx, st, off, address, NULL, NULL); return address; } -- H.J.
Re: [build] Move MD_UNWIND_SUPPORT to toplevel libgcc
On 07/05/2011 02:13 PM, Thomas Schwinge wrote: Hallo! On Tue, 05 Jul 2011 13:52:08 +0200, Rainer Orthr...@cebitec.uni-bielefeld.de wrote: Like this? [...] Instead of nested cases, I'd rather use one i[34567]86-*-linux* case and another for the rest, duplicating extra_parts and tmake_file. Same for x86_64-*-linux* vs. the rest. But that's just me. My idea was to keep the GNU systems' extra_parts and tmake_file stanzas together. But it's a bit wishi washi anyway in all these configuration files, so we might as well use the following patch. Manually tested as before -- more testing required? libgcc/ config.host: Use i386/linux-unwind.h only for *-*-linux*. Ok, with changelog entry like * config.host (i[34567]86-*-kfreebsd*-gnu, i[34567]86-*-knetbsd*-gnu, i[34567]86-*-gnu*, i[34567]86-*-kopensolaris*-gnu): Remove md_unwind_header by splitting out of... (i[34567]86-*-linux*): ... this. * config.host (x86_64-*-kfreebsd*-gnu, x86_64-*-knetbsd*-gnu): Remove md_unwind_header by splitting out of... (x86_64-*-linux*): ... this. Paolo
Re: [PATCH] Address lowering [1/3] Main patch
On Mon, 2011-07-04 at 17:30 +0200, Michael Matz wrote: Hi, On Mon, 4 Jul 2011, Richard Guenther wrote: I still do not like the implementation of yet another CSE machinery given that we already have two. From reading it it really seems to be a normal block-local CSE, without anything fancy. Hence, moving the pass just a little earlier (before pass_vrp/pass_dominator) should already provide for all optimizations. If not those should be improved. I see that it is used for also getting rid of the zero-offset statements in case non-zero-offsets follow. I think that's generally worthwhile so probably should be done in one of the above optimizers. OK, thanks for the suggestion. I can certainly look into this. You handle NOP_EXPR different from CONVERT_EXPR. The middle-end doesn't distinguish between them (yes, ignore the comment about one generating code, the other not). Ah, thanks, good to know. Your check for small types: + if (TYPE_MODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == SImode) + ref_found = true; You probably want != BLKmode . OK. + if (changed is_zero_offset_ref (gimple_assign_lhs (stmt))) +VEC_safe_push (gimple, heap, zero_offset_refs, stmt); + + rhs1 = gimple_assign_rhs1_ptr (stmt); + rhs_changed = tree_ssa_lower_addr_tree (rhs1, gsi, speed, false); + + /* Record zero-offset mem_refs on the RHS. */ + if (rhs_changed is_zero_offset_ref (gimple_assign_rhs1 (stmt))) +VEC_safe_push (gimple, heap, zero_offset_refs, stmt); This possibly adds stmt twice to zero_offset_refs. Do you really want this? Hm, I didn't think it was (currently) possible for a gimple statement to have a mem-ref on both RHS and LHS. Is that incorrect? This is easily changed if so, or if the possibility should be left open for the future. Ciao, Michael. Thanks, Bill
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 4:07 PM, H.J. Lu hjl.to...@gmail.com wrote: Ping. That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? HJ? What testcase does this fix? Please add it at least. That said, this patch seems to paper over a problem that exists elsewhere. Richard. On Sat, Jun 25, 2011 at 9:20 AM, H.J. Lu hongjiu...@intel.com wrote: Hi, I was informed that MEM_REF only works in ptr_mode. This patch changes addr_for_mem_ref to use ptr_mode. OK for trunk? Thanks. H.J. --- 2011-06-25 H.J. Lu hongjiu...@intel.com PR middle-end/47383 * tree-ssa-address.c (addr_for_mem_ref): Use ptr_mode instead of targetm.addr_space.address_mode. diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c index e3934e1..ddc6d58 100644 --- a/gcc/tree-ssa-address.c +++ b/gcc/tree-ssa-address.c @@ -188,12 +188,12 @@ rtx addr_for_mem_ref (struct mem_address *addr, addr_space_t as, bool really_expand) { - enum machine_mode address_mode = targetm.addr_space.address_mode (as); rtx address, sym, bse, idx, st, off; struct mem_addr_template *templ; if (addr-step !integer_onep (addr-step)) - st = immed_double_int_const (tree_to_double_int (addr-step), address_mode); + st = immed_double_int_const (tree_to_double_int (addr-step), + ptr_mode); else st = NULL_RTX; @@ -201,7 +201,7 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, off = immed_double_int_const (double_int_sext (tree_to_double_int (addr-offset), TYPE_PRECISION (TREE_TYPE (addr-offset))), - address_mode); + ptr_mode); else off = NULL_RTX; @@ -220,16 +220,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, if (!templ-ref) { sym = (addr-symbol ? - gen_rtx_SYMBOL_REF (address_mode, ggc_strdup (test_symbol)) + gen_rtx_SYMBOL_REF (ptr_mode, ggc_strdup (test_symbol)) : NULL_RTX); bse = (addr-base ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 1) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 1) : NULL_RTX); idx = (addr-index ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 2) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 2) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, + gen_addr_rtx (ptr_mode, sym, bse, idx, st? const0_rtx : NULL_RTX, off? const0_rtx : NULL_RTX, templ-ref, @@ -247,16 +247,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, /* Otherwise really expand the expressions. */ sym = (addr-symbol - ? expand_expr (addr-symbol, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-symbol, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); bse = (addr-base - ? expand_expr (addr-base, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-base, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); idx = (addr-index - ? expand_expr (addr-index, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-index, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, st, off, address, NULL, NULL); + gen_addr_rtx (ptr_mode, sym, bse, idx, st, off, address, NULL, NULL); return address; } -- H.J.
Re: [PATCH] Address lowering [1/3] Main patch
Hi, On Tue, 5 Jul 2011, William J. Schmidt wrote: Hm, I didn't think it was (currently) possible for a gimple statement to have a mem-ref on both RHS and LHS. Is that incorrect? This is easily changed if so, or if the possibility should be left open for the future. Think aggregate copies: void bla (struct S *d, struct S *s) { *d = *s; } Ciao, Michael.
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 7:15 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:07 PM, H.J. Lu hjl.to...@gmail.com wrote: Ping. That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? HJ? What testcase does this fix? Please add it at least. There are many failures in gcc and glibc builds/tests. But they only show up on x32 target. There is a simple one at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47383 That said, this patch seems to paper over a problem that exists elsewhere. This patch tries to deal with MEM_REF which only works in ptr_mode. Richard. On Sat, Jun 25, 2011 at 9:20 AM, H.J. Lu hongjiu...@intel.com wrote: Hi, I was informed that MEM_REF only works in ptr_mode. This patch changes addr_for_mem_ref to use ptr_mode. OK for trunk? Thanks. H.J. --- 2011-06-25 H.J. Lu hongjiu...@intel.com PR middle-end/47383 * tree-ssa-address.c (addr_for_mem_ref): Use ptr_mode instead of targetm.addr_space.address_mode. diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c index e3934e1..ddc6d58 100644 --- a/gcc/tree-ssa-address.c +++ b/gcc/tree-ssa-address.c @@ -188,12 +188,12 @@ rtx addr_for_mem_ref (struct mem_address *addr, addr_space_t as, bool really_expand) { - enum machine_mode address_mode = targetm.addr_space.address_mode (as); rtx address, sym, bse, idx, st, off; struct mem_addr_template *templ; if (addr-step !integer_onep (addr-step)) - st = immed_double_int_const (tree_to_double_int (addr-step), address_mode); + st = immed_double_int_const (tree_to_double_int (addr-step), + ptr_mode); else st = NULL_RTX; @@ -201,7 +201,7 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, off = immed_double_int_const (double_int_sext (tree_to_double_int (addr-offset), TYPE_PRECISION (TREE_TYPE (addr-offset))), - address_mode); + ptr_mode); else off = NULL_RTX; @@ -220,16 +220,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, if (!templ-ref) { sym = (addr-symbol ? - gen_rtx_SYMBOL_REF (address_mode, ggc_strdup (test_symbol)) + gen_rtx_SYMBOL_REF (ptr_mode, ggc_strdup (test_symbol)) : NULL_RTX); bse = (addr-base ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 1) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 1) : NULL_RTX); idx = (addr-index ? - gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 2) + gen_raw_REG (ptr_mode, LAST_VIRTUAL_REGISTER + 2) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, + gen_addr_rtx (ptr_mode, sym, bse, idx, st? const0_rtx : NULL_RTX, off? const0_rtx : NULL_RTX, templ-ref, @@ -247,16 +247,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, /* Otherwise really expand the expressions. */ sym = (addr-symbol - ? expand_expr (addr-symbol, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-symbol, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); bse = (addr-base - ? expand_expr (addr-base, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-base, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); idx = (addr-index - ? expand_expr (addr-index, NULL_RTX, address_mode, EXPAND_NORMAL) + ? expand_expr (addr-index, NULL_RTX, ptr_mode, EXPAND_NORMAL) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, st, off, address, NULL, NULL); + gen_addr_rtx (ptr_mode, sym, bse, idx, st, off, address, NULL, NULL); return address; } -- H.J.
PING: PATCH [4/n]: Prepare x32: Permute the conversion and addition if one operand is a constant
Ping. On Sat, Jun 11, 2011 at 8:58 AM, H.J. Lu hongjiu...@intel.com wrote: Hi, convert_memory_address_addr_space has a special PLUS/MULT case for POINTERS_EXTEND_UNSIGNED 0. It turns out that it is also needed for all Pmode != ptr_mode cases. OK for trunk? Thanks. H.J. --- 2011-06-11 H.J. Lu hongjiu...@intel.com PR middle-end/47727 * explow.c (convert_memory_address_addr_space): Permute the conversion and addition if one operand is a constant. diff --git a/gcc/explow.c b/gcc/explow.c index 7387dad..b343bf8 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -383,18 +383,13 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED, case PLUS: case MULT: - /* For addition we can safely permute the conversion and addition - operation if one operand is a constant and converting the constant - does not change it or if one operand is a constant and we are - using a ptr_extend instruction (POINTERS_EXTEND_UNSIGNED 0). - We can always safely permute them if we are making the address - narrower. */ + /* For addition we safely permute the conversion and addition + operation if one operand is a constant since we can't generate + new instructions. We can always safely permute them if we are + making the address narrower. */ if (GET_MODE_SIZE (to_mode) GET_MODE_SIZE (from_mode) || (GET_CODE (x) == PLUS - CONST_INT_P (XEXP (x, 1)) - (XEXP (x, 1) == convert_memory_address_addr_space - (to_mode, XEXP (x, 1), as) - || POINTERS_EXTEND_UNSIGNED 0))) + CONST_INT_P (XEXP (x, 1 return gen_rtx_fmt_ee (GET_CODE (x), to_mode, convert_memory_address_addr_space (to_mode, XEXP (x, 0), as), -- H.J.
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 4:25 PM, H.J. Lu hjl.to...@gmail.com wrote: On Tue, Jul 5, 2011 at 7:15 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:07 PM, H.J. Lu hjl.to...@gmail.com wrote: Ping. That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? HJ? What testcase does this fix? Please add it at least. There are many failures in gcc and glibc builds/tests. But they only show up on x32 target. There is a simple one at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47383 That said, this patch seems to paper over a problem that exists elsewhere. This patch tries to deal with MEM_REF which only works in ptr_mode. TARGET_MEM_REF you mean. How did it work for other ptr_mode != Pmode targets sofar? Why do you think it doesn't work for you? It's definitely the case that it assumes to compute (Pmode)((ptr_mode)TMR_BASE + (ptr_mode)TMR_INDEX) as address. But disregarding the address-space compeltely looks bogus. I think you need to convert the result to the proper address-space mode. Richard.
Re: [PATCH][C][C++] Move common tree node building to c_common_nodes_and_builtins
OK. Jason
PATCH [1/n] X32: Add initial -x32 support
Hi, I'd like to start submitting a series of patches to enable x32: https://sites.google.com/site/x32abi/ The GCC x32 branch is very stable. There are no unexpected failures in C, C++, Fortran and Objective C testsuites. SPEC CPU 2K/2006 compile and run correctly at -O2 and -O3. More than 90% of changes are in x86 backend. I have submitted non-x86 backend patches. Most of them have been reviewed and checked in. Only 4 patches are pending for review/approval. This is the first x86 backend patch to support x32. By default, x32 is disabled and x32 run-time support isn't required. OK for trunk? Thanks. H.J. --- 2011-06-14 H.J. Lu hongjiu...@intel.com * config.gcc: Support --enable-x32/--enable-ia32 for x86 Linux targets. * configure.ac: Support --enable-x32/--enable-ia32. * configure: Regenerated. * config/i386/gnu-user64.h (SPEC_64): Support x32. (SPEC_32): Likewise. (ASM_SPEC): Likewise. (LINK_SPEC): Likewise. (TARGET_THREAD_SSP_OFFSET): Likewise. (TARGET_THREAD_SPLIT_STACK_OFFSET): Likewise. (SPEC_X32): New. * config/i386/i386.h (TARGET_X32): New. (TARGET_LP64): New. (LONG_TYPE_SIZE): Likewise. (POINTER_SIZE): Likewise. (POINTERS_EXTEND_UNSIGNED): Likewise. (OPT_ARCH64): Support x32. (OPT_ARCH32): Likewise. * config/i386/i386.opt (mx32): New. * config/i386/kfreebsd-gnu64.h (GNU_USER_LINK_EMULATIONX32): New. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/linux64.h (GNU_USER_LINK_EMULATIONX32): Likewise. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/t-linux-x32: New. * config/i386/t-linux64-x32: Likewise. * config/linux.h (UCLIBC_DYNAMIC_LINKERX32): New. (BIONIC_DYNAMIC_LINKERX32): Likewise. (GNU_USER_DYNAMIC_LINKERX32): Likewise. * doc/install.texi: Document --enable-ia32 and --enable-x32. * doc/invoke.texi: Document -mx32. diff --git a/gcc/config.gcc b/gcc/config.gcc index e9704f3..e2b72df 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -1232,7 +1232,17 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i if test x$enable_targets = xall; then tm_file=${tm_file} i386/x86-64.h i386/gnu-user64.h i386/linux64.h tm_defines=${tm_defines} TARGET_BI_ARCH=1 - tmake_file=${tmake_file} i386/t-linux64 + case x${enable_x32}${enable_ia32} in + xyesyes) + tmake_file=${tmake_file} i386/t-linux-x32 + ;; + xyesno) + tmake_file=${tmake_file} i386/t-linux64-x32 + ;; + *) + tmake_file=${tmake_file} i386/t-linux64 + ;; + esac need_64bit_hwint=yes need_64bit_isa=yes case X${with_cpu} in @@ -1270,7 +1280,18 @@ x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu) x86_64-*-kfreebsd*-gnu) tm_file=${tm_file} kfreebsd-gnu.h i386/kfreebsd-gnu64.h ;; x86_64-*-knetbsd*-gnu) tm_file=${tm_file} knetbsd-gnu.h ;; esac - tmake_file=${tmake_file} i386/t-linux64 i386/t-crtstuff i386/t-crtpc i386/t-crtfm t-dfprules + case x${enable_x32}${enable_ia32} in + xyesyes) + tmake_file=${tmake_file} i386/t-linux-x32 + ;; + xyesno) + tmake_file=${tmake_file} i386/t-linux64-x32 + ;; + *) + tmake_file=${tmake_file} i386/t-linux64 + ;; + esac + tmake_file=${tmake_file} i386/t-crtstuff i386/t-crtpc i386/t-crtfm t-dfprules ;; i[34567]86-pc-msdosdjgpp*) xm_file=i386/xm-djgpp.h diff --git a/gcc/config/i386/gnu-user64.h b/gcc/config/i386/gnu-user64.h index b069975..954f3b2 100644 --- a/gcc/config/i386/gnu-user64.h +++ b/gcc/config/i386/gnu-user64.h @@ -58,25 +58,31 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if TARGET_64BIT_DEFAULT #define SPEC_32 m32 -#define SPEC_64 !m32 +#define SPEC_64 m32|mx32:; +#define SPEC_X32 mx32 #else -#define SPEC_32 !m64 +#define SPEC_32 m64|mx32:; #define SPEC_64 m64 +#define SPEC_X32 mx32 #endif #undef ASM_SPEC -#define ASM_SPEC %{ SPEC_32 :--32} %{ SPEC_64 :--64} \ +#define ASM_SPEC %{ SPEC_32 :--32} \ + %{ SPEC_64 :--64} \ + %{ SPEC_X32 :--x32} \ %{!mno-sse2avx:%{mavx:-msse2avx}} %{msse2avx:%{!mavx:-msse2avx}} #undef LINK_SPEC #define LINK_SPEC %{ SPEC_64 :-m GNU_USER_LINK_EMULATION64 } \ %{ SPEC_32 :-m GNU_USER_LINK_EMULATION32 } \ + %{ SPEC_X32 :-m GNU_USER_LINK_EMULATIONX32 } \ %{shared:-shared} \
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 7:30 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:25 PM, H.J. Lu hjl.to...@gmail.com wrote: On Tue, Jul 5, 2011 at 7:15 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:07 PM, H.J. Lu hjl.to...@gmail.com wrote: Ping. That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? HJ? What testcase does this fix? Please add it at least. There are many failures in gcc and glibc builds/tests. But they only show up on x32 target. There is a simple one at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47383 That said, this patch seems to paper over a problem that exists elsewhere. This patch tries to deal with MEM_REF which only works in ptr_mode. TARGET_MEM_REF you mean. How did it work for other ptr_mode != Pmode targets sofar? Why do you think it doesn't work for you? It's definitely the case that I think x32 is the only zero-extend ptr_mode != Pmode target. it assumes to compute (Pmode)((ptr_mode)TMR_BASE + (ptr_mode)TMR_INDEX) as address. But disregarding the address-space compeltely looks bogus. I think you need to convert the result to the proper address-space mode. I will give it a try. Thanks. -- H.J.
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
Richard Guenther wrote: That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? Common code should basically *never* directly refer to Pmode or ptr_mode; instead it should always use the per-address-space modes targetm.addr_space.address_mode (as) or targetm.addr_space.pointer_mode (as) The address_mode is the mode to handle addresses at the instruction level (i.e. the mode of the address within a MEM (or TARGET_MEM_REF)). This defaults to Pmode for the default address space. The pointer_mode is the mode to handle pointers at source level. This defaults to ptr_mode for the default address space. [ Note that there is one exception: common code today continues to hard- code Pmode for addresses known to refer to the stack; the stack must lie in the default address space at this point. ] So the patch below, changing targetm.addr_space.address_mode (as) to just ptr_mode is definitely wrong and will break multiple address spaces. If at this point indeed a pointer mode is required, you should use targetm.addr_space.pointer_mode (as) instead. However, this still seems odd to me, as I had understood the address in a TARGET_MEM_REF needs to be an *address*, i.e. use address_mode. If this is not true (has changed?) a lot of other places would need to change as well ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
PING: PATCH [2/n]: Prepare x32: PR middle-end/47715: Convert pointer to TLS symbol if needed
PING. On Fri, Jul 1, 2011 at 9:37 AM, Richard Sandiford richard.sandif...@linaro.org wrote: H.J. Lu hjl.to...@gmail.com writes: On Wed, Jun 29, 2011 at 7:06 AM, H.J. Lu hjl.to...@gmail.com wrote: On Wed, Jun 29, 2011 at 1:45 AM, Richard Sandiford richard.sandif...@linaro.org wrote: H.J. Lu hongjiu...@intel.com writes: @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, pseudo now. TLS symbols sometimes need a call to resolve. */ if (CONSTANT_P (args[i].value) !targetm.legitimate_constant_p (args[i].mode, args[i].value)) - args[i].value = force_reg (args[i].mode, args[i].value); + { + if (GET_MODE (args[i].value) != args[i].mode) + args[i].value = convert_to_mode (args[i].mode, + args[i].value, + args[i].unsignedp); + args[i].value = force_reg (args[i].mode, args[i].value); + } But if GET_MODE (args[i].value) != args[i].mode, then the call to targetm.legitimate_constant_p looks wrong. The mode passed in the first argument is supposed to the mode of the second argument. Is there any reason why this and the following: /* If we are to promote the function arg to a wider mode, do it now. */ if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) args[i].value = convert_modes (args[i].mode, TYPE_MODE (TREE_TYPE (args[i].tree_value)), args[i].value, args[i].unsignedp); need to be done in the current order? I can't think of any off-hand. If not, would swapping them also fix the bug? (I can't review this either way, of course.) It works on the testcase. I will do a full test. It works. There are no regressions on Linux/x86-64. Great! I can't approve it, but FWIW, it looks good to me. The new order seems to make more conceptual sense: coerce the value into the right mode, then coerce it into the right type of rtx. Richard -- H.J.
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 7:42 AM, Ulrich Weigand uweig...@de.ibm.com wrote: Richard Guenther wrote: That doesn't look correct without also ensuring we never create a TARGET_MEM_REF with a base that is not in the default address-space. In fact, with this patch the address-space argument to addr_for_mem_ref should go away or we need a hook that provides a non-promoted mode for address-spaces. Uli? Common code should basically *never* directly refer to Pmode or ptr_mode; instead it should always use the per-address-space modes targetm.addr_space.address_mode (as) or targetm.addr_space.pointer_mode (as) The address_mode is the mode to handle addresses at the instruction level (i.e. the mode of the address within a MEM (or TARGET_MEM_REF)). This defaults to Pmode for the default address space. Here is the problem. For x32, Pmode is the address_mode. But TARGET_MEM_REF only works on ptr_mode, not Pmode unless Pmode is the same as ptr_mode. The pointer_mode is the mode to handle pointers at source level. This defaults to ptr_mode for the default address space. [ Note that there is one exception: common code today continues to hard- code Pmode for addresses known to refer to the stack; the stack must lie in the default address space at this point. ] So the patch below, changing targetm.addr_space.address_mode (as) to just ptr_mode is definitely wrong and will break multiple address spaces. If at this point indeed a pointer mode is required, you should use targetm.addr_space.pointer_mode (as) instead. I will give it try. However, this still seems odd to me, as I had understood the address in a TARGET_MEM_REF needs to be an *address*, i.e. use address_mode. If this is not true (has changed?) a lot of other places would need to change as well ... I was told that TARGET_MEM_REF needs ptr_mode. Thanks. -- H.J.
RFA: Making attribute values avaliable for options
There is often an enum corresponding to a target.md attribute that you want as numeric values for an Enum option declaration. Alas, insn-attr.h is not included by options.c, and an attempt to include it with the HeaderInclude record is doomed because of all the headers that insn-attr.h requires to be included first, and also because of circular dependency issues. To use the enum values, we need the enum types only known in options.c, though - the variable itself can be declared as type int to avoid problems with the lack of enum forward declarations. But it doesn't really make sense to get all the other cruft from insn-attr.h into options.c . This patch splits out a new generator genattr-enum from genattr, and it generates insn-attr-enum.h, which just makes the enum declarations. This new header file is then included by options.c and insn-attr.h . Bootstrapped on x86_64-unknown-linux-gnu . 2011-07-05 Joern Rennecke joern.renne...@embecosm.com * genattr-enum.c: New file, split out of ... * genattr.c. Make output file include insn-attr-enum.h. * optc-gen.awk: Make output file include insn-attr-enum.h. * Makefile.in (INSN_ATTR_H): Add insn-attr-enum.h. (MOSTLYCLEANFILES, PRECIOUS, simple_rtl_generated_h): Likewise. (build/genattr-enum.o): New rule. (genprogrtl): Add attr-enum. Index: gcc/genattr.c === --- gcc/genattr.c (revision 175852) +++ gcc/genattr.c (working copy) @@ -30,23 +30,15 @@ Software Foundation; either version 3, o #include gensupport.h -static void write_upcase (const char *); static void gen_attr (rtx); -static void -write_upcase (const char *str) -{ - for (; *str; str++) -putchar (TOUPPER(*str)); -} - static VEC (rtx, heap) *const_attrs, *reservations; static void gen_attr (rtx attr) { - const char *p, *tag; + const char *p; int is_const = GET_CODE (XEXP (attr, 2)) == CONST; if (is_const) @@ -65,23 +57,8 @@ gen_attr (rtx attr) printf (extern int get_attr_%s (%s);\n, XSTR (attr, 0), (is_const ? void : rtx)); else - { - printf (enum attr_%s {, XSTR (attr, 0)); - - while ((tag = scan_comma_elt (p)) != 0) - { - write_upcase (XSTR (attr, 0)); - putchar ('_'); - while (tag != p) - putchar (TOUPPER (*tag++)); - if (*p == ',') - fputs (, , stdout); - } - fputs (};\n, stdout); - - printf (extern enum attr_%s get_attr_%s (%s);\n\n, - XSTR (attr, 0), XSTR (attr, 0), (is_const ? void : rtx)); - } + printf (extern enum attr_%s get_attr_%s (%s);\n\n, + XSTR (attr, 0), XSTR (attr, 0), (is_const ? void : rtx)); } /* If `length' attribute, write additional function definitions and define @@ -181,6 +158,7 @@ main (int argc, char **argv) puts (#define GCC_INSN_ATTR_H\n); puts (#include \insn-attr-common.h\\n); + puts (#include \insn-attr-enum.h\\n); /* For compatibility, define the attribute `alternative', which is just a reference to the variable `which_alternative'. */ Index: gcc/optc-gen.awk === --- gcc/optc-gen.awk(revision 175852) +++ gcc/optc-gen.awk(working copy) @@ -37,6 +37,7 @@ for (i = 1; i = n_headers; i++) print #include quote headers[i] quote print #include quote opts.h quote print #include quote intl.h quote +print #include quote insn-attr-enum.h quote print if (n_extra_c_includes 0) { Index: gcc/Makefile.in === --- gcc/Makefile.in (revision 175852) +++ gcc/Makefile.in (working copy) @@ -965,7 +965,7 @@ GCC_H = gcc.h version.h $(DIAGNOSTIC_COR GGC_H = ggc.h gtype-desc.h statistics.h GGC_INTERNAL_H = ggc-internal.h $(GGC_H) TIMEVAR_H = timevar.h timevar.def -INSN_ATTR_H = insn-attr.h insn-attr-common.h $(INSN_ADDR_H) +INSN_ATTR_H = insn-attr.h insn-attr-common.h insn-attr-enum.h $(INSN_ADDR_H) INSN_ADDR_H = $(srcdir)/insn-addr.h vecprim.h C_COMMON_H = c-family/c-common.h c-family/c-common.def \ $(SPLAY_TREE_H) $(CPPLIB_H) $(GGC_H) $(DIAGNOSTIC_CORE_H) @@ -1523,7 +1523,7 @@ BACKEND = main.o @TREEBROWSER@ libbacken MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ insn-output.c insn-recog.c insn-emit.c insn-extract.c insn-peep.c \ - insn-attr.h insn-attr-common.h insn-attrtab.c insn-opinit.c \ + insn-attr.h insn-attr-common.h insn-attr-enum.h insn-attrtab.c insn-opinit.c \ insn-preds.c insn-constants.h \ tm-preds.h tm-constrs.h checksum-options \ tree-check.h min-insn-modes.c insn-modes.c insn-modes.h \ @@ -3589,7 +3589,7 @@ mips-tdump.o : mips-tdump.c $(CONFIG_H) .PRECIOUS: insn-config.h insn-flags.h insn-codes.h insn-constants.h \ insn-emit.c insn-recog.c insn-extract.c insn-output.c
[PATCH] Merge build_common_tree_nodes and build_common_tree_nodes_2
This merges the two common tree node creation functions now that all callers are sufficiently close to make frontend pieces obvious. Pending bootstrap and regtest on x86_64-unknown-linux-gnu, I will commit this tomorrow unless somebody objects by then. Richard. 2011-07-05 Richard Guenther rguent...@suse.de * tree.c (build_common_tree_nodes_2): Merge with build_common_tree_nodes. * tree.h (build_common_tree_nodes): Adjust prototype. (build_common_tree_nodes_2): Remove. * doc/tm.texi (lang_hooks.builtin_function): Adjust. c-family/ * c-common.c (c_common_nodes_and_builtins): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. fortran/ * f95-lang.c (gfc_init_decl_processing): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. go/ * go-lang.c (go_langhook_init): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. java/ * decl.c (java_init_decl_processing): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. lto/ * lto-lang.c (lto_init): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. ada/ * gcc-interface/misc.c (gnat_init): Merge calls to build_common_tree_nodes and build_common_tree_nodes_2. Index: gcc/tree.c === --- gcc/tree.c (revision 175855) +++ gcc/tree.c (working copy) @@ -9167,10 +9167,12 @@ make_or_reuse_accum_type (unsigned size, } /* Create nodes for all integer types (and error_mark_node) using the sizes - of C datatypes. */ + of C datatypes. SIGNED_CHAR specifies whether char is signed, + SHORT_DOUBLE specifies whether double should be of the same precision + as float. */ void -build_common_tree_nodes (bool signed_char) +build_common_tree_nodes (bool signed_char, bool short_double) { error_mark_node = make_node (ERROR_MARK); TREE_TYPE (error_mark_node) = error_mark_node; @@ -9247,14 +9249,7 @@ build_common_tree_nodes (bool signed_cha access_public_node = get_identifier (public); access_protected_node = get_identifier (protected); access_private_node = get_identifier (private); -} - -/* Call this function after calling build_common_tree_nodes. - It will create several other common tree nodes. */ -void -build_common_tree_nodes_2 (int short_double) -{ /* Define these next since types below may used them. */ integer_zero_node = build_int_cst (integer_type_node, 0); integer_one_node = build_int_cst (integer_type_node, 1); Index: gcc/tree.h === --- gcc/tree.h (revision 175855) +++ gcc/tree.h (working copy) @@ -5396,8 +5396,7 @@ extern int real_onep (const_tree); extern int real_twop (const_tree); extern int real_minus_onep (const_tree); extern void init_ttree (void); -extern void build_common_tree_nodes (bool); -extern void build_common_tree_nodes_2 (int); +extern void build_common_tree_nodes (bool, bool); extern void build_common_builtin_nodes (void); extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT, int); extern tree build_range_type (tree, tree, tree); Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 175856) +++ gcc/c-family/c-common.c (working copy) @@ -4576,8 +4576,7 @@ c_common_nodes_and_builtins (void) tree va_list_ref_type_node; tree va_list_arg_type_node; - build_common_tree_nodes (flag_signed_char); - build_common_tree_nodes_2 (flag_short_double); + build_common_tree_nodes (flag_signed_char, flag_short_double); /* Define `int' and `char' first so that dbx will output them first. */ record_builtin_type (RID_INT, NULL, integer_type_node); Index: gcc/fortran/f95-lang.c === --- gcc/fortran/f95-lang.c (revision 175855) +++ gcc/fortran/f95-lang.c (working copy) @@ -588,9 +588,8 @@ gfc_init_decl_processing (void) /* Build common tree nodes. char_type_node is unsigned because we only use it for actual characters, not for INTEGER(1). Also, we want double_type_node to actually have double precision. */ - build_common_tree_nodes (false); + build_common_tree_nodes (false, false); - build_common_tree_nodes_2 (0); void_list_node = build_tree_list (NULL_TREE, void_type_node); /* Set up F95 type nodes. */ Index: gcc/go/go-lang.c === --- gcc/go/go-lang.c(revision 175855) +++ gcc/go/go-lang.c(working copy) @@ -86,9 +86,7 @@ struct GTY(()) language_function static bool go_langhook_init (void) { - build_common_tree_nodes (false); - - build_common_tree_nodes_2 (0); + build_common_tree_nodes (false, false); /* We must create the
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
H.J. Lu wrote: However, this still seems odd to me, as I had understood the address in a TARGET_MEM_REF needs to be an *address*, i.e. use address_mode. =A0If this is not true (has changed?) a lot of other places would need to change as well ... I was told that TARGET_MEM_REF needs ptr_mode. Can you elaborate? We are talking about the mode returned from addr_for_mem_ref here. I do now understand how this can be anything but an address mode: If you look at the users of addr_for_mem_ref, they all look someting like this: op0 = addr_for_mem_ref (addr, as, true); op0 = memory_address_addr_space (mode, op0, as); mem = gen_rtx_MEM (mode, op0); So the result is used as address argument to create a MEM. This must certainly be address mode (Pmode) then -- that's basically the definition of address mode (the mode of the address argument to MEM). Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: PATCH [1/n] X32: Add initial -x32 support
On Tue, Jul 5, 2011 at 4:39 PM, H.J. Lu hongjiu...@intel.com wrote: I'd like to start submitting a series of patches to enable x32: https://sites.google.com/site/x32abi/ The GCC x32 branch is very stable. There are no unexpected failures in C, C++, Fortran and Objective C testsuites. SPEC CPU 2K/2006 compile and run correctly at -O2 and -O3. More than 90% of changes are in x86 backend. I have submitted non-x86 backend patches. Most of them have been reviewed and checked in. Only 4 patches are pending for review/approval. This is the first x86 backend patch to support x32. By default, x32 is disabled and x32 run-time support isn't required. OK for trunk? Please strip out --enable-ia32 stuff, it complicates things ATM. I assume that --enable-x32 applies only to 64bit targets, so this part @@ -1232,7 +1232,17 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i if test x$enable_targets = xall; then tm_file=${tm_file} i386/x86-64.h i386/gnu-user64.h i386/linux64.h tm_defines=${tm_defines} TARGET_BI_ARCH=1 - tmake_file=${tmake_file} i386/t-linux64 + case x${enable_x32}${enable_ia32} in + xyesyes) + tmake_file=${tmake_file} i386/t-linux-x32 + ;; + xyesno) + tmake_file=${tmake_file} i386/t-linux64-x32 + ;; + *) + tmake_file=${tmake_file} i386/t-linux64 + ;; + esac should be simplified to something: if (enable_x32) tmake_file = ... i386/t-linux64-x32 else tmake_file = ... i386/t-linux64 diff --git a/gcc/config/i386/gnu-user64.h b/gcc/config/i386/gnu-user64.h index b069975..954f3b2 100644 --- a/gcc/config/i386/gnu-user64.h +++ b/gcc/config/i386/gnu-user64.h @@ -58,25 +58,31 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if TARGET_64BIT_DEFAULT #define SPEC_32 m32 -#define SPEC_64 !m32 +#define SPEC_64 m32|mx32:; +#define SPEC_X32 mx32 #else -#define SPEC_32 !m64 +#define SPEC_32 m64|mx32:; #define SPEC_64 m64 +#define SPEC_X32 mx32 #endif #undef ASM_SPEC -#define ASM_SPEC %{ SPEC_32 :--32} %{ SPEC_64 :--64} \ +#define ASM_SPEC %{ SPEC_32 :--32} \ + %{ SPEC_64 :--64} \ + %{ SPEC_X32 :--x32} \ %{!mno-sse2avx:%{mavx:-msse2avx}} %{msse2avx:%{!mavx:-msse2avx}} Are you sure that above is correct? AFAICS, you are enabling SPEC_64 for m32 in case of TARGET_64BIT_DEFAULT. /* SSE4.1 defines round instructions */ #define OPTION_MASK_ISA_ROUND OPTION_MASK_ISA_SSE4_1 @@ -517,8 +519,8 @@ extern const char *host_detect_local_cpu (int argc, const char **argv); #define OPT_ARCH64 !m32 #define OPT_ARCH32 m32 #else -#define OPT_ARCH64 m64 -#define OPT_ARCH32 !m64 +#define OPT_ARCH64 m64|mx32 +#define OPT_ARCH32 m64|mx32:; #endif Same here. Uros.
Re: PING: PATCH [2/n]: Prepare x32: PR middle-end/47715: Convert pointer to TLS symbol if needed
On Tue, Jul 5, 2011 at 4:45 PM, H.J. Lu hjl.to...@gmail.com wrote: PING. Ok. Thanks, Richard. On Fri, Jul 1, 2011 at 9:37 AM, Richard Sandiford richard.sandif...@linaro.org wrote: H.J. Lu hjl.to...@gmail.com writes: On Wed, Jun 29, 2011 at 7:06 AM, H.J. Lu hjl.to...@gmail.com wrote: On Wed, Jun 29, 2011 at 1:45 AM, Richard Sandiford richard.sandif...@linaro.org wrote: H.J. Lu hongjiu...@intel.com writes: @@ -706,7 +706,13 @@ precompute_register_parameters (int num_actuals, struct arg_data *args, pseudo now. TLS symbols sometimes need a call to resolve. */ if (CONSTANT_P (args[i].value) !targetm.legitimate_constant_p (args[i].mode, args[i].value)) - args[i].value = force_reg (args[i].mode, args[i].value); + { + if (GET_MODE (args[i].value) != args[i].mode) + args[i].value = convert_to_mode (args[i].mode, + args[i].value, + args[i].unsignedp); + args[i].value = force_reg (args[i].mode, args[i].value); + } But if GET_MODE (args[i].value) != args[i].mode, then the call to targetm.legitimate_constant_p looks wrong. The mode passed in the first argument is supposed to the mode of the second argument. Is there any reason why this and the following: /* If we are to promote the function arg to a wider mode, do it now. */ if (args[i].mode != TYPE_MODE (TREE_TYPE (args[i].tree_value))) args[i].value = convert_modes (args[i].mode, TYPE_MODE (TREE_TYPE (args[i].tree_value)), args[i].value, args[i].unsignedp); need to be done in the current order? I can't think of any off-hand. If not, would swapping them also fix the bug? (I can't review this either way, of course.) It works on the testcase. I will do a full test. It works. There are no regressions on Linux/x86-64. Great! I can't approve it, but FWIW, it looks good to me. The new order seems to make more conceptual sense: coerce the value into the right mode, then coerce it into the right type of rtx. Richard -- H.J.
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 4:56 PM, Ulrich Weigand uweig...@de.ibm.com wrote: H.J. Lu wrote: However, this still seems odd to me, as I had understood the address in a TARGET_MEM_REF needs to be an *address*, i.e. use address_mode. =A0If this is not true (has changed?) a lot of other places would need to change as well ... I was told that TARGET_MEM_REF needs ptr_mode. Can you elaborate? We are talking about the mode returned from addr_for_mem_ref here. I do now understand how this can be anything but an address mode: That is an address mode, but the intermediate computation (base + index * step + offset) is done in pointer mode. The code currently performs this in address mode as well which is bogus. Which is why I suggested to use pointer mode for the computation (now, with the other target hook you mention) and then convert the result to address mode. Richard.
Re: RFA: Making attribute values avaliable for options
On Tue, 5 Jul 2011, Joern Rennecke wrote: This patch splits out a new generator genattr-enum from genattr, and it generates insn-attr-enum.h, which just makes the enum declarations. This new header file is then included by options.c and insn-attr.h . Is there a particular reason for making this separate from the existing genattr-common that I created recently? Like opts.c, options.c is a file shared by both the driver and the core compiler that can't include the full insn-attr.h for the same reason. -- Joseph S. Myers jos...@codesourcery.com
Re: Ping: C-family stack check for threads
On 07/04/2011 03:25 PM, Thomas Klein wrote: There is a emit_multi_reg_push but is there something like emit_multi_reg_pop, too. There's a multi-reg push because that's one instruction. Are the other operations (compare, branche, ..) still allowed? Of course. Everything is still allowed. See the alpha prologue where the stack is probed in a loop to make sure that each page is present. Yes, if the failure function is taken the info will be wrong. If this is a major problem do I have to add this info after any push and pop operation? Of course it is a major problem. How are you supposed to debug a failure? Guess at the stack trace? Will the rtl push/pop do this already for me? It'll do some of it, yes. r~
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
Richard Guenther wrote: On Tue, Jul 5, 2011 at 4:56 PM, Ulrich Weigand uweig...@de.ibm.com wrote: Can you elaborate? =A0We are talking about the mode returned from addr_for_mem_ref here. =A0I do now understand how this can be anything but an address mode: That is an address mode, but the intermediate computation (base + index * step + offset) is done in pointer mode. The code currently performs this in address mode as well which is bogus. Which is why I suggested to use pointer mode for the computation (now, with the other target hook you mention) and then convert the result to address mode. Ah, OK. Yes, this does make sense ... Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: RFA: Making attribute values avaliable for options
Quoting Joseph S. Myers jos...@codesourcery.com: On Tue, 5 Jul 2011, Joern Rennecke wrote: This patch splits out a new generator genattr-enum from genattr, and it generates insn-attr-enum.h, which just makes the enum declarations. This new header file is then included by options.c and insn-attr.h . Is there a particular reason for making this separate from the existing genattr-common that I created recently? I don't see any technical reason why it would to build failures, short of a name clash (although name clashes are not that far-fetched - one of the reasons insn-attr.h can't be included by option.h is that genautomata.c has a name clash on state_t). It's just that AFAICS the opts.c has no need to know about these enum values. You'll have to tolerate every enum constant of every target in the namespace of opts.c If we decide that insn-attr-common.h is open for anything that doesn't require another header to be included, and name clashes should be avoided / resolved as appropriate, that's fine with me. Like opts.c, options.c is a file shared by both the driver and the core compiler that can't include the full insn-attr.h for the same reason. Well, FWIW, at the moment, I can actually build both cc1 and xgcc when I hand-edit options.c to include inns-attr.h . It just doesn't seem like a good idea, with all these masses of dependencies that are implied.
Re: RFA: Making attribute values avaliable for options
On Tue, 5 Jul 2011, Joern Rennecke wrote: Like opts.c, options.c is a file shared by both the driver and the core compiler that can't include the full insn-attr.h for the same reason. Well, FWIW, at the moment, I can actually build both cc1 and xgcc when I hand-edit options.c to include inns-attr.h . It just doesn't seem like a good idea, with all these masses of dependencies that are implied. It's specifically building the stage1 compiler that fails because of inline functions in headers required by insn-attr.h; the later stages will build fine if you just build those. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC PATCH, go]: Port to ALPHA arch - sysinfo.go fixup
On Wed, Jun 1, 2011 at 7:49 PM, Ian Lance Taylor i...@google.com wrote: One problem remains in the libgo testsuite: certain tests have to be compiled with -mieee, otherwise FPE is generated for unordered values. Any suggestions, where -mieee should be placed? That's an interesting question. I think that ideally we would like -mieee to become the default when using gccgo. If the language spec requires it, then it should go into gcc/go. See java_post_options: static bool java_post_options (const char **pfilename) { /* Excess precision other than fast requires front-end support. */ if (flag_excess_precision_cmdline == EXCESS_PRECISION_STANDARD TARGET_FLT_EVAL_METHOD_NON_DEFAULT) sorry (-fexcess-precision=standard for Java); flag_excess_precision_cmdline = EXCESS_PRECISION_FAST; Sure, the Go frontend does stuff like that too. But of course the Go frontend can't directly set -mieee, because -mieee is a machine dependent option. so, you could check the setting and reset any flag that should be off or error out on incompatible flags. I'd like to think we could get more milage out of making a flag like -mieee be machine independent and then ports could just check the base flag for validating machine specific flags. Certainly alpha isn't the only port that has -mieee. There are likely to be very few flags promoted because of this, ieee being the most obvious example. What I think you are suggesting here is another approach: Alpha should set -mieee based on a machine-independent option, and then the Go frontend can set that option instead. I'm fine with that approach too. I don't think we currently have a machine-independent option which corresponds to the Alpha -mieee option. According to the documentation, -mieee does two things: adds support for NaN and infinity, and adds support for denormal numbers. The first is the -fno-finite-math-only option, which is actually the default for other targets. The second has no machine independent option as far as I know. Attached patch also does the trick for me. Please note that we set -mieee flag to compile .go files from library and also we add this flag to default testsuite compile flags. Using this patch, I was able to fix all floating point exceptions errors from libgo testsuite. What remains is a couple of unrelated failures in the testsuite: Epoll unexpected fd=0 pollServer: unexpected wakeup for fd=0 mode=w panic: test timed out ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388: 7123 Aborted ./a.out -test.short -test.timeout=$timeout $@ FAIL: http gmake[2]: *** [http/check] Error 1 2011/07/05 18:43:28 Test RPC server listening on 127.0.0.1:50334 2011/07/05 18:43:28 Test HTTP RPC server listening on 127.0.0.1:49010 2011/07/05 18:43:28 rpc.Serve: accept:accept tcp 127.0.0.1:50334: Resource temporarily unavailable FAIL: rpc gmake[2]: *** [rpc/check] Error 1 2011/07/05 18:44:22 Test WebSocket server listening on 127.0.0.1:40893 Epoll unexpected fd=0 pollServer: unexpected wakeup for fd=0 mode=w panic: test timed out ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388: 12993 Aborted ./a.out -test.short -test.timeout=$timeout $@ FAIL: websocket gmake[2]: *** [websocket/check] Error 1 ../../../gcc-svn/trunk/libgo/testsuite/gotest: line 388: 13945 Segmentation fault ./a.out -test.short -test.timeout=$timeout $@ FAIL: compress/flate gmake[2]: *** [compress/flate/check] Error 1 Any ideas how to attack these? Uros. Index: configure === --- configure (revision 175840) +++ configure (working copy) @@ -616,6 +616,7 @@ USING_SPLIT_STACK_FALSE USING_SPLIT_STACK_TRUE SPLIT_STACK +IEEE_FLAGS OSCFLAGS GO_DEBUG_PROC_REGS_OS_ARCH_FILE GO_SYSCALLS_SYSCALL_OS_ARCH_FILE @@ -10913,7 +10914,7 @@ lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 10916 configure +#line 10917 configure #include confdefs.h #if HAVE_DLFCN_H @@ -11019,7 +11020,7 @@ lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 11022 configure +#line 11023 configure #include confdefs.h #if HAVE_DLFCN_H @@ -13580,6 +13581,13 @@ esac +case ${host_cpu} in + alpha*) +IEEE_FLAGS=-mieee +;; +esac + + { $as_echo $as_me:${as_lineno-$LINENO}: checking whether -fsplit-stack is supported 5 $as_echo_n checking whether -fsplit-stack is supported... 6; } if test ${libgo_cv_c_split_stack_supported+set} = set; then : Index: Makefile.in === --- Makefile.in (revision 175840) +++ Makefile.in (working copy) @@ -365,6 +365,7 @@ GO_DEBUG_PROC_REGS_OS_ARCH_FILE = @GO_DEBUG_PROC_REGS_OS_ARCH_FILE@ GO_SYSCALLS_SYSCALL_OS_ARCH_FILE = @GO_SYSCALLS_SYSCALL_OS_ARCH_FILE@ GREP = @GREP@ +IEEE_FLAGS = @IEEE_FLAGS@
Re: [PATCH] Address lowering [1/3] Main patch
On Mon, 2011-07-04 at 17:30 +0200, Michael Matz wrote: From reading it it really seems to be a normal block-local CSE, without anything fancy. Hence, moving the pass just a little earlier (before pass_vrp/pass_dominator) should already provide for all optimizations. If not those should be improved. I did a quick hack-up, and this looks like it will work well, at least on the few simple examples I threw at it. I see that it is used for also getting rid of the zero-offset statements in case non-zero-offsets follow. I think that's generally worthwhile so probably should be done in one of the above optimizers. I will experiment with adding it to pass_dominator. Looks like tree-ssa-dom.c:optimize_stmt is the place to start. Thanks, Bill
Re: PING: PATCH [9/n]: Prepare x32: PR middle-end/47383: ivopts miscompiles Pmode != ptr_mode
On Tue, Jul 5, 2011 at 8:24 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:56 PM, Ulrich Weigand uweig...@de.ibm.com wrote: H.J. Lu wrote: However, this still seems odd to me, as I had understood the address in a TARGET_MEM_REF needs to be an *address*, i.e. use address_mode. =A0If this is not true (has changed?) a lot of other places would need to change as well ... I was told that TARGET_MEM_REF needs ptr_mode. Can you elaborate? We are talking about the mode returned from addr_for_mem_ref here. I do now understand how this can be anything but an address mode: That is an address mode, but the intermediate computation (base + index * step + offset) is done in pointer mode. The code currently performs this in address mode as well which is bogus. Which is why I suggested to use pointer mode for the computation (now, with the other target hook you mention) and then convert the result to address mode. I am testing this [patch. OK for trunk if there are no regressions? Thanks. -- H.J. --- diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32 index c5edfe7..7d85746 100644 --- a/gcc/ChangeLog.x32 +++ b/gcc/ChangeLog.x32 @@ -1,5 +1,11 @@ 2011-07-05 H.J. Lu hongjiu...@intel.com + PR middle-end/47383 + * tree-ssa-address.c (addr_for_mem_ref): Use pointer_mode for + address computation and convert to address_mode if needed. + +2011-07-05 H.J. Lu hongjiu...@intel.com + * tree-ssa-address.c (addr_for_mem_ref): Use targetm.addr_space.address_mode. diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32 index cde8d41..492be5c 100644 --- a/gcc/testsuite/ChangeLog.x32 +++ b/gcc/testsuite/ChangeLog.x32 @@ -1,3 +1,8 @@ +2011-07-05 H.J. Lu hongjiu...@intel.com + + PR middle-end/47383 + * gcc.dg/pr47383.c: New. + 2011-06-23 H.J. Lu hongjiu...@intel.com * gcc.target/i386/pr49504.c (main): Check correct return value. diff --git a/gcc/testsuite/gcc.dg/pr47383.c b/gcc/testsuite/gcc.dg/pr47383.c new file mode 100644 index 000..3e2b9ba --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr47383.c @@ -0,0 +1,31 @@ +/* { dg-do run { target fpic } } */ +/* { dg-options -O2 -fPIC } */ + +static int heap[2*(256 +1+29)+1]; +static int heap_len; +static int heap_max; +void +__attribute__ ((noinline)) +foo (int elems) +{ + int n, m; + int max_code = -1; + int node = elems; + heap_len = 0, heap_max = (2*(256 +1+29)+1); + for (n = 0; n elems; n++) +heap[++heap_len] = max_code = n; + do { +n = heap[1]; +heap[1] = heap[heap_len--]; +m = heap[1]; +heap[--heap_max] = n; +heap[--heap_max] = m; + } while (heap_len = 2); +} + +int +main () +{ + foo (286); + return 0; +} diff --git a/gcc/tree-ssa-address.c b/gcc/tree-ssa-address.c index e3934e1..c6dced1 100644 --- a/gcc/tree-ssa-address.c +++ b/gcc/tree-ssa-address.c @@ -189,11 +189,12 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, bool really_expand) { enum machine_mode address_mode = targetm.addr_space.address_mode (as); + enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as); rtx address, sym, bse, idx, st, off; struct mem_addr_template *templ; if (addr-step !integer_onep (addr-step)) -st = immed_double_int_const (tree_to_double_int (addr-step), address_mode); +st = immed_double_int_const (tree_to_double_int (addr-step), pointer_mode); else st = NULL_RTX; @@ -201,7 +202,7 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, off = immed_double_int_const (double_int_sext (tree_to_double_int (addr-offset), TYPE_PRECISION (TREE_TYPE (addr-offset))), -address_mode); +pointer_mode); else off = NULL_RTX; @@ -220,16 +221,16 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, if (!templ-ref) { sym = (addr-symbol ? -gen_rtx_SYMBOL_REF (address_mode, ggc_strdup (test_symbol)) +gen_rtx_SYMBOL_REF (pointer_mode, ggc_strdup (test_symbol)) : NULL_RTX); bse = (addr-base ? -gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 1) +gen_raw_REG (pointer_mode, LAST_VIRTUAL_REGISTER + 1) : NULL_RTX); idx = (addr-index ? -gen_raw_REG (address_mode, LAST_VIRTUAL_REGISTER + 2) +gen_raw_REG (pointer_mode, LAST_VIRTUAL_REGISTER + 2) : NULL_RTX); - gen_addr_rtx (address_mode, sym, bse, idx, + gen_addr_rtx (pointer_mode, sym, bse, idx, st? const0_rtx : NULL_RTX, off? const0_rtx : NULL_RTX, templ-ref, @@ -247,16 +248,18 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as, /* Otherwise really expand the expressions. */ sym = (addr-symbol -? expand_expr (addr-symbol,
Re: [RFC PATCH, go]: Port to ALPHA arch - sysinfo.go fixup
On Jul 5, 2011, at 9:51 AM, Uros Bizjak ubiz...@gmail.com wrote: Attached patch also does the trick for me. Please note that we set -mieee flag to compile .go files from library and also we add this flag to default testsuite compile flags. Ick, I think this patch might be expedient, but, wrong. Ian will have to think about it and decide.
Re: [RFC PATCH, go]: Port to ALPHA arch - sysinfo.go fixup
On Tue, Jul 5, 2011 at 7:17 PM, Mike Stump mikest...@comcast.net wrote: On Jul 5, 2011, at 9:51 AM, Uros Bizjak ubiz...@gmail.com wrote: Attached patch also does the trick for me. Please note that we set -mieee flag to compile .go files from library and also we add this flag to default testsuite compile flags. Ick, I think this patch might be expedient, but, wrong. Ian will have to think about it and decide. Well, this is how libgfortran handles -mieee in SH case. Uros.
Re: [testsuite]: Add require fopenmp as needed
On Jul 5, 2011, at 3:07 AM, Georg-Johann Lay wrote: There is a testcase that fails if no openmp is available. This patch fixed that. CCed contributor. Not quite sure what this means. So, on patches you want approval on, the custom is to ask Ok? so that we can quickly tell which need review. If you think it qualifies under the obvious rule, typically these don't ask Ok?. Anyway, I assume you'd like to get it approved so that you can check it in... Ok.
Re: PATCH [1/n] X32: Add initial -x32 support
On Tue, Jul 5, 2011 at 8:16 AM, Uros Bizjak ubiz...@gmail.com wrote: On Tue, Jul 5, 2011 at 4:39 PM, H.J. Lu hongjiu...@intel.com wrote: I'd like to start submitting a series of patches to enable x32: https://sites.google.com/site/x32abi/ The GCC x32 branch is very stable. There are no unexpected failures in C, C++, Fortran and Objective C testsuites. SPEC CPU 2K/2006 compile and run correctly at -O2 and -O3. More than 90% of changes are in x86 backend. I have submitted non-x86 backend patches. Most of them have been reviewed and checked in. Only 4 patches are pending for review/approval. This is the first x86 backend patch to support x32. By default, x32 is disabled and x32 run-time support isn't required. OK for trunk? Please strip out --enable-ia32 stuff, it complicates things ATM. I assume that --enable-x32 applies only to 64bit targets, so this part Done. @@ -1232,7 +1232,17 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i if test x$enable_targets = xall; then tm_file=${tm_file} i386/x86-64.h i386/gnu-user64.h i386/linux64.h tm_defines=${tm_defines} TARGET_BI_ARCH=1 - tmake_file=${tmake_file} i386/t-linux64 + case x${enable_x32}${enable_ia32} in + xyesyes) + tmake_file=${tmake_file} i386/t-linux-x32 + ;; + xyesno) + tmake_file=${tmake_file} i386/t-linux64-x32 + ;; + *) + tmake_file=${tmake_file} i386/t-linux64 + ;; + esac should be simplified to something: if (enable_x32) tmake_file = ... i386/t-linux64-x32 else tmake_file = ... i386/t-linux64 Done. diff --git a/gcc/config/i386/gnu-user64.h b/gcc/config/i386/gnu-user64.h index b069975..954f3b2 100644 --- a/gcc/config/i386/gnu-user64.h +++ b/gcc/config/i386/gnu-user64.h @@ -58,25 +58,31 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if TARGET_64BIT_DEFAULT #define SPEC_32 m32 -#define SPEC_64 !m32 +#define SPEC_64 m32|mx32:; +#define SPEC_X32 mx32 #else -#define SPEC_32 !m64 +#define SPEC_32 m64|mx32:; #define SPEC_64 m64 +#define SPEC_X32 mx32 #endif #undef ASM_SPEC -#define ASM_SPEC %{ SPEC_32 :--32} %{ SPEC_64 :--64} \ +#define ASM_SPEC %{ SPEC_32 :--32} \ + %{ SPEC_64 :--64} \ + %{ SPEC_X32 :--x32} \ %{!mno-sse2avx:%{mavx:-msse2avx}} %{msse2avx:%{!mavx:-msse2avx}} Are you sure that above is correct? AFAICS, you are enabling SPEC_64 for m32 in case of TARGET_64BIT_DEFAULT. I have #define SPEC_64 m32|mx32:; There are :;. This is similar to if then else. SPEC_64 will be false if -m32 or -mx32 is used. /* SSE4.1 defines round instructions */ #define OPTION_MASK_ISA_ROUND OPTION_MASK_ISA_SSE4_1 @@ -517,8 +519,8 @@ extern const char *host_detect_local_cpu (int argc, const char **argv); #define OPT_ARCH64 !m32 #define OPT_ARCH32 m32 #else -#define OPT_ARCH64 m64 -#define OPT_ARCH32 !m64 +#define OPT_ARCH64 m64|mx32 +#define OPT_ARCH32 m64|mx32:; #endif Same here. See above. Here is the updated patch. OK for trunk? Thanks. -- H.J. --- 2011-07-05 H.J. Lu hongjiu...@intel.com * config.gcc: Support --enable-x32 for x86 Linux targets. * configure.ac: Support --enable-x32. * configure: Regenerated. * config/i386/gnu-user64.h (SPEC_64): Support x32. (SPEC_32): Likewise. (ASM_SPEC): Likewise. (LINK_SPEC): Likewise. (TARGET_THREAD_SSP_OFFSET): Likewise. (TARGET_THREAD_SPLIT_STACK_OFFSET): Likewise. (SPEC_X32): New. * config/i386/i386.h (TARGET_X32): New. (TARGET_LP64): New. (LONG_TYPE_SIZE): Likewise. (POINTER_SIZE): Likewise. (POINTERS_EXTEND_UNSIGNED): Likewise. (OPT_ARCH64): Support x32. (OPT_ARCH32): Likewise. * config/i386/i386.opt (mx32): New. * config/i386/kfreebsd-gnu64.h (GNU_USER_LINK_EMULATIONX32): New. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/linux64.h (GNU_USER_LINK_EMULATIONX32): Likewise. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/t-linux-x32: New. * config/linux.h (UCLIBC_DYNAMIC_LINKERX32): New. (BIONIC_DYNAMIC_LINKERX32): Likewise. (GNU_USER_DYNAMIC_LINKERX32): Likewise. * doc/install.texi: Document --enable-x32. * doc/invoke.texi: Document -mx32. 2011-07-05 H.J. Lu hongjiu...@intel.com * config.gcc: Support --enable-x32 for x86 Linux targets. * configure.ac: Support --enable-x32. * configure: Regenerated. * config/i386/gnu-user64.h (SPEC_64): Support x32. (SPEC_32):
Re: [testsuite]: Add require fopenmp as needed
Mike Stump wrote: On Jul 5, 2011, at 3:07 AM, Georg-Johann Lay wrote: There is a testcase that fails if no openmp is available. This patch fixed that. CCed contributor. Not quite sure what this means. So, on patches you want approval on, the custom is to ask Ok? so that we can quickly tell which need review. If you think it Ok, Thanks for pointing that out. qualifies under the obvious rule, typically these don't ask Ok?. Anyway, I assume you'd like to get it approved so that you can check it in... Ok.
Re: [testsuite]: Fix testcases that need int = 32 bits.
On Jul 5, 2011, at 2:02 AM, Georg-Johann Lay wrote: There are still testcase that break on targets with int 32 bits. [ Be sure to ask Ok? ] Ok.
Re: PATCH [1/n] X32: Add initial -x32 support
On Tue, Jul 5, 2011 at 7:54 PM, H.J. Lu hjl.to...@gmail.com wrote: I'd like to start submitting a series of patches to enable x32: https://sites.google.com/site/x32abi/ The GCC x32 branch is very stable. There are no unexpected failures in C, C++, Fortran and Objective C testsuites. SPEC CPU 2K/2006 compile and run correctly at -O2 and -O3. More than 90% of changes are in x86 backend. I have submitted non-x86 backend patches. Most of them have been reviewed and checked in. Only 4 patches are pending for review/approval. This is the first x86 backend patch to support x32. By default, x32 is disabled and x32 run-time support isn't required. OK for trunk? Please strip out --enable-ia32 stuff, it complicates things ATM. I assume that --enable-x32 applies only to 64bit targets, so this part I think that better name of the file would be t-linux64-x32. @@ -2631,6 +2640,7 @@ esac case ${target} in i[34567]86-*-linux* | x86_64-*-linux*) tmake_file=${tmake_file} i386/t-pmm_malloc i386/t-i386 + libgcc_tm_file=${libgcc_tm_file} i386/value-unwind.h Not yet. @@ -58,25 +58,31 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if TARGET_64BIT_DEFAULT #define SPEC_32 m32 -#define SPEC_64 !m32 +#define SPEC_64 m32|mx32:; +#define SPEC_X32 mx32 #else -#define SPEC_32 !m64 +#define SPEC_32 m64|mx32:; #define SPEC_64 m64 +#define SPEC_X32 mx32 #endif I really think that !(m64|mx32) is more descriptive and clear... #undef LINK_SPEC #define LINK_SPEC %{ SPEC_64 :-m GNU_USER_LINK_EMULATION64 } \ %{ SPEC_32 :-m GNU_USER_LINK_EMULATION32 } \ + %{ SPEC_X32 :-m GNU_USER_LINK_EMULATIONX32 } \ %{shared:-shared} \ %{!shared: \ %{!static: \ %{rdynamic:-export-dynamic} \ %{ SPEC_32 :-dynamic-linker GNU_USER_DYNAMIC_LINKER32 } \ - %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 }} \ + %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 } \ + %{ SPEC_X32 :-dynamic-linker GNU_USER_DYNAMIC_LINKERX32 }} \ %{static:-static}} On the border of bikesheding, GNU_USER_LINK_EMULATION64_X32 and GNU_USER_DYNAMIC_LINKER64_X32 sounds better to me. Same with the below: +#define GLIBC_DYNAMIC_LINKERX32 /libx32/ld-linux-x32.so.2 +#define UCLIBC_DYNAMIC_LINKERX32 /lib/ldx32-uClibc.so.0 +#define BIONIC_DYNAMIC_LINKERX32 /system/bin/linkerx32 +++ b/gcc/config/i386/t-linux-x32 Please rename above file to t-linux64-x32. With above small changes, the patch looks OK to me. Please also wait for build and options maintainer (CC'd) approvals. Thanks, Uros.
Re: [patch, fortran] Always return malloc(1) for empty arrays in the library
Hi Janne, Since size_t is unsigned, just test (size == 0). Otherwise Ok. Thanks for the patch. OK. Übertrage Daten ... Revision 175880 übertragen. Thanks for the review! Thomas
Re: PATCH [1/n] X32: Add initial -x32 support
On Tue, Jul 5, 2011 at 11:14 AM, Uros Bizjak ubiz...@gmail.com wrote: On Tue, Jul 5, 2011 at 7:54 PM, H.J. Lu hjl.to...@gmail.com wrote: I'd like to start submitting a series of patches to enable x32: https://sites.google.com/site/x32abi/ The GCC x32 branch is very stable. There are no unexpected failures in C, C++, Fortran and Objective C testsuites. SPEC CPU 2K/2006 compile and run correctly at -O2 and -O3. More than 90% of changes are in x86 backend. I have submitted non-x86 backend patches. Most of them have been reviewed and checked in. Only 4 patches are pending for review/approval. This is the first x86 backend patch to support x32. By default, x32 is disabled and x32 run-time support isn't required. OK for trunk? Please strip out --enable-ia32 stuff, it complicates things ATM. I assume that --enable-x32 applies only to 64bit targets, so this part I think that better name of the file would be t-linux64-x32. @@ -2631,6 +2640,7 @@ esac case ${target} in i[34567]86-*-linux* | x86_64-*-linux*) tmake_file=${tmake_file} i386/t-pmm_malloc i386/t-i386 + libgcc_tm_file=${libgcc_tm_file} i386/value-unwind.h Not yet. Removed. @@ -58,25 +58,31 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if TARGET_64BIT_DEFAULT #define SPEC_32 m32 -#define SPEC_64 !m32 +#define SPEC_64 m32|mx32:; +#define SPEC_X32 mx32 #else -#define SPEC_32 !m64 +#define SPEC_32 m64|mx32:; #define SPEC_64 m64 +#define SPEC_X32 mx32 #endif I really think that !(m64|mx32) is more descriptive and clear... I don't know if gcc spec supports it. I just followed mips backend. #undef LINK_SPEC #define LINK_SPEC %{ SPEC_64 :-m GNU_USER_LINK_EMULATION64 } \ %{ SPEC_32 :-m GNU_USER_LINK_EMULATION32 } \ + %{ SPEC_X32 :-m GNU_USER_LINK_EMULATIONX32 } \ %{shared:-shared} \ %{!shared: \ %{!static: \ %{rdynamic:-export-dynamic} \ %{ SPEC_32 :-dynamic-linker GNU_USER_DYNAMIC_LINKER32 } \ - %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 }} \ + %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 } \ + %{ SPEC_X32 :-dynamic-linker GNU_USER_DYNAMIC_LINKERX32 }} \ %{static:-static}} On the border of bikesheding, GNU_USER_LINK_EMULATION64_X32 and GNU_USER_DYNAMIC_LINKER64_X32 sounds better to me. Same with the below: +#define GLIBC_DYNAMIC_LINKERX32 /libx32/ld-linux-x32.so.2 +#define UCLIBC_DYNAMIC_LINKERX32 /lib/ldx32-uClibc.so.0 +#define BIONIC_DYNAMIC_LINKERX32 /system/bin/linkerx32 +++ b/gcc/config/i386/t-linux-x32 Please rename above file to t-linux64-x32. X32 is the name of the psABI: https://sites.google.com/site/x32abi/ We have -mx32, -m32 and -m64 command line options and There are macros like TARGET_X32. I'd like to be consistent and avoid 64 when referring to x32 if possible. But I won't insist. Please let me know that you really won't like x32 without 64. With above small changes, the patch looks OK to me. Please also wait for build and options maintainer (CC'd) approvals. Thanks, Uros. Thanks. -- H.J. 2011-07-05 H.J. Lu hongjiu...@intel.com * config.gcc: Support --enable-x32 for x86 Linux targets. * configure.ac: Support --enable-x32. * configure: Regenerated. * config/i386/gnu-user64.h (SPEC_64): Support x32. (SPEC_32): Likewise. (ASM_SPEC): Likewise. (LINK_SPEC): Likewise. (TARGET_THREAD_SSP_OFFSET): Likewise. (TARGET_THREAD_SPLIT_STACK_OFFSET): Likewise. (SPEC_X32): New. * config/i386/i386.h (TARGET_X32): New. (TARGET_LP64): New. (LONG_TYPE_SIZE): Likewise. (POINTER_SIZE): Likewise. (POINTERS_EXTEND_UNSIGNED): Likewise. (OPT_ARCH64): Support x32. (OPT_ARCH32): Likewise. * config/i386/i386.opt (mx32): New. * config/i386/kfreebsd-gnu64.h (GNU_USER_LINK_EMULATIONX32): New. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/linux64.h (GNU_USER_LINK_EMULATIONX32): Likewise. (GLIBC_DYNAMIC_LINKERX32): Likewise. * config/i386/t-linux-x32: New. * config/linux.h (UCLIBC_DYNAMIC_LINKERX32): New. (BIONIC_DYNAMIC_LINKERX32): Likewise. (GNU_USER_DYNAMIC_LINKERX32): Likewise. * doc/install.texi: Document --enable-x32. * doc/invoke.texi: Document -mx32. diff --git a/gcc/config.gcc b/gcc/config.gcc index c77f40b..4787d6b 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -1279,7 +1279,11 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i if test x$enable_targets = xall; then tm_file=${tm_file} i386/x86-64.h i386/gnu-user64.h i386/linux64.h tm_defines=${tm_defines} TARGET_BI_ARCH=1 - tmake_file=${tmake_file} i386/t-linux64 + if test x${enable_x32}
Re: [2/11] Neater tests for signbits
On 07/01/2011 10:29 AM, Bernd Schmidt wrote: * cse.c (find_comparison_args): Use val_mode_signbit_set_p. * simplify-rtx.c (mode_signbit_p): Use GET_MODE_PRECISION. (val_mode_signbit_p, val_mode_signbit_set_p): New functions. (simplify_const_unary_operation, simplify_binary_operation_1, simplify_const_binary_operation, simplify_const_relational_operation): Use them. Use GET_MODE_MASK for masking and sign-extensions. * combine.c (set_nonzero_bits_and_sign_copies, simplify_set, combine_simplify_rtx, force_to_mode, reg_nonzero_bits_for_combine, simplify_shift_const_1, simplify_comparison): Likewise. * expr.c (convert_modes): Likewise. * rtlanal.c (nonzero_bits1, canonicalize_condition): Likewise. * expmed.c (emit_cstore, emit_store_flag_1, emit_store_flag): Likewise. * rtl.h (val_mode_signbit_p, val_mode_signbit_set_p): Declare. Ok, but, /* We must sign or zero-extend in this case. Start by zero-extending, then sign extend if we need to. */ - val = ((HOST_WIDE_INT) 1 width) - 1; + val = GET_MODE_MASK (oldmode); if (! unsignedp -(val ((HOST_WIDE_INT) 1 (width - 1 - val |= (HOST_WIDE_INT) (-1) width; +val_signbit_known_set_p (oldmode, val)) + val |= ~GET_MODE_MASK (oldmode); return gen_int_mode (val, mode); Shouldn't that sign-extension already be done by gen_int_mode? There are at least 4 more copies of this idiom in your patch. As a follow-up could you pull that out into a new function? Perhaps signextend_int_mode, akin to gen_int_mode? r~
Re: [3/11] Remove some dead code
On 07/01/2011 10:30 AM, Bernd Schmidt wrote: * simplify-rtx.c (simplify_ternary_operation): Remove dead code. Index: baseline-trunk/gcc/simplify-rtx.c === --- baseline-trunk.orig/gcc/simplify-rtx.c +++ baseline-trunk/gcc/simplify-rtx.c @@ -4948,15 +4948,6 @@ simplify_ternary_operation (enum rtx_cod val |= ~ (((unsigned HOST_WIDE_INT) 1 INTVAL (op1)) - 1); } - /* Clear the bits that don't belong in our mode, - unless they and our sign bit are all one. - So we get either a reasonable negative value or a reasonable - unsigned value for this mode. */ - if (width HOST_BITS_PER_WIDE_INT -((val ((unsigned HOST_WIDE_INT) (-1) (width - 1))) - != ((unsigned HOST_WIDE_INT) (-1) (width - 1 - val = ((unsigned HOST_WIDE_INT) 1 width) - 1; - return gen_int_mode (val, mode); Hah. My question re patch 2. Obviously ok. r~
Re: [4/11] Use precisions for TRULY_NOOP_TRUNCATION
On 07/01/2011 10:31 AM, Bernd Schmidt wrote: * machmode.h (TRULY_NOOP_TRUNCATION_MODES_P): New macro. * combine.c (make_extraction, gen_lowpart_or_truncate, apply_distributive_law, simplify_comparison, reg_truncated_to_mode, record_truncated_value): Use it. * cse.c (notreg_cost): Likewise. * expmed.c (store_bit_field_1, extract_bit_field_1): Likewise. * expr.c (convert_move, convert_modes): Likewise. * optabs.c (expand_binop, expand_unop): Likewise. * postreload.c (move2add_last_label): Likewise. * regmove.c (optimize_reg_copy_3): Likewise. * rtlhooks.c (gen_lowpart_general): Likewise. * simplify-rtx.c (simplify_unary_operation_1): Likewise. Ok. r~
Ping^2: TARGET_HAVE_NAMED_SECTIONS cleanup
Ping^2. The patch http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01642.html is still pending review. -- Joseph S. Myers jos...@codesourcery.com
Re: [5/11] Neater tests for paradoxical subregs
On 07/01/2011 10:33 AM, Bernd Schmidt wrote: * emit-rtl.c (paradoxical_subreg_p): New function. * rtl.h (paradoxical_subreg_p): Declare. * combine.c (set_nonzero_bits_and_sign_copies, get_last_value, apply_distributive_law, simplify_comparison, simplify_set): Use it. * cse.c (record_jump_cond, cse_insn): Likewise. * expr.c (force_operand): Likewise. * rtlanal.c (num_sign_bit_copies1): Likewise. * reload1.c (eliminate_regs_1, strip_paradoxical_subreg): Likewise. * reload.c (push_secondary_reload, find_reloads_toplev): Likewise. (push_reload): Use precision to check for paradoxical subregs. * expmed.c (extract_bit_field_1): Likewise. Ok. r~
Re: [6/11] Tests for HOST_WIDE_INT representability
On 07/01/2011 10:34 AM, Bernd Schmidt wrote: * machmode.h (HWI_COMPUTABLE_MODE_P): New macro. * combine.c (set_nonzero_bits_and_sign_copies): Use it. (find_split-point, combine_simplify_rtx, simplify_if_then_else, simplify_set, simplify_logical, expand_compound_operation, make_extraction, force_to_mode, if_then_else_cond, extended_count, try_widen_shift_mode, simplify_shift_const_1, simplify_comparison, record_value_for_reg): Likewise. * expmed.c (expand_widening_mult, expand_mult_highpart): Likewise. * simplify-rtx. c (simplify_unary_operation_1, simplify_binary_operation_1, simplify_const_relational_operation): Likewise. Ok. r~
Re: [PATCH, go] Re: Should rename ELFOSABI_LINUX into ELFOSABI_GNU, and drop ELFOSABI_HURD
Thomas Schwinge tho...@schwinge.name writes: The only ELFOSABI_* occurrences in GCC trunk are in libgo. Ian, what do you think about the following patch (untested -- what testing does this need)? Is it even worth keeping the ELFOSABI_LINUX alias? (It can never be returned via osabiStrings.) These files are simply copied directly from the master Go library, so the change should be made there. See libgo/README and http://golang.org/doc/contribute.html. I can probably take care of it at some point if you don't want to bother. Ian
Re: PATCH [1/n] X32: Add initial -x32 support
On Tue, Jul 5, 2011 at 8:49 PM, H.J. Lu hjl.to...@gmail.com wrote: #undef LINK_SPEC #define LINK_SPEC %{ SPEC_64 :-m GNU_USER_LINK_EMULATION64 } \ %{ SPEC_32 :-m GNU_USER_LINK_EMULATION32 } \ + %{ SPEC_X32 :-m GNU_USER_LINK_EMULATIONX32 } \ %{shared:-shared} \ %{!shared: \ %{!static: \ %{rdynamic:-export-dynamic} \ %{ SPEC_32 :-dynamic-linker GNU_USER_DYNAMIC_LINKER32 } \ - %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 }} \ + %{ SPEC_64 :-dynamic-linker GNU_USER_DYNAMIC_LINKER64 } \ + %{ SPEC_X32 :-dynamic-linker GNU_USER_DYNAMIC_LINKERX32 }} \ %{static:-static}} On the border of bikesheding, GNU_USER_LINK_EMULATION64_X32 and GNU_USER_DYNAMIC_LINKER64_X32 sounds better to me. Same with the below: +#define GLIBC_DYNAMIC_LINKERX32 /libx32/ld-linux-x32.so.2 +#define UCLIBC_DYNAMIC_LINKERX32 /lib/ldx32-uClibc.so.0 +#define BIONIC_DYNAMIC_LINKERX32 /system/bin/linkerx32 +++ b/gcc/config/i386/t-linux-x32 Please rename above file to t-linux64-x32. X32 is the name of the psABI: https://sites.google.com/site/x32abi/ We have -mx32, -m32 and -m64 command line options and There are macros like TARGET_X32. I'd like to be consistent and avoid 64 when referring to x32 if possible. But I won't insist. Please let me know that you really won't like x32 without 64. I would like to point out that the base target is in fact 64 bit and a subtarget is x32, so ...64-x32. I could read this as 64bit target with x32 ABI. Perhaps we will have 64-xxx or whatever different ABIs that all apply to the same 64bit hardware. These are my personal preferences, so I will leave the final decision about names of defines and file names to you. Thanks, Uros.