[Bug c++/66211] [5/6 Regression] Rvalue conversion in ternary operator causes internal compiler error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66211 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-05-20 CC||jakub at gcc dot gnu.org Target Milestone|--- |5.2 Summary|Rvalue conversion in|[5/6 Regression] Rvalue |ternary operator causes |conversion in ternary |internal compiler error |operator causes internal ||compiler error Ever confirmed|0 |1 --- Comment #1 from Jakub Jelinek jakub at gcc dot gnu.org --- Started with r217279.
[PINGv7][PATCH] ASan on unaligned accesses
On 05/12/2015 02:16 PM, Marat Zakirov wrote: On 04/07/2015 03:22 PM, Jakub Jelinek wrote: How are the automatic misaligned variables different from say heap allocated ones, or global vars etc.? No difference you are right Jakub. Shadow memory initialization for heap values and globals of course also should be changed but it is a task for libsanitizer not ASan for which I am sending patch. Fix for libsanitizer to support unaligned heaps and globals will be committed by a separate patch. Well, a RTL solution I've tried at http://gcc.gnu.org/PR22141, but it gave mixed results, so either it needs more cost tuning when it is desirable and when it is not, or perhaps better do that still on GIMPLE instead, together with trying to optimize bitfield accesses and other cases of adjacent location accesses. But if we handle that on GIMPLE, it won't really affect what asan RTL emitting code produces. Jakub I fixed the issue with 'movq' you were mentioned in a previous mail. --Marat gcc/ChangeLog: 2015-02-25 Marat Zakirov m.zaki...@samsung.com * asan.c (asan_emit_stack_protection): Support for misalign accesses. (asan_expand_check_ifn): Likewise. * params.def: New option asan-catch-misaligned. * params.h: New param ASAN_CATCH_MISALIGNED. * doc/invoke.texi: New asan param description. gcc/testsuite/ChangeLog: 2015-02-25 Marat Zakirov m.zaki...@samsung.com * c-c++-common/asan/misalign-catch.c: New test. diff --git a/gcc/asan.c b/gcc/asan.c index 9e4a629..f9d052f 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1050,7 +1050,6 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, rtx_code_label *lab; rtx_insn *insns; char buf[30]; - unsigned char shadow_bytes[4]; HOST_WIDE_INT base_offset = offsets[length - 1]; HOST_WIDE_INT base_align_bias = 0, offset, prev_offset; HOST_WIDE_INT asan_frame_size = offsets[0] - base_offset; @@ -1059,6 +1058,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT; tree str_cst, decl, id; int use_after_return_class = -1; + bool misalign = (flag_sanitize SANITIZE_KERNEL_ADDRESS) + || ASAN_CATCH_MISALIGNED; if (shadow_ptr_types[0] == NULL_TREE) asan_init_shadow_ptr_types (); @@ -1193,11 +1194,37 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, if (STRICT_ALIGNMENT) set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode))); prev_offset = base_offset; + + vecrtx shadow_mems; + vecunsigned char shadow_bytes; + + shadow_mems.create (0); + shadow_bytes.create (0); + for (l = length; l; l -= 2) { if (l == 2) cur_shadow_byte = ASAN_STACK_MAGIC_RIGHT; offset = offsets[l - 1]; + if (l != length misalign) + { + HOST_WIDE_INT aoff + = base_offset + ((offset - base_offset) + ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1)) + - ASAN_RED_ZONE_SIZE; + if (aoff prev_offset) + { + shadow_mem = adjust_address (shadow_mem, VOIDmode, + (aoff - prev_offset) + ASAN_SHADOW_SHIFT); + prev_offset = aoff; + shadow_bytes.safe_push (0); + shadow_bytes.safe_push (0); + shadow_bytes.safe_push (0); + shadow_bytes.safe_push (0); + shadow_mems.safe_push (shadow_mem); + } + } if ((offset - base_offset) (ASAN_RED_ZONE_SIZE - 1)) { int i; @@ -1212,13 +1239,13 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, if (aoff offset) { if (aoff offset - (1 ASAN_SHADOW_SHIFT) + 1) - shadow_bytes[i] = 0; + shadow_bytes.safe_push (0); else - shadow_bytes[i] = offset - aoff; + shadow_bytes.safe_push (offset - aoff); } else - shadow_bytes[i] = ASAN_STACK_MAGIC_PARTIAL; - emit_move_insn (shadow_mem, asan_shadow_cst (shadow_bytes)); + shadow_bytes.safe_push (ASAN_STACK_MAGIC_PARTIAL); + shadow_mems.safe_push (shadow_mem); offset = aoff; } while (offset = offsets[l - 2] - ASAN_RED_ZONE_SIZE) @@ -1227,12 +1254,21 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, (offset - prev_offset) ASAN_SHADOW_SHIFT); prev_offset = offset; - memset (shadow_bytes, cur_shadow_byte, 4); - emit_move_insn (shadow_mem, asan_shadow_cst (shadow_bytes)); + shadow_bytes.safe_push (cur_shadow_byte); + shadow_bytes.safe_push (cur_shadow_byte); + shadow_bytes.safe_push (cur_shadow_byte); + shadow_bytes.safe_push (cur_shadow_byte); + shadow_mems.safe_push (shadow_mem); offset += ASAN_RED_ZONE_SIZE; } cur_shadow_byte = ASAN_STACK_MAGIC_MIDDLE; } + for (unsigned i = 0; misalign i shadow_bytes.length () - 1; i++) +if (shadow_bytes[i] == 0 shadow_bytes[i + 1] 0) + shadow_bytes[i] = 8 + (shadow_bytes[i + 1] 7 ? 0 : shadow_bytes[i + 1]); + for (unsigned i = 0; i shadow_mems.length (); i++) +emit_move_insn (shadow_mems[i], asan_shadow_cst
Re: [c++std-parallel-1614] Re: Compilers and RCU readers: Once more unto the breach!
On 05/20/2015 04:34 AM, Paul E. McKenney wrote: On Tue, May 19, 2015 at 06:57:02PM -0700, Linus Torvalds wrote: - the you can add/subtract integral values still opens you up to language lawyers claiming (char *)ptr - (intptr_t)ptr preserving the dependency, which it clearly doesn't. But language-lawyering it does, since all those operations (cast to pointer, cast to integer, subtracting an integer) claim to be dependency-preserving operations. [...] There are some stranger examples, such as (char *)ptr - ((intptr_t)ptr)/7, but in that case, if the resulting pointer happens by chance to reference valid memory, I believe a dependency would still be carried. [...] From a language lawyer standpoint, pointer arithmetic is only valid within an array. These examples seem to go beyond the bounds of the array and therefore have undefined behavior. C++ standard section 5.7 paragraph 4 If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined. C99 and C11 identical phrasing in 6.5.6 paragraph 8 Jens
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #13 from rguenther at suse dot de rguenther at suse dot de --- On Wed, 20 May 2015, jakub at gcc dot gnu.org wrote: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #12 from Jakub Jelinek jakub at gcc dot gnu.org --- hashval_t should be unsigned int? Is it some other type on your host, or is CHAR_BIT bigger than 8, or do you have 64-bit unsigned int? Yeah, it looks odd. All hosts should have uint64_t nowadays, so even before honzas patch we _always_ should have gone the mul_mod path. Can you attach pre-processed source with the revision you bisected reverted? (pre-processed source of genmatch.c, that is?) Might be that bconfig.h/system.h combo for your host makes the difference somehow.
[Bug gcov-profile/66209] Out of memory when compiling with --coverage and optimizations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66209 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Blocks||64928 --- Comment #1 from Richard Biener rguenth at gcc dot gnu.org --- Maybe related to PR64928 Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64928 [Bug 64928] [4.8/4.9/5/6 Regression] Inordinate cpu time and memory usage in phase opt and generate with -ftest-coverage -fprofile-arcs
Re: [gomp4] bootstrap broken, function enclosing_target_ctx defined but not used
Hi! On Tue, 19 May 2015 09:24:51 +0200, Tom de Vries tom_devr...@mentor.com wrote: On 18-05-15 17:31, Tom de Vries wrote: In ran into this bootstrap failure with branch gomp-4_0-branch: ... src/gcc-gomp-4_0-branch/gcc/omp-low.c:2897:1: error: 'omp_context* enclosing_target_ctx(omp_context*)' defined but not used [-Werror=unused-function] enclosing_target_ctx (omp_context *ctx) ^ cc1plus: all warnings being treated as errors make[3]: *** [omp-low.o] Error 1 ... I can only encourage everyone to pay attention to compiler warnings. This patch fixes bootstrap by commenting out the unused function enclosing_target_ctx. The patch just comments it out, since I'm not sure whether: - the function needs to be removed, or - a user of the function will soon be committed. Well, looking at the recent revision history, I see that in r223222 Cesar has removed the single use of enclosing_target_ctx, http://news.gmane.org/find-root.php?message_id=%3C5556368D.7010904%40codesourcery.com%3E, so I'd assume it is no longer needed? That is, Cesar, please remove the function in this case. Committed to fix bootstrap. Thanks! Comment out unused enclosing_target_ctx 2015-05-19 Tom de Vries t...@codesourcery.com * omp-low.c (enclosing_target_ctx): Comment out. --- gcc/omp-low.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 914549c..3414ab5 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -2893,6 +2893,7 @@ finish_taskreg_scan (omp_context *ctx) } +#if 0 static omp_context * enclosing_target_ctx (omp_context *ctx) { @@ -2902,6 +2903,7 @@ enclosing_target_ctx (omp_context *ctx) gcc_assert (ctx != NULL); return ctx; } +#endif static bool oacc_loop_or_target_p (gimple stmt) Grüße, Thomas pgp6wa5iTpD1K.pgp Description: PGP signature
[Bug tree-optimization/65961] [6 Regression] ice in vect_is_simple_use_1 with -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65961 --- Comment #6 from Richard Biener rguenth at gcc dot gnu.org --- It might be mitigated for the testcase in question but the underlying problem didn't get fixed.
[Bug c++/66211] [5/6 Regression] Rvalue conversion in ternary operator causes internal compiler error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66211 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org --- Comment #2 from Richard Biener rguenth at gcc dot gnu.org --- I will have a look.
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #12 from Jakub Jelinek jakub at gcc dot gnu.org --- hashval_t should be unsigned int? Is it some other type on your host, or is CHAR_BIT bigger than 8, or do you have 64-bit unsigned int?
[Bug tree-optimization/65961] [6 Regression] ice in vect_is_simple_use_1 with -O3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65961 --- Comment #5 from David Binderman dcb314 at hotmail dot com --- As of trunk 20150520, this bug looks fixed to me.
[Bug target/65979] Multiple issues in conftest.c prevent build on sh4-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 Oleg Endo olegendo at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-05-20 Ever confirmed|0 |1 --- Comment #14 from Oleg Endo olegendo at gcc dot gnu.org --- (In reply to Kazumoto Kojima from comment #13) Looks that this doesn't work when the operand 1 is equal to the operand 2 which is the case for the above insns 92, 93 and 83. The peephole removed with the fix in PR65153 transformed these insns prior to the above peephole so to avoid the problem. The patch below fixes this case. It looks there are similar peephole patterns,though. Oleg, if you get the spare time, could you look into these peepholes? -- diff --git a/config/sh/sh.md b/config/sh/sh.md index 27f0074..5bc3401 100644 --- a/config/sh/sh.md +++ b/config/sh/sh.md @@ -14750,6 +14750,7 @@ label: TARGET_SH1 peep2_reg_dead_p (3, operands[0]) !reg_overlap_mentioned_p (operands[0], operands[3]) +!reg_overlap_mentioned_p (operands[1], operands[2]) (REGNO (operands[0]) == REGNO (operands[4]) || REGNO (operands[0]) == REGNO (operands[5])) (REGNO (operands[2]) == REGNO (operands[4]) Sorry for the long chain of trouble and pain. Adding the !reg_overlap_mentioned_p (operands[1], operands[2]) condition should fix the problem. However, the peephole is actually meant to transform (insn 92 14 15 3 (set (reg:SI 1 r1 [orig:171 D.2078 ] [171]) (reg:SI 2 r2 [172])) 252 {movsi_ie} (expr_list:REG_DEAD (reg:SI 2 r2 [172]) (nil))) (insn 93 16 83 3 (set (reg:SI 2 r2) (const_int 66602 [0x1042a])) 252 {movsi_ie} (nil)) (insn 83 93 18 3 (set (reg:SI 147 t) (eq:SI (and:SI (reg:SI 1 r1 [orig:171 D.2078 ] [171]) (reg:SI 2 r2)) (const_int 0 [0]))) 1 {tstsi_t} (expr_list:REG_DEAD (reg:SI 2 r2) (expr_list:REG_DEAD (reg:SI 1 r1 [orig:171 D.2078 ] [171]) (nil into (insn 93 16 83 3 (set (reg:SI 2 r1) (const_int 66602 [0x1042a])) 252 {movsi_ie} (nil)) (insn 83 93 18 3 (set (reg:SI 147 t) (eq:SI (and:SI (reg:SI 1 r1 [orig:171 D.2078 ] [171]) (reg:SI 2 r2)) (const_int 0 [0]))) 1 {tstsi_t} (expr_list:REG_DEAD (reg:SI 2 r2) (expr_list:REG_DEAD (reg:SI 1 r1 [orig:171 D.2078 ] [171]) (nil I think the check operands[1] / operands[2] check should go into the preparation statement. operands[0] is dying after this peephole, so I guess this should work: { if (reg_overlap_mentioned_p (operands[1], operands[2])) std::swap (operands[0], operands[2]) sh_check_add_incdec_notes (emit_move_insn (operands[2], operands[3])); emit_insn (gen_tstsi_t (operands[2], gen_rtx_REG (SImode, (REGNO (operands[1]); } I guess that the following peephole (sh.md line 14733) will also have a similar problem.
Re: [patch,gomp4] error on invalid acc loop clauses
On Wed, May 20, 2015 at 11:32:20AM +0200, Thomas Schwinge wrote: Hi! On Wed, 20 May 2015 10:43:27 +0200, Jakub Jelinek ja...@redhat.com wrote: On Wed, May 20, 2015 at 10:23:21AM +0200, Thomas Schwinge wrote: I see that some checking is also being done gcc/omp-low.c:scan_omp_for: »gang, worker and vector may occur only once in a loop nest«, and »gang, worker and vector must occur in this order in a loop nest«. Don't know if that conceptually also belongs into gcc/omp-low.c:check_omp_nesting_restrictions? Doesn't look like anything related to construct/region nesting... It is checking invalid nesting of loop constructs, for example: #pragma acc loop gang for [...] { #pragma acc loop gang // gang, worker and vector may occur only once in a loop nest for [...] ..., or: #pragma acc loop vector for [...] { #pragma acc loop gang // gang, worker and vector must occur in this order in a loop nest for [...] ..., and so on. Ah, in that case it is the right function for that. Jakub
[Bug target/65979] Multiple issues in conftest.c prevent build on sh4-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #16 from Kazumoto Kojima kkojima at gcc dot gnu.org --- (In reply to Oleg Endo from comment #15) Thanks for a quick look! However, I think that the emit_move_insn could also be a source of hidden problems. For instance, if the captured insn Also arguments of emit_move_insn must have the same integer modes. if (reg_overlap_mentioned_p (operands[1], operands[2])) std::swap (operands[0], operands[2]); sh_check_add_incdec_notes (emit_move_insn (operands[2], operands[3])); might ICE if operands[0] and operands[2] have different modes and swap happens, though I'm not sure whether such insns are real or not.
[C PATCH] Use VAR_OR_FUNCTION_DECL_P
The following patch is an effort to use the macro where appropriate in c/ and c-family/ directories. No functional changes intended. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-05-20 Marek Polacek pola...@redhat.com * c-pragma.c: Use VAR_OR_FUNCTION_DECL_P throughout. * c-common.c: Likewise. * c-decl.c: Use VAR_OR_FUNCTION_DECL_P throughout. * c-typeck.c: Likewise. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 3998b23..a2b3793 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -7406,7 +7406,7 @@ handle_externally_visible_attribute (tree *pnode, tree name, { tree node = *pnode; - if (TREE_CODE (node) == FUNCTION_DECL || TREE_CODE (node) == VAR_DECL) + if (VAR_OR_FUNCTION_DECL_P (node)) { if ((!TREE_STATIC (node) TREE_CODE (node) != FUNCTION_DECL !DECL_EXTERNAL (node)) || !TREE_PUBLIC (node)) @@ -7437,7 +7437,7 @@ handle_no_reorder_attribute (tree *pnode, { tree node = *pnode; - if ((TREE_CODE (node) != FUNCTION_DECL TREE_CODE (node) != VAR_DECL) + if (!VAR_OR_FUNCTION_DECL_P (node) !(TREE_STATIC (node) || DECL_EXTERNAL (node))) { warning (OPT_Wattributes, @@ -7893,7 +7893,7 @@ handle_section_attribute (tree *node, tree ARG_UNUSED (name), tree args, user_defined_section_attribute = true; - if (TREE_CODE (decl) != FUNCTION_DECL TREE_CODE (decl) != VAR_DECL) + if (!VAR_OR_FUNCTION_DECL_P (decl)) { error (section attribute not allowed for %q+D, *node); goto fail; @@ -8172,8 +8172,7 @@ handle_weak_attribute (tree *node, tree name, *no_add_attrs = true; return NULL_TREE; } - else if (TREE_CODE (*node) == FUNCTION_DECL - || TREE_CODE (*node) == VAR_DECL) + else if (VAR_OR_FUNCTION_DECL_P (*node)) { struct symtab_node *n = symtab_node::get (*node); if (n n-refuse_visibility_changes) @@ -8309,7 +8308,7 @@ handle_weakref_attribute (tree *node, tree ARG_UNUSED (name), tree args, such symbols do not even have a DECL_WEAK field. */ if (decl_function_context (*node) || current_function_decl - || (TREE_CODE (*node) != VAR_DECL TREE_CODE (*node) != FUNCTION_DECL)) + || !VAR_OR_FUNCTION_DECL_P (*node)) { warning (OPT_Wattributes, %qE attribute ignored, name); *no_add_attrs = true; @@ -8466,8 +8465,7 @@ handle_visibility_attribute (tree *node, tree name, tree args, bool c_determine_visibility (tree decl) { - gcc_assert (TREE_CODE (decl) == VAR_DECL - || TREE_CODE (decl) == FUNCTION_DECL); + gcc_assert (VAR_OR_FUNCTION_DECL_P (decl)); /* If the user explicitly specified the visibility with an attribute, honor that. DECL_VISIBILITY will have been set during @@ -9014,8 +9012,7 @@ handle_tm_wrap_attribute (tree *node, tree name, tree args, if (error_operand_p (wrap_decl)) ; else if (TREE_CODE (wrap_decl) != IDENTIFIER_NODE - TREE_CODE (wrap_decl) != VAR_DECL - TREE_CODE (wrap_decl) != FUNCTION_DECL) + !VAR_OR_FUNCTION_DECL_P (wrap_decl)) error (%qE argument not an identifier, name); else { @@ -9089,8 +9086,7 @@ handle_deprecated_attribute (tree *node, tree name, if (TREE_CODE (decl) == TYPE_DECL || TREE_CODE (decl) == PARM_DECL - || TREE_CODE (decl) == VAR_DECL - || TREE_CODE (decl) == FUNCTION_DECL + || VAR_OR_FUNCTION_DECL_P (decl) || TREE_CODE (decl) == FIELD_DECL || objc_method_decl (TREE_CODE (decl))) TREE_DEPRECATED (decl) = 1; diff --git gcc/c-family/c-pragma.c gcc/c-family/c-pragma.c index 6894f0e..b82ca9f 100644 --- gcc/c-family/c-pragma.c +++ gcc/c-family/c-pragma.c @@ -306,7 +306,7 @@ maybe_apply_pragma_weak (tree decl) /* If it's not a function or a variable, it can't be weak. FIXME: what kinds of things are visible outside this file but aren't functions or variables? Should this be an assert instead? */ - if (TREE_CODE (decl) != FUNCTION_DECL TREE_CODE (decl) != VAR_DECL) + if (!VAR_OR_FUNCTION_DECL_P (decl)) return; if (DECL_ASSEMBLER_NAME_SET_P (decl)) @@ -486,8 +486,7 @@ handle_pragma_redefine_extname (cpp_reader * ARG_UNUSED (dummy)) } if ((TREE_PUBLIC (decl) || DECL_EXTERNAL (decl)) - (TREE_CODE (decl) == FUNCTION_DECL - || TREE_CODE (decl) == VAR_DECL)) + VAR_OR_FUNCTION_DECL_P (decl)) { found = true; if (DECL_ASSEMBLER_NAME_SET_P (decl)) @@ -547,7 +546,7 @@ maybe_apply_renaming_pragma (tree decl, tree asmname) /* The renaming pragmas are only applied to declarations with external linkage. */ - if ((TREE_CODE (decl) != FUNCTION_DECL TREE_CODE (decl) != VAR_DECL) + if (!VAR_OR_FUNCTION_DECL_P (decl) || (!TREE_PUBLIC (decl) !DECL_EXTERNAL (decl)) || !has_c_linkage (decl)) return asmname; diff --git
Re: Demangle symbols in debug assertion messages
On 20/05/15 11:17 +0100, Jonathan Wakely wrote: On 04/05/15 22:31 +0200, François Dumont wrote: Hi Here is the patch to demangle symbols in debug messages. I have also simplify code in formatter.h. Here is an example of assertion message: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: error: function requires a valid iterator range [__first, __last). Objects involved in the operation: iterator __first @ 0x0x7fff165d68b0 { type = __gnu_debug::_Safe_iterator__gnu_cxx::__normal_iteratorint*, std::__cxx1998::vectorint, std::allocatorint , std::__debug::vectorint, std::allocatorint (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vectorint, std::allocatorint ' @ 0x0x7fff165d69d0 } iterator __last @ 0x0x7fff165d68e0 { type = __gnu_debug::_Safe_iterator__gnu_cxx::__normal_iteratorint*, std::__cxx1998::vectorint, std::allocatorint , std::__debug::vectorint, std::allocatorint (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vectorint, std::allocatorint ' @ 0x0x7fff165d69d0 } * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify usage of typeid. (_Error_formatter::_M_print_type): New. * src/c++11/debug.cc (_Error_formatter::_Parameter::_M_print_field): Use latter. (_Error_formatter::_M_print_type): Implement latter using __cxaabiv1::__cxa_demangle to print demangled type name. I just hope that __cxa_demangle is portable. It's provided by GCC itself so is always available in the runtime. (It is also portable, because it's defined by the Itanium C++ ABI). Ok to commit ? Yes, this is great, thanks! Does this fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65392 ?
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #15 from Douglas Mencken dougmencken at gmail dot com --- I'm going to surround calls to gcc_[checking_]assert (in gcc/hash-table.*) with #ifdef ENABLE_CHECKING {--disable-checking is in my config already}. Let's see where it lands.
[AArch64] Implement -fpic for -mcmodel=small
Currently, AArch64 don't differentiate -fpic and -fPIC. For -mcmodel=small, both allow 4G GOT table size, then we always need two instructions to address GOT entry. This patch implements -fpic for -mcmodel=small which allow 32K GOT table size, smaller than -fPIC, but then we can use one instruction to address GOT entry given pic_offset_table_rtx initialized properly. (As we are using page base, the first page may be wasted in the worsest scenario, then only 28K space for GOT.) the generate instruction sequence for accessing global variable is ldr reg, [pic_offset_table_rtx, #:gotpage_lo15:sym] or ldr reg, [pic_offset_table_rtx, #:gotpage_lo14:sym] for ILP32 Only one instruction needed. But we must initialize global pointer (pic_offset_table_rtx) properly. Currently, We initialize it for every global access, and let CSE to remove all redundant ones. The final instruction sequences will looks like the following for multiply global variables access. adrp pic_offset_table_rtx, _GLOBAL_OFFSET_TABLE_ ldr reg, [pic_offset_table_rtx, #:gotpage_lo15:sym1] ldr reg, [pic_offset_table_rtx, #:gotpage_lo15:sym2] ldr reg, [pic_offset_table_rtx, #:gotpage_lo15:sym3] ... instead of the the following less efficient -fPIC version: adrp rA, :got:sym1 ldr rA, [rA, #:got_lo12:sym1] adrp rB, :got:sym2 ldr rB, [rB, #:got_lo12:sym2] adrp rC, :got:sym3 ldr rC, [rC, #:got_lo12:sym3] ... AArch64 don't reserve any register as gp, we use pseudo pic reg, and let register allocator to use any one possible. Binutils correspondent test done = gcc bootstrap OK on aarch64 board with BOOT_CFLAGS=-O2 -fpic. built glibc under -fpic, code size slightly smaller. Ok for trunk? 2015-05-20 Jiong. Wang jiong.w...@arm.com gcc/ * config/aarch64/aarch64.md: (ldr_got_small_mode): Support new GOT relocation modifiers. (ldr_got_small_sidi): Ditto. * config/aarch64/iterators.md (got_modifier): New mode iterator. * config/aarch64/aarch64-otps.h (aarch64_code_model): New model. * config/aarch64/aarch64.c (aarch64_load_symref_appropriately): Support -fpic. (aarch64_rtx_costs): Add costs for new instruction sequences. (initialize_aarch64_code_model): Initialize new model. (aarch64_classify_symbol): Recognize new model. (aarch64_asm_preferred_eh_data_format): Support new model. (aarch64_load_symref_appropriately): Generate new instruction sequences for -fpic. (TARGET_USE_PSEUDO_PIC_REG): New definition. (aarch64_use_pseudo_pic_reg): New function. gcc/testsuite/ * gcc.target/aarch64/pic-small.c: New testcase. -- Regards, Jiong diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h index ea64cf4..49a990a 100644 --- a/gcc/config/aarch64/aarch64-opts.h +++ b/gcc/config/aarch64/aarch64-opts.h @@ -53,6 +53,9 @@ enum aarch64_code_model { /* Static code and data fit within a 4GB region. The default non-PIC code model. */ AARCH64_CMODEL_SMALL, + /* -fpic for small memory model. + GOT size to 28KiB (4K*8-4K) or 3580 entries. */ + AARCH64_CMODEL_SMALL_SPIC, /* Static code, data and GOT/PLT fit within a 4GB region. The default PIC code model. */ AARCH64_CMODEL_SMALL_PIC, diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 7a34e49..4b6e648 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -840,10 +840,55 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, rtx tmp_reg = dest; machine_mode mode = GET_MODE (dest); - if (can_create_pseudo_p ()) - tmp_reg = gen_reg_rtx (mode); + if (aarch64_cmodel != AARCH64_CMODEL_SMALL_SPIC) + { + if (can_create_pseudo_p ()) + tmp_reg = gen_reg_rtx (mode); + + emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm)); + } + /* NOTE: pic_offset_table_rtx can be NULL_RTX, because we can reach + here before rtl expand. Tree IVOPT will generate rtl pattern to + decide rtx costs, in which case pic_offset_table_rtx is not + initialized. For that case no need to generate the first adrp + instruction as the the final cost for global variable access is + one instruction. */ + else if (pic_offset_table_rtx != NULL_RTX) + { + /* -fpic for -mcmodel=small allow 32K GOT table size (but we are + using the page base as GOT base, the first page may be wasted, + in the worst scenario, there is only 28K space for GOT). + + The generate instruction sequence for accessing global variable + is: + + ldr reg, [pic_offset_table_rtx, #:gotpage_lo15:sym] + + Only one instruction needed. But we must initialize + pic_offset_table_rtx properly. We generate initialize insn for + every global access, and allow CSE to remove all redundant. + + The final instruction sequences will look like the following + for multiply global variables access. + + adrp pic_offset_table_rtx, _GLOBAL_OFFSET_TABLE_ + + ldr reg,
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On 20 May 2015 at 16:17, Prathamesh Kulkarni prathamesh.kulka...@linaro.org wrote: Hi, This patch rejects expanding operator-list to implicit 'for'. On second thoughts, should we reject expansion of operator-list _only_ if it's mixed with 'for' ? We could define multiple operator-lists in simplify to be the same as enclosing the simplify in 'for' with number of iterators equal to number of operator-lists. So we could allow (define_operator_list op1 ...) (define_operator_list op2 ...) (simplify (op1 (op2 ... ))) is equivalent to: (for temp1 (op1) temp2 (op2) (simplify (temp1 (temp2 ... I think we have patterns like these in match-builtin.pd in the match-and-simplify branch And reject mixing of 'for' and operator-lists. Admittedly the implicit 'for' behavior is not obvious from the syntax -;( Thanks, Prathamesh OK for trunk after bootstrap+testing ? Thanks, Prathamesh
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
Hi, On Tue, 19 May 2015, Richard Henderson wrote: It is. The relaxation that HJ is working on requires that the reads from the got not be hoisted. I'm not especially convinced that what he's working on is a win. With LTO, the compiler can do the same job that he's attempting in the linker, without an extra nop. Without LTO, leaving it to the linker means that you can't hoist the load and hide the memory latency. Well, hoisting always needs a register, and if hoisted out of a loop (which you all seem to be after) that register is live through the whole loop body. You need a register for each different called function in such loop, trading the one GOT pointer with N other registers. For register-starved machines this is a real problem, even x86-64 doesn't have that many. I.e. I'm not convinced that this hoisting will really be much of a win that often, outside toy examples. Sure, the compiler can hoist function addresses trivially, but I think it will lead to spilling more often than not, or alternatively the hoisting will be undone by the register allocators rematerialization. Of course, this would have to be measured for real not hand-waved, but, well, I'd be surprised if it's not so. Ciao, Michael.
Re: Demangle symbols in debug assertion messages
On 04/05/15 22:31 +0200, François Dumont wrote: Hi Here is the patch to demangle symbols in debug messages. I have also simplify code in formatter.h. Here is an example of assertion message: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/functions.h:213: error: function requires a valid iterator range [__first, __last). Objects involved in the operation: iterator __first @ 0x0x7fff165d68b0 { type = __gnu_debug::_Safe_iterator__gnu_cxx::__normal_iteratorint*, std::__cxx1998::vectorint, std::allocatorint , std::__debug::vectorint, std::allocatorint (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vectorint, std::allocatorint ' @ 0x0x7fff165d69d0 } iterator __last @ 0x0x7fff165d68e0 { type = __gnu_debug::_Safe_iterator__gnu_cxx::__normal_iteratorint*, std::__cxx1998::vectorint, std::allocatorint , std::__debug::vectorint, std::allocatorint (mutable iterator); state = dereferenceable; references sequence with type `std::__debug::vectorint, std::allocatorint ' @ 0x0x7fff165d69d0 } * include/debug/formatter.h (_GLIBCXX_TYPEID): New macro to simplify usage of typeid. (_Error_formatter::_M_print_type): New. * src/c++11/debug.cc (_Error_formatter::_Parameter::_M_print_field): Use latter. (_Error_formatter::_M_print_type): Implement latter using __cxaabiv1::__cxa_demangle to print demangled type name. I just hope that __cxa_demangle is portable. It's provided by GCC itself so is always available in the runtime. (It is also portable, because it's defined by the Itanium C++ ABI). Ok to commit ? Yes, this is great, thanks!
[match-and-simplify] reject expanding operator-list to implicit 'for'
Hi, This patch rejects expanding operator-list to implicit 'for'. OK for trunk after bootstrap+testing ? Thanks, Prathamesh 2015-05-20 Prathamesh Kulkarni prathamesh.kulka...@linaro.org * genmatch.c (parser::record_operlist): Remove. (parser::oper_lists_set): Likewise. (parser::oper_lists): Likewise. (parser::parse_operation): Reject operator-list and remove call to parser::record_operlist. (parser::parse_c_expr): Remove call to parser::record_operlist. (parser::push_simplify): Remove pushing and popping parser::oper_lists in parser::active_fors. (parser::parse_simplify): Avoid initializing parser::oper_lists and parser::oper_lists_set. (parser::parser): Likewise. Index: genmatch.c === --- genmatch.c (revision 223437) +++ genmatch.c (working copy) @@ -2714,7 +2714,6 @@ c_expr *parse_c_expr (cpp_ttype); operand *parse_op (); - void record_operlist (source_location, user_id *); void parse_pattern (); void push_simplify (vecsimplify *, operand *, source_location, @@ -2729,9 +2728,6 @@ cpp_reader *r; vecif_or_with active_ifs; vecvecuser_id * active_fors; - hash_setuser_id * *oper_lists_set; - vecuser_id * oper_lists; - cid_map_t *capture_ids; public: @@ -2860,22 +2856,6 @@ return (const char *)token-val.str.text; } - -/* Record an operator-list use for transparent for handling. */ - -void -parser::record_operlist (source_location loc, user_id *p) -{ - if (!oper_lists_set-add (p)) -{ - if (!oper_lists.is_empty () - oper_lists[0]-substitutes.length () != p-substitutes.length ()) - fatal_at (loc, User-defined operator list does not have the - same number of entries as others used in the pattern); - oper_lists.safe_push (p); -} -} - /* Parse the operator ID, special-casing convert?, convert1? and convert2? */ @@ -2913,7 +2893,7 @@ user_id *p = dyn_castuser_id * (op); if (p p-is_oper_list) -record_operlist (id_tok-src_loc, p); +fatal_at (id_tok, invalid use of operator-list %s, id); return op; } @@ -3051,11 +3031,8 @@ /* If this is possibly a user-defined identifier mark it used. */ if (token-type == CPP_NAME) { - id_base *idb = get_operator ((const char *)CPP_HASHNODE - (token-val.node.node)-ident.str); - user_id *p; - if (idb (p = dyn_castuser_id * (idb)) p-is_oper_list) - record_operlist (token-src_loc, p); + get_operator ((const char *)CPP_HASHNODE + (token-val.node.node)-ident.str); } /* Record the token. */ @@ -3140,16 +3117,9 @@ operand *match, source_location match_loc, operand *result, source_location result_loc) { - /* Build and push a temporary for for operator list uses in expressions. */ - if (!oper_lists.is_empty ()) -active_fors.safe_push (oper_lists); - simplifiers.safe_push (new simplify (match, match_loc, result, result_loc, active_ifs.copy (), active_fors.copy (), capture_ids)); - - if (!oper_lists.is_empty ()) -active_fors.pop (); } /* Parse @@ -3170,11 +3140,7 @@ /* Reset the capture map. */ if (!capture_ids) capture_ids = new cid_map_t; - /* Reset oper_lists and set. */ - hash_set user_id * olist; - oper_lists_set = olist; - oper_lists = vNULL; - + const cpp_token *loc = peek (); parsing_match_operand = true; struct operand *match = parse_op (); @@ -3563,8 +3529,6 @@ active_ifs = vNULL; active_fors = vNULL; simplifiers = vNULL; - oper_lists_set = NULL; - oper_lists = vNULL; capture_ids = NULL; user_predicates = vNULL; parsing_match_operand = false;
[Bug tree-optimization/65752] Too strong optimizations int - pointer casts
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 --- Comment #26 from Chung-Kil Hur gil.hur at sf dot snu.ac.kr --- Thanks for the detailed explanations. The C standard only guarantees that you can convert a pointer to uintptr_t and back, it doesn't guarantee that you can convert a modified uintptr_t back to a pointer that is valid. Thus, doing (int *)((xp + i) - j) is invoking undefined behavior. I didn't know about this rule. I thought this cast is valid because (xp+i)-j is even safely-derived. Could you give a reference for that rule in the standard? Thanks!
[Bug libgcc/66212] Exception handling broken on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66212 --- Comment #1 from Andri Yngvason andri.yngvason at marel dot com --- I've now compiled the same toolchain for i686 and I have the same issue there, so I assume that I'm doing something wrong. It's hard to pin down what I'm doing wrong though. Everything seems to be linked correctly: # ldd stdexcept linux-gate.so.1 (0xb77ad000) libstdc++.so.6 = /lib/libstdc++.so.6 (0xb7633000) libm.so.6 = /lib/libm.so.6 (0xb75ea000) libgcc_s.so.1 = /lib/glibc2.21/libgcc_s.so.1 (0xb75ce000) libc.so.6 = /lib/libc.so.6 (0xb742) /lib/ld-linux.so.2 (0xb77ae000) # ls -l /lib/libstdc++.so.6 /lib/libm.so.6 /lib/libc.so.6 /lib/glibc2.21/libgcc_s.so.1 /lib/ld-linux.so.2 -rw-r--r--1 root root397872 May 20 10:11 /lib/glibc2.21/libgcc_s.so.1 lrwxrwxrwx1 root root10 May 20 10:57 /lib/ld-linux.so.2 - ld-2.21.so lrwxrwxrwx1 root root12 May 20 10:57 /lib/libc.so.6 - libc-2.21.so lrwxrwxrwx1 root root12 May 20 10:57 /lib/libm.so.6 - libm-2.21.so lrwxrwxrwx1 root root19 May 20 10:57 /lib/libstdc++.so.6 - libstdc++.so.6.0.21 I'll try 4.9...
[AArch64][TLSLE][2/N] Rename tlsle_small to tlsle
Similar to the rename from SYMBOL_SMALL_TPREL to SYMBOL_TLSLE, this patch rename the rtl pattern name. ok for trunk? 2015-05-19 Jiong Wang jiong.w...@arm.com gcc/ * config/aarch64/aarch64.md (tlsle_small): Rename to tlsle. (tlsle_small_mode): Rename to tlsle_mode. * config/aarc64/aarch64.c (aarch64_load_symref_appropriately): Use new pattern name. -- Regards, Jiong commit 271f54f9660e9518e294bfda9eb108b53eaab9d4 Author: Jiong Wang jiong.w...@arm.com Date: Fri May 15 09:48:12 2015 +0100 Rename insn diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 99a534c..55b166c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -985,7 +985,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, if (GET_MODE (dest) != Pmode) tp = gen_lowpart (GET_MODE (dest), tp); - emit_insn (gen_tlsle_small (dest, tp, imm)); + emit_insn (gen_tlsle (dest, tp, imm)); set_unique_reg_note (get_last_insn (), REG_EQUIV, imm); return; } diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index c55d70b..44bcc5c 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -4295,7 +4295,7 @@ (set_attr length 8)] ) -(define_expand tlsle_small +(define_expand tlsle [(set (match_operand 0 register_operand =r) (unspec [(match_operand 1 register_operand r) (match_operand 2 aarch64_tls_le_symref S)] @@ -4304,14 +4304,12 @@ { machine_mode mode = GET_MODE (operands[0]); emit_insn ((mode == DImode - ? gen_tlsle_small_di - : gen_tlsle_small_si) (operands[0], - operands[1], - operands[2])); + ? gen_tlsle_di + : gen_tlsle_si) (operands[0], operands[1], operands[2])); DONE; }) -(define_insn tlsle_small_mode +(define_insn tlsle_mode [(set (match_operand:P 0 register_operand =r) (unspec:P [(match_operand:P 1 register_operand r) (match_operand 2 aarch64_tls_le_symref S)]
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On Wed, 20 May 2015, Prathamesh Kulkarni wrote: Hi, This patch rejects expanding operator-list to implicit 'for'. OK for trunk after bootstrap+testing ? Ok. Thanks, Richard.
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #17 from rguenther at suse dot de rguenther at suse dot de --- On Wed, 20 May 2015, dougmencken at gmail dot com wrote: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #14 from Douglas Mencken dougmencken at gmail dot com --- sizeof(hashval_t) = 4, CHAR_BIT = 8 Just checked it manually. Built with patch subset, genmatch problem is here again. It isn't related to changes in hash_table_mod1 hash_table_mod2. What's left? abort() replaced by gcc_checking_assert()? Well, it shouldn't segfault in this case either (well - maybe we replace gcc_checking_assert () with gcc_unreachable () with --disable-checking). Indeed we do. That would explain seeing a segfault instead of an assertion failure (so yes, try without --disable-checking)
[Bug target/66215] New: [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Bug ID: 66215 Summary: [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch Product: gcc Version: 6.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: target Assignee: unassigned at gcc dot gnu.org Reporter: marxin at gcc dot gnu.org Target Milestone: --- Target: 390-linux Hi. Starting from r221381 GCC does not place nops right after function label. Unfortunately, the problematic patch was also backported to gcc4-[89] and gcc-5. $ cat /tmp/s390.c static int foo() { return 0; } int main(int argc, char **argv) { return foo(); } $ ./xgcc -B. /tmp/s390.c -mhotpatch=2,3 -o o0.s -S -fno-inline $ cat o0.s | head .. .text .align 8 .type foo, @function nopr%r7 # pre-label NOPs for hotpatch (2 halfwords) nopr%r7 # alignment for hotpatch .align 8 foo: # post-label NOPs for hotpatch (3 halfwords) .LFB0: stm %r11,%r14,44(%r15) .LCFI0: lr %r11,%r15 .LCFI1: nop 0 nopr%r7 lhi %r1,0 lr %r2,%r1 l %r4,56(%r11) lm %r11,%r14,44(%r11) .LCFI2: br %r4 Problem is that patched compiler relies that NOTE_INSN_FUNCTION_BEG is at the beginning of function. /* Inject nops for hotpatching. */ + for (insn = get_insns (); insn; insn = NEXT_INSN (insn)) + { + if (NOTE_P (insn) NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG) + break; + } Problem is that if you use -O2, 256r.sched2 reorders insns after pro_and_epilogue, where NOTE_INSN_FUNCTION_BEG is placed at the beginning. On the other hand, if you try -O0, -O1, as the pass is not executed, emission of NOTE_INSN_FUNCTION_BEG is not reordered. $ cat s390.c.242r.pro_and_epilogue foo (note 1 0 5 NOTE_INSN_DELETED) (note 5 1 18 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (insn/f 18 5 19 2 (set (mem:SI (plus:SI (reg/f:SI 15 %r15) (const_int 56 [0x38])) [1 S4 A8]) (reg:SI 14 %r14)) /tmp/s390.c:7 -1 (nil)) (insn 19 18 20 2 (set (reg:SI 5 %r5) (unspec_volatile [ (const_int 0 [0]) ] UNSPECV_MAIN_POOL)) /tmp/s390.c:7 -1 (nil)) (note 20 19 17 2 NOTE_INSN_PROLOGUE_END) (insn 17 20 4 2 (set (reg:SI 5 %r5) (unspec_volatile [ (const_int 0 [0]) ] UNSPECV_MAIN_POOL)) 675 {main_pool} (nil)) (note 4 17 8 2 NOTE_INSN_FUNCTION_BEG) (insn 8 4 24 2 (set (reg:SI 1 %r1) (mem/u/c:SI (unspec:SI [ (symbol_ref/u:SI (*.LC0) [flags 0x2]) (reg:SI 5 %r5) ] UNSPEC_LTREF) [2 S4 A32])) /tmp/s390.c:8 68 {*movsi_esa} (expr_list:REG_EQUAL (symbol_ref:SI (foo) [flags 0x3] function_decl 0x7fc55997e3e0 foo) (nil))) (note 24 8 23 2 NOTE_INSN_EPILOGUE_BEG) (insn/f 23 24 9 2 (set (reg:SI 14 %r14) (mem:SI (plus:SI (reg/f:SI 15 %r15) (const_int 56 [0x38])) [1 S4 A8])) /tmp/s390.c:9 -1 (expr_list:REG_CFA_DEF_CFA (plus:SI (reg/f:SI 15 %r15) (const_int 96 [0x60])) (expr_list:REG_CFA_RESTORE (reg:SI 14 %r14) (nil (call_insn/u/j 9 23 10 2 (set (reg:SI 2 %r2) (call (mem:QI (reg:SI 1 %r1) [0 foo S1 A8]) (const_int 0 [0]))) /tmp/s390.c:8 631 {*sibcall_value_br} (expr_list:REG_CALL_DECL (symbol_ref:SI (foo) [flags 0x3] function_decl 0x7fc55997e3e0 foo) (expr_list:REG_EH_REGION (const_int 0 [0]) (nil))) (nil)) (barrier 10 9 16) (note 16 10 0 NOTE_INSN_DELETED) $ cat s390.c.256r.sched2 (note 1 0 3 NOTE_INSN_DELETED) (note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (note 2 3 15 2 NOTE_INSN_FUNCTION_BEG) (insn/f 15 2 16 2 (set (mem:SI (plus:SI (reg/f:SI 15 %r15) (const_int 56 [0x38])) [1 S4 A8]) (reg:SI 14 %r14)) /tmp/s390.c:2 68 {*movsi_esa} (expr_list:REG_DEAD (reg:SI 14 %r14) (nil))) (insn 16 15 17 2 (set (reg:SI 5 %r5) (unspec_volatile [ (const_int 0 [0]) ] UNSPECV_MAIN_POOL)) /tmp/s390.c:2 675 {main_pool} (expr_list:REG_UNUSED (reg:SI 5 %r5) (nil))) (note 17 16 14 2 NOTE_INSN_PROLOGUE_END) (insn 14 17 25 2 (set (reg:SI 5 %r5) (unspec_volatile [ (const_int 0 [0]) ] UNSPECV_MAIN_POOL)) 675 {main_pool} (expr_list:REG_UNUSED (reg:SI 5 %r5) (nil))) (note 25 14 21 2 NOTE_INSN_EPILOGUE_BEG) (insn 21 25 9 2 (set (reg:SI 4 %r4) (mem:SI (plus:SI (reg/f:SI 15 %r15) (const_int 56 [0x38])) [1 S4 A8])) /tmp/s390.c:4 68 {*movsi_esa} (nil)) (insn 9 21 22 2 (set (reg/i:SI 2 %r2) (const_int 0 [0])) /tmp/s390.c:4 68 {*movsi_esa} (nil)) (insn/f 22 9 10 2 (set (reg:SI 14 %r14) (mem:SI (plus:SI (reg/f:SI 15 %r15) (const_int 56
[Bug c/66213] New: unsigned char value range can be greater than sizeof unsigned char
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66213 Bug ID: 66213 Summary: unsigned char value range can be greater than sizeof unsigned char Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: z.hege...@t-systems.com Target Milestone: --- Unsigned char can be greater than 1 byte (== sizeof(unsigned char)) Example: #include stdio.h int main() { unsigned char a=200, b=80; int z = a+b; printf(z=: %d\n, z); return 0; } When data type char or signed char is used in the example above a char overflow occurs (as expected) but if unsigned char is used the return value can be greater than 1 byte (or sizeof(unsigned char)) Reason: Breakpoint 8, 0x004011f6 in main () at char.c:8 (gdb) i r eax0x50 80 edx0xc8 200 Breakpoint 10, 0x00401215 in main () at char.c:10 0x0040121a 10 z = c+d; (gdb) ni (gdb) i r eax0x50 80 edx0xffc8 -56 Where c and d are signed chars. When unsigned char is used gcc uses movzx instead of movsx and probably the sign bit is overwritten.
RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
gcc/ * config/mips/mips.h (micromips_globals): Declare. OK, thanks. Matthew Committed as r223438. Robert
[committed] Use *NARY_CLASS_P more
No functional changes. Bootstrapped/regtested on x86_64-linux, applying to trunk. 2015-05-20 Marek Polacek pola...@redhat.com * cfgexpand.c (expand_debug_expr): Use UNARY_CLASS_P. * c-omp.c (check_omp_for_incr_expr): Use BINARY_CLASS_P. diff --git gcc/c-family/c-omp.c gcc/c-family/c-omp.c index 86a9f54..168cae9 100644 --- gcc/c-family/c-omp.c +++ gcc/c-family/c-omp.c @@ -395,7 +395,7 @@ check_omp_for_incr_expr (location_t loc, tree exp, tree decl) { tree op1 = TREE_OPERAND (exp, 1); tree temp = TARGET_EXPR_SLOT (op0); - if (TREE_CODE_CLASS (TREE_CODE (op1)) == tcc_binary + if (BINARY_CLASS_P (op1) TREE_OPERAND (op1, 1) == temp) { op1 = copy_node (op1); diff --git gcc/cfgexpand.c gcc/cfgexpand.c index 09e668a..f65e1fc 100644 --- gcc/cfgexpand.c +++ gcc/cfgexpand.c @@ -4039,7 +4039,7 @@ expand_debug_expr (tree exp) op0 = simplify_gen_subreg (mode, op0, inner_mode, subreg_lowpart_offset (mode, inner_mode)); - else if (TREE_CODE_CLASS (TREE_CODE (exp)) == tcc_unary + else if (UNARY_CLASS_P (exp) ? TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (exp, 0))) : unsignedp) op0 = simplify_gen_unary (ZERO_EXTEND, mode, op0, inner_mode); Marek
[Bug middle-end/66214] [6 Regression] ICE verify_type failed with -O0 -g via gen_type_die_with_usage's dwarf2out.c:20250
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66214 Tobias Burnus burnus at gcc dot gnu.org changed: What|Removed |Added Known to work||5.1.0 Target Milestone|--- |6.0 Known to fail||6.0 --- Comment #1 from Tobias Burnus burnus at gcc dot gnu.org --- (In reply to Tobias Burnus from comment #0) The bug reminds me of bug 66103, but is one is without LTO.
[Bug tree-optimization/65752] Too strong optimizations int - pointer casts
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 --- Comment #27 from Chung-Kil Hur gil.hur at sf dot snu.ac.kr --- (In reply to Chung-Kil Hur from comment #26) Thanks for the detailed explanations. The C standard only guarantees that you can convert a pointer to uintptr_t and back, it doesn't guarantee that you can convert a modified uintptr_t back to a pointer that is valid. Thus, doing (int *)((xp + i) - j) is invoking undefined behavior. I didn't know about this rule. I thought this cast is valid because (xp+i)-j is even safely-derived. Could you give a reference for that rule in the standard? Thanks! It seems that the following rule might be the one. = 7.20.1.4 Integer types capable of holding object pointers The following type designates a signed integer type with the property that any valid pointer to void can be converted to this type, then converted back to pointer to void, and the result will compare equal to the original pointer: intptr_t The following type designates an unsigned integer type with the property that any valid pointer to void can be converted to this type, then converted back to pointer to void, and the result will compare equal to the original pointer: uintptr_t These types are optional. = Right. This does not say that we are allowed to cast a modified integer back to a pointer. What I remember might be from the C++ standard, where safely derived integers are allowed to be cast back to pointers. Umm. This might also be implementation-defined. Anyway, thanks very much for taking your time to respond to my questions!! Best, Gil
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On Wed, 20 May 2015, Prathamesh Kulkarni wrote: On 20 May 2015 at 16:17, Prathamesh Kulkarni prathamesh.kulka...@linaro.org wrote: Hi, This patch rejects expanding operator-list to implicit 'for'. On second thoughts, should we reject expansion of operator-list _only_ if it's mixed with 'for' ? At least that, yes. We could define multiple operator-lists in simplify to be the same as enclosing the simplify in 'for' with number of iterators equal to number of operator-lists. So we could allow (define_operator_list op1 ...) (define_operator_list op2 ...) (simplify (op1 (op2 ... ))) is equivalent to: (for temp1 (op1) temp2 (op2) (simplify (temp1 (temp2 ... I think we have patterns like these in match-builtin.pd in the match-and-simplify branch And reject mixing of 'for' and operator-lists. Admittedly the implicit 'for' behavior is not obvious from the syntax -;( Hmm, indeed we have for example /* Optimize pow(1.0,y) = 1.0. */ (simplify (POW real_onep@0 @1) @0) and I remember wanting that implicit for to make those less ugly. So can you rework only rejecting it within for? Thanks, Richard. Thanks, Prathamesh OK for trunk after bootstrap+testing ? Thanks, Prathamesh -- Richard Biener rguent...@suse.de SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
[Bug libstdc++/60936] [4.9/5/6 Regression] Binary code bloat with std::string
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60936 --- Comment #13 from Jonathan Wakely redi at gcc dot gnu.org --- Created attachment 35575 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35575action=edit Lightweight __throw_out_of_range_fmt for non-verbose builds This is what I had in mind.
[AArch64][TLSLE][3/N] Add UNSPEC_TLSLE
Add new unspec name UNSPEC_TLSLE, use it for all tlsle pattern. ok for trunk? 2015-05-19 Jiong Wang jiong.w...@arm.com gcc/ * config/aarch64/aarch64.md (UNSPEC_TLSLE): New enumeration. (tlsle): Use new unspec name. (tlsle_mode): Ditto. -- Regards, Jiong diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 44bcc5c..b1425a3 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -116,6 +116,7 @@ UNSPEC_ST4_LANE UNSPEC_TLS UNSPEC_TLSDESC +UNSPEC_TLSLE UNSPEC_USHL_2S UNSPEC_VSTRUCTDUMMY UNSPEC_SP_SET @@ -4299,7 +4300,7 @@ [(set (match_operand 0 register_operand =r) (unspec [(match_operand 1 register_operand r) (match_operand 2 aarch64_tls_le_symref S)] - UNSPEC_GOTSMALLTLS))] + UNSPEC_TLSLE))] { machine_mode mode = GET_MODE (operands[0]); @@ -4313,7 +4314,7 @@ [(set (match_operand:P 0 register_operand =r) (unspec:P [(match_operand:P 1 register_operand r) (match_operand 2 aarch64_tls_le_symref S)] - UNSPEC_GOTSMALLTLS))] + UNSPEC_TLSLE))] add\\t%w0, %w1, #%G2, lsl #12\;add\\t%w0, %w0, #%L2 [(set_attr type alu_sreg)
[PATCH][AArch64][obvious] In aarch64_class_max_nregs use UNITS_PER_VREG and UNITS_PER_WORD
Hi all, This patch replaces 15, 16, 7 and 8 in aarch64_class_max_nregs with the macro that they represent. This should make the logic of that function easier to understand. Bootstrapped and tested on aarch64. Applying as obvious. 2015-05-20 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/aarch64/aarch64.c (aarch64_class_max_nregs): Use UNITS_PER_VREG and UNITS_PER_WORD instead of their direct values. commit 8abd208611b50e8f477b6efb8d8604b3390a9072 Author: Kyrylo Tkachov kyrylo.tkac...@arm.com Date: Mon May 18 12:01:24 2015 +0100 [AArch64] In aarch64_class_max_nregs use UNITS_PER_VREG and UNITS_PER_WORD diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index c939a4a..5f23359 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4923,8 +4923,9 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode) case FP_REGS: case FP_LO_REGS: return - aarch64_vector_mode_p (mode) ? (GET_MODE_SIZE (mode) + 15) / 16 : - (GET_MODE_SIZE (mode) + 7) / 8; + aarch64_vector_mode_p (mode) + ? (GET_MODE_SIZE (mode) + UNITS_PER_VREG - 1) / UNITS_PER_VREG + : (GET_MODE_SIZE (mode) + UNITS_PER_WORD - 1) / UNITS_PER_WORD; case STACK_REG: return 1;
Re: Compilers and RCU readers: Once more unto the breach!
Hi Paul, On Wed, May 20, 2015 at 03:41:48AM +0100, Paul E. McKenney wrote: On Tue, May 19, 2015 at 07:10:12PM -0700, Linus Torvalds wrote: On Tue, May 19, 2015 at 6:57 PM, Linus Torvalds torva...@linux-foundation.org wrote: So I think you're better off just saying that operations designed to drop significant bits break the dependency chain, and give things like 1 and (char *)ptr-(uintptr_t)ptr as examples of such. Making that just an extension of your existing 0 language would seem to be natural. Works for me! I added the following bullet to the list of things that break dependencies: If a pointer is part of a dependency chain, and if the values added to or subtracted from that pointer cancel the pointer value so as to allow the compiler to precisely determine the resulting value, then the resulting value will not be part of any dependency chain. For example, if p is part of a dependency chain, then ((char *)p-(uintptr_t)p)+65536 will not be. Seem reasonable? Whilst I understand what you're saying (the ARM architecture makes these sorts of distinctions when calling out dependency-based ordering), it feels like we're dangerously close to defining the difference between a true and a false dependency. If we want to do this in the context of the C language specification, you run into issues because you need to evaluate the program in order to determine data values in order to determine the nature of the dependency. You tackle this above by saying to allow the compiler to precisely determine the resulting value, but I can't see how that can be cleanly fitted into something like the C language specification. Even if it can, then we'd need to reword the ?: treatment that you currently have: If a pointer is part of a dependency chain, and that pointer appears in the entry of a ?: expression selected by the condition, then the chain extends to the result. which I think requires the state of the condition to be known statically if we only want to extend the chain from the selected expression. In the general case, wouldn't a compiler have to assume that the chain is extended from both? Additionally, what about the following code? char *x = y ? z : z; Does that extend a dependency chain from z to x? If so, I can imagine a CPU breaking that in practice. Humans will understand, and compiler writers won't care. They will either depend on hardware semantics anyway (and argue that your language is tight enough that they don't need to do anything special) or they will turn the consume into an acquire (on platforms that have too weak hardware). Agreed. Plus Core Working Group will hammer out the exact wording, should this approach meet their approval. For the avoidance of doubt, I'm completely behind any attempts to tackle this problem, but I anticipate an uphill struggle getting this text into the C standard. Is your intention to change the carries-a-dependency relation to encompass this change? Cheers, Will
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On 20 May 2015 at 17:01, Richard Biener rguent...@suse.de wrote: On Wed, 20 May 2015, Prathamesh Kulkarni wrote: On 20 May 2015 at 16:17, Prathamesh Kulkarni prathamesh.kulka...@linaro.org wrote: Hi, This patch rejects expanding operator-list to implicit 'for'. On second thoughts, should we reject expansion of operator-list _only_ if it's mixed with 'for' ? At least that, yes. We could define multiple operator-lists in simplify to be the same as enclosing the simplify in 'for' with number of iterators equal to number of operator-lists. So we could allow (define_operator_list op1 ...) (define_operator_list op2 ...) (simplify (op1 (op2 ... ))) is equivalent to: (for temp1 (op1) temp2 (op2) (simplify (temp1 (temp2 ... I think we have patterns like these in match-builtin.pd in the match-and-simplify branch And reject mixing of 'for' and operator-lists. Admittedly the implicit 'for' behavior is not obvious from the syntax -;( Hmm, indeed we have for example /* Optimize pow(1.0,y) = 1.0. */ (simplify (POW real_onep@0 @1) @0) and I remember wanting that implicit for to make those less ugly. So can you rework only rejecting it within for? This patch rejects expanding operator-list inside 'for'. OK for trunk after bootstrap+testing ? Thanks, Prathamesh Thanks, Richard. Thanks, Prathamesh OK for trunk after bootstrap+testing ? Thanks, Prathamesh -- Richard Biener rguent...@suse.de SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg) 2015-05-20 Prathamesh Kulkarni prathamesh.kulka...@linaro.org * genmatch.c (parser::parse_operation): Reject expanding operator-list inside 'for'. Index: genmatch.c === --- genmatch.c (revision 223437) +++ genmatch.c (working copy) @@ -2913,7 +2913,10 @@ user_id *p = dyn_castuser_id * (op); if (p p-is_oper_list) -record_operlist (id_tok-src_loc, p); +if (active_fors.length() == 0) + record_operlist (id_tok-src_loc, p); +else + fatal_at (id_tok, operator-list %s cannot be exapnded inside 'for', id); return op; }
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #14 from Douglas Mencken dougmencken at gmail dot com --- sizeof(hashval_t) = 4, CHAR_BIT = 8 Just checked it manually. Built with patch subset, genmatch problem is here again. It isn't related to changes in hash_table_mod1 hash_table_mod2. What's left? abort() replaced by gcc_checking_assert()?
[Bug libgcc/66212] New: Exception handling broken on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66212 Bug ID: 66212 Summary: Exception handling broken on powerpc Product: gcc Version: 5.1.0 Status: UNCONFIRMED Severity: major Priority: P3 Component: libgcc Assignee: unassigned at gcc dot gnu.org Reporter: andri.yngvason at marel dot com Target Milestone: --- All exceptions cause the running process to be aborted. The following program is aborted when trying to unwind the stack: #include exception #include stdexcept #include iostream using std::cout; using std::endl; void foobar() { cout ... endl; throw std::runtime_error(Whoohoo); cout Wtf? endl; } int main(int, char**) { cout Throwing standard exception... endl; try { foobar(); } catch(std::exception e) { cout Caught: e.what() endl; } cout Done! endl; return 0; } Backtrace: #0 0x0fbff76c in raise () from /lib/libc.so.6 #1 0x0fc010cc in abort () from /lib/libc.so.6 #2 0x0fd5fc1c in uw_init_context_1 () from /lib/glibc2.21/libgcc_s.so.1 #3 0x0fd60408 in _Unwind_RaiseException () from /lib/glibc2.21/libgcc_s.so.1 #4 0x0fed10ac in __cxa_throw () at ../../../../gcc-5.1.0/libstdc++-v3/libsupc++/eh_throw.cc:82 #5 0x1c28 in foobar() () #6 0x1cac in main () $ powerpc-marel-linux-gnu-g++ -v Using built-in specs. COLLECT_GCC=powerpc-marel-linux-gnu-g++ COLLECT_LTO_WRAPPER=/opt/plutotoolchain/libexec/gcc/powerpc-marel-linux-gnu/5.1.0/lto-wrapper Target: powerpc-marel-linux-gnu Configured with: ../gcc-5.1.0/configure --prefix=/opt/plutotoolchain --target=powerpc-marel-linux-gnu --enable-languages=c,c++ --enable-threads=posix --enable-shared --disable-multilib --enable-__cxa_atexit --disable-sjlj-exceptions --disable-nls --enable-symvers=gnu --enable-c99 --enable-long-long --enable-profile --with-tune=e300c3 --disable-altivec Thread model: posix gcc version 5.1.0 (GCC) I tried to compile libgcc with debug symbols so that I could see which assertion fails, but my attempts had no effect. I'd be happy to learn how to get those debug symbols in there.
[Bug bootstrap/66038] [5 regression] (stage 2) build/genmatch segfaults in operand::gen_transform (gcc/hash-table.h:223)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66038 --- Comment #16 from Andreas Schwab sch...@linux-m68k.org --- After, = 32 triggers assert (-- failure). This is backwards. The failure case is sizeof (hashval_t) * CHAR_BIT 32.
[Bug c/66213] unsigned char value range can be greater than sizeof unsigned char
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66213 Marek Polacek mpolacek at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED CC||mpolacek at gcc dot gnu.org Resolution|--- |INVALID --- Comment #1 from Marek Polacek mpolacek at gcc dot gnu.org --- I don't know where you think is a bug. The usual arithmetic conversion are performed on that addition so it is done on ints.
[committed] Use DECL_P more
Use DECL_P where appropriate. No functional changes. Bootstrapped/regtested on x86_64-linux, applying to trunk. 2015-05-20 Marek Polacek pola...@redhat.com * gimple-fold.c (fold_const_aggregate_ref_1): Use DECL_P. * gimplify.c (gimplify_modify_expr_rhs): Likewise. * c-ada-spec.c (dump_sloc): Use DECL_P. diff --git gcc/c-family/c-ada-spec.c gcc/c-family/c-ada-spec.c index 8d6e014..b4e159e 100644 --- gcc/c-family/c-ada-spec.c +++ gcc/c-family/c-ada-spec.c @@ -1629,7 +1629,7 @@ dump_sloc (pretty_printer *buffer, tree node) xloc.file = NULL; - if (TREE_CODE_CLASS (TREE_CODE (node)) == tcc_declaration) + if (DECL_P (node)) xloc = expand_location (DECL_SOURCE_LOCATION (node)); else if (EXPR_HAS_LOCATION (node)) xloc = expand_location (EXPR_LOCATION (node)); diff --git gcc/gimple-fold.c gcc/gimple-fold.c index 2cc5628..4bef350 100644 --- gcc/gimple-fold.c +++ gcc/gimple-fold.c @@ -5518,7 +5518,7 @@ fold_const_aggregate_ref_1 (tree t, tree (*valueize) (tree)) if (TREE_THIS_VOLATILE (t)) return NULL_TREE; - if (TREE_CODE_CLASS (TREE_CODE (t)) == tcc_declaration) + if (DECL_P (t)) return get_symbol_constant_value (t); tem = fold_read_from_constant_string (t); diff --git gcc/gimplify.c gcc/gimplify.c index c5eccf0..2720d02 100644 --- gcc/gimplify.c +++ gcc/gimplify.c @@ -4222,7 +4222,7 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p, { if (TREE_THIS_VOLATILE (t) != volatile_p) { - if (TREE_CODE_CLASS (TREE_CODE (t)) == tcc_declaration) + if (DECL_P (t)) t = build_simple_mem_ref_loc (EXPR_LOCATION (*from_p), build_fold_addr_expr (t)); if (REFERENCE_CLASS_P (t)) Marek
[committed] Use COMPARISON_CLASS_P more
Use COMPARISON_CLASS_P where appropriate. No functional changes. Bootstrapped/regtested on x86_64-linux, applying to trunk. 2015-05-20 Marek Polacek pola...@redhat.com * expr.c (expand_cond_expr_using_cmove): Use COMPARISON_CLASS_P. * gimple-expr.c (gimple_cond_get_ops_from_tree): Likewise. * gimple-fold.c (canonicalize_bool): Likewise. (same_bool_result_p): Likewise. * tree-if-conv.c (parse_predicate): Likewise. diff --git gcc/expr.c gcc/expr.c index e91383f..cf33808 100644 --- gcc/expr.c +++ gcc/expr.c @@ -8073,7 +8073,7 @@ expand_cond_expr_using_cmove (tree treeop0 ATTRIBUTE_UNUSED, unsignedp = TYPE_UNSIGNED (type); comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp); } - else if (TREE_CODE_CLASS (TREE_CODE (treeop0)) == tcc_comparison) + else if (COMPARISON_CLASS_P (treeop0)) { tree type = TREE_TYPE (TREE_OPERAND (treeop0, 0)); enum tree_code cmpcode = TREE_CODE (treeop0); diff --git gcc/gimple-expr.c gcc/gimple-expr.c index efc93b7..4d683d6 100644 --- gcc/gimple-expr.c +++ gcc/gimple-expr.c @@ -607,7 +607,7 @@ void gimple_cond_get_ops_from_tree (tree cond, enum tree_code *code_p, tree *lhs_p, tree *rhs_p) { - gcc_assert (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison + gcc_assert (COMPARISON_CLASS_P (cond) || TREE_CODE (cond) == TRUTH_NOT_EXPR || is_gimple_min_invariant (cond) || SSA_VAR_P (cond)); diff --git gcc/gimple-fold.c gcc/gimple-fold.c index 2cc5628..01a85e9 100644 --- gcc/gimple-fold.c +++ gcc/gimple-fold.c @@ -3846,7 +3846,7 @@ canonicalize_bool (tree expr, bool invert) else if (TREE_CODE (expr) == SSA_NAME) return fold_build2 (EQ_EXPR, boolean_type_node, expr, build_int_cst (TREE_TYPE (expr), 0)); - else if (TREE_CODE_CLASS (TREE_CODE (expr)) == tcc_comparison) + else if (COMPARISON_CLASS_P (expr)) return fold_build2 (invert_tree_comparison (TREE_CODE (expr), false), boolean_type_node, TREE_OPERAND (expr, 0), @@ -3865,7 +3865,7 @@ canonicalize_bool (tree expr, bool invert) else if (TREE_CODE (expr) == SSA_NAME) return fold_build2 (NE_EXPR, boolean_type_node, expr, build_int_cst (TREE_TYPE (expr), 0)); - else if (TREE_CODE_CLASS (TREE_CODE (expr)) == tcc_comparison) + else if (COMPARISON_CLASS_P (expr)) return fold_build2 (TREE_CODE (expr), boolean_type_node, TREE_OPERAND (expr, 0), @@ -3946,12 +3946,12 @@ same_bool_result_p (const_tree op1, const_tree op2) /* Check the cases where at least one of the operands is a comparison. These are a bit smarter than operand_equal_p in that they apply some identifies on SSA_NAMEs. */ - if (TREE_CODE_CLASS (TREE_CODE (op2)) == tcc_comparison + if (COMPARISON_CLASS_P (op2) same_bool_comparison_p (op1, TREE_CODE (op2), TREE_OPERAND (op2, 0), TREE_OPERAND (op2, 1))) return true; - if (TREE_CODE_CLASS (TREE_CODE (op1)) == tcc_comparison + if (COMPARISON_CLASS_P (op1) same_bool_comparison_p (op2, TREE_CODE (op1), TREE_OPERAND (op1, 0), TREE_OPERAND (op1, 1))) diff --git gcc/tree-if-conv.c gcc/tree-if-conv.c index 49ff458..a85c7a2 100644 --- gcc/tree-if-conv.c +++ gcc/tree-if-conv.c @@ -339,7 +339,7 @@ parse_predicate (tree cond, tree *op0, tree *op1) return ERROR_MARK; } - if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) + if (COMPARISON_CLASS_P (cond)) { *op0 = TREE_OPERAND (cond, 0); *op1 = TREE_OPERAND (cond, 1); Marek
[Bug target/65979] Multiple issues in conftest.c prevent build on sh4-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #17 from John Paul Adrian Glaubitz glaubitz at physik dot fu-berlin.de --- Thanks a lot guys for working on this! I'm really glad you're doing this :).
[Bug middle-end/66214] New: [6 Regression] ICE verify_type failed with -O0 -g via gen_type_die_with_usage's dwarf2out.c:20250
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66214 Bug ID: 66214 Summary: [6 Regression] ICE verify_type failed with -O0 -g via gen_type_die_with_usage's dwarf2out.c:20250 Product: gcc Version: 6.0 Status: UNCONFIRMED Keywords: ice-on-valid-code Severity: normal Priority: P3 Component: middle-end Assignee: unassigned at gcc dot gnu.org Reporter: burnus at gcc dot gnu.org CC: hubicka at gcc dot gnu.org Target Milestone: --- Created attachment 35574 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35574action=edit Test case (input13.ii): Compile with g++ -std=c++11 -g The bug reminds me of 66103, but is one is without LTO. Compiling the attached code snipped with g++ -std=c++11 -g give the following ICE. Without -g, it doesn't crash: input13.ii:53:34: required from here input13.ii:44:74: error: TYPE_CANONICAL is not compatible template typename _Tp, typename _Alloc = std::allocator _Tp class vector:protected _Vector_base _Tp, [...] input13.ii:44:74: internal compiler error: verify_type failed 0xf3e280 verify_type(tree_node const*) ../../gcc/tree.c:13257 0x96dc04 gen_type_die_with_usage ../../gcc/dwarf2out.c:20250 0x96e2a3 gen_type_die_with_usage ../../gcc/dwarf2out.c:20337 0x97f21c gen_type_die ../../gcc/dwarf2out.c:20434 0x97f21c gen_formal_types_die ../../gcc/dwarf2out.c:18027
[AArch64][TLSLE][1/N] Rename SYMBOL_SMALL_TPREL to SYMBOL_TLSLE
For AArch64, TLS local-exec mode for all memory model (tiny/small/large) is actually the same. TLS LE Instruction generation depends on how big tls section is instead of the memory model used. The four instruction sequences we can implement based on relocations provided: sequence 1 == add t0, tp, #:tprel_lo12:x1 R_AARCH64_TLSLE_ADD_TPREL_LO12 x1 sequence 2 == add t0, tp, #:tprel_hi12:x1, lsl #12 R_AARCH64_TLSLE_ADD_TPREL_HI12 x2 add t0, #:tprel_lo12_nc:x1R_AARCH64_TLSLE_ADD_TPREL_LO12_NCx2 sequence 2 == movz t0, #:tprel_g1:x3 R_AARCH64_TLSLE_MOVW_TPREL_G1x3 movk t0, #:tprel_g0_nc:x3 R_AARCH64_TLSLE_MOVW_TPREL_G0_NC x3 add t0, tp, t0 sequence 4 == movz t0, #:tprel_g2:x4 R_AARCH64_TLSLE_MOVW_TPREL_G2x4 movk t0, #:tprel_g1_nc:x4 R_AARCH64_TLSLE_MOVW_TPREL_G1_NC x4 movk t0, #:tprel_g0_nc:x4 R_AARCH64_TLSLE_MOVW_TPREL_G0_NC x4 add t0, t0, tp Under tiny model, we still can't use the simplest sequence 1, because the allowed loadable segment size is 1M, while 12bit offset (4K) still can't access. While even under large model, if the tls-size is small than 4K, we still can use the simplest sequence 1 for local-exec. This is the first patch to cleanup TLSLE support which generalize TLSE variable/marco name for all memory models. OK for trunk? 2015-05-19 Marcus Shawcroft marcus.shawcr...@arm.com Jiong Wang jiong.w...@arm.com gcc/ * config/aarch64/aarch64-protos.h (arch64_symbol_type): Rename SYMBOL_SMALL_TPREL to SYMBOL_TLSLE. (aarch64_symbol_context): Ditto. * config/aarch64/aarch64.c (aarch64_load_symref_appropriately): Ditto. (aarch64_expand_mov_immediate): Ditto. (aarch64_print_operand): Ditto. (aarch64_classify_tls_symbol): Ditto. -- Regards, Jiong diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index 931c8b8..12cc5ee 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -64,7 +64,7 @@ enum aarch64_symbol_context SYMBOL_SMALL_TLSGD SYMBOL_SMALL_TLSDESC SYMBOL_SMALL_GOTTPREL - SYMBOL_SMALL_TPREL + SYMBOL_TLSLE Each of of these represents a thread-local symbol, and corresponds to the thread local storage relocation operator for the symbol being referred to. @@ -98,9 +98,9 @@ enum aarch64_symbol_type SYMBOL_SMALL_TLSGD, SYMBOL_SMALL_TLSDESC, SYMBOL_SMALL_GOTTPREL, - SYMBOL_SMALL_TPREL, SYMBOL_TINY_ABSOLUTE, SYMBOL_TINY_GOT, + SYMBOL_TLSLE, SYMBOL_FORCE_TO_MEM }; diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index c7b936d..99a534c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -978,7 +978,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, return; } -case SYMBOL_SMALL_TPREL: +case SYMBOL_TLSLE: { rtx tp = aarch64_load_tp (NULL); @@ -1537,9 +1537,9 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm) } /* FALLTHRU */ -case SYMBOL_SMALL_TPREL: case SYMBOL_SMALL_ABSOLUTE: case SYMBOL_TINY_ABSOLUTE: + case SYMBOL_TLSLE: aarch64_load_symref_appropriately (dest, imm, sty); return; @@ -4416,7 +4416,7 @@ aarch64_print_operand (FILE *f, rtx x, char code) asm_fprintf (asm_out_file, :gottprel:); break; - case SYMBOL_SMALL_TPREL: + case SYMBOL_TLSLE: asm_fprintf (asm_out_file, :tprel:); break; @@ -4449,7 +4449,7 @@ aarch64_print_operand (FILE *f, rtx x, char code) asm_fprintf (asm_out_file, :gottprel_lo12:); break; - case SYMBOL_SMALL_TPREL: + case SYMBOL_TLSLE: asm_fprintf (asm_out_file, :tprel_lo12_nc:); break; @@ -4467,7 +4467,7 @@ aarch64_print_operand (FILE *f, rtx x, char code) switch (aarch64_classify_symbolic_expression (x, SYMBOL_CONTEXT_ADR)) { - case SYMBOL_SMALL_TPREL: + case SYMBOL_TLSLE: asm_fprintf (asm_out_file, :tprel_hi12:); break; default: @@ -7212,7 +7212,7 @@ aarch64_classify_tls_symbol (rtx x) return SYMBOL_SMALL_GOTTPREL; case TLS_MODEL_LOCAL_EXEC: - return SYMBOL_SMALL_TPREL; + return SYMBOL_TLSLE; case TLS_MODEL_EMULATED: case TLS_MODEL_NONE:
[Bug c/66213] unsigned char value range can be greater than sizeof unsigned char
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66213 --- Comment #2 from zh__ z.hege...@t-systems.com --- Yep, sorry. My bad.
[Bug tree-optimization/65752] Too strong optimizations int - pointer casts
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 --- Comment #28 from rguenther at suse dot de rguenther at suse dot de --- On Wed, 20 May 2015, gil.hur at sf dot snu.ac.kr wrote: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 --- Comment #27 from Chung-Kil Hur gil.hur at sf dot snu.ac.kr --- (In reply to Chung-Kil Hur from comment #26) Thanks for the detailed explanations. The C standard only guarantees that you can convert a pointer to uintptr_t and back, it doesn't guarantee that you can convert a modified uintptr_t back to a pointer that is valid. Thus, doing (int *)((xp + i) - j) is invoking undefined behavior. I didn't know about this rule. I thought this cast is valid because (xp+i)-j is even safely-derived. Could you give a reference for that rule in the standard? Thanks! It seems that the following rule might be the one. = 7.20.1.4 Integer types capable of holding object pointers The following type designates a signed integer type with the property that any valid pointer to void can be converted to this type, then converted back to pointer to void, and the result will compare equal to the original pointer: intptr_t The following type designates an unsigned integer type with the property that any valid pointer to void can be converted to this type, then converted back to pointer to void, and the result will compare equal to the original pointer: uintptr_t These types are optional. = Yes, that's the one I remember. Right. This does not say that we are allowed to cast a modified integer back to a pointer. What I remember might be from the C++ standard, where safely derived integers are allowed to be cast back to pointers. Umm. This might also be implementation-defined. Yeah, what is safely derived is the question here (you might not break the dependency chain in any (non-)obvious way).
[Bug libstdc++/38265] STL treats explicit constructors as converting constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38265 Jonathan Wakely redi at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |INVALID --- Comment #15 from Jonathan Wakely redi at gcc dot gnu.org --- (In reply to Geoff Romer from comment #14) But see [sequence.reqmts]/p3: i and j denote iterators satisfying input iterator requirements and refer to elements implicitly convertible to value_type. So this is not intended to work per se; a conforming library could disallow it. However, the notes on LWG 536 say Some support, not universal, for respecting the explicit qualifier, so it looks like the standard intentionally permits this as a conforming extension. Which makes the original testcase invalid, so it's a bug in the user's code, not a bug in libstdc++ to accept it. In principle, I think perfect initialization is what's called for here: the range ctor should be explicit if and only if it uses an explicit constructor for value_type. I don't think that would be a good idea. I don't think it follows that X(int) being explicit should mean dequeX(int*, int*) should be explicit. By that logic shared_ptr(unique_ptrT) should be explicit, because unique_ptr(T*) is explicit. int* is not int, and dequeX is not X.
[gomp4] New builtins, preparation for oacc vector-single
To implement OpenACC vector-single mode, we need to ensure that only one thread out of the group representing a worker executes. The others skip computations but follow along the CFG, so the results of conditional branch decisions must be broadcast to them. The patch below adds a new builtin and nvptx pattern to implement that broadcast functionality. Committed on gomp-4_0-branch. Bernd Index: gcc/ChangeLog.gomp === --- gcc/ChangeLog.gomp (revision 223360) +++ gcc/ChangeLog.gomp (working copy) @@ -1,3 +1,16 @@ +2015-05-19 Bernd Schmidt ber...@codesourcery.com + + * omp-builtins.def (GOACC_thread_broadcast, + GOACC_thread_broadcast_ll): New builtins. + * optabs.def (oacc_thread_broadcast_optab): New optab. + * builtins.c (expand_builtin_oacc_thread_broadcast): New function. + (expand_builtin): Use it. + * config/nvptx/nvptx.c (nvptx_cannot_copy_insn_p): New function. + (TARGET_CANNOT_COPY_INSN_P): Define. + * config/nvptx/nvptx.md (UNSPECV_WARP_BCAST): New constant. + (oacc_thread_broadcastsi): New pattern. + (oacc_thread_broadcastdi): New expander. + 2015-05-19 Tom de Vries t...@codesourcery.com * omp-low.c (enclosing_target_ctx): Comment out. Index: gcc/builtins.c === --- gcc/builtins.c (revision 223360) +++ gcc/builtins.c (working copy) @@ -6022,6 +6022,43 @@ expand_oacc_ganglocal_ptr (rtx target AT return NULL_RTX; } +/* Handle a GOACC_thread_broadcast builtin call EXP with target TARGET. + Return the result. */ + +static rtx +expand_builtin_oacc_thread_broadcast (tree exp, rtx target) +{ + tree arg0 = CALL_EXPR_ARG (exp, 0); + enum insn_code icode; + + enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg0)); + gcc_assert (INTEGRAL_MODE_P (mode)); + do +{ + icode = direct_optab_handler (oacc_thread_broadcast_optab, mode); + mode = GET_MODE_WIDER_MODE (mode); +} + while (icode == CODE_FOR_nothing mode != VOIDmode); + if (icode == CODE_FOR_nothing) +return expand_expr (arg0, NULL_RTX, VOIDmode, EXPAND_NORMAL); + + rtx tmp = target; + machine_mode mode0 = insn_data[icode].operand[0].mode; + machine_mode mode1 = insn_data[icode].operand[1].mode; + if (!REG_P (tmp) || GET_MODE (tmp) != mode0) +tmp = gen_reg_rtx (mode0); + rtx op1 = expand_expr (arg0, NULL_RTX, mode1, EXPAND_NORMAL); + if (GET_MODE (op1) != mode1) +op1 = convert_to_mode (mode1, op1, 0); + + rtx insn = GEN_FCN (icode) (tmp, op1); + if (insn != NULL_RTX) +{ + emit_insn (insn); + return tmp; +} + return const0_rtx; +} /* Expand an expression EXP that calls a built-in function, with result going to TARGET if that's convenient @@ -7177,6 +7214,10 @@ expand_builtin (tree exp, rtx target, rt return target; break; +case BUILT_IN_GOACC_THREAD_BROADCAST: +case BUILT_IN_GOACC_THREAD_BROADCAST_LL: + return expand_builtin_oacc_thread_broadcast (exp, target); + default: /* just do library call, if unknown builtin */ break; } Index: gcc/config/nvptx/nvptx.c === --- gcc/config/nvptx/nvptx.c (revision 223360) +++ gcc/config/nvptx/nvptx.c (working copy) @@ -2029,6 +2029,15 @@ nvptx_vector_alignment (const_tree type) return MIN (align, BIGGEST_ALIGNMENT); } + +static bool +nvptx_cannot_copy_insn_p (rtx_insn *insn) +{ + if (recog_memoized (insn) == CODE_FOR_oacc_thread_broadcastsi) +return true; + return false; +} + /* Record a symbol for mkoffload to enter into the mapping table. */ @@ -2153,6 +2162,9 @@ nvptx_file_end (void) #undef TARGET_VECTOR_ALIGNMENT #define TARGET_VECTOR_ALIGNMENT nvptx_vector_alignment +#undef TARGET_CANNOT_COPY_INSN_P +#define TARGET_CANNOT_COPY_INSN_P nvptx_cannot_copy_insn_p + struct gcc_target targetm = TARGET_INITIALIZER; #include gt-nvptx.h Index: gcc/config/nvptx/nvptx.md === --- gcc/config/nvptx/nvptx.md (revision 223360) +++ gcc/config/nvptx/nvptx.md (working copy) @@ -61,6 +61,7 @@ (define_c_enum unspecv [ UNSPECV_LOCK UNSPECV_CAS UNSPECV_XCHG + UNSPECV_WARP_BCAST ]) (define_attr subregs_ok false,true @@ -1322,6 +1323,37 @@ (define_expand oacc_ctaid FAIL; }) +(define_insn oacc_thread_broadcastsi + [(set (match_operand:SI 0 nvptx_register_operand ) + (unspec_volatile:SI [(match_operand:SI 1 nvptx_register_operand )] + UNSPECV_WARP_BCAST))] + + %.\\tshfl.idx.b32\\t%0, %1, 0, 31;) + +(define_expand oacc_thread_broadcastdi + [(set (match_operand:DI 0 nvptx_register_operand ) + (unspec_volatile:DI [(match_operand:DI 1 nvptx_register_operand )] + UNSPECV_WARP_BCAST))] + +{ + rtx t = gen_reg_rtx (DImode); + emit_insn (gen_lshrdi3 (t, operands[1], GEN_INT (32))); + rtx op0 = force_reg (SImode, gen_lowpart (SImode, t)); + rtx op1 = force_reg (SImode, gen_lowpart (SImode,
Re: [c++std-parallel-1614] Re: Compilers and RCU readers: Once more unto the breach!
On Wed, May 20, 2015 at 11:03:00AM +0200, Richard Biener wrote: On Wed, May 20, 2015 at 9:34 AM, Jens Maurer jens.mau...@gmx.net wrote: On 05/20/2015 04:34 AM, Paul E. McKenney wrote: On Tue, May 19, 2015 at 06:57:02PM -0700, Linus Torvalds wrote: - the you can add/subtract integral values still opens you up to language lawyers claiming (char *)ptr - (intptr_t)ptr preserving the dependency, which it clearly doesn't. But language-lawyering it does, since all those operations (cast to pointer, cast to integer, subtracting an integer) claim to be dependency-preserving operations. [...] There are some stranger examples, such as (char *)ptr - ((intptr_t)ptr)/7, but in that case, if the resulting pointer happens by chance to reference valid memory, I believe a dependency would still be carried. [...] From a language lawyer standpoint, pointer arithmetic is only valid within an array. These examples seem to go beyond the bounds of the array and therefore have undefined behavior. C++ standard section 5.7 paragraph 4 If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined. C99 and C11 identical phrasing in 6.5.6 paragraph 8 Of course you can try to circumvent that by doing (char*)((intptr_t)ptr - (intptr_t)ptr + (intptr_t)ptr) (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 for extra fun). Which (IMHO) gets you into the standard language that only makes conversion of the exact same integer back to a pointer well-defined(?) I am feeling good about leaving the restriction and calling out the two paragraphs in a footnote, then. ;-) Thanx, Paul
Re: [c++std-parallel-1616] Re: Compilers and RCU readers: Once more unto the breach!
On Wed, May 20, 2015 at 09:34:10AM +0200, Jens Maurer wrote: On 05/20/2015 04:34 AM, Paul E. McKenney wrote: On Tue, May 19, 2015 at 06:57:02PM -0700, Linus Torvalds wrote: - the you can add/subtract integral values still opens you up to language lawyers claiming (char *)ptr - (intptr_t)ptr preserving the dependency, which it clearly doesn't. But language-lawyering it does, since all those operations (cast to pointer, cast to integer, subtracting an integer) claim to be dependency-preserving operations. [...] There are some stranger examples, such as (char *)ptr - ((intptr_t)ptr)/7, but in that case, if the resulting pointer happens by chance to reference valid memory, I believe a dependency would still be carried. [...] From a language lawyer standpoint, pointer arithmetic is only valid within an array. These examples seem to go beyond the bounds of the array and therefore have undefined behavior. C++ standard section 5.7 paragraph 4 If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined. C99 and C11 identical phrasing in 6.5.6 paragraph 8 Even better! I added a footnote calling out these two paragraphs. Thax, Paul
Re: optimization question
On 5/20/2015 3:27 AM, Richard Biener wrote: On Mon, May 18, 2015 at 10:01 PM, mark maule mark.ma...@oracle.com wrote: I have a loop which hangs when compiled with -O2, but runs fine when compiled with -O1. Not sure what information is required to get an answer, so starting with the full src code. I have not attempted to reduce to a simpler test case yet. Attachments: bs_destage.c - full source code bs_destage.dis.O2 - gdb disassembly of bs_destageLoop() bs_destage.dis+m.O2 - src annotated version of the above The function in question is bs_destageSearch(). When I compile bs_destage.c with -O2, it seems that the dgHandle condition at line 741 is being ignored, leading to an infinite loop. I can see in the disassembly that dgHandle is still in the code as a 16-bit value stored at 0x32(%rsp), and a running 32-bit copy stored at 0x1c(%rsp). I can also see that the 16 bit version at 0x32(%rsp) is being incremented at the end of the loop, but I don't see anywhere in the code where either version of dgHandle is being used when determining if the while() at 741 should be continued. I'm not very familiar with the optimizations that are done in O2 vs O1, or even what happens in these optimizations. So, I'm wondering if this is a bug, or a subtle valid optimization that I don't understand. Any help would be appreciated. Note: changing the declaration of dgHandle to be volitile appears to modify the code sufficiently that it looks like the dgHandle check is honored (have not tested). Thanks in advance for any help/advice. The usual issue with this kind of behavior is out-of-bound accesses of arrays in a loop or invoking undefined behavior when signed integer operations wrap. uint32_toutLun[ BS_CFG_DRIVE_GROUPS ]; and while ( ( dgHandle ( BS_CFG_DRIVE_GROUPS + 1 ) ) ... dgDestageOut = bs_destageData.outLun[ dgHandle ]; looks like this might access outLun[BS_CFG_DRIVE_GROUPS] which is out-of-bounds. Richard. You are correct, and when I change outLun[] to be size BS_CFG_DRIVE_GROUPS+1, the generated asm looks like it will account for dgHandle in the while() loop. I will pass this back to our development team to get a proper fix. Now, the followon: Something in the compiler/optimizer recognized this out of bounds situation - should a warning have been emitted? Or are there ambiguities which make a warning generation here inappropriate? And an additional question: It still seems wrong to omit the dgHandle check from the while() condition vs. leaving it in and letting the code access beyond the end of the array. Is this one of those areas where if there's a bug in the code all bets are off and your mileage may vary? Thanks to everyone who helped me out here. Mark
[Bug target/65979] Multiple issues in conftest.c prevent build on sh4-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #15 from Oleg Endo olegendo at gcc dot gnu.org --- (In reply to Oleg Endo from comment #14) I think the check operands[1] / operands[2] check should go into the preparation statement. operands[0] is dying after this peephole, so I guess this should work: I have confirmed that the following fixes the test case in attachment 35572: Index: gcc/config/sh/sh.md === --- gcc/config/sh/sh.md (revision 223416) +++ gcc/config/sh/sh.md (working copy) @@ -14721,6 +14721,9 @@ || REGNO (operands[2]) == REGNO (operands[5])) [(const_int 0)] { + if (reg_overlap_mentioned_p (operands[1], operands[2])) +std::swap (operands[0], operands[2]); + sh_check_add_incdec_notes (emit_move_insn (operands[2], operands[3])); emit_insn (gen_tstsi_t (operands[2], gen_rtx_REG (SImode, (REGNO (operands[1]); However, I think that the emit_move_insn could also be a source of hidden problems. For instance, if the captured insn (set (match_operand:SI 2 arith_reg_dest) (match_operand:SI 3)) is not a move insn, but some computation like (set (reg) (plus:SI ... )). I'm not sure what emit_move_insn is going to do in this case. Maybe a better way would be to copy/re-emit the captured original insn in a different way.
[Bug libstdc++/60936] [4.9/5/6 Regression] Binary code bloat with std::string
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60936 Markus Eisenmann meisenmann@fh-salzburg.ac.at changed: What|Removed |Added CC||meisenmann.lba@fh-salzburg. ||ac.at --- Comment #12 from Markus Eisenmann meisenmann@fh-salzburg.ac.at --- Created attachment 35573 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35573action=edit Using __throw_out_of_range (instead of __throw_out_of_range_fmt), if configured with --disable-libstdcxx-verbose My patch (file gcc-4.9-pr60936.patch) is a fix/work-around as suggested by Jonathan in Comment 4. Calling __throw_out_of_range_fmt is replaced by the (simpler) function __throw_out_of_range(), if the gcc-build is configured with the option --disable-libstdcxx-verbose. Note: I have used the previous call to __throw_out_of_range as used in GCC-release 4.8.4. Maybe the patch has to be applied with the option -p1 (or change the patch-file), because the path begins with 'gcc-4.9.2/' ... Following source-files will be changed (by this patch): [gcc-4.9.2/] libstdc++-v3/include/bits/basic_string.h [gcc-4.9.2/] libstdc++-v3/include/bits/functexcept.h [gcc-4.9.2/] libstdc++-v3/include/bits/stl_bvector.h [gcc-4.9.2/] libstdc++-v3/include/bits/stl_deque.h [gcc-4.9.2/] libstdc++-v3/include/bits/stl_vector.h [gcc-4.9.2/] libstdc++-v3/include/debug/array [gcc-4.9.2/] libstdc++-v3/include/experimental/string_view [gcc-4.9.2/] libstdc++-v3/include/ext/vstring.h [gcc-4.9.2/] libstdc++-v3/include/profile/array [gcc-4.9.2/] libstdc++-v3/include/std/array [gcc-4.9.2/] libstdc++-v3/include/std/bitset [gcc-4.9.2/] libstdc++-v3/src/c++11/functexcept.cc [gcc-4.9.2/] libstdc++-v3/testsuite/util/exception/safety.h Best regards, Markus
RE: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
From: Steven Bosscher [mailto:stevenb@gmail.com] Sent: Tuesday, May 19, 2015 7:21 PM Not OK. This will break in move_invariants() when it looks at REGNO (inv-reg). Indeed. I'm even surprised all tests passed. Ok I will just prevent moving in such a case. I'm running the tests now and will get back to you tomorrow. Best regards, Thomas
Re: [patch,gomp4] error on invalid acc loop clauses
Hi! On Wed, 20 May 2015 10:43:27 +0200, Jakub Jelinek ja...@redhat.com wrote: On Wed, May 20, 2015 at 10:23:21AM +0200, Thomas Schwinge wrote: I see that some checking is also being done gcc/omp-low.c:scan_omp_for: »gang, worker and vector may occur only once in a loop nest«, and »gang, worker and vector must occur in this order in a loop nest«. Don't know if that conceptually also belongs into gcc/omp-low.c:check_omp_nesting_restrictions? Doesn't look like anything related to construct/region nesting... It is checking invalid nesting of loop constructs, for example: #pragma acc loop gang for [...] { #pragma acc loop gang // gang, worker and vector may occur only once in a loop nest for [...] ..., or: #pragma acc loop vector for [...] { #pragma acc loop gang // gang, worker and vector must occur in this order in a loop nest for [...] ..., and so on. Grüße, Thomas pgpnWE1nL_OFR.pgp Description: PGP signature
Re: [i386] Scalar DImode instructions on XMM registers
On 19 May 11:22, Vladimir Makarov wrote: On 05/18/2015 08:13 AM, Ilya Enkovich wrote: 2015-05-06 17:18 GMT+03:00 Ilya Enkovich enkovich@gmail.com: Hi Vladimir, Could you please comment on this? Ilya, I think that the idea is worth to try but results might be mixed. It is hard to say until you actually try it (as example, Jan implemented -fpmath=both and it looks a pretty good idea at least for me but when I checked SPEC2000 the results were not so good even with IRA/LRA). Long ago I did some experiments and found that spilling into SSE would benefitial for Intel CPUs but not for AMD ones. As I remember I also found that storing several scalar values into one SSE reg and extracting it when you need to do some (fp) arithmetics would benefitial for AMD but not for Intel CPUs. In literature more general approach is called bitwise register allocator. Actually it would be a pretty big IRA/LRA project from which some targets might benefit. I suspect such things are not trivially done in IRA/LRA and want to make it as an independent optimization because its application seems to be quite narrow. As for the wrong code, it is hard for me to say anything w/o RA dumps. If you send me the dump (-fira-verbose=16), i might say more what is going on. Here are some dumps from my reproducer. The problematic register is r108. Thanks, Ilya ;; Function test (test, funcdef_no=0, decl_uid=1933, cgraph_uid=0, symbol_order=0) scanning new insn with uid = 79. starting the processing of deferred insns ending the processing of deferred insns df_analyze called df_worklist_dataflow_doublequeue:n_basic_blocks 5 n_edges 6 count 5 (1) starting the processing of deferred insns ending the processing of deferred insns df_analyze called Reg 119: local to bb 2 def dominates all uses has unique first use Reg 125 uninteresting Reg 118: local to bb 2 def dominates all uses has unique first use Reg 126 uninteresting Reg 127 uninteresting Found def insn 26 for 119 to be not moveable ;; 2 loops found ;; ;; Loop 0 ;; header 0, latch 1 ;; depth 0, outer -1 ;; nodes: 0 1 2 3 4 ;; ;; Loop 1 ;; header 3, latch 3 ;; depth 1, outer 0 ;; nodes: 3 ;; 2 succs { 3 4 } ;; 3 succs { 3 4 } ;; 4 succs { 1 } starting the processing of deferred insns ending the processing of deferred insns df_analyze called init_insns for 117: (insn_list:REG_DEP_TRUE 22 (nil)) test Dataflow summary: ;; invalidated by call 0 [ax] 1 [dx] 2 [cx] 8 [st] 9 [st(1)] 10 [st(2)] 11 [st(3)] 12 [st(4)] 13 [st(5)] 14 [st(6)] 15 [st(7)] 17 [flags] 18 [fpsr] 19 [fpcr] 21 [xmm0] 22 [xmm1] 23 [xmm2] 24 [xmm3] 25 [xmm4] 26 [xmm5] 27 [xmm6] 28 [xmm7] 29 [mm0] 30 [mm1] 31 [mm2] 32 [mm3] 33 [mm4] 34 [mm5] 35 [mm6] 36 [mm7] 37 [] 38 [] 39 [] 40 [] 41 [] 42 [] 43 [] 44 [] 45 [] 46 [] 47 [] 48 [] 49 [] 50 [] 51 [] 52 [] 53 [] 54 [] 55 [] 56 [] 57 [] 58 [] 59 [] 60 [] 61 [] 62 [] 63 [] 64 [] 65 [] 66 [] 67 [] 68 [] 69 [] 70 [] 71 [] 72 [] 73 [] 74 [] 75 [] 76 [] 77 [] 78 [] 79 [] 80 [] ;; hardware regs used 7 [sp] 16 [argp] 20 [frame] ;; regular block artificial uses6 [bp] 7 [sp] 16 [argp] 20 [frame] ;; eh block artificial uses 6 [bp] 7 [sp] 16 [argp] 20 [frame] ;; entry block defs 0 [ax] 1 [dx] 2 [cx] 6 [bp] 7 [sp] 16 [argp] 20 [frame] 21 [xmm0] 22 [xmm1] 23 [xmm2] 29 [mm0] 30 [mm1] 31 [mm2] ;; exit block uses 6 [bp] 7 [sp] 20 [frame] ;; regs ever live 3[bx] 7[sp] 17[flags] ;; ref usage r0={2d} r1={2d} r2={2d} r3={1d,1u} r6={1d,4u} r7={1d,7u} r8={1d} r9={1d} r10={1d} r11={1d} r12={1d} r13={1d} r14={1d} r15={1d} r16={1d,4u,1e} r17={5d,2u} r18={1d} r19={1d} r20={1d,4u} r21={2d} r22={2d} r23={2d} r24={1d} r25={1d} r26={1d} r27={1d} r28={1d} r29={2d} r30={2d} r31={2d} r32={1d} r33={1d} r34={1d} r35={1d} r36={1d} r37={1d} r38={1d} r39={1d} r40={1d} r41={1d} r42={1d} r43={1d} r44={1d} r45={1d} r46={1d} r47={1d} r48={1d} r49={1d} r50={1d} r51={1d} r52={1d} r53={1d} r54={1d} r55={1d} r56={1d} r57={1d} r58={1d} r59={1d} r60={1d} r61={1d} r62={1d} r63={1d} r64={1d} r65={1d} r66={1d} r67={1d} r68={1d} r69={1d} r70={1d} r71={1d} r72={1d} r73={1d} r74={1d} r75={1d} r76={1d} r77={1d} r78={1d} r79={1d} r80={1d} r107={1d,1u} r108={2d,4u} r117={2d,5u,2e} r118={1d,1u} r119={1d,1u} r123={2d,3u} r124={2d,3u} r125={1d,1u} r126={1d,1u} r127={1d,1u} r128={2d,2u} r129={2d,2u} ;;total ref usage 160{110d,47u,3e} in 25{24 regular + 1 call} insns. (note 21 0 24 NOTE_INSN_DELETED) (note 24 21 79 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (insn/f 79 24 22 2 (parallel [ (set (reg:SI 107) (unspec:SI [ (const_int 0 [0]) ] UNSPEC_SET_GOT)) (clobber (reg:CC 17 flags)) ]) 694 {set_got} (expr_list:REG_UNUSED (reg:CC 17 flags) (expr_list:REG_EQUIV (unspec:SI [ (const_int 0 [0]) ] UNSPEC_SET_GOT) (expr_list:REG_CFA_FLUSH_QUEUE (nil) (nil) (insn
Re: optimization question
On Mon, May 18, 2015 at 10:01 PM, mark maule mark.ma...@oracle.com wrote: I have a loop which hangs when compiled with -O2, but runs fine when compiled with -O1. Not sure what information is required to get an answer, so starting with the full src code. I have not attempted to reduce to a simpler test case yet. Attachments: bs_destage.c - full source code bs_destage.dis.O2 - gdb disassembly of bs_destageLoop() bs_destage.dis+m.O2 - src annotated version of the above The function in question is bs_destageSearch(). When I compile bs_destage.c with -O2, it seems that the dgHandle condition at line 741 is being ignored, leading to an infinite loop. I can see in the disassembly that dgHandle is still in the code as a 16-bit value stored at 0x32(%rsp), and a running 32-bit copy stored at 0x1c(%rsp). I can also see that the 16 bit version at 0x32(%rsp) is being incremented at the end of the loop, but I don't see anywhere in the code where either version of dgHandle is being used when determining if the while() at 741 should be continued. I'm not very familiar with the optimizations that are done in O2 vs O1, or even what happens in these optimizations. So, I'm wondering if this is a bug, or a subtle valid optimization that I don't understand. Any help would be appreciated. Note: changing the declaration of dgHandle to be volitile appears to modify the code sufficiently that it looks like the dgHandle check is honored (have not tested). Thanks in advance for any help/advice. The usual issue with this kind of behavior is out-of-bound accesses of arrays in a loop or invoking undefined behavior when signed integer operations wrap. uint32_toutLun[ BS_CFG_DRIVE_GROUPS ]; and while ( ( dgHandle ( BS_CFG_DRIVE_GROUPS + 1 ) ) ... dgDestageOut = bs_destageData.outLun[ dgHandle ]; looks like this might access outLun[BS_CFG_DRIVE_GROUPS] which is out-of-bounds. Richard. Mark Maule gcc version: Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.8.3/lto-wrapper Target: x86_64-redhat-linux Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto --enable-plugin --enable-initfini-array --disable-libgcj --with-isl=/builddir/build/BUILD/gcc-4.8.3-20140911/obj-x86_64-redhat-linux/isl-install --with-cloog=/builddir/build/BUILD/gcc-4.8.3-20140911/obj-x86_64-redhat-linux/cloog-install --enable-gnu-indirect-function --with-tune=generic --with-arch_32=x86-64 --build=x86_64-redhat-linux Thread model: posix gcc version 4.8.3 20140911 (Red Hat 4.8.3-9) (GCC)
[PATCH, CHKP] Clean-up redundant gimple_build_nop calls
Hi, This patch removes redundant gimple_build_nop calls from tree-chkp.c. MPX-bootstrapped and regtested for x86_64-unknown-linux-gnu. Applied to trunk. Thanks, Ilya -- 2015-05-20 Ilya Enkovich enkovich@gmail.com * tree-chkp.c (chkp_maybe_copy_and_register_bounds): Remove useless gimple_build_nop calls. (chkp_find_bounds_for_elem): Likewise. (chkp_get_zero_bounds): Likewise. (chkp_get_none_bounds): Likewise. (chkp_get_bounds_by_definition): Likewise. (chkp_generate_extern_var_bounds): Likewise. (chkp_get_bounds_for_decl_addr): Likewise. (chkp_get_bounds_for_string_cst): Likewise. diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index 288470b..4f84a22 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -1172,10 +1172,10 @@ chkp_maybe_copy_and_register_bounds (tree ptr, tree bnd) gimple_stmt_iterator gsi; if (bnd_var) - copy = make_ssa_name (bnd_var, gimple_build_nop ()); + copy = make_ssa_name (bnd_var); else copy = make_temp_ssa_name (pointer_bounds_type_node, - gimple_build_nop (), + NULL, CHKP_BOUND_TMP_NAME); assign = gimple_build_assign (copy, bnd); @@ -1534,7 +1534,7 @@ chkp_find_bounds_for_elem (tree elem, tree *all_bounds, { if (!all_bounds[offs / POINTER_SIZE]) { - tree temp = make_temp_ssa_name (type, gimple_build_nop (), ); + tree temp = make_temp_ssa_name (type, NULL, ); gimple assign = gimple_build_assign (temp, elem); gimple_stmt_iterator gsi; @@ -2043,7 +2043,7 @@ chkp_get_zero_bounds (void) gimple_stmt_iterator gsi = gsi_start_bb (chkp_get_entry_block ()); gimple stmt; - zero_bounds = chkp_get_tmp_reg (gimple_build_nop ()); + zero_bounds = chkp_get_tmp_reg (NULL); stmt = gimple_build_assign (zero_bounds, chkp_get_zero_bounds_var ()); gsi_insert_before (gsi, stmt, GSI_SAME_STMT); } @@ -2073,7 +2073,7 @@ chkp_get_none_bounds (void) gimple_stmt_iterator gsi = gsi_start_bb (chkp_get_entry_block ()); gimple stmt; - none_bounds = chkp_get_tmp_reg (gimple_build_nop ()); + none_bounds = chkp_get_tmp_reg (NULL); stmt = gimple_build_assign (none_bounds, chkp_get_none_bounds_var ()); gsi_insert_before (gsi, stmt, GSI_SAME_STMT); } @@ -2728,7 +2728,7 @@ chkp_get_bounds_by_definition (tree node, gimple def_stmt, var = chkp_get_bounds_var (SSA_NAME_VAR (node)); else var = make_temp_ssa_name (pointer_bounds_type_node, - gimple_build_nop (), + NULL, CHKP_BOUND_TMP_NAME); else var = chkp_get_tmp_var (); @@ -2908,7 +2908,7 @@ chkp_generate_extern_var_bounds (tree var) gimple_seq_add_stmt (seq, stmt); lb = chkp_build_addr_expr (var); - size = make_ssa_name (chkp_get_size_tmp_var (), gimple_build_nop ()); + size = make_ssa_name (chkp_get_size_tmp_var ()); if (flag_chkp_zero_dynamic_size_as_infinite) { @@ -3005,7 +3005,7 @@ chkp_get_bounds_for_decl_addr (tree decl) gimple_stmt_iterator gsi = gsi_start_bb (chkp_get_entry_block ()); gimple stmt; - bounds = chkp_get_tmp_reg (gimple_build_nop ()); + bounds = chkp_get_tmp_reg (NULL); stmt = gimple_build_assign (bounds, bnd_var); gsi_insert_before (gsi, stmt, GSI_SAME_STMT); } @@ -3049,7 +3049,7 @@ chkp_get_bounds_for_string_cst (tree cst) gimple_stmt_iterator gsi = gsi_start_bb (chkp_get_entry_block ()); gimple stmt; - bounds = chkp_get_tmp_reg (gimple_build_nop ()); + bounds = chkp_get_tmp_reg (NULL); stmt = gimple_build_assign (bounds, bnd_var); gsi_insert_before (gsi, stmt, GSI_SAME_STMT); }
[Bug fortran/66193] ICE for initialisation of some non-zero-sized arrays
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66193 --- Comment #10 from Gerhard Steinmetz gerhard.steinmetz.fort...@t-online.de --- Perhaps it's better to make one factor larger. Maybe the following will help. $ cat zz1.f90 program p real :: z(2) z = 10 + [real :: 1, 2] print *, z end # you may check your patch $ cat zz2.f90 program p real :: z(2) z = 10. + [real :: 1, 2] print *, z end # you may check your patch --- $ cat zz3.f90 program p real :: z(2) z = [real :: 1, 2] + 10 print *, z end $ gfortran zz3.f90 $ a.out 1. 2. # expected: 11. 12. $ cat zz4.f90 program p real :: z(2) z = [real :: 1, 2] + 10. print *, z end $ gfortran zz4.f90 $ a.out 1. 2. # expected: 11. 12. --- Or to use an other basic operation : $ cat zz5.f90 program p real :: z(2) z = -10 * [real :: 1, 2] print *, z end # you may check your patch $ cat zz6.f90 program p real :: z(2) z = -10. * [real :: 1, 2] print *, z end # you may check your patch
[PR c/52952] More precise locations within format strings
This is a new version of the patch submitted here: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00663.html but handling (some) escape sequences. I could not figure out a way to re-use the code from libcpp for this, thus I implemented a simple function that given a string and offset in bytes, it computes the visual column corresponding to that offset. The function is very conservative: As soon as something unknown or inconsistent is detected, it returns zero, thus preserving the current behavior. This also preserves the current behavior for non-concatenated tokens. Bootstrapped and regression tested on x86_64-linux-gnu. OK? gcc/testsuite/ChangeLog: 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org PR c/52952 * gcc.dg/redecl-4.c: Update column numbers. * gcc.dg/format/bitfld-1.c: Likewise. * gcc.dg/format/attr-2.c: Likewise. * gcc.dg/format/attr-6.c: Likewise. * gcc.dg/format/attr-7.c (baz): Likewise. * gcc.dg/format/asm_fprintf-1.c: Likewise. * gcc.dg/format/attr-4.c: Likewise. * gcc.dg/format/branch-1.c: Likewise. * gcc.dg/format/c90-printf-1.c: Likewise. Add tests for column locations within strings with embedded escape sequences. gcc/c-family/ChangeLog: 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org PR c/52952 * c-format.c (location_column_from_byte_offset): New. (location_from_offset): New. (struct format_wanted_type): Add offset_loc field. (check_format_info): Move handling of location for extra arguments closer to the point of warning. (check_format_arg): Set offset_is_invalid. (check_format_info_main): Pass the result of location_from_offset to warning_at. (format_type_warning): Pass the result of location_from_offset to warning_at. Index: gcc/c-family/c-format.c === --- gcc/c-family/c-format.c (revision 223371) +++ gcc/c-family/c-format.c (working copy) @@ -76,10 +76,90 @@ static bool cmp_attribs (const char *tat static int first_target_format_type; static const char *format_name (int format_num); static int format_flags (int format_num); +/* FIXME: This indicates that loc is not the location of the format + string, thus computing an offset is useless. This happens, for + example, when the format string is a constant array. + Unfortunately, GCC does not keep track of the location of the + initializer of the array yet. */ +static bool offset_is_invalid; + +/* Given a string S of length LINE_WIDTH, find the visual column + corresponding to OFFSET bytes. */ + +static unsigned int +location_column_from_byte_offset (const char *s, int line_width, + unsigned int offset) +{ + const char * c = s; + if (*c != '') +return 0; + + c++, offset--; + while (offset 0) +{ + if (c - s = line_width) + return 0; + + switch (*c) + { + case '\\': + c++; + if (c - s = line_width) + return 0; + switch (*c) + { + case '\\': case '\'': case '': case '?': + case '(': case '{': case '[': case '%': + case 'a': case 'b': case 'f': case 'n': + case 'r': case 't': case 'v': + case 'e': case 'E': + c++, offset--; + break; + + default: + return 0; + } + break; + + case '': + /* We found the end of the string too early. */ + return 0; + + default: + c++, offset--; + break; + } +} + return c - s; +} + +/* Return a location that encodes the same location as LOC but shifted + by OFFSET bytes. */ + +static location_t +location_from_offset (location_t loc, int offset) +{ + gcc_checking_assert (offset = 0); + if (offset_is_invalid + || linemap_location_from_macro_expansion_p (line_table, loc) + || offset 0) +return loc; + + expanded_location s = expand_location_to_spelling_point (loc); + int line_width; + const char *line = location_get_source_line (s, line_width); + line += s.column - 1 ; + line_width -= s.column - 1; + unsigned int column = +location_column_from_byte_offset (line, line_width, (unsigned) offset); + + return linemap_position_for_loc_and_offset (line_table, loc, column); +} + /* Check that we have a pointer to a string suitable for use as a format. The default is to check for a char type. For objective-c dialects, this is extended to include references to string objects validated by objc_string_ref_type_p (). Targets may also provide a string object type that can be used within c and @@ -388,10 +468,13 @@ typedef struct format_wanted_type int format_length; /* The actual parameter to check against the wanted type. */ tree param; /* The argument number of that parameter. */ int arg_num; + /* The offset location of this argument with respect to the format + string
Re: [patch,gomp4] error on invalid acc loop clauses
Hi! On Fri, 15 May 2015 11:10:21 -0700, Cesar Philippidis ce...@codesourcery.com wrote: This patch teaches the c and c++ front ends to error on invalid and conflicting acc loop clauses. E.g., an acc loop cannot have 'gang seq' and the worker and vector clauses inside parallel regions cannot have optional kernel-specific arguments. Thanks! The c and c++ front end also error when it detects a parallel or kernels region nested inside a parallel or kernels region. E.g. #pragma acc parallel { #pragma acc parallel ... } OK, but see below. I included two new test cases in this patch. They are mostly identical but, unfortunately, the c and c++ front ends emit slightly different error messages. The preference is to keep these as single files (so that C and C++ can easily be maintained together), and use the appropriate dg-* directives to select the expected C or C++ error message, respectively, or use regular expressions so as to match both the expected C and C++ error variants in one go, if they're similar enough. The front ends still need to be cleaned before this functionality should be considered for mainline. So for the time being I've applied this patch to gomp-4_0-branch. What remains to be done? Then, what about the Fortran front end? Checking already done as well as test coverage existing, similar to C and C++? Patch review comments: --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -234,6 +234,10 @@ typedef struct GTY(()) c_parser { /* True if we are in a context where the Objective-C Property attribute keywords are valid. */ BOOL_BITFIELD objc_property_attr_context : 1; + /* True if we are inside a OpenACC parallel region. */ + BOOL_BITFIELD oacc_parallel_region : 1; + /* True if we are inside a OpenACC kernels region. */ + BOOL_BITFIELD oacc_kernels_region : 1; Hmm. @@ -10839,6 +10843,7 @@ c_parser_oacc_shape_clause (c_parser *parser, pragma_omp_clause c_kind, mark_exp_read (expr); require_positive_expr (expr, expr_loc, str); *op_to_parse = expr; + op_to_parse = op0; } while (!c_parser_next_token_is (parser, CPP_CLOSE_PAREN)); c_parser_consume_token (parser); @@ -10852,6 +10857,17 @@ c_parser_oacc_shape_clause (c_parser *parser, pragma_omp_clause c_kind, if (op1) OMP_CLAUSE_OPERAND (c, 1) = op1; OMP_CLAUSE_CHAIN (c) = list; + + if (parser-oacc_parallel_region (op0 != NULL || op1 != NULL)) +{ + if (c_kind != PRAGMA_OACC_CLAUSE_GANG) + c_parser_error (parser, c_kind == PRAGMA_OACC_CLAUSE_WORKER ? + worker clause arguments are not supported in OpenACC parallel regions + : vector clause arguments are not supported in OpenACC parallel regions); + else if (op0 != NULL) + c_parser_error (parser, non-static argument to clause gang); +} Instead of in c_parser_oacc_shape_clause, shouldn't such checking rather be done inside the function invoking c_parser_oacc_shape_clause, that is, c_parser_oacc_parallel, etc.? @@ -12721,7 +12737,10 @@ static tree c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name, omp_clause_mask mask, tree *cclauses) { - tree stmt, clauses, block; + tree stmt, clauses, block, c; + bool gwv = false; + bool auto_clause = false; + bool seq_clause = false; strcat (p_name, loop); mask |= OACC_LOOP_CLAUSE_MASK; @@ -12732,6 +12751,33 @@ c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name, if (cclauses) clauses = oacc_split_loop_clauses (clauses, cclauses); + for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) +{ + switch (OMP_CLAUSE_CODE (c)) + { + case OMP_CLAUSE_GANG: + case OMP_CLAUSE_WORKER: + case OMP_CLAUSE_VECTOR: + gwv = true; + break; + case OMP_CLAUSE_AUTO: + auto_clause = true; + break; + case OMP_CLAUSE_SEQ: + seq_clause = true; + break; + default: + ; + } +} + + if (gwv auto_clause) +c_parser_error (parser, incompatible use of clause %auto%); + else if (gwv seq_clause) +c_parser_error (parser, incompatible use of clause %seq%); + else if (auto_clause seq_clause) +c_parser_error (parser, incompatible use of clause %seq% and %auto%); + block = c_begin_compound_stmt (true); stmt = c_parser_omp_for_loop (loc, parser, OACC_LOOP, clauses, NULL); block = c_end_compound_stmt (loc, block, true); I would have expected such checking to be done in c_omp_finish_clauses -- But maybe it's also OK to do it here, given that the loop construct is the only one where these clauses can appear. Jakub, any strong preference? @@ -12774,6 +12820,13 @@ c_parser_oacc_kernels (location_t loc, c_parser *parser, char *p_name) strcat (p_name, kernels); + if (parser-oacc_parallel_region || parser-oacc_kernels_region) +{ +
Re: [Patch, fortran, pr65548, 2nd take, v5] [5/6 Regression] gfc_conv_procedure_call
Hi Mikael, when I got you right on IRC, then you proposed this change about the pointer attribute: diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 6d565ae..545f778 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -5361,6 +5361,7 @@ gfc_trans_allocate (gfc_code * code) /* Mark the symbol referenced or gfc_trans_assignment will bug. */ newsym-n.sym-attr.referenced = 1; + newsym-n.sym-attr.pointer = 1; e3rhs-expr_type = EXPR_VARIABLE; /* Set the symbols type, upto it was BT_UNKNOWN. */ newsym-n.sym-ts = e3rhs-ts; @@ -5374,7 +5375,6 @@ gfc_trans_allocate (gfc_code * code) /* Set the dimension and pointer attribute for arrays to be on the safe side. */ newsym-n.sym-attr.dimension = 1; - newsym-n.sym-attr.pointer = 1; newsym-n.sym-as = arr; gfc_add_full_array_ref (e3rhs, arr); } Unfortunately does this lead to numerous regressions in the testsuite. For example: ./gfortran.sh -g allocate_alloc_opt_6.f90 -o allocate_alloc_opt_6 Fortraning using ***DEVelopment*** version... allocate_alloc_opt_6.f90:26:0: allocate(t, source=mytype(1.0,2)) ^ internal compiler error: Segmentation fault 0xe09a08 crash_signal /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/toplev.c:380 0xa9cbe1 useless_type_conversion_p(tree_node*, tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimple-expr.c:83 0x10622ae tree_ssa_useless_type_conversion(tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/tree-ssa.c:1178 0x10622fe tree_ssa_strip_useless_type_conversions(tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/tree-ssa.c:1190 0xb6c4ae gimplify_expr(tree_node**, gimple_statement_base**, gimple_statement_base**, bool (*)(tree_node*), int) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimplify.c:7815 0xb5e883 gimplify_modify_expr /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimplify.c:4644 I therefore came to a more elaborate change (revert the above one before testing this): diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 6d565ae..7b466de 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -5378,6 +5378,10 @@ gfc_trans_allocate (gfc_code * code) newsym-n.sym-as = arr; gfc_add_full_array_ref (e3rhs, arr); } + else if (POINTER_TYPE_P (TREE_TYPE (expr3))) + newsym-n.sym-attr.pointer = 1; + else + newsym-n.sym-attr.value = 1; /* The string length is known to. Set it for char arrays. */ if (e3rhs-ts.type == BT_CHARACTER) newsym-n.sym-ts.u.cl-backend_decl = expr3_len; This patch bootstraps and regtests fine again. Ok to commit? Regards, Andre On Tue, 19 May 2015 16:02:18 +0200 Mikael Morin mikael.mo...@sfr.fr wrote: Le 19/05/2015 10:50, Andre Vehreschild a écrit : Hi all, find attached latest version to fix 65548. Bootstraps and regtests ok on x86_64-linux-gnu/f21. OK. Thanks. Mikael -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [patch,gomp4] error on invalid acc loop clauses
On Wed, May 20, 2015 at 10:23:21AM +0200, Thomas Schwinge wrote: + if (gwv auto_clause) +c_parser_error (parser, incompatible use of clause %auto%); + else if (gwv seq_clause) +c_parser_error (parser, incompatible use of clause %seq%); + else if (auto_clause seq_clause) +c_parser_error (parser, incompatible use of clause %seq% and %auto%); + block = c_begin_compound_stmt (true); stmt = c_parser_omp_for_loop (loc, parser, OACC_LOOP, clauses, NULL); block = c_end_compound_stmt (loc, block, true); I would have expected such checking to be done in c_omp_finish_clauses -- But maybe it's also OK to do it here, given that the loop construct is the only one where these clauses can appear. Jakub, any strong preference? In the C FE, it is kind of arbitrary, some checks are done during parsing immediately, others are done in c_omp_finish_clauses. In the C++ FE, obviously more care on where things are diagnosed is needed, so many more checks are done in finish_omp_clauses, because we might want to wait until templates are instantiated. ..., and this: why not do such nesting checking in gcc/omp-low.c:check_omp_nesting_restrictions? Currently (changed by Bernd in internal r442824, 2014-11-29) we're allowing all OpenACC-inside-OpenACC nesting -- shouldn't that be changed instead of repeating the checks in every front end (Jakub?)? Yeah, testing nesting restrictions should be done in omp-low.c if possible. Adding ugly hacks to the FEs tracking the current state and duplicating across all 3 FEs is undesirable. Note, in C++ FE we already have sk_omp so some kind of OpenMP binding scope, but I think we don't have anything similar in the C FE. I see that some checking is also being done gcc/omp-low.c:scan_omp_for: »gang, worker and vector may occur only once in a loop nest«, and »gang, worker and vector must occur in this order in a loop nest«. Don't know if that conceptually also belongs into gcc/omp-low.c:check_omp_nesting_restrictions? Doesn't look like anything related to construct/region nesting... Jakub
Re: Cleanup and improve canonical type construction in LTO
On Wed, 20 May 2015, Jan Hubicka wrote: Richard, this is my attempt to make sense of TYPE_CANONICAL at LTO. My undrestanding is that gimple_canonical_types_compatible_p needs to return true for all pairs of types that are considered compatible across compilation unit for any of languages we support (and in a sane way for cross language, too) and moreover it needs to form an equivalence so it can be used to do canonical type merging. Now C definition of type compatibility ignores type names and only boils down to structural compare (which we get wrong for unions, but I will look into that incrementally, also C explicitely require fields names to match, which we don't) and it of course says that incompete type can match complete. field-names are difficult to match cross-language. This is bit generous on structures and unions, because every incomplete RECORD_TYPE is compatible with every RECORD_TYPE in program and similarly incomplete UNION_TYPE is compatible with every UNION_TYPE in program. Now from the fact that gimple_canonical_types_compatible_p must be equivalence (and thus transitive) we immmediately get that there is no way to make difference between two RECORD_TYPEs (or UNION_TYPEs) at all: there always may be incomplete that forces them equivalent. This is not how the code works. gimple_canonical_types_compatible_p will not match complete type with incomplete and this is not a prolblem only because TYPE_CANONICAL matters for complete types only. TBAA machinery never needs alias sets of an incomplete type (modulo bugs). Correct. More precisely we have two equivalences: 1) full structural equivalence matching fields, array sizes and function parameters, where pointer types are however recursively matched only with 2) Not sure about function parameters (well, function types at all - they don't play a role in TBAA) - function members are always pointers, so see 2) 2) structural equivalence ignoring any info from complete types: here all RECORD_TYPEs are equal, so are UNION_TYPEs, for functions we can only match return value (because of existence of non-prototypes), for arrays only TREE_TYPE. In this equivalence we also can't match TYPE_MODE of aggregates/arrays because it may not be set for incomplete ones. Now our implementation somehow compute only 1) and 2) is approximated by matching TREE_CODE of the pointer-to type. This is unnecesarily pesimistic. Pointer to pointer to int does not need to match pointer to pointer to structure. Note that you have (a lot of!) pointer members that point to structures in various state of completeness. A pointer to an incomplete type needs to match all other pointer types (well, the current code tries to make the exception that a pointer to an aggregate stays a pointer to an aggregate - thus the matching of pointed-to type - sorry to only remember now the connection to incompleteness ...) The patch bellow changes it in the following way: a) it adds MATCH_INCOMPLETE_TYPES parameter to gimple_canonical_types_compatible_p and gimple_canonical_type_hash to determine whether we compute equivalence 1) or 2). The way we handle pointers is updated so we set MATCH_INCOMPLETE_TYPES when recursing down to pointer type. This makes it possible for complete structure referring incomplete pointer type to be equivalent with a complete structure referring complete pointer type. But does this really end up getting more equivalence classes than the crude approach matching TREE_CODE? I believe that in this definition we do best possible equivalence passing the rules above and we do not need to care about SCC - the only way how type can reffer itself is via pointer and that will make us to drop to MATCH_INCOMPLETE_TYPES. b) it disables TYPE_CANONICAL calculation for incomplete types and functions types. It makes it clear that TYPE_CANONICAL is always 1) which is not defined on these. Sounds good (please split up the patch - I'm actually not looking at it right now). This seems to reduce number of canonical types computed to 1/3. We get bit more recursion in gimple_canonical_types_compatible_p and gimple_canonical_type_hash but only in MATCH_INCOMPLETE_TYPES mode that converges quite quickly. I know that it is not how other FEs works, but it is because they do have type equivalence notion that include TYPE_NAME so it is possible to determine TYPE_CANONICAL uniquely before the type is completed. The code was never intended to be generic it was LTO specific and middle-end specific (for TBAA and useless_type_conversion_p). Frontends (well, the C++ frontend) use TYPE_CANONICAL for their own idea of canonicalness. c) adds sanity checking - I can check that canonical_type_hash is not used for incomplete types (modulo ARRAY_TYPE that may appear as a field of complete
Re: [c++std-parallel-1614] Re: Compilers and RCU readers: Once more unto the breach!
On Wed, May 20, 2015 at 9:34 AM, Jens Maurer jens.mau...@gmx.net wrote: On 05/20/2015 04:34 AM, Paul E. McKenney wrote: On Tue, May 19, 2015 at 06:57:02PM -0700, Linus Torvalds wrote: - the you can add/subtract integral values still opens you up to language lawyers claiming (char *)ptr - (intptr_t)ptr preserving the dependency, which it clearly doesn't. But language-lawyering it does, since all those operations (cast to pointer, cast to integer, subtracting an integer) claim to be dependency-preserving operations. [...] There are some stranger examples, such as (char *)ptr - ((intptr_t)ptr)/7, but in that case, if the resulting pointer happens by chance to reference valid memory, I believe a dependency would still be carried. [...] From a language lawyer standpoint, pointer arithmetic is only valid within an array. These examples seem to go beyond the bounds of the array and therefore have undefined behavior. C++ standard section 5.7 paragraph 4 If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined. C99 and C11 identical phrasing in 6.5.6 paragraph 8 Of course you can try to circumvent that by doing (char*)((intptr_t)ptr - (intptr_t)ptr + (intptr_t)ptr) (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 for extra fun). Which (IMHO) gets you into the standard language that only makes conversion of the exact same integer back to a pointer well-defined(?) Richard. Jens
Re: miter_base simplification
On 03/05/15 22:19 +0200, François Dumont wrote: On 30/04/2015 13:18, Jonathan Wakely wrote: On 30/04/15 10:40 +0200, François Dumont wrote: On 27/04/2015 13:55, Jonathan Wakely wrote: (Alternatively, could the same simplification be made for __miter_base? Do we need _Miter_base or just two overloads of __miter_base()?) Definitely, I already have a patch for that. Great :-) And here is the patch for this part. I have implemented it in such a way that it will also remove several layers of move_iterator. 2015-05-04 François Dumont fdum...@gcc.gnu.org * include/bits/cpp_type_traits.h (std::move_iterator): Delete declaration. (std::__is_move_iteratormove_iterator): Move partial specialization... * include/bits/stl_iterator.h: ... here. (std::__miter_base): Overloads for std::reverse_iterator and std::move_iterator. * include/bits/stl_algobase.h (std::__miter_base): Provide default implementation. Tested under Linux x86_64. Ok to commit ? Yes OK, thanks (sorry for forgetting about this patch).
Re: [C PATCH] Use VAR_OR_FUNCTION_DECL_P
On Wed, May 20, 2015 at 12:01:21PM +0200, Marek Polacek wrote: The following patch is an effort to use the macro where appropriate in c/ and c-family/ directories. No functional changes intended. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-05-20 Marek Polacek pola...@redhat.com * c-pragma.c: Use VAR_OR_FUNCTION_DECL_P throughout. * c-common.c: Likewise. * c-decl.c: Use VAR_OR_FUNCTION_DECL_P throughout. * c-typeck.c: Likewise. Ok. Jakub
[Bug tree-optimization/65752] Too strong optimizations int - pointer casts
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 --- Comment #25 from Richard Biener rguenth at gcc dot gnu.org --- (In reply to Chung-Kil Hur from comment #24) (In reply to schwab from comment #23) gil.hur at sf dot snu.ac.kr gcc-bugzi...@gcc.gnu.org writes: Since hello is not printed, I think the if-statement is the same as no-op. Thus, removing the if-statement should not change the behavior of the program according to ISO C11. Unless you are invoking undefined behaviour. Andreas. == #include stdio.h int main() { int x = 0; uintptr_t xp = (uintptr_t) x; uintptr_t i, j; for (i = 0; i xp; i++) { } j = i; *(int*)((xp+i)-j) = 15; printf(%d\n, x); } = This prints 15. And I do not think there is any UB. Please correct me if I am wrong. Then, I add the if-statement. == #include stdio.h int main() { int x = 0; uintptr_t xp = (uintptr_t) x; uintptr_t i, j; for (i = 0; i xp; i++) { } j = i; /** begin ***/ if (j != xp) { printf(hello\n); j = xp; } /** end */ *(int*)((xp+i)-j) = 15; printf(%d\n, x); } = This prints 0 without printing hello. Thus, this raises no UB unless j != xp raises UB. If you think j != xp raises UB, please explain why and give some reference. Otherwise, I think it is a bug of GCC. The C standard only guarantees that you can convert a pointer to uintptr_t and back, it doesn't guarantee that you can convert a modified uintptr_t back to a pointer that is valid. Thus, doing (int *)((xp + i) - j) is invoking undefined behavior. That you see an effect of this undefined behavior just with the added if is because that if confuses early GCC optimizations which would have cancelled i - j to zero, retaining (int *)xp. Instead it enables later optimization to see that xp - j cancels and thus it is left with (int *)i. This is because you are essentially computing (xp + xp) - xp == xp but dependent on what two pieces get cancelled the pointer is based on either xp (ok) or on i (not ok - that is related to xp only via an implicit equivalency). The net result is that I can't see how to detect this kind of situation in points-to analysis in a way that does not pessimize all pointer-to-integer / integer-to-pointer conversions. In theory it would be possible to add a flag similar to -fno-strict-aliasing to do this pessimization (but there is already -fno-tree-pta which avoids the issue as well). So in the end my conclusion is that either the testcase invokes undefined behavior or the C standard has a defect. Thus the bug is WONTFIX unless somebody can come up with a way to handle these kind of equivalences in the points-to algorithm in GCC in a way not pessimizing everything. One might consider an incomplete approach like that in comment #6 but I am not convinced about this hack (and one would need to evaluate its effects on code generation).
[PATCH GCC]Improve how we handle overflow for type conversion in scev/ivopts, part I
Hi, As we know, GCC is too conservative when checking overflow behavior in SCEV and loop related optimizers. Result is some variable can't be recognized as scalar evolution and thus optimizations are missed. To be specific, optimizers like ivopts and vectorizer are affected. This issue is more severe on 64 bit platforms, for example, PR62173 is failed on aarch64; scev-3.c and scev-4.c were marked as XFAIL on lp64 platforms. As the first part to improve overflow checking in GCC, this patch does below improvements: 1) Ideally, chrec_convert should be responsible to convert scev like (type){base, step} to scev like {(type)base, (type)step} when the result scev doesn't overflow; chrec_convert_aggressive should do the conversion if the result scev could overflow/wrap. Unfortunately, current implementation may use chrec_convert_aggressive to return a scev that won't overflow. This is because of a) the static parameter fold_conversions for instantiate_scev_convert can only tracks whether chrec_convert_aggressive may be called, rather than if it does some overflow conversion or not; b) the implementation of instantiate_scev_convert sometimes shortcuts the call to chrec_convert and misses conversion opportunities. This patch improves this. 2) iv-no_overflow computed in simple_iv is too conservative. With 1) fixed, iv-no_overflow should reflects whether chrec_convert_aggressive does return an overflow scev. This patch improves this. 3) chrec_convert should be able to prove the resulting scev won't overflow with loop niter information. This patch doesn't finish this, but it factored a new interface out of scev_probably_wraps_p for future improvement. And that will be the part II patch. With the improvements in SCEV, this patch also improves optimizer(IVOPT) that uses scev information like below: For array reference in the form of arr[IV], GCC tries to derive new address iv {arr+iv.base, iv.step*elem_size} from IV. If IV overflow wrto a type that is narrower than address space, this derivation is not true because arr[IV] isn't a scev. Root cause why scev-*.c are failed now is the overflow information of IV is too conservative. IVOPT has to be conservative to reject arr[IV] as a scev. With more accurate overflow information, IVOPT can be improved too. So this patch fixes the mentioned long standing issues. Bootstrap and test on x86_64, x86 and aarch64. BTW, test gcc.target/i386/pr49781-1.c failed on x86_64, but I can confirmed it's not this patch's fault. So what's your opinion on this?. Thanks, bin 2015-05-20 Bin Cheng bin.ch...@arm.com PR tree-optimization/62173 * tree-ssa-loop-ivopts.c (struct iv): New field. Reorder fields. (alloc_iv, set_iv): New parameter. (determine_biv_step): Delete. (find_bivs): Inline original determine_biv_step. Pass new argument to set_iv. (idx_find_step): Use no_overflow information for conversion. * tree-scalar-evolution.c (analyze_scalar_evolution_in_loop): Let resolve_mixers handle folded_casts. (instantiate_scev_name): Change bool parameter to bool pointer. (instantiate_scev_poly, instantiate_scev_binary): Ditto. (instantiate_array_ref, instantiate_scev_not): Ditto. (instantiate_scev_3, instantiate_scev_2): Ditto. (instantiate_scev_1, instantiate_scev_r): Ditto. (instantiate_scev_convert, ): Change parameter. Pass argument to chrec_convert_aggressive. (instantiate_scev): Change argument. (resolve_mixers): New parameter and set it. (scev_const_prop): New argument. * tree-scalar-evolution.h (resolve_mixers): New parameter. * tree-chrec.c (convert_affine_scev): Call chrec_convert instead of chrec_conert_1. (chrec_convert): New parameter. Move definition below. (chrec_convert_aggressive): New parameter and set it. Call convert_affine_scev. * tree-chrec.h (chrec_convert): New parameter. (chrec_convert_aggressive): Ditto. * tree-ssa-loop-niter.c (loop_exits_before_overflow): New function. (scev_probably_wraps_p): Factor loop niter related code into loop_exits_before_overflow. gcc/testsuite/ChangeLog 2015-05-20 Bin Cheng bin.ch...@arm.com PR tree-optimization/62173 * gcc.dg/tree-ssa/scev-3.c: Remove xfail. * gcc.dg/tree-ssa/scev-4.c: Ditto. * gcc.dg/tree-ssa/scev-8.c: New. Index: gcc/testsuite/gcc.dg/tree-ssa/scev-4.c === --- gcc/testsuite/gcc.dg/tree-ssa/scev-4.c (revision 222758) +++ gcc/testsuite/gcc.dg/tree-ssa/scev-4.c (working copy) @@ -20,5 +20,5 @@ f(int k) } } -/* { dg-final { scan-tree-dump-times a 1 optimized { xfail { lp64 || llp64 } } } } */ +/* { dg-final { scan-tree-dump-times a 1 optimized } } */ /* { dg-final { cleanup-tree-dump optimized } } */ Index:
[Bug target/62231] [4.8/4.9 regression] Exception handling broken on powerpc-e500v2-linux-gnuspe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62231 --- Comment #16 from Andri Yngvason andri.yngvason at marel dot com --- Sorry, Joseph, I wasn't sure if this issue was fixed or not since the status is NEW. I'll report a new issue.
RE: [PATCH, MIPS]: Fix internal compiler error: in check_bool_attrs, at recog.c:2218 for micromips attribute
We could add -mflip-micromips complementing -mflip-mips16 and use that for testing too. Chances are it'd reveal further issues. Looking at how -mflip-mips16 has been implemented it does not appear to me adding -mflip-micromips would be a lot of effort. I'm in favour of adding such a switch since the testsuite doesn't cover a mixture of MIPS and microMIPS code. It certainly seems that we need a bit more coverage here in order that we can mostly stick to testing one or two MIPS configurations per commit. We'll have some MIPS machines in the compile farm shortly which may allow us to at least do the full all-config build of the toolchain more easily even if that doesn't extend to testing all the configs. Regards, Robert gcc/ * config/mips/mips.h (micromips_globals): Declare. OK, thanks. Matthew
Re: [PATCH][Testsuite] Disable tests with dg-require-fork for simulated targets
On 18 May 2015 at 20:25, Mike Stump mikest...@comcast.net wrote: On May 18, 2015, at 8:01 AM, Alan Lawrence alan.lawre...@arm.com wrote: Simulators such as qemu report the presence of fork (it's in glibc) but generally do not support synchronization primitives between threads, so any tests using fork are unreliable. Hum, I have a simulator (binutils/sim) that has fork. All those tests pass for me. They seem to be reliable for me. I didn’t do anything special as I recall. ? Thanks for having a look at this problem. I thought about this a while ago, and was wondering whether the guard shouldn't be are we using qemu?. Indeed as Mike, other simulators might support fork and threads quite well. I did add enough libc (aka newlib) to bootstrap gcc, which maybe is slightly more than some do, but, existence of additional libraries shouldn’t change it much. To the extent it does, it should be easy to notice any extra required libraries directly. If a qmu bug or design deficiency, do you have a pointer to the reported bug or the design where they talk about tit. I believe qemu broken support for threads is a well-known issue. For instance: https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02156.html Remember, the point of the test suite is to find bugs to be fixed. Papering over bugs by turning it off, is fine, but, we should have named bug reports that when fixed, cause us to go back and turn back on those that were turned off. This patch disables the subset of such tests that identify themselves using dg-require-fork. At present, such tests are limited to (a) gcc.dg/torture/ftrapv-1.c. and (b) some tests in the 27_io section of the libstdc++ testsuite, listed below. Further patches can add dg-require-fork to the many other tests that call fork(). Cross-tested on aarch64-none-linux-gnu using qemu, with these tests becoming UNSUPPORTED: (gcc) gcc.dg/torture/ftrapv-1.c So, I reviewed this test case. What about it doesn’t work? Kinda simple and small, easy to understand. Is this patch OK for trunk? No. Let’s talk about it before turning off a to of test cases.
[Bug c++/66211] [5/6 Regression] Rvalue conversion in ternary operator causes internal compiler error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66211 --- Comment #3 from Richard Biener rguenth at gcc dot gnu.org --- So we fold (and did fold before) 1 0 ? x : y to (float) x (thus an rvalue). Then later we call ocp_convert on that requesting a conversion to int which does 810 converted = fold_if_not_in_template (convert_to_integer (type, e)); where convert_to_integer ends up just doing 910 return build1 (FIX_TRUNC_EXPR, type, expr); and fold then applying the simplification /* If we are converting an integer to a floating-point that can represent it exactly and back to an integer, we can skip the floating-point conversion. */ (if (inside_int inter_float final_int (unsigned) significand_size (TYPE_MODE (inter_type)) = inside_prec - !inside_unsignedp) (convert @0)) and (for cvt (convert view_convert float fix_trunc) (simplify (cvt @0) (if ((GIMPLE useless_type_conversion_p (type, TREE_TYPE (@0))) || (GENERIC type == TREE_TYPE (@0))) @0))) where wrapping the result as (non_lvalue @0) fixes the regression. The bug is of course the C++ frontend folding stuff too early (and too aggressive) here. But for GCC 5 the above might be a good-enough workaround (eventually we can conditionalize building non_lvalue exprs to non-C-frontends).
Re: [match-and-simplify] fix incorrect code-gen in 'for' pattern
On Wed, 20 May 2015, Prathamesh Kulkarni wrote: On 19 May 2015 at 14:34, Richard Biener rguent...@suse.de wrote: On Tue, 19 May 2015, Prathamesh Kulkarni wrote: On 18 May 2015 at 20:17, Prathamesh Kulkarni prathamesh.kulka...@linaro.org wrote: On 18 May 2015 at 14:12, Richard Biener rguent...@suse.de wrote: On Sat, 16 May 2015, Prathamesh Kulkarni wrote: Hi, genmatch generates incorrect code for following (artificial) pattern: (for op (plus) op2 (op) (simplify (op @x @y) (op2 @x @y) generated gimple code: http://pastebin.com/h1uau9qB 'op' is not replaced in the generated code on line 33: *res_code = op; I think it would be a better idea to make op2 iterate over same set of operators (op2-substitutes = op-substitutes). I have attached patch for the same. Bootstrap + testing in progress on x86_64-unknown-linux-gnu. OK for trunk after bootstrap+testing completes ? Hmm, but then the example could as well just use 'op'. I think we should instead reject this. Consider (for op (plus minus) (for op2 (op) (simplify ... where it is not clear what would be desired. Simple replacement of 'op's value would again just mean you could have used 'op' in the first place. Doing what you propose would get you (for op (plus minus) (for op2 (plus minus) (simplify ... thus a different iteration. I wonder if we really need is_oper_list flag in user_id ? We can determine if user_id is an operator list if user_id::substitutes is not empty ? After your change yes. That will lose the ability to distinguish between user-defined operator list and list-iterator in for like op/op2, but I suppose we (so far) don't need to distinguish between them ? Well, your change would simply make each list-iterator a (temporary) user-defined operator list as well as the current iterator element (dependent on context - see the nested for example). I think that adds to confusion. AFAIU, the way it's implemented in lower_for, the iterator is handled the same as a user-defined operator list. I was wondering if we should get rid of 'for' altogether and have it replaced by operator-list ? IMHO having two different things - iterator and operator-list is unnecessary and we could brand iterator as a local operator-list. We could extend syntax of 'simplify' to accommodate local operator-lists. So we can say, using an operator-list within 'match' replaces it by corresponding operators in that list. Operator-lists can be global (visible to all patterns), or local to a particular pattern. eg: a) single for (for op (...) (simplify (op ...))) can be written as: (simplify op (...) // define local operator-list op. (op ...)) // proceed here the same way as for lowering global operator list. it's not shorter and it's harder to parse. And you can't share the operator list with multiple simplifies like (for op (...) (simplify ...) (simplify ...)) which is already done I think. I missed that -;) Well we can have a workaround syntax for that if desired. b) multiple iterators: (for op1 (...) op2 (...) (simplify (op1 (op2 ... can be written as: (simplify op1 (...) op2 (...) (op1 (op2 ...))) c) nested for (for op1 (...) (for op2 (...) (simplify (op1 (op2 ... can be written as: (simplify op1 (...) (simplify op2 (...) (op1 (op2 ... My rationale behind removing 'for' is we don't need to distinguish between an operator-list and iterator, and only have an operator-list -;) Also we can reuse parser::parse_operator_list (in parser::parse_for parsing oper-list is duplicated) and get rid of 'parser::parse_for'. We don't need to change lowering, since operator-lists are handled the same way as 'for' (we can keep lowering of simplify::for_vec as it is). Does it sound reasonable ? I dont' think the proposed syntax is simpler or more powerful. Hmm I tend to agree. My motivation to remove 'for' was that it is not more powerful than operator-list and we can re-write 'for' with equivalent operator-list with some syntax changes (like putting operator-list in simplify etc.) So there's only one of doing the same thing. Richard. Thanks, Prathamesh So - can you instead reject this use? I have attached patch for rejecting this use of iterator. Ok for trunk after bootstrap+testing ? Ok. Thanks, Richard. Thanks, Prathamesh Well my intention was to have support for walking operator list in reverse. For eg: (for bitop (bit_and bit_ior) rbitop (bit_ior bit_and) ...) Could be replaced by sth like: (for bitop (bit_and bit_ior) rbitop (~bitop)) ...) where
Re: Refactor gimple_expr_type
On Tue, May 19, 2015 at 6:50 PM, Aditya K hiradi...@msn.com wrote: Date: Tue, 19 May 2015 11:33:16 +0200 Subject: Re: Refactor gimple_expr_type From: richard.guent...@gmail.com To: hiradi...@msn.com CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org On Tue, May 19, 2015 at 12:04 AM, Aditya K hiradi...@msn.com wrote: Date: Mon, 18 May 2015 12:08:58 +0200 Subject: Re: Refactor gimple_expr_type From: richard.guent...@gmail.com To: hiradi...@msn.com CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org On Sun, May 17, 2015 at 5:31 PM, Aditya K hiradi...@msn.com wrote: Date: Sat, 16 May 2015 11:53:57 -0400 From: tbsau...@tbsaunde.org To: hiradi...@msn.com CC: gcc-patches@gcc.gnu.org Subject: Re: Refactor gimple_expr_type On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote: Hi, I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if. Please review this patch. for some reason your mail client seems to be inserting non breaking spaces all over the place. Please either configure it to not do that, or use git send-email for patches. Please see the updated patch. Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type didn't exist btw...) Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it? I can look into refactoring more (if it is not too complicated) since I'm already doing this. Look at each caller - usually they should be fine with using TREE_TYPE (gimple_get_lhs ()) (or a more specific one dependent on what stmts are expected at the place). You might want to first refactor the code else if (code == GIMPLE_COND) gcc_unreachable (); and deal with the fallout in callers (similar for the void_type_node return). Thanks for the suggestions. I looked at the use cases there are 47 usages in different files. That might be a lot of changes I assume, and would take some time. This patch passes bootstrap and make check (although I'm not very confident that my way of make check ran all the regtests) If this patch is okay to merge please do that. I'll continue working on removing gimle_expr_type. Please re-send the patch as attachment, your mailer garbles the text (send mails as non-unicode text/plain). Richard. Thanks, -Aditya Richard. -Aditya Thanks, Richard. gcc/ChangeLog: 2015-05-15 hiraditya hiradi...@msn.com * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if. diff --git a/gcc/gimple.h b/gcc/gimple.h index 95e4fc8..3a83e8f 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -5717,36 +5717,26 @@ static inline tree gimple_expr_type (const_gimple stmt) { enum gimple_code code = gimple_code (stmt); - - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL) + /* In general we want to pass out a type that can be substituted + for both the RHS and the LHS types if there is a possibly + useless conversion involved. That means returning the + original RHS type as far as we can reconstruct it. */ + if (code == GIMPLE_CALL) { - tree type; - /* In general we want to pass out a type that can be substituted - for both the RHS and the LHS types if there is a possibly - useless conversion involved. That means returning the - original RHS type as far as we can reconstruct it. */ - if (code == GIMPLE_CALL) - { - const gcall *call_stmt = as_a const gcall * (stmt); - if (gimple_call_internal_p (call_stmt) - gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE) - type = TREE_TYPE (gimple_call_arg (call_stmt, 3)); - else - type = gimple_call_return_type (call_stmt); - } + const gcall *call_stmt = as_a const gcall * (stmt); + if (gimple_call_internal_p (call_stmt) + gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE) + return TREE_TYPE (gimple_call_arg (call_stmt, 3)); + else + return gimple_call_return_type (call_stmt); + } + else if (code == GIMPLE_ASSIGN) + { + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR) + return TREE_TYPE (gimple_assign_rhs1 (stmt)); else - switch (gimple_assign_rhs_code (stmt)) - { - case POINTER_PLUS_EXPR: - type = TREE_TYPE (gimple_assign_rhs1 (stmt)); - break; - - default: - /* As fallback use the type of the LHS. */ - type = TREE_TYPE (gimple_get_lhs (stmt)); - break; - } - return type; + /* As fallback use the type of the LHS. */ + return TREE_TYPE (gimple_get_lhs (stmt)); } else if (code == GIMPLE_COND) return boolean_type_node; Thanks, -Aditya Thanks, -Aditya gcc/ChangeLog: 2015-05-15 hiraditya hiradi...@msn.com * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if. diff --git a/gcc/gimple.h b/gcc/gimple.h index 95e4fc8..168d3ba 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -5717,35 +5717,28 @@ static inline
Re: ODR merging and implicit typedefs
I bootstrapped/regtested on x86_64-linux the patch bellow. If it will work for Firefox and Chrome I will go ahead with it at least temporarily. Really? This introduced a LTO failure in the gnat.dg testsuite: FAIL: gnat.dg/lto8.adb (internal compiler error) FAIL: gnat.dg/lto8.adb (test for excess errors) WARNING: gnat.dg/lto8.adb compilation failed to produce executable lto1: internal compiler error: in odr_types_equivalent_p, at ipa-devirt.c:1276 0x86a263 odr_types_equivalent_p /home/eric/svn/gcc/gcc/ipa-devirt.c:1276 0x86bf44 odr_types_equivalent_p(tree_node*, tree_node*) /home/eric/svn/gcc/gcc/ipa-devirt.c:1718 0x5c563a warn_type_compatibility_p /home/eric/svn/gcc/gcc/lto/lto-symtab.c:219 0x5c6103 lto_symtab_merge /home/eric/svn/gcc/gcc/lto/lto-symtab.c:336 0x5c6103 lto_symtab_merge_decls_2 /home/eric/svn/gcc/gcc/lto/lto-symtab.c:520 0x5c6103 lto_symtab_merge_decls_1 /home/eric/svn/gcc/gcc/lto/lto-symtab.c:671 0x5c6103 lto_symtab_merge_decls() /home/eric/svn/gcc/gcc/lto/lto-symtab.c:694 0x5bb9cc read_cgraph_and_symbols /home/eric/svn/gcc/gcc/lto/lto.c:2891 0x5bb9cc lto_main() /home/eric/svn/gcc/gcc/lto/lto.c:3277 -- Eric Botcazou
Re: optimization question
On 05/20/2015 01:04 PM, mark maule wrote: Is this one of those areas where if there's a bug in the code all bets are off and your mileage may vary? Yes. Do not access beyond the end of an array: daemons may fly out of your nose. [1] Andrew. [1] https://groups.google.com/forum/?hl=en#!msg/comp.std.c/ycpVKxTZkgw/S2hHdTbv4d8J
[Bug c++/52742] Initializing an array using brace initializer and template parameters
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52742 Jonathan Wakely redi at gcc dot gnu.org changed: What|Removed |Added Known to work||4.8.1 Known to fail||4.7.4, 4.8.0 --- Comment #3 from Jonathan Wakely redi at gcc dot gnu.org --- This is fixed in 4.8.1
[Bug c++/66218] New: [c++-concepts] inconsistent deduction for ‘auto’ with a partial-concept-id in a deduction constraint
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66218 Bug ID: 66218 Summary: [c++-concepts] inconsistent deduction for ‘auto’ with a partial-concept-id in a deduction constraint Product: gcc Version: 6.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c++ Assignee: unassigned at gcc dot gnu.org Reporter: Casey at Carter dot net Target Milestone: --- Created attachment 35576 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35576action=edit testcase.cpp Compiling this correct program with r223444 of the c++-concepts branch: #include type_traits template class T, class U concept bool Same = std::is_sameT, U::value; template class T concept bool C = requires(T t) { { t } - SameT; }; template class constexpr bool f() { return false; } template C constexpr bool f() { return true; } static_assert(fchar(), ); static_assert(fint(), ); static_assert(fdouble(), ); int main() {} produces errors: bug2.cpp:19:22: error: inconsistent deduction for ‘auto’: ‘char’ and then ‘int’ static_assert(fint(), ); ^ bug2.cpp:19:1: error: static assertion failed: static_assert(fint(), ); ^ bug2.cpp:20:25: error: inconsistent deduction for ‘auto’: ‘char’ and then ‘double’ static_assert(fdouble(), ); ^ bug2.cpp:20:1: error: static assertion failed: static_assert(fdouble(), ); ^ It appears that the result of the first deduction is stored in memory instead of being discarded.
[Bug target/52144] ARM should support arm/thumb function attribute to permit different instruction sets in the same source
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52144 chrbr at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED CC||chrbr at gcc dot gnu.org --- Comment #3 from chrbr at gcc dot gnu.org --- patch set posted : (2.1/6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01185.html (2.2/6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01198.html (4/6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01537.html (5.1/6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01539.html (5.2/6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01558.html (6 /6) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01542.html
[Bug middle-end/66214] [6 Regression] ICE verify_type failed with -O0 -g via gen_type_die_with_usage's dwarf2out.c:20250
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66214 Marek Polacek mpolacek at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-05-20 CC||mpolacek at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #2 from Marek Polacek mpolacek at gcc dot gnu.org --- Confirmed.
[Bug sanitizer/62216] UBSan can read past valid memory region
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62216 Marek Polacek mpolacek at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #4 from Marek Polacek mpolacek at gcc dot gnu.org --- https://llvm.org/bugs/show_bug.cgi?id=20721 is fixed now, closing this one as well.
Re: [PATCH, C, ARM] PING c-family builtin export + attribute target (thumb,arm) [2.1/6] respin (5th)
On 05/20/2015 01:19 AM, Christian Bruel wrote: Hi, Could a global reviewer have a look at the c-family part ?, this is blocking for the TARGET_CPU_CPP_BUILTINS macro redefinition in C (arm but probably others) https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01185.html The c-family bits are OK. Sorry I totally ignored this thread not realizing there were things outside the ARM port that needed review. Thanks, Jeff
[Bug c/66220] New: -Wmisleading-indentation false/inconsistent warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220 Bug ID: 66220 Summary: -Wmisleading-indentation false/inconsistent warning Product: gcc Version: 6.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: sirl at gcc dot gnu.org Target Milestone: --- Created attachment 35578 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35578action=edit Testcase to reproduce The following indenting style generates a false warning: int test1(int v) { int res = 28; if (v == 2) { res = 27; } else { res = 18; } return res; } test-indent.c: In function 'test1': test-indent.c:13:5: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] return res; ^ test-indent.c:9:7: note: ...this 'else' clause, but it is not } else ^ Even though I don't like this style, I don't think it's misleading. If you change the 'else' to 'else if ()' the warning goes away, that's why think it's at least inconsistent.
RE: Refactor gimple_expr_type
Date: Wed, 20 May 2015 11:11:52 +0200 Subject: Re: Refactor gimple_expr_type From: richard.guent...@gmail.com To: hiradi...@msn.com CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org On Tue, May 19, 2015 at 6:50 PM, Aditya K hiradi...@msn.com wrote: Date: Tue, 19 May 2015 11:33:16 +0200 Subject: Re: Refactor gimple_expr_type From: richard.guent...@gmail.com To: hiradi...@msn.com CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org On Tue, May 19, 2015 at 12:04 AM, Aditya K hiradi...@msn.com wrote: Date: Mon, 18 May 2015 12:08:58 +0200 Subject: Re: Refactor gimple_expr_type From: richard.guent...@gmail.com To: hiradi...@msn.com CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org On Sun, May 17, 2015 at 5:31 PM, Aditya K hiradi...@msn.com wrote: Date: Sat, 16 May 2015 11:53:57 -0400 From: tbsau...@tbsaunde.org To: hiradi...@msn.com CC: gcc-patches@gcc.gnu.org Subject: Re: Refactor gimple_expr_type On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote: Hi, I have tried to refactor gimple_expr_type to make it more readable. Removed the switch block and redundant if. Please review this patch. for some reason your mail client seems to be inserting non breaking spaces all over the place. Please either configure it to not do that, or use git send-email for patches. Please see the updated patch. Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type didn't exist btw...) Thanks for the review. Do you have any suggestions on how to remove gimple_expr_type. Are there any alternatives to it? I can look into refactoring more (if it is not too complicated) since I'm already doing this. Look at each caller - usually they should be fine with using TREE_TYPE (gimple_get_lhs ()) (or a more specific one dependent on what stmts are expected at the place). You might want to first refactor the code else if (code == GIMPLE_COND) gcc_unreachable (); and deal with the fallout in callers (similar for the void_type_node return). Thanks for the suggestions. I looked at the use cases there are 47 usages in different files. That might be a lot of changes I assume, and would take some time. This patch passes bootstrap and make check (although I'm not very confident that my way of make check ran all the regtests) If this patch is okay to merge please do that. I'll continue working on removing gimle_expr_type. Please re-send the patch as attachment, your mailer garbles the text (send mails as non-unicode text/plain). Sure. I have attached the file. Thanks, -Aditya Richard. Thanks, -Aditya Richard. -Aditya Thanks, Richard. gcc/ChangeLog: 2015-05-15 hiraditya hiradi...@msn.com * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant if. diff --git a/gcc/gimple.h b/gcc/gimple.h index 95e4fc8..3a83e8f 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -5717,36 +5717,26 @@ static inline tree gimple_expr_type (const_gimple stmt) { enum gimple_code code = gimple_code (stmt); - - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL) + /* In general we want to pass out a type that can be substituted + for both the RHS and the LHS types if there is a possibly + useless conversion involved. That means returning the + original RHS type as far as we can reconstruct it. */ + if (code == GIMPLE_CALL) { - tree type; - /* In general we want to pass out a type that can be substituted - for both the RHS and the LHS types if there is a possibly - useless conversion involved. That means returning the - original RHS type as far as we can reconstruct it. */ - if (code == GIMPLE_CALL) - { - const gcall *call_stmt = as_a const gcall * (stmt); - if (gimple_call_internal_p (call_stmt) - gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE) - type = TREE_TYPE (gimple_call_arg (call_stmt, 3)); - else - type = gimple_call_return_type (call_stmt); - } + const gcall *call_stmt = as_a const gcall * (stmt); + if (gimple_call_internal_p (call_stmt) + gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE) + return TREE_TYPE (gimple_call_arg (call_stmt, 3)); + else + return gimple_call_return_type (call_stmt); + } + else if (code == GIMPLE_ASSIGN) + { + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR) + return TREE_TYPE (gimple_assign_rhs1 (stmt)); else - switch (gimple_assign_rhs_code (stmt)) - { - case POINTER_PLUS_EXPR: - type = TREE_TYPE (gimple_assign_rhs1 (stmt)); - break; - - default: - /* As fallback use the type of the LHS. */ - type = TREE_TYPE (gimple_get_lhs (stmt)); - break; - } - return type; + /* As fallback use the type of the LHS. */ + return TREE_TYPE (gimple_get_lhs (stmt)); } else if (code == GIMPLE_COND) return boolean_type_node; Thanks, -Aditya Thanks, -Aditya
Re: [PR c/52952] More precise locations within format strings
On 05/20/2015 02:15 AM, Manuel López-Ibáñez wrote: This is a new version of the patch submitted here: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00663.html but handling (some) escape sequences. I could not figure out a way to re-use the code from libcpp for this, thus I implemented a simple function that given a string and offset in bytes, it computes the visual column corresponding to that offset. The function is very conservative: As soon as something unknown or inconsistent is detected, it returns zero, thus preserving the current behavior. This also preserves the current behavior for non-concatenated tokens. Bootstrapped and regression tested on x86_64-linux-gnu. OK? gcc/testsuite/ChangeLog: 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org PR c/52952 * gcc.dg/redecl-4.c: Update column numbers. * gcc.dg/format/bitfld-1.c: Likewise. * gcc.dg/format/attr-2.c: Likewise. * gcc.dg/format/attr-6.c: Likewise. * gcc.dg/format/attr-7.c (baz): Likewise. * gcc.dg/format/asm_fprintf-1.c: Likewise. * gcc.dg/format/attr-4.c: Likewise. * gcc.dg/format/branch-1.c: Likewise. * gcc.dg/format/c90-printf-1.c: Likewise. Add tests for column locations within strings with embedded escape sequences. gcc/c-family/ChangeLog: 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org PR c/52952 * c-format.c (location_column_from_byte_offset): New. (location_from_offset): New. (struct format_wanted_type): Add offset_loc field. (check_format_info): Move handling of location for extra arguments closer to the point of warning. (check_format_arg): Set offset_is_invalid. (check_format_info_main): Pass the result of location_from_offset to warning_at. (format_type_warning): Pass the result of location_from_offset to warning_at. So if I'm understanding the situation correctly, with this new version behaviour for non-concatenated tokens is preserved which was the only behaviour regression in the prior patch, right? Thus, this version of the patch is strictly an improvement (points to the issue within the format string rather than to the start of the string). Right? I don't particularly like file scoped offset_is_invalid variable. It appears that it's only set within check_format_arg, but it's used from a variety of other locations via location_from_offset. Given the current structure of the code, alternatives would be even uglier. Ok for the trunk. Thanks, Jeff
Re: [gomp4] New builtins, preparation for oacc vector-single
On Wed, May 20, 2015 at 02:01:44PM +0200, Bernd Schmidt wrote: To implement OpenACC vector-single mode, we need to ensure that only one thread out of the group representing a worker executes. The others skip computations but follow along the CFG, so the results of conditional branch decisions must be broadcast to them. The patch below adds a new builtin and nvptx pattern to implement that broadcast functionality. So, is the goal of this that threads in the warp other than the 0th don't do anything except in vectorized regions, where all the threads in the warp participate in the vectorization? Thus, for OpenMP, should the whole warp be a single thread (thus omp_get_thread_num () would be tid.x 5)? If so, is the GCC vectorizer going to be taught about this? Jakub
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On Wed, 20 May 2015, Prathamesh Kulkarni wrote: On 20 May 2015 at 17:01, Richard Biener rguent...@suse.de wrote: On Wed, 20 May 2015, Prathamesh Kulkarni wrote: On 20 May 2015 at 16:17, Prathamesh Kulkarni prathamesh.kulka...@linaro.org wrote: Hi, This patch rejects expanding operator-list to implicit 'for'. On second thoughts, should we reject expansion of operator-list _only_ if it's mixed with 'for' ? At least that, yes. We could define multiple operator-lists in simplify to be the same as enclosing the simplify in 'for' with number of iterators equal to number of operator-lists. So we could allow (define_operator_list op1 ...) (define_operator_list op2 ...) (simplify (op1 (op2 ... ))) is equivalent to: (for temp1 (op1) temp2 (op2) (simplify (temp1 (temp2 ... I think we have patterns like these in match-builtin.pd in the match-and-simplify branch And reject mixing of 'for' and operator-lists. Admittedly the implicit 'for' behavior is not obvious from the syntax -;( Hmm, indeed we have for example /* Optimize pow(1.0,y) = 1.0. */ (simplify (POW real_onep@0 @1) @0) and I remember wanting that implicit for to make those less ugly. So can you rework only rejecting it within for? This patch rejects expanding operator-list inside 'for'. OK for trunk after bootstrap+testing ? Ok. Thanks, Richard. Thanks, Prathamesh Thanks, Richard. Thanks, Prathamesh OK for trunk after bootstrap+testing ? Thanks, Prathamesh -- Richard Biener rguent...@suse.de SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg) -- Richard Biener rguent...@suse.de SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH 3/4] split-stack for powerpc64
On Wed, May 20, 2015 at 8:13 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: On 05/19/2015 07:52 PM, Alan Modra wrote: On Tue, May 19, 2015 at 07:40:15AM -0500, Lynn A. Boger wrote: Questions on the use of the options for split stack: - The way this is implemented, split stack is generated if the target platform supports split stack, on ppc64/ppc64le as well as on x86, and the use of -fno-split-stack doesn't seem to affect it for any of these. Is that the way it should work? I would expect -fno-split-stack to disable it completely. Can you give a testcase to show what you mean? Picking one of the go testsuite programs at random, I see $ gcc/xgcc -Bgcc/ -S -I powerpc64le-linux/libgo /src/gcc-virgin/gcc/testsuite/go.test/test/args.go $ grep morestack args.s bl __morestack bl __morestack $ gcc/xgcc -Bgcc/ -fno-split-stack -S -I powerpc64le-linux/libgo /src/gcc-virgin/gcc/testsuite/go.test/test/args.go $ grep morestack args.s $ That shows -fno-split-stack being honoured. You are correct. I made some mistake in my testing. - The comments say that the gold linker is used for some situations but I don't see any reference in the code to enabling the gold linker for ppc64le, ppc64, or x86. Is the user expected to add the option for the gold linker if needed? At the moment I believe this is true. I have been trying to use the gold linker with your patch and seems to work fine. I added the following to the STACK_SPLIT_SPEC in gcc/gcc.c to enable the gold linker if -fsplit-stack is set, but that will cause problems on systems where the gold linker (and the correct level of binutils for Power) is not available. Is this an absolute requirement to use split stack? Could the configure determine if gold is available and generate this one way or another? --- gcc.c (revision 223217) +++ gcc.c (working copy) @@ -541,7 +541,8 @@ proper position among the other output files. */ libgcc. This is not yet a real spec, though it could become one; it is currently just stuffed into LINK_SPEC. FIXME: This wrapping only works with GNU ld and gold. */ -#define STACK_SPLIT_SPEC %{fsplit-stack: --wrap=pthread_create} +#define STACK_SPLIT_SPEC \ + %{fsplit-stack: --wrap=pthread_create -fuse-ld=gold} #ifndef LIBASAN_SPEC #define STATIC_LIBASAN_LIBS \ Lynn, split-stack does not require Gold linker. This is a non-starter. Gold is necessary for some corner cases of mixing split-stack and non-split-stack modules. - David
Re: [PATCH 1/4] rs6000_stack_info changes for -fsplit-stack
On Tue, May 19, 2015 at 9:09 PM, Alan Modra amo...@gmail.com wrote: On Mon, May 18, 2015 at 02:05:59PM -0400, David Edelsohn wrote: On Sun, May 17, 2015 at 10:54 PM, Alan Modra amo...@gmail.com wrote: This patch changes rs6000_stack_info to keep save areas offsets even when not used. I need lr_save_offset valid for split-stack, and it seemed reasonable to treat the other offsets the same. Not zeroing the offsets requires just one change in code that uses them, the use_backchain_to_restore_sp expression in rs6000_emit_epilogue, not counting the debug_stack_info changes. * config/rs6000/rs6000.c (rs6000_stack_info): Don't zero offsets when not saving registers. (debug_stack_info): Adjust to omit printing unused offsets, as before. (rs6000_emit_epilogue): Adjust use_backchain_to_restore_sp expression. I think that the vrsave_save_offset change may break saving of callee-saved VRs. See PR 55276. I checked. It doesn't break that testcase. PR 55276 was really caused by using vrsave_mask for two purposes, firstly to track which altivec registers have been saved, and secondly to control use of the vrsave stack slot and whether mfvrsave/mtvrsave insns are generated. Patch 2/4 removes this conflation. Okay, but that confirms Patch 1 is not safe without the patch series. - David
[Bug c++/66211] [5/6 Regression] Rvalue conversion in ternary operator causes internal compiler error
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66211 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added CC||jason at redhat dot com Version|unknown |5.1.0 --- Comment #4 from Richard Biener rguenth at gcc dot gnu.org --- Unfortunately this causes FAIL: gcc.dg/tree-ssa/foldcast-1.c scan-tree-dump-times original return x; 2 FAIL: gcc.dg/tree-ssa/pr31261.c scan-tree-dump-times original return b 7; 1 FAIL: gfortran.dg/assumed_type_2.f90 -O0 scan-tree-dump-times original sub_ scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O1 scan-tree-dump-times original sub_ scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O2 scan-tree-dump-times original sub_ scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O3 -fomit-frame-pointer scan-tree-dump -times original sub_scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O3 -fomit-frame-pointer -funroll-all-loo ps -finline-functions scan-tree-dump-times original sub_scalar .(struct t 1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O3 -fomit-frame-pointer -funroll-loops scan-tree-dump-times original sub_scalar .(struct t1 .) array_class_t1 _ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -O3 -g scan-tree-dump-times original s ub_scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/assumed_type_2.f90 -Os scan-tree-dump-times original sub_ scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL: gfortran.dg/c_f_pointer_tests_3.f90 -O scan-tree-dump-times original fptr_array.data = cptr; 1 FAIL: gfortran.dg/c_loc_test_22.f90 -O scan-tree-dump-times original D.[0-9 ]+ = parm.[0-9]+.data;[^;]+ptr[1-4] = D.[0-9]+; 4 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O0 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O0 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O1 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O1 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O2 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O2 scan-tree-dump-times original fgsl _file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer scan-tree-dump -times original fgsl_file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer scan-tree-dump -times original fgsl_file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer -funroll-all-loo ps -finline-functions scan-tree-dump-times original fgsl_file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer -funroll-all-loo ps -finline-functions scan-tree-dump-times original fgsl_file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer -funroll-loops scan-tree-dump-times original fgsl_file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -fomit-frame-pointer -funroll-loops scan-tree-dump-times original fgsl_file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -g scan-tree-dump-times original f gsl_file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -O3 -g scan-tree-dump-times original f gsl_file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -Os scan-tree-dump-times original fgsl _file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_14.f90 -Os scan-tree-dump-times original fgsl _file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_15.f90 -O scan-tree-dump-times original fgsl_ file.[0-9]+.gsl_file = c_ptr.[0-9]+; 1 FAIL: gfortran.dg/c_ptr_tests_15.f90 -O scan-tree-dump-times original fgsl_ file.[0-9]+.gsl_func = c_funptr.[0-9]+; 1 FAIL: gfortran.dg/coarray_31.f90 -O scan-tree-dump original a.y.d._data.dat a = D.[0-9]+.y.d._data.data; FAIL: gfortran.dg/coarray_31.f90 -O scan-tree-dump original a.y.x.data = D. [0-9]+.y.x.data; FAIL: gfortran.dg/coarray_31.f90 -O scan-tree-dump original a.y.z._data.dat a = D.[0-9]+.y.z._data.data; FAIL: gfortran.dg/no_arg_check_2.f90 -O0 scan-tree-dump-times original sub_ scalar .(struct t1 .) array_class_t1_ptr._data.dat 1 FAIL:
Re: Compilers and RCU readers: Once more unto the breach!
Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Additionally, what about the following code? char *x = y ? z : z; Does that extend a dependency chain from z to x? If so, I can imagine a CPU breaking that in practice. I am not seeing this. I would expect the compiler to optimize to something like this: char *x = z; Why? What if y has a potential side-effect (say it makes a function call)? David
[Bug c/66220] -Wmisleading-indentation false/inconsistent warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66220 Marek Polacek mpolacek at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-05-20 CC||dmalcolm at gcc dot gnu.org, ||mpolacek at gcc dot gnu.org Target Milestone|--- |6.0 Ever confirmed|0 |1 --- Comment #1 from Marek Polacek mpolacek at gcc dot gnu.org --- Confirmed.
Re: [Patch, fortran, pr65548, 2nd take, v5] [5/6 Regression] gfc_conv_procedure_call
Le 20/05/2015 10:24, Andre Vehreschild a écrit : Hi Mikael, when I got you right on IRC, then you proposed this change about the pointer attribute: diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 6d565ae..545f778 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -5361,6 +5361,7 @@ gfc_trans_allocate (gfc_code * code) /* Mark the symbol referenced or gfc_trans_assignment will bug. */ newsym-n.sym-attr.referenced = 1; + newsym-n.sym-attr.pointer = 1; e3rhs-expr_type = EXPR_VARIABLE; /* Set the symbols type, upto it was BT_UNKNOWN. */ newsym-n.sym-ts = e3rhs-ts; @@ -5374,7 +5375,6 @@ gfc_trans_allocate (gfc_code * code) /* Set the dimension and pointer attribute for arrays to be on the safe side. */ newsym-n.sym-attr.dimension = 1; - newsym-n.sym-attr.pointer = 1; newsym-n.sym-as = arr; gfc_add_full_array_ref (e3rhs, arr); } Unfortunately does this lead to numerous regressions in the testsuite. For example: ./gfortran.sh -g allocate_alloc_opt_6.f90 -o allocate_alloc_opt_6 Fortraning using ***DEVelopment*** version... allocate_alloc_opt_6.f90:26:0: allocate(t, source=mytype(1.0,2)) ^ internal compiler error: Segmentation fault 0xe09a08 crash_signal /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/toplev.c:380 0xa9cbe1 useless_type_conversion_p(tree_node*, tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimple-expr.c:83 0x10622ae tree_ssa_useless_type_conversion(tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/tree-ssa.c:1178 0x10622fe tree_ssa_strip_useless_type_conversions(tree_node*) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/tree-ssa.c:1190 0xb6c4ae gimplify_expr(tree_node**, gimple_statement_base**, gimple_statement_base**, bool (*)(tree_node*), int) /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimplify.c:7815 0xb5e883 gimplify_modify_expr /home/vehre/Projekte/c_gcc_fortran2003_enhancements_cmbant_freelancer//gcc/gcc/gimplify.c:4644 I therefore came to a more elaborate change (revert the above one before testing this): diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 6d565ae..7b466de 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -5378,6 +5378,10 @@ gfc_trans_allocate (gfc_code * code) newsym-n.sym-as = arr; gfc_add_full_array_ref (e3rhs, arr); } + else if (POINTER_TYPE_P (TREE_TYPE (expr3))) + newsym-n.sym-attr.pointer = 1; + else + newsym-n.sym-attr.value = 1; /* The string length is known to. Set it for char arrays. */ if (e3rhs-ts.type == BT_CHARACTER) newsym-n.sym-ts.u.cl-backend_decl = expr3_len; This patch bootstraps and regtests fine again. Ok to commit? You can drop the else branch. OK to commit with that change. Thanks. Mikael
Re: [PATCH 1/4] rs6000_stack_info changes for -fsplit-stack
On Wed, May 20, 2015 at 09:02:40AM -0400, David Edelsohn wrote: On Tue, May 19, 2015 at 9:09 PM, Alan Modra amo...@gmail.com wrote: On Mon, May 18, 2015 at 02:05:59PM -0400, David Edelsohn wrote: On Sun, May 17, 2015 at 10:54 PM, Alan Modra amo...@gmail.com wrote: This patch changes rs6000_stack_info to keep save areas offsets even when not used. I need lr_save_offset valid for split-stack, and it seemed reasonable to treat the other offsets the same. Not zeroing the offsets requires just one change in code that uses them, the use_backchain_to_restore_sp expression in rs6000_emit_epilogue, not counting the debug_stack_info changes. * config/rs6000/rs6000.c (rs6000_stack_info): Don't zero offsets when not saving registers. (debug_stack_info): Adjust to omit printing unused offsets, as before. (rs6000_emit_epilogue): Adjust use_backchain_to_restore_sp expression. I think that the vrsave_save_offset change may break saving of callee-saved VRs. See PR 55276. I checked. It doesn't break that testcase. PR 55276 was really caused by using vrsave_mask for two purposes, firstly to track which altivec registers have been saved, and secondly to control use of the vrsave stack slot and whether mfvrsave/mtvrsave insns are generated. Patch 2/4 removes this conflation. Okay, but that confirms Patch 1 is not safe without the patch series. No, patch 1/4 is safe by itself. That's what I tested when I said I'd checked. Patch 2/4 doesn't correct a fault in patch 1/4. The explanation I gave re PR 55276 is saying that patch 2/4 prevents the confusion that caused PR 55276 from re-occurring, at least as far as vrsave_mask is concerned. -- Alan Modra Australia Development Lab, IBM
Re: [PATCH 3/4] split-stack for powerpc64
On 05/19/2015 07:52 PM, Alan Modra wrote: On Tue, May 19, 2015 at 07:40:15AM -0500, Lynn A. Boger wrote: Questions on the use of the options for split stack: - The way this is implemented, split stack is generated if the target platform supports split stack, on ppc64/ppc64le as well as on x86, and the use of -fno-split-stack doesn't seem to affect it for any of these. Is that the way it should work? I would expect -fno-split-stack to disable it completely. Can you give a testcase to show what you mean? Picking one of the go testsuite programs at random, I see $ gcc/xgcc -Bgcc/ -S -I powerpc64le-linux/libgo /src/gcc-virgin/gcc/testsuite/go.test/test/args.go $ grep morestack args.s bl __morestack bl __morestack $ gcc/xgcc -Bgcc/ -fno-split-stack -S -I powerpc64le-linux/libgo /src/gcc-virgin/gcc/testsuite/go.test/test/args.go $ grep morestack args.s $ That shows -fno-split-stack being honoured. You are correct. I made some mistake in my testing. - The comments say that the gold linker is used for some situations but I don't see any reference in the code to enabling the gold linker for ppc64le, ppc64, or x86. Is the user expected to add the option for the gold linker if needed? At the moment I believe this is true. I have been trying to use the gold linker with your patch and seems to work fine. I added the following to the STACK_SPLIT_SPEC in gcc/gcc.c to enable the gold linker if -fsplit-stack is set, but that will cause problems on systems where the gold linker (and the correct level of binutils for Power) is not available. Is this an absolute requirement to use split stack? Could the configure determine if gold is available and generate this one way or another? --- gcc.c (revision 223217) +++ gcc.c (working copy) @@ -541,7 +541,8 @@ proper position among the other output files. */ libgcc. This is not yet a real spec, though it could become one; it is currently just stuffed into LINK_SPEC. FIXME: This wrapping only works with GNU ld and gold. */ -#define STACK_SPLIT_SPEC %{fsplit-stack: --wrap=pthread_create} +#define STACK_SPLIT_SPEC \ + %{fsplit-stack: --wrap=pthread_create -fuse-ld=gold} #ifndef LIBASAN_SPEC #define STATIC_LIBASAN_LIBS \
Re: Compilers and RCU readers: Once more unto the breach!
On Wed, May 20, 2015 at 12:47:45PM +0100, Will Deacon wrote: Hi Paul, On Wed, May 20, 2015 at 03:41:48AM +0100, Paul E. McKenney wrote: On Tue, May 19, 2015 at 07:10:12PM -0700, Linus Torvalds wrote: On Tue, May 19, 2015 at 6:57 PM, Linus Torvalds torva...@linux-foundation.org wrote: So I think you're better off just saying that operations designed to drop significant bits break the dependency chain, and give things like 1 and (char *)ptr-(uintptr_t)ptr as examples of such. Making that just an extension of your existing 0 language would seem to be natural. Works for me! I added the following bullet to the list of things that break dependencies: If a pointer is part of a dependency chain, and if the values added to or subtracted from that pointer cancel the pointer value so as to allow the compiler to precisely determine the resulting value, then the resulting value will not be part of any dependency chain. For example, if p is part of a dependency chain, then ((char *)p-(uintptr_t)p)+65536 will not be. Seem reasonable? Whilst I understand what you're saying (the ARM architecture makes these sorts of distinctions when calling out dependency-based ordering), it feels like we're dangerously close to defining the difference between a true and a false dependency. If we want to do this in the context of the C language specification, you run into issues because you need to evaluate the program in order to determine data values in order to determine the nature of the dependency. Indeed, something like this does -not- carry a dependency from the memory_order_consume load to q: char *p, q; p = atomic_load_explicit(gp, memory_order_consume); q = gq + (intptr_t)p - (intptr_t)p; If this was compiled with -O0, ARM and Power might well carry a dependency, but given any optimization, the assembly language would have no hint of any such dependency. So I am not seeing any particular danger. You tackle this above by saying to allow the compiler to precisely determine the resulting value, but I can't see how that can be cleanly fitted into something like the C language specification. I am sure that there will be significant rework from where this document is to language appropriate from the standard. Which is why I am glad that Jens is taking an interest in this, as he is particularly good at producing standards language. Even if it can, then we'd need to reword the ?: treatment that you currently have: If a pointer is part of a dependency chain, and that pointer appears in the entry of a ?: expression selected by the condition, then the chain extends to the result. which I think requires the state of the condition to be known statically if we only want to extend the chain from the selected expression. In the general case, wouldn't a compiler have to assume that the chain is extended from both? In practice, yes, if the compiler cannot determine which expression is selected, it must arrange for the dependency to be carried from either, depending on the run-time value of the condition. But you would have to work pretty hard to create code that did not carry the dependencies as require, not? Additionally, what about the following code? char *x = y ? z : z; Does that extend a dependency chain from z to x? If so, I can imagine a CPU breaking that in practice. I am not seeing this. I would expect the compiler to optimize to something like this: char *x = z; How does this avoid carrying the dependency? Or are you saying that ARM loses the dependency via a store to memory and a later reload? That would be a bit surprising... Humans will understand, and compiler writers won't care. They will either depend on hardware semantics anyway (and argue that your language is tight enough that they don't need to do anything special) or they will turn the consume into an acquire (on platforms that have too weak hardware). Agreed. Plus Core Working Group will hammer out the exact wording, should this approach meet their approval. For the avoidance of doubt, I'm completely behind any attempts to tackle this problem, but I anticipate an uphill struggle getting this text into the C standard. Is your intention to change the carries-a-dependency relation to encompass this change? I completely agree that this won't be easy, but this is the task at hand. And yes, the intent is to change carries-a-dependency, given that the current wording isn't helping anything. ;-) Thanx, Paul