[PATCH] tree-optimization/108523 - testcase for the bug
This adds a reduced testcase for the PR.
Tested on x86_64-unknown-linux-gnu, pushed.
PR tree-optimization/108523
* gcc.dg/torture/pr108523.c: New testcase.
---
gcc/testsuite/gcc.dg/torture/pr108523.c | 16
1 file changed, 16 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr108523.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr108523.c
b/gcc/testsuite/gcc.dg/torture/pr108523.c
new file mode 100644
index 000..a04160baf2a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr108523.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+
+int g_149, g_167, g_481;
+int main() {
+ int *l_1478 = _149;
+ *l_1478 ^= g_167;
+lbl_1481:
+ for (;;) {
+g_481 = 1;
+for (; g_481 < 10; g_481 += 1) {
+ g_167 ^= *l_1478;
+ if (g_149)
+goto lbl_1481;
+}
+ }
+}
--
2.35.3
Re: [PATCH] tree-optimization/108522 Use component_ref_field_offset
On Thu, Jan 26, 2023 at 4:32 AM Siddhesh Poyarekar wrote:
>
> Instead of using TREE_OPERAND (expr, 2) directly, use
> component_ref_field_offset instead, which does scaling for us. The
> function also substitutes PLACEHOLDER_EXPRs, which is probably what we
> want anyway but I'm not sure if it's relevant for tree-object-size.
OK. PLACEHOLDER_EXPR are only relevant pre simplification.
Thanks,
Richard.
> gcc/ChangeLog:
>
> PR tree-optimization/108522
> * tree-object-size.cc (compute_object_offset): Make EXPR
> argument non-const. Call component_ref_field_offset.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/108522
> * gcc.dg/builtin-dynamic-object-size-0.c (DEFSTRUCT): New
> macro.
> (test_dynarray_struct_member_b, test_dynarray_struct_member_c,
> test_dynarray_struct_member_d,
> test_dynarray_struct_member_subobj_b,
> test_dynarray_struct_member_subobj_c,
> test_dynarray_struct_member_subobj_d): New tests.
> (main): Call them.
>
> Signed-off-by: Siddhesh Poyarekar
> ---
> Testing:
> - Tested i686 to confirm that there are no new regressions
> - Tested x86_64 bootstrap and confirmed that there are no new
> regressions
> - ubsan config bootstrap in progress
>
> .../gcc.dg/builtin-dynamic-object-size-0.c| 81 +--
> gcc/tree-object-size.cc | 7 +-
> 2 files changed, 77 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index 569c0a87722..76079d8702e 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -315,19 +315,70 @@ test_dynarray_struct_subobj2 (size_t sz, size_t off,
> size_t *objsz)
> }
>
> /* See pr #108522. */
> +
> +#define DEFSTRUCT(_s, _n) \
> + struct DS
> \
> +{
> \
> + char a[_n];
> \
> + unsigned long long b;
> \
> + int c;
> \
> + char d[2 * _n];
> \
> +} _s
> +
> size_t
> __attribute__ ((noinline))
> -test_dynarray_struct_member (size_t sz)
> +test_dynarray_struct_member_b (size_t sz)
> {
> - struct
> -{
> - char a[sz];
> - char b;
> -} s;
> + DEFSTRUCT (s, sz);
>
>return __builtin_dynamic_object_size (, 0);
> }
>
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member_c (size_t sz)
> +{
> + DEFSTRUCT (s, sz);
> +
> + return __builtin_dynamic_object_size (, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member_d (size_t sz, size_t offset)
> +{
> + DEFSTRUCT (s, sz);
> +
> + return __builtin_dynamic_object_size ([offset], 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member_subobj_b (size_t sz)
> +{
> + DEFSTRUCT (s, sz);
> +
> + return __builtin_dynamic_object_size (, 1);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member_subobj_c (size_t sz)
> +{
> + DEFSTRUCT (s, sz);
> +
> + return __builtin_dynamic_object_size (, 1);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member_subobj_d (size_t sz, size_t offset)
> +{
> + DEFSTRUCT (s, sz);
> +
> + return __builtin_dynamic_object_size ([offset], 1);
> +}
> +
> size_t
> __attribute__ ((noinline))
> test_substring (size_t sz, size_t off)
> @@ -633,7 +684,23 @@ main (int argc, char **argv)
>if (test_dynarray_struct_subobj2 (42, 4, )
> != objsz - 4 - sizeof (long) - sizeof (int))
> FAIL ();
> - if (test_dynarray_struct_member (42) != sizeof (char))
> + DEFSTRUCT(ds, 64);
> + const size_t n = sizeof (ds.a);
> + if (test_dynarray_struct_member_b (n)
> + != sizeof (ds) - __builtin_offsetof (struct DS, b))
> +FAIL ();
> + if (test_dynarray_struct_member_c (n)
> + != sizeof (ds) - __builtin_offsetof (struct DS, c))
> +FAIL ();
> + if (test_dynarray_struct_member_d (n, 0)
> + != sizeof (ds) - __builtin_offsetof (struct DS, d))
> +FAIL ();
> + if (test_dynarray_struct_member_subobj_b (n) != sizeof (ds.b))
> +FAIL ();
> + if (test_dynarray_struct_member_subobj_c (n) != sizeof (ds.c))
> +FAIL ();
> + if (test_dynarray_struct_member_subobj_d (n, n - 2)
> + != sizeof (ds) - __builtin_offsetof (struct DS, d) - n + 2)
> FAIL ();
>if (test_substring_ptrplus (128, 4) != (128 - 4) * sizeof (int))
> FAIL ();
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index de93ffad9c9..9a936a91983 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -56,7
Re: [PATCH] PR tree-optimization/108447 - Do not try to logical fold floating point relations.
On Thu, Jan 26, 2023 at 8:09 AM Richard Biener wrote: > > On Wed, Jan 25, 2023 at 7:05 PM Andrew MacLeod wrote: > > > > This boils down to a single place where we are trying to do things with > > relations in ranger that are simply not safe when we need to honor NANs. > > > > I think we can avoid all the other shuffling of relations, and simply > > not perform this optimization when it comes to floats. > > > > The case the routine handles is: > > > > c_2 = a_6 > b_7 > > c_3 = a_6 < b_7 > > c_4 = c_2 && c_3 > > > > c_2 and c_3 can never be true at the same time, Therefore c_4 can always > > resolve to false based purely on the relations. > > > > > > Range-ops is unable to do this optimization directly as it requires > > examining things from outside the statement, and is not easily > > communicated a simple relation to operator_logical_and. > > > > This routine proceeds to look at the definitions of c_2 and c_3, tries > > to determine if there are common operands, and queries for any relations > > between them. If it turns out there is something, depending on whether > > its && or || , we use intersection or union to determine if the result > > of the logical operation can be folded. If HONOR_NANS is true for the > > float type, then we cannot do this optimization, and bail early. > > > > At this point I do not think we need to do any of the other things > > proposed to relations, so we don't need either of the other 2 patches > > this release. > > > > Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for trunk? > > + if (HONOR_NANS (TREE_TYPE (ssa1_dep1))) > +return; > > would that rather be !(range-includes-nan (ssa1_dep1) || > range-includes-nan (ssa1_dep2) || ..)? Saw the discussion in the other thread only now, so OK. > That said, if the other 2 patches fix some latent issues in the new > frange code I'd > rather have them fixed. So do we know bugs in the current code? You said some buggy function isn't used, so better delete it. Are there other latent issues? Thanks, Richard. > Richard. > > > > > Andrew
Re: [PATCH] PR tree-optimization/108447 - Do not try to logical fold floating point relations.
On Wed, Jan 25, 2023 at 7:05 PM Andrew MacLeod wrote: > > This boils down to a single place where we are trying to do things with > relations in ranger that are simply not safe when we need to honor NANs. > > I think we can avoid all the other shuffling of relations, and simply > not perform this optimization when it comes to floats. > > The case the routine handles is: > > c_2 = a_6 > b_7 > c_3 = a_6 < b_7 > c_4 = c_2 && c_3 > > c_2 and c_3 can never be true at the same time, Therefore c_4 can always > resolve to false based purely on the relations. > > > Range-ops is unable to do this optimization directly as it requires > examining things from outside the statement, and is not easily > communicated a simple relation to operator_logical_and. > > This routine proceeds to look at the definitions of c_2 and c_3, tries > to determine if there are common operands, and queries for any relations > between them. If it turns out there is something, depending on whether > its && or || , we use intersection or union to determine if the result > of the logical operation can be folded. If HONOR_NANS is true for the > float type, then we cannot do this optimization, and bail early. > > At this point I do not think we need to do any of the other things > proposed to relations, so we don't need either of the other 2 patches > this release. > > Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for trunk? + if (HONOR_NANS (TREE_TYPE (ssa1_dep1))) +return; would that rather be !(range-includes-nan (ssa1_dep1) || range-includes-nan (ssa1_dep2) || ..)? That said, if the other 2 patches fix some latent issues in the new frange code I'd rather have them fixed. Richard. > > Andrew
[PATCH] tree-optimization/108522 Use component_ref_field_offset
Instead of using TREE_OPERAND (expr, 2) directly, use
component_ref_field_offset instead, which does scaling for us. The
function also substitutes PLACEHOLDER_EXPRs, which is probably what we
want anyway but I'm not sure if it's relevant for tree-object-size.
gcc/ChangeLog:
PR tree-optimization/108522
* tree-object-size.cc (compute_object_offset): Make EXPR
argument non-const. Call component_ref_field_offset.
gcc/testsuite/ChangeLog:
PR tree-optimization/108522
* gcc.dg/builtin-dynamic-object-size-0.c (DEFSTRUCT): New
macro.
(test_dynarray_struct_member_b, test_dynarray_struct_member_c,
test_dynarray_struct_member_d,
test_dynarray_struct_member_subobj_b,
test_dynarray_struct_member_subobj_c,
test_dynarray_struct_member_subobj_d): New tests.
(main): Call them.
Signed-off-by: Siddhesh Poyarekar
---
Testing:
- Tested i686 to confirm that there are no new regressions
- Tested x86_64 bootstrap and confirmed that there are no new
regressions
- ubsan config bootstrap in progress
.../gcc.dg/builtin-dynamic-object-size-0.c| 81 +--
gcc/tree-object-size.cc | 7 +-
2 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
index 569c0a87722..76079d8702e 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
@@ -315,19 +315,70 @@ test_dynarray_struct_subobj2 (size_t sz, size_t off,
size_t *objsz)
}
/* See pr #108522. */
+
+#define DEFSTRUCT(_s, _n) \
+ struct DS \
+{\
+ char a[_n];\
+ unsigned long long b; \
+ int c; \
+ char d[2 * _n];\
+} _s
+
size_t
__attribute__ ((noinline))
-test_dynarray_struct_member (size_t sz)
+test_dynarray_struct_member_b (size_t sz)
{
- struct
-{
- char a[sz];
- char b;
-} s;
+ DEFSTRUCT (s, sz);
return __builtin_dynamic_object_size (, 0);
}
+size_t
+__attribute__ ((noinline))
+test_dynarray_struct_member_c (size_t sz)
+{
+ DEFSTRUCT (s, sz);
+
+ return __builtin_dynamic_object_size (, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_dynarray_struct_member_d (size_t sz, size_t offset)
+{
+ DEFSTRUCT (s, sz);
+
+ return __builtin_dynamic_object_size ([offset], 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_dynarray_struct_member_subobj_b (size_t sz)
+{
+ DEFSTRUCT (s, sz);
+
+ return __builtin_dynamic_object_size (, 1);
+}
+
+size_t
+__attribute__ ((noinline))
+test_dynarray_struct_member_subobj_c (size_t sz)
+{
+ DEFSTRUCT (s, sz);
+
+ return __builtin_dynamic_object_size (, 1);
+}
+
+size_t
+__attribute__ ((noinline))
+test_dynarray_struct_member_subobj_d (size_t sz, size_t offset)
+{
+ DEFSTRUCT (s, sz);
+
+ return __builtin_dynamic_object_size ([offset], 1);
+}
+
size_t
__attribute__ ((noinline))
test_substring (size_t sz, size_t off)
@@ -633,7 +684,23 @@ main (int argc, char **argv)
if (test_dynarray_struct_subobj2 (42, 4, )
!= objsz - 4 - sizeof (long) - sizeof (int))
FAIL ();
- if (test_dynarray_struct_member (42) != sizeof (char))
+ DEFSTRUCT(ds, 64);
+ const size_t n = sizeof (ds.a);
+ if (test_dynarray_struct_member_b (n)
+ != sizeof (ds) - __builtin_offsetof (struct DS, b))
+FAIL ();
+ if (test_dynarray_struct_member_c (n)
+ != sizeof (ds) - __builtin_offsetof (struct DS, c))
+FAIL ();
+ if (test_dynarray_struct_member_d (n, 0)
+ != sizeof (ds) - __builtin_offsetof (struct DS, d))
+FAIL ();
+ if (test_dynarray_struct_member_subobj_b (n) != sizeof (ds.b))
+FAIL ();
+ if (test_dynarray_struct_member_subobj_c (n) != sizeof (ds.c))
+FAIL ();
+ if (test_dynarray_struct_member_subobj_d (n, n - 2)
+ != sizeof (ds) - __builtin_offsetof (struct DS, d) - n + 2)
FAIL ();
if (test_substring_ptrplus (128, 4) != (128 - 4) * sizeof (int))
FAIL ();
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index de93ffad9c9..9a936a91983 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -56,7 +56,7 @@ struct GTY(()) object_size
tree wholesize;
};
-static tree compute_object_offset (const_tree, const_tree);
+static tree compute_object_offset (tree, const_tree);
static bool addr_object_size (struct object_size_info *,
const_tree, int, tree *, tree *t = NULL);
static tree alloc_object_size (const gcall *, int);
@@ -396,7 +396,7 @@ size_for_offset (tree sz, tree offset, tree wholesize =
Re: [PATCH] opts: SANITIZE_ADDRESS wrongly cleared [PR108543]
On Wed, Jan 25, 2023 at 3:26 PM Marek Polacek via Gcc-patches
wrote:
>
> Here we crash on a null fndecl ultimately because we haven't defined
> the built-ins described in sanitizer.def. So
> builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
> returns NULL_TREE, causing an ICE later.
>
> DEF_SANITIZER_BUILTIN only actually defines the built-ins when flag_sanitize
> has SANITIZE_ADDRESS, or some of the other SANITIZE_*, but it doesn't check
> SANITIZE_KERNEL_ADDRESS or SANITIZE_USER_ADDRESS. Unfortunately, with
> -fsanitize=address -fno-sanitize=kernel-address
> or
> -fsanitize=kernel-address -fno-sanitize=address
> SANITIZE_ADDRESS ends up being unset from flag_sanitize even though
> _USER/_KERNEL are set. That's because -fsanitize=address means
> SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS and -fsanitize=kernel-address
> is SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS but parse_sanitizer_options
> does
> flags &= ~sanitizer_opts[i].flag;
> so the subsequent -fno- unsets SANITIZE_ADDRESS. Then no sanitizer
> built-ins are actually defined.
>
> I'm not sure why SANITIZE_ADDRESS isn't just SANITIZE_USER_ADDRESS |
> SANITIZE_KERNEL_ADDRESS, I don't think we need 3 bits.
I am trying to follow the code but I can't tell if -fsanitize=address
-fno-sanitize=address still will work?
It would make sense to add a testcase for that case too.
I suspect it does not work though, because it would still leave
SANITIZE_ADDRESS enabled ...
Thanks,
Andrew
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12/11?
>
> PR middle-end/108543
>
> gcc/ChangeLog:
>
> * opts.cc (parse_sanitizer_options): Don't clear SANITIZE_ADDRESS if
> it
> was previously set.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/asan/pointer-subtract-5.c: New test.
> * c-c++-common/asan/pointer-subtract-6.c: New test.
> * c-c++-common/asan/pointer-subtract-7.c: New test.
> * c-c++-common/asan/pointer-subtract-8.c: New test.
> ---
> gcc/opts.cc | 5 -
> .../c-c++-common/asan/pointer-subtract-5.c| 15 +++
> .../c-c++-common/asan/pointer-subtract-6.c| 15 +++
> .../c-c++-common/asan/pointer-subtract-7.c| 15 +++
> .../c-c++-common/asan/pointer-subtract-8.c| 15 +++
> 5 files changed, 64 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
> create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
> create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
> create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c
>
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index 9ba47d7deaa..c042b03bd9f 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -2246,7 +2246,10 @@ parse_sanitizer_options (const char *p, location_t
> loc, int scode,
> flags |= sanitizer_opts[i].flag;
> }
> else
> - flags &= ~sanitizer_opts[i].flag;
> + /* Don't clear SANITIZE_ADDRESS if it was previously set;
> +-fsanitize=address -fno-sanitize=kernel-address should
> +leave SANITIZE_ADDRESS set. */
> + flags &= (~sanitizer_opts[i].flag | SANITIZE_ADDRESS);
> found = true;
> break;
> }
> diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
> b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
> new file mode 100644
> index 000..867eda0e61e
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
> @@ -0,0 +1,15 @@
> +/* PR middle-end/108543 */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=address -fno-sanitize=kernel-address
> -fsanitize=pointer-subtract" } */
> +
> +struct S {
> + long _M_p;
> +};
> +
> +typedef struct S S;
> +
> +__PTRDIFF_TYPE__
> +f (S __x, S __y)
> +{
> + return &__x._M_p - &__y._M_p;
> +}
> diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
> b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
> new file mode 100644
> index 000..785b90b3d98
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
> @@ -0,0 +1,15 @@
> +/* PR middle-end/108543 */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=kernel-address -fno-sanitize=address
> -fsanitize=pointer-subtract" } */
> +
> +struct S {
> + long _M_p;
> +};
> +
> +typedef struct S S;
> +
> +__PTRDIFF_TYPE__
> +f (S __x, S __y)
> +{
> + return &__x._M_p - &__y._M_p;
> +}
> diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
> b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
> new file mode 100644
> index 000..11b63401b8c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
> @@ -0,0 +1,15 @@
> +/* PR middle-end/108543 */
> +/* { dg-do compile } */
> +/* { dg-options "-fno-sanitize=kernel-address -fsanitize=address
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On Wed, Jan 25, 2023 at 06:23:25PM -0500, Andrew MacLeod wrote:
> > > --- a/gcc/gimple-range-fold.cc
> > > +++ b/gcc/gimple-range-fold.cc
> > > @@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
> > > lhs_range, gimple *s,
> > > if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
> > > return;
> > >
> > > + if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
> > > + return;
> > > +
> > > // Make sure they are the same dependencies, and detect the order of
> > > the
> > > // relationship.
> > > bool reverse_op2 = true;
> > >
> > >
> > If this works, maybe (does something check if ssa1_dep1 has certain
> > type (I assume ssa1_dep2 has to have the same) then ssa2_dep{1,2} has that
> > type too (or uselessly equal to that)? Though, if this spot has both
> > operands of the comparison, could we for HONOR_NANS case ask frange if
> > any of them is maybe_nan or known_nan and punt only if anything can be NAN?
>
>
> it bootstraps with no regressions.
>
> all the ssa?dep? must have the same type, or the comparisons for similarity
> are going to fail. ir requires the same 2 ssa-name in both relational
> expressions, which means they must all be the same type.
>
> At this point we don't actually know ranges.. this is a higher level thing
> before any queries have happen. we could query, but I'd punt on that for
> next release :-) And think about how applicable it is. Id like to revisit
> this entire situation.
LGTM for GCC 13. For GCC 14 we should IMHO really support all 16
possible relations and deal with them correctly.
Jakub
[PATCH] opts: SANITIZE_ADDRESS wrongly cleared [PR108543]
Here we crash on a null fndecl ultimately because we haven't defined
the built-ins described in sanitizer.def. So
builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
returns NULL_TREE, causing an ICE later.
DEF_SANITIZER_BUILTIN only actually defines the built-ins when flag_sanitize
has SANITIZE_ADDRESS, or some of the other SANITIZE_*, but it doesn't check
SANITIZE_KERNEL_ADDRESS or SANITIZE_USER_ADDRESS. Unfortunately, with
-fsanitize=address -fno-sanitize=kernel-address
or
-fsanitize=kernel-address -fno-sanitize=address
SANITIZE_ADDRESS ends up being unset from flag_sanitize even though
_USER/_KERNEL are set. That's because -fsanitize=address means
SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS and -fsanitize=kernel-address
is SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS but parse_sanitizer_options
does
flags &= ~sanitizer_opts[i].flag;
so the subsequent -fno- unsets SANITIZE_ADDRESS. Then no sanitizer
built-ins are actually defined.
I'm not sure why SANITIZE_ADDRESS isn't just SANITIZE_USER_ADDRESS |
SANITIZE_KERNEL_ADDRESS, I don't think we need 3 bits.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12/11?
PR middle-end/108543
gcc/ChangeLog:
* opts.cc (parse_sanitizer_options): Don't clear SANITIZE_ADDRESS if it
was previously set.
gcc/testsuite/ChangeLog:
* c-c++-common/asan/pointer-subtract-5.c: New test.
* c-c++-common/asan/pointer-subtract-6.c: New test.
* c-c++-common/asan/pointer-subtract-7.c: New test.
* c-c++-common/asan/pointer-subtract-8.c: New test.
---
gcc/opts.cc | 5 -
.../c-c++-common/asan/pointer-subtract-5.c| 15 +++
.../c-c++-common/asan/pointer-subtract-6.c| 15 +++
.../c-c++-common/asan/pointer-subtract-7.c| 15 +++
.../c-c++-common/asan/pointer-subtract-8.c| 15 +++
5 files changed, 64 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c
diff --git a/gcc/opts.cc b/gcc/opts.cc
index 9ba47d7deaa..c042b03bd9f 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -2246,7 +2246,10 @@ parse_sanitizer_options (const char *p, location_t loc,
int scode,
flags |= sanitizer_opts[i].flag;
}
else
- flags &= ~sanitizer_opts[i].flag;
+ /* Don't clear SANITIZE_ADDRESS if it was previously set;
+-fsanitize=address -fno-sanitize=kernel-address should
+leave SANITIZE_ADDRESS set. */
+ flags &= (~sanitizer_opts[i].flag | SANITIZE_ADDRESS);
found = true;
break;
}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
new file mode 100644
index 000..867eda0e61e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c
@@ -0,0 +1,15 @@
+/* PR middle-end/108543 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address -fno-sanitize=kernel-address
-fsanitize=pointer-subtract" } */
+
+struct S {
+ long _M_p;
+};
+
+typedef struct S S;
+
+__PTRDIFF_TYPE__
+f (S __x, S __y)
+{
+ return &__x._M_p - &__y._M_p;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
new file mode 100644
index 000..785b90b3d98
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c
@@ -0,0 +1,15 @@
+/* PR middle-end/108543 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=kernel-address -fno-sanitize=address
-fsanitize=pointer-subtract" } */
+
+struct S {
+ long _M_p;
+};
+
+typedef struct S S;
+
+__PTRDIFF_TYPE__
+f (S __x, S __y)
+{
+ return &__x._M_p - &__y._M_p;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
new file mode 100644
index 000..11b63401b8c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c
@@ -0,0 +1,15 @@
+/* PR middle-end/108543 */
+/* { dg-do compile } */
+/* { dg-options "-fno-sanitize=kernel-address -fsanitize=address
-fsanitize=pointer-subtract" } */
+
+struct S {
+ long _M_p;
+};
+
+typedef struct S S;
+
+__PTRDIFF_TYPE__
+f (S __x, S __y)
+{
+ return &__x._M_p - &__y._M_p;
+}
diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c
b/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c
new file mode 100644
index 000..ac2b9c3c1d9
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c
@@ -0,0 +1,15 @@
+/* PR middle-end/108543 */
+/* { dg-do compile } */
+/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address
-fsanitize=pointer-subtract" } */
+
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On 1/25/23 17:35, Jakub Jelinek wrote:
On Wed, Jan 25, 2023 at 11:12:11AM -0500, Andrew MacLeod via Gcc-patches wrote:
In GCC13, I don't think there are any uses of the relation oracle outside of
ranger and range-ops.
So, given that, perhaps the simplest thing to do is bail on all this change,
and instead simply do the following which also fixes the PR. (im running it
thru tests as we speak)
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 91eb6298254..9c5359a3fc6 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
lhs_range, gimple *s,
if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
return;
+ if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
+ return;
+
// Make sure they are the same dependencies, and detect the order of the
// relationship.
bool reverse_op2 = true;
If this works, maybe (does something check if ssa1_dep1 has certain
type (I assume ssa1_dep2 has to have the same) then ssa2_dep{1,2} has that
type too (or uselessly equal to that)? Though, if this spot has both
operands of the comparison, could we for HONOR_NANS case ask frange if
any of them is maybe_nan or known_nan and punt only if anything can be NAN?
it bootstraps with no regressions.
all the ssa?dep? must have the same type, or the comparisons for
similarity are going to fail. ir requires the same 2 ssa-name in both
relational expressions, which means they must all be the same type.
At this point we don't actually know ranges.. this is a higher level
thing before any queries have happen. we could query, but I'd punt on
that for next release :-) And think about how applicable it is. Id
like to revisit this entire situation.
Andrew
Jakub
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On Wed, Jan 25, 2023 at 11:12:11AM -0500, Andrew MacLeod via Gcc-patches wrote:
> In GCC13, I don't think there are any uses of the relation oracle outside of
> ranger and range-ops.
>
> So, given that, perhaps the simplest thing to do is bail on all this change,
> and instead simply do the following which also fixes the PR. (im running it
> thru tests as we speak)
>
>
>
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index 91eb6298254..9c5359a3fc6 100644
> --- a/gcc/gimple-range-fold.cc
> +++ b/gcc/gimple-range-fold.cc
> @@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
> lhs_range, gimple *s,
> if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
> return;
>
> + if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
> + return;
> +
> // Make sure they are the same dependencies, and detect the order of the
> // relationship.
> bool reverse_op2 = true;
>
>
If this works, maybe (does something check if ssa1_dep1 has certain
type (I assume ssa1_dep2 has to have the same) then ssa2_dep{1,2} has that
type too (or uselessly equal to that)? Though, if this spot has both
operands of the comparison, could we for HONOR_NANS case ask frange if
any of them is maybe_nan or known_nan and punt only if anything can be NAN?
Jakub
Re: [PATCH] Fortran: fix ICE in check_host_association [PR108544]
On Wed, Jan 25, 2023 at 10:59:22PM +0100, Harald Anlauf via Fortran wrote: > Dear all, > > the attached patch fixes two issues: first it addresses a NULL pointer > dereference on invalid input, triggered by the provided testcase. > > Second, while analyzing the context of the affected code, I looked into > the testcase for PR96102, and by varying it slightly, i.e. replacing > functions by subroutines I found that we accept invalid code that is > rejected by several other brands tested. To fix this, I removed one > line of a condition that did not seem to make sense to me. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > Yes. I briefly looked at this by simply commenting out the assert, which gives too many odd error messages. Returning 'false' seems to be best. -- Steve
[PATCH] Fortran: fix ICE in check_host_association [PR108544]
Dear all,
the attached patch fixes two issues: first it addresses a NULL pointer
dereference on invalid input, triggered by the provided testcase.
Second, while analyzing the context of the affected code, I looked into
the testcase for PR96102, and by varying it slightly, i.e. replacing
functions by subroutines I found that we accept invalid code that is
rejected by several other brands tested. To fix this, I removed one
line of a condition that did not seem to make sense to me.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 59034b3b938a2f5e3391208fca56fcf54d5b6d18 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Wed, 25 Jan 2023 22:47:26 +0100
Subject: [PATCH] Fortran: fix ICE in check_host_association [PR108544]
gcc/fortran/ChangeLog:
PR fortran/108544
* resolve.cc (check_host_association): Extend host association check
so that it is not restricted to functions. Also prevent NULL pointer
dereference.
gcc/testsuite/ChangeLog:
PR fortran/108544
* gfortran.dg/pr108544.f90: New test.
* gfortran.dg/pr96102b.f90: New test.
---
gcc/fortran/resolve.cc | 4 +++-
gcc/testsuite/gfortran.dg/pr108544.f90 | 11 +++
gcc/testsuite/gfortran.dg/pr96102b.f90 | 24
3 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr108544.f90
create mode 100644 gcc/testsuite/gfortran.dg/pr96102b.f90
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 94213cd3cd4..9e2edf7be71 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -6087,7 +6087,6 @@ check_host_association (gfc_expr *e)
gfc_find_symbol (e->symtree->name, gfc_current_ns, 1, );
if (sym && old_sym != sym
- && sym->ts.type == old_sym->ts.type
&& sym->attr.flavor == FL_PROCEDURE
&& sym->attr.contained)
{
@@ -6132,6 +6131,9 @@ check_host_association (gfc_expr *e)
return false;
}
+ if (ref == NULL)
+ return false;
+
gcc_assert (ref->type == REF_ARRAY);
/* Grab the start expressions from the array ref and
diff --git a/gcc/testsuite/gfortran.dg/pr108544.f90 b/gcc/testsuite/gfortran.dg/pr108544.f90
new file mode 100644
index 000..783cb7aaf7b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108544.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/108544 - ICE in check_host_association
+! Contributed by G.Steinmetz
+
+module m
+contains
+ subroutine s
+select type (s => 1) ! { dg-error "Selector shall be polymorphic" }
+end select
+ end
+end
diff --git a/gcc/testsuite/gfortran.dg/pr96102b.f90 b/gcc/testsuite/gfortran.dg/pr96102b.f90
new file mode 100644
index 000..82147da3893
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr96102b.f90
@@ -0,0 +1,24 @@
+! { dg-do compile }
+!
+! PR fortran/108544 - host association
+! Variation of testcase pr96102.f90 using subroutines instead of functions
+
+module m
+ type mytype
+integer :: i
+ end type
+ type(mytype) :: d = mytype (42) ! { dg-error "is host associated" }
+ integer :: n = 2 ! { dg-error "is host associated" }
+contains
+ subroutine s
+if ( n /= 0 ) stop 1 ! { dg-error "internal procedure of the same name" }
+if ( d%i /= 0 ) stop 2 ! { dg-error "internal procedure of the same name" }
+ contains
+subroutine n()
+end
+subroutine d()
+end
+ end
+end
+
+! { dg-prune-output "Operands of comparison operator" }
--
2.35.3
[PATCH v5 2/5] libcpp: add a function to determine UTF-8 validity of a C string
This simplifies the interface for other UTF-8 validity detections when a
simple "yes" or "no" answer is sufficient.
libcpp/
* charset.cc: Add `_cpp_valid_utf8_str` which determines whether
a C string is valid UTF-8 or not.
* internal.h: Add prototype for `_cpp_valid_utf8_str`.
Signed-off-by: Ben Boeckel
---
libcpp/charset.cc | 20
libcpp/internal.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/libcpp/charset.cc b/libcpp/charset.cc
index f7ae12ea5a2..616be9d02ee 100644
--- a/libcpp/charset.cc
+++ b/libcpp/charset.cc
@@ -1868,6 +1868,26 @@ _cpp_valid_utf8 (cpp_reader *pfile,
return true;
}
+/* Detect whether a C-string is a valid UTF-8-encoded set of bytes. Returns
+`false` if any contained byte sequence encodes an invalid Unicode codepoint
+or is not a valid UTF-8 sequence. Returns `true` otherwise. */
+
+extern bool
+_cpp_valid_utf8_str (const char *name)
+{
+ const uchar* in = (const uchar*)name;
+ size_t len = strlen (name);
+ cppchar_t cp;
+
+ while (*in)
+{
+ if (one_utf8_to_cppchar (, , ))
+ return false;
+}
+
+ return true;
+}
+
/* Subroutine of convert_hex and convert_oct. N is the representation
in the execution character set of a numeric escape; write it into the
string buffer TBUF and update the end-of-string pointer therein. WIDE
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 9724676a8cd..48520901b2d 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -834,6 +834,8 @@ extern bool _cpp_valid_utf8 (cpp_reader *pfile,
struct normalize_state *nst,
cppchar_t *cp);
+extern bool _cpp_valid_utf8_str (const char *str);
+
extern void _cpp_destroy_iconv (cpp_reader *);
extern unsigned char *_cpp_convert_input (cpp_reader *, const char *,
unsigned char *, size_t, size_t,
--
2.39.0
[PATCH v5 5/5] c++modules: report module mapper files as a dependency
It affects the build, and if used as a static file, can reliably be
tracked using the `-MF` mechanism.
gcc/cp/:
* mapper-client.cc, mapper-client.h (open_module_client): Accept
dependency tracking and track module mapper files as
dependencies.
* module.cc (make_mapper, get_mapper): Pass the dependency
tracking class down.
Signed-off-by: Ben Boeckel
---
gcc/cp/mapper-client.cc | 4
gcc/cp/mapper-client.h | 1 +
gcc/cp/module.cc| 18 +-
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
index 39e80df2d25..0ce5679d659 100644
--- a/gcc/cp/mapper-client.cc
+++ b/gcc/cp/mapper-client.cc
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic-core.h"
#include "mapper-client.h"
#include "intl.h"
+#include "mkdeps.h"
#include "../../c++tools/resolver.h"
@@ -132,6 +133,7 @@ spawn_mapper_program (char const **errmsg, std::string
,
module_client *
module_client::open_module_client (location_t loc, const char *o,
+ class mkdeps *deps,
void (*set_repo) (const char *),
char const *full_program_name)
{
@@ -285,6 +287,8 @@ module_client::open_module_client (location_t loc, const
char *o,
errmsg = "opening";
else
{
+ /* Add the mapper file to the dependency tracking. */
+ deps_add_dep (deps, name.c_str ());
if (int l = r->read_tuple_file (fd, ident, false))
{
if (l > 0)
diff --git a/gcc/cp/mapper-client.h b/gcc/cp/mapper-client.h
index b32723ce296..a3b0b8adc51 100644
--- a/gcc/cp/mapper-client.h
+++ b/gcc/cp/mapper-client.h
@@ -55,6 +55,7 @@ public:
public:
static module_client *open_module_client (location_t loc, const char *option,
+ class mkdeps *,
void (*set_repo) (const char *),
char const *);
static void close_module_client (location_t loc, module_client *);
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index dbd1b721616..37066bf072b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3969,12 +3969,12 @@ static GTY(()) vec
*partial_specializations;
/* Our module mapper (created lazily). */
module_client *mapper;
-static module_client *make_mapper (location_t loc);
-inline module_client *get_mapper (location_t loc)
+static module_client *make_mapper (location_t loc, class mkdeps *deps);
+inline module_client *get_mapper (location_t loc, class mkdeps *deps)
{
auto *res = mapper;
if (!res)
-res = make_mapper (loc);
+res = make_mapper (loc, deps);
return res;
}
@@ -14031,7 +14031,7 @@ get_module (const char *ptr)
/* Create a new mapper connecting to OPTION. */
module_client *
-make_mapper (location_t loc)
+make_mapper (location_t loc, class mkdeps *deps)
{
timevar_start (TV_MODULE_MAPPER);
const char *option = module_mapper_name;
@@ -14039,7 +14039,7 @@ make_mapper (location_t loc)
option = getenv ("CXX_MODULE_MAPPER");
mapper = module_client::open_module_client
-(loc, option, _cmi_repo,
+(loc, option, deps, _cmi_repo,
(save_decoded_options[0].opt_index == OPT_SPECIAL_program_name)
&& save_decoded_options[0].arg != progname
? save_decoded_options[0].arg : nullptr);
@@ -19503,7 +19503,7 @@ maybe_translate_include (cpp_reader *reader, line_maps
*lmaps, location_t loc,
dump.push (NULL);
dump () && dump ("Checking include translation '%s'", path);
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
size_t len = strlen (path);
path = canonicalize_header_name (NULL, loc, true, path, len);
@@ -19619,7 +19619,7 @@ module_begin_main_file (cpp_reader *reader, line_maps
*lmaps,
static void
name_pending_imports (cpp_reader *reader)
{
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
if (!vec_safe_length (pending_imports))
/* Not doing anything. */
@@ -20089,7 +20089,7 @@ init_modules (cpp_reader *reader)
if (!flag_module_lazy)
/* Get the mapper now, if we're not being lazy. */
-get_mapper (cpp_main_loc (reader));
+get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
if (!flag_preprocess_only)
{
@@ -20299,7 +20299,7 @@ late_finish_module (cpp_reader *reader,
module_processing_cookie *cookie,
if (!errorcount)
{
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
mapper->ModuleCompiled (state->get_flatname ());
}
else if (cookie->cmi_name)
--
2.39.0
[PATCH v5 4/5] c++modules: report imported CMI files as dependencies
They affect the build, so report them via `-MF` mechanisms.
gcc/cp/
* module.cc (do_import): Report imported CMI files as
dependencies.
Signed-off-by: Ben Boeckel
---
gcc/cp/module.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ebd30f63d81..dbd1b721616 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18966,6 +18966,8 @@ module_state::do_import (cpp_reader *reader, bool
outermost)
dump () && dump ("CMI is %s", file);
if (note_module_cmi_yes || inform_cmi_p)
inform (loc, "reading CMI %qs", file);
+ /* Add the CMI file to the dependency tracking. */
+ deps_add_dep (cpp_get_deps (reader), file);
fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY);
e = errno;
}
--
2.39.0
[PATCH v5 3/5] p1689r5: initial support
This patch implements support for [P1689R5][] to communicate to a build system the C++20 module dependencies to build systems so that they may build `.gcm` files in the proper order. Support is communicated through the following three new flags: - `-fdeps-format=` specifies the format for the output. Currently named `p1689r5`. - `-fdeps-file=` specifies the path to the file to write the format to. - `-fdep-output=` specifies the `.o` that will be written for the TU that is scanned. This is required so that the build system can correlate the dependency output with the actual compilation that will occur. CMake supports this format as of 17 Jun 2022 (to be part of 3.25.0) using an experimental feature selection (to allow for future usage evolution without committing to how it works today). While it remains experimental, docs may be found in CMake's documentation for experimental features. Future work may include using this format for Fortran module dependencies as well, however this is still pending work. [P1689R5]: https://isocpp.org/files/papers/P1689R5.html [cmake-experimental]: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Help/dev/experimental.rst TODO: - header-unit information fields Header units (including the standard library headers) are 100% unsupported right now because the `-E` mechanism wants to import their BMIs. A new mode (i.e., something more workable than existing `-E` behavior) that mocks up header units as if they were imported purely from their path and content would be required. - non-utf8 paths The current standard says that paths that are not unambiguously represented using UTF-8 are not supported (because these cases are rare and the extra complication is not worth it at this time). Future versions of the format might have ways of encoding non-UTF-8 paths. For now, this patch just doesn't support non-UTF-8 paths (ignoring the "unambiguously represetable in UTF-8" case). - figure out why junk gets placed at the end of the file Sometimes it seems like the file gets a lot of `NUL` bytes appended to it. It happens rarely and seems to be the result of some `ftruncate`-style call which results in extra padding in the contents. Noting it here as an observation at least. libcpp/ * include/cpplib.h: Add cpp_deps_format enum. (cpp_options): Add format field (cpp_finish): Add dependency stream parameter. * include/mkdeps.h (deps_add_module_target): Add new preprocessor parameter used for C++ module tracking. * init.cc (cpp_finish): Add new preprocessor parameter used for C++ module tracking. * mkdeps.cc (mkdeps): Implement P1689R5 output. gcc/ * doc/invoke.texi: Document -fdeps-format=, -fdep-file=, and -fdep-output= flags. gcc/c-family/ * c-opts.cc (c_common_handle_option): Add fdeps_file variable and -fdeps-format=, -fdep-file=, and -fdep-output= parsing. * c.opt: Add -fdeps-format=, -fdep-file=, and -fdep-output= flags. gcc/cp/ * module.cc (preprocessed_module): Pass whether the module is exported to dependency tracking. gcc/testsuite/ * g++.dg/modules/depflags-f-MD.C: New test. * g++.dg/modules/depflags-f.C: New test. * g++.dg/modules/depflags-fi.C: New test. * g++.dg/modules/depflags-fj-MD.C: New test. * g++.dg/modules/depflags-fj.C: New test. * g++.dg/modules/depflags-fjo-MD.C: New test. * g++.dg/modules/depflags-fjo.C: New test. * g++.dg/modules/depflags-fo-MD.C: New test. * g++.dg/modules/depflags-fo.C: New test. * g++.dg/modules/depflags-j-MD.C: New test. * g++.dg/modules/depflags-j.C: New test. * g++.dg/modules/depflags-jo-MD.C: New test. * g++.dg/modules/depflags-jo.C: New test. * g++.dg/modules/depflags-o-MD.C: New test. * g++.dg/modules/depflags-o.C: New test. * g++.dg/modules/p1689-1.C: New test. * g++.dg/modules/p1689-1.exp.json: New test expectation. * g++.dg/modules/p1689-2.C: New test. * g++.dg/modules/p1689-2.exp.json: New test expectation. * g++.dg/modules/p1689-3.C: New test. * g++.dg/modules/p1689-3.exp.json: New test expectation. * g++.dg/modules/p1689-4.C: New test. * g++.dg/modules/p1689-4.exp.json: New test expectation. * g++.dg/modules/p1689-5.C: New test. * g++.dg/modules/p1689-5.exp.json: New test expectation. * g++.dg/modules/modules.exp: Load new P1689 library routines. * g++.dg/modules/test-p1689.py: New tool for validating P1689 output. * lib/modules.exp: Support for validating P1689 outputs. Signed-off-by: Ben Boeckel --- gcc/c-family/c-opts.cc| 40 +++- gcc/c-family/c.opt| 12 + gcc/cp/module.cc | 3 +- gcc/doc/invoke.texi | 15 ++
[PATCH v5 1/5] libcpp: reject codepoints above 0x10FFFF
Unicode does not support such values because they are unrepresentable in UTF-16. libcpp/ * charset.cc: Reject encodings of codepoints above 0x10. UTF-16 does not support such codepoints and therefore all Unicode rejects such values. Signed-off-by: Ben Boeckel --- libcpp/charset.cc | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libcpp/charset.cc b/libcpp/charset.cc index 3c47d4f868b..f7ae12ea5a2 100644 --- a/libcpp/charset.cc +++ b/libcpp/charset.cc @@ -158,6 +158,10 @@ struct _cpp_strbuf encoded as any of DF 80, E0 9F 80, F0 80 9F 80, F8 80 80 9F 80, or FC 80 80 80 9F 80. Only the first is valid. + Additionally, Unicode declares that all codepoints above 0010 are + invalid because they cannot be represented in UTF-16. As such, all 5- and + 6-byte encodings are invalid. + An implementation note: the transformation from UTF-16 to UTF-8, or vice versa, is easiest done by using UTF-32 as an intermediary. */ @@ -216,7 +220,7 @@ one_utf8_to_cppchar (const uchar **inbufp, size_t *inbytesleftp, if (c <= 0x3FF && nbytes > 5) return EILSEQ; /* Make sure the character is valid. */ - if (c > 0x7FFF || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; + if (c > 0x10 || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; *cp = c; *inbufp = inbuf; @@ -320,7 +324,7 @@ one_utf32_to_utf8 (iconv_t bigend, const uchar **inbufp, size_t *inbytesleftp, s += inbuf[bigend ? 2 : 1] << 8; s += inbuf[bigend ? 3 : 0]; - if (s >= 0x7FFF || (s >= 0xD800 && s <= 0xDFFF)) + if (s > 0x10 || (s >= 0xD800 && s <= 0xDFFF)) return EILSEQ; rval = one_cppchar_to_utf8 (s, outbufp, outbytesleftp); -- 2.39.0
[PATCH v5 0/5] P1689R5 support
Hi, This patch series adds initial support for ISO C++'s [P1689R5][], a format for describing C++ module requirements and provisions based on the source code. This is required because compiling C++ with modules is not embarrassingly parallel and need to be ordered to ensure that `import some_module;` can be satisfied in time by making sure that any TU with `export import some_module;` is compiled first. [P1689R5]: https://isocpp.org/files/papers/P1689R5.html I've also added patches to include imported module CMI files and the module mapper file as dependencies of the compilation. I briefly looked into adding dependencies on response files as well, but that appeared to need some code contortions to have a `class mkdeps` available before parsing the command line or to keep the information around until one was made. I'd like feedback on the approach taken here with respect to the user-visible flags. I'll also note that header units are not supported at this time because the current `-E` behavior with respect to `import ;` is to search for an appropriate `.gcm` file which is not something such a "scan" can support. A new mode will likely need to be created (e.g., replacing `-E` with `-fc++-module-scanning` or something) where headers are looked up "normally" and processed only as much as scanning requires. FWIW, Clang as taken an alternate approach with its `clang-scan-deps` tool rather than using the compiler directly. Thanks, --Ben --- v4 -> v5: - add dependency tracking for imported modules to `-MF` - add dependency tracking for static module mapper files given to `-fmodule-mapper=` v3 -> v4: - add missing spaces between function names and arguments v2 -> v3: - changelog entries moved to commit messages - documentation updated/added in the UTF-8 routine editing v1 -> v2: - removal of the `deps_write(extra)` parameter to option-checking where ndeeded - default parameter of `cpp_finish(fdeps_stream = NULL)` - unification of libcpp UTF-8 validity functions from v1 - test cases for flag parsing states (depflags-*) and p1689 output (p1689-*) Ben Boeckel (5): libcpp: reject codepoints above 0x10 libcpp: add a function to determine UTF-8 validity of a C string p1689r5: initial support c++modules: report imported CMI files as dependencies c++modules: report module mapper files as a dependency gcc/c-family/c-opts.cc| 40 +++- gcc/c-family/c.opt| 12 + gcc/cp/mapper-client.cc | 4 + gcc/cp/mapper-client.h| 1 + gcc/cp/module.cc | 23 +- gcc/doc/invoke.texi | 15 ++ gcc/testsuite/g++.dg/modules/depflags-f-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-f.C | 1 + gcc/testsuite/g++.dg/modules/depflags-fi.C| 3 + gcc/testsuite/g++.dg/modules/depflags-fj-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-fj.C| 4 + .../g++.dg/modules/depflags-fjo-MD.C | 4 + gcc/testsuite/g++.dg/modules/depflags-fjo.C | 5 + gcc/testsuite/g++.dg/modules/depflags-fo-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-fo.C| 4 + gcc/testsuite/g++.dg/modules/depflags-j-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-j.C | 3 + gcc/testsuite/g++.dg/modules/depflags-jo-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-jo.C| 4 + gcc/testsuite/g++.dg/modules/depflags-o-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-o.C | 3 + gcc/testsuite/g++.dg/modules/modules.exp | 1 + gcc/testsuite/g++.dg/modules/p1689-1.C| 18 ++ gcc/testsuite/g++.dg/modules/p1689-1.exp.json | 27 +++ gcc/testsuite/g++.dg/modules/p1689-2.C| 16 ++ gcc/testsuite/g++.dg/modules/p1689-2.exp.json | 16 ++ gcc/testsuite/g++.dg/modules/p1689-3.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-3.exp.json | 16 ++ gcc/testsuite/g++.dg/modules/p1689-4.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-4.exp.json | 14 ++ gcc/testsuite/g++.dg/modules/p1689-5.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-5.exp.json | 14 ++ gcc/testsuite/g++.dg/modules/test-p1689.py| 222 ++ gcc/testsuite/lib/modules.exp | 71 ++ libcpp/charset.cc | 28 ++- libcpp/include/cpplib.h | 12 +- libcpp/include/mkdeps.h | 17 +- libcpp/init.cc| 13 +- libcpp/internal.h | 2 + libcpp/mkdeps.cc | 149 +++- 40 files changed, 789 insertions(+), 30 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f-MD.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fi.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj.C create
Re: [PATCH] c++: Define built-in for std::tuple_element [PR100157]
On Tue, 17 Jan 2023, Jason Merrill wrote: > On 1/9/23 14:25, Patrick Palka via Gcc-patches wrote: > > On Mon, 9 Jan 2023, Patrick Palka wrote: > > > > > On Wed, 5 Oct 2022, Patrick Palka wrote: > > > > > > > On Thu, 7 Jul 2022, Jonathan Wakely via Gcc-patches wrote: > > > > > > > > > This adds a new built-in to replace the recursive class template > > > > > instantiations done by traits such as std::tuple_element and > > > > > std::variant_alternative. The purpose is to select the Nth type from a > > > > > list of types, e.g. __builtin_type_pack_element(1, char, int, float) > > > > > is > > > > > int. > > > > > > > > > > For a pathological example tuple_element_t<1000, tuple<2000 types...>> > > > > > the compilation time is reduced by more than 90% and the memory used > > > > > by > > > > > the compiler is reduced by 97%. In realistic examples the gains will > > > > > be > > > > > much smaller, but still relevant. > > > > > > > > > > Clang has a similar built-in, __type_pack_element, but that's > > > > > a > > > > > "magic template" built-in using <> syntax, which GCC doesn't support. > > > > > So > > > > > this provides an equivalent feature, but as a built-in function using > > > > > parens instead of <>. I don't really like the name "type pack element" > > > > > (it gives you an element from a pack of types) but the > > > > > semi-consistency > > > > > with Clang seems like a reasonable argument in favour of keeping the > > > > > name. I'd be open to alternative names though, e.g. __builtin_nth_type > > > > > or __builtin_type_at_index. > > > > > > > > Rather than giving the trait a different name from __type_pack_element, > > > > I wonder if we could just special case cp_parser_trait to expect <> > > > > instead of parens for this trait? > > > > > > > > Btw the frontend recently got a generic TRAIT_TYPE tree code, which gets > > > > rid of much of the boilerplate of adding a new type-yielding built-in > > > > trait, see e.g. cp-trait.def. > > > > > > Here's a tested patch based on Jonathan's original patch that implements > > > the built-in in terms of TRAIT_TYPE, names it __type_pack_element > > > instead of __builtin_type_pack_element, and treats invocations of it > > > like a template-id instead of a call (to match Clang). > > > > > > -- >8 -- > > > > > > Subject: [PATCH] c++: Define built-in for std::tuple_element [PR100157] > > > > > > This adds a new built-in to replace the recursive class template > > > instantiations done by traits such as std::tuple_element and > > > std::variant_alternative. The purpose is to select the Nth type from a > > > list of types, e.g. __type_pack_element<1, char, int, float> is int. > > > We implement it as a special kind of TRAIT_TYPE. > > > > > > For a pathological example tuple_element_t<1000, tuple<2000 types...>> > > > the compilation time is reduced by more than 90% and the memory used by > > > the compiler is reduced by 97%. In realistic examples the gains will be > > > much smaller, but still relevant. > > > > > > Unlike the other built-in traits, __type_pack_element uses template-id > > > syntax instead of call syntax and is SFINAE-enabled, matching Clang's > > > implementation. And like the other built-in traits, it's not mangleable > > > so we can't use it directly in function signatures. > > > > > > Some caveats: > > > > > >* Clang's version of the built-in seems to act like a "magic template" > > > that can e.g. be used as a template template argument. For > > > simplicity > > > we implement it in a more ad-hoc way. > > >* Our parsing of the <>'s in __type_pack_element<...> is currently > > > rudimentary and doesn't try to disambiguate a trailing >> vs > > > > > as cp_parser_enclosed_template_argument_list does. > > > > Hmm, this latter caveat turns out to be inconvenient (for code such as > > type_pack_element3.C) and admits an easy workaround inspired by what > > cp_parser_enclosed_template_argument_list does. > > > > v2: Consider the >> in __type_pack_element<0, int, char>> to be two >'s. > > Handle non-type TRAIT_TYPE_TYPE1 in strip_typedefs (for sake of > > CPTK_TYPE_PACK_ELEMENT). > > Why not use cp_parser_enclosed_template_argument_list directly? If we used cp_parser_enclosed_template_argument_list we would then need to convert the returned TREE_VEC into a TREE_LIST and also diagnose argument kind mismatches (i.e. verify the first argument is an expression and the rest are types). It seemed like more complexity overall then just duplicating the >> splitting logic, but I can do that if you prefer? > > > -- >8 -- > > > > Subject: [PATCH] c++: Define built-in for std::tuple_element [PR100157] > > > > This adds a new built-in to replace the recursive class template > > instantiations done by traits such as std::tuple_element and > > std::variant_alternative. The purpose is to select the Nth type from a > > list of types, e.g. __type_pack_element<1, char, int, float> is int. > > We
[PATCH] c++ modules: uninstantiated template friend class [PR104234]
Here we're not clearing DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P for
the instantiated/injected template friend class B, which confuses a
later call to get_originating_module_decl for B. This patch fixes this
by clearing the flag in tsubst_friend_class (as is already done for
template friend functions by r11-5730-gf7aeb823d9b0de).
After fixing that, we still fail to compile the testcase, rejecting the
later definition of B with
friend-6_a.C:10:26: error: cannot declare ‘struct B’ in a different module
ultimately because DECL_MODULE_ATTACH_P wasn't set on the original
(injected) declaration of B. This patch fixes this by calling
set_originating_module in tsubst_friend_class, but for that to work it
seems we need to relax the assert in this latter function since
get_originating_module_decl when called on the TYPE_DECL for B returns
the corresponding TEMPLATE_DECL.
(Alternatively we can instead call set_originating_module on the
TYPE_DECL B as soon as it's created in lookup_template_class (which is
what pushtag does), which doesn't need this assert change because at
this point the TYPE_DECL doesn't have any TEMPLATE_INFO so
get_originating_module_decl becomes a no-op. Would that be preferable?)
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
PR c++/104234
gcc/cp/ChangeLog:
* module.cc (set_originating_module): Document default argument.
Relax assert to look through DECL_TEMPLATE_RESULT in the result
of get_originating_module_decl.
* pt.cc (tsubst_friend_class): Clear
DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P and call
set_originating_module on the instantiated template friend class.
gcc/testsuite/ChangeLog:
* g++.dg/modules/friend-6_a.C: New test.
---
gcc/cp/module.cc | 8 ++--
gcc/cp/pt.cc | 3 +++
gcc/testsuite/g++.dg/modules/friend-6_a.C | 10 ++
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7133009dba5..234ce43b70f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18843,14 +18843,18 @@ set_defining_module (tree decl)
}
void
-set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
+set_originating_module (tree decl, bool friend_p /* = false */)
{
set_instantiating_module (decl);
if (!DECL_NAMESPACE_SCOPE_P (decl))
return;
- gcc_checking_assert (friend_p || decl == get_originating_module_decl (decl));
+ if (!friend_p)
+{
+ tree o = get_originating_module_decl (decl);
+ gcc_checking_assert (STRIP_TEMPLATE (o) == decl);
+}
if (module_attach_p ())
{
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index cbe5898b553..f2ee74025e7 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11520,6 +11520,9 @@ tsubst_friend_class (tree friend_tmpl, tree args)
CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))
= INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)));
+ DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (tmpl) = false;
+ set_originating_module (DECL_TEMPLATE_RESULT (tmpl));
+
/* Substitute into and set the constraints on the new declaration. */
if (tree ci = get_constraints (friend_tmpl))
{
diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C
b/gcc/testsuite/g++.dg/modules/friend-6_a.C
new file mode 100644
index 000..97017e4ee78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
@@ -0,0 +1,10 @@
+// PR c++/104234
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi pr104234 }
+export module pr104234;
+
+template struct A {
+ template friend struct B;
+};
+A a;
+template struct B { };
--
2.39.1.348.g5dec958dcf
[PATCH, committed] Fortran: ICE in gfc_compare_array_spec [PR108528]
Dear all,
I've committed the attached simple and obvious patch by
Steve after regtesting on x86_64-pc-linux-gnu.
Instead of generating an internal error when wrong types
are passed to a bounds comparison, simply return false.
Committed:
https://gcc.gnu.org/g:9fb9da3d38513d320bfea72050f7a59688595e0b
Thanks,
Harald
From 9fb9da3d38513d320bfea72050f7a59688595e0b Mon Sep 17 00:00:00 2001
From: Steve Kargl
Date: Wed, 25 Jan 2023 20:38:43 +0100
Subject: [PATCH] Fortran: ICE in gfc_compare_array_spec [PR108528]
gcc/fortran/ChangeLog:
PR fortran/108528
* array.cc (compare_bounds): Return false instead of generating an
internal error on an invalid argument type.
gcc/testsuite/ChangeLog:
PR fortran/108528
* gfortran.dg/pr108528.f90: New test.
---
gcc/fortran/array.cc | 4 ++--
gcc/testsuite/gfortran.dg/pr108528.f90 | 9 +
2 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr108528.f90
diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index e8a2c32a627..be5eb8b6a0f 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -967,7 +967,7 @@ gfc_copy_array_spec (gfc_array_spec *src)
/* Returns nonzero if the two expressions are equal.
- We should not need to support more than constant values, as thatâs what is
+ We should not need to support more than constant values, as that's what is
allowed in derived type component array spec. However, we may create types
with non-constant array spec for dummy variable class container types, for
which the _data component holds the array spec of the variable declaration.
@@ -979,7 +979,7 @@ compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
if (bound1 == NULL || bound2 == NULL
|| bound1->ts.type != BT_INTEGER
|| bound2->ts.type != BT_INTEGER)
-gfc_internal_error ("gfc_compare_array_spec(): Array spec clobbered");
+return false;
/* What qualifies as identical bounds? We could probably just check that the
expressions are exact clones. We avoid rewriting a specific comparison
diff --git a/gcc/testsuite/gfortran.dg/pr108528.f90 b/gcc/testsuite/gfortran.dg/pr108528.f90
new file mode 100644
index 000..7a353cb7eab
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108528.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/108528 -
+! Contributed by G.Steinmetz
+
+function f() ! { dg-error "mismatched array specifications" }
+ integer :: f((2.)) ! { dg-error "must be of INTEGER type" }
+ integer :: g((2))
+entry g()
+end
--
2.35.3
Re: [PATCH v4] c++: Reject UDLs in certain contexts [PR105300]
On 1/13/23 18:22, Marek Polacek wrote:
On Sat, Dec 03, 2022 at 02:58:16PM -0500, Jason Merrill wrote:
On 12/2/22 18:58, Marek Polacek wrote:
On Fri, Nov 18, 2022 at 08:39:10PM -0500, Jason Merrill wrote:
On 11/18/22 18:52, Marek Polacek wrote:
+/* Parse a string literal or user defined string literal.
+
+ user-defined-string-literal :
+ string-literal ud-suffix
+
+ Parameters as for cp_parser_string_literal. If LOOKUP_UDLIT, perform
+ a lookup for a suitable template function. */
+
+static inline cp_expr
+cp_parser_userdef_string_literal (cp_parser *parser, bool translate,
+ bool wide_ok, bool lookup_udlit = true)
I think this function doesn't need the translate and wide_ok parms, they can
always be true.
I've dropped the wide_ok one, but not the other, because...
+{
+ return cp_parser_string_literal_common (parser, translate, wide_ok,
+ /*udl_ok=*/true, lookup_udlit);
+}
+
/* Look up a literal operator with the name and the exact arguments. */
static tree
@@ -4913,7 +4955,7 @@ cp_parser_userdef_numeric_literal (cp_parser *parser)
as arguments. */
static tree
-cp_parser_userdef_string_literal (tree literal)
+finish_userdef_string_literal (tree literal)
{
tree suffix_id = USERDEF_LITERAL_SUFFIX_ID (literal);
tree name = cp_literal_operator_id (IDENTIFIER_POINTER (suffix_id));
@@ -5652,10 +5694,10 @@ cp_parser_primary_expression (cp_parser *parser,
case CPP_UTF8STRING_USERDEF:
/* ??? Should wide strings be allowed when parser->translate_strings_p
is false (i.e. in attributes)? If not, we can kill the third
-argument to cp_parser_string_literal. */
I think the answer to this old question is no: if we have an
encoding-prefix, we should be translating.
...I don't actually know how to resolve this. wide_ok is always true here.
Should that change? Or rather, should translate be false for CPP_STRING only?
Sorry it's taken so long to get back to this.
The one current exception to my assertion above is static_assert, for which
we currently allow encoding-prefixes but don't translate. I think this is
wrong, that we should translate the string. But I'm not confident of that.
But to your question, yes: when translate is false, I think we also don't
want to allow UDLs. So _userdef can always pass true for translate. And as
below we should call it only when translate would be true.
Done: _userdef no longer has the translate paramater and it's only called
when parser->translate_strings_p.
Incidentally, it seems that we set translate off for all attributes, even
ones that would take a normal expression argument where presumably we do
want translation (and UDLs). The whole business of different parsing for
different attributes is a headache. You don't need to deal with this now.
- return (cp_parser_string_literal (parser,
- parser->translate_strings_p,
- true)
+argument to cp_parser_{,userdef}string_literal. */
+ return (cp_parser_userdef_string_literal (parser,
+ parser->translate_strings_p,
+ /*wide_ok=*/true)
For CPP_*STRING* without _USERDEF, we should still call
cp_parser_string_literal.
It looks like we always have to call cp_parser_userdef_string_literal
otherwise this would be reejcted:
std::string concat01 = "Hello, " "World!"_www;
Because first we see a CPP_STRING but the subsequent UDL shouldn't
be rejected.
Ah, I didn't notice the function was handling a sequence of string-literals.
So maybe we want to call _userdef here when translate_strings_p, and not
when it's false.
Resolved by the change above. Thanks,
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK, thanks.
-- >8 --
In this PR, we are crashing because we've encountered a UDL where a
string-literal is expected. This patch makes the parser reject string
and character UDLs in all places where the grammar requires a
string-literal and not a user-defined-string-literal.
I've introduced two new wrappers; the existing cp_parser_string_literal
was renamed to cp_parser_string_literal_common and should not be called
directly. finish_userdef_string_literal is renamed from
cp_parser_userdef_string_literal.
PR c++/105300
gcc/c-family/ChangeLog:
* c-pragma.cc (handle_pragma_message): Warn for CPP_STRING_USERDEF.
gcc/cp/ChangeLog:
* parser.cc: Remove unnecessary forward declarations.
(cp_parser_string_literal): New wrapper.
(cp_parser_string_literal_common): Renamed from
cp_parser_string_literal. Add a bool parameter. Give an error when
UDLs are not permitted.
(cp_parser_userdef_string_literal): New wrapper.
(finish_userdef_string_literal): Renamed from
[PATCH] minor optimization bug in basic_string move assignment
Let's submit a proper patch proposal then.
The occasion for me to ask if there is any reason for cow string not
being C++11 allocator compliant ? Just lack of interest ?
I wanted to consider it to get rid of the __gnu_debug::_Safe_container
_IsCxx11AllocatorAware template parameter.
libstdc++: Optimize basic_string move assignment
Since resolution of Issue 2593 [1] we can consider that equal
allocators
before the propagate-on-move-assignment operations will still be equal
afterward.
So we can extend the optimization of transfering the storage of the
move-to
instance to the move-from one that is currently limited to always equal
allocators.
[1] https://cplusplus.github.io/LWG/issue2593
libstdc++-v3/ChangeLog:
* include/bits/basic_string.h (operator=(basic_string&&)):
Transfer move-to
storage to the move-from instance when allocators are equal.
*
testsuite/21_strings/basic_string/allocator/char/move_assign.cc (test04):
New test case.
Tested under linux x86_64, ok to commit ?
François
On 17/01/23 20:18, Jonathan Wakely wrote:
On Wed, 4 Jan 2023 at 18:21, François Dumont via Libstdc++
wrote:
On 04/01/23 00:11, waffl3x via Libstdc++ wrote:
Example: https://godbolt.org/z/sKhGqG1qK
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/basic_string.h;hb=HEAD#l880
When move assigning to a basic_string, the allocated memory of the moved into
string is stored into the source string instead of deallocating it, a good
optimization when everything is compatible. However in the case of a stateful
allocator (is_always_true() evaluating as false) this optimization is never
taken. Unless there is some reason I can't think of that makes equal stateful
allocators incompatible here, I believe the if statement on line 880 of
basic_string.h should also compare the equality of each strings allocator. The
first condition in the function seems to indicate to me that this scenario was
being considered and just forgotten about, as the memory doesn't get
deallocated immediately if the two allocators are equal. I'll note that because
of how everything is handled, this doesn't result in a leak so this bug is
still only a minor missed optimization.
mailto:libstd...@gcc.gnu.org
Hmmm, I don't know, at least it is not as simple as you present it.
You cannot add a check on allocator equality as you are proposing
because it is too late. __str allocator might have already been
propagated to *this on the previous call to std::__alloc_on_move. Note
that current check is done only if
!_Alloc_traits::_S_propagate_on_move_assign().
This patch might do the job but I wonder if equal allocators can become
un-equal after the propagate-on-move-assignment ?
Since https://cplusplus.github.io/LWG/issue2593 they can't. But I
think when I wrote that code, they could do, which is probably why the
optimization wasn't done.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index aa018262c98..c81dc0d425a 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -844,9 +844,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
operator=(basic_string&& __str)
noexcept(_Alloc_traits::_S_nothrow_move())
{
+ const bool __equal_allocs = _Alloc_traits::_S_always_equal()
+ || _M_get_allocator() == __str._M_get_allocator();
if (!_M_is_local() && _Alloc_traits::_S_propagate_on_move_assign()
- && !_Alloc_traits::_S_always_equal()
- && _M_get_allocator() != __str._M_get_allocator())
+ && !__equal_allocs)
{
// Destroy existing storage before replacing allocator.
_M_destroy(_M_allocated_capacity);
@@ -868,16 +869,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_M_set_length(__str.size());
}
}
- else if (_Alloc_traits::_S_propagate_on_move_assign()
- || _Alloc_traits::_S_always_equal()
- || _M_get_allocator() == __str._M_get_allocator())
+ else if (_Alloc_traits::_S_propagate_on_move_assign() || __equal_allocs)
{
// Just move the allocated pointer, our allocator can free it.
pointer __data = nullptr;
size_type __capacity;
if (!_M_is_local())
{
- if (_Alloc_traits::_S_always_equal())
+ if (__equal_allocs)
{
// __str can reuse our existing storage.
__data = _M_data();
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/move_assign.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/move_assign.cc
index cc58348e116..21e0b1cb4f4 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/move_assign.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/move_assign.cc
@@ -28,6 +28,8 @@ const C c = 'a';
using traits = std::char_traits;
using __gnu_test::propagating_allocator;
+using __gnu_test::tracker_allocator_counter;
+using __gnu_test::tracker_allocator;
void
[PATCH] PR tree-optimization/108447 - Do not try to logical fold floating point relations.
This boils down to a single place where we are trying to do things with
relations in ranger that are simply not safe when we need to honor NANs.
I think we can avoid all the other shuffling of relations, and simply
not perform this optimization when it comes to floats.
The case the routine handles is:
c_2 = a_6 > b_7
c_3 = a_6 < b_7
c_4 = c_2 && c_3
c_2 and c_3 can never be true at the same time, Therefore c_4 can always
resolve to false based purely on the relations.
Range-ops is unable to do this optimization directly as it requires
examining things from outside the statement, and is not easily
communicated a simple relation to operator_logical_and.
This routine proceeds to look at the definitions of c_2 and c_3, tries
to determine if there are common operands, and queries for any relations
between them. If it turns out there is something, depending on whether
its && or || , we use intersection or union to determine if the result
of the logical operation can be folded. If HONOR_NANS is true for the
float type, then we cannot do this optimization, and bail early.
At this point I do not think we need to do any of the other things
proposed to relations, so we don't need either of the other 2 patches
this release.
Bootstraps on x86_64-pc-linux-gnu with no regressions. OK for trunk?
Andrew
commit 0b080e9579045dd054e9b3289d123d6b66567e3e
Author: Andrew MacLeod
Date: Wed Jan 25 11:13:23 2023 -0500
Do not try to logical fold floating point relations.
relation_fold_and_or looks for relations among common operands feeding
logical ands and ors. With no knowledge of NANs, it should not attempt
to do this with floating point ssa names.
PR tree-optimization/108447
gcc/
* gimple-range-fold.cc (old_using_range::relation_fold_and_or):
Do not attempt to fold HONOR_NAN types.
gcc/testsuite/
* gcc.dg/pr108447.c: New.
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 91eb6298254..9c5359a3fc6 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
lhs_range, gimple *s,
if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
return;
+ if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
+return;
+
// Make sure they are the same dependencies, and detect the order of the
// relationship.
bool reverse_op2 = true;
diff --git a/gcc/testsuite/gcc.dg/pr108447.c b/gcc/testsuite/gcc.dg/pr108447.c
new file mode 100644
index 000..cfbaba6d0aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr108447.c
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+__attribute__((noipa)) int
+foo (float x, float y)
+{
+ _Bool cmp1 = x <= y;
+ _Bool cmp2 = x >= y;
+ if (cmp1 && cmp2)
+return 1;
+ else if (!cmp1 && !cmp2)
+return -1;
+ return 0;
+}
+
+int
+main ()
+{
+ if (foo (0.0f, __builtin_nanf ("")) != -1)
+__builtin_abort ();
+ if (foo (__builtin_nanf (""), -42.0f) != -1)
+__builtin_abort ();
+ if (foo (0.0f, -0.0f) != 1)
+__builtin_abort ();
+ if (foo (42.0f, 42.0f) != 1)
+__builtin_abort ();
+ if (foo (42.0f, -0.0f) != 0)
+__builtin_abort ();
+ if (foo (0.0f, -42.0f) != 0)
+__builtin_abort ();
+ return 0;
+}
+
Re: [PATCH 3/3] arm: Fix MVE predicates synthesis [PR 108443]
Looks like the first patch was missing a change I had made to prevent
mve_bool_vec_to_const ICEing if called with a non-vector immediate. Now
included.
On 24/01/2023 13:56, Andre Vieira (lists) via Gcc-patches wrote:
Hi,
This patch fixes the way we synthesize MVE predicate immediates and
fixes some other inconsistencies around predicates. For instance this
patch fixes the modes used in the vctp intrinsics, to couple them with
predicate modes with the appropriate lane numbers. For this V2QI is
added to represent a predicate created by vctp64q. The reason we use
V2QI and not for instance a V2BI with 8-bit boolean modes is because we
are trying to avoid having two 'INT' modes of the same size. We make
sure we use the V2QI mode instead of HI for any instruction working on
two lanes of 64-bits consuming a predicate.
Bootstrapped on aarch64-none-linux-gnu and regression tested on
arm-none-eabi and armeb-none-eabi for armv8.1-m.main+mve.fp.
OK for trunk?
gcc/ChangeLog:
PR target/108443
* config/arm/arm.h (VALID_MVE_PRED_MODE): Add V2QI.
* config/arm/arm.cc (thumb2_legitimate_address_p): Use HImode for
addressing MVE predicate modes.
(mve_bool_vec_to_const): Change to represent correct MVE predicate
format.
(arm_hard_regno_mode_ok): Use VALID_MVE_PRED_MODE instead of
checking modes.
(arm_vector_mode_supported_p): Likewise.
(arm_mode_to_pred_mode): Add V2QI.
* config/arm/arm-builtins.cc (UNOP_PRED_UNONE_QUALIFIERS): New
qualifier.
(UNOP_PRED_PRED_QUALIFIERS): New qualifier
(BINOP_PRED_UNONE_PRED_QUALIFIERS): New qualifier.
(v2qi_UP): New macro.
(v4bi_UP): New macro.
(v8bi_UP): New macro.
(v16bi_UP): New macro.
(arm_expand_builtin_args): Make it able to expand the new predicate
modes.
* config/arm/arm-modes.def (V2QI): New mode.
* config/arm/arm-simd-builtin-types.def (Pred1x16_t, Pred2x8_t
Pred4x4_t): Remove unused predicate builtin types.
* config/arm/arm_mve.h (__arm_vctp16q, __arm_vctp32q, __arm_vctp64q,
__arm_vctp8q, __arm_vpnot, __arm_vctp8q_m, __arm_vctp64q_m,
__arm_vctp32q_m, __arm_vctp16q_m): Use predicate modes.
* config/arm/arm_mve_builtins.def (vctp16q, vctp32q, vctp64q, vctp8q,
vpnot, vctp8q_m, vctp16q_m, vctp32q_m, vctp64q_m): Likewise.
* config/arm/constraints.md (DB): Check for VALID_MVE_PRED_MODE
instead
of MODE_VECTOR_BOOL.
* config/arm/iterators.md (MVE_7, MVE_7_HI): Add V2QI
(MVE_VPRED): Likewise.
(MVE_vpred): Add V2QI and map upper case predicate modes to
lower case.
(MVE_vctp): New mode attribute.
(mode1): Remove.
(VCTPQ): Remove.
(VCTPQ_M): Remove.
* config/arm/mve.md (mve_vctpqhi): Rename this...
(mve_vctpq): ... to this. And use new mode
attributes.
(mve_vpnothi): Rename this...
(mve_vpnotv16bi): ... to this.
(mve_vctpq_mhi): Rename this...
(mve_vctpq_m):... to this.
(mve_vldrdq_gather_base_z_v2di,
mve_vldrdq_gather_offset_z_v2di,
mve_vldrdq_gather_shifted_offset_z_v2di,
mve_vstrdq_scatter_base_p_v2di,
mve_vstrdq_scatter_offset_p_v2di,
mve_vstrdq_scatter_offset_p_v2di_insn,
mve_vstrdq_scatter_shifted_offset_p_v2di,
mve_vstrdq_scatter_shifted_offset_p_v2di_insn,
mve_vstrdq_scatter_base_wb_p_v2di,
mve_vldrdq_gather_base_wb_z_v2di,
mve_vldrdq_gather_base_nowb_z_v2di,
mve_vldrdq_gather_base_wb_z_v2di_insn): Use V2QI insead of
HI for predicates.
* config/arm/unspecs.md (VCTP8Q, VCTP16Q, VCTP32Q, VCTP64Q): Replace
these...
(VCTP): ... with this.
(VCTP8Q_M, VCTP16Q_M, VCTP32Q_M, VCTP64Q_M): Replace these...
(VCTP_M): ... with this.
* config/arm/vfp.md (*thumb2_movhi_vfp, *thumb2_movhi_fp16): Use
VALID_MVE_PRED_MODE
instead of checking for MODE_VECTOR_BOOL class.
gcc/testsuite/ChangeLog:
* gcc.dg/rtl/arm/mve-vxbi.c: Use new predicate modes.
* gcc.target/arm/mve/pr108443-run.c: New test.
* gcc.target/arm/mve/pr108443.c: New test.diff --git a/gcc/config/arm/arm-builtins.cc b/gcc/config/arm/arm-builtins.cc
index
6c67cec93ff76a4b42f3a0b305f697142e88fcd9..22fc7f10e92b33a861a321e43becee4647738b61
100644
--- a/gcc/config/arm/arm-builtins.cc
+++ b/gcc/config/arm/arm-builtins.cc
@@ -384,6 +384,19 @@ arm_unop_unone_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
#define UNOP_UNONE_IMM_QUALIFIERS \
(arm_unop_unone_imm_qualifiers)
+static enum arm_type_qualifiers
+arm_unop_pred_unone_qualifiers[SIMD_MAX_BUILTIN_ARGS]
+ = { qualifier_predicate, qualifier_unsigned };
+#define UNOP_PRED_UNONE_QUALIFIERS \
+ (arm_unop_pred_unone_qualifiers)
+
+static enum arm_type_qualifiers
+arm_unop_pred_pred_qualifiers[SIMD_MAX_BUILTIN_ARGS]
+ = { qualifier_predicate, qualifier_predicate };
+#define UNOP_PRED_PRED_QUALIFIERS \
+ (arm_unop_pred_pred_qualifiers)
+
+
static enum arm_type_qualifiers
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On 1/25/23 09:48, Jakub Jelinek wrote:
On Wed, Jan 25, 2023 at 09:30:44AM -0500, Andrew MacLeod wrote:
But I'm afraid the above has VREL_OTHER for too many important cases,
unlike intersect where it is for none unless VREL_OTHER is involved, or just
a few ones for union.
Im not sure it is quite that bad. Floating point ranges and range-ops does
a pretty good job of tracking NANs in the ranges. They then utilize any
available relation in addition to that. So within floating point processing,
What I meant is that when we need to (and we have to, trying to do some
weird changes in intersect doesn't really improve anything) change the
relation_negate or its callers of a relation for floating point with
possible NANs from current inversion of VREL_{LT,GT,LE,GE} which are quite
frequent to VREL_OTHER (I don't know), it can affect a lot of code.
Now, sure, we could try to improve the situation a little bit by not
using just HONOR_NANS (type) as the decider whether we need the new 16
cases VREL_* handling (or 8 + VREL_OTHER) or whether we can use just the 8
cases VREL_* handling. Because, if HONOR_NANS (type) and frange can prove
that neither operand is maybe_nan and neither operand is known_nan, then
we can also use just the old 8 VREL_* codes and their relationships.
And perhaps if either operand is known_nan, then on the other side we know
it is VREL_OTHER (VREL_UNORDERED), not anything else.
Though, exactly for this I'd say it is more work and something for GCC 14.
Proper handling of relation_negate is I'm afraid required for GCC 13.
Except we don't actually use relation_negate () anywhere... I can
delete the functions in class value_relation and value-relation.h/cc and
everything compiles fine. Its provided because I figured it would be
used someday, but the range-ops handlers simply issues the appropriate
relation for the LHS.. be it true or false. they don't pick one and
negate it to produce the other.
I think you are missing that the VREL_* relation is not the end result
used to calculate things, merely a tag that used by the range-ops
handlers to assist with understanding un-obvious relations between
operands on the stmt.
This change is mostly for the benefit of the one place in ranger where
its slightly beyond range-ops.. when we have 2 conditions feeding and
logical && or ||, we look to see if there is any common ground/relation
between the operands feeding the logical operation:
// c_2 = a_6 > b_7
// c_3 = a_6 < b_7
// c_4 = c_2 && c_3
// c_2 and c_3 can never be true at the same time,
// Therefore c_4 can always resolve to false based purely on the relations.
void fold_using_range::relation_fold_and_or (irange& lhs_range, gimple
*s, fur_source )
Range-ops is unable to do anything with this as it requires examining
things from outside the statement, and is not easily communicated a
simple relation to operator_logical_and.
THIs routine proceeds to look at the defintions of c_2 and c_3, tries to
determine if there are common operands, and queries for any relations
between them. If it turns out, this is something, we use intersection
or fold to determine if the result of the logical operation can be
folded. THis fix is almost exclusively about that.
In GCC13, I don't think there are any uses of the relation oracle
outside of ranger and range-ops.
So, given that, perhaps the simplest thing to do is bail on all this
change, and instead simply do the following which also fixes the PR. (im
running it thru tests as we speak)
diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 91eb6298254..9c5359a3fc6 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1039,6 +1039,9 @@ fold_using_range::relation_fold_and_or (irange&
lhs_range, gimple *s,
if (!ssa1_dep1 || !ssa1_dep2 || !ssa2_dep1 || !ssa2_dep2)
return;
+ if (HONOR_NANS (TREE_TYPE (ssa1_dep1)))
+ return;
+
// Make sure they are the same dependencies, and detect the order of the
// relationship.
bool reverse_op2 = true;
Re: [PATCH] Add support for x86_64-*-gnu-* targets to build x86_64 gnumach/hurd
Ping
On Mon, Jan 9, 2023 at 1:00 PM Flávio Cruz wrote:
> Friendly ping
>
> On Mon, Dec 26, 2022 at 12:34 PM Flavio Cruz wrote:
>
>> Tested by building a toolchain and compiling gnumach for x86_64 [1].
>> This is the basic version without unwind support which I think is only
>> required to
>> implement exceptions.
>>
>> [1] https://github.com/flavioc/cross-hurd/blob/master/bootstrap-kernel.sh
>> .
>>
>> ---
>> gcc/config.gcc | 5 -
>> gcc/config/i386/gnu64.h | 40 +
>> libgcc/config.host | 8 ++-
>> libgcc/config/i386/gnu-unwind.h | 10 +
>> 4 files changed, 61 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/config/i386/gnu64.h
>>
>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>> index 95190233820..0e2b15768bf 100644
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -1955,7 +1955,7 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu |
>> i[34567]86-*-gnu* | i[34567]8
>> ;;
>> esac
>> ;;
>> -x86_64-*-linux* | x86_64-*-kfreebsd*-gnu)
>> +x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | x86_64-*-gnu*)
>> tm_file="${tm_file} i386/unix.h i386/att.h elfos.h gnu-user.h
>> glibc-stdint.h \
>> i386/x86-64.h i386/gnu-user-common.h i386/gnu-user64.h"
>> case ${target} in
>> @@ -1966,6 +1966,9 @@ x86_64-*-linux* | x86_64-*-kfreebsd*-gnu)
>> x86_64-*-kfreebsd*-gnu)
>> tm_file="${tm_file} kfreebsd-gnu.h i386/kfreebsd-gnu64.h"
>> ;;
>> + x86_64-*-gnu*)
>> + tm_file="${tm_file} gnu.h i386/gnu64.h"
>> + ;;
>> esac
>> tmake_file="${tmake_file} i386/t-linux64"
>> x86_multilibs="${with_multilib_list}"
>> diff --git a/gcc/config/i386/gnu64.h b/gcc/config/i386/gnu64.h
>> new file mode 100644
>> index 000..a1ecfaa1cdb
>> --- /dev/null
>> +++ b/gcc/config/i386/gnu64.h
>> @@ -0,0 +1,40 @@
>> +/* Configuration for an x86_64 running GNU with ELF as the target
>> machine. */
>> +
>> +/*
>> +Copyright (C) 2022 Free Software Foundation, Inc.
>> +
>> +This file is part of GCC.
>> +
>> +GCC 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 of the License, or
>> +(at your option) any later version.
>> +
>> +GCC 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 GCC. If not, see
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On Wed, Jan 25, 2023 at 09:30:44AM -0500, Andrew MacLeod wrote:
> > But I'm afraid the above has VREL_OTHER for too many important cases,
> > unlike intersect where it is for none unless VREL_OTHER is involved, or just
> > a few ones for union.
>
> Im not sure it is quite that bad. Floating point ranges and range-ops does
> a pretty good job of tracking NANs in the ranges. They then utilize any
> available relation in addition to that. So within floating point processing,
What I meant is that when we need to (and we have to, trying to do some
weird changes in intersect doesn't really improve anything) change the
relation_negate or its callers of a relation for floating point with
possible NANs from current inversion of VREL_{LT,GT,LE,GE} which are quite
frequent to VREL_OTHER (I don't know), it can affect a lot of code.
Now, sure, we could try to improve the situation a little bit by not
using just HONOR_NANS (type) as the decider whether we need the new 16
cases VREL_* handling (or 8 + VREL_OTHER) or whether we can use just the 8
cases VREL_* handling. Because, if HONOR_NANS (type) and frange can prove
that neither operand is maybe_nan and neither operand is known_nan, then
we can also use just the old 8 VREL_* codes and their relationships.
And perhaps if either operand is known_nan, then on the other side we know
it is VREL_OTHER (VREL_UNORDERED), not anything else.
Though, exactly for this I'd say it is more work and something for GCC 14.
Proper handling of relation_negate is I'm afraid required for GCC 13.
Jakub
[Patch][v2] OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
Hi Jakub, hi all,
updated patch included, i.e. avoiding 'count' for 'j' when a 'j.0' would
do (i.e. only local var without the different step calculation). I also
now reject if there is a non-unit step on the loop using an outer var.
Eventually still to be done: replace the 'sorry' by working code, i.e.
implement the suggestions to handle some/all non-unit iteration steps as
proposed in this thread.
On 20.01.23 18:39, Jakub Jelinek wrote:
I think instead of non-unity etc. it is better to talk about constant
step 1 or -1.
I concur.
The actual problem with non-simple loops for non-rectangular loops is
both in case it is an inner loop which uses some outer loop's iterator,
or if it is outer loop whose iterator is used, both of those cases
will not be handled properly.
I have now added a check for the other case as well.
Just to confirm, the following is fine, isn't it?
!$omp simd collapse(4)
do i = 1, 10, 2
do outer_var = 1, 10 ! step = + 1
do j = 1, 10, 2
do inner_var = 1, outer_var ! step = 1
i.e. both the inner_var and outer_var have 'step = 1',
even if other loops in the 'collapse' have step != 1.
I think it should be fine.
OK mainline?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
This patch ensures that loop bounds depending on outer loop vars use the
proper TREE_VEC format. It additionally gives a sorry if such an outer
var has a non-one/non-minus-one increment as currently a count variable
is used in this case (see PR).
Finally, it avoids 'count' and just uses a local loop variable if the
step increment is +/-1.
PR fortran/107424
gcc/fortran/ChangeLog:
* trans-openmp.cc (struct dovar_init_d): Add 'sym' and
'non_unit_incr' members.
(gfc_nonrect_loop_expr): New.
(gfc_trans_omp_do): Call it; use normal loop bounds
for unit stride - and only create local loop var.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/non-rectangular-loop-1.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-2.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-3.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-4.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-5.f90: New test.
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/privatization-1-compute-loop.f90: Update dg-note.
* gfortran.dg/goacc/privatization-1-routine_gang-loop.f90: Likewise.
gcc/fortran/trans-openmp.cc| 238 ++--
.../goacc/privatization-1-compute-loop.f90 | 6 +-
.../goacc/privatization-1-routine_gang-loop.f90| 3 +-
.../libgomp.fortran/non-rectangular-loop-1.f90 | 637 +
.../libgomp.fortran/non-rectangular-loop-1a.f90| 374
.../libgomp.fortran/non-rectangular-loop-2.f90 | 243
.../libgomp.fortran/non-rectangular-loop-3.f90 | 186 ++
.../libgomp.fortran/non-rectangular-loop-4.f90 | 188 ++
.../libgomp.fortran/non-rectangular-loop-5.f90 | 28 +
9 files changed, 1854 insertions(+), 49 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 87213de0918..ccee9e16648 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5116,10 +5116,135 @@ gfc_trans_omp_critical (gfc_code *code)
}
typedef struct dovar_init_d {
+ gfc_symbol *sym;
tree var;
tree init;
+ bool non_unit_iter;
} dovar_init;
+static bool
+gfc_nonrect_loop_expr (stmtblock_t *pblock, gfc_se *sep, int loop_n,
+ gfc_code *code, gfc_expr *expr, vec *inits,
+ int simple, gfc_expr *curr_loop_var)
+{
+ int i;
+ for (i = 0; i < loop_n; i++)
+{
+ gcc_assert (code->ext.iterator->var->expr_type == EXPR_VARIABLE);
+ if (gfc_find_sym_in_expr (code->ext.iterator->var->symtree->n.sym, expr))
+ break;
+ code = code->block->next;
+}
+ if (i >= loop_n)
+return false;
+
+ /* Canonic format: TREE_VEC with [var, multiplier, offset]. */
+ gfc_symbol *var = code->ext.iterator->var->symtree->n.sym;
+
+ tree tree_var = NULL_TREE;
+ tree a1 = integer_one_node;
+ tree a2 = integer_zero_node;
+
+ if (!simple)
+{
+ /* FIXME: Handle non-unit iter steps, cf. PR fortran/107424. */
+ sorry_at (gfc_get_location (_loop_var->where),
+ "non-rectangular loop nest with step other than constant 1 "
+ "or -1 for %qs", curr_loop_var->symtree->n.sym->name);
+ return false;
+}
+
+ dovar_init *di;
+ unsigned ix;
+ FOR_EACH_VEC_ELT (*inits, ix, di)
+if (di->sym == var && !di->non_unit_iter)
+ {
+ tree_var = di->init;
+ gcc_assert (DECL_P (tree_var));
+ break;
+ }
+else if (di->sym == var)
+ {
+ /* FIXME:
Re: [PATCH v3 1/2] aarch64: fix warning emission for ABI break since GCC 9.1
On 1/12/23 14:03, Richard Sandiford wrote: Christophe Lyon writes: While looking at PR 105549, which is about fixing the ABI break introduced in GCC 9.1 in parameter alignment with bit-fields, we noticed that the GCC 9.1 warning is not emitted in all the cases where it should be. This patch fixes that and the next patch in the series fixes the GCC 9.1 break. We split this into two patches since patch #2 introduces a new ABI break starting with GCC 13.1. This way, patch #1 can be back-ported to release branches if needed to fix the GCC 9.1 warning issue. The main idea is to add a new global boolean that indicates whether we're expanding the start of a function, so that aarch64_layout_arg can emit warnings for callees as well as callers. This removes the need for aarch64_function_arg_boundary to warn (with its incomplete information). However, in the first patch there are still cases where we emit warnings were we should not; this is fixed in patch #2 where we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. The fix in aarch64_function_arg_boundary (replacing & with &&) looks like an oversight of a previous commit in this area which changed 'abi_break' from a boolean to an integer. We also take the opportunity to fix the comment above aarch64_function_arg_alignment since the value of the abi_break parameter was changed in a previous commit, no longer matching the description. v2->v3: removed a bogus comment, added C++ tests (copied from the C ones) 2022-11-28 Christophe Lyon Richard Sandiford gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix comment. (aarch64_layout_arg): Factorize warning conditions. (aarch64_function_arg_boundary): Fix typo. * function.cc (currently_expanding_function_start): New variable. (expand_function_start): Handle currently_expanding_function_start. * function.h (currently_expanding_function_start): Declare. gcc/testsuite/ChangeLog: * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning.h: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning.h: New test. OK for trunk, and for backports after a while. Thanks for doing this, and for your patience through it all. I've just backported this patch (only, not the other ones which were not intended for backport) - gcc-12: applied cleanly - gcc-11: applied cleany but I had to replace dg-note with dg-message in the tests - gcc-10: a small conflict, and I had to replace dg-note with dg-message in the tests Thanks, Christophe Richard --- gcc/config/aarch64/aarch64.cc | 28 +++- gcc/function.cc | 5 + gcc/function.h| 2 + .../bitfield-abi-warning-align16-O2-extra.C | 86 .../aarch64/bitfield-abi-warning-align16-O2.C | 87 .../bitfield-abi-warning-align32-O2-extra.C | 119 + .../aarch64/bitfield-abi-warning-align32-O2.C | 119 + .../aarch64/bitfield-abi-warning-align8-O2.C | 16 +++ .../g++.target/aarch64/bitfield-abi-warning.h | 125 ++ .../bitfield-abi-warning-align16-O2-extra.c | 86 .../aarch64/bitfield-abi-warning-align16-O2.c | 87 .../bitfield-abi-warning-align32-O2-extra.c | 119 + .../aarch64/bitfield-abi-warning-align32-O2.c | 119 + .../aarch64/bitfield-abi-warning-align8-O2.c | 16 +++ .../gcc.target/aarch64/bitfield-abi-warning.h | 125 ++ 15 files changed, 1132 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning.h create mode
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On 1/25/23 06:15, Jakub Jelinek wrote:
On Tue, Jan 24, 2023 at 10:57:12AM -0500, Andrew MacLeod wrote:
That is the way VREL_OTHER is implemented in the table.
So the problem is not on the true side of the IF condition as in your
example, its on the false side. We see this commonly in code like this
if (x <= y) // FALSE side registers x > y
blah()
else if (x >= y) // FALSE side registers x < y
blah()
else
// Here we get VREL_UNDEFINED.
In that case, I think the problem isn't in the intersection, which works
correctly, but in wherever we (again, for HONOR_NANS (type) only) make the
assumptions about the else branches.
In the above case, the first blah has VREL_LE, that is correct,
but the else of it (when the condition isn't true but is false)
isn't VREL_GT, but VREL_UNGT (aka VREL_OTHER).
So, for the HONOR_NANS (type) cases we need to adjust relation_negate
callers.
On trees, you can e.g. look at fold-const.cc (invert_tree_comparison),
though that one for HONOR_NANS (type) && flag_trapping_math punts in most
cases to preserve exceptions. But you can see there the !honor_nans
cases where it acts like relation_negate, and then the honor_nans cases,
where VREL_*:
VARYING UNDEFINED LT LE GT GE EQ NE LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE
UNEQ
negate to
UNDEFINED VARYING UNGE UNGT UNLE UNLT NE EQ UNEQ UNORDERED ORDERED GE GT LE LT
LTGT
Or, in the world where VREL_OTHER is a wildcard for
LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE UNEQ
the less useful
VARYING UNDEFINED LT LE GT GE EQ NE OTHER
negates to
UNDEFINED VARYING OTHER OTHER OTHER OTHER NE EQ OTHER
But I'm afraid the above has VREL_OTHER for too many important cases,
unlike intersect where it is for none unless VREL_OTHER is involved, or just
a few ones for union.
Im not sure it is quite that bad. Floating point ranges and range-ops
does a pretty good job of tracking NANs in the ranges. They then utilize
any available relation in addition to that. So within floating point
processing, all our relations are assumed to possibly include NAN, then
the range processing in range-ops decides which variant of the relation
is applicable. When Aldy and I went thru this exercise initally, we
concluded we didnt need all the various forms of relations, that its was
completely isolated within the frange implementation.. which is ideal.
The only place this is causing real "trouble" is within ranger where we
pre-fold a stmt based on the relation, without it ever getting to
rangeops. Yes,we dont represent <> or ordered or unordered in the
relation table, but those are really very specific floating point things
and ranger doesnt need to be aware of them anyway I don't think. theres
not much it can do with them.
Merge points use union and cascading conditions use intersection. The
current fix will eliminate ranger from making any presumptions about
these cases early. Those are about the only things ranger itself uses
relations for.. everything else is deferred to range-ops which
understands all the things like ordered/unordered etc and it will still
fold these expression properly when evaluated.
ie without any relational representation of unordered and unordered, we
still fold this properly because frange tracks it all:
if (x_2(D) unord x_2(D))
goto ; [INV]
else
goto ; [INV]
:
if (x_2(D) ord x_2(D))
goto ; [INV]
else
goto ; [INV]
:
link_error ();
and value relations don't need to be involved. I am currently unaware
of anything with floating point relations we don't track properly in the
end.
So, I wonder if we just shouldn't do it properly immediately and instead of
VREL_OTHER introduce those
VREL_LTGT, /* Less than or greater than. */
VREL_ORDERED, /* Operands not NAN. */
VREL_UNORDERED, /* One or both operands NAN. */
VREL_UNLT, /* Unordered or less than. */
VREL_UNLE, /* Unordered or less than or equal. */
VREL_UNGT, /* Unordered or greater than. */
VREL_UNGE, /* Unordered or greater than or equal. */
VREL_UNEQ /* Unordered or equal. */
In that case, rather than using relation_{negate,union,intersect} etc.
either those functions should take an argument whether it is a HONOR_NANS
(type) floating point or not and use two separate tables under the hood,
or have two sets of routines and choose which one to use in the callers.
I think one routine with an extra argument would be cleaner though.
The above would grow the tables quite a bit (though, we could for the
non-FP cases limit ourselves to a subset), but because all the VREL_*
constants are enumerals with small integers values and the arrays
should be (but aren't for some reason?) static just make the arrays
contain unsigned char and do the casts to the enum in the
relation_{negate,intersect,union} etc. wrappers.
Also, I think the rr_*_table arrays should be const, we don't want to change
them, right? And a few other arrays too, like relation_to_code.
BTW, with the above
Re: [committed] testsuite: Fix up new51.C test on various targets [PR108533]
On 1/24/23 18:16, Jakub Jelinek wrote:
On Mon, Jan 23, 2023 at 10:26:14PM -0500, Jason Merrill via Gcc-patches wrote:
* g++.dg/init/new51.C: New test.
The test fails on targets where size_t is not unsigned long
due to extra diagnostics.
As the testcase is tested in C++98 too, I'm not using decltype (sizeof 0)
but __SIZE_TYPE__.
Tested on x86_64-linux and i686-linux (plus verified with older snapshots
that it ICEs even with the change with both -m32/-m64), committed to
trunk as obvious.
Thanks.
2023-01-25 Jakub Jelinek
PR c++/107329
PR testsuite/108533
* g++.dg/init/new51.C (size_t): New typedef.
(RexxClass::operator new, RexxClass::operator delete): Use size_t
instead of unsigned long.
--- gcc/testsuite/g++.dg/init/new51.C.jj2023-01-24 11:10:13.0
+0100
+++ gcc/testsuite/g++.dg/init/new51.C 2023-01-25 00:05:10.767472447 +0100
@@ -1,9 +1,10 @@
// PR c++/107329
+typedef __SIZE_TYPE__ size_t;
struct RexxClass {
- void *operator new(unsigned long, unsigned long, const char *, RexxClass *,
+ void *operator new(size_t, size_t, const char *, RexxClass *,
RexxClass *);
- void operator delete(void *, unsigned long, const char *, RexxClass *,
+ void operator delete(void *, size_t, const char *, RexxClass *,
RexxClass *);
RexxClass();
};
Jakub
Re: [PATCH] c++: Fix up mangling of static lambdas [PR108525]
On 1/24/23 18:19, Jakub Jelinek wrote:
Hi!
Before the P1169R4 changes, operator () of a lambda was
always a method, so it was fine to pass method_p = 1 unconditionally,
but it isn't always the case, so this patch adds a check for whether
it is a method or nor.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
2023-01-25 Jakub Jelinek
PR c++/108525
* mangle.cc (write_closure_type_name): Don't assume all
lambda operator() fns are methods.
* g++.dg/cpp23/static-operator-call5.C: New test.
--- gcc/cp/mangle.cc.jj 2023-01-16 11:52:16.0 +0100
+++ gcc/cp/mangle.cc2023-01-24 18:59:09.335156301 +0100
@@ -1816,7 +1816,7 @@ write_closure_type_name (const tree type
if (abi_warn_or_compat_version_crosses (18))
G.need_abi_warning = true;
- write_method_parms (parms, /*method_p=*/1, fn);
+ write_method_parms (parms, TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE, fn);
write_char ('E');
if ((LAMBDA_EXPR_SCOPE_SIG_DISCRIMINATOR (lambda)
!= LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR (lambda))
--- gcc/testsuite/g++.dg/cpp23/static-operator-call5.C.jj 2023-01-24
19:03:21.770469929 +0100
+++ gcc/testsuite/g++.dg/cpp23/static-operator-call5.C 2023-01-24
19:02:45.224003615 +0100
@@ -0,0 +1,5 @@
+// PR c++/108525
+// { dg-do compile { target c++23 } }
+
+auto b = [](...) static { return 1; };
+auto foo () { return b (); }
Jakub
[r13-5318 Regression] FAIL: g++.dg/init/new51.C -std=c++98 (test for excess errors) on Linux/x86_64
This is the recent regression on gcc trunk.
Seems it got fixed. If that is the case, plz ignore that.
As mentioned in previous thread, before the mail system got fixed on server, I
will manually forward this email.
BRs,
Haochen
> -Original Message-
> From: haochen.jiang
> Sent: Wednesday, January 25, 2023 7:27 AM
> To: ja...@redhat.com; gcc-regress...@gcc.gnu.org; gcc-patches@gcc.gnu.org;
> Jiang, Haochen
> Subject: [r13-5318 Regression] FAIL: g++.dg/init/new51.C -std=c++98 (test for
> excess errors) on Linux/x86_64
>
> On Linux/x86_64,
>
> 049a52909075117f5112971cc83952af2a818bc1 is the first bad commit commit
> 049a52909075117f5112971cc83952af2a818bc1
> Author: Jason Merrill
> Date: Mon Jan 23 16:25:07 2023 -0500
>
> c++: TARGET_EXPR collapsing [PR107303]
>
> caused
>
> FAIL: g++.dg/init/new51.C -std=c++14 (test for excess errors)
> FAIL: g++.dg/init/new51.C -std=c++17 (test for excess errors)
> FAIL: g++.dg/init/new51.C -std=c++20 (test for excess errors)
> FAIL: g++.dg/init/new51.C -std=c++98 (test for excess errors)
>
> with GCC configured with
>
> ../../gcc/configure --prefix=/export/users/haochenj/src/gcc-
> bisect/master/master/r13-5318/usr --enable-clocale=gnu --with-system-zlib --
> with-demangler-in-ld --with-fpmath=sse --enable-languages=c,c++,fortran --
> enable-cet --without-isl --enable-libmpx x86_64-linux --disable-bootstrap
>
> To reproduce:
>
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=g++.dg/init/new51.C --target_board='unix{-m32}'"
> $ cd {build_dir}/gcc && make check
> RUNTESTFLAGS="dg.exp=g++.dg/init/new51.C --target_board='unix{-m32\ -
> march=cascadelake}'"
>
> (Please do not reply to this email, for question about this report, contact
> me at
> haochen dot jiang at intel.com)
Re: [PATCH 23/23] arm: fix missing extern "C" in MVE tests
Kyrylo Tkachov writes: [...] > > Ok. > Thanks, > Kyrill Hi Kyrill, thanks for reviewing. These and all the previous ones are in with the requested ChangeLogs changes. Regards Andrea
Re: [PATCH] lto: pass through -funwind-tables and -fasynchronous-unwind-tables
On Jan 25 2023, Richard Biener wrote: > where we'd prefer -funwind-tables over -fno-unwind-tables when the > options do not match > across TUs. Note that you likely want to add > -f[asynchronous-]unwind-tables handling > in lto-options.cc:lto_write_options as well so the default is streamed > as explicit option. > Otherwise a single TU with -fno-unwind-tables on x86-64 would cause > the whole LTO > compilation to be built without. I don't think we actually need to handle -fasynchronous-unwind-tables here, since that implies -funwind-tables, and only the latter is relevant for dwarf2out_do_eh_frame. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH] lto: pass through -funwind-tables and -fasynchronous-unwind-tables
On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches wrote: > > The -funwind-tables and -fasynchronous-unwind-tables options are relevant > for the output pass, thus they need to be passed through by the lto > wrapper. > > gcc/ > * lto-wrapper.cc (merge_and_complain): Pass through > -funwind-tables and -fasynchronous-unwind-tables. > (append_compiler_options): Likewise. > --- > gcc/lto-wrapper.cc | 4 > 1 file changed, 4 insertions(+) > > diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc > index 11c4d1b38a4..627e8238606 100644 > --- a/gcc/lto-wrapper.cc > +++ b/gcc/lto-wrapper.cc > @@ -314,6 +314,8 @@ merge_and_complain (vec > _options, > case OPT_fshow_column: > case OPT_fcommon: > case OPT_fgnu_tm: > + case OPT_funwind_tables: > + case OPT_fasynchronous_unwind_tables: > case OPT_g: > /* Do what the old LTO code did - collect exactly one option > setting per OPT code, we pick the first we encounter. A better place is case OPT_fopenmp: case OPT_fopenacc: /* For selected options we can merge conservatively. */ if (existing_opt == -1) decoded_options.safe_push (*foption); /* -fopenmp > -fno-openmp, -fopenacc > -fno-openacc */ else if (foption->value > decoded_options[existing_opt].value) decoded_options[existing_opt] = *foption; break; where we'd prefer -funwind-tables over -fno-unwind-tables when the options do not match across TUs. Note that you likely want to add -f[asynchronous-]unwind-tables handling in lto-options.cc:lto_write_options as well so the default is streamed as explicit option. Otherwise a single TU with -fno-unwind-tables on x86-64 would cause the whole LTO compilation to be built without. > @@ -737,6 +739,8 @@ append_compiler_options (obstack *argv_obstack, > vec opts) > case OPT_fopenacc_dim_: > case OPT_foffload_abi_: > case OPT_fcf_protection_: > + case OPT_funwind_tables: > + case OPT_fasynchronous_unwind_tables: > case OPT_g: > case OPT_O: > case OPT_Ofast: > -- > 2.39.1 > > > -- > Andreas Schwab, SUSE Labs, sch...@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
[PATCH] tree-optimization/108523 - fix endless iteration in VN
The following fixes not converging iteration in value-numbering of
PHI nodes when we use an equivalence to prove the PHI node is
degenerate. We have to avoid the situation where we oscillate
between the two equivalent values because the result is fed back
via a backedge.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
PR tree-optimization/108523
* tree-ssa-sccvn.cc (visit_phi): Avoid using the exclusive
backedge value for the result when using predication to
prove equivalence.
---
gcc/tree-ssa-sccvn.cc | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 0dba3f39aa0..edb553b07cb 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -5826,7 +5826,7 @@ visit_phi (gimple *phi, bool *inserted, bool
backedges_varying_p)
poly_int64 soff, doff;
unsigned n_executable = 0;
edge_iterator ei;
- edge e;
+ edge e, sameval_e = NULL;
/* TODO: We could check for this in initialization, and replace this
with a gcc_assert. */
@@ -5867,7 +5867,10 @@ visit_phi (gimple *phi, bool *inserted, bool
backedges_varying_p)
&& ssa_undefined_value_p (def, false))
seen_undef = def;
else if (sameval == VN_TOP)
- sameval = def;
+ {
+ sameval = def;
+ sameval_e = e;
+ }
else if (!expressions_equal_p (def, sameval))
{
/* We know we're arriving only with invariant addresses here,
@@ -5916,6 +5919,8 @@ visit_phi (gimple *phi, bool *inserted, bool
backedges_varying_p)
fprintf (dump_file, " are equal on edge %d -> %d\n",
e->src->index, e->dest->index);
}
+ if (sameval_e && (sameval_e->flags & EDGE_DFS_BACK))
+ sameval = def;
continue;
}
/* If on all previous edges the value was equal to def
@@ -5935,7 +5940,8 @@ visit_phi (gimple *phi, bool *inserted, bool
backedges_varying_p)
EDGE_PRED (bb, 0)->src->index,
EDGE_PRED (bb, 0)->dest->index);
}
- sameval = def;
+ if (!(e->flags & EDGE_DFS_BACK))
+ sameval = def;
continue;
}
}
@@ -5943,6 +5949,8 @@ visit_phi (gimple *phi, bool *inserted, bool
backedges_varying_p)
sameval = NULL_TREE;
break;
}
+ else
+ sameval_e = NULL;
}
/* If the value we want to use is flowing over the backedge and we
--
2.35.3
[PATCH] Fixup LTO internal docs for option processing
Andreas noticed that when I removed lto_read_all_file_options I
failed to update the internals manual which refers to it. The
following attempts to reflect the current situation.
Pushed.
* doc/lto.texi (Command line options): Reword and update reference
to removed lto_read_all_file_options.
---
gcc/doc/lto.texi | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/gcc/doc/lto.texi b/gcc/doc/lto.texi
index e591e8d2915..eb5f54bf908 100644
--- a/gcc/doc/lto.texi
+++ b/gcc/doc/lto.texi
@@ -170,13 +170,11 @@ object files. This is used at link time to determine the
optimization
level and other settings when they are not explicitly specified at the
linker command line.
-Currently, GCC does not support combining LTO object files compiled
-with different set of the command line options into a single binary.
-At link time, the options given on the command line and the options
-saved on all the files in a link-time set are applied globally. No
-attempt is made at validating the combination of flags (other than the
-usual validation done by option processing). This is implemented in
-@file{lto/lto.cc}:@code{lto_read_all_file_options}.
+Most options are recorded at a per function level and their setting
+restored when processing the functions at link time. Global options
+are composed from options specified at compile time and link time.
+How exactly they are combined or mismatches diagnosed is implemented in
+@file{lto-wrapper.cc}:@code{find_and_merge_options}.
@item Symbol table (@code{.gnu.lto_.symtab})
--
2.35.3
Re: [PATCH] modula-2: Fixes for preprocessing [PR102343, PR108182]
Iain Sandoe writes: > Tested on x86_64, powerpc64 - linux-gnu (with 32b multilibs), > i686, powerpc darwin (with 64b multilibs) x86_64 darwin (with and without > 32b multilib). > OK for trunk? > thanks > Iain > > --- 8< --- > > Modula-2 uses the C preprocessor to implement handling for conditional > code and macros. However, this is not done directly, because the process > is applied recursively to imported definitions and modules. > > The cc1gm2 executable records the parameters as a template command line > needed to create a composite 'cc1 -E' for each file to be preprocessed > starting with the main file from the original command line. > > This patch fixes the capture of the C preprocessor template to include > the target information needed for correct multilib operation. > > In order to match the existing semantics of '-E, -M and -MM' these have > to be handled as a 'pre-processor only' job (i.e. the recursion is omitted > and only the main file is processed). > > Whereas C-family front ends always pre-process, Modula-2 only does so > when specifically requested (via the -fcpp option). > > '-MD, -MMD and -MQ' also require special handling, since (in principle) > these options can be applied to any command line (with -fcpp) providing > dependency information as a by-product. > > TODO: the preprocessor is not able to determine def and mod dependencies > for Modula-2 and so the output of this only shows the object to module > dep. We should be able to append the .def and .mod dependencies. > > The patch amends save-temps handling to cater for the preprocessor > recursion and to avoid writing saved files into the source directories. > > The patch changes the extension for Modula-2 preprocessed source to .m2i > to avoid clashes with .i. > > The main driver code is amended to add default handlers for .mod and .m2i > so that a useful error message will be emitted if the Modula-2 compiler > is not built-in. > > The compiler will now also handle code generation from a .m2i preprocessed > source. > > TODO: We should not need to pass the '-c' option to the compiler to alter > the processing of init code. > > Signed-off-by: Iain Sandoe > > PR modula2/102343 > PR modula2/108182 > > gcc/ChangeLog: > > * gcc.cc: Provide default specs for Modula-2 so that when the > language is not built-in better diagnostics are emitted for > attempts to use .mod or .m2i file extensions. > > gcc/m2/ChangeLog: > > * gm2-compiler/M2Comp.mod: Early exit for pre-processor-only jobs. > * gm2-compiler/M2Options.def (SetPPOnly, GetPPOnly, SetMD, GetMD, > SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir, > GetDumpDir):New. > * gm2-compiler/M2Options.mod:(SetPPOnly, GetPPOnly, SetMD, GetMD, > SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir, > GetDumpDir):New. > * gm2-compiler/M2Preprocess.def (PreprocessModule): Add flag to > indicate the main file. > * gm2-compiler/M2Preprocess.mod: Handle Preprocess-only jobs, > handle MD, MMD and MQ options. > * gm2-gcc/m2options.h (M2Options_SetPPOnly, M2Options_GetPPOnly, > M2Options_SetDumpDir, M2Options_SetMD, M2Options_GetMD, > M2Options_SetMMD, M2Options_GetMMD, M2Options_SetMQ, M2Options_GetMQ, > M2Options_SetObj, M2Options_GetObj): New. > * gm2-gcc/m2type.cc (m2type_InitBaseTypes): Early exit for pre- > processor-only jobs. > * gm2-lang.cc (gm2_langhook_init): Handle preprocess-only commands. > (gm2_langhook_option_lang_mask): Claim C and Driver options so that > we can intercept them for building pre-processor commands. > (gm2_langhook_init_options): Collect the preprocessor line here. > Save options that have different actions for preprocessor and compile > commands. > (gm2_langhook_handle_option): Only handle the modula-2 options here. > (gm2_langhook_post_options): Do not create a back-end for pre- > processor-only jobs. > * gm2spec.cc (lang_specific_driver): Ignore PCH options, append a > scaffold-main for cases where we are building a main module with > -c. > * lang-specs.h: Revise to handle preprocessor-only jobs and to > consume pre-processed files. > * lang.opt: Remove Driver and C options copies (we claim these > separately). Hi Iain, yes LGTM - thanks for the fixes and it is great to have the ability for gm2 to handle preprocessed source regards, Gaius
Re: [aarch64] Use wzr/xzr for assigning vector element to 0
On Mon, 23 Jan 2023 at 22:26, Richard Sandiford
wrote:
>
> Prathamesh Kulkarni writes:
> > On Wed, 18 Jan 2023 at 19:59, Richard Sandiford
> > wrote:
> >>
> >> Prathamesh Kulkarni writes:
> >> > On Tue, 17 Jan 2023 at 18:29, Richard Sandiford
> >> > wrote:
> >> >>
> >> >> Prathamesh Kulkarni writes:
> >> >> > Hi Richard,
> >> >> > For the following (contrived) test:
> >> >> >
> >> >> > void foo(int32x4_t v)
> >> >> > {
> >> >> > v[3] = 0;
> >> >> > return v;
> >> >> > }
> >> >> >
> >> >> > -O2 code-gen:
> >> >> > foo:
> >> >> > fmovs1, wzr
> >> >> > ins v0.s[3], v1.s[0]
> >> >> > ret
> >> >> >
> >> >> > I suppose we can instead emit the following code-gen ?
> >> >> > foo:
> >> >> > ins v0.s[3], wzr
> >> >> > ret
> >> >> >
> >> >> > combine produces:
> >> >> > Failed to match this instruction:
> >> >> > (set (reg:V4SI 95 [ v ])
> >> >> > (vec_merge:V4SI (const_vector:V4SI [
> >> >> > (const_int 0 [0]) repeated x4
> >> >> > ])
> >> >> > (reg:V4SI 97)
> >> >> > (const_int 8 [0x8])))
> >> >> >
> >> >> > So, I wrote the following pattern to match the above insn:
> >> >> > (define_insn "aarch64_simd_vec_set_zero"
> >> >> > [(set (match_operand:VALL_F16 0 "register_operand" "=w")
> >> >> > (vec_merge:VALL_F16
> >> >> > (match_operand:VALL_F16 1 "const_dup0_operand" "w")
> >> >> > (match_operand:VALL_F16 3 "register_operand" "0")
> >> >> > (match_operand:SI 2 "immediate_operand" "i")))]
> >> >> > "TARGET_SIMD"
> >> >> > {
> >> >> > int elt = ENDIAN_LANE_N (, exact_log2 (INTVAL
> >> >> > (operands[2])));
> >> >> > operands[2] = GEN_INT ((HOST_WIDE_INT) 1 << elt);
> >> >> > return "ins\\t%0.[%p2], wzr";
> >> >> > }
> >> >> > )
> >> >> >
> >> >> > which now matches the above insn produced by combine.
> >> >> > However, in reload dump, it creates a new insn for assigning
> >> >> > register to (const_vector (const_int 0)),
> >> >> > which results in:
> >> >> > (insn 19 8 13 2 (set (reg:V4SI 33 v1 [99])
> >> >> > (const_vector:V4SI [
> >> >> > (const_int 0 [0]) repeated x4
> >> >> > ])) "wzr-test.c":8:1 1269 {*aarch64_simd_movv4si}
> >> >> > (nil))
> >> >> > (insn 13 19 14 2 (set (reg/i:V4SI 32 v0)
> >> >> > (vec_merge:V4SI (reg:V4SI 33 v1 [99])
> >> >> > (reg:V4SI 32 v0 [97])
> >> >> > (const_int 8 [0x8]))) "wzr-test.c":8:1 1808
> >> >> > {aarch64_simd_vec_set_zerov4si}
> >> >> > (nil))
> >> >> >
> >> >> > and eventually the code-gen:
> >> >> > foo:
> >> >> > moviv1.4s, 0
> >> >> > ins v0.s[3], wzr
> >> >> > ret
> >> >> >
> >> >> > To get rid of redundant assignment of 0 to v1, I tried to split the
> >> >> > above pattern
> >> >> > as in the attached patch. This works to emit code-gen:
> >> >> > foo:
> >> >> > ins v0.s[3], wzr
> >> >> > ret
> >> >> >
> >> >> > However, I am not sure if this is the right approach. Could you
> >> >> > suggest,
> >> >> > if it'd be possible to get rid of UNSPEC_SETZERO in the patch ?
> >> >>
> >> >> The problem is with the "w" constraint on operand 1, which tells LRA
> >> >> to force the zero into an FPR. It should work if you remove the
> >> >> constraint.
> >> > Ah indeed, sorry about that, changing the constrained works.
> >>
> >> "i" isn't right though, because that's for scalar integers.
> >> There's no need for any constraint here -- the predicate does
> >> all of the work.
> >>
> >> > Does the attached patch look OK after bootstrap+test ?
> >> > Since we're in stage-4, shall it be OK to commit now, or queue it for
> >> > stage-1 ?
> >>
> >> It needs tests as well. :-)
> >>
> >> Also:
> >>
> >> > Thanks,
> >> > Prathamesh
> >> >
> >> >
> >> >>
> >> >> Also, I think you'll need to use zr for the zero, so that
> >> >> it uses xzr for 64-bit elements.
> >> >>
> >> >> I think this and the existing patterns ought to test
> >> >> exact_log2 (INTVAL (operands[2])) >= 0 in the insn condition,
> >> >> since there's no guarantee that RTL optimisations won't form
> >> >> vec_merges that have other masks.
> >> >>
> >> >> Thanks,
> >> >> Richard
> >> >
> >> > [aarch64] Use wzr/xzr for assigning 0 to vector element.
> >> >
> >> > gcc/ChangeLog:
> >> > * config/aaarch64/aarch64-simd.md
> >> > (aarch64_simd_vec_set_zero):
> >> > New pattern.
> >> > * config/aarch64/predicates.md (const_dup0_operand): New.
> >> >
> >> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> >> > b/gcc/config/aarch64/aarch64-simd.md
> >> > index 104088f67d2..8e54ee4e886 100644
> >> > --- a/gcc/config/aarch64/aarch64-simd.md
> >> > +++ b/gcc/config/aarch64/aarch64-simd.md
> >> > @@ -1083,6 +1083,20 @@
> >> >[(set_attr "type" "neon_ins, neon_from_gp,
> >> > neon_load1_one_lane")]
> >> > )
> >> >
> >> > +(define_insn "aarch64_simd_vec_set_zero"
> >> > + [(set (match_operand:VALL_F16 0 "register_operand"
Re: [PATCH] tree-optimization/108522 Use COMPONENT_REF offset when available
On Wed, Jan 25, 2023 at 12:27:13PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Wed, Jan 25, 2023 at 06:22:56AM -0500, Siddhesh Poyarekar wrote:
> > On 2023-01-25 02:44, Richard Biener wrote:
> > > > t = TREE_OPERAND (expr, 1);
> > > > - off = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (t),
> > > > + off = size_binop (PLUS_EXPR,
> > > > + (TREE_OPERAND (expr, 2) ? TREE_OPERAND (expr, 2)
> > > > +: DECL_FIELD_OFFSET (t)),
> > >
> > > That isn't correct - operand 2 is the field offset in units of
> > > DECL_OFFSET_ALIGN (t) / BITS_PER_UNIT.
> > > See component_ref_filed_offset (), maybe you should be using that
> > > function instead?
> >
> > Ahh, and it passed my testing only because I was testing a char. Thanks,
> > I'll test and send an update with additional tests.
>
> I think you want something like:
> struct S {
> char a[n];
> unsigned long long b;
> int d;
> char e[2 * n];
> } s;
> and test say bdos of , 0 and [n - 2], 0
And in the caller compared that to offsetof/sizeof based expressions
for a similar structure which just uses constants instead of the n
in there.
Jakub
Re: [PATCH] tree-optimization/108522 Use COMPONENT_REF offset when available
On Wed, Jan 25, 2023 at 06:22:56AM -0500, Siddhesh Poyarekar wrote:
> On 2023-01-25 02:44, Richard Biener wrote:
> > > t = TREE_OPERAND (expr, 1);
> > > - off = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (t),
> > > + off = size_binop (PLUS_EXPR,
> > > + (TREE_OPERAND (expr, 2) ? TREE_OPERAND (expr, 2)
> > > +: DECL_FIELD_OFFSET (t)),
> >
> > That isn't correct - operand 2 is the field offset in units of
> > DECL_OFFSET_ALIGN (t) / BITS_PER_UNIT.
> > See component_ref_filed_offset (), maybe you should be using that
> > function instead?
>
> Ahh, and it passed my testing only because I was testing a char. Thanks,
> I'll test and send an update with additional tests.
I think you want something like:
struct S {
char a[n];
unsigned long long b;
int d;
char e[2 * n];
} s;
and test say bdos of , 0 and [n - 2], 0
Jakub
[pushed] aarch64: Restore generation of SVE UQDEC instructions
The addition of TARGET_CSSC meant that we wouldn't generate SVE
UQDEC instructions unless +cssc was also enabled.
Fixes:
- gcc.target/aarch64/sve/slp_4.c
- gcc.target/aarch64/sve/slp_10.c
- gcc.target/aarch64/sve/while_4.c
Tested on aarch64-linux-gnu & pushed.
Richard
gcc/
* config/aarch64/aarch64.md (umax3): Separate the CNT and CSSC
tests.
---
gcc/config/aarch64/aarch64.md | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index e4d7587aac7..0b326d497b4 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4457,8 +4457,9 @@ (define_expand "umax3"
{
if (aarch64_sve_cnt_immediate (operands[1], mode))
std::swap (operands[1], operands[2]);
-else if (!aarch64_sve_cnt_immediate (operands[2], mode)
-&& TARGET_CSSC)
+else if (aarch64_sve_cnt_immediate (operands[2], mode))
+ ;
+else if (TARGET_CSSC)
{
if (aarch64_uminmax_immediate (operands[1], mode))
std::swap (operands[1], operands[2]);
--
2.25.1
[pushed] aarch64: Update sizeless tests
The sizeless-*.c tests contained (deliberately) invalid constructors
that had two errors. The first error now suppresses the second error,
but the second error was the main focus of the test. This patch
therefore rewrites it into a different form.
Tested on aarch64-linux-gnu & pushed.
Richard
gcc/testsuite/
* gcc.target/aarch64/sve/acle/general-c/sizeless-1.c: Avoid
"initializer element is not constant" error.
* gcc.target/aarch64/sve/acle/general-c/sizeless-2.c: Likewise.
---
.../gcc.target/aarch64/sve/acle/general-c/sizeless-1.c | 3 +--
.../gcc.target/aarch64/sve/acle/general-c/sizeless-2.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
index 4b34a71c1fe..01cfd14f873 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
@@ -30,8 +30,7 @@ union union1 {
/* Pointers to sizeless types. */
svint8_t *global_sve_sc_ptr;
-svint8_t *invalid_sve_sc_ptr = &(svint8_t) { *global_sve_sc_ptr }; /* {
dg-error {initializer element is not constant} } */
- /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "2nd line" {
target *-*-* } .-1 } */
+svint8_t *invalid_sve_sc_ptr = &(svint8_t) {}; /* { dg-error {SVE type
'svint8_t' does not have a fixed size} } */
/* Sizeless arguments and return values. */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
index 34dfd598e32..613b9c47878 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
@@ -30,8 +30,7 @@ union union1 {
/* Pointers to sizeless types. */
svint8_t *global_sve_sc_ptr;
-svint8_t *invalid_sve_sc_ptr = &(svint8_t) { *global_sve_sc_ptr }; /* {
dg-error {initializer element is not constant} } */
- /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "2nd line" {
target *-*-* } .-1 } */
+svint8_t *invalid_sve_sc_ptr = &(svint8_t) {}; /* { dg-error {SVE type
'svint8_t' does not have a fixed size} } */
/* Sizeless arguments and return values. */
--
2.25.1
Re: [PATCH] tree-optimization/108522 Use COMPONENT_REF offset when available
On 2023-01-25 02:44, Richard Biener wrote: t = TREE_OPERAND (expr, 1); - off = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (t), + off = size_binop (PLUS_EXPR, + (TREE_OPERAND (expr, 2) ? TREE_OPERAND (expr, 2) +: DECL_FIELD_OFFSET (t)), That isn't correct - operand 2 is the field offset in units of DECL_OFFSET_ALIGN (t) / BITS_PER_UNIT. See component_ref_filed_offset (), maybe you should be using that function instead? Ahh, and it passed my testing only because I was testing a char. Thanks, I'll test and send an update with additional tests. Thanks, Sid
Re: [PATCH 2/2] Add VREL_OTHER for FP unsupported relations.
On Tue, Jan 24, 2023 at 10:57:12AM -0500, Andrew MacLeod wrote:
> That is the way VREL_OTHER is implemented in the table.
>
> So the problem is not on the true side of the IF condition as in your
> example, its on the false side. We see this commonly in code like this
>
>
> if (x <= y) // FALSE side registers x > y
> blah()
> else if (x >= y) // FALSE side registers x < y
> blah()
> else
> // Here we get VREL_UNDEFINED.
In that case, I think the problem isn't in the intersection, which works
correctly, but in wherever we (again, for HONOR_NANS (type) only) make the
assumptions about the else branches.
In the above case, the first blah has VREL_LE, that is correct,
but the else of it (when the condition isn't true but is false)
isn't VREL_GT, but VREL_UNGT (aka VREL_OTHER).
So, for the HONOR_NANS (type) cases we need to adjust relation_negate
callers.
On trees, you can e.g. look at fold-const.cc (invert_tree_comparison),
though that one for HONOR_NANS (type) && flag_trapping_math punts in most
cases to preserve exceptions. But you can see there the !honor_nans
cases where it acts like relation_negate, and then the honor_nans cases,
where VREL_*:
VARYING UNDEFINED LT LE GT GE EQ NE LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE
UNEQ
negate to
UNDEFINED VARYING UNGE UNGT UNLE UNLT NE EQ UNEQ UNORDERED ORDERED GE GT LE LT
LTGT
Or, in the world where VREL_OTHER is a wildcard for
LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE UNEQ
the less useful
VARYING UNDEFINED LT LE GT GE EQ NE OTHER
negates to
UNDEFINED VARYING OTHER OTHER OTHER OTHER NE EQ OTHER
But I'm afraid the above has VREL_OTHER for too many important cases,
unlike intersect where it is for none unless VREL_OTHER is involved, or just
a few ones for union.
So, I wonder if we just shouldn't do it properly immediately and instead of
VREL_OTHER introduce those
VREL_LTGT, /* Less than or greater than. */
VREL_ORDERED, /* Operands not NAN. */
VREL_UNORDERED, /* One or both operands NAN. */
VREL_UNLT, /* Unordered or less than. */
VREL_UNLE, /* Unordered or less than or equal. */
VREL_UNGT, /* Unordered or greater than. */
VREL_UNGE, /* Unordered or greater than or equal. */
VREL_UNEQ /* Unordered or equal. */
In that case, rather than using relation_{negate,union,intersect} etc.
either those functions should take an argument whether it is a HONOR_NANS
(type) floating point or not and use two separate tables under the hood,
or have two sets of routines and choose which one to use in the callers.
I think one routine with an extra argument would be cleaner though.
The above would grow the tables quite a bit (though, we could for the
non-FP cases limit ourselves to a subset), but because all the VREL_*
constants are enumerals with small integers values and the arrays
should be (but aren't for some reason?) static just make the arrays
contain unsigned char and do the casts to the enum in the
relation_{negate,intersect,union} etc. wrappers.
Also, I think the rr_*_table arrays should be const, we don't want to change
them, right? And a few other arrays too, like relation_to_code.
BTW, with the above extra 8 codes we could use the ORDERED_EXPR etc. tree
codes there.
Jakub
[pushed] wwwdocs: gcc-6: Switch www.open-std.org links to https
Pushed. Gerald --- htdocs/gcc-6/changes.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/gcc-6/changes.html b/htdocs/gcc-6/changes.html index 612ea48d..b400dd9c 100644 --- a/htdocs/gcc-6/changes.html +++ b/htdocs/gcc-6/changes.html @@ -300,7 +300,7 @@ within strings: C++ The default mode has been changed to -std=gnu++14. -http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4377.pdf;>C++ +https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4377.pdf;>C++ Concepts are now supported when compiling with -fconcepts. -flifetime-dse is more -- 2.39.0
[pushed] doc/contrib.texi: Add Jose E. Marchesi
This fixes an obvious submission (of which I believe there are more - please feel free to help address those). Gerald gcc/ChangeLog: * doc/contrib.texi: Add Jose E. Marchesi. --- gcc/doc/contrib.texi | 4 1 file changed, 4 insertions(+) diff --git a/gcc/doc/contrib.texi b/gcc/doc/contrib.texi index 7e9c16f1f56..758805dc5db 100644 --- a/gcc/doc/contrib.texi +++ b/gcc/doc/contrib.texi @@ -634,6 +634,10 @@ and unit testing. @item Bob Manson for his behind the scenes work on dejagnu. +@item +Jose E. Marchesi for contributing the eBPF backend and his ongoing +work maintaining it. + @item John Marino for contributing the DragonFly BSD port. -- 2.39.0
Re: [PATCH] store-merging: Disable string_concatenate mode if start or end aren't byte aligned [PR108498]
On Wed, 25 Jan 2023, Jakub Jelinek wrote:
> Hi!
>
> The first of the following testcases is miscompiled on powerpc64-linux -O2
> -m64 at least, the latter at least on x86_64-linux -m32/-m64.
> Since GCC 11 store-merging has a separate string_concatenation mode which
> turns stores into setting a MEM_REF from a STRING_CST.
> This mode is triggered if at least one of the to be merged stores
> is a STRING_CST store and either the first store (to earliest address)
> is that STRING_CST store or the first store is 8-bit INTEGER_CST store
> and then there are some rules when to turn that mode off or not merge
> further stores into it.
>
> The problem with these 2 testcases is that the actual implementation
> relies on start/width of the store to be at byte boundaries, as it
> simply creates a char array, MEM_REF can be only on byte boundaries
> and the char array too, plus obviously STRING_CST as well.
> But as can be easily seen in the second testcase, nothing verifies this,
> while the first store has to be a STRING_CST (which will be aligned)
> or 8-bit INTEGER_CST, that 8-bit INTEGER_CST store could be a bitfield
> store, nothing verifies any stores in between whether they actually are
> 8-bit and aligned, the only major requirement is that all the stores
> are consecutive.
>
> For GCC 14 I think we should reconsider this, simply treat STRING_CST
> stores during the merging like INTEGER_CST stores and deal with it only
> during split_group where we can create multiple parts, this part
> would be a normal store, this part would be STRING_CST store, this part
> another normal store etc. But that is quite a lot of work, the following
> patch just disables the string_concatenate mode if boundaries aren't byte
> aligned in the spot where we disable it if it is too short too.
> If that happens, we'll just try to do the merging using normal 1/2/4/8 etc.
> byte stores as usually with RMW masking for any bits that shouldn't be
> touched or punt if we end up with too many stores compared to the original.
>
> Note, an original STRING_CST store will count as one store in that case,
> something we might want to reconsider later too (but, after all, CONSTRUCTOR
> stores (aka zeroing) already have the same problem, they can be large and
> expensive and we still count them as one store).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux plus tested on the
> first testcase with cross to powerpc64-linux, ok for trunk?
LGTM.
> 2023-01-25 Jakub Jelinek
>
> PR tree-optimization/108498
> * gimple-ssa-store-merging.cc (class store_operand_info):
> End coment with full stop rather than comma.
> (split_group): Likewise.
> (merged_store_group::apply_stores): Clear string_concatenation if
> start or end aren't on a byte boundary.
>
> * gcc.c-torture/execute/pr108498-1.c: New test.
> * gcc.c-torture/execute/pr108498-2.c: New test.
>
> --- gcc/gimple-ssa-store-merging.cc.jj2023-01-02 09:32:33.811120195
> +0100
> +++ gcc/gimple-ssa-store-merging.cc 2023-01-24 16:43:59.700075344 +0100
> @@ -1614,7 +1614,7 @@ namespace {
> then VAL represents the constant and all the other fields are zero, or
> a memory load, then VAL represents the reference, BASE_ADDR is non-NULL
> and the other fields also reflect the memory load, or an SSA name, then
> - VAL represents the SSA name and all the other fields are zero, */
> + VAL represents the SSA name and all the other fields are zero. */
>
> class store_operand_info
> {
> @@ -2309,6 +2309,10 @@ merged_store_group::apply_stores ()
>if (buf_size <= MOVE_MAX)
> string_concatenation = false;
>
> + /* String concatenation only works for byte aligned start and end. */
> + if (start % BITS_PER_UNIT != 0 || width % BITS_PER_UNIT != 0)
> +string_concatenation = false;
> +
>/* Create a power-of-2-sized buffer for native_encode_expr. */
>if (!string_concatenation)
> buf_size = 1 << ceil_log2 (buf_size);
> @@ -3631,7 +3635,7 @@ split_group (merged_store_group *group,
>
>/* For bswap framework using sets of stores, all the checking has been done
> earlier in try_coalesce_bswap and the result always needs to be emitted
> - as a single store. Likewise for string concatenation, */
> + as a single store. Likewise for string concatenation. */
>if (group->stores[0]->rhs_code == LROTATE_EXPR
>|| group->stores[0]->rhs_code == NOP_EXPR
>|| group->string_concatenation)
> --- gcc/testsuite/gcc.c-torture/execute/pr108498-1.c.jj 2023-01-24
> 16:53:23.920800393 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr108498-1.c 2023-01-24
> 16:52:37.610479583 +0100
> @@ -0,0 +1,82 @@
> +/* PR tree-optimization/108498 */
> +
> +struct A
> +{
> + signed char a1;
> + int a2;
> +};
> +
> +struct B
> +{
> + struct A b1;
> + unsigned char b2:1, b3:1, b4:2, b5:1, b6:1, b7[4];
> +};
> +
> +struct C
> +{
> + unsigned char c1;
> + char c2;
> + signed
Re: [committed] C-SKY: Define SYSROOT_SUFFIX_SPEC.
On Fri, Jan 20, 2023 at 05:27:26PM +, Joseph Myers wrote:
> I think this caused the build failures with build-many-glibcs.py shown by
> my bot. SYSROOT_SUFFIX_SPEC should not be defined for a
> --disable-multilib build; in such a build you can expect a single sysroot
> without a suffix involved. Thus, you should put the SYSROOT_SUFFIX_SPEC
> definition in a separate header only used if test "x${enable_multilib}" =
> xyes (as in the config.gcc code removed in the older patch you refer to
> above), or something similar.
>
Hi Joseph,
Sorry, this is indeed caused by this patch. I tested the multilib scenario, but
I didn't test the non-multilib, so I didn't find this problem in time.
Now I have fixed this problem, the patch can be seen in
https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610534.html
Hope it also works for you, If you still have problems, please contact me.
Thanks,
Cooper
[PATCH] store-merging: Disable string_concatenate mode if start or end aren't byte aligned [PR108498]
Hi!
The first of the following testcases is miscompiled on powerpc64-linux -O2
-m64 at least, the latter at least on x86_64-linux -m32/-m64.
Since GCC 11 store-merging has a separate string_concatenation mode which
turns stores into setting a MEM_REF from a STRING_CST.
This mode is triggered if at least one of the to be merged stores
is a STRING_CST store and either the first store (to earliest address)
is that STRING_CST store or the first store is 8-bit INTEGER_CST store
and then there are some rules when to turn that mode off or not merge
further stores into it.
The problem with these 2 testcases is that the actual implementation
relies on start/width of the store to be at byte boundaries, as it
simply creates a char array, MEM_REF can be only on byte boundaries
and the char array too, plus obviously STRING_CST as well.
But as can be easily seen in the second testcase, nothing verifies this,
while the first store has to be a STRING_CST (which will be aligned)
or 8-bit INTEGER_CST, that 8-bit INTEGER_CST store could be a bitfield
store, nothing verifies any stores in between whether they actually are
8-bit and aligned, the only major requirement is that all the stores
are consecutive.
For GCC 14 I think we should reconsider this, simply treat STRING_CST
stores during the merging like INTEGER_CST stores and deal with it only
during split_group where we can create multiple parts, this part
would be a normal store, this part would be STRING_CST store, this part
another normal store etc. But that is quite a lot of work, the following
patch just disables the string_concatenate mode if boundaries aren't byte
aligned in the spot where we disable it if it is too short too.
If that happens, we'll just try to do the merging using normal 1/2/4/8 etc.
byte stores as usually with RMW masking for any bits that shouldn't be
touched or punt if we end up with too many stores compared to the original.
Note, an original STRING_CST store will count as one store in that case,
something we might want to reconsider later too (but, after all, CONSTRUCTOR
stores (aka zeroing) already have the same problem, they can be large and
expensive and we still count them as one store).
Bootstrapped/regtested on x86_64-linux and i686-linux plus tested on the
first testcase with cross to powerpc64-linux, ok for trunk?
2023-01-25 Jakub Jelinek
PR tree-optimization/108498
* gimple-ssa-store-merging.cc (class store_operand_info):
End coment with full stop rather than comma.
(split_group): Likewise.
(merged_store_group::apply_stores): Clear string_concatenation if
start or end aren't on a byte boundary.
* gcc.c-torture/execute/pr108498-1.c: New test.
* gcc.c-torture/execute/pr108498-2.c: New test.
--- gcc/gimple-ssa-store-merging.cc.jj 2023-01-02 09:32:33.811120195 +0100
+++ gcc/gimple-ssa-store-merging.cc 2023-01-24 16:43:59.700075344 +0100
@@ -1614,7 +1614,7 @@ namespace {
then VAL represents the constant and all the other fields are zero, or
a memory load, then VAL represents the reference, BASE_ADDR is non-NULL
and the other fields also reflect the memory load, or an SSA name, then
- VAL represents the SSA name and all the other fields are zero, */
+ VAL represents the SSA name and all the other fields are zero. */
class store_operand_info
{
@@ -2309,6 +2309,10 @@ merged_store_group::apply_stores ()
if (buf_size <= MOVE_MAX)
string_concatenation = false;
+ /* String concatenation only works for byte aligned start and end. */
+ if (start % BITS_PER_UNIT != 0 || width % BITS_PER_UNIT != 0)
+string_concatenation = false;
+
/* Create a power-of-2-sized buffer for native_encode_expr. */
if (!string_concatenation)
buf_size = 1 << ceil_log2 (buf_size);
@@ -3631,7 +3635,7 @@ split_group (merged_store_group *group,
/* For bswap framework using sets of stores, all the checking has been done
earlier in try_coalesce_bswap and the result always needs to be emitted
- as a single store. Likewise for string concatenation, */
+ as a single store. Likewise for string concatenation. */
if (group->stores[0]->rhs_code == LROTATE_EXPR
|| group->stores[0]->rhs_code == NOP_EXPR
|| group->string_concatenation)
--- gcc/testsuite/gcc.c-torture/execute/pr108498-1.c.jj 2023-01-24
16:53:23.920800393 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr108498-1.c2023-01-24
16:52:37.610479583 +0100
@@ -0,0 +1,82 @@
+/* PR tree-optimization/108498 */
+
+struct A
+{
+ signed char a1;
+ int a2;
+};
+
+struct B
+{
+ struct A b1;
+ unsigned char b2:1, b3:1, b4:2, b5:1, b6:1, b7[4];
+};
+
+struct C
+{
+ unsigned char c1;
+ char c2;
+ signed char c3;
+ unsigned char c4, c5[4], c6:1, c7:1, c8:1, c9:3, c10:1;
+ struct A c11;
+ struct B c12[3];
+};
+
+static inline struct C
+foo (unsigned char a, unsigned b, int c, struct A d,
+ unsigned e, struct B f, struct B g, struct B h)
+{
+ struct C x
+
[PATCH] modula-2: Fixes for preprocessing [PR102343, PR108182]
Tested on x86_64, powerpc64 - linux-gnu (with 32b multilibs), i686, powerpc darwin (with 64b multilibs) x86_64 darwin (with and without 32b multilib). OK for trunk? thanks Iain --- 8< --- Modula-2 uses the C preprocessor to implement handling for conditional code and macros. However, this is not done directly, because the process is applied recursively to imported definitions and modules. The cc1gm2 executable records the parameters as a template command line needed to create a composite 'cc1 -E' for each file to be preprocessed starting with the main file from the original command line. This patch fixes the capture of the C preprocessor template to include the target information needed for correct multilib operation. In order to match the existing semantics of '-E, -M and -MM' these have to be handled as a 'pre-processor only' job (i.e. the recursion is omitted and only the main file is processed). Whereas C-family front ends always pre-process, Modula-2 only does so when specifically requested (via the -fcpp option). '-MD, -MMD and -MQ' also require special handling, since (in principle) these options can be applied to any command line (with -fcpp) providing dependency information as a by-product. TODO: the preprocessor is not able to determine def and mod dependencies for Modula-2 and so the output of this only shows the object to module dep. We should be able to append the .def and .mod dependencies. The patch amends save-temps handling to cater for the preprocessor recursion and to avoid writing saved files into the source directories. The patch changes the extension for Modula-2 preprocessed source to .m2i to avoid clashes with .i. The main driver code is amended to add default handlers for .mod and .m2i so that a useful error message will be emitted if the Modula-2 compiler is not built-in. The compiler will now also handle code generation from a .m2i preprocessed source. TODO: We should not need to pass the '-c' option to the compiler to alter the processing of init code. Signed-off-by: Iain Sandoe PR modula2/102343 PR modula2/108182 gcc/ChangeLog: * gcc.cc: Provide default specs for Modula-2 so that when the language is not built-in better diagnostics are emitted for attempts to use .mod or .m2i file extensions. gcc/m2/ChangeLog: * gm2-compiler/M2Comp.mod: Early exit for pre-processor-only jobs. * gm2-compiler/M2Options.def (SetPPOnly, GetPPOnly, SetMD, GetMD, SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir, GetDumpDir):New. * gm2-compiler/M2Options.mod:(SetPPOnly, GetPPOnly, SetMD, GetMD, SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir, GetDumpDir):New. * gm2-compiler/M2Preprocess.def (PreprocessModule): Add flag to indicate the main file. * gm2-compiler/M2Preprocess.mod: Handle Preprocess-only jobs, handle MD, MMD and MQ options. * gm2-gcc/m2options.h (M2Options_SetPPOnly, M2Options_GetPPOnly, M2Options_SetDumpDir, M2Options_SetMD, M2Options_GetMD, M2Options_SetMMD, M2Options_GetMMD, M2Options_SetMQ, M2Options_GetMQ, M2Options_SetObj, M2Options_GetObj): New. * gm2-gcc/m2type.cc (m2type_InitBaseTypes): Early exit for pre- processor-only jobs. * gm2-lang.cc (gm2_langhook_init): Handle preprocess-only commands. (gm2_langhook_option_lang_mask): Claim C and Driver options so that we can intercept them for building pre-processor commands. (gm2_langhook_init_options): Collect the preprocessor line here. Save options that have different actions for preprocessor and compile commands. (gm2_langhook_handle_option): Only handle the modula-2 options here. (gm2_langhook_post_options): Do not create a back-end for pre- processor-only jobs. * gm2spec.cc (lang_specific_driver): Ignore PCH options, append a scaffold-main for cases where we are building a main module with -c. * lang-specs.h: Revise to handle preprocessor-only jobs and to consume pre-processed files. * lang.opt: Remove Driver and C options copies (we claim these separately). --- gcc/gcc.cc | 1 + gcc/m2/gm2-compiler/M2Comp.mod | 38 +++-- gcc/m2/gm2-compiler/M2Options.def| 90 +- gcc/m2/gm2-compiler/M2Options.mod| 139 +++- gcc/m2/gm2-compiler/M2Preprocess.def | 2 +- gcc/m2/gm2-compiler/M2Preprocess.mod | 130 +-- gcc/m2/gm2-gcc/m2options.h | 11 ++ gcc/m2/gm2-gcc/m2type.cc | 4 + gcc/m2/gm2-lang.cc | 237 --- gcc/m2/gm2spec.cc| 33 +++- gcc/m2/lang-specs.h | 40 +++-- gcc/m2/lang.opt | 119 +- 12 files changed, 619 insertions(+), 225 deletions(-) diff --git a/gcc/gcc.cc b/gcc/gcc.cc index
Re: [PATCH] tree-optimization/108522 Use COMPONENT_REF offset when available
On Wed, Jan 25, 2023 at 08:44:31AM +0100, Richard Biener wrote: > > --- a/gcc/tree-object-size.cc > > +++ b/gcc/tree-object-size.cc > > @@ -412,7 +412,9 @@ compute_object_offset (const_tree expr, const_tree var) > > return base; > > > >t = TREE_OPERAND (expr, 1); > > - off = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (t), > > + off = size_binop (PLUS_EXPR, > > + (TREE_OPERAND (expr, 2) ? TREE_OPERAND (expr, 2) > > +: DECL_FIELD_OFFSET (t)), > > That isn't correct - operand 2 is the field offset in units of > DECL_OFFSET_ALIGN (t) / BITS_PER_UNIT. > See component_ref_filed_offset (), maybe you should be using that > function instead? Oops. s/filed/field/ I was looking for such a function, but didn't find it last night :(. Jakub
