Re: [PATCH v3 3/3] PR80791 Consider doloop cmp use in ivopts
On Mon, 22 Jul 2019, Segher Boessenkool wrote: > Hi! > > (Maybe I am missing half of the discussion -- sorry if so). > > I think we should have a new iv for just the doloop (which can have the > same starting value and step and type as another iv). > > Has this been considered? I was also suggesting this (maybe with too many words ;)). If it's a doloop target add such IV (candidate!) which has zero use-cost for the increment and compare but a (target configurable) penalty for other uses. Invasiveness of this approach is probably that you need to distinguish this candidate by making it a new kind (or maybe we can just have a specia candidate number...). Richard.
Re: Handle strncpy in tree-ssa-dse.c
On Fri, Jul 19, 2019 at 7:04 PM Jeff Law wrote:
>
>
> While looking at BZ 80576 I realized a few things.
>
> First for STRNCPY we know the exact count of bytes written and we can
> treat it just like MEMCPY and others, both in terms of removing/trimming
> them and in terms of using them to allow removal of other stores.
>
> This patch adds support for those routines in DSE. We test that
> subsequent statements can make those calls dead and vice versa and that
> we can trim from the head or tail appropriately.
>
> While writing that code I also stumbled over a blob of code that I think
> I copied from tree-ssa-alias.c that isn't necessary. In the relevant
> code the byte count is always found in the same place. There's no need
> to check the number of operands to the call to figure out where the
> count would be. So that little blob of code is simplified ever so slightly.
>
> Finally, while writing the tests for strncpy I stumbled over a case that
> we're still not handling well.
>
> In particular something like this:
>
>
>
> void h (char *s)
> {
> extern char a[8];
> __builtin_memset (a, 0, sizeof a);
> __builtin_strncpy (a, s, sizeof a);
> frob (a);
> }
>
> In this case ref_maybe_used_by_stmt_p returns true for the "a" array at
> the strncpy call. AFAICT that appears to happen because "a" and "s"
> could alias each other.
>
> strncpy is documented as not allowing overlap between the source and
> destination objects. So in theory we could consider them not aliasing
> for this call. I haven't implemented this, but I've got some ideas
> here.
But it does allow things like strncpy (&a[0], &a[n+1], n) for example?
I think this kind of specialities should be handled in
ref_maybe_used_by_call_p_1
directly, but I'm not exactly sure how - it's probably another case of
a missing must-alias query, with that we could do
/* If REF overlaps with the strncpy destination then the source argument
may not alias it. */
if (refs_must_alias_p (ref_for_strncpy_dest, ref))
return false;
hmm, OTOH for REF covering &a[n/2] to &a[3*n/2] (half overlapping
source and destination) and the above strncpy we have a must-alias
(not kill!) of REF but also are using it.
So it's not so easy and would cover only very specific cases.
> Anyway, I've included an xfailed test for this case in this patch.
>
> Bootstrapped and regression tested on x86_64, ppc64, ppc64le, aarch64 &
> sparc64. Installing on the trunk momentarily.
>
> We could in theory handle stpncpy too, we just have to be more careful
> with its return value.
>
> Jeff
Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git
> On Jul 16, 2019, at 5:14 PM, Maxim Kuvyrkov wrote: > >> On Jul 16, 2019, at 3:34 PM, Jason Merrill wrote: >> >> On Tue, Jul 16, 2019 at 12:18 PM Maxim Kuvyrkov >> wrote: >>> >>> Hi Everyone, >>> >>> I've been swamped with other projects for most of June, which gave me time >>> to digest all the feedback I've got on GCC's conversion from SVN to Git. >>> >>> The scripts have heavily evolved from the initial version posted here. >>> They have become fairly generic in that they have no implied knowledge >>> about GCC's repo structure. Due to this I no longer plan to merge them >>> into GCC tree, but rather publish as a separate project on github. For >>> now, you can track the current [hairy] version at >>> https://review.linaro.org/c/toolchain/gcc/+/31416 . >>> >>> The initial version of scripts used heuristics to construct branch tree, >>> which turned out to be error-prone. The current version parse entire >>> history of SVN repo to detect all trees that start at /trunk@1. Therefore >>> all branches in the converted repo converge to the same parent at the >>> beginning of their histories. >>> >>> As far as GCC conversion goes, below is what I plan to do and what not to >>> do. This is based on comments from everyone in this thread: >>> >>> 1. Construct GCC's git repo from SVN using same settings as current git >>> mirror. >>> 2. Compare the resulting git repo with current GCC mirror -- they should >>> match on the commit hash level for trunk, branches/gcc-*-branch, and other >>> "normal" branches. >>> 3. Investigate any differences between converted GCC repo and current GCC >>> mirror. These can be due to bugs in git-svn or other misconfigurations. >>> 4. Import git-only branches from current GCC mirror. >>> 5. Publish this "raw" repo for community to sanity-check its contents. >> >> Why not start from the current mirror? Perhaps a mirror of the mirror? > > To check that git-svn is self-consistent and generates same commits now as it > was several years ago when you setup the current mirror. Unfortunately, current mirror does not and could not account for rewrites of SVN commit log messages. For trunk the histories of diverge in 2008 due to commit message change of r138154. This is not a single occurrence; I've compared histories only of trunk and gcc-6-branch, and both had commit message change (for gcc-6-branch see r259978). It's up to the community is to weigh pros and cons of re-using existing GCC mirror as conversion base vs regenerating history from scratch: Pros of using GCC mirror: + No need to rebase public git-only branches + No need to rebase private branches + No need to rebase current clones, checkouts, work-in-progress trees Cons of using GCC mirror: - Poor author / committer IDs (this breaks patch statistics software) - Several commit messages will not be the current "fixed" version Thoughts? -- Maxim Kuvyrkov www.linaro.org
Re: GCC 9 backports
Hi.
One more patch I've just tested.
Martin
>From eb62ef9ec1edc109aa69137ed077620cafad5253 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Mon, 22 Jul 2019 10:00:07 +0200
Subject: [PATCH] Backport r273660
gcc/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* opts-common.c (decode_cmdline_option): Decode
argument of -Werror and check it for a wrong language.
* opts-global.c (complain_wrong_lang): Remove such case.
gcc/testsuite/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* gcc.dg/pr91172.c: New test.
---
gcc/opts-common.c | 20 +++-
gcc/opts-global.c | 6 +-
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 3c2553368ac..9b0d76d1148 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -438,7 +438,8 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
extra_args = 0;
- opt_index = find_opt (argv[0] + 1, lang_mask);
+ const char *opt_value = argv[0] + 1;
+ opt_index = find_opt (opt_value, lang_mask);
i = 0;
while (opt_index == OPT_SPECIAL_unknown
&& i < ARRAY_SIZE (option_map))
@@ -641,6 +642,23 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
/* Check if this is a switch for a different front end. */
if (!option_ok_for_language (option, lang_mask))
errors |= CL_ERR_WRONG_LANG;
+ else if (strcmp (option->opt_text, "-Werror=") == 0
+ && strchr (opt_value, ',') == NULL)
+{
+ /* Verify that -Werror argument is a valid warning
+ for a language. */
+ char *werror_arg = xstrdup (opt_value + 6);
+ werror_arg[0] = 'W';
+
+ size_t warning_index = find_opt (werror_arg, lang_mask);
+ if (warning_index != OPT_SPECIAL_unknown)
+ {
+ const struct cl_option *warning_option
+ = &cl_options[warning_index];
+ if (!option_ok_for_language (warning_option, lang_mask))
+ errors |= CL_ERR_WRONG_LANG;
+ }
+}
/* Convert the argument to lowercase if appropriate. */
if (arg && option->cl_tolower)
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index bfa2afb1987..b45d18996a3 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -100,10 +100,14 @@ complain_wrong_lang (const struct cl_decoded_option *decoded,
text, bad_lang);
else if (lang_mask == CL_DRIVER)
gcc_unreachable ();
- else
+ else if (ok_langs[0] != '\0')
/* Eventually this should become a hard error IMO. */
warning (0, "command line option %qs is valid for %s but not for %s",
text, ok_langs, bad_lang);
+ else
+/* Happens for -Werror=warning_name. */
+warning (0, "%<-Werror=%> argument %qs is not valid for %s",
+ text, bad_lang);
free (ok_langs);
free (bad_lang);
--
2.22.0
Re: GCC 8 backports
Hi.
One more patch I've just tested.
Martin
>From eb62ef9ec1edc109aa69137ed077620cafad5253 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Mon, 22 Jul 2019 10:00:07 +0200
Subject: [PATCH] Backport r273660
gcc/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* opts-common.c (decode_cmdline_option): Decode
argument of -Werror and check it for a wrong language.
* opts-global.c (complain_wrong_lang): Remove such case.
gcc/testsuite/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* gcc.dg/pr91172.c: New test.
---
gcc/opts-common.c | 20 +++-
gcc/opts-global.c | 6 +-
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 3c2553368ac..9b0d76d1148 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -438,7 +438,8 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
extra_args = 0;
- opt_index = find_opt (argv[0] + 1, lang_mask);
+ const char *opt_value = argv[0] + 1;
+ opt_index = find_opt (opt_value, lang_mask);
i = 0;
while (opt_index == OPT_SPECIAL_unknown
&& i < ARRAY_SIZE (option_map))
@@ -641,6 +642,23 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
/* Check if this is a switch for a different front end. */
if (!option_ok_for_language (option, lang_mask))
errors |= CL_ERR_WRONG_LANG;
+ else if (strcmp (option->opt_text, "-Werror=") == 0
+ && strchr (opt_value, ',') == NULL)
+{
+ /* Verify that -Werror argument is a valid warning
+ for a language. */
+ char *werror_arg = xstrdup (opt_value + 6);
+ werror_arg[0] = 'W';
+
+ size_t warning_index = find_opt (werror_arg, lang_mask);
+ if (warning_index != OPT_SPECIAL_unknown)
+ {
+ const struct cl_option *warning_option
+ = &cl_options[warning_index];
+ if (!option_ok_for_language (warning_option, lang_mask))
+ errors |= CL_ERR_WRONG_LANG;
+ }
+}
/* Convert the argument to lowercase if appropriate. */
if (arg && option->cl_tolower)
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index bfa2afb1987..b45d18996a3 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -100,10 +100,14 @@ complain_wrong_lang (const struct cl_decoded_option *decoded,
text, bad_lang);
else if (lang_mask == CL_DRIVER)
gcc_unreachable ();
- else
+ else if (ok_langs[0] != '\0')
/* Eventually this should become a hard error IMO. */
warning (0, "command line option %qs is valid for %s but not for %s",
text, ok_langs, bad_lang);
+ else
+/* Happens for -Werror=warning_name. */
+warning (0, "%<-Werror=%> argument %qs is not valid for %s",
+ text, bad_lang);
free (ok_langs);
free (bad_lang);
--
2.22.0
Re: GCC 7 backport
Hi.
One more patch I've just tested.
Martin
>From eb62ef9ec1edc109aa69137ed077620cafad5253 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Mon, 22 Jul 2019 10:00:07 +0200
Subject: [PATCH] Backport r273660
gcc/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* opts-common.c (decode_cmdline_option): Decode
argument of -Werror and check it for a wrong language.
* opts-global.c (complain_wrong_lang): Remove such case.
gcc/testsuite/ChangeLog:
2019-07-22 Martin Liska
PR driver/91172
* gcc.dg/pr91172.c: New test.
---
gcc/opts-common.c | 20 +++-
gcc/opts-global.c | 6 +-
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 3c2553368ac..9b0d76d1148 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -438,7 +438,8 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
extra_args = 0;
- opt_index = find_opt (argv[0] + 1, lang_mask);
+ const char *opt_value = argv[0] + 1;
+ opt_index = find_opt (opt_value, lang_mask);
i = 0;
while (opt_index == OPT_SPECIAL_unknown
&& i < ARRAY_SIZE (option_map))
@@ -641,6 +642,23 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
/* Check if this is a switch for a different front end. */
if (!option_ok_for_language (option, lang_mask))
errors |= CL_ERR_WRONG_LANG;
+ else if (strcmp (option->opt_text, "-Werror=") == 0
+ && strchr (opt_value, ',') == NULL)
+{
+ /* Verify that -Werror argument is a valid warning
+ for a language. */
+ char *werror_arg = xstrdup (opt_value + 6);
+ werror_arg[0] = 'W';
+
+ size_t warning_index = find_opt (werror_arg, lang_mask);
+ if (warning_index != OPT_SPECIAL_unknown)
+ {
+ const struct cl_option *warning_option
+ = &cl_options[warning_index];
+ if (!option_ok_for_language (warning_option, lang_mask))
+ errors |= CL_ERR_WRONG_LANG;
+ }
+}
/* Convert the argument to lowercase if appropriate. */
if (arg && option->cl_tolower)
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index bfa2afb1987..b45d18996a3 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -100,10 +100,14 @@ complain_wrong_lang (const struct cl_decoded_option *decoded,
text, bad_lang);
else if (lang_mask == CL_DRIVER)
gcc_unreachable ();
- else
+ else if (ok_langs[0] != '\0')
/* Eventually this should become a hard error IMO. */
warning (0, "command line option %qs is valid for %s but not for %s",
text, ok_langs, bad_lang);
+ else
+/* Happens for -Werror=warning_name. */
+warning (0, "%<-Werror=%> argument %qs is not valid for %s",
+ text, bad_lang);
free (ok_langs);
free (bad_lang);
--
2.22.0
[PATCH] Remove a pointless global var
This removes label_for_bb, it's allocated and deallocated in a single
function so we can just pass it down as needed.
Bootstrap / regtest running on x86_64-unknown-linux-gnu.
I've noticed that we have quite some instances using qsort
and passing down state to the comparator via a global var.
Now that we have our own qsort implementation I wonder what
the best course of action is to add a variant passing down
such state to the comparator? Copying nearly the whole
implementation would be possible but also some marshalling
of three-argument comparator calls to two-argument functions
(and some ABIs can do without actual marshalling). The
other option is templating everything (ugh).
Richard.
2019-07-22 Richard Biener
* tree-cfg.c (label_for_bb): Remove global var.
(main_block_label): Take label_for_bb as argument.
(cleanup_dead_labels_eh): Likewise, adjust.
(cleanup_dead_labels): Adjust.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index b386d60c1f2..3d18457e3ae 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -1438,19 +1438,19 @@ make_gimple_asm_edges (basic_block bb)
(almost) no new labels should be created. */
/* A map from basic block index to the leading label of that block. */
-static struct label_record
+struct label_record
{
/* The label. */
tree label;
/* True if the label is referenced from somewhere. */
bool used;
-} *label_for_bb;
+};
/* Given LABEL return the first label in the same basic block. */
static tree
-main_block_label (tree label)
+main_block_label (tree label, label_record *label_for_bb)
{
basic_block bb = label_to_block (cfun, label);
tree main_label = label_for_bb[bb->index].label;
@@ -1469,7 +1469,7 @@ main_block_label (tree label)
/* Clean up redundant labels within the exception tree. */
static void
-cleanup_dead_labels_eh (void)
+cleanup_dead_labels_eh (label_record *label_for_bb)
{
eh_landing_pad lp;
eh_region r;
@@ -1482,7 +1482,7 @@ cleanup_dead_labels_eh (void)
for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)
if (lp && lp->post_landing_pad)
{
- lab = main_block_label (lp->post_landing_pad);
+ lab = main_block_label (lp->post_landing_pad, label_for_bb);
if (lab != lp->post_landing_pad)
{
EH_LANDING_PAD_NR (lp->post_landing_pad) = 0;
@@ -1504,7 +1504,7 @@ cleanup_dead_labels_eh (void)
{
lab = c->label;
if (lab)
- c->label = main_block_label (lab);
+ c->label = main_block_label (lab, label_for_bb);
}
}
break;
@@ -1512,7 +1512,7 @@ cleanup_dead_labels_eh (void)
case ERT_ALLOWED_EXCEPTIONS:
lab = r->u.allowed.label;
if (lab)
- r->u.allowed.label = main_block_label (lab);
+ r->u.allowed.label = main_block_label (lab, label_for_bb);
break;
}
}
@@ -1527,7 +1527,8 @@ void
cleanup_dead_labels (void)
{
basic_block bb;
- label_for_bb = XCNEWVEC (struct label_record, last_basic_block_for_fn
(cfun));
+ label_record *label_for_bb = XCNEWVEC (struct label_record,
+last_basic_block_for_fn (cfun));
/* Find a suitable label for each block. We use the first user-defined
label if there is one, or otherwise just the first label we see. */
@@ -1583,7 +1584,7 @@ cleanup_dead_labels (void)
label = gimple_cond_true_label (cond_stmt);
if (label)
{
- new_label = main_block_label (label);
+ new_label = main_block_label (label, label_for_bb);
if (new_label != label)
gimple_cond_set_true_label (cond_stmt, new_label);
}
@@ -1591,7 +1592,7 @@ cleanup_dead_labels (void)
label = gimple_cond_false_label (cond_stmt);
if (label)
{
- new_label = main_block_label (label);
+ new_label = main_block_label (label, label_for_bb);
if (new_label != label)
gimple_cond_set_false_label (cond_stmt, new_label);
}
@@ -1608,7 +1609,7 @@ cleanup_dead_labels (void)
{
tree case_label = gimple_switch_label (switch_stmt, i);
label = CASE_LABEL (case_label);
- new_label = main_block_label (label);
+ new_label = main_block_label (label, label_for_bb);
if (new_label != label)
CASE_LABEL (case_label) = new_label;
}
@@ -1623,7 +1624,7 @@ cleanup_dead_labels (void)
for (i = 0; i < n; ++i)
{
tree cons = gimple_asm_label_op (asm_stmt, i);
- tree label = main_block_label (TREE_VALUE (cons));
+ tree label = main_block_label (TREE_VALUE (cons), label_for_bb);
TREE_VALUE (cons) = label;
}
break;
@@ -1636,7 +1637,7 @@
[PATCH] Fix PR91221
Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.
Richard.
2019-07-22 Richard Biener
PR tree-optimization/91221
* tree-ssa-sccvn.c (vn_reference_lookup_3): Appropriately
restrict partial-def handling of empty constructors and
memset to refs with known offset.
* g++.dg/pr91221.C: New testcase.
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273657)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -2455,7 +2507,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
(vuse, vr->set, vr->type, vr->operands, val);
}
/* For now handle clearing memory with partial defs. */
- else if (integer_zerop (gimple_call_arg (def_stmt, 1))
+ else if (known_eq (ref->size, maxsize)
+ && integer_zerop (gimple_call_arg (def_stmt, 1))
&& tree_to_poly_int64 (len).is_constant (&leni)
&& offset.is_constant (&offseti)
&& offset2.is_constant (&offset2i)
@@ -2503,7 +2556,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
return vn_reference_lookup_or_insert_for_pieces
(vuse, vr->set, vr->type, vr->operands, val);
}
- else if (maxsize.is_constant (&maxsizei)
+ else if (known_eq (ref->size, maxsize)
+ && maxsize.is_constant (&maxsizei)
&& maxsizei % BITS_PER_UNIT == 0
&& offset.is_constant (&offseti)
&& offseti % BITS_PER_UNIT == 0
Index: gcc/testsuite/g++.dg/pr91221.C
===
--- gcc/testsuite/g++.dg/pr91221.C (nonexistent)
+++ gcc/testsuite/g++.dg/pr91221.C (working copy)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-ipa-pure-const -fpack-struct
-Wno-address-of-packed-member" }
+
+void printf(...);
+struct A {
+A() : bar_(), dbar_() {
+ for (int i;; i++)
+ printf(i, bar_[i]);
+}
+int bar_[5];
+double dbar_[5];
+};
+void fn1() { A a; }
Re: [PATCH,fortran] Handle BOZ in accordance to Fortran 2018 standard (1st batch)
(A) I see the following failures FAIL: libgomp.fortran/reduction4.f90 -O0 (test for excess errors) … FAIL: libgomp.fortran/reduction4.f90 -Os (test for excess errors) FAIL: libgomp.fortran/reduction5.f90 -O0 (test for excess errors) … FAIL: libgomp.fortran/reduction5.f90 -Os (test for excess errors) I have silenced the failures with following patch: --- ../_clean/libgomp/testsuite/libgomp.fortran/reduction4.f90 2018-03-25 18:07:25.0 +0200 +++ libgomp/testsuite/libgomp.fortran/reduction4.f902019-07-19 00:04:29.0 +0200 @@ -4,12 +4,12 @@ integer (kind = 4) :: i, ia (6), j, ja (6), k, ka (6), ta (6), n, cnt, x logical :: v - i = Z'0f' - ia = Z'f0ff0f' - j = Z'0f' - ja = Z'0f5a00' - k = Z'055aa0' - ka = Z'05a5a5' + i = int(Z'0f') + ia = int(Z'f0ff0f') + j = int(Z'0f') + ja = int(Z'0f5a00') + k = int(Z'055aa0') + ka = int(Z'05a5a5') v = .false. cnt = -1 x = not(0) @@ -22,35 +22,35 @@ n = omp_get_thread_num () if (n .eq. 0) then cnt = omp_get_num_threads () -i = Z'ff7fff' -ia(3:5) = Z'f1' -j = Z'078000' +i = int(Z'ff7fff') +ia(3:5) = int(Z'f1') +j = int(Z'078000') ja(1:3) = 1 -k = Z'78' -ka(3:6) = Z'f0f' +k = int(Z'78') +ka(3:6) = int(Z'f0f') else if (n .eq. 1) then -i = Z'77' -ia(2:5) = Z'ffafff' -j = Z'007800' +i = int(Z'77') +ia(2:5) = int(Z'ffafff') +j = int(Z'007800') ja(2:5) = 8 -k = Z'57' -ka(3:4) = Z'f0108' +k = int(Z'57') +ka(3:4) = int(Z'f0108') else -i = Z'777fff' -ia(1:2) = Z'f3' -j = Z'000780' -ja(5:6) = Z'f00' -k = Z'1000' -ka(6:6) = Z'777' +i = int(Z'777fff') +ia(1:2) = int(Z'f3') +j = int(Z'000780') +ja(5:6) = int(Z'f00') +k = int(Z'1000') +ka(6:6) = int(Z'777') end if !$omp end parallel if (v) STOP 1 if (cnt .eq. 3) then -ta = (/Z'f0ff03', Z'f0af03', Z'f0af01', Z'f0af01', Z'f0af01', Z'f0ff0f'/) -if (i .ne. Z'777f07' .or. any (ia .ne. ta)) STOP 2 -ta = (/Z'f5a01', Z'f5a09', Z'f5a09', Z'f5a08', Z'f5f08', Z'f5f00'/) -if (j .ne. Z'fff80' .or. any (ja .ne. ta)) STOP 3 -ta = (/Z'5a5a5', Z'5a5a5', Z'aaba2', Z'aaba2', Z'5', Z'5addd'/) -if (k .ne. Z'54a8f' .or. any (ka .ne. ta)) STOP 4 +ta = (/int(Z'f0ff03'), int(Z'f0af03'), int(Z'f0af01'), int(Z'f0af01'), int(Z'f0af01'), int(Z'f0ff0f')/) +if (i .ne. int(Z'777f07') .or. any (ia .ne. ta)) STOP 2 +ta = (/int(Z'f5a01'), int(Z'f5a09'), int(Z'f5a09'), int(Z'f5a08'), int(Z'f5f08'), int(Z'f5f00')/) +if (j .ne. int(Z'fff80') .or. any (ja .ne. ta)) STOP 3 +ta = (/int(Z'5a5a5'), int(Z'5a5a5'), int(Z'aaba2'), int(Z'aaba2'), int(Z'5'), int(Z'5addd')/) +if (k .ne. int(Z'54a8f') .or. any (ka .ne. ta)) STOP 4 end if end --- ../_clean/libgomp/testsuite/libgomp.fortran/reduction5.f90 2018-03-25 18:07:24.0 +0200 +++ libgomp/testsuite/libgomp.fortran/reduction5.f902019-07-18 23:56:59.0 +0200 @@ -10,7 +10,7 @@ contains subroutine test1 use reduction5, bitwise_or => ior integer :: n -n = Z'f' +n = int(Z'f') !$omp parallel sections num_threads (3) reduction (bitwise_or: n) n = ior (n, Z'20') !$omp section @@ -18,7 +18,7 @@ contains !$omp section n = bitwise_or (n, Z'2000') !$omp end parallel sections -if (n .ne. Z'243f') STOP 1 +if (n .ne. int(Z'243f')) STOP 1 end subroutine subroutine test2 use reduction5, min => max, max => min (B) The points mentioned in https://gcc.gnu.org/ml/fortran/2017-10/msg00037.html have been fixed, except the points (3) and (6) which still give an ICE. I understand that the coddles are invalid, but IMO they should give an error along the line "BOZ literal at %L outside a DATA statement and outside INT/REAL/DBLE/CMPLX » I have more comments for a second batch Dominique
Re: [PATCH] Remove a pointless global var
Hi!
On Mon, 22 Jul 2019, Richard Biener wrote:
> I've noticed that we have quite some instances using qsort
> and passing down state to the comparator via a global var.
> Now that we have our own qsort implementation I wonder what
> the best course of action is to add a variant passing down
> such state to the comparator? Copying nearly the whole
> implementation would be possible but also some marshalling
> of three-argument comparator calls to two-argument functions
> (and some ABIs can do without actual marshalling). The
> other option is templating everything (ugh).
I think there are three choices.
1. Have gcc/sort.cc implement only qsort_r, use a "universal comparator
adapter"
int cmp2to3(void *a, void *b, void *ctx)
{
return (int (*)(void *, void *))ctx(a, b);
}
to adapt existing qsort uses.
2. Have gcc/sort.cc implement both qsort and qsort_r by duplicating code,
either copying manually (ugh) or by turning netsort, mergesort, qsort_chk
to templates.
3. Have gcc/sort.cc implement only qsort_r, but have existing qsort callers
pass their comparators to a fancy C++ "universal comparator adapter" so
they can be inlined into the three-argument wrapper and thus
speed/size penalties are tiny (only from always loading the context pointer
that is ignored by legacy two-argument comparators):
void qsort_r(void *, size_t, size_t, int cmp(void *, void *, void *));
template
int cmp2to3(void *a, void *b, void *c)
{
return cmp(a, b);
}
#define qsort(base, sz, n, cmp) \
qsort_r(base, sz, n, cmp2to3)
static int my_cmp(void *a, void *b)
{
return 0;
}
void test(void *base, size_t sz, size_t n)
{
qsort(base, sz, n, my_cmp);
}
Alexander
Re: [PATCH v2] [rs6000] Add _mm_blend_epi16 and _mm_blendv_epi8
On 7/22/19 12:58 AM, Segher Boessenkool wrote: > On Sun, Jul 21, 2019 at 05:22:19PM -0500, Paul Clarke wrote: >> Add compatibility implementations of _mm_blend_epi16 and _mm_blendv_epi8 >> intrinsics. >> >> Respective test cases are copied almost verbatim (minor changes to >> the dejagnu head lines) from i386. >> >> 2019-07-21 Paul A. Clarke >> >> [gcc] >> >> * config/rs6000/smmintrin.h (_mm_blend_epi16): New. >> (_mm_blendv_epi8): New. >> >> [gcc/testsuite] >> >> * gcc.target/powerpc/sse4_1-check.h: New. >> * gcc.target/powerpc/sse4_1-pblendvb.c: New. >> * gcc.target/powerpc/sse4_1-pblendw.c: New. >> * gcc.target/powerpc/sse4_1-pblendw-2.c: New. >> >> Tested on 64bit LE, 64bit and 32bit BE. >> >> v2: algorithm improvements as suggested by Segher. Note that >> _mm_blend_epi16, >> which now uses vec_gb, also requires the use of vec_unpackh to handle the >> 16 bit elements. It also requires a vec_reve on big endian, due to the >> endian >> characteristics of vec_gb. Both are still much shorter. Thanks, Segher! > Ah yes, I missed those "details". Glad to hear it still helps. > > Approved for trunk, please apply. Thanks! > > Do we need/want backports for this? IMHO, yes, it would be really nice to have this in GCC 9.2 at least. Bill > > > Segher >
Re: [PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.
PING^1 On 7/11/19 8:45 AM, Martin Liška wrote: > On 7/9/19 11:00 PM, Jason Merrill wrote: >> On 7/9/19 6:17 AM, Marc Glisse wrote: >>> On Tue, 9 Jul 2019, Martin Liška wrote: >>> On 7/9/19 9:49 AM, Marc Glisse wrote: > On Tue, 9 Jul 2019, Marc Glisse wrote: > >> On Mon, 8 Jul 2019, Martin Liška wrote: >> The patch apparently has DECL_IS_OPERATOR_DELETE only on the replaceable global deallocation functions, not all delete operators, contrary to DECL_IS_OPERATOR_NEW, so the name is misleading. On the other hand, those seem to be the ones for which the optimization is legal (well, not quite, the rules are in terms of operator new, and I am not sure how well operator delete has to match, but close enough). >>> >>> Are you talking about this location where we set OPERATOR_NEW: >>> https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/decl.c#L13643 >>> ? >>> >>> That's the only place where we set OPERATOR_NEW flag and not >>> OPERATOR_DELETE. >> >> Yes, I think that's the place. >> >> Again, not setting DECL_IS_OPERATOR_DELETE on local operator delete >> seems misleading, but setting it would let us optimize in cases where we >> are not really allowed to. Maybe just rename your macro to >> DECL_IS_GLOBAL_OPERATOR_DELETE? > > Hmm, I replied too fast. > > Global operator delete does not seem like a good terminology, the ones > marked in the patch would be the usual (=non-placement) replaceable > deallocation functions. > > I cannot find a requirement that operator new and operator delete should > match. The rules to omit allocation are stated in terms of which operator > new is called, but do not seem to care which operator delete is used. So > allocating with the global operator new and deallocating with a class > overload of operator delete can be removed, but not the reverse (not sure > how they came up with such a rule...). >> >> Correct. The standard just says that an implementation is allowed to omit a >> call to the replaceable ::operator new; it does not place any constraints on >> that, the conditions for such omission are left up to the implementation. >> >> If the user's code uses global new and class delete for the same pointer, >> that would suggest that they're doing something odd, and we might as well >> leave it alone. I would expect this to be very rare. >> > Which means we would need: Thank you Mark for digging deep in that. > > keep DECL_IS_OPERATOR_NEW for the current uses > > DECL_IS_REPLACEABLE_OPERATOR_NEW (equivalent to DECL_IS_OPERATOR_NEW && > DECL_IS_MALLOC? not exactly but close I think) for DCE > > DECL_IS_OPERATOR_DELETE (which also includes some class overloads) for DCE Note that with the current version of the patch we are out of free bits in struct GTY(()) tree_function_decl. Would it be possible to tweak the current patch to cover what you described? >>> >>> If you approximate DECL_IS_REPLACEABLE_OPERATOR_NEW with >>> DECL_IS_OPERATOR_NEW && DECL_IS_MALLOC, it shouldn't need more bits than >>> the current patch. I think the main difference is if a user adds attribute >>> malloc to his class-specific operator new, where it will enable DCE, but >>> since the attribute is non-standard, we can just document that behavior, it >>> might even be desirable. >> >> Sure, it seems desirable to me. >> >> Jason > > Ok, I hopefully addressed the suggested improvements to the patch. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin
[PATCH v2] [rs6000] Add documentation for __builtin_mtfsf
2019-07-21 Paul A. Clarke
[gcc]
* doc/extend.texi: Add documentation for __builtin_mtfsf.
v2: wordsmithing at Segher's request. I'm having a hard time not saying too
much. :-)
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 273615)
+++ gcc/doc/extend.texi (working copy)
@@ -16848,6 +16848,7 @@ unsigned long __builtin_ppc_mftb ();
double __builtin_unpack_ibm128 (__ibm128, int);
__ibm128 __builtin_pack_ibm128 (double, double);
double __builtin_mffs (void);
+double __builtin_mtfsf (const int, double);
void __builtin_mtfsb0 (const int);
void __builtin_mtfsb1 (const int);
void __builtin_set_fpscr_rn (int);
@@ -16863,7 +16864,10 @@ the most significant word on 32-bit environments.
return the value of the FPSCR register. Note, ISA 3.0 supports the
@code{__builtin_mffsl()} which permits software to read the control and
non-sticky status bits in the FSPCR without the higher latency associated with
-accessing the sticky status bits. The
+accessing the sticky status bits. The @code{__builtin_mtfsf} takes a constant
+8-bit integer field mask and a representation of the new value of the FPSCR
+and generates the @code{mtfsf} (extended mnemonic) instruction to write new
+values to selected fields of the FPSCR. The
@code{__builtin_mtfsb0} and @code{__builtin_mtfsb1} take the bit to change
as an argument. The valid bit range is between 0 and 31. The builtins map to
the @code{mtfsb0} and @code{mtfsb1} instructions which take the argument and
[Ada] Fix internal error on array slice in loop and Loop_Invariant
This fixes an internal error caused by the presence of an Itype in a
wrong scope. This Itype is created for an array slice present in the
condition of a while loop whose body also contains a pragma
Loop_Invariant, initially in the correct scope but then relocated into a
function created for the pragma.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* exp_attr.adb (Expand_Loop_Entry_Attribute): Copy the condition
of a while loop instead of simply relocating it.
gcc/testsuite/
* gnat.dg/loop_invariant1.adb, gnat.dg/loop_invariant1.ads: New
testcase.--- gcc/ada/exp_attr.adb
+++ gcc/ada/exp_attr.adb
@@ -1384,6 +1384,8 @@ package body Exp_Attr is
Stmts : List_Id;
begin
+ Func_Id := Make_Temporary (Loc, 'F');
+
-- Wrap the condition of the while loop in a Boolean function.
-- This avoids the duplication of the same code which may lead
-- to gigi issues with respect to multiple declaration of the
@@ -1403,7 +1405,9 @@ package body Exp_Attr is
Append_To (Stmts,
Make_Simple_Return_Statement (Loc,
- Expression => Relocate_Node (Condition (Scheme;
+ Expression =>
+ New_Copy_Tree (Condition (Scheme),
+ New_Scope => Func_Id)));
-- Generate:
--function Fnn return Boolean is
@@ -1411,7 +1415,6 @@ package body Exp_Attr is
--
--end Fnn;
- Func_Id := Make_Temporary (Loc, 'F');
Func_Decl :=
Make_Subprogram_Body (Loc,
Specification =>
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/loop_invariant1.adb
@@ -0,0 +1,15 @@
+-- { dg-do compile }
+-- { dg-options "-gnata" }
+
+package body Loop_Invariant1 is
+
+ procedure Proc (A : Arr; N : Integer) is
+ I : Integer := A'First;
+ begin
+ while i <= A'Last and then A(A'First .. A'Last) /= A loop
+ pragma Loop_Invariant (N = N'Loop_Entry);
+ i := i + 1;
+ end loop;
+ end;
+
+end Loop_Invariant1;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/loop_invariant1.ads
@@ -0,0 +1,7 @@
+package Loop_Invariant1 is
+
+ type Arr is array (Natural range <>) of Integer;
+
+ procedure Proc (A : Arr; N : Integer);
+
+end Loop_Invariant1;
[Ada] Spurious warning about a useless assignment
This patch removes a spurious warning about a useless assignment, when a
composite object is the target of an assignment and is an actual for an
out parameter in a subsewuent call, and there is an intervening use of
the object as the prefix of a selected component in an intervening
operation.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Ed Schonberg
gcc/ada/
* sem_res.adb (Resolve_Selected_Component): If the prefix has a
deferred reference, generate the correct reference now, to
indicate that the previous assignment is used. This prevents
spurious warnings on useless assignments when compiling with all
warnings enabled. when there is a subsequent call in the same
stqtement list, in which the prefix of the selected component is
the actual for an out parameter.
gcc/testsuite/
* gnat.dg/warn22.adb: New testcase.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -10625,8 +10625,25 @@ package body Sem_Res is
if Is_Access_Type (Etype (P)) then
T := Designated_Type (Etype (P));
Check_Fully_Declared_Prefix (T, P);
+
else
T := Etype (P);
+
+ -- If the prefix is an entity it may have a deferred reference set
+ -- during analysis of the selected component. After resolution we
+ -- can transform it into a proper reference. This prevents spurious
+ -- warnings on useless assignments when the same selected component
+ -- is the actual for an out parameter in a subsequent call.
+
+ if Is_Entity_Name (P)
+ and then Has_Deferred_Reference (Entity (P))
+ then
+if May_Be_Lvalue (N) then
+ Generate_Reference (Entity (P), P, 'm');
+else
+ Generate_Reference (Entity (P), P, 'r');
+end if;
+ end if;
end if;
-- Set flag for expander if discriminant check required on a component
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/warn22.adb
@@ -0,0 +1,34 @@
+-- { dg-do compile }
+-- { dg-options "-gnatwa" }
+
+with Ada.Text_IO;
+
+procedure Warn22
+is
+ type X is
+ record
+ Str : String (1 .. 3);
+ end record;
+
+ type T is
+ record
+ Value : X;
+ end record;
+
+ procedure Consume_Data (Item : out T) is
+ begin
+ Item := (Value => (Str => "Bar"));
+ end Consume_Data;
+
+ Baz : T;
+begin
+
+ Baz := (Value => (Str => "Foo"));
+
+ Ada.Text_IO.Put_Line (Baz.Value.Str);
+
+ Consume_Data (Baz);
+
+ Ada.Text_IO.Put_Line (Baz.Value.Str);
+
+end Warn22;
[Ada] Crash in C++ constructor without external and link name
The compiler blows up processing the declaration of a tagged type
variable that has a C++ constructor without external or link name. After
this patch the frontend reports an error.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Javier Miranda
gcc/ada/
* freeze.adb (Freeze_Subprogram): Check that C++ constructors
must have external or link name.
gcc/testsuite/
* gnat.dg/cpp_constructor2.adb: New testcase.--- gcc/ada/freeze.adb
+++ gcc/ada/freeze.adb
@@ -62,6 +62,7 @@ with Sem_Util; use Sem_Util;
with Sinfo; use Sinfo;
with Snames;use Snames;
with Stand; use Stand;
+with Stringt; use Stringt;
with Targparm; use Targparm;
with Tbuild;use Tbuild;
with Ttypes;use Ttypes;
@@ -8766,6 +8767,20 @@ package body Freeze is
Set_Is_Pure (E, False);
end if;
+ -- For C++ constructors check that their external name has been given
+ -- (either in pragma CPP_Constructor or in a pragma import).
+
+ if Is_Constructor (E)
+and then
+ (No (Interface_Name (E))
+ or else String_Equal
+(L => Strval (Interface_Name (E)),
+ R => Strval (Get_Default_External_Name (E
+ then
+ Error_Msg_N
+ ("'C++ constructor must have external name or link name", E);
+ end if;
+
-- We also reset the Pure indication on a subprogram with an Address
-- parameter, because the parameter may be used as a pointer and the
-- referenced data may change even if the address value does not.
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/cpp_constructor2.adb
@@ -0,0 +1,19 @@
+-- { dg-do compile }
+
+procedure CPP_Constructor2 is
+
+ package P is
+ type X is tagged limited record
+ A, B, C, D : Integer;
+ end record;
+ pragma Import (Cpp, X);
+
+ procedure F1 (V : X);
+ pragma Import (Cpp, F1);
+
+ function F2 return X; -- { dg-error "C\\+\\+ constructor must have external name or link name" }
+ pragma Cpp_Constructor (F2);
+ end P;
+begin
+ null;
+end CPP_Constructor2;
[Ada] Ensure Ctrl-C is not emited on terminated processes
Due to the reuse policy of PID on Windows. Sending a Ctrl-C to a dead process might result in a Ctrl-C sent to the wrong process. The check is also implemented on Unix platforms and avoid unecessary waits. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Nicolas Roche gcc/ada/ * terminals.c (__gnat_tty_waitpid): Support both blocking and not blocking mode. * libgnat/g-exptty.ads (Is_Process_Running): New function. * libgnat/g-exptty.adb (Close): Don't try to interrupt/terminate a process if it is already dead.--- gcc/ada/libgnat/g-exptty.adb +++ gcc/ada/libgnat/g-exptty.adb @@ -38,6 +38,28 @@ package body GNAT.Expect.TTY is On_Windows : constant Boolean := Directory_Separator = '\'; -- True when on Windows + function Waitpid (Process : System.Address; Blocking : Integer) + return Integer; + pragma Import (C, Waitpid, "__gnat_tty_waitpid"); + -- Wait for a specific process id, and return its exit code + + + -- Is_Process_Running -- + + + function Is_Process_Running + (Descriptor : in out TTY_Process_Descriptor) + return Boolean + is + begin + if Descriptor.Process = System.Null_Address then + return False; + end if; + + Descriptor.Exit_Status := Waitpid (Descriptor.Process, Blocking => 0); + return Descriptor.Exit_Status = Still_Active; + end Is_Process_Running; + --- -- Close -- --- @@ -49,10 +71,6 @@ package body GNAT.Expect.TTY is procedure Terminate_Process (Process : System.Address); pragma Import (C, Terminate_Process, "__gnat_terminate_process"); - function Waitpid (Process : System.Address) return Integer; - pragma Import (C, Waitpid, "__gnat_tty_waitpid"); - -- Wait for a specific process id, and return its exit code - procedure Free_Process (Process : System.Address); pragma Import (C, Free_Process, "__gnat_free_process"); @@ -63,7 +81,7 @@ package body GNAT.Expect.TTY is -- If we haven't already closed the process if Descriptor.Process = System.Null_Address then - Status := -1; + Status := Descriptor.Exit_Status; else -- Send a Ctrl-C to the process first. This way, if the launched @@ -75,9 +93,6 @@ package body GNAT.Expect.TTY is -- signal, so this needs to be done while the file descriptors are -- still open (it used to be after the closes and that was wrong). - Interrupt (Descriptor); - delay (0.05); - if Descriptor.Input_Fd /= Invalid_FD then Close (Descriptor.Input_Fd); end if; @@ -92,8 +107,23 @@ package body GNAT.Expect.TTY is Close (Descriptor.Output_Fd); end if; - Terminate_Process (Descriptor.Process); - Status := Waitpid (Descriptor.Process); + if Descriptor.Exit_Status = Still_Active then +Status := Waitpid (Descriptor.Process, Blocking => 0); + +if Status = Still_Active then + -- In theory the process might hav died since the check. In + -- practice the following calls should not cause any issue. + Interrupt (Descriptor); + delay (0.05); + Terminate_Process (Descriptor.Process); + Status := Waitpid (Descriptor.Process, Blocking => 1); + Descriptor.Exit_Status := Status; +end if; + else +-- If Exit_Status is not STILL_ACTIVE just retrieve the saved +-- exit status +Status := Descriptor.Exit_Status; + end if; if not On_Windows then Close_TTY (Descriptor.Process); @@ -258,6 +288,7 @@ package body GNAT.Expect.TTY is pragma Import (C, Internal, "__gnat_setup_communication"); begin + Pid.Exit_Status := Still_Active; if Internal (Pid.Process'Address) /= 0 then raise Invalid_Process with "cannot setup communication."; end if; --- gcc/ada/libgnat/g-exptty.ads +++ gcc/ada/libgnat/g-exptty.ads @@ -92,6 +92,11 @@ package GNAT.Expect.TTY is Columns: Natural); -- Sets up the size of the terminal as reported to the spawned process + function Is_Process_Running + (Descriptor : in out TTY_Process_Descriptor) + return Boolean; + -- Return True is the process is still alive + private -- All declarations in the private part must be fully commented ??? @@ -129,9 +134,14 @@ private Cmd : String; Args : System.Address); + Still_Active : constant Integer := -1; + type TTY_Process_Descriptor is new Process_Descriptor with record - Process : System.Address; -- Underlying structure used in C - Use_Pipes : Boolean := True; + Process : System.Address; + -- Underlying structure used in C + Exit_Status : Integer := Still_
[Ada] Incorrect values in conversion from fixed-point subtype with 'Small
This patch fixes incorrect computations involving a fixed-point subtype whose parent type has an aspect specification for 'Small. Executing the following: gnatmake -q conv ./conv must yield: 9000.00 9.00E+03 9000.00 9.00E+03 9.00E+03 9.00E+03 9.00E+03 9.00E+03 with Text_IO; use Text_IO; procedure Conv is V_P : constant := 10.0 ** (-6); M_V : constant := 9000.0; N_V : constant := -9000.0; type V_T is delta V_P range N_V .. M_V with Small => V_P; subtype S_T is V_T range 0.0 .. M_V; function Convert (Input : in S_T) return Long_Float is begin Put_Line (Input'Img); Put_Line (Long_Float'Image (Long_Float (Input))); return Long_Float (Input); end Convert; begin declare Var_S : constant S_T := S_T'Last; Output : constant Long_Float := Convert (Var_S); begin Put_Line (Long_Float'Image (Convert (Var_S))); Put_Line (Long_Float'Image (Long_Float (Var_S))); Put_Line (Output'Img); end; Put_Line (Long_Float'Image (Long_Float (S_T'Last))); end Conv; Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Ed Schonberg gcc/ada/ * freeze.adb (Freeze_Fixed_Point_Type): When freezing a fixed-point subtype, check whether the parent type declarastion includes an aspect specification for the 'Small type attribute, and inherit the specified value.--- gcc/ada/freeze.adb +++ gcc/ada/freeze.adb @@ -8003,6 +8003,7 @@ package body Freeze is Brng : constant Node_Id:= Scalar_Range (Btyp); BLo : constant Node_Id:= Low_Bound (Brng); BHi : constant Node_Id:= High_Bound (Brng); + Par : constant Entity_Id := First_Subtype (Typ); Small : constant Ureal := Small_Value (Typ); Loval : Ureal; Hival : Ureal; @@ -8055,6 +8056,16 @@ package body Freeze is end if; end if; + -- The 'small attribute may have been specified with an aspect, + -- in which case it is processed after a subtype declaration, so + -- inherit now the specified value. + + if Typ /= Par +and then Present (Find_Aspect (Par, Aspect_Small)) + then + Set_Small_Value (Typ, Small_Value (Par)); + end if; + -- Immediate return if the range is already analyzed. This means that -- the range is already set, and does not need to be computed by this -- routine.
[Ada] Fix wrong assumption on bounds in GNAT.Encode_String
This fixes a couple of oversights in the GNAT.Encode_String package,
whose effect is to assume that all the strings have a lower bound of 1.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* libgnat/g-encstr.adb (Encode_Wide_String): Fix oversight.
(Encode_Wide_Wide_String): Likewise.
gcc/testsuite/
* gnat.dg/encode_string1.adb, gnat.dg/encode_string1_pkg.adb,
gnat.dg/encode_string1_pkg.ads: New testcase.--- gcc/ada/libgnat/g-encstr.adb
+++ gcc/ada/libgnat/g-encstr.adb
@@ -79,12 +79,12 @@ package body GNAT.Encode_String is
Ptr : Natural;
begin
- Ptr := S'First;
+ Ptr := Result'First;
for J in S'Range loop
Encode_Wide_Character (S (J), Result, Ptr);
end loop;
- Length := Ptr - S'First;
+ Length := Ptr - Result'First;
end Encode_Wide_String;
-
@@ -108,12 +108,12 @@ package body GNAT.Encode_String is
Ptr : Natural;
begin
- Ptr := S'First;
+ Ptr := Result'First;
for J in S'Range loop
Encode_Wide_Wide_Character (S (J), Result, Ptr);
end loop;
- Length := Ptr - S'First;
+ Length := Ptr - Result'First;
end Encode_Wide_Wide_String;
---
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/encode_string1.adb
@@ -0,0 +1,48 @@
+-- { dg-do run }
+
+with Ada.Text_IO;use Ada.Text_IO;
+with Encode_String1_Pkg;
+with GNAT.Encode_String;
+with System.WCh_Con; use System.WCh_Con;
+
+procedure Encode_String1 is
+ High_WS : constant Wide_String (1000 .. 1009) := (others => '1');
+ High_WWS : constant Wide_Wide_String (1000 .. 1009) := (others => '2');
+ Low_WS : constant Wide_String (3 .. 12) := (others => '3');
+ Low_WWS : constant Wide_Wide_String (3 .. 12) := (others => '4');
+
+ procedure Test_Method (Method : WC_Encoding_Method);
+ -- Test Wide_String and Wide_Wide_String encodings using method Method to
+ -- encode them.
+
+ -
+ -- Test_Method --
+ -
+
+ procedure Test_Method (Method : WC_Encoding_Method) is
+ package Encoder is new GNAT.Encode_String (Method);
+
+ procedure WS_Tester is new Encode_String1_Pkg
+(C => Wide_Character,
+ S => Wide_String,
+ Encode => Encoder.Encode_Wide_String);
+
+ procedure WWS_Tester is new Encode_String1_Pkg
+(C => Wide_Wide_Character,
+ S => Wide_Wide_String,
+ Encode => Encoder.Encode_Wide_Wide_String);
+ begin
+ WS_Tester (High_WS);
+ WS_Tester (Low_WS);
+
+ WWS_Tester (High_WWS);
+ WWS_Tester (Low_WWS);
+ end Test_Method;
+
+-- Start of processing for Main
+
+begin
+ for Method in WC_Encoding_Method'Range loop
+ Test_Method (Method);
+ end loop;
+end;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/encode_string1_pkg.adb
@@ -0,0 +1,15 @@
+with Ada.Exceptions; use Ada.Exceptions;
+with Ada.Text_IO;use Ada.Text_IO;
+
+procedure Encode_String1_Pkg (Val : S) is
+begin
+ declare
+ Result : constant String := Encode (Val);
+ begin
+ Put_Line (Result);
+ end;
+
+exception
+ when Ex : others =>
+ Put_Line ("ERROR: Unexpected exception " & Exception_Name (Ex));
+end;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/encode_string1_pkg.ads
@@ -0,0 +1,6 @@
+generic
+ type C is private;
+ type S is array (Positive range <>) of C;
+ with function Encode (Val : S) return String;
+
+procedure Encode_String1_Pkg (Val : S);
[Ada] Fix missing Constraint_Error for Enum_Val attribute
This fixes an old issue involving the Enum_Val attribute: it does not always raise a Constraint_Error exception when the specified value is not valid for the enumeration type (instead a modulo computation is applied to the value). Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Eric Botcazou gcc/ada/ * exp_attr.adb (Expand_N_Attribute_Reference) : Set No_Truncation on the N_Unchecked_Type_Conversion built around the argument passed to the attribute. gcc/testsuite/ * gnat.dg/enum_val1.adb: New testcase.--- gcc/ada/exp_attr.adb +++ gcc/ada/exp_attr.adb @@ -3282,6 +3282,13 @@ package body Exp_Attr is Expr := Unchecked_Convert_To (Ptyp, First (Exprs)); + -- Ensure that the expression is not truncated since the "bad" bits + -- are desired. + + if Nkind (Expr) = N_Unchecked_Type_Conversion then +Set_No_Truncation (Expr); + end if; + Insert_Action (N, Make_Raise_Constraint_Error (Loc, Condition => --- /dev/null new file mode 100644 +++ gcc/testsuite/gnat.dg/enum_val1.adb @@ -0,0 +1,22 @@ +with Ada.Text_IO; use Ada.Text_IO; + +procedure Enum_Val1 is + type Enum is (Two, Four); + for Enum use (2, 4); + + Count : Natural := 0; + +begin + for I in 10 .. 11 loop + begin + Put (Integer'Image (I) & ": "); + Put_Line (Enum'Image (Enum'Enum_Val (I))); + exception + when Constraint_Error => +Count := Count + 1; + end; + end loop; + if Count /= 2 then + raise Program_Error; + end if; +end;
[Ada] Premature finalization of controlled temporaries in case expressions
The compiler was generating finalization of temporary objects used in
evaluating case expressions for controlled types in cases where the case
statement created by Expand_N_Expression_With_Actions is rewritten as an
if statement. This is fixed by inheriting the From_Condition_Expression
flag from the rewritten case statement.
The test below must generate the following output when executed:
$ main
Xs(1): 1
package Test is
type E is (E1, E2);
procedure Test (A : in E);
end Test;
with Ada.Text_IO;
with Ada.Finalization;
package body Test is
type T is new Ada.Finalization.Controlled with
record
N : Natural := 0;
end record;
overriding procedure Finalize (X : in out T) is
begin
X.N := 42;
end Finalize;
type T_Array is array (Positive range <>) of T;
function Make_T (N : Natural) return T is
begin
return (Ada.Finalization.Controlled with N => N);
end Make_T;
X1 : constant T := Make_T (1);
X2 : constant T := Make_T (2);
procedure Test (A : in E)
is
Xs : constant T_Array := (case A is
when E1 => (1 => X1),
when E2 => (1 => X2));
begin
Ada.Text_IO.Put_Line ("Xs(1):" & Natural'Image (Xs (1).N));
end Test;
end Test;
with Test;
procedure Main is
begin
Test.Test (Test.E1);
end Main;
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Gary Dismukes
gcc/ada/
* exp_ch5.adb (Expand_N_Case_Statement): In the case where a
case statement is rewritten as an equivalent if statement,
inherit the From_Condition_Expression flag from the case
statement.--- gcc/ada/exp_ch5.adb
+++ gcc/ada/exp_ch5.adb
@@ -2856,13 +2856,14 @@ package body Exp_Ch5 is
-
procedure Expand_N_Case_Statement (N : Node_Id) is
- Loc: constant Source_Ptr := Sloc (N);
- Expr : constant Node_Id:= Expression (N);
- Alt: Node_Id;
- Len: Nat;
- Cond : Node_Id;
- Choice : Node_Id;
- Chlist : List_Id;
+ Loc: constant Source_Ptr := Sloc (N);
+ Expr : constant Node_Id:= Expression (N);
+ From_Cond_Expr : constant Boolean:= From_Conditional_Expression (N);
+ Alt: Node_Id;
+ Len: Nat;
+ Cond : Node_Id;
+ Choice : Node_Id;
+ Chlist : List_Id;
begin
-- Check for the situation where we know at compile time which branch
@@ -3073,7 +3074,15 @@ package body Exp_Ch5 is
Condition => Cond,
Then_Statements => Then_Stms,
Else_Statements => Else_Stms));
+
+ -- The rewritten if statement needs to inherit whether the
+ -- case statement was expanded from a conditional expression,
+ -- for proper handling of nested controlled objects.
+
+ Set_From_Conditional_Expression (N, From_Cond_Expr);
+
Analyze (N);
+
return;
end if;
end if;
[Ada] Type inconsistency in floating_point type declarations
This patch fixes an inconsistency in the typing of the bounds of a floting point type declaration, when some bound is given by a dtatic constant of an explicit type, instead of a real literal, Previous to this patch the bound of the type retained the given type, leading to spurious errors in Codepeer. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Ed Schonberg gcc/ada/ * sem_ch3.adb (Convert_Bound): Subsidiary of Floating_Point_Type_Declaration, to handle properly range specifications with bounds that may include static constants of a given type rather than real literals.--- gcc/ada/sem_ch3.adb +++ gcc/ada/sem_ch3.adb @@ -17827,12 +17827,16 @@ package body Sem_Ch3 is Digs_Val : Uint; Base_Typ : Entity_Id; Implicit_Base : Entity_Id; - Bound : Node_Id; function Can_Derive_From (E : Entity_Id) return Boolean; -- Find if given digits value, and possibly a specified range, allows -- derivation from specified type + procedure Convert_Bound (B : Node_Id); + -- If specified, the bounds must be static but may be of different + -- types. They must be converted into machine numbers of the base type, + -- in accordance with RM 4.9(38). + function Find_Base_Type return Entity_Id; -- Find a predefined base type that Def can derive from, or generate -- an error and substitute Long_Long_Float if none exists. @@ -17870,6 +17874,28 @@ package body Sem_Ch3 is return True; end Can_Derive_From; + --- + -- Convert_Bound -- + + + procedure Convert_Bound (B : Node_Id) is + begin + -- If the bound is not a literal it can only be static if it is + -- a static constant, possibly of a specified type. + + if Is_Entity_Name (B) + and then Ekind (Entity (B)) = E_Constant + then +Rewrite (B, Constant_Value (Entity (B))); + end if; + + if Nkind (B) = N_Real_Literal then +Set_Realval (B, Machine (Base_Typ, Realval (B), Round, B)); +Set_Is_Machine_Number (B); +Set_Etype (B, Base_Typ); + end if; + end Convert_Bound; + -- Find_Base_Type -- @@ -17967,24 +17993,8 @@ package body Sem_Ch3 is Set_Scalar_Range (T, Real_Range_Specification (Def)); Set_Is_Constrained (T); - -- The bounds of this range must be converted to machine numbers - -- in accordance with RM 4.9(38). - - Bound := Type_Low_Bound (T); - - if Nkind (Bound) = N_Real_Literal then -Set_Realval - (Bound, Machine (Base_Typ, Realval (Bound), Round, Bound)); -Set_Is_Machine_Number (Bound); - end if; - - Bound := Type_High_Bound (T); - - if Nkind (Bound) = N_Real_Literal then -Set_Realval - (Bound, Machine (Base_Typ, Realval (Bound), Round, Bound)); -Set_Is_Machine_Number (Bound); - end if; + Convert_Bound (Type_Low_Bound (T)); + Convert_Bound (Type_High_Bound (T)); else Set_Scalar_Range (T, Scalar_Range (Base_Typ));
[Ada] Further fix non-stored discriminant in aggregate for GNATprove
GNATprove expects discriminants appearing in aggregates and their types to be resolved to stored discriminants. This extends the machinery that makes sure this is the case for default initialization expressions so as to also handle component associations in these expressions. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Eric Botcazou gcc/ada/ * sem_aggr.adb (Rewrite_Bound): Be prepared for discriminals too. (Rewrite_Range;): Minor tweak. (Resolve_Record_Aggregate): For a component with default initialization whose expression is an array aggregate, also rewrite the bounds of the component associations, if any.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -4264,8 +4264,15 @@ package body Sem_Aggr is Expr_Disc : Node_Id) is begin -if Nkind (Bound) = N_Identifier - and then Entity (Bound) = Disc +if Nkind (Bound) /= N_Identifier then + return; +end if; + +-- We expect either the discriminant or the discriminal + +if Entity (Bound) = Disc + or else (Ekind (Entity (Bound)) = E_In_Parameter +and then Discriminal_Link (Entity (Bound)) = Disc) then Rewrite (Bound, New_Copy_Tree (Expr_Disc)); end if; @@ -4280,9 +4287,7 @@ package body Sem_Aggr is -- Start of processing for Rewrite_Range begin - if Has_Discriminants (Root_Type) - and then Nkind (Rge) = N_Range - then + if Has_Discriminants (Root_Type) and then Nkind (Rge) = N_Range then Low := Low_Bound (Rge); High := High_Bound (Rge); @@ -4903,7 +4908,9 @@ package body Sem_Aggr is -- Root record type whose discriminants may be used as -- bounds in range nodes. -Index : Node_Id; +Assoc : Node_Id; +Choice : Node_Id; +Index : Node_Id; begin -- Rewrite the range nodes occurring in the indexes @@ -4919,12 +4926,26 @@ package body Sem_Aggr is end loop; -- Rewrite the range nodes occurring as aggregate --- bounds. +-- bounds and component associations. -if Nkind (Expr) = N_Aggregate - and then Present (Aggregate_Bounds (Expr)) -then - Rewrite_Range (Rec_Typ, Aggregate_Bounds (Expr)); +if Nkind (Expr) = N_Aggregate then + if Present (Aggregate_Bounds (Expr)) then + Rewrite_Range (Rec_Typ, Aggregate_Bounds (Expr)); + end if; + + if Present (Component_Associations (Expr)) then + Assoc := First (Component_Associations (Expr)); + while Present (Assoc) loop + Choice := First (Choices (Assoc)); + while Present (Choice) loop +Rewrite_Range (Rec_Typ, Choice); + +Next (Choice); + end loop; + + Next (Assoc); + end loop; + end if; end if; end; end if;
[Ada] Spurious error on private subtype of derived access type
This patch fixes a spurious type error on a dynamic predicate on a subtype of a private type whose full view is a derived access type. Prior to it, the base type of the subtype would appear to be the parent type of the derived type instead of the derived type itself, leading to problems downstream. The following package must now compile quietly: with S; package T is type B_Pointer is private; Null_B_Pointer : constant B_Pointer; function OK (B : B_Pointer) return Boolean is (B /= Null_B_Pointer); subtype Valid_B_Pointer is B_Pointer with Dynamic_Predicate => OK (Valid_B_Pointer); private type B_Pointer is new S.A_Pointer; Null_B_Pointer : constant B_Pointer := B_Pointer (S.Null_A_Pointer); end; package S is type A_Type is new Integer; type A_Pointer is access A_Type; Null_A_Pointer : constant A_Pointer := null; end; Moreover, it also plugs a loophole in the compiler whereby an instantiation of a generic with a formal subprogram declaration nested in an enclosing generic package would be done even if there was a mismatch between an original and a derived types involved in the instantiation. The compiler must now gives the following error: p.adb:11:43: no visible subprogram matches the specification for "Action" on with Q; with R; with G; procedure P is package My_G is new G (Q.T); procedure Proc (Value : R.T) is null; procedure Iter is new My_G.Iteration_G (Proc); begin null; end; with R; package Q is type T is new R.T; end Q; package R is type T is private; private type T is access Integer; end R; generic type Value_T is private; package G is generic with procedure Action (Value : Value_T); procedure Iteration_G; end G; package body G is procedure Iteration_G is null; end G; Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Eric Botcazou gcc/ada/ * sem_ch3.adb (Complete_Private_Subtype): Rework the setting of the Etype of the full view for full base types that cannot contain any discriminant. Remove code and comment about it in the main path.--- gcc/ada/sem_ch3.adb +++ gcc/ada/sem_ch3.adb @@ -12351,48 +12351,73 @@ package body Sem_Ch3 is -- Next_Entity field of full to ensure that the calls to Copy_Node do -- not corrupt the entity chain. - -- Note that the type of the full view is the same entity as the type - -- of the partial view. In this fashion, the subtype has access to the - -- correct view of the parent. - -- The list below included access types, but this leads to several - -- regressions. How should the base type of the full view be - -- set consistently for subtypes completed by access types? - Save_Next_Entity := Next_Entity (Full); Save_Homonym := Homonym (Priv); - case Ekind (Full_Base) is - when Class_Wide_Kind -| Private_Kind -| Protected_Kind -| Task_Kind -| E_Record_Subtype -| E_Record_Type - => -Copy_Node (Priv, Full); + if Ekind (Full_Base) in Private_Kind +or else Ekind (Full_Base) in Protected_Kind +or else Ekind (Full_Base) in Record_Kind +or else Ekind (Full_Base) in Task_Kind + then + Copy_Node (Priv, Full); -Set_Has_Discriminants - (Full, Has_Discriminants (Full_Base)); -Set_Has_Unknown_Discriminants - (Full, Has_Unknown_Discriminants (Full_Base)); -Set_First_Entity (Full, First_Entity (Full_Base)); -Set_Last_Entity (Full, Last_Entity (Full_Base)); + -- Note that the Etype of the full view is the same as the Etype of + -- the partial view. In this fashion, the subtype has access to the + -- correct view of the parent. --- If the underlying base type is constrained, we know that the --- full view of the subtype is constrained as well (the converse --- is not necessarily true). + Set_Has_Discriminants (Full, Has_Discriminants (Full_Base)); + Set_Has_Unknown_Discriminants + (Full, Has_Unknown_Discriminants (Full_Base)); + Set_First_Entity (Full, First_Entity (Full_Base)); + Set_Last_Entity (Full, Last_Entity (Full_Base)); -if Is_Constrained (Full_Base) then - Set_Is_Constrained (Full); -end if; + -- If the underlying base type is constrained, we know that the + -- full view of the subtype is constrained as well (the converse + -- is not necessarily true). - when others => -Copy_Node (Full_Base, Full); + if Is_Constrained (Full_Base) then +Set_Is_Constrained (Full); + end if; -Set_Chars (Full, Chars (Priv)); -Conditional_Delay (Full, Priv); -Set_Slo
[Ada] Remove obsolete Is_For_Access_Subtype machinery
This change removes the Is_For_Access_Subtype machinery from the
compiler. This machinery was devised a long time ago to deal with a
peculiarity of the freezing for access-to-record subtypes but has been
degenerate for quite some time now and does not seem to serve any useful
purpose at this point.
Morever it has an annoying side effect whereby it causes Underlying_Type
to return the (unconstrained) base record type when invoked on the
designated record subtype, which is very problematic for GNATprove.
There should be no functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* einfo.ads (Is_For_Access_Subtype): Delete.
(Set_Is_For_Access_Subtype): Likewise.
* einfo.adb (Is_For_Access_Subtype): Likewise.
(Set_Is_For_Access_Subtype): Likewise.
(Write_Entity_Flags): Do not write Is_For_Access_Subtype.
* exp_ch4.adb (Expand_N_Selected_Component): Do not deal with
it.
* exp_spark.adb (Expand_SPARK_N_Selected_Component): Likewise.
* sem_ch4.adb (Analyze_Explicit_Dereference): Likewise.
* sem_ch3.adb (Build_Discriminated_Subtype): Do not build a
special private subtype for access-to-record subtypes.--- gcc/ada/einfo.adb
+++ gcc/ada/einfo.adb
@@ -421,7 +421,6 @@ package body Einfo is
--Never_Set_In_Source Flag115
--Is_Visible_Lib_Unit Flag116
--Is_Unchecked_Union Flag117
- --Is_For_Access_Subtype Flag118
--Has_Convention_Pragma Flag119
--Has_Primitive_OperationsFlag120
@@ -2303,12 +2302,6 @@ package body Einfo is
return Flag70 (Id);
end Is_First_Subtype;
- function Is_For_Access_Subtype (Id : E) return B is
- begin
- pragma Assert (Ekind_In (Id, E_Record_Subtype, E_Private_Subtype));
- return Flag118 (Id);
- end Is_For_Access_Subtype;
-
function Is_Formal_Subprogram (Id : E) return B is
begin
return Flag111 (Id);
@@ -5526,12 +5519,6 @@ package body Einfo is
Set_Flag70 (Id, V);
end Set_Is_First_Subtype;
- procedure Set_Is_For_Access_Subtype (Id : E; V : B := True) is
- begin
- pragma Assert (Ekind_In (Id, E_Record_Subtype, E_Private_Subtype));
- Set_Flag118 (Id, V);
- end Set_Is_For_Access_Subtype;
-
procedure Set_Is_Formal_Subprogram (Id : E; V : B := True) is
begin
Set_Flag111 (Id, V);
@@ -9826,7 +9813,6 @@ package body Einfo is
W ("Is_Exported", Flag99 (Id));
W ("Is_Finalized_Transient", Flag252 (Id));
W ("Is_First_Subtype",Flag70 (Id));
- W ("Is_For_Access_Subtype", Flag118 (Id));
W ("Is_Formal_Subprogram",Flag111 (Id));
W ("Is_Frozen", Flag4 (Id));
W ("Is_Generic_Actual_Subprogram",Flag274 (Id));
--- gcc/ada/einfo.ads
+++ gcc/ada/einfo.ads
@@ -2608,12 +2608,6 @@ package Einfo is
--Is_Formal_Subprogram (Flag111)
-- Defined in all entities. Set for generic formal subprograms.
---Is_For_Access_Subtype (Flag118)
--- Defined in E_Private_Subtype and E_Record_Subtype entities. Means the
--- sole purpose of the type is to be designated by an Access_Subtype and
--- hence should not be expanded into components because the type may not
--- have been found or frozen yet.
-
--Is_Frozen (Flag4)
-- Defined in all type and subtype entities. Set if type or subtype has
-- been frozen.
@@ -6458,7 +6452,6 @@ package Einfo is
--Stored_Constraint (Elist23)
--Has_Completion (Flag26)
--Is_Controlled_Active(Flag42) (base type only)
- --Is_For_Access_Subtype (Flag118) (subtype only)
--(plus type attributes)
-- E_Procedure
@@ -7311,7 +7304,6 @@ package Einfo is
function Is_Exported (Id : E) return B;
function Is_Finalized_Transient (Id : E) return B;
function Is_First_Subtype(Id : E) return B;
- function Is_For_Access_Subtype (Id : E) return B;
function Is_Frozen (Id : E) return B;
function Is_Generic_Instance (Id : E) return B;
function Is_Hidden (Id : E) return B;
@@ -8012,7 +8004,6 @@ package Einfo is
procedure Set_Is_Exported (Id : E; V : B := True);
procedure Set_Is_Finalized_Transient (Id : E; V : B := True);
procedure Set_Is_First_Subtype(Id : E; V : B := True);
- procedure Set_Is_For_Access_Subtype (Id : E; V : B := True);
procedure Set_Is_Formal_Subprogram(Id : E; V : B := True);
procedure Set_Is_Frozen (Id : E; V : B := True);
procedure Set_Is_Generic_Actual_Subprogram(Id : E; V : B := Tru
[Ada] Internal error on iterator for limited private discriminated type
This patch further extends the short-circuit, aka optimization, present
in the Check_Constrained_Object procedure used for renaming declarations
to all limited types, so as to prevent type mismatches downstream in
more cases.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* sem_ch8.adb (Check_Constrained_Object): Further extend the
special optimization to all limited types.
gcc/testsuite/
* gnat.dg/iter5.adb, gnat.dg/iter5_pkg.ads: New testcase.--- gcc/ada/sem_ch8.adb
+++ gcc/ada/sem_ch8.adb
@@ -809,18 +809,12 @@ package body Sem_Ch8 is
-- in particular with record types with an access discriminant
-- that are used in iterators. This is an optimization, but it
-- also prevents typing anomalies when the prefix is further
--- expanded. This also applies to limited types with access
--- discriminants.
+-- expanded.
-- Note that we cannot just use the Is_Limited_Record flag because
-- it does not apply to records with limited components, for which
-- this syntactic flag is not set, but whose size is also fixed.
-elsif (Is_Record_Type (Typ) and then Is_Limited_Type (Typ))
- or else
-(Ekind (Typ) = E_Limited_Private_Type
- and then Has_Discriminants (Typ)
- and then Is_Access_Type (Etype (First_Discriminant (Typ
-then
+elsif Is_Limited_Type (Typ) then
null;
else
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/iter5.adb
@@ -0,0 +1,10 @@
+-- { dg-do compile }
+
+with Iter5_Pkg;
+
+procedure Iter5 is
+begin
+ for The_Filename of Iter5_Pkg.Iterator_For ("C:\Program_Files") loop
+ null;
+ end loop;
+end Iter5;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/iter5_pkg.ads
@@ -0,0 +1,127 @@
+with Ada.Calendar;
+with Ada.Directories;
+
+with Ada.Iterator_Interfaces;
+
+package Iter5_Pkg is
+
+ subtype Size is Ada.Directories.File_Size;
+
+ type Folder is new String;
+
+ function Folder_Separator return Character;
+
+ function "+" (Directory : String) return Folder;
+
+ function "+" (Left, Right : String) return Folder;
+
+ function "+" (Left : Folder;
+Right : String) return Folder;
+
+ function Composure (Directory : Folder;
+ Filename : String;
+ Extension : String) return String;
+
+ function Composure (Directory : String;
+ Filename : String;
+ Extension : String) return String;
+ -- no exception
+
+ function Base_Name_Of (Name : String) return String
+renames Ada.Directories.Base_Name;
+
+ function Extension_Of (Name : String) return String
+renames Ada.Directories.Extension;
+
+ function Containing_Directory_Of (Name : String) return String
+renames Ada.Directories.Containing_Directory;
+
+ function Exists (Name : String) return Boolean;
+ -- no exception
+
+ function Size_Of (Name : String) return Size renames Ada.Directories.Size;
+
+ function Directory_Exists (Name : String) return Boolean;
+ -- no exception
+
+ function Modification_Time_Of (Name : String) return Ada.Calendar.Time
+renames Ada.Directories.Modification_Time;
+
+ function Is_Newer (The_Name : String;
+ Than_Name : String) return Boolean;
+
+ procedure Delete (Name : String);
+ -- no exception if no existance
+
+ procedure Create_Directory (Path : String);
+ -- creates the whole directory path
+
+ procedure Delete_Directory (Name : String); -- including contents
+ -- no exception if no existance
+
+ procedure Rename (Old_Name : String;
+New_Name : String) renames Ada.Directories.Rename;
+
+ procedure Copy (Source_Name : String;
+ Target_Name : String;
+ Form : String := "")
+renames Ada.Directories.Copy_File;
+
+ function Is_Leaf_Directory (Directory : String) return Boolean;
+
+ procedure Iterate_Over_Leaf_Directories (From_Directory : String;
+ Iterator : access procedure
+ (Leaf_Directory : String));
+
+ function Found_Directory (Simple_Name : String;
+In_Directory : String) return String;
+
+ Not_Found : exception;
+
+ Name_Error : exception renames Ada.Directories.Name_Error;
+ Use_Error : exception renames Ada.Directories.Use_Error;
+
+
+ -- File Iterator Loop --
+
+ -- Example:
+ -- for The_Filename of Iter5_Pkg.Iterator_For ("C:\Program_Files") loop
+ --Log.Write (The_Filename);
+ -- end loop;
+
+ type Item (Name_Length : Natural) is limited private;
+
+ function Iterator_For (Name : String) return Item;
+
+private
+ type Cursor;
+
+ fu
[Ada] Fix spurious loop warning for function with Out parameter
The compiler gives a spurious warning about a possible infinite while
loop whose condition contains a call to a function that takes an Out or
In/Out parameter and whose actual is a variable that is not modified in
the loop, because it still thinks that functions can only have In
parameters.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* sem_warn.adb (Find_Var): Bail out for a function call with an
Out or In/Out parameter.
gcc/testsuite/
* gnat.dg/warn23.adb: New testcase.--- gcc/ada/sem_warn.adb
+++ gcc/ada/sem_warn.adb
@@ -333,6 +333,11 @@ package body Sem_Warn is
elsif Has_Warnings_Off (Entity (Name (N))) then
return;
+
+-- Forget it if the parameter is not In
+
+elsif Has_Out_Or_In_Out_Parameter (Entity (Name (N))) then
+ return;
end if;
-- OK, see if we have one argument
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/warn23.adb
@@ -0,0 +1,17 @@
+-- { dg-do compile }
+
+procedure Warn23 is
+
+ type Enum_Type is (A, B, C);
+
+ function Poll (E : out Enum_Type) return Boolean
+ with Convention => Ada,
+ Import => True;
+
+ E : Enum_Type;
+
+begin
+ while Poll (E) loop
+ null;
+ end loop;
+end;
[Ada] Fix spurious visibility error for tagged type with inlining
This fixes a spurious visibility error for the very peculiar case where
an operator that operates on the class-wide type of a tagged type is
declared in a package, the operator is renamed in another package where
a subtype of the tagged type is declared, and both packages end up in
the transititive closure of a unit compiled with optimization and
inter-inlining (-gnatn).
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* sem_ch8.adb (End_Use_Type): Reset the In_Use flag on the
class-wide type if the type is tagged.
(Use_One_Type): Add commentary on the handling of the class-wide
type.
gcc/testsuite/
* gnat.dg/inline17.adb, gnat.dg/inline17_pkg1.adb,
gnat.dg/inline17_pkg1.ads, gnat.dg/inline17_pkg2.ads,
gnat.dg/inline17_pkg3.adb, gnat.dg/inline17_pkg3.ads: New
testcase.--- gcc/ada/sem_ch8.adb
+++ gcc/ada/sem_ch8.adb
@@ -4836,6 +4836,13 @@ package body Sem_Ch8 is
Set_In_Use (Base_Type (T), False);
Set_Current_Use_Clause (T, Empty);
Set_Current_Use_Clause (Base_Type (T), Empty);
+
+-- See Use_One_Type for the rationale. This is a bit on the naive
+-- side, but should be good enough in practice.
+
+if Is_Tagged_Type (T) then
+ Set_In_Use (Class_Wide_Type (T), False);
+end if;
end if;
end if;
@@ -9985,7 +9992,10 @@ package body Sem_Ch8 is
Set_In_Use (T);
-- If T is tagged, primitive operators on class-wide operands are
- -- also available.
+ -- also deemed available. Note that this is really necessary only
+ -- in semantics-only mode, because the primitive operators are not
+ -- fully constructed in this mode, but we do it in all modes for the
+ -- sake of uniformity, as this should not matter in practice.
if Is_Tagged_Type (T) then
Set_In_Use (Class_Wide_Type (T));
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17.adb
@@ -0,0 +1,10 @@
+-- { dg-do compile }
+-- { dg-options "-O -gnatn" }
+with Inline17_Pkg1; use Inline17_Pkg1;
+with Inline17_Pkg2; use Inline17_Pkg2;
+
+procedure Inline17 is
+ use type SQL_Field;
+begin
+ Test;
+end;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17_pkg1.adb
@@ -0,0 +1,15 @@
+with Inline17_Pkg2; use Inline17_Pkg2;
+
+package body Inline17_Pkg1 is
+
+ procedure Test is
+ begin
+ null;
+ end;
+
+ function Get (Field : SQL_Field) return Integer is
+ begin
+ return +Field;
+ end;
+
+end Inline17_Pkg1;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17_pkg1.ads
@@ -0,0 +1,7 @@
+
+package Inline17_Pkg1 is
+
+ procedure Test;
+ pragma Inline (Test);
+
+end Inline17_Pkg1;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17_pkg2.ads
@@ -0,0 +1,10 @@
+with Inline17_Pkg3; use Inline17_Pkg3;
+
+package Inline17_Pkg2 is
+
+ subtype SQL_Field is Inline17_Pkg3.SQL_Field;
+
+ function "+" (Field : SQL_Field'Class) return Integer renames
+ Inline17_Pkg3."+";
+
+end Inline17_Pkg2;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17_pkg3.adb
@@ -0,0 +1,14 @@
+
+package body Inline17_Pkg3 is
+
+ function "+" (Field : SQL_Field'Class) return Integer is
+ begin
+ return 0;
+ end;
+
+ function Unchecked_Get (Self : Ref) return Integer is
+ begin
+ return Self.Data;
+ end;
+
+end Inline17_Pkg3;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/inline17_pkg3.ads
@@ -0,0 +1,16 @@
+
+package Inline17_Pkg3 is
+
+ type SQL_Field is tagged null record;
+
+ function "+" (Field : SQL_Field'Class) return Integer;
+
+ type Ref is record
+ Data : Integer;
+ end record;
+
+ function Unchecked_Get (Self : Ref) return Integer with Inline_Always;
+
+ function Get (Self : Ref) return Integer is (Unchecked_Get (Self));
+
+end Inline17_Pkg3;
[Ada] Misleading warning on variable not assigned
This patch removes a warning on a referenced entity with no explicit
prior assignment, if the type of the entity has
Preelaborable_Initialixation, such as Exception_Occurrence.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Ed Schonberg
gcc/ada/
* sem_warn.adb (Check_References): Do not emit s warning on a
referenced entity with no explicit assignment if the type of the
entity has Preelaborable_Initialixation, such as
Exception_Occurrence.
gcc/testsuite/
* gnat.dg/warn25.adb: New testcase.--- gcc/ada/sem_warn.adb
+++ gcc/ada/sem_warn.adb
@@ -1413,9 +1413,13 @@ package body Sem_Warn is
goto Continue;
end if;
- -- Check for unset reference
+ -- Check for unset reference. If type of object has
+ -- preelaborable initialization, warning is misleading.
- if Warn_On_No_Value_Assigned and then Present (UR) then
+ if Warn_On_No_Value_Assigned
+ and then Present (UR)
+ and then not Known_To_Have_Preelab_Init (Etype (E1))
+ then
-- For other than access type, go back to original node to
-- deal with case where original unset reference has been
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/warn25.adb
@@ -0,0 +1,23 @@
+-- { dg-do compile }
+-- { dg-options "-gnatwa" }
+
+with Ada.Exceptions;
+procedure Warn25 is
+CASA_Unavailable : Ada.Exceptions.Exception_Occurrence;
+use Ada.Exceptions;
+begin
+ while True loop
+declare
+begin
+ if Exception_Identity (CASA_Unavailable) = Null_Id then
+ exit;
+ end if;
+ exception
+ when E : others =>
+ Save_Occurrence (Source => E, Target => CASA_Unavailable);
+ end;
+ end loop;
+ if Exception_Identity (CASA_Unavailable) /= Null_Id then
+ Reraise_Occurrence (CASA_Unavailable);
+ end if;
+end;
[Ada] Issue warning or error message on ignored typing constraint
GNAT ignores the discriminant constraint on a component when it applies
to the type of the record being analyzed. Now issue a warning on Ada
code when ignoring this constraint, or an error on SPARK code.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Yannick Moy
gcc/ada/
* sem_ch3.adb (Constrain_Access): Issue a message about ignored
constraint.
gcc/testsuite/
* gnat.dg/warn24.adb: New testcase.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -12970,6 +12970,10 @@ package body Sem_Ch3 is
if Desig_Type = Current_Scope
and then No (Def_Id)
then
+Error_Msg_Warn := SPARK_Mode /= On;
+Error_Msg_N ("<
[Ada] Optimization loses exception in improper use of 'Value
This patch prevents an improper removal of an evaluation of attribute
'Value on an illegal input that will raise Constraint_Error, when a
subsequent use of this evaluation might be optimized away by the
back-end.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Ed Schonberg
gcc/ada/
* libgnat/s-valboo.ads, libgnat/s-valcha.ads,
libgnat/s-valdec.ads, libgnat/s-valenu.ads,
libgnat/s-valint.ads, libgnat/s-vallld.ads,
libgnat/s-vallli.ads, libgnat/s-valllu.ads,
libgnat/s-valrea.ads, libgnat/s-valuns.ads,
libgnat/s-valwch.ads: Change categorization of packages that
implement attribute 'Value from Pure to Preelaborate, to prevent
undesirable optimizations when the evaluation of the attribute
raises Constraint_Error, but subsequent use of the result of
this evsaluation is removed by a subsequent optimization.
gcc/testsuite/
* gnat.dg/opt80.adb: New testcase.--- gcc/ada/libgnat/s-valboo.ads
+++ gcc/ada/libgnat/s-valboo.ads
@@ -30,7 +30,7 @@
--
package System.Val_Bool is
- pragma Pure;
+ pragma Preelaborate;
function Value_Boolean (Str : String) return Boolean;
-- Computes Boolean'Value (Str)
--- gcc/ada/libgnat/s-valcha.ads
+++ gcc/ada/libgnat/s-valcha.ads
@@ -30,7 +30,7 @@
--
package System.Val_Char is
- pragma Pure;
+ pragma Preelaborate;
function Value_Character (Str : String) return Character;
-- Computes Character'Value (Str)
--- gcc/ada/libgnat/s-valdec.ads
+++ gcc/ada/libgnat/s-valdec.ads
@@ -34,7 +34,7 @@
-- Decimal_IO, and the Value attribute for such decimal types.
package System.Val_Dec is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Decimal
(Str : String;
--- gcc/ada/libgnat/s-valenu.ads
+++ gcc/ada/libgnat/s-valenu.ads
@@ -34,7 +34,7 @@
-- details of the format of constructed image tables.
package System.Val_Enum is
- pragma Pure;
+ pragma Preelaborate;
function Value_Enumeration_8
(Names : String;
--- gcc/ada/libgnat/s-valint.ads
+++ gcc/ada/libgnat/s-valint.ads
@@ -33,7 +33,7 @@
-- in Text_IO.Integer_IO, and the Value attribute.
package System.Val_Int is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Integer
(Str : String;
--- gcc/ada/libgnat/s-vallld.ads
+++ gcc/ada/libgnat/s-vallld.ads
@@ -34,7 +34,7 @@
-- Decimal_IO, and the Value attribute for such decimal types.
package System.Val_LLD is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Long_Long_Decimal
(Str : String;
--- gcc/ada/libgnat/s-vallli.ads
+++ gcc/ada/libgnat/s-vallli.ads
@@ -33,7 +33,7 @@
-- values for use in Text_IO.Integer_IO, and the Value attribute.
package System.Val_LLI is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Long_Long_Integer
(Str : String;
--- gcc/ada/libgnat/s-valllu.ads
+++ gcc/ada/libgnat/s-valllu.ads
@@ -35,7 +35,7 @@
with System.Unsigned_Types;
package System.Val_LLU is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Raw_Long_Long_Unsigned
(Str : String;
--- gcc/ada/libgnat/s-valrea.ads
+++ gcc/ada/libgnat/s-valrea.ads
@@ -30,7 +30,7 @@
--
package System.Val_Real is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Real
(Str : String;
--- gcc/ada/libgnat/s-valuns.ads
+++ gcc/ada/libgnat/s-valuns.ads
@@ -35,7 +35,7 @@
with System.Unsigned_Types;
package System.Val_Uns is
- pragma Pure;
+ pragma Preelaborate;
function Scan_Raw_Unsigned
(Str : String;
--- gcc/ada/libgnat/s-valwch.ads
+++ gcc/ada/libgnat/s-valwch.ads
@@ -34,7 +34,7 @@
with System.WCh_Con;
package System.Val_WChar is
- pragma Pure;
+ pragma Preelaborate;
function Value_Wide_Character
(Str : String;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/opt80.adb
@@ -0,0 +1,15 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Text_IO; use Ada.Text_IO;
+
+procedure Opt80 is
+ Item : Integer;
+begin
+ Item := Integer'Value ("zzz");
+ Put_Line (Boolean'Image (Item'Valid));
+ raise Program_Error;
+exception
+ when Constraint_Error =>
+ null;
+end;
[Ada] Spurious error passing access to class-wide interface type
The compiler reports an spurious error when the formal parameter of a
subprogram is an access to a class wide interface type and the actual
parameter is an allocator of an object covering such interface type.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Javier Miranda
gcc/ada/
* sem_res.adb (Resolve_Actuals): Replace code that displaces the
pointer to an allocated object to reference its secondary
dispatch table by a type conversion (which takes care of
handling all cases).
gcc/testsuite/
* gnat.dg/class_wide5.adb: New testcase.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -4190,17 +4190,16 @@ package body Sem_Res is
DDT : constant Entity_Id :=
Directly_Designated_Type (Base_Type (Etype (F)));
- New_Itype : Entity_Id;
-
begin
+ -- Displace the pointer to the object to reference its
+ -- secondary dispatch table.
+
if Is_Class_Wide_Type (DDT)
and then Is_Interface (DDT)
then
-New_Itype := Create_Itype (E_Anonymous_Access_Type, A);
-Set_Etype (New_Itype, Etype (A));
-Set_Directly_Designated_Type
- (New_Itype, Directly_Designated_Type (Etype (A)));
-Set_Etype (A, New_Itype);
+Rewrite (A, Convert_To (Etype (F), Relocate_Node (A)));
+Analyze_And_Resolve (A, Etype (F),
+ Suppress => Access_Check);
end if;
-- Ada 2005, AI-162:If the actual is an allocator, the
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/class_wide5.adb
@@ -0,0 +1,11 @@
+-- { dg-do compile }
+
+procedure Class_Wide5 is
+ type B is interface;
+ type B_Child is new B with null record;
+ type B_Ptr is access B'Class;
+
+ procedure P (Obj : B_Ptr) is begin null; end;
+begin
+ P (new B_child); -- Test
+end Class_Wide5;
[Ada] Remove misleading warning/suggestion in membership test
This patch removes a warning on a membership test whose right operand is
given by a range. In many cases the check can be replaced by the use of
attribute 'Valid, but if the bounds of range are type conversion this
replacement would be invorrect and the warning and suggestion are
misleading.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Ed Schonberg
gcc/ada/
* exp_ch4.adb (Expand_N_In): Do not suggest the use of attribute
'Valid as a replacement for a range check on a discrete type
when the bounds of the range are given by type conversions,
because in such a case there are distinct types involved and the
subbested attribute replacement would be misplaced.
gcc/testsuite/
* gnat.dg/warn26.adb: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -6272,6 +6272,10 @@ package body Exp_Ch4 is
-- Similarly, do not rewrite membership as a validity check if
-- within the predicate function for the type.
+ -- Finally, if the original bounds are type conversions, even
+ -- if they have been folded into constants, there are different
+ -- types involved and 'Valid is not appropriate.
+
then
if In_Instance
or else (Ekind (Current_Scope) = E_Function
@@ -6279,6 +6283,11 @@ package body Exp_Ch4 is
then
null;
+ elsif Nkind (Lo_Orig) = N_Type_Conversion
+ or else Nkind (Hi_Orig) = N_Type_Conversion
+ then
+ null;
+
else
Substitute_Valid_Check;
goto Leave;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/warn26.adb
@@ -0,0 +1,20 @@
+-- { dg-do compile }
+
+procedure Warn26 is
+
+ Monitor_Period_Min : constant := 5;
+ Monitor_Period_Max : constant := 30;
+
+ type Monitor_Period is range Monitor_Period_Min .. Monitor_Period_Max;
+
+ subtype Period_T is Positive range 5 .. 30;
+
+ function Id (X : Period_T) return Period_T is (X);
+ Input_Period : Period_T := Id (20);
+begin
+ if Input_Period in
+ Integer (Monitor_Period'First) .. Integer ( Monitor_Period'Last)
+ then
+ null;
+ end if;
+end Warn26;
[Ada] Adapt ownership checking in SPARK to traversal functions
A traversal function, especially when implemented as an expression function, may need to return an if-expression or case-expression, while still respecting Legality Rule SPARK RM 3.10(5). This case is now allowed in GNATprove. There is no impact on compilation. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Yannick Moy gcc/ada/ * sem_spark.adb (Get_Root_Object, Is_Path_Expression, Is_Subpath_Expression): Add parameter Is_Traversal to adapt these functions to the case of paths returned from a traversal function. (Read_Indexes): Handle the case of an if-expression or case-expression. (Check_Statement): Check Emit_Messages only when issuing an error message. This is important as Emit_Messages may store the information that an error was detected.--- gcc/ada/sem_spark.adb +++ gcc/ada/sem_spark.adb @@ -715,10 +715,14 @@ package body Sem_SPARK is function Get_Root_Object (Expr : Node_Id; - Through_Traversal : Boolean := True) return Entity_Id; + Through_Traversal : Boolean := True; + Is_Traversal : Boolean := False) return Entity_Id; -- Return the root of the path expression Expr, or Empty for an allocator, -- NULL, or a function call. Through_Traversal is True if it should follow - -- through calls to traversal functions. + -- through calls to traversal functions. Is_Traversal is True if this + -- corresponds to a value returned from a traversal function, which should + -- allow if-expressions and case-expressions that refer to the same root, + -- even if the paths are not the same in all branches. generic with procedure Handle_Parameter_Or_Global @@ -745,11 +749,19 @@ package body Sem_SPARK is -- A procedure that is called when deep globals or aliased globals are used -- without any global aspect. - function Is_Path_Expression (Expr : Node_Id) return Boolean; - -- Return whether Expr corresponds to a path + function Is_Path_Expression + (Expr : Node_Id; + Is_Traversal : Boolean := False) return Boolean; + -- Return whether Expr corresponds to a path. Is_Traversal is True if this + -- corresponds to a value returned from a traversal function, which should + -- allow if-expressions and case-expressions. - function Is_Subpath_Expression (Expr : Node_Id) return Boolean; - -- Return True if Expr can be part of a path expression + function Is_Subpath_Expression + (Expr : Node_Id; + Is_Traversal : Boolean := False) return Boolean; + -- Return True if Expr can be part of a path expression. Is_Traversal is + -- True if this corresponds to a value returned from a traversal function, + -- which should allow if-expressions and case-expressions. function Is_Prefix_Or_Almost (Pref, Expr : Node_Id) return Boolean; -- Determine if the candidate Prefix is indeed a prefix of Expr, or almost @@ -1749,6 +1761,31 @@ package body Sem_SPARK is end loop; end; +when N_If_Expression => + declare + Cond : constant Node_Id := First (Expressions (Expr)); + Then_Part : constant Node_Id := Next (Cond); + Else_Part : constant Node_Id := Next (Then_Part); + begin + Read_Expression (Cond); + Read_Indexes (Then_Part); + Read_Indexes (Else_Part); + end; + +when N_Case_Expression => + declare + Cases: constant List_Id := Alternatives (Expr); + Cur_Case : Node_Id := First (Cases); + + begin + Read_Expression (Expression (Expr)); + + while Present (Cur_Case) loop + Read_Indexes (Expression (Cur_Case)); + Next (Cur_Case); + end loop; + end; + when N_Attribute_Reference => pragma Assert (Get_Attribute_Id (Attribute_Name (Expr)) = @@ -3115,14 +3152,14 @@ package body Sem_SPARK is if Is_Anonymous_Access_Type (Return_Typ) then pragma Assert (Is_Traversal_Function (Subp)); -if Nkind (Expr) /= N_Null and then Emit_Messages then +if Nkind (Expr) /= N_Null then declare Expr_Root : constant Entity_Id := -Get_Root_Object (Expr); +Get_Root_Object (Expr, Is_Traversal => True); Param : constant Entity_Id := First_Formal (Subp); begin - if Param /= Expr_Root then + if Param /= Expr_Root and then Emit_Messages then
[Ada] Overhaul code implementing conversions involving fixed-point types
This ovehauls the code implementing conversions involving fixed-point types in the front-end because it leaks the Do_Range_Check flag in several places to the back-end, which is a violation of the documented interface between front-end and back-end. This also does a bit of housekeeping work throughout it in the process. There should be essentially no functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Eric Botcazou gcc/ada/ * checks.adb (Apply_Type_Conversion_Checks): Do not set Do_Range_Check flag on conversions from fixed-point types either. * exp_attr.adb: Add use and with clause for Expander. (Expand_N_Attribute_Reference) : Set the Conversion_OK flag and do not generate overflow/range checks manually. * exp_ch4.adb (Expand_N_Qualified_Expression): Remove superfluous clearing of Do_Range_Check flag. (Discrete_Range_Check): New procedure to generate a range check for discrete types. (Real_Range_Check): Remove redundant local variable and adjust. Remove useless shortcut. Clear Do_Range_Check flag on all paths. (Expand_N_Type_Conversion): Remove redundant test on Conversion_OK. Call Discrete_Range_Check to generate range checks on discrete types. Remove obsolete code for float-to-integer conversions. Add code to generate range checks for conversions involving fixed-point types.--- gcc/ada/checks.adb +++ gcc/ada/checks.adb @@ -3622,13 +3622,14 @@ package body Checks is -- will not be generated. if GNATprove_Mode -or else not Is_Fixed_Point_Type (Expr_Type) +or else (not Is_Fixed_Point_Type (Expr_Type) + and then not Is_Fixed_Point_Type (Target_Type)) then Apply_Scalar_Range_Check (Expr, Target_Type, Fixed_Int => Conv_OK); else - Set_Do_Range_Check (Expression (N), False); + Set_Do_Range_Check (Expr, False); end if; -- If the target type has predicates, we need to indicate --- gcc/ada/exp_attr.adb +++ gcc/ada/exp_attr.adb @@ -39,6 +39,7 @@ with Exp_Pakd; use Exp_Pakd; with Exp_Strm; use Exp_Strm; with Exp_Tss; use Exp_Tss; with Exp_Util; use Exp_Util; +with Expander; use Expander; with Freeze; use Freeze; with Gnatvsn; use Gnatvsn; with Itypes; use Itypes; @@ -3540,7 +3541,7 @@ package body Exp_Attr is -- We transform -- fixtype'Fixed_Value (integer-value) - -- inttype'Fixed_Value (fixed-value) + -- inttype'Integer_Value (fixed-value) -- into @@ -3549,75 +3550,30 @@ package body Exp_Attr is -- respectively. - -- We do all the required analysis of the conversion here, because we do - -- not want this to go through the fixed-point conversion circuits. Note - -- that the back end always treats fixed-point as equivalent to the - -- corresponding integer type anyway. - -- However, in order to remove the handling of Do_Range_Check from the - -- backend, we force the generation of a check on the result by - -- setting the result type appropriately. Apply_Conversion_Checks - -- will generate the required expansion. + -- We set Conversion_OK on the conversion because we do not want it + -- to go through the fixed-point conversion circuits. when Attribute_Fixed_Value | Attribute_Integer_Value => - Rewrite (N, - Make_Type_Conversion (Loc, - Subtype_Mark => New_Occurrence_Of (Entity (Pref), Loc), - Expression => Relocate_Node (First (Exprs; + Rewrite (N, OK_Convert_To (Entity (Pref), First (Exprs))); - -- Indicate that the result of the conversion may require a - -- range check (see below); - - Set_Etype (N, Base_Type (Entity (Pref))); - Set_Analyzed (N); - - -- Note: it might appear that a properly analyzed unchecked + -- Note that it might appear that a properly analyzed unchecked -- conversion would be just fine here, but that's not the case, - -- since the full range checks performed by the following code + -- since the full range checks performed by the following calls -- are critical. - -- Given that Fixed-point conversions are not further expanded - -- to prevent the involvement of real type operations we have to - -- construct two checks explicitly: one on the operand, and one - -- on the result. This used to be done in part in the back-end, - -- but for other targets (E.g. LLVM) it is preferable to create - -- the tests in full in the front-end. - - if Is_Fixed_Point_Type (Etype (N
[Ada] More complete information level for -gnatR4 output
This instructs -gnatR4 to also list the Etype of user-declared objects if it is compiler-generated, for example in: package P2 is Arr_V : array (1 .. 5) of Integer; end P2; Tested on x86_64-pc-linux-gnu, committed on trunk 2019-07-22 Eric Botcazou gcc/ada/ * repinfo.adb (List_Entities): Also list compiled-generated types present as Etype of objects.--- gcc/ada/repinfo.adb +++ gcc/ada/repinfo.adb @@ -563,6 +563,13 @@ package body Repinfo is E_Loop_Parameter, E_Variable) then + -- The type is relevant for an object + + if List_Representation_Info = 4 and then Is_Itype (Etype (E)) + then + Relevant_Entities.Set (Etype (E), True); + end if; + if List_Representation_Info >= 2 then List_Object_Info (E); end if;
[Ada] Fix missing check for no-op conversion to fixed-point type
This plugs a small loophole in the compiler for the case of a
multiplication or a division in a fixed-point type wrapped in a no-op
conversion, e.g. to the same fixed-point type.
The front-end fails to generate a range check for the operation. This
used to be caught by the back-end, which would generate the range check,
but this is no longer the case because we now make sure to reset the
Do_Range_Check flag in all cases before invoking the back-end.
Tested on x86_64-pc-linux-gnu, committed on trunk
2019-07-22 Eric Botcazou
gcc/ada/
* exp_ch4.adb (Expand_N_Type_Conversion): Beef up comment.
(Fixup_Universal_Fixed_Operation): Set the base type instead of
the type of the enclosing type conversion on the operation.
gcc/testsuite/
* gnat.dg/fixedpnt6.adb: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -12072,7 +12072,10 @@ package body Exp_Ch4 is
-- Check: are these rules stated in sinfo??? if so, why restate here???
-- The only remaining step is to generate a range check if we still have
- -- a type conversion at this stage and Do_Range_Check is set.
+ -- a type conversion at this stage and Do_Range_Check is set. Note that
+ -- we need to deal with at most 8 out of the 9 possible cases of numeric
+ -- conversions here, because the float-to-integer case is entirely dealt
+ -- with by Apply_Float_Conversion_Check.
if Nkind (N) = N_Type_Conversion
and then Do_Range_Check (Expression (N))
@@ -12726,13 +12729,13 @@ package body Exp_Ch4 is
if Nkind (Parent (Conv)) = N_Attribute_Reference
and then Attribute_Name (Parent (Conv)) = Name_Round
then
- Set_Etype (N, Etype (Parent (Conv)));
+ Set_Etype (N, Base_Type (Etype (Parent (Conv;
Set_Rounded_Result (N);
-- Normal case where type comes from conversion above us
else
- Set_Etype (N, Etype (Conv));
+ Set_Etype (N, Base_Type (Etype (Conv)));
end if;
end Fixup_Universal_Fixed_Operation;
--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/fixedpnt6.adb
@@ -0,0 +1,21 @@
+-- { dg-do run }
+-- { dg-options "-O0" }
+
+procedure Fixedpnt6 is
+
+ type T is delta 0.125 range -2.0 .. 1.875;
+
+ function Mult (A, B : T) return T is
+ begin
+return T (A * B);
+ end;
+
+ R : T;
+
+begin
+ R := Mult (T'Last, T'Last);
+ raise Program_Error;
+exception
+ when Constraint_Error =>
+ null;
+end;
Re: [PATCH][gcc] libgccjit: check result_type in gcc_jit_context_new_unary_op
Hi all,
second version of the patch here addressing comments.
make check-jit runs clean
Bests
Andrea
gcc/jit/ChangeLog
2019-07-18 Andrea Corallo
* jit-recording.c (unary_op_reproducer_strings): Make it extern.
(binary_op_reproducer_strings): Likewise.
* jit-recording.h (unary_op_reproducer_strings): Likewise.
(binary_op_reproducer_strings): Likewise.
* libgccjit.c (gcc_jit_context_new_unary_op): Check result_type to be a
numeric type.
* libgccjit.c (gcc_jit_context_new_binary_op): Improve error message.
gcc/testsuite/ChangeLog
2019-07-04 Andrea Corallo
* jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res-type.c:
New testcase.
* jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res-type.c:
Adjust error message.
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 495ac7f..2f75395 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -4888,7 +4888,7 @@ recording::unary_op::make_debug_string ()
m_a->get_debug_string ());
}
-static const char * const unary_op_reproducer_strings[] = {
+const char * const unary_op_reproducer_strings[] = {
"GCC_JIT_UNARY_OP_MINUS",
"GCC_JIT_UNARY_OP_BITWISE_NEGATE",
"GCC_JIT_UNARY_OP_LOGICAL_NEGATE",
@@ -4968,7 +4968,7 @@ recording::binary_op::make_debug_string ()
m_b->get_debug_string_parens (prec));
}
-static const char * const binary_op_reproducer_strings[] = {
+const char * const binary_op_reproducer_strings[] = {
"GCC_JIT_BINARY_OP_PLUS",
"GCC_JIT_BINARY_OP_MINUS",
"GCC_JIT_BINARY_OP_MULT",
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 13ec7ea..4bd346e 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -30,6 +30,9 @@ namespace gcc {
namespace jit {
+extern const char * const unary_op_reproducer_strings[];
+extern const char * const binary_op_reproducer_strings[];
+
class result;
class dump;
class reproducer;
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index 23e83e2..eec2f00 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1336,6 +1336,13 @@ gcc_jit_context_new_unary_op (gcc_jit_context *ctxt,
"unrecognized value for enum gcc_jit_unary_op: %i",
op);
RETURN_NULL_IF_FAIL (result_type, ctxt, loc, "NULL result_type");
+ RETURN_NULL_IF_FAIL_PRINTF3 (
+result_type->is_numeric (), ctxt, loc,
+"gcc_jit_unary_op %s with operand %s "
+"has non-numeric result_type: %s",
+gcc::jit::unary_op_reproducer_strings[op],
+rvalue->get_debug_string (),
+result_type->get_debug_string ());
RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
return (gcc_jit_rvalue *)ctxt->new_unary_op (loc, op, result_type, rvalue);
@@ -1387,9 +1394,10 @@ gcc_jit_context_new_binary_op (gcc_jit_context *ctxt,
b->get_type ()->get_debug_string ());
RETURN_NULL_IF_FAIL_PRINTF4 (
result_type->is_numeric (), ctxt, loc,
-"gcc_jit_binary_op %i with operands a: %s b: %s "
-"has non numeric result_type: %s",
-op, a->get_debug_string (), b->get_debug_string (),
+"gcc_jit_binary_op %s with operands a: %s b: %s "
+"has non-numeric result_type: %s",
+gcc::jit::binary_op_reproducer_strings[op],
+a->get_debug_string (), b->get_debug_string (),
result_type->get_debug_string ());
return (gcc_jit_rvalue *)ctxt->new_binary_op (loc, op, result_type, a, b);
diff --git a/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res-type.c b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res-type.c
index abadc9f..fbbb2e7 100644
--- a/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res-type.c
+++ b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res-type.c
@@ -35,7 +35,7 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
/* Verify that the correct error message was emitted. */
CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt),
- "gcc_jit_context_new_binary_op: gcc_jit_binary_op 1 with"
- " operands a: (int)1 b: (int)2 has non numeric "
- "result_type: void *");
+ "gcc_jit_context_new_binary_op: gcc_jit_binary_op "
+ "GCC_JIT_BINARY_OP_MINUS with operands a: "
+ "(int)1 b: (int)2 has non-numeric result_type: void *");
}
diff --git a/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res-type.c b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res-type.c
new file mode 100644
index 000..fae722a
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res-type.c
@@ -0,0 +1,38 @@
+#include
+#include
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+/* Try to create an unary operator with invalid result type. */
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ gcc_jit_type *int_type =
+gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_type *void_ptr_type =
+
Re: Handle strncpy in tree-ssa-dse.c
On 7/22/19 2:01 AM, Richard Biener wrote:
> On Fri, Jul 19, 2019 at 7:04 PM Jeff Law wrote:
>>
>>
>> While looking at BZ 80576 I realized a few things.
>>
>> First for STRNCPY we know the exact count of bytes written and we can
>> treat it just like MEMCPY and others, both in terms of removing/trimming
>> them and in terms of using them to allow removal of other stores.
>>
>> This patch adds support for those routines in DSE. We test that
>> subsequent statements can make those calls dead and vice versa and that
>> we can trim from the head or tail appropriately.
>>
>> While writing that code I also stumbled over a blob of code that I think
>> I copied from tree-ssa-alias.c that isn't necessary. In the relevant
>> code the byte count is always found in the same place. There's no need
>> to check the number of operands to the call to figure out where the
>> count would be. So that little blob of code is simplified ever so slightly.
>>
>> Finally, while writing the tests for strncpy I stumbled over a case that
>> we're still not handling well.
>>
>> In particular something like this:
>>
>>
>>
>> void h (char *s)
>> {
>> extern char a[8];
>> __builtin_memset (a, 0, sizeof a);
>> __builtin_strncpy (a, s, sizeof a);
>> frob (a);
>> }
>>
>> In this case ref_maybe_used_by_stmt_p returns true for the "a" array at
>> the strncpy call. AFAICT that appears to happen because "a" and "s"
>> could alias each other.
>>
>> strncpy is documented as not allowing overlap between the source and
>> destination objects. So in theory we could consider them not aliasing
>> for this call. I haven't implemented this, but I've got some ideas
>> here.
>
> But it does allow things like strncpy (&a[0], &a[n+1], n) for example?
Given that they're not allowed to overlap, I would think not. If that
were allowed then the code which aggressively transforms strncpy to
memcpy would need to be disabled (or at least throttled back) as well.
>
> I think this kind of specialities should be handled in
> ref_maybe_used_by_call_p_1
> directly, but I'm not exactly sure how - it's probably another case of
> a missing must-alias query, with that we could do
>
> /* If REF overlaps with the strncpy destination then the source argument
> may not alias it. */
> if (refs_must_alias_p (ref_for_strncpy_dest, ref))
> return false;
>
> hmm, OTOH for REF covering &a[n/2] to &a[3*n/2] (half overlapping
> source and destination) and the above strncpy we have a must-alias
> (not kill!) of REF but also are using it.
>
> So it's not so easy and would cover only very specific cases.
I'd been working under the assumption there would be nothing we could do
from a global standpoint in the alias oracle. Except for trivial
straightline functions, the call is always going to be in some kind of
control context. Thus the ability to exploit would be context dependent
on the control path leading to the call and not globally true.
With that in mind I was thinking that we could tackle in DSE.
Essentially asking if there's an overlap between the object we're
tracking for DSE and the destination of the str[n]cpy just before the
call to ref_maybe_used_by_stmt_p.
Obviously any such check has to avoid doing the wrong thing for memmove
and any other call where the source/destination are allowed to overlap.
That work would be a few patches deep in the queue of things I'm looking
at. First up is handling strcpy in tree-ssa-dse.c as well as
non-constant write sizes, neither of which are particularly complex.
Jeff
Re: [PATCH][gcc] libgccjit: check result_type in gcc_jit_context_new_unary_op
On Thu, 2019-07-18 at 14:20 +, Andrea Corallo wrote: > Hi all, > I've just realized that what we has been done recently for > gcc_jit_context_new_binary_op should be done also for the unary > version. > This patch checks at record time for the result type of > gcc_jit_context_new_unary_op to be numeric type plus add a testcase > for the new check. > > make check-jit runs clean > > Is it okay for trunk? Thanks - this is good for trunk. Dave
Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
On 7/16/19 2:37 PM, Andrew MacLeod wrote: On 7/9/19 5:56 AM, Richard Biener wrote: On Tue, Jul 9, 2019 at 9:28 AM Aldy Hernandez wrote: On 7/4/19 6:33 AM, Richard Biener wrote: On Wed, Jul 3, 2019 at 2:17 PM Aldy Hernandez wrote: On 7/3/19 7:08 AM, Richard Biener wrote: On Wed, Jul 3, 2019 at 11:19 AM Aldy Hernandez wrote: How about we keep VARYING and UNDEFINED typeless until right before we call into the ranger. At which point, we have can populate min/max because we have the tree_code and the type handy. So right before we call into the ranger do: if (varying_p ()) foo->set_varying(TYPE); This would avoid the type cache, and keep the ranger happy. you cannot do set_varying on the static const range but instead you'd do value_range tem (*foo); if (varying_p ()) tem->set_full_range (TYPE); which I think we already do in some places. Thus my question _where_ you actually need this. Basically, everywhere. By having a type for varying/undefined, we don't have to special case anything. Sure, we could for example, special case the invert operation for undefined / varying. And we could special case everything dealing with ranges to handle varying and undefined, but why? We could also pass a type argument everywhere, but that's just ugly. However, I do understand your objection to the type cache. How about the attached approach? Set the type for varying/undefined when we know it, while avoiding touching the CONST varying. Then right before calling the ranger, pass down a new varying node with min/max for any varyings that were still typeless until that point. I have taken care of never adding a set_varying() that was not already there. Would this keep the const happy? Technically we don't need to set varying/undef types for every instance in VRP, but we need it at least for the code that will be shared with range-ops (extract_range_from_multiplicative_op, union, intersect, etc). I just figured if we have the information, might as well set it for consistency. If you like this approach, I can rebase the other patches that depend on this one. OK, so I went ant checked what you do for class irange which has a type but no kind member (but constructors with a kind). It also uses wide_int members for storage. For a pure integer constant range representation this represents somewhat odd choices; I'd have elided the m_type member completely here, it seems fully redundant. Only range operations need to be carried out in a specific type (what I was suggesting above). Even the precision encoded in the wide_int members is redundant then (I'd have expected widest_int here and trailing-wide-ints for optimizing storage). What irange contains internally is a bit arbitrary. It's really an API for managing a set of subranges. We could have used trees internally just as easily, then we wouldnt need a type field. Since we were doing lots of operations, rather than going back and forth from trees all the time, we just used the underlying wide_int directly. we could have fiddle farted around with HOST_WIDE_INT or whatever, but wide_int is there, has all the operations, and it works fine. so thats what it currently is on the branch. We are treating a range object as a unique self contained object. Therefore, the range has a type so we know how to print it, and can confirm before any operation that the ranges being operated on are appropriately matched. We found and opened bugzillas over the past couple years for places where our code caught bugs because a range was created and then operated on in a way that was not compatible with another range. I think there is a still an open one against ada(?) where the switch and case are different precision. From my point of view, a range object is similar to a tree node. A tree node has the bits to indicate what the value is, but also associates a type with those bits within the same object. This is less error prone than passing around the bits and the type separately. As ranges are starting to be used in many places outside of VRP, we should do the same thing with ranges. WIth value_range it would actually be free since there is already a tree for the bounds already which contains the type. to fold_range/op_range? The API also seems to be oddly constrained to binary ops. Anyway, the way you build the operator table requires an awful lot of global C++ ctor invocations, sth we generally try to avoid. But I'm getting into too many details here. Its "oddly constrained" because what you are looking at is just the standardized unary/binary ops code.. ie the equivalent of extract_range_from_binary_expr() and extract_range_from_unary_expr(). The other ops we care about have specialized requirements, like PHIs and the arbitrary numbers of parameters in a call, or anything less common than one or two operands. You are just not seeing those parts. So - to answer your
Re: [PATCH v2] [rs6000] Add _mm_blend_epi16 and _mm_blendv_epi8
On Mon, Jul 22, 2019 at 08:28:33AM -0500, Bill Schmidt wrote: > On 7/22/19 12:58 AM, Segher Boessenkool wrote: > > On Sun, Jul 21, 2019 at 05:22:19PM -0500, Paul Clarke wrote: > >> Add compatibility implementations of _mm_blend_epi16 and _mm_blendv_epi8 > >> intrinsics. > >> > >> Respective test cases are copied almost verbatim (minor changes to > >> the dejagnu head lines) from i386. > >> > >> 2019-07-21 Paul A. Clarke > >> > >> [gcc] > >> > >>* config/rs6000/smmintrin.h (_mm_blend_epi16): New. > >>(_mm_blendv_epi8): New. > >> > >> [gcc/testsuite] > >> > >>* gcc.target/powerpc/sse4_1-check.h: New. > >>* gcc.target/powerpc/sse4_1-pblendvb.c: New. > >>* gcc.target/powerpc/sse4_1-pblendw.c: New. > >>* gcc.target/powerpc/sse4_1-pblendw-2.c: New. > >> > >> Tested on 64bit LE, 64bit and 32bit BE. > >> > >> v2: algorithm improvements as suggested by Segher. Note that > >> _mm_blend_epi16, > >> which now uses vec_gb, also requires the use of vec_unpackh to handle the > >> 16 bit elements. It also requires a vec_reve on big endian, due to the > >> endian > >> characteristics of vec_gb. Both are still much shorter. Thanks, Segher! > > Ah yes, I missed those "details". Glad to hear it still helps. > > > > Approved for trunk, please apply. Thanks! > > > > Do we need/want backports for this? > > IMHO, yes, it would be really nice to have this in GCC 9.2 at least. Approved for GCC 9, then. Thanks, Segher
Re: Handle strncpy in tree-ssa-dse.c
On 7/22/19 8:55 AM, Jeff Law wrote:
On 7/22/19 2:01 AM, Richard Biener wrote:
On Fri, Jul 19, 2019 at 7:04 PM Jeff Law wrote:
While looking at BZ 80576 I realized a few things.
First for STRNCPY we know the exact count of bytes written and we can
treat it just like MEMCPY and others, both in terms of removing/trimming
them and in terms of using them to allow removal of other stores.
This patch adds support for those routines in DSE. We test that
subsequent statements can make those calls dead and vice versa and that
we can trim from the head or tail appropriately.
While writing that code I also stumbled over a blob of code that I think
I copied from tree-ssa-alias.c that isn't necessary. In the relevant
code the byte count is always found in the same place. There's no need
to check the number of operands to the call to figure out where the
count would be. So that little blob of code is simplified ever so slightly.
Finally, while writing the tests for strncpy I stumbled over a case that
we're still not handling well.
In particular something like this:
void h (char *s)
{
extern char a[8];
__builtin_memset (a, 0, sizeof a);
__builtin_strncpy (a, s, sizeof a);
frob (a);
}
In this case ref_maybe_used_by_stmt_p returns true for the "a" array at
the strncpy call. AFAICT that appears to happen because "a" and "s"
could alias each other.
strncpy is documented as not allowing overlap between the source and
destination objects. So in theory we could consider them not aliasing
for this call. I haven't implemented this, but I've got some ideas
here.
But it does allow things like strncpy (&a[0], &a[n+1], n) for example?
Given that they're not allowed to overlap, I would think not. If that
were allowed then the code which aggressively transforms strncpy to
memcpy would need to be disabled (or at least throttled back) as well.
I think there's some (maybe too much) room for interpretation here
as to exactly what the sentence
If copying takes place between objects that overlap, the behavior
is undefined.
means. Taken to an extreme, one might say that the following
violates the requirement:
char a[4] = "123\0";
strcpy (a, a + 2);
because the call copies bytes within the same object (the array a)
which inevitably overlaps itself. But I'm pretty sure that's not
the intended interpretation -- the object itself does overlap but
not the bytes that are copied. (This is also the view -Wrestrict
takes.) If it were undefined, then so would be the following:
memcpy (a, a + 2, 2);
With that in mind, I would be inclined to expect the following not
to violate the requirement either:
strncpy (a, a + 2, 4);
because the bytes that are copied do not overlap. With a set to
"123\0" as done above it's equivalent to:
memcpy (a, a + 2, 2);
memset (a + 2, 0, 2);
Admittedly, the examples aren't exactly the same which makes this
question interesting. Is it worth raising an interpretation request
with WG14?
Martin
I think this kind of specialities should be handled in
ref_maybe_used_by_call_p_1
directly, but I'm not exactly sure how - it's probably another case of
a missing must-alias query, with that we could do
/* If REF overlaps with the strncpy destination then the source argument
may not alias it. */
if (refs_must_alias_p (ref_for_strncpy_dest, ref))
return false;
hmm, OTOH for REF covering &a[n/2] to &a[3*n/2] (half overlapping
source and destination) and the above strncpy we have a must-alias
(not kill!) of REF but also are using it.
So it's not so easy and would cover only very specific cases.
I'd been working under the assumption there would be nothing we could do
from a global standpoint in the alias oracle. Except for trivial
straightline functions, the call is always going to be in some kind of
control context. Thus the ability to exploit would be context dependent
on the control path leading to the call and not globally true.
With that in mind I was thinking that we could tackle in DSE.
Essentially asking if there's an overlap between the object we're
tracking for DSE and the destination of the str[n]cpy just before the
call to ref_maybe_used_by_stmt_p.
Obviously any such check has to avoid doing the wrong thing for memmove
and any other call where the source/destination are allowed to overlap.
That work would be a few patches deep in the queue of things I'm looking
at. First up is handling strcpy in tree-ssa-dse.c as well as
non-constant write sizes, neither of which are particularly complex.
Jeff
Re: [PATCH][gcc] libgccjit: check result_type in gcc_jit_context_new_unary_op
David Malcolm writes: > On Thu, 2019-07-18 at 14:20 +, Andrea Corallo wrote: >> Hi all, >> I've just realized that what we has been done recently for >> gcc_jit_context_new_binary_op should be done also for the unary >> version. >> This patch checks at record time for the result type of >> gcc_jit_context_new_unary_op to be numeric type plus add a testcase >> for the new check. >> >> make check-jit runs clean >> >> Is it okay for trunk? > > Thanks - this is good for trunk. > > Dave Thanks, last version committed as r273700 Bests Andrea
Re: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops
Hi Maciej, I'm glad to hear from you again! > I think that should be a GAS warning really (similarly to macros that > expand to multiple instructions in a delay slot) as people ought to be > allowed to do what they wish, and then `-Werror' can be used for code > quality enforcement (and possibly disabled on a case-by-case basis). How can one reasonably prevent the bug when compiling a whole Linux distribution with thousands of packages if GAS merely warns and proceeds in many cases? > > [ In theory, GAS could actually insert NOPs prior to the noreorder > > directive, > > to make the loop longer that six instructions, but GAS does not have that > > kind of capability. Another option that GCC prevents is to place a NOP in > > the delay slot. ] > > Well, GAS does have that capability, although of course it is not enabled > for `noreorder' code. Hmm? I'm unable to make sense of that, since such NOPs are unaffected by noreorder. This is what I meant: loop: addiu $5,$5,1 addiu $4,$4,1 lb $2,-1($5) nop /* These two NOPs would prevent the */ nop /* bug while preserving noreorder. */ .setnoreorder .setnomacro bne $2,$3,loop sb $2,-1($4) .setmacro .setreorder > For generated code I think however that usually it > will be cheaper performance-wise if a non-trivial delay-slot instruction > is never produced in the affected cases (i.e. a dummy NOP is always used). I suppose so too. One observation is that R5900 BogoMIPS went down from 584 to 195 when switching from the generic kernel variant .setnoreorder 1: bneza0,1b addiu a0,a0,-1 .setreorder that is undefined for R5900 in arch/mips/lib/delay.c, to a special variant beqza0,2f 1: addiu a0,a0,-1 bneza0,1b 2: for the R5900 where GAS will place a NOP in the branch delay slot. With loop unrolling BogoMIPS could be inflated again, of course. In general, unrolling is a bit better for the R5900 than general MIPS. Perhaps GCC could be informed about that, possibly via profile-guided optimisation. > > A reasonable fix for GCC is perhaps to update gcc/config/mips/mips.md to not > > make explicit use of the branch delay slot, as suggested by the patch below? > > Then GCC will emit > > > > loop: addiu $5,$5,1 > > addiu $4,$4,1 > > lb $2,-1($5) > > sb $2,-1($4) > > bne $2,$3,loop > > > > that GAS will adjust in the ELF object to > > > >4: 24a50001addiu a1,a1,1 > >8: 24840001addiu a0,a0,1 > >c: 80a2lb v0,-1(a1) > > 10: a082sb v0,-1(a0) > > 14: 1443fffbbne v0,v1,4 > > 18: nop > > > > where a NOP is placed in the delay slot to avoid the bug. For longer loops, > > this relies on GAS making appropriate use of the delay slot. I'm not sure if > > GAS is good at that, though? > > I'm sort-of surprised that GCC has produced `reorder' code here, making > it rely on GAS for delay slot scheduling. Have you used an unusual set of > options that prevents GCC from making `noreorder' code, which as I recall > is the usual default (not for the MIPS16 mode IIRC, as well as some > obscure corner cases)? I used the suggested patch, and recompiled the kernel with it, and verified with the machine code tool that there were no undefined loops. Wouldn't that be enough? > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md > > index e17b1d522f0..acd31a8960c 100644 > > --- a/gcc/config/mips/mips.md > > +++ b/gcc/config/mips/mips.md > > @@ -1091,6 +1091,7 @@ > > > > (define_delay (and (eq_attr "type" "branch") > >(not (match_test "TARGET_MIPS16")) > > + (not (match_test "TARGET_FIX_R5900")) > >(eq_attr "branch_likely" "yes")) > >[(eq_attr "can_delay" "yes") > > (nil) > > @@ -1100,6 +1101,7 @@ > > ;; not annul on false. > > (define_delay (and (eq_attr "type" "branch") > >(not (match_test "TARGET_MIPS16")) > > + (not (match_test "TARGET_FIX_R5900")) > >(ior (match_test "TARGET_CB_NEVER") > > (and (eq_attr "compact_form" "maybe") > > (not (match_test "TARGET_CB_ALWAYS"))) > > > > I think you need to modify the default `can_delay' attribute definition > instead (in the same way). I tried to limit the case to branch delays only, which is a rough approximation. Call and jump delay slots are acceptable. Are you referring to this piece? ;; Can the instruction be put into a delay slot? (define_attr "can_delay" "no,yes" (if_then_else (and (eq_attr "type" "!branch,call,jump") (eq_attr "hazard" "none") (match_test "get_attr_insn_count (insn) == 1")) (const_string
[PATCH, fortran, arm] Fix PR 78314 on arm and aarch64
This patch fixes PR libfortran/78314, the failure of
gfortran.dg/ieee/ieee_6.f90 on some Arm and Aarch64 platforms.
As mentioned in the PR trapping fpu exceptions is optional
on ARM and this function cannot do a runtime check for support
so we should return 0.
There are a couple of discussion strings about this issue,
pointers to them are in the PR. This patch has already been
suggested by a couple of people (Uros and Christophe) but
never been checked in. Can we go ahead and check it in?
Tested on Aarch64 (ThunderX) with no regressions.
Steve Ellcey
sell...@marvell.com
2019-07-22 Steve Ellcey
PR libfortran/78314
* config/fpu-glibc.h (support_fpu_trap): Return 0 for Arm/Aarch64.
diff --git a/libgfortran/config/fpu-glibc.h b/libgfortran/config/fpu-glibc.h
index df2588e..692091c 100644
--- a/libgfortran/config/fpu-glibc.h
+++ b/libgfortran/config/fpu-glibc.h
@@ -129,7 +129,11 @@ get_fpu_trap_exceptions (void)
int
support_fpu_trap (int flag)
{
+#if defined(__arm__) || defined(__aarch64__)
+ return 0;
+#else
return support_fpu_flag (flag);
+#endif
}
[PATCH][ARM] Cleanup DImode shifts
Like the logical operations, expand all shifts early rather than only
sometimes. The Neon shift expansions are never emitted (not even with
-fneon-for-64bits), so they are not useful. So all the late expansions
and Neon shift patterns can be removed, and shifts are more optimized
as a result. Since some extend patterns use Neon DImode shifts, remove
the Neon extend variants and related splits.
A simple example (relying on [1]) generates the same efficient code after
this patch with -mfpu=neon and -mfpu=vfp (previously just the fact of
having Neon enabled resulted inefficient code for no reason).
unsigned long long f(unsigned long long x, unsigned long long y)
{ return x & (y >> 33); }
Before:
strdr4, r5, [sp, #-8]!
lsr r4, r3, #1
mov r5, #0
and r1, r1, r5
and r0, r0, r4
ldrdr4, r5, [sp]
add sp, sp, #8
bx lr
After:
and r0, r0, r3, lsr #1
mov r1, #0
bx lr
Bootstrap and regress OK on arm-none-linux-gnueabihf --with-cpu=cortex-a57
[1] https://gcc.gnu.org/ml/gcc-patches/2019-07/msg01301.html
ChangeLog:
2019-07-19 Wilco Dijkstra
* config/arm/iterators.md (qhs_extenddi_cstr): Update.
(qhs_extenddi_cstr): Likewise.
* config/arm/arm.md (ashldi3): Always expand early.
(ashlsi3): Likewise.
(ashrsi3): Likewise.
(zero_extenddi2): Remove Neon variants.
(extenddi2): Likewise.
* config/arm/neon.md (ashldi3_neon_noclobber): Remove.
(signed_shift_di3_neon): Likewise.
(unsigned_shift_di3_neon): Likewise.
(ashrdi3_neon_imm_noclobber): Likewise.
(lshrdi3_neon_imm_noclobber): Likewise.
(di3_neon): Likewise.
(split extend): Remove DI extend split patterns.
testsuite/
* gcc.target/arm/neon-extend-1.c: Remove test.
* gcc.target/arm/neon-extend-2.c: Remove test.
---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index
0dba97a4ebeed0c2133936ca662f1c9e86ffc6ba..10ed70dac4384354c0a2453c5e51a29108c6c062
100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -3601,44 +3601,14 @@ (define_insn "*satsi__shift"
(define_expand "ashldi3"
[(set (match_operand:DI0 "s_register_operand")
(ashift:DI (match_operand:DI 1 "s_register_operand")
- (match_operand:SI 2 "general_operand")))]
+ (match_operand:SI 2 "reg_or_int_operand")))]
"TARGET_32BIT"
"
- if (TARGET_NEON)
-{
- /* Delay the decision whether to use NEON or core-regs until
-register allocation. */
- emit_insn (gen_ashldi3_neon (operands[0], operands[1], operands[2]));
- DONE;
-}
- else
-{
- /* Only the NEON case can handle in-memory shift counts. */
- if (!reg_or_int_operand (operands[2], SImode))
-operands[2] = force_reg (SImode, operands[2]);
-}
-
- if (!CONST_INT_P (operands[2]) && TARGET_REALLY_IWMMXT)
-; /* No special preparation statements; expand pattern as above. */
- else
-{
- rtx scratch1, scratch2;
-
- /* Ideally we should use iwmmxt here if we could know that operands[1]
- ends up already living in an iwmmxt register. Otherwise it's
- cheaper to have the alternate code being generated than moving
- values to iwmmxt regs and back. */
-
- /* Expand operation using core-registers.
-'FAIL' would achieve the same thing, but this is a bit smarter. */
- scratch1 = gen_reg_rtx (SImode);
- scratch2 = gen_reg_rtx (SImode);
- arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
-operands[2], scratch1, scratch2);
- DONE;
-}
- "
-)
+ arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
+operands[2], gen_reg_rtx (SImode),
+gen_reg_rtx (SImode));
+ DONE;
+")
(define_expand "ashlsi3"
[(set (match_operand:SI0 "s_register_operand")
@@ -3661,35 +3631,11 @@ (define_expand "ashrdi3"
(match_operand:SI 2 "reg_or_int_operand")))]
"TARGET_32BIT"
"
- if (TARGET_NEON)
-{
- /* Delay the decision whether to use NEON or core-regs until
-register allocation. */
- emit_insn (gen_ashrdi3_neon (operands[0], operands[1], operands[2]));
- DONE;
-}
-
- if (!CONST_INT_P (operands[2]) && TARGET_REALLY_IWMMXT)
-; /* No special preparation statements; expand pattern as above. */
- else
-{
- rtx scratch1, scratch2;
-
- /* Ideally we should use iwmmxt here if we could know that operands[1]
- ends up already living in an iwmmxt register. Otherwise it's
- cheaper to have the alternate code being generated than moving
- values to iwmmxt regs and back. */
-
- /* Expand operation using core-registers.
-'FAIL' would achieve the same thing, but this is a bit
[PATCH][ARM] Remove remaining Neon DImode support
Remove the remaining Neon adddi3, subdi3 and negdi2 patterns. As a result
adddi3, subdi3 and negdi2 can now always be expanded early irrespectively of
whether Neon is available. Also expand the extenddi patterns at the same
time. Several Neon arch attributes are no longer used and removed.
Code generation is improved in all cases, saving another 400-500 instructions
from the PR77308 testcase (total improvement is over 1700 instructions with
-mcpu=cortex-a57 -O2).
Bootstrap & regress OK on arm-none-linux-gnueabihf --with-cpu=cortex-a57
ChangeLog:
2019-07-19 Wilco Dijkstra
* config/arm/arm.md (neon_for_64bits): Remove.
(avoid_neon_for_64bits): Remove.
(arm_adddi3): Always split early.
(arm_subdi3): Always split early.
(negdi2): Remove Neon expansion.
(split zero_extend): Split before reload.
(split sign_extend): Split before reload.
---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index
10ed70dac4384354c0a2453c5e51a29108c6c062..6d8a5a54997caee0e6956f01018cb5300a9a07e1
100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -125,7 +125,7 @@ (define_attr "length" ""
; arm_arch6. "v6t2" for Thumb-2 with arm_arch6 and "v8mb" for ARMv8-M
; Baseline. This attribute is used to compute attribute "enabled",
; use type "any" to enable an alternative in all cases.
-(define_attr "arch"
"any,a,t,32,t1,t2,v6,nov6,v6t2,v8mb,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3,neon"
+(define_attr "arch"
"any,a,t,32,t1,t2,v6,nov6,v6t2,v8mb,iwmmxt,iwmmxt2,armv6_or_vfpv3,neon"
(const_string "any"))
(define_attr "arch_enabled" "no,yes"
@@ -168,16 +168,6 @@ (define_attr "arch_enabled" "no,yes"
(match_test "TARGET_THUMB1 && arm_arch8"))
(const_string "yes")
-(and (eq_attr "arch" "avoid_neon_for_64bits")
- (match_test "TARGET_NEON")
- (not (match_test "TARGET_PREFER_NEON_64BITS")))
-(const_string "yes")
-
-(and (eq_attr "arch" "neon_for_64bits")
- (match_test "TARGET_NEON")
- (match_test "TARGET_PREFER_NEON_64BITS"))
-(const_string "yes")
-
(and (eq_attr "arch" "iwmmxt2")
(match_test "TARGET_REALLY_IWMMXT2"))
(const_string "yes")
@@ -450,13 +440,8 @@ (define_expand "adddi3"
(clobber (reg:CC CC_REGNUM))])]
"TARGET_EITHER"
"
- if (TARGET_THUMB1)
-{
- if (!REG_P (operands[1]))
-operands[1] = force_reg (DImode, operands[1]);
- if (!REG_P (operands[2]))
-operands[2] = force_reg (DImode, operands[2]);
- }
+ if (TARGET_THUMB1 && !REG_P (operands[2]))
+operands[2] = force_reg (DImode, operands[2]);
"
)
@@ -465,9 +450,9 @@ (define_insn_and_split "*arm_adddi3"
(plus:DI (match_operand:DI 1 "arm_general_register_operand" "%0, 0, r,
0, r")
(match_operand:DI 2 "arm_general_adddi_operand""r, 0, r,
Dd, Dd")))
(clobber (reg:CC CC_REGNUM))]
- "TARGET_32BIT && !TARGET_NEON"
+ "TARGET_32BIT"
"#"
- "TARGET_32BIT && ((!TARGET_NEON && !TARGET_IWMMXT) || reload_completed)"
+ "TARGET_32BIT"
[(parallel [(set (reg:CC_C CC_REGNUM)
(compare:CC_C (plus:SI (match_dup 1) (match_dup 2))
(match_dup 1)))
@@ -1290,24 +1275,16 @@ (define_expand "subdi3"
(clobber (reg:CC CC_REGNUM))])]
"TARGET_EITHER"
"
- if (TARGET_THUMB1)
-{
- if (!REG_P (operands[1]))
-operands[1] = force_reg (DImode, operands[1]);
- if (!REG_P (operands[2]))
-operands[2] = force_reg (DImode, operands[2]);
- }
- "
-)
+")
(define_insn_and_split "*arm_subdi3"
[(set (match_operand:DI 0 "arm_general_register_operand"
"=&r,&r,&r")
(minus:DI (match_operand:DI 1 "arm_general_register_operand" "0,r,0")
(match_operand:DI 2 "arm_general_register_operand" "r,0,0")))
(clobber (reg:CC CC_REGNUM))]
- "TARGET_32BIT && !TARGET_NEON"
+ "TARGET_32BIT"
"#" ; "subs\\t%Q0, %Q1, %Q2\;sbc\\t%R0, %R1, %R2"
- "&& (!TARGET_IWMMXT || reload_completed)"
+ "TARGET_32BIT"
[(parallel [(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 1) (match_dup 2)))
(set (match_dup 0) (minus:SI (match_dup 1) (match_dup 2)))])
@@ -4164,13 +4141,6 @@ (define_expand "negdi2"
(neg:DI (match_operand:DI 1 "s_register_operand")))
(clobber (reg:CC CC_REGNUM))])]
"TARGET_EITHER"
- {
-if (TARGET_NEON)
- {
-emit_insn (gen_negdi2_neon (operands[0], operands[1]));
- DONE;
- }
- }
)
;; The constraints here are to prevent a *partial* overlap (where %Q0 == %R1).
@@ -4182,7 +4152,7 @@ (define_insn_and_split "*negdi2_insn"
"TARGET_32BIT"
"#" ; rsbs %Q0, %Q1, #0; rsc %R0, %R1, #0 (ARM)
; negs %Q0, %Q1; sbc %R0, %R1, %R1, lsl #1 (Thumb-2)
- "&& reload_completed"
+ "TARGET_32BIT"
[(parallel [(set (reg:CC CC_REGNUM)
(compare
Re: [patch][aarch64]: add usra and ssra combine patterns
On Mon, Jun 17, 2019 at 05:42:45PM +0100, Sylvia Taylor wrote: > Updating patch with missing scan-assembler checks. This is OK. I committed it on your behalf as r273703. Thanks, James > Cheers, > Syl > > -Original Message- > From: Sylvia Taylor > Sent: 04 June 2019 12:24 > To: James Greenhalgh > Cc: Richard Earnshaw ; Marcus Shawcroft > ; gcc-patches@gcc.gnu.org; nd > Subject: RE: [patch][aarch64]: add usra and ssra combine patterns > > Hi James, > > I've managed to remove the odd redundant git diff change. > > Regarding aarch64_sra_n, this patch shouldn't affect it. > > I am also not aware of any way of enabling this combine inside the pattern > used for those intrinsics, so I kept them separate. > > Cheers, > Syl > > -Original Message- > From: James Greenhalgh > Sent: 03 June 2019 11:20 > To: Sylvia Taylor > Cc: Richard Earnshaw ; Marcus Shawcroft > ; gcc-patches@gcc.gnu.org; nd > Subject: Re: [patch][aarch64]: add usra and ssra combine patterns > > On Thu, May 30, 2019 at 03:25:19PM +0100, Sylvia Taylor wrote: > > Greetings, > > > > This patch adds support to combine: > > > > 1) ushr and add into usra, example: > > > > ushrv0.16b, v0.16b, 2 > > add v0.16b, v0.16b, v2.16b > > --- > > usrav2.16b, v0.16b, 2 > > > > 2) sshr and add into ssra, example: > > > > sshrv1.16b, v1.16b, 2 > > add v1.16b, v1.16b, v3.16b > > --- > > ssrav3.16b, v1.16b, 2 > > > > Bootstrapped and tested on aarch64-none-linux-gnu. > > > > Ok for trunk? If yes, I don't have any commit rights, so can someone > > please commit it on my behalf. > > This patch has an unrelated change to > aarch64_get_lane_zero_extend Please revert that and > resend. > > What changes (if any) should we make to aarch64_sra_n based on > this patch, and to the vsra_n intrinsics in arm_neon.h ? > > Thanks, > James > > > > > Cheers, > > Syl > > > > gcc/ChangeLog: > > > > 2019-05-30 Sylvia Taylor > > > > * config/aarch64/aarch64-simd.md > > (*aarch64_simd_sra): New. > > * config/aarch64/iterators.md > > (SHIFTRT): New iterator. > > (sra_op): New attribute. > > > > gcc/testsuite/ChangeLog: > > > > 2019-05-30 Sylvia Taylor > > > > * gcc.target/aarch64/simd/ssra.c: New test. > > * gcc.target/aarch64/simd/usra.c: New test. >
Re: [PATCH][ARM] Cleanup DImode shifts
On 22/07/2019 17:16, Wilco Dijkstra wrote:
Like the logical operations, expand all shifts early rather than only
sometimes. The Neon shift expansions are never emitted (not even with
-fneon-for-64bits), so they are not useful. So all the late expansions
and Neon shift patterns can be removed, and shifts are more optimized
as a result. Since some extend patterns use Neon DImode shifts, remove
the Neon extend variants and related splits.
A simple example (relying on [1]) generates the same efficient code after
this patch with -mfpu=neon and -mfpu=vfp (previously just the fact of
having Neon enabled resulted inefficient code for no reason).
unsigned long long f(unsigned long long x, unsigned long long y)
{ return x & (y >> 33); }
Before:
strdr4, r5, [sp, #-8]!
lsr r4, r3, #1
mov r5, #0
and r1, r1, r5
and r0, r0, r4
ldrdr4, r5, [sp]
add sp, sp, #8
bx lr
After:
and r0, r0, r3, lsr #1
mov r1, #0
bx lr
Bootstrap and regress OK on arm-none-linux-gnueabihf --with-cpu=cortex-a57
[1] https://gcc.gnu.org/ml/gcc-patches/2019-07/msg01301.html
Thanks for this patch set - What I'm missing in this is any analysis as
to what's the impact on code generation for neon intrinsics that use
uint64_t ? Especially things like v_u64 ?
Ramana
ChangeLog:
2019-07-19 Wilco Dijkstra
* config/arm/iterators.md (qhs_extenddi_cstr): Update.
(qhs_extenddi_cstr): Likewise.
* config/arm/arm.md (ashldi3): Always expand early.
(ashlsi3): Likewise.
(ashrsi3): Likewise.
(zero_extenddi2): Remove Neon variants.
(extenddi2): Likewise.
* config/arm/neon.md (ashldi3_neon_noclobber): Remove.
(signed_shift_di3_neon): Likewise.
(unsigned_shift_di3_neon): Likewise.
(ashrdi3_neon_imm_noclobber): Likewise.
(lshrdi3_neon_imm_noclobber): Likewise.
(di3_neon): Likewise.
(split extend): Remove DI extend split patterns.
testsuite/
* gcc.target/arm/neon-extend-1.c: Remove test.
* gcc.target/arm/neon-extend-2.c: Remove test.
---
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index
0dba97a4ebeed0c2133936ca662f1c9e86ffc6ba..10ed70dac4384354c0a2453c5e51a29108c6c062
100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -3601,44 +3601,14 @@ (define_insn "*satsi__shift"
(define_expand "ashldi3"
[(set (match_operand:DI0 "s_register_operand")
(ashift:DI (match_operand:DI 1 "s_register_operand")
- (match_operand:SI 2 "general_operand")))]
+ (match_operand:SI 2 "reg_or_int_operand")))]
"TARGET_32BIT"
"
- if (TARGET_NEON)
-{
- /* Delay the decision whether to use NEON or core-regs until
-register allocation. */
- emit_insn (gen_ashldi3_neon (operands[0], operands[1], operands[2]));
- DONE;
-}
- else
-{
- /* Only the NEON case can handle in-memory shift counts. */
- if (!reg_or_int_operand (operands[2], SImode))
-operands[2] = force_reg (SImode, operands[2]);
-}
-
- if (!CONST_INT_P (operands[2]) && TARGET_REALLY_IWMMXT)
-; /* No special preparation statements; expand pattern as above. */
- else
-{
- rtx scratch1, scratch2;
-
- /* Ideally we should use iwmmxt here if we could know that operands[1]
- ends up already living in an iwmmxt register. Otherwise it's
- cheaper to have the alternate code being generated than moving
- values to iwmmxt regs and back. */
-
- /* Expand operation using core-registers.
-'FAIL' would achieve the same thing, but this is a bit smarter. */
- scratch1 = gen_reg_rtx (SImode);
- scratch2 = gen_reg_rtx (SImode);
- arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
-operands[2], scratch1, scratch2);
- DONE;
-}
- "
-)
+ arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
+operands[2], gen_reg_rtx (SImode),
+gen_reg_rtx (SImode));
+ DONE;
+")
(define_expand "ashlsi3"
[(set (match_operand:SI0 "s_register_operand")
@@ -3661,35 +3631,11 @@ (define_expand "ashrdi3"
(match_operand:SI 2 "reg_or_int_operand")))]
"TARGET_32BIT"
"
- if (TARGET_NEON)
-{
- /* Delay the decision whether to use NEON or core-regs until
-register allocation. */
- emit_insn (gen_ashrdi3_neon (operands[0], operands[1], operands[2]));
- DONE;
-}
-
- if (!CONST_INT_P (operands[2]) && TARGET_REALLY_IWMMXT)
-; /* No special preparation statements; expand pattern as above. */
- else
-{
- rtx scratch1, scratch2;
-
- /* Ideally we should use iwmmxt here if we could know that operands[1]
- ends up already living in
Re: [PATCH 1/3] add -Wstruct-not-pod, -Wclass-is-pod, -Wmismatched-tags (PR 61339)
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00622.html On 7/8/19 3:58 PM, Martin Sebor wrote: The attached patch implements three new warnings: * -Wstruct-not-pod triggers for struct definitions that are not POD structs, * -Wclass-is-pod triggers for class definitions that satisfy the requirements on POD structs, and * -Wmismatched-tags that triggers for class and struct declarations with class-key that doesn't match either their definition or the first declaration (if no definition is provided). The implementation of -Wclass-is-pod and -Wstruct-not-pod is fairly straightforward but the -Wmismatched-tags solution is slightly unusual. It collects struct and class declarations first and diagnoses mismatches only after the whole tramslation unit has been processed. This is so that the definition of a class can guide which declarations to diagnose no matter which come first. Martin
[PATCH] Use GCC_PICFLAG to collect host-specific PICFLAG from ../config/picflag.m4
The gcc configure script does not use the config/picflag.m4 macro to customize PICFLAG according to the host when using --enable-host-shared. Fix configure.ac to do so. Tested bootstrap on x86_64-linux-gnu. 2019-07-22 Arvind Sankar * gcc/configure.ac: Use GCC_PICFLAG. --- gcc/configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/configure.ac b/gcc/configure.ac index c620dd2f447..f6bdfd52fa6 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -6543,7 +6543,7 @@ fi AC_ARG_ENABLE(host-shared, [AS_HELP_STRING([--enable-host-shared], [build host code as shared libraries])], -[PICFLAG=-fPIC], [PICFLAG=]) +[GCC_PICFLAG], [PICFLAG=]) AC_SUBST(enable_host_shared) AC_SUBST(PICFLAG) -- 2.21.0
Re: [patch][aarch64]: fix unrecognizable insn for ldr got in ilp32 tiny
On 18/06/2019 14:50, Sylvia Taylor wrote: Hi Wilco, Combined them into one pattern. Updated the diff and the changelog is now: gcc/ChangeLog: 2019-06-18 Sylvia Taylor * config/aarch64/aarch64.c (aarch64_load_symref_appropriately): Change SYMBOL_TINY_GOT. * config/aarch64/aarch64.md (ldr_got_tiny_): New pattern. (ldr_got_tiny_sidi): New pattern. Cheers, Syl -Original Message- From: Wilco Dijkstra Sent: 13 June 2019 18:42 To: Sylvia Taylor Cc: nd ; GCC Patches ; Richard Earnshaw ; James Greenhalgh Subject: Re: [patch][aarch64]: fix unrecognizable insn for ldr got in ilp32 tiny Hi Sylvia, -(define_insn "ldr_got_tiny" +(define_insn "ldr_got_tiny_di" [(set (match_operand:DI 0 "register_operand" "=r") - (unspec:DI [(match_operand:DI 1 "aarch64_valid_symref" "S")] - UNSPEC_GOTTINYPIC))] + (unspec:DI + [(match_operand:DI 1 "aarch64_valid_symref" "S")] + UNSPEC_GOTTINYPIC))] "" "ldr\\t%0, %L1" [(set_attr "type" "load_8")] ) +(define_insn "ldr_got_tiny_si" + [(set (match_operand:SI 0 "register_operand" "=r") + (unspec:SI + [(match_operand:SI 1 "aarch64_valid_symref" "S")] + UNSPEC_GOTTINYPIC))] + "TARGET_ILP32" + "ldr\\t%0, %L1" + [(set_attr "type" "load_4")] +) These can be easily combined like the related ldr_got_small_. Wilco -Original Message- From: Sylvia Taylor Sent: 11 June 2019 14:25 To: Richard Earnshaw ; James Greenhalgh ; Marcus Shawcroft ; gcc-patches@gcc.gnu.org Cc: nd Subject: [patch][aarch64]: fix unrecognizable insn for ldr got in ilp32 tiny Greetings, This patch addresses a bug in ldr GOT for mcmodel=tiny in which this instruction is not generated for ilp32 modes. Defined 2 new patterns for ldr_got_tiny. Added additional checks to use the appropriate rtl pattern for any of the modes. Examples of previously unrecognized instructions: ldrx1, :got:_ZTIi// [c=4 l=4] ldr_got_tiny_si ldrx0, :got:global // [c=4 l=4] ldr_got_tiny_sidi Bootstrapped and tested on aarch64-none-linux-gnu. Bug fix tested with aarch64-none-elf-g++ -mabi=ilp32 -mcmodel=tiny -fpic. The existing test now fixed is: testsuite/g++.dg/torture/stackalign/throw-1.C Ok for trunk? If yes, I don't have any commit rights, so can someone please commit it on my behalf. Cheers, Syl gcc/ChangeLog: 2019-06-11 Sylvia Taylor * config/aarch64/aarch64.c (aarch64_load_symref_appropriately): Change SYMBOL_TINY_GOT. * config/aarch64/aarch64.md (ldr_got_tiny): Change to ldr_got_tiny_di. (ldr_got_tiny_si): New pattern. (ldr_got_tiny_sidi): New pattern. [...] > +(define_insn "ldr_got_tiny_" If you change the above to: (define_insn "@ldr_got_tiny_" > + [(set (match_operand:PTR 0 "register_operand" "=r") > + (unspec:PTR > +[(match_operand:PTR 1 "aarch64_valid_symref" "S")] > + UNSPEC_GOTTINYPIC))] then > + if (mode == DImode) > +emit_insn (gen_ldr_got_tiny_di (dest, imm)); > + else > +/* TARGET_ILP32. */ > +emit_insn (gen_ldr_got_tiny_si (dest, imm)); can be simplified to emit_insn (gen_ldr_got_tiny (mode, dest, imm)); What's more, the compiler will abort if mode turns out (for some obscure reason) to be one we don't have a pattern for. See Parameterized Names in the gcc manual for details R.
Re: [PATCH] Change std::ceil2 to be undefined if the result can't be represented
On 25/06/19 10:40 +0100, Jonathan Wakely wrote:
* include/std/bit (__ceil2): Make unrepresentable results undefined,
as per P1355R2. Add debug assertion. Perform one left shift, not two,
so that out of range values cause undefined behaviour. Ensure that
shift will still be undefined if left operand is promoted.
* testsuite/26_numerics/bit/bit.pow.two/ceil2.cc: Replace checks for
unrepresentable values with checks that they are not core constant
expressions.
* testsuite/26_numerics/bit/bit.pow.two/ceil2_neg.cc: New test.
I'm not committing this yet, because P1355 hasn't been accepted into
the draft, but here's a patch to implement it (this reverses the
changes in r263986, and adds special handling for types that undergo
integer promotion).
This has now been committed to trunk.
I'll backport it to gcc-9-branch soon too.
The goal is that undefined shifts are detectable in three ways, even
if the type is promoted:
* In constant expressions they make the program ill-formed.
* At runtime they cause UBSan errors.
* At runtime they abort when _GLIBCXX_ASSERTIONS is defined.
commit fd8d9b7898083c8806d2cd300f78739d2afc3503
Author: Jonathan Wakely
Date: Fri Jun 14 13:32:39 2019 +0100
Change std::ceil2 to be undefined if the result can't be represented
* include/std/bit (__ceil2): Make unrepresentable results undefined,
as per P1355R2. Add debug assertion. Perform one left shift, not two,
so that out of range values cause undefined behaviour. Ensure that
shift will still be undefined if left operand is promoted.
* testsuite/26_numerics/bit/bit.pow.two/ceil2.cc: Replace checks for
unrepresentable values with checks that they are not core constant
expressions.
* testsuite/26_numerics/bit/bit.pow.two/ceil2_neg.cc: New test.
diff --git a/libstdc++-v3/include/std/bit b/libstdc++-v3/include/std/bit
index e0c53e53756..eb0a7578b8d 100644
--- a/libstdc++-v3/include/std/bit
+++ b/libstdc++-v3/include/std/bit
@@ -197,9 +197,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr auto _Nd = numeric_limits<_Tp>::digits;
if (__x == 0 || __x == 1)
return 1;
- const unsigned __n = _Nd - std::__countl_zero((_Tp)(__x - 1u));
- const _Tp __y_2 = (_Tp)1u << (__n - 1u);
- return __y_2 << 1u;
+ auto __shift_exponent = _Nd - std::__countl_zero((_Tp)(__x - 1u));
+ // If the shift exponent equals _Nd then the correct result is not
+ // representable as a value of _Tp, and so the result is undefined.
+ // Want that undefined behaviour to be detected in constant expressions,
+ // by UBSan, and by debug assertions.
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+ if (!__builtin_is_constant_evaluated())
+ __glibcxx_assert( __shift_exponent != numeric_limits<_Tp>::digits );
+#endif
+ using __promoted_type = decltype(__x << 1);
+ if _GLIBCXX17_CONSTEXPR (!is_same<__promoted_type, _Tp>::value)
+ {
+ // If __x undergoes integral promotion then shifting by _Nd is
+ // not undefined. In order to make the shift undefined, so that
+ // it is diagnosed in constant expressions and by UBsan, we also
+ // need to "promote" the shift exponent to be too large for the
+ // promoted type.
+ const int __extra_exp = sizeof(__promoted_type) / sizeof(_Tp) / 2;
+ __shift_exponent |= (__shift_exponent & _Nd) << __extra_exp;
+ }
+ return (_Tp)1u << __shift_exponent;
}
template
diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bit.pow.two/ceil2.cc
b/libstdc++-v3/testsuite/26_numerics/bit/bit.pow.two/ceil2.cc
index 6ffb5f70edb..788c008129e 100644
--- a/libstdc++-v3/testsuite/26_numerics/bit/bit.pow.two/ceil2.cc
+++ b/libstdc++-v3/testsuite/26_numerics/bit/bit.pow.two/ceil2.cc
@@ -20,6 +20,21 @@
#include
+template
+ constexpr T max = std::numeric_limits::max();
+// Largest representable power of two (i.e. has most significant bit set)
+template
+ constexpr T maxpow2 = T(1) << (std::numeric_limits::digits - 1);
+
+// Detect whether std::ceil2(N) is a constant expression.
+template
+ struct ceil2_valid
+ : std::false_type { };
+
+template
+ struct ceil2_valid>
+ : std::true_type { };
+
template
constexpr auto
test(UInt x)
@@ -55,13 +70,18 @@ test(UInt x)
static_assert( std::ceil2(UInt(3) << 64) == (UInt(4) << 64) );
}
- constexpr UInt msb = UInt(1) << (std::numeric_limits::digits - 1);
+ constexpr UInt msb = maxpow2;
+ static_assert( ceil2_valid() );
static_assert( std::ceil2( msb ) == msb );
- // Larger values cannot be represented so the return value is unspecified,
- // but must still be valid in constant expressions, i.e. not undefined.
- static_assert( std::ceil2( UInt(msb + 1) ) != 77 );
- static_assert( std::ceil2( UInt(msb + 2) ) != 77 );
- static_assert( std::ceil2( UInt(msb + 77) ) != 77 );
+ static_assert(
[PATCH] Adjust std::rotl, std::rotr etc to match final P0553R4 proposal
This proposal has now been accepted for C++20, with a few changes. This
patch adjusts std::rotl and std::rotr to match the final specification
and declares the additions for C++2a mode even when __STRICT_ANSI__ is
defined.
* include/std/bit (__rotl, __rotr): Change second parameter from
unsigned int to int and handle negative values.
(rotl, rotr): Remove check for __STRICT_ANSI__. Change second
parameter from unsigned int to int. Add nodiscard attribute.
* testsuite/26_numerics/bit/bitops.rot/rotl.cc: Rename to ...
* testsuite/26_numerics/bit/bit.rotate/rotl.cc: Here. Test negative
shifts.
* testsuite/26_numerics/bit/bitops.rot/rotr.cc: Rename to ...
* testsuite/26_numerics/bit/bit.rotate/rotr.cc: Here. Test negative
shifts.
Tested x86_64-linux, committed to trunk.
I'll backport this to gcc-9-branch too.
commit b58d3908b4fcf83343f93bba8c4f5e49e982b894
Author: redi
Date: Mon Jul 22 16:53:36 2019 +
Adjust std::rotl, std::rotr etc to match final P0553R4 proposal
This proposal has now been accepted for C++20, with a few changes. This
patch adjusts std::rotl and std::rotr to match the final specification
and declares the additions for C++2a mode even when __STRICT_ANSI__ is
defined.
* include/std/bit (__rotl, __rotr): Change second parameter from
unsigned int to int and handle negative values.
(rotl, rotr): Remove check for __STRICT_ANSI__. Change second
parameter from unsigned int to int. Add nodiscard attribute.
* testsuite/26_numerics/bit/bitops.rot/rotl.cc: Rename to ...
* testsuite/26_numerics/bit/bit.rotate/rotl.cc: Here. Test negative
shifts.
* testsuite/26_numerics/bit/bitops.rot/rotr.cc: Rename to ...
* testsuite/26_numerics/bit/bit.rotate/rotr.cc: Here. Test negative
shifts.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@273706
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/libstdc++-v3/include/std/bit b/libstdc++-v3/include/std/bit
index d019b1ee600..f17d2f1bd59 100644
--- a/libstdc++-v3/include/std/bit
+++ b/libstdc++-v3/include/std/bit
@@ -42,20 +42,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
constexpr _Tp
-__rotl(_Tp __x, unsigned int __s) noexcept
+__rotl(_Tp __x, int __s) noexcept
{
constexpr auto _Nd = numeric_limits<_Tp>::digits;
- const unsigned __sN = __s % _Nd;
- return (__x << __sN) | (__x >> ((_Nd - __sN) % _Nd));
+ const int __r = __s % _Nd;
+ if (__r == 0)
+ return __x;
+ else if (__r > 0)
+ return (__x << __r) | (__x >> ((_Nd - __r) % _Nd));
+ else
+ return (__x >> -__r) | (__x << ((_Nd + __r) % _Nd)); // rotr(x, -r)
}
template
constexpr _Tp
-__rotr(_Tp __x, unsigned int __s) noexcept
+__rotr(_Tp __x, int __s) noexcept
{
constexpr auto _Nd = numeric_limits<_Tp>::digits;
- const unsigned __sN = __s % _Nd;
- return (__x >> __sN) | (__x << ((_Nd - __sN) % _Nd));
+ const int __r = __s % _Nd;
+ if (__r == 0)
+ return __x;
+ else if (__r > 0)
+ return (__x >> __r) | (__x << ((_Nd - __r) % _Nd));
+ else
+ return (__x << -__r) | (__x >> ((_Nd + __r) % _Nd)); // rotl(x, -r)
}
template
@@ -244,20 +254,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using _If_is_unsigned_integer
= enable_if_t<__is_unsigned_integer<_Tp>::value, _Up>;
-#if ! __STRICT_ANSI__
- // [bitops.rot], rotating
+ // [bit.rot], rotating
template
-constexpr _If_is_unsigned_integer<_Tp>
-rotl(_Tp __x, unsigned int __s) noexcept
+[[nodiscard]] constexpr _If_is_unsigned_integer<_Tp>
+rotl(_Tp __x, int __s) noexcept
{ return std::__rotl(__x, __s); }
template
-constexpr _If_is_unsigned_integer<_Tp>
-rotr(_Tp __x, unsigned int __s) noexcept
+[[nodiscard]] constexpr _If_is_unsigned_integer<_Tp>
+rotr(_Tp __x, int __s) noexcept
{ return std::__rotr(__x, __s); }
- // [bitops.count], counting
+ // [bit.count], counting
template
constexpr _If_is_unsigned_integer<_Tp, int>
@@ -283,9 +292,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr _If_is_unsigned_integer<_Tp, int>
popcount(_Tp __x) noexcept
{ return std::__popcount(__x); }
-#endif
- // Integral power-of-two operations
+ // [bit.pow.two], integral powers of 2
template
constexpr _If_is_unsigned_integer<_Tp, bool>
diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.rot/rotl.cc
b/libstdc++-v3/testsuite/26_numerics/bit/bit.rotate/rotl.cc
similarity index 89%
rename from libstdc++-v3/testsuite/26_numerics/bit/bitops.rot/rotl.cc
rename to libstdc++-v3/testsuite/26_numerics/bit/bit.rotate/rotl.cc
+++ b/libstdc++-v3/testsuite/26_numerics/bit/bit.rotate/rotl.cc
@@ -20,12 +20,27 @@
#include
+template
+constexpr bool
+test_negative_shifts()
+{
+ constexpr
Re: [PATCH] Adjust std::rotl, std::rotr etc to match final P0553R4 proposal
On 22/07/19 17:55 +0100, Jonathan Wakely wrote: This proposal has now been accepted for C++20, with a few changes. This patch adjusts std::rotl and std::rotr to match the final specification and declares the additions for C++2a mode even when __STRICT_ANSI__ is defined. * include/std/bit (__rotl, __rotr): Change second parameter from unsigned int to int and handle negative values. (rotl, rotr): Remove check for __STRICT_ANSI__. Change second parameter from unsigned int to int. Add nodiscard attribute. * testsuite/26_numerics/bit/bitops.rot/rotl.cc: Rename to ... * testsuite/26_numerics/bit/bit.rotate/rotl.cc: Here. Test negative shifts. * testsuite/26_numerics/bit/bitops.rot/rotr.cc: Rename to ... * testsuite/26_numerics/bit/bit.rotate/rotr.cc: Here. Test negative shifts. This patch applies some more testsuite renaming. Tested x86_64-linux, committed to trunk. I'll backport this to gcc-9-branch too. commit e29d5f9fb11aa5ba05503e66d7d54080ad474be2 Author: Jonathan Wakely Date: Mon Jul 22 17:57:10 2019 +0100 Rename testsuite directory to match P0553R4 stable names * testsuite/26_numerics/bit/bitops.count/*: Rename to ... * testsuite/26_numerics/bit/bit.count/*: Here. diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countl_one.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countl_one.cc similarity index 100% rename from libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countl_one.cc rename to libstdc++-v3/testsuite/26_numerics/bit/bit.count/countl_one.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countl_zero.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countl_zero.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countl_zero.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countl_zero.cc similarity index 100% rename from libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countl_zero.cc rename to libstdc++-v3/testsuite/26_numerics/bit/bit.count/countl_zero.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_one.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_one.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_one.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_one.cc similarity index 100% rename from libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_one.cc rename to libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_one.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_zero.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_zero.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_zero.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_zero.cc similarity index 100% rename from libstdc++-v3/testsuite/26_numerics/bit/bitops.count/countr_zero.cc rename to libstdc++-v3/testsuite/26_numerics/bit/bit.count/countr_zero.cc diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bitops.count/popcount.cc b/libstdc++-v3/testsuite/26_numerics/bit/bit.count/popcount.cc
Re: [PATCH][ARM] Cleanup DImode shifts
Hi Ramana,
> Thanks for this patch set - What I'm missing in this is any analysis as
> to what's the impact on code generation for neon intrinsics that use
> uint64_t ? Especially things like v_u64 ?
Well things like this continue to work exactly like before:
uint64x1_t f20(uint64x1_t x, uint64x1_t y)
{
return vshl_u64 (x, y);
}
uint64x1_t f21(uint64x1_t x)
{
return vshl_n_u64 (x, 10);
}
f20:
vmovd16, r0, r1 @ int
vmovd17, r2, r3 @ int
vshl.u64d16, d16, d17
vmovr0, r1, d16 @ int
bx lr
f21:
vmovd16, r0, r1 @ int
vshl.i64d16, d16, #10
vmovr0, r1, d16 @ int
bx lr
As you can see there is a problem with the uint64x1_t type which for a strange
reason maps to DImode, so avoiding Neon here would avoid lots of moves...
The vadd_u64 variant emits the right code already:
uint64x1_t f22(uint64x1_t x, uint64x1_t y)
{
return vadd_u64 (x, y);
}
f22:
addsr0, r0, r2
adc r1, r1, r3
bx lr
Wilco
Re: [PATCH, GCC, AArch64] Enable Transactional Memory Extension
On Wed, Jul 10, 2019 at 07:55:42PM +0100, Sudakshina Das wrote:
> Hi
>
> This patch enables the new Transactional Memory Extension announced
> recently as part of Arm's new architecture technologies.
> We introduce a new optional extension "tme" to enable this. The
> following instructions are part of the extension:
> * tstart
> * ttest
> * tcommit
> * tcancel #
> The documentation for the above can be found here:
> https://developer.arm.com/docs/ddi0602/latest/base-instructions-alphabetic-order
>
> We have also added ACLE intrinsics for the instructions above according to:
> https://developer.arm.com/docs/101028/latest/transactional-memory-extension-tme-intrinsics
>
> Builds and regression tested on aarch64-none-linux-gnu and added new
> tests for the new instructions.
>
> Is this okay for trunk?
This looks good to me.
OK for trunk.
Thanks,
James
>
> Thanks
> Sudi
>
> *** gcc/ChangeLog ***
>
> 2019-xx-xx Sudakshina Das
>
> * config/aarch64/aarch64-builtins.c (enum aarch64_builtins): Add
> AARCH64_TME_BUILTIN_TSTART, AARCH64_TME_BUILTIN_TCOMMIT,
> AARCH64_TME_BUILTIN_TTEST and AARCH64_TME_BUILTIN_TCANCEL.
> (aarch64_init_tme_builtins): New.
> (aarch64_init_builtins): Call aarch64_init_tme_builtins.
> (aarch64_expand_builtin_tme): New.
> (aarch64_expand_builtin): Handle TME builtins.
> * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Define
> __ARM_FEATURE_TME when enabled.
> * config/aarch64/aarch64-option-extensions.def: Add "tme".
> * config/aarch64/aarch64.h (AARCH64_FL_TME, AARCH64_ISA_TME): New.
> (TARGET_TME): New.
> * config/aarch64/aarch64.md (define_c_enum "unspec"): Add UNSPEC_TTEST.
> (define_c_enum "unspecv"): Add UNSPECV_TSTART, UNSPECV_TCOMMIT and
> UNSPECV_TCANCEL.
> (tstart, ttest, tcommit, tcancel): New instructions.
> * config/aarch64/arm_acle.h (__tstart, __tcommit): New.
> (__tcancel, __ttest): New.
> (_TMFAILURE_REASON, _TMFAILURE_RTRY, _TMFAILURE_CNCL): New macro.
> (_TMFAILURE_MEM, _TMFAILURE_IMP, _TMFAILURE_ERR): Likewise.
> (_TMFAILURE_SIZE, _TMFAILURE_NEST, _TMFAILURE_DBG): Likewise.
> (_TMFAILURE_INT, _TMFAILURE_TRIVIAL): Likewise.
> * config/arm/types.md: Add new tme type attr.
> * doc/invoke.texi: Document "tme".
>
> *** gcc/testsuite/ChangeLog ***
>
> 2019-xx-xx Sudakshina Das
>
> * gcc.target/aarch64/acle/tme.c: New test.
> * gcc.target/aarch64/pragma_cpp_predefs_2.c: New test.
> diff --git a/gcc/config/aarch64/aarch64-builtins.c
> b/gcc/config/aarch64/aarch64-builtins.c
> index
> 549a6c249243372eacb5d29923b5d1abce4ac79a..16c1d42ea2be0f477692be592e30ba8ce27f05a7
> 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -438,6 +438,11 @@ enum aarch64_builtins
>/* Special cased Armv8.3-A Complex FMA by Lane quad Builtins. */
>AARCH64_SIMD_FCMLA_LANEQ_BUILTIN_BASE,
>AARCH64_SIMD_FCMLA_LANEQ_BUILTINS
> + /* TME builtins. */
> + AARCH64_TME_BUILTIN_TSTART,
> + AARCH64_TME_BUILTIN_TCOMMIT,
> + AARCH64_TME_BUILTIN_TTEST,
> + AARCH64_TME_BUILTIN_TCANCEL,
>AARCH64_BUILTIN_MAX
> };
>
> @@ -1067,6 +1072,35 @@ aarch64_init_pauth_hint_builtins (void)
> NULL_TREE);
> }
>
> +/* Initialize the transactional memory extension (TME) builtins. */
> +static void
> +aarch64_init_tme_builtins (void)
> +{
> + tree ftype_uint64_void
> += build_function_type_list (uint64_type_node, NULL);
> + tree ftype_void_void
> += build_function_type_list (void_type_node, NULL);
> + tree ftype_void_uint64
> += build_function_type_list (void_type_node, uint64_type_node, NULL);
> +
> + aarch64_builtin_decls[AARCH64_TME_BUILTIN_TSTART]
> += add_builtin_function ("__builtin_aarch64_tstart", ftype_uint64_void,
> + AARCH64_TME_BUILTIN_TSTART, BUILT_IN_MD,
> + NULL, NULL_TREE);
> + aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
> += add_builtin_function ("__builtin_aarch64_ttest", ftype_uint64_void,
> + AARCH64_TME_BUILTIN_TTEST, BUILT_IN_MD,
> + NULL, NULL_TREE);
> + aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
> += add_builtin_function ("__builtin_aarch64_tcommit", ftype_void_void,
> + AARCH64_TME_BUILTIN_TCOMMIT, BUILT_IN_MD,
> + NULL, NULL_TREE);
> + aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
> += add_builtin_function ("__builtin_aarch64_tcancel", ftype_void_uint64,
> + AARCH64_TME_BUILTIN_TCANCEL, BUILT_IN_MD,
> + NULL, NULL_TREE);
> +}
> +
> void
> aarch64_init_builtins (void)
> {
> @@ -1104,6 +1138,9 @@ aarch64_init_builtins (void)
> register them. */
>if (!TARGET_ILP32)
> aarch64_init_pauth_hint_builtins ();
> +
> + if (TARGET_TME)
> +aarch
Re: [PATCH v2] [rs6000] Add _mm_blend_epi16 and _mm_blendv_epi8
On 7/22/19 10:38 AM, Segher Boessenkool wrote: > On Mon, Jul 22, 2019 at 08:28:33AM -0500, Bill Schmidt wrote: >> On 7/22/19 12:58 AM, Segher Boessenkool wrote: >>> On Sun, Jul 21, 2019 at 05:22:19PM -0500, Paul Clarke wrote: Add compatibility implementations of _mm_blend_epi16 and _mm_blendv_epi8 intrinsics. >>> Approved for trunk, please apply. Thanks! >>> >>> Do we need/want backports for this? >> >> IMHO, yes, it would be really nice to have this in GCC 9.2 at least. > > Approved for GCC 9, then. Thanks, Thanks, and as we discussed offline, I'll push to GCC 8 as well. PC
Re: Ping: [PATCH] x86/AVX512: improve generated code for mask-to-vector-register conversions
On 7/18/19 10:07 AM, Jan Beulich wrote:
On 27.06.19 at 10:59, wrote:
>> Conversion of comparison results to full vectors does, when VPMOVM2* are
>> unavailable, not require any intermediate VMOVDQ{A,U}*: Simply use
>> embedded masking on VPTERNLOG* right away, which is available with
>> AVX512F (while VPMOVM2{D,Q} are available only with AVX512DQ).
>>
>> Note that the chosen immediate is only one of many possible ones; I was
>> trying to make the insn here distinguishable from the pre-existing uses
>> of vpternlog.
>>
>> gcc/
>> 2019-06-27 Jan Beulich
>>
>> * config/i386/sse.md (_cvtmask2):
>> Require only AVX512F.
>> (*_cvtmask2): Likewise. Add
>> alternative expanding to vpternlog.
OK
jeff
Re: Implement more rtx vector folds on variable-length vectors
On 7/15/19 9:30 AM, Richard Sandiford wrote: > Richard Sandiford writes: >> This patch extends the tree-level folding of variable-length vectors >> so that it can also be used on rtxes. The first step is to move >> the tree_vector_builder new_unary/binary_operator routines to the >> parent vector_builder class (which in turn means adding a new >> template parameter). The second step is to make simplify-rtx.c >> use a direct rtx analogue of the VECTOR_CST handling in fold-const.c. >> >> Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu. >> OK to install? >> >> Richard > > Here's a version updated for the earlier patch, so that we take > both HONOR_NANS and HONOR_SNANS into account. Tested on > aarch64-linux-gnu to far. > > Thanks, > Richard > > > 2019-07-15 Richard Sandiford > > gcc/ > * rtl.h (bit_and_conditions, bit_ior_conditions): Declare. > * jump.c (flags_to_condition): Add an honor_nans_p argument. > (bit_ior_conditions, bit_and_conditions): New functions. > * simplify-rtx.c (simplify_binary_operation_1): Try to fold an > AND or IOR of two comparisons into a single comparison. > (simplify_ternary_operation): Try to fold an if_then_else involving > two conditions into an AND of two conditions. > (test_merged_comparisons): New function. > (simplify_rtx_c_tests): Call it. This is fine once the prereqs are approved. jeff
Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
On Fri, 2019-07-19 at 19:24 +0100, Richard Sandiford wrote:
>
> You can probably also remove:
>
> tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
> ...
> TREE_TYPE (node->decl) = new_type;
>
> in simd_clone_adjust_argument_types.
>
> I'm happy doing it this way or doing the copy in the AArch64 hook.
> It's really Jakub's call.
You are right, that is no longer needed with the current patch. I
removed it and retested with no regressions. Jakub, do you have
any preference? I have attached a new version of the patch to this
email.
> I don't think the tests need:
>
> /* { dg-require-effective-target aarch64_variant_pcs } */
>
> since they're only dg-do compile. Leaving the line out would get more
> coverage for people using older binutils.
>
> The tests are OK with that change, thanks.
OK, I made that change to the tests.
Latest version of the patch:
2019-07-22 Steve Ellcey
* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
build_distinct_type_copy.
(simd_clone_adjust_argument_types): Ditto.
(simd_clone_adjust): Call build_distinct_type_copy here.
(expand_simd_clones): Ditto.
2019-07-22 Steve Ellcey
* gcc.target/aarch64/simd_pcs_attribute.c: New test.
* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index caa8da3cba5..da01d9b8e6c 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -498,7 +498,6 @@ simd_clone_adjust_return_type (struct cgraph_node *node)
/* Adjust the function return type. */
if (orig_rettype == void_type_node)
return NULL_TREE;
- TREE_TYPE (fndecl) = build_distinct_type_copy (TREE_TYPE (fndecl));
t = TREE_TYPE (TREE_TYPE (fndecl));
if (INTEGRAL_TYPE_P (t) || POINTER_TYPE_P (t))
veclen = node->simdclone->vecsize_int;
@@ -724,11 +723,6 @@ simd_clone_adjust_argument_types (struct cgraph_node *node)
else
new_reversed = void_list_node;
}
-
- tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
- TYPE_ARG_TYPES (new_type) = new_reversed;
- TREE_TYPE (node->decl) = new_type;
-
adjustments.release ();
}
args.release ();
@@ -1164,6 +1158,7 @@ simd_clone_adjust (struct cgraph_node *node)
{
push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+ TREE_TYPE (node->decl) = build_distinct_type_copy (TREE_TYPE (node->decl));
targetm.simd_clone.adjust (node);
tree retval = simd_clone_adjust_return_type (node);
@@ -1737,6 +1732,8 @@ expand_simd_clones (struct cgraph_node *node)
simd_clone_adjust (n);
else
{
+ TREE_TYPE (n->decl)
+ = build_distinct_type_copy (TREE_TYPE (n->decl));
targetm.simd_clone.adjust (n);
simd_clone_adjust_return_type (n);
simd_clone_adjust_argument_types (n);
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
index e69de29bb2d..8eec6824f37 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double foo (double x);
+
+void bar(double * f, int n)
+{
+ int i;
+ for (i = 0; i < n; i++)
+ f[i] = foo(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
index e69de29bb2d..95f6a6803e8 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+double foo (double x);
+
+void bar(double *f, int n)
+{
+ int i;
+ for (i = 0; i < n; i++)
+ f[i] = foo(f[i]);
+}
+
+double foo(double x)
+{
+ return x * x / 3.0;
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM2v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
index e69de29bb2d..eddcef3597c 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
Re: Generalise VEC_DUPLICATE folding for variable-length vectors
On 7/11/19 2:06 AM, Richard Sandiford wrote: > This patch uses the constant vector encoding scheme to handle > more cases of a VEC_DUPLICATE of another vector. Duplicating > any fixed-length vector is fine, and duplicating a variable-length > vector is OK as long as that vector is also a duplicate of a > fixed-length sequence. > > Other cases fell through to: > > if (VECTOR_MODE_P (mode) && GET_CODE (op) == CONST_VECTOR) > > which was only expecting to deal with elementwise operations. > > Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu. > OK to install? > > Richard > > > 2019-07-11 Richard Sandiford > > gcc/ > * simplify-rtx.c (simplify_const_unary_operation): Fold a > VEC_DUPLICATE of a fixed-length vector even if the result > is variable-length. Likewise fold a duplicate of a > variable-length vector if the variable-length vector is > itself a duplicate of a fixed-length sequence. > (test_vector_ops_duplicate): Test more cases. OK jeff
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Sat, Jul 20, 2019 at 12:28:34PM -0400, David Edelsohn wrote: > This patch needs to add rs6000-internal.h to tm_file in gcc/config.gcc > for all of the powerpc/rs6000 targets. It also may need tm_p_file and > tm_d_file definitions. While I agree it would make things easier if the declarations are available to everybody, why wasn't this an issue when rs6000-internal.h was created to allow stuff to move out from rs6000.c to rs6000-logues.c? -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Sun, Jul 21, 2019 at 12:41:51PM -0500, Segher Boessenkool wrote: > On Sat, Jul 20, 2019 at 12:13:08PM -0400, Michael Meissner wrote: > > I will be iterating on patch #9 and sending out a replacement shortly. > > > > This is patch #10. It moves the various data structures from rs6000.c to > > I'll review this tomorrow, fwiw. > > A general request: please don't send patches as replies to other emails. In the past you had asked me to group patches under a single overview patch. I can go back to sending patches individually. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Mon, Jul 22, 2019 at 12:42:13AM -0500, Segher Boessenkool wrote: > Hi! > > On Sat, Jul 20, 2019 at 12:13:08PM -0400, Michael Meissner wrote: > > 2019-07-20 Michael Meissner > > > > * config/rs6000/rs6000-internal.h (rs6000_hard_regno_mode_ok_p): > > Move various declarations relating to addressing and register > > allocation to rs6000-internal.h from rs6000.c so that in the > > future we can move things out of rs6000.c. > > Just say > (rs6000_hard_regno_mode_ok_p): New declaration. > for the things that only had a definition before. > > > Make the static arrays global, > > That's not this entry. Say that in the entries where it applies. > > > and define them in rs6000.c. > > Say that in the corresponding entry for rs6000.c . > > > (enum rs6000_reg_type): Likewise. > > This one always was a declaration. This was a mechanical patch, just moving the swath of code from rs6000.c to rs6000-internal.h. > (... ten gazillion "Likewise." ...) > Most of those are *not* the same thing. Don't say "likewise" if not > the same comment applies. > > If it is hard to write a proper changelog, your patch series probably > could use some restructuring. Or sometimes the changelog you need just > is more work than you would prefer. > > You don't necessarily have to keep the same order in the changelog as > in the patch, if that helps. But roughly the same order helps review, > so please consider that too ;-) Well yes, while in general I would prefer to group things logically together for ChangeLog, I don't tend to do it because as you say it makes it easier for the review. > We haven't had GO_IF_LEGITIMATE_ADDRESS for ten years now, please don't > introduce new references to it ;-) Yep. > > +#define RELOAD_REG_VALID 0x0001 /* Mode valid in register.. */ > > +#define RELOAD_REG_MULTIPLE0x0002 /* Mode takes multiple > > registers. */ > > +#define RELOAD_REG_INDEXED 0x0004 /* Reg+reg addressing. */ > > +#define RELOAD_REG_OFFSET 0x0008 /* Reg+offset addressing. */ > > +#define RELOAD_REG_PRE_INCDEC 0x0010 /* PRE_INC/PRE_DEC valid. */ > > +#define RELOAD_REG_PRE_MODIFY 0x0020 /* PRE_MODIFY valid. */ > > +#define RELOAD_REG_AND_M16 0x0040 /* AND -16 addressing. */ > > +#define RELOAD_REG_QUAD_OFFSET 0x0080 /* quad offset is limited. */ > > Why all the extra zeroes? If you introduce some 0x100 later, just leave > the 0x80 as 0x80 please, that is much more readable. As I mentioned, I will be adding at least one new bit (RELOAD_REG_DS_OFFSET), but I have potential patches that add a few more (those are still in flux), and those would need the extra column. I personally DO NOT find mask definitions like: #define RELOAD_REG_VALID0x01/* Mode valid in register.. */ #define RELOAD_REG_MULTIPLE 0x02/* Mode takes multiple registers. */ #define RELOAD_REG_INDEXED 0x04/* Reg+reg addressing. */ #define RELOAD_REG_OFFSET 0x08/* Reg+offset addressing. */ #define RELOAD_REG_PRE_INCDEC 0x10/* PRE_INC/PRE_DEC valid. */ #define RELOAD_REG_PRE_MODIFY 0x20/* PRE_MODIFY valid. */ #define RELOAD_REG_AND_M16 0x40/* AND -16 addressing. */ #define RELOAD_REG_QUAD_OFFSET 0x80/* quad offset is limited. */ #define RELOAD_REG_DS_OFFSET0x100 /* bottom 2 bits must be zero. */ to be readable. So in change, I can go back to just: #define RELOAD_REG_VALID0x01/* Mode valid in register.. */ #define RELOAD_REG_MULTIPLE 0x02/* Mode takes multiple registers. */ #define RELOAD_REG_INDEXED 0x04/* Reg+reg addressing. */ #define RELOAD_REG_OFFSET 0x08/* Reg+offset addressing. */ #define RELOAD_REG_PRE_INCDEC 0x10/* PRE_INC/PRE_DEC valid. */ #define RELOAD_REG_PRE_MODIFY 0x20/* PRE_MODIFY valid. */ #define RELOAD_REG_AND_M16 0x40/* AND -16 addressing. */ #define RELOAD_REG_QUAD_OFFSET 0x80/* quad offset is limited. */ And in the next patch, change it all to: #define RELOAD_REG_VALID0x001 /* Mode valid in register.. */ #define RELOAD_REG_MULTIPLE 0x002 /* Mode takes multiple registers. */ #define RELOAD_REG_INDEXED 0x004 /* Reg+reg addressing. */ #define RELOAD_REG_OFFSET 0x008 /* Reg+offset addressing. */ #define RELOAD_REG_PRE_INCDEC 0x010 /* PRE_INC/PRE_DEC valid. */ #define RELOAD_REG_PRE_MODIFY 0x020 /* PRE_MODIFY valid. */ #define RELOAD_REG_AND_M16 0x040 /* AND -16 addressing. */ #define RELOAD_REG_QUAD_OFFSET 0x080 /* quad offset is limited. */ #define RELOAD_REG_DS_OFFSET0x100 /* bottom 2 bits must be 0. */ Some time later if I continue with the current code, that would be changed to: #define RELOAD_REG_VALID0x0001 /* Mode valid in register.. */ #define RELOAD_REG_MULTIPLE 0x0002 /* Mode takes multiple registers. */ #define RELOAD_REG_INDEXED 0x0004 /* Reg+reg addressing. */ #define RELOAD_REG_OFFSET 0x0008 /* Reg+offset
Re: [ARM/FDPIC v5 17/21] [ARM][testsuite] FDPIC: Handle *-*-uclinux*
On Jul 19, 2019, at 1:56 AM, Kyrill Tkachov wrote:
>
> On 5/15/19 1:39 PM, Christophe Lyon wrote:
>> Add *-*-uclinux* to tests that work on this target.
>>
>> 2019-XX-XX Christophe Lyon
>>
>> gcc/testsuite/
>> * g++.dg/abi/forced.C: Add *-*-uclinux*.
>> * g++.dg/abi/guard2.C: Likewise.
>> * g++.dg/ext/cleanup-10.C: Likewise.
>> * g++.dg/ext/cleanup-11.C: Likewise.
>> * g++.dg/ext/cleanup-8.C: Likewise.
>> * g++.dg/ext/cleanup-9.C: Likewise.
>> * g++.dg/ext/sync-4.C: Likewise.
>> * g++.dg/ipa/comdat.C: Likewise.
>> * gcc.dg/20041106-1.c: Likewise.
>> * gcc.dg/cleanup-10.c: Likewise.
>> * gcc.dg/cleanup-11.c: Likewise.
>> * gcc.dg/cleanup-8.c: Likewise.
>> * gcc.dg/cleanup-9.c: Likewise.
>> * gcc.dg/fdata-sections-1.c: Likewise.
>> * gcc.dg/fdata-sections-2.c: Likewise.
>> * gcc.dg/pr39323-1.c: Likewise.
>> * gcc.dg/pr39323-2.c: Likewise.
>> * gcc.dg/pr39323-3.c: Likewise.
>> * gcc.dg/pr65780-1.c: Likewise.
>> * gcc.dg/pr65780-2.c: Likewise.
>> * gcc.dg/pr67338.c: Likewise.
>> * gcc.dg/pr78185.c: Likewise.
>> * gcc.dg/pr83100-1.c: Likewise.
>> * gcc.dg/pr83100-4.c: Likewise.
>> * gcc.dg/strlenopt-12g.c: Likewise.
>> * gcc.dg/strlenopt-14g.c: Likewise.
>> * gcc.dg/strlenopt-14gf.c: Likewise.
>> * gcc.dg/strlenopt-16g.c: Likewise.
>> * gcc.dg/strlenopt-17g.c: Likewise.
>> * gcc.dg/strlenopt-18g.c: Likewise.
>> * gcc.dg/strlenopt-1f.c: Likewise.
>> * gcc.dg/strlenopt-22g.c: Likewise.
>> * gcc.dg/strlenopt-2f.c: Likewise.
>> * gcc.dg/strlenopt-31g.c: Likewise.
>> * gcc.dg/strlenopt-33g.c: Likewise.
>> * gcc.dg/strlenopt-4g.c: Likewise.
>> * gcc.dg/strlenopt-4gf.c: Likewise.
>> * gcc.dg/strncmp-2.c: Likewise.
>> * gcc.dg/struct-ret-3.c: Likewise.
>> * gcc.dg/torture/pr69760.c: Likewise.
>> * gcc.target/arm/div64-unwinding.c: Likewise.
>> * gcc.target/arm/stack-checking.c: Likewise.
>> * gcc.target/arm/synchronize.c: Likewise.
>> * gcc.target/arm/pr66912.c: Add arm*-*-uclinuxfdpiceabi.
>> * lib/target-supports.exp (check_effective_target_pie): Likewise.
>> (check_effective_target_sync_long_long_runtime): Likewise.
>> (check_effective_target_sync_int_long): Likewise.
>> (check_effective_target_sync_char_short): Likewise.
>>
> I think these are ok, but you're changing many generic test targets.
>
> Are the testsuite maintainers ok with this change?
Yes. The patch is approved.
I looked them all over, they look fine. For these sorts of target changes, the
target maintainers can just approve the usual and customary changes to the test
suite. People can always ask for review for any reason they want, but as
people skill up on usual and customary, the target maintains usually do a good
job in this area. This patch to me seems usual and customary.
>> Change-Id: I89bfea79d4490c5df0b6470def5a31d7f31ac2cc
>>
>> diff --git a/gcc/testsuite/g++.dg/abi/forced.C
>> b/gcc/testsuite/g++.dg/abi/forced.C
>> index 0e6be28..2d1ec53 100644
>> --- a/gcc/testsuite/g++.dg/abi/forced.C
>> +++ b/gcc/testsuite/g++.dg/abi/forced.C
>> @@ -1,4 +1,4 @@
>> -// { dg-do run { target *-*-linux* *-*-gnu* } }
>> +// { dg-do run { target *-*-linux* *-*-gnu* *-*-uclinux* } }
>> // { dg-options "-pthread" }
>>
>> #include
>> diff --git a/gcc/testsuite/g++.dg/abi/guard2.C
>> b/gcc/testsuite/g++.dg/abi/guard2.C
>> index c35fa7e..74139a8 100644
>> --- a/gcc/testsuite/g++.dg/abi/guard2.C
>> +++ b/gcc/testsuite/g++.dg/abi/guard2.C
>> @@ -1,6 +1,6 @@
>> // PR c++/41611
>> // Test that the guard gets its own COMDAT group.
>> -// { dg-final { scan-assembler "_ZGVZN1A1fEvE1i,comdat" { target *-*-linux*
>> *-*-gnu* } } }
>> +// { dg-final { scan-assembler "_ZGVZN1A1fEvE1i,comdat" { target *-*-linux*
>> *-*-gnu* *-*-uclinux* } } }
>>
>> struct A {
>>static int f()
>> diff --git a/gcc/testsuite/g++.dg/ext/cleanup-10.C
>> b/gcc/testsuite/g++.dg/ext/cleanup-10.C
>> index 66c7b76..56aeb66 100644
>> --- a/gcc/testsuite/g++.dg/ext/cleanup-10.C
>> +++ b/gcc/testsuite/g++.dg/ext/cleanup-10.C
>> @@ -1,4 +1,4 @@
>> -/* { dg-do run { target hppa*-*-hpux* *-*-linux* *-*-gnu*
>> powerpc*-*-darwin* *-*-darwin[912]* } } */
>> +/* { dg-do run { target hppa*-*-hpux* *-*-linux* *-*-gnu*
>> powerpc*-*-darwin* *-*-darwin[912]* *-*-uclinux* } } */
>> /* { dg-options "-fexceptions -fnon-call-exceptions -O2" } */
>> /* Verify that cleanups work with exception handling through signal frames
>> on alternate stack. */
>> diff --git a/gcc/testsuite/g++.dg/ext/cleanup-11.C
>> b/gcc/testsuite/g++.dg/ext/cleanup-11.C
>> index 6e96521..c6d3560 100644
>> --- a/gcc/testsuite/g++.dg/ext/cleanup-11.C
>> +++ b/gcc/testsuite/g++.dg/ext/cleanup-11.C
>> @@ -1,4 +1,4 @@
>
Re: [ARM/FDPIC v5 18/21] [ARM][testsuite] FDPIC: Enable tests on pie_enabled targets
On Jul 19, 2019, at 1:57 AM, Kyrill Tkachov wrote:
>
> On 5/15/19 1:39 PM, Christophe Lyon wrote:
>> Some tests have the "nonpic" guard, but pass on
>> arm*-*-uclinuxfdpiceabi because it is in PIE mode by default. Rather
>> than adding this target to all these tests, add the "pie_enabled"
>> effective target.
>>
>> 2019-XX-XX Christophe Lyon
>>
>> gcc/testsuite/
>> * g++.dg/cpp0x/noexcept03.C: Add pie_enabled.
>> * g++.dg/ipa/devirt-c-7.C: Likewise.
>> * g++.dg/ipa/ivinline-1.C: Likewise.
>> * g++.dg/ipa/ivinline-2.C: Likewise.
>> * g++.dg/ipa/ivinline-3.C: Likewise.
>> * g++.dg/ipa/ivinline-4.C: Likewise.
>> * g++.dg/ipa/ivinline-5.C: Likewise.
>> * g++.dg/ipa/ivinline-7.C: Likewise.
>> * g++.dg/ipa/ivinline-8.C: Likewise.
>> * g++.dg/ipa/ivinline-9.C: Likewise.
>> * g++.dg/tls/pr79288.C: Likewise.
>> * gcc.dg/addr_equal-1.c: Likewise.
>> * gcc.dg/const-1.c: Likewise.
>> * gcc.dg/ipa/pure-const-1.c: Likewise.
>> * gcc.dg/noreturn-8.c: Likewise.
>> * gcc.dg/pr33826.c: Likewise.
>> * gcc.dg/torture/ipa-pta-1.c: Likewise.
>> * gcc.dg/tree-ssa/alias-2.c: Likewise.
>> * gcc.dg/tree-ssa/ipa-split-5.c: Likewise.
>> * gcc.dg/tree-ssa/loadpre6.c: Likewise.
>> * gcc.dg/uninit-19.c: Likewise.
>>
> Looks sensible, but this is not an arm-specific patch.
>
> CC'ing testsuite maintainers.
Seem sensible to me as well. Darwin is a pie by default sort of platform as I
recall, and as long as it doesn't trip up there (you can just watch for darwin
fallout), should be fine.
Patch is approved. Do watch for darwin fallout, and if there is some, we'd
have to think a little more about it. I'm not expecting any fall out (but I
haven't tested).
>> Change-Id: I1a0d836b892c23891f739fccdc467d0f354ab82c
>>
>> diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept03.C
>> b/gcc/testsuite/g++.dg/cpp0x/noexcept03.C
>> index 2d37867..906a44d 100644
>> --- a/gcc/testsuite/g++.dg/cpp0x/noexcept03.C
>> +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept03.C
>> @@ -1,6 +1,6 @@
>> // Runtime test for noexcept-specification.
>> // { dg-options "-Wnoexcept" }
>> -// { dg-do run { target nonpic } }
>> +// { dg-do run { target { nonpic || pie_enabled } } }
>> // { dg-require-effective-target c++11 }
>>
>> #include
>> diff --git a/gcc/testsuite/g++.dg/ipa/devirt-c-7.C
>> b/gcc/testsuite/g++.dg/ipa/devirt-c-7.C
>> index 2e76cbe..efb65c2 100644
>> --- a/gcc/testsuite/g++.dg/ipa/devirt-c-7.C
>> +++ b/gcc/testsuite/g++.dg/ipa/devirt-c-7.C
>> @@ -1,7 +1,6 @@
>> /* Verify that ipa-cp will not get confused by placement new constructing an
>> object within another one when looking for dynamic type change . */
>> -/* { dg-do run } */
>> -/* { dg-require-effective-target nonpic } */
>> +/* { dg-do run { target { nonpic || pie_enabled } } } */
>> /* { dg-options "-O3 -Wno-attributes" } */
>>
>> extern "C" void abort (void);
>> diff --git a/gcc/testsuite/g++.dg/ipa/ivinline-1.C
>> b/gcc/testsuite/g++.dg/ipa/ivinline-1.C
>> index 9b10d20..2d988bc 100644
>> --- a/gcc/testsuite/g++.dg/ipa/ivinline-1.C
>> +++ b/gcc/testsuite/g++.dg/ipa/ivinline-1.C
>> @@ -1,6 +1,6 @@
>> /* Verify that simple virtual calls are inlined even without early
>> inlining. */
>> -/* { dg-do run { target nonpic } } */
>> +/* { dg-do run { target { nonpic || pie_enabled } } } */
>> /* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" }
>> */
>>
>> extern "C" void abort (void);
>> diff --git a/gcc/testsuite/g++.dg/ipa/ivinline-2.C
>> b/gcc/testsuite/g++.dg/ipa/ivinline-2.C
>> index 21cd46f..d978638 100644
>> --- a/gcc/testsuite/g++.dg/ipa/ivinline-2.C
>> +++ b/gcc/testsuite/g++.dg/ipa/ivinline-2.C
>> @@ -1,6 +1,6 @@
>> /* Verify that simple virtual calls using this pointer are inlined
>> even without early inlining.. */
>> -/* { dg-do run { target nonpic } } */
>> +/* { dg-do run { target { nonpic || pie_enabled } } } */
>> /* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" }
>> */
>>
>> extern "C" void abort (void);
>> diff --git a/gcc/testsuite/g++.dg/ipa/ivinline-3.C
>> b/gcc/testsuite/g++.dg/ipa/ivinline-3.C
>> index 1e24644..f756a16 100644
>> --- a/gcc/testsuite/g++.dg/ipa/ivinline-3.C
>> +++ b/gcc/testsuite/g++.dg/ipa/ivinline-3.C
>> @@ -1,6 +1,6 @@
>> /* Verify that simple virtual calls on an object refrence are inlined
>> even without early inlining. */
>> -/* { dg-do run { target nonpic } } */
>> +/* { dg-do run { target { nonpic || pie_enabled } } } */
>> /* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" }
>> */
>>
>> extern "C" void abort (void);
>> diff --git a/gcc/testsuite/g++.dg/ipa/ivinline-4.C
>> b/gcc/testsuite/g++.dg/ipa/ivinline-4.C
>> index cf0d980..5fbd3ef 100644
>> --- a/gcc/testsuite/g++.dg/ipa/ivinline-4.C
>> +++ b/gcc/testsuite/g++.dg/ipa/ivinline-4.C
>> @@ -1,7 +1,7 @@
>> /*
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Mon, Jul 22, 2019 at 02:34:53PM -0400, Michael Meissner wrote: > On Sat, Jul 20, 2019 at 12:28:34PM -0400, David Edelsohn wrote: > > This patch needs to add rs6000-internal.h to tm_file in gcc/config.gcc > > for all of the powerpc/rs6000 targets. It also may need tm_p_file and > > tm_d_file definitions. > > While I agree it would make things easier if the declarations are available to > everybody, why wasn't this an issue when rs6000-internal.h was created to > allow > stuff to move out from rs6000.c to rs6000-logues.c? AIUI it is mostly needed to get dependencies right? Segher
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Mon, Jul 22, 2019 at 02:36:00PM -0400, Michael Meissner wrote: > On Sun, Jul 21, 2019 at 12:41:51PM -0500, Segher Boessenkool wrote: > > On Sat, Jul 20, 2019 at 12:13:08PM -0400, Michael Meissner wrote: > > > I will be iterating on patch #9 and sending out a replacement shortly. > > > > > > This is patch #10. It moves the various data structures from rs6000.c to > > > > I'll review this tomorrow, fwiw. > > > > A general request: please don't send patches as replies to other emails. > > In the past you had asked me to group patches under a single overview patch. > I > can go back to sending patches individually. When you send a *series*, please send it as series; when you send individual patches, please send them individually. This is all about how people will get it in their email, and how they will handle it. This really *is* a series, just most patches do not exist yet, if I understand correctly. That is not ideal of course -- I cannot review it as a series, because I don't have the later patches. Sending the patches as replies to weeks-old other patches is less than useful. Segher
Ping^2: [PATCH] RISC-V: Add -malign-data= option.
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01609.html Andrew, Palmer, I think all issues was fixed in https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01689.html . Do you have any concerns about the patch? Best regards, Ilia.
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Mon, Jul 22, 2019 at 02:59:39PM -0400, Michael Meissner wrote: > On Mon, Jul 22, 2019 at 12:42:13AM -0500, Segher Boessenkool wrote: > > On Sat, Jul 20, 2019 at 12:13:08PM -0400, Michael Meissner wrote: > > > 2019-07-20 Michael Meissner > > > > > > * config/rs6000/rs6000-internal.h (rs6000_hard_regno_mode_ok_p): > > > Move various declarations relating to addressing and register > > > allocation to rs6000-internal.h from rs6000.c so that in the > > > future we can move things out of rs6000.c. > > > > Just say > > (rs6000_hard_regno_mode_ok_p): New declaration. > > for the things that only had a definition before. > > > > > Make the static arrays global, > > > > That's not this entry. Say that in the entries where it applies. > > > > > and define them in rs6000.c. > > > > Say that in the corresponding entry for rs6000.c . > > > > > (enum rs6000_reg_type): Likewise. > > > > This one always was a declaration. > > This was a mechanical patch, just moving the swath of code from rs6000.c to > rs6000-internal.h. And yet, your changelog should be correct ;-) > So in change, I can go back to just: > > #define RELOAD_REG_VALID 0x01/* Mode valid in register.. */ > #define RELOAD_REG_MULTIPLE 0x02/* Mode takes multiple registers. */ > #define RELOAD_REG_INDEXED0x04/* Reg+reg addressing. */ > #define RELOAD_REG_OFFSET 0x08/* Reg+offset addressing. */ > #define RELOAD_REG_PRE_INCDEC 0x10/* PRE_INC/PRE_DEC valid. */ > #define RELOAD_REG_PRE_MODIFY 0x20/* PRE_MODIFY valid. */ > #define RELOAD_REG_AND_M160x40/* AND -16 addressing. */ > #define RELOAD_REG_QUAD_OFFSET0x80/* quad offset is limited. */ Yes. Just keep it as is. This is supposed to be a move, not a change. > Some time later if I continue with the current code, that would be changed to: Maybe. We'll see then. > I was just trying to save a step since I was moving the definitions any way to > add the alignment. A move should not introduce any unnecessary changes. This only is much more work for everyone (you: you need to write extra changelog for it. Me: I see there are extra changes, and now I have to check everything else, too). > > It's hard to tell whether the problem is factored sanely, or if this > > creates a big mountain of spaghetti instead. Can you show how this is > > used later? > > The intention is to move bits to other files. In particular, I want to create > a new rs6000-prefixed.c file that contains the bits specific to prefixed > addressing. That isn't a good split. All addressing-related stuff in one file might make sense, just like the rs6000-call.c split did (which was worthwhile because all the huge builtin stuff naturally fit there as well). But only one kind of addressing? Nope, that does not sound good. > However, longer term, it would be nice to move other things from rs6000.c to > other files, such as the functions that deal with p8 fusion, and the > legitimate > address functions (which I still tend to mentally classify as GO_IF_LEGITIMATE > type functions, even as you mention, we no longer have GO_IF_LEGITIMATE*). But only where that makes sense. Dividing things is only good if a) the division has an obvious, fundamental meaning; b) the interface between that part and the rest is thin; c) there is something to actually *win* from dividing things. > > Normally, you send a whole series, and then perhaps many of the first > > are preparatory only, but a reviewer can see where things are headed, > > and *then* simple refactorings like this can make sense. The way this > > patch looks now you are just making a lot of data global. > > This was intended to be just a mechanical patch to move things to the .h file. That still needs an explanation: why is this a good thing, why do you want that change? Sometimes that is obvious of course, but here it is not. It would be a lot more obvious if there was more context. Segher
Re: [PATCH] handle multibyte stores larger than char in strlen (PR 91183, 86888)
On 7/19/19 4:04 PM, Martin Sebor wrote:
> On targets with permissive alignment requirements GCC sometimes
> lowers stores of short (between two and 16 bytes), power-of-two
> char sequences to single integer stores of the corresponding
> width. This happens for sequences of ordinary character stores
> as well as for some calls to memcpy.
>
> However, the strlen pass is only prepared to handle character
> stores and not those of wider integers. As a result, the strlen
> optimization tends to get defeated in cases when it could benefit
> the most: very short strings. I counted 1544 instances where
> the strlen optimization was disabled in a GCC build on x86_64
> due to this sort of early store merging, and over two thousand
> in a build of the Linux kernel.
>
> In addition, -Wstringop-overflow only considers calls to string
> functions and is ineffective against past-the-end accesses by
> these merged multibyte stores.
>
> To improve the effectiveness of both the optimization as well
> as the warning the attached patch enhances the strlen pass to
> consider these wide accesses. Since the infrastructure for doing
> this is already in place (strlen can compute multibyte accesses
> via MEM_REFs of character arrays), the enhancement isn't very
> intrusive. It relies on the native_encode_expr function to
> determine the encoding of an expression and its "length".
>
> I tested the patch on x86_64. I expect the tests the patch
> adds will need some adjustment for big-endian and strictly
> aligned targets.
>
> Martin
>
> gcc-91183.diff
>
> PR tree-optimization/91183 - strlen of a strcpy result with a conditional
> source not folded
> PR tree-optimization/86688 - missing -Wstringop-overflow using a non-string
> local array in strnlen with excessive bound
>
> gcc/ChangeLog:
>
> PR tree-optimization/91183
> PR tree-optimization/86688
> * builtins.c (compute_objsize): Handle MEM_REF.
> * tree-ssa-strlen.c (class ssa_name_limit_t): New.
> (get_min_string_length): Remove.
> (count_nonzero_bytes): New function.
> (handle_char_store): Rename...
> (handle_store): to this. Handle multibyte stores via integer types.
> (strlen_check_and_optimize_stmt): Adjust conditional and the called
> function name.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/91183
> PR tree-optimization/86688
> * gcc.dg/attr-nonstring-2.c: Remove xfails.
> * gcc.dg/strlenopt-70.c: New test.
> * gcc.dg/strlenopt-71.c: New test.
> * gcc.dg/strlenopt-72.c: New test.
> * gcc.dg/strlenopt-8.c: Remove xfails.
>
> +
>if (TREE_CODE (dest) != ADDR_EXPR)
> return NULL_TREE;
>
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index 88b6bd7869e..ca1aca3ed9e 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> +
> +/* If the SSA_NAME has already been "seen" return a positive value.
> + Otherwise add it to VISITED. If the SSA_NAME limit has been
> + reached, return a negative value. Otherwise return zero. */
> +
> +int ssa_name_limit_t::next_ssa_name (tree ssa_name)
Nit. Return type on a different line than the function's name. The
point behind that convention is to make it easier to search for a
function's definition.
> +/* Determine the minimum and maximum number of leading non-zero bytes
> + in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
> + to each. Set LENRANGE[2] to the total number of bytes in
> + the representation. Set *NULTREM if the representation contains
> + a zero byte, and set *ALLNUL if all the bytes are zero. Avoid
> + recursing deeper than the limits in SNLIM allow. Return true
> + on success and false otherwise. */
S/NULTREM/NULTERM/
>
>if (si != NULL)
> {
> - int cmp = compare_nonzero_chars (si, offset);
> - gcc_assert (offset == 0 || cmp >= 0);
> - if (storing_all_zeros_p && cmp == 0 && si->full_string_p)
> + /* Set to 1 if the string described by SI is being written into
> + before the terminating nul (if it has one), to zero if the nul
> + is being overwritten but not beyond, or negative otherwise. */
> + int store_b4_nul[2];
Umm "store_b4_nul"? Are you really trying to save 3 characters in the
name by writing it this way? :-)
You've got two entries in the array, but the comment reads as if there's
just one element. What is the difference between the two array elements?
I didn't see anything terribly concerning so far, but I do want to look
at handle_store again after the comment is fixed so that I'm sure I'm
interpreting things correctly.
Jeff
Re: [PATCH v2] [rs6000] Add documentation for __builtin_mtfsf
On Mon, Jul 22, 2019 at 09:00:08AM -0500, Paul Clarke wrote:
>
> 2019-07-21 Paul A. Clarke
>
> [gcc]
>
> * doc/extend.texi: Add documentation for __builtin_mtfsf.
It should mention the section this is in... That is "Basic PowerPC
Built-in Functions Available on all Configurations" I think?
> v2: wordsmithing at Segher's request. I'm having a hard time not saying too
> much. :-)
:-)
> -accessing the sticky status bits. The
> +accessing the sticky status bits. The @code{__builtin_mtfsf} takes a
> constant
> +8-bit integer field mask and a representation of the new value of the FPSCR
> +and generates the @code{mtfsf} (extended mnemonic) instruction to write new
> +values to selected fields of the FPSCR. The
"The @code{__builtin_mtfsf} takes a constant 8-bit integer field mask
and a double precision floating point argument, and generates" etc.?
(It's not a representation of the new value, it says nothing about the
fields you do *not* write).
Okay for trunk with such a change. All backports are fine as well.
Thanks!
Segher
Re: [PATCH v3 3/3] PR80791 Consider doloop cmp use in ivopts
On Mon, Jul 22, 2019 at 09:18:10AM +0200, Richard Biener wrote: > On Mon, 22 Jul 2019, Segher Boessenkool wrote: > > > Hi! > > > > (Maybe I am missing half of the discussion -- sorry if so). > > > > I think we should have a new iv for just the doloop (which can have the > > same starting value and step and type as another iv). > > > > Has this been considered? > > I was also suggesting this (maybe with too many words ;)). If > it's a doloop target add such IV (candidate!) which has zero > use-cost for the increment and compare but a (target configurable) > penalty for other uses. Invasiveness of this approach is probably > that you need to distinguish this candidate by making it a new > kind (or maybe we can just have a specia candidate number...). Or just set some (boolean) flag in the candidate. I think it should simply not be allowed for any use except the doloop uses at all? You can have multiple ivs for the same loop just fine, right? And costs will make everything work out, if the costs are set correctly? Segher
Re: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops
Hi Fredrik, > I'm glad to hear from you again! I'm not dead, just distracted. > > I think that should be a GAS warning really (similarly to macros that > > expand to multiple instructions in a delay slot) as people ought to be > > allowed to do what they wish, and then `-Werror' can be used for code > > quality enforcement (and possibly disabled on a case-by-case basis). > > How can one reasonably prevent the bug when compiling a whole Linux > distribution with thousands of packages if GAS merely warns and proceeds > in many cases? I think the tools must not set a policy. By using `.set noreorder' the user told the toolchain he knows better and asked to keep its hands away. As I say you can use `-Werror' for code auditing. And in most cases manually scheduled delay slots in handcoded assembly are a sign of a problem with the piece of code affected, regardless of the R5900 erratum. > > > [ In theory, GAS could actually insert NOPs prior to the noreorder > > > directive, > > > to make the loop longer that six instructions, but GAS does not have that > > > kind of capability. Another option that GCC prevents is to place a NOP in > > > the delay slot. ] > > > > Well, GAS does have that capability, although of course it is not enabled > > for `noreorder' code. > > Hmm? I'm unable to make sense of that, since such NOPs are unaffected by > noreorder. This is what I meant: > > loop: addiu $5,$5,1 > addiu $4,$4,1 > lb $2,-1($5) > nop /* These two NOPs would prevent the */ > nop /* bug while preserving noreorder. */ > .setnoreorder > .setnomacro > bne $2,$3,loop > sb $2,-1($4) > .setmacro > .setreorder See `nops_for_insn'. However again, as I noted, `.set noreorder' tells GAS not to analyse the dependencies for code within. There is no need to schedule this delay slot manually, and if this is generated code, then the producer (GCC) should have taken care of the hazards, be it architectural or errata. If this is manually written code, then the author asked for trouble. > > For generated code I think however that usually it > > will be cheaper performance-wise if a non-trivial delay-slot instruction > > is never produced in the affected cases (i.e. a dummy NOP is always used). > > I suppose so too. One observation is that R5900 BogoMIPS went down from > 584 to 195 when switching from the generic kernel variant > > .setnoreorder > 1:bneza0,1b > addiu a0,a0,-1 > .setreorder > > that is undefined for R5900 in arch/mips/lib/delay.c, to a special variant > > beqza0,2f > 1:addiu a0,a0,-1 > bneza0,1b > 2: > > for the R5900 where GAS will place a NOP in the branch delay slot. With > loop unrolling BogoMIPS could be inflated again, of course. In general, > unrolling is a bit better for the R5900 than general MIPS. Perhaps GCC > could be informed about that, possibly via profile-guided optimisation. Well, BogoMIPS is just an arbitrary number. There is a data dependency here, so manual delay slot scheduling was unavoidable. I'd suggest using this as a functional equivalent for the R5900: addiu a0,a0,1 1: addiu a0,a0,-1 bneza0,1b and avoiding the irregularity for a0==0. > > > A reasonable fix for GCC is perhaps to update gcc/config/mips/mips.md to > > > not > > > make explicit use of the branch delay slot, as suggested by the patch > > > below? > > > Then GCC will emit > > > > > > loop: addiu $5,$5,1 > > > addiu $4,$4,1 > > > lb $2,-1($5) > > > sb $2,-1($4) > > > bne $2,$3,loop > > > > > > that GAS will adjust in the ELF object to > > > > > >4: 24a50001addiu a1,a1,1 > > >8: 24840001addiu a0,a0,1 > > >c: 80a2lb v0,-1(a1) > > > 10: a082sb v0,-1(a0) > > > 14: 1443fffbbne v0,v1,4 > > > 18: nop > > > > > > where a NOP is placed in the delay slot to avoid the bug. For longer > > > loops, > > > this relies on GAS making appropriate use of the delay slot. I'm not sure > > > if > > > GAS is good at that, though? > > > > I'm sort-of surprised that GCC has produced `reorder' code here, making > > it rely on GAS for delay slot scheduling. Have you used an unusual set of > > options that prevents GCC from making `noreorder' code, which as I recall > > is the usual default (not for the MIPS16 mode IIRC, as well as some > > obscure corner cases)? > > I used the suggested patch, and recompiled the kernel with it, and verified > with the machine code tool that there were no undefined loops. Wouldn't that > be enough? It would break if GAS was invoked without `-mfix-r5900' (e.g. by using an explicit `-Wa,-mno-fix-r5900' option). > > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md > > > i
Re: [PATCH] Use -flto instead of -flto=N in DWARF producer string.
On 7/10/19 5:16 AM, Martin Liška wrote:
> On 7/10/19 1:15 PM, Jakub Jelinek wrote:
>> On Wed, Jul 10, 2019 at 01:08:52PM +0200, Martin Liška wrote:
>>> --- a/gcc/dwarf2out.c
>>> +++ b/gcc/dwarf2out.c
>>> @@ -24460,6 +24460,13 @@ gen_producer_string (void)
>>>case OPT_fchecking_:
>>> /* Ignore these. */
>>> continue;
>>> + case OPT_flto_:
>>> + {
>>> + const char *lto_canonical = "-flto";
>>> + switches.safe_push (lto_canonical);
>>> + len += strlen (lto_canonical) + 1;
>>> + break;
>>> + }
>> The indentation looks off, when case is indented by 6 columns,
>> { should be by 8 (i.e. a tab) and const by 10 (i.e. a tab + 2 spaces).
>>
>> Jakub
>>
> You are right, sorry for that.
>
> Martin
>
>
> 0001-Use-flto-instead-of-flto-N-in-DWARF-producer-string.patch
>
> From eda41b25bf8b91412683ad542074724c872b18a4 Mon Sep 17 00:00:00 2001
> From: Martin Liska
> Date: Wed, 10 Jul 2019 13:05:19 +0200
> Subject: [PATCH] Use -flto instead of -flto=N in DWARF producer string.
>
> gcc/ChangeLog:
>
> 2019-07-10 Martin Liska
>
> * dwarf2out.c (gen_producer_string): Canonize -flto=N
> to -flto in dwarf producer string.
OK
jeff
Re: Ping^2: [PATCH] RISC-V: Add -malign-data= option.
On Mon, Jul 22, 2019 at 1:45 PM Ilia Diachkov wrote: > Ping: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01609.html I'm looking at this now. I've been busy dealing with far too many problems, but have mostly caught up on my backlog, at least enough that I can deal with this now. I think the changes you made adequately address the concerns raised by Andrew and Palmer. I want to do a little testing of my own before I approve and install the patch. Jim
Re: [PATCH 1/2] [ARC] Fix and refurbish the interrupts.
On 7/9/19 10:23 AM, claz...@gmail.com wrote: > Hi Jeff, > > Please find attached the updated patch. > > What is new: > - mailing list feedback is taken into account. > - some comments are updated. > - a new test is added. > - the ARC AUX registers used by ZOL (hardware loop) and FPX (a custom > floating point implementation) are saved before fp-register. > - the millicode optimization is not used by ISR. > > > Thank you, > Claudiu > > > 0001-ARC-Fix-and-refurbish-the-interrupts.patch > > From d22368681b7aab4bef4b5c32a9a472808f2c16cd Mon Sep 17 00:00:00 2001 > From: Claudiu Zissulescu > Date: Fri, 17 May 2019 14:48:17 +0300 > Subject: [PATCH] [ARC] Fix and refurbish the interrupts. > > When entering an interrupt, not only the call save registers needs to > be place on stack but also the call clobbers one. More over, the > ARC700 return from interrupt instruction needs to be rtie, the same > like ARCv2 CPUs. While the ARC6xx family uses j.f [ilinkX] > instruction. Additionally, we need to save the state of the ZOL > machinery, namely the lp_count, lp_end and lp_start registers. For > architectures which are using extension registers (i.e., HS48) we need > to save/restore them as well. > > gcc/ > -xx-xx Claudiu Zissulescu > > * config/arc/arc-protos.h (arc_output_function_epilogue): Delete > declaration. > (arc_compute_frame_size): Millicode is disabled when compiling > ISR. > (arc_return_address_register): Likewise. > (arc_compute_function_type): Likewise. > (arc_compute_frame_size): Likewise. > (secondary_reload_info): Likewise. > (arc_get_unalign): Likewise. > (arc_can_use_return_insn): Declare. > * config/arc/arc.c (AUX_LP_START): Define > (AUX_LP_END): Likewise. > (arc_frame_info): Update gmask member to 64-bit datum. > (GMASK_LEN): Update. > (arc_compute_function_type): Make it static, move it forward. > (arc_must_save_register): Update, consider the extra regs. > (arc_compute_millicode_save_restore_regs): Update to use the 64 > bit gmask. > (arc_compute_frame_size): Likewise. > (arc_enter_leave_p): Likewise. > (arc_save_callee_saves): Likewise. > (arc_restore_callee_saves): Likewise. > (arc_save_callee_enter): Likewise. > (arc_restore_callee_leave): Likewise. > (arc_save_callee_milli): Likewise. > (arc_restore_callee_milli): Likewise. > (arc_expand_prologue): Add new interrupt handling. > (arc_return_address_register): Make it static, move it forward. > (arc_expand_epilogue): Add new interrupt handling. > (arc_get_unalign): Delete. > (arc_epilogue_uses): Make sure we do not remove the extra > saved/restored registers when interrupt. > (arc_can_use_return_insn): New function. > (push_reg): Likewise. > (pop_reg): Likewise. > (arc_save_callee_saves): Add ZOL and FPX aux registers saving > procedures. > (arc_restore_callee_saves): Likewise, but restoring. > * config/arc/arc.md (VUNSPEC_ARC_ARC600_RTIE): Define. > (R33_REG): Likewise. > (R34_REG): Likewise. > (R35_REG): Likewise. > (R36_REG): Likewise. > (R37_REG): Likewise. > (R38_REG): Likewise. > (R39_REG): Likewise. > (R45_REG): Likewise. > (R46_REG): Likewise. > (R47_REG): Likewise. > (R48_REG): Likewise. > (R49_REG): Likewise. > (R50_REG): Likewise. > (R51_REG): Likewise. > (R52_REG): Likewise. > (R53_REG): Likewise. > (R54_REG): Likewise. > (R55_REG): Likewise. > (R56_REG): Likewise. > (R58_REG): Likewise. > (type): Add rtie attribute. > (in_call_delay_slot): Use RETURN_ADDR_REGNUM. > (movsi_insn): Accept moves to lp_count. > (rtie): Update pattern. > (simple_return): Simplify it, don't use this pattern as a return > from an interrupt. > (arc600_rtie): New pattern. > (p_return_i): Clean up. > (return): Likewise. > * config/arc/builtins.def (rtie): Only available for non ARC6xx > family CPUs. > * config/arc/predicates.md (move_src_operand): Consider lp_count > as a register. > > gcc/testsuite > -xx-xx Claudiu Zissulescu > > * gcc.target/arc/arc.exp (check_effective_target_accregs): New > predicate. > * gcc.target/arc/builtin_special.c: Update test/ > * gcc.target/arc/interrupt-1.c: Likewise. > * gcc.target/arc/interrupt-10.c: New test. > * gcc.target/arc/interrupt-11.c: Likewise. > * gcc.target/arc/interrupt-12.c: Likewise. OK Jeff
[PATCH] report as disabled options unsupported by a language (PR 80545)
While resolving PR80545 - option -Wstringop-overflow not recognized
by Fortran, I discovered that a command line options that's supported
only by a subset of languages is considered as enabled when tested
by the middle-end even when the current language doesn't support
the option.
When the option controls a warning, there is no good way to suppress
its false positives via the usual mechanisms (i.e., specifying
-Wno-stringop-overflow either on the command line or via a pragma)
because the option is not recognized by the languages that do not
support it.
The attached patch arranges for such options to be treated as disabled
when queried by the middle-end when the current language doesn't support
them. The fix wasn't as straightforward as testing
lang_hooks.option_lang_mask () in the diagnostics subsystem because
it doesn't have access to lang_hooks. To get around it the patch adds
a new member, diagnostic_context::lang_mask, and initializes it with
the result of the hook.
To make debugging and testing this fix easier I enhanced the output
of the -Q --help= options to clearly indicate when an otherwise
recognized option isn't supported by a front-end. For example,
where the current trunk prints
-Walign-commons [enabled]
for the Fortran-only option -Walign-commons even when GCC is invoked
via a driver for a language like C or C++, with the patch applied it
prints
-Walign-commons [available in Fortran]
I also changed the output to indicate the what option an alias
is for, so that for example the -Wattribute-alias option that's
an alias for -Wattribute-alias=1 is shown with the alias target
as the value:
-Wattribute-alias -Wattribute-alias=1
Besides this, I also corrected the printing of byte-size arguments
(they were sliced from 64 to 32 bits).
Martin
PR driver/80545 - option -Wstringop-overflow not recognized by Fortran
gcc/cp/ChangeLog:
PR driver/80545
* decl.c (finish_function): Use lang_mask.
gcc/testsuite/ChangeLog:
PR driver/80545
* gcc.misc-tests/help.exp: Add tests.
* lib/options.exp: Handle C++.
gcc/ChangeLog:
PR driver/80545
* diagnostic.c (diagnostic_classify_diagnostic): Use lang_mask.
(diagnostic_report_diagnostic): Same.
* diagnostic.h (diagnostic_context::option_enabled): Add an argument.
(diagnostic_context::lang_mask): New data member.
* ipa-pure-const.c (suggest_attribute): Use
lang_hooks.option_lang_mask ().
* opts-common.c (option_enabled): Handle new argument.
(get_option_state): Pass an additional argument.
* opts.c (print_filtered_help): Print supported languages for
unsupported options. Adjust printing of current state.
* opts.h (option_enabled): Add argument.
* toplev.c (print_switch_values): Use lang_mask.
(general_init): Set global_dc->lang_mask.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index dbcf681c783..76bb5837140 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -16301,6 +16301,7 @@ finish_function (bool inline_p)
&& same_type_ignoring_top_level_qualifiers_p
(TREE_TYPE (valtype), TREE_TYPE (current_class_ref))
&& global_dc->option_enabled (OPT_Wreturn_type,
+ global_dc->lang_mask,
global_dc->option_state))
add_return_star_this_fixit (&richloc, fndecl);
}
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 4761b4349d3..96b6fa30052 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "edit-context.h"
#include "selftest.h"
#include "selftest-diagnostic.h"
+#include "opts.h"
#ifdef HAVE_TERMIOS_H
# include
@@ -696,6 +697,7 @@ diagnostic_classify_diagnostic (diagnostic_context *context,
if (old_kind == DK_UNSPECIFIED)
{
old_kind = !context->option_enabled (option_index,
+ context->lang_mask,
context->option_state)
? DK_IGNORED : (context->warning_as_error_requested
? DK_ERROR : DK_WARNING);
@@ -957,6 +959,7 @@ diagnostic_report_diagnostic (diagnostic_context *context,
/* This tests if the user provided the appropriate -Wfoo or
-Wno-foo option. */
if (! context->option_enabled (diagnostic->option_index,
+ context->lang_mask,
context->option_state))
return false;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 46c3b50a540..530acb45b38 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -180,7 +180,7 @@ struct diagnostic_context
/* Client hook to say whether the option controlling a diagnostic is
enabled. Returns nonzero if enabled, zero if disabled. */
- int (*option_enabled) (int, void *);
+ int (*option_enabled) (int, unsigned, void *);
/* Client information to pass as second argument to
option_enabled. */
@@ -206,6 +206,9 @@ struct diagnostic_context
int lock;
+ /* A copy of lang_hooks.option_lang_mask (). */
+ unsigned lang_mask;
+
bool inhibit_notes_p;
/* When printing source code, should the ch
Re: [PATCH] MIPS: Fix GCC `noreorder' for undefined R5900 short loops
On 7/22/19 3:47 PM, Maciej W. Rozycki wrote: > Hi Fredrik, > >> I'm glad to hear from you again! > > I'm not dead, just distracted. > >>> I think that should be a GAS warning really (similarly to macros that >>> expand to multiple instructions in a delay slot) as people ought to be >>> allowed to do what they wish, and then `-Werror' can be used for code >>> quality enforcement (and possibly disabled on a case-by-case basis). >> >> How can one reasonably prevent the bug when compiling a whole Linux >> distribution with thousands of packages if GAS merely warns and proceeds >> in many cases? > > I think the tools must not set a policy. By using `.set noreorder' the > user told the toolchain he knows better and asked to keep its hands away. Agreed, 100%. Jeff
Re: [PATCH 1/3] add -Wstruct-not-pod, -Wclass-is-pod, -Wmismatched-tags (PR 61339)
On 7/8/19 3:58 PM, Martin Sebor wrote: > The attached patch implements three new warnings: > > * -Wstruct-not-pod triggers for struct definitions that are not > POD structs, > * -Wclass-is-pod triggers for class definitions that satisfy > the requirements on POD structs, and > * -Wmismatched-tags that triggers for class and struct declarations > with class-key that doesn't match either their definition or > the first declaration (if no definition is provided). > > The implementation of -Wclass-is-pod and -Wstruct-not-pod is fairly > straightforward but the -Wmismatched-tags solution is slightly unusual. > It collects struct and class declarations first and diagnoses mismatches > only after the whole tramslation unit has been processed. This is so > that the definition of a class can guide which declarations to diagnose > no matter which come first. So there was some discussion on whether or not we even wanted to keep the struct vs class convention for GCC. Did that reach any kind of conclusion? I don't have a strong opinion here and will adjust to whatever the consensus is. Jeff
Re: [PATCH], Patch #10, move PowerPC data structures & helper functions from rs6000.c to rs6000-internal.h
On Mon, Jul 22, 2019 at 03:56:26PM -0500, Segher Boessenkool wrote: > That still needs an explanation: why is this a good thing, why do you > want that change? Sometimes that is obvious of course, but here it is > not. It would be a lot more obvious if there was more context. The trouble is to get that much context really relies on about several additional patches to get to the functions in particular that should be split out. As I implement stuff, I find myself neeting/wanting to access the stuff in reg_addr in other files (predicates.md, and the new file rs6000-prefix.c). In patch #9 in particular, I added yet another data stucture that was parallel to the information in reg_addr (originally called rs6000_offset_format, but based on your comments is now rs6000_insn_form for instruction format). Then I needed functions that given were a hard register and the move insn and it determined what instruction format (D/DS/DQ) was used so that we can decide whether the instruction is traditional or prefixed. I originally wrote with a lot of code to do that mapping, but once I added the DS_OFFSET mask and about 10 lines of code to setup DS_OFFSET, it became much simpler. But for now, I think I will just not worry about making rs6000.c smaller, and instead do the main work there rather than trying to split it out. I was hoping the split would be quick, and it doesn't seem to be. Splitting it out is somewhat of a side issue to me, and it looks like it is impeding getting the rest of the patches submitted. But it would be nice to have that information available to the other .c files as well as the .md files. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] i386: Properly encode xmm16-xmm31/ymm16-ymm31 for vector move
On 2/22/19 9:24 AM, H.J. Lu wrote:
> Hi Jan, Uros,
>
> This patch fixes the wrong code bug:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89229
>
> Tested on AVX2 and AVX512 with and without --with-arch=native.
>
> OK for trunk?
>
> Thanks.
>
> H.J.
> --
> i386 backend has
>
> INT_MODE (OI, 32);
> INT_MODE (XI, 64);
>
> So, XI_MODE represents 64 INTEGER bytes = 64 * 8 = 512 bit operation,
> in case of const_1, all 512 bits set.
>
> We can load zeros with narrower instruction, (e.g. 256 bit by inherent
> zeroing of highpart in case of 128 bit xor), so TImode in this case.
>
> Some targets prefer V4SF mode, so they will emit float xorps for zeroing.
>
> sse.md has
>
> (define_insn "mov_internal"
> [(set (match_operand:VMOVE 0 "nonimmediate_operand"
> "=v,v ,v ,m")
> (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand"
> " C,BC,vm,v"))]
>
> /* There is no evex-encoded vmov* for sizes smaller than 64-bytes
> in avx512f, so we need to use workarounds, to access sse registers
> 16-31, which are evex-only. In avx512vl we don't need workarounds.
> */
> if (TARGET_AVX512F && < 64 && !TARGET_AVX512VL
> && (EXT_REX_SSE_REG_P (operands[0])
> || EXT_REX_SSE_REG_P (operands[1])))
> {
> if (memory_operand (operands[0], mode))
> {
> if ( == 32)
> return "vextract64x4\t{$0x0, %g1, %0|%0, %g1,
> 0x0}";
> else if ( == 16)
> return "vextract32x4\t{$0x0, %g1, %0|%0, %g1,
> 0x0}";
> else
> gcc_unreachable ();
> }
> ...
>
> However, since ix86_hard_regno_mode_ok has
>
> /* TODO check for QI/HI scalars. */
> /* AVX512VL allows sse regs16+ for 128/256 bit modes. */
> if (TARGET_AVX512VL
> && (mode == OImode
> || mode == TImode
> || VALID_AVX256_REG_MODE (mode)
> || VALID_AVX512VL_128_REG_MODE (mode)))
> return true;
>
> /* xmm16-xmm31 are only available for AVX-512. */
> if (EXT_REX_SSE_REGNO_P (regno))
> return false;
>
> if (TARGET_AVX512F && < 64 && !TARGET_AVX512VL
> && (EXT_REX_SSE_REG_P (operands[0])
> || EXT_REX_SSE_REG_P (operands[1])))
>
> is a dead code.
>
> Also for
>
> long long *p;
> volatile __m256i yy;
>
> void
> foo (void)
> {
>_mm256_store_epi64 (p, yy);
> }
>
> with AVX512VL, we should generate
>
> vmovdqa %ymm0, (%rax)
>
> not
>
> vmovdqa64 %ymm0, (%rax)
>
> All TYPE_SSEMOV vector moves are consolidated to ix86_output_ssemov:
>
> 1. If xmm16-xmm31/ymm16-ymm31 registers aren't used, SSE/AVX vector
> moves will be generated.
> 2. If xmm16-xmm31/ymm16-ymm31 registers are used:
>a. With AVX512VL, AVX512VL vector moves will be generated.
>b. Without AVX512VL, xmm16-xmm31/ymm16-ymm31 register to register
> move will be done with zmm register move.
>
> ext_sse_reg_operand is removed since it is no longer needed.
>
> Tested on AVX2 and AVX512 with and without --with-arch=native.
>
> gcc/
>
> PR target/89229
> PR target/89346
> * config/i386/i386-protos.h (ix86_output_ssemov): New prototype.
> * config/i386/i386.c (ix86_get_ssemov): New function.
> (ix86_output_ssemov): Likewise.
> * config/i386/i386.md (*movxi_internal_avx512f): Call
> ix86_output_ssemov for TYPE_SSEMOV.
> (*movoi_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> Remove ext_sse_reg_operand and TARGET_AVX512VL check.
> (*movti_internal): Likewise.
> (*movdi_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> Remove ext_sse_reg_operand check.
> (*movsi_internal): Likewise.
> (*movtf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> (*movdf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> Remove TARGET_AVX512F, TARGET_PREFER_AVX256, TARGET_AVX512VL
> and ext_sse_reg_operand check.
> (*movsf_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> Remove TARGET_PREFER_AVX256, TARGET_AVX512VL and
> ext_sse_reg_operand check.
> * config/i386/mmx.md (MMXMODE:*mov_internal): Call
> ix86_output_ssemov for TYPE_SSEMOV. Remove ext_sse_reg_operand
> check.
> * config/i386/sse.md (VMOVE:mov_internal): Call
> ix86_output_ssemov for TYPE_SSEMOV. Remove TARGET_AVX512VL
> check.
> * config/i386/predicates.md (ext_sse_reg_operand): Removed.
>
> gcc/testsuite/
>
> PR target/89229
> PR target/89346
> * gcc.target/i386/avx512vl-vmovdqa64-1.c: Updated.
> * gcc.target/i386/pr89229-2a.c: New test.
> * gcc.target/i386/pr89229-2b.c: Likewise.
> * gcc.target/i386/pr89229-2c.c: Likewise.
> * gcc.target/i386/pr89229-3a.c: Likewise.
> * gcc.target/i386/pr89229-3b.c: Likewise.
> * gcc.target/i386/pr89229-3c.c: Likewise
Re: [PATCH] handle multibyte stores larger than char in strlen (PR 91183, 86888)
On 7/22/19 3:33 PM, Jeff Law wrote:
On 7/19/19 4:04 PM, Martin Sebor wrote:
On targets with permissive alignment requirements GCC sometimes
lowers stores of short (between two and 16 bytes), power-of-two
char sequences to single integer stores of the corresponding
width. This happens for sequences of ordinary character stores
as well as for some calls to memcpy.
However, the strlen pass is only prepared to handle character
stores and not those of wider integers. As a result, the strlen
optimization tends to get defeated in cases when it could benefit
the most: very short strings. I counted 1544 instances where
the strlen optimization was disabled in a GCC build on x86_64
due to this sort of early store merging, and over two thousand
in a build of the Linux kernel.
In addition, -Wstringop-overflow only considers calls to string
functions and is ineffective against past-the-end accesses by
these merged multibyte stores.
To improve the effectiveness of both the optimization as well
as the warning the attached patch enhances the strlen pass to
consider these wide accesses. Since the infrastructure for doing
this is already in place (strlen can compute multibyte accesses
via MEM_REFs of character arrays), the enhancement isn't very
intrusive. It relies on the native_encode_expr function to
determine the encoding of an expression and its "length".
I tested the patch on x86_64. I expect the tests the patch
adds will need some adjustment for big-endian and strictly
aligned targets.
Martin
gcc-91183.diff
PR tree-optimization/91183 - strlen of a strcpy result with a conditional
source not folded
PR tree-optimization/86688 - missing -Wstringop-overflow using a non-string
local array in strnlen with excessive bound
gcc/ChangeLog:
PR tree-optimization/91183
PR tree-optimization/86688
* builtins.c (compute_objsize): Handle MEM_REF.
* tree-ssa-strlen.c (class ssa_name_limit_t): New.
(get_min_string_length): Remove.
(count_nonzero_bytes): New function.
(handle_char_store): Rename...
(handle_store): to this. Handle multibyte stores via integer types.
(strlen_check_and_optimize_stmt): Adjust conditional and the called
function name.
gcc/testsuite/ChangeLog:
PR tree-optimization/91183
PR tree-optimization/86688
* gcc.dg/attr-nonstring-2.c: Remove xfails.
* gcc.dg/strlenopt-70.c: New test.
* gcc.dg/strlenopt-71.c: New test.
* gcc.dg/strlenopt-72.c: New test.
* gcc.dg/strlenopt-8.c: Remove xfails.
+
if (TREE_CODE (dest) != ADDR_EXPR)
return NULL_TREE;
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 88b6bd7869e..ca1aca3ed9e 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
+
+/* If the SSA_NAME has already been "seen" return a positive value.
+ Otherwise add it to VISITED. If the SSA_NAME limit has been
+ reached, return a negative value. Otherwise return zero. */
+
+int ssa_name_limit_t::next_ssa_name (tree ssa_name)
Nit. Return type on a different line than the function's name. The
point behind that convention is to make it easier to search for a
function's definition.
+/* Determine the minimum and maximum number of leading non-zero bytes
+ in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
+ to each. Set LENRANGE[2] to the total number of bytes in
+ the representation. Set *NULTREM if the representation contains
+ a zero byte, and set *ALLNUL if all the bytes are zero. Avoid
+ recursing deeper than the limits in SNLIM allow. Return true
+ on success and false otherwise. */
S/NULTREM/NULTERM/
if (si != NULL)
{
- int cmp = compare_nonzero_chars (si, offset);
- gcc_assert (offset == 0 || cmp >= 0);
- if (storing_all_zeros_p && cmp == 0 && si->full_string_p)
+ /* Set to 1 if the string described by SI is being written into
+before the terminating nul (if it has one), to zero if the nul
+is being overwritten but not beyond, or negative otherwise. */
+ int store_b4_nul[2];
Umm "store_b4_nul"? Are you really trying to save 3 characters in the
name by writing it this way? :-)
I'm actually saving four characters over "store_before_nul" ;-)
It's just a name I picked because I like it. Would you prefer to
go back to the original 'cmp' instead? It doesn't get much shorter
than that, or less descriptive, especially when there is no comment
explaining what it's for. (FWIW, I renamed it because I found myself
going back to the description of compare_nonzero_chars to remember
what cmp actually meant.)
You've got two entries in the array, but the comment reads as if there's
just one element. What is the difference between the two array elements?
Since handle_store deals with sequences of one or more bytes some
of the optimizations it implements(*) need to consider both where
the first byte of the sequence is store
Re: Handle strncpy in tree-ssa-dse.c
On 7/22/19 9:40 AM, Martin Sebor wrote: >> Given that they're not allowed to overlap, I would think not. If that >> were allowed then the code which aggressively transforms strncpy to >> memcpy would need to be disabled (or at least throttled back) as well. > > I think there's some (maybe too much) room for interpretation here > as to exactly what the sentence > > If copying takes place between objects that overlap, the behavior > is undefined. > > means. Taken to an extreme, one might say that the following > violates the requirement: > > char a[4] = "123\0"; > strcpy (a, a + 2); > > because the call copies bytes within the same object (the array a) > which inevitably overlaps itself. But I'm pretty sure that's not > the intended interpretation -- the object itself does overlap but > not the bytes that are copied. (This is also the view -Wrestrict > takes.) If it were undefined, then so would be the following: > > memcpy (a, a + 2, 2); > > With that in mind, I would be inclined to expect the following not > to violate the requirement either: > > strncpy (a, a + 2, 4); > > because the bytes that are copied do not overlap. With a set to > "123\0" as done above it's equivalent to: > > memcpy (a, a + 2, 2); > memset (a + 2, 0, 2); > > Admittedly, the examples aren't exactly the same which makes this > question interesting. Is it worth raising an interpretation request > with WG14? Hmm, I'd tend to go with the more conservative interpretation. ie, something like your first example is OK since there is no overlap between the two regions accessed within the array. If I confused things by dismissing Richi's example too quickly, then I apologize. Can't hurt to get clarification though. Jeff
Re: [PATCH] report as disabled options unsupported by a language (PR 80545)
On 7/22/19 3:54 PM, Martin Sebor wrote: > While resolving PR80545 - option -Wstringop-overflow not recognized > by Fortran, I discovered that a command line options that's supported > only by a subset of languages is considered as enabled when tested > by the middle-end even when the current language doesn't support > the option. > > When the option controls a warning, there is no good way to suppress > its false positives via the usual mechanisms (i.e., specifying > -Wno-stringop-overflow either on the command line or via a pragma) > because the option is not recognized by the languages that do not > support it. > > The attached patch arranges for such options to be treated as disabled > when queried by the middle-end when the current language doesn't support > them. The fix wasn't as straightforward as testing > lang_hooks.option_lang_mask () in the diagnostics subsystem because > it doesn't have access to lang_hooks. To get around it the patch adds > a new member, diagnostic_context::lang_mask, and initializes it with > the result of the hook. > > To make debugging and testing this fix easier I enhanced the output > of the -Q --help= options to clearly indicate when an otherwise > recognized option isn't supported by a front-end. For example, > where the current trunk prints > > -Walign-commons [enabled] > > for the Fortran-only option -Walign-commons even when GCC is invoked > via a driver for a language like C or C++, with the patch applied it > prints > > -Walign-commons [available in Fortran] > > I also changed the output to indicate the what option an alias > is for, so that for example the -Wattribute-alias option that's > an alias for -Wattribute-alias=1 is shown with the alias target > as the value: > > -Wattribute-alias -Wattribute-alias=1 > > Besides this, I also corrected the printing of byte-size arguments > (they were sliced from 64 to 32 bits). > > Martin > > gcc-80545.diff > > PR driver/80545 - option -Wstringop-overflow not recognized by Fortran > > gcc/cp/ChangeLog: > > PR driver/80545 > * decl.c (finish_function): Use lang_mask. > > gcc/testsuite/ChangeLog: > > PR driver/80545 > * gcc.misc-tests/help.exp: Add tests. > * lib/options.exp: Handle C++. > > gcc/ChangeLog: > > PR driver/80545 > * diagnostic.c (diagnostic_classify_diagnostic): Use lang_mask. > (diagnostic_report_diagnostic): Same. > * diagnostic.h (diagnostic_context::option_enabled): Add an argument. > (diagnostic_context::lang_mask): New data member. > * ipa-pure-const.c (suggest_attribute): Use > lang_hooks.option_lang_mask (). > * opts-common.c (option_enabled): Handle new argument. > (get_option_state): Pass an additional argument. > * opts.c (print_filtered_help): Print supported languages for > unsupported options. Adjust printing of current state. > * opts.h (option_enabled): Add argument. > * toplev.c (print_switch_values): Use lang_mask. > (general_init): Set global_dc->lang_mask. Ironically this might have helped me today. I was looking at a case where an LTO build get a fatal warning, but the non-LTO build got a different (and non-fatal due to flags warning). It wasn't until I started debugging the diagnostic machinery that I realized -Wall was a language specific flag and that accounted for the difference between the LTO and non-LTO builds. I think this is OK, but give other folks 48hrs to chime in just in case. jeff
Re: [PATCH 1/3] add -Wstruct-not-pod, -Wclass-is-pod, -Wmismatched-tags (PR 61339)
On Jul 22, 2019, at 3:19 PM, Jeff Law wrote: > > On 7/8/19 3:58 PM, Martin Sebor wrote: >> The attached patch implements three new warnings: >> >> * -Wstruct-not-pod triggers for struct definitions that are not >> POD structs, >> * -Wclass-is-pod triggers for class definitions that satisfy >> the requirements on POD structs, and >> * -Wmismatched-tags that triggers for class and struct declarations >> with class-key that doesn't match either their definition or >> the first declaration (if no definition is provided). >> >> The implementation of -Wclass-is-pod and -Wstruct-not-pod is fairly >> straightforward but the -Wmismatched-tags solution is slightly unusual. >> It collects struct and class declarations first and diagnoses mismatches >> only after the whole tramslation unit has been processed. This is so >> that the definition of a class can guide which declarations to diagnose >> no matter which come first. > So there was some discussion on whether or not we even wanted to keep > the struct vs class convention for GCC. > > Did that reach any kind of conclusion? Last I knew, the consensus was to accept patches that make the usage of struct and class consistent (no warning from clang by default, no defeat necessary). I don't recall if it was settled if patches would be rejected merely because they they were not consistent. I'd think we'd not reject patches on those grounds, but rather, let the people that build on clang style systems front the work, and contribute if they want. But, I have a long memory, and the above might be a consensus or two ago. :-) I glanced around, and my google-fu turned up [1] and [2]. 1 - https://gcc.gnu.org/PR61339 2 - https://gcc.gnu.org/wiki/FAQ#Wmismatched-tags Kinda annoying to have a complaint for this, for no other reason other than a bad MS compiler. It seems to have infected clang, and now in danger of infecting gcc, all for no good reason. I'd rather MS remove the warning, clang remove the warning, and be done with the whole mess. Oh well. In 20 more years, no one will even recall why we had the warning and/or won't question it's existence.
Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
On 7/16/19 12:37 PM, Andrew MacLeod wrote: > On 7/9/19 5:56 AM, Richard Biener wrote: >> On Tue, Jul 9, 2019 at 9:28 AM Aldy Hernandez wrote: >>> >>> >>> On 7/4/19 6:33 AM, Richard Biener wrote: On Wed, Jul 3, 2019 at 2:17 PM Aldy Hernandez wrote: > On 7/3/19 7:08 AM, Richard Biener wrote: >> On Wed, Jul 3, 2019 at 11:19 AM Aldy Hernandez >> wrote: > How about we keep VARYING and UNDEFINED typeless until right before we > call into the ranger. At which point, we have can populate min/max > because we have the tree_code and the type handy. So right before we > call into the ranger do: > > if (varying_p ()) > foo->set_varying(TYPE); > > This would avoid the type cache, and keep the ranger happy. you cannot do set_varying on the static const range but instead you'd do value_range tem (*foo); if (varying_p ()) tem->set_full_range (TYPE); which I think we already do in some places. Thus my question _where_ you actually need this. >>> Basically, everywhere. By having a type for varying/undefined, we don't >>> have to special case anything. Sure, we could for example, special case >>> the invert operation for undefined / varying. And we could special case >>> everything dealing with ranges to handle varying and undefined, but why? >>> We could also pass a type argument everywhere, but that's just ugly. >>> However, I do understand your objection to the type cache. >>> >>> How about the attached approach? Set the type for varying/undefined >>> when we know it, while avoiding touching the CONST varying. Then right >>> before calling the ranger, pass down a new varying node with min/max for >>> any varyings that were still typeless until that point. >>> >>> I have taken care of never adding a set_varying() that was not already >>> there. Would this keep the const happy? >>> >>> Technically we don't need to set varying/undef types for every instance >>> in VRP, but we need it at least for the code that will be shared with >>> range-ops (extract_range_from_multiplicative_op, union, intersect, etc). >>> I just figured if we have the information, might as well set it for >>> consistency. >>> >>> If you like this approach, I can rebase the other patches that depend on >>> this one. >> OK, so I went ant checked what you do for class irange which has >> a type but no kind member (but constructors with a kind). It also >> uses wide_int members for storage. For a pure integer constant >> range representation this represents somewhat odd choices; I'd >> have elided the m_type member completely here, it seems fully >> redundant. Only range operations need to be carried out in a >> specific type (what I was suggesting above). Even the precision >> encoded in the wide_int members is redundant then (I'd have >> expected widest_int here and trailing-wide-ints for optimizing >> storage). > > What irange contains internally is a bit arbitrary. It's really an API > for managing a set of subranges. We could have used trees internally > just as easily, then we wouldnt need a type field. Since we were doing > lots of operations, rather than going back and forth from trees all the > time, we just used the underlying wide_int directly. we could have > fiddle farted around with HOST_WIDE_INT or whatever, but wide_int is > there, has all the operations, and it works fine. so thats what it > currently is on the branch. > > We are treating a range object as a unique self contained object. > Therefore, the range has a type so we know how to print it, and can > confirm before any operation that the ranges being operated on are > appropriately matched. We found and opened bugzillas over the past > couple years for places where our code caught bugs because a range was > created and then operated on in a way that was not compatible with > another range. I think there is a still an open one against ada(?) > where the switch and case are different precision. > > From my point of view, a range object is similar to a tree node. A tree > node has the bits to indicate what the value is, but also associates a > type with those bits within the same object. This is less error prone > than passing around the bits and the type separately. As ranges are > starting to be used in many places outside of VRP, we should do the same > thing with ranges. WIth value_range it would actually be free since > there is already a tree for the bounds already which contains the type. > > > > > >> to fold_range/op_range? The API also seems to be oddly >> constrained to binary ops. Anyway, the way you build >> the operator table requires an awful lot of global C++ ctor >> invocations, sth we generally try to avoid. But I'm getting >> into too many details here. > > Its "oddly constrained" because what you are looking at is just the > standardized unary/binary ops code.. ie the equivalent of > extract_
Re: [PATCH] Automatics in equivalence statements
On 7/1/19 3:35 AM, Mark Eggleston wrote: > > On 25/06/2019 14:17, Mark Eggleston wrote: >> >> On 25/06/2019 00:17, Jeff Law wrote: >>> On 6/24/19 2:19 AM, Bernhard Reutner-Fischer wrote: On Fri, 21 Jun 2019 07:10:11 -0700 Steve Kargl wrote: > On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote: >> Currently variables with the AUTOMATIC attribute can not appear in an >> EQUIVALENCE statement. However its counterpart, STATIC, can be >> used in >> an EQUIVALENCE statement. >> >> Where there is a clear conflict in the attributes of variables in an >> EQUIVALENCE statement an error message will be issued as is currently >> the case. >> >> If there is no conflict e.g. a variable with a AUTOMATIC attribute >> and a >> variable(s) without attributes all variables in the EQUIVALENCE will >> become AUTOMATIC. >> >> Note: most of this patch was written by Jeff Law >> >> Please review. >> >> ChangeLogs: >> >> gcc/fortran >> >> Jeff Law >> Mark Eggleston >> >> * gfortran.h: Add check_conflict declaration. > This is wrong. By convention a routine that is not static > has the gfc_ prefix. > Updated the code to use gfc_check_conflict instead. > Furthermore doesn't this export indicate that you're committing a layering violation somehow? >>> Possibly. I'm the original author, but my experience in our fortran >>> front-end is minimal. I fully expected this patch to need some >>> tweaking. >>> >>> We certainly don't want to recreate all the checking that's done in >>> check_conflict. We just need to defer it to a later point -- >>> find_equivalence seemed like a good point since we've got the full >>> equivalence list handy and can accumulate the attributes across the >>> entire list, then check for conflicts. >>> >>> If there's a concrete place where you think we should be doing this, I'm >>> all ears. >>> >> Any suggestions will be appreciate. > * symbol.c (check_conflict): Remove automatic in equivalence > conflict > check. > * symbol.c (save_symbol): Add check for in equivalence to > stop the > the save attribute being added. > * trans-common.c (build_equiv_decl): Add is_auto parameter and > add !is_auto to condition where TREE_STATIC (decl) is set. > * trans-common.c (build_equiv_decl): Add local variable is_auto, > set it true if an atomatic attribute is encountered in the > variable atomatic? I read atomic but you mean automatic. >>> Yes. >>> > list. Call build_equiv_decl with is_auto as an additional > parameter. > flag_dec_format_defaults is enabled. > * trans-common.c (accumulate_equivalence_attributes) : New > subroutine. > * trans-common.c (find_equivalence) : New local variable > dummy_symbol, > accumulated equivalence attributes from each symbol then > check for > conflicts. I'm just curious why you don't gfc_copy_attr for the most part of accumulate_equivalence_attributes? thanks, >>> Simply didn't know about it. It could probably significantly simplify >>> the accumulation of attributes step. >> Using gfc_copy_attr causes a great many "Duplicate DIMENSION attribute >> specified at (1)" errors. This is because there is a great deal of >> checking done instead of simply keeping track of the attributes used >> which is all that is required for determining whether there is a >> conflict in the equivalence statement. >> >> Also, the final section of accumulate_equivalence_attributes involving >> SAVE, INTENT and ACCESS look suspect to me. I'll check and update the >> patch if necessary. > > No need to check intent as there is already a conflict with DUMMY and > INTENT can only be present for dummy variables. > > Please find attached an updated patch. Change logs: > > gcc/fortran > > Jeff Law > Mark Eggleston > > * gfortran.h: Add gfc_check_conflict declaration. > * symbol.c (check_conflict): Rename cfg_check_conflict and remove > static. > * symbol.c (cfg_check_conflict): Remove automatic in equivalence > conflict check. > * symbol.c (save_symbol): Add check for in equivalence to stop the > the save attribute being added. > * trans-common.c (build_equiv_decl): Add is_auto parameter and > add !is_auto to condition where TREE_STATIC (decl) is set. > * trans-common.c (build_equiv_decl): Add local variable is_auto, > set it true if an atomatic attribute is encountered in the variable > list. Call build_equiv_decl with is_auto as an additional parameter. > flag_dec_format_defaults is enabled. > * trans-common.c (accumulate_equivalence_attributes) : New subroutine. > * trans-common.c (find_equivalence) : New local variable dummy_symbol, > accumulated equivalence attribute
Re: [PATCH PR Fortran/89286] Intrinsic sign and GNU Extension
On 7/10/19 9:52 AM, Steve Kargl wrote: > On Wed, Jul 10, 2019 at 02:50:47PM +0100, Mark Eggleston wrote: >> The attached patch treats the intrinsic SIGN in the same way as MOD and >> DIM as it has the same arguments. >> >> Tested using make -j 8 check-fortran on x86_64 >> >> Conditional compilation using #ifdef __GFC_REAL_16__ has been employed >> where appropriate in the test cases so should be OK on platforms without >> REAL(16). >> >> Change logs: >> >> gcc/fortran >> >> Mark Eggleston >> >> PR fortran/89286 >> * check.c (gfc_check_sign): Deleted. >> * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p >> instead of gfc_check_sign for "sign". >> * intrinsic.h: Remove declaration of gfc_check_sign. >> * iresolve.c (gfc_resolve_sign): Check for largest kind of the actual >> arguments and convert the smaller. Set return kind to be the largest. >> * simplify.c (gfc_simplify_sign): Use the largest kind of the actual >> arguments for return >> * intrinsic.texi: Add GNU extension notes for return value to SIGN. >> >> gcc/testsuite >> >> Mark Eggleston >> >> PR fortran/89240 >> * gfortran.dg/sign_gnu_extension_1.f90: New test. >> * gfortran.dg/sign_gnu_extension_2.f90: New test. >> * gfortran.dg/pr78619.f90: Check for "must have" instead of "must be". >> >> If OK please can someone commit as I do not have the privileges. >> > > We really need to get you commit access to the tree. I will handle that directly with Mark. > > I also am not a fan of this type of change. Having spent the > last few days working on fixing BOZ to conform to F2018, I'm > finding all sorts of undocumented "extensions". Personally, > I think gfortran should be moving towards the standard by > deprecating of these types of extensions. I'd tend to agree. The problem is getting real world codebases moved can be exceedingly difficult. At the very least these extensions should be behind suitable flags IMHO. jeff
Re: [PATCH PR Fortran/89286] Intrinsic sign and GNU Extension
On 7/11/19 6:16 AM, Mark Eggleston wrote: > > On 10/07/2019 17:20, Bernhard Reutner-Fischer wrote: >> On 10 July 2019 17:52:40 CEST, Steve Kargl >> wrote: >>> On Wed, Jul 10, 2019 at 02:50:47PM +0100, Mark Eggleston wrote: The attached patch treats the intrinsic SIGN in the same way as MOD >>> and DIM as it has the same arguments. Tested using make -j 8 check-fortran on x86_64 Conditional compilation using #ifdef __GFC_REAL_16__ has been >>> employed where appropriate in the test cases so should be OK on platforms >>> without REAL(16). Change logs: gcc/fortran Mark Eggleston PR fortran/89286 * check.c (gfc_check_sign): Deleted. >> ChangeLog has to be in present tense per convention. >> * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p instead of gfc_check_sign for "sign". * intrinsic.h: Remove declaration of gfc_check_sign. * iresolve.c (gfc_resolve_sign): Check for largest kind of the >>> actual arguments and convert the smaller. Set return kind to be the >>> largest. * simplify.c (gfc_simplify_sign): Use the largest kind of the >>> actual arguments for return * intrinsic.texi: Add GNU extension notes for return value to >>> SIGN. gcc/testsuite Mark Eggleston PR fortran/89240 * gfortran.dg/sign_gnu_extension_1.f90: New test. * gfortran.dg/sign_gnu_extension_2.f90: New test. * gfortran.dg/pr78619.f90: Check for "must have" instead of >>> "must be". If OK please can someone commit as I do not have the privileges. >>> We really need to get you commit access to the tree. >>> >>> I also am not a fan of this type of change. Having spent the >>> last few days working on fixing BOZ to conform to F2018, I'm >>> finding all sorts of undocumented "extensions". Personally, >>> I think gfortran should be moving towards the standard by >>> deprecating of these types of extensions. > > I agree. > > I think that -std=gnu should not be the default, if gnu extensions are > required you have to ask for them. > >> At least make them explicit under explicit extension or at least >> -legacy or whatever its called. >> >> thanks, > > I agree, at the moment the GNU extension is silently supported by DIM > and MOD So if Mark was to put this behavior behind the -fdec or whatever the appropriate switch is, could this move forward? jeff
Re: [PATCH PR Fortran/89286] Intrinsic sign and GNU Extension
On 7/10/19 7:50 AM, Mark Eggleston wrote: > The attached patch treats the intrinsic SIGN in the same way as MOD and > DIM as it has the same arguments. > > Tested using make -j 8 check-fortran on x86_64 > > Conditional compilation using #ifdef __GFC_REAL_16__ has been employed > where appropriate in the test cases so should be OK on platforms without > REAL(16). > > Change logs: > > gcc/fortran > > Mark Eggleston > > PR fortran/89286 > * check.c (gfc_check_sign): Deleted. > * intrinsic.c (add_functions): Call add_sym_2 with gfc_check_a_p > instead of gfc_check_sign for "sign". > * intrinsic.h: Remove declaration of gfc_check_sign. > * iresolve.c (gfc_resolve_sign): Check for largest kind of the actual > arguments and convert the smaller. Set return kind to be the largest. > * simplify.c (gfc_simplify_sign): Use the largest kind of the actual > arguments for return > * intrinsic.texi: Add GNU extension notes for return value to SIGN. > > gcc/testsuite > > Mark Eggleston > > PR fortran/89240 > * gfortran.dg/sign_gnu_extension_1.f90: New test. > * gfortran.dg/sign_gnu_extension_2.f90: New test. > * gfortran.dg/pr78619.f90: Check for "must have" instead of "must be". > > If OK please can someone commit as I do not have the privileges. > So FWIW, this is in my tester now too. jeff
Go patch committed: Follow-on fix for finalizing imported methods
This Go frontend patch by Than McIntosh is a revision to
https://golang.org/cl/185518
(https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00821.html), which
added code to perform finalization of methods on types created by the
importer and not directly reachable until inlining is done.
The original fix invoked the finalization code at the end of
Import::read_types(), but it turns out this doesn't handle the case
where a type with methods is read in due to a reference from something
later in the export data (a function or variable). The fix is to move
the import finalization call to the end of Import::import().
A testcase for this bug is in https://golang.org/cl/187057.
This fixes https://golang.org/issue/33219.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 273611)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e242929304e7a524ced56dc94605bbf6d83e6489
+b7bce0dbccb978d33eb8ce0bffc02fae2c2857c1
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/import.cc
===
--- gcc/go/gofrontend/import.cc (revision 273577)
+++ gcc/go/gofrontend/import.cc (working copy)
@@ -450,6 +450,14 @@ Import::import(Gogo* gogo, const std::st
this->require_c_string("\n");
}
+ // Finalize methods for any imported types. This call is made late in the
+ // import process so as to A) avoid finalization of a type whose methods
+ // refer to types that are only partially read in, and B) capture both the
+ // types imported by read_types() directly, and those imported indirectly
+ // because they are referenced by an imported function or variable.
+ // See issues #33013 and #33219 for more on why this is needed.
+ this->finalize_methods();
+
return this->package_;
}
@@ -678,12 +686,6 @@ Import::read_types()
this->gogo_->add_named_type(nt);
}
- // Finalize methods for any imported types. This is done after most of
- // read_types() is complete so as to avoid method finalization of a type
- // whose methods refer to types that are only partially read in.
- // See issue #33013 for more on why this is needed.
- this->finalize_methods();
-
return true;
}
Re: [PATCH 1/3] add -Wstruct-not-pod, -Wclass-is-pod, -Wmismatched-tags (PR 61339)
On 7/22/19 4:19 PM, Jeff Law wrote: On 7/8/19 3:58 PM, Martin Sebor wrote: The attached patch implements three new warnings: * -Wstruct-not-pod triggers for struct definitions that are not POD structs, * -Wclass-is-pod triggers for class definitions that satisfy the requirements on POD structs, and * -Wmismatched-tags that triggers for class and struct declarations with class-key that doesn't match either their definition or the first declaration (if no definition is provided). The implementation of -Wclass-is-pod and -Wstruct-not-pod is fairly straightforward but the -Wmismatched-tags solution is slightly unusual. It collects struct and class declarations first and diagnoses mismatches only after the whole tramslation unit has been processed. This is so that the definition of a class can guide which declarations to diagnose no matter which come first. So there was some discussion on whether or not we even wanted to keep the struct vs class convention for GCC. Did that reach any kind of conclusion? I don't have a strong opinion here and will adjust to whatever the consensus is. I'm not really sure how to gauge consensus here but this patch doesn't actually enforce the GCC convention, it just makes it possible (none of the new options is included in -Wall or -Wextra; they all have to be enabled explicitly). I myself wouldn't adopt a class/struct convention like that if given the choice and wouldn't be at all upset if we dropped it going forward, despite all the effort I put into the cleanup (which has already been committed). But us dropping it won't affect other projects that also use it(*), albeit without enforcement, and so I would rather not tie the consideration of the patch to the GCC guideline. I would think helping other projects enforce it if they find it helpful would be seen as valuable even if we don't find the convention useful in GCC anymore. The struct/class convention aside, and ignoring the Visual C++ bug that likely gave rise to it, the -Wmismatched-tags option is useful for all projects that value consistency: declaring struct using the same class-key everywhere, and same for classes. Martin PS The top-rated answer to the article below gives some idea of how popular this struct/class rule of thumb might be out there: https://stackoverflow.com/questions/54585 Incidentally, among the many hits I get when searching for struct vs class online are guidelines to prefer one over the other due to better interoperability, efficiency, or expressiveness in some languages that do make more of a distinction between the two than C++ does (e.g., C#, D, Ruby, or Swift). I can see mixed language projects or users coming to C++ from those other languages wanting to use a convention in C++ that reflects the use of the two keywords in those other languages even if the use doesn't actually have the same effect.
Re: Ping^2: [PATCH] RISC-V: Add -malign-data= option.
On Mon, Jul 22, 2019 at 2:50 PM Jim Wilson wrote: > On Mon, Jul 22, 2019 at 1:45 PM Ilia Diachkov > wrote: > > Ping: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01609.html It looks good. I modified the ChangeLog entry a little to change Added to New. I modified the doc entry to improve the English a little. Built it to verify that I didn't break it and committed it. Jim
[PATCH] PR91195: fix -Wmaybe-uninitialized warning for conditional store optimization
This patch is to fix PR91195. Is it OK for trunk? diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 711a31ea597..4db36644160 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2019-07-22 Jiangning Liu + + PR middle-end/91195 + * tree-ssa-phiopt.c (cond_store_replacement): Work around + -Wmaybe-uninitialized warning. + 2019-07-22 Stafford Horne * config/or1k/or1k.c (or1k_expand_compare): Check for int before diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index b64bde695f4..7a86007d087 100644 --- a/gcc/tree-ssa-phiopt.c +++ b/gcc/tree-ssa-phiopt.c @@ -2240,6 +2240,14 @@ cond_store_replacement (basic_block middle_bb, basic_block join_bb, tree base = get_base_address (lhs); if (!auto_var_p (base) || TREE_ADDRESSABLE (base)) return false; + + /* The transformation below will inherently introduce a memory load, +for which LHS may not be initialized yet if it is not in NOTRAP, +so a -Wmaybe-uninitialized warning message could be triggered. +Since it's a bit hard to have a very accurate uninitialization +analysis to memory reference, we disable the warning here to avoid +confusion. */ + TREE_NO_WARNING (lhs) = 1; } /* Now we've checked the constraints, so do the transformation: Thanks, -Jiangning
Re: [PATCH v3 3/3] PR80791 Consider doloop cmp use in ivopts
on 2019/7/22 下午3:18, Richard Biener wrote: > On Mon, 22 Jul 2019, Segher Boessenkool wrote: > >> Hi! >> >> (Maybe I am missing half of the discussion -- sorry if so). >> >> I think we should have a new iv for just the doloop (which can have the >> same starting value and step and type as another iv). >> >> Has this been considered? > > I was also suggesting this (maybe with too many words ;)). If > it's a doloop target add such IV (candidate!) which has zero > use-cost for the increment and compare but a (target configurable) > penalty for other uses. Invasiveness of this approach is probably > that you need to distinguish this candidate by making it a new > kind (or maybe we can just have a specia candidate number...). > Hi Richard, Really appreciate your comments on this, very sorry not to go with this. Since this patch is for TARGET_HAVE_COUNT_REG_DECR_P, I was thinking it's fairly enough to reuse the existing IV cands and just zeroing doloop use cost with them. I'm very happy to unify it. If you/Segher/Bin don't have any concerns, I'd like to make it as one follow up item. One thing to double check is this dedicated IV will follow decrement instead of increment align with doloop optimize? Then it looks to shape the loop closing to doloop pattern, at least it's decrement. Thanks, Kewen
Re: [PATCH v3 3/3] PR80791 Consider doloop cmp use in ivopts
Hi Segher, on 2019/7/23 上午5:43, Segher Boessenkool wrote: > On Mon, Jul 22, 2019 at 09:18:10AM +0200, Richard Biener wrote: >> On Mon, 22 Jul 2019, Segher Boessenkool wrote: >> >>> Hi! >>> >>> (Maybe I am missing half of the discussion -- sorry if so). >>> >>> I think we should have a new iv for just the doloop (which can have the >>> same starting value and step and type as another iv). >>> >>> Has this been considered? >> >> I was also suggesting this (maybe with too many words ;)). If >> it's a doloop target add such IV (candidate!) which has zero >> use-cost for the increment and compare but a (target configurable) >> penalty for other uses. Invasiveness of this approach is probably >> that you need to distinguish this candidate by making it a new >> kind (or maybe we can just have a specia candidate number...). > > Or just set some (boolean) flag in the candidate. > > I think it should simply not be allowed for any use except the doloop > uses at all? For the targets where the iteration count doesn't sit in its hardware count register, we may need to allow the IV to be used for other suitable uses? > You can have multiple ivs for the same loop just fine, > right? Yes. > And costs will make everything work out, if the costs are set > correctly? There are some cases requiring to do IV elimination, it might require some cost adjustment/tuning to keep this. I met this when I did pre-bind the BIV for it, if the dedicated IV has the best cost and is associated to doloop use, it probably stops the others to merge. If my understanding is correct, this is more like to transform the loop into doloop pattern earlier, the penalty of mis-predication of doloop can be more? Pros is the setup code sequence for iteration count happens in middle-end, can be optimized better (RTL misses some range info).
[PATCH v5 3/3] PR80791 Consider doloop cmp use in ivopts
Hi Bin,
This patch follows your suggestion, to avoid use infinite cost iv cand to
rewrite.
In order to allow other IV cands to be considered, zeroing the iv cand cost if
its users are only doloop uses. (See the typical case in previous reply.)
Could you please have a look? Thanks in advance!
Kewen
-
gcc/ChangeLog
2019-07-23 Kewen Lin
PR middle-end/80791
* target.def (have_count_reg_decr_p): New hook.
* doc/tm.texi.in (TARGET_HAVE_COUNT_REG_DECR_P): New hook.
* doc/tm.texi: Regenerate.
* config/rs6000/rs6000.c (rs6000_have_count_reg_decr_p): New function.
(TARGET_HAVE_COUNT_REG_DECR_P): New macro.
* tree-ssa-loop-ivopts.c (adjust_group_iv_cost_for_doloop): New
function.
(find_doloop_use): Likewise.
(record_group): Init doloop_p.
(determine_group_iv_cost): Call adjust_group_iv_cost_for_doloop.
(tree_ssa_iv_optimize_loop): Call function have_count_reg_decr_p,
generic_predict_doloop_p and find_doloop_use.
(generic_predict_doloop_p): Update attribute.
(iv_ca_set_no_cp): Adjust cand cost handling for doloop.
(iv_ca_set_cp): Likewise.
(iv_ca_new): Init n_cand_doloop_uses.
(iv_ca_free): Free n_cand_doloop_uses.
gcc/testsuite/ChangeLog
2019-07-23 Kewen Lin
PR middle-end/80791
* gcc.dg/tree-ssa/ivopts-lt.c: Adjust.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6667cd0..e98aba9 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1912,6 +1912,9 @@ static const struct attribute_spec
rs6000_attribute_table[] =
#undef TARGET_PREDICT_DOLOOP_P
#define TARGET_PREDICT_DOLOOP_P rs6000_predict_doloop_p
+#undef TARGET_HAVE_COUNT_REG_DECR_P
+#define TARGET_HAVE_COUNT_REG_DECR_P true
+
#undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV
#define TARGET_ATOMIC_ASSIGN_EXPAND_FENV rs6000_atomic_assign_expand_fenv
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index c2aa4d0..5477294 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11618,6 +11618,14 @@ loops, and will help ivopts to make some decisions.
The default version of this hook returns false.
@end deftypefn
+@deftypevr {Target Hook} bool TARGET_HAVE_COUNT_REG_DECR_P
+Return true if the target supports hardware count register for decrement
+and branch. This count register can't be used as general register since
+moving to/from a general register from/to it is very expensive.
+For the targets with this support, ivopts can take doloop use as zero cost.
+The default value is false.
+@end deftypevr
+
@deftypefn {Target Hook} bool TARGET_CAN_USE_DOLOOP_P (const widest_int
@var{&iterations}, const widest_int @var{&iterations_max}, unsigned int
@var{loop_depth}, bool @var{entered_at_top})
Return true if it is possible to use low-overhead loops (@code{doloop_end}
and @code{doloop_begin}) for a particular loop. @var{iterations} gives the
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index b4d57b8..5f43b27 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7946,6 +7946,8 @@ to by @var{ce_info}.
@hook TARGET_PREDICT_DOLOOP_P
+@hook TARGET_HAVE_COUNT_REG_DECR_P
+
@hook TARGET_CAN_USE_DOLOOP_P
@hook TARGET_INVALID_WITHIN_DOLOOP
diff --git a/gcc/target.def b/gcc/target.def
index 71b6972..8a64e5b 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4246,6 +4246,16 @@ The default version of this hook returns false.",
bool, (struct loop *loop),
default_predict_doloop_p)
+DEFHOOKPOD
+(have_count_reg_decr_p,
+ "Return true if the target supports hardware count register for decrement\n\
+and branch. This count register can't be used as general register since\n\
+moving to/from a general register from/to it is very expensive.\n\
+For the targets with this support, ivopts can take doloop use as zero cost.\n\
+The default value is false.",
+ bool, false)
+
+
DEFHOOK
(can_use_doloop_p,
"Return true if it is possible to use low-overhead loops (@code{doloop_end}\n\
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c
b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c
index 7d5859b..3486e1a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c
@@ -18,5 +18,5 @@ f1 (char *p, uintptr_t i, uintptr_t n)
}
/* { dg-final { scan-tree-dump-times "PHI" 1 "ivopts" } } */
-/* { dg-final { scan-tree-dump-times "PHI vuses;
};
@@ -612,6 +614,9 @@ struct ivopts_data
/* Whether the loop body can only be exited via single exit. */
bool loop_single_exit_p;
+
+ /* Whether the loop has doloop comparison use. */
+ bool doloop_use_p;
};
/* An assignment of iv candidates to uses. */
@@ -630,6 +635,9 @@ struct iv_ca
/* Number of times each candidate is used. */
unsigned *n_cand_uses;
+ /* How many doloop uses for each candidates. */
+ unsigned *n_cand_doloop_uses;
+
/* The candidates used. */
bitmap cands;
@@ -1528,6 +1536,7 @@ record_group (struct ivop
