[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: [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
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
[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 ?
[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;
[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.
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
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)
[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: [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; }
[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
[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:
[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: [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
[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); }
[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: 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
[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:
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.
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: [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
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
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: [PATCH i386] Allow sibcalls in no-PLT PIC
On Wed, May 20, 2015 at 5:10 AM, Michael Matz m...@suse.de wrote: 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. We should replace call/jmp *foo@GOTPCREL(%rip) with call/jmp *foo@GOTRELAX(%rip). As an option, we apply -fno-plt to both PIC and non-PIC codes, if foo is externally defined. It will save one indirect branch if GCC is right. If GCC is wrong and foo is defined locally, we get a nop prefix/suffix. We have nothing to lose. -- H.J.
Re: [match-and-simplify] reject expanding operator-list to implicit 'for'
On 20 May 2015 at 18:18, Richard Biener rguent...@suse.de wrote: 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. Well I suppose we could extend it to be mixed with 'for' ? Add the operator lists to the inner-most 'for'. eg: (define_operator_list olist ...) (for op (...) (simplify (op (olist ... would be equivalent to: (for op (...) temp (olist) (simplify (op (olist ... operator-list expansion can be said to simply a short-hand for single 'for' with number of iterators = number of operator-lists. If the operator-lists are enclosed within 'for', add them to the innermost 'for'. Thanks, Prathamesh 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)
[gomp4] Unidirectional branches for nvptx
This adds functionality to the nvptx backend to emit uni-directional branches. The idea is to recognize the previously introduced warp-broadcast pattern; we know that its result is constant across an entire warp of threads, so any value based on that result has the same property. If a jump condition is constant across a warp, add .uni. Committed on gomp-4_0-branch. Bernd Index: gcc/ChangeLog.gomp === --- gcc/ChangeLog.gomp (revision 223443) +++ gcc/ChangeLog.gomp (working copy) @@ -1,3 +1,13 @@ +2015-05-20 Bernd Schmidt ber...@codesourcery.com + + * config/nvptx/nvptx.c: Include dumpfile,h. + (condition_unidirectional_p): New static function. + (nvptx_print_operand): Use it for new 'U' handling. + (nvptx_reorg): Compute warp_equal_pseudos. + * config/nvptx/nvptx.h (struct machine_function): New field + warp_equal_pseudos. + * config/nvptx/nvptx.md (br_true, br_false): Add %U modifier. + 2015-05-19 Bernd Schmidt ber...@codesourcery.com * omp-builtins.def (GOACC_thread_broadcast, Index: gcc/config/nvptx/nvptx.c === --- gcc/config/nvptx/nvptx.c (revision 223443) +++ gcc/config/nvptx/nvptx.c (working copy) @@ -72,6 +72,7 @@ #include cfgrtl.h #include stor-layout.h #include df.h +#include dumpfile.h #include builtins.h /* Record the function decls we've written, and the libfuncs and function @@ -1646,6 +1647,23 @@ nvptx_print_operand_address (FILE *file, nvptx_print_address_operand (file, addr, VOIDmode); } +/* Return true if the value of COND is the same across all threads in a + warp. */ + +static bool +condition_unidirectional_p (rtx cond) +{ + if (CONSTANT_P (cond)) +return true; + if (GET_CODE (cond) == REG) +return cfun-machine-warp_equal_pseudos[REGNO (cond)]; + if (GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMPARE + || GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMM_COMPARE) +return (condition_unidirectional_p (XEXP (cond, 0)) + condition_unidirectional_p (XEXP (cond, 1))); + return false; +} + /* Print an operand, X, to FILE, with an optional modifier in CODE. Meaning of CODE: @@ -1659,7 +1677,9 @@ nvptx_print_operand_address (FILE *file, f -- print a full reg even for something that must always be split t -- print a type opcode suffix, promoting QImode to 32 bits T -- print a type size in bits - u -- print a type opcode suffix without promotions. */ + u -- print a type opcode suffix without promotions. + U -- print .uni if a condition consists only of values equal across all +threads in a warp. */ static void nvptx_print_operand (FILE *file, rtx x, int code) @@ -1732,6 +1752,11 @@ nvptx_print_operand (FILE *file, rtx x, fprintf (file, @!); goto common; +case 'U': + if (condition_unidirectional_p (x)) + fprintf (file, .uni); + break; + case 'c': op_mode = GET_MODE (XEXP (x, 0)); switch (x_code) @@ -1899,6 +1924,12 @@ nvptx_reorg (void) df_clear_flags (DF_LR_RUN_DCE); df_analyze (); + regstat_init_n_sets_and_refs (); + int max_regs = max_reg_num (); + + for (int i = LAST_VIRTUAL_REGISTER + 1; i max_regs; i++) +if (REG_N_SETS (i) == 0 REG_N_REFS (i) == 0) + regno_reg_rtx[i] = const0_rtx; thread_prologue_and_epilogue_insns (); @@ -1911,6 +1942,11 @@ nvptx_reorg (void) siregs.mode = SImode; diregs.mode = DImode; + cfun-machine-warp_equal_pseudos += ggc_cleared_vec_allocchar (max_regs); + + auto_vecunsigned warp_reg_worklist; + for (insn = get_insns (); insn; insn = next) { next = NEXT_INSN (insn); @@ -1919,11 +1955,25 @@ nvptx_reorg (void) || GET_CODE (PATTERN (insn)) == USE || GET_CODE (PATTERN (insn)) == CLOBBER) continue; + qiregs.n_in_use = 0; hiregs.n_in_use = 0; siregs.n_in_use = 0; diregs.n_in_use = 0; extract_insn (insn); + + if (recog_memoized (insn) == CODE_FOR_oacc_thread_broadcastsi + || (GET_CODE (PATTERN (insn)) == SET + CONSTANT_P (SET_SRC (PATTERN (insn) + { + rtx dest = recog_data.operand[0]; + if (REG_P (dest) REG_N_SETS (REGNO (dest)) == 1) + { + cfun-machine-warp_equal_pseudos[REGNO (dest)] = true; + warp_reg_worklist.safe_push (REGNO (dest)); + } + } + enum attr_subregs_ok s_ok = get_attr_subregs_ok (insn); for (int i = 0; i recog_data.n_operands; i++) { @@ -1978,12 +2028,55 @@ nvptx_reorg (void) } } - int maxregs = max_reg_num (); - regstat_init_n_sets_and_refs (); + while (!warp_reg_worklist.is_empty ()) +{ + int regno = warp_reg_worklist.pop (); + + df_ref use = DF_REG_USE_CHAIN (regno); + for (; use; use = DF_REF_NEXT_REG (use)) + { + rtx_insn *insn; + if (!DF_REF_INSN_INFO (use)) + continue; + insn = DF_REF_INSN (use); + if (DEBUG_INSN_P (insn)) + continue; - for (int i = LAST_VIRTUAL_REGISTER + 1; i
[AArch64] PR 63521. define REG_ALLOC_ORDER/HONOR_REG_ALLOC_ORDER
Current IRA still use both target macros in a few places. Tell IRA to use the order we defined rather than with it's own cost calculation. Allocate caller saved first, then callee saved. This is especially useful for LR/x30, as it's free to allocate and is pure caller saved when used in leaf function. Haven't noticed significant impact on benchmarks, but by grepping some keywords like Spilling, Push.*spill etc in ira rtl dump, the number is smaller. OK for trunk? 2015-05-19 Jiong. Wang jiong.w...@arm.com gcc/ PR 63521 * config/aarch64/aarch64.h (REG_ALLOC_ORDER): Define. (HONOR_REG_ALLOC_ORDER): Define. Regards, Jiong diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index bf59e40..0acdf10 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -337,6 +337,31 @@ extern unsigned long aarch64_tune_flags; V_ALIASES(28), V_ALIASES(29), V_ALIASES(30), V_ALIASES(31) \ } +#define REG_ALLOC_ORDER\ +{ \ + /* Reverse order for argument registers. */ \ + 7, 6, 5, 4, 3, 2, 1, 0, \ + /* Other caller-saved registers. */ \ + 8, 9, 10, 11, 12, 13, 14, 15, \ + 16, 17, 18, 30,\ + /* Callee-saved registers. */ \ + 19, 20, 21, 22, 23, 24, 25, 26, \ + 27, 28, \ + /* All other registers. */ \ + 29, 31, \ + /* Reverse order for argument vregisters. */ \ + 39, 38, 37, 36, 35, 34, 33, 32, \ + /* Other caller-saved vregisters. */ \ + 48, 49, 50, 51, 52, 53, 54, 55, \ + 56, 57, 58, 59, 60, 61, 62, 63, \ + /* Callee-saved vregisters. */ \ + 40, 41, 42, 43, 44, 45, 46, 47, \ + /* Other pseudo registers. */ \ + 64, 65, 66 \ +} + +#define HONOR_REG_ALLOC_ORDER 1 + /* Say that the epilogue uses the return address register. Note that in the case of sibcalls, the values used by the epilogue are considered live at the start of the called function. */
Re: [gomp4] New builtins, preparation for oacc vector-single
On 05/20/2015 02:39 PM, Jakub Jelinek wrote: 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? Yes. Thus, for OpenMP, should the whole warp be a single thread (thus omp_get_thread_num () would be tid.x 5)? Do you mean for an OMP port to nvptx? I haven't looked at OpenMP enough to say if or how it could be mapped to GPU hardware; it's not something we intend to do for this project. Bernd
Re: [PATCH] Simple optimization for MASK_STORE.
Hi All, Here is updated patch to optimize mask stores. The main goal of it is to avoid execution of mask store if its mask is zero vector since loads that follow it can be blocked. The following changes were done: 1. A test on sink legality was added - it simply prohibits to cross statements with non-null vdef or vuse. 2. New phi node is created in join block for moved MASK_STORE statements. 3. Test was changed to check that 2 MASK_STORE statements are not moved to the same block. 4. New field was added to loop_vec_info structure to mark loops having MASK_STORE's. Any comments will be appreciated. Yuri. 2015-05-20 Yuri Rumyantsev ysrum...@gmail.com * config/i386/i386.c: Include files stringpool.h and tree-ssanames.h. (ix86_vectorize_is_zero_vector): New function. (TARGET_VECTORIZE_IS_ZERO_VECTOR): New target macro * doc/tm.texi.in: Add @hook TARGET_VECTORIZE_IS_ZERO_VECTOR. * doc/tm.texi: Updated. * target.def (is_zero_vector): New DEFHOOK. * tree-vect-stmts.c : Include tree-into-ssa.h. (vectorizable_mask_load_store): Initialize has_mask_store field. (is_valid_sink): New function. (optimize_mask_stores): New function. * tree-vectorizer.c (vectorize_loops): Invoke optimaze_mask_stores for loops having masked stores. * tree-vectorizer.h (loop_vec_info): Add new has_mask_store field and correspondent macros. (optimize_mask_stores): Update prototype. gcc/testsuite/ChangeLog: * gcc.target/i386/avx2-vect-mask-store-move1.c: New test. patch.2 Description: Binary data
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Wed, May 20, 2015 at 02:10:41PM +0200, Michael Matz wrote: 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. The obvious example where it's useful on x86_64 is a major class: anything where the majority of the callee's data is floating point and thus kept in xmm registers. In that case register pressure is a lot lower, and there's also an obvious class of cross-DSO functions calls you'd be making over and over again: anything from libm. Rich
Re: [PATCH][tree-ssa-math-opts] Expand pow (x, CONST) using square roots when possible
On 18/05/15 11:32, Richard Biener wrote: On Wed, May 13, 2015 at 5:33 PM, Kyrill Tkachov kyrylo.tkac...@foss.arm.com wrote: Hi Richard, On 13/05/15 12:27, Richard Biener wrote: I notice that we don't have a testuite check that the target has a hw sqrt instructions. Would you like me to add one? Or can I make the testcase aarch64-specific? Would be great to have a testsuite check for this. I've committed the patch with r223167. The attached patch adds a testsuite check for hardware sqrt instructions. In this version I've included arm (on the condition that vfp is possible), aarch64, x86_64 and powerpc with vsx. Is this definition ok? I'm particularly not familiar with the powerpc architectures. With this check in place, I've migrated the pow synthesis test from gcc.target/aarch64 to gcc.dg. This test passes on arm-none-eabi, aarch64-none-elf and x86_64-linux. Ok? Ok. Thanks. However, after some discussion on IRC I'd prefer to rename the testsuite check to sqrt_insn so as not to give the impression that it is a runtime hardware check. Also, this version adds an entry in sourcebuild.texi. I'll commit this version in 24 hours unless someone objects. Test still passes on arm, x86_64 and aarch64. Cheers, Kyrill 2015-05-20 Kyrylo Tkachov kyrylo.tkac...@arm.com * lib/target-supports.exp (check_effective_target_sqrt_insn): New check. * gcc.dg/pow-sqrt-synth-1.c: New test. * gcc.target/aarch64/pow-sqrt-synth-1.c: Delete. 2015-05-20 Kyrylo Tkachov kyrylo.tkac...@arm.com * doc/sourcebuild.texi (7.2.3.9 Other hardware attributes): Document sqrt_insn. Thanks, Richard. 2015-05-13 Kyrylo Tkachov kyrylo.tkac...@arm.com * lib/target-supports.exp (check_effective_target_hw_sqrt): New check. * gcc.dg/pow-sqrt-synth-1.c: New test. * gcc.target/aarch64/pow-sqrt-synth-1.c: Delete. commit e35362535c9888daf00d1430e2d3a932d7ece228 Author: Kyrylo Tkachov kyrylo.tkac...@arm.com Date: Wed May 13 16:08:03 2015 +0100 Add testsuite check for hw sqrt. Add generic test for pow sqrt synthesis diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index c6ef40e..abe0779 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1695,6 +1695,9 @@ Target supports FPU instructions. @item non_strict_align Target does not require strict alignment. +@item sqrt_insn +Target has a square root instruction that the compiler can generate. + @item sse Target supports compiling @code{sse} instructions. diff --git a/gcc/testsuite/gcc.dg/pow-sqrt-synth-1.c b/gcc/testsuite/gcc.dg/pow-sqrt-synth-1.c new file mode 100644 index 000..d55b626 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pow-sqrt-synth-1.c @@ -0,0 +1,38 @@ +/* { dg-do compile { target sqrt_insn } } */ +/* { dg-options -fdump-tree-sincos -Ofast --param max-pow-sqrt-depth=8 } */ +/* { dg-additional-options -mfloat-abi=softfp -mfpu=neon-vfpv4 { target arm*-*-* } } */ + +double +foo (double a) +{ + return __builtin_pow (a, -5.875); +} + +double +foof (double a) +{ + return __builtin_pow (a, 0.75f); +} + +double +bar (double a) +{ + return __builtin_pow (a, 1.0 + 0.00390625); +} + +double +baz (double a) +{ + return __builtin_pow (a, -1.25) + __builtin_pow (a, 5.75) - __builtin_pow (a, 3.375); +} + +#define N 256 +void +vecfoo (double *a) +{ + for (int i = 0; i N; i++) +a[i] = __builtin_pow (a[i], 1.25); +} + +/* { dg-final { scan-tree-dump-times synthesizing 7 sincos } } */ +/* { dg-final { cleanup-tree-dump sincos } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/pow-sqrt-synth-1.c b/gcc/testsuite/gcc.target/aarch64/pow-sqrt-synth-1.c deleted file mode 100644 index 52514fb..000 --- a/gcc/testsuite/gcc.target/aarch64/pow-sqrt-synth-1.c +++ /dev/null @@ -1,38 +0,0 @@ -/* { dg-do compile } */ -/* { dg-options -fdump-tree-sincos -Ofast --param max-pow-sqrt-depth=8 } */ - - -double -foo (double a) -{ - return __builtin_pow (a, -5.875); -} - -double -foof (double a) -{ - return __builtin_pow (a, 0.75f); -} - -double -bar (double a) -{ - return __builtin_pow (a, 1.0 + 0.00390625); -} - -double -baz (double a) -{ - return __builtin_pow (a, -1.25) + __builtin_pow (a, 5.75) - __builtin_pow (a, 3.375); -} - -#define N 256 -void -vecfoo (double *a) -{ - for (int i = 0; i N; i++) -a[i] = __builtin_pow (a[i], 1.25); -} - -/* { dg-final { scan-tree-dump-times synthesizing 7 sincos } } */ -/* { dg-final { cleanup-tree-dump sincos } } */ \ No newline at end of file diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 3728927..e3c4416 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -4668,6 +4668,27 @@ proc check_effective_target_vect_call_copysignf { } { return $et_vect_call_copysignf_saved } +# Return 1 if the target supports hardware square root instructions. + +proc check_effective_target_sqrt_insn { } { +global et_sqrt_insn_saved + +if [info exists
[Patch AArch64] PR target/66200 - gcc / libstdc++ TLC for weak memory models.
Hi, Someone privately pointed out that the ARM and AArch64 ports do not define TARGET_RELAXED_ORDERING given that the architecture(s) mandates a weak memory model. This patch fixes it for AArch64, the ARM patch follows in due course after appropriate testing. I will also note that we can define __test_and_acquire as well as __set_and_release and I'm toying with a follow-up patch for the same. Also it may make sense to consider changing the defaults to a safer form, or indeed forcing ports to define some of this rather than allowing for silent wrong code issues. However I'm not about to do so in the context of this patch. Bootstrapped and regression tested on aarch64-none-linux-gnu with no regressions. Ok to apply to trunk and all release branches ? gcc/ PR target/66200 * config/aarch64/aarch64.c (TARGET_RELAXED_ORDERING): Define libstdc++-v3/ PR target/66200 * configure.host (host_cpu): Add aarch64 case. * config/cpu/aarch64/atomic_word.h: New file regards Ramana P.S. It's interesting to note that ia64 doesn't define the barriers which appear to be used in a number of other places than just the constructor guard functions (probably wrongly on the assumption that one doesn't need the barriers elsewhere). I suspect other architectures like MIPS may also be affected by this. commit 414345c424fa020717c6c3083089cd987f3032db Author: Ramana Radhakrishnan ramana.radhakrish...@arm.com Date: Wed May 20 13:55:44 2015 +0100 Add relaxed memory ordering cases. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 7f0cc0d..273aa06 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -11644,6 +11644,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool load, #undef TARGET_SCHED_FUSION_PRIORITY #define TARGET_SCHED_FUSION_PRIORITY aarch64_sched_fusion_priority +#undef TARGET_RELAXED_ORDERING +#define TARGET_RELAXED_ORDERING true + struct gcc_target targetm = TARGET_INITIALIZER; #include gt-aarch64.h diff --git a/libstdc++-v3/config/cpu/aarch64/atomic_word.h b/libstdc++-v3/config/cpu/aarch64/atomic_word.h new file mode 100644 index 000..4afe6ed --- /dev/null +++ b/libstdc++-v3/config/cpu/aarch64/atomic_word.h @@ -0,0 +1,44 @@ +// Low-level type for atomic operations -*- C++ -*- + +// Copyright (C) 2015 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// Under Section 7 of GPL version 3, you are granted additional +// permissions described in the GCC Runtime Library Exception, version +// 3.1, as published by the Free Software Foundation. + +// You should have received a copy of the GNU General Public License and +// a copy of the GCC Runtime Library Exception along with this program; +// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +// http://www.gnu.org/licenses/. + +/** @file atomic_word.h + * This file is a GNU extension to the Standard C++ Library. + */ + +#ifndef _GLIBCXX_ATOMIC_WORD_H +#define _GLIBCXX_ATOMIC_WORD_H 1 + + +typedef int _Atomic_word; + +// This one prevents loads from being hoisted across the barrier; +// in other words, this is a Load-Load acquire barrier. +// This is necessary iff TARGET_RELAXED_ORDERING is defined in tm.h. +#define _GLIBCXX_READ_MEM_BARRIER __asm __volatile (dmb ishld:::memory) + +// This one prevents stores from being sunk across the barrier; in other +// words, a Store-Store release barrier. +#define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile (dmb ishst:::memory) + +#endif diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host index a349ce3..42a45d9 100644 --- a/libstdc++-v3/configure.host +++ b/libstdc++-v3/configure.host @@ -153,6 +153,9 @@ esac # Most can just use generic. # THIS TABLE IS SORTED. KEEP IT THAT WAY. case ${host_cpu} in + aarch64*) +atomic_word_dir=cpu/aarch64 +;; alpha*) atomic_word_dir=cpu/alpha ;;
Re: [patch,gomp4] error on invalid acc loop clauses
On 05/20/2015 01:23 AM, Thomas Schwinge wrote: 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? Jakub made some general comments a couple of weeks ago when you applied our internal changes to gomp-4_0-branch. I was planning on addressing those comments first before requesting the merge to trunk. Then, what about the Fortran front end? Checking already done as well as test coverage existing, similar to C and C++? Fortran is good. 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. What's wrong with this? Fortran does something similar. Besides, this is only temporary until OpenACC 2.5. @@ -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.? I don't think that will help. c_parser_oacc_shape_clause parses 'gang', 'worker' and 'vector' which aren't available to acc parallel or acc kernels. Well, they are, be rigged up the front end to split acc parallel loops and acc kernels loop. @@ -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) +{ + c_parser_error (parser, nested kernels region); +} + + parser-oacc_kernels_region = true; Regarding this... @@ -12843,6 +12898,13 @@ c_parser_oacc_parallel (location_t loc, c_parser *parser, char *p_name) strcat (p_name, parallel); + if (parser-oacc_parallel_region || parser-oacc_kernels_region) +{ + c_parser_error (parser, nested parallel region); +} + + parser-oacc_parallel_region = true; ..., 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?)? The fortran front end is doing this. Also, Joseph told me the front ends should report error messages when possible. I have no problems reverting back to the original behavior though. 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? Yeah, someone needs to clean that up. I tried to keep this patch local to the c and c++ front ends. --- a/gcc/testsuite/c-c++-common/goacc/nesting-fail-1.c +++ b/gcc/testsuite/c-c++-common/goacc/nesting-fail-1.c @@ -7,9 +7,9 @@ f_acc_parallel (void) { #pragma acc parallel { -#pragma acc parallel /* { dg-bogus parallel construct inside of parallel region not implemented
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
Hi, On Wed, 20 May 2015, Rich Felker wrote: 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. The obvious example where it's useful on x86_64 is a major class: Yes, I can construct all kinds of examples where it's useful. That doesn't touch the topic of real-world cases or hard numbers actually comparing the number of hoisted callee addresses, the number that stay hoisted until after register allocation and the number of spills added by hoisting, on some relevant code base, like gcc itself, or SPEC. anything where the majority of the callee's data is floating point and thus kept in xmm registers. This code tends to work on multiple arrays in practice, and hence integer registers are required for all the addresses and offsets and loop counters. In that case register pressure is a lot lower, Register pressure on x86 is never low :) Yes, x86-64 and others are much better in this regard. and there's also an obvious class of cross-DSO functions calls you'd be making over and over again: anything from libm. Ciao, Michael.
Re: [PATCH v2 6/6] i386: Implement asm flag outputs
On 05/15/2015 09:37 AM, Richard Henderson wrote: Version 2 includes proper test cases and documentation. Hopefully the documentation even makes sense. Suggestions and improvements there gratefully appreciated. r~ --- gcc/config/i386/constraints.md | 5 ++ gcc/config/i386/i386.c | 137 +++-- gcc/doc/extend.texi| 76 gcc/testsuite/gcc.target/i386/asm-flag-0.c | 15 gcc/testsuite/gcc.target/i386/asm-flag-1.c | 18 gcc/testsuite/gcc.target/i386/asm-flag-2.c | 16 gcc/testsuite/gcc.target/i386/asm-flag-3.c | 22 + gcc/testsuite/gcc.target/i386/asm-flag-4.c | 20 + gcc/testsuite/gcc.target/i386/asm-flag-5.c | 19 9 files changed, 321 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-0.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-1.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-2.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-3.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-4.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-5.c It all seems to make sense. Obviously you'll need a ChangeLog and the usual testing before committing. I won't stress much if this needs a bit of further tweaking as the kernel folks start to exploit the capability and we find weaknesses in the implementation. What I don't see is any way to know if the target supports asm flag outputs. Are we expecting the kernel folks to do some kind of test then enable/disable based on the result? I'm going to assume the mapping of the constraints to the actual modes and codes is correct. Jeff
Re: [PATCH] 65479 - sanitizer stack trace missing frames past #0 on powerpc64
On 04/20/2015 04:32 PM, Martin Sebor wrote: I also wonder if other targets need -fasynchronous-unwind-tables and whether or not we should just add it unconditionally. I initially only tested powerpc64* and x86_64. I had tried s370 but asan doesn't appear to be built there (is it not supported?) I've now also tried aarch64 (partly because the patch also fixes a latent bug there). Of these targets, only powerpc64* needs the option, and only until the fast unwinding that Jakub mentioned is implemented. I plan to work on it but I wanted to get this simpler fix in first if only so there is a working baseline to start from. Sorry for the deep context switch asan is only supported on a few platforms and I'm pretty sure s390 isn't one of them. Now that I know a bit more from Jakub Yuri's comments, I don't think we should be turning that flag on for all the targets in the testsuite. I was primarily trying to avoid someone else having to go through the same analysis and reach the same conclusion for another port. But I'm less concerned about that now. I totally understand the desire to have a good baseline. Jakub seems to prefer not to make this change since it's a short-lived workaround, which I understand as well. My inclination is to go ahead with flags changes in the testsuite. Cleaner results are, in and of themselves, a good thing. Pulling those lines out once the port is using the fast unwind stuff is easy enough to do. Is libsanitizer maintained in LLVM? If so, we want to minimize divergence, so it may be better to get this approved in LLVM then pick it up via a merge. I can certainly see about submitting the sanitizer bits of the patch to LLVM. It will probably take some time and I was hoping for cleaner powerpc test results in 5.0 (or 5.1 as it sounds like the release will be called). I don't yet have a sense of whether it's preferable to do one or the other or whether it makes sense to do both (i.e., commit the fix now and then merge). This part should definitely hit the LLVM side first, then we can pull it into GCC. So that process should be started. Given this hits 3 distinct pieces of code, do any of them make sense in isolation or do they have to land together as a unit? 65479 (this bug) depends on 65749 (sanitizer stack trace pc off by 1). The asan tests cannot very well be made to pass without fixing the latter bug. Let me know how you'd like to proceed with the patch. That's part of what I was trying to figure out myself :-) So I'll ask the question(s) in a slightly different way and see if that guides us one direction or another. Do the libbacktrace changes make sense independently? ie, are they the right thing to do, even if they don't fix a visible bug? ISTM the answer to both questions is yes. In which case, that part ought to go forward now rather than waiting. The testsuite changes have two components. One is the new flag the other is some slight tweaks to the expected output. I'd hazard a guess that the expected output changes ought to go forward independently too. Again under the same it's the right thing to do, even if it doesn't fix a visible bug. The testsuite flags change isn't as clear cut. I'd think they'd need to visibly improve the test results before they could go in. So they may need to wait (I'm assuming nothing actually shows visible improvement without the libsanitizer fixes). Thoughts? jeff
Re: [PR c/52952] More precise locations within format strings
On 20 May 2015 at 15:33, Jeff Law l...@redhat.com wrote: 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? The new version will also handle most escape sequences correctly and simply preserve the current location for those that are not handled. 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 hope so :) 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. This comes from the previous version of the patch, but it is not necessary anymore, since before using an offset, we try to read the string at the location, and if there is no string, the offset is zero. The variable is set here: if (TREE_CODE (format_tree) == VAR_DECL TREE_CODE (TREE_TYPE (format_tree)) == ARRAY_TYPE (array_init = decl_constant_value (format_tree)) != format_tree TREE_CODE (array_init) == STRING_CST) { /* Extract the string constant initializer. Note that this may include a trailing NUL character that is not in the array (e.g. const char a[3] = foo;). */ array_size = DECL_SIZE_UNIT (format_tree); format_tree = array_init; offset_is_invalid = true; } to handle this case: void foo() { const char a[] = %d ; __builtin_printf(a, 0.5); } in such a case, the location we get is the one of the use of 'a', thus we cannot get at the actual string %d to find an offset. Thus, it preserves the current (not ideal) behavior. OK with offset_is_invalid removed after regtesting? Cheers, Manuel.
RFA: PATCH to use -std=c++98 in stage 1 of bootstrap
I want to explicitly pass -std=c++98 to the compiler used in building stage 1. Does this seem like the right way to do that? Tested x86_64-pc-linux-gnu. commit 97e77ef17e558cdb6d26d440e691fea710e2a2dc Author: Jason Merrill ja...@redhat.com Date: Mon May 18 23:58:41 2015 -0400 * configure.ac: Add -std=c++98 to stage1_cxxflags. * Makefile.in (STAGE1_CXXFLAGS): And substitute it. * configure: Regenerate. diff --git a/Makefile.in b/Makefile.in index c221a0b..c59671a 100644 --- a/Makefile.in +++ b/Makefile.in @@ -489,6 +489,7 @@ STAGEfeedback_CONFIGURE_FLAGS = $(STAGE_CONFIGURE_FLAGS) # overrideable (for a bootstrap build stage1 also builds gcc.info). STAGE1_CFLAGS = @stage1_cflags@ +STAGE1_CXXFLAGS = @stage1_cxxflags@ STAGE1_CHECKING = @stage1_checking@ STAGE1_LANGUAGES = @stage1_languages@ # * We force-disable intermodule optimizations, even if diff --git a/configure b/configure index d804329..37079fb 100755 --- a/configure +++ b/configure @@ -559,6 +559,7 @@ compare_exclusions host_shared stage2_werror_flag stage1_checking +stage1_cxxflags stage1_cflags MAINT MAINTAINER_MODE_FALSE @@ -14755,6 +14756,13 @@ case $build in *) stage1_cflags=-g -J ;; esac ;; esac +stage1_cxxflags=$stage1_cflags +if test $GCC = yes; then + # Build stage 1 in C++98 mode to ensure that a C++98 compiler can still + # start the bootstrap. + stage1_cxxflags=$stage1_cxxflags -std=c++98 +fi + diff --git a/configure.ac b/configure.ac index 4da04b7..2bf3245 100644 --- a/configure.ac +++ b/configure.ac @@ -3476,8 +3476,15 @@ case $build in *) stage1_cflags=-g -J ;; esac ;; esac +stage1_cxxflags=$stage1_cflags +if test $GCC = yes; then + # Build stage 1 in C++98 mode to ensure that a C++98 compiler can still + # start the bootstrap. + stage1_cxxflags=$stage1_cxxflags -std=c++98 +fi AC_SUBST(stage1_cflags) +AC_SUBST(stage1_cxxflags) # Enable --enable-checking in stage1 of the compiler. AC_ARG_ENABLE(stage1-checking,
[patch] libstdc++/66078 __make_move_if_noexcept_iterator should return a constant iterator or a move iterator
As discussed in the thread starting at https://gcc.gnu.org/ml/libstdc++/2014-05/msg00027.html when __make_move_if_noexcept_iterator decides not to move it returns a mutable iterator, which can then result in the wrong constructor being used. This ensures that when not moving we will get a pointer-to-const which will result in the copy constructor being called. Tested powerpc64-linux, committed to trunk. commit 7dc39df13857920ecbb3da1336c2469fdfb30e42 Author: Jonathan Wakely jwak...@redhat.com Date: Fri May 9 13:51:39 2014 +0100 PR libstdc++/66078 * include/bits/stl_iterator.h (__make_move_if_noexcept_iterator): Add overload for pointers. * testsuite/20_util/specialized_algorithms/uninitialized_copy/ 808590.cc: Add -std=gnu++03 switch. * testsuite/20_util/specialized_algorithms/uninitialized_copy/ 808590-cxx11.cc: Copy of 808590.cc to test with -std=gnu++11. * testsuite/23_containers/vector/modifiers/push_back/ strong_guarantee.cc: New. diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index d4ea657..b8e79df 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -1194,6 +1194,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __make_move_if_noexcept_iterator(_Iterator __i) { return _ReturnType(__i); } + // Overload for pointers that matches std::move_if_noexcept more closely, + // returning a constant iterator when we don't want to move. + templatetypename _Tp, typename _ReturnType += typename conditional__move_if_noexcept_cond_Tp::value, + const _Tp*, move_iterator_Tp*::type +inline _ReturnType +__make_move_if_noexcept_iterator(_Tp* __i) +{ return _ReturnType(__i); } + // @} group iterators templatetypename _Iterator diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590-cxx11.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590-cxx11.cc new file mode 100644 index 000..9597a7b --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590-cxx11.cc @@ -0,0 +1,53 @@ +// Copyright (C) 2012-2015 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// http://www.gnu.org/licenses/. + +// { dg-options -std=gnu++11 } + +// This is identical to ./808590.cc but using -std=gnu++11 +// See https://gcc.gnu.org/ml/libstdc++/2014-05/msg00027.html + +#include vector +#include stdexcept + +// 4.4.x only +struct c +{ + void *m; + + c(void* o = 0) : m(o) {} + c(const c r) : m(r.m) {} + + templateclass T +explicit c(T o) : m((void*)0xdeadbeef) { } +}; + +int main() +{ + std::vectorc cbs; + const c cb((void*)0xcafebabe); + + for (int fd = 62; fd 67; ++fd) +{ + cbs.resize(fd + 1); + cbs[fd] = cb; +} + + for (int fd = 62; fd 67; ++fd) +if (cb.m != cbs[fd].m) + throw std::runtime_error(wrong); + return 0; +} diff --git a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590.cc b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590.cc index 53b2d6d..7d20f85 100644 --- a/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590.cc +++ b/libstdc++-v3/testsuite/20_util/specialized_algorithms/uninitialized_copy/808590.cc @@ -15,11 +15,13 @@ // with this library; see the file COPYING3. If not see // http://www.gnu.org/licenses/. +// { dg-options -std=gnu++03 } + #include vector #include stdexcept // 4.4.x only -struct c +struct c { void *m; @@ -30,12 +32,12 @@ struct c explicit c(T o) : m((void*)0xdeadbeef) { } }; -int main() +int main() { std::vectorc cbs; const c cb((void*)0xcafebabe); - for (int fd = 62; fd 67; ++fd) + for (int fd = 62; fd 67; ++fd) { cbs.resize(fd + 1); cbs[fd] = cb; diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/strong_guarantee.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/strong_guarantee.cc new file mode 100644 index 000..461f6ea --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/push_back/strong_guarantee.cc @@ -0,0 +1,88 @@ +// Copyright (C) 2015 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++
[PATCH] -Wmisleading-indentation: Increase test coverage
Add various new tests to Wmisleading-indentation.c: * Ensure that users can use pragma to turn off -Wmisleading-indentation for a range of code. * Add functions demonstrating a variety of indentation styles seen: (a) on http://en.wikipedia.org/wiki/Indent_style (b) via the manpage of GNU indent to verify that -Wmisleading-indentation doesn't emit false positives for these. Tested with: make check-gcc RUNTESTFLAGS=-v -v dg.exp=Wmisleading-indentation.c # of expected passes 42 make check-g++ RUNTESTFLAGS=-v -v dg.exp=Wmisleading-indentation.c # of expected passes 126 In both cases, the # of expected passes remained unchanged, and no new fails were reported. OK for trunk? gcc/testsuite/ChangeLog: * c-c++-common/Wmisleading-indentation.c (fn_32): New. (fn_33_k_and_r_style): New. (fn_33_stroustrup_style): New. (fn_33_allman_style): New. (fn_33_whitesmiths_style): New. (fn_33_horstmann_style): New. (fn_33_ratliff_banner_style): New. (fn_33_lisp_style): New. (fn_34_indent_dash_gnu): New. (fn_34_indent_dash_kr): New. (fn_34_indent_dash_orig): New. (fn_34_indent_linux_style): New. --- .../c-c++-common/Wmisleading-indentation.c | 224 + 1 file changed, 224 insertions(+) diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index 3dbbb8b..6363d71 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -429,3 +429,227 @@ void fn_31 (void) else foo (3); } + +/* Ensure that we can disable the warning. */ +int +fn_32 (int flag) +{ + int x = 4, y = 5; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored -Wmisleading-indentation + if (flag) +x = 3; +y = 2; +#pragma GCC diagnostic pop + + return x * y; +} + +/* Verify that a variety of different indentation styles are supported + without leading to warnings. */ +void +fn_33_k_and_r_style (void) +{ + int i; + for (i = 0; i 10; i++) { +if (flagB) { + foo(0); + foo(1); +} else { + foo(2); + foo(3); +} +foo(4); + } +} + +void +fn_33_stroustrup_style (void) +{ + int i; + for (i = 0; i 10; i++) { +if (flagA) { + foo(0); + foo(1); +} +else { + foo(2); + foo(3); +} +foo(4); + } +} + +void +fn_33_allman_style (void) +{ + int i; + for (i = 0; i 10; i++) + { +if (flagA) +{ + foo(0); + foo(1); +} +else +{ + foo(2); + foo(3); +} +foo(4); + } +} + +void +fn_33_whitesmiths_style (void) +{ +int i; +for (i = 0; i 10; i++) +{ +if (flagA) +{ +foo(0); +foo(1); +} +else +{ +foo(2); +foo(3); +} +foo(4); +} +} + +void +fn_33_horstmann_style (void) +{ +int i; +for (i = 0; i 10; i++) +{ if (flagA) +{ foo(0); +foo(1); +} +else +{ foo(2); +foo(3); +} +foo(4); +} +} + +void +fn_33_ratliff_banner_style (void) +{ +int i; +for (i = 0; i 10; i++) { + if (flagA) { + foo(0); + foo(1); + } + else { +foo(2); +foo(3); +} + foo(4); + } +} + +void +fn_33_lisp_style (void) +{ + int i; + for (i = 0; i 10; i++) { +if (flagA) { +foo(0); +foo(1); } +else { +foo(2); +foo(3); } +foo(4); } +} + +/* A function run through GNU indent with various options. + None of these should lead to warnings. */ + +/* indent -gnu. */ +void +fn_34_indent_dash_gnu (void) +{ + int i; + while (flagA) +for (i = 0; i 10; i++) + { + if (flagB) + { + foo (0); + foo (1); + } + else + { + foo (2); + foo (3); + } + foo (4); + } + foo (5); +} + +/* indent -kr. */ +void fn_34_indent_dash_kr(void) +{ +int i; +while (flagA) + for (i = 0; i 10; i++) { + if (flagB) { + foo(0); + foo(1); + } else { + foo(2); + foo(3); + } + foo(4); + } +foo(5); +} + +/* indent -orig. */ +void +fn_34_indent_dash_orig(void) +{ +int i; +while (flagA) + for (i = 0; i 10; i++) { + if (flagB) { + foo(0); + foo(1); + } else { + foo(2); + foo(3); + } + foo(4); + } +foo(5); +} + +/* Linux style: + indent \ + -nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4 \ + -cli0 -d0 -di1 -nfc1 -i8 -ip0 -l80 -lp -npcs -nprs -npsl -sai \ +
Re: [C++17] Implement N3928 - Extending static_assert
On 05/02/2015 04:16 PM, Ed Smith-Rowland wrote: This extends' static assert to not require a message string. I elected to make this work also for C++11 and C++14 and warn only with -pedantic. I think many people just write static_assert(thing, ); . I took the path of building an empty string in the parser in this case. I wasn't sure if setting message to NULL_TREE would cause sadness later on or not. Hmm. Yes, this technically implements the feature, but my impression of the (non-normative) intent was that they wanted leaving out the string to print the argument expression, in about the same way as #define BOOST_STATIC_ASSERT( B ) static_assert(B, #B) So the patch is OK as is, but you might also look into some libcpp magic to insert a second argument that stringizes the first. Jason
Re: [PATCH] -Wmisleading-indentation: Increase test coverage
On 05/20/2015 09:43 AM, David Malcolm wrote: Add various new tests to Wmisleading-indentation.c: * Ensure that users can use pragma to turn off -Wmisleading-indentation for a range of code. * Add functions demonstrating a variety of indentation styles seen: (a) on http://en.wikipedia.org/wiki/Indent_style (b) via the manpage of GNU indent to verify that -Wmisleading-indentation doesn't emit false positives for these. Tested with: make check-gcc RUNTESTFLAGS=-v -v dg.exp=Wmisleading-indentation.c # of expected passes 42 make check-g++ RUNTESTFLAGS=-v -v dg.exp=Wmisleading-indentation.c # of expected passes 126 In both cases, the # of expected passes remained unchanged, and no new fails were reported. OK for trunk? gcc/testsuite/ChangeLog: * c-c++-common/Wmisleading-indentation.c (fn_32): New. (fn_33_k_and_r_style): New. (fn_33_stroustrup_style): New. (fn_33_allman_style): New. (fn_33_whitesmiths_style): New. (fn_33_horstmann_style): New. (fn_33_ratliff_banner_style): New. (fn_33_lisp_style): New. (fn_34_indent_dash_gnu): New. (fn_34_indent_dash_kr): New. (fn_34_indent_dash_orig): New. (fn_34_indent_linux_style): New. OK. jeff
Fix two more memory leaks in threader
These fix the remaining leaks in the threader that I'm aware of. We failed to properly clean-up when we had to cancel certain jump threading opportunities. So thankfully this wasn't a big leak. Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index fe4dfc4..27435c6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2015-05-20 Jeff Law l...@redhat.com + + * tree-ssa-threadupdate.c (mark_threaded_blocks): Properly + dispose of the jump thread path when the jump threading + opportunity is cancelled. + 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org * diagnostic.c (diagnostic_print_caret_line): Fix off-by-one error diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c index c5b78a4..4bccad0 100644 --- a/gcc/tree-ssa-threadupdate.c +++ b/gcc/tree-ssa-threadupdate.c @@ -2159,9 +2159,16 @@ mark_threaded_blocks (bitmap threaded_blocks) { /* Attach the path to the starting edge if none is yet recorded. */ if ((*path)[0]-e-aux == NULL) -(*path)[0]-e-aux = path; - else if (dump_file (dump_flags TDF_DETAILS)) - dump_jump_thread_path (dump_file, *path, false); + { + (*path)[0]-e-aux = path; + } + else + { + paths.unordered_remove (i); + if (dump_file (dump_flags TDF_DETAILS)) + dump_jump_thread_path (dump_file, *path, false); + delete_jump_thread_path (path); + } } } /* Second, look for paths that have any other jump thread attached to @@ -2185,8 +2192,10 @@ mark_threaded_blocks (bitmap threaded_blocks) else { e-aux = NULL; + paths.unordered_remove (i); if (dump_file (dump_flags TDF_DETAILS)) dump_jump_thread_path (dump_file, *path, false); + delete_jump_thread_path (path); } } }
Re: [PATCH] Fix PR target/65730
On Tue, May 19, 2015 at 8:31 PM, Max Filippov jcmvb...@gmail.com wrote: 2015-05-20 Max Filippov jcmvb...@gmail.com gcc/ * config/xtensa/xtensa.c (init_alignment_context): Replace MULT by BITS_PER_UNIT with ASHIFT by exact_log2 (BITS_PER_UNIT). Approved, please apply.
[PATCH] PR c/66220: Fix false positive from -Wmisleading-indentation
This patch fixes the false positive seen from -Wmisleading-indentation on this code: if (v == 2) { res = 27; } else { res = 18; } return res; ^ FALSE POSITIVE HERE along with similar code seen when I tested it with linux-4.0.3. The patch adds a reject for the case where the guard (else in the above example) is more indented than the things it guards. Doing so uncovered an issue with this testcase: #define FOR_EACH(VAR, START, STOP) for ((VAR) = (START); (VAR) (STOP); (VAR++)) /* { dg-message 36: ...this 'for' clause, but it is not } */ void fn_15 (void) { int i; FOR_EACH (i, 0, 10) /* { dg-message 3: in expansion of macro } */ foo (i); bar (i, i); /* { dg-warning statement is indented as if it were guarded by... } */ } #undef FOR_EACH which would then fail to report the warning, due to it using the location of the for in the definition of macro FOR_EACH, rather than the location of the FOR_EACH (i, 0, 10). The fix for this is to use expand_location to get file/line/col of each thing, rather than expand_location_to_spelling_point. With that, all testcases in Wmisleading-indentation.c pass (including the new ones posted in https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01846.html ). OK for trunk if it passes bootstrapregrest? (only tested with make check-gcc RUNTESTFLAGS=dg.exp=Wmisleading-indentation.c make check-g++ RUNTESTFLAGS=dg.exp=Wmisleading-indentation.c so far) gcc/c-family/ChangeLog: PR c/66220: * c-indentation.c (should_warn_for_misleading_indentation): Use expand_location rather than expand_location_to_spelling_point. Don't warn if the guarding statement is more indented than the next/body stmts. gcc/testsuite/ChangeLog: PR c/66220: * c-c++-common/Wmisleading-indentation.c (fn_35): New. (fn_36): New. --- gcc/c-family/c-indentation.c | 26 ++- .../c-c++-common/Wmisleading-indentation.c | 38 ++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 9aeebae..1e3a6d8 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -230,10 +230,8 @@ should_warn_for_misleading_indentation (location_t guard_loc, if (next_tok_type == CPP_SEMICOLON) return false; - expanded_location body_exploc -= expand_location_to_spelling_point (body_loc); - expanded_location next_stmt_exploc -= expand_location_to_spelling_point (next_stmt_loc); + expanded_location body_exploc = expand_location (body_loc); + expanded_location next_stmt_exploc = expand_location (next_stmt_loc); /* They must be in the same file. */ if (next_stmt_exploc.file != body_exploc.file) @@ -257,8 +255,7 @@ should_warn_for_misleading_indentation (location_t guard_loc, ^ DON'T WARN HERE. */ if (next_stmt_exploc.line == body_exploc.line) { - expanded_location guard_exploc - = expand_location_to_spelling_point (guard_loc); + expanded_location guard_exploc = expand_location (guard_loc); if (guard_exploc.file != body_exploc.file) return true; if (guard_exploc.line body_exploc.line) @@ -299,6 +296,15 @@ should_warn_for_misleading_indentation (location_t guard_loc, #endif bar (); ^ DON'T WARN HERE + +if (flag) { + foo (); +} else +{ + bar (); +} +baz (); +^ DON'T WARN HERE */ if (next_stmt_exploc.line body_exploc.line) { @@ -319,14 +325,18 @@ should_warn_for_misleading_indentation (location_t guard_loc, /* Don't warn if they aren't aligned on the same column as the guard itself (suggesting autogenerated code that doesn't bother indenting at all). */ - expanded_location guard_exploc - = expand_location_to_spelling_point (guard_loc); + expanded_location guard_exploc = expand_location (guard_loc); unsigned int guard_vis_column; if (!get_visual_column (guard_exploc, guard_vis_column)) return false; if (guard_vis_column == body_vis_column) return false; + /* PR 66220: Don't warn if the guarding statement is more +indented than the next/body stmts. */ + if (guard_vis_column body_vis_column) + return false; + /* Don't warn if there is multiline preprocessor logic between the two statements. */ if (detect_preprocessor_logic (body_exploc, next_stmt_exploc)) diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index 6363d71..443e3dd 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -653,3 +653,41 @@ void
Re: [PATCH 3/4] split-stack for powerpc64
Anytime go code built with gccgo is linked against libraries built with gcc (without split stack) there could be mixing of split stack and non split stack code. I think that will be a common case. My understanding is that if you don't use the gold linker in these cases, it is possible that the app could fail and it won't be clear why. Maybe the gold linker isn't required to make it work for most cases, but it will fail for some cases without it. On 05/20/2015 07:58 AM, David Edelsohn wrote: 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: Fix two more memory leaks in threader
On Wed, May 20, 2015 at 10:36:25AM -0600, Jeff Law wrote: These fix the remaining leaks in the threader that I'm aware of. We failed to properly clean-up when we had to cancel certain jump threading opportunities. So thankfully this wasn't a big leak. Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. Jeff diff --git a/gcc/ChangeLog b/gcc/ChangeLog index fe4dfc4..27435c6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2015-05-20 Jeff Law l...@redhat.com + + * tree-ssa-threadupdate.c (mark_threaded_blocks): Properly + dispose of the jump thread path when the jump threading + opportunity is cancelled. + 2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org * diagnostic.c (diagnostic_print_caret_line): Fix off-by-one error diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c index c5b78a4..4bccad0 100644 --- a/gcc/tree-ssa-threadupdate.c +++ b/gcc/tree-ssa-threadupdate.c @@ -2159,9 +2159,16 @@ mark_threaded_blocks (bitmap threaded_blocks) { /* Attach the path to the starting edge if none is yet recorded. */ if ((*path)[0]-e-aux == NULL) -(*path)[0]-e-aux = path; - else if (dump_file (dump_flags TDF_DETAILS)) - dump_jump_thread_path (dump_file, *path, false); + { + (*path)[0]-e-aux = path; + } Why the braces around single stmt if body? Also, the indentation seems to be weird. Jakub
Re: [v3 patch] Fix some Filesystem TS operations
On 15/05/15 19:37 +0100, Jonathan Wakely wrote: Testing revealed a few bugs in how I handled paths that don't exist. The new __gnu_test::nonexistent_path() function is a bit hacky but should be good enough for the testsuite. This makes it even hackier but avoids linker warnings for using the evil tempnam() function. I know it should use snprintf not sprintf, but that depends on _GLIBCXX_USE_C99 which may not be defined (because we incorrectly test for a C99 lib using -std=gnu++98, which I'm going to fix). commit fc7f3808e940243362d29acde4a09ae90aa0df81 Author: Jonathan Wakely jwak...@redhat.com Date: Wed May 20 18:17:56 2015 +0100 * testsuite/util/testsuite_fs.h (nonexistent_path): Don't use tempnam. diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h index f404a7a..3873a60 100644 --- a/libstdc++-v3/testsuite/util/testsuite_fs.h +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h @@ -26,10 +26,8 @@ #include iostream #include string #include cstdio -#if defined(_GNU_SOURCE) || _XOPEN_SOURCE = 500 || _POSIX_C_SOURCE = 200112L -# include stdlib.h -# include unistd.h -#endif +#include stdlib.h +#include unistd.h namespace __gnu_test { @@ -84,12 +82,9 @@ namespace __gnu_test ::close(fd); p = tmp; #else -char* tmp = tempnam(., test.); -if (!tmp) - throw std::experimental::filesystem::filesystem_error(tempnam failed, - std::error_code(errno, std::generic_category())); -p = tmp; -::free(tmp); +char buf[64]; +std::sprintf(buf, test.%lu, (unsigned long)::getpid()); +p = buf; #endif return p; }
Re: [PATCH v2 6/6] i386: Implement asm flag outputs
On 05/20/2015 09:21 AM, Jeff Law wrote: What I don't see is any way to know if the target supports asm flag outputs. Are we expecting the kernel folks to do some kind of test then enable/disable based on the result? I'd forgotten that we'd talked about a cpp symbol. I'll add that. r~
[obvious fix] fix off-by-one error when printing the caret character
It seems I made an off-by-one error in my last patch for multiple locations. This only affected the position of the caret character, which we don't test (since the testsuite uses -fno-diagnostics-show-caret). Fixed thusly and added a comment to remind me and others that locations start at 1, but we still want to start at 0 (but not start at 0 and add an extra space like I did before). Cheers, Manuel. Index: ChangeLog === --- ChangeLog (revision 223445) +++ ChangeLog (working copy) @@ -1,3 +1,8 @@ +2015-05-20 Manuel López-Ibáñez m...@gcc.gnu.org + + * diagnostic.c (diagnostic_print_caret_line): Fix off-by-one error + when printing the caret character. + 2015-05-20 Marek Polacek pola...@redhat.com * cfgexpand.c (expand_debug_expr): Use UNARY_CLASS_P. Index: diagnostic.c === --- diagnostic.c(revision 223445) +++ diagnostic.c(working copy) @@ -420,7 +420,8 @@ int caret_min = cmin == xloc1.column ? caret1 : caret2; int caret_max = cmin == xloc1.column ? caret2 : caret1; - pp_space (context-printer); + /* cmin is = 1, but we indent with an extra space at the start like + we did above. */ int i; for (i = 0; i cmin; i++) pp_space (context-printer);
Re: [PR c/52952] More precise locations within format strings
On 05/20/2015 10:08 AM, Manuel López-Ibáñez wrote: 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. This comes from the previous version of the patch, but it is not necessary anymore, since before using an offset, we try to read the string at the location, and if there is no string, the offset is zero. Ah, well, if it isn't needed, then let's kill it :-) The variable is set here: if (TREE_CODE (format_tree) == VAR_DECL TREE_CODE (TREE_TYPE (format_tree)) == ARRAY_TYPE (array_init = decl_constant_value (format_tree)) != format_tree TREE_CODE (array_init) == STRING_CST) { /* Extract the string constant initializer. Note that this may include a trailing NUL character that is not in the array (e.g. const char a[3] = foo;). */ array_size = DECL_SIZE_UNIT (format_tree); format_tree = array_init; offset_is_invalid = true; } to handle this case: void foo() { const char a[] = %d ; __builtin_printf(a, 0.5); } in such a case, the location we get is the one of the use of 'a', thus we cannot get at the actual string %d to find an offset. Thus, it preserves the current (not ideal) behavior. OK with offset_is_invalid removed after regtesting? Yes. jeff
Re: [PATCH v2 6/6] i386: Implement asm flag outputs
Well, these kinds of asm are inherently target specific, but I did already ask for a cpp symbol to indicate this faculty us available. On May 20, 2015 9:21:07 AM PDT, Jeff Law l...@redhat.com wrote: On 05/15/2015 09:37 AM, Richard Henderson wrote: Version 2 includes proper test cases and documentation. Hopefully the documentation even makes sense. Suggestions and improvements there gratefully appreciated. r~ --- gcc/config/i386/constraints.md | 5 ++ gcc/config/i386/i386.c | 137 +++-- gcc/doc/extend.texi| 76 gcc/testsuite/gcc.target/i386/asm-flag-0.c | 15 gcc/testsuite/gcc.target/i386/asm-flag-1.c | 18 gcc/testsuite/gcc.target/i386/asm-flag-2.c | 16 gcc/testsuite/gcc.target/i386/asm-flag-3.c | 22 + gcc/testsuite/gcc.target/i386/asm-flag-4.c | 20 + gcc/testsuite/gcc.target/i386/asm-flag-5.c | 19 9 files changed, 321 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-0.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-1.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-2.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-3.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-4.c create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-5.c It all seems to make sense. Obviously you'll need a ChangeLog and the usual testing before committing. I won't stress much if this needs a bit of further tweaking as the kernel folks start to exploit the capability and we find weaknesses in the implementation. What I don't see is any way to know if the target supports asm flag outputs. Are we expecting the kernel folks to do some kind of test then enable/disable based on the result? I'm going to assume the mapping of the constraints to the actual modes and codes is correct. Jeff -- Sent from my mobile phone. Please pardon brevity and lack of formatting.
[patch] testsuite enable PIE tests on FreeBSD
Hi, the attached patch enables some PIE tests on FreeBSD. Ok for trunk? Thanks, Andreas 2015-05-20 Andreas Tobler andre...@gcc.gnu.org * gcc.target/i386/pr32219-1.c: Enable test on FreeBSD. * gcc.target/i386/pr32219-2.c: Likewise. * gcc.target/i386/pr32219-3.c: Likewise. * gcc.target/i386/pr32219-4.c: Likewise. * gcc.target/i386/pr32219-5.c: Likewise. * gcc.target/i386/pr32219-6.c: Likewise * gcc.target/i386/pr32219-7.c: Likewise. * gcc.target/i386/pr32219-8.c: Likewise. * gcc.target/i386/pr39013-1.c: Likewise. * gcc.target/i386/pr39013-2.c: Likewise. * gcc.target/i386/pr64317.c: Likewise. Index: gcc.target/i386/pr32219-1.c === --- gcc.target/i386/pr32219-1.c (revision 223412) +++ gcc.target/i386/pr32219-1.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpie } */ /* Initialized common symbol with -fpie. */ Index: gcc.target/i386/pr32219-2.c === --- gcc.target/i386/pr32219-2.c (revision 223412) +++ gcc.target/i386/pr32219-2.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpic } */ /* Common symbol with -fpic. */ Index: gcc.target/i386/pr32219-3.c === --- gcc.target/i386/pr32219-3.c (revision 223412) +++ gcc.target/i386/pr32219-3.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpie } */ /* Weak common symbol with -fpie. */ Index: gcc.target/i386/pr32219-4.c === --- gcc.target/i386/pr32219-4.c (revision 223412) +++ gcc.target/i386/pr32219-4.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpic } */ /* Weak common symbol with -fpic. */ Index: gcc.target/i386/pr32219-5.c === --- gcc.target/i386/pr32219-5.c (revision 223412) +++ gcc.target/i386/pr32219-5.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpie } */ /* Initialized symbol with -fpie. */ Index: gcc.target/i386/pr32219-6.c === --- gcc.target/i386/pr32219-6.c (revision 223412) +++ gcc.target/i386/pr32219-6.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpic } */ /* Initialized symbol with -fpic. */ Index: gcc.target/i386/pr32219-7.c === --- gcc.target/i386/pr32219-7.c (revision 223412) +++ gcc.target/i386/pr32219-7.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpie } */ /* Weak initialized symbol with -fpie. */ Index: gcc.target/i386/pr32219-8.c === --- gcc.target/i386/pr32219-8.c (revision 223412) +++ gcc.target/i386/pr32219-8.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target *-*-linux* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* } } */ /* { dg-options -O2 -fpic } */ /* Weak initialized symbol with -fpic. */ Index: gcc.target/i386/pr39013-1.c === --- gcc.target/i386/pr39013-1.c (revision 223412) +++ gcc.target/i386/pr39013-1.c (working copy) @@ -1,5 +1,5 @@ /* PR target/39013 */ -/* { dg-do compile { target *-*-linux* *-*-gnu* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* *-*-gnu* } } */ /* { dg-options -O2 -fpie -std=gnu89 } */ inline int foo (void); Index: gcc.target/i386/pr39013-2.c === --- gcc.target/i386/pr39013-2.c (revision 223412) +++ gcc.target/i386/pr39013-2.c (working copy) @@ -1,5 +1,5 @@ /* PR target/39013 */ -/* { dg-do compile { target *-*-linux* *-*-gnu* } } */ +/* { dg-do compile { target *-*-freebsd* *-*-linux* *-*-gnu* } } */ /* { dg-options -O2 -fpie -std=gnu99 } */ inline int foo (void); /* { dg-warning declared but never defined } */ Index: gcc.target/i386/pr64317.c === --- gcc.target/i386/pr64317.c (revision 223412) +++ gcc.target/i386/pr64317.c (working copy) @@
Re: [PATCH] Contribute FreeBSD unwind support (x86_64 and x86)
On 20.05.15 21:49, John Marino wrote: I have maintained unwind support for FreeBSD i386 and x86_64 in my gnat-aux repository for many years (I created it). I've always intended on contributing it back to GCC, but I never got around to proving it worked until now. The version I've been using actually has two flavors: FreeBSD 8 and below and FreeBSD 9 and above. However, the last of the FreeBSD 8 releases reaches EOL at the end of June so the unwind support I've attached here drops the FreeBSD 8 variation for simplicity's sake. I was under the impression that MD unwinding was used for more than just GNAT but it looks like that impression was wrong. When I ran the testsuite, the only tests affected were Ada tests. It is, libjava uses it. Note that I provided a similar unwind support for DragonFly a few months ago. Please consider applying the attached patch to gcc trunk. (copy of patch found here: http://leaf.dragonflybsd.org/~marino/freebsd/freebsd-unwind-support.diff ) Suggested text for libgcc/ChangeLog: 2015-05-XX John Marino gnu...@marino.st * config.host (i[34567]86-*-freebsd*, x86_64-*-freebsd*): Set md_unwind_header * config/i386/freebsd-unwind.h: New. Also please recall that my copyright assignment to FSF is in order! Testing patch locally now. Thanks, Andreas
Re: [patch 10/10] debug-early merge: compiler proper
commit 8824b5ecba26cef065e47b34609c72677c3c36fc Author: Aldy Hernandez al...@redhat.com Date: Wed May 20 16:31:14 2015 -0400 Set DECL_IGNORED_P on temporary arrays created in the switch conversion pass. diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c index 6b68a16..a4bcdba 100644 --- a/gcc/tree-switch-conversion.c +++ b/gcc/tree-switch-conversion.c @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree arr_index_type, DECL_ARTIFICIAL (decl) = 1; TREE_CONSTANT (decl) = 1; TREE_READONLY (decl) = 1; + DECL_IGNORED_P (decl) = 1; varpool_node::finalize_decl (decl); This looks obvious enough to me. Technically speaking the array type constructed probalby should be TREE_ARTIFICAIL, but probably it does not matter. If you grep for finalize_decl, there are several other calls: asan.c: varpool_node::finalize_decl (var); asan.c: varpool_node::finalize_decl (var); cgraphbuild.c: varpool_node::finalize_decl (decl); cgraphunit.c:- varpool_finalize_decl cgraphunit.c: varpool_node::finalize_decl (decl); cgraphunit.c:varpool_node::finalize_decl (tree decl) coverage.c: varpool_node::finalize_decl (var); coverage.c: varpool_node::finalize_decl (var); coverage.c: varpool_node::finalize_decl (fn_info_ary); coverage.c: varpool_node::finalize_decl (gcov_info_var); omp-low.c:varpool_node::finalize_decl (t); omp-low.c:varpool_node::finalize_decl (t); omp-low.c:varpool_node::finalize_decl (decl); omp-low.c: varpool_node::finalize_decl (vars_decl); omp-low.c: varpool_node::finalize_decl (funcs_decl); passes.c: varpool_node::finalize_decl (decl); tree-chkp.c: varpool_node::finalize_decl (var); tree-chkp.c: varpool_node::finalize_decl (bnd_var); tree-profile.c: varpool_node::finalize_decl (ic_void_ptr_var); tree-profile.c: varpool_node::finalize_decl (ic_gcov_type_ptr_var); tree-switch-conversion.c: varpool_node::finalize_decl (decl); ubsan.c: varpool_node::finalize_decl (decl); ubsan.c: varpool_node::finalize_decl (var); ubsan.c: varpool_node::finalize_decl (array); varasm.c: varpool_node::finalize_decl (decl); varpool.c: Unlike finalize_decl function is intended to be used varpool.c: varpool_node::finalize_decl (decl); I would say most of them needs similar treatment (I am not 100% sure about OMP ones that may be user visible) Honza fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
Re-enable shadd insns on the PA
This is the first in a series of patches to fix the fallout from recent combiner changes in how shift-add style insns are canonicalized. This patch effectively just adds a new shift-add insn to the PA port. The old shift-add insn stays for now, but will be removed in a follow-up once I'm confident all the ways we might generate that insn have been updated on the trunk. My sole remaining PA is dead and the one I had access to via parisc-linux.org doesn't seem to be responding. So testing is limited. The goal is to generate the same code as we had before the combiner changes across the 300+ .i files for a gcc-4.7.3 build on the PA. This patch (in conjuction with several others) has accomplished that goal. I'm obviously cherry picking individual hunks out of that work, cleaning them up and committing them. This patch also creates a hppa target testsuite. Not that it'll ever be all that large, we might as well not pollute the rest of the suite with PA specific tests. Installed on the trunk. Jeff commit 7176b30ff9f6b2ea07950d392cad0876123dc5e4 Author: Jeff Law l...@redhat.com Date: Wed May 20 15:07:34 2015 -0600 2015-05-20 Jeff Law l...@redhat.com * config/pa/pa.c (pa_print_operand): New 'o' output modifier. (pa_mem_shadd_constant_p): Renamed from pa_shadd_constant_p. (pa_shadd_constant_p): Allow constants for shadd insns rather than valid scaling constants for memory addresses. * config/pa/pa-protos.h (pa_mem_shadd_constant_p): Add prototype. * config/pa/predicates.md (mem_shadd_operand): New predicate. * config/pa/pa.md (shift-add insns using MULT): Use mem_shadd_operand. (shift-add insns using ASHIFT): New patterns. * gcc.target/hppa/hppa.exp: New target test driver. * gcc.target/hppa/shadd-1.c: New test. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5bcbcb4..490386e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2015-05-20 Jeff Law l...@redhat.com + + * config/pa/pa.c (pa_print_operand): New 'o' output modifier. + (pa_mem_shadd_constant_p): Renamed from pa_shadd_constant_p. + (pa_shadd_constant_p): Allow constants for shadd insns rather + than valid scaling constants for memory addresses. + * config/pa/pa-protos.h (pa_mem_shadd_constant_p): Add prototype. + * config/pa/predicates.md (mem_shadd_operand): New predicate. + * config/pa/pa.md (shift-add insns using MULT): Use mem_shadd_operand. + (shift-add insns using ASHIFT): New patterns. + 2015-05-20 Mikhail Maltsev malts...@gmail.com * bb-reorder.c (set_edge_can_fallthru_flag): Use rtx_jump_insn where diff --git a/gcc/config/pa/pa-protos.h b/gcc/config/pa/pa-protos.h index 4a44dab..58cc463 100644 --- a/gcc/config/pa/pa-protos.h +++ b/gcc/config/pa/pa-protos.h @@ -85,6 +85,7 @@ extern int pa_and_mask_p (unsigned HOST_WIDE_INT); extern int pa_cint_ok_for_move (HOST_WIDE_INT); extern int pa_ior_mask_p (unsigned HOST_WIDE_INT); extern int pa_ldil_cint_p (HOST_WIDE_INT); +extern int pa_mem_shadd_constant_p (int); extern int pa_shadd_constant_p (int); extern int pa_zdepi_cint_p (unsigned HOST_WIDE_INT); diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c index cfdafa6..f99cf33 100644 --- a/gcc/config/pa/pa.c +++ b/gcc/config/pa/pa.c @@ -5242,6 +5242,11 @@ pa_print_operand (FILE *file, rtx x, int code) gcc_assert (GET_CODE (x) == CONST_INT); fprintf (file, HOST_WIDE_INT_PRINT_DEC, 32 - (INTVAL (x) 31)); return; +case 'o': + gcc_assert (GET_CODE (x) == CONST_INT + (INTVAL (x) == 1 || INTVAL (x) == 2 || INTVAL (x) == 3)); + fprintf (file, %d, INTVAL (x)); + return; case 'O': gcc_assert (GET_CODE (x) == CONST_INT exact_log2 (INTVAL (x)) = 0); fprintf (file, %d, exact_log2 (INTVAL (x))); @@ -8729,11 +8734,22 @@ pa_fmpysuboperands (rtx *operands) } /* Return 1 if the given constant is 2, 4, or 8. These are the valid + constants for a MULT embedded inside a memory address. */ +int +pa_mem_shadd_constant_p (int val) +{ + if (val == 2 || val == 4 || val == 8) +return 1; + else +return 0; +} + +/* Return 1 if the given constant is 1, 2, or 3. These are the valid constants for shadd instructions. */ int pa_shadd_constant_p (int val) { - if (val == 2 || val == 4 || val == 8) + if (val == 1 || val == 2 || val == 3) return 1; else return 0; diff --git a/gcc/config/pa/pa.md b/gcc/config/pa/pa.md index cc077a4..73c8f6b 100644 --- a/gcc/config/pa/pa.md +++ b/gcc/config/pa/pa.md @@ -6337,7 +6337,7 @@ (define_insn [(set (match_operand:SI 0 register_operand =r) (plus:SI (mult:SI (match_operand:SI 2 register_operand r) - (match_operand:SI 3 shadd_operand )) + (match_operand:SI 3 mem_shadd_operand )) (match_operand:SI 1 register_operand r)))] {sh%O3addl
Re: [PATCH] [PATCH][ARM] Fix sibcall testcases.
On Wed, 20 May 2015, Alex Velenko wrote: Hi, This patch prevents arm_thumb1_ok XPASS in sibcall-3.c and sibcall-4.c testcases. Sibcalls are not ok for Thumb1 and testcases need to be fixed. arm_thumb1_ok means this is an ARM target where -mthumb causes Thumb-1 to be used. It only ever makes sense to use it in tests that use an explicit -mthumb, which these tests don't. If you want to check is this test being built for Thumb-1 by the multilib options, use arm_thumb1. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch 10/10] debug-early merge: compiler proper
How does this version, which has been committed to the debug-early branch, look? One more thing Richi. I merged trunk into the branch once again, and Go broke. I tracked it down to a temporary that was being created late that IMO shouldn't even get debug info. The fact that it gets created with create_tmp_var_name() in the first place is suspect. The problem is actually the type, which doesn't even get passed through rest_of_type* or the debug_hooks-type_decl(). However, I see no reason to have these temporary variables even get fed to the debugger, so I'm marking them as DECL_IGNORED_P. If you want I can repost the whole compiler proper patch, but this is a small enough change that y'all can just wave through. I've committed the snippet below to the branch. Everything else is as it was. Branch retested on x86-64 Linux and has been merged with trunk. commit 8824b5ecba26cef065e47b34609c72677c3c36fc Author: Aldy Hernandez al...@redhat.com Date: Wed May 20 16:31:14 2015 -0400 Set DECL_IGNORED_P on temporary arrays created in the switch conversion pass. diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c index 6b68a16..a4bcdba 100644 --- a/gcc/tree-switch-conversion.c +++ b/gcc/tree-switch-conversion.c @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree arr_index_type, DECL_ARTIFICIAL (decl) = 1; TREE_CONSTANT (decl) = 1; TREE_READONLY (decl) = 1; + DECL_IGNORED_P (decl) = 1; varpool_node::finalize_decl (decl); fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
Re: PING^3: [PATCH]: New configure options that make the compiler use -fPIE and -pie as default option
On Tue, 19 May 2015, H.J. Lu wrote: Here is the complete patch. Tested on Linux/x86-64. It is also available on hjl/pie/master branch in git mirror. OK a week after you CC all relevant target maintainers on the patch, in the absence of objections from those target maintainers. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch 3/10] debug-early merge: C++ front-end
On 05/20/2015 03:18 PM, Jason Merrill wrote: On 05/08/2015 09:14 PM, Aldy Hernandez wrote: + if (!flag_syntax_only) +c_parse_final_cleanups (); The condition is a significant change of behavior for the C++ front end; doing final instantiation and such even with -fsyntax-only was a deliberate choice. Can we drop the condition? Sure. Done. Will repost this as part of the C front-end which has the c-family/ bits where this appeared. + timevar_stop (TV_PHASE_PARSING); + timevar_start (TV_PHASE_DBGINFO); perform_deferred_noexcept_checks (); The only debug info stuff that was here has been removed, so there's no longer any need to switch to a debug timevar. I think we should stay in DEFERRED for the whole function. Fixed. How does this look? Bootstrapped and retested on x86-64 for --enable-languages=all. Aldy gcc/cp/ * cp-objcp-common.c: Adjust comment for cxx_warn_unused_global_decl. * cp-objcp-common.h (LANG_HOOKS_WRITE_GLOBALS): Remove (LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS): New. * cp-tree.h (note_mangling_alias): Protoize. (cp_write_global_declarations): Remove. (cxx_post_compilation_parsing_cleanups): Protoize. * decl.c (wrapup_globals_for_namespace): Remove use of DATA argument. * decl2.c (mangling_aliases): New global. (build_java_method_aliases): New. Adapted from collect_candidates_for_java_method_aliases. (collect_candidates_for_java_method_aliases): Remove. (build_java_method_aliases): Remove. (generate_mangling_aliases): New. (note_mangling_alias): New. Moved from mangle_decl. (locus_at_end_of_parsing): New global. (c_parse_final_cleanups): Rename from cp_write_global_declarations. Use locus_at_end_of_parsing. Call generate_mangling_aliases. Rename call to collect_candidates_for_java_method_aliases into build_java_method_aliases. Remove call to finalize_compilation_unit. Move vtable handling into cxx_post_compilation_parsing_cleanups. Do not call check_global_declarations or emit_debug_global_declarations. (cxx_post_compilation_parsing_cleanups): New. * mangle.c (mangle_decl): Move code to note_mangling_alias. * name-lookup.c (do_namespace_alias): Call early_global_decl. diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c index dd8e7c5..40b13ef 100644 --- a/gcc/cp/cp-objcp-common.c +++ b/gcc/cp/cp-objcp-common.c @@ -60,7 +60,7 @@ cxx_get_alias_set (tree t) return c_common_get_alias_set (t); } -/* Called from check_global_declarations. */ +/* Called from check_global_declaration. */ bool cxx_warn_unused_global_decl (const_tree decl) diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h index 8a36e7f..c8572a7 100644 --- a/gcc/cp/cp-objcp-common.h +++ b/gcc/cp/cp-objcp-common.h @@ -84,8 +84,8 @@ extern void cp_common_init_ts (void); #define LANG_HOOKS_PRINT_ERROR_FUNCTION cxx_print_error_function #undef LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL #define LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL cxx_warn_unused_global_decl -#undef LANG_HOOKS_WRITE_GLOBALS -#define LANG_HOOKS_WRITE_GLOBALS cp_write_global_declarations +#undef LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS +#define LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS cxx_post_compilation_parsing_cleanups #undef LANG_HOOKS_BUILTIN_FUNCTION #define LANG_HOOKS_BUILTIN_FUNCTION cxx_builtin_function #undef LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 4136d98..1998992 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -5469,7 +5469,7 @@ extern tree cp_reconstruct_complex_type (tree, tree); extern bool attributes_naming_typedef_ok (tree); extern void cplus_decl_attributes (tree *, tree, int); extern void finish_anon_union (tree); -extern void cp_write_global_declarations (void); +extern void cxx_post_compilation_parsing_cleanups (void); extern tree coerce_new_type (tree); extern tree coerce_delete_type (tree); extern void comdat_linkage (tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 261a12d..8dadb39 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -892,30 +892,19 @@ walk_namespaces (walk_namespaces_fn f, void* data) return walk_namespaces_r (global_namespace, f, data); } -/* Call wrapup_globals_declarations for the globals in NAMESPACE. If - DATA is non-NULL, this is the last time we will call - wrapup_global_declarations for this NAMESPACE. */ +/* Call wrapup_globals_declarations for the globals in NAMESPACE. */ int -wrapup_globals_for_namespace (tree name_space, void* data) +wrapup_globals_for_namespace (tree name_space, void* data ATTRIBUTE_UNUSED) { cp_binding_level *level = NAMESPACE_LEVEL (name_space); vectree, va_gc *statics = level-static_decls; tree *vec = statics-address (); int len = statics-length (); - int last_time = (data != 0); - - if (last_time) -{ - check_global_declarations (vec, len); - emit_debug_global_declarations (vec, len); - return 0; -} /* Write out any globals that need to be output. */
Re: [PATCH] [PATCH][ARM] Fix split-live-ranges-for-shrink-wrap.c testcase.
On Wed, 20 May 2015, Alex Velenko wrote: Hi, This patch limits testcase split-live-ranges-for-shrink-wrap.c runs to supported achitecture versions. Object size with -march=armv4t check fails because pop pc is not interworking safe on armv4t. This test is not supported for -march=armv7 as this test is for thumb1. Is patch ok? Again, the condition you propose to add doesn't make sense. arm_arch_X_ok is only appropriate for tests using an explicit -march=X. Testing with -march=armv7* should automatically skip this test anyway because it would cause arm_thumb1_ok to fail. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] PR c/66220: Fix false positive from -Wmisleading-indentation
On 05/20/2015 11:05 AM, David Malcolm wrote: This patch fixes the false positive seen from -Wmisleading-indentation on this code: if (v == 2) { res = 27; } else { res = 18; } return res; ^ FALSE POSITIVE HERE along with similar code seen when I tested it with linux-4.0.3. The patch adds a reject for the case where the guard (else in the above example) is more indented than the things it guards. Doing so uncovered an issue with this testcase: #define FOR_EACH(VAR, START, STOP) for ((VAR) = (START); (VAR) (STOP); (VAR++)) /* { dg-message 36: ...this 'for' clause, but it is not } */ void fn_15 (void) { int i; FOR_EACH (i, 0, 10) /* { dg-message 3: in expansion of macro } */ foo (i); bar (i, i); /* { dg-warning statement is indented as if it were guarded by... } */ } #undef FOR_EACH which would then fail to report the warning, due to it using the location of the for in the definition of macro FOR_EACH, rather than the location of the FOR_EACH (i, 0, 10). The fix for this is to use expand_location to get file/line/col of each thing, rather than expand_location_to_spelling_point. With that, all testcases in Wmisleading-indentation.c pass (including the new ones posted in https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01846.html ). OK for trunk if it passes bootstrapregrest? (only tested with make check-gcc RUNTESTFLAGS=dg.exp=Wmisleading-indentation.c make check-g++ RUNTESTFLAGS=dg.exp=Wmisleading-indentation.c so far) gcc/c-family/ChangeLog: PR c/66220: * c-indentation.c (should_warn_for_misleading_indentation): Use expand_location rather than expand_location_to_spelling_point. Don't warn if the guarding statement is more indented than the next/body stmts. gcc/testsuite/ChangeLog: PR c/66220: * c-c++-common/Wmisleading-indentation.c (fn_35): New. (fn_36): New. OK. jeff
C++ PATCH for variable template partial specialization bug
register_specialization complains about a specialization in a different namespace from the main template; we shouldn't give that error about an instantiation of a partial specialization. And we were about to SET_DECL_IMPLICIT_INSTANTIATION anyway, so let's just do that before we call register_specialization. Tested x86_64-pc-linux-gnu, applying to trunk. commit a1ab0a6ef49dcb604c12058b9af7522e471bcc5b Author: Jason Merrill ja...@redhat.com Date: Wed May 20 16:53:57 2015 -0400 * pt.c (tsubst_decl) [VAR_DECL]: SET_DECL_IMPLICIT_INSTANTIATION before register_specialization. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 60f3958..7555114 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -11407,9 +11407,9 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) processing here. */ DECL_EXTERNAL (r) = 1; - register_specialization (r, gen_tmpl, argvec, false, hash); DECL_TEMPLATE_INFO (r) = build_template_info (tmpl, argvec); SET_DECL_IMPLICIT_INSTANTIATION (r); + register_specialization (r, gen_tmpl, argvec, false, hash); } else if (!cp_unevaluated_operand) register_local_specialization (r, t); diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ27.C b/gcc/testsuite/g++.dg/cpp1y/var-templ27.C new file mode 100644 index 000..da06b01 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ27.C @@ -0,0 +1,9 @@ +// { dg-do compile { target c++14 } } + +namespace A +{ + template class T int I = 0; + template class T int IT* = 42; +} + +int i = A::Ivoid*;
[PATCH v2] Handle OS X deployment targets correctly
As described in PR target/63810, this addresses several problems with the validation and encoding of deployment target version strings for the __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ macro. There are currently four testcases exercising inputs to -mmacosx-version-min (gcc/testsuite/gcc.dg/darwin-minversion-*), but they provide minimal coverage, and one is incorrect. * The tiny number is now respected, if present. Thus 10.9.4 correctly becomes 1094 instead of 1090, and 10.10.1 becomes 101001 instead of 101000. * Zero padding is now ignored. Thus 10.09 correctly becomes 1090 instead of 100900, and 10.00010 becomes 101000 instead of being treated as invalid. * Deployment targets that are missing minor and tiny numbers are no longer considered invalid. Thus 10 is treated as 10.0.0, which becomes 1000 without causing an error. With this change, trunk matches the behavior of Apple LLVM Compiler 6.1.0 on 8,451 of 8,464 generated test inputs. (The discrepancies are due to a bug in Clang.) I don't notice any testsuite regressions on OS X 10.10 Yosemite x86-64. 2015-05-15 Lawrence Velázquez v...@larryv.me PR target/63810 * gcc/config/darwin-c.c (version_components): New global enum. (parse_version, version_as_legacy_macro) (version_as_modern_macro, macosx_version_as_macro): New functions. (version_as_macro): Remove. (darwin_cpp_builtins): Use new function. * gcc/testsuite/gcc.dg/darwin-minversion-3.c: Update testcase. * gcc/testsuite/gcc.dg/darwin-minversion-4.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-5.c: New testcase. * gcc/testsuite/gcc.dg/darwin-minversion-6.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-7.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-8.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-9.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-10.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-11.c: Ditto. * gcc/testsuite/gcc.dg/darwin-minversion-12.c: Ditto. --- Re-roll to address patch review on PR target/63810. Provided as text/x-patch attachment to accommodate Apple Mail wonkiness. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63810#c21 gcc/config/darwin-c.c | 168 +++- gcc/testsuite/gcc.dg/darwin-minversion-10.c | 16 +++ gcc/testsuite/gcc.dg/darwin-minversion-11.c | 16 +++ gcc/testsuite/gcc.dg/darwin-minversion-12.c | 16 +++ gcc/testsuite/gcc.dg/darwin-minversion-3.c | 6 +- gcc/testsuite/gcc.dg/darwin-minversion-4.c | 6 +- gcc/testsuite/gcc.dg/darwin-minversion-5.c | 16 +++ gcc/testsuite/gcc.dg/darwin-minversion-6.c | 15 +++ gcc/testsuite/gcc.dg/darwin-minversion-7.c | 15 +++ gcc/testsuite/gcc.dg/darwin-minversion-8.c | 16 +++ gcc/testsuite/gcc.dg/darwin-minversion-9.c | 15 +++ 11 files changed, 273 insertions(+), 32 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-10.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-11.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-12.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-5.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-6.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-7.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-8.c create mode 100644 gcc/testsuite/gcc.dg/darwin-minversion-9.c diff --git a/gcc/config/darwin-c.c b/gcc/config/darwin-c.c index 3803e75..6d49f05 100644 --- a/gcc/config/darwin-c.c +++ b/gcc/config/darwin-c.c @@ -599,42 +599,158 @@ find_subframework_header (cpp_reader *pfile, const char *header, cpp_dir **dirp) return 0; } -/* Return the value of darwin_macosx_version_min suitable for the - __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ macro, so '10.4.2' - becomes 1040 and '10.10.0' becomes 101000. The lowest digit is - always zero, as is the second lowest for '10.10.x' and above. - Print a warning if the version number can't be understood. */ +/* Given an OS X version VERSION_STR, return it as a statically-allocated array + of three integers. If VERSION_STR is invalid, return NULL. + + VERSION_STR must consist of one, two, or three tokens, each separated by + a single period. Each token must contain only the characters '0' through + '9' and is converted to an equivalent non-negative decimal integer. Omitted + tokens become zeros. For example: + +10 becomes {10,0,0} +10.10 becomes {10,10,0} +10.10.1 becomes {10,10,1} +10.10.1 becomes {10,10,1} +10.010.001 becomes {10,10,1} +10.10.1 becomes {10,10,1} +.9.1is invalid +10..9 is invalid +10.10. is invalid */ + +enum version_components { MAJOR, MINOR, TINY }; + +static const unsigned long * +parse_version (const char
Re: [patch] testsuite enable PIE tests on FreeBSD
On 05/20/2015 11:04 AM, Andreas Tobler wrote: Hi, the attached patch enables some PIE tests on FreeBSD. Ok for trunk? Thanks, Andreas 2015-05-20 Andreas Tobler andre...@gcc.gnu.org * gcc.target/i386/pr32219-1.c: Enable test on FreeBSD. * gcc.target/i386/pr32219-2.c: Likewise. * gcc.target/i386/pr32219-3.c: Likewise. * gcc.target/i386/pr32219-4.c: Likewise. * gcc.target/i386/pr32219-5.c: Likewise. * gcc.target/i386/pr32219-6.c: Likewise * gcc.target/i386/pr32219-7.c: Likewise. * gcc.target/i386/pr32219-8.c: Likewise. * gcc.target/i386/pr39013-1.c: Likewise. * gcc.target/i386/pr39013-2.c: Likewise. * gcc.target/i386/pr64317.c: Likewise. Wouldn't it be better to remove the target selector and instead add: /* { dg-require-effective-target pie } */ In each of those tests? While the net effect is the same today, it means there's only one place to change if another x86 target gains PIE support in the future. Pre-approved using that style. jeff
Re: [patch 10/10] debug-early merge: compiler proper
On 05/20/2015 05:01 PM, Jan Hubicka wrote: commit 8824b5ecba26cef065e47b34609c72677c3c36fc Author: Aldy Hernandez al...@redhat.com Date: Wed May 20 16:31:14 2015 -0400 Set DECL_IGNORED_P on temporary arrays created in the switch conversion pass. diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c index 6b68a16..a4bcdba 100644 --- a/gcc/tree-switch-conversion.c +++ b/gcc/tree-switch-conversion.c @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree arr_index_type, DECL_ARTIFICIAL (decl) = 1; TREE_CONSTANT (decl) = 1; TREE_READONLY (decl) = 1; + DECL_IGNORED_P (decl) = 1; varpool_node::finalize_decl (decl); This looks obvious enough to me. Technically speaking the array type constructed probalby should be TREE_ARTIFICAIL, but probably it does not matter. Yeah, that's what I thought. I ignored the type because it won't make it to the debugging back end if we stop things at the DECL itself. FWIW, Ada is filled with these temporaries and/or types that should really be ignored, and are currently causing grief. If you grep for finalize_decl, there are several other calls: asan.c: varpool_node::finalize_decl (var); asan.c: varpool_node::finalize_decl (var); cgraphbuild.c: varpool_node::finalize_decl (decl); cgraphunit.c:- varpool_finalize_decl cgraphunit.c: varpool_node::finalize_decl (decl); cgraphunit.c:varpool_node::finalize_decl (tree decl) coverage.c: varpool_node::finalize_decl (var); coverage.c: varpool_node::finalize_decl (var); Etc etc. Hmmm, I bet mainline is generating dwarf for all this. I don't feel comfortable touching all this (ok, I'm lazy), but it would seem like almost all of these calls would benefit from DECL_IGNORED_P. Perhaps we could add an argument to finalize_decl() and do it in there. Aldy coverage.c: varpool_node::finalize_decl (fn_info_ary); coverage.c: varpool_node::finalize_decl (gcov_info_var); omp-low.c:varpool_node::finalize_decl (t); omp-low.c:varpool_node::finalize_decl (t); omp-low.c:varpool_node::finalize_decl (decl); omp-low.c: varpool_node::finalize_decl (vars_decl); omp-low.c: varpool_node::finalize_decl (funcs_decl); passes.c: varpool_node::finalize_decl (decl); tree-chkp.c: varpool_node::finalize_decl (var); tree-chkp.c: varpool_node::finalize_decl (bnd_var); tree-profile.c: varpool_node::finalize_decl (ic_void_ptr_var); tree-profile.c: varpool_node::finalize_decl (ic_gcov_type_ptr_var); tree-switch-conversion.c: varpool_node::finalize_decl (decl); ubsan.c: varpool_node::finalize_decl (decl); ubsan.c: varpool_node::finalize_decl (var); ubsan.c: varpool_node::finalize_decl (array); varasm.c: varpool_node::finalize_decl (decl); varpool.c: Unlike finalize_decl function is intended to be used varpool.c: varpool_node::finalize_decl (decl); I would say most of them needs similar treatment (I am not 100% sure about OMP ones that may be user visible) Honza fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
C++ PATCH to change -Wc++14-compat operator delete warning
I noticed that -Wc++14-compat was warning about headers that had been updated to include a declaration of a global sized operator delete. This was intended to catch problematic placement deletes, but now I think that C++14 headers that just don't bother to guard the declaration with a C++14 #if are going to be much more common than placement deletes. So this patch changes the warning to only trigger if we actually use the operator delete as a placement delete. Tested x86_64-pc-linux-gnu, applying to trunk. commit 9b7c323b8806bb8d9c21a72ab074f7bc961840c9 Author: Jason Merrill ja...@redhat.com Date: Wed May 20 14:23:50 2015 -0400 * decl.c (grok_op_properties): Don't complain about size_t placement delete here. * call.c (second_parm_is_size_t): Split out from... (non_placement_deallocation_fn_p): ...here. (build_op_delete_call): Warn about size_t placement delete with -Wc++14-compat. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 07ccea9..bad49f1 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -5748,6 +5748,18 @@ build_new_op (location_t loc, enum tree_code code, int flags, return ret; } +/* Returns true if FN has two parameters, of which the second has type + size_t. */ + +static bool +second_parm_is_size_t (tree fn) +{ + tree t = FUNCTION_ARG_CHAIN (fn); + return (t + same_type_p (TREE_VALUE (t), size_type_node) + TREE_CHAIN (t) == void_list_node); +} + /* Returns true iff T, an element of an OVERLOAD chain, is a usual deallocation function (3.7.4.2 [basic.stc.dynamic.deallocation]). */ @@ -5768,11 +5780,9 @@ non_placement_deallocation_fn_p (tree t) of which has type std::size_t (18.2), then this function is a usual deallocation function. */ bool global = DECL_NAMESPACE_SCOPE_P (t); - t = FUNCTION_ARG_CHAIN (t); - if (t == void_list_node - || (t same_type_p (TREE_VALUE (t), size_type_node) - (!global || flag_sized_deallocation) - TREE_CHAIN (t) == void_list_node)) + if (FUNCTION_ARG_CHAIN (t) == void_list_node + || ((!global || flag_sized_deallocation) + second_parm_is_size_t (t))) return true; return false; } @@ -5859,23 +5869,49 @@ build_op_delete_call (enum tree_code code, tree addr, tree size, function (3.7.4.2) and that function, considered as a placement deallocation function, would have been selected as a match for the allocation function, the program is ill-formed. */ - if (non_placement_deallocation_fn_p (fn)) + if (second_parm_is_size_t (fn)) { + const char *msg1 + = G_(exception cleanup for this placement new selects + non-placement operator delete); + const char *msg2 + = G_(%q+D is a usual (non-placement) deallocation + function in C++14 (or with -fsized-deallocation)); + /* But if the class has an operator delete (void *), then that is the usual deallocation function, so we shouldn't complain about using the operator delete (void *, size_t). */ - for (t = BASELINK_P (fns) ? BASELINK_FUNCTIONS (fns) : fns; - t; t = OVL_NEXT (t)) + if (DECL_CLASS_SCOPE_P (fn)) + for (t = BASELINK_P (fns) ? BASELINK_FUNCTIONS (fns) : fns; + t; t = OVL_NEXT (t)) + { + tree elt = OVL_CURRENT (t); + if (non_placement_deallocation_fn_p (elt) + FUNCTION_ARG_CHAIN (elt) == void_list_node) + goto ok; + } + /* Before C++14 a two-parameter global deallocation function is + always a placement deallocation function, but warn if + -Wc++14-compat. */ + else if (!flag_sized_deallocation) { - tree elt = OVL_CURRENT (t); - if (non_placement_deallocation_fn_p (elt) - FUNCTION_ARG_CHAIN (elt) == void_list_node) - goto ok; + if ((complain tf_warning) + warning (OPT_Wc__14_compat, msg1)) + inform (0, msg2, fn); + goto ok; } - if (complain tf_error) + + if (complain tf_warning_or_error) { - permerror (0, non-placement deallocation function %q+D, fn); - permerror (input_location, selected for placement delete); + if (permerror (input_location, msg1)) + { + /* Only mention C++14 for namespace-scope delete. */ + if (DECL_NAMESPACE_SCOPE_P (fn)) + inform (0, msg2, fn); + else + inform (0, %q+D is a usual (non-placement) deallocation + function, fn); + } } else return error_mark_node; diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 261a12d..e4d3c1d 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -11767,16 +11767,6 @@ grok_op_properties (tree decl, bool complain) error (%qD may not be declared as static, decl); return false; } - if (!flag_sized_deallocation warn_cxx14_compat) - { - tree parm = FUNCTION_ARG_CHAIN (decl); - if (parm same_type_p (TREE_VALUE (parm), size_type_node) - TREE_CHAIN (parm) == void_list_node) - warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc__14_compat, - %qD is a usual (non-placement) deallocation -
Re: [patch 2/10] debug-early merge: C front-end (include c-family/)
Update for the c-family bits removing the flag_syntax_only part Jason requested. And BTW, ping for you C front-end maintainers (unless Jason is reviewing the C bits, in which case the rest of you can sit back and look pretty). Aldy gcc/c-family/ * c-common.h (c_parse_final_cleanups): New prototype. * c-opts.c (c_common_parse_file): Call c_parse_final_cleanups. gcc/c/ * c-decl.c (finish_struct): Save C_TYPE_INCOMPLETE_VARS and immediately clobber it. (c_write_global_declarations_1): Remove call to check_global_declaration_1. (c_write_global_declarations_2): Remove. (c_write_final_cleanups): Rename from c_write_global_declarations. Remove call to finalize_compilation_unit. Remove calls to debugging hooks. * c-objc-common.c: Adjust comment for c_warn_unused_global_decl. * c-objc-common.h: Remove LANG_HOOKS_WRITE_GLOBALS. * c-tree.h: Remove c_write_global_declarations. diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 62eac9f..155b799 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -892,6 +892,8 @@ extern HOST_WIDE_INT c_common_to_target_charset (HOST_WIDE_INT); /* This is the basic parsing function. */ extern void c_parse_file (void); +extern void c_parse_final_cleanups (void); + extern void warn_for_omitted_condop (location_t, tree); /* These macros provide convenient access to the various _STMT nodes. */ diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c index e9eb511..89e7fbb 100644 --- a/gcc/c-family/c-opts.c +++ b/gcc/c-family/c-opts.c @@ -1090,6 +1090,8 @@ c_common_parse_file (void) if (!this_input_filename) break; } + + c_parse_final_cleanups (); } /* Returns the appropriate dump file for PHASE to dump with FLAGS. */ diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 4f6761d..ca30a7c 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -7827,10 +7827,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, } /* If this structure or union completes the type of any previous - variable declaration, lay it out and output its rtl. */ - for (x = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); - x; - x = TREE_CHAIN (x)) + variable declaration, lay it out and output its rtl. + + Note: C_TYPE_INCOMPLETE_VARS overloads TYPE_VFIELD which is used + in dwarf2out via rest_of_decl_compilation below and means + something totally different. Since we will be clearing + C_TYPE_INCOMPLETE_VARS shortly after we iterate through them, + clear it ahead of time and avoid problems in dwarf2out. Ideally, + C_TYPE_INCOMPLETE_VARS should use some language specific + node. */ + tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); + C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)) = 0; + for (x = incomplete_vars; x; x = TREE_CHAIN (x)) { tree decl = TREE_VALUE (x); if (TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE) @@ -7843,7 +7851,6 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, rest_of_decl_compilation (decl, toplevel, 0); } } - C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)) = 0; /* Update type location to the one of the definition, instead of e.g. a forward declaration. */ @@ -10667,9 +10674,8 @@ finish_declspecs (struct c_declspecs *specs) return specs; } -/* A subroutine of c_write_global_declarations. Perform final processing - on one file scope's declarations (or the external scope's declarations), - GLOBALS. */ +/* Perform final processing on one file scope's declarations (or the + external scope's declarations), GLOBALS. */ static void c_write_global_declarations_1 (tree globals) @@ -10682,7 +10688,7 @@ c_write_global_declarations_1 (tree globals) { /* Check for used but undefined static functions using the C standard's definition of used, and set TREE_NO_WARNING so - that check_global_declarations doesn't repeat the check. */ + that check_global_declaration doesn't repeat the check. */ if (TREE_CODE (decl) == FUNCTION_DECL DECL_INITIAL (decl) == 0 DECL_EXTERNAL (decl) @@ -10703,21 +10709,6 @@ c_write_global_declarations_1 (tree globals) reconsider |= wrapup_global_declaration_2 (decl); } while (reconsider); - - for (decl = globals; decl; decl = DECL_CHAIN (decl)) -check_global_declaration_1 (decl); -} - -/* A subroutine of c_write_global_declarations Emit debug information for each - of the declarations in GLOBALS. */ - -static void -c_write_global_declarations_2 (tree globals) -{ - tree decl; - - for (decl = globals; decl ; decl = DECL_CHAIN (decl)) -debug_hooks-global_decl (decl); } /* Callback to collect a source_ref from a DECL. */ @@ -10767,8 +10758,11 @@ for_each_global_decl (void (*callback) (tree decl)) callback (decl); } +/* Perform any final parser cleanups and generate initial debugging + information. */ + void -c_write_global_declarations (void)
Re: Demangle symbols in debug assertion messages
On 20/05/2015 12:19, Jonathan Wakely wrote: 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 ? With the patch this code of the bug report generates the following debug message: /home/fdt/dev/gcc/build/x86_64-unknown-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:395: error: attempt to retreat a past-the-end iterator 2 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x0x7fff32365c50 { type = __gnu_debug::_Safe_iteratorstd::__cxx1998::_Deque_iteratorint, int, int*, std::__debug::dequeint, std::allocatorint (mutable iterator); state = past-the-end; references sequence with type `std::__debug::dequeint, std::allocatorint ' @ 0x0x7fff32365cd0 } which looks nice. However I wouldn't say that bug is fixed because debug mode do not generate mangle name, it simply rely on typeid to get it. Shouldn't bug report be saying so ? Whatever, symbol generated by typeid can be demangle by __cxa_demangle so it mustn't be that bad. François
Re: [PATCH/libiberty] fix build of gdb/binutils with clang.
This is OK if GCC bootstraps. Thanks. Ian On Wed, May 20, 2015 at 11:25 AM, Yunlian Jiang yunl...@google.com wrote: I have the following change to make libiberty compile with _GNU_SOURCE defined and remove the declaration of asprintf in libiberty.h if HAVE_DECL_ASPRINTF is not defined. diff --git a/include/libiberty.h b/include/libiberty.h index b33dd65..8e096a0 100644 --- a/include/libiberty.h +++ b/include/libiberty.h @@ -621,7 +621,7 @@ extern int pexecute (const char *, char * const *, const char *, extern int pwait (int, int *, int); -#if !HAVE_DECL_ASPRINTF +#if defined(HAVE_DECL_ASPRINTF) !HAVE_DECL_ASPRINTF /* Like sprintf but provides a pointer to malloc'd storage, which must be freed by the caller. */ diff --git a/libiberty/Makefile.in b/libiberty/Makefile.in index f06cc69..624420d 100644 --- a/libiberty/Makefile.in +++ b/libiberty/Makefile.in @@ -113,7 +113,8 @@ installcheck: installcheck-subdir INCDIR=$(srcdir)/$(MULTISRCTOP)../include -COMPILE.c = $(CC) -c @DEFS@ $(CFLAGS) $(CPPFLAGS) -I. -I$(INCDIR) $(HDEFINES) @ac_libiberty_warn_cflags@ +COMPILE.c = $(CC) -c @DEFS@ $(CFLAGS) $(CPPFLAGS) -I. -I$(INCDIR) \ + $(HDEFINES) @ac_libiberty_warn_cflags@ -D_GNU_SOURCE # Just to make sure we don't use a built-in rule with VPATH .c.$(objext): diff --git a/libiberty/configure b/libiberty/configure index b06cab2..c6758b0 100755 --- a/libiberty/configure +++ b/libiberty/configure @@ -5130,6 +5130,9 @@ $as_echo #define NEED_DECLARATION_ERRNO 1 confdefs.h fi +$as_echo #define _GNU_SOURCE 1 confdefs.h + + # Determine sizes of some types. # The cast to long int works around a bug in the HP C Compiler # version HP92453-01 B.11.11.23709.GP, which incorrectly rejects diff --git a/libiberty/configure.ac b/libiberty/configure.ac index 922aa86..9f2d661 100644 --- a/libiberty/configure.ac +++ b/libiberty/configure.ac @@ -272,6 +272,8 @@ AC_HEADER_TIME libiberty_AC_DECLARE_ERRNO +AC_DEFINE(_GNU_SOURCE) + # Determine sizes of some types. AC_CHECK_SIZEOF([int]) AC_CHECK_SIZEOF([long]) diff --git a/libiberty/floatformat.c b/libiberty/floatformat.c index 789fa05..4e73a2d 100644 --- a/libiberty/floatformat.c +++ b/libiberty/floatformat.c @@ -19,7 +19,9 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ /* This is needed to pick up the NAN macro on some systems. */ +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #ifdef HAVE_CONFIG_H #include config.h On Tue, May 19, 2015 at 11:15 AM, Ian Lance Taylor i...@google.com wrote: On Tue, May 19, 2015 at 11:08 AM, Yunlian Jiang yunl...@google.com wrote: I could do that and it make the compilation of libiberty passes. However, I have some other problem when using clang to build gdb because of libiberty. Some c file from other component may include 'libiberty.h' which contains the following #if !HAVE_DECL_ASPRINTF /* Like sprintf but provides a pointer to malloc'd storage, which must be freed by the caller. */ extern int asprintf (char **, const char *, ...) ATTRIBUTE_PRINTF_2; #endif The HAVE_DECL_ASPRINTF is defined in config.h under libiberty directory. If the other c file only includes libiberty.h and does not include the libiberty/config.h and at the same time, _GNU_SOURCE is defind, the same error happens. Probably if HAVE_DECL_ASPRINTF is not defined at all, we should not declare asprintf in libiberty.h. Ian
[PATCH] [PATCH][ARM] Fix sibcall testcases.
Hi, This patch prevents arm_thumb1_ok XPASS in sibcall-3.c and sibcall-4.c testcases. Sibcalls are not ok for Thumb1 and testcases need to be fixed. Is patch ok? gcc/testsuite 2015-05-20 Alex Velenko alex.vele...@arm.com * gcc.dg/sibcall-3.c (dg-skip-if): Skip if arm_thumb1_ok. * gcc.dg/sibcall-4.c (dg-skip-if): Likewise. --- gcc/testsuite/gcc.dg/sibcall-3.c | 1 + gcc/testsuite/gcc.dg/sibcall-4.c | 1 + 2 files changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.dg/sibcall-3.c b/gcc/testsuite/gcc.dg/sibcall-3.c index eafe8dd..37f44a1 100644 --- a/gcc/testsuite/gcc.dg/sibcall-3.c +++ b/gcc/testsuite/gcc.dg/sibcall-3.c @@ -8,6 +8,7 @@ /* { dg-do run { xfail { { cris-*-* crisv32-*-* h8300-*-* hppa*64*-*-* m32r-*-* mcore-*-* mn10300-*-* msp430*-*-* nds32*-*-* xstormy16-*-* v850*-*-* vax-*-* xtensa*-*-* } || { arm*-*-* { ! arm32 } } } } } */ /* -mlongcall disables sibcall patterns. */ /* { dg-skip-if { powerpc*-*-* } { -mlongcall } { } } */ +/* { dg-skip-if { arm*-*-* arm_thumb1_ok } } */ /* { dg-options -O2 -foptimize-sibling-calls } */ /* The option -foptimize-sibling-calls is the default, but serves as diff --git a/gcc/testsuite/gcc.dg/sibcall-4.c b/gcc/testsuite/gcc.dg/sibcall-4.c index 1e039c6..9554a95 100644 --- a/gcc/testsuite/gcc.dg/sibcall-4.c +++ b/gcc/testsuite/gcc.dg/sibcall-4.c @@ -8,6 +8,7 @@ /* { dg-do run { xfail { { cris-*-* crisv32-*-* h8300-*-* hppa*64*-*-* m32r-*-* mcore-*-* mn10300-*-* msp430*-*-* nds32*-*-* xstormy16-*-* v850*-*-* vax-*-* xtensa*-*-* } || { arm*-*-* { ! arm32 } } } } } */ /* -mlongcall disables sibcall patterns. */ /* { dg-skip-if { powerpc*-*-* } { -mlongcall } { } } */ +/* { dg-skip-if { arm*-*-* arm_thumb1_ok } } */ /* { dg-options -O2 -foptimize-sibling-calls } */ /* The option -foptimize-sibling-calls is the default, but serves as -- 1.8.1.2
Re: Fwd: Re: [PATCH, RFC]: Next stage1, refactoring: propagating rtx subclasses
On 05/20/2015 12:28 AM, Jeff Law wrote: Yes, this is OK for the trunk. Please commit. jeff Applied to trunk (revision 223454). -- Regards, Mikhail Maltsev
Re: [PATCH/libiberty] fix build of gdb/binutils with clang.
I have the following change to make libiberty compile with _GNU_SOURCE defined and remove the declaration of asprintf in libiberty.h if HAVE_DECL_ASPRINTF is not defined. diff --git a/include/libiberty.h b/include/libiberty.h index b33dd65..8e096a0 100644 --- a/include/libiberty.h +++ b/include/libiberty.h @@ -621,7 +621,7 @@ extern int pexecute (const char *, char * const *, const char *, extern int pwait (int, int *, int); -#if !HAVE_DECL_ASPRINTF +#if defined(HAVE_DECL_ASPRINTF) !HAVE_DECL_ASPRINTF /* Like sprintf but provides a pointer to malloc'd storage, which must be freed by the caller. */ diff --git a/libiberty/Makefile.in b/libiberty/Makefile.in index f06cc69..624420d 100644 --- a/libiberty/Makefile.in +++ b/libiberty/Makefile.in @@ -113,7 +113,8 @@ installcheck: installcheck-subdir INCDIR=$(srcdir)/$(MULTISRCTOP)../include -COMPILE.c = $(CC) -c @DEFS@ $(CFLAGS) $(CPPFLAGS) -I. -I$(INCDIR) $(HDEFINES) @ac_libiberty_warn_cflags@ +COMPILE.c = $(CC) -c @DEFS@ $(CFLAGS) $(CPPFLAGS) -I. -I$(INCDIR) \ + $(HDEFINES) @ac_libiberty_warn_cflags@ -D_GNU_SOURCE # Just to make sure we don't use a built-in rule with VPATH .c.$(objext): diff --git a/libiberty/configure b/libiberty/configure index b06cab2..c6758b0 100755 --- a/libiberty/configure +++ b/libiberty/configure @@ -5130,6 +5130,9 @@ $as_echo #define NEED_DECLARATION_ERRNO 1 confdefs.h fi +$as_echo #define _GNU_SOURCE 1 confdefs.h + + # Determine sizes of some types. # The cast to long int works around a bug in the HP C Compiler # version HP92453-01 B.11.11.23709.GP, which incorrectly rejects diff --git a/libiberty/configure.ac b/libiberty/configure.ac index 922aa86..9f2d661 100644 --- a/libiberty/configure.ac +++ b/libiberty/configure.ac @@ -272,6 +272,8 @@ AC_HEADER_TIME libiberty_AC_DECLARE_ERRNO +AC_DEFINE(_GNU_SOURCE) + # Determine sizes of some types. AC_CHECK_SIZEOF([int]) AC_CHECK_SIZEOF([long]) diff --git a/libiberty/floatformat.c b/libiberty/floatformat.c index 789fa05..4e73a2d 100644 --- a/libiberty/floatformat.c +++ b/libiberty/floatformat.c @@ -19,7 +19,9 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ /* This is needed to pick up the NAN macro on some systems. */ +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #ifdef HAVE_CONFIG_H #include config.h On Tue, May 19, 2015 at 11:15 AM, Ian Lance Taylor i...@google.com wrote: On Tue, May 19, 2015 at 11:08 AM, Yunlian Jiang yunl...@google.com wrote: I could do that and it make the compilation of libiberty passes. However, I have some other problem when using clang to build gdb because of libiberty. Some c file from other component may include 'libiberty.h' which contains the following #if !HAVE_DECL_ASPRINTF /* Like sprintf but provides a pointer to malloc'd storage, which must be freed by the caller. */ extern int asprintf (char **, const char *, ...) ATTRIBUTE_PRINTF_2; #endif The HAVE_DECL_ASPRINTF is defined in config.h under libiberty directory. If the other c file only includes libiberty.h and does not include the libiberty/config.h and at the same time, _GNU_SOURCE is defind, the same error happens. Probably if HAVE_DECL_ASPRINTF is not defined at all, we should not declare asprintf in libiberty.h. Ian
Re: [patch 3/10] debug-early merge: C++ front-end
On 05/08/2015 09:14 PM, Aldy Hernandez wrote: + if (!flag_syntax_only) +c_parse_final_cleanups (); The condition is a significant change of behavior for the C++ front end; doing final instantiation and such even with -fsyntax-only was a deliberate choice. Can we drop the condition? + timevar_stop (TV_PHASE_PARSING); + timevar_start (TV_PHASE_DBGINFO); perform_deferred_noexcept_checks (); The only debug info stuff that was here has been removed, so there's no longer any need to switch to a debug timevar. I think we should stay in DEFERRED for the whole function. Jason
[PATCH] Contribute FreeBSD unwind support (x86_64 and x86)
I have maintained unwind support for FreeBSD i386 and x86_64 in my gnat-aux repository for many years (I created it). I've always intended on contributing it back to GCC, but I never got around to proving it worked until now. The version I've been using actually has two flavors: FreeBSD 8 and below and FreeBSD 9 and above. However, the last of the FreeBSD 8 releases reaches EOL at the end of June so the unwind support I've attached here drops the FreeBSD 8 variation for simplicity's sake. I was under the impression that MD unwinding was used for more than just GNAT but it looks like that impression was wrong. When I ran the testsuite, the only tests affected were Ada tests. FreeBSD 10.1 / gcc-6.0.0 before patch applied: http://leaf.dragonflybsd.org/~marino/freebsd/summary-100F64.txt FreeBSD 10.1 / gcc-6.0.0 after patch applied: http://leaf.dragonflybsd.org/~marino/freebsd/summary-unwind-100F64.txt Difference between runs: http://leaf.dragonflybsd.org/~marino/freebsd/unwind-diff-100F64.txt FreeBSD 11/amd64 with patch (Ada only): http://leaf.dragonflybsd.org/~marino/freebsd/summary-110F64.txt FreeBSD 11/i386 with patch (Ada only) http://leaf.dragonflybsd.org/~marino/freebsd/summary-110F32.txt Note that I provided a similar unwind support for DragonFly a few months ago. Please consider applying the attached patch to gcc trunk. (copy of patch found here: http://leaf.dragonflybsd.org/~marino/freebsd/freebsd-unwind-support.diff ) Suggested text for libgcc/ChangeLog: 2015-05-XX John Marino gnu...@marino.st * config.host (i[34567]86-*-freebsd*, x86_64-*-freebsd*): Set md_unwind_header * config/i386/freebsd-unwind.h: New. Also please recall that my copyright assignment to FSF is in order! Thanks, John Marino --- /dev/null +++ libgcc/config/i386/freebsd-unwind.h @@ -0,0 +1,173 @@ +/* DWARF2 EH unwinding support for FreeBSD: AMD x86-64 and x86. + Copyright (C) 2015 Free Software Foundation, Inc. + Contributed by John Marino gnu...@marino.st + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +http://www.gnu.org/licenses/. */ + +/* Do code reading to identify a signal frame, and set the frame + state data appropriately. See unwind-dw2.c for the structs. */ + +#include sys/types.h +#include signal.h +#include sys/ucontext.h +#include machine/sigframe.h + +#define REG_NAME(reg) sf_uc.uc_mcontext.mc_## reg + +#ifdef __x86_64__ +#define MD_FALLBACK_FRAME_STATE_FOR x86_64_freebsd_fallback_frame_state + +static _Unwind_Reason_Code +x86_64_freebsd_fallback_frame_state +(struct _Unwind_Context *context, _Unwind_FrameState *fs) +{ + struct sigframe *sf; + long new_cfa; + + /* Prior to FreeBSD 9, the signal trampoline was located immediately + before the ps_strings. To support non-executable stacks on AMD64, + the sigtramp was moved to a shared page for FreeBSD 9. Unfortunately + this means looking frame patterns again (sys/amd64/amd64/sigtramp.S) + rather than using the robust and convenient KERN_PS_STRINGS trick. + + pc + 00: lea 0x10(%rsp),%rdi + pc + 05: pushq $0x0 + pc + 17: mov $0x1a1,%rax + pc + 14: syscall + + If we can't find this pattern, we're at the end of the stack. + */ + + if (!( *(unsigned int *)(context-ra) == 0x247c8d48 + *(unsigned int *)(context-ra + 4) == 0x48006a10 + *(unsigned int *)(context-ra + 8) == 0x01a1c0c7 + *(unsigned int *)(context-ra + 12) == 0x050f )) +return _URC_END_OF_STACK; + + sf = (struct sigframe *) context-cfa; + new_cfa = sf-REG_NAME(rsp); + fs-regs.cfa_how = CFA_REG_OFFSET; + /* Register 7 is rsp */ + fs-regs.cfa_reg = 7; + fs-regs.cfa_offset = new_cfa - (long) context-cfa; + + /* The SVR4 register numbering macros aren't usable in libgcc. */ + fs-regs.reg[0].how = REG_SAVED_OFFSET; + fs-regs.reg[0].loc.offset = (long)sf-REG_NAME(rax) - new_cfa; + fs-regs.reg[1].how = REG_SAVED_OFFSET; + fs-regs.reg[1].loc.offset = (long)sf-REG_NAME(rdx) - new_cfa; + fs-regs.reg[2].how = REG_SAVED_OFFSET; + fs-regs.reg[2].loc.offset = (long)sf-REG_NAME(rcx) - new_cfa; + fs-regs.reg[3].how = REG_SAVED_OFFSET; +