Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)
+(for cmp (gt ge lt le)
+ outp (convert convert negate negate)
+ outn (negate negate convert convert)
+ /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ (simplify
+ (cond (cmp @0 real_zerop) real_onep real_minus_onep)
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
+ && types_match (type, TREE_TYPE (@0)))
+ (switch
+(if (types_match (type, float_type_node))
+ (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0)))
+(if (types_match (type, double_type_node))
+ (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0)))
+(if (types_match (type, long_double_type_node))
+ (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0))
There is already a 1.0 of the right type in the input, it would be easier
to reuse it in the output than build a new one.
Non-generic builtins like copysign are such a pain... We also end up
missing the 128-bit case that way (pre-existing problem, not your patch).
We seem to have a corresponding internal function, but apparently it is
not used until expansion (well, maybe during vectorization).
+ /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
+ /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
+ /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */
+ /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */
+ (simplify
+ (cond (cmp @0 real_zerop) real_minus_onep real_onep)
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
+ && types_match (type, TREE_TYPE (@0)))
+ (switch
+(if (types_match (type, float_type_node))
+ (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0)))
+(if (types_match (type, double_type_node))
+ (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0)))
+(if (types_match (type, long_double_type_node))
+ (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0)))
+
+/* Transform X * copysign (1.0, X) into abs(X). */
+(simplify
+ (mult:c @0 (COPYSIGN real_onep @0))
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
+ (abs @0)))
I would have expected it do to the right thing for signed zero and qNaN.
Can you describe a case where it would give the wrong result, or are the
conditions just conservative?
+/* Transform X * copysign (1.0, -X) into -abs(X). */
+(simplify
+ (mult:c @0 (COPYSIGN real_onep (negate @0)))
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
+ (negate (abs @0
+
+/* Transform copysign (-1.0, X) into copysign (1.0, X). */
+(simplify
+ (COPYSIGN real_minus_onep @0)
+ (COPYSIGN { build_one_cst (type); } @0))
(simplify
(COPYSIGN REAL_CST@0 @1)
(if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
(COPYSIGN (negate @0) @1)))
? Or does that create trouble with sNaN and only the 1.0 case is worth
the trouble?
--
Marc Glisse
Re: [BUILDROBOT] error: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long int’ (was: [PATCH] [ARC] Recognise add_n and sub_n in combine again)
Hi Graham,
On Mon, 2017-06-12 11:40:39 +0200, Jan-Benedict Glaw wrote:
> On Fri, 2017-05-12 20:14:23 +0100, Graham Markall
> wrote:
> > Since the combine pass canonicalises shift-add insns using plus and
> > ashift (as opposed to plus and mult which it previously used to do), it
> > no longer creates *add_n or *sub_n insns, as the patterns match plus and
> > mult only. The outcome of this is that some opportunities to generate
> > add{1,2,3} and sub{1,2,3} instructions are missed.
>
> [...]
> > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> > index 91c28e7..42730d5 100644
> > --- a/gcc/config/arc/arc.c
> > +++ b/gcc/config/arc/arc.c
> > @@ -3483,6 +3483,14 @@ arc_print_operand (FILE *file, rtx x, int code)
> >
> >return;
> >
> > +case 'c':
> > + if (GET_CODE (x) == CONST_INT)
> > +fprintf (file, "%d", INTVAL (x) );
> > + else
> > +output_operand_lossage ("invalid operands to %%c code");
> > +
> > + return;
> > +
>
>
> Build robot found something (see log at
> http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=704773),
> seems to be introduced with the above patch fragment:
>
> g++ -fno-PIE -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE
> -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall
> -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute
> -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros
> -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -I.
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc/.
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc/../include
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc/../libcpp/include
> -I/opt/cfarm/mpc/include
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc/../libdecnumber
> -I/home/jbglaw/repos-configlist_mk/gcc/gcc/../libdecnumber/dpd
> -I../libdecnumber -I/home/jbglaw/repos-configlist_mk/gcc/gcc/../libbacktrace
> -o arc.o -MT arc.o -MMD -MP -MF ./.deps/arc.TPo
> /home/jbglaw/repos-configlist_mk/gcc/gcc/config/arc/arc.c
> /home/jbglaw/repos-configlist_mk/gcc/gcc/config/arc/arc.c: In function ‘void
> arc_print_operand(FILE*, rtx, int)’:
> /home/jbglaw/repos-configlist_mk/gcc/gcc/config/arc/arc.c:3503:41: error:
> format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘long
> int’ [-Werror=format=]
> fprintf (file, "%d", INTVAL (x) );
> ^
> cc1plus: all warnings being treated as errors
This still doesn't build, see the two most current builds at
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=705416
(arc-elf32, --with-cpu=arc600) and
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=705415
(arceb-linux-uclibc, --with-cpu=arc700).
MfG, JBG
--
Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481
Signature of: The course of history shows that as a government grows, liberty
the second : decreases." (Thomas Jefferson)
signature.asc
Description: Digital signature
Re: [Patch, fortran] PR34640 - ICE when assigning item of a derived-component to a pointer
Hi Paul, I want to sound out if this is acceptable as the way to fix these problems before going to the trouble of doing the final clean up; especially of trans.c (gfc_build_array_ref) and trans-array.c(build_array_ref). The method you use looks OK to me, and the time till completion of the Great Array Descrptor Reform (TM) tends to become longer, not shorter. So, OK to proceed from my side. And thank you very much for taking on this thorny (and, for gfortran, very fundamental problem). Regards Thomas
Re: [Patch, fortran] PR34640 - ICE when assigning item of a derived-component to a pointer
Dear All, Dominique pointed out that the changes to libgfortran.h were missing from the patch. This came about because I wrongly named kernels-alias-4.f95 in the diff so it was missing too. Please find attached the complete patch. Thomas, thanks for the early feedback. Paul On 24 June 2017 at 11:48, Paul Richard Thomas wrote: > Dear All, > > Please find attached a draft patch for the above PR, together with PRs > 40737, 55763, 57019 and 57116. These PRs constitute problems > associated with the last F95 feature that gfortran does not completely > implement. > > I want to sound out if this is acceptable as the way to fix these > problems before going to the trouble of doing the final clean up; > especially of trans.c (gfc_build_array_ref) and > trans-array.c(build_array_ref). > > The problem concerns pointers to derived type array components. eg: > pointer_array(:) => derived_array (:)%component > > At present gfortran uses a rather crude fix, where a 'span' variable > with value of sizeof(element of derived_array) is used for pointer > arithmetic to access elements of the array; > &pointer_array(i) = &derived_array(1)%component + span*(i-1) > > The difficulty of using a variable 'span' is that it is not passed to > procedures and it is not available to array pointer components. This > patch fixes this by the introduction of a span field in the array > descriptor. Note that this is only used for intrinsic type, pointer > arrays in this version of the patch. A considerable simplification > would arise from using the span field in class arrays too. This might > well be one result of the clean up mentioned above. > > Tobias Burnus and I have been putting off fixing these PRs for a long > time because of the pending array descriptor reform. However, work on > fortran-dev has once again stopped and neither I nor, I think, anybody > else has the time to restart this work anytime soon. > > pointer[1,2].f90 in the libgomp testsuite fail if this modification to > array referencing is exposed to them. For the time being, > trans-array.c(is_pointer_array) has: > + if (flag_openmp) > + return false; > to switch off the modification. I will come back to this during the > clean up, with the hope of putting it right. > > Bootstraps and regtests on FC23/x86_64 - OK to proceed to completion > and submission? > > Paul > > > 2017-06-24 Paul Thomas > > PR fortran/34640 > PR fortran/40737 > PR fortran/55763 > PR fortran/57019 > PR fortran/57116 > > * trans-array.c: Add SPAN_FIELD and update indices for > subsequent fields. > (gfc_conv_descriptor_span, gfc_conv_descriptor_span_get, > gfc_conv_descriptor_span_set, is_pointer_array, > get_array_span): New functions. > (gfc_conv_scalarized_array_ref): If the expression is a subref > array, make sure that info->descriptor is a descriptor type. > Otherwise, if info->descriptor is a pointer array, set 'decl' > and fix it if it is a component reference. > (gfc_conv_array_ref): Similarly set 'decl'. > (gfc_array_allocate): Set the span field if this is a pointer > array. > (gfc_conv_expr_descriptor): Set the span field for pointer > assignments. > * trans-array.h: Prototypes for gfc_conv_descriptor_span_get > and gfc_conv_descriptor_span_set added. > * trans.c (gfc_build_array_ref): GFC_DECL_SUBREF_ARRAY_P change > to GFC_DECL_PTR_ARRAY_P and defreference if a PARM_DECL. > trans-decl.c (gfc_get_symbol_decl): If a non-class pointer > array, mark the declaration as a GFC_DECL_PTR_ARRAY_P. Remove > the setting of GFC_DECL_SPAN. > (gfc_trans_deferred_vars): Set the span field to zero in the > originating scope. > * trans-expr.c (gfc_trans_pointer_assignment): Remove code for > setting of GFC_DECL_SPAN. Set the 'span' field for non-class > pointers to class function results. Likewise for rank remap. > * trans.h : Rename DECL_LANG_FLAG_6, GFC_DECL_SUBREF_ARRAY_P, > as GFC_DECL_PTR_ARRAY_P. > * trans-intrinsic.c (conv_expr_ref_to_caf_ref): Pick up the > 'token' offset from the field decl in the descriptor. > (conv_isocbinding_subroutine): Set the 'span' field. > * trans-io.c (gfc_trans_transfer): Always scalarize pointer > array io. > * trans-stmt.c (trans_associate_var): Set the 'span' field. > * trans-types.c (gfc_get_array_descriptor_base): Add the 'span' > field to the array descriptor. > (gfc_get_derived_type): Pointer array components are marked as > GFC_DECL_PTR_ARRAY_P. > (gfc_get_array_descr_info): Jump one more in the DECL_CHAIN to > access the offset field. > > > 2017-06-24 Paul Thomas > > PR fortran/34640 > * gfortran.dg/assumed_type_2.f90: Adjust some of the tree dump > checks. > * gfortran.dg/no_arg_check_2.f90: Likewise. > * gfortran.dg/pointer_array_1.f90: New test. > * gfortran.dg/pointer_array_2.f90: New test. > * gfortran.dg/pointer_array_component_1.f90
Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)
On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse wrote:
> +(for cmp (gt ge lt le)
> + outp (convert convert negate negate)
> + outn (negate negate convert convert)
> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
> + (simplify
> + (cond (cmp @0 real_zerop) real_onep real_minus_onep)
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
> + && types_match (type, TREE_TYPE (@0)))
> + (switch
> +(if (types_match (type, float_type_node))
> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0)))
> +(if (types_match (type, double_type_node))
> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0)))
> +(if (types_match (type, long_double_type_node))
> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0))
>
> There is already a 1.0 of the right type in the input, it would be easier to
> reuse it in the output than build a new one.
Right. Fixed.
>
> Non-generic builtins like copysign are such a pain... We also end up missing
> the 128-bit case that way (pre-existing problem, not your patch). We seem to
> have a corresponding internal function, but apparently it is not used until
> expansion (well, maybe during vectorization).
Yes I noticed that while working on a different patch related to
copysign; The generic version of a*copysign(1.0, b) [see the other
thread where the ARM folks started a patch for it; yes it was by pure
accident that I was working on this and really did not notice that
thread until yesterday].
I was looking into a nice way of creating copysign without having to
do the switch but I could not find one. In the end I copied was done
already in a different location in match.pd; this is also the reason
why I had the build_one_cst there.
>
> + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
> + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
> + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */
> + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */
> + (simplify
> + (cond (cmp @0 real_zerop) real_minus_onep real_onep)
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
> + && types_match (type, TREE_TYPE (@0)))
> + (switch
> +(if (types_match (type, float_type_node))
> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0)))
> +(if (types_match (type, double_type_node))
> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0)))
> +(if (types_match (type, long_double_type_node))
> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0)))
> +
> +/* Transform X * copysign (1.0, X) into abs(X). */
> +(simplify
> + (mult:c @0 (COPYSIGN real_onep @0))
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
> + (abs @0)))
>
> I would have expected it do to the right thing for signed zero and qNaN. Can
> you describe a case where it would give the wrong result, or are the
> conditions just conservative?
I was just being conservative; maybe too conservative but I was a bit
worried I could get it incorrect.
>
> +/* Transform X * copysign (1.0, -X) into -abs(X). */
> +(simplify
> + (mult:c @0 (COPYSIGN real_onep (negate @0)))
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
> + (negate (abs @0
> +
> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */
> +(simplify
> + (COPYSIGN real_minus_onep @0)
> + (COPYSIGN { build_one_cst (type); } @0))
>
> (simplify
> (COPYSIGN REAL_CST@0 @1)
> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
> (COPYSIGN (negate @0) @1)))
> ? Or does that create trouble with sNaN and only the 1.0 case is worth
> the trouble?
No that is the correct way; I Noticed the other thread about copysign
had something similar as what should be done too.
I will send out a new patch after testing soon.
Thanks,
Andrew
>
> --
> Marc Glisse
[PING][PATCH][PR sanitizer/77631] Support separate debug info in libbacktrace
Hello everyone, this is a ping for patch https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01209.html
[RFC][PR 67336][PING^2] Verify pointers during stack unwind
Hi all,
Libgcc unwinder currently does not do any verification of pointers
which it chases on stack. In practice this not so rarely causes
segfaults when unwinding on corrupted stacks (e.g. when when trying to
print diagnostic on
fatal error) [1]. Ironically this usually happens in error reporting
code which puzzles users.
I've attached one motivating example - with current libgcc it will
abort inside unwinder when trying to access invalid address
(0x0a0a0a0a).
There is an old request to provide a safer version of
_Unwind_Backtrace (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67336)
that would check pointer validity prior to dereferencing. I've
attached a draft patch to see if this approach is viable and hopefully
get some advice.
The change is rather straightforward: I add a new
_Unwind_Backtrace_Checked which checks return value of msync on every
potentially unsafe access (libunwind does something like this as well,
although in a very incomplete manner).
To avoid paying for syscalls too often, I cache the last checked
memory page. Potentially parsing /proc/$$/maps would allow for much
faster verification but I didn't bother too much as new APIs are
intended for reporting fatal errors where speed isn't an issue.
The code is only implemented for DW2 unwinder (probably used on most targets).
So my questions now are:
1) Would this feature considered useful i.e. will it be accepted for
trunk once implementation is polished/tested?
2) Should I strive to implement it for all possible targets or DW2
would do for now? I don't have easy access to other platforms (ARM,
C6x, etc.) so this may delay implementation.
3) Any suggestions/comments on this attached draft implementation?
E.g. alternative syscalls to use (Andrew suggested futex), how many
verified addresses to cache, whether I should verify unwind table
accesses in addition to stack accesses, etc.
-Y
[1] The issue has been reported in the past e.g. "Adventure with Stack
Smashing Protector" (http://site.pi3.com.pl/papers/ASSP.pdf).
#include
#include
struct _Unwind_Context;
typedef int (*_Unwind_Trace_Fn)(struct _Unwind_Context *, void *vdata);
extern int _Unwind_Backtrace(_Unwind_Trace_Fn trace, void * trace_argument);
extern int _Unwind_Backtrace_Checked(_Unwind_Trace_Fn trace, void * trace_argument);
extern void *_Unwind_GetIP (struct _Unwind_Context *context);
int simple_unwind (struct _Unwind_Context *context, void *vdata) {
printf("Next frame: ");
void *pc = _Unwind_GetIP(context);
printf("%p\n", pc);
return 0;
}
#define noinline __attribute__((noinline))
noinline int foo() {
// Clobber stack to provoke errors in unwinder
int x;
void *p = &x;
asm("" :: "r"(p));
memset(p, 0xa, 128);
printf("After clobbering stack\n");
int ret = _Unwind_Backtrace(simple_unwind, 0);
printf("After unwind: %d\n", ret);
return 0;
}
noinline int bar() {
int x = foo();
return x + 1;
}
int main() {
bar();
return 0;
}
safe-unwind-1.patch
Description: Binary data
Re: [RFC][PR 67336][PING^2] Verify pointers during stack unwind
On Sun, Jun 25, 2017 at 12:08 PM, Yuri Gribov wrote: > Hi all, > > Libgcc unwinder currently does not do any verification of pointers > which it chases on stack. In practice this not so rarely causes > segfaults when unwinding on corrupted stacks (e.g. when when trying to > print diagnostic on > fatal error) [1]. Ironically this usually happens in error reporting > code which puzzles users. > > I've attached one motivating example - with current libgcc it will > abort inside unwinder when trying to access invalid address > (0x0a0a0a0a). > > There is an old request to provide a safer version of > _Unwind_Backtrace (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67336) > that would check pointer validity prior to dereferencing. I've > attached a draft patch to see if this approach is viable and hopefully > get some advice. > > The change is rather straightforward: I add a new > _Unwind_Backtrace_Checked which checks return value of msync on every > potentially unsafe access (libunwind does something like this as well, > although in a very incomplete manner). > To avoid paying for syscalls too often, I cache the last checked > memory page. Potentially parsing /proc/$$/maps would allow for much > faster verification but I didn't bother too much as new APIs are > intended for reporting fatal errors where speed isn't an issue. > > The code is only implemented for DW2 unwinder (probably used on most targets). > > So my questions now are: > 1) Would this feature considered useful i.e. will it be accepted for > trunk once implementation is polished/tested? > 2) Should I strive to implement it for all possible targets or DW2 > would do for now? I don't have easy access to other platforms (ARM, > C6x, etc.) so this may delay implementation. > 3) Any suggestions/comments on this attached draft implementation? > E.g. alternative syscalls to use (Andrew suggested futex), how many > verified addresses to cache, whether I should verify unwind table > accesses in addition to stack accesses, etc. The version script should be using GCC_8.0.0 since 6.2.0 has already shipped months ago. Also all patches should be submitted against the trunk and not a released version. Thanks, Andrew > > -Y > > [1] The issue has been reported in the past e.g. "Adventure with Stack > Smashing Protector" (http://site.pi3.com.pl/papers/ASSP.pdf).
[C++ Patch] PR 65775 ("Late-specified return type bypasses return type checks (qualified, function, array)")
Hi,
in grokdeclarator the checks on the return type do nothing useful in
case of late-specified return type because they happen too early, before
splice_late_return_type is called. A straightforward way to solve the
problem involves separating the checks themselves to a new
check_function_return_type and calling it again only in case
splice_late_return_type actually spliced the late return type (in that
case, we should also consistently update type_quals).
As a side issue, if we want to use good and consistent locations for the
error messages (the testcase below checks that too), we need a fall back
to input_location for typespec_loc, because otherwise an
UNKNOWN_LOCATION is used for the error messages of testcases like
lambda-syntax1.C and pr60190.C.
Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2017-06-25 Paolo Carlini
PR c++/65775
* decl.c (check_function_return_type): New.
(grokdeclarator): Use the latter; if declspecs->locations[ds_type_spec]
is UNKNOWN_LOCATION fall back to input_location.
/testsuite
2017-06-25 Paolo Carlini
PR c++/65775
* g++.dg/cpp0x/trailing14.C: New.
Index: testsuite/g++.dg/cpp0x/trailing14.C
===
--- testsuite/g++.dg/cpp0x/trailing14.C (revision 0)
+++ testsuite/g++.dg/cpp0x/trailing14.C (working copy)
@@ -0,0 +1,15 @@
+// PR c++/65775
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wignored-qualifiers" }
+
+using Qi = int const volatile;
+Qi q1(); // { dg-warning "1: type qualifiers ignored" }
+auto q2() -> Qi; // { dg-warning "1: type qualifiers ignored" }
+
+using Fi = int();
+Fi f1(); // { dg-error "1: 'f1' declared as function returning a
function" }
+auto f2() -> Fi; // { dg-error "1: 'f2' declared as function returning a
function" }
+
+using Ai = int[5];
+Ai a1(); // { dg-error "1: 'a1' declared as function returning an
array" }
+auto a2() -> Ai; // { dg-error "1: 'a2' declared as function returning an
array" }
Index: cp/decl.c
===
--- cp/decl.c (revision 249632)
+++ cp/decl.c (working copy)
@@ -9789,7 +9789,43 @@ mark_inline_variable (tree decl)
}
}
+/* If TYPE has TYPE_QUALS != TYPE_UNQUALIFIED issue a warning controlled by
+ -Wignored-qualifiers and clear TYPE_QUALS to avoid cascading diagnostics.
+ If TYPE is not valid, issue an error message and return error_mark_node,
+ otherwise return TYPE itself. */
+static tree
+check_function_return_type (tree type, int& type_quals,
+ const char* name, location_t loc)
+{
+ if (type_quals != TYPE_UNQUALIFIED)
+{
+ if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))
+ warning_at (loc, OPT_Wignored_qualifiers, "type "
+ "qualifiers ignored on function return type");
+
+ /* We now know that the TYPE_QUALS don't apply to the
+decl, but to its return type. */
+ type_quals = TYPE_UNQUALIFIED;
+}
+
+ /* Error about some types functions can't return. */
+
+ if (TREE_CODE (type) == FUNCTION_TYPE)
+{
+ error_at (loc, "%qs declared as function returning a function", name);
+ return error_mark_node;
+}
+
+ if (TREE_CODE (type) == ARRAY_TYPE)
+{
+ error_at (loc, "%qs declared as function returning an array", name);
+ return error_mark_node;
+}
+
+ return type;
+}
+
/* Assign a typedef-given name to a class or enumeration type declared
as anonymous at first. This was split out of grokdeclarator
because it is also used in libcc1. */
@@ -9993,6 +10029,8 @@ grokdeclarator (const cp_declarator *declarator,
declspecs->locations);
if (typespec_loc == UNKNOWN_LOCATION)
typespec_loc = declspecs->locations[ds_type_spec];
+ if (typespec_loc == UNKNOWN_LOCATION)
+typespec_loc = input_location;
/* Look inside a declarator for the name being declared
and get it as a string, for an error message. */
@@ -10828,31 +10866,10 @@ grokdeclarator (const cp_declarator *declarator,
/* Declaring a function type.
Make sure we have a valid type for the function to return. */
- if (type_quals != TYPE_UNQUALIFIED)
- {
- if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))
- {
- warning_at (typespec_loc, OPT_Wignored_qualifiers, "type "
- "qualifiers ignored on function return type");
- }
- /* We now know that the TYPE_QUALS don't apply to the
- decl, but to its return type. */
- type_quals = TYPE_UNQUALIFIED;
- }
+ if (check_function_return_type (type, type_quals, name,
typespec_loc)
+ == error_mark_node)
+ return error_mark_node;
- /*
Merge from GCC trunk to gccgo branch
I merged GCC trunk revision 249632 to the gccgo branch. Ian
Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)
On Sun, Jun 25, 2017 at 11:18 AM, Andrew Pinski wrote:
> On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse wrote:
>> +(for cmp (gt ge lt le)
>> + outp (convert convert negate negate)
>> + outn (negate negate convert convert)
>> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
>> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
>> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
>> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
>> + (simplify
>> + (cond (cmp @0 real_zerop) real_onep real_minus_onep)
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
>> + && types_match (type, TREE_TYPE (@0)))
>> + (switch
>> +(if (types_match (type, float_type_node))
>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0)))
>> +(if (types_match (type, double_type_node))
>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0)))
>> +(if (types_match (type, long_double_type_node))
>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0))
>>
>> There is already a 1.0 of the right type in the input, it would be easier to
>> reuse it in the output than build a new one.
>
> Right. Fixed.
>
>>
>> Non-generic builtins like copysign are such a pain... We also end up missing
>> the 128-bit case that way (pre-existing problem, not your patch). We seem to
>> have a corresponding internal function, but apparently it is not used until
>> expansion (well, maybe during vectorization).
>
> Yes I noticed that while working on a different patch related to
> copysign; The generic version of a*copysign(1.0, b) [see the other
> thread where the ARM folks started a patch for it; yes it was by pure
> accident that I was working on this and really did not notice that
> thread until yesterday].
> I was looking into a nice way of creating copysign without having to
> do the switch but I could not find one. In the end I copied was done
> already in a different location in match.pd; this is also the reason
> why I had the build_one_cst there.
>
>>
>> + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
>> + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
>> + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */
>> + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */
>> + (simplify
>> + (cond (cmp @0 real_zerop) real_minus_onep real_onep)
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
>> + && types_match (type, TREE_TYPE (@0)))
>> + (switch
>> +(if (types_match (type, float_type_node))
>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0)))
>> +(if (types_match (type, double_type_node))
>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0)))
>> +(if (types_match (type, long_double_type_node))
>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0)))
>> +
>> +/* Transform X * copysign (1.0, X) into abs(X). */
>> +(simplify
>> + (mult:c @0 (COPYSIGN real_onep @0))
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>> + (abs @0)))
>>
>> I would have expected it do to the right thing for signed zero and qNaN. Can
>> you describe a case where it would give the wrong result, or are the
>> conditions just conservative?
>
> I was just being conservative; maybe too conservative but I was a bit
> worried I could get it incorrect.
>
>>
>> +/* Transform X * copysign (1.0, -X) into -abs(X). */
>> +(simplify
>> + (mult:c @0 (COPYSIGN real_onep (negate @0)))
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>> + (negate (abs @0
>> +
>> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */
>> +(simplify
>> + (COPYSIGN real_minus_onep @0)
>> + (COPYSIGN { build_one_cst (type); } @0))
>>
>> (simplify
>> (COPYSIGN REAL_CST@0 @1)
>> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
>> (COPYSIGN (negate @0) @1)))
>> ? Or does that create trouble with sNaN and only the 1.0 case is worth
>> the trouble?
>
> No that is the correct way; I Noticed the other thread about copysign
> had something similar as what should be done too.
>
> I will send out a new patch after testing soon.
New patch.
OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions.
Thanks,
Andrew Pinski
ChangeLog:
* match.pd (X >/>=/
> Thanks,
> Andrew
>
>>
>> --
>> Marc Glisse
Index: match.pd
===
--- match.pd(revision 249626)
+++ match.pd(working copy)
@@ -155,6 +155,58 @@
|| !COMPLEX_FLOAT_TYPE_P (type)))
(negate @0)))
+(for cmp (gt ge lt le)
+ outp (convert convert negate negate)
+ outn (negate negate convert convert)
+ /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ (simplify
+ (cond (cmp @0 real_zerop) real_onep@1 real_minus_onep)
+ (if (!HONO
Re: [RFC][AARCH64]Add 'r' integer register operand modifier. Document the common asm modifier for aarch64 target.
On Tue, Jun 6, 2017 at 3:56 AM, Renlin Li wrote: > Hi all, > > In this patch, a new integer register operand modifier 'r' is added. This > will use the > proper register name according to the mode of corresponding operand. > > 'w' register for scalar integer mode smaller than DImode > 'x' register for DImode > > This allows more flexibility and would meet people's expectations. > It will help for ILP32 and LP64, and big-endian case. > > A new section is added to document the AArch64 operand modifiers which might > be used in inline assembly. It's not an exhaustive list covers every > modifier. > Only the most common and useful ones are documented. > > The default behavior of integer operand without modifier is clearly > documented > as well. It's not changed so that the patch shouldn't break anything. > > So with this patch, it should resolve the issues in PR63359. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63359 > > > aarch64-none-elf regression test Okay. Okay to check in? I think 'r' modifier is very fragile and can be used incorrectly and wrong in some cases really.. I like the documentation though. Thanks, Andrew > > gcc/ChangeLog: > > 2017-06-06 Renlin Li > > PR target/63359 > * config/aarch64/aarch64.c (aarch64_print_operand): Add 'r' > modifier. > * doc/extend.texi (AArch64Operandmodifiers): New section.
Re: [C++ Patch] PR 65775 ("Late-specified return type bypasses return type checks (qualified, function, array)")
... in fact, simply moving the checks forward, past the
splice_late_return_type call, appears to work fine. I'm finishing
testing the below.
Thanks!
Paolo.
/
/cp
2017-06-25 Paolo Carlini
PR c++/65775
* decl.c (grokdeclarator): Move checks on function return type after
the splice_late_return_type call; if declspecs->locations[ds_type_spec]
is UNKNOWN_LOCATION fall back to input_location.
/testsuite
2017-06-25 Paolo Carlini
PR c++/65775
* g++.dg/cpp0x/trailing14.C: New.
Index: testsuite/g++.dg/cpp0x/trailing14.C
===
--- testsuite/g++.dg/cpp0x/trailing14.C (revision 0)
+++ testsuite/g++.dg/cpp0x/trailing14.C (working copy)
@@ -0,0 +1,15 @@
+// PR c++/65775
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wignored-qualifiers" }
+
+using Qi = int const volatile;
+Qi q1(); // { dg-warning "1: type qualifiers ignored" }
+auto q2() -> Qi; // { dg-warning "1: type qualifiers ignored" }
+
+using Fi = int();
+Fi f1(); // { dg-error "1: 'f1' declared as function returning a
function" }
+auto f2() -> Fi; // { dg-error "1: 'f2' declared as function returning a
function" }
+
+using Ai = int[5];
+Ai a1(); // { dg-error "1: 'a1' declared as function returning an
array" }
+auto a2() -> Ai; // { dg-error "1: 'a2' declared as function returning an
array" }
Index: cp/decl.c
===
--- cp/decl.c (revision 249632)
+++ cp/decl.c (working copy)
@@ -9993,6 +9993,8 @@ grokdeclarator (const cp_declarator *declarator,
declspecs->locations);
if (typespec_loc == UNKNOWN_LOCATION)
typespec_loc = declspecs->locations[ds_type_spec];
+ if (typespec_loc == UNKNOWN_LOCATION)
+typespec_loc = input_location;
/* Look inside a declarator for the name being declared
and get it as a string, for an error message. */
@@ -10825,34 +10827,8 @@ grokdeclarator (const cp_declarator *declarator,
tree arg_types;
int funcdecl_p;
- /* Declaring a function type.
- Make sure we have a valid type for the function to return. */
+ /* Declaring a function type. */
- if (type_quals != TYPE_UNQUALIFIED)
- {
- if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))
- {
- warning_at (typespec_loc, OPT_Wignored_qualifiers, "type "
- "qualifiers ignored on function return type");
- }
- /* We now know that the TYPE_QUALS don't apply to the
- decl, but to its return type. */
- type_quals = TYPE_UNQUALIFIED;
- }
-
- /* Error about some types functions can't return. */
-
- if (TREE_CODE (type) == FUNCTION_TYPE)
- {
- error ("%qs declared as function returning a function", name);
- return error_mark_node;
- }
- if (TREE_CODE (type) == ARRAY_TYPE)
- {
- error ("%qs declared as function returning an array", name);
- return error_mark_node;
- }
-
input_location = declspecs->locations[ds_type_spec];
abstract_virtuals_error (ACU_RETURN, type);
input_location = saved_loc;
@@ -10959,8 +10935,36 @@ grokdeclarator (const cp_declarator *declarator,
return error_mark_node;
if (late_return_type)
- late_return_type_p = true;
+ {
+ late_return_type_p = true;
+ type_quals = cp_type_quals (type);
+ }
+ if (type_quals != TYPE_UNQUALIFIED)
+ {
+ if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))
+ warning_at (typespec_loc, OPT_Wignored_qualifiers, "type "
+ "qualifiers ignored on function return type");
+ /* We now know that the TYPE_QUALS don't apply to the
+ decl, but to its return type. */
+ type_quals = TYPE_UNQUALIFIED;
+ }
+
+ /* Error about some types functions can't return. */
+
+ if (TREE_CODE (type) == FUNCTION_TYPE)
+ {
+ error_at (typespec_loc, "%qs declared as function returning "
+ "a function", name);
+ return error_mark_node;
+ }
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ {
+ error_at (typespec_loc, "%qs declared as function returning "
+ "an array", name);
+ return error_mark_node;
+ }
+
if (ctype == NULL_TREE
&& decl_context == FIELD
&& funcdecl_p
[PATCH, libgfortran] proposed fix for SPEC CPU2017 621.wrf_s failures
As mentioned in bug 81195, I see openmp related failures due to a lack
of locking of the newunit_stack and newunit_tos variables. The code
locks when pushing onto the stack, but does not lock when popping from
the stack. This can cause multiple threads to pop the same structure,
which then eventually leads to an error.
This patch was tested with an aarch64 bootstrap and make check. There
were no regressions. It was also tested with a SPEC CPU2017 run, and
solves the 621.wrf_s failure I get without the patch.
gcc 7 has the same problem. gcc 6 is OK.
Jim
libgfortran/
PR libfortran/81195
* io/unit.c (get_unit): Call __gthread_mutex_lock before newunit_stack
and newunit_tos references. Call __gthread_mutex_unlock afterward.
Index: libgfortran/io/unit.c
===
--- libgfortran/io/unit.c (revision 249613)
+++ libgfortran/io/unit.c (working copy)
@@ -583,14 +583,17 @@
}
else
{
+ __gthread_mutex_lock (&unit_lock);
if (newunit_tos)
{
dtp->common.unit = newunit_stack[newunit_tos].unit_number;
unit = newunit_stack[newunit_tos--].unit;
+ __gthread_mutex_unlock (&unit_lock);
unit->fbuf->act = unit->fbuf->pos = 0;
}
else
{
+ __gthread_mutex_unlock (&unit_lock);
dtp->common.unit = newunit_alloc ();
unit = xcalloc (1, sizeof (gfc_unit));
fbuf_init (unit, 128);
@@ -603,12 +606,15 @@
/* If an internal unit number is passed from the parent to the child
it should have been stashed on the newunit_stack ready to be used.
Check for it now and return the internal unit if found. */
+ __gthread_mutex_lock (&unit_lock);
if (newunit_tos && (dtp->common.unit <= NEWUNIT_START)
&& (dtp->common.unit == newunit_stack[newunit_tos].unit_number))
{
unit = newunit_stack[newunit_tos--].unit;
+ __gthread_mutex_unlock (&unit_lock);
return unit;
}
+ __gthread_mutex_unlock (&unit_lock);
/* Has to be an external unit. */
dtp->u.p.unit_is_internal = 0;
Re: [GCC][PATCH][mid-end] Optimize x * copysign (1.0, y) [Patch (1/2)]
On Sat, Jun 24, 2017 at 4:53 PM, Andrew Pinski wrote:
> On Mon, Jun 12, 2017 at 12:56 AM, Tamar Christina
> wrote:
>> Hi All,
>>
>> this patch implements a optimization rewriting
>>
>> x * copysign (1.0, y) and
>> x * copysign (-1.0, y)
>
>
> This reminds me:
> copysign(-1.0, y) can be just optimized to:
> copysign(1.0, y)
>
> I did that in my patch here:
> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01860.html
I updated the patch to handle all constants and not just -1.0.
>
> This should allow you to reduce the number of patterns needed to match here.
> Note I still think we could do this in expand without a new
> builtin/internal function.
> I might go and code that up soonish.
Also something like attached (NOTE this is NOT a full patch and needs
the xorsign optabs part of your patch) should work for the expand side
rather than creating a new builtin. There still needs to handling of
the vector based copysign. But you should get the general idea. I
would like to see more of these special expand patterns really.
NOTE you can remove the target hook part and just check if xorsign
optab is there. I don't know if that is what we want to do if not
allow for generic expanding of this.
Thanks,
Andrew Pinski
>
> Thanks,
> Andrew
>
>>
>> to:
>>
>> x ^ (y & (1 << sign_bit_position))
>>
>> This is done by creating a special builtin during matching and generate the
>> appropriate instructions during expand. This new builtin is called XORSIGN.
>>
>> The expansion of xorsign depends on if the backend has an appropriate optab
>> available. If this is not the case then we use a modified version of the
>> existing
>> copysign which does not take the abs value of the first argument as a fall
>> back.
>>
>> This patch is a revival of a previous patch
>> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00069.html
>>
>> Bootstrapped on both aarch64-none-linux-gnu and x86_64 with no issues.
>> Regression done on aarch64-none-linux-gnu and no regressions.
>>
>> Ok for trunk?
>>
>> gcc/
>> 2017-06-07 Tamar Christina
>>
>> * builtins.def (BUILT_IN_XORSIGN, BUILT_IN_XORSIGNF): New.
>> (BUILT_IN_XORSIGNL, BUILT_IN_XORSIGN_FLOAT_NX): Likewise.
>> * match.pd (mult (COPYSIGN:s real_onep @0) @1): New simplifier.
>> (mult (COPYSIGN:s real_mus_onep @0) @1): Likewise.
>> (copysigns @0 (negate @1)): Likewise.
>> * builtins.c (expand_builtin_copysign): Promoted local to argument.
>> (expand_builtin): Added CASE_FLT_FN_FLOATN_NX (BUILT_IN_XORSIGN) and
>> CASE_FLT_FN (BUILT_IN_XORSIGN).
>> (BUILT_IN_COPYSIGN): Updated function call.
>> * optabs.h (expand_copysign): New bool.
>> (expand_xorsign): New.
>> * optabs.def (xorsign_optab): New.
>> * optabs.c (expand_copysign): New parameter.
>> * fortran/f95-lang.c (xorsignl, xorsign, xorsignf): New.
>> * fortran/mathbuiltins.def (XORSIGN): New.
>>
>> gcc/testsuite/
>> 2017-06-07 Tamar Christina
>>
>> * gcc.dg/tree-ssa/xorsign.c: New.
>> * gcc.dg/xorsign_exec.c: New.
>> * gcc.dg/vec-xorsign_exec.c: New.
>> * gcc.dg/tree-ssa/reassoc-39.c (f2, f3): Updated constant to 2.
Index: gcc/expr.c
===
--- gcc/expr.c (revision 249619)
+++ gcc/expr.c (working copy)
@@ -8182,6 +8182,59 @@
return NULL_RTX;
}
+static bool
+is_copysign_call_with_1 (gimple *call)
+{
+ if (!is_gimple_call (call))
+return false;
+
+ if (gimple_call_builtin_p (call, BUILT_IN_NORMAL))
+{
+ gcall *c = as_a (call);
+ tree decl = gimple_call_fndecl (call);
+ switch (DECL_FUNCTION_CODE (decl))
+ {
+CASE_FLT_FN (BUILT_IN_COPYSIGN):
+ CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
+ return real_one_p (gimple_call_arg (c, 0));
+ default:
+ return false;
+ }
+}
+}
+
+static rtx
+maybe_expand_mult_copysign (tree treeop0, tree treeop1, rtx target)
+{
+ tree type = TREE_TYPE (treeop0);
+ rtx op0, op1;
+
+ if (!SCALAR_FLOAT_TYPE_P (type)
+ && VECTOR_FLOAT_TYPE_P (type))
+return NULL;
+
+ if (HONOR_SNANS (type))
+return NULL;
+
+ if (!targetm.expand_mult_copysign_xor ())
+return NULL;
+
+ if (TREE_CODE (treeop0) == SSA_NAME)
+{
+ gimple *call0 = SSA_NAME_DEF_STMT (treeop0);
+ if (is_copysign_call_with_1 (call0))
+ {
+ gcall *c = as_a (call0);
+ treeop0 = gimple_call_arg (c, 1);
+ expand_operands (treeop1, treeop0, NULL_RTX, &op0, &op1,
EXPAND_NORMAL);
+ return expand_copysign (op0, op1, target, true);
+ }
+}
+
+ return NULL;
+}
+
+
rtx
expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
enum expand_modifier modifier)
@@ -8791,6 +8844,10 @@
if (modifier == EXPAND_STACK_PARM)
target = 0;
+ temp = maybe_expand_mult_copysign (treeop0, treeop1);
+ if (temp)
+ return temp;
+
expand_opera
Re: [PATCH, libgfortran] proposed fix for SPEC CPU2017 621.wrf_s failures
On 06/25/2017 05:50 PM, Jim Wilson wrote: > As mentioned in bug 81195, I see openmp related failures due to a lack > of locking of the newunit_stack and newunit_tos variables. The code > locks when pushing onto the stack, but does not lock when popping from > the stack. This can cause multiple threads to pop the same structure, > which then eventually leads to an error. > > This patch was tested with an aarch64 bootstrap and make check. There > were no regressions. It was also tested with a SPEC CPU2017 run, and > solves the 621.wrf_s failure I get without the patch. > > gcc 7 has the same problem. gcc 6 is OK. > > Jim > OK to commit for trunk and 7. Thanks for spotting this, Jerry
