{PATCH] New C++ warning -Wcatch-value
Hi, catching exceptions by value is a bad thing, as it may cause slicing, i.e. a) a superfluous copy b) which is only partial. See also https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#e15-catch-exceptions-from-a-hierarchy-by-reference To warn the user about catch handlers of non-reference type, the following patch adds a new C++/ObjC++ warning option "-Wcatch-value". Bootstrapped and regtested on x86_64-pc-linux-gnu. OK for trunk? Regards, Volker 2017-05-01 Volker Reichelt * doc/invoke.texi (-Wcatch-value): Document new warning option. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 247416) +++ gcc/doc/invoke.texi (working copy) @@ -265,7 +265,7 @@ -Wno-builtin-declaration-mismatch @gol -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol --Wchar-subscripts -Wchkp -Wclobbered -Wcomment @gol +-Wchar-subscripts -Wchkp -Wcatch-value -Wclobbered -Wcomment @gol -Wconditionally-supported @gol -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol -Wdelete-incomplete @gol @@ -5827,6 +5827,11 @@ literals to @code{char *}. This warning is enabled by default for C++ programs. +@item -Wcatch-value @r{(C++ and Objective-C++ only)} +@opindex Wcatch-value +@opindex Wno-catch-value +Warn about catch handler of non-reference type. + @item -Wclobbered @opindex Wclobbered @opindex Wno-clobbered === 2017-05-01 Volker Reichelt * c.opt (Wcatch-value): New C++ warning flag. Index: gcc/c-family/c.opt === --- gcc/c-family/c.opt (revision 247416) +++ gcc/c-family/c.opt (working copy) @@ -388,6 +388,10 @@ C ObjC C++ ObjC++ Var(warn_cast_qual) Warning Warn about casts which discard qualifiers. +Wcatch-value +C++ ObjC++ Var(warn_catch_value) Warning +Warn about catch handlers of non-reference type. + Wchar-subscripts C ObjC C++ ObjC++ Var(warn_char_subscripts) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about subscripts whose type is \"char\". === 2017-05-01 Volker Reichelt * semantics.c (finish_handler_parms): Warn about non-reference type catch handlers. Index: gcc/cp/semantics.c === --- gcc/cp/semantics.c (revision 247416) +++ gcc/cp/semantics.c (working copy) @@ -1321,7 +1321,15 @@ } } else -type = expand_start_catch_block (decl); +{ + type = expand_start_catch_block (decl); + if (warn_catch_value + && type != NULL_TREE + && type != error_mark_node + && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE) + warning (OPT_Wcatch_value, +"catching non-reference type %qT", TREE_TYPE (decl)); +} HANDLER_TYPE (handler) = type; } === 2017-05-01 Volker Reichelt * g++.dg/warn/Wcatch-value-1.C: New test. Index: gcc/testsuite/g++.dg/warn/Wcatch-value-1.C === --- gcc/testsuite/g++.dg/warn/Wcatch-value-1.C 2017-05-01 +++ gcc/testsuite/g++.dg/warn/Wcatch-value-1.C 2017-05-01 @@ -0,0 +1,45 @@ +// { dg-options "-Wcatch-value" } + +struct A {}; +struct B {}; +struct C {}; + +void foo() +{ + try {} + catch (int) {} // { dg-warning "catching non-reference type" } + catch (double*) {} // { dg-warning "catching non-reference type" } + catch (float&) {} + catch (A){} // { dg-warning "catching non-reference type" } + catch (A[2]) {} // { dg-warning "catching non-reference type" } + catch (B*) {} // { dg-warning "catching non-reference type" } + catch (B&) {} + catch (const C&) {} +} + +template void foo1() +{ + try {} + catch (T) {} +} + +void bar1() +{ + foo1(); + foo1(); +} + +template void foo2() +{ + try {} + catch (T) {} // { dg-warning "catching non-reference type" } + + try {} + catch (T&) {} +} + +void bar2() +{ + foo2(); // { dg-message "required" } + foo2(); // { dg-message "required" } +} ===
Re: [PATCH] x86: vpermil2p{s,d} have no commutative operands
On Fri, Apr 28, 2017 at 4:51 PM, Jan Beulich wrote: > While either of the last two operands can be in memory, they can't be > swapped. > > gcc/ > 2017-04-28 Jan Beulich > > * config/i386/sse.md (xop_vpermil23): Use alternatives. Please write a more descriptive ChangeLog entry, e.g. "Do not allow operand swapping, add (x,x,m,x,n) alternative. OK with above change for mainline and release branches. Thanks, Uros. > --- a/gcc/config/i386/sse.md > +++ b/gcc/config/i386/sse.md > @@ -17092,12 +17092,12 @@ > (set_attr "mode" "TI")]) > > (define_insn "xop_vpermil23" > - [(set (match_operand:VF_128_256 0 "register_operand" "=x") > + [(set (match_operand:VF_128_256 0 "register_operand" "=x,x") > (unspec:VF_128_256 > - [(match_operand:VF_128_256 1 "register_operand" "x") > - (match_operand:VF_128_256 2 "nonimmediate_operand" "%x") > - (match_operand: 3 "nonimmediate_operand" "xm") > - (match_operand:SI 4 "const_0_to_3_operand" "n")] > + [(match_operand:VF_128_256 1 "register_operand" "x,x") > + (match_operand:VF_128_256 2 "nonimmediate_operand" "x,m") > + (match_operand: 3 "nonimmediate_operand" "xm,x") > + (match_operand:SI 4 "const_0_to_3_operand" "n,n")] > UNSPEC_VPERMIL2))] >"TARGET_XOP" >"vpermil2\t{%4, %3, %2, %1, %0|%0, %1, %2, %3, %4}" > > >
Re: [PATCH] simplify-rtx: Fix compare of comparisons (PR60818)
Hi! Is this okay for backport to 5 and 6? Segher On Fri, Mar 31, 2017 at 09:40:34PM +, Segher Boessenkool wrote: > The function simplify_binary_operation_1 has code that does > /* Convert (compare (gt (flags) 0) (lt (flags) 0)) to (flags). */ > but this transformation is only valid if "flags" has the same machine > mode as the outer compare. This fixes it. > > Bootstrapped and tested on powerpc64-linux {-m32,-m64} (and tested the > new testcase with {-m32,-m64}{,-misel}). Is this okay for trunk? > > > Segher > > > 2017-03-31 Segher Boessenkool > > PR rtl-optimization/60818 > * simplify-rtx.c (simplify_binary_operation_1): Do not replace > a compare of comparisons with the thing compared if this results > in a different machine mode. > > gcc/testsuite/ > PR rtl-optimization/60818 > * gcc.c-torture/compile/pr60818.c: New testcase. > > --- > gcc/simplify-rtx.c| 6 +++--- > gcc/testsuite/gcc.c-torture/compile/pr60818.c | 5 + > 2 files changed, 8 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr60818.c > > diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c > index 3022779..10c2800 100644 > --- a/gcc/simplify-rtx.c > +++ b/gcc/simplify-rtx.c > @@ -2367,10 +2367,10 @@ > return xop00; > > if (REG_P (xop00) && REG_P (xop10) > - && GET_MODE (xop00) == GET_MODE (xop10) > && REGNO (xop00) == REGNO (xop10) > - && GET_MODE_CLASS (GET_MODE (xop00)) == MODE_CC > - && GET_MODE_CLASS (GET_MODE (xop10)) == MODE_CC) > + && GET_MODE (xop00) == mode > + && GET_MODE (xop10) == mode > + && GET_MODE_CLASS (mode) == MODE_CC) > return xop00; > } >break; > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr60818.c > b/gcc/testsuite/gcc.c-torture/compile/pr60818.c > new file mode 100644 > index 000..b6171bb > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/pr60818.c > @@ -0,0 +1,5 @@ > +int > +lx (int oi, int mb) > +{ > + return (oi < mb) < (mb < oi); > +} > -- > 1.9.3
Re: [PATCH] Check for sp push/pop insns in reg_set_p (PR target/79430)
On Thu, Apr 27, 2017 at 6:13 PM, Jeff Law wrote: > On 04/27/2017 01:32 AM, Jakub Jelinek wrote: >> >> Hi! >> >> As mentioned in the PR and can be seen on the testcase (too large for >> testsuite, with lots of delta reduction I got 48KB *.f90 file still using >> a dozen of modules), we miscompile it because we have mem(sp+64) memory >> (what %st is loaded from) and are checking whether it is safe to move >> earlier in the insn stream, and modified_between_p tells us it is, except >> there is a stack pop instruction (i.e. sp autoinc). >> And sp autoinc is apparently special in GCC: >>/* There are no REG_INC notes for SP. */ > > Right. It's been the source of numerous problems through the years. One > could argue that we should just bite the bullet and add them. The cost > can't be that high and it'd avoid these kinds of problems in the future and > allow for some code cleanups as well. > > We could probably scan the IL at the end of auto-inc-dec.c to add the > missing notes. > > I thought I saw a comment once which indicates the rationale behind not > including REG_INC notes for pushes/pops, but I can't find it anymore. > >> >> The following patch handles that, plus then undoes that in >> ix86_agi_dependent >> where from what I understood we want the previous behavior - push, pop and >> call modifications of SP don't cause AGI stalls for addresses that have >> SP base (SP can't appear as index). >> >> Not really sure about the == stack_pointer_rtx vs. >> REG_P () && REGNO () == STACK_POINTER_REGNUM, there is lots of code that >> just uses pointer comparisons and others that check REGNO, as an example >> of the former e.g. push/pop_operand. So, is SP always shared, or can >> there >> be other REGs with SP regno? > > SP is supposed to be shared, you should be able to compare against > stack_pointer_rtx. > > >> >> Other than the ix86_agi_dependent which in my stats was the single case >> that hit this difference, I've seen it making a difference e.g. in ifcvt >> decisions, but at least the cases I've debugged didn't end up in any code >> generation changes. E.g. both x86_64 and i686 libstdc++.so.6 and >> libgo.so.11 as the two largest shared libraries built during bootstrap >> are identical without/with this patch (objdump -dr is identical that is). >> While without the config/i386/i386.c changes there were tons of >> differences. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> 2017-04-27 Jakub Jelinek >> >> PR target/79430 >> * rtlanal.c (reg_set_p): If reg is a stack_pointer_rtx, also >> check for stack push/pop autoinc. >> * config/i386/i386.c (ix86_agi_dependent): Return false >> if the only reason why modified_in_p returned true is that >> addr is SP based and set_insn is a push or pop. > > THe rtlanal.c changes are fine by me. Uros should chime in on the x86 > specific bits. LGTM, with comparison to stack_pointer_rtx, as mentioned by Jeff above. Thanks, Uros.
Re: [PATCH][x86] Add missing intrinsics for ADD[SD,SS] and SUB[SD,SS]
On Thu, Apr 27, 2017 at 10:22 AM, Peryt, Sebastian wrote: > Hi, > > This patch adds missing intrinsics for ADDSD, ADDSS, SUBSD and SUBSS > instructions. > > gcc/ > * config/i386/avx512fintrin.h (_mm_mask_add_round_sd, > _mm_maskz_add_round_sd, _mm_mask_add_round_ss, > _mm_maskz_add_round_ss, _mm_mask_sub_round_sd, > _mm_maskz_sub_round_sd, _mm_mask_sub_round_ss, > _mm_maskz_sub_round_ss, _mm_mask_add_sd, > _mm_maskz_add_sd, _mm_mask_add_ss, _mm_maskz_add_ss, > _mm_mask_sub_sd, _mm_maskz_sub_sd, _mm_mask_sub_ss, > _mm_maskz_sub_ss): New intrinsics. > * config/i386/i386-builtin-types.def > (V2DF_FTYPE_V2DF_V2DF_V2DF_UQI_INT, > V4SF_FTYPE_V4SF_V4SF_V4SF_UQI_INT): New function type aliases. > * config/i386/i386-builtin.def (__builtin_ia32_addsd_mask_round, > __builtin_ia32_addss_mask_round, __builtin_ia32_subsd_mask_round, > __builtin_ia32_subss_mask_round): New builtins. > * config/i386/i386.c (V2DF_FTYPE_V2DF_V2DF_V2DF_UQI_INT, > V4SF_FTYPE_V4SF_V4SF_V4SF_UQI_INT): Handle new types. > * config/i386/sse.md (_vm3): > Renamed to ... > (_vm3): ... this. > (v\t{%2, %1, > %0|%0, %1, %2}): Changed to ... > (v\t{%2, %1, > %0|%0, %1, %2}): ... this. > > gcc/testsuite/ > * gcc.target/i386/avx512f-vaddsd-1.c (_mm_mask_add_sd, > _mm_maskz_add_sd, _mm_mask_add_round_sd, > _mm_maskz_add_round_sd): Test new intrinsics. > * gcc.target/i386/avx512f-vaddsd-2.c: New. > * gcc.target/i386/avx512f-vaddss-1.c (_mm_mask_add_ss, > _mm_maskz_add_ss, _mm_mask_add_round_ss, > _mm_maskz_add_round_ss): Test new intrinsics. > * gcc.target/i386/avx512f-vaddss-2.c: New. > * gcc.target/i386/avx512f-vsubsd-1.c (_mm_mask_sub_sd, > _mm_maskz_sub_sd, _mm_mask_sub_round_sd, > _mm_maskz_sub_round_sd): Test new intrinsics. > * gcc.target/i386/avx512f-vsubsd-2.c: New. > * gcc.target/i386/avx512f-vsubss-1.c (_mm_mask_sub_ss, > _mm_maskz_sub_ss, _mm_mask_sub_round_ss, > _mm_maskz_sub_round_ss): Test new intrinsics. > * gcc.target/i386/avx512f-vsubss-2.c: New. > * gcc.target/i386/avx-1.c (__builtin_ia32_addsd_mask_round, > __builtin_ia32_addss_mask_round, __builtin_ia32_subsd_mask_round, > __builtin_ia32_subss_mask_round): Test new builtins. > * gcc.target/i386/sse-13.c: Ditto. > * gcc.target/i386/sse-23.c: Ditto. > * gcc.target/i386/sse-14.c (_mm_maskz_add_round_sd, > _mm_maskz_add_round_ss, _mm_maskz_sub_round_sd, > _mm_maskz_sub_round_ss, _mm_mask_add_round_sd, > _mm_mask_add_round_ss, _mm_mask_sub_round_sd, > _mm_mask_sub_round_ss): Test new intrinsics. > * gcc.target/i386/testround-1.c: Ditto. > > Is it ok for trunk? OK. Thanks, Uros.
[patch] Inline size functions in more cases
Because variable-sized types are first-class citizens in Ada, the compiler factors out size (and offset) computations into size functions in order to make these types less heavyweight to manipulate. The counterpart is that it needs to inline back these size functions into regular expressions in some cases, for example when it constrains an originally unconstrained type. This patch makes the compiler inline these functions in a little more cases. Tested on x86_64-suse-linux, applied on the mainline (it only affects Ada). 2017-05-01 Eric Botcazou * tree.c (substitute_in_expr) : Also inline a call if the replacement expression is another instance of one of its arguments. -- Eric BotcazouIndex: tree.c === --- tree.c (revision 247405) +++ tree.c (working copy) @@ -3886,15 +3886,29 @@ substitute_in_expr (tree exp, tree f, tr new_tree = NULL_TREE; - /* If we are trying to replace F with a constant, inline back + /* If we are trying to replace F with a constant or with another + instance of one of the arguments of the call, inline back functions which do nothing else than computing a value from the arguments they are passed. This makes it possible to fold partially or entirely the replacement expression. */ - if (CONSTANT_CLASS_P (r) && code == CALL_EXPR) + if (code == CALL_EXPR) { - tree t = maybe_inline_call_in_expr (exp); - if (t) - return SUBSTITUTE_IN_EXPR (t, f, r); + bool maybe_inline = false; + if (CONSTANT_CLASS_P (r)) + maybe_inline = true; + else + for (i = 3; i < TREE_OPERAND_LENGTH (exp); i++) + if (operand_equal_p (TREE_OPERAND (exp, i), r, 0)) + { + maybe_inline = true; + break; + } + if (maybe_inline) + { + tree t = maybe_inline_call_in_expr (exp); + if (t) + return SUBSTITUTE_IN_EXPR (t, f, r); + } } for (i = 1; i < TREE_OPERAND_LENGTH (exp); i++)
Re: [PATCH] RISC-V: Don't built 64-bit multilibs on 32-bit targets
On Mon, 1 May 2017, Palmer Dabbelt wrote: > > Specifically, in such cases the GCC configure option is > > --enable-target=all to enable 64-bit multilibs for a default-32-bit > > target, and the binutils/GDB configure option is --enable-64-bit-bfd. But > > you can also make a binutils/GDB target include 64-bit support > > unconditionally without requiring --enable-64-bit-bfd. > > Do you happen to know how to do that (enable 64-bit BFDs in a 32-bit binutils > build)? I'd prefer to have the riscv32 and riscv64 ports the same if > possible, > but poked around a bit and couldn't find a way to do it. Well, certainly you should add riscv_elf64_vec to targ_selvecs in config.bfd for the riscv32-*-* case. Generally, check all the configure fragments used for riscv* to make sure they include both 32-bit and 64-bit support. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 09/12] [i386] Add patterns and predicates foutline-msabi-xlouges
On Thu, Apr 27, 2017 at 10:09 AM, Daniel Santos wrote: > Adds the predicates save_multiple and restore_multiple to predicates.md, > which are used by following patterns in sse.md: > > * save_multiple - insn that calls a save stub > * restore_multiple - call_insn that calls a save stub and returns to the > function to allow a sibling call (which should typically offer better > optimization than the restore stub as the tail call) > * restore_multiple_and_return - a jump_insn that returns from the > function as a tail-call. > * restore_multiple_leave_return - like the above, but restores the frame > pointer before returning. > > Signed-off-by: Daniel Santos > --- > gcc/config/i386/predicates.md | 155 > ++ > gcc/config/i386/sse.md| 37 ++ > 2 files changed, 192 insertions(+) > > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > index 8f250a2e720..36fe8abc3f4 100644 > --- a/gcc/config/i386/predicates.md > +++ b/gcc/config/i386/predicates.md > @@ -1657,3 +1657,158 @@ >(ior (match_operand 0 "register_operand") > (and (match_code "const_int") > (match_test "op == constm1_rtx" > + > +;; Return true if: > +;; 1. first op is a symbol reference, > +;; 2. >= 13 operands, and > +;; 3. operands 2 to end is one of: > +;; a. save a register to a memory location, or > +;; b. restore stack pointer. > +(define_predicate "save_multiple" > + (match_code "parallel") > +{ > + const unsigned nregs = XVECLEN (op, 0); > + rtx head = XVECEXP (op, 0, 0); > + unsigned i; > + > + if (GET_CODE (head) != USE) > +return false; > + else > +{ > + rtx op0 = XEXP (head, 0); > + if (op0 == NULL_RTX || GET_CODE (op0) != SYMBOL_REF) > + return false; > +} > + > + if (nregs < 13) > +return false; > + > + for (i = 2; i < nregs; i++) > +{ > + rtx e, src, dest; > + > + e = XVECEXP (op, 0, i); > + > + switch (GET_CODE (e)) > + { > + case SET: > + src = SET_SRC (e); > + dest = SET_DEST (e); > + > + /* storing a register to memory. */ > + if (GET_CODE (src) == REG && GET_CODE (dest) == MEM) Please use REG_P (...) and MEM_P (...) - and possible others - predicates in the code. > + { > + rtx addr = XEXP (dest, 0); > + > + /* Good if dest address is in RAX. */ > + if (GET_CODE (addr) == REG > + && REGNO (addr) == AX_REG) > + continue; > + > + /* Good if dest address is offset of RAX. */ > + if (GET_CODE (addr) == PLUS > + && GET_CODE (XEXP (addr, 0)) == REG > + && REGNO (XEXP (addr, 0)) == AX_REG) > + continue; > + } > + break; > + > + default: > + break; > + } > + return false; > +} > + return true; > +}) > + > +;; Return true if: > +;; * first op is (return) or a a use (symbol reference), > +;; * >= 14 operands, and > +;; * operands 2 to end are one of: > +;; - restoring a register from a memory location that's an offset of RSI. > +;; - clobbering a reg > +;; - adjusting SP > +(define_predicate "restore_multiple" > + (match_code "parallel") > +{ > + const unsigned nregs = XVECLEN (op, 0); > + rtx head = XVECEXP (op, 0, 0); > + unsigned i; > + > + switch (GET_CODE (head)) > +{ > + case RETURN: > + i = 3; > + break; > + > + case USE: > + { > + rtx op0 = XEXP (head, 0); > + > + if (op0 == NULL_RTX || GET_CODE (op0) != SYMBOL_REF) > + return false; > + > + i = 1; > + break; > + } > + > + default: > + return false; > +} > + > + if (nregs < i + 12) > +return false; > + > + for (; i < nregs; i++) > +{ > + rtx e, src, dest; > + > + e = XVECEXP (op, 0, i); > + > + switch (GET_CODE (e)) > + { > + case CLOBBER: > + continue; I don't see where CLOBBER is genreated in ix86_emit_outlined_ms2sysv_restore. > + > + case SET: > + src = SET_SRC (e); > + dest = SET_DEST (e); > + > + /* Restoring a register from memory. */ > + if (GET_CODE (src) == MEM && GET_CODE (dest) == REG) > + { > + rtx addr = XEXP (src, 0); > + > + /* Good if src address is in RSI. */ > + if (GET_CODE (addr) == REG > + && REGNO (addr) == SI_REG) > + continue; > + > + /* Good if src address is offset of RSI. */ > + if (GET_CODE (addr) == PLUS > + && GET_CODE (XEXP (addr, 0)) == REG > + && REGNO (XEXP (addr, 0)) == SI_REG) > + continue; > + > + /* Good if adjusting stack pointer. */ > + if (GET_CODE (dest) == REG > + && REGNO (dest) ==
Re: [PATCH v4 0/12] [i386] Improve 64-bit Microsoft to System V ABI pro/epilogues
On Thu, Apr 27, 2017 at 10:04 AM, Daniel Santos wrote: > All of patches are concerned with 64-bit Microsoft ABI functions that call > System V ABI function which clobbers RSI, RDI and XMM6-15 and are aimed at > improving performance and .text size of Wine 64. I had previously submitted > these as separate patch sets, but have combined them for simplicity. (Does > this make the ChangeLogs too big? Please let me know if you want me to break > these back apart.) Below are the included patchsets and a summary of changes > since the previous post(s): Well, the ChangeLog is acceptable. I have comments on how new RTX patterns are generated and checked (patches 9/12 and 11/12). Other patches look good to me, so after issues with 9/12 and 11/12 are resolved, I think the patch set is ready to go. After the above issue is addressed, I propose to move forward by committing the patchset, and resolve any possible issues later. There are just too many code paths in the stack frame construction and teardown to notice all possible interactions between new and old code. It looks that existing code won't be affected without activating new option, so we can be a bit less cautious with the patchset. An important part is thus a comprehensive added test suite, which seems to pass. I also assume that Cygwin and MinGW people agree with the patch and the functionality itself. Uros. > 1.) PR78962 Use aligned SSE movs for re-aligned MS ABI pro/epilogues. > https://gcc.gnu.org/ml/gcc-patches/2016-12/msg01859.html > > Changes: > > * The SEH unwind emit code (in winnt.c) does not currently support >CFA_REG_EXPRESSION, which is required to make this work, so I have >disabled it on SEH targets. > * Updated comments on CFA_REG_EXPRESSION in winnt.c. > > > 2.) Add option to call out-of-line stubs instead of emitting inline saves > and restores. https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00548.html > > Changes: > > * Renamed option from -moutline-msabi-xlogues to -mcall-ms2sysv-xlogues > * Since this patch set depends upon aligned SSE MOVs after stack >realignment, I have disabled it on SEH targets with a sorry(). > * I was previously trying to cache the rtx for symbols to the libgcc >stubs instead of creating new ones, but this caused problems in >subsequent passes and it was disabled with a "TODO" comment. I have >removed this code, as well as the rtx cache that was just wasting >memory in class xlogue_layout. > * Improved comment documentation. > > > 3.) A comprehensive test program to validate correct behavior in these pro- > and epilogues. https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00542.html > > Changes: > > * The previous version repeated all tests for each -j instead of >running in parallel. I have fixed this implementing a primitive but >effective file-based parallelization scheme. > * I noticed that there was gcc/testsuite/gcc.target/x86_64/abi >directory for tests specific to testing 64-bit abi issues, so I've >moved my tests to an "ms-sysv" subdirectory of that (instead of >gcc/testsuite/gcc.target/i386/msabi). > * Fixed breakages on Cygwin. > * Corrected a bad "_noinfo" optimization barrier (function call by >volatile pointer). > * Minor cleanup/improvements. > > > gcc/Makefile.in| 2 + > gcc/config/i386/i386.c | 916 > +++-- > gcc/config/i386/i386.h | 33 +- > gcc/config/i386/i386.opt | 4 + > gcc/config/i386/predicates.md | 155 > gcc/config/i386/sse.md | 37 + > gcc/config/i386/winnt.c| 3 +- > gcc/doc/invoke.texi| 13 +- > .../gcc.target/x86_64/abi/ms-sysv/do-test.S| 163 > gcc/testsuite/gcc.target/x86_64/abi/ms-sysv/gen.cc | 807 ++ > .../gcc.target/x86_64/abi/ms-sysv/ms-sysv.c| 373 + > .../gcc.target/x86_64/abi/ms-sysv/ms-sysv.exp | 178 > libgcc/config.host | 2 +- > libgcc/config/i386/i386-asm.h | 82 ++ > libgcc/config/i386/resms64.S | 57 ++ > libgcc/config/i386/resms64f.S | 55 ++ > libgcc/config/i386/resms64fx.S | 57 ++ > libgcc/config/i386/resms64x.S | 59 ++ > libgcc/config/i386/savms64.S | 57 ++ > libgcc/config/i386/savms64f.S | 55 ++ > libgcc/config/i386/t-msabi | 7 + > 21 files changed, 3020 insertions(+), 95 deletions(-) > > > gcc/ChangeLog: > > 2017-04-25 Daniel Santos > > * config/i386/i386.opt: Add option -mcall-ms2sysv-xlogues. > * config/i386/i386.h > (x86_64_ms_sysv_extra_clobbered_registers): Change type to unsigned. > (NUM_X86_64_MS_CLOBBERED_REGS): New macro. > (st
[testsuite, committed, PR65941] Add and use effective target rdrand
Hi, atm I get these failures for x86_64 -m32: ... FAIL: g++.dg/other/pr59492.C -std=gnu++11 (test for excess errors) FAIL: g++.dg/other/pr59492.C -std=gnu++14 (test for excess errors) FAIL: g++.dg/other/pr59492.C -std=gnu++98 (test for excess errors) ... More specifically: ... /tmp/ccSwV0hl.s: Assembler messages: /tmp/ccSwV0hl.s:24: Error: no such instruction: `rdrand %edx' compiler exited with status 1 ... This patch turns the FAIL into an UNSUPPORTED. Committed as obvious. Thanks, - Tom Add and use effective target rdrand 2017-05-01 Tom de Vries PR testsuite/65941 * lib/target-supports.exp (check_effective_target_rdrand): New proc. * g++.dg/other/pr59492.C: Require effective target rdrand. --- gcc/testsuite/g++.dg/other/pr59492.C | 1 + gcc/testsuite/lib/target-supports.exp | 14 ++ 2 files changed, 15 insertions(+) diff --git a/gcc/testsuite/g++.dg/other/pr59492.C b/gcc/testsuite/g++.dg/other/pr59492.C index 84bd255..92694ae 100644 --- a/gcc/testsuite/g++.dg/other/pr59492.C +++ b/gcc/testsuite/g++.dg/other/pr59492.C @@ -2,6 +2,7 @@ // { dg-options "-mx32 -fPIC" } // { dg-require-ifunc "" } // { dg-require-effective-target maybe_x32 } +// { dg-require-effective-target rdrand } void __throw_runtime_error(const char*) __attribute__((__noreturn__)); diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 342af27..83e7f26 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -8296,6 +8296,20 @@ proc check_effective_target_store_merge { } { return 0 } +# Return 1 if we're able to assemble rdrand + +proc check_effective_target_rdrand { } { +return [check_no_compiler_messages_nocache rdrand object { + unsigned int + __foo(void) + { + unsigned int val; + __builtin_ia32_rdrand32_step(&val); + return val; + } +} "-mrdrnd" ] +} + # Return 1 if the target supports coprocessor instructions: cdp, ldc, stc, mcr and # mrc. proc check_effective_target_arm_coproc1_ok_nocache { } {
[PATCH] RISC-V: Add -mstrict-align option
From: Andrew Waterman The RISC-V user ISA permits misaligned accesses, but they may trap and be emulated. That emulation software needs to be compiled assuming strict alignment. Even when strict alignment is not required, set SLOW_UNALIGNED_ACCESS based upon -mtune to avoid a performance pitfall. 2017-01-19 Andrew Waterman * config/riscv/riscv.opt (mstrict-align): New option. * config/riscv/riscv.h (STRICT_ALIGNMENT): Use it. Update comment. (SLOW_UNALIGNED_ACCESS): Define. (riscv_slow_unaligned_access): Declare. * config/riscv/riscv.c (riscv_tune_info): Add slow_unaligned_access field. (riscv_slow_unaligned_access): New variable. (rocket_tune_info): Set slow_unaligned_access to true. (optimize_size_tune_info): Set slow_unaligned_access to false. (riscv_cpu_info_table): Add entry for optimize_size_tune_info. (riscv_valid_lo_sum_p): Use TARGET_STRICT_ALIGN. (riscv_option_override): Set riscv_slow_unaligned_access. * doc/invoke.texi: Add -mstrict-align to RISC-V. --- gcc/ChangeLog | 16 gcc/config/riscv/riscv.c | 20 +--- gcc/config/riscv/riscv.h | 10 ++ gcc/config/riscv/riscv.opt | 4 gcc/doc/invoke.texi| 6 ++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 2ad14ae..02c9808 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,19 @@ +2017-01-19 Andrew Waterman + + * config/riscv/riscv.opt (mstrict-align): New option. + * config/riscv/riscv.h (STRICT_ALIGNMENT): Use it. Update comment. + (SLOW_UNALIGNED_ACCESS): Define. + (riscv_slow_unaligned_access): Declare. + * config/riscv/riscv.c (riscv_tune_info): Add slow_unaligned_access + field. + (riscv_slow_unaligned_access): New variable. + (rocket_tune_info): Set slow_unaligned_access to true. + (optimize_size_tune_info): Set slow_unaligned_access to false. + (riscv_cpu_info_table): Add entry for optimize_size_tune_info. + (riscv_valid_lo_sum_p): Use TARGET_STRICT_ALIGN. + (riscv_option_override): Set riscv_slow_unaligned_access. + * doc/invoke.texi: Add -mstrict-align to RISC-V. + 2017-04-28 Palmer Dabbelt * config/riscv/t-elf-multilib32: New file. diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c index d5928c3..f7fec4b 100644 --- a/gcc/config/riscv/riscv.c +++ b/gcc/config/riscv/riscv.c @@ -255,6 +255,7 @@ struct riscv_tune_info unsigned short issue_rate; unsigned short branch_cost; unsigned short memory_cost; + bool slow_unaligned_access; }; /* Information about one CPU we know about. */ @@ -268,6 +269,9 @@ struct riscv_cpu_info { /* Global variables for machine-dependent things. */ +/* Whether unaligned accesses execute very slowly. */ +bool riscv_slow_unaligned_access; + /* Which tuning parameters to use. */ static const struct riscv_tune_info *tune_info; @@ -301,7 +305,8 @@ static const struct riscv_tune_info rocket_tune_info = { {COSTS_N_INSNS (6), COSTS_N_INSNS (6)}, /* int_div */ 1, /* issue_rate */ 3, /* branch_cost */ - 5/* memory_cost */ + 5, /* memory_cost */ + true,/* slow_unaligned_access */ }; /* Costs to use when optimizing for size. */ @@ -313,12 +318,14 @@ static const struct riscv_tune_info optimize_size_tune_info = { {COSTS_N_INSNS (1), COSTS_N_INSNS (1)}, /* int_div */ 1, /* issue_rate */ 1, /* branch_cost */ - 2/* memory_cost */ + 2, /* memory_cost */ + false, /* slow_unaligned_access */ }; /* A table describing all the processors GCC knows about. */ static const struct riscv_cpu_info riscv_cpu_info_table[] = { { "rocket", &rocket_tune_info }, + { "size", &optimize_size_tune_info }, }; /* Return the riscv_cpu_info entry for the given name string. */ @@ -726,7 +733,8 @@ riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, enum machine_mode mode) /* We may need to split multiword moves, so make sure that each word can be accessed without inducing a carry. */ if (GET_MODE_SIZE (mode) > UNITS_PER_WORD - && GET_MODE_BITSIZE (mode) > GET_MODE_ALIGNMENT (mode)) + && (!TARGET_STRICT_ALIGN + || GET_MODE_BITSIZE (mode) > GET_MODE_ALIGNMENT (mode))) return false; return true; @@ -3773,6 +3781,12 @@ riscv_option_override (void) RISCV_TUNE_STRING_DEFAULT); tune_info = optimize_size ? &optimize_size_tune_info : cpu->tune_info;
[PATCH, i386]: Fix PR 68491, __get_cpuid with 0 level breaks on early 486
Hello! > I've been asked to send the patch against trunk here. I have fixed the PR in a slightly different way. 2017-05-01 Uros Bizjak PR target/68491 * config/i386/cpuid.h (__get_cpuid): Always return 0 when __get_cpuid_max returns 0. (__get_cpuid_count): Ditto. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Committed to mainline SVN, and will port to release branches in a couple of days. Uros. Index: config/i386/cpuid.h === --- config/i386/cpuid.h (revision 247428) +++ config/i386/cpuid.h (working copy) @@ -246,8 +246,9 @@ __get_cpuid (unsigned int __leaf, unsigned int *__ecx, unsigned int *__edx) { unsigned int __ext = __leaf & 0x8000; + unsigned int __maxlevel = __get_cpuid_max (__ext, 0); - if (__get_cpuid_max (__ext, 0) < __leaf) + if (__maxlevel == 0 || __maxlevel < __leaf) return 0; __cpuid (__leaf, *__eax, *__ebx, *__ecx, *__edx); @@ -262,8 +263,9 @@ __get_cpuid_count (unsigned int __leaf, unsigned i unsigned int *__ecx, unsigned int *__edx) { unsigned int __ext = __leaf & 0x8000; + unsigned int __maxlevel = __get_cpuid_max (__ext, 0); - if (__get_cpuid_max (__ext, 0) < __leaf) + if (__maxlevel == 0 || __maxlevel < __leaf) return 0; __cpuid_count (__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)
Pedro's suggestions all sound good to me. Jason
Re: [PATCH] Implement a warning for bogus sizeof(pointer) / sizeof(pointer[0])
On Fri, Apr 28, 2017 at 1:05 PM, Bernd Edlinger wrote: > On 04/28/17 17:29, Martin Sebor wrote: >> On 04/28/2017 08:12 AM, Bernd Edlinger wrote: >>> >>> Do you want me to change the %qT format strings to %T ? >> >> Yes, with the surrounding %< and %> the nested directives should >> use the unquoted forms, otherwise the printer would end up quoting >> both the whole expression and the type operand. >> >> FWIW, to help avoid this mistake, I think this might be something >> for GCC -Wformat to warn on and the pretty-printer to detect (and >> ICE on). >> > > Ah, now I understand. That's pretty advanced. > > Here is the modified patch with correct quoting of the expression. > > Bootstrap and reg-testing on x86_64-pc-linux-gnu. > * cp-gimplify.c (cp_fold): Implement the -Wsizeof_pointer_div warning. I think this warning belongs in cp_build_binary_op rather than cp_fold. Jason
Re: [C++ Patch] Remove is_auto_or_concept, etc (Was: Re: [C++, concepts] Two slightly obscure pt.c functions?)
OK. On Thu, Apr 27, 2017 at 2:02 PM, Paolo Carlini wrote: > Hi again, > > On 26/04/2017 12:32, Paolo Carlini wrote: >> >> Hi, >> >> in 2013 (2013-09-16) Adam added two slightly obscure functions and I can't >> find much around in terms of rationale, etc: >> >> /* Returns true iff TYPE is a TEMPLATE_TYPE_PARM representing 'auto', >>'decltype(auto)' or a concept. */ >> >> bool >> is_auto_or_concept (const_tree type) >> { >> return is_auto (type); // or concept >> } >> >> /* Returns the TEMPLATE_TYPE_PARM in TYPE representing a generic type >> (`auto' or >>a concept identifier) iff TYPE contains a use of a generic type. >> Returns >>NULL_TREE otherwise. */ >> >> tree >> type_uses_auto_or_concept (tree type) >> { >> return find_type_usage (type, is_auto_or_concept); >> } >> >> The latter seems completely unused (it's meant for debugging purposes?); >> the former evidently simply forwards to is_auto, and we end up in the >> front-end with uses of both, which in fact are equivalent, which seems >> weird: IMHO, if they are actually equivalent in our implementation we should >> clearly explain that in the comment and have only one. Or what? > > > ... replying to myself, in practice we could do the below, which certainly > passes testing, and in fact now seems to me even more obvious than I thought > a couple of days ago... > > Thanks, > Paolo. > > /
Re: [PATCH, i386]: Fix PR 68491, __get_cpuid with 0 level breaks on early 486
On May 1, 5:42pm, ubiz...@gmail.com (Uros Bizjak) wrote: -- Subject: [PATCH, i386]: Fix PR 68491, __get_cpuid with 0 level breaks on e | Hello! | | > I've been asked to send the patch against trunk here. | | I have fixed the PR in a slightly different way. Thank you! christos
[patch, fortran] Check bounds for matrix-vector multiplication
Hello world, the attached patch also performs a check for matrix-vector multiplication. I found it by poking around the source while looking to do some improvements, which should be coming up shortly once this is out of the way. Regression-tested. OK for trunk? Regards Thomas 2017-05-01 Thomas Koenig PR fortran/37131 * frontend-passes.c (inline_matmul_assign): Also check bounds for allocatable lhs and matrix-vector-multiplication. 2017-05-01 Thomas Koenig PR fortran/37131 * gfortran.dg/matmul_bounds_11.f90: New test. Index: frontend-passes.c === --- frontend-passes.c (Revision 247003) +++ frontend-passes.c (Arbeitskopie) @@ -3068,20 +3068,31 @@ inline_matmul_assign (gfc_code **c, int *walk_subt /* Only need to check a single dimension for the A2B2 case for bounds checking, the rest will be allocated. */ - if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS && m_case == A2B2) + if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) { gfc_code *test; gfc_expr *a2, *b1; - a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); - b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); - test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " + if (m_case == A2B2) + { + a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); + b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); + test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " + "in MATMUL intrinsic: Is %ld, should be %ld"); + *next_code_point = test; + next_code_point = &test->next; + } + else if (m_case == A2B1) + { + a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); + b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); + test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " "in MATMUL intrinsic: Is %ld, should be %ld"); - *next_code_point = test; - next_code_point = &test->next; + *next_code_point = test; + next_code_point = &test->next; + } } - lhs_alloc = matmul_lhs_realloc (expr1, matrix_a, matrix_b, m_case); *next_code_point = lhs_alloc; ! { dg-do run } ! { dg-options "-O -finline-matmul-limit=30 -fcheck=all" } ! { dg-shouldfail "Dimension of array B incorrect in MATMUL intrinsic" } program main real, dimension(:,:), allocatable :: a real, dimension(:), allocatable :: b real, dimension(:), allocatable :: res allocate (a(2,2), b(3)) call random_number(a) call random_number(b) res = matmul(a,b) print *,res end program main ! { dg-output "Fortran runtime error: Dimension of array B incorrect in MATMUL intrinsic.*" }
Re: [patch, fortran] Check bounds for matrix-vector multiplication
On Mon, May 01, 2017 at 06:19:28PM +0200, Thomas Koenig wrote: > + if (m_case == A2B2) > + { > + a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); > + b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); > + test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " > + "in MATMUL intrinsic: Is %ld, should be > %ld"); > + *next_code_point = test; > + next_code_point = &test->next; > + } > + else if (m_case == A2B1) > + { > + a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); > + b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); > + test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " > "in MATMUL intrinsic: Is %ld, should be > %ld"); > - *next_code_point = test; > - next_code_point = &test->next; > + *next_code_point = test; > + next_code_point = &test->next; > + } > } Why the duplicate code? Seems like an OR is needed. if (m_case == A2B1 || m_case == A2B2) { a2 = get_array_inq_function (GFC_ISYM_SIZE, matrix_a, 2); b1 = get_array_inq_function (GFC_ISYM_SIZE, matrix_b, 1); test = runtime_error_ne (b1, a2, "Dimension of array B incorrect " "in MATMUL intrinsic: Is %ld, should be %ld"); *next_code_point = test; next_code_point = &test->next; } -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Re: PR79715: Special case strcpy/strncpy for dse
On 04/24/2017 03:05 AM, Prathamesh Kulkarni wrote: On 1 March 2017 at 13:24, Richard Biener wrote: On Tue, 28 Feb 2017, Jeff Law wrote: On 02/28/2017 05:59 AM, Prathamesh Kulkarni wrote: On 28 February 2017 at 15:40, Jakub Jelinek wrote: On Tue, Feb 28, 2017 at 03:33:11PM +0530, Prathamesh Kulkarni wrote: Hi, The attached patch adds special-casing for strcpy/strncpy to dse pass. Bootstrapped+tested on x86_64-unknown-linux-gnu. OK for GCC 8 ? What is special on strcpy/strncpy? Unlike memcpy/memmove/memset, you don't know the length they store (at least not in general), you don't know the value, all you know is that they are for the first argument plain store without remembering the pointer or anything based on it anywhere except in the return value. I believe stpcpy, stpncpy, strcat, strncat at least have the same behavior. On the other side, without knowing the length of the store, you can't treat it as killing something (ok, some calls like strcpy or stpcpy store at least the first byte). Well, I assumed a store to dest by strcpy (and friends), which gets immediately freed would count as a dead store since free would kill the whole memory block pointed to by dest ? Yes. But does it happen often in practice? I wouldn't mind exploring this for gcc-8, but I'd like to see real-world code where this happens. Actually I don't mind for "real-world code" - the important part is that I believe it's reasonable to assume it can happen from some C++ abstraction and optimization. Hi, I have updated the patch to include stp[n]cpy and str[n]cat. In initialize_ao_ref_for_dse for strncat, I suppose for strncat we need to treat size as NULL_TREE instead of setting it 2nd arg since we cannot know size (in general) after strncat ? Patch passes bootstrap+test on x86_64-unknown-linux-gnu. Cross-tested on arm*-*-*, aarch64*-*-*. I think I made a mistake when I opened PR79715: I used the wrong -fdump-tree- option and was looking at the wrong dump. It turns out that the test case there is optimized as I expected, except by a later pass, and has been since r233267. I resolved the bug as fixed before I realized that this patch (that I've been meaning to review) is for that bug, except that it enables the optimization to take place earlier. That sounds like a fine idea to me but now that I closed the bug I wonder if it should be done under a different bug. This is just a suggestion and if you don't want to take the time to raise a new bug for I don't mind if you reopen bug 79715. Sorry if I made things more complicated than they need to be. Martin
Re: [PATCH] RISC-V: Add -mstrict-align option
On 05/01/2017 09:40 AM, Palmer Dabbelt wrote: [snip] diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt index 0466bb2..0422e07 100644 --- a/gcc/config/riscv/riscv.opt +++ b/gcc/config/riscv/riscv.opt @@ -84,6 +84,10 @@ mcmodel= Target Report RejectNegative Joined Enum(code_model) Var(riscv_cmodel) Init(TARGET_DEFAULT_CMODEL) Specify the code model. +mstrict-align +Target Report Mask(STRICT_ALIGN) Save +Assume that unaligned memory accesses are disallowed. + It's not clear from that description who is disallowing the accesses, and what the assumption implies. How about just: Do not generate unaligned memory accesses. @@ -20945,6 +20946,11 @@ Put global and static data smaller than @var{n} bytes into a special section @opindex msave-restore Use smaller but slower prologue and epilogue code. +@item -mstrict-align +@itemx -mno-strict-align +@opindex mstrict-align +Assume that unaligned memory accesses are disallowed. + Ditto here. -Sandra
[PATCH/AARCH64] Improve aarch64 conditional compare usage
This is a resubmittal of an earlier patch (https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00203.html) to improve the use of ccmp (conditional compare) on aarch64. I made a couple of tweaks after the first submittal and retested now that we are back in stage 1. Most of the changes are restructuring the code to allow the change and do not affect the actual output. The actual behavour change is in ccmp_tree_comparison_p where we recoginize a boolean variable as well as a compare expression as code that can be done with a conditionial compare and in get_compare_parts where we treat a boolean variable X as 'X != 0' and generate that comparision. Since the code in ccmp.c is ony used when TARGET_GEN_CCMP_FIRST is set and TARGET_GEN_CCMP_FIRST is only set for aarch64 this change will only affect aarch64. Tested with no regressions and a new test is added to verify that we generate a ccmp instruction with the change. I ran the SPEC2006 int tests and got a .02 increase in the SPECmark on a ThunderX box. The biggest increases were in mcf and astar. One test, xlancbmk, did slow down but the overall SPEC result was a speedup. OK for checkin? Steve Ellcey sell...@cavium.com GCC ChangeLog: 2017-05-01 Steve Ellcey * ccmp.c (ccmp_tree_comparison_p): New function. (ccmp_candidate_p): Update to use above function. (get_compare_parts): New function. (expand_ccmp_next): Update to use new functions. (expand_ccmp_expr_1): Take tree arg instead of gimple, update to use new functions. (expand_ccmp_expr): Pass tree instead of gimple to expand_ccmp_expr_1, take mode as argument. * ccmp.h (expand_ccmp_expr): Add mode as argument. * expr.c (expand_expr_real_1): Pass mode as argument. GCC Testsuite ChangeLog: 2017-05-01 Steve Ellcey * gcc.target/aarch64/ccmp_2.c: New test.diff --git a/gcc/ccmp.c b/gcc/ccmp.c index 92ca133..4fa3ebd 100644 --- a/gcc/ccmp.c +++ b/gcc/ccmp.c @@ -38,6 +38,29 @@ along with GCC; see the file COPYING3. If not see #include "ccmp.h" #include "predict.h" +/* Check whether T is a simple boolean variable or a SSA name + set by a comparison operator in the same basic block. */ +static bool +ccmp_tree_comparison_p (tree t, basic_block bb) +{ + gimple *g = get_gimple_for_ssa_name (t); + tree_code tcode; + + /* If we have a boolean variable allow it and generate a compare + to zero reg when expanding. */ + if (!g) +return (TREE_CODE (TREE_TYPE (t)) == BOOLEAN_TYPE); + + /* Check to see if SSA name is set by a comparison operator in + the same basic block. */ + if (!is_gimple_assign (g)) +return false; + if (bb != gimple_bb (g)) +return false; + tcode = gimple_assign_rhs_code (g); + return TREE_CODE_CLASS (tcode) == tcc_comparison; +} + /* The following functions expand conditional compare (CCMP) instructions. Here is a short description about the over all algorithm: * ccmp_candidate_p is used to identify the CCMP candidate @@ -71,49 +94,69 @@ along with GCC; see the file COPYING3. If not see static bool ccmp_candidate_p (gimple *g) { - tree rhs = gimple_assign_rhs_to_tree (g); + tree rhs; tree lhs, op0, op1; gimple *gs0, *gs1; - tree_code tcode, tcode0, tcode1; - tcode = TREE_CODE (rhs); + tree_code tcode; + basic_block bb; + + if (!g) +return false; + rhs = gimple_assign_rhs_to_tree (g); + tcode = TREE_CODE (rhs); if (tcode != BIT_AND_EXPR && tcode != BIT_IOR_EXPR) return false; lhs = gimple_assign_lhs (g); op0 = TREE_OPERAND (rhs, 0); op1 = TREE_OPERAND (rhs, 1); + bb = gimple_bb (g); if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME) || !has_single_use (lhs)) return false; - gs0 = get_gimple_for_ssa_name (op0); - gs1 = get_gimple_for_ssa_name (op1); - if (!gs0 || !gs1 || !is_gimple_assign (gs0) || !is_gimple_assign (gs1) - /* g, gs0 and gs1 must be in the same basic block, since current stage - is out-of-ssa. We can not guarantee the correctness when forwording - the gs0 and gs1 into g whithout DATAFLOW analysis. */ - || gimple_bb (gs0) != gimple_bb (gs1) - || gimple_bb (gs0) != gimple_bb (g)) -return false; + gs0 = get_gimple_for_ssa_name (op0); /* gs0 may be NULL */ + gs1 = get_gimple_for_ssa_name (op1); /* gs1 may be NULL */ - tcode0 = gimple_assign_rhs_code (gs0); - tcode1 = gimple_assign_rhs_code (gs1); - if (TREE_CODE_CLASS (tcode0) == tcc_comparison - && TREE_CODE_CLASS (tcode1) == tcc_comparison) + if (ccmp_tree_comparison_p (op0, bb) && ccmp_tree_comparison_p (op1, bb)) return true; - if (TREE_CODE_CLASS (tcode0) == tcc_comparison - && ccmp_candidate_p (gs1)) + if (ccmp_tree_comparison_p (op0, bb) && ccmp_candidate_p (gs1)) return true; - else if (TREE_CODE_CLASS (tcode1) == tcc_comparison - && ccmp_candidate_p (gs0)) + if (ccmp_tree_comparison_p (op1, bb) && ccmp_candidate_p (gs0)) return true; /* We s
Re: [PATCH] RISC-V: Add -mstrict-align option
On Mon, 01 May 2017 10:08:08 PDT (-0700), san...@codesourcery.com wrote: > On 05/01/2017 09:40 AM, Palmer Dabbelt wrote: >> [snip] >> >> diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt >> index 0466bb2..0422e07 100644 >> --- a/gcc/config/riscv/riscv.opt >> +++ b/gcc/config/riscv/riscv.opt >> @@ -84,6 +84,10 @@ mcmodel= >> Target Report RejectNegative Joined Enum(code_model) Var(riscv_cmodel) >> Init(TARGET_DEFAULT_CMODEL) >> Specify the code model. >> >> +mstrict-align >> +Target Report Mask(STRICT_ALIGN) Save >> +Assume that unaligned memory accesses are disallowed. >> + > > It's not clear from that description who is disallowing the accesses, > and what the assumption implies. How about just: > > Do not generate unaligned memory accesses. That sounds better, if there's no other comments then I'll commit that version. > >> @@ -20945,6 +20946,11 @@ Put global and static data smaller than @var{n} >> bytes into a special section >> @opindex msave-restore >> Use smaller but slower prologue and epilogue code. >> >> +@item -mstrict-align >> +@itemx -mno-strict-align >> +@opindex mstrict-align >> +Assume that unaligned memory accesses are disallowed. >> + > > Ditto here. Thanks!
Re: [patch, fortran] Check bounds for matrix-vector multiplication
Hi Steve, Why the duplicate code? Seems like an OR is needed. You are quite right. Here's an updated patch: Index: frontend-passes.c === --- frontend-passes.c (Revision 247003) +++ frontend-passes.c (Arbeitskopie) @@ -3066,9 +3066,10 @@ gfc_code *lhs_alloc; /* Only need to check a single dimension for the A2B2 case for -bounds checking, the rest will be allocated. */ +bounds checking, the rest will be allocated. Also check this +for A2B1. */ - if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS && m_case == A2B2) + if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) && (m_case == A2B2 || m_case == A2B1)) { gfc_code *test; gfc_expr *a2, *b1; OK for trunk? Regards Thomas
Re: [patch, fortran] Check bounds for matrix-vector multiplication
On Mon, May 01, 2017 at 07:21:03PM +0200, Thomas Koenig wrote: > Hi Steve, > > > Why the duplicate code? Seems like an OR is needed. > > You are quite right. > > Here's an updated patch: > > Index: frontend-passes.c > === > --- frontend-passes.c (Revision 247003) > +++ frontend-passes.c (Arbeitskopie) > @@ -3066,9 +3066,10 @@ > gfc_code *lhs_alloc; > > /* Only need to check a single dimension for the A2B2 case for > -bounds checking, the rest will be allocated. */ > +bounds checking, the rest will be allocated. Also check this > +for A2B1. */ > > - if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS && m_case == A2B2) > + if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) && (m_case == A2B2 > || m_case == A2B1)) > { >gfc_code *test; >gfc_expr *a2, *b1; > > OK for trunk? > Yes. -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
[wwwdocs] GCC 7 changes: OpenACC
Hi! Committed: Index: htdocs/gcc-7/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/changes.html,v retrieving revision 1.79 retrieving revision 1.80 diff -u -p -r1.79 -r1.80 --- htdocs/gcc-7/changes.html 21 Apr 2017 23:52:13 - 1.79 +++ htdocs/gcc-7/changes.html 1 May 2017 17:51:14 - 1.80 @@ -157,7 +157,13 @@ main (int argc, char **argv) New Languages and Language specific improvements +OpenACC support in C, C++, and Fortran continues to be maintained and +improved. +See the https://gcc.gnu.org/wiki/OpenACC";>OpenACC +and https://gcc.gnu.org/wiki/Offloading";>Offloading wiki pages +for further information. + Grüße Thomas
[PATCH 0/5][GIMPLE FE] PR testsuite/80580. Fix some ICEs on invalid code
Hi, all. As I have already mentioned in the bug report (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80580), I performed some fuzz testing of the GIMPLE front end. I used a technique proposed by John Regehr in his blog post http://blog.regehr.org/archives/1284 for testing C++ compilers. In short, this technique works as follows: 1. take a valid input file as a starting point (I used the GIMPLE code from the GCC test suite) 2. try to remove several tokens from the current input file in such way that the file remains valid (using CReduce) 3. repeat step 2 while possible +record all ICEs found during this process. As a result I found 46 GIMPLE source files that cause ICEs and produce distinct backtraces (see the attachment in Bugzilla). This series of patches fixes some of these ICEs. I have bootstrapped and regtested the unified patch on x86_64-pc-linux-gnu with no regressions (although, I see some noise in the tree-prof tests). The patches are intended for GCC 8. -- Regards, Mikhail Maltsev
[PATCH 1/5][GIMPLE FE] PR testsuite/80580. Handle missing labels in goto statements
The first problem happens because we don't check for missing labels when parsing 'goto' statements. I.e.: __GIMPLE() void fn1() { if (1) goto } The fix is pretty obvious: just add a check. My question is: which functions should I use to produce diagnostics? The surrounding code uses 'c_parser_error', but this function does not handle locations very well (in fact, it uses input_location). -- Regards, Mikhail Maltsev gcc/testsuite/ChangeLog: 2017-05-01 Mikhail Maltsev * gcc.dg/gimplefe-error-4.c: New test. * gcc.dg/gimplefe-error-5.c: New test. gcc/c/ChangeLog: 2017-05-01 Mikhail Maltsev * gimple-parser.c (c_parser_gimple_if_stmt): Check for empty labels. From 07453ba1bf0b1290cef54dcb028cb477b17966df Mon Sep 17 00:00:00 2001 From: Mikhail Maltsev Date: Fri, 24 Feb 2017 13:09:10 +0300 Subject: [PATCH 1/5] GIMPLEFE: handle missing labels in goto statements --- gcc/c/gimple-parser.c | 10 ++ gcc/testsuite/gcc.dg/gimplefe-error-4.c | 7 +++ gcc/testsuite/gcc.dg/gimplefe-error-5.c | 9 + 3 files changed, 26 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-4.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-5.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index 0d6384b..a99b502 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -1315,6 +1315,11 @@ c_parser_gimple_if_stmt (c_parser *parser, gimple_seq *seq) loc = c_parser_peek_token (parser)->location; c_parser_consume_token (parser); label = c_parser_peek_token (parser)->value; + if (! label) + { + c_parser_error (parser, "expected label"); + return; + } t_label = lookup_label_for_goto (loc, label); c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1339,6 +1344,11 @@ c_parser_gimple_if_stmt (c_parser *parser, gimple_seq *seq) loc = c_parser_peek_token (parser)->location; c_parser_consume_token (parser); label = c_parser_peek_token (parser)->value; + if (! label) + { + c_parser_error (parser, "expected label"); + return; + } f_label = lookup_label_for_goto (loc, label); c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-4.c b/gcc/testsuite/gcc.dg/gimplefe-error-4.c new file mode 100644 index 000..c61539c --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-4.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void fn1() { + if (1) +goto +} /* { dg-error "expected label" } */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-5.c b/gcc/testsuite/gcc.dg/gimplefe-error-5.c new file mode 100644 index 000..7398861 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-5.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void fn1() { + if (1) +goto lbl; + else +goto +} /* { dg-error "expected label" } */ -- 2.1.4
Re: [PATCH] adding missing LTO to some warning options (PR 78606)
On 04/30/2017 02:02 PM, Tom de Vries wrote: On 01/10/2017 11:16 PM, Martin Sebor wrote: + __builtin_sprintf (d, "%32s", "x"); /* { dg-warning "directive writing 32 bytes into a region of size 12" "-Wformat-length" { xfail *-*-* } } */ This xpasses for me on an older system: ... XPASS: gcc.dg/pr78768.c -Wformat-overflow (test for warnings, line 11) ... The mechanism is as follows: - the system doesn't have linker plugin support, and sets HAVE_LTO_PLUGIN to 0 - consequently, opts.c:finish_options() sets x_flag_fat_lto_objects to 1 in cc1 - cgraphunit.c:symbol_table::compile() does not return here: ... /* Do nothing else if any IPA pass found errors or if we are just streaming LTO. */ if (seen_error () || (!in_lto_p && flag_lto && !flag_fat_lto_objects)) { timevar_pop (TV_CGRAPHOPT); return; } ... and ends up calling expand_all_functions, which calls pass_sprintf_length - the warning is generated by cc1 Maybe the test needs: ... /* { dg-require-linker-plugin "" } */ ... ? That seems possible. IIUC, without linker plugin support the pass will run when the ordinary object file is created during the first stage of compilation. I don't have a system to confirm it on but if pr78768.c xfails when you add the directive I'd say go ahead and commit the fix as obvious. Thanks Martin
[PATCH 2/5][GIMPLE FE] PR testsuite/80580: handle invalid types of "->" operands
This bug happens when the LHS of operator '->' is either missing, i.e.: (->a) = 0; or it is not a pointer: int b; b_2->c = 0; LHS should be validated. -- Regards, Mikhail Maltsev gcc/testsuite/ChangeLog: 2017-05-01 Mikhail Maltsev * gcc.dg/gimplefe-error-6.c: New test. * gcc.dg/gimplefe-error-7.c: New test. gcc/c/ChangeLog: 2017-05-01 Mikhail Maltsev * gimple-parser.c (c_parser_gimple_postfix_expression_after_primary): Check LHS of operator '->' From 8fc6470a428f312bad1dd32d526b51d44d34e16e Mon Sep 17 00:00:00 2001 From: Mikhail Maltsev Date: Fri, 24 Feb 2017 18:26:03 +0300 Subject: [PATCH 2/5] GIMPLEFE: handle invalid types of "->" operands --- gcc/c/gimple-parser.c | 9 + gcc/testsuite/gcc.dg/gimplefe-error-6.c | 8 gcc/testsuite/gcc.dg/gimplefe-error-7.c | 7 +++ 3 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-6.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-7.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index a99b502..2e11567 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -1052,6 +1052,15 @@ c_parser_gimple_postfix_expression_after_primary (c_parser *parser, start = expr.get_start (); finish = c_parser_peek_token (parser)->get_finish (); c_parser_consume_token (parser); + if (!TREE_TYPE (expr.value) + || !POINTER_TYPE_P (TREE_TYPE (expr.value))) + { + error_at (op_loc, "invalid dereference"); + expr.set_error (); + expr.original_code = ERROR_MARK; + expr.original_type = NULL; + return expr; + } expr.value = build_component_ref (op_loc, build_simple_mem_ref_loc (op_loc, expr.value), diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-6.c b/gcc/testsuite/gcc.dg/gimplefe-error-6.c new file mode 100644 index 000..83ea19e --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-6.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void a() +{ + int b; + b_2->c = 0; /* { dg-error "dereference" } */ +} diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-7.c b/gcc/testsuite/gcc.dg/gimplefe-error-7.c new file mode 100644 index 000..ac5c1ad --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-7.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void b() +{ + (->a) = 0; /* { dg-error "expected|dereference" } */ +} -- 2.1.4
[PATCH 3/5][GIMPLE FE] PR testsuite/80580. Handle invalid unary "*" operand type
This is essentially the same problem as in patch 2, but with unary '*'. We should verify that its argument is a pointer. -- Regards, Mikhail Maltsev gcc/c/ChangeLog: 2017-05-01 Mikhail Maltsev * gimple-parser.c (c_parser_gimple_unary_expression): Check argument type of unary '*'. gcc/testsuite/ChangeLog: 2017-05-01 Mikhail Maltsev * gcc.dg/gimplefe-error-8.c: New test. From 3e0452e939e42af06365d7d49157c227f53a7522 Mon Sep 17 00:00:00 2001 From: Mikhail Maltsev Date: Fri, 24 Feb 2017 18:29:57 +0300 Subject: [PATCH 3/5] GIMPLEFE: handle invalid unary "*" operand type --- gcc/c/gimple-parser.c | 5 + gcc/testsuite/gcc.dg/gimplefe-error-8.c | 7 +++ 2 files changed, 12 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-8.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index 2e11567..5249e8a 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -567,6 +567,11 @@ c_parser_gimple_unary_expression (c_parser *parser) op = c_parser_gimple_postfix_expression (parser); if (op.value == error_mark_node) return ret; + if (! POINTER_TYPE_P (TREE_TYPE (op.value))) + { + error_at (op_loc, "invalid type argument of unary %<*%>"); + return ret; + } finish = op.get_finish (); location_t combined_loc = make_location (op_loc, op_loc, finish); ret.value = build_simple_mem_ref_loc (combined_loc, op.value); diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-8.c b/gcc/testsuite/gcc.dg/gimplefe-error-8.c new file mode 100644 index 000..faf699d --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-8.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void a() +{ + *0 = 1; /* { dg-error "invalid type" } */ +} -- 2.1.4
[PATCH 4/5][GIMPLE FE] PR testsuite/80580. Handle invalid __MEM
This patch deals with invalid __MEM construct. Before we start building an expression for __MEM, we must check that parsing succeeded and that the __MEM operand is a pointer. -- Regards, Mikhail Maltsev gcc/c/ChangeLog: 2017-05-01 Mikhail Maltsev * gimple-parser.c (c_parser_gimple_postfix_expression): Handle invalid __MEM. gcc/testsuite/ChangeLog: 2017-05-01 Mikhail Maltsev * gcc.dg/gimplefe-error-9.c: New test. From fc2fe1f2f74ce399f9617fb526668bf1d57b0162 Mon Sep 17 00:00:00 2001 From: Mikhail Maltsev Date: Fri, 24 Feb 2017 20:45:45 +0300 Subject: [PATCH 4/5] GIMPLEFE: handle invalid __MEM --- gcc/c/gimple-parser.c | 10 ++ gcc/testsuite/gcc.dg/gimplefe-error-9.c | 7 +++ 2 files changed, 17 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-9.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index 5249e8a..f3af840 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -804,6 +804,16 @@ c_parser_gimple_postfix_expression (c_parser *parser) } } ptr = c_parser_gimple_unary_expression (parser); + if (ptr.value == error_mark_node + || ! POINTER_TYPE_P (TREE_TYPE (ptr.value))) + { + if (ptr.value != error_mark_node) + error_at (ptr.get_start (), + "invalid type of %<__MEM%> operand"); + c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, +"expected %<)%>"); + return expr; + } if (! alias_type) alias_type = TREE_TYPE (ptr.value); /* Optional constant offset. */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-9.c b/gcc/testsuite/gcc.dg/gimplefe-error-9.c new file mode 100644 index 000..2bdb398 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-9.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +__GIMPLE() void a() +{ + __MEM() = 0; /* { dg-error "expected .<." } */ +} -- 2.1.4
[PATCH 5/5][GIMPLE FE] PR testsuite/80580: Handle invalid SSA names
When parsing SSA names, we should check that parent names are scalars. In fact, this patch just uses the condition of a 'gcc_assert' in 'make_ssa_name_fn'. -- Regards, Mikhail Maltsev gcc/testsuite/ChangeLog: 2017-05-01 Mikhail Maltsev * gcc.dg/gimplefe-error-11.c: New test. gcc/c/ChangeLog: 2017-05-01 Mikhail Maltsev * gimple-parser.c (c_parser_parse_ssa_name): Validate SSA name base. From bae6cf05131c284fc8ae9a02f2ba99d447d04fd2 Mon Sep 17 00:00:00 2001 From: Mikhail Maltsev Date: Fri, 24 Feb 2017 20:54:40 +0300 Subject: [PATCH 5/5] GIMPLEFE: Handle invalid SSA names --- gcc/c/gimple-parser.c| 8 gcc/testsuite/gcc.dg/gimplefe-error-11.c | 9 + 2 files changed, 17 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-11.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index f3af840..ac8e7a7 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -692,6 +692,14 @@ c_parser_parse_ssa_name (c_parser *parser, c_parser_error (parser, "base variable or SSA name undeclared"); return error_mark_node; } + if (!(VAR_P (parent) + || TREE_CODE (parent) == PARM_DECL + || TREE_CODE (parent) == RESULT_DECL + || (TYPE_P (parent) && is_gimple_reg_type (parent + { + error ("invalid SSA name %qE", parent); + return error_mark_node; + } if (VECTOR_TYPE_P (TREE_TYPE (parent)) || TREE_CODE (TREE_TYPE (parent)) == COMPLEX_TYPE) DECL_GIMPLE_REG_P (parent) = 1; diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-11.c b/gcc/testsuite/gcc.dg/gimplefe-error-11.c new file mode 100644 index 000..c73b85c --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-11.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +void a(int); + +__GIMPLE() void b() +{ + a_2 = 0; /* { dg-error "invalid" } */ +} -- 2.1.4
Re: PR79715: Special case strcpy/strncpy for dse
On April 28, 2017 8:14:56 PM GMT+02:00, Jeff Law wrote: >On 04/24/2017 03:05 AM, Prathamesh Kulkarni wrote: >> On 1 March 2017 at 13:24, Richard Biener wrote: >>> On Tue, 28 Feb 2017, Jeff Law wrote: >>> On 02/28/2017 05:59 AM, Prathamesh Kulkarni wrote: > On 28 February 2017 at 15:40, Jakub Jelinek >wrote: >> On Tue, Feb 28, 2017 at 03:33:11PM +0530, Prathamesh Kulkarni >wrote: >>> Hi, >>> The attached patch adds special-casing for strcpy/strncpy to dse >pass. >>> Bootstrapped+tested on x86_64-unknown-linux-gnu. >>> OK for GCC 8 ? >> What is special on strcpy/strncpy? Unlike memcpy/memmove/memset, >you >> don't >> know the length they store (at least not in general), you don't >know the >> value, all you know is that they are for the first argument plain >store >> without remembering the pointer or anything based on it anywhere >except in >> the return value. >> I believe stpcpy, stpncpy, strcat, strncat at least have the same >> behavior. >> On the other side, without knowing the length of the store, you >can't >> treat >> it as killing something (ok, some calls like strcpy or stpcpy >store at >> least >> the first byte). > Well, I assumed a store to dest by strcpy (and friends), which >gets > immediately freed would count > as a dead store since free would kill the whole memory block >pointed > to by dest ? Yes. But does it happen often in practice? I wouldn't mind >exploring this for gcc-8, but I'd like to see real-world code where this happens. >>> Actually I don't mind for "real-world code" - the important part is >>> that I believe it's reasonable to assume it can happen from some C++ >>> abstraction and optimization. >> Hi, >> I have updated the patch to include stp[n]cpy and str[n]cat. >> In initialize_ao_ref_for_dse for strncat, I suppose for strncat we >> need to treat size as NULL_TREE >> instead of setting it 2nd arg since we cannot know size (in general) >> after strncat ? >The problem isn't the size, strncat will write the appropriate number >of >characters, just like strncpy, stpncpy. The problem is that we don't >know where the write will start. I'll touch further on this. > > >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu. >> Cross-tested on arm*-*-*, aarch64*-*-*. >> >> Thanks, >> Prathamesh >>> Richard. >> >> pr79715-2.txt >> >> >> 2017-04-24 Prathamesh Kulkarni >> >> * tree-ssa-dse.c (initialize_ao_ref_for_dse): Add cases for >> BUILT_IN_STRNCPY, BUILT_IN_STRCPY, BUILT_IN_STPNCPY, >BUILT_IN_STPCPY, >> BUILT_IN_STRNCAT, BUILT_IN_STRCAT. >> (maybe_trim_memstar_call): Likewise. >> (dse_dom_walker::dse_optimize_stmt): Likewise. >> >> testsuite/ >> * gcc.dg/tree-ssa/pr79715.c: New test. >I'd still be surprised if this kind of think happens in the real world, > >even with layers of abstraction & templates. > > > >> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c >> index 90230ab..752b2fa 100644 >> --- a/gcc/tree-ssa-dse.c >> +++ b/gcc/tree-ssa-dse.c >> @@ -92,15 +92,24 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref >*write) >> /* It's advantageous to handle certain mem* functions. */ >> if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) >> { >> + tree size = NULL_TREE; >> switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) >> { >>case BUILT_IN_MEMCPY: >>case BUILT_IN_MEMMOVE: >>case BUILT_IN_MEMSET: >> + case BUILT_IN_STRNCPY: >> + case BUILT_IN_STRCPY: >> + case BUILT_IN_STPNCPY: >> + case BUILT_IN_STPCPY: >> { >> - tree size = NULL_TREE; >>if (gimple_call_num_args (stmt) == 3) >> size = gimple_call_arg (stmt, 2)This part seems reasonable. We >know the size for strncpy, stpncpy which >we extract from argument #3. For strcpy and stpcpy we'd have NULL for >the size which is perfect. In all 4 cases the write starts at offset 0 > >in the destination string. >. > > > >> +} >> + /* fallthrough. */ >> + case BUILT_IN_STRCAT: >> + case BUILT_IN_STRNCAT: >> +{ >>tree ptr = gimple_call_arg (stmt, 0); >>ao_ref_init_from_ptr_and_size (write, ptr, size); >For str[n]cat, I think this is going to result in WRITE not accurately >reflecting what bytes are going to be written -- write.offset would >have >to account for the length of the destination string. > >I don't see a way in the ao_ref structure to indicate that the offset >of >a write is unknown. You can't make offset entirely unknown but you can use, for strcat, offset zero, size and max_size -1. It matters that the access range is correct. This is similar to how we treat array accesses with variable index. > >I'm really hesitant to allow handling of str[n]cat given the inaccuracy > >in how WRITE is initialized. > >To handle str[n]cat I think you have to either somehow indicate the >offse
Re: [PATCH] C++: fix-it hints suggesting accessors for private fields
On Thu, Apr 27, 2017 at 7:23 AM, Nathan Sidwell wrote: > On 04/26/2017 12:34 PM, David Malcolm wrote: > >> Thanks - yes; that gives information on the const vs non-const of the >> "this" parameter, but doesn't say whether the argument was const vs non >> -const. > > >> However, within: >> >> int test_const_ptr (const t1 *ptr) >> { >>return ptr->m_color; >> } >> from which we can see the const-ness of the t1: > > > correct. > >> but the call to lookup_member from within >> finish_class_member_access_expr discards this information, giving just >> "access_path": a BINFO that wraps the RECORD_TYPE for t1 directly. > > > Correct. > > lookup_member just looks for a matching name. the BINFO represents the > class hierarchy - it's not modified depending on the cvquals of where you > came from. > >> A somewhat invasive solution would be for lookup_member to grow an extra: >>tree object >> parameter, and to pass this information down through the access >> -enforcement code, so that locate_field_accessor can look at the const >> -ness of the lookup, and avoid suggesting const methods when the object >> is const. The code would probably need to support the new param being >> NULL_TREE for cases where we're looking up a static member. Or maybe >> an enum of access style for const vs non-const vs static. >> Maybe name the param "access_hint" to signify that it's merely there >> for the purpose of hints for the user, and not to affect the parsing >> itself? > > Hm, that does seem rather unfortunate. >> >> Another solution would be to not bother offering non-const methods as >> accessors. > > > I think that would be very unfortunate. > > How about adding a tsubst_flag value? > > tf_const_obj = 1 << 11, /* For alternative accessor suggestion help. */ > > and pass that in? the tsubst flags have grown in meaning somewhat since > they first appeared -- their name is no longer so appropriate. > > (of course we have the same problem with volatile, but that's probably > overkill for first attempt.) > > Jason, WDYT? I'd suggest handling this diagnostic in finish_class_member_access_expr, rather than try to push down context information into lookup_member. Perhaps by adding another parameter to lookup_member for passing back the inaccessible or ambiguous lookup result? Jason
Re: [PATCH 5/7] [D] libiberty: Fixes for demangling qualified symbol names
On 15 April 2017 at 17:25, Iain Buclaw wrote: > This patch removes `dlang_parse_symbol', and replaces it with > `dlang_parse_qualified' and `dlang_parse_mangle'. All callers have > been updated to reflect as to whether we expect either a `MangleName' > or `QualifiedName' to be the next token we encounter, which matches > many clarifications that have been made in the specification. > > This also removes the special case of matching `V' - or encoded > extern(Pascal) functions - and instead uses a more generic approach in > that if a `CallConvention' symbol was encountered whilst consuming > `QualifiedName', it *must* be followed by another `QualifiedName', > otherwise we've reached the end of the matched rule. It still defeats > the purpose of the aiming to having a context-free grammar, but at > least it's a little less evil than before. > > --- FYI, the following amendments were made to a commented grammar rule --- /* Qualified names are identifiers separated by their encoded length. Nested functions also encode their argument types without specifying what they return. QualifiedName: SymbolName SymbolName QualifiedName SymbolName TypeFunctionNoReturn QualifiedName - SymbolName M TypeFunctionNoReturn QualifiedName + SymbolName M TypeModifiers TypeFunctionNoReturn QualifiedName ^ The start pointer should be at the above location. */ --- Along with relevant code adjustment in the patch. I also discovered that this also fixed a another problematic symbol, so added that to the testsuite also. Regards Iain --- libiberty/ChangeLog: 2017-05-01 Iain Buclaw * d-demangle.c (dlang_parse_symbol): Remove function. (dlang_parse_qualified): New function. (dlang_parse_mangle): New function. (dlang_type): Update to call dlang_parse_qualified. (dlang_identifier): Update to call either dlang_parse_qualified or dlang_parse_mangle. (dlang_type_modifier_p): Remove function. (dlang_call_convention_p): Don't allow for type modifiers in mangle. (dlang_template_args): Update to call dlang_identifier. (dlang_demangle): Update to call dlang_parse_mangle. * testsuite/d-demangle-expected: Add tests. diff --git a/libiberty/d-demangle.c b/libiberty/d-demangle.c index b30641f0a6..8b0f628016 100644 --- a/libiberty/d-demangle.c +++ b/libiberty/d-demangle.c @@ -186,7 +186,10 @@ static const char *dlang_type (string *, const char *); static const char *dlang_value (string *, const char *, const char *, char); -static const char *dlang_parse_symbol (string *, const char *, +static const char *dlang_parse_qualified (string *, const char *, + enum dlang_symbol_kinds); + +static const char *dlang_parse_mangle (string *, const char *, enum dlang_symbol_kinds); static const char *dlang_parse_tuple (string *, const char *); @@ -561,7 +564,7 @@ dlang_type (string *decl, const char *mangled) case 'E': /* enum T */ case 'T': /* typedef T */ mangled++; - return dlang_parse_symbol (decl, mangled, dlang_type_name); + return dlang_parse_qualified (decl, mangled, dlang_type_name); case 'D': /* delegate T */ { string mods; @@ -743,12 +746,10 @@ dlang_identifier (string *decl, const char *mangled, /* Check whether template parameter is a function with a valid return type or an untyped identifier. */ if (ISDIGIT (*mangled)) - mangled = dlang_parse_symbol (decl, mangled, dlang_template_ident); + mangled = dlang_parse_qualified (decl, mangled, + dlang_template_ident); else if (strncmp (mangled, "_D", 2) == 0) - { - mangled += 2; - mangled = dlang_parse_symbol (decl, mangled, dlang_function); - } + mangled = dlang_parse_mangle (decl, mangled, dlang_function); /* Check for name length mismatch. */ if (mangled && (mangled - pend) == psize) @@ -1298,49 +1299,11 @@ dlang_value (string *decl, const char *mangled, const char *name, char type) return mangled; } -/* Extract the type modifiers from MANGLED and return the string - length that it consumes in MANGLED on success or 0 on failure. */ -static int -dlang_type_modifier_p (const char *mangled) -{ - int i; - - switch (*mangled) -{ -case 'x': case 'y': - return 1; - -case 'O': - mangled++; - i = dlang_type_modifier_p (mangled); - return i + 1; - -case 'N': - mangled++; - if (*mangled == 'g') - { - mangled++; - i = dlang_type_modifier_p (mangled); - return i + 2; - } -} - - return 0; -} - /* Extract the function calling convention from MANGLED and return 1 on success or 0 on failure. */ static int dlang_call_convention_p (const char *mangled) { - /* Prefix for functions needing 'this' */ - if (*mangled == 'M') -{ - mangled++; - /* Also skip over any type modifiers. */ - mangled += dlang_type_modifier_p (mangled); -} -
[committed] fix ILP32 testsuite failures for after PR 80503
I committed r247444 as an obvious fix for the ILP32 failures in gcc.dg/tree-ssa/builtin-sprintf-warn-18.c. The committed change is below for reference. Martin diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index c3c717d..a3153c1 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -1390,6 +1390,7 @@ format_integer (const directive &dir, tree arg) res.range.max = tree_digits (arg, base, dir.prec[1], maybesign, maybebase); res.range.likely = res.range.min; + res.knownrange = true; } res.range.unlikely = res.range.max; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c index 55c3974..961fa48 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c @@ -114,18 +114,15 @@ void test_characters () void test_width_and_precision_out_of_range (char *d) { -#if __LONG_MAX__ == 2147483647 -# define MAX_P1_STR "2147483648" -#elif __LONG_MAX__ == 9223372036854775807 -# define MAX_P1_STR "9223372036854775808" -#endif - - T ("%" MAX_P1_STR "i", 0);/* { dg-warning "width out of range" } */ - /* { dg-warning "result to exceed .INT_MAX. " "" { target *-*-* } .-1 } */ - T ("%." MAX_P1_STR "i", 0); /* { dg-warning "precision out of range" } */ + /* The range here happens to be a property of the compiler, not + one of the target. */ + T ("%9223372036854775808i", 0);/* { dg-warning "width out of range" } */ + /* { dg-warning "result to exceed .INT_MAX." "" { target *-*-* } .-1 } */ + T ("%.9223372036854775808i", 0); /* { dg-warning "precision out of range" } */ + /* { dg-warning "causes result to exceed .INT_MAX." "" { target *-*-* } .-1 } */ /* The following is diagnosed by -Wformat (disabled here). */ - /* T ("%" MAX_P1_STR "$i", 0); */ + /* T ("%9223372036854775808$i", 0); */ } /* Verify that an excessively long directive is truncated and the truncation
Re: [committed] fix ILP32 testsuite failures for after PR 80503
On Mai 01 2017, Martin Sebor wrote: > + /* The range here happens to be a property of the compiler, not > + one of the target. */ > + T ("%9223372036854775808i", 0);/* { dg-warning "width out of range" > } */ > + /* { dg-warning "result to exceed .INT_MAX." "" { target *-*-* } .-1 } */ > + T ("%.9223372036854775808i", 0); /* { dg-warning "precision out of > range" } */ > + /* { dg-warning "causes result to exceed .INT_MAX." "" { target *-*-* > } .-1 } */ Please also make the test names unique. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška wrote: > On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>> Hello. >>> >>> This is patch that was originally installed by Jason and later reverted due >>> to PR70422. >>> In the later PR Richi suggested a fix for that and Segher verified that it >>> helped him >>> to survive regression tests. That's reason why I'm resending that. >>> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>> Ready to be installed? >>> Martin >> >>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>> From: marxin >>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>> symbol entry. >>> >>> gcc/cp/ChangeLog: >>> >>> 2017-04-20 Jason Merrill >>> Martin Liska >>> Segher Boessenkool >>> >>> PR c++/64266 >>> PR c++/70353 >>> PR bootstrap/70422 >>> Core issue 1962 >>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>> DECL_IGNORED_P. Don't call cp_finish_decl. >> >> If we don't emit those into the debug info, will the debugger be >> able to handle __FUNCTION__ etc. properly? > > No, debugger with the patch can't handled these. Similar to how clang > behaves currently. Maybe it can be conditionally enabled with -g3, or -g? > >> Admittedly, right now we emit it into debug info only if those decls >> are actually used, say on: >> const char * foo () { return __FUNCTION__; } >> const char * bar () { return ""; } >> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >> has to have some handling of it anyway. But while in functions >> that don't refer to __FUNCTION__ it is always the debugger that needs >> to synthetize those and thus they will be always pointer-equal, >> if there are some uses of it and for other uses the debugger would >> synthetize it, there is the possibility that the debugger synthetized >> string will not be the same object as actually used in the function. > > You're right, currently one has to use a special function to be able to > print it in debugger. I believe we've already discussed that, according > to spec, the strings don't have to point to a same string. > > Suggestions what we should do with the patch? We need to emit debug information for these variables. From Jim's description in 70422 it seems that the problem is that the reference to the string from the debug information is breaking function_mergeable_rodata_prefix, which relies on current_function_decl. It seems to me that its callers should pass along their decl parameter so that f_m_r_p can use the decl's DECL_CONTEXT rather than rely on current_function_decl being set properly. Jason
[committed] Eliminate fixit_hint class hierarchy
The original implementation of fix-it hints (r230674) had an abstract base class "fixit_hint" and three subclasses, representing each of insertions, replacements, and deletions. Having multiple classes for fix-it hints was a nuisance, as it required per-class logic everywhere that the hints were handled. In r239632 I eliminated the deletion subclass in favor of replacement with the empty string (two subclasses are easier than three). This patch eliminates the class hierarchy altogether by implementing insertion in terms of replacement, by representing replacements via a half-open interval (so that for an insertion, start == next location, and we're effectively replacing an empty range at the insertion point with the new string). This greatly simplifies the code for handling fix-it hints; for example it allows removal of a parallel class hierarchy of line_event within edit-context.c. It also improves consolidation of hints: we can now consolidate insertions at the same location, affecting a couple of tests (selftest::test_one_liner_many_fixits and gcc.dg/Wmissing-braces-fixits.c). Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. Committed to trunk as r247445. gcc/ChangeLog: * diagnostic-show-locus.c (layout::get_expanded_location): Rewrite to use new fixit_hint representation, using the "replace" logic. (get_line_span_for_fixit_hint): Likewise. (layout::print_any_fixits): Likewise. (selftest::test_one_liner_many_fixits): Rename to... (selftest::test_one_liner_many_fixits_1): ...this, and update comment and expected output to reflect that the multiple fix-it hints are now consolidated into one insertion. (selftest::test_one_liner_many_fixits_2): New test. (selftest::test_diagnostic_show_locus_one_liner): Update for above. (selftest::test_fixit_consolidation): Update for fix-it API change. * diagnostic.c (print_parseable_fixits): Likewise. * edit-context.c (edited_line::m_line_events): Convert from auto_vec to auto_vec . (class line_event): Convert from abstract base class to a concrete class, taking over the role of replace_event. (class insert_event): Delete. (class replace_event): Rename to class line_event. Convert to half-open range. (edit_context::add_fixits): Reimplement. (edit_context::apply_insert): Delete. (edit_context::apply_replace): Rename to... (edit_context::apply_fixit): ...this. Convert to half-open range. (edited_file::apply_insert): Delete. (edited_file::apply_replace): Rename to... (edited_file::apply_fixit): ...this. (edited_line::~edited_line): Drop deletion of events. (edited_line::apply_insert): Delete. (edited_line::apply_replace): Rename to... (edited_line::apply_fixit): ...this. Convert to half-open range. Update for change to type of m_line_events. * edit-context.h (edit_context::apply_insert): Delete. (edit_context::apply_replace): Rename to... (edit_context::apply_fixit): ...this. gcc/testsuite/ChangeLog: * gcc.dg/Wmissing-braces-fixits.c: Update expected output to reflect insertion fix-it hints at the same location now being consolidated. libcpp/ChangeLog: * include/line-map.h (source_range::intersects_line_p): Delete. (rich_location::add_fixit): Delete. (rich_location::maybe_add_fixit): New method. (class fixit_hint): Reimplement in terms of... (class fixit_replace): ...this. (class fixit_insert): Delete. * line-map.c (linemap_position_for_loc_and_offset): Drop overzealous linemap_assert_fails. (source_range::intersects_line_p): Rename to... (fixit_hint::affects_line_p): New function. (rich_location::add_fixit_insert_before): Reimplement in terms of maybe_add_fixit, moving validation there. (rich_location::add_fixit_insert_after): Likewise. (column_before_p): Delete. (rich_location::add_fixit_replace): Reimplement in terms of maybe_add_fixit, moving validation there. Convert closed input range to half-open range. (rich_location::add_fixit): Delete. (rich_location::maybe_add_fixit): New function. (fixit_insert::fixit_insert): Delete. (fixit_insert::~fixit_insert): Delete. (fixit_insert::affects_line_p): Delete. (fixit_insert::maybe_append_replace): Delete. (fixit_replace::fixit_replace): Rename to... (fixit_hint::fixit_hint): ...this, rewriting as necessary. (fixit_replace::~fixit_replace): Delete. (fixit_replace::affects_line_p): Delete. (fixit_replace::maybe_append_replace): Rename to... (fixit_hint::maybe_append): ...this, rewriting as necessary. --- gcc/diagnostic-show-locus.c | 227 +--
Re: [PATCH] RISC-V: Don't built 64-bit multilibs on 32-bit targets
On Mon, 01 May 2017 03:54:49 PDT (-0700), jos...@codesourcery.com wrote: > On Mon, 1 May 2017, Palmer Dabbelt wrote: > >> > Specifically, in such cases the GCC configure option is >> > --enable-target=all to enable 64-bit multilibs for a default-32-bit >> > target, and the binutils/GDB configure option is --enable-64-bit-bfd. But >> > you can also make a binutils/GDB target include 64-bit support >> > unconditionally without requiring --enable-64-bit-bfd. >> >> Do you happen to know how to do that (enable 64-bit BFDs in a 32-bit binutils >> build)? I'd prefer to have the riscv32 and riscv64 ports the same if >> possible, >> but poked around a bit and couldn't find a way to do it. > > Well, certainly you should add riscv_elf64_vec to targ_selvecs in > config.bfd for the riscv32-*-* case. Generally, check all the configure > fragments used for riscv* to make sure they include both 32-bit and 64-bit > support. Thanks, I think that was all I was missing. I've submitted a binutils patch, so let's consider this matter closed in GCC land.
Re: PR79715: Special case strcpy/strncpy for dse
On 05/01/2017 12:17 PM, Richard Biener wrote: On April 28, 2017 8:14:56 PM GMT+02:00, Jeff Law wrote: On 04/24/2017 03:05 AM, Prathamesh Kulkarni wrote: On 1 March 2017 at 13:24, Richard Biener wrote: On Tue, 28 Feb 2017, Jeff Law wrote: On 02/28/2017 05:59 AM, Prathamesh Kulkarni wrote: On 28 February 2017 at 15:40, Jakub Jelinek wrote: On Tue, Feb 28, 2017 at 03:33:11PM +0530, Prathamesh Kulkarni wrote: Hi, The attached patch adds special-casing for strcpy/strncpy to dse pass. Bootstrapped+tested on x86_64-unknown-linux-gnu. OK for GCC 8 ? What is special on strcpy/strncpy? Unlike memcpy/memmove/memset, you don't know the length they store (at least not in general), you don't know the value, all you know is that they are for the first argument plain store without remembering the pointer or anything based on it anywhere except in the return value. I believe stpcpy, stpncpy, strcat, strncat at least have the same behavior. On the other side, without knowing the length of the store, you can't treat it as killing something (ok, some calls like strcpy or stpcpy store at least the first byte). Well, I assumed a store to dest by strcpy (and friends), which gets immediately freed would count as a dead store since free would kill the whole memory block pointed to by dest ? Yes. But does it happen often in practice? I wouldn't mind exploring this for gcc-8, but I'd like to see real-world code where this happens. Actually I don't mind for "real-world code" - the important part is that I believe it's reasonable to assume it can happen from some C++ abstraction and optimization. Hi, I have updated the patch to include stp[n]cpy and str[n]cat. In initialize_ao_ref_for_dse for strncat, I suppose for strncat we need to treat size as NULL_TREE instead of setting it 2nd arg since we cannot know size (in general) after strncat ? The problem isn't the size, strncat will write the appropriate number of characters, just like strncpy, stpncpy. The problem is that we don't know where the write will start. I'll touch further on this. Patch passes bootstrap+test on x86_64-unknown-linux-gnu. Cross-tested on arm*-*-*, aarch64*-*-*. Thanks, Prathamesh Richard. pr79715-2.txt 2017-04-24 Prathamesh Kulkarni * tree-ssa-dse.c (initialize_ao_ref_for_dse): Add cases for BUILT_IN_STRNCPY, BUILT_IN_STRCPY, BUILT_IN_STPNCPY, BUILT_IN_STPCPY, BUILT_IN_STRNCAT, BUILT_IN_STRCAT. (maybe_trim_memstar_call): Likewise. (dse_dom_walker::dse_optimize_stmt): Likewise. testsuite/ * gcc.dg/tree-ssa/pr79715.c: New test. I'd still be surprised if this kind of think happens in the real world, even with layers of abstraction & templates. diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index 90230ab..752b2fa 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -92,15 +92,24 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write) /* It's advantageous to handle certain mem* functions. */ if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) { + tree size = NULL_TREE; switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt))) { case BUILT_IN_MEMCPY: case BUILT_IN_MEMMOVE: case BUILT_IN_MEMSET: + case BUILT_IN_STRNCPY: + case BUILT_IN_STRCPY: + case BUILT_IN_STPNCPY: + case BUILT_IN_STPCPY: { - tree size = NULL_TREE; if (gimple_call_num_args (stmt) == 3) size = gimple_call_arg (stmt, 2)This part seems reasonable. We know the size for strncpy, stpncpy which we extract from argument #3. For strcpy and stpcpy we'd have NULL for the size which is perfect. In all 4 cases the write starts at offset 0 in the destination string. . + } + /* fallthrough. */ + case BUILT_IN_STRCAT: + case BUILT_IN_STRNCAT: + { tree ptr = gimple_call_arg (stmt, 0); ao_ref_init_from_ptr_and_size (write, ptr, size); For str[n]cat, I think this is going to result in WRITE not accurately reflecting what bytes are going to be written -- write.offset would have to account for the length of the destination string. I don't see a way in the ao_ref structure to indicate that the offset of a write is unknown. You can't make offset entirely unknown but you can use, for strcat, offset zero, size and max_size -1. It matters that the access range is correct. This is similar to how we treat array accesses with variable index. I suspect with a size/maxsize of -1 other code in DSE would probably reject the ao_ref. Though that's probably easier to solve in a way that would allow removal of the dead strcat/strncat calls, but not muck up other things. jeff
Basketball Enthusiasts List
Hi, Would you are interested in acquiring an email list of "Basketball Enthusiasts"? from USA. Our Databases:- Students List, Tennis Enthusiasts List, Soccer Enthusiasts List, Softball Enthusiasts List, Hockey Enthusiasts List, Golfers List, Sports Enthusiasts List, Health and Fitness Enthusiasts, Running Enthusiasts List, Boxing Enthusiasts List, Skiers Enthusiasts and Many more.., Each record in the list contains Contact Name (First and Last Name), Mailing Address, List type and Opt-in email address. If you are interested, I would be more happy to reach out with more information Best Regards, Lilly Walker We respect your privacy, if you do not wish to receive any further emails from our end, please reply with a subject “Leave Out”. --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
[PATCH] stack adjustment is unsigned
I happened to notice that stack adjustment code was confusingly testing for an unsigned value being > 0. In this case unadjusted_alignment is of type unsigned HOST_WIDE_INT, so the 'else' portion of the has no effect. This patch simplifies the adjustment to be less confusing (IMHO). booted and tested on x86_64-linux-gnu, ok? -- Nathan Sidwell 2017-05-01 Nathan Sidwell * calls.c (combine_pending_stack_adjustment_and_call): Remove unnecessary unadjusted_alignment check. Index: gcc/calls.c === --- gcc/calls.c (revision 247416) +++ gcc/calls.c (working copy) @@ -2644,13 +2644,8 @@ combine_pending_stack_adjustment_and_cal adjustment = pending_stack_adjust; /* Push enough additional bytes that the stack will be aligned after the arguments are pushed. */ - if (preferred_unit_stack_boundary > 1) -{ - if (unadjusted_alignment > 0) - adjustment -= preferred_unit_stack_boundary - unadjusted_alignment; - else - adjustment += unadjusted_alignment; -} + if (preferred_unit_stack_boundary > 1 && unadjusted_alignment) +adjustment -= preferred_unit_stack_boundary - unadjusted_alignment; /* Now, sets ARGS_SIZE->CONSTANT so that we pop the right number of bytes after the call. The right number is the entire
Re: [PATCH v2] Destroy arguments for _Cilk_spawn calling in the child (PR 80038)
On 04/28/2017 09:20 PM, Xi Ruoyao wrote: On 2017-04-28 08:42 -0600, Jeff Law wrote: On 04/28/2017 08:31 AM, Xi Ruoyao wrote: Should I prepare (re-diff) a patch for current trunk? If you want for the trunk, yes. Rediff for current GCC trunk. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University pr80038-v2.patch From aa16cfe3b8f949abc0bb2da4e09f7f61bd01a05d Mon Sep 17 00:00:00 2001 From: Xi Ruoyao Date: Fri, 24 Mar 2017 04:35:23 +0800 Subject: [PATCH] Destroy temps for _Cilk_spawn calling in the child (PR c++/80038) Revert r227423, and re-fix PR60586 without breaking cilk specs. 2017-03-24 Xi Ruoyao PR c++/80038 * c-family/c-common.h (cilk_gimplify_call_params_in_spawned_fn, cilk_install_body_pedigree_operations): Remove prototypes. * c-family/c-gimplify.c (c_gimplify_expr): Remove the calls to the function cilk_gimplify_call_params_in_spawned_fn. * c-family/cilk.c: (cilk_set_spawn_marker): Mark the function calls which should be detached. (cilk_gimplify_call_params_in_spawned_fn, cilk_install_body_pedigree_operations): Remove function. (gimplify_cilk_spawn): Add EXPR_STMT and CLEANUP_POINT_EXPR unwrapping. * c/c-typeck.c (cilk_install_body_with_frame_cleanup): Don't add pedigree operation and detach call here. * cp/cp-cilkplus.c (cilk_install_body_with_frame_cleanup): Ditto. * cp/cp-gimplify.c (cilk_cp_gimplify_call_params_in_spawned_fn): Remove function. (cp_gimplify_expr): Remove the calls to the function cilk_cp_gimplify_call_params_in_spawned_fn. * cp/semantics.c: Preserve the flag of function calls should be detached. * cilk_common.c (expand_builtin_cilk_detach): Move pedigree operations here. * gimplify.c (gimplify_cilk_detach): New static function. (gimplify_call_expr, gimplify_modify_expr): Call it for the function calls should be detached. * lto/lto-lang.c (lto_init): Set in_lto_p earlier. * tree-core.h: Document new macro EXPR_CILK_SPAWN. * tree.h: Add new macro EXPR_CILK_SPAWN. * testsuite/g++.dg/cilk-plus/CK/pr80038.cc: New test. Thanks. I fixed up the ChangeLog and installed this on the trunk. jeff
Re: [PATCH] stack adjustment is unsigned
On 05/01/2017 04:23 PM, Nathan Sidwell wrote: I happened to notice that stack adjustment code was confusingly testing for an unsigned value being > 0. In this case unadjusted_alignment is of type unsigned HOST_WIDE_INT, so the 'else' portion of the has no effect. This patch simplifies the adjustment to be less confusing (IMHO). booted and tested on x86_64-linux-gnu, ok? OK I guess. It's not obvious what would happen with a wrong direction target, but the one we've got (PA) doesn't use this code because it's an ACCUMULATE_OUTGOING_ARGS target. It appears that the size passed in would always be positive (it's a size after all), but I couldn't guarantee that without a lot more auditing. Jeff
[gomp4] Make OpenACC orphan gang reductions errors
This patch promotes all OpenACC gang reductions on orphan loops as errors. Accord to the spec, orphan loops are those which are not lexically nested inside an OpenACC parallel or kernels regions. I.e., acc loops inside acc routines. At first I thought this could be a warning because the gang reduction finalizer uses an atomic update. However, because there is no synchronization between gangs, there is way to guarantee that reduction will have completed once a single gang entity returns from the acc routine call. I've applied this patch to gomp-4_0-branch. Cesar 2017-05-01 Cesar Philippidis gcc/c/ * c-typeck.c (c_finish_omp_clauses): Emit an error on orphan OpenACC gang reductions. gcc/cp/ * semantics.c (finish_omp_clauses): Emit an error on orphan OpenACC gang reductions. gcc/fortran/ * openmp.c (resolve_oacc_loop_blocks): Emit an error on orphan OpenACC gang reductions. gcc/ * omp-low.c (enum oacc_loop_flags): Add OLF_REDUCTION enum. (lower_oacc_head_mark): Use it to mark OpenACC reductions. (oacc_loop_auto_partitions): Don't assign gang level parallelism to orphan reductions. gcc/testsuite/ * c-c++-common/goacc/orphan-reductions-1.c: New test. * c-c++-common/goacc/orphan-reductions-2.c: New test. * c-c++-common/goacc/routine-4.c: Update test case. * gcc.dg/goacc/loop-processing-1.c: Likewise. * gfortran.dg/goacc/orphan-reductions-1.f90: New test. * gfortran.dg/goacc/orphan-reductions-2.f90: New test. diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 61a95b0..b04db44 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -12602,6 +12602,14 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort) goto check_dup_generic; case OMP_CLAUSE_REDUCTION: + if (ort == C_ORT_ACC && get_oacc_fn_attrib (current_function_decl) + && find_omp_clause (clauses, OMP_CLAUSE_GANG)) + { + error_at (OMP_CLAUSE_LOCATION (c), + "gang reduction on an orphan loop"); + remove = true; + break; + } need_implicitly_determined = true; t = OMP_CLAUSE_DECL (c); if (TREE_CODE (t) == TREE_LIST) diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 9760f07..6e8fb17 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5870,6 +5870,14 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort) field_ok = ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP); goto check_dup_generic; case OMP_CLAUSE_REDUCTION: + if (ort == C_ORT_ACC && get_oacc_fn_attrib (current_function_decl) + && find_omp_clause (clauses, OMP_CLAUSE_GANG)) + { + error_at (OMP_CLAUSE_LOCATION (c), + "gang reduction on an orphan loop"); + remove = true; + break; + } field_ok = ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP); t = OMP_CLAUSE_DECL (c); if (TREE_CODE (t) == TREE_LIST) diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 72c6669..fb51b40 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -6090,6 +6090,18 @@ resolve_oacc_loop_blocks (gfc_code *code) break; } + if (code->op == EXEC_OACC_LOOP + && code->ext.omp_clauses->lists[OMP_LIST_REDUCTION] + && code->ext.omp_clauses->gang) +{ + for (c = omp_current_ctx; c; c = c->previous) + if (!oacc_is_loop (c->code)) + break; + if (c == NULL || !(oacc_is_parallel (c->code) + || oacc_is_kernels (c->code))) + gfc_error ("gang reduction on an orphan loop at %L", &code->loc); +} + if (code->ext.omp_clauses->seq) { if (code->ext.omp_clauses->independent) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index cc209ba..d6c62f9 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -272,9 +272,10 @@ enum oacc_loop_flags { OLF_INDEPENDENT = 1u << 2, /* Iterations are known independent. */ OLF_GANG_STATIC = 1u << 3, /* Gang partitioning is static (has op). */ OLF_TILE = 1u << 4, /* Tiled loop. */ + OLF_REDUCTION = 1u << 5, /* Reduction loop. */ /* Explicitly specified loop axes. */ - OLF_DIM_BASE = 5, + OLF_DIM_BASE = 6, OLF_DIM_GANG = 1u << (OLF_DIM_BASE + GOMP_DIM_GANG), OLF_DIM_WORKER = 1u << (OLF_DIM_BASE + GOMP_DIM_WORKER), OLF_DIM_VECTOR = 1u << (OLF_DIM_BASE + GOMP_DIM_VECTOR), @@ -6616,6 +6617,10 @@ lower_oacc_head_mark (location_t loc, tree ddvar, tree clauses, tag |= OLF_TILE; break; + case OMP_CLAUSE_REDUCTION: + tag |= OLF_REDUCTION; + break; + case OMP_CLAUSE_DEVICE_TYPE: /* TODO: Add device type handling. */ goto done; @@ -20942,7 +20947,14 @@ oacc_loop_auto_partitions (oacc_loop *loop, unsigned outer_mask, /* Allocate outermost and non-innermost loops at the outermost non-innermost available level. */ unsigned this_mask = GOMP_DIM_MASK (GOMP_DIM_GANG); - + + /* Orphan reductions cannot have gang partitioning. */ + if ((loop->flags & OLF_REDUCTION) + && get_oacc_fn_attrib (current_function_decl) + && !lookup_attribute ("omp target entrypoint", +DECL_ATTRIBUTES (current_function
[gomp4] Make front ends emit warnings when OpenACC routines directives lack parallelism
The language regarding OpenACC routines have changed in OpenACC 2.5 such that the end user must explicitly specify one of gang, worker, vector or seq partitioning. I guess some other compiler need those directives to generate specialized versions of those functions accordingly. However, GCC currently falls back to 'seq' partitioning, and that seems to be a reasonable compromise in most cases. This patch teaches the FE's how to emit a warning whenever a routine directive doesn't include a partitioning clause. The rationale for making this a warning vs an error is that it enables us to have some backwards compatibility with OpenACC 2.0a. Unfortunately, the fortran FE doesn't make use of verify_oacc_routine_clauses because it parses/matches routines much earlier than c/c++, so that part of the patch is slightly more involved. I've also had to update a lot of test cases to make them conform to the new routine behavior. This patch has been committed to gomp-4_0-branch. Cesar 2017-05-01 Cesar Philippidis gcc/fortran/ * gfortran.h (enum oacc_function): Add OACC_FUNCTION_AUTO. * openmp.c (gfc_oacc_routine_dims): Return OACC_FUNCTION_AUTO when no parallelism was detected. (gfc_match_oacc_routine): Emit a warning when the user doesn't supply a gang, worker, vector or seq clause to an OpenACC routine construct. gcc/ * omp-low.c (verify_oacc_routine_clauses): Emit a warning when the user doesn't supply a gang, worker, vector or seq clause to an OpenACC routine construct. gcc/testsuite/ * c-c++-common/goacc-gomp/nesting-fail-1.c: Update usage and expected output of OpenACC routines. * c-c++-common/goacc/Wparentheses-1.c: Likewise. * c-c++-common/goacc/nesting-fail-1.c: Likewise. * c-c++-common/goacc/routine-1.c: Likewise. * c-c++-common/goacc/routine-2.c: Likewise. * c-c++-common/goacc/routine-5.c: Likewise. * c-c++-common/goacc/routine-level-of-parallelism-1.c: Likewise. * c-c++-common/goacc/routine-level-of-parallelism-2.c: Likewise. * c-c++-common/goacc/routine-nohost-1.c: Likewise. * c-c++-common/goacc/routine-nohost-2.c: Likewise. * g++.dg/goacc/routine-1.C: Likewise. * g++.dg/goacc/routine-2.C: Likewise. * g++.dg/goacc/template.C: Likewise. * gfortran.dg/goacc/dtype-1.f95: Likewise. * gfortran.dg/goacc/pr71704.f90: Likewise. * gfortran.dg/goacc/pr72741-2.f: Likewise. * gfortran.dg/goacc/routine-6.f90: Likewise. * gfortran.dg/goacc/routine-8.f90: Likewise. * gfortran.dg/goacc/routine-9.f90: Likewise. * gfortran.dg/goacc/routine-level-of-parallelism-1.f90: Likewise. * gfortran.dg/goacc/routine-without-clauses.f90: New test. libgomp/ * testsuite/libgomp.oacc-c++/pr71959-a.C: Adjust test case conform to OpenACC 2.5 routines. * testsuite/libgomp.oacc-c-c++-common/declare-2.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/declare-3.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/declare-4.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/host_data-1.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/loop-default.h: Likewise. * testsuite/libgomp.oacc-c-c++-common/mode-transitions.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/parallel-loop-2.h: Likewise. * testsuite/libgomp.oacc-c-c++-common/routine-1.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/routine-3.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/routine-bind-nohost-1.c: Likewise. * testsuite/libgomp.oacc-fortran/host_data-2.f90: Likewise. * testsuite/libgomp.oacc-fortran/host_data-3.f: Likewise. * testsuite/libgomp.oacc-fortran/host_data-4.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-1.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-2.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-3.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-4.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-5.f90: Likewise. * testsuite/libgomp.oacc-fortran/routine-9.f90: Likewise. diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 7913ac7..3c762a8 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -316,7 +316,8 @@ enum oacc_function OACC_FUNCTION_GANG, OACC_FUNCTION_WORKER, OACC_FUNCTION_VECTOR, - OACC_FUNCTION_SEQ + OACC_FUNCTION_SEQ, + OACC_FUNCTION_AUTO }; /* Strings for all symbol attributes. We use these for dumping the diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 72c6669..88ccff2 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -2379,7 +2379,7 @@ static oacc_function gfc_oacc_routine_dims (gfc_omp_clauses *clauses) { int level = -1; - oacc_function ret = OACC_FUNCTION_SEQ; + oacc_function ret = OACC_FUNCTION_AUTO; if (clauses) { @@ -2401,7 +2401,10 @@ gfc_oacc_routine_dims (gfc_omp_clauses *clauses) ret = OACC_FUNCTION_VECTOR; } if (clauses->seq) - level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level); + { + level = GOMP_DIM_MAX, mask |= GOMP_DIM_MASK (level); + ret = OACC_FUNCTION_SEQ; + } if (mask != (mask & -mask)) ret = OACC_FU
[PATCH] C++ parser: Fix typos in error messages
Hi, the following patch fixes two typos in error messages of the C++ parser (which have gone unnoticed since GCC 3.4.0). Bootstrapped and regtested on x86_64-pc-linux-gnu. OK for trunk? Should this go also to GCC 7.2? Regards, Volker 2017-05-01 Volker Reichelt * parser.c (cp_parser_base_specifier): Fix typos in error messages. Index: gcc/cp/parser.c === --- gcc/cp/parser.c (revision 247445) +++ gcc/cp/parser.c (working copy) @@ -23716,7 +23716,7 @@ if (virtual_p && !duplicate_virtual_error_issued_p) { cp_parser_error (parser, - "% specified more than once in base-specified"); + "% specified more than once in base-specifier"); duplicate_virtual_error_issued_p = true; } @@ -23736,7 +23736,7 @@ && !duplicate_access_error_issued_p) { cp_parser_error (parser, - "more than one access specifier in base-specified"); + "more than one access specifier in base-specifier"); duplicate_access_error_issued_p = true; } ===