Re: [PATCH] Fortran : Fill in missing array dimensions using the lower, bound (for review)
Hi Mark, Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec. A warning that the lower bound is being used for a mission dimension is output unless suppressed by using -Wno-missing-index. This is... seriously problematic. I forsee all sorts of not-so-funny interactions with more modern features. What do people actually do with this kind of code? What kind of test cases do you have that "work" with this? And people would actually want to save a few keystrokes so they don't have to write A(N,M,1) instead of A(N,M)? And is this even the right fix, how sure are you of the semantics; is there documentation for this feature (maybe on Bitsavers)? If not, this is not be done. If this goes in at all, I want this rejected with any modern Fortran feature, i.e. it should not be contain - allocatable arrays - coarrays - pointers - derived types - CLASS - assumed-shape arrays - assumed-rank arrays (well, it probably doesn't make sense) - KIND=4 characters - as an argument to any of the array intrinsics like MATMUL, EOSHIFT, ... but even with these restrictions, I will still take a lot of convincing that this make sense. Just imagine what will happen if people specify -fdec for some relatively benign reason (for example because they want -fdec-math) and start not finding bugs in their code because of this feature. Best regards Thomas
Re: [PATCH] PR fortran/95881 - [9/10/11 Regression] ICE in resolve_symbol, at fortran/resolve.c:15175
Hi Harald, here's another NULL pointer dereference on invalid code. Regtested on x86_64-pc-linux-gnu. OK for master / backports where appropriate? OK. (Also would have classified as obvious and simple, I think). Thanks for the patch Regards Thomas
[PATCH] c-family: Use TYPE_OVERFLOW_UNDEFINED instead of !TYPE_UNSIGNED in pointer_sum [PR95903]
Hi!
For lp64 targets and int off ... ptr[off + 1]
is lowered in pointer_sum to *(ptr + ((sizetype) off + (sizetype) 1)).
That is fine when signed integer wrapping is undefined (and is not done
already if off has unsigned type), but changes behavior for -fwrapv, where
overflow is well defined. Runtime test could be:
int
main ()
{
char *p = __builtin_malloc (0x1UL);
if (!p) return 0;
char *q = p + 0x8000UL;
int o = __INT_MAX__;
q[o + 1] = 1;
if (q[-__INT_MAX__ - 1] != 1) __builtin_abort ();
return 0;
}
with -fwrapv or so, not included in the testsuite because it requires 4GB
allocation (with some other test it would be enough to have something
slightly above 2GB, but still...).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
On the other side, it would be nice with undefined signed integer overflow
to optimize
extern signed char arr[];
int foo (int off) { off++; return arr[off]; }
at least on targets that have addressing that can include both index and
some immediate, right now we emit
addl$1, %edi
movslq %edi, %rdi
movsbl arr(%rdi), %eax
but we could emit
movslq %edi, %rdi
movsbl arr+1(%rdi), %eax
Not sure where to do that; after expansion it is too late, because signed
vs. unsigned addition is lost at that point, so maybe the isel pass or some
other late pass? Wonder if ivopts is able to do that, but ivopts doesn't
work outside of loops, right?
2020-06-27 Jakub Jelinek
PR middle-end/95903
* c-common.c (pointer_int_sum): Use TYPE_OVERFLOW_UNDEFINED instead of
!TYPE_UNSIGNED check to see if we can apply distributive law and handle
smaller precision intop operands separately.
* c-c++-common/pr95903.c: New test.
--- gcc/c-family/c-common.c.jj 2020-06-18 11:26:09.522400264 +0200
+++ gcc/c-family/c-common.c 2020-06-26 10:28:10.385144734 +0200
@@ -3141,7 +3141,7 @@ pointer_int_sum (location_t loc, enum tr
/* If the constant is unsigned, and smaller than the pointer size,
then we must skip this optimization. This is because it could cause
an overflow error if the constant is negative but INTOP is not. */
- && (!TYPE_UNSIGNED (TREE_TYPE (intop))
+ && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (intop))
|| (TYPE_PRECISION (TREE_TYPE (intop))
== TYPE_PRECISION (TREE_TYPE (ptrop)
{
--- gcc/testsuite/c-c++-common/pr95903.c.jj 2020-06-26 10:43:19.325937680
+0200
+++ gcc/testsuite/c-c++-common/pr95903.c2020-06-26 10:47:24.017382649
+0200
@@ -0,0 +1,19 @@
+/* PR middle-end/95903 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -fwrapv -fdump-tree-optimized" } */
+/* Verify that for -fwrapv the + 1 addition is performed in the parameter's
+ type before sign extending it. */
+/* { dg-final { scan-tree-dump-times "off_\[0-9]+\\\(D\\\) \\+ 1" 2
"optimized" } } */
+
+char
+foo (const char *ptr, int off)
+{
+ off += 1;
+ return ptr[off];
+}
+
+char
+bar (const char *ptr, int off)
+{
+ return ptr[off + 1];
+}
Jakub
[pushed] coroutines: Improve diagnostics for one allocator case.
Hi
This adds a diagnostic only,
tested on x86_64-linux,darwin powerpc64-linux,
applied to master as obvious,
thanks
Iain
-
If the user provides operator new and that is noexcept, this
implies that it can fail with a null return. At that point, we expect
to be able to call get_return_object_on_allocation_failure().
This diagnoses the case where such an operator new has been
provided, but the g-r-o-o-a-f is either missing or unusable.
gcc/cp/ChangeLog:
* coroutines.cc (morph_fn_to_coro): Diagnose unavailable
get_return_object_on_allocation_failure.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: New test.
---
gcc/cp/coroutines.cc | 4
.../coro-bad-grooaf-01-grooaf-expected.C | 14 ++
2 files changed, 18 insertions(+)
create mode 100644
gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 6e723c402db..8b8d00e8e0c 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4041,6 +4041,10 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
*destroyer)
else if (grooaf && !TYPE_NOTHROW_P (TREE_TYPE (func)))
error_at (fn_start, "%qE is provided by %qT but %qE is not marked"
" % or %", grooaf, promise_type, nwname);
+else if (!grooaf && TYPE_NOTHROW_P (TREE_TYPE (func)))
+ warning_at (fn_start, 0, "%qE is marked % or % but"
+ " no usable %"
+ " is provided by %qT ", nwname, promise_type);
}
else /* No operator new in the promise. */
{
diff --git
a/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
b/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
new file mode 100644
index 000..9fa3d64a9f2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
@@ -0,0 +1,14 @@
+/* g-r-o-o-a-f would be expected, since we have a noexcept op new. */
+
+#define USE_FAILING_OP_NEW
+#include "coro1-allocators.h"
+
+int used_grooaf = 0;
+
+struct coro1
+f () noexcept // { dg-warning {'operator new' is marked 'throw\(\)' or
'noexcept' but no usable 'get_return_object_on_allocation_failure' is provided
by 'std::__n4861::__coroutine_traits_impl::promise_type' \{aka
'coro1::promise_type'\}} }
+{
+ PRINT ("coro1: about to return");
+ co_return;
+}
+
--
2.24.1
Re: [PATCH] c-family: Use TYPE_OVERFLOW_UNDEFINED instead of !TYPE_UNSIGNED in pointer_sum [PR95903]
On June 27, 2020 10:58:56 AM GMT+02:00, Jakub Jelinek wrote:
>Hi!
>
>For lp64 targets and int off ... ptr[off + 1]
>is lowered in pointer_sum to *(ptr + ((sizetype) off + (sizetype) 1)).
>That is fine when signed integer wrapping is undefined (and is not done
>already if off has unsigned type), but changes behavior for -fwrapv,
>where
>overflow is well defined. Runtime test could be:
>int
>main ()
>{
> char *p = __builtin_malloc (0x1UL);
> if (!p) return 0;
> char *q = p + 0x8000UL;
> int o = __INT_MAX__;
> q[o + 1] = 1;
> if (q[-__INT_MAX__ - 1] != 1) __builtin_abort ();
> return 0;
>}
>with -fwrapv or so, not included in the testsuite because it requires
>4GB
>allocation (with some other test it would be enough to have something
>slightly above 2GB, but still...).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>On the other side, it would be nice with undefined signed integer
>overflow
>to optimize
>extern signed char arr[];
>int foo (int off) { off++; return arr[off]; }
>at least on targets that have addressing that can include both index
>and
>some immediate, right now we emit
> addl$1, %edi
> movslq %edi, %rdi
> movsbl arr(%rdi), %eax
>but we could emit
> movslq %edi, %rdi
> movsbl arr+1(%rdi), %eax
>Not sure where to do that; after expansion it is too late, because
>signed
>vs. unsigned addition is lost at that point, so maybe the isel pass or
>some
>other late pass? Wonder if ivopts is able to do that, but ivopts
>doesn't
>work outside of loops, right?
Right. SLSR was supposed to eventually handle this.
Richard.
>2020-06-27 Jakub Jelinek
>
> PR middle-end/95903
> * c-common.c (pointer_int_sum): Use TYPE_OVERFLOW_UNDEFINED instead of
> !TYPE_UNSIGNED check to see if we can apply distributive law and
>handle
> smaller precision intop operands separately.
>
> * c-c++-common/pr95903.c: New test.
>
>--- gcc/c-family/c-common.c.jj 2020-06-18 11:26:09.522400264 +0200
>+++ gcc/c-family/c-common.c2020-06-26 10:28:10.385144734 +0200
>@@ -3141,7 +3141,7 @@ pointer_int_sum (location_t loc, enum tr
> /* If the constant is unsigned, and smaller than the pointer size,
>then we must skip this optimization. This is because it could cause
>an overflow error if the constant is negative but INTOP is not. */
>- && (!TYPE_UNSIGNED (TREE_TYPE (intop))
>+ && (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (intop))
> || (TYPE_PRECISION (TREE_TYPE (intop))
> == TYPE_PRECISION (TREE_TYPE (ptrop)
> {
>--- gcc/testsuite/c-c++-common/pr95903.c.jj2020-06-26
>10:43:19.325937680 +0200
>+++ gcc/testsuite/c-c++-common/pr95903.c 2020-06-26 10:47:24.017382649
>+0200
>@@ -0,0 +1,19 @@
>+/* PR middle-end/95903 */
>+/* { dg-do compile { target lp64 } } */
>+/* { dg-options "-O2 -fwrapv -fdump-tree-optimized" } */
>+/* Verify that for -fwrapv the + 1 addition is performed in the
>parameter's
>+ type before sign extending it. */
>+/* { dg-final { scan-tree-dump-times "off_\[0-9]+\\\(D\\\) \\+ 1" 2
>"optimized" } } */
>+
>+char
>+foo (const char *ptr, int off)
>+{
>+ off += 1;
>+ return ptr[off];
>+}
>+
>+char
>+bar (const char *ptr, int off)
>+{
>+ return ptr[off + 1];
>+}
>
> Jakub
[committed] openmp: Non-rectangular loop support for non-composite worksharing loops and distribute
Hi!
This implements the fallback mentioned in
https://gcc.gnu.org/pipermail/gcc/2020-June/232874.html
Special cases for triangular loops etc. to follow later, also composite
constructs not supported yet (need to check the passing of temporaries around)
and lastprivate might not give the same answers as serial loop if the last
innermost body iteration isn't the last one for some of the outer loops
(that will need to be solved separately together with rectangular loops that
have no
innermost body iterations, but some of the outer loops actually iterate).
Also, simd needs work.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2020-06-27 Jakub Jelinek
* omp-general.h (struct omp_for_data_loop): Add non_rect_referenced
member, move outer member.
(struct omp_for_data): Add first_nonrect and last_nonrect members.
* omp-general.c (omp_extract_for_data): Initialize first_nonrect,
last_nonrect and non_rect_referenced members.
* omp-expand.c (expand_omp_for_init_counts): Handle non-rectangular
loops.
(expand_omp_for_init_vars): Add nonrect_bounds parameter. Handle
non-rectangular loops.
(extract_omp_for_update_vars): Likewise.
(expand_omp_for_generic, expand_omp_for_static_nochunk,
expand_omp_for_static_chunk, expand_omp_simd,
expand_omp_taskloop_for_outer, expand_omp_taskloop_for_inner): Adjust
expand_omp_for_init_vars and extract_omp_for_update_vars callers.
(expand_omp_for): Don't sorry on non-composite worksharing-loop or
distribute.
* testsuite/libgomp.c/loop-17.c: New test.
* testsuite/libgomp.c/loop-18.c: New test.
--- gcc/omp-general.h.jj2020-06-23 15:58:46.608022000 +0200
+++ gcc/omp-general.h 2020-06-24 15:08:07.283089199 +0200
@@ -47,13 +47,16 @@ enum oacc_loop_flags {
or for non-rectangular loops:
for (V = M1 * W + N1; V cond M2 * W + N2; V += STEP;
where W is V of the OUTER-th loop (e.g. for OUTER 1 it is the
- the index of the immediately surrounding loop). */
+ the index of the immediately surrounding loop).
+ NON_RECT_REFERENCED is true for loops referenced by loops
+ with non-NULL M1 or M2. */
struct omp_for_data_loop
{
tree v, n1, n2, step, m1, m2;
- int outer;
enum tree_code cond_code;
+ int outer;
+ bool non_rect_referenced;
};
/* A structure describing the main elements of a parallel loop. */
@@ -67,6 +70,7 @@ struct omp_for_data
tree tiling; /* Tiling values (if non null). */
int collapse; /* Collapsed loops, 1 for a non-collapsed loop. */
int ordered;
+ int first_nonrect, last_nonrect;
bool have_nowait, have_ordered, simd_schedule, have_reductemp;
bool have_pointer_condtemp, have_scantemp, have_nonctrl_scantemp;
bool non_rect;
--- gcc/omp-general.c.jj2020-06-23 15:58:46.594022202 +0200
+++ gcc/omp-general.c 2020-06-24 16:58:37.77351 +0200
@@ -206,6 +206,8 @@ omp_extract_for_data (gomp_for *for_stmt
fd->tiling = NULL_TREE;
fd->collapse = 1;
fd->ordered = 0;
+ fd->first_nonrect = -1;
+ fd->last_nonrect = -1;
fd->sched_kind = OMP_CLAUSE_SCHEDULE_STATIC;
fd->sched_modifiers = 0;
fd->chunk_size = NULL_TREE;
@@ -372,18 +374,24 @@ omp_extract_for_data (gomp_for *for_stmt
loop->m1 = NULL_TREE;
loop->m2 = NULL_TREE;
loop->outer = 0;
+ loop->non_rect_referenced = false;
if (TREE_CODE (loop->n1) == TREE_VEC)
{
for (int j = i - 1; j >= 0; j--)
if (TREE_VEC_ELT (loop->n1, 0) == gimple_omp_for_index (for_stmt,
j))
{
loop->outer = i - j;
+ if (loops != NULL)
+ loops[j].non_rect_referenced = true;
+ if (fd->first_nonrect == -1 || fd->first_nonrect > j)
+ fd->first_nonrect = j;
break;
}
gcc_assert (loop->outer);
loop->m1 = TREE_VEC_ELT (loop->n1, 1);
loop->n1 = TREE_VEC_ELT (loop->n1, 2);
fd->non_rect = true;
+ fd->last_nonrect = i;
}
loop->cond_code = gimple_omp_for_cond (for_stmt, i);
@@ -401,12 +409,17 @@ omp_extract_for_data (gomp_for *for_stmt
if (TREE_VEC_ELT (loop->n2, 0) == gimple_omp_for_index (for_stmt,
j))
{
loop->outer = i - j;
+ if (loops != NULL)
+ loops[j].non_rect_referenced = true;
+ if (fd->first_nonrect == -1 || fd->first_nonrect > j)
+ fd->first_nonrect = j;
break;
}
gcc_assert (loop->outer);
loop->m2 = TREE_VEC_ELT (loop->n2, 1);
loop->n2 = TREE_VEC_ELT (loop->n2, 2);
fd->non_rect = true;
+ fd->last_nonrect = i;
}
t = gimple_omp_for_incr (for_stmt, i);
--- gcc/omp-expand.c.jj 2020-06-23 15:58:46.593022216 +0200
+++ gcc/omp-expand.c2020-06-26 21:
Re: [RFC patch] Clean all (sub)?module files
Hi Dominieque, While investigating pr95538, I see several module files that were not cleaned. Several were cleaned by a patch I had in my working directory. However new ones were not cleaned (e.g., gfortran.dg/pr95091.f90) due to continuation lines. This is now fixed with the attached patch (patch-mod). Thanks for working on this. My problem is that I my dejagnu-fu is almost nonexistent, so I could, in theory, review and commit this, but I do not really understand what you did. So, maybe if somebody more knowledgable about this could this could comment on https://gcc.gnu.org/pipermail/fortran/2020-June/054533.html ? Best regards Thomas
Re: [PATCH] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p.
On Sat, 27 Jun 2020, Ilya Leoshkevich via Gcc-patches wrote:
Is there something specific that a compiler user should look out for?
For example, here is the kernel code, from which the test was derived:
static inline void atomic_add(int i, atomic_t *v)
{
#ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
if (__builtin_constant_p(i) && (i > -129) && (i < 128)) {
__atomic_add_const(i, &v->counter);
return;
}
#endif
__atomic_add(i, &v->counter);
}
It looks very straightforward - can there still be something wrong
with its usage of b_c_p?
I'd recommend looking at the .ssa dump and walk forward from there if
the .ssa dump looks correct.
Well, 021t.ssa already has:
__attribute__((gnu_inline))
__atomic_add_const (intD.6 valD.2269, intD.6 * ptrD.2270)
{
intD.6 val_3(D) = valD.2269;
intD.6 * ptr_2(D) = ptrD.2270;
;; basic block 2, loop depth 0, maybe hot
;;prev block 0, next block 1, flags: (NEW)
;;pred: ENTRY (FALLTHRU)
# .MEM_4 = VDEF <.MEM_1(D)>
__asm__ __volatile__("asi %0,%1
" : "ptr" "=Q" *ptr_2(D) : "val" "i" val_3(D), "Q" *ptr_2(D) :
"memory", "cc");
# VUSE <.MEM_4>
return;
;;succ: EXIT
}
which is, strictly speaking, not correct, because val_3(D) and
valD.2269 are not constant. But as far as I understand we are willing
to tolerate trees like this until a certain point.
What is this point supposed to be? If I understood you right,
106t.thread1 is already too late - why is it so?
Small remark: shouldn't __atomic_add_const be marked with the
always_inline attribute, since it isn't usable when it isn't inlined?
--
Marc Glisse
Re: [PATCH PR95700] Use nullptr instead of NULL as a sentinel value
Ilya Leoshkevich via Gcc-patches writes: > Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-linux > and s390x-redhat-linux. Agree we should do this FWIW, but as a belt-and-braces fix, would it make sense to define NULL to nullptr in system.h for all hosts? Currently we have: /* Define a generic NULL if one hasn't already been defined. */ #ifndef NULL #define NULL 0 #endif which we might be able to change to: #undef NULL #define NULL nullptr The current position is probably too early though. I think it should instead be after all system headers have been included, so that there's no chance of a multiple definition error, and no risk that our definition confuses the system headers. This of course relies on files sticking to the “don't include system headers directly“ rule, and using INCLUDE_* macros instead. (Which isn't pretty, but that's where we are…) Thanks, Richard
Re: [PATCH] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p.
On Sat, 2020-06-27 at 13:38 +0200, Marc Glisse wrote:
> On Sat, 27 Jun 2020, Ilya Leoshkevich via Gcc-patches wrote:
>
> > Is there something specific that a compiler user should look out
> > for?
> > For example, here is the kernel code, from which the test was
> > derived:
> >
> > static inline void atomic_add(int i, atomic_t *v)
> > {
> > #ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
> >if (__builtin_constant_p(i) && (i > -129) && (i < 128)) {
> >__atomic_add_const(i, &v->counter);
> >return;
> >}
> > #endif
> >__atomic_add(i, &v->counter);
> > }
> >
> > It looks very straightforward - can there still be something wrong
> > with its usage of b_c_p?
> >
> > > I'd recommend looking at the .ssa dump and walk forward from
> > > there if
> > > the .ssa dump looks correct.
> >
> > Well, 021t.ssa already has:
> >
> > __attribute__((gnu_inline))
> > __atomic_add_const (intD.6 valD.2269, intD.6 * ptrD.2270)
> > {
> > intD.6 val_3(D) = valD.2269;
> > intD.6 * ptr_2(D) = ptrD.2270;
> > ;; basic block 2, loop depth 0, maybe hot
> > ;;prev block 0, next block 1, flags: (NEW)
> > ;;pred: ENTRY (FALLTHRU)
> > # .MEM_4 = VDEF <.MEM_1(D)>
> > __asm__ __volatile__("asi %0,%1
> > " : "ptr" "=Q" *ptr_2(D) : "val" "i" val_3(D), "Q" *ptr_2(D) :
> > "memory", "cc");
> > # VUSE <.MEM_4>
> > return;
> > ;;succ: EXIT
> >
> > }
> >
> > which is, strictly speaking, not correct, because val_3(D) and
> > valD.2269 are not constant. But as far as I understand we are
> > willing
> > to tolerate trees like this until a certain point.
> >
> > What is this point supposed to be? If I understood you right,
> > 106t.thread1 is already too late - why is it so?
>
> Small remark: shouldn't __atomic_add_const be marked with the
> always_inline attribute, since it isn't usable when it isn't inlined?
I agree, this would be a good improvement (from both readability and
correctness perspectives).
Still, I just tried it, and unfortunately it did not help with the
issue at hand, most likely because the function is inlined either way.
021t.ssa still contains:
__attribute__((always_inline, gnu_inline))
__atomic_add_const (intD.6 valD.2269, intD.6 * ptrD.2270)
{
and threading still eliminates its inlined version.
Re: [PATCH] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p.
On Fri, 26 Jun 2020, Jeff Law via Gcc-patches wrote:
In theory yes, but there are cases where paths converge (like you've shown)
where
you may have evaluated to a constant on the paths, but it's not a constant at
the
convergence point. You have to be very careful using b_c_p like this and it's
been a regular source of kernel bugs.
I'd recommend looking at the .ssa dump and walk forward from there if the .ssa
dump looks correct.
Here is the last dump before thread1 (105t.mergephi2). I don't see
anything incorrect in it.
ledtrig_cpu (_Bool is_active)
{
int old;
int iftmp.0_1;
int _5;
[local count: 1073741824]:
if (is_active_2(D) != 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 536870913]:
[local count: 1073741824]:
# iftmp.0_1 = PHI <1(2), -1(3)>
_5 = __builtin_constant_p (iftmp.0_1);
if (_5 != 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 536870913]:
if (iftmp.0_1 >= -128)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 268435456]:
if (iftmp.0_1 <= 127)
goto ; [34.00%]
else
goto ; [66.00%]
[local count: 91268056]:
__asm__ __volatile__("asi %0,%1
" : "ptr" "=Q" MEM[(int *)&num_active_cpus] : "val" "i" iftmp.0_1, "Q" MEM[(int *)&num_active_cpus]
: "memory", "cc");
goto ; [100.00%]
[local count: 982473769]:
__asm__ __volatile__("laa %0,%2,%1
" : "old" "=d" old_8, "ptr" "=Q" MEM[(int *)&num_active_cpus] : "val" "d" iftmp.0_1, "Q" MEM[(int
*)&num_active_cpus] : "memory", "cc");
[local count: 1073741824]:
return;
}
There is a single _b_c_p, the immediate asm argument is exactly the
argument of _b_c_p, and it is in the branch protected by _b_c_p.
Now the thread1 dump, for comparison
ledtrig_cpu (_Bool is_active)
{
int old;
int iftmp.0_4;
int iftmp.0_6;
int _7;
int _12;
int iftmp.0_13;
int iftmp.0_14;
[local count: 1073741824]:
if (is_active_2(D) != 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 536870912]:
# iftmp.0_6 = PHI <1(2)>
_7 = __builtin_constant_p (iftmp.0_6);
if (_7 != 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 536870912]:
# iftmp.0_4 = PHI <-1(2)>
_12 = __builtin_constant_p (iftmp.0_4);
if (_12 != 0)
goto ; [50.00%]
else
goto ; [50.00%]
[local count: 268435456]:
if (iftmp.0_4 >= -128)
goto ; [20.00%]
else
goto ; [80.00%]
[local count: 214748364]:
if (iftmp.0_6 <= 127)
goto ; [12.00%]
else
goto ; [88.00%]
[local count: 91268056]:
# iftmp.0_13 = PHI
__asm__ __volatile__("asi %0,%1
" : "ptr" "=Q" MEM[(int *)&num_active_cpus] : "val" "i" iftmp.0_13, "Q" MEM[(int
*)&num_active_cpus] : "memory", "cc");
goto ; [100.00%]
[local count: 982473769]:
# iftmp.0_14 = PHI
__asm__ __volatile__("laa %0,%2,%1
" : "old" "=d" old_8, "ptr" "=Q" MEM[(int *)&num_active_cpus] : "val" "d" iftmp.0_14, "Q" MEM[(int
*)&num_active_cpus] : "memory", "cc");
[local count: 1073741824]:
return;
}
Thread1 decides to separate the paths is_active and !is_active
(surprisingly, for one it optimizes out the comparison <= 127 and for the
other the comparison >= -128, while it could optimize both in both cases).
And it decides to converge after the comparisons, but before the asm.
What the pass did does seem to hurt. It looks like if we duplicate _b_c_p,
we may need to duplicate far enough to include all the blocks dominated by
_b_c_p==true (including the asm, here). Otherwise, any _b_c_p can be
optimized to true, because for a boolean
b is the same as b ? true : false
__builtin_constant_p(b ? true : false) would be the same as b ?
__builtin_constant_p(true) : __builtin_constant_p(false), i.e. true.
It is too bad we don't have any optimization pass using ranges between IPA
and thread1, that would have gotten rid of the comparisons, and hence the
temptation to thread. Adding always_inline on atomic_add (or flatten on
the caller) does help: EVRP removes the comparisons.
Do you see a way forward without changing what thread1 does or declaring
the testcase as unsupported?
--
Marc Glisse
Re: [PATCH] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
On Fri, 26 Jun 2020 at 21:20, Jonathan Wakely wrote:
> For these three tests I think this would be slightly better:
>
> // { dg-additional-options "-Wno-deprecated" { target c++17 } }
>
> That way we only ignore the warning when actually needed.
Sure thing. The test run revealed some additional things to tweak. OK for trunk
and GCC 10?
2020-06-27 Ville Voutilainen
PR libstdc++/95915
* include/std/type_traits (is_literal_type, is_literal_type_v):
Deprecate in C++17.
* include/std/variant (_Uninitialized):
Adjust the condition and the comment.
* testsuite/20_util/is_literal_type/deprecated-1z.cc: New.
* testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc:
Adjust.
* testsuite/20_util/is_literal_type/requirements/typedefs.cc: Likewise.
* testsuite/20_util/is_literal_type/value.cc: Likewise.
* testsuite/20_util/optional/constexpr/nullopt.cc:
Use __is_literal_type directly.
* testsuite/20_util/optional/nullopt.cc: Likewise.
* testsuite/20_util/variable_templates_for_traits.cc: Adjust.
* testsuite/20_util/variant/95915.cc: New.
* testsuite/20_util/variant/compile.cc: Add new test.
* testsuite/experimental/optional/constexpr/nullopt.cc:
Use __is_literal_type directly.
* testsuite/experimental/optional/nullopt.cc: Likewise.
* testsuite/experimental/type_traits/value.cc: Adjust.
* testsuite/util/testsuite_common_types.h:
Use __is_literal_type directly.
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index bc9a45b3746..9cd3a2df41a 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -703,7 +703,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// is_literal_type
template
-struct is_literal_type
+struct
+_GLIBCXX17_DEPRECATED
+is_literal_type
: public integral_constant
{
static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
@@ -3085,10 +3087,11 @@ template
template
_GLIBCXX20_DEPRECATED("use is_standard_layout_v && is_trivial_v instead")
inline constexpr bool is_pod_v = is_pod<_Tp>::value;
-#pragma GCC diagnostic pop
template
+ _GLIBCXX17_DEPRECATED
inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
-template
+#pragma GCC diagnostic pop
+ template
inline constexpr bool is_empty_v = is_empty<_Tp>::value;
template
inline constexpr bool is_polymorphic_v = is_polymorphic<_Tp>::value;
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6eeb3c80ec2..c9504914365 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -202,15 +202,9 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
- // _Uninitialized is guaranteed to be a literal type, even if T is not.
- // We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
- // yet. When it's implemented, _Uninitialized can be changed to the alias
- // to T, therefore equivalent to being removed entirely.
- //
- // Another reason we may not want to remove _Uninitialzied may be that, we
- // want _Uninitialized to be trivially destructible, no matter whether T
- // is; but we will see.
- template>
+ // _Uninitialized is guaranteed to be a trivially destructible type,
+ // even if T is not.
+ template>
struct _Uninitialized;
template
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
new file mode 100644
index 000..a91ff56dcf6
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
@@ -0,0 +1,26 @@
+// Copyright (C) 2020 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] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
On Sat, 27 Jun 2020 at 17:53, Ville Voutilainen
wrote:
>
> On Fri, 26 Jun 2020 at 21:20, Jonathan Wakely wrote:
> > For these three tests I think this would be slightly better:
> >
> > // { dg-additional-options "-Wno-deprecated" { target c++17 } }
> >
> > That way we only ignore the warning when actually needed.
>
> Sure thing. The test run revealed some additional things to tweak. OK for
> trunk
> and GCC 10?
With a twist, I mean. I don't plan to backport the deprecation, just
the bug fix for variant.
New Swedish PO file for 'gcc' (version 10.1.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Swedish team of translators. The file is available at: https://translationproject.org/latest/gcc/sv.po (This file, 'gcc-10.1.0.sv.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH 7/7] PowerPC test: Add prefixed stack protect test
On Sat, Jun 27, 2020 at 01:50:48AM -0400, Michael Meissner wrote:
> On Thu, Jun 25, 2020 at 12:18:42PM -0500, Segher Boessenkool wrote:
> > > +/* { dg-do compile } */
> > > +/* { dg-require-effective-target powerpc_prefixed_addr } */
> >
> > Is this test necessary anymore, does -mcpu=power10 not guarantee that?
>
> I believe the test is necessary to prevent regressions. As I said in the
> test,
> we stumbled on this problem when building GLIBC with -mcpu=future/power10.
We now have
/* Enable -mprefixed by default on power10 systems. */
if (TARGET_POWER10 && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0)
rs6000_isa_flags |= OPTION_MASK_PREFIXED;
so the testsuite can just assume it is enabled on all targets it is
supported for at all.
(The selector doesn't directly *hurt* of course, but it leads to cargo-
culting, future tests using this unneeded selector as well).
Segher
Re: [PATCH 5/7, V2] PowerPC tests: Prefixed insn with large offsets
On Sat, Jun 27, 2020 at 01:55:19AM -0400, Michael Meissner wrote:
> On Thu, Jun 25, 2020 at 12:09:41PM -0500, Segher Boessenkool wrote:
> > On Thu, Jun 04, 2020 at 01:03:51PM -0400, Michael Meissner wrote:
> > > +/* { dg-final { scan-assembler-times {\mpaddi\M|\mpli|\mpla\M} 3 } } */
> >
> > Is there are reason you don't have \M on pli, or is that an oversight?
>
> Just an oversight.
>
> > Okay for trunk with that looked at, and the necessary "no future"
> > changes (and retesting ofc). Thanks!
>
> I simplified the test to use just the instruction (PLI) that is currently
> generated. I was just trying to be general to accomidate possible future code
> generation changes (i.e. instead of loading up the offset with PLI, we could
> potentionally do a PADDI to the base register that was loaded), but we can
> change the test if/when we change the compiler.
That is a good idea, yes. But, please post what you committed, then?
Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Sat, Jun 27, 2020 at 01:49:23AM -0400, Michael Meissner wrote: > On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > > Add tests for -mcpu=future that test the generation of PADDI (and PLI > > > which > > > becomes PADDI). > > > > > > 2020-06-01 Michael Meissner > > > > > > * gcc.target/powerpc/prefix-add.c: New test. > > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > > and the selector names have changed to be something with power10 instead > > of something with future; please retest before commit). > > Done. I was just about to resubmit the patches with the -mcpu=power10 > changes. > I did retest on power8/power9 little endian, and power8 big endian (both > 32/64-bit). In running the tests, I discovered 3 of the tests that needed > ILP64 to test the particular code that I was looking for that I previously > hadn't flagged. Please post what you committed, then? Segher
Re: [PATCH 2/7] PowerPC tests: Add PLI/PADDI tests.
On Sat, Jun 27, 2020 at 01:57:52AM -0400, Michael Meissner wrote: > On Thu, Jun 25, 2020 at 11:52:50AM -0500, Segher Boessenkool wrote: > > On Mon, Jun 01, 2020 at 03:53:37PM -0400, Michael Meissner wrote: > > > Add tests for -mcpu=future that test the generation of PADDI (and PLI > > > which > > > becomes PADDI). > > > > > > 2020-06-01 Michael Meissner > > > > > > * gcc.target/powerpc/prefix-add.c: New test. > > > * gcc.target/powerpc/prefix-si-constant.c: New test. > > > * gcc.target/powerpc/prefix-di-constant.c: New test. > > > > This is okay for trunk (with required changes: -mdejagnu-cpu=power10, > > and the selector names have changed to be something with power10 instead > > of something with future; please retest before commit). > > Since all of these tests are for code that is in GCC 10, can I apply these > patches to the GCC 10 branch after the completion of the -mcpu=power10 changes > for GCC 10 and a suitable waiting period and retest? Sure, thanks! Segher
[PATCH] coroutines: Collect the function body rewrite code.
Hi,
This is a enabler patch that enables a reasonable approach to
fixing 5 reported PRs (it doesn’t fix anything in its own right).
It has been tested on x86_64-linux, darwin, powerpc64-linux
OK for master / 10.2?
thanks
Iain
--
The standard describes a rewrite of the body of the user-authored
function (which wraps it in a try-catch block and provides the
initial and final suspend expressions). The exact arrangement of
this was still in flux right up until the WG21 meeting in Prague and
as a consequence was a bit of a moving target.
The net result was a fragmented implementation of the parts of
this rewrite which is now impeding progress in fixing other issues.
This patch collates the rewrite action into a single function and
carries this out earlier.
gcc/cp/ChangeLog:
* coroutines.cc (expand_one_await_expression): Remove
code dealing with initial suspend.
(build_actor_fn): Remove code special-casing initial
and final suspend. Handle the final suspend and marking
of the coroutine as done.
(coro_rewrite_function_body): New.
(bind_expr_find_in_subtree): Remove.
(coro_body_contains_bind_expr_p): Remove.
(morph_fn_to_coro): Split the rewrite of the original
function into coro_rewrite_function_body and call it.
---
gcc/cp/coroutines.cc | 534 +--
1 file changed, 208 insertions(+), 326 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 54f9cb3b4e4..8e0f0e09b56 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1573,8 +1573,6 @@ expand_one_await_expression (tree *stmt, tree
*await_expr, void *d)
tree awaiter_calls = TREE_OPERAND (saved_co_await, 3);
tree source = TREE_OPERAND (saved_co_await, 4);
- bool is_initial =
-(source && TREE_INT_CST_LOW (source) == (int) INITIAL_SUSPEND_POINT);
bool is_final = (source
&& TREE_INT_CST_LOW (source) == (int) FINAL_SUSPEND_POINT);
bool needs_dtor = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (var));
@@ -1724,16 +1722,6 @@ expand_one_await_expression (tree *stmt, tree
*await_expr, void *d)
resume_label = build_stmt (loc, LABEL_EXPR, resume_label);
append_to_statement_list (resume_label, &stmt_list);
- if (is_initial)
-{
- /* Note that we are about to execute the await_resume() for the initial
-await expression. */
- r = build2_loc (loc, MODIFY_EXPR, boolean_type_node, data->i_a_r_c,
- boolean_true_node);
- r = coro_build_cvt_void_expr_stmt (r, loc);
- append_to_statement_list (r, &stmt_list);
-}
-
/* This will produce the value (if one is provided) from the co_await
expression. */
tree resume_call = TREE_VEC_ELT (awaiter_calls, 2); /* await_resume(). */
@@ -2102,19 +2090,16 @@ static void
build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
tree orig, hash_map *param_uses,
hash_map *local_var_uses,
- vec *param_dtor_list, tree initial_await,
- tree final_await, unsigned body_count, tree frame_size)
+ vec *param_dtor_list,
+ tree fs_label, tree resume_fn_field,
+ unsigned body_count, tree frame_size)
{
verify_stmt_tree (fnbody);
/* Some things we inherit from the original function. */
- tree coro_frame_ptr = build_pointer_type (coro_frame_type);
tree handle_type = get_coroutine_handle_type (orig);
tree self_h_proxy = get_coroutine_self_handle_proxy (orig);
tree promise_type = get_coroutine_promise_type (orig);
tree promise_proxy = get_coroutine_promise_proxy (orig);
- tree act_des_fn_type
-= build_function_type_list (void_type_node, coro_frame_ptr, NULL_TREE);
- tree act_des_fn_ptr = build_pointer_type (act_des_fn_type);
/* One param, the coro frame pointer. */
tree actor_fp = DECL_ARGUMENTS (actor);
@@ -2145,21 +2130,15 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
DECL_CONTEXT (continuation) = actor;
BIND_EXPR_VARS (actor_bind) = continuation;
- /* Update the block associated with the outer scope of the orig fn. */
+ /* Link in the block associated with the outer scope of the re-written
+ function body. */
tree first = expr_first (fnbody);
- if (first && TREE_CODE (first) == BIND_EXPR)
-{
- /* We will discard this, since it's connected to the original scope
-nest. */
- tree block = BIND_EXPR_BLOCK (first);
- if (block) /* For this to be missing is probably a bug. */
- {
- gcc_assert (BLOCK_SUPERCONTEXT (block) == NULL_TREE);
- gcc_assert (BLOCK_CHAIN (block) == NULL_TREE);
- BLOCK_SUPERCONTEXT (block) = top_block;
- BLOCK_SUBBLOCKS (top_block) = block;
- }
-}
+ gcc_checking_assert (first && TREE_CODE (first) == BIND_EXPR);
+ tree block = BIND_EXPR_BLOCK (first);
+ gcc_checking_assert (BLOCK_SU
[PATCH] PR fortran/95880 - [9/10/11 Regression] ICE in gfc_add_type, at fortran/symbol.c:2030
A rather straightforward issue (mis-)referencing the proper symbol name
in an error message.
OK for master / backport?
Thanks,
Harald
PR fortran/95880 - ICE in gfc_add_type, at fortran/symbol.c:2030
The fix for PR39695 did not properly distinguish between procedure names
and other symbols names in errors emitted for invalid code. Fix that.
gcc/fortran/
PR fortran/95880
* symbol.c (gfc_add_type): If sym->ns->proc_name is set, use it,
otherwise fall back to sym->name.
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index ba388ff598d..bfec57ccd9e 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -2027,7 +2027,12 @@ gfc_add_type (gfc_symbol *sym, gfc_typespec *ts, locus *where)
|| (flavor == FL_PROCEDURE && sym->attr.subroutine)
|| flavor == FL_DERIVED || flavor == FL_NAMELIST)
{
- gfc_error ("Symbol %qs at %L cannot have a type", sym->ns->proc_name->name, where);
+ if (sym->ns->proc_name)
+ gfc_error ("Symbol %qs at %L cannot have a type",
+ sym->ns->proc_name->name, where);
+ else
+ gfc_error ("Symbol %qs at %L cannot have a type",
+ sym->name, where);
return false;
}
diff --git a/gcc/testsuite/gfortran.dg/pr95880.f90 b/gcc/testsuite/gfortran.dg/pr95880.f90
new file mode 100644
index 000..b7a573cba05
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95880.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/95880 - ICE in gfc_add_type, at fortran/symbol.c:2030
+
+module m
+end
+block data
+ use m
+ integer m! { dg-error "cannot have a type" }
+end block data
RE: [PATCH PR95854] ICE in find_bswap_or_nop_1 of pass store-merging
Hi, Thanks for your good suggestions! This patch was remade and attached. Does the v2 patch look better? Bootstrap and new testcase tested on aarch64 & x86 Linux platform. Kaipeng Zhou PR95854-v2.diff Description: PR95854-v2.diff
