Re: MIPS GCC test failure: gcc.dg/tree-ssa/ssa-dom-thread-4.c
"Steve Ellcey " writes:
> I was looking at the test failure of gcc.dg/tree-ssa/ssa-dom-thread-4.c
> on MIPS. The failure is in the scan of how many jumps are threaded
> which has changed from 6 to 4 on MIPS.
Yeah, I'd been leaving this one until I could devote a bit of time to it.
> I tracked down the change to this patch:
>
> 2013-11-19 Jeff Law
>
> * tree-ssa-threadedge.c (thread_across_edge): After threading
> through a joiner, allow threading a normal block requiring
> duplication.
>
> * tree-ssa-threadupdate.c (thread_block_1): Improve code to detect
> jump threading requests that would muck up the loop structures.
>
> * tree-ssa-threadupdate.c: Fix trailing whitespace.
> * tree-ssa-threadupdate.h: Likewise.
Thanks for tracking down the patch that changed it.
> Now my initial thought is to just change the mips scan to look for
> 4 'Threaded' lines instead of 6 and be done with it, but the test has
> a long explanation of why 6 is the right answer on MIPS and I don't know
> what to do with this. Even after looking at the new and old dom1 dump
> files I can't really explain why 4 is the right number now instead of 6.
It's because the earlier vrp1 pass is now doing the !kill_elt ->
continuation point threading (the third item from the list).
vrp1 runs before the header of the inner while loop is duplicated,
so here the transformation only happens once rather than twice.
vrp1 is now also duplicating the b_elt-testing block in:
if (b_elt && kill_elt && kill_elt->indx == b_elt->indx
...
for the "kill_elt->indx >= b_elt->indx" branch of the while loop
so that it can thread out the known-true kill_elt test. I.e.,
if the while loop stops on "kill_elt->indx >= b_elt->indx",
the if statement will test:
if (b_elt && kill_elt->indx == b_elt->indx
...
But it's still down to dom1 to recognise that the b_elt test itself is
also redundant in this case, so the fourth item on the list still holds.
I applied this patch to update the MIPS results after checking
that it also works for arc-elf and avr-elf.
Thanks,
Richard
gcc/testsuite/
* gcc.dg/tree-ssa/ssa-dom-thread-4.c: Adjust expected MIPS output.
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c2014-02-01
11:37:12.793270725 +
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-4.c2014-02-01
11:37:13.021272556 +
@@ -66,7 +66,7 @@ bitmap_ior_and_compl (bitmap dst, const_
"a_elt || b_elt" and "b_elt && kill_elt" into two conditions each,
rather than using "(var1 != 0) op (var2 != 0)". Also, as on other targets,
we duplicate the header of the inner "while" loop. There are then
- 6 threading opportunities:
+ 4 threading opportunities:
1x "!a_elt && b_elt" in the outer "while" loop
-> the start of the inner "while" loop,
@@ -74,16 +74,13 @@ bitmap_ior_and_compl (bitmap dst, const_
1x "!b_elt" in the first condition
-> the outer "while" loop's continuation point,
skipping the known-false "b_elt" in the second condition.
- 2x "!kill_elt" in the inner "while" loop
- -> the outer "while" loop's continuation point,
-skipping the known-false "b_elt && kill_elt" in the second condition
- 2x "kill_elt->indx < b_elt->indx" in the first "while" loop
+ 2x "kill_elt->indx >= b_elt->indx" in the first "while" loop
-> "kill_elt->indx == b_elt->indx" in the second condition,
skipping the known-true "b_elt && kill_elt" in the second
condition. */
/* Likewise for arc. */
/* For avr, BRANCH_COST is by default 0, so the default
LOGICAL_OP_NON_SHORT_CIRCUIT definition also computes as 0. */
-/* { dg-final { scan-tree-dump-times "Threaded" 6 "dom1" { target mips*-*-*
avr-*-* arc*-*-* } } } */
+/* { dg-final { scan-tree-dump-times "Threaded" 4 "dom1" { target mips*-*-*
avr-*-* arc*-*-* } } } */
/* { dg-final { cleanup-tree-dump "dom1" } } */
Re: -Wformat-security warnings generated in gcc build
On Sun, Jan 26, 2014 at 3:56 PM, Prathamesh Kulkarni wrote: > On Fri, Jan 24, 2014 at 9:19 PM, Prathamesh Kulkarni > wrote: >> On Thu, Jan 23, 2014 at 9:09 PM, Prathamesh Kulkarni >> wrote: >>> On Thu, Jan 23, 2014 at 8:24 PM, Dodji Seketeli wrote: Prathamesh Kulkarni writes: > > Shall it be correct then to replace calls to error() and friends, > taking only format string with no-argument specifiers > to error_at_no_args() ? (similarly we shall need warning_at_no_args, > pedwarn_no_args, etc.) I would guess so, yes. >> >> Also, you'd need to modify cp/error.c:cp_printer in a similar way, to >> issue an internal_error each time we try to access a null test->args_ptr. > > Shall check for text->args_ptr be required in each case label of > argument specifier in pp_format() > and client-specific functions like cp_printer() ? Yes, I think so. Maybe you can make that a bit more maintainable by creating a macro like those used to access text->args_ptr in cp_printer, e.g: #define next_int va_arg (*text->args_ptr, int) In that macro, make the check for text->args_ptr before accessing it, and then use that macro to access text->args_ptr through the function. >>> >>> I was going through diagnostic.c, it appears that many functions in >>> (error(), error_at(), warning(), etc.) share common code (mostly >>> contains call to diagnostic_set_info() followed by call to >>> report_diagnostic()), which I guess can be re-written in terms of >>> emit_diagnostic(), however since it's variadic that's not possible. I >>> wrote a v_emit_diagnostic() function, that takes same arguments as >>> emit_diagnostic(), with additional va_list * at end (va_list * instead >>> of va_list, so I could pass NULL for error_no_args() and friends). Is >>> it correct to write these other functions (emit_diagnostic(), error(), >>> inform(), etc.) in terms of v_emit_diagnostic() ? >> >> silly mistake in warning_at(), it should be: >> ret = v_emit_diagnostic (DK_WARNING, location, opt, gmsgid, &ap); > The attached patch removes all -Wformat-security warnings > I put assertions for tree->args_ptr in pretty-print.c:pp_format(), > cp/error.c:cp_printer() and > c/c-objc-common.c:c_tree_printer(). > > However, adding *_no_args() functions in include/cpplib.h (eg: > cpp_error_no_args(), etc.), generates a new set of warnings > of type -Wsuggest-attribute=format gnu_printf for these functions. > > I removed them by adding the following in include/ansidecl.h > +#ifndef ATTRIBUTE_GNU_PRINTF > +#define ATTRIBUTE_GNU_PRINTF(m, n) __attribute__ ((__format__ > (__gnu_printf__, m, n))) ATTRIBUTE_NONNULL(m) > +#define ATTRIBUTE_GNU_PRINTF_3_0 ATTRIBUTE_GNU_PRINTF(3, 0) > +#define ATTRIBUTE_GNU_PRINTF_5_0 ATTRIBUTE_GNU_PRINTF(5, 0) > +#endif /* ATTRIBUTE_GNU_PRINTF */ > + > > and declarations changed to: > +extern bool cpp_error_no_args (cpp_reader *, int, const char *) > + ATTRIBUTE_GNU_PRINTF_3_0; > + > > Is this correct way to fix it ? > Thanks! Ping ? > >> >>> >>> >>> >>> >>> -- Dodji
gcc-4.7-20140201 is now available
Snapshot gcc-4.7-20140201 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.7-20140201/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.7 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_7-branch revision 207390 You'll find: gcc-4.7-20140201.tar.bz2 Complete GCC MD5=e13db7cbf8b43ac890459e5bdb599087 SHA1=c2330063cd6b8edca70549b171ba8eeb4bfc1d48 Diffs from 4.7-20140125 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.7 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
