Re: Add value_range_base::contains_p
On Wed, Jun 12, 2019 at 11:33 PM Aldy Hernandez wrote:
>
> On 6/12/19 5:33 AM, Richard Biener wrote:
> > On Wed, Jun 12, 2019 at 1:57 AM Martin Sebor wrote:
> >>
> >> On 6/11/19 3:02 PM, Aldy Hernandez wrote:
> >>>
> >>>
> >>> On 6/11/19 12:52 PM, Martin Sebor wrote:
> On 6/11/19 10:26 AM, Aldy Hernandez wrote:
> >
> >
> > On 6/11/19 12:17 PM, Martin Sebor wrote:
> >> On 6/11/19 9:05 AM, Aldy Hernandez wrote:
> >>>
> >>>
> >>> On 6/11/19 9:45 AM, Richard Biener wrote:
> On Tue, Jun 11, 2019 at 12:40 PM Aldy Hernandez
> wrote:
> >
> > This patch cleans up the various contains, may_contain, and
> > value_inside_range variants we have throughout, in favor of one--
> > contains_p. There should be no changes in functionality.
> >
> > I have added a note to range_includes_zero_p, perhaps as a personal
> > question than anything else. This function was/is returning true
> > for
> > UNDEFINED. From a semantic sense, that doesn't make sense.
> > UNDEFINED
> > is really the empty set. Is the functionality wrong, or should
> > we call
> > this function something else? Either way, I'm fine removing the
> > comment
> > but I'm genuinely curious.
>
> So this affects for example this hunk:
>
> - if (vr && !range_includes_p (vr, 1))
> + if (vr && (!vr->contains_p (build_one_cst (TREE_TYPE (name)))
> +&& !vr->undefined_p ()))
> {
>
> I think it's arbitrary how we compute X in UNDEFINED and I'm fine
> with changing the affected predicates to return false. This means
> not testing for !vr->undefined_p here.
> >>>
> >>> Excellent.
> >>>
>
> Note I very much dislike the build_one_cst () call here so please
> provide an overload hiding this.
> >>>
> >>> Good idea. I love it.
> >>>
>
> Not sure why you keep range_includes_zero_p.
> >>>
> >>> I wasn't sure if there was some subtle reason why we were including
> >>> undefined.
> >>>
> >>> OK pending tests?
> >>
> >> Should the second overload:
> >>
> >> + bool contains_p (tree) const;
> >> + bool contains_p (int) const;
> >>
> >> take something like HOST_WIDE_INT or even one of those poly_ints
> >> like build_int_cst does? (With the former, contains_p (0) will
> >> likely be ambiguous since 0 is int and HOST_WIDE_INT is long).
> >
> > We have a type, so there should be no confusion:
> >
> > + return contains_p (build_int_cst (type (), val));
> >
> > (UNDEFINED and VARYING don't have a type, so they are special cased
> > prior).
>
> I didn't mean the overloads are confusing, just that there the one
> that takes an int doesn't make it possible to test whether a value
> outside the range of an int is in the range. For example, in
> the call
>
> contains_p (SIZE_MAX)
>
> the argument will get sliced (and trigger a -Woverflow). One will
> need to go back to the more verbose way of calling it.
> >>>
> >>> The int version is not really meant to pass anything but simple
> >>> constants. For anything fancy, one should really be using the tree
> >>> version. But I can certainly change the argument to HOST_WIDE_INT if
> >>> preferred.
> >>>
>
> Changing the argument type to HOST_WIDE_INT would avoid the slicing
> and warning but then make contains_p (0) ambiguous because 0 isn't
> a perfect match for either void* or long (so it requires a conversion).
> >>>
> >>> Just a plain 0 will match the int version, instead of the tree version,
> >>> right? Nobody should be passing NULL to the tree version, so that seems
> >>> like a non-issue.
> >>
> >> Right, NULL isn't a problem, but I would expect any integer to work
> >> (I thought that's what Richard was asking for) So my suggestion was
> >> to have contains_p() a poly_int64 and provide just as robust an API
> >> as build_int_cst. The argument ends up converted to the poly_int64
> >> anyway when it's passed to the latter. I.e., why not define
> >> contains_p simply like this:
> >>
> >> bool
> >> value_range_base::contains_p (poly_int64 val) const
> >> {
> >> if (varying_p ())
> >> return true;
> >> if (undefined_p ())
> >> return false;
> >>
> >> return contains_p (build_int_cst (type (), val));
> >> }
> >
> > I agree that plain 'int' is bad. Given we currently store
> > INTEGER_CSTs only (and not POLY_INT_CSTs) a
> > widest_int argument should be fine. Note widest
> > because when interpreted signed all unsigned values
> > fit.
> >
> > The existing contains_p check is also a bit fishy
> > for the cases TREE_TYPE of the value has a
> > value-range not
[PATCH] Make static vars inside of target regions or declare target routines implicitly declare target to (PR middle-end/90779)
Hi!
The OpenMP specification isn't clear on this, I'll work on getting that
clarified for 5.1, but the agreement on omp-lang has been that it should
work the way the patch implements it, static block scope variables inside of
#pragma omp target or #pragma omp declare target routines are handled as if
they have #pragma omp declare target to (variable).
Bootstrapped/regtested on x86_64-linux and i686-linux, unfortunately it
regresses:
+FAIL: c-c++-common/goacc/routine-5.c (test for errors, line 204)
Thus, I'm not committing it right now and want to ask what should be done
for OpenACC. The patch uses & ORT_TARGET tests, so it affects both OpenMP
target region, and OpenACC parallel/kernels and both OpenMP and OpenACC
target routines. Is it ok to do it that way and just adjust the routine-5.c
test, or shall it test (ctx->region_type & (ORT_TARGET | ORT_ACC)) ==
ORT_TARGET, i.e. only OpenMP and not OpenACC? If so, there is still the
problem that gimplify_body.c does:
if (flag_openacc || flag_openmp)
{
gcc_assert (gimplify_omp_ctxp == NULL);
if (lookup_attribute ("omp declare target", DECL_ATTRIBUTES (fndecl)))
gimplify_omp_ctxp = new_omp_context (ORT_TARGET);
}
We'd need different attribute (or additional attribute) for OpenACC routines
and would need to use new_omp_context (cond ? ORT_TARGET : ORT_ACC_PARALLEL)
or similar to express OpenACC routines.
2019-06-12 Jakub Jelinek
PR middle-end/90779
* gimplify.c (gimplify_bind_expr): Add "omp declare target" attributes
to static block scope variables inside of target region or target
functions.
* testsuite/libgomp.c/pr90779.c: New test.
* testsuite/libgomp.fortran/pr90779.f90: New test.
--- gcc/gimplify.c.jj 2019-06-10 19:42:03.868959986 +0200
+++ gcc/gimplify.c 2019-06-12 13:00:18.76516 +0200
@@ -1323,17 +1323,37 @@ gimplify_bind_expr (tree *expr_p, gimple
struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp;
/* Mark variable as local. */
- if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t)
- && (! DECL_SEEN_IN_BIND_EXPR_P (t)
- || splay_tree_lookup (ctx->variables,
- (splay_tree_key) t) == NULL))
+ if (ctx && ctx->region_type != ORT_NONE && !DECL_EXTERNAL (t))
{
- if (ctx->region_type == ORT_SIMD
- && TREE_ADDRESSABLE (t)
- && !TREE_STATIC (t))
- omp_add_variable (ctx, t, GOVD_PRIVATE | GOVD_SEEN);
- else
- omp_add_variable (ctx, t, GOVD_LOCAL | GOVD_SEEN);
+ if (! DECL_SEEN_IN_BIND_EXPR_P (t)
+ || splay_tree_lookup (ctx->variables,
+ (splay_tree_key) t) == NULL)
+ {
+ if (ctx->region_type == ORT_SIMD
+ && TREE_ADDRESSABLE (t)
+ && !TREE_STATIC (t))
+ omp_add_variable (ctx, t, GOVD_PRIVATE | GOVD_SEEN);
+ else
+ omp_add_variable (ctx, t, GOVD_LOCAL | GOVD_SEEN);
+ }
+ /* Static locals inside of target construct or offloaded
+routines need to be "omp declare target". */
+ if (TREE_STATIC (t))
+ for (; ctx; ctx = ctx->outer_context)
+ if ((ctx->region_type & ORT_TARGET) != 0)
+ {
+ if (!lookup_attribute ("omp declare target",
+DECL_ATTRIBUTES (t)))
+ {
+ tree id = get_identifier ("omp declare target");
+ DECL_ATTRIBUTES (t)
+ = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (t));
+ varpool_node *node = varpool_node::get (t);
+ if (node)
+ node->offloadable = 1;
+ }
+ break;
+ }
}
DECL_SEEN_IN_BIND_EXPR_P (t) = 1;
--- libgomp/testsuite/libgomp.c/pr90779.c.jj2019-06-12 13:01:57.081667587
+0200
+++ libgomp/testsuite/libgomp.c/pr90779.c 2019-06-12 12:41:15.637730797
+0200
@@ -0,0 +1,18 @@
+/* PR middle-end/90779 */
+
+extern void abort (void);
+
+int
+main ()
+{
+ int i, j;
+ for (i = 0; i < 2; ++i)
+#pragma omp target map(from: j)
+{
+ static int k = 5;
+ j = ++k;
+}
+ if (j != 7)
+abort ();
+ return 0;
+}
--- libgomp/testsuite/libgomp.fortran/pr90779.f90.jj2019-06-12
12:43:17.891825811 +0200
+++ libgomp/testsuite/libgomp.fortran/pr90779.f90 2019-06-12
12:43:08.421973375 +0200
@@ -0,0 +1,12 @@
+! PR middle-end/90779
+
+program pr90779
+ implicit none
+ integer :: v(4), i
+
+ !$omp target map(from:v)
+v(:) = (/ (i, i=1,4) /)
+ !$omp end target
+
+ if (any (v .ne. (/ (i, i=1,4) /))) stop 1
+end program
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
Hi Martin,
>> The testsuite part cannot have been tested very thoroughly: for one,
>> this snippet
>
> Please try r272218.
this cured a couple of issues, like this one:
>> ERROR: (DejaGnu) proc "4" does not exist.
However...
>> Even with that fixed, I see many failures:
>>
>> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
>> "MEM[.*&i][j.*] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
>> "MEM[.*&i][j.*] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
>> "MEM[.*&i][j.*] =.* 1;"
>> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
>> [(struct FixBuf *)& +
>> [0-9]+B] = {}" 1
>>
>> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
these failures remain...
>> +FAIL: g++.dg/tree-ssa/pr19807.C -std=gnu++14 scan-tree-dump-times
>> optimized "&MEM[(void .)&a + 8B]" 3
>> +FAIL: g++.dg/tree-ssa/pr19807.C -std=gnu++17 scan-tree-dump-times
>> optimized "&MEM[(void .)&a + 8B]" 3
>> +FAIL: g++.dg/tree-ssa/pr19807.C -std=gnu++98 scan-tree-dump-times
>> optimized "&MEM[(void .)&a + 8B]" 3
as does this one...
>> +FAIL: gcc.dg/tree-prof/stringop-2.c scan-tree-dump optimized
>> "MEM[(void .)&a] = 168430090"
and this one...
>> +FAIL: gcc.dg/tree-ssa/pr30375.c scan-tree-dump-times dse1
>> "MEM[(struct _s *)&signInfo + [0-9]+B] = {}" 1
>> +FAIL: gcc.dg/tree-ssa/slsr-27.c scan-tree-dump-times dom3
>> "MEM[(struct x *)[^\\r\\n]*_d+" 3
>> +FAIL: gcc.dg/tree-ssa/slsr-28.c scan-tree-dump-times dom3
>> "MEM[(struct x *)[^\\r\\n]*_d+" 9
>> +FAIL: gcc.dg/tree-ssa/slsr-29.c scan-tree-dump-times dom3
>> "MEM[(struct x *)[^\\r\\n]*_d+" 9
>> +FAIL: gcc.dg/tree-ssa/ssa-dse-24.c scan-tree-dump-times dse1
>> "MEM[(struct printf_info *)&info + [0-9]+B] = {}" 1
>>
>> on 32 and 64-bit sparc-sun-solaris2.11 (the first of those also on
>> m68k-unknown-linux-gnu).
while this group is gone now.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] [ARC][DOC] Add documentation naked, ilink and firq
Thank you for your review,
Claudiu
On Wed, Jun 12, 2019 at 12:00 AM Andrew Burgess
wrote:
>
> * Claudiu Zissulescu [2019-06-06 10:37:09 +0300]:
>
> > Hi Sandra, Andrew
> >
> > Please find a small patch on the documentation which adds info about naked,
> > ilink and firq function attributes.
> >
> > Ok to apply?
> > Claudiu
> >
> > gcc/
> > -xx-xx Claudiu Zissulescu
> >
> > * doc/extend.texi (ARC Function Attributes): Update info.
>
> This change looks good to me.
>
> Thanks,
> Andrew
>
>
> > ---
> > gcc/doc/extend.texi | 14 +-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index e8563fd0803..c2e675afa0f 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -4139,7 +4139,8 @@ void f () __attribute__ ((interrupt ("ilink1")));
> > @end smallexample
> >
> > Permissible values for this parameter are: @w{@code{ilink1}} and
> > -@w{@code{ilink2}}.
> > +@w{@code{ilink2}} for ARCv1 architecture, and @w{@code{ilink}} and
> > +@w{@code{firq}} for ARCv2 architecture.
> >
> > @item long_call
> > @itemx medium_call
> > @@ -4182,7 +4183,17 @@ This attribute allows one to mark secure-code
> > functions that are
> > callable from normal mode. The location of the secure call function
> > into the @code{sjli} table needs to be passed as argument.
> >
> > +@item naked
> > +@cindex @code{naked} function attribute, ARC
> > +This attribute allows the compiler to construct the requisite function
> > +declaration, while allowing the body of the function to be assembly
> > +code. The specified function will not have prologue/epilogue sequences
> > +generated by the compiler. Only basic @code{asm} statements can safely
> > +be included in naked functions (@pxref{Basic Asm}). While using
> > +extended @code{asm} or a mixture of basic @code{asm} and C code may
> > +appear to work, they cannot be depended upon to work reliably and are
> > +not supported.
> > +
> > @end table
> >
> > @node ARM Function Attributes
> > --
> > 2.21.0
> >
[PATCH] Make the vectorizer version outer loops
This builds upon the previously posted patch to re-use if-conversion
versioning versions from vect_loop_versioning. The patch is actually
combined, it doesn't seem worth to split after all.
The patch computes the outermost loop the combined versioning
condition is invariant in and applies versioning to that loop,
reducing the overall runtime cost of versioning with the cost
of extra code-size from versioning a deeper loop nest.
The versioning is restricted to operate on a perfect nest
with loop versions created by if-conversion considered
"perfect" and to loops with single exits. Whenever possible
the versioning is elided when if-conversion already created
a suitable one.
I've tested the patch on SPEC CPU 2006 and 2017 where from
3254 versioned loops in 2006, 109 of them are changed to operate
on an outer loop and 132 of them avoided creating another version.
>From 3704 versioned loops in 2017 speed, 71 of them are changed
to operate on an outer loop and 368 avoided creating another version.
I have not (yet) done any performance measurements (autotesters
will show but I don't expect any big effects). I have not looked
as to whether there are simple improvements possible to make
more versioning conditions invariant.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
Richard.
* tree-vectorizer.h (vect_loop_vectorized_call): Declare.
* tree-vectorizer.c (vect_loop_vectorized_call): Export and
also return the condition stmt.
* tree-vect-loop-manip.c (vect_loop_versioning): Compute outermost
loop we can version and version that, reusing the loop version
created by if-conversion instead of versioning again.
* gcc.dg/vect/vect-version-1.c: New testcase.
* gcc.dg/vect/vect-version-2.c: Likewise.
diff --git a/gcc/testsuite/gcc.dg/vect/vect-version-1.c
b/gcc/testsuite/gcc.dg/vect/vect-version-1.c
new file mode 100644
index 000..4540a11b36c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-version-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_double } */
+/* { dg-require-effective-target vect_condition } */
+
+void foo (double *x, double *y, int m, int n, int o, int p)
+{
+ for (int i = 0; i < m; ++i)
+for (int j = 0; j < n; ++j)
+ for (int k = 0; k < o; ++k)
+ for (int l = 0; l < p; ++l)
+ {
+ double tem = x[l] + y[l];
+ if (tem != 0.)
+ y[l] = x[l];
+ else
+ y[l] = 0.;
+ }
+}
+
+/* { dg-final { scan-tree-dump "applying loop versioning to outer loop 1"
"vect" } } */
diff --git a/gcc/testsuite/gcc.dg/vect/vect-version-2.c
b/gcc/testsuite/gcc.dg/vect/vect-version-2.c
new file mode 100644
index 000..0ea39e31801
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-version-2.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_double } */
+/* { dg-require-effective-target vect_condition } */
+
+void foo (double *x, double *y, int m, int n, int o, int p)
+{
+ for (int i = 0; i < m; ++i)
+for (int j = 0; j < n; ++j)
+ for (int k = 0; k < o; ++k)
+ for (int l = 0; l < k; ++l)
+ {
+ double tem = x[l] + y[l];
+ if (tem != 0.)
+ y[l] = x[l];
+ else
+ y[l] = 0.;
+ }
+}
+
+/* { dg-final { scan-tree-dump "reusing loop version created by if conversion"
"vect" } } */
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index b3fae5ba4da..83be8c316e8 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -3032,7 +3032,8 @@ vect_loop_versioning (loop_vec_info loop_vinfo,
vect_create_cond_for_niters_checks (loop_vinfo, &cond_expr);
if (cond_expr)
-cond_expr = force_gimple_operand_1 (cond_expr, &cond_expr_stmt_list,
+cond_expr = force_gimple_operand_1 (unshare_expr (cond_expr),
+ &cond_expr_stmt_list,
is_gimple_condexpr, NULL_TREE);
if (version_align)
@@ -3076,45 +3077,136 @@ vect_loop_versioning (loop_vec_info loop_vinfo,
is_gimple_condexpr, NULL_TREE);
gimple_seq_add_seq (&cond_expr_stmt_list, gimplify_stmt_list);
- initialize_original_copy_tables ();
- if (scalar_loop)
+ /* Compute the outermost loop cond_expr and cond_expr_stmt_list are
+ invariant in. */
+ struct loop *outermost = outermost_invariant_loop_for_expr (loop, cond_expr);
+ for (gimple_stmt_iterator gsi = gsi_start (cond_expr_stmt_list);
+ !gsi_end_p (gsi); gsi_next (&gsi))
{
- edge scalar_e;
- basic_block preheader, scalar_preheader;
+ gimple *stmt = gsi_stmt (gsi);
+ update_stmt (stmt);
+ ssa_op_iter iter;
+ use_operand_p use_p;
+ basic_block def_bb;
+ FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_USE)
+ if ((def_bb = gimple_bb (SSA_NAME_DEF_STMT (USE_FROM_PTR (use_p
Results for 10.0.0 20190612 (experimental) [trunk revision 272202] (GCC) testsuite on x86_64-apple-darwin18
LAST_UPDATED: Wed Jun 12 19:12:53 UTC 2019 (revision 272202) === acats tests === === acats Summary === # of expected passes2320 # of unexpected failures0 Native configuration is x86_64-apple-darwin18 === g++ tests === Running target unix FAIL: g++.dg/ipa/pr67056.C scan-ipa-dump cp "Speculative outer type:struct CompositeClass" FAIL: g++.dg/pr80481.C -std=gnu++14 scan-assembler-not vmovaps FAIL: g++.dg/pr80481.C -std=gnu++17 scan-assembler-not vmovaps FAIL: g++.dg/pr80481.C -std=gnu++98 scan-assembler-not vmovaps FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1 "MEM[.*&i][j.*] =.* 1;" FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1 "MEM[.*&i][j.*] =.* 1;" FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1 "MEM[.*&i][j.*] =.* 1;" FAIL: g++.dg/gcov/pr16855.C -std=gnu++14 gcov: 2 failures in line counts, 0 in branch percentages, 0 in return percentages, 0 in intermediate format FAIL: g++.dg/gcov/pr16855.C -std=gnu++14 line 21: is #:should be 1 FAIL: g++.dg/gcov/pr16855.C -std=gnu++14 line 45: is #:should be 1 FAIL: g++.dg/gcov/pr16855.C -std=gnu++17 gcov: 2 failures in line counts, 0 in branch percentages, 0 in return percentages, 0 in intermediate format FAIL: g++.dg/gcov/pr16855.C -std=gnu++17 line 21: is #:should be 1 FAIL: g++.dg/gcov/pr16855.C -std=gnu++17 line 45: is #:should be 1 FAIL: g++.dg/gcov/pr16855.C -std=gnu++98 gcov: 2 failures in line counts, 0 in branch percentages, 0 in return percentages, 0 in intermediate format FAIL: g++.dg/gcov/pr16855.C -std=gnu++98 line 21: is #:should be 1 FAIL: g++.dg/gcov/pr16855.C -std=gnu++98 line 45: is #:should be 1 FAIL: g++.dg/lto/alias-1 cp_lto_alias-1_0.o-cp_lto_alias-1_1.o execute -O2 -flto FAIL: g++.dg/lto/pr65276 cp_lto_pr65276_0.o-cp_lto_pr65276_1.o link, -flto -O0 -std=c++11 FAIL: g++.dg/lto/pr87906 cp_lto_pr87906_0.o-cp_lto_pr87906_1.o link, -O -fPIC -flto FAIL: g++.dg/lto/pr89335 cp_lto_pr89335_0.o assemble, -O2 -flto -Wsuggest-final-methods FAIL: g++.dg/pch/pch.C -g -I. -Dwith_PCH (test for excess errors) FAIL: g++.dg/pch/pch.C -g assembly comparison FAIL: g++.old-deja/g++.law/profile1.C -std=gnu++14 execution test FAIL: g++.old-deja/g++.law/profile1.C -std=gnu++17 execution test FAIL: g++.old-deja/g++.law/profile1.C -std=gnu++98 execution test === g++ Summary === # of expected passes131136 # of unexpected failures25 # of expected failures 534 # of unsupported tests 6405 /Volumes/Data/10-14-moj/gcc-trunk-unpatched/gcc/testsuite/g++/../../xg++ version 10.0.0 20190612 (experimental) [trunk revision 272202] (GCC) === gcc tests === Running target unix FAIL: c-c++-common/asan/strncpy-overflow-1.c -O0 output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -O1 (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -O1 output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 -flto (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 -flto output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 -flto -flto-partition=none (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -O2 -flto -flto-partition=none output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -O3 -g (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -O3 -g output pattern test FAIL: c-c++-common/asan/strncpy-overflow-1.c -Os (test for excess errors) FAIL: c-c++-common/asan/strncpy-overflow-1.c -Os output pattern test FAIL: gcc.dg/cpp/isysroot-1.c (test for excess errors) FAIL: gcc.dg/debug/dwarf2/inline4.c scan-assembler DW_TAG_inlined_subroutine[^(]*([^)]*)[^(]*(DIE (0x[0-9a-f]*) DW_TAG_formal_parameter[^(]*(DIE (0x[0-9a-f]*) DW_TAG_variable FAIL: gcc.dg/debug/dwarf2/inline5.c scan-assembler-not (DIE (0x([0-9a-f]*)) DW_TAG_lexical_block)[^#/!@;|]*[#/!@;|]+ +[^(].*DW_TAG_lexical_block)[^#/!@;|x]*x1[^#/!@;|]*[#/!@;|] +DW_AT_abstract_origin FAIL: gcc.dg/Wnonnull.c (test for warnings, line 21) FAIL: gcc.dg/Wnonnull.c (test for warnings, line 24) FAIL: gcc.dg/aru-2.c execution test FAIL: gcc.dg/darwin-minversion-1.c (test for excess errors) UNRESOLVED: gcc.dg/darwin-minversion-1.c compilation failed to produce executable FAIL: gcc.dg/darwin-minversion-2.c (test for excess errors) UNRESOLVED: gcc.dg/darwin-minversion-2.c compilation failed to produce executable FAIL: gcc.dg/framework-1.c (test for excess errors) FAIL: gcc.dg/nest.c execution test FAIL: gcc.dg/nested-func-4.c
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
> >> Even with that fixed, I see many failures:
> >>
> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
> >> "MEM[.*&i][j.*] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
> >> "MEM[.*&i][j.*] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
> >> "MEM[.*&i][j.*] =.* 1;"
> >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
> >> [(struct FixBuf *)& +
> >> [0-9]+B] = {}" 1
> >>
> >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
>
> these failures remain...
On i686-linux I can reproduce just the above ones.
The following should fix it. As we don't match exact offset on the MEM
because it varries between different architectures (24 bytes on with -m64,
28 bytes with -m32), we shouldn't match the store size either, as it is
200 - that offset, so 176 or 172 etc.
Tested on x86_64-linux, -m32/-m64, ok for trunk?
2019-06-13 Jakub Jelinek
* g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
= {} store.
* g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
-fdump-tree-forwprop1 in dg-options. Expect in MEM.
--- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj2019-06-13
00:35:49.654840275 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C 2019-06-13 12:40:14.492568336
+0200
@@ -98,4 +98,4 @@ int main()
/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)&
\\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
- { dg-final { scan-tree-dump-times "MEM \\\[\\(struct FixBuf
\\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } }
*/
+ { dg-final { scan-tree-dump-times "MEM \\\[\\(struct
FixBuf \\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge
} } } } */
--- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj 2015-05-29 15:04:33.039803414
+0200
+++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C 2019-06-13 12:24:15.895576933
+0200
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O -fdump-tree-forwprop" } */
+/* { dg-options "-O -fdump-tree-forwprop1" } */
/* We should be able to optimize this to i[j] = 1 during
early optimizations. */
@@ -12,4 +12,4 @@ void foo (int j)
*q = 1;
}
-/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1"
} } */
+/* { dg-final { scan-tree-dump "MEM \\\[.*&i\\\]\\\[j.*\\\] =.*
1;" "forwprop1" } } */
Jakub
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On Thu, Jun 13, 2019 at 12:45 PM Jakub Jelinek wrote:
>
> On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
> > >> Even with that fixed, I see many failures:
> > >>
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
> > >> "MEM[.*&i][j.*] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
> > >> "MEM[.*&i][j.*] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
> > >> "MEM[.*&i][j.*] =.* 1;"
> > >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
> > >> [(struct FixBuf *)& +
> > >> [0-9]+B] = {}" 1
> > >>
> > >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
> >
> > these failures remain...
>
> On i686-linux I can reproduce just the above ones.
> The following should fix it. As we don't match exact offset on the MEM
> because it varries between different architectures (24 bytes on with -m64,
> 28 bytes with -m32), we shouldn't match the store size either, as it is
> 200 - that offset, so 176 or 172 etc.
>
> Tested on x86_64-linux, -m32/-m64, ok for trunk?
OK.
Richard.
> 2019-06-13 Jakub Jelinek
>
> * g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
> = {} store.
> * g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
> -fdump-tree-forwprop1 in dg-options. Expect in MEM.
>
> --- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj2019-06-13
> 00:35:49.654840275 +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C 2019-06-13 12:40:14.492568336
> +0200
> @@ -98,4 +98,4 @@ int main()
>
>
> /* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf
> \\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } }
> } }
> - { dg-final { scan-tree-dump-times "MEM \\\[\\(struct
> FixBuf \\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target {
> store_merge } } } } */
> + { dg-final { scan-tree-dump-times "MEM \\\[\\(struct
> FixBuf \\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target {
> store_merge } } } } */
> --- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj 2015-05-29 15:04:33.039803414
> +0200
> +++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C 2019-06-13 12:24:15.895576933
> +0200
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-forwprop" } */
> +/* { dg-options "-O -fdump-tree-forwprop1" } */
>
> /* We should be able to optimize this to i[j] = 1 during
> early optimizations. */
> @@ -12,4 +12,4 @@ void foo (int j)
>*q = 1;
> }
>
> -/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;"
> "forwprop1" } } */
> +/* { dg-final { scan-tree-dump "MEM \\\[.*&i\\\]\\\[j.*\\\]
> =.* 1;" "forwprop1" } } */
>
>
> Jakub
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
Hi Jakub,
> On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
>> >> Even with that fixed, I see many failures:
>> >>
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
>> >> "MEM[.*&i][j.*] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
>> >> "MEM[.*&i][j.*] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
>> >> "MEM[.*&i][j.*] =.* 1;"
>> >> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
>> >> [(struct FixBuf *)& +
>> >> [0-9]+B] = {}" 1
>> >>
>> >> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
>>
>> these failures remain...
>
> On i686-linux I can reproduce just the above ones.
right: I'm seeing the rest on sparc-sun-solaris2.11 only (and some of
those also on a couple of other targets according to gcc-testresults).
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH V4] Find constant definition for by-ref argument using dominance information (PR ipa/90401)
On Tue, Jun 11, 2019 at 4:22 AM Feng Xue OS wrote:
>
> > For future reference, there should be two spaces at the end of the sentence
> > before */. You can use gcc/contrib/check_GNU_style.sh foo.patch to catch
> > stuff like this before posting a final patch.
>
> It's good. Thanks for pointing it out.
Looks good from my side - please have Martin have a final look and ack.
Thanks,
Richard.
> Feng
>
> ---
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 526ed45be89..a21cd737e84 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,14 @@
> +2019-06-04 Feng Xue
> +
> + PR ipa/90401
> + * ipa-prop.c (add_to_agg_contents_list): New function.
> + (clobber_by_agg_contents_list_p): Likewise.
> + (extract_mem_content): Likewise.
> + (get_place_in_agg_contents_list): Delete.
> + (determine_known_aggregate_parts): Renamed from
> + determine_locally_known_aggregate_parts. New parameter
> + aa_walk_budget_p.
> +
> 2019-06-04 Segher Boessenkool
>
> * config/rs6000/constraints.md (define_register_constraint "wp"):
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index d86c2f3db55..a53a6ec5f32 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1458,7 +1458,7 @@ get_ssa_def_if_simple_copy (tree rhs)
>return rhs;
> }
>
> -/* Simple linked list, describing known contents of an aggregate beforere
> +/* Simple linked list, describing known contents of an aggregate before
> call. */
>
> struct ipa_known_agg_contents_list
> @@ -1471,41 +1471,48 @@ struct ipa_known_agg_contents_list
>struct ipa_known_agg_contents_list *next;
> };
>
> -/* Find the proper place in linked list of ipa_known_agg_contents_list
> - structures where to put a new one with the given LHS_OFFSET and LHS_SIZE,
> - unless there is a partial overlap, in which case return NULL, or such
> - element is already there, in which case set *ALREADY_THERE to true. */
> +/* Add a known content item into a linked list of ipa_known_agg_contents_list
> + structure, in which all elements are sorted ascendingly by offset. */
>
> -static struct ipa_known_agg_contents_list **
> -get_place_in_agg_contents_list (struct ipa_known_agg_contents_list **list,
> - HOST_WIDE_INT lhs_offset,
> - HOST_WIDE_INT lhs_size,
> - bool *already_there)
> +static inline void
> +add_to_agg_contents_list (struct ipa_known_agg_contents_list **plist,
> + struct ipa_known_agg_contents_list *item)
> {
> - struct ipa_known_agg_contents_list **p = list;
> - while (*p && (*p)->offset < lhs_offset)
> + struct ipa_known_agg_contents_list *list = *plist;
> +
> + for (; list; list = list->next)
> {
> - if ((*p)->offset + (*p)->size > lhs_offset)
> - return NULL;
> - p = &(*p)->next;
> + if (list->offset >= item->offset)
> + break;
> +
> + plist = &list->next;
> }
>
> - if (*p && (*p)->offset < lhs_offset + lhs_size)
> + item->next = list;
> + *plist = item;
> +}
> +
> +/* Check whether a given known content is clobbered by certain element in
> + a linked list of ipa_known_agg_contents_list. */
> +
> +static inline bool
> +clobber_by_agg_contents_list_p (struct ipa_known_agg_contents_list *list,
> + struct ipa_known_agg_contents_list *item)
> +{
> + for (; list; list = list->next)
> {
> - if ((*p)->offset == lhs_offset && (*p)->size == lhs_size)
> - /* We already know this value is subsequently overwritten with
> - something else. */
> - *already_there = true;
> - else
> - /* Otherwise this is a partial overlap which we cannot
> - represent. */
> - return NULL;
> + if (list->offset >= item->offset)
> + return list->offset < item->offset + item->size;
> +
> + if (list->offset + list->size > item->offset)
> + return true;
> }
> - return p;
> +
> + return false;
> }
>
> /* Build aggregate jump function from LIST, assuming there are exactly
> - CONST_COUNT constant entries there and that th offset of the passed
> argument
> + CONST_COUNT constant entries there and that offset of the passed argument
> is ARG_OFFSET and store it into JFUNC. */
>
> static void
> @@ -1528,26 +1535,79 @@ build_agg_jump_func_from_list (struct
> ipa_known_agg_contents_list *list,
> }
> }
>
> +/* If STMT is a memory store to the object whose address is BASE, extract
> + information (offset, size, and value) into CONTENT, and return true,
> + otherwise we conservatively assume the whole object is modified with
> + unknown content, and return false. CHECK_REF means that access to object
> + is expected to be in form of MEM_REF expression. */
> +
> +static bool
> +extract_mem_content (gimple *stmt, tree base, bool check_ref,
> +struct ipa_known_agg_contents_list *content)
> +{
> + HOST_WIDE_INT lhs
indirect_ref_may_alias_decl_p fix
Hi,
after spending some time on the view converting MEM_REFs, I noticed that
most of reasons we give up on view converts is not actually MEM_REF created
but test
same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (base2))
in indirect_ref_may_alias_decl_p
Here base2 is VAR_DECL while dbase2 is MEM_REF used to access it.
In the testcase:
struct a {int a1; int a2;};
struct b:a {};
struct b bvar,*bptr2;
int
test(void)
{
struct a *bptr = &bvar;
bptr->a2=0;
bptr2->a1=1;
return bptr->a2;
}
We have variable of type b, while we access it via its basetype.
This mean that TREE_TYPE (base) is "struct b" while TREE_TYPE (dbase)
is "struct a" which is perfectly normal and we could to the access path
tests at least in the same strength as we would do if bptr=$bvar was
not visible to compiler (in that case we optimize things correctly).
Of course later in aliasing_component_refs_p we should not assume that
"struct a" is the type of memory slot since the access path may contain
b, but I think we can not assume that about "struct b" either, see below.
We should not give up on this case and just proceed the same way as
indirect_refs_may_alias_p does. In fact I would like to commonize the
access path oracle of these functions incremetally but first I want to
drop main differences. In particular
1) indirect_refs_may_alias_decl_p passing ref2_is_decl as true to
aliasing_component_refs_p.
This makes aliasing_component_refs_p to assume that all access paths
conflicting with REF2 must start by type of BASE2 or its subtype.
IMO this is not quite right in gimple memory model where decls are just
untyped memory slots, since I can, for example, I can rewrite decl
of type a by a new data of type struct b {struct a a;};
which will confuse this logic.
I will try to get rid of this incrementally - I would like to have it
logged how much optimization we lose here.
2) indirect_refs_may_alias_decl_p does
if ((TREE_CODE (base1) != TARGET_MEM_REF
|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
&& same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (dbase2)) == 1
&& (TREE_CODE (TREE_TYPE (base1)) != ARRAY_TYPE
|| (TYPE_SIZE (TREE_TYPE (base1))
&& TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) == INTEGER_CST)))
return ranges_maybe_overlap_p (doffset1, max_size1, doffset2, max_size2);
while indirect_refs_may_alias_p does:
if ((TREE_CODE (base1) != TARGET_MEM_REF
|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
&& (TREE_CODE (base2) != TARGET_MEM_REF
|| (!TMR_INDEX (base2) && !TMR_INDEX2 (base2)))
&& same_type_for_tbaa (TREE_TYPE (ptrtype1),
TREE_TYPE (ptrtype2)) == 1
/* But avoid treating arrays as "objects", instead assume they
can overlap by an exact multiple of their element size.
See gcc.dg/torture/alias-2.c. */
&& TREE_CODE (TREE_TYPE (ptrtype1)) != ARRAY_TYPE)
return ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2);
Sincce we already checked that TREEE_TYPE (ptrtype) is same as TREE_TYPE
(base1)
the same_type_for_tbaa check is equivalent in both.
Notice however that first tests that array is VLA, while other
supports partial overlaps on all array types. I suppose we want to
choose which way we support that and go with one or another.
Of course even in that case overlap check is not completely lost,
I attached testcase for that to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90869
All we need is to wrap the checks by array size.
With these differences sorted out I think both functions may dispatch to
common access path oracle after doing the case specific work.
Bootstrapped/regtested x86_64-linux, makes sense?
PR tree-optimize/90869
* tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Watch for view
converts in MEM_REF referencing decl rather than view converts
from decl type to MEM_REF type.
* g++.dg/tree-ssa/alias-access-path-1.C: New testcase.
Index: tree-ssa-alias.c
===
--- tree-ssa-alias.c(revision 272037)
+++ tree-ssa-alias.c(working copy)
@@ -1370,11 +1410,16 @@ indirect_ref_may_alias_decl_p (tree ref1
poly_offset_int doffset2 = offset2;
if (TREE_CODE (dbase2) == MEM_REF
|| TREE_CODE (dbase2) == TARGET_MEM_REF)
-doffset2 -= mem_ref_offset (dbase2) << LOG2_BITS_PER_UNIT;
+{
+ doffset2 -= mem_ref_offset (dbase2) << LOG2_BIT
Re: indirect_ref_may_alias_decl_p fix
On Thu, 13 Jun 2019, Jan Hubicka wrote:
> Hi,
> after spending some time on the view converting MEM_REFs, I noticed that
> most of reasons we give up on view converts is not actually MEM_REF created
> but test
> same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (base2))
> in indirect_ref_may_alias_decl_p
>
> Here base2 is VAR_DECL while dbase2 is MEM_REF used to access it.
>
> In the testcase:
> struct a {int a1; int a2;};
> struct b:a {};
>
> struct b bvar,*bptr2;
> int
> test(void)
> {
> struct a *bptr = &bvar;
> bptr->a2=0;
> bptr2->a1=1;
> return bptr->a2;
> }
>
> We have variable of type b, while we access it via its basetype.
> This mean that TREE_TYPE (base) is "struct b" while TREE_TYPE (dbase)
> is "struct a" which is perfectly normal and we could to the access path
> tests at least in the same strength as we would do if bptr=$bvar was
> not visible to compiler (in that case we optimize things correctly).
>
> Of course later in aliasing_component_refs_p we should not assume that
> "struct a" is the type of memory slot since the access path may contain
> b, but I think we can not assume that about "struct b" either, see below.
>
> We should not give up on this case and just proceed the same way as
> indirect_refs_may_alias_p does. In fact I would like to commonize the
> access path oracle of these functions incremetally but first I want to
> drop main differences. In particular
>
> 1) indirect_refs_may_alias_decl_p passing ref2_is_decl as true to
> aliasing_component_refs_p.
>
> This makes aliasing_component_refs_p to assume that all access paths
> conflicting with REF2 must start by type of BASE2 or its subtype.
>
> IMO this is not quite right in gimple memory model where decls are just
> untyped memory slots, since I can, for example, I can rewrite decl
> of type a by a new data of type struct b {struct a a;};
> which will confuse this logic.
The above check you complain about guards against this.
> I will try to get rid of this incrementally - I would like to have it
> logged how much optimization we lose here.
>
> 2) indirect_refs_may_alias_decl_p does
>
> if ((TREE_CODE (base1) != TARGET_MEM_REF
>
>|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
>
> && same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (dbase2)) == 1
>
> && (TREE_CODE (TREE_TYPE (base1)) != ARRAY_TYPE
>
> || (TYPE_SIZE (TREE_TYPE (base1))
>
> && TREE_CODE (TYPE_SIZE (TREE_TYPE (base1))) == INTEGER_CST)))
>
> return ranges_maybe_overlap_p (doffset1, max_size1, doffset2, max_size2);
>
>
> while indirect_refs_may_alias_p does:
>
> if ((TREE_CODE (base1) != TARGET_MEM_REF
>
>|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1)))
>
> && (TREE_CODE (base2) != TARGET_MEM_REF
>
> || (!TMR_INDEX (base2) && !TMR_INDEX2 (base2)))
>
> && same_type_for_tbaa (TREE_TYPE (ptrtype1),
>
> TREE_TYPE (ptrtype2)) == 1
>
> /* But avoid treating arrays as "objects", instead assume they
>
> can overlap by an exact multiple of their element size.
>
> See gcc.dg/torture/alias-2.c. */
>
> && TREE_CODE (TREE_TYPE (ptrtype1)) != ARRAY_TYPE)
>
> return ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2);
>
>
> Sincce we already checked that TREEE_TYPE (ptrtype) is same as TREE_TYPE
> (base1)
> the same_type_for_tbaa check is equivalent in both.
>
> Notice however that first tests that array is VLA, while other
> supports partial overlaps on all array types. I suppose we want to
> choose which way we support that and go with one or another.
Let's go with the stricter check for the purpose of unification and work
on this issue as followup.
> Of course even in that case overlap check is not completely lost,
> I attached testcase for that to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90869
> All we need is to wrap the checks by array size.
>
> With these differences sorted out I think both functions may dispatch to
> common access path oracle after doing the case specific work.
>
> Bootstrapped/regtested x86_64-linux, makes sense?
Yes, the patch below makes sense.
Thanks,
Richard.
> PR tree-optimize/90869
> * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Watch for view
> converts in MEM_REF referencing decl rather than view converts
> from decl type to MEM_REF type.
>
> * g++.dg/tree-ssa/alias-access-path-1.C: New testcas
Re: indirect_ref_may_alias_decl_p fix
> > 1) indirect_refs_may_alias_decl_p passing ref2_is_decl as true to
> > aliasing_component_refs_p.
> >
> > This makes aliasing_component_refs_p to assume that all access paths
> > conflicting with REF2 must start by type of BASE2 or its subtype.
> >
> > IMO this is not quite right in gimple memory model where decls are just
> > untyped memory slots, since I can, for example, I can rewrite decl
> > of type a by a new data of type struct b {struct a a;};
> > which will confuse this logic.
>
> The above check you complain about guards against this.
Not sufficinly. ref1 can be store of type "struct b". With ref2 of
base type "struct a" (which is in conflict) the oracle will currently
disambiguate because it will not consider the path
"struct b"..."struct a" due to that ref2_is_decl check.
> > Sincce we already checked that TREEE_TYPE (ptrtype) is same as
> > TREE_TYPE (base1)
> > the same_type_for_tbaa check is equivalent in both.
> >
> > Notice however that first tests that array is VLA, while other
> > supports partial overlaps on all array types. I suppose we want to
> > choose which way we support that and go with one or another.
>
> Let's go with the stricter check for the purpose of unification and work
> on this issue as followup.
Yep, that is my plan. W/o that this that alias-2.c array overlap
testcase would break (and it breaks if one of accesses is through decl).
Concerning the unification, I am still confinced we want to test
base2_alias_set to be non-zero in indirect_ref_may_alias_decl_p
(same way as we do in indirect_refs_may_alias_p) as discussed in thread
https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00087.html
Because we can not assume that there are no dynamic type changes on
decls in gimple memory model.
I also need to dig into the divergence between
nonoverlapping_component_refs_of_decl_p and
nonoverlapping_component_refs_p. It seems to me that having the decl in
hand does not buy us much here either.
Notice that one tries to match structures only, while other handles
unions too. For LTO as well for code merging even w/o LTO it would be
nice to keep alias between
union {int a; int b;};
and ptr->a an ptr->b (which we currently do), but it seems it is
guaranteed only by the second function.
Honza
Re: [PATCH] Fix 3 warnings in nvptx.c
On 12-06-19 09:30, Jakub Jelinek wrote:
> Hi!
>
> While testing the second PR90811 patch with nvptx offloading, I've noticed 3
> warnings in nvptx.c.
> The first two hunks are obvious, the last warning is about unused mode_label
> variable.
> Either we can do what the patch does, or another option is throw away
> both the mode_jump and mode_label temporaries, keep the last hunk unmodified
> and
> - *mode_jump = label_insn;
> + if (mode == GOMP_DIM_VECTOR)
> + vector_jump = neuter_start;
> + else
> + worker_jump = neuter_start;
>
Hi Jakub,
LGTM, thanks for fixing this.
My intention with the mode_jump/mode_label temporaries was to get more
straight line code, and the patch achieves this.
Thanks,
- Tom
> 2019-06-12 Jakub Jelinek
>
> * config/nvptx/nvptx.c (nvptx_sese_number, nvptx_sese_pseudo): Don't
> wrap ei variable name in the declaration in ()s.
> (nvptx_single): Actually use mode_label variable. Formatting fix.
>
> --- gcc/config/nvptx/nvptx.c.jj 2019-06-11 23:21:28.068148871 +0200
> +++ gcc/config/nvptx/nvptx.c 2019-06-11 23:27:30.412633621 +0200
> @@ -3551,7 +3551,7 @@ nvptx_sese_number (int n, int p, int dir
>size_t offset = (dir > 0 ? offsetof (edge_def, dest)
> : offsetof (edge_def, src));
>edge e;
> - edge_iterator (ei);
> + edge_iterator ei;
>
>FOR_EACH_EDGE (e, ei, edges)
> {
> @@ -3574,7 +3574,7 @@ nvptx_sese_pseudo (basic_block me, bb_se
> vec *edges, size_t offset)
> {
>edge e;
> - edge_iterator (ei);
> + edge_iterator ei;
>int hi_back = depth;
>pseudo_node_t node_back (0, depth);
>int hi_child = depth;
> @@ -4402,8 +4402,10 @@ nvptx_single (unsigned mask, basic_block
>{
> rtx_code_label *label = gen_label_rtx ();
> rtx pred = cfun->machine->axis_predicate[mode - GOMP_DIM_WORKER];
> - rtx_insn **mode_jump = mode == GOMP_DIM_VECTOR ? &vector_jump :
> &worker_jump;
> - rtx_insn **mode_label = mode == GOMP_DIM_VECTOR ? &vector_label :
> &worker_label;
> + rtx_insn **mode_jump
> + = mode == GOMP_DIM_VECTOR ? &vector_jump : &worker_jump;
> + rtx_insn **mode_label
> + = mode == GOMP_DIM_VECTOR ? &vector_label : &worker_label;
>
> if (!pred)
> {
> @@ -4437,10 +4439,7 @@ nvptx_single (unsigned mask, basic_block
> emit_insn_after (gen_exit (), label_insn);
> }
>
> - if (mode == GOMP_DIM_VECTOR)
> - vector_label = label_insn;
> - else
> - worker_label = label_insn;
> + *mode_label = label_insn;
>}
>
>/* Now deal with propagating the branch condition. */
>
> Jakub
>
Re: [RFC] ARM -mfpu=auto woes
On Jun 12, 2019, Alexandre Oliva wrote: > I'm looking into a regression between gcc-7 and gcc-8 that causes > compilation with -mfloat-abi=hard to fail on arm-eabi with: > $ arm-eabi-gcc -c -mfloat-abi=hard t.c > cc1: error: -mfloat-abi=hard: selected processor lacks an FPU > Per the documentation, -mfpu=auto was supposed to be the default, but > the implicit -mcpu=armv7tdmi option from configure_default_options, or > the -march=armv4t derived from it, seems to be taken as choosing to > disable the fpu. Or rather they don't have any fpu-related options, so none of the fpu bits are set, and -mfpu=auto thus necessarily resolves to nofp. What's confusing to me is that specifying -mfpu=vfp is accepted, even though -mcpu=arm7tdmi+vfp isn't, and with that, -mfloat-abi=hard goes through as before. Would it make sense, for backward-compatibility purposes, to implicitly enable vfpv2 for -mfpu=auto, even for CPUs not configured as having an FP option, when given -mfloat-abi=hard? Or is this error introduced in GCC 8.* actually desirable, and requiring an explicit override of -mfpu, say =vfpv2 along for -mfloat-abi=hard, when the default CPU, e.g. arv7tdmi, doesn't have an FP, something that users are expected to do? Thanks in advance, -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain EngineerFree Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara
[PATCH] Fix PR90865
The following fixes SRA dropping address-space information when
reconstructing a reference from a model in a different address-space.
Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.
Richard.
PR tree-optimization/90856
* tree-sra.c (build_ref_for_model): Only use
build_reconstructed_reference when address-spaces are the same.
* gcc.target/i386/pr90856.c: New testcase.
diff --git a/gcc/testsuite/gcc.target/i386/pr90856.c
b/gcc/testsuite/gcc.target/i386/pr90856.c
new file mode 100644
index 000..a9a909b9eda
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr90856.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+typedef struct { int v; } S1;
+typedef struct { S1 s1[32]; } S2;
+
+S1 clearS1() { S1 s; s.v = 1; return s; }
+
+void
+clearS2(__seg_gs S2 *p, int n)
+{
+ for (int i = 0; i < n; ++i)
+p->s1[i] = clearS1();
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 074d4964379..03c1a2ae9e9 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -1854,6 +1854,8 @@ build_ref_for_model (location_t loc, tree base,
HOST_WIDE_INT offset,
tree res;
if (model->grp_same_access_path
&& !TREE_THIS_VOLATILE (base)
+ && (TYPE_ADDR_SPACE (TREE_TYPE (base))
+ == TYPE_ADDR_SPACE (TREE_TYPE (model->expr)))
&& offset <= model->offset
/* build_reconstructed_reference can still fail if we have already
massaged BASE because of another type incompatibility. */
Re: [PATCH] True IPA reimplementation of IPA-SRA
Hi,
i read all changes except for ipa-sra itself. Here are some comments,
I will look at the remaining file next.
Honza
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 9a19d83fffb..3f838c08e76 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
struct GTY(()) cgraph_clone_info
{
+ /* Constants discovered by IPA-CP, i.e. which parameter should be
replaced
+ with what. */
vec *tree_map;
- bitmap args_to_skip;
- bitmap combined_args_to_skip;
+ /* Parameter modification that IPA-SRA decided to perform. */
+ ipa_param_adjustments *param_adjustments;
+ /* Lists of all splits with their offsets for each dummy variables
+ representing a replaced-by-splits parameter. */
+ vec *performed_splits;
Please explain what dummy variables are :)
+/* Return true if we would like to remove a parameter from NODE when
cloning it
+ with KNOWN_CSTS scalar constants. */
+
+static bool
+want_remove_some_param_p (cgraph_node *node, vec known_csts)
+{
+ auto_vec surviving;
+ bool filled_vec = false;
+ ipa_node_params *info = IPA_NODE_REF (node);
+ int i, count = ipa_get_param_count (info);
+ for (i = 0; i < count; i++)
vertical space after the declarations :)
- tree t = known_csts[i];
-
- if (t || !ipa_is_param_used (info, i))
- bitmap_set_bit (args_to_skip, i);
+ ipa_adjusted_param *old_adj = &(*old_adjustments->m_adj_params)[i];
+ if (!node->local.can_change_signature
+ || old_adj->op != IPA_PARAM_OP_COPY
+ || (!known_csts[old_adj->base_index]
+ && ipa_is_param_used (info, old_adj->base_index)))
+ {
+ ipa_adjusted_param new_adj;
+ memcpy (&new_adj, old_adj, sizeof (new_adj));
Why this is not *new_adj=*old_adj?
+/* Names of parameters for dumping. Keep in sync with enum
ipa_parm_op. */
+
+static const char *ipa_param_op_names[] = {"IPA_PARAM_OP_UNDEFINED",
+ "IPA_PARAM_OP_COPY",
+ "IPA_PARAM_OP_NEW",
+ "IPA_PARAM_OP_SPLIT"};
Given brave new C++ world, can't we statically assert that size of array
match
the enum?
Also it seems to me that ipa-param-modification would benefit from some
toplevel
comment of what it does and what is the main API how to use it.
Also functions like
ipa_fill_vector_with_formal_parms
ipa_fill_vector_with_formal_parm_types
does not seem very IPA specific and it was not quite obvoius from name
what
it does. Perhaps it should go somewhere to common tree manipulatoin and
have more fitting name?
If it was something like push_function_arg_decls or
push_function_arg_types
it would be more obvious to me what it does :)
Also I wonder if the code would not be more readable if functions was
returning auto_vecs references. Then they can be just something like
"function_arg_types" and the API would be self explanatory.
+tree
+ipa_param_adjustments::adjust_decl (tree orig_decl)
+{
+ tree new_decl = copy_node (orig_decl);
+ tree orig_type = TREE_TYPE (orig_decl);
+ if (prototype_p (orig_type)
+ || (m_skip_return && !VOID_TYPE_P (TREE_TYPE (orig_type
+{
+ tree new_type = build_new_function_type (orig_type, false);
+ TREE_TYPE (new_decl) = new_type;
+}
+ if (method2func_p (orig_type))
+DECL_VINDEX (new_decl) = NULL_TREE;
I think you want to clear DECL_VINDEX in every case since the method is
no
longer one pointed to by virtual table. But I see we have similar code
elsewhere
so lets do that incrementally.
With early debug info we probably can forget about it completely.
+/* Structure to hold declarations representing transitive IPA-SRA
splits. In
+ essence, if we need to pass UNIT_OFFSET of a parameter which
originally has
+ number BASE_INDEX, we should pass down REPL. */
+
+struct transitive_split_map
+{
+ tree repl;
+ unsigned base_index;
+ unsigned unit_offset;
+};
It is not quite clear to me what those transitive splits are, so perhaps
better description would help :)
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gimple_stmt_iterator prev_gsi = gsi;
+ gsi_prev (&prev_gsi);
I think we still maintain vertical space after declarations at the
beggining
of block. It may help to break out this function a bit. With all the
debug
handling it is now quite monster.
+/* Common initialization performed by all ipa_param_body_adjustments
+ constructors. OLD_FNDECL is the declaration we take original
arguments
+ from, (it may be the same as M_FNDECL). VARS, if non-NULL, is a
pointer to
+ a chained list of new local variables. TREE_MAP is the IPA-CP
produced
+ mapping of trees to constants. */
+
+void
+ipa_param_body_adjustments::common_initialization (tree old_fndecl,
+ tree *vars,
+ vec *tree_map)
Some comments int he body about what the code does would help IMO :)
It
[PATCH, wwwdocs] Update on existence of free emulators
pinging this with more changes: https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01471.html S A Free simulator does not exist. HPPA and alpha are supported by QEMU. https://wiki.qemu.org/Features/HPPA https://wiki.qemu.org/Documentation/Platforms/Alpha VAX is supported by SIMH. http://simh.trailing-edge.com/ Index: backends.html === RCS file: /cvs/gcc/wwwdocs/htdocs/backends.html,v retrieving revision 1.84 diff -u -r1.84 backends.html --- backends.html 18 Jan 2019 11:52:12 - 1.84 +++ backends.html 13 Jun 2019 14:35:24 - @@ -69,7 +69,7 @@ Target | HMSLQNFICBD lqrcpbfmgiates ---+--- aarch64| Qq b gia s -alpha | ?? Q Cq mgi e +alpha | ? Q Cq mgi e arc| B b gia arm| b ia s avr|L FIl cp g @@ -101,7 +101,7 @@ nds32 | F Cia s nios2 | Cia nvptx | S Q Cq mg e -pa | ? Q CBD qr b i e +pa | Q CBD qr b i e pdp11 |L ICqr b e powerpcspe | Q Cqr pb ia riscv | Q Cqr gia @@ -116,7 +116,7 @@ tilegx | S Q Cq gi e tilepro| S F C gi e v850 | g a s -vax| M?I c b i e +vax| M I c b i e visium | B g t s xtensa | C
[PATCH][ARM] Add support for "noinit" attribute
Hi,
Similar to what already exists for TI msp430 or in TI compilers for
arm, this patch adds support for "noinit" attribute for arm. It's very
similar to the corresponding code in GCC for msp430.
It is useful for embedded targets where the user wants to keep the
value of some data when the program is restarted: such variables are
not zero-initialized.It is mostly a helper/shortcut to placing
variables in a dedicated section.
It's probably desirable to add the following chunk to the GNU linker:
diff --git a/ld/emulparams/armelf.sh b/ld/emulparams/armelf.sh
index 272a8bc..9555cec 100644
--- a/ld/emulparams/armelf.sh
+++ b/ld/emulparams/armelf.sh
@@ -10,7 +10,19 @@ OTHER_TEXT_SECTIONS='*(.glue_7t) *(.glue_7)
*(.vfp11_veneer) *(.v4_bx)'
OTHER_BSS_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__bss_start__ =
.${CREATE_SHLIB+)};"
OTHER_BSS_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}_bss_end__ =
.${CREATE_SHLIB+)}; ${CREATE_SHLIB+PROVIDE (}__bss_end__ =
.${CREATE_SHLIB+)};"
OTHER_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__end__ = .${CREATE_SHLIB+)};"
-OTHER_SECTIONS='.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }'
+OTHER_SECTIONS='
+.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }
+ /* This section contains data that is not initialised during load
+ *or* application reset. */
+ .noinit (NOLOAD) :
+ {
+ . = ALIGN(2);
+ PROVIDE (__noinit_start = .);
+ *(.noinit)
+ . = ALIGN(2);
+ PROVIDE (__noinit_end = .);
+ }
+'
so that the noinit section has the "NOLOAD" flag.
I'll submit that part separately to the binutils project if OK.
OK?
Thanks,
Christophe
gcc/ChangeLog:
2019-06-13 Christophe Lyon
* config/arm/arm.c (arm_attribute_table): Add "noinit" entry.
(arm_data_attr): New helper function.
(TARGET_ASM_SELECT_SECTION): New.
(arm_select_section): New function.
(arm_elf_section_type_flags): Add support for "noinit" section.
* doc/extend.texi: Add "noinit" attribute documentation.
gcc/testsuite/ChangeLog:
2019-06-13 Christophe Lyon
* gcc.target/arm/data-attributes.c: New test.
commit e04bcd361a87925ac8240bc106f64a37917bb804
Author: Christophe Lyon
Date: Tue Jun 11 21:09:08 2019 +
Add support for noinit attribute.
Change-Id: Ib7090c037f67e521ad9753e1a78ed5731996fefe
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e3e71ea..332c41b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -151,6 +151,7 @@ static tree arm_handle_notshared_attribute (tree *, tree,
tree, int, bool *);
#endif
static tree arm_handle_cmse_nonsecure_entry (tree *, tree, tree, int, bool *);
static tree arm_handle_cmse_nonsecure_call (tree *, tree, tree, int, bool *);
+static tree arm_data_attr (tree *, tree, tree, int, bool *);
static void arm_output_function_epilogue (FILE *);
static void arm_output_function_prologue (FILE *);
static int arm_comp_type_attributes (const_tree, const_tree);
@@ -375,7 +376,8 @@ static const struct attribute_spec arm_attribute_table[] =
arm_handle_cmse_nonsecure_entry, NULL },
{ "cmse_nonsecure_call", 0, 0, true, false, false, true,
arm_handle_cmse_nonsecure_call, NULL },
- { NULL, 0, 0, false, false, false, false, NULL, NULL }
+ { "noinit", 0, 0, true, false, false, false, arm_data_attr, NULL },
+ { NULL, 0, 0, false, false, false, false, NULL, NULL },
};
/* Initialize the GCC target structure. */
@@ -808,6 +810,10 @@ static const struct attribute_spec arm_attribute_table[] =
#undef TARGET_CONSTANT_ALIGNMENT
#define TARGET_CONSTANT_ALIGNMENT arm_constant_alignment
+
+#undef TARGET_ASM_SELECT_SECTION
+#define TARGET_ASM_SELECT_SECTION arm_select_section
+
/* Obstack for minipool constant handling. */
static struct obstack minipool_obstack;
@@ -7150,6 +7156,47 @@ arm_handle_cmse_nonsecure_call (tree *node, tree name,
return NULL_TREE;
}
+/* Called when the noinit attribute is used. Check whether the
+ attribute is allowed here and add the attribute to the variable
+ decl tree or otherwise issue a diagnostic. This function checks
+ NODE is of the expected type and issues diagnostics otherwise using
+ NAME. If it is not of the expected type *NO_ADD_ATTRS will be set
+ to true. */
+
+static tree
+arm_data_attr (tree * node,
+ tree name,
+ tree args ATTRIBUTE_UNUSED,
+ intflags ATTRIBUTE_UNUSED,
+ bool * no_add_attrs ATTRIBUTE_UNUSED)
+{
+ const char * message = NULL;
+
+ gcc_assert (DECL_P (* node));
+ gcc_assert (args == NULL);
+
+ if (TREE_CODE (* node) != VAR_DECL)
+message = G_("%qE attribute only applies to variables");
+
+ /* Check that it's possible for the variable to have a section. */
+ if ((TREE_STATIC (* node) || DECL_EXTERNAL (* node) || in_lto_p)
+ && DECL_SECTION_NAME (* node))
+message = G_("%qE attribute cannot be applied to variables with specific
sections");
+
+ /* If this var is thought to be common, th
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On 6/13/19 4:44 AM, Jakub Jelinek wrote:
On Thu, Jun 13, 2019 at 10:53:55AM +0200, Rainer Orth wrote:
Even with that fixed, I see many failures:
+FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++14 scan-tree-dump forwprop1
"MEM[.*&i][j.*] =.* 1;"
+FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++17 scan-tree-dump forwprop1
"MEM[.*&i][j.*] =.* 1;"
+FAIL: g++.dg/tree-ssa/pr31146.C -std=gnu++98 scan-tree-dump forwprop1
"MEM[.*&i][j.*] =.* 1;"
+FAIL: g++.dg/tree-ssa/ssa-dse-1.C scan-tree-dump-times dse1 "MEM
[(struct FixBuf *)& + [0-9]+B] = {}" 1
on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
these failures remain...
On i686-linux I can reproduce just the above ones.
The following should fix it. As we don't match exact offset on the MEM
because it varries between different architectures (24 bytes on with -m64,
28 bytes with -m32), we shouldn't match the store size either, as it is
200 - that offset, so 176 or 172 etc.
Tested on x86_64-linux, -m32/-m64, ok for trunk?
2019-06-13 Jakub Jelinek
* g++.dg/tree-ssa/ssa-dse-1.C: Don't match exact number of chars of
= {} store.
* g++.dg/tree-ssa/pr31146.C: Change -fdump-tree-forwprop to
-fdump-tree-forwprop1 in dg-options. Expect in MEM.
--- gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C.jj2019-06-13
00:35:49.654840275 +0200
+++ gcc/testsuite/g++.dg/tree-ssa/ssa-dse-1.C 2019-06-13 12:40:14.492568336
+0200
@@ -98,4 +98,4 @@ int main()
/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct FixBuf \\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
- { dg-final { scan-tree-dump-times "MEM \\\[\\(struct FixBuf \\*\\)&
\\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
+ { dg-final { scan-tree-dump-times "MEM \\\[\\(struct FixBuf
\\*\\)& \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
--- gcc/testsuite/g++.dg/tree-ssa/pr31146.C.jj 2015-05-29 15:04:33.039803414
+0200
+++ gcc/testsuite/g++.dg/tree-ssa/pr31146.C 2019-06-13 12:24:15.895576933
+0200
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O -fdump-tree-forwprop" } */
+/* { dg-options "-O -fdump-tree-forwprop1" } */
/* We should be able to optimize this to i[j] = 1 during
early optimizations. */
@@ -12,4 +12,4 @@ void foo (int j)
*q = 1;
}
-/* { dg-final { scan-tree-dump "MEM\\\[.*&i\\\]\\\[j.*\\\] =.* 1;" "forwprop1" } } */
+/* { dg-final { scan-tree-dump "MEM \\\[.*&i\\\]\\\[j.*\\\] =.* 1;"
"forwprop1" } } */
Thanks for cleaning this up.
The size of the access above doesn't look right. The test is:
int i[5];
void foo (int j)
{
void *p = &i[j];
int *q = (int *)p;
*q = 1;
}
and the MEM_REF is for the assignment *q = 1. It assigns a single
int, not the whole array. The -gimple output looks pretty much
the same:
__MEM ((int *)&i)[j_1(D)] = 1;
I expected it to mention the size of the access, e.g., like this:
__MEM ((int *)&i)[j_1(D)] = 1;
It was the entire goal of the change I made to be able to be able
to see the size of the access so it doesn't look like I got it
right and some more tweaking is necessary. Which also raises
the question: what is the purpose of the MEM_REF type if not to
encode the size/alignment of the access?
Martin
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
On 6/12/19 9:25 AM, Michael Matz wrote:
> Hi,
>
> On Wed, 12 Jun 2019, Martin Sebor wrote:
>
>>> Otherwise LGTM as the patch, but I'd like to hear from others whether
>>> it is kosher to add such a special case to the warn_unused_result
>>> attribute warning. And if the agreement is yes, I think it should be
>>> documented somewhere that alloca (0) will not warn even when the call
>>> has such an attribute (probably in the description of
>>> warn_unused_result attribute).
>>
>> I'm not very happy about adding another special case to alloca
>> (on top of not diagnosing zero allocation by -Walloc-zero).
>> There is no valid use case for the zero argument, whether or not
>> the return value is used.
>
> That's the thing, there _is_ a valid use case for supplying a zero
> argument and then the returned value should _not_ be used. There are
> alloca implementations that do something (freeing memory) when
> called with a zero size, so some (older) programs contain such calls.
> Warning on those calls for the unused results is exactly the wrong thing
> to do, if anything if the result is used we'd have to warn. (That's of
> course non-standard, but so is alloca itself) And just removing these
> calls isn't correct either except if it's ensured to not use an alloca
> implementation with that behaviour.
Agreed. Removing those calls simply can't be right unless you're
dropping support for the old C-alloca implementations completely.
>
> (In fact I think our builtin_alloca implementation could benefit when we
> added that behaviour as well; it's a natural wish to be able to free
> memory that you allocated).
But do we really want to cater to alloca more than we already are? I'd
argue that programmers simply aren't capable of using alloca
appropriately :-) That's based on seeing mis-uses exploited repeatedly
through the years.
Also note that simply sprinkling alloca(0) calls won't magically release
memory. In a stack implementation releasing happens when the frame is
removed. Doing something more complex than that seems unwise.
In a C implementation, allocations (or groups of allocations) carry
additional information -- specifically the stack pointer at the time the
object was allocated. Deallocation of the areas only occurs at
subsequent calls to alloca when the current stack pointer is larger than
the saved stack pointer (or if you're on a PA, the opposite :-)
So something like this
for (...) {
alloca (space);
}
alloca (0);
Won't release anything because the saved stack pointer for the
allocations in the loop is the same as the stack pointer at the point of
the alloca(0) call.
Where alloca(0) is useful is in something like this:
for (...) {
somefunc ();
}
Where somefunc allocates space with alloca. If you think about the
implementation details here, you'll realize that calls to alloca from
within somefunc called in this loop will all have the same stack
pointer. Thus garbage will keep accumulating on the stack across
iterations of the loop. To avoid that you put an alloca(0) in the loop
like this:
for (...) {
somefunc ();
alloca (0);
}
Now when we call alloca (0) the stack pointer will indicate that the
alloca calls that occurred within somefunc are all dead and thus the
alloca(0) will release memory.
Jeff
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote: > The size of the access above doesn't look right. The test is: It is correct. You have MEM [(int *)&i], which is the same thing as i itself, and on this you apply an ARRAY_REF, which is printed after it, with index j_1(D). ARRAY_REF is applied on arrays and the result type is the array element type, so int in this case. Jakub
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
On 6/12/19 10:40 AM, Jakub Jelinek wrote: > On Wed, Jun 12, 2019 at 10:13:57AM -0600, Martin Sebor wrote: >> But GCC doesn't support such an implementation, does it? > > Why would that be relevant? The warning would cause people to make portable > code less portable (by removing the alloca (0) calls that were added there > for portability reasons), or add hacks to work around the warning (whether > #pragma GCC diagnostic or something else). That is something we do not > want people to do. I'd like to move C-alloca support to the ash heap of history. But I'm not sure we can realistically do that. Given that reality I think we need to honor the intent of alloca(0). I also understand Martin's point that not warning on it could easily miss programming errors. I wonder if we could set TREE_NOWARNING on the CALL_EXPR early in the front-end of the argument is a constant 0 (before simplification, folding, etc). That way we'd be able to support alloca(0), but also capture cases where 0 flows into alloca via other mechansisms (which are likely programming errors). Jeff
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On 6/13/19 9:34 AM, Jakub Jelinek wrote: On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote: The size of the access above doesn't look right. The test is: It is correct. You have MEM [(int *)&i], which is the same thing as i itself, and on this you apply an ARRAY_REF, which is printed after it, with index j_1(D). ARRAY_REF is applied on arrays and the result type is the array element type, so int in this case. Aah, it's two REFs in one. I misread the array index as being a part of the MEM_REF operand, like this: MEM [((int *)&i)[j_1(D)]] = 1; I guess I've never noticed this before. Why is the whole thing not simplified to an ARRAY_REF? i[j_2(D)] = 1; Martin
Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)
On Thu, Jun 13, 2019 at 09:50:16AM -0600, Martin Sebor wrote: > On 6/13/19 9:34 AM, Jakub Jelinek wrote: > > On Thu, Jun 13, 2019 at 09:30:37AM -0600, Martin Sebor wrote: > > > The size of the access above doesn't look right. The test is: > > > > It is correct. You have MEM [(int *)&i], which is > > the same thing as i itself, and on this you apply an ARRAY_REF, > > which is printed after it, with index j_1(D). ARRAY_REF is applied on > > arrays and the result type is the array element type, so int in this case. > > Aah, it's two REFs in one. I misread the array index as being > a part of the MEM_REF operand, like this: > > MEM [((int *)&i)[j_1(D)]] = 1; > > I guess I've never noticed this before. Why is the whole thing > not simplified to an ARRAY_REF? > > i[j_2(D)] = 1; No idea in this case, though of course there can be other cases e.g. where the MEM_REF has different number of elements, different element type etc. from the underlying variable or where the MEM_REF first operand is not address, but pointer. Jakub
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
Hi,
On Thu, 13 Jun 2019, Jeff Law wrote:
> > (In fact I think our builtin_alloca implementation could benefit when we
> > added that behaviour as well; it's a natural wish to be able to free
> > memory that you allocated).
>
> Also note that simply sprinkling alloca(0) calls won't magically release
> memory. In a stack implementation releasing happens when the frame is
> removed. Doing something more complex than that seems unwise.
Yeah, on reconsideration I think I'm pedaling back on this suggestion
(which really was to do something more complex). We have the necessary
support for this in GCC (for VLAs), and then we'd be able to support code
like this:
for () {
dostuff (alloca (size));
morestuff (alloca (size2));
alloca(0); // free all allocas
}
without running the risk of unlimited stack use. But of course this would
promote a programming style that'd only work with our alloca (and not even
C-alloca), and we want to avoid that. I thought it a cute idea, but was
carried away by the cuteness ;-)
I like the suggestion of setting (and carrying through the pipeline) the
TREE_NO_WARNING flag on an source 'alloca(0)'.
Ciao,
Michael.
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
On Thu, Jun 13, 2019 at 03:59:33PM +, Michael Matz wrote: > without running the risk of unlimited stack use. But of course this would > promote a programming style that'd only work with our alloca (and not even > C-alloca), and we want to avoid that. I thought it a cute idea, but was > carried away by the cuteness ;-) > > I like the suggestion of setting (and carrying through the pipeline) the > TREE_NO_WARNING flag on an source 'alloca(0)'. Ditto, though it would be nice one day to implement TREE_NO_WARNING/gimple_no_warning_p as a bit indicating on the side sets of disabled warnings rather than the current big hammer where that bit disables all warnings. Jakub
[AArch64] Use scvtf fbits option where appropriate
Hi all,
There was previously no backend pattern to utilise the scvtf fbits option.
Where a fixed point is converted to a float, and divided by a power of 2, (or
multiplied by the reciprocal of a power of 2), this can be combined into a
single scvtf with fbits operation. This patch adds a pattern to combine these
instructions, and adds a helper function.
For the following test case:
float f(int a) { return ((float) a) / 65536.0; }
double g(int a) { return ((double) a) / 4096.0; }
the output generated is currently:
f:
scvtf s1, w0 // 6[c=8 l=4] floatsisf2/1
mov w1, 931135488 // 17 [c=4 l=4] *movsi_aarch64/3
fmovs0, w1 // 18 [c=4 l=4] *movsf_aarch64/1
fmuls0, s1, s0 // 13 [c=8 l=4] mulsf3
ret // 24 [c=0 l=4] *do_return
g:
scvtf d1, w0 // 6[c=8 l=4] floatsidf2
mov x1, 4553139223271571456 // 17 [c=4 l=4] *movdi_aarch64/3
fmovd0, x1 // 18 [c=4 l=4] *movdf_aarch64/1
fmuld0, d1, d0 // 13 [c=8 l=4] muldf3
ret // 24 [c=0 l=4] *do_return
The output with this patch applied is:
f:
scvtf s0, w0, #16 // 13 [c=24 l=4] *combine_scvtf_SI_sf3/1
ret // 22 [c=0 l=4] *do_return
g:
scvtf d0, w0, #12 // 13 [c=24 l=4] *combine_scvtf_SI_df3
ret // 22 [c=0 l=4] *do_return
gcc/ChangeLog:
2019-06-12 Joel Hutton
* config/aarch64/aarch64-protos.h (aarch64_fpconst_pow2_recip): New
prototype
* config/aarch64/aarch64.c (aarch64_fpconst_pow2_recip): New function
* config/aarch64/aarch64.md
(*aarch64_cvtf__2_mult): New pattern
(aarch64_cvtf__2_mult): New pattern
* config/aarch64/constraints.md (Dt): New constraint
* config/aarch64/predicates.md (aarch64_fpconst_pow2_recip): New
predicate
gcc/testsuite/ChangeLog:
2019-06-12 Joel Hutton
* gcc.target/aarch64/fmul_scvtf.c: New test.
Bootstrapped and regression tested on aarch64-linux-none target.From 6aac0d56dc7d34e7a6fcabc1e8b0c7c291c0d51a Mon Sep 17 00:00:00 2001
From: Joel Hutton
Date: Thu, 13 Jun 2019 11:08:56 +0100
Subject: [PATCH] SCVTF fbits
---
gcc/config/aarch64/aarch64-protos.h | 1 +
gcc/config/aarch64/aarch64.c | 30
gcc/config/aarch64/aarch64.md | 34 +
gcc/config/aarch64/constraints.md | 7 +
gcc/config/aarch64/predicates.md | 4 +
gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c | 140 ++
6 files changed, 216 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/aarch64/fmul_scvtf.c
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 1e3b1c91db1..ad1ba458a3f 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -494,6 +494,7 @@ enum aarch64_symbol_type aarch64_classify_tls_symbol (rtx);
enum reg_class aarch64_regno_regclass (unsigned);
int aarch64_asm_preferred_eh_data_format (int, int);
int aarch64_fpconst_pow_of_2 (rtx);
+int aarch64_fpconst_pow2_recip (rtx);
machine_mode aarch64_hard_regno_caller_save_mode (unsigned, unsigned,
machine_mode);
int aarch64_uxt_size (int, HOST_WIDE_INT);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9a035dd9ed8..49a7d9256cf 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18707,6 +18707,36 @@ aarch64_fpconst_pow_of_2 (rtx x)
return exact_log2 (real_to_integer (r));
}
+/* If X is a positive CONST_DOUBLE with a value that is the reciprocal of a
+ power of 2 (i.e 1/2^n) return the number of float bits. e.g. for x==(1/2^n)
+ return log2 (n). Otherwise return 0. */
+int
+aarch64_fpconst_pow2_recip (rtx x)
+{
+ REAL_VALUE_TYPE r0;
+
+ if (!CONST_DOUBLE_P (x))
+return 0;
+
+ r0 = *CONST_DOUBLE_REAL_VALUE (x);
+ if (exact_real_inverse (DFmode, &r0)
+ && !REAL_VALUE_NEGATIVE (r0))
+{
+ if (exact_real_truncate (DFmode, &r0))
+ {
+ HOST_WIDE_INT value = real_to_integer (&r0);
+ value = value & 0x;
+ if ((value != 0) && ( (value & (value - 1)) == 0))
+ {
+ int ret = exact_log2 (value);
+ gcc_assert (IN_RANGE (ret, 0, 31));
+ return ret;
+ }
+ }
+}
+ return 0;
+}
+
/* If X is a vector of equal CONST_DOUBLE values and that value is
Y, return the aarch64_fpconst_pow_of_2 of Y. Otherwise return -1. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 526c7fb0dab..60bcf1bc8d9 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -6016,6 +6016,40 @@
[(set_attr "type" "f_cvtf2i")]
)
+(define_insn "*aarch64_cvtf__2_mult"
+ [(set (match_operand:GPF 0 "register_operand" "=w,w")
+ (mult:GPF (FLOATUORS:GPF
+ (match_operand: 1 "register_operand" "w,?r"))
+ (match_operand 2 "aarch64_fp_pow2_recip
[PATCH] Define C11 macros such as FLT_DECIMAL_DIG for C++17
The C++17 library was rebased on C11 by https://wg21.link/p0063r3 so
these nine macros are needed for C++17 conformance.
gcc:
* ginclude/float.h (FLT_DECIMAL_DIG, DBL_DECIMAL_DIG, LDBL_DECIMAL_DIG)
(FLT_HAS_SUBNORM, DBL_HAS_SUBNORM, LDBL_HAS_SUBNORM, FLT_TRUE_MIN)
(DBL_TRUE_MIN, LDBL_TRUE_MIN): Also define for C++17.
libstdc++-v3:
* testsuite/18_support/headers/cfloat/values_c++17.cc: New test.
Tested x86_64-linux and powerpc64le-linux.
OK for trunk and gcc-9-branch?
commit ce40ee46ca226c102c548032184345fe9f5d352e
Author: Jonathan Wakely
Date: Thu Jun 13 11:38:33 2019 +0100
Define C11 macros such as FLT_DECIMAL_DIG for C++17
The C++17 library was rebased on C11 by https://wg21.link/p0063r3 so
these nine macros are needed for C++17 conformance.
gcc:
* ginclude/float.h (FLT_DECIMAL_DIG, DBL_DECIMAL_DIG,
LDBL_DECIMAL_DIG)
(FLT_HAS_SUBNORM, DBL_HAS_SUBNORM, LDBL_HAS_SUBNORM, FLT_TRUE_MIN)
(DBL_TRUE_MIN, LDBL_TRUE_MIN): Also define for C++17.
libstdc++-v3:
* testsuite/18_support/headers/cfloat/values_c++17.cc: New test.
diff --git a/gcc/ginclude/float.h b/gcc/ginclude/float.h
index 9ebae057d34..4767d7b9dfb 100644
--- a/gcc/ginclude/float.h
+++ b/gcc/ginclude/float.h
@@ -210,7 +210,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
If not, see
#endif /* C99 */
-#if defined (__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
+#if (defined (__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) \
+ || (defined (__cplusplus) && __cplusplus >= 201703L)
/* Versions of DECIMAL_DIG for each floating-point type. */
#undef FLT_DECIMAL_DIG
#undef DBL_DECIMAL_DIG
diff --git a/libstdc++-v3/testsuite/18_support/headers/cfloat/values_c++17.cc
b/libstdc++-v3/testsuite/18_support/headers/cfloat/values_c++17.cc
new file mode 100644
index 000..f3efd204d17
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/headers/cfloat/values_c++17.cc
@@ -0,0 +1,37 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2019 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: [PATCH] Do not warn with warn_unused_result for alloca(0).
> "Jeff" == Jeff Law writes: Jeff> I'd like to move C-alloca support to the ash heap of history. But I'm Jeff> not sure we can realistically do that. Are there still platforms or compilers in use where it's needed? For gdb I was planning to just remove these calls. Tom
Re: [PATCH V4] Find constant definition for by-ref argument using dominance information (PR ipa/90401)
Hi, On Thu, Jun 13 2019, Richard Biener wrote: > On Tue, Jun 11, 2019 at 4:22 AM Feng Xue OS > wrote: >> >> > For future reference, there should be two spaces at the end of the sentence >> > before */. You can use gcc/contrib/check_GNU_style.sh foo.patch to catch >> > stuff like this before posting a final patch. >> >> It's good. Thanks for pointing it out. > > Looks good from my side - please have Martin have a final look and ack. The last version of the patch is OK with me. Sorry that it took me so much time to get to it and thanks for working on this. Martin
[PATCH, testsuite] Require alias support for pr90760.c
This test fails on Darwin, because it doesn’t support symbol aliases.
Fixed by adding the dg-requires line,
tested on x86_64-darwin16, applied as obvious,
thanks
Iain
gcc/testsuite/
2019-06-13 Iain Sandoe
* gcc.dg/pr90760.c: Require alias support.
diff --git a/gcc/testsuite/gcc.dg/pr90760.c b/gcc/testsuite/gcc.dg/pr90760.c
index 525a21a..6092a4e 100644
--- a/gcc/testsuite/gcc.dg/pr90760.c
+++ b/gcc/testsuite/gcc.dg/pr90760.c
@@ -1,5 +1,6 @@
/* PR c/90760 */
/* { dg-do compile } */
+/* { dg-require-alias "" } */
/* { dg-require-named-sections "" } */
void bar (void) {}
Re: [AArch64] Use scvtf fbits option where appropriate
Hi Joel,
A few comments below:
+/* If X is a positive CONST_DOUBLE with a value that is the reciprocal of a
+ power of 2 (i.e 1/2^n) return the number of float bits. e.g. for x==(1/2^n)
+ return log2 (n). Otherwise return 0. */
+int
+aarch64_fpconst_pow2_recip (rtx x)
+{
+ REAL_VALUE_TYPE r0;
+
+ if (!CONST_DOUBLE_P (x))
+return 0;
+
+ r0 = *CONST_DOUBLE_REAL_VALUE (x);
+ if (exact_real_inverse (DFmode, &r0)
+ && !REAL_VALUE_NEGATIVE (r0))
+{
+ if (exact_real_truncate (DFmode, &r0))
Truncate to double? That doesn't do anything...
+ {
+ HOST_WIDE_INT value = real_to_integer (&r0);
+ value = value & 0x;
+ if ((value != 0) && ( (value & (value - 1)) == 0))
+ {
+ int ret = exact_log2 (value);
+ gcc_assert (IN_RANGE (ret, 0, 31));
+ return ret;
+ }
Wouldn't it be easier to just do exact_log2 (real_to_integer (&r0))
and then check the range is in 1..31?
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -6016,6 +6016,40 @@
[(set_attr "type" "f_cvtf2i")]
)
+(define_insn "*aarch64_cvtf__2_mult"
+ [(set (match_operand:GPF 0 "register_operand" "=w,w")
+ (mult:GPF (FLOATUORS:GPF
+ (match_operand: 1 "register_operand" "w,?r"))
+ (match_operand 2 "aarch64_fp_pow2_recip""Dt,Dt")))]
We should add a comment before both define_insn similar to the other
conversions, explaining what they do and why there are 2 separate patterns
(the default versions of the conversions appear to be missing a comment too).
Wilco
[PATCH, Darwin, testsuite] Adjust two tests for modern OS versions.
Newer OS versions (10.14+) do not provide some of the CRTs that are used for
older ones (e.g. 10.5), and thus link tests that specify targeting such a
revision fail.
For the two tests affected, we retain the testing of the correct defined OS
version
number but switch to compile-only testing. A link test will be added as part
of a
follow-up.
tested on x86_64-darwin16, 18, powerpc-darwin9, i686-darwin9.
applied to mainline
thanks
Iain
gcc/testsuite/
2019-06-13 Iain Sandoe
* gcc.dg/darwin-minversion-1.c: Use compile rather than link/run.
* gcc.dg/darwin-minversion-2.c: Likewise.
diff --git a/gcc/testsuite/gcc.dg/darwin-minversion-1.c
b/gcc/testsuite/gcc.dg/darwin-minversion-1.c
index ee6493a..5f8524f 100644
--- a/gcc/testsuite/gcc.dg/darwin-minversion-1.c
+++ b/gcc/testsuite/gcc.dg/darwin-minversion-1.c
@@ -1,6 +1,6 @@
/* Basic test for -mmacosx-version-min switch on Darwin. */
/* { dg-options "-mmacosx-version-min=10.5" } */
-/* { dg-do run { target *-*-darwin* } } */
+/* { dg-do compile { target *-*-darwin* } } */
int
main ()
diff --git a/gcc/testsuite/gcc.dg/darwin-minversion-2.c
b/gcc/testsuite/gcc.dg/darwin-minversion-2.c
index 46fab67..3dbbca6 100644
--- a/gcc/testsuite/gcc.dg/darwin-minversion-2.c
+++ b/gcc/testsuite/gcc.dg/darwin-minversion-2.c
@@ -1,6 +1,6 @@
/* Basic test for -mmacosx-version-min switch on Darwin. */
/* { dg-options "-mmacosx-version-min=10.1 -mmacosx-version-min=10.5" } */
-/* { dg-do run { target *-*-darwin* } } */
+/* { dg-do compile { target *-*-darwin* } } */
int
main ()
Re: [patch][aarch64]: fix unrecognizable insn for ldr got in ilp32 tiny
Hi Sylvia, -(define_insn "ldr_got_tiny" +(define_insn "ldr_got_tiny_di" [(set (match_operand:DI 0 "register_operand" "=r") - (unspec:DI [(match_operand:DI 1 "aarch64_valid_symref" "S")] - UNSPEC_GOTTINYPIC))] + (unspec:DI + [(match_operand:DI 1 "aarch64_valid_symref" "S")] + UNSPEC_GOTTINYPIC))] "" "ldr\\t%0, %L1" [(set_attr "type" "load_8")] ) +(define_insn "ldr_got_tiny_si" + [(set (match_operand:SI 0 "register_operand" "=r") + (unspec:SI + [(match_operand:SI 1 "aarch64_valid_symref" "S")] + UNSPEC_GOTTINYPIC))] + "TARGET_ILP32" + "ldr\\t%0, %L1" + [(set_attr "type" "load_4")] +) These can be easily combined like the related ldr_got_small_. Wilco
Re: [PATCH] PR fortran/88810 -- Code clean up
Committed revision 272254. I'm sure that when you authored the code, it wasn't confusing at all. -- steve On Thu, Jun 13, 2019 at 06:14:59AM +0100, Paul Richard Thomas wrote: > Hi Steve, > > As the original author of the confusing code, I want to thank you for > de-confusing it! > > OK > > Paul > > On Wed, 12 Jun 2019 at 20:01, Steve Kargl > wrote: > > > > This PR flags a section of confusing but correct code. > > The attach patch rewrites the code to hopefully provide > > some clarity. It has lived my tree for around 6 months, > > so has sufferred through numerous regression tests. > > OK to commit? > > > > 2019-06-12 Steven G. Kargl > > > > PR fortran/88810 > > * dependency.c (gfc_dep_resolver): Re-arrange code to make the logic > > a bit more transparent. Fix 2 nearby formatting issues. > > > > -- > > Steve > > > > -- > "If you can't explain it simply, you don't understand it well enough" > - Albert Einstein -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Re: [PATCH, wwwdocs] Update on existence of free emulators
On 6/13/19 8:44 AM, co...@sdf.org wrote: > pinging this with more changes: > https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01471.html > > S A Free simulator does not exist. > > HPPA and alpha are supported by QEMU. > https://wiki.qemu.org/Features/HPPA > https://wiki.qemu.org/Documentation/Platforms/Alpha > VAX is supported by SIMH. > http://simh.trailing-edge.com/ THanks. Installed. jeff
Re: [PATCH V4] A jump threading opportunity for condition branch
On 6/3/19 11:28 PM, Jiufu Guo wrote:
>
> Hi,
>
> This patch implements a new opportunity of jump threading for PR77820.
> In this optimization, conditional jumps are merged with unconditional
> jump. And then moving CMP result to GPR is eliminated.
>
> This version is based on the proposal of Richard, Jeff and Andrew on
> previous versions, and refined to incorporate comments, such as accept
> the path with single_succ_p (e->src).
> Thanks for the reviews!
>
> Bootstrapped and tested on powerpc64le, powerpc64 and sh (with help
> from Jeff) with no regressions (two cases are improved and updated
> to keep original test coverage) and new testcases are added.
> Is this ok for trunk?
>
> Example of this opportunity looks like below:
>
>
> p0 = a CMP b
> goto ;
>
>
> p1 = c CMP d
> goto ;
>
>
> # phi = PHI
> if (phi != 0) goto ; else goto ;
>
> Could be transformed to:
>
>
> p0 = a CMP b
> if (p0 != 0) goto ; else goto ;
>
>
> p1 = c CMP d
> if (p1 != 0) goto ; else goto ;
>
>
> This optimization eliminates:
> 1. saving CMP result: p0 = a CMP b.
> 2. additional CMP on branch: if (phi != 0).
> 3. converting CMP result if there is phi = (INT) p0 if there is.
>
> Thanks!
> Jiufu Guo
>
> [gcc]
> 2019-06-04 Jiufu Guo
> Lijia He
>
> PR tree-optimization/77820
> * tree-ssa-threadedge.c
> (edge_forwards_cmp_to_conditional_jump_through_empty_bb_p): New
> function.
> (thread_across_edge): Add call to
> edge_forwards_cmp_to_conditional_jump_through_empty_bb_p.
>
> [gcc/testsuite]
> 2019-06-04 Jiufu Guo
> Lijia He
>
> PR tree-optimization/77820
> * gcc.dg/tree-ssa/phi_on_compare-1.c: New testcase.
> * gcc.dg/tree-ssa/phi_on_compare-2.c: New testcase.
> * gcc.dg/tree-ssa/phi_on_compare-3.c: New testcase.
> * gcc.dg/tree-ssa/phi_on_compare-4.c: New testcase.
> * gcc.dg/tree-ssa/split-path-6.c: Update testcase.
> * gcc.target/sh/pr51244-20.c: Update testcase.
Yes, this is OK for the trunk. I'll commit it momentarily.
Jeff
>
>
> ---
> gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c | 30 ++
> gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c | 23
> gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c | 25 +
> gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c | 40 ++
> gcc/testsuite/gcc.dg/tree-ssa/split-path-6.c | 2 +-
> gcc/testsuite/gcc.target/sh/pr51244-20.c | 2 +-
> gcc/tree-ssa-threadedge.c| 70
> +++-
> 7 files changed, 187 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> new file mode 100644
> index 000..5227c87
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (int);
> +void g1 (int);
> +
> +void
> +f (long a, long b, long c, long d, long x)
> +{
> + _Bool t;
> + if (x)
> +{
> + g (a + 1);
> + t = a < b;
> + c = d + x;
> +}
> + else
> +{
> + g (b + 1);
> + a = c + d;
> + t = c > d;
> +}
> +
> + if (t)
> +g1 (c);
> +
> + g (a);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> new file mode 100644
> index 000..eaf89bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (void);
> +void g1 (void);
> +
> +void
> +f (long a, long b, long c, long d, int x)
> +{
> + _Bool t;
> + if (x)
> +t = c < d;
> + else
> +t = a < b;
> +
> + if (t)
> +{
> + g1 ();
> + g ();
> +}
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> new file mode 100644
> index 000..d5a1e0b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (void);
> +void g1 (void);
> +
> +void
> +f (long a, long b, long c, long d, int x)
> +{
> + int t;
> + if (x)
> +t = a < b;
> + else if (d == x)
> +t = c < b;
> + else
> +t = d > c;
> +
> + if (t)
> +{
> + g1 ();
> + g ();
> +}
> +}
> +
> +/* { dg-fin
[Darwin, testsuite] - Fix pr71694 fail for m32.
This test fails for Darwin m32 because it's scanning for absence of an
instruction
that's validly used in PIC code. Fixed, in this case, by using non-PIC codegen.
tested on x86_64-darwin16, x86_64-linux-gnu (m32, m64).
applied to mainline
thanks
Iain
2019-06-13 Iain Sandoe
* g++.dg/pr71694.C: Use non-PIC codegen for Darwin m32.
diff --git a/gcc/testsuite/g++.dg/pr71694.C b/gcc/testsuite/g++.dg/pr71694.C
index 0a8baf230b..5b59f879fb 100644
--- a/gcc/testsuite/g++.dg/pr71694.C
+++ b/gcc/testsuite/g++.dg/pr71694.C
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -fno-store-merging" } */
+/* { dg-additional-options "-fno-common -mdynamic-no-pic" { target { ia32 && {
x86_64-*-darwin* i?86-*-darwin* } } } } */
struct B {
B() {}
--
2.17.1
[PATCH, Darwin, Driver, committed] Improve processing of macosx-version-min=
For PR target/63810 some improvements were made in the parsing of the version
string
at the point it's used to define the built-in
__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__.
This is fine, but the specs processing also uses the version, and specs
version-compare
doesn't like leading zeros on components. This means that while we succeed in
processing -mmacosx-version-min=010.2.99 on compile lines, it fails for
any
other line that uses the value as part of a spec (in particular, link lines
fail).
To fix this, we need to apply a bit of clean-up to the version that’s presented
to the driver,
and push that back into the command line opts.
The value can come from four places:
1. User-entered on the command line
2. User-entered as MACOSX_DEPLOYMENT_TARGET= environment var.
3. Absent those two
3a For self-hosting systems, look-up from the kernel
3b For cross-compilers, as a default supplied at configure time.
We apply the clean-up to all 4 (although it shouldn't really be needed
for the cases under 3).
We also supply a test-case that adapts to the target-version of the
system, so that the link requirements are met by the SDK in use (if you
try to link i686-darwin9 on an x86-64-darwin18 SDK, it will fail).
tested across a range of Darwin boxes,
applied to mainline,
thanks
Iain
gcc/
2019-06-13 Iain Sandoe
* config/darwin-driver.c (validate_macosx_version_min): New.
(darwin_default_min_version): Cleanup and validate supplied version.
(darwin_driver_init): Likewise and push cleaned version into opts.
gcc/testsuite
2019-06-13 Iain Sandoe
* gcc.dg/darwin-minversion-link.c: New test.
diff --git a/gcc/config/darwin-driver.c b/gcc/config/darwin-driver.c
index 6a7c859..01238e2 100644
--- a/gcc/config/darwin-driver.c
+++ b/gcc/config/darwin-driver.c
@@ -26,6 +26,91 @@ along with GCC; see the file COPYING3. If not see
#include "opts.h"
#include "diagnostic-core.h"
+/* Validate a version string (either given on the command line or, perhaps
+ as MACOSX_DEPLOYMENT_TARGET).
+
+ The specs %version-compare() function doesn't accept leading '0' on
+ numbers so strip them out. Do sanity checking here too.
+
+ Return:
+ * original string means it was OK and we didn't want to change it.
+ * new string means it was OK but we rewrote it to avoid possible format
+ problems.
+ * NULL means we didn't like what we saw.
+*/
+
+static const char *
+validate_macosx_version_min (const char *version_str)
+{
+ size_t version_len;
+ unsigned long major, minor, tiny = 0;
+ char *end;
+ const char *old_version = version_str;
+ bool need_rewrite = false;
+
+ version_len = strlen (version_str);
+ if (version_len < 4) /* The minimum would be 10.x */
+return NULL;
+
+ /* Version string must consist of digits and periods only. */
+ if (strspn (version_str, "0123456789.") != version_len)
+return NULL;
+
+ if (!ISDIGIT (version_str[0]) || !ISDIGIT (version_str[version_len - 1]))
+return NULL;
+
+ if (version_str[0] == '0')
+need_rewrite = true;
+
+ major = strtoul (version_str, &end, 10);
+ version_str = end + ((*end == '.') ? 1 : 0);
+
+ if (major != 10) /* So far .. all MacOS 10 ... */
+return NULL;
+
+ /* Version string components must be present and numeric. */
+ if (!ISDIGIT (version_str[0]))
+return NULL;
+
+ /* If we have one or more leading zeros on a component, then rewrite the
+ version string. */
+ if (version_str[0] == '0' && version_str[1] != '\0'
+ && version_str[1] != '.')
+need_rewrite = true;
+
+ minor = strtoul (version_str, &end, 10);
+ version_str = end + ((*end == '.') ? 1 : 0);
+ if (minor > 99)
+return NULL;
+
+ /* If 'tiny' is present it must be numeric. */
+ if (*end != '\0' && !ISDIGIT (version_str[0]))
+return NULL;
+
+ /* If we have one or more leading zeros on a component, then rewrite the
+ version string. */
+ if (*end != '\0' && version_str[0] == '0'
+ && version_str[1] != '\0')
+need_rewrite = true;
+
+ tiny = strtoul (version_str, &end, 10);
+ if (tiny > 99)
+return NULL;
+
+ /* Version string must contain no more than three tokens. */
+ if (*end != '\0')
+return NULL;
+
+ if (need_rewrite)
+{
+ char *new_version;
+ asprintf (&new_version, "10.%lu.%lu", minor, tiny);
+ return new_version;
+}
+
+ return old_version;
+}
+
#ifndef CROSS_DIRECTORY_STRUCTURE
#include
#include "xregex.h"
@@ -114,12 +199,13 @@ darwin_default_min_version (void)
if (new_flag != NULL)
{
- size_t len = strlen (new_flag);
- if (len > 128) { /* Arbitrary limit, number should be like xx.yy.zz */
- warning (0, "couldn%'t understand version %s\n", new_flag);
- return NULL;
- }
- new_flag = xstrndup (new_flag, len);
+ const char *checked = validate_macosx_version_min (new_flag);
+ if (checked == NULL)
+ {
+ warning (0, "couldn%'t understand version %s\n"
[PATCH] avoid ice due to inconsistent argument types to fold_build (PR 90662)
Attached is a fix for the fold_build call with inconsistent
argument types introduced in a recent commit of mine.
Tested on x86_64-linux.
Martin
PR tree-optimization/90662 - strlen of a string in a vla plus offset not folded
gcc/ChangeLog:
PR tree-optimization/90662
* tree-ssa-strlen.c (get_stridx): Convert fold_build2 operands
to the same type.
gcc/testsuite/ChangeLog:
PR tree-optimization/90662
* gcc.dg/pr90866-2.c: New test.
* gcc.dg/pr90866.c: Ditto.
Index: gcc/testsuite/gcc.dg/pr90866-2.c
===
--- gcc/testsuite/gcc.dg/pr90866-2.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr90866-2.c (working copy)
@@ -0,0 +1,24 @@
+/* PR tree-optimization/90866 - ICE in fold_binary_loc, at fold-const.c:9827
+ { dg-do compile }
+ { dg-options "-O2 -fsanitize=thread" } */
+
+typedef enum { a } b;
+typedef struct {
+ int c[0];
+} d;
+typedef struct {
+ int *data;
+} e;
+typedef struct {
+ e buffer;
+} f;
+int g, h;
+int i();
+int i(f *j, d *k, b l, int m) {
+ if (l)
+if (m) {
+ h = j->buffer.data[0];
+ k->c[g] = k->c[g] * 8;
+}
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/pr90866.c
===
--- gcc/testsuite/gcc.dg/pr90866.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr90866.c (working copy)
@@ -0,0 +1,18 @@
+/* PR tree-optimization/90866 - ICE in fold_binary_loc, at fold-const.c:9827
+ { dg-do compile }
+ { dg-options "-O3 -Wall -fno-tree-loop-optimize" } */
+
+int a[1024], b[1024];
+
+void f (void)
+{
+ int i = 0;
+ for ( ; ; i++)
+{
+b[16] = a[i * 16 + 10];
+b[i * 16 + 11] = a[i * 16 + 11] * 3;
+b[i * 16 + 12] = a[i * 16 + 12] * 4;
+b[i * 16 + 13] = a[i * 16 + 13] * 4;
+b[i * 16 + 14] = a[i * 16 + 14] * 3;
+ }
+}
Index: gcc/tree-ssa-strlen.c
===
--- gcc/tree-ssa-strlen.c (revision 272252)
+++ gcc/tree-ssa-strlen.c (working copy)
@@ -322,11 +322,17 @@ get_stridx (tree exp)
if (TREE_CODE (ptr) == ARRAY_REF)
{
off = TREE_OPERAND (ptr, 1);
- /* Scale the array index by the size of the element
- type (normally 1 for char). */
- off = fold_build2 (MULT_EXPR, TREE_TYPE (off), off,
- eltsize);
ptr = TREE_OPERAND (ptr, 0);
+ if (!integer_onep (eltsize))
+ {
+ /* Scale the array index by the size of the element
+ type in the rare case that it's greater than
+ the typical 1 for char, making sure both operands
+ have the same type. */
+ eltsize = fold_convert (ssizetype, eltsize);
+ off = fold_convert (ssizetype, off);
+ off = fold_build2 (MULT_EXPR, ssizetype, off, eltsize);
+ }
}
else
off = integer_zero_node;
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
On 6/13/19 10:46 AM, Tom Tromey wrote: "Jeff" == Jeff Law writes: Jeff> I'd like to move C-alloca support to the ash heap of history. But I'm Jeff> not sure we can realistically do that. Are there still platforms or compilers in use where it's needed? For gdb I was planning to just remove these calls. The only implementation I know about is Doug Gwyn's alloca. A Google search returns a pointer to QNX CAR that documents it with the following note: * By default, alloca() is implemented as __builtin_alloca(). If you compile with the -fno-builtin option, you'll use the libc version of alloca(), which is a cover for malloc() and behaves in a slightly different manner: - It keeps track of all blocks allocated with alloca() and reclaims any that are found to be deeper in the stack than the current invocation. It therefore doesn't reclaim storage as soon as it becomes invalid, but it will do so eventually. - As a special case, alloca(0) reclaims storage without allocating any. You can use this feature to force garbage collection. The implementation details aren't important. What matters is that to call the library implementation one has to prevent GCC from treating alloca as a built-in and eliminating the zero size calls. When the function is not treated as a built-in no warnings for calls to it are issued and so no problem for us to solve. Martin http://www.qnx.com/developers/docs/qnxcar2/index.jsp?topic=%2Fcom.qnx.doc.neutrino.lib_ref%2Ftopic%2Fa%2Falloca.html
Re: indirect_ref_may_alias_decl_p fix
On Thu, 13 Jun 2019 at 14:29, Jan Hubicka wrote:
>
> > > 1) indirect_refs_may_alias_decl_p passing ref2_is_decl as true to
> > > aliasing_component_refs_p.
> > >
> > > This makes aliasing_component_refs_p to assume that all access paths
> > > conflicting with REF2 must start by type of BASE2 or its subtype.
> > >
> > > IMO this is not quite right in gimple memory model where decls are
> > > just
> > > untyped memory slots, since I can, for example, I can rewrite decl
> > > of type a by a new data of type struct b {struct a a;};
> > > which will confuse this logic.
> >
> > The above check you complain about guards against this.
>
> Not sufficinly. ref1 can be store of type "struct b". With ref2 of
> base type "struct a" (which is in conflict) the oracle will currently
> disambiguate because it will not consider the path
> "struct b"..."struct a" due to that ref2_is_decl check.
>
> > > Sincce we already checked that TREEE_TYPE (ptrtype) is same as
> > > TREE_TYPE (base1)
> > > the same_type_for_tbaa check is equivalent in both.
> > >
> > > Notice however that first tests that array is VLA, while other
> > > supports partial overlaps on all array types. I suppose we want to
> > > choose which way we support that and go with one or another.
> >
> > Let's go with the stricter check for the purpose of unification and work
> > on this issue as followup.
>
> Yep, that is my plan. W/o that this that alias-2.c array overlap
> testcase would break (and it breaks if one of accesses is through decl).
>
> Concerning the unification, I am still confinced we want to test
> base2_alias_set to be non-zero in indirect_ref_may_alias_decl_p
> (same way as we do in indirect_refs_may_alias_p) as discussed in thread
> https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00087.html
> Because we can not assume that there are no dynamic type changes on
> decls in gimple memory model.
>
> I also need to dig into the divergence between
> nonoverlapping_component_refs_of_decl_p and
> nonoverlapping_component_refs_p. It seems to me that having the decl in
> hand does not buy us much here either.
>
> Notice that one tries to match structures only, while other handles
> unions too. For LTO as well for code merging even w/o LTO it would be
> nice to keep alias between
> union {int a; int b;};
> and ptr->a an ptr->b (which we currently do), but it seems it is
> guaranteed only by the second function.
>
Hi!
Since this was committed (r272247), I've noticed a failure to build
glibc-2.29 for aarch64:
#'target_mem_ref' not supported by expression#'pmap_rmt.c: In function
'clnt_broadcast':
pmap_rmt.c:298:19: error: may be used uninitialized in this function
[-Werror=maybe-uninitialized]
298 |baddr.sin_addr = addrs[i];
|~~~^~
while compiling sunrpc/pmap_rmt.os
Christophe
> Honza
Re: indirect_ref_may_alias_decl_p fix
> Hi! > > Since this was committed (r272247), I've noticed a failure to build > glibc-2.29 for aarch64: > #'target_mem_ref' not supported by expression#'pmap_rmt.c: In function > 'clnt_broadcast': > pmap_rmt.c:298:19: error: may be used uninitialized in this function > [-Werror=maybe-uninitialized] > 298 |baddr.sin_addr = addrs[i]; > |~~~^~ > > while compiling sunrpc/pmap_rmt.os It is hard to tell w/o preprocessed source, but looking at https://github.com/lattera/glibc/blob/master/sunrpc/pmap_rmt.c addrs is initialized by getbroadcastnests which has early exit when getifaddrs fails leaving adds uninitialized. So this may be just an usual false positive of -Wmaybe-uninitialized caused by better optimization :) Honza > > Christophe > > > > > Honza
Re: indirect_ref_may_alias_decl_p fix
Hi Christophe, > Since this was committed (r272247), I've noticed a failure to build > glibc-2.29 for aarch64: > #'target_mem_ref' not supported by expression#'pmap_rmt.c: In function > 'clnt_broadcast': > pmap_rmt.c:298:19: error: may be used uninitialized in this function > [-Werror=maybe-uninitialized] > 298 |baddr.sin_addr = addrs[i]; > |~~~^~ > > while compiling sunrpc/pmap_rmt.os indeed: I just came to the same conclusion via a reghunt. Even worse, it breaks i386-pc-solaris2.11, sparc*-sun-solaris2.11, and i686-pc-solaris2.11 bootstrap. This is PR bootstrap/90873. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Replace std::to_string for integers with optimized version
Hi, On Wed, 12 Jun 2019 at 16:54, Jonathan Wakely wrote: > > The std::to_chars functions from C++17 can be used to implement > std::to_string with much better performance than calling snprintf. Only > the __detail::__to_chars_len and __detail::__to_chars_10 functions are > needed for to_string, because it always outputs base 10 representations. > > The return type of __detail::__to_chars_10 should not be declared before > C++17, so the function body is extracted into a new function that can be > reused by to_string and __detail::__to_chars_10. > > The existing tests for to_chars rely on to_string to check for correct > answers. Now that they use the same code that doesn't actually ensure > correctness, so add new tests for std::to_string that compare against > printf output. > > * include/Makefile.am: Add new header. > * include/Makefile.in: Regenerate. > * include/bits/basic_string.h (to_string(int), to_string(unsigned)) > (to_string(long), to_string(unsigned long), to_string(long long)) > (to_string(unsigned long long)): Rewrite to use __to_chars_10_impl. > * include/bits/charconv.h: New header. > (__detail::__to_chars_len): Move here from . > (__detail::__to_chars_10_impl): New function extracted from > __detail::__to_chars_10. > * include/std/charconv (__cpp_lib_to_chars): Add, but comment out. > (__to_chars_unsigned_type): New class template that reuses > __make_unsigned_selector_base::__select to pick a type. > (__unsigned_least_t): Redefine as __to_chars_unsigned_type::type. > (__detail::__to_chars_len): Move to new header. > (__detail::__to_chars_10): Add inline specifier. Move code doing the > output to __detail::__to_chars_10_impl and call that. > * include/std/version (__cpp_lib_to_chars): Add, but comment out. > * testsuite/21_strings/basic_string/numeric_conversions/char/ > to_string.cc: Fix reference in comment. Remove unused variable. > * testsuite/21_strings/basic_string/numeric_conversions/char/ > to_string_int.cc: New test. > > Tested x86_64-linux, committed to trunk. > The new test to_string_int.cc fails on arm-none-eabi: PASS: 21_strings/basic_string/numeric_conversions/char/to_string_int.cc (test for excess errors) spawn /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/utils/bin/qemu-wrapper.sh ./to_string_int.exe /libstdc++-v3/testsuite/21_strings/basic_string/numeric_conversions/char/to_string_int.cc:105: void check_value(T) [with T = long long int]: Assertion 's == expected' failed. FAIL: 21_strings/basic_string/numeric_conversions/char/to_string_int.cc execution test Christophe
Re: libbacktrace integration for _GLIBCXX_DEBUG mode
Here is a new proposal which I think take into account all your remarks. I discovered the great "%.*s" printf format so I was able to do some cleanup on the function name without any allocation. I also agree that counting the '>' or '<' is not reliable so I remove this and limit the cleanup to the __cxx1998 namespace and the __ uglification, it is still better than nothing. I complete the doc as advised. I also added a note about making sure that _GLIBCXX_DEBUG_BACKTRACE is defined consistently throughout the application otherwise it would break the famous ODR rule. I introduced a _GLIBCXX_DEBUG_USE_LIBBACKTRACE to know when the system is able to handle libbacktrace even if the user didn't activate it. I could undef if when I am not building the library but I don't remember if there is a macro to signal that library is being built ? Here is an output sample now: /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/string:177: In function: gnu_debug::basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT*, gnu_debug::basic_string<_CharT, _Traits, _Allocator>::size_type, const _Allocator&) [with _CharT = char; _Traits = std::char_traits; _Allocator = std::allocator; gnu_debug::basic_string<_CharT, _Traits, _Allocator>::size_type = long unsigned int] Backtrace: 0x400afd char const* gnu_debug::check_stringlong>(char const*, unsigned long, char const*, unsigned int, char const*) /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/string:56 0x400afd char const* gnu_debug::check_stringlong>(char const*, unsigned long, char const*, unsigned int, char const*) /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/string:49 0x400afd gnu_debug::basic_string, std::allocator >::basic_string(char const*, unsigned long, std::allocator const&) /home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/string:177 0x400afd test01() /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/21_strings/basic_string/debug/1_neg.cc:27 0x4009c8 main /home/fdt/dev/gcc/git/libstdc++-v3/testsuite/21_strings/basic_string/debug/1_neg.cc:32 Error: __s != 0 || __n == 0. XFAIL: 21_strings/basic_string/debug/1_neg.cc execution test * include/debug/formatter.h [_GLIBCXX_DEBUG_BACKTRACE]: Include . [_GLIBCXX_DEBUG_BACKTRACE && BACKTRACE_SUPPORTED]: Include . [(!_GLIBCXX_DEBUG_BACKTRACE || !BACKTRACE_SUPPORTED) && _GLIBCXX_USE_C99_STDINT_TR1]: Include . [BACKTRACE_SUPPORTED || _GLIBCXX_USE_C99_STDINT_TR1] (_GLIBCXX_DEBUG_USE_LIBBACKTRACE): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](__backtrace_error_cb): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](__backtrace_full_cb): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](__backtrace_state): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](_Error_formatter::_Bt_full_t): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](_Error_formatter::_M_print_backtrace): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE](_Error_formatter::_M_backtrace_state): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE] (_Error_formatter::_M_backtrace_full_func): New. * src/c++11/debug.cc: Include . (_Print_func_t): New. (print_word): Use '%.*s' format in fprintf to render only expected chars. (print_raw(PrintContext&, const char*, ptrdiff_t)): New. (print_function(PrintContext&, const char*, _Print_func_t)): New. (print_type): Use latter. (print_string(PrintContext&, const char*, const _Parameter*, size_t)): Change signature to... (print_string(PrintContext&, const char*, ptrdiff_t, const _Parameter*, size_t)): ...this and adapt. Remove intermediate buffer to render input string. (print_string(PrintContext&, const char*, ptrdiff_t)): New. [_GLIBCXX_DEBUG_USE_LIBBACKTRACE] (print_backtrace(void*, uintptr_t, const char*, int, const char*)): New. (_Error_formatter::_M_error()): Adapt. * doc/xml/manual/debug_mode.xml: Document _GLIBCXX_DEBUG_BACKTRACE. * doc/xml/manual/using.xml: Likewise. Tested under Linux x86_64 normal and debug modes. Ok to commit ? François diff --git a/libstdc++-v3/doc/xml/manual/debug_mode.xml b/libstdc++-v3/doc/xml/manual/debug_mode.xml index 23a5df975a2..ebfc1b89fd7 100644 --- a/libstdc++-v3/doc/xml/manual/debug_mode.xml +++ b/libstdc++-v3/doc/xml/manual/debug_mode.xml @@ -162,6 +162,13 @@ which always works correctly. GLIBCXX_DEBUG_MESSAGE_LENGTH can be used to request a different length. +Starting with GCC 10 libstdc++ is able to use + http://www.w3.org/1999/xlink"; + xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace + to produce backtraces on error. Use -D_GLIBCXX_DEBUG_BACKTRACE to + activate it. Note that when defined, if libbacktrace is not properly installed, + compilation will fail. You'll also have to use -lbacktrace to build + your application. Using a Specific Debug Container diff --git a/libstdc++-v3/doc/xml/manual/using.xml b/l
Re: indirect_ref_may_alias_decl_p fix
Hi, actually since Rainer's testcase has TARGET_MEM_REF I think the problem may be due to fact that we now can get TARGET_MEM_REF ofsetting the array housed in decl while previously we gave up on types mismatch. Does the following help? Index: tree-ssa-alias.c === --- tree-ssa-alias.c(revision 272265) +++ tree-ssa-alias.c(working copy) @@ -1393,8 +1393,10 @@ But avoid treating variable length arrays as "objects", instead assume they can overlap by an exact multiple of their element size. See gcc.dg/torture/alias-2.c. */ - if ((TREE_CODE (base1) != TARGET_MEM_REF - || (!TMR_INDEX (base1) && !TMR_INDEX2 (base1))) + if (((TREE_CODE (base1) != TARGET_MEM_REF +|| (!TMR_INDEX (base1) && !TMR_INDEX2 (base1))) + && (TREE_CODE (dbase2) != TARGET_MEM_REF + || (!TMR_INDEX (dbase2) && !TMR_INDEX2 (dbase2 && same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (dbase2)) == 1 && (TREE_CODE (TREE_TYPE (base1)) != ARRAY_TYPE || (TYPE_SIZE (TREE_TYPE (base1))
C++ PATCH to detect narrowing in case values (PR c++/90805)
Case values are converted constant expressions, so narrowing conversion is not
permitted. This patch adds detecting narrowing to case_conversion; it's a
handy spot because we have both the value and the (adjusted) type of the
condition.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-13 Marek Polacek
PR c++/90805 - detect narrowing in case values.
* decl.c (case_conversion): Detect narrowing in case values.
* c-c++-common/pr89888.c: Update expected dg-error.
* g++.dg/cpp0x/Wnarrowing17.C: New test.
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 0a3ef452536..655de1ea6af 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -3610,16 +3610,21 @@ case_conversion (tree type, tree value)
value = mark_rvalue_use (value);
+ if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
+type = type_promotes_to (type);
+
if (cxx_dialect >= cxx11
&& (SCOPED_ENUM_P (type)
|| !INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (value
-{
- if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
- type = type_promotes_to (type);
- value = (perform_implicit_conversion_flags
- (type, value, tf_warning_or_error,
- LOOKUP_IMPLICIT | LOOKUP_NO_NON_INTEGRAL));
-}
+value = (perform_implicit_conversion_flags
+(type, value, tf_warning_or_error,
+ LOOKUP_IMPLICIT | LOOKUP_NO_NON_INTEGRAL));
+
+ /* The constant-expression VALUE shall be a converted constant expression
+ of the adjusted type of the switch condition, which doesn't allow
+ narrowing conversions. */
+ check_narrowing (type, value, tf_warning_or_error, /*const_only=*/true);
+
return cxx_constant_value (value);
}
diff --git gcc/testsuite/c-c++-common/pr89888.c
gcc/testsuite/c-c++-common/pr89888.c
index d9e11d6f26a..f14881ca052 100644
--- gcc/testsuite/c-c++-common/pr89888.c
+++ gcc/testsuite/c-c++-common/pr89888.c
@@ -11,8 +11,8 @@ foo (unsigned char x)
{
case -1: y = -1; break;/* { dg-message "previously
used here" } */
/* { dg-warning "case label
value is less than minimum value for type" "" { target *-*-* } .-1 } */
-case 0x: y = 0x; break;/* { dg-error "duplicate case
value" } */
-case ~0U: y = ~0U; break; /* { dg-error "duplicate case
value" } */
+case 0x: y = 0x; break;/* { dg-error "duplicate case
value|narrowing" } */
+case ~0U: y = ~0U; break; /* { dg-error "duplicate case
value|narrowing" } */
}
}
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C
gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C
new file mode 100644
index 000..064de531cb3
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing17.C
@@ -0,0 +1,19 @@
+// PR c++/90805 - detect narrowing in case values.
+// { dg-do compile { target c++11 } }
+
+void f(int i, char c, unsigned u)
+{
+ switch (i)
+{
+case 2149056512u:; // { dg-error "narrowing conversion of .2149056512.
from .unsigned int. to .int." }
+case (long long int) 1e10:; // { dg-error "narrowing conversion of
.100. from .long long int. to .int." }
+// { dg-warning "overflow in conversion" "overflow" { target *-*-* } .-1 }
+}
+
+ switch (c)
+// No narrowing, the adjusted type is int.
+case 300:; // { dg-warning "exceeds maximum value for type" }
+
+ switch (u)
+case -42:; // { dg-error "narrowing conversion of .-42. from .int. to
.unsigned int." }
+}
Re: indirect_ref_may_alias_decl_p fix
Hi, I am testing the following patch which solves the bogus warning in tree-ssa-forwprop.c and -m32 and plan to commit it as obvoius to unbreak bootstrap once testing converges. Previously we did not get here wince we got mismatch between TMR type and decl type but same code is present in indirect_ref_may_alias_p. Honza PR bootstrap/90873 * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also check that dbase is not TARGET_MEM_REF. Index: tree-ssa-alias.c === --- tree-ssa-alias.c(revision 272247) +++ tree-ssa-alias.c(working copy) @@ -1393,8 +1393,10 @@ indirect_ref_may_alias_decl_p (tree ref1 But avoid treating variable length arrays as "objects", instead assume they can overlap by an exact multiple of their element size. See gcc.dg/torture/alias-2.c. */ - if ((TREE_CODE (base1) != TARGET_MEM_REF + if (((TREE_CODE (base1) != TARGET_MEM_REF || (!TMR_INDEX (base1) && !TMR_INDEX2 (base1))) + && (TREE_CODE (dbase2) != TARGET_MEM_REF + || (!TMR_INDEX (dbase2) && !TMR_INDEX2 (base2 && same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (dbase2)) == 1 && (TREE_CODE (TREE_TYPE (base1)) != ARRAY_TYPE || (TYPE_SIZE (TREE_TYPE (base1))
[PATCH, i386]: Use gen_{add2,sub3,extend}_insn some more
Attached patch converts a bunch of indirect function calls to use
generic gen_{add2,sub3,extend}_insn functions. The patch corrects
and3 expander, which generated invalid RTX (non-existent
QI/HImode->DImode extend instructions for 32bit STV targets), so fake
insn-and-split patterns were necessary. The new approach is to leave
DImode and instructions up to STV pass, and split them just before
register allocation to zero-extends.
2019-06-13 Uroš Bizjak
* config/i386/i386.md (SWIM1248s): Rename from SWIM1248x.
Update all uses.
(and3): Use gen_extend_insn instead of indirect functions.
Do not generate DImode extends for 32bit targets.
(and->zext post-reload splitter): Use gen_extend_insn
instead of indirect functions.
(anddi->zext pre-reload splitter): New.
(*zext_doubleword_and): Remove.
(*zext_doubleword): Ditto.
(*zextsi_doubleword): Dittto.
2019-06-13 Uroš Bizjak
* config/i386/i386-expand.c (ix86_expand_int_sse_cmp):
Use gen_sub3_insn instead of indirect function.
(ix86_expand_ashl_const): Use gen_add2_insn instead of
indirect function.
(ix86_adjust_counter): Ditto.
Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index 0585c7d08bd0..770106162fb0 100644
--- a/gcc/config/i386/i386-expand.c
+++ b/gcc/config/i386/i386-expand.c
@@ -4303,27 +4303,15 @@ ix86_expand_int_sse_cmp (rtx dest, enum rtx_code code,
rtx cop0, rtx cop1,
case E_V2DImode:
{
rtx t1, t2, mask;
- rtx (*gen_sub3) (rtx, rtx, rtx);
- switch (mode)
- {
- case E_V16SImode: gen_sub3 = gen_subv16si3; break;
- case E_V8DImode: gen_sub3 = gen_subv8di3; break;
- case E_V8SImode: gen_sub3 = gen_subv8si3; break;
- case E_V4DImode: gen_sub3 = gen_subv4di3; break;
- case E_V4SImode: gen_sub3 = gen_subv4si3; break;
- case E_V2DImode: gen_sub3 = gen_subv2di3; break;
- default:
- gcc_unreachable ();
- }
/* Subtract (-(INT MAX) - 1) from both operands to make
them signed. */
mask = ix86_build_signbit_mask (mode, true, false);
t1 = gen_reg_rtx (mode);
- emit_insn (gen_sub3 (t1, cop0, mask));
+ emit_insn (gen_sub3_insn (t1, cop0, mask));
t2 = gen_reg_rtx (mode);
- emit_insn (gen_sub3 (t2, cop1, mask));
+ emit_insn (gen_sub3_insn (t2, cop1, mask));
cop0 = t1;
cop1 = t2;
@@ -4339,9 +4327,8 @@ ix86_expand_int_sse_cmp (rtx dest, enum rtx_code code,
rtx cop0, rtx cop1,
case E_V8HImode:
/* Perform a parallel unsigned saturating subtraction. */
x = gen_reg_rtx (mode);
- emit_insn (gen_rtx_SET (x, gen_rtx_US_MINUS (mode, cop0,
- cop1)));
-
+ emit_insn (gen_rtx_SET
+(x, gen_rtx_US_MINUS (mode, cop0, cop1)));
cop0 = x;
cop1 = CONST0_RTX (mode);
code = EQ;
@@ -5562,18 +5549,17 @@ ix86_split_long_move (rtx operands[])
static void
ix86_expand_ashl_const (rtx operand, int count, machine_mode mode)
{
- rtx (*insn)(rtx, rtx, rtx);
-
if (count == 1
|| (count * ix86_cost->add <= ix86_cost->shift_const
&& !optimize_insn_for_size_p ()))
{
- insn = mode == DImode ? gen_addsi3 : gen_adddi3;
while (count-- > 0)
- emit_insn (insn (operand, operand, operand));
+ emit_insn (gen_add2_insn (operand, operand));
}
else
{
+ rtx (*insn)(rtx, rtx, rtx);
+
insn = mode == DImode ? gen_ashlsi3 : gen_ashldi3;
emit_insn (insn (operand, operand, GEN_INT (count)));
}
@@ -6506,10 +6492,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx
value, rtx vec_value,
static void
ix86_adjust_counter (rtx countreg, HOST_WIDE_INT value)
{
- rtx (*gen_add)(rtx, rtx, rtx)
-= GET_MODE (countreg) == DImode ? gen_adddi3 : gen_addsi3;
-
- emit_insn (gen_add (countreg, countreg, GEN_INT (-value)));
+ emit_insn (gen_add2_insn (countreg, GEN_INT (-value)));
}
/* Depending on ISSETMEM, copy enough from SRCMEM to DESTMEM or set enough to
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 01338e294ab4..b484caebc453 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1018,10 +1018,11 @@
(HI "TARGET_HIMODE_MATH")
SI])
-;; Math-dependant integer modes with DImode.
-(define_mode_iterator SWIM1248x [(QI "TARGET_QIMODE_MATH")
-(HI "TARGET_HIMODE_MATH")
[Patch, fortran] PR90577, PR90578 - FAIL: gfortran.dg/lrshift_1.f90 & Wrong code with LSHIFT and optimization
Dear all,
it was already discussed in the above PRs that the testcase
lrshift_1.f90 needs to be corrected because it invokes negative
values of the SHIFT argument. This is fixed, as well as the
related documentation, which did not match e.g. the Fortran standard.
Looking closer at the actual implementation, it also turned out that the
runtime implementation of SHIFTA/RSHIFT gave different (=wrong) results
when SHIFT==BIT_SIZE(arg1). This is fixed and tested by the attached
patches.
I did not implement any run-time checks for the SHIFT argument.
All previous undefined behavior stays the same.
OK for trunk?
Thanks,
Harald
2019-06-13 Harald Anlauf
PR fortran/90577
PR fortran/90578
* trans-intrinsic.c (gfc_conv_intrinsic_shift): Properly
distinguish logical/arithmetic shifts.
* intrinsic.texi: Update documentation for SHIFTR/SHIFTL/SHIFTA
(Fortran 2008) and LSHIFT/RSHIFT (GNU extensions).
2019-06-13 Harald Anlauf
PR fortran/90577
PR fortran/90578
* gfortran.dg/lrshift_1.f90: Adjust testcase.
* gfortran.dg/shiftalr_3.f90: New testcase.
Index: gcc/fortran/intrinsic.texi
===
--- gcc/fortran/intrinsic.texi (revision 272261)
+++ gcc/fortran/intrinsic.texi (working copy)
@@ -9689,10 +9689,10 @@
@table @asis
@item @emph{Description}:
@code{LSHIFT} returns a value corresponding to @var{I} with all of the
-bits shifted left by @var{SHIFT} places. If the absolute value of
-@var{SHIFT} is greater than @code{BIT_SIZE(I)}, the value is undefined.
-Bits shifted out from the left end are lost; zeros are shifted in from
-the opposite end.
+bits shifted left by @var{SHIFT} places. @var{SHIFT} shall be
+nonnegative and less than or equal to @code{BIT_SIZE(I)}, otherwise
+the result value is undefined. Bits shifted out from the left end are
+lost; zeros are shifted in from the opposite end.
This function has been superseded by the @code{ISHFT} intrinsic, which
is standard in Fortran 95 and later, and the @code{SHIFTL} intrinsic,
@@ -12244,11 +12244,12 @@
@table @asis
@item @emph{Description}:
@code{RSHIFT} returns a value corresponding to @var{I} with all of the
-bits shifted right by @var{SHIFT} places. If the absolute value of
-@var{SHIFT} is greater than @code{BIT_SIZE(I)}, the value is undefined.
-Bits shifted out from the right end are lost. The fill is arithmetic: the
-bits shifted in from the left end are equal to the leftmost bit, which in
-two's complement representation is the sign bit.
+bits shifted right by @var{SHIFT} places. @var{SHIFT} shall be
+nonnegative and less than or equal to @code{BIT_SIZE(I)}, otherwise
+the result value is undefined. Bits shifted out from the right end
+are lost. The fill is arithmetic: the bits shifted in from the left
+end are equal to the leftmost bit, which in two's complement
+representation is the sign bit.
This function has been superseded by the @code{SHIFTA} intrinsic, which
is standard in Fortran 2008 and later.
@@ -12783,11 +12784,12 @@
@table @asis
@item @emph{Description}:
@code{SHIFTA} returns a value corresponding to @var{I} with all of the
-bits shifted right by @var{SHIFT} places. If the absolute value of
-@var{SHIFT} is greater than @code{BIT_SIZE(I)}, the value is undefined.
-Bits shifted out from the right end are lost. The fill is arithmetic: the
-bits shifted in from the left end are equal to the leftmost bit, which in
-two's complement representation is the sign bit.
+bits shifted right by @var{SHIFT} places. @var{SHIFT} that be
+nonnegative and less than or equal to @code{BIT_SIZE(I)}, otherwise
+the result value is undefined. Bits shifted out from the right end
+are lost. The fill is arithmetic: the bits shifted in from the left
+end are equal to the leftmost bit, which in two's complement
+representation is the sign bit.
@item @emph{Standard}:
Fortran 2008 and later
@@ -12823,10 +12825,10 @@
@table @asis
@item @emph{Description}:
@code{SHIFTL} returns a value corresponding to @var{I} with all of the
-bits shifted left by @var{SHIFT} places. If the absolute value of
-@var{SHIFT} is greater than @code{BIT_SIZE(I)}, the value is undefined.
-Bits shifted out from the left end are lost, and bits shifted in from
-the right end are set to 0.
+bits shifted left by @var{SHIFT} places. @var{SHIFT} shall be
+nonnegative and less than or equal to @code{BIT_SIZE(I)}, otherwise
+the result value is undefined. Bits shifted out from the left end are
+lost, and bits shifted in from the right end are set to 0.
@item @emph{Standard}:
Fortran 2008 and later
@@ -12862,10 +12864,10 @@
@table @asis
@item @emph{Description}:
@code{SHIFTR} returns a value corresponding to @var{I} with all of the
-bits shifted right by @var{SHIFT} places. If the absolute value of
-@var{SHIFT} is greater than @code{BIT_SIZE(I)}, the value is undefined.
-Bits shifted out from the right end are lost, and bi
[PATCH] PR fortran/89646 -- suppress spurious warnings
The attached patch suppresses the spurious warnings that
would otherwise occur for the testcase. Regression
tested cleanly on x86_64-*-freebsd. OK to commit?
2019-06-13 Steven G. Kargl
PR fortran/89646
* dependency.c (gfc_check_argument_var_dependency): Suppress spurious
warnings by comparing variable names.
2019-06-13 Steven G. Kargl
PR fortran/89646
* gfortran.dg/pr89646.f90: New test.
--
Steve
Index: gcc/fortran/dependency.c
===
--- gcc/fortran/dependency.c (revision 272257)
+++ gcc/fortran/dependency.c (working copy)
@@ -979,10 +979,14 @@ gfc_check_argument_var_dependency (gfc_expr *var, sym_
If a dependency is found in the case
elemental == ELEM_CHECK_VARIABLE, we will generate
a temporary, so we don't need to bother the user. */
- gfc_warning (0, "INTENT(%s) actual argument at %L might "
- "interfere with actual argument at %L.",
- intent == INTENT_OUT ? "OUT" : "INOUT",
- &var->where, &expr->where);
+
+ if (var->expr_type == EXPR_VARIABLE
+ && expr->expr_type == EXPR_VARIABLE
+ && strcmp(var->symtree->name, expr->symtree->name) == 0)
+ gfc_warning (0, "INTENT(%s) actual argument at %L might "
+ "interfere with actual argument at %L.",
+ intent == INTENT_OUT ? "OUT" : "INOUT",
+ &var->where, &expr->where);
}
return 0;
}
Index: gcc/testsuite/gfortran.dg/pr89646.f90
===
--- gcc/testsuite/gfortran.dg/pr89646.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr89646.f90 (working copy)
@@ -0,0 +1,24 @@
+! { dg-do compile }
+! PR fortran/89646
+! Original testcase contributed by Ian Harvey
+!
+! This code use to give spurious warnings about aliasing.
+!
+module m
+ implicit none
+ type :: t
+ end type t
+ contains
+ ! To reproduce, both actual arguments must be TARGET,
+ ! both arguments must be of derived type.
+ subroutine s
+ type(t), target :: a(5)
+ type(t), target :: b(5)
+ call move(a, b)
+ end subroutine s
+ ! To reproduce, called procedure must be elemental.
+ elemental subroutine move(x, y)
+ type(t), intent(inout) :: x
+ type(t), intent(out) :: y
+ end subroutine move
+end module m
Re: [Patch, fortran] PR90577, PR90578 - FAIL: gfortran.dg/lrshift_1.f90 & Wrong code with LSHIFT and optimization
On Thu, Jun 13, 2019 at 11:50:23PM +0200, Harald Anlauf wrote: > > it was already discussed in the above PRs that the testcase > lrshift_1.f90 needs to be corrected because it invokes negative > values of the SHIFT argument. This is fixed, as well as the > related documentation, which did not match e.g. the Fortran standard. > > Looking closer at the actual implementation, it also turned out that the > runtime implementation of SHIFTA/RSHIFT gave different (=wrong) results > when SHIFT==BIT_SIZE(arg1). This is fixed and tested by the attached > patches. > > I did not implement any run-time checks for the SHIFT argument. > All previous undefined behavior stays the same. > > OK for trunk? > Looks good to me. -- Steve
V3: [PATCH] Update preferred_stack_boundary only when expanding function call
On Sat, Jun 8, 2019 at 12:14 AM Richard Sandiford wrote: > > "H.J. Lu" writes: > > On Fri, Jun 7, 2019 at 1:22 AM Richard Biener wrote: > >> > >> On Fri, 7 Jun 2019, Richard Sandiford wrote: > >> > >> > "H.J. Lu" writes: > >> > > locate_and_pad_parm is called when expanding function call from > >> > > initialize_argument_information and when generating function body > >> > > from assign_parm_find_entry_rtl: > >> > > > >> > > /* Remember if the outgoing parameter requires extra alignment on the > >> > > calling function side. */ > >> > > if (crtl->stack_alignment_needed < boundary) > >> > > crtl->stack_alignment_needed = boundary; > >> > > if (crtl->preferred_stack_boundary < boundary) > >> > > crtl->preferred_stack_boundary = boundary; > >> > > > >> > > stack_alignment_needed and preferred_stack_boundary should be updated > >> > > only when expanding function call, not when generating function body. > >> > > Add an argument, outgoing_p, to locate_and_pad_parm to indicate for > >> > > expanding function call. Update stack_alignment_needed and > >> > > preferred_stack_boundary if the parameter is passed on stack and only > >> > > when expanding function call. > >> > > > >> > > Tested on Linux/x86-64. > >> > > > >> > > OK for trunk? > >> > > > >> > > Thanks. > >> > > > >> > > -- > >> > > H.J. > >> > > > >> > > From e91e20ad8e10373db2c6d8f99a3da0bbf46c5c34 Mon Sep 17 00:00:00 2001 > >> > > From: "H.J. Lu" > >> > > Date: Wed, 5 Jun 2019 12:55:19 -0700 > >> > > Subject: [PATCH] Update preferred_stack_boundary only when expanding > >> > > function > >> > > call > >> > > > >> > > locate_and_pad_parm is called when expanding function call from > >> > > initialize_argument_information and when generating function body > >> > > from assign_parm_find_entry_rtl: > >> > > > >> > > /* Remember if the outgoing parameter requires extra alignment on the > >> > > calling function side. */ > >> > > if (crtl->stack_alignment_needed < boundary) > >> > > crtl->stack_alignment_needed = boundary; > >> > > if (crtl->preferred_stack_boundary < boundary) > >> > > crtl->preferred_stack_boundary = boundary; > >> > > > >> > > stack_alignment_needed and preferred_stack_boundary should be updated > >> > > only when expanding function call, not when generating function body. > >> > > Add an argument, outgoing_p, to locate_and_pad_parm to indicate for > >> > > expanding function call. Update stack_alignment_needed and > >> > > preferred_stack_boundary if the parameter is passed on stack and only > >> > > when expanding function call. > >> > > > >> > > Tested on Linux/x86-64. > >> > > > >> > > gcc/ > >> > > > >> > > PR rtl-optimization/90765 > >> > > * function.c (assign_parm_find_entry_rtl): Pass false to > >> > > locate_and_pad_parm. > >> > > (locate_and_pad_parm): Add an argument, outgoing_p, to indicate > >> > > for expanding function call. Update stack_alignment_needed and > >> > > preferred_stack_boundary only if outgoing_p is true and the > >> > > the parameter is passed on stack. > >> > > * function.h (locate_and_pad_parm): Add an argument, outgoing_p, > >> > > defaulting to true. > >> > > > >> > > gcc/testsuite/ > >> > > > >> > > PR rtl-optimization/90765 > >> > > * gcc.target/i386/pr90765-1.c: New test. > >> > > * gcc.target/i386/pr90765-2.c: Likewise. > >> > > --- > >> > > gcc/function.c| 21 + > >> > > gcc/function.h| 3 ++- > >> > > gcc/testsuite/gcc.target/i386/pr90765-1.c | 11 +++ > >> > > gcc/testsuite/gcc.target/i386/pr90765-2.c | 18 ++ > >> > > 4 files changed, 44 insertions(+), 9 deletions(-) > >> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-1.c > >> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr90765-2.c > >> > > > >> > > diff --git a/gcc/function.c b/gcc/function.c > >> > > index e30ee259bec..9b6673f6f0d 100644 > >> > > --- a/gcc/function.c > >> > > +++ b/gcc/function.c > >> > > @@ -2601,7 +2601,7 @@ assign_parm_find_entry_rtl (struct > >> > > assign_parm_data_all *all, > >> > >locate_and_pad_parm (data->promoted_mode, data->passed_type, > >> > > in_regs, > >> > >all->reg_parm_stack_space, > >> > >entry_parm ? data->partial : 0, > >> > > current_function_decl, > >> > > - &all->stack_args_size, &data->locate); > >> > > + &all->stack_args_size, &data->locate, false); > >> > > > >> > >/* Update parm_stack_boundary if this parameter is passed in the > >> > > stack. */ > >> > > @@ -3954,7 +3954,8 @@ locate_and_pad_parm (machine_mode passed_mode, > >> > > tree type, int in_regs, > >> > > int reg_parm_stack_space, int partial, > >> > > tree fndecl ATTRIBUTE_UNUSED, > >> > > struct args_size *initial_offset_ptr, > >> > > -struct locate_and_pad_arg
[PATCH] improve strcmp folding of unequal strings (PR 90626)
While integrating the strlen and sprintf passes and investigating
optimization opportunities that it opens up I noticed a few related
to a strcmp optimization implemented in GCC 9. One is to take
advantage of the fact that a nul-terminated string of a known
length cannot be equal to a string whose length is greater, even
if it isn't known exactly. For example, the equality below must
be false:
int f (char * restrict a, char * restrict b)
{
memcpy (a, "1234", 4); // length >= 4
strcpy (b, "123"); // length == 3
return strcmp (a, b) == 0; // must be false
}
The attached patch enhances the existing optimization to exploit
this opportunity.
Tested on x86_64-linux.
Martin
PS There are several more improvements to be made here. I've been
raising bugs for them and I plan to submit patches for them in
the near future. (Incidentally, they are all in the spirit of
the suggestion made back in 2012 in pr52171.)
PR tree-optimization/90626 - fold strcmp(a, b) == 0 to zero when one string length is exact and the other is unequal
gcc/ChangeLog:
PR tree-optimization/90626
* tree-ssa-strlen.c (strxcmp_unequal): New function.
(handle_builtin_string_cmp): Call it.
gcc/testsuite/ChangeLog:
PR tree-optimization/90626
* gcc.dg/strlenopt-65.c: New test.
* gcc.dg/strlenopt-66.c: New test.
* gcc.dg/strlenopt.h (strcmp, strncmp): Declare.
diff --git a/gcc/testsuite/gcc.dg/strlenopt-65.c b/gcc/testsuite/gcc.dg/strlenopt-65.c
new file mode 100644
index 000..a34d178faa1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-65.c
@@ -0,0 +1,162 @@
+/* PRE tree-optimization/90626 - fold strcmp(a, b) == 0 to zero when
+ one string length is exact and the other is unequal
+ { dg-do compile }
+ { dg-options "-O2 -Wall -fdump-tree-optimized" } */
+
+#include "strlenopt.h"
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void abort (void);
+extern void* memcpy (void *, const void *, size_t);
+extern int strcmp (const char *, const char *);
+extern int strncmp (const char *, const char *, size_t);
+
+#define CAT(x, y) x ## y
+#define CONCAT(x, y) CAT (x, y)
+#define FAILNAME(name) CONCAT (call_ ## name ##_on_line_, __LINE__)
+
+#define FAIL(name) do {\
+extern void FAILNAME (name) (void); \
+FAILNAME (name)();\
+ } while (0)
+
+/* Macro to emit a call to function named
+ call_in_true_branch_not_eliminated_on_line_NNN()
+ for each call that's expected to be eliminated. The dg-final
+ scan-tree-dump-time directive at the bottom of the test verifies
+ that no such call appears in output. */
+#define ELIM_IF_TRUE(expr) \
+ if (!(expr)) FAIL (in_true_branch_not_eliminated); else (void)0
+
+/* Macro to emit a call to a function named
+ call_made_in_{true,false}_branch_on_line_NNN()
+ for each call that's expected to be retained. The dg-final
+ scan-tree-dump-time directive at the bottom of the test verifies
+ that the expected number of both kinds of calls appears in output
+ (a pair for each line with the invocation of the KEEP() macro. */
+#define TEST_KEEP(expr)\
+ if (expr) \
+FAIL (made_in_true_branch); \
+ else \
+FAIL (made_in_false_branch)
+
+#define FOLD(init1, init2, arg1, arg2, bnd) \
+ do { \
+char a[8], b[8]; \
+sink (a, b); \
+memcpy (a, init1, sizeof init1 - 1); \
+memcpy (b, init2, sizeof init2 - 1); \
+ELIM_IF_TRUE (0 != CMPFUNC (arg1, arg2, bnd)); \
+ } while (0)
+
+#define KEEP(init1, init2, arg1, arg2, bnd) \
+ do { \
+char a[8], b[8];\
+sink (a, b);\
+memcpy (a, init1, sizeof init1 - 1); \
+memcpy (b, init2, sizeof init2 - 1); \
+TEST_KEEP (0 == CMPFUNC (arg1, arg2, bnd)); \
+ } while (0)
+
+const char s0[1] = "";
+const char s00[2] = "\0";
+const char s10[2] = "1";
+const char s20[2] = "2";
+
+void sink (void*, ...);
+
+void test_strcmp_elim (void)
+{
+#undef CMPFUNC
+#define CMPFUNC(a, b, dummy) strcmp (a, b)
+
+ FOLD (s00, s10, "\0", "1", -1);
+ FOLD (s00, s10, "\0", b, -1);
+ FOLD (s00, s10, "\0", s10, -1);
+
+ FOLD (s00, s10, s0, "1", -1);
+ FOLD (s00, s10, s0, b, -1);
+ FOLD (s00, s10, s0, s10, -1);
+
+ FOLD ("\0", "1", s0, "1", -1);
+ FOLD ("\0", "1", s0, b, -1);
+ FOLD ("\0", "1", s0, s10, -1);
+
+ FOLD ("2", "\0", "2", "\0", -1);
+ FOLD ("2", "\0", s20, s0, -1);
+
+ FOLD ("\0", "1", a, b, -1);
+ FOLD ("2", "\0", a, b, -1);
+
+ FOLD ("4\0", "44", a, b, -1);
+ FOLD ("55", "5\0", a, b, -1);
+
+ FOLD ("666\0", "", a, "", -1);
+ FOLD ("666\0", "", a, b, -1);
+ FOLD ("", "777\0", a, b, -1);
+
+ /* Avoid testing substrings of equal length with different characters.
+ The optimization doesn't have access to the contents of the strings
+ so it can't determine whether they are equal.
+
+ FOLD ("111\0", "112", a, b, -1);
+ FOLD ("112", "111\0", a, b, -1); */
+}
+
+const char s123[] = "123";
+const char s1230[] = "123\0";
+
+const char s1234[] = "1234";
+const char s1234
[C++ Patch] More grokdeclarator locations
Hi,
should all be rather straightforward but I spent quite a bit of time on
the testsuite changes... Tested x86_64-linux, as usual.
Thanks, Paolo.
PS: at some point we should start talking about all the error, error_at,
etc, which are *never* exercised by our testsuite. I'm coming to the
conclusion that some should really go. Any general procedural suggestions?
///
/cp
2019-06-14 Paolo Carlini
* decl.c (grokdeclarator): Use id_loc, typespec_loc, and
locations[ds_storage_class] in a few additional places.
/testsuite
2019-06-14 Paolo Carlini
* g++.dg/diagnostic/auto-storage-1.C: New.
* g++.dg/diagnostic/no-type-1.C: Likewise.
* g++.dg/diagnostic/no-type-2.C: Likewise.
* g++.dg/diagnostic/top-level-auto-1.C: Likewise.
* g++.dg/cpp0x/auto9.C: Test some locations too.
* g++.dg/cpp1z/register1.C: Likewise.
* g++.dg/cpp1z/register2.C: Likewise.
* g++.dg/cpp1z/register3.C: Likewise.
* g++.dg/other/error34.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 272259)
+++ cp/decl.c (working copy)
@@ -10797,13 +10797,14 @@ grokdeclarator (const cp_declarator *declarator,
else if (in_system_header_at (input_location) || flag_ms_extensions)
/* Allow it, sigh. */;
else if (! is_main)
- permerror (input_location, "ISO C++ forbids declaration of %qs with no
type", name);
+ permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
+ name);
else if (pedantic)
- pedwarn (input_location, OPT_Wpedantic,
+ pedwarn (id_loc, OPT_Wpedantic,
"ISO C++ forbids declaration of %qs with no type", name);
else
- warning (OPT_Wreturn_type,
-"ISO C++ forbids declaration of %qs with no type", name);
+ warning_at (id_loc, OPT_Wreturn_type,
+ "ISO C++ forbids declaration of %qs with no type", name);
if (type_was_error_mark_node && template_parm_flag)
/* FIXME we should be able to propagate the error_mark_node as is
@@ -11232,7 +11233,8 @@ grokdeclarator (const cp_declarator *declarator,
else if (toplevel_bindings_p ())
{
if (storage_class == sc_auto)
- error ("top-level declaration of %qs specifies %", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "top-level declaration of %qs specifies %", name);
}
else if (thread_p
&& storage_class != sc_extern
@@ -12323,9 +12325,10 @@ grokdeclarator (const cp_declarator *declarator,
&& !(cxx_dialect >= cxx17 && template_parm_flag))
{
if (cxx_dialect >= cxx14)
- error ("% parameter not permitted in this context");
+ error_at (typespec_loc,
+ "% parameter not permitted in this context");
else
- error ("parameter declared %");
+ error_at (typespec_loc, "parameter declared %");
type = error_mark_node;
}
@@ -12746,9 +12749,12 @@ grokdeclarator (const cp_declarator *declarator,
// FIXME:gcc_assert (original_name == dname);
if (storage_class == sc_auto)
- error ("storage class % invalid for function %qs", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "storage class % invalid for function %qs", name);
else if (storage_class == sc_register)
- error ("storage class % invalid for function %qs", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "storage class % invalid for function %qs",
+ name);
else if (thread_p)
{
if (declspecs->gnu_thread_keyword_p)
Index: testsuite/g++.dg/cpp0x/auto9.C
===
--- testsuite/g++.dg/cpp0x/auto9.C (revision 272259)
+++ testsuite/g++.dg/cpp0x/auto9.C (working copy)
@@ -78,10 +78,10 @@ enum struct D : auto * { FF = 0 }; // { dg-error
void
bar ()
{
- try { } catch (auto i) { } // { dg-error "parameter" }
- try { } catch (auto) { } // { dg-error "parameter" }
- try { } catch (auto *i) { } // { dg-error "parameter" }
- try { } catch (auto *) { } // { dg-error "parameter" }
+ try { } catch (auto i) { } // { dg-error "18:parameter" }
+ try { } catch (auto) { } // { dg-error "18:parameter" }
+ try { } catch (auto *i) { } // { dg-error "18:parameter" }
+ try { } catch (auto *) { } // { dg-error "18:parameter" }
}
void
@@ -111,7 +111,7 @@ badthrow2 () throw (auto &) // { dg-error
"inval
{ // { dg-error "dynamic
exception specification" "" { target c++17 } .-1 }
}
C++ PATCH for c++/90881 - bogus -Wunused-value in unevaluated context
As Jon points out in the PR, emitting -Wunused warnings in unevaluated contexts
doesn't make a whole lot of sense, because in such contexts, we're not reading
the values in any case. Disabling this particular warning is trivial. There
are likely other warnings like this, but I haven't audited them.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-13 Marek Polacek
PR c++/90881 - bogus -Wunused-value in unevaluated context.
* cvt.c (convert_to_void): Don't emit unused warnings in
an unevaluated context.
* g++.dg/cpp0x/Wunused-value1.C: New test.
diff --git gcc/cp/cvt.c gcc/cp/cvt.c
index 1c405ecd7b6..23d2aabc483 100644
--- gcc/cp/cvt.c
+++ gcc/cp/cvt.c
@@ -1518,7 +1518,8 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
if (implicit != ICV_CAST
&& warn_unused_value
&& !TREE_NO_WARNING (expr)
- && !processing_template_decl)
+ && !processing_template_decl
+ && !cp_unevaluated_operand)
{
/* The middle end does not warn about expressions that have
been explicitly cast to void, so we must do so here. */
diff --git gcc/testsuite/g++.dg/cpp0x/Wunused-value1.C
gcc/testsuite/g++.dg/cpp0x/Wunused-value1.C
new file mode 100644
index 000..a0683b693a1
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wunused-value1.C
@@ -0,0 +1,20 @@
+// PR c++/90881
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wall" }
+
+namespace std {
+ struct true_type { static const bool value = true; };
+ struct false_type { static const bool value = false; };
+}
+
+template struct status : std::false_type{};
+
+template struct status :
std::true_type {}; // { dg-bogus "left operand of comma operator has no effect"
}
+
+struct s1{int member;};
+struct s2{int _member;};
+
+int main(){
+ static_assert(status::value, "has member");
+ static_assert(!status::value, "has no member");
+}
Re: [PATCH] Do not warn with warn_unused_result for alloca(0).
On 6/13/19 10:46 AM, Tom Tromey wrote: >> "Jeff" == Jeff Law writes: > > Jeff> I'd like to move C-alloca support to the ash heap of history. But I'm > Jeff> not sure we can realistically do that. > > Are there still platforms or compilers in use where it's needed? > > For gdb I was planning to just remove these calls. Probably not that we care about for building gcc, gdb & friends. I'd be more hesitant to declare C-alloca dead in the more general sense. jeff
Re: [PATCH] avoid ice due to inconsistent argument types to fold_build (PR 90662)
On 6/13/19 1:10 PM, Martin Sebor wrote: > Attached is a fix for the fold_build call with inconsistent > argument types introduced in a recent commit of mine. > > Tested on x86_64-linux. > > Martin > > gcc-90662.diff > > PR tree-optimization/90662 - strlen of a string in a vla plus offset not > folded > > gcc/ChangeLog: > > PR tree-optimization/90662 > * tree-ssa-strlen.c (get_stridx): Convert fold_build2 operands > to the same type. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/90662 > * gcc.dg/pr90866-2.c: New test. > * gcc.dg/pr90866.c: Ditto. OK jeff
Re: indirect_ref_may_alias_decl_p fix
Hi Jan, > I am testing the following patch which solves the bogus warning in > tree-ssa-forwprop.c and -m32 and plan to commit it as obvoius to unbreak > bootstrap once testing converges. Previously we did not get here wince > we got mismatch between TMR type and decl type but same code is present > in indirect_ref_may_alias_p. > > Honza > > PR bootstrap/90873 > * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Also check that > dbase is not TARGET_MEM_REF. I've included the patch in nightly bootstraps on i386-pc-solaris2.11, sparc-sun-solaris2.11, and i686-pc-linux-gnu. All completed without regressions, thanks. One last issue, though. The error messages totally feel like line noise to me: /vol/gcc/src/hg/trunk/local/gcc/tree-ssa-forwprop.c: In function 'bool simplify_rotate(gimple_stmt_iterator*)': /vol/gcc/src/hg/trunk/local/gcc/tree-ssa-forwprop.c:1650:40: error: '#'target_mem_ref' not supported by dump_expr#' may be used uninitialized in this function [-Werror=maybe-uninitialized] 1650 | if (cdef_arg2[i] == def_arg2[1 - i] | ~~^ Is this really something we mean to expose to users? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
