Re: [PATCH] Fix combine's simplify_if_then_else (PR rtl-optimization/81553)
On Fri, Nov 24, 2017 at 07:19:29PM -0600, Segher Boessenkool wrote: > Hi, > > On Fri, Nov 24, 2017 at 10:38:13PM +0100, Jakub Jelinek wrote: > > The following testcase ICEs in wide-int*, but the reason is a mode mismatch > > (we build a SImode MULT with one QImode argument and one VOIDmode argument, > > then it is folded into SImode NEG with QImode argument, ...). > > The bug is in assuming that the mode of c1 must be m, that is usually the > > case, but shifts are special, the second argument can have a different mode. > > > > The following patch makes sure we perform the computation of the new shift > > count in the right mode. > > Okay, thanks! Is this better than bailing out though, do you have > an example? I don't, but I haven't bootstrapped/regtested with a patch to gather statistics on whether it would ever be successful (how? Should I note in a flag successful optimization of this and then check that flag upon successful try_combine and clear it at the end of any try_combine? It would need to be done for all targets probably). This code dates from 1994, but testsuite was separate, so it is hard to check if this was covered by any testcases. If C1 is a constant (but then it would likely have VOIDmode), then I can imagine it would match, though hard to find testcases, because usually GIMPLE folding should propagate constants far earlier. If C1 is a pseudo, then we'd need an instruction that has (any_shift (something) (mult (something) (something))) which is quite unlikely. But it is similarly unlikely for (ior (something) (mult (something) (something))) and similar. Or can try_combine then split it into two separate instructions, one doing the mult and one doing the other, say when combining from original 3 or 4 instructions? If so, then it could be likely it hits. Jakub
Re: [PATCH][PR target/81535] Fix tests on Power
On Tue, Aug 8, 2017 at 12:32 AM, Segher Boessenkool
wrote:
> Hi Yuri,
>
> Sorry I missed this. Please cc: me to prevent that from happening.
Segher,
Sorry, somehow I missed your reply!
> On Fri, Jul 28, 2017 at 05:42:00AM +0100, Yury Gribov wrote:
>> This patch fixes issues reported in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81535
>>
>> I removed call to g in pr79439.c because gcc was duplicating the basic
>> block with call depending on compiler flags (so scan-assembler-times
>> pattern wasn't reliable anymore). I also added alias to prevent
>> inlining introduced by recent PR56727 patch.
>>
>> I added Power-specific pattern in pr56727-2.c testcase and disabled
>> testing on Power in pr56727-1.c.
>>
>> Tested on powerpc64-unknown-linux-gnu. Ok for trunk?
>
> Did you also test this with -m32? And on powerpc64le-linux, the target
> the bug is reported against? The three ABIs are different.
No, only tested standard powerpc64. I'll go check other targets then.
>> PR target/81535
>> * gcc.dg/pr56727-1.c: Do not check output on Power.
>> * gcc.dg/pr56727-2.c: Fix pattern for Power.
>
> Please name the actual target triple here, i.e. powerpc*-*-* .
Makes sense.
>> * gcc.target/powerpc/pr79439.c: Prevent inlining.
>
>> diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-1.c
>> gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c
>> --- gcc/gcc/testsuite/gcc.dg/pr56727-1.c 2017-07-28 02:39:54.770046466
>> +
>> +++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c2017-07-28
>> 04:25:04.805648587 +
>> @@ -1,6 +1,6 @@
>> /* { dg-do compile { target fpic } } */
>> /* { dg-options "-O2 -fPIC" } */
>> -/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-*
>> x86_64-*-* powerpc*-*-* } } } */
>> +/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-*
>> x86_64-*-* } } } */
>>
>> #define define_func(type) \
>>void f_ ## type (type b) { f_ ## type (0); } \
>
> Is it correct that current GCC does not do the call via the PLT?
Well, it was decided in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56727 that it would be a
valid optimization because the only way to expose the difference would
be through dlsym hackery. Note that original PowerPC use-case
(reported in https://sourceware.org/bugzilla/show_bug.cgi?id=21116)
would benefit from this optimization as because PLT call + indirection
would be replaced by normal PC-relative call.
>> diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-2.c
>> gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c
>> --- gcc/gcc/testsuite/gcc.dg/pr56727-2.c 2017-07-28 02:39:54.770046466
>> +
>> +++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c2017-07-28
>> 04:21:19.195215187 +
>> @@ -1,10 +1,10 @@
>> /* { dg-do compile { target fpic } } */
>> /* { dg-options "-O2 -fPIC" } */
>> -/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-*
>> powerpc*-*-linux* } } } */
>>
>> __attribute__((noinline, noclone))
>> void f (short b)
>> {
>> + __builtin_setjmp (0); /* Prevent tailcall */
>>f (0);
>> }
>>
>
> This change is not mentioned in the changelog.
Well, this was a minor hack to prevent tailcalling, I thought it does
not deserve a comment...
>> @@ -14,3 +14,5 @@ void h ()
>> {
>>g (0);
>> }
>> +/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* }
>> } } */
>> +/* { dg-final { scan-assembler "bl f\n\[ \t\]*nop" { target
>> powerpc*-*-linux* } } } */
>
> Is there a real reason there cannot be a tailcall here? You can write
> this as { scan-assembler {\mbl f\s+nop\M} } btw.
Yes, setjmp will block tailcall optimization. Will fix the pattern, thanks!
>> diff -rupN gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c
>> gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c
>> --- gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c2017-07-28
>> 02:39:55.750048426 +
>> +++ gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c 2017-07-28
>> 04:13:47.834177237 +
>> @@ -8,22 +8,17 @@
>>
>> int f (void);
>>
>> -void
>> -g (void)
>> -{
>> -}
>> -
>> int
>> rec (int a)
>> {
>>int ret = 0;
>>if (a > 10 && f ())
>> ret += rec (a - 1);
>> - g ();
>>return a + ret;
>> }
>>
>> +void rec_alias (short) __attribute__ ((alias ("rec")));
>> +
>> /* { dg-final { scan-assembler-times {\mbl f\M} 1 } } */
>> -/* { dg-final { scan-assembler-times {\mbl g\M} 1 } } */
>> /* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
>> -/* { dg-final { scan-assembler-times {\mnop\M}3 } } */
>> +/* { dg-final { scan-assembler-times {\mnop\M}2 } } */
>
> The changelog says "prevent inlining", but you actually delete the code.
> Is that okay, wasn't the testcase actually testing for something here?
TBH at this point I don't remember. I think call to g() didn't add
anything to the test so I removed it. Let me comment on this once I
retest the patch as discussed above.
-Y
Re: [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275)
On Fri, Nov 24, 2017 at 10:59:53PM +0100, Jakub Jelinek wrote:
> The testcase below has a useless break; that causes a bogus -Wreturn-type
> warning. The C++ FE already has code to avoid adding a BREAK_STMT
> after a return or similar sequence that is known not to return.
> The following patch extends block_may_fallthrough to also return false
> for SWITCH_STMTs that can't fall through.
>
> Those are ones with non-empty body where the whole body can't fallthrough,
> additionally they need to have a default: case label (or cover the whole
> range of values, but that is not what this patch can compute, that would
> be too big duplication of the gimplification processing) and no BREAK_STMT.
>
> For the default: case label we need to look in all SWITCH_BODY children
> except for nested SWITCH_STMTs, for BREAK_STMTs also not in
> {FOR,DO,WHILE}_BODY.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Actually, thinking about it some more, maybe it would be more efficient
to gather this information during construction of the SWITCH_STMT in some
new flag on the tree, so cxx_block_may_fallthru would just:
case SWITCH_STMT:
if (SWITCH_STMT_CANT_FALLTHRU_P (stmt)
&& SWITCH_STMT_BODY (stmt)
&& !block_may_fallthru (SWITCH_STMT_BODY (stmt)))
return false;
return true;
and
/* Set if the body of a switch stmt contains a default: case label
and does not contain any break; stmts, thus if SWITCH_STMT_BODY
is not empty and doesn't fallthru, then the whole switch stmt
can't. */
#define SWITCH_STMT_CANT_FALLTHRU_P(NODE) \
TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE))
Seems the C++ FE already has switch_stack, so we could just add there
a has_default_p, has_break_stmt_p and inside_loop_p flags and both
inside of templates and outside in finish_case_label, in
finish_break_stmt if actually adding BREAK_STMT and when entering
a body of a FOR/DO/WHILE loop tweak those flags. Seems switch_stack
is also maintained during pt.c, but we should compute it both during
parsing and during pt.c (start with the bit clear on a new SWITCH_STMT).
Thoughts on this?
Guess for the C FE we should similarly handle SWITCH_EXPR, though a break;
in there is a goto outside of the switch, so all we need to note is a
default: case label.
Jakub
[i386] Mask generation in avx2intrin.h
Hello,
the way full masks are generated currently in avx2intrin.h is
questionable: opaque for the inline functions, weird/wrong for the macros.
It is possible we may want to add code so the constant mask with all ones
may be generated with vxorpd+vcmpeqpd instead of loading it from memory,
but that looks like something that should be decided globally, not in each
instruction that uses it.
Bootstrap+regtest on x86_64-pc-linux-gnu (skylake).
2017-11-27 Marc Glisse
PR target/80885
* config/i386/avx2intrin.h (_mm_i32gather_pd): Rewrite mask generation.
(_mm256_i32gather_pd): Likewise.
(_mm_i64gather_pd): Likewise.
(_mm256_i64gather_pd): Likewise.
(_mm_i32gather_ps): Likewise.
(_mm256_i32gather_ps): Likewise.
(_mm_i64gather_ps): Likewise.
(_mm256_i64gather_ps): Likewise.
--
Marc GlisseIndex: gcc/config/i386/avx2intrin.h
===
--- gcc/config/i386/avx2intrin.h (revision 255140)
+++ gcc/config/i386/avx2intrin.h (working copy)
@@ -1241,22 +1241,21 @@ __attribute__ ((__gnu_inline__, __always
_mm_srlv_epi64 (__m128i __X, __m128i __Y)
{
return (__m128i) __builtin_ia32_psrlv2di ((__v2di)__X, (__v2di)__Y);
}
#ifdef __OPTIMIZE__
extern __inline __m128d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm_i32gather_pd (double const *__base, __m128i __index, const int __scale)
{
- __v2df __zero = _mm_setzero_pd ();
- __v2df __mask = _mm_cmpeq_pd (__zero, __zero);
+ __v2df __mask = (__v2df)_mm_set1_epi64x (-1);
return (__m128d) __builtin_ia32_gathersiv2df (_mm_undefined_pd (),
__base,
(__v4si)__index,
__mask,
__scale);
}
extern __inline __m128d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
@@ -1267,22 +1266,21 @@ _mm_mask_i32gather_pd (__m128d __src, do
__base,
(__v4si)__index,
(__v2df)__mask,
__scale);
}
extern __inline __m256d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm256_i32gather_pd (double const *__base, __m128i __index, const int __scale)
{
- __v4df __zero = _mm256_setzero_pd ();
- __v4df __mask = _mm256_cmp_pd (__zero, __zero, _CMP_EQ_OQ);
+ __v4df __mask = (__v4df)_mm256_set1_epi64x (-1);
return (__m256d) __builtin_ia32_gathersiv4df (_mm256_undefined_pd (),
__base,
(__v4si)__index,
__mask,
__scale);
}
extern __inline __m256d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
@@ -1294,21 +1292,21 @@ _mm256_mask_i32gather_pd (__m256d __src,
(__v4si)__index,
(__v4df)__mask,
__scale);
}
extern __inline __m128d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm_i64gather_pd (double const *__base, __m128i __index, const int __scale)
{
__v2df __src = _mm_setzero_pd ();
- __v2df __mask = _mm_cmpeq_pd (__src, __src);
+ __v2df __mask = (__v2df)_mm_set1_epi64x (-1);
return (__m128d) __builtin_ia32_gatherdiv2df (__src,
__base,
(__v2di)__index,
__mask,
__scale);
}
extern __inline __m128d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
@@ -1320,21 +1318,21 @@ _mm_mask_i64gather_pd (__m128d __src, do
(__v2di)__index,
(__v2df)__mask,
__scale);
}
extern __inline __m256d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm256_i64gather_pd (double const *__base, __m256i __index, const int __scale)
{
__v4df __src = _mm256_setzero_pd ();
- __v4df __mask = _mm256_cmp_pd (__src, __src, _CMP_EQ_OQ);
+ __v4df __mask = (__v4df)_mm256_set1_epi64x (-1);
return (__m256d) __builtin_ia32_gatherdiv4df (__src,
__base,
(__v4di)__index,
__mask,
__scale);
}
extern __inline __m256d
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
@@ -1346,21 +1344,21 @@ _mm256_mask_i64gather_pd (__m256d __src,
(__v4di)__index,
(__v4df)__mask,
__scale);
}
extern __inline __m128
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm_i32gather_ps (float const *__base, __m128i __index, const int __scale)
{
__v4sf __src = _mm_setzero_ps ();
- __v4sf __mask = _mm_cmpeq_ps (__src, __src);
+ __v4sf __mask = (__v4sf)_mm_set1_epi64x (-1);
return (__m128) __builtin_ia32_gathersiv4sf (__src,
__base,
(__v4si)__index,
__mask,
__scale);
}
extern __inline __m128
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
@@ -1372,21 +1370,21 @@ _mm_mask_i32gather_ps (__m128 __src, flo
(__v4si)__index,
(__v4sf)__mask,
__scale);
}
extern __inline __m256
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm256_i32gather_ps (float const *__base, __m256i __index, const int __scale)
{
__v8sf __src = _mm256_setzero_ps ();
- __v8sf
[C/C++] Add support for #pragma GCC unroll v3
Hi,
this is the (hopefully) final implementation of the support for the unrolling
pragma in the C and C++ front-ends. It contains a couple of fixes for the C++
front-ends to make it correctly handle unroll and ivdep for the same loop.
Tested on x86_64-suse-linux, OK for the mainline?
2017-11-25 Mike Stump
Eric Botcazou
ChangeLog/
* doc/extend.texi (Loop-Specific Pragmas): Document pragma GCC unroll.
c-family/ChangeLog:
* c-pragma.c (init_pragma): Register pragma GCC unroll.
* c-pragma.h (enum pragma_kind): Add PRAGMA_UNROLL.
c/ChangeLog:
* c-parser.c (c_parser_while_statement): Add unroll parameter and
build ANNOTATE_EXPR if present. Add 3rd operand to ANNOTATE_EXPR.
(c_parser_do_statement): Likewise.
(c_parser_for_statement): Likewise.
(c_parser_statement_after_labels): Adjust calls to above.
(c_parse_pragma_ivdep): New static function.
(c_parser_pragma_unroll): Likewise.
(c_parser_pragma) : Add support for pragma Unroll.
: New case.
cp/ChangeLog:
* constexpr.c (cxx_eval_constant_expression) : Remove
assertion on 2nd operand.
(potential_constant_expression_1): Likewise.
* cp-array-notation.c (create_an_loop): Adjut call to finish_for_cond.
* cp-tree.h (cp_convert_range_for): Adjust prototype.
(finish_while_stmt_cond): Likewise.
(finish_do_stmt): Likewise.
(finish_for_cond): Likewise.
* init.c (build_vec_init): Adjut call to finish_for_cond.
* parser.c (cp_parser_statement): Adjust call to
cp_parser_iteration_statement.
(cp_parser_for): Add unroll parameter and pass it in calls to
cp_parser_range_for and cp_parser_c_for.
(cp_parser_c_for): Add unroll parameter and pass it in call to
finish_for_cond.
(cp_parser_range_for): Add unroll parameter and pass it in call to
cp_convert_range_for.
(cp_convert_range_for): Add unroll parameter and pass it in call to
finish_for_cond.
(cp_parser_iteration_statement): Add unroll parameter and pass it in
calls to finish_while_stmt_cond, finish_do_stmt and cp_parser_for.
(cp_parser_pragma_ivdep): New static function.
(cp_parser_pragma_unroll): Likewise.
(cp_parser_pragma) : Add support for pragma Unroll.
: New case.
* pt.c (tsubst_expr): Adjut calls to finish_for_cond,
cp_convert_range_for, finish_while_stmt_cond and finish_do_stmt.
: Propagate 3rd operand.
* semantics.c (finish_while_stmt_cond): Add unroll parameter and
build ANNOTATE_EXPR if present. Add 3rd operand to ANNOTATE_EXPR.
(finish_do_stmt): Likewise.
(finish_for_cond): Likewise.
testsuite/ChangeLog:
* c-c++-common/unroll-1.c: New test.
* c-c++-common/unroll-2.c: Likewise.
* c-c++-common/unroll-3.c: Likewise.
* c-c++-common/unroll-4.c: Likewise.
* c-c++-common/unroll-5.c: Likewise.
--
Eric BotcazouIndex: doc/extend.texi
===
--- doc/extend.texi (revision 255147)
+++ doc/extend.texi (working copy)
@@ -22343,9 +22343,7 @@ function. The parenthesis around the op
The @code{#pragma GCC target} pragma is presently implemented for
x86, ARM, AArch64, PowerPC, S/390, and Nios II targets only.
-@end table
-@table @code
@item #pragma GCC optimize (@var{"string"}...)
@cindex pragma GCC optimize
@@ -22356,9 +22354,7 @@ if @code{attribute((optimize("STRING")))
function. The parenthesis around the options is optional.
@xref{Function Attributes}, for more information about the
@code{optimize} attribute and the attribute syntax.
-@end table
-@table @code
@item #pragma GCC push_options
@itemx #pragma GCC pop_options
@cindex pragma GCC push_options
@@ -22369,15 +22365,14 @@ options. It is intended for include fil
to switch to using a different @samp{#pragma GCC target} or
@samp{#pragma GCC optimize} and then to pop back to the previous
options.
-@end table
-@table @code
@item #pragma GCC reset_options
@cindex pragma GCC reset_options
This pragma clears the current @code{#pragma GCC target} and
@code{#pragma GCC optimize} to use the default switches as specified
on the command line.
+
@end table
@node Loop-Specific Pragmas
@@ -22386,7 +22381,6 @@ on the command line.
@table @code
@item #pragma GCC ivdep
@cindex pragma GCC ivdep
-@end table
With this pragma, the programmer asserts that there are no loop-carried
dependencies which would prevent consecutive iterations of
@@ -22421,6 +22415,16 @@ void ignore_vec_dep (int *a, int k, int
@}
@end smallexample
+@item #pragma GCC unroll @var{n}
+@cindex pragma GCC unroll @var{n}
+
+You can use this pragma to control how many times a loop should be unrolled.
+It must be placed immediately before a @code{for}, @code{while} or @code{do}
+loop or a @code{#pragma GCC iv
[fortran] Add support for #pragma GCC unroll v3
Hi,
this is the (hopefully) final implementation of the support for the unrolling
pragma in the Fortran front-end. However the documentation is still missing
because I don't really know where and under which form to put it.
Tested on x86_64-suse-linux, OK for the mainline?
2017-11-25 Bernhard Reutner-Fischer
Eric Botcazou
fortran/ChangeLog:
* array.c (gfc_copy_iterator): Copy unroll field.
* decl.c (directive_unroll): New global variable.
(gfc_match_gcc_unroll): New function.
* gfortran.h (gfc_iterator]): Add unroll field.
(directive_unroll): Declare:
* match.c (gfc_match_do): Use memset to initialize the iterator.
* match.h (gfc_match_gcc_unroll): New prototype.
* parse.c (decode_gcc_attribute): Match "unroll".
(parse_do_block): Set iterator's unroll.
(parse_executable): Diagnose misplaced unroll directive.
* trans-stmt.c (gfc_trans_simple_do) Annotate loop condition with
annot_expr_unroll_kind.
(gfc_trans_do): Likewise.
testsuite/ChangeLog:
* gfortran.dg/directive_unroll_1.f90: New test.
* gfortran.dg/directive_unroll_2.f90: Likewise.
* gfortran.dg/directive_unroll_3.f90: Lkewise.
* gfortran.dg/directive_unroll_4.f90: Likewise.
* gfortran.dg/directive_unroll_5.f90: Likewise.
--
Eric BotcazouIndex: fortran/array.c
===
--- fortran/array.c (revision 255147)
+++ fortran/array.c (working copy)
@@ -2123,6 +2123,7 @@ gfc_copy_iterator (gfc_iterator *src)
dest->start = gfc_copy_expr (src->start);
dest->end = gfc_copy_expr (src->end);
dest->step = gfc_copy_expr (src->step);
+ dest->unroll = src->unroll;
return dest;
}
Index: fortran/decl.c
===
--- fortran/decl.c (revision 255147)
+++ fortran/decl.c (working copy)
@@ -95,6 +95,9 @@ gfc_symbol *gfc_new_block;
bool gfc_matching_function;
+/* Set upon parsing a !GCC$ unroll n directive for use in the next loop. */
+int directive_unroll = -1;
+
/* If a kind expression of a component of a parameterized derived type is
parameterized, temporarily store the expression here. */
static gfc_expr *saved_kind_expr = NULL;
@@ -104,7 +107,6 @@ static gfc_expr *saved_kind_expr = NULL;
static gfc_actual_arglist *decl_type_param_list;
static gfc_actual_arglist *type_param_spec_list;
-
/* DATA statement subroutines */
static bool in_match_data = false;
@@ -10943,3 +10945,37 @@ syntax:
gfc_error ("Syntax error in !GCC$ ATTRIBUTES statement at %C");
return MATCH_ERROR;
}
+
+
+/* Match a !GCC$ UNROLL statement of the form:
+ !GCC$ UNROLL n
+
+ The parameter n is the number of times we are supposed to unroll.
+
+ When we come here, we have already matched the !GCC$ UNROLL string. */
+match
+gfc_match_gcc_unroll (void)
+{
+ int value;
+
+ if (gfc_match_small_int (&value) == MATCH_YES)
+{
+ if (value < 0 || value > USHRT_MAX)
+ {
+ gfc_error ("% directive requires a"
+ " non-negative integral constant"
+ " less than or equal to %u at %C",
+ USHRT_MAX
+ );
+ return MATCH_ERROR;
+ }
+ if (gfc_match_eos () == MATCH_YES)
+ {
+ directive_unroll = value == 0 ? 1 : value;
+ return MATCH_YES;
+ }
+}
+
+ gfc_error ("Syntax error in !GCC$ UNROLL directive at %C");
+ return MATCH_ERROR;
+}
Index: fortran/gfortran.h
===
--- fortran/gfortran.h (revision 255147)
+++ fortran/gfortran.h (working copy)
@@ -2350,6 +2350,7 @@ gfc_case;
typedef struct
{
gfc_expr *var, *start, *end, *step;
+ unsigned short unroll;
}
gfc_iterator;
@@ -2724,6 +2725,7 @@ gfc_finalizer;
/* decl.c */
bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
+extern int directive_unroll;
/* Handling Parameterized Derived Types */
bool gfc_insert_kind_parameter_exprs (gfc_expr *);
Index: fortran/match.c
===
--- fortran/match.c (revision 255147)
+++ fortran/match.c (working copy)
@@ -2539,8 +2539,8 @@ gfc_match_do (void)
old_loc = gfc_current_locus;
+ memset (&iter, '\0', sizeof (gfc_iterator));
label = NULL;
- iter.var = iter.start = iter.end = iter.step = NULL;
m = gfc_match_label ();
if (m == MATCH_ERROR)
Index: fortran/match.h
===
--- fortran/match.h (revision 255147)
+++ fortran/match.h (working copy)
@@ -241,6 +241,7 @@ match gfc_match_contiguous (void);
match gfc_match_dimension (void);
match gfc_match_external (void);
match gfc_match_gcc_attributes (void);
+match gfc_match_gcc_unroll (void);
match gfc_match_import (void);
match gfc_match_intent (void);
match gfc_match_intrinsic (void);
Index: fortran/parse.c
==
Re: [PATCH] Implement std::to_address for C++2a
(Just a minor update to the last patch to use is_function_v instead
of is_function::value)
Implement std::to_address for C++2a
2017-11-25 Glen Joseph Fernandes
* include/bits/ptr_traits.h (to_address): Implement to_address.
* testsuite/20_util/to_address/1.cc: New tests.
Tested x86_64-pc-linux-gnu.
commit f0452d8420a8ef82fa611d53cb9b35f0afecd875
Author: Glen Joseph Fernandes
Date: Sat Nov 25 10:28:23 2017 -0500
Implement std::to_address for C++2a
2017-11-25 Glen Joseph Fernandes
* include/bits/ptr_traits.h (to_address): Implement to_address.
* testsuite/20_util/to_address/1.cc: New tests.
diff --git a/libstdc++-v3/include/bits/ptr_traits.h
b/libstdc++-v3/include/bits/ptr_traits.h
index 74d4c18126c..67cc7e97a80 100644
--- a/libstdc++-v3/include/bits/ptr_traits.h
+++ b/libstdc++-v3/include/bits/ptr_traits.h
@@ -151,10 +151,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__to_address(_Tp* __ptr) noexcept
{ return __ptr; }
+#if __cplusplus <= 201703L
template
constexpr typename std::pointer_traits<_Ptr>::element_type*
__to_address(const _Ptr& __ptr)
{ return std::__to_address(__ptr.operator->()); }
+#else
+ template
+constexpr auto
+__to_address(const _Ptr& __ptr) noexcept
+-> decltype(std::pointer_traits<_Ptr>::to_address(__ptr))
+{ return std::pointer_traits<_Ptr>::to_address(__ptr); }
+
+ template
+constexpr auto
+__to_address(const _Ptr& __ptr, _None...) noexcept
+{ return std::__to_address(__ptr.operator->()); }
+
+ /**
+ * @brief Obtain address referenced by a pointer to an object
+ * @param __ptr A pointer to an object
+ * @return @c __ptr
+ * @ingroup pointer_abstractions
+ */
+ template
+constexpr _Tp*
+to_address(_Tp* __ptr) noexcept
+{
+ static_assert(!std::is_function_v<_Tp>, "not a pointer to function");
+ return __ptr;
+}
+
+ /**
+ * @brief Obtain address referenced by a pointer to an object
+ * @param __ptr A pointer to an object
+ * @return @c pointer_traits<_Ptr>::to_address(__ptr) if that expression is
+ well-formed, otherwise @c to_address(__ptr.operator->())
+ * @ingroup pointer_abstractions
+ */
+ template
+constexpr auto
+to_address(const _Ptr& __ptr) noexcept
+{ return std::__to_address(__ptr); }
+#endif // C++2a
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
diff --git a/libstdc++-v3/testsuite/20_util/to_address/1.cc
b/libstdc++-v3/testsuite/20_util/to_address/1.cc
new file mode 100644
index 000..507d20e18a8
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/to_address/1.cc
@@ -0,0 +1,146 @@
+// Copyright (C) 2011-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: [fortran] Add support for #pragma GCC unroll v3
On Sat, Nov 25, 2017 at 11:21:49AM +0100, Eric Botcazou wrote: > > this is the (hopefully) final implementation of the support for the unrolling > pragma in the Fortran front-end. However the documentation is still missing > because I don't really know where and under which form to put it. > > Tested on x86_64-suse-linux, OK for the mainline? > Eric, The patch looks ok to me. For documentation, the gfortran manual has 2 sections: 6.1 Extensions implemented in GNU Fortran 7.2 GNU Fortran Compiler Directives 6.1 describes extension covering legacy code and vendor extensions. 7.2 describes other !$GCC directives. Currently, the section is mainly calling conventions (CDECL, STDCALL, etc) and library macroc (DLLEXPORT). These should probably be in 7.2.1 and the UNROLL directive in 7.2.2. I can help with the documentation (although it might take a weekend or two to get done), but need to know sematics. Does the directive apply to only the immediately following loop? Does it apply to all loops that follow the directive? What is the interaction of the directive with -funroll-loops and --param max-unroll-times=4? -- Steve
[wwwdocs] Adjust a comment in bin/preprocess
I've had this in a local tree since February; better to flush local changes. This just adjust a comment to match the code wrt. ordering of steps. Applied, and I updated the instance on gcc.gnu.org as well. Gerald Index: bin/preprocess === RCS file: /cvs/gcc/wwwdocs/bin/preprocess,v retrieving revision 1.51 diff -u -r1.51 preprocess --- bin/preprocess 25 May 2016 20:35:09 - 1.51 +++ bin/preprocess 25 Nov 2017 18:45:12 - @@ -119,9 +119,9 @@ exit 1 fi -# Then remove leading blank lines and single line comments. # Use sed to work around makeinfo 4.7 brokenness. # Use sed to work around MetaHTML brokenness wrt. . +# Then remove leading blank lines and single line comments. cat $TMPDIR/output.raw \ | sed -e 's/_002d/-/g' -e 's/_002a/*/g' \ | sed -e 's/
Fix -Wreturn-type fallout
Installed as obvious.
Andreas.
* g++.dg/abi/structret1.C (FrameworkObject::action): Return a
value.
Index: g++.dg/abi/structret1.C
===
--- g++.dg/abi/structret1.C (revision 255148)
+++ g++.dg/abi/structret1.C (working copy)
@@ -20,6 +20,7 @@
if (this != r33) {
abort ();
}
+ return ConstructedObject();
}
int main()
--
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."
[wwwdocs] Shorten bugs/index.html a little
Just having pointed someone to our bug reporting instructions, I had a look myself and noticed that they are a little unwieldy . This is a first step of shortening the text a little. Help and contributions very welcome! Gerald Index: index.html === RCS file: /cvs/gcc/wwwdocs/htdocs/bugs/index.html,v retrieving revision 1.119 diff -u -r1.119 index.html --- index.html 19 Mar 2017 07:50:55 - 1.119 +++ index.html 25 Nov 2017 19:00:10 - @@ -38,9 +38,8 @@ Reporting Bugs -The main purpose of a bug report is to enable us to fix the bug. The -most important prerequisite for this is that the report must be complete -and self-contained. +A good bug report, which is complete and self-contained, enables us +to fix the bug. Before you report a bug, please check the list of well-known bugs and, if possible, @@ -56,13 +55,12 @@ Summarized bug reporting instructions -After this summary, you'll find detailed bug reporting -instructions, that explain how to obtain some of the information -requested in this summary. +After this summary, you'll find detailed instructions that explain +how to obtain some of the information requested in this summary. What we need -Please include in your bug report all of the following items, the first +Please include all of the following items, the first three of which can be obtained from the output of gcc -v:
Re: [PATCH] Fix combine's simplify_if_then_else (PR rtl-optimization/81553)
On Sat, Nov 25, 2017 at 09:39:54AM +0100, Jakub Jelinek wrote: > > Okay, thanks! Is this better than bailing out though, do you have > > an example? > > I don't, but I haven't bootstrapped/regtested with a patch to gather > statistics on whether it would ever be successful (how? Should I note in a > flag successful optimization of this and then check that flag upon > successful try_combine and clear it at the end of any try_combine? It would > need to be done for all targets probably). This code dates from 1994, but > testsuite was separate, so it is hard to check if this was covered by any > testcases. > > If C1 is a constant (but then it would likely have VOIDmode), then I can > imagine it would match, though hard to find testcases, because usually > GIMPLE folding should propagate constants far earlier. If C1 is a pseudo, > then we'd need an instruction that has > (any_shift (something) (mult (something) (something))) > which is quite unlikely. But it is similarly unlikely for > (ior (something) (mult (something) (something))) and similar. const_int is always VOIDmode yes. So we're left with a shift of a mul of pseudos, and when that is split it is extremely likely split on a boundary that was already there when we began, so I don't see it likely it helps. I thought you might have hit on an example where it does :-) The patch is fine either way. > Or can try_combine then split it into two separate instructions, one doing > the mult and one doing the other, say when combining from original 3 or 4 > instructions? If so, then it could be likely it hits. Yes: it will use a define_split if there is one, or else it will find what it thinks is the best spot to split, and try that. That should usually split the mul off, and you usually will have started with a mul in a separate insn, so says this was i1+i2+i3 with i3 the mul, in effect this is then just combining i1+i2. Sometimes of course that will then work, where combining just i1+i2 won't (because of known register values, or REG_DEAD notes, or similar). Segher
Re: RFA (hash-map): PATCH to support GTY((cache)) with hash_map
On 2017.11.14 at 13:32 +0100, Richard Biener wrote: > On Fri, Sep 15, 2017 at 11:45 PM, Jason Merrill wrote: > > The hash_map interface is a lot more convenient than that of > > hash_table for cases where it makes sense, but there hasn't been a way > > to get the ggc_cache_remove behavior with a hash_map. In other words, > > not marking elements during the initial ggc marking phase, but maybe > > marking them during the clear_caches phase based on keep_cache_entry. > > > > This patch implements that by: > > > > Adding a ggc_maybe_mx member function to ggc_remove, and overriding > > that instead of ggc_mx in ggc_cache_remove. > > Calling H::ggc_maybe_mx instead of H::ggc_mx in gt_ggc_mx (hash_table *). > > Calling H::ggc_mx in gt_cleare_caches (hash_table *) rather than > > relying on an extern declaration of a plain function that cannot be > > declared for hash_map::hash_entry. > > Adding ggc_maybe_mx and keep_cache_entry to hash_map::hash_entry. > > Adding gt_cleare_cache for hash_map. > > Adding a boolean constant to the hash-map traits indicating whether we > > want the cache behavior above. > > > > I then define a typedef tree_cache_map to use this functionality, and > > use it in a few places in the C++ front end. > > > > Tested x86_64-pc-linux-gnu, OK for trunk? > > Ok. The patch causes UB in some cases: gcc/hash-map.h:277:19: runtime error: member access within null pointer of type 'struct hash_map' -- Markus
Re: [RFTesting] New POINTER_DIFF_EXPR
On Mon, 20 Nov 2017, Marc Glisse wrote: new version, regtested on powerpc64le-unknown-linux-gnu. The front-end parts are up for review. 2017-10-28 Marc Glisse gcc/c/ * c-fold.c (c_fully_fold_internal): Handle POINTER_DIFF_EXPR. * c-typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. gcc/c-family/ * c-pretty-print.c (pp_c_additive_expression, c_pretty_printer::expression): Handle POINTER_DIFF_EXPR. gcc/cp/ * constexpr.c (cxx_eval_constant_expression, potential_constant_expression_1): Handle POINTER_DIFF_EXPR. * cp-gimplify.c (cp_fold): Likewise. * error.c (dump_expr): Likewise. * typeck.c (pointer_diff): Use POINTER_DIFF_EXPR. I'm afraid this broke building Wine. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83164 ddeml.c: In function ‘DDEML_AddThunk’: ddeml.c:181:28: error: type mismatch in pointer diff expression static struct ddeml_thunk* DDEML_AddThunk(DWORD instId, DWORD pfn16) ^~ int struct HDDEDATA__ * (*) (DWORD, UINT, UINT, struct HCONV__ *, struct HSZ__ *, struct HSZ__ *, struct HDDEDATA__ *, ULONG_PTR, ULONG_PTR) DWORD * _6 = WDML_InvokeCallback16 - _5; ddeml.c:181:28: internal compiler error: verify_gimple failed David, in addition to the ICE diagnostics also looks, umm, rather confusing? Gerald
Re: [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275)
On Sat, Nov 25, 2017 at 10:01:22AM +0100, Jakub Jelinek wrote:
> Actually, thinking about it some more, maybe it would be more efficient
> to gather this information during construction of the SWITCH_STMT in some
> new flag on the tree, so cxx_block_may_fallthru would just:
Here it is implemented, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?
2017-11-25 Jakub Jelinek
PR sanitizer/81275
* cp-tree.h (SWITCH_STMT_CANNOT_FALLTHRU_P): Define.
(note_break_stmt, note_iteration_stmt_body_start,
note_iteration_stmt_body_end): Declare.
* decl.c (struct cp_switch): Add has_default_p, break_stmt_seen_p
and in_loop_body_p fields.
(push_switch): Clear them.
(pop_switch): Set SWITCH_STMT_CANNOT_FALLTHRU_P if has_default_p
and !break_stmt_seen_p. Assert in_loop_body_p is false.
(note_break_stmt, note_iteration_stmt_body_start,
note_iteration_stmt_body_end): New functions.
(finish_case_label): Set has_default_p when both low and high
are NULL_TREE.
* parser.c (cp_parser_iteration_statement): Use
note_iteration_stmt_body_start and note_iteration_stmt_body_end
around parsing iteration body.
* pt.c (tsubst_expr): Likewise.
* cp-objcp-common.c (cxx_block_may_fallthru): Return false for
SWITCH_STMT which contains no BREAK_STMTs, contains a default:
CASE_LABEL_EXPR and where SWITCH_STMT_BODY isn't empty and
can't fallthru.
* semantics.c (finish_break_stmt): Call note_break_stmt.
* g++.dg/warn/pr81275-1.C: New test.
* g++.dg/warn/pr81275-2.C: New test.
* g++.dg/warn/pr81275-3.C: New test.
--- gcc/cp/cp-tree.h.jj 2017-11-17 08:40:32.0 +0100
+++ gcc/cp/cp-tree.h2017-11-25 21:25:48.277897180 +0100
@@ -364,6 +364,7 @@ extern GTY(()) tree cp_global_trees[CPTI
IF_STMT_CONSTEXPR_P (IF_STMT)
TEMPLATE_TYPE_PARM_FOR_CLASS (TEMPLATE_TYPE_PARM)
DECL_NAMESPACE_INLINE_P (in NAMESPACE_DECL)
+ SWITCH_STMT_CANNOT_FALLTHRU_P (in SWITCH_STMT)
1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE)
TI_PENDING_TEMPLATE_FLAG.
TEMPLATE_PARMS_FOR_INLINE.
@@ -4840,6 +4841,12 @@ more_aggr_init_expr_args_p (const aggr_i
#define SWITCH_STMT_BODY(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 1)
#define SWITCH_STMT_TYPE(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 2)
#define SWITCH_STMT_SCOPE(NODE)TREE_OPERAND (SWITCH_STMT_CHECK (NODE),
3)
+/* Set if the body of a switch stmt contains a default: case label
+ and does not contain any break; stmts, thus if SWITCH_STMT_BODY
+ is not empty and doesn't fallthru, then the whole SWITCH_STMT
+ can't fallthru either. */
+#define SWITCH_STMT_CANNOT_FALLTHRU_P(NODE) \
+ TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE))
/* STMT_EXPR accessor. */
#define STMT_EXPR_STMT(NODE) TREE_OPERAND (STMT_EXPR_CHECK (NODE), 0)
@@ -6102,6 +6109,9 @@ enum cp_tree_node_structure_enum cp_tree
extern void finish_scope (void);
extern void push_switch(tree);
extern void pop_switch (void);
+extern void note_break_stmt(void);
+extern bool note_iteration_stmt_body_start (void);
+extern void note_iteration_stmt_body_end (bool);
extern tree make_lambda_name (void);
extern int decls_match (tree, tree);
extern bool maybe_version_functions(tree, tree);
--- gcc/cp/decl.c.jj2017-11-22 21:37:46.0 +0100
+++ gcc/cp/decl.c 2017-11-25 21:25:26.306162437 +0100
@@ -3427,6 +3427,13 @@ struct cp_switch
/* Remember whether there was a case value that is outside the
range of the original type of the controlling expression. */
bool outside_range_p;
+ /* Remember whether a default: case label has been seen. */
+ bool has_default_p;
+ /* Remember whether a BREAK_STMT has been seen in this SWITCH_STMT. */
+ bool break_stmt_seen_p;
+ /* Set if inside of {FOR,DO,WHILE}_BODY nested inside of a switch,
+ where BREAK_STMT doesn't belong to the SWITCH_STMT. */
+ bool in_loop_body_p;
};
/* A stack of the currently active switch statements. The innermost
@@ -3449,6 +3456,9 @@ push_switch (tree switch_stmt)
p->switch_stmt = switch_stmt;
p->cases = splay_tree_new (case_compare, NULL, NULL);
p->outside_range_p = false;
+ p->has_default_p = false;
+ p->break_stmt_seen_p = false;
+ p->in_loop_body_p = false;
switch_stack = p;
}
@@ -3469,11 +3479,50 @@ pop_switch (void)
SWITCH_STMT_COND (cs->switch_stmt),
bool_cond_p, cs->outside_range_p);
+ /* For the benefit of block_may_fallthru remember if the switch body
+ contains a default: case label and no break; stmts. */
+ if (cs->has_default_p && !cs->break_stmt_seen_p)
+SWITCH_STMT_CANNOT_FALLTHRU_P (cs->switch_stmt) = 1;
+ gcc_assert (!cs->i
Re: [PING 2][PATCH] enhance -Wrestrict to handle string built-ins (PR 78918)
On 11/22/2017 04:50 PM, Jeff Law wrote: On 11/16/2017 02:29 PM, Martin Sebor wrote: Ping. I've fixed the outstanding false positive exposed by the Linux kernel. The kernel builds with four instances of the warning, all of them valid (perfect overlap in memcpy). I also built Glibc. It shows one instance of the warning, also a true positive (cause by calling a restrict-qualified function with two copies of the same argument). Finally, I built Binutils and GDB with no warnings. The attached patch includes just that one fix, with everything else being the same. On 11/09/2017 04:57 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01642.html On 10/23/2017 08:42 PM, Martin Sebor wrote: Attached is a reworked solution to enhance -Wrestrict while avoiding changing tree-vrp.c or any other VRP machinery. Richard, in considering you suggestions I realized that the ao_ref struct isn't general enough to detect the kinds of problems I needed to etect (storing bit-offsets in HOST_WIDE_INT means out-of-bounds offsets cannot be represented or detected, leading to either false positives or false negatives). So this seems to be a recurring theme, which makes me wonder if we should have an ao_ref-like structure that deals in bytes rather than bits and make it a first class citizen. There's certainly clients that work on bits and certainly clients that would prefer to work on bytes. The class I introduced serves a different purpose than ao_ref and stores a lot more data. In https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82042#c3 Richard says that "this [offset being HOST_WIDE_INT and storing bits] is a know deficiency in ao_ref 'offset' (and also size and maxsize). Blowing up to offset_int isn't really a good idea." Instead, the solution adds a couple of small classes to builtins.c to overcome this limitation (I'm contemplating moving them along with -Wstringop-overflow to a separate file to keep builtins.c from getting too much bigger). The detection of out-of-bounds offsets and overlapping accesses is relatively simple but the rest of the changes are somewhat involved because of the computation of the offsets and sizes of the overlaps. Jeff, as per your suggestion/request in an earlier review (bug 81117) I've renamed some of the existing functions to better reflect their new function (including renaming strlen_optimize_stmt in tree-ssa-strlen.c to strlen_check_and_optimize_stmt). There's quite a bit of churn due to some of this renaming. I don't think this renaming makes the review too difficult but if you feel differently I can [be persuaded to] split it out into a separate patch. Understood. FWIW, one way to deal with this that I've found works reasonably well is to have an initial patch that just does the renaming you want to do. That separates the mechanical stuff from the meat of the change. They can be git squashed together just before committing or committed as two changes back-to-back to eliminate or minimize the time where the variable and function names are inconsistent. It's also the case that independent little fixes should just go upstream immediately. I didn't see many, but the alloc_max_size fix for KB seems like it should have just gone forward independent of the rest of the changes. Sure, I think that change was incidental, not intentional. There should be no others (AFAIK). To validate the patch I compiled the Linux kernel and Binutils/GDB. There's one false positive I'm working on resolving that's caused by an incorrect interpretation of an offset in a range whose lower bound is greater than its upper bound. This it so say that while I'm aware the patch isn't perfect it already works reasonably well in practice and I think it's close enough to review. Thanks Martin gcc-78918.diff PR tree-optimization/78918 - missing -Wrestrict on memcpy copying over self gcc/c-family/ChangeLog: PR tree-optimization/78918 * c-common.c (check_function_restrict): Avoid checking built-ins. * c.opt (-Wrestrict): Include in -Wall. gcc/ChangeLog: PR tree-optimization/78918 * builtins.c (builtin_memref, builtin_access): New classes. (check_bounds_or_overlap, maybe_diag_overlap): New static functions. (maybe_diag_offset_bounds): Same. (check_sizes): Rename... (check_access): ...to this. Rename function arguments for clarity. (check_memop_sizes): Adjust names. (expand_builtin_memchr, expand_builtin_memcpy): Same. (expand_builtin_memmove, expand_builtin_mempcpy): Same. (expand_builtin_strcat, expand_builtin_stpncpy): Same. (check_strncat_sizes, expand_builtin_strncat): Same. (expand_builtin_strncpy, expand_builtin_memset): Same. (expand_builtin_bzero, expand_builtin_memcmp): Same. (expand_builtin_memory_chk, maybe_emit_chk_warning): Same. (maybe_emit_sprintf_chk_warning): Same. (expand_builtin_strcpy): Adjus
