[PATCH] Fix array-quals-1.c for RISC-V
RISC-V will put those variable on srodata rather than rodata.
gcc/testsuite/ChangeLog:
* gcc.dg/array-quals-1.c: Allow srodata.
---
gcc/testsuite/gcc.dg/array-quals-1.c | 40 ++--
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/array-quals-1.c
b/gcc/testsuite/gcc.dg/array-quals-1.c
index 31aa1d3cb3f..c8d36296434 100644
--- a/gcc/testsuite/gcc.dg/array-quals-1.c
+++ b/gcc/testsuite/gcc.dg/array-quals-1.c
@@ -6,46 +6,46 @@
/* { dg-options "-Wno-discarded-array-qualifiers" } */
/* The MMIX port always switches to the .data section at the end of a file. */
/* { dg-final { scan-assembler-not "\\.data(?!\\.rel\\.ro)" { xfail
powerpc*-*-aix* mmix-*-* x86_64-*-mingw* } } } */
-/* { dg-final { scan-assembler-symbol-section {^_?a$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?a$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
static const int a[2] = { 1, 2 };
-/* { dg-final { scan-assembler-symbol-section {^_?a1$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?a1$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
const int a1[2] = { 1, 2 };
typedef const int ci;
-/* { dg-final { scan-assembler-symbol-section {^_?b$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?b$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
static ci b[2] = { 3, 4 };
-/* { dg-final { scan-assembler-symbol-section {^_?b1$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?b1$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
ci b1[2] = { 3, 4 };
typedef int ia[2];
-/* { dg-final { scan-assembler-symbol-section {^_?c$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?c$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
static const ia c = { 5, 6 };
-/* { dg-final { scan-assembler-symbol-section {^_?c1$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?c1$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
const ia c1 = { 5, 6 };
typedef const int cia[2];
-/* { dg-final { scan-assembler-symbol-section {^_?d$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?d$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
static cia d = { 7, 8 };
-/* { dg-final { scan-assembler-symbol-section {^_?d1$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?d1$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
cia d1 = { 7, 8 };
-/* { dg-final { scan-assembler-symbol-section {^_?e$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?e$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
static cia e[2] = { { 1, 2 }, { 3, 4 } };
-/* { dg-final { scan-assembler-symbol-section {^_?e1$}
{^\.(const|rodata)|\[RO\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?e1$}
{^\.(const|rodata|srodata)|\[RO\]} } } */
cia e1[2] = { { 1, 2 }, { 3, 4 } };
-/* { dg-final { scan-assembler-symbol-section {^_?p$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?p$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const p = &a;
-/* { dg-final { scan-assembler-symbol-section {^_?q$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?q$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const q = &b;
-/* { dg-final { scan-assembler-symbol-section {^_?r$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?r$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const r = &c;
-/* { dg-final { scan-assembler-symbol-section {^_?s$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?s$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const s = &d;
-/* { dg-final { scan-assembler-symbol-section {^_?t$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?t$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const t = &e;
-/* { dg-final { scan-assembler-symbol-section {^_?p1$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?p1$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const p1 = &a1;
-/* { dg-final { scan-assembler-symbol-section {^_?q1$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?q1$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const q1 = &b1;
-/* { dg-final { scan-assembler-symbol-section {^_?r1$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?r1$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const r1 = &c1;
-/* { dg-final { scan-assembler-symbol-section {^_?s1$}
{^\.(const|rodata)|\[RW\]} } } */
+/* { dg-final { scan-assembler-symbol-section {^_?s1$}
{^\.(const|rodata|srodata)|\[RW\]} } } */
void *const s1 = &d1;
-/* { dg-final { scan-assembler-symbol-section {^_?t1$}
{^\.(const|rodata)|\[RW\]} } } */
[PATCH] tree-optimization/98513 - fix bug in range intersection code
This fixes a premature optimization in the range intersection code
which assumes earlier branches have to be taken, not taking into
account that for symbolic ranges we cannot always compare endpoints.
The fix is to instantiate the compare deemed redundant (which then
fails as undecidable for the testcase).
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
2021-01-06 Richard Biener
PR tree-optimization/98513
* value-range.cc (intersect_ranges): Compare the upper bounds
for the expected relation.
* gcc.dg/tree-ssa/pr98513.c: New testcase.
---
gcc/testsuite/gcc.dg/tree-ssa/pr98513.c | 47 +
gcc/value-range.cc | 6 ++--
2 files changed, 51 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr98513.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr98513.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr98513.c
new file mode 100644
index 000..c15d6bd708e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr98513.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgimple" } */
+
+__attribute__((noipa))
+void __GIMPLE (ssa,startwith("evrp"))
+foo (int x, int minus_1)
+{
+ int tem;
+ unsigned int _1;
+ unsigned int _2;
+
+ __BB(2):
+ tem_4 = minus_1_3(D);
+ tem_5 = tem_4 + 2;
+ _1 = (unsigned int) x_6(D);
+ _2 = _1 + 2147483647u;
+ if (_2 > 1u)
+goto __BB3;
+ else
+goto __BB6;
+
+ __BB(3):
+ if (x_6(D) <= tem_5)
+goto __BB4;
+ else
+goto __BB6;
+
+ __BB(4):
+ if (x_6(D) > 5)
+goto __BB5;
+ else
+goto __BB6;
+
+ __BB(5):
+ __builtin_exit (0);
+
+ __BB(6):
+ return;
+
+}
+
+int
+main()
+{
+ foo (10, 100);
+ __builtin_abort ();
+}
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index c404787ccd0..9c42f82a105 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -974,7 +974,8 @@ intersect_ranges (enum value_range_kind *vr0type,
}
else if ((operand_less_p (vr1min, *vr0max) == 1
|| operand_equal_p (vr1min, *vr0max, 0))
- && operand_less_p (*vr0min, vr1min) == 1)
+ && operand_less_p (*vr0min, vr1min) == 1
+ && operand_less_p (*vr0max, vr1max) == 1)
{
/* [ ( ] ) or [ ]( ) */
if (*vr0type == VR_ANTI_RANGE
@@ -1008,7 +1009,8 @@ intersect_ranges (enum value_range_kind *vr0type,
}
else if ((operand_less_p (*vr0min, vr1max) == 1
|| operand_equal_p (*vr0min, vr1max, 0))
- && operand_less_p (vr1min, *vr0min) == 1)
+ && operand_less_p (vr1min, *vr0min) == 1
+ && operand_less_p (vr1max, *vr0max) == 1)
{
/* ( [ ) ] or ( )[ ] */
if (*vr0type == VR_ANTI_RANGE
--
2.26.2
Re: [PATCH] add g_nonstandard_bool attribute for GIMPLE FE use
On Tue, 5 Jan 2021, Joseph Myers wrote:
> On Tue, 5 Jan 2021, Richard Biener wrote:
>
> > would maybe result in a surprising result. One alternative
> > would be to make the attribute have the signedness specified as well
> > (C doesn't accept 'unsigned _Bool' or 'signed _Bool') or
> > simply name the attribute "signed_bool_precision". I guess the bool case
> > is really special compared to the desire to eventually allow
> > declaring of a 3 bit precision signed/unsigned integer type.
> >
> > Allowing 'signed _Bool' with -fgimple might be another option
> > of course.
>
> Something that makes clear it's a signed boolean type with the given
> precision seems a good idea (I'd have assumed a nonstandard boolean type
> with a given precision was unsigned).
OK, I've used signed_bool_precision, re-bootstrapped and tested on
x86_64-unknown-linux-gnu and pushed as below.
Richard.
>From c9ee9c1e3553247c776f33eb0fe0aadee094a192 Mon Sep 17 00:00:00 2001
From: Richard Biener
Date: Fri, 11 Dec 2020 09:50:59 +0100
Subject: [PATCH] add signed_bool_precision attribute for GIMPLE FE use
To: gcc-patches@gcc.gnu.org
This adds __attribute__((signed_bool_precision(precision))) to be able
to construct nonstandard boolean types which for the included testcase
is needed to simulate Ada and LTO interaction (Ada uses a 8 bit
precision boolean_type_node). This will also be useful for vector
unit testcases where we need to produce vector types with
non-standard precision signed boolean type components.
2021-01-06 Richard Biener
PR tree-optimization/95582
gcc/c-family/
* c-attribs.c (c_common_attribute_table): Add entry for
signed_bool_precision.
(handle_signed_bool_precision_attribute): New.
gcc/testsuite/
* gcc.dg/pr95582.c: New testcase.
---
gcc/c-family/c-attribs.c | 41 ++
gcc/testsuite/gcc.dg/pr95582.c | 19
2 files changed, 60 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr95582.c
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index ef7cec9b2e8..84ec86b2091 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -161,6 +161,8 @@ static tree handle_copy_attribute (tree *, tree, tree, int,
bool *);
static tree handle_nsobject_attribute (tree *, tree, tree, int, bool *);
static tree handle_objc_root_class_attribute (tree *, tree, tree, int, bool *);
static tree handle_objc_nullability_attribute (tree *, tree, tree, int, bool
*);
+static tree handle_signed_bool_precision_attribute (tree *, tree, tree, int,
+ bool *);
/* Helper to define attribute exclusions. */
#define ATTR_EXCL(name, function, type, variable) \
@@ -274,6 +276,8 @@ const struct attribute_spec c_common_attribute_table[] =
{
/* { name, min_len, max_len, decl_req, type_req, fn_type_req,
affects_type_identity, handler, exclude } */
+ { "signed_bool_precision", 1, 1, false, true, false, true,
+ handle_signed_bool_precision_attribute, NULL },
{ "packed", 0, 0, false, false, false, false,
handle_packed_attribute,
attr_aligned_exclusions },
@@ -894,6 +898,43 @@ validate_attr_arg (tree node[2], tree name, tree newarg)
/* Attribute handlers common to C front ends. */
+/* Handle a "signed_bool_precision" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_signed_bool_precision_attribute (tree *node, tree name, tree args,
+ int, bool *no_add_attrs)
+{
+ *no_add_attrs = true;
+ if (!flag_gimple)
+{
+ warning (OPT_Wattributes, "%qE attribute ignored", name);
+ return NULL_TREE;
+}
+
+ if (!TYPE_P (*node) || TREE_CODE (*node) != BOOLEAN_TYPE)
+{
+ warning (OPT_Wattributes, "%qE attribute only supported on "
+ "boolean types", name);
+ return NULL_TREE;
+}
+
+ unsigned HOST_WIDE_INT prec = HOST_WIDE_INT_M1U;
+ if (tree_fits_uhwi_p (TREE_VALUE (args)))
+prec = tree_to_uhwi (TREE_VALUE (args));
+ if (prec > MAX_FIXED_MODE_SIZE)
+{
+ warning (OPT_Wattributes, "%qE attribute with unsupported boolean "
+ "precision", name);
+ return NULL_TREE;
+}
+
+ tree new_type = build_nonstandard_boolean_type (prec);
+ *node = lang_hooks.types.reconstruct_complex_type (*node, new_type);
+
+ return NULL_TREE;
+}
+
/* Handle a "packed" attribute; arguments as in
struct attribute_spec.handler. */
diff --git a/gcc/testsuite/gcc.dg/pr95582.c b/gcc/testsuite/gcc.dg/pr95582.c
new file mode 100644
index 000..cc2ab46ec95
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr95582.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O3" } */
+
+typedef _Bool bool8 __attribute__((signed_bool_precision(8)));
+
+bool8 data[16];
+
+void __GIMPLE(ssa) foo(int f)
+{
+ _Bool t;
+
Re: git commit hook does not record my patches to PRs
On 1/5/21 3:04 PM, Uros Bizjak wrote: Hello! For some reason git commit hook does not record my patches to PRs, mentioned in the commit message. Some recent examples: PR 98521: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=951bdbde6ade56eb63af1dfa18777348a8a0d89e and PR98522: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1ff0ddcd8b4728bcc96e1daf2e70a03dc9fbf171 These two patches were committed as a single push (but one commit per push also doesn't reach the PR), and the second one is also missing in the gcc-cvs mailing list archive: https://gcc.gnu.org/pipermail/gcc-cvs/2021-January/date.html In the commit message, I have put PR marks everywhere I can think of, but the commit hook is still ignoring them. Can someone please check, what is wrong with the flow and eventually advise me what to do to enjoy the benefits of the automation. Hello. I can reproduce that for gcc-reposurgeon-8 repository and yes, it's again related to a unicode characters (in your case 'š' letter in your first name). I'll debug that with Jakub. Martin Thanks, Uros.
Re: [PATCH] Add line debug info for virtual thunks (PR ipa/97937)
On Wed, Jan 06, 2021 at 08:50:43AM +0100, Richard Biener wrote: > > Theoretically we could exclude the range of the no-loc function > > from the .debug_ranges, then gdb would not even step into the function. > > I'd argue we're failing to emit a .endloc at the end of functions > (rather than issueing a .noloc at the start of functions with no > locations). I wonder if using a special file ID and switching to that > would be an effective workaround? When gas is extended we could use > file ID zero for this (which gas currently rejects). If we had .endloc, emitting it at the end of functions would be theoretically more correct, but I'd be afraid it would unnecessarily grow the .debug_line size, because nobody should care about line information in padding bytes between functions and the .endloc would add there a change even when it would affect just one byte of padding (probably even when it wouldn't affect anything, depending on how it is implemented). Jakub
Fix PR testsuite/90806 on the 9 branch
The underlying bug was fixed by Marc in July 2019:
2019-07-10 Marc Glisse
* wide-int.h (wi::lshift): Reject negative values for the fastpath.
and Richard B. agreed to the backport onto the 9 branch.
Bootstrapped/regtested and applied on the 9 branch.
2021-01-06 Eric Botcazou
PR testsuite/90806
Backport from mainline
2019-07-10 Marc Glisse
* wide-int.h (wi::lshift): Reject negative values for the fastpath.
--
Eric Botcazoudiff --git a/gcc/wide-int.h b/gcc/wide-int.h
index 25ea0545439..77aa01f2c98 100644
--- a/gcc/wide-int.h
+++ b/gcc/wide-int.h
@@ -3032,8 +3032,7 @@ wi::lshift (const T1 &x, const T2 &y)
if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
? (STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT - 1)
&& xi.len == 1
- && xi.val[0] <= (HOST_WIDE_INT) ((unsigned HOST_WIDE_INT)
- HOST_WIDE_INT_MAX >> shift))
+ && IN_RANGE (xi.val[0], 0, HOST_WIDE_INT_MAX >> shift))
: precision <= HOST_BITS_PER_WIDE_INT)
{
val[0] = xi.ulow () << shift;
Re: make FOR_EACH_IMM_USE_STMT safe for early exits
On Jan 4, 2021, Richard Biener wrote: > Hmm - while the change looks good, doesn't it end up > calling end_imm_use_stmt_tranverse twice for those > uses still calling BREAK_FROM_IMM_USE_STMT? It does. I'd considered introducing a separate method to call end_imm_use_stmt_traverse if imm is not NULL, and then set it to NULL, but calling the function multiple times is not a problem: delink_imm_use just returns immediately the second time. > Thus, please remove uses of BREAK_FROM_IMM_USE_STMT > together with this patch. And RETURN_FROM_IMM_USE_STMT, I suppose? I wasn't sure whether to remove them and their users. -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
Re: [PATCH] Add line debug info for virtual thunks (PR ipa/97937)
> currently there is a problem when debugging a virtual thunk. That is > a decl with DECL_IGNORED_P. Currently the line information displayed > in gdb is completely bogus, thus the last line of whatever function > is immediately before the PC of the thunk. DECL_IGNORED_P means completely ignored for debug info purposes though and I think that the Ada compiler expects this semantic. > This patch improves the debug experience at least a bit by emitting > at the line number information where the thunk has been defined. > I do not dare to touch anything but dwarf2 debug info, therefore > the patch is a bit awkward. Please run the GDB testsuite with Ada support on the patch. -- Eric Botcazou
Re: [PATCH toplevel] libctf: new testsuite
On 5 Jan 2021, Alan Modra via Binutils told this: > It doesn't apply due to gcc missing binutils 87279e3cef5b2c5 changes > too. I could fix that easily enough but I'm going to ask that you > post a combined patch to bring the gcc repo up to date with any libctf > changes. Oops! That never occurred to me, but of course all the earlier stuff is probably missing too. Will come up with one once I've properly woken up. (Sorry, I really should have tried to apply it :( ) -- NULL && (void)
Re: [PATCH] Add line debug info for virtual thunks (PR ipa/97937)
On 1/6/21 12:49 PM, Eric Botcazou wrote:
>> currently there is a problem when debugging a virtual thunk. That is
>> a decl with DECL_IGNORED_P. Currently the line information displayed
>> in gdb is completely bogus, thus the last line of whatever function
>> is immediately before the PC of the thunk.
>
> DECL_IGNORED_P means completely ignored for debug info purposes though and I
> think that the Ada compiler expects this semantic.
>
>> This patch improves the debug experience at least a bit by emitting
>> at the line number information where the thunk has been defined.
>> I do not dare to touch anything but dwarf2 debug info, therefore
>> the patch is a bit awkward.
>
> Please run the GDB testsuite with Ada support on the patch.
>
Yes, I always do that, and this patch did not affect any Ada tests.
Anyway, I guess this patch will need some rework, to probably attack
the DECL_IGNORED_P without line info.
One thing I remembered from previous testing, and I was not sure if
it is a correctness issue or not, is this:
As I had to call begin_function also for DECL_IGNORED_P,
I noticed that there are a number of Ada functions with
DECL_IGNORED_P, but their crtl->has_bb_partition is set,
and therefore this block is executed with my patch, but
it is currently not executed:
See dwarf2out_begin_function:
if (crtl->has_bb_partition && !cold_text_section)
{
gcc_assert (current_function_decl == fun);
cold_text_section = unlikely_text_section ();
switch_to_section (cold_text_section);
ASM_OUTPUT_LABEL (asm_out_file, cold_text_section_label);
switch_to_section (sec);
}
I had expected that functions with DECL_IGNORED_P should not need
that side effect. Anyway I had not seen any effect from this.
Bernd.
[PATCH v2 toplevel] sync libctf toplevel from binutils-gdb
This pulls in the toplevel portions of these binutils-gdb commits:
1ff6de031241c59d0ff bfd, ld: add CTF section linking
87279e3cef5b2c54f4a libctf: installable libctf as a shared library
c59e30ed1727135f8ef libctf: new testsuite
* Makefile.def: Sync with binutils-gdb:
(dependencies): all-ld depends on all-libctf.
(host_modules): libctf is no longer no_install.
No longer no_check. Checking depends on all-ld.
* Makefile.in: Regenerated.
---
ChangeLog| 8
Makefile.def | 5 +++--
Makefile.in | 42 --
3 files changed, 51 insertions(+), 4 deletions(-)
This should be a little better (though I only had three hours sleep last
night so it's quite possible I messed it up again).
I reviewed the diff with GCC and nothing more libctf-related is missing.
This was generated against GCC trunk directly so should definitely apply
this time. Sorry about that.
diff --git a/ChangeLog b/ChangeLog
index bd87d5fc6ee..0a352870cd6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2021-01-06 Nick Alcock
+
+ * Makefile.def: Sync with binutils-gdb:
+ (dependencies): all-ld depends on all-libctf.
+ (host_modules): libctf is no longer no_install.
+ No longer no_check. Checking depends on all-ld.
+ * Makefile.in: Regenerated.
+
2021-01-05 Samuel Thibault
* libtool.m4: Match gnu* along other GNU systems.
diff --git a/Makefile.def b/Makefile.def
index c45be5bff45..3e38f61193f 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -141,8 +141,7 @@ host_modules= { module= lto-plugin; bootstrap=true;
extra_make_flags='@extra_linker_plugin_flags@'; };
host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
host_modules= { module= gotools; };
-host_modules= { module= libctf; no_install=true; no_check=true;
- bootstrap=true; };
+host_modules= { module= libctf; bootstrap=true; };
target_modules = { module= libstdc++-v3;
bootstrap=true;
@@ -463,6 +462,7 @@ dependencies = { module=all-binutils; on=all-build-bison; };
dependencies = { module=all-binutils; on=all-intl; };
dependencies = { module=all-binutils; on=all-gas; };
dependencies = { module=all-binutils; on=all-libctf; };
+dependencies = { module=all-ld; on=all-libctf; };
// We put install-opcodes before install-binutils because the installed
// binutils might be on PATH, and they might need the shared opcodes
@@ -561,6 +561,7 @@ dependencies = { module=configure-libctf; on=all-bfd; };
dependencies = { module=configure-libctf; on=all-intl; };
dependencies = { module=configure-libctf; on=all-zlib; };
dependencies = { module=configure-libctf; on=all-libiconv; };
+dependencies = { module=check-libctf; on=all-ld; };
// Warning, these are not well tested.
dependencies = { module=all-bison; on=all-intl; };
diff --git a/Makefile.in b/Makefile.in
index 2307c8dd083..247cb9c8711 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -41747,6 +41747,12 @@ maybe-check-libctf:
maybe-check-libctf: check-libctf
check-libctf:
+ @: $(MAKE); $(unstage)
+ @r=`${PWD_COMMAND}`; export r; \
+ s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
+ $(HOST_EXPORTS) $(EXTRA_HOST_EXPORTS) \
+ (cd $(HOST_SUBDIR)/libctf && \
+ $(MAKE) $(FLAGS_TO_PASS) $(EXTRA_BOOTSTRAP_FLAGS) check)
@endif libctf
@@ -41755,7 +41761,13 @@ maybe-install-libctf:
@if libctf
maybe-install-libctf: install-libctf
-install-libctf:
+install-libctf: installdirs
+ @: $(MAKE); $(unstage)
+ @r=`${PWD_COMMAND}`; export r; \
+ s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
+ $(HOST_EXPORTS) \
+ (cd $(HOST_SUBDIR)/libctf && \
+ $(MAKE) $(FLAGS_TO_PASS) install)
@endif libctf
@@ -41764,7 +41776,13 @@ maybe-install-strip-libctf:
@if libctf
maybe-install-strip-libctf: install-strip-libctf
-install-strip-libctf:
+install-strip-libctf: installdirs
+ @: $(MAKE); $(unstage)
+ @r=`${PWD_COMMAND}`; export r; \
+ s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
+ $(HOST_EXPORTS) \
+ (cd $(HOST_SUBDIR)/libctf && \
+ $(MAKE) $(FLAGS_TO_PASS) install-strip)
@endif libctf
@@ -61206,6 +61224,16 @@ all-stagetrain-binutils: maybe-all-stagetrain-libctf
all-stagefeedback-binutils: maybe-all-stagefeedback-libctf
all-stageautoprofile-binutils: maybe-all-stageautoprofile-libctf
all-stageautofeedback-binutils: maybe-all-stageautofeedback-libctf
+all-ld: maybe-all-libctf
+all-stage1-ld: maybe-all-stage1-libctf
+all-stage2-ld: maybe-all-stage2-libctf
+all-stage3-ld: maybe-all-stage3-libctf
+all-stage4-ld: maybe-all-stage4-libctf
+all-stageprofile-ld: maybe-all-stageprofile-libctf
+all-stagetrain-ld: maybe-all-stagetrain-libctf
+all-stagefeedback-ld: maybe-all-stagefeedback-libctf
+all-stageautoprofile-ld: maybe-all-stageautoprofile-libctf
+all-stageautofeedback-ld: maybe-all-stageautofeedback-libctf
install-binut
Re: Patch RFA: Support non-ASCII file names in git-changelog
On 1/6/21 8:25 AM, Martin Liška wrote: Anyway, I've got a workaround that I'm going to push. It's fixed now. @Ian: Can you please try to push the changes now? Thanks, Martin
Re: git commit hook does not record my patches to PRs
On 1/6/21 10:53 AM, Martin Liška wrote: I'll debug that with Jakub. https://github.com/AdaCore/git-hooks/issues/18
Re: [PATCH] libphobos: Allow building libphobos using Solaris/x86 assembler
Hi Iain, >> This patch removes the disabling of libphobos when the Solaris/x86 >> assembler is being used. >> >> Since r11-6373, D symbols are now compressed using back references, this >> helped reduce the average symbol length by a factor of about 3, while >> the longest symbol shrank from 416133 to 1142 characters. So the issues >> that were seen on Solaris/x86 should no longer be a problem. >> >> However, I have only used x86_64-apple-darwin10 for testing, as >> libphobos couldn't be built on that target for the same reason, except >> it was the system linker segfaulting due to long symbol names. >> >> It would be good to know if Solaris has also benefitted from the change. > > great, thanks. I'll give this a whirl once today's regular bootstraps > have finished. here's what I found: the build itself worked just fine and the libphobos test results are identical to those with gas. However, a few gdc tests fail when Solaris/x86 as is used, for two reasons: +UNRESOLVED: gdc.test/runnable/mangle.d compilation failed to produce executable +UNRESOLVED: gdc.test/runnable/mangle.d -shared-libphobos compilation failed to produce executable Assembler: mangle.d "/var/tmp//ccG72ALc.s", line 200 : Syntax error Near line: "movzbl test_эльфийские_письмена_9, %eax" [...] +UNRESOLVED: gdc.test/runnable/testmodule.d compilation failed to produce executable +UNRESOLVED: gdc.test/runnable/testmodule.d -shared-libphobos compilation failed to produce executable Assembler: testmodule.d "/var/tmp//ccw9j5oa.s", line 20 : Syntax error Near line: "call_D7dstress3run17unicode_06_哪里6哪里FiZi" [...] +UNRESOLVED: gdc.test/runnable/ufcs.d compilation failed to produce executable +UNRESOLVED: gdc.test/runnable/ufcs.d -shared-libphobos compilation failed to produce executable Assembler: ufcs.d "/var/tmp//ccWd6kud.s", line 7774 : Syntax error Near line: ".globl _D4ufcs6α8503FiZv" [...] The Solaris assemblers don't support UTF-8 identifiers. Unless gdc can encode them in some way for toolchains like this (no idea if this is worth the effort), it may be possible to guard the tests with the ucn effective-target keyword. Apart from that, it seems strange that the failing tests should only show up as UNSUPPORTED. I'd have expected the compilation to FAIL, but IIRC the gdc testsuite has to ignore all output, so the test for excess errors which would usually catch this is disabled effectively. The last failure is different and due to how COMDAT group handling is done with Solaris as: +UNRESOLVED: gdc.test/runnable/test42.d compilation failed to produce executable +UNRESOLVED: gdc.test/runnable/test42.d -shared-libphobos compilation failed to produce executable which yields Input string too long, limit 10240 The offending input lines are (stripped for brevity) .section.tdata._D6test42__T5Foo71VAyaa2623[...] .group _D6test42__T5Foo71VAyaa2623_68656c6c6f616[...] The first line is 10597 chars, the second even 15869. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [stage1][PATCH] Change semantics of -frecord-gcc-switches and add -frecord-gcc-switches-format.
PING On 12/4/20 2:30 PM, Martin Liška wrote: On 12/4/20 10:03 AM, Richard Biener wrote: Otherwise 0001- looks good to me. Pushed that to master. As said I'd like to see opinions from others on the driver / backend communication for 0002. To be honest, we moved back to the original implementation which used a temporary file. There hasn't been any opinion for last 8 months :( Martin
Re: [PATCH v2] Add --ld-path= to specify an arbitrary executable as the linker
PING^2
On 12/4/20 2:45 PM, Martin Liška wrote:
PING
May I please ping the patch, it's waiting here for a review
for quite some time.
Thanks,
Martin
On 7/23/20 12:17 PM, Martin Liška wrote:
On 7/21/20 6:07 AM, Fangrui Song wrote:
If the value does not contain any path component separator (e.g. a
slash), the linker will be searched for using COMPILER_PATH followed by
PATH. Otherwise, it is either an absolute path or a path relative to the
current working directory.
--ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the
future, we want to make dfferent linker option decisions we can let
-fuse-ld= represent the linker flavor and --ld-path= the linker path.
Hello.
I have just few nits:
=== ERROR type #3: trailing operator (1 error(s)) ===
gcc/collect2.c:1155:14: ld_file_name =
PR driver/93645
* common.opt (--ld-path=): Add --ld-path=
* opts.c (common_handle_option): Handle OPT__ld_path_.
* gcc.c (driver_handle_option): Likewise.
* collect2.c (main): Likewise.
* doc/invoke.texi: Document --ld-path=.
---
Changes in v2:
* Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).
The option does not affect code generation and is not a language feature,
-f* is not suitable. Additionally, clang has other similar --*-path=
options, e.g. --cuda-path=.
---
gcc/collect2.c | 63 +++--
gcc/common.opt | 4 +++
gcc/doc/invoke.texi | 9 +++
gcc/gcc.c | 2 +-
gcc/opts.c | 1 +
5 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..caa1b96ab52 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -844,6 +844,7 @@ main (int argc, char **argv)
const char **ld1;
bool use_plugin = false;
bool use_collect_ld = false;
+ const char *ld_path = NULL;
/* The kinds of symbols we will have to consider when scanning the
outcome of a first pass link. This is ALL to start with, then might
@@ -961,12 +962,21 @@ main (int argc, char **argv)
if (selected_linker == USE_DEFAULT_LD)
selected_linker = USE_PLUGIN_LD;
}
- else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
- selected_linker = USE_BFD_LD;
- else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
- selected_linker = USE_GOLD_LD;
- else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
- selected_linker = USE_LLD_LD;
+ else if (strncmp (argv[i], "-fuse-ld=", 9) == 0
+ && selected_linker != USE_LD_MAX)
+ {
+ if (strcmp (argv[i] + 9, "bfd") == 0)
+ selected_linker = USE_BFD_LD;
+ else if (strcmp (argv[i] + 9, "gold") == 0)
+ selected_linker = USE_GOLD_LD;
+ else if (strcmp (argv[i] + 9, "lld") == 0)
+ selected_linker = USE_LLD_LD;
+ }
+ else if (strncmp (argv[i], "--ld-path=", 10) == 0)
+ {
+ ld_path = argv[i] + 10;
+ selected_linker = USE_LD_MAX;
+ }
else if (strncmp (argv[i], "-o", 2) == 0)
{
/* Parse the output filename if it's given so that we can make
@@ -1117,14 +1127,34 @@ main (int argc, char **argv)
ld_file_name = find_a_file (&cpath, collect_ld_suffix, X_OK);
use_collect_ld = ld_file_name != 0;
}
- /* Search the compiler directories for `ld'. We have protection against
- recursive calls in find_a_file. */
- if (ld_file_name == 0)
- ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], X_OK);
- /* Search the ordinary system bin directories
- for `ld' (if native linking) or `TARGET-ld' (if cross). */
- if (ld_file_name == 0)
- ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker],
X_OK);
+ if (selected_linker == USE_LD_MAX)
+ {
+ /* If --ld-path= does not contain a path component separator, search for
+ the command using cpath, then using path. Otherwise find the linker
+ relative to the current working directory. */
+ if (lbasename (ld_path) == ld_path)
+ {
+ ld_file_name = find_a_file (&cpath, ld_path, X_OK);
+ if (ld_file_name == 0)
+ ld_file_name = find_a_file (&path, ld_path, X_OK);
+ }
+ else if (file_exists (ld_path))
+ {
^^^ these braces are not needed.
+ ld_file_name = ld_path;
+ }
+ }
+ else
+ {
+ /* Search the compiler directories for `ld'. We have protection against
+ recursive calls in find_a_file. */
+ if (ld_file_name == 0)
I would prefer '== NULL'.
+ ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], X_OK);
+ /* Search the ordinary system bin directories
+ for `ld' (if native linking) or `TARGET-ld' (if cross). */
+ if (ld_file_name == 0)
+ ld_file_name =
+ find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
+ }
#ifdef REAL_NM_FILE_NAME
nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME, X_OK);
@@ -1461,6 +1491,11 @@ main (int argc, char **argv)
Re: [PATCH] [AVX512] Fix ICE: Convert integer mask to vector in ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp [PR98537]
On Wed, Jan 06, 2021 at 02:49:13PM +0800, Hongtao Liu wrote:
> ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp are used by vec_cmpmn
> for vector comparison to vector mask, but ix86_expand_sse_cmp(which is
> called in upper 2 functions.) may return integer mask whenever integer
> mask is available, so convert integer mask back to vector mask if
> needed.
>
> gcc/ChangeLog:
>
> PR target/98537
> * config/i386/i386-expand.c (ix86_expand_fp_vec_cmp):
> When cmp is integer mask, convert it to vector.
> (ix86_expand_int_vec_cmp): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> PR target/98537
> * g++.target/i386/avx512bw-pr98537-1.C: New test.
> * g++.target/i386/avx512vl-pr98537-1.C: New test.
> * g++.target/i386/avx512vl-pr98537-2.C: New test.
Do we optimize it then to an AVX/AVX2 comparison if possible?
@@ -4024,8 +4025,18 @@ ix86_expand_fp_vec_cmp (rtx operands[])
cmp = ix86_expand_sse_cmp (operands[0], code, operands[2], operands[3],
operands[1], operands[2]);
- if (operands[0] != cmp)
-emit_move_insn (operands[0], cmp);
+if (operands[0] != cmp)
+{
The indentation of the if above looks wrong.
Otherwise LGTM.
Jakub
Re: [PATCH 0/2] RISC-V: Introduce new architecture extension test macros
Found some build issue on MacOS, will update v2 patches in next few days On Mon, Jan 4, 2021 at 5:49 PM Kito Cheng wrote: > > This patch set introduce new set of architecture extension test macros > which is accept on riscv-c-api-doc[1] recently. > > The motivation of this scheme is have an unify naming scheme for > extension macro and add the capability to checking version. > > [1] > https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md#architecture-extension-test-macro > > >
Re: [PATCH] PR fortran/78746 - invalid access after error recovery
Hi Harald, OK for master? Open branches where testcase class_61.f90 exists? OK for both (as you wrote, this one is really obvious). Best regards Thomas
[PATCH] gimple-isel: Fall back to using vcond_mask [PR98560]
PR98560 is about a case in which the vectoriser initially generates:
mask_1 = a < 0;
mask_2 = mask_1 & ...;
res = VEC_COND_EXPR ;
The vectoriser thus expects res to be calculated using vcond_mask.
However, we later manage to fold mask_2 to mask_1, leaving:
mask_1 = a < 0;
res = VEC_COND_EXPR ;
gimple-isel then required a combined vcond to exist.
On most targets, it's not too onerous to provide all possible
(compare x select) combinations. For each data mode, you just
need to provide unsigned comparisons, signed comparisons, and
floating-point comparisons, with the data mode and type of
comparison uniquely determining the mode of the compared values.
But for targets like SVE that support “unpacked” vectors,
it's not that simple: the level of unpacking adds another
degree of freedom.
Rather than insist that the combined versions exist, I think
we should be prepared to fall back to using separate comparisons
and vcond_masks. I think that makes more sense on targets like
AArch64 and AArch32 in which compares and selects are fundementally
separate operations anyway.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
gcc/
PR tree-optimization/98560
* gimple-isel.cc (gimple_expand_vec_cond_expr): If we fail to use
IFN_VCOND{,U,EQ}, fall back on IFN_VCOND_MASK.
gcc/testsuite/
PR tree-optimization/98560
* gcc.dg/vect/pr98560-1.c: New test.
---
gcc/gimple-isel.cc| 9 -
gcc/testsuite/gcc.dg/vect/pr98560-1.c | 17 +
2 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/pr98560-1.c
diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
index d40338ce4a2..9c07d79a86c 100644
--- a/gcc/gimple-isel.cc
+++ b/gcc/gimple-isel.cc
@@ -254,7 +254,14 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi,
}
}
- gcc_assert (icode != CODE_FOR_nothing);
+ if (icode == CODE_FOR_nothing)
+{
+ gcc_assert (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (op0))
+ && (get_vcond_mask_icode (mode, TYPE_MODE (TREE_TYPE (op0)))
+ != CODE_FOR_nothing));
+ return gimple_build_call_internal (IFN_VCOND_MASK, 3, op0, op1, op2);
+}
+
tree tcode_tree = build_int_cst (integer_type_node, tcode);
return gimple_build_call_internal (unsignedp ? IFN_VCONDU : IFN_VCOND,
5, op0a, op0b, op1, op2, tcode_tree);
diff --git a/gcc/testsuite/gcc.dg/vect/pr98560-1.c
b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
new file mode 100644
index 000..2583fc48f8a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3 -fno-tree-vrp -fno-tree-fre -fno-tree-pre
-fno-code-hoisting -fvect-cost-model=dynamic" } */
+/* { dg-additional-options "-msve-vector-bits=128" { target aarch64_sve } } */
+
+#include
+
+void
+f (uint16_t *restrict dst, uint32_t *restrict src1, float *restrict src2)
+{
+ int i = 0;
+ for (int j = 0; j < 4; ++j)
+{
+ uint16_t tmp = src1[i] >> 1;
+ dst[i] = (uint16_t) (src2[i] < 0 && i < 4 ? tmp : 1);
+ i += 1;
+}
+}
[PATCH] gimple-isel: Check whether IFN_VCONDEQ is supported [PR98560]
This patch follows on from the previous one for the PR and
makes sure that we can handle == as well as <. Previously
we assumed without checking that IFN_VCONDEQ was available
if IFN_VCOND or IFN_VCONDU wasn't.
The patch also fixes the definition of the IFN_VCOND* functions.
The optabs are convert optabs in which the first mode is the
data mode and the second mode is the comparison or mask mode.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
gcc/
PR tree-optimization/98560
* internal-fn.def (IFN_VCONDU, IFN_VCONDEQ): Use type vec_cond.
* internal-fn.c (vec_cond_mask_direct): Get the data mode from
argument 1.
(vec_cond_direct): Likewise argument 2.
(vec_condu_direct, vec_condeq_direct): Delete.
(expand_vect_cond_optab_fn): Rename to...
(expand_vec_cond_optab_fn): ...this, replacing old macro.
(expand_vec_condu_optab_fn, expand_vec_condeq_optab_fn): Delete.
(expand_vect_cond_mask_optab_fn): Rename to...
(expand_vec_cond_mask_optab_fn): ...this, replacing old macro.
(direct_vec_cond_mask_optab_supported_p): Treat the optab as a
convert optab.
(direct_vec_cond_optab_supported_p): Likewise.
(direct_vec_condu_optab_supported_p): Delete.
(direct_vec_condeq_optab_supported_p): Delete.
* gimple-isel.cc: Include internal-fn.h.
(gimple_expand_vec_cond_expr): Check that IFN_VCONDEQ is supported
before using it.
gcc/testsuite/
PR tree-optimization/98560
* gcc.dg/vect/pr98560-2.c: New test.
---
gcc/gimple-isel.cc| 6 +-
gcc/internal-fn.c | 22 ++
gcc/internal-fn.def | 4 ++--
gcc/testsuite/gcc.dg/vect/pr98560-2.c | 17 +
4 files changed, 30 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/pr98560-2.c
diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
index 9c07d79a86c..3ca29191c24 100644
--- a/gcc/gimple-isel.cc
+++ b/gcc/gimple-isel.cc
@@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. If not see
#include "memmodel.h"
#include "optabs.h"
#include "gimple-fold.h"
+#include "internal-fn.h"
/* Expand all ARRAY_REF(VIEW_CONVERT_EXPR) gimple assignments into calls to
internal function based on vector type of selected expansion.
@@ -246,7 +247,10 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi,
Try changing it to NE_EXPR. */
tcode = NE_EXPR;
}
- if (tcode == EQ_EXPR || tcode == NE_EXPR)
+ if ((tcode == EQ_EXPR || tcode == NE_EXPR)
+ && direct_internal_fn_supported_p (IFN_VCONDEQ, TREE_TYPE (lhs),
+TREE_TYPE (op0a),
+OPTIMIZE_FOR_BOTH))
{
tree tcode_tree = build_int_cst (integer_type_node, tcode);
return gimple_build_call_internal (IFN_VCONDEQ, 5, op0a, op0b, op1,
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 996f0fb6c67..dd7173126fb 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -110,10 +110,8 @@ init_internal_fns ()
#define mask_store_direct { 3, 2, false }
#define store_lanes_direct { 0, 0, false }
#define mask_store_lanes_direct { 0, 0, false }
-#define vec_cond_mask_direct { 0, 0, false }
-#define vec_cond_direct { 0, 0, false }
-#define vec_condu_direct { 0, 0, false }
-#define vec_condeq_direct { 0, 0, false }
+#define vec_cond_mask_direct { 1, 0, false }
+#define vec_cond_direct { 2, 0, false }
#define scatter_store_direct { 3, 1, false }
#define len_store_direct { 3, 3, false }
#define vec_set_direct { 3, 3, false }
@@ -2766,7 +2764,7 @@ expand_partial_store_optab_fn (internal_fn, gcall *stmt,
convert_optab optab)
The expansion of STMT happens based on OPTAB table associated. */
static void
-expand_vect_cond_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
+expand_vec_cond_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
{
class expand_operand ops[6];
insn_code icode;
@@ -2802,15 +2800,11 @@ expand_vect_cond_optab_fn (internal_fn, gcall *stmt,
convert_optab optab)
emit_move_insn (target, ops[0].value);
}
-#define expand_vec_cond_optab_fn expand_vect_cond_optab_fn
-#define expand_vec_condu_optab_fn expand_vect_cond_optab_fn
-#define expand_vec_condeq_optab_fn expand_vect_cond_optab_fn
-
/* Expand VCOND_MASK optab internal function.
The expansion of STMT happens based on OPTAB table associated. */
static void
-expand_vect_cond_mask_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
+expand_vec_cond_mask_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
{
class expand_operand ops[4];
@@ -2844,8 +2838,6 @@ expand_vect_cond_mask_optab_fn (internal_fn, gcall *stmt,
convert_optab optab)
emit_move_insn (target, ops[0].value);
}
-#define expand_vec_cond_mask_optab_fn expand_vect_cond_mask_optab_fn
-
/*
Re: [PATCH] genemit: Handle `const_double_zero' rtx
"Maciej W. Rozycki" writes:
> On Wed, 16 Dec 2020, Maciej W. Rozycki wrote:
>
>> > CONST_DOUBLE_ATOF ("0", VOIDmode) seems malformed though, and I'd expect
>> > it to assert in REAL_MODE_FORMAT (via the format_helper constructor).
>> > I'm not sure the patch is strictly safer than the status quo.
>>
>> I may have missed that, though I did follow the chain of calls involved
>> here to see if there is anything problematic. As I say I have a limited
>> way to verify this in practice as the PDP-11 code involved appears to me
>> to be dead, and the situation does not apply to the VAX backend. Maybe I
>> could simulate it somehow artificially to see what happens.
>
> I have made an experiment and arranged for a couple of builtins to refer
> to CONST_DOUBLE_ATOF ("0", VOIDmode) via expanders and it works just fine
> except for failing to match an RTL insn, like:
>
> builtin.c: In function 't':
> builtin.c:18:1: error: unrecognizable insn:
>18 | }
> | ^
> (insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
> (plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
> (reg/v:SF 32 [ f ]))) "builtin.c":5:6 -1
> (nil))
> during RTL pass: vregs
> builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315
>
> so it does work in principle and would produce something if there was a
> matching insn.
Ah, yeah, I'd missed that there was a special case for VOIDmode
in format_helper::format_helper. Still…
>> > FWIW, I agree with Jeff that this ought to be CONST0_RTX (mode).
>>
>> I'll have to update several places then and push the changes through full
>> regression testing, so it'll probably take until the next week.
>
> FWIW, CONST_DOUBLE_ATOF ("0", VOIDmode) is of course not equivalent to
> CONST0_RTX (VOIDmode), as the latter produces a CONST_INT rather than a
> CONST_DOUBLE rtx:
>
> builtin.c: In function 't':
> builtin.c:18:1: error: unrecognizable insn:
>18 | }
> | ^
> (insn 6 3 7 2 (set (reg/v:SF 23 [ f ])
> (plus:SF (const_int 0 [0])
> (reg/v:SF 32 [ f ]))) "builtin1.c":5:6 -1
> (nil))
> during RTL pass: vregs
> builtin.c:18:1: internal compiler error: in extract_insn, at recog.c:2315
>
> I suppose we do not have to support VOIDmode here, but I feel a bit uneasy
> about the lack of identity mapping between machine description (where in
> principle we could already use `const_double' with any mode, not only ones
> CONST0_RTX expands to a CONST_DOUBLE for) and RTL produced if CONST0_RTX
> was used rather than CONST_DOUBLE_ATOF, as CONST0_RTX does not always
> return a CONST_DOUBLE rtx.
Both of the insns above are malformed though. Floating-point
constants have to have floating-point modes. const_ints and
VOIDmode const_doubles are always integers instead.
So even if CONST_DOUBLE_ATOF (…, VOIDmode) fails to ICE, I think it's
a constraint violation in that it's using a floating-point routine to
construct an integer rtx representation.
VOIDmode const_doubles should only be used for integers that cannot
be expressed as a sign-extended HOST_WIDE_INT. So (const_double 0 0)
is an invalid rtx in both integer and FP contexts.
> For the sake of the experiment I modified machine description further so
> as to actually let the rtx produced by CONST_DOUBLE_ATOF ("0", VOIDmode)
> through by providing suitable insns, and here's an excerpt from annotated
> artificial assembly produced:
>
> #(insn 6 21 7 (set (reg/v:SF 0 %r0 [orig:23 f ] [23])
> #(plus:SF (const_double 0 [0] 0 [0] 0 [0] 0 [0])
> #(mem/c:SF (plus:SI (reg/f:SI 12 %ap)
> #(const_int 4 [0x4])) [1 f+0 S4 A32]))) "builtin.c":5:6
> 199 {*addzsf3}
> # (expr_list:REG_DEAD (reg/f:SI 12 %ap)
> #(nil)))
> #addf3 $0,4(%ap),%r0# 6 [c=40] *addzsf3
>
> The CONST_DOUBLE rtx yielded the `$0' operand, so the expression made it
> through the backend down to generated assembly (the leading comment
> character comes from the artificial `*addzsf3' insn in modified MD).
Yeah, unfortunately RTL lacks the sanitary checks that gimple has,
so it's not too surprising that the pattern makes it through to final.
It's still malformed though.
(FTR, the constant should also be the second operand to the plus,
but that's obviously tangential.)
Thanks,
Richard
Re: [PATCH v3] libgcc: Thumb-1 Floating-Point Library for Cortex M0
On 06/01/2021 11:20, Daniel Engel wrote: > Hi Christophe, > > On Wed, Dec 16, 2020, at 9:15 AM, Christophe Lyon wrote: >> On Wed, 2 Dec 2020 at 04:31, Daniel Engel wrote: >>> >>> Hi Christophe, >>> >>> On Thu, Nov 26, 2020, at 1:14 AM, Christophe Lyon wrote: Hi, On Fri, 13 Nov 2020 at 00:03, Daniel Engel wrote: > > Hi, > > This patch adds an efficient assembly-language implementation of IEEE- > 754 compliant floating point routines for Cortex M0 EABI (v6m, thumb- > 1). This is the libgcc portion of a larger library originally > described in 2018: > > https://gcc.gnu.org/legacy-ml/gcc/2018-11/msg00043.html > > Since that time, I've separated the libm functions for submission to > newlib. The remaining libgcc functions in the attached patch have > the following characteristics: > > Function(s) Size (bytes)Cycles > Stack Accuracy > __clzsi242 23 0 > exact > __clzsi2 (OPTIMIZE_SIZE)22 55 0 > exact > __clzdi28+__clzsi2 4+__clzsi2 0 > exact > > __umulsidi3 44 24 0 > exact > __mulsidi3 30+__umulsidi3 24+__umulsidi3 8 > exact > __muldi3 (__aeabi_lmul) 10+__umulsidi3 6+__umulsidi3 0 > exact > __ashldi3 (__aeabi_llsl)22 13 0 > exact > __lshrdi3 (__aeabi_llsr)22 13 0 > exact > __ashrdi3 (__aeabi_lasr)22 13 0 > exact > > __aeabi_lcmp20 13 0 > exact > __aeabi_ulcmp 16 10 0 > exact > > __udivsi3 (__aeabi_uidiv) 56 72 – 3850 > < 1 lsb > __divsi3 (__aeabi_idiv) 38+__udivsi326+__udivsi38 > < 1 lsb > __udivdi3 (__aeabi_uldiv) 164 103 – 1394 > 16 < 1 lsb > __udivdi3 (OPTIMIZE_SIZE) 142 120 – 1392 > 16 < 1 lsb > __divdi3 (__aeabi_ldiv) 54+__udivdi336+__udivdi3 > 32 < 1 lsb > > __shared_float 178 > __shared_float (OPTIMIZE_SIZE) 154 > > __addsf3 (__aeabi_fadd) 116+__shared_float 31 – 76 8 > <= 0.5 ulp > __addsf3 (OPTIMIZE_SIZE)112+__shared_float 74 8 > <= 0.5 ulp > __subsf3 (__aeabi_fsub) 8+__addsf3 6+__addsf3 8 > <= 0.5 ulp > __aeabi_frsub 8+__addsf3 6+__addsf3 8 > <= 0.5 ulp > __mulsf3 (__aeabi_fmul) 112+__shared_float 73 – 97 8 > <= 0.5 ulp > __mulsf3 (OPTIMIZE_SIZE)96+__shared_float 93 8 > <= 0.5 ulp > __divsf3 (__aeabi_fdiv) 132+__shared_float 83 – 3618 > <= 0.5 ulp > __divsf3 (OPTIMIZE_SIZE)120+__shared_float 263 – 359 8 > <= 0.5 ulp > > __cmpsf2/__lesf2/__ltsf272 33 0 > exact > __eqsf2/__nesf2 4+__cmpsf2 3+__cmpsf2 0 > exact > __gesf2/__gesf2 4+__cmpsf2 3+__cmpsf2 0 > exact > __unordsf2 (__aeabi_fcmpun) 4+__cmpsf2 3+__cmpsf2 0 > exact > __aeabi_fcmpeq 4+__cmpsf2 3+__cmpsf2 0 > exact > __aeabi_fcmpne 4+__cmpsf2 3+__cmpsf2 0 > exact > __aeabi_fcmplt 4+__cmpsf2 3+__cmpsf2 0 > exact > __aeabi_fcmple 4+__cmpsf2 3+__cmpsf2 0 > exact > __aeabi_fcmpge 4+__cmpsf2 3+__cmpsf2 0 > exact > > __floatundisf (__aeabi_ul2f)14+__shared_float 40 – 81 8 > <= 0.5 ulp > __floatundisf (OPTIMIZE_SIZE) 14+__shared_float 40 – 2378 > <= 0.5 ulp > __floatunsisf (__aeabi_ui2f)0+__floatundisf 1+__floatundisf 8 > <= 0.5 ulp > __floatdisf (__aeabi_l2f) 14+__floatundisf7+__floatundisf 8 > <= 0.5 ulp > __floatsisf (__aeabi_i2f) 0+__floatdisf 1+__floatdisf 8 > <= 0.5 ulp > > __fixsfdi (__aeabi_f2lz)74
[pushed] c++: Fix g++.dg/warn/Wmismatched-dealloc.C for C++11 [PR98566]
C++ sized deallocation only came in C++14, so this test wasn't
working properly in C++11, which isn't tested by default. Fixed
thus by constraining the dg-errors to C++14 only.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/testsuite/ChangeLog:
PR testsuite/98566
* g++.dg/warn/Wmismatched-dealloc.C: Use target c++14 in
dg-error.
---
gcc/testsuite/g++.dg/warn/Wmismatched-dealloc.C | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc.C
b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc.C
index 682db6f02cb..3072e240e28 100644
--- a/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc.C
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc.C
@@ -12,9 +12,9 @@ void* A (mydealloc, 2) myalloc (void*);
void* A (operator delete, 1)
- bad_new (size_t); // { dg-error "attribute argument 1 is
ambiguous" }
+ bad_new (size_t); // { dg-error "attribute argument 1 is
ambiguous" "" { target c++14 } }
void* A (operator delete[], 1)
- bad_array_new (size_t); // { dg-error "attribute argument 1 is
ambiguous" }
+ bad_array_new (size_t); // { dg-error "attribute argument 1 is
ambiguous" "" { target c++14 } }
void my_delete (const char*, void*);
void my_array_delete (const char*, void*);
base-commit: 6d0b075d662e277a9847f7e8c17d34e7866f0cec
--
2.29.2
[PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause
The Zihintpause extension uses an opcode from the 'fence' opcode range
to add a true hint instruction (i.e. if it is not supported on any
given platform, the 'fence' that is encoded will not enforce any
specific ordering on memory accesses) for entering a low-power state
(e.g. in an idle thread). We expose this new instruction through a
machine-dependent builtin to allow generating it without a requirement
for any inline assembly.
Given that the encoding of 'pause' is valid (as a 'fence' encoding)
even for processors that do not (yet) support Zihintpause, we make
this builtin available without any further TARGET_* constraints.
The new builtin takes no arguments and has no return (void -> void),
which requires a change to maybe_gen_insn; similar builtins w/o
arguments and results in other architectures (e.g. rx_brk) bypass
maybe_gen_insn... making this the first time that nops == 0 is seen
here.
gcc/ChangeLog:
* config/riscv/riscv-builtins.c: add the pause machine-dependent builtin
with no result and no arguments; mark it as always present (pause is a
true hint that encodes into a fence-insn, if not supported with the new
pause semantics).
* config/riscv/riscv-ftypes.def: Add type for void -> void.
* config/riscv/riscv.md: Add risc_pause and UNSPECV_PAUSE
* doc/extend.texi: Document.
* optabs.c (maybe_gen_insn): Allow nops == 0 (void -> void).
gcc/testsuite/ChangeLog:
* gcc.target/riscv/builtin_pause.c: New test.
---
gcc/config/riscv/riscv-builtins.c | 4 +++-
gcc/config/riscv/riscv-ftypes.def | 1 +
gcc/config/riscv/riscv.md | 8
gcc/doc/extend.texi| 4
gcc/optabs.c | 2 ++
gcc/testsuite/gcc.target/riscv/builtin_pause.c | 10 ++
6 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c
diff --git a/gcc/config/riscv/riscv-builtins.c
b/gcc/config/riscv/riscv-builtins.c
index bc959389c76..18b9dc579a1 100644
--- a/gcc/config/riscv/riscv-builtins.c
+++ b/gcc/config/riscv/riscv-builtins.c
@@ -86,6 +86,7 @@ struct riscv_builtin_description {
};
AVAIL (hard_float, TARGET_HARD_FLOAT)
+AVAIL (always, (!0))
/* Construct a riscv_builtin_description from the given arguments.
@@ -129,7 +130,8 @@ AVAIL (hard_float, TARGET_HARD_FLOAT)
static const struct riscv_builtin_description riscv_builtins[] = {
DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),
- DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float)
+ DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
+ DIRECT_NO_TARGET_BUILTIN (pause, RISCV_VOID_FTYPE, always),
};
/* Index I is the function declaration for riscv_builtins[I], or null if the
diff --git a/gcc/config/riscv/riscv-ftypes.def
b/gcc/config/riscv/riscv-ftypes.def
index 1c6bc4e9dce..fcb04db 100644
--- a/gcc/config/riscv/riscv-ftypes.def
+++ b/gcc/config/riscv/riscv-ftypes.def
@@ -27,4 +27,5 @@ along with GCC; see the file COPYING3. If not see
argument type. */
DEF_RISCV_FTYPE (0, (USI))
+DEF_RISCV_FTYPE (0, (VOID))
DEF_RISCV_FTYPE (1, (VOID, USI))
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 254147c112a..b8fb2b8c279 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -69,6 +69,9 @@ (define_c_enum "unspecv" [
;; Stack Smash Protector
UNSPEC_SSP_SET
UNSPEC_SSP_TEST
+
+ ;; Zihintpause unspec
+ UNSPECV_PAUSE
])
(define_constants
@@ -1559,6 +1562,11 @@ (define_insn "fence_i"
"TARGET_ZIFENCEI"
"fence.i")
+(define_insn "riscv_pause"
+ [(unspec_volatile [(const_int 0)] UNSPECV_PAUSE)]
+ ""
+ "pause")
+
;;
;;
;;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e73464a7f19..4cd19c2ebbb 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -22056,6 +22056,10 @@ processors.
Returns the value that is currently set in the @samp{tp} register.
@end deftypefn
+@deftypefn {Built-in Function} void __builtin_riscv_pause (void)
+Generates the @code{pause} (hint) machine instruction.
+@end deftypefn
+
@node RX Built-in Functions
@subsection RX Built-in Functions
GCC supports some of the RX instructions which cannot be expressed in
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 0427063e277..f7a1bf5be1c 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -,6 +,8 @@ maybe_gen_insn (enum insn_code icode, unsigned int nops,
switch (nops)
{
+case 0:
+ return GEN_FCN (icode) ();
case 1:
return GEN_FCN (icode) (ops[0].value);
case 2:
diff --git a/gcc/testsuite/gcc.target/riscv/builtin_pause.c
b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
new file mode 100644
index 000..9250937cabb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options
[PATCH] c++: Fix access checking of scoped non-static member [PR98515]
In the first testcase below, we incorrectly reject the use of the
protected non-static member A::var0 from C::g() because
check_accessibility_of_qualified_id, at template parse time, determines
that the access doesn't go through 'this'. (This happens because the
dependent base B of C doesn't have a binfo object, so it appears
to DERIVED_FROM_P that A is not an indirect base of C.) From there
we create the corresponding deferred access check, which we then
perform at instantiation time and which (unsurprisingly) fails.
The problem ultimately seems to be that we can't, in general, know
whether a use of a scoped non-static member goes through 'this' until
instantiation time, as the second testcase below demonstrates. So this
patch makes check_accessibility_of_qualified_id punt in this situation.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to
commit?
gcc/cp/ChangeLog:
PR c++/98515
* semantics.c (check_accessibility_of_qualified_id): Punt if
we're checking the access of a scoped non-static member at
class template parse time.
gcc/testsuite/ChangeLog:
PR c++/98515
* g++.dg/template/access32.C: New test.
* g++.dg/template/access33.C: New test.
---
gcc/cp/semantics.c | 20 +++-
gcc/testsuite/g++.dg/template/access32.C | 8
gcc/testsuite/g++.dg/template/access33.C | 9 +
3 files changed, 32 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/access32.C
create mode 100644 gcc/testsuite/g++.dg/template/access33.C
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index b448efe024a..f52b2e4d1e7 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2107,14 +2107,24 @@ check_accessibility_of_qualified_id (tree decl,
/* If the reference is to a non-static member of the
current class, treat it as if it were referenced through
`this'. */
- tree ct;
if (DECL_NONSTATIC_MEMBER_P (decl)
- && current_class_ptr
- && DERIVED_FROM_P (scope, ct = current_nonlambda_class_type ()))
- qualifying_type = ct;
+ && current_class_ptr)
+ {
+ if (dependent_type_p (TREE_TYPE (current_class_ptr)))
+ /* In general we can't know whether this access goes through `this'
+until instantiation of the current class. Punt now, or else
+we might create a deferred access check that's relative to the
+wrong class. We'll check this access again after substitution,
+e.g. from tsubst_qualified_id. */
+ return true;
+
+ if (tree current = current_nonlambda_class_type ())
+ if (DERIVED_FROM_P (scope, current))
+ qualifying_type = current;
+ }
/* Otherwise, use the type indicated by the
nested-name-specifier. */
- else
+ if (!qualifying_type)
qualifying_type = nested_name_specifier;
}
else
diff --git a/gcc/testsuite/g++.dg/template/access32.C
b/gcc/testsuite/g++.dg/template/access32.C
new file mode 100644
index 000..08faa9f0f97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access32.C
@@ -0,0 +1,8 @@
+// PR c++/98515
+// { dg-do compile }
+
+struct A { protected: int var0; };
+template struct B : public A { };
+template struct C : public B { void g(); };
+template void C::g() { A::var0++; }
+template class C;
diff --git a/gcc/testsuite/g++.dg/template/access33.C
b/gcc/testsuite/g++.dg/template/access33.C
new file mode 100644
index 000..9fb9b9a1236
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access33.C
@@ -0,0 +1,9 @@
+// PR c++/98515
+// { dg-do compile }
+
+struct A { protected: int var0; };
+template struct B : public A { };
+template struct C : public B { void g(); };
+template void C::g() { A::var0++; } // { dg-error
"protected|invalid" }
+template <> struct B { };
+template class C;
--
2.30.0
[PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
Use __SIZEOF_LONG__ and __SIZEOF_LONG_LONG__ to determine the type of streamoff at compile time instead of _GLIBCXX_HAVE_INT64_T_LONG and _GLIBCXX_HAVE_INT64_T_LONG_LONG. Currently the type of streamoff is determined at libstdc++ configure time, chosen by the definitions of _GLIBCXX_HAVE_INT64_T_LONG and _GLIBCXX_HAVE_INT64_T_LONG_LONG. For a multilib configuration, the difference is encoded in the different multilib header file paths. For "FAT" library targets that package 32 bit and 64 bit libraries together, G++ also expects a single header file directory hierarchy, causing an incorrect value for streamoff in some situations. This patch changes the only use of _GLIBCXX_HAVE_INT64_T_XXX to test __SIZEOF_LONG__ and __SIZEOF__LONG_LONG__ at compile time. Because libstdc++ explicitly probes the type of __int64_t to choose the declaration of streamoff, this patch only performs the dynamic determination if either _GLIBCXX_HAVE_INT64_T_LONG or _GLIBCXX_HAVE_INT64_T_LONG_LONG are defined. In other words, it does assume that if either one is defined, the other is defined, i.e., that __int64_t is a typedef for one of those two types when one or the other is present. But it then dynamically chooses the type based on the size of "long" and "long long" instead of using the configure time value so that the header dynamically adapts to the 32/64 mode of GCC. If neither __GLIBCXX_HAVE_INT64_T_XXX macro is defined, the original logic proceeds, using either __int64_t or "long long". Based on the configure time definitions, the size should be one or the other when one of the macros is defined. I can add an error case if neither __SIZEOF_LONG__ nor __SIZEOF_LONG_LONG__ are 8 despite __GLIBCXX_HAVE_INT64_T_XXX defined. Is this an acceptable solution to determine the value at compile-time? Thanks, David diff --git a/libstdc++-v3/include/bits/postypes.h b/libstdc++-v3/include/bits/postypes.h index cb44cfe1396..81b9c4c6ae5 100644 --- a/libstdc++-v3/include/bits/postypes.h +++ b/libstdc++-v3/include/bits/postypes.h @@ -84,10 +84,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * Note: In versions of GCC up to and including GCC 3.3, streamoff * was typedef long. */ -#ifdef _GLIBCXX_HAVE_INT64_T_LONG +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ +|| defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) + +#if __SIZEOF_LONG__ == 8 typedef long streamoff; -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) +#elif __SIZEOF_LONG_LONG__ == 8 typedef long long streamoff; +#endif + #elif defined(_GLIBCXX_HAVE_INT64_T) typedef int64_t streamoff; #else
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, 6 Jan 2021, David Edelsohn via Gcc-patches wrote: Currently the type of streamoff is determined at libstdc++ configure time, chosen by the definitions of _GLIBCXX_HAVE_INT64_T_LONG and _GLIBCXX_HAVE_INT64_T_LONG_LONG. For a multilib configuration, the difference is encoded in the different multilib header file paths. For "FAT" library targets that package 32 bit and 64 bit libraries together, G++ also expects a single header file directory hierarchy, causing an incorrect value for streamoff in some situations. Shouldn't we change that? I don't see why using the same directory for linking should imply using the same directory for includes. -- Marc Glisse
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 6, 2021 at 1:55 PM Marc Glisse wrote: > > On Wed, 6 Jan 2021, David Edelsohn via Gcc-patches wrote: > > > Currently the type of streamoff is determined at libstdc++ configure > > time, chosen by the definitions of _GLIBCXX_HAVE_INT64_T_LONG and > > _GLIBCXX_HAVE_INT64_T_LONG_LONG. For a multilib configuration, the > > difference is encoded in the different multilib header file paths. > > For "FAT" library targets that package 32 bit and 64 bit libraries > > together, G++ also expects a single header file directory hierarchy, > > causing an incorrect value for streamoff in some situations. > > Shouldn't we change that? I don't see why using the same directory for > linking should imply using the same directory for includes. First, the search path assumption is rather strongly embedded in the GCC driver in somewhat delicate code. There already is a lot of fragile, complicated processing for multilibs and search paths and exception. It is more risky, both in the potential new search logic and in possible breakage, to perform surgery on the multilib support code. Second, it's confusing and error-prone to have different search behavior for different parts of the compiler until we can completely remove multilib headers. Third, there is no inherent reason that the header files should be multilib-dependent. I'm not trying to boil the ocean and remove all multilib differences in headers. I'm trying to solve specific issues caused by the differences in header files that also happen to move GCC in a direction of less multilib differences encoded in header files, which I think is a GOOD THING[tm]. Thanks, David
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 06, 2021 at 01:38:25PM -0500, David Edelsohn via Gcc-patches wrote: > Is this an acceptable solution to determine the value at compile-time? This looks wrong to me. The fact that long is 64-bit doesn't imply that int64_t as defined by stdint.h must be long, it could be long long too. And while e.g. for C it doesn't really matter much whether streamoff will be long or long long if those two have the same precision, for C++ it matters a lot (affects mangling etc.). > diff --git a/libstdc++-v3/include/bits/postypes.h > b/libstdc++-v3/include/bits/postypes.h > index cb44cfe1396..81b9c4c6ae5 100644 > --- a/libstdc++-v3/include/bits/postypes.h > +++ b/libstdc++-v3/include/bits/postypes.h > @@ -84,10 +84,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > * Note: In versions of GCC up to and including GCC 3.3, streamoff > * was typedef long. >*/ > -#ifdef _GLIBCXX_HAVE_INT64_T_LONG > +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ > +|| defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > + > +#if __SIZEOF_LONG__ == 8 >typedef long streamoff; > -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > +#elif __SIZEOF_LONG_LONG__ == 8 >typedef long long streamoff; > +#endif > + > #elif defined(_GLIBCXX_HAVE_INT64_T) >typedef int64_t streamoff; > #else Jakub
Re: [RFC] [avr] Toolchain Integration for Testsuite Execution (avr cc0 to mode_cc0 conversion)
On Tue, 5 Jan 2021 at 20:24, Jeff Law wrote: > > > On 1/5/21 10:54 AM, Rainer Orth wrote: > > > > I fear I'm a bit lost here myself. I do have a little experience > > running various builders: > > > > * I inherited a Golang one on Solaris/amd64 (based on their own builder > > infrastructure). > > > > * I do run builders for GDB (mostly dormant since Sergio left RedHat) > > and LLVM on Solaris/amd64 and sparcv9 (both using buildbot). > > > > In all three cases the projects provide documentation how to configure > > your own builders and add them to the infrastructure. Is something like > > this possible for the GCC Jenkins (say adding Solaris builders) and if > > so how? Or would one need to setup one's own instance, in which case it > > would be extremely helpful to learn the necessary config: doing > > something like this from scratch is a major effort, as seen in Paul > > Matos' effort (also buildbot-based) of a couple of years ago. > We don't have any procedures in place for this (yet). I'd like to add > them, but I'm swamped. > Ok, I've done the first step, possibly enough folks is interested to get this done: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98574 I'm certainly open to having others contribute here. As a long standing > member of the community I'd be happy to set up an account for you so you > could wire in a sparc/solaris system executor and set up the build scripts. > > Jeff > >
[pushed] testsuite, coroutines : Fix a bad testcase [PR96504].
Hi
Where possible (i.e. where that doesn't alter the intent of a test) we
use a suspend_always as the final suspend and a test that the coroutine
was 'done' to check that the state machine had terminated correctly.
Sometimes, filed PRs have 'suspend_never' as the final suspend expression
and that needs to be changed to match the testsuite style. This is one
I missed and means that the call to 'done()' on the handle is made to an
already-destructed coroutine. Surprisngly, that didn't actually trigger
a failure until glibc 2-32.
Fixed by changing the final suspend to be 'suspend_always’.
tested on x86_64-linux-gnu with valgrind (since the problem does not show
on most versions of glibc).
pushed to master,
thanks
Iain
gcc/testsuite/ChangeLog:
PR c++/96504
* g++.dg/coroutines/torture/pr95519-05-gro.C: Use suspend_always
as the final suspend point so that we can check that the state
machine has reached the expected point.
---
gcc/testsuite/g++.dg/coroutines/torture/pr95519-05-gro.C | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr95519-05-gro.C
b/gcc/testsuite/g++.dg/coroutines/torture/pr95519-05-gro.C
index fbbce97ed17..2e7218371bc 100644
--- a/gcc/testsuite/g++.dg/coroutines/torture/pr95519-05-gro.C
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr95519-05-gro.C
@@ -5,7 +5,7 @@
struct pt_b
{
std::suspend_always initial_suspend() const noexcept { return {}; }
- std::suspend_never final_suspend() const noexcept { return {}; }
+ std::suspend_always final_suspend() const noexcept { return {}; }
constexpr void return_void () noexcept {};
constexpr void unhandled_exception() const noexcept {}
};
--
2.24.1
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 6, 2021 at 2:37 PM Jakub Jelinek wrote: > > On Wed, Jan 06, 2021 at 01:38:25PM -0500, David Edelsohn via Gcc-patches > wrote: > > Is this an acceptable solution to determine the value at compile-time? > > This looks wrong to me. The fact that long is 64-bit doesn't imply that > int64_t as defined by stdint.h must be long, it could be long long too. > And while e.g. for C it doesn't really matter much whether streamoff > will be long or long long if those two have the same precision, > for C++ it matters a lot (affects mangling etc.). Your response doesn't correspond to the patch nor to what I described. Nowhere did I say that int64_t must correspond to "long". The patch specifically chooses "long" or "long long" based on the __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. Currently libstdc++ configure tests for the type at configuration time. My patch changes the behavior to retain the test for the type at configure time but chooses "long" or "long long" at compile time. I don't unilaterally choose "long" or "long long" as the type, but rely on the configure test to ensure that __int64_t is a typedef for either "long" or "long long". The patch does assume that if __int64_t is a typedef for "long" or "long long" and this is a 32/64 multilib, then the typedef for the other 32/64 mode is an equivalent typedef, which is the case for GLIBC, AIX, and other systems that I have checked. Thanks, David > > > diff --git a/libstdc++-v3/include/bits/postypes.h > > b/libstdc++-v3/include/bits/postypes.h > > index cb44cfe1396..81b9c4c6ae5 100644 > > --- a/libstdc++-v3/include/bits/postypes.h > > +++ b/libstdc++-v3/include/bits/postypes.h > > @@ -84,10 +84,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > * Note: In versions of GCC up to and including GCC 3.3, streamoff > > * was typedef long. > >*/ > > -#ifdef _GLIBCXX_HAVE_INT64_T_LONG > > +#if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ > > +|| defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > > + > > +#if __SIZEOF_LONG__ == 8 > >typedef long streamoff; > > -#elif defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) > > +#elif __SIZEOF_LONG_LONG__ == 8 > >typedef long long streamoff; > > +#endif > > + > > #elif defined(_GLIBCXX_HAVE_INT64_T) > >typedef int64_t streamoff; > > #else > > Jakub >
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 06, 2021 at 04:20:22PM -0500, David Edelsohn wrote: > Your response doesn't correspond to the patch nor to what I described. > > Nowhere did I say that int64_t must correspond to "long". The patch > specifically chooses "long" or "long long" based on the > __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. > > Currently libstdc++ configure tests for the type at configuration > time. My patch changes the behavior to retain the test for the type > at configure time but chooses "long" or "long long" at compile time. > I don't unilaterally choose "long" or "long long" as the type, but > rely on the configure test to ensure that __int64_t is a typedef for > either "long" or "long long". > > The patch does assume that if __int64_t is a typedef for "long" or > "long long" and this is a 32/64 multilib, then the typedef for the > other 32/64 mode is an equivalent typedef, which is the case for > GLIBC, AIX, and other systems that I have checked. Yes, on glibc it is the case, but it doesn't have to be the case on other OSes, which is why there is a configure check. Other OSes can typedef int64_t to whatever they want (or what somebody choose years ago and is now an ABI issue). So, if you wanted to do this, you'd need to check at configure time both multilibs and determine what to do for both. I don't really understand the problem you are trying to solve, because normally the c++config.h header that defines _GLIBCXX_HAVE_INT64_T_LONG etc. comes from a multilib specific header directory, e.g. on powerpc64-linux, one has: /usr/include/c++/11/powerpc64-unknown-linux-gnu/bits/c++config.h and /usr/include/c++/11/powerpc64-unknown-linux-gnu/32/bits/c++config.h (or instead /64/, depending on which multilib is the default) and g++ driver arranges for the search paths to first look at the multilib specific subdirectory and only later at the generic one. Jakub
[committed] patch to fix PR97978
The following fixes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97978
The patch was successfully bootstrapped on x86-64.
commit fbf9b2b634e376516cd21d7aa92ef3fd5778aa10 (HEAD -> master)
Author: Vladimir N. Makarov
Date: Wed Jan 6 14:48:53 2021 -0500
[PR97978] LRA: Permit temporary allocation incorrectness after hard reg split.
LRA can crash when a hard register was split and the same hard register
was assigned on the previous assignment sub-pass. The following
patch fixes this problem.
gcc/ChangeLog:
PR rtl-optimization/97978
* lra-int.h (lra_hard_reg_split_p): New external.
* lra.c (lra_hard_reg_split_p): New global.
(lra): Set up lra_hard_reg_split_p after splitting a hard reg.
* lra-assigns.c (lra_assign): Don't check allocation correctness
after hard reg splitting.
gcc/testsuite/ChangeLog:
PR rtl-optimization/97978
* gcc.target/i386/pr97978.c: New.
diff --git a/gcc/lra-assigns.c b/gcc/lra-assigns.c
index 9335e4c876e..c6a941fe663 100644
--- a/gcc/lra-assigns.c
+++ b/gcc/lra-assigns.c
@@ -1636,10 +1636,11 @@ lra_assign (bool &fails_p)
bitmap_initialize (&all_spilled_pseudos, ®_obstack);
create_live_range_start_chains ();
setup_live_pseudos_and_spill_after_risky_transforms (&all_spilled_pseudos);
- if (! lra_asm_error_p && flag_checking)
-/* Check correctness of allocation for call-crossed pseudos but
- only when there are no asm errors as in the case of errors the
- asm is removed and it can result in incorrect allocation. */
+ if (! lra_hard_reg_split_p && ! lra_asm_error_p && flag_checking)
+/* Check correctness of allocation but only when there are no hard reg
+ splits and asm errors as in the case of errors explicit insns involving
+ hard regs are added or the asm is removed and this can result in
+ incorrect allocation. */
for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
if (lra_reg_info[i].nrefs != 0
&& reg_renumber[i] >= 0
diff --git a/gcc/lra-int.h b/gcc/lra-int.h
index 75ba6560bcc..1b8f7b6ae61 100644
--- a/gcc/lra-int.h
+++ b/gcc/lra-int.h
@@ -273,6 +273,7 @@ typedef class lra_insn_recog_data *lra_insn_recog_data_t;
extern FILE *lra_dump_file;
+extern bool lra_hard_reg_split_p;
extern bool lra_asm_error_p;
extern bool lra_reg_spill_p;
diff --git a/gcc/lra.c b/gcc/lra.c
index 380a21ac2ac..aa49de6f154 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -2211,6 +2211,9 @@ bitmap_head lra_subreg_reload_pseudos;
/* File used for output of LRA debug information. */
FILE *lra_dump_file;
+/* True if we split hard reg after the last constraint sub-pass. */
+bool lra_hard_reg_split_p;
+
/* True if we found an asm error. */
bool lra_asm_error_p;
@@ -2359,6 +2362,7 @@ lra (FILE *f)
if (live_p)
lra_clear_live_ranges ();
bool fails_p;
+ lra_hard_reg_split_p = false;
do
{
/* We need live ranges for lra_assign -- so build them.
@@ -2403,6 +2407,7 @@ lra (FILE *f)
live_p = false;
if (! lra_split_hard_reg_for ())
break;
+ lra_hard_reg_split_p = true;
}
}
while (fails_p);
diff --git a/gcc/testsuite/gcc.target/i386/pr97978.c b/gcc/testsuite/gcc.target/i386/pr97978.c
new file mode 100644
index 000..263bca8708d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr97978.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fno-PIC" } */
+int sg;
+long int kk;
+
+void
+bp (int jz, int tj, long int li)
+{
+ if (jz == 0 || tj == 0)
+__builtin_unreachable ();
+
+ kk = li;
+}
+
+void
+qp (void)
+{
+ ++kk;
+
+ for (;;)
+bp (1l / sg, 0, ~0u);
+}
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 6, 2021 at 4:42 PM Jakub Jelinek wrote: > > On Wed, Jan 06, 2021 at 04:20:22PM -0500, David Edelsohn wrote: > > Your response doesn't correspond to the patch nor to what I described. > > > > Nowhere did I say that int64_t must correspond to "long". The patch > > specifically chooses "long" or "long long" based on the > > __SIZEOF_LONG__ *and* __SIZEOF_LONG_LONG__. > > > > Currently libstdc++ configure tests for the type at configuration > > time. My patch changes the behavior to retain the test for the type > > at configure time but chooses "long" or "long long" at compile time. > > I don't unilaterally choose "long" or "long long" as the type, but > > rely on the configure test to ensure that __int64_t is a typedef for > > either "long" or "long long". > > > > The patch does assume that if __int64_t is a typedef for "long" or > > "long long" and this is a 32/64 multilib, then the typedef for the > > other 32/64 mode is an equivalent typedef, which is the case for > > GLIBC, AIX, and other systems that I have checked. > > Yes, on glibc it is the case, but it doesn't have to be the case on other > OSes, which is why there is a configure check. Other OSes can typedef > int64_t to whatever they want (or what somebody choose years ago and is now > an ABI issue). > So, if you wanted to do this, you'd need to check at configure time both > multilibs and determine what to do for both. You continue to not respond to the actual patch and to raise issues that don't exist in the actual patch. libstdc++ configure tests if __int64_t has any type, and separately tests if __int64_t uses typedef "long" or "long long", setting separate macros. The patch uses __SIZEOF_LONG__ and __SIZEOF_LONG_LONG__ if _GLIBCXX_HAVE_INT64_T_LONG or _GLIBCXX_HAVE_INT64_T_LONG_LONG is defined -- exactly for the situation where configure already has determined that __int64_t is either "long" or "long long" and not some other definition or type. If those are not defined, the patch falls back to the current, existing behavior which uses __int64_t, if __int64_t is defined, or "long long" if nothing is defined. This is exactly how the current code behaves if __int64_t is not "long" nor "long long". So, exactly as you state, __int64_t could be a different typedef and the patch continues to use that typedef if the OS didn'f use "long" or "long long". What is not clear about that and how does that not address your concern? > > I don't really understand the problem you are trying to solve, because > normally the c++config.h header that defines _GLIBCXX_HAVE_INT64_T_LONG etc. > comes from a multilib specific header directory, e.g. on powerpc64-linux, > one has: > /usr/include/c++/11/powerpc64-unknown-linux-gnu/bits/c++config.h > and > /usr/include/c++/11/powerpc64-unknown-linux-gnu/32/bits/c++config.h > (or instead /64/, depending on which multilib is the default) and > g++ driver arranges for the search paths to first look at the multilib > specific subdirectory and only later at the generic one. AIX uses what Darwin calls "FAT" libraries. A single archive, in the case of AIX, contains both 32 bit and 64 bit objects and shared objects. Darwin places the two shared objects into another special file, but it's the same effect. On AIX there is (or should be) one libstdc++.a, for both ppc32 and ppc64. (pthreads suppose is a separate multilib axis.) The fact that GCC historically uses multilibs is a problem. Also, when AIX added 64 bit multilibs, 32 bit was not defined as its own multilib separate from the "native" library. There historically has been a ppc64 multilib subdirectory but not a ppc32 multilib subdirectory. GCC on AIX now is being enhanced so that GCC itself can be built as a 32 bit or 64 bit application. This means that the "native" library is either 32 bit for GCC32 or 64 bit for GCC64. Ideally GCC32 and GCC64 should be able to create 32 bit and 64 bit applications that interoperate, but because of the multilib differences, they look in different locations for libraries. If GCC followed normal AIX behavior and placed all 32 bit and 64 bit shared objects into the same archive and same directory, the interoperability issue would not exist. Currently GCC32 apps look in the top-level directory and GCC32 -m64 look in the ppc64 multilib, but GCC64 apps look in the top-level directory and GCC64 -m32 apps look in the ppc32 multilib. Depending on which toolchain is used to build a 32 bit or 64 bit application, it will look for shared objects in different locations. AIX does not have a /lib64 or /lib32 directory, there only is /lib because both object modes can co-exist. The effort last year builds all of the multilibs but places the objects and shared objects into the same archive. And changes to MULTILIB_MATCHES teaches GCC to look for both multilibs in the same directory, matching AIX behavior. The remaining issue is GCC also looks for headers in the same relative multilib directories. Instructing GCC to loo
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Wed, Jan 06, 2021 at 06:01:23PM -0500, David Edelsohn wrote:
> You continue to not respond to the actual patch and to raise issues
> that don't exist in the actual patch.
We are talking past each other.
Consider an OS that has in stdint.h
typedef long long int64_t;
supports 32-bit and 64-bit multilibs and has the usual type sizes,
i.e. 32-bit int, 32/64-bit long and 64-bit long long.
On such a target, libstdc++ configury will define
_GLIBCXX_HAVE_INT64_T_LONG_LONG for both 32-bit and 64-bit multilib,
and without your patch will typedef long long streamoff;
for both the multilibs, so e.g.
void bar (streamoff) {} will mangle as _Z3barx on both.
Now, with your patch, _GLIBCXX_HAVE_INT64_T_LONG_LONG is defined,
but on the 64-bit multilib __SIZEOF_LONG__ is 8 and so you
instead typedef long streamoff; for the 64-bit multilib (and keep
typedef long long streamoff; for the 32-bit one).
The function that previously mangled _Z3barx now mangles _Z3barl,
so the ABI broke.
Anyway, I'll defer to Jonathan who is the libstdc++ maintainer.
Jakub
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
On Thu, Jan 07, 2021 at 12:39:39AM +0100, Jakub Jelinek wrote: > We are talking past each other. > > Consider an OS that has in stdint.h > typedef long long int64_t; And, from grepping INT64_TYPE in config/* config/*/* it isn't just theoretic, Darwin and OpenBSD behave that way. Jakub
Re: [PATCH, libstdc++] GLIBCXX_HAVE_INT64_T
Thanks for clarifying the issue. As you implicitly point out, GCC knows the type of INT64 and defines the macro __INT64_TYPE__ . The revised code can use that directly, such as: #if defined(_GLIBCXX_HAVE_INT64_T_LONG) \ || defined(_GLIBCXX_HAVE_INT64_T_LONG_LONG) typedef __INT64_TYPE__ streamoff; #elif defined(_GLIBCXX_HAVE_INT64_T) typedef int64_t streamoff; #else typedef long long streamoff; #endif Are there any additional issues not addressed by that approach, other than possible further simplification? Thanks, David On Wed, Jan 6, 2021 at 6:45 PM Jakub Jelinek wrote: > > On Thu, Jan 07, 2021 at 12:39:39AM +0100, Jakub Jelinek wrote: > > We are talking past each other. > > > > Consider an OS that has in stdint.h > > typedef long long int64_t; > > And, from grepping INT64_TYPE in config/* config/*/* > it isn't just theoretic, Darwin and OpenBSD behave that way. > > Jakub >
Re: [PATCH v3] libgcc: Thumb-1 Floating-Point Library for Cortex M0
--snip-- On Wed, Jan 6, 2021, at 9:05 AM, Richard Earnshaw wrote: > > Thanks for working on this, Daniel. > > This is clearly stage1 material, so we've got time for a couple of > iterations to sort things out. I appreciate your feedback. I had been hoping that with no regressions this might still be eligible for stage2. Christophe never indicated either way. but the fact that he was looking at it seemed positive. I thought I would be a couple of weeks faster with this last iteration, but holidays got in the way. I actually think your comments below could all be addressable within a couple of days. But, I'm not accounting for the review process. > Firstly, the patch is very large, but contains a large number of > distinct changes, so it would really benefit from being broken down into > a number of distinct patches. This will make reviewing the individual > changes much more straight-forward. I have no context for "large" or "small" with respect to gcc. This patch comprises about 30% of a previously-monolithic library that's been shipping since ~2016 (the rest is libm material). Other than (1) the aforementioned change to div0(), (2) a nascent adaptation for __truncdfsf2() (not enabled), and (3) the gratuitous addition of the bitwise functions, the library remains pretty much as it was originally released. The driving force in the development of this library was small size, which of course was never possible with the softfp routines. It's not half-slow, either, for the limitations of the M0 architecture. And, it's IEEE compliant. But, that means that most of the functions are highly interconnected. So, some of it can be broken up as you outline below, but that last patch is still worth more than half of the total. I also have ~70k lines of test vectors that seem mostly redundant, but not completely. I haven't decided what to do here. For example, I have coverage for __aeabi_u/ldivmod, while GCC does not. If I do anything with this code it will be in a separate thread. > I'd suggest: > > 1) Some basic makefile cleanups to ease initial integration - in > particular where we have things like > > LIB1FUNCS += > > that this be rewritten with one function per line (and sorted > alphabetically) - then we can see which functions are being changed in > subsequent patches. It makes the Makefile fragments longer, but the > improvement in clarity for makes this worthwhile. I know next to nothing about Makefiles, particularly ones as complex as GCC's. I was just trying to work with the existing style to avoid breaking something. However, I can certainly adopt this suggestion. > 2) The changes for the existing integer functions - preferably one > function per patch. > > 3) The new integer functions that you're adding These wouldn't be too hard to do, but what are the expectations for testing? A clean build of GCC takes about 6 hours in my VM, and regression testing takes about 4 hours per architecture. You would want a full regression report for each incremental patch? I have no idea how to target regression tests that apply to particular runtime functions without the risk of missing something. > 4) The floating-point support. > > Some more general observations: > > - where functions are already in lib1funcs.asm, please leave them there. I guess I have a different vision here. I have had a really hard time following all of the nested #ifdefs in lib1funcs, so I thought it would be helpful to begin breaking it up into logical units. The functions removed were all functions for which I had THUMB1 sequences faster/smaller than lib1funcs: __clzsi2, __clzdi2, __ctzsi2, __ashrdi3, __lshrdi3, __ashldi3. In fact, the new THUMB1 of __clzsi2 is the same number of instructions as the previous ARM/THUMB2 version. You will find all of the previous ARM versions of these functions merged into the new files (with attribution) and the same preprocessor selection path. So no architecture variant should be any worse off than before this patch, and some beyond v6m should benefit. In the future, I think that my versions of __divsi3 and __divdi3 will prove faster/smaller than the existing THUMB2 versions. I know that my float routines are less than half the compiled size of THUMB2 versions in 'ieee754-sf.S'. However, I haven't profiled the exact performance differences so I have left all this work for future patches. (It's also quite likely that my version can be further-refined with a few judicious uses of THUMB2 alternatives.) My long-term vision would be use lib1funcs as an architectural wrapper distinct from the implementation code. > - lets avoid having the cm0 subdirectory - in particular we should do > this when there is existing code for other targets in the same source > files. It's OK to have any new files in the main 'arm' directory of the > source tree - just name the files appropriately if really needed. Fair point on the name. In v1 of this patch, all these files were a
[committed] analyzer: fix missing bitmap_clear [PR98564]
Fix verified using valgrind.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-6512-gcffe6dd2ce358c2cb550c9fb3c57cec65eee1c93.
gcc/analyzer/ChangeLog:
PR analyzer/98564
* engine.cc (exploded_path::feasible_p): Add missing call to
bitmap_clear.
gcc/testsuite/ChangeLog:
PR analyzer/98564
* gcc.dg/analyzer/pr98564.c: New test.
---
gcc/analyzer/engine.cc | 1 +
gcc/testsuite/gcc.dg/analyzer/pr98564.c | 6 ++
2 files changed, 7 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr98564.c
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 3ea4524bd65..8bc9adf5ee6 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -3374,6 +3374,7 @@ exploded_path::feasible_p (logger *logger,
feasibility_problem **out,
LOG_SCOPE (logger);
auto_sbitmap snodes_visited (eg->get_supergraph ().m_nodes.length ());
+ bitmap_clear (snodes_visited);
/* Traverse the path, updating this model. */
region_model model (eng->get_model_manager ());
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr98564.c
b/gcc/testsuite/gcc.dg/analyzer/pr98564.c
new file mode 100644
index 000..74b1abec6bf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr98564.c
@@ -0,0 +1,6 @@
+void *calloc (__SIZE_TYPE__, __SIZE_TYPE__);
+
+void test_1 (void)
+{
+ int *p = calloc (0, 1); /* { dg-message "allocated here" } */
+} /* { dg-warning "leak of 'p'" } */
--
2.26.2
[committed] analyzer: fix false leak reports when merging states [PR97074]
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-6513-gbe6c485b24f2b47ac85380dd2bea025d353f1f91.
gcc/analyzer/ChangeLog:
PR analyzer/97074
* store.cc (binding_cluster::can_merge_p): Add "out_store" param
and pass to calls to binding_cluster::make_unknown_relative_to.
(binding_cluster::make_unknown_relative_to): Add "out_store"
param. Use it to mark base regions that are pointed to by
pointers that become unknown as having escaped.
(store::can_merge_p): Pass out_store to
binding_cluster::can_merge_p.
* store.h (binding_cluster::can_merge_p): Add "out_store" param.
(binding_cluster::make_unknown_relative_to): Likewise.
* svalue.cc (region_svalue::implicitly_live_p): New vfunc.
* svalue.h (region_svalue::implicitly_live_p): New vfunc decl.
gcc/testsuite/ChangeLog:
PR analyzer/97074
* gcc.dg/analyzer/pr97074.c: New test.
---
gcc/analyzer/store.cc | 23 +++---
gcc/analyzer/store.h| 2 ++
gcc/analyzer/svalue.cc | 16 +
gcc/analyzer/svalue.h | 2 ++
gcc/testsuite/gcc.dg/analyzer/pr97074.c | 32 +
5 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr97074.c
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index b4a4d5f3bb2..23118d05685 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -1177,6 +1177,7 @@ bool
binding_cluster::can_merge_p (const binding_cluster *cluster_a,
const binding_cluster *cluster_b,
binding_cluster *out_cluster,
+ store *out_store,
store_manager *mgr,
model_merger *merger)
{
@@ -1197,14 +1198,14 @@ binding_cluster::can_merge_p (const binding_cluster
*cluster_a,
{
gcc_assert (cluster_b != NULL);
gcc_assert (cluster_b->m_base_region == out_cluster->m_base_region);
- out_cluster->make_unknown_relative_to (cluster_b, mgr);
+ out_cluster->make_unknown_relative_to (cluster_b, out_store, mgr);
return true;
}
if (cluster_b == NULL)
{
gcc_assert (cluster_a != NULL);
gcc_assert (cluster_a->m_base_region == out_cluster->m_base_region);
- out_cluster->make_unknown_relative_to (cluster_a, mgr);
+ out_cluster->make_unknown_relative_to (cluster_a, out_store, mgr);
return true;
}
@@ -1298,6 +1299,7 @@ binding_cluster::can_merge_p (const binding_cluster
*cluster_a,
void
binding_cluster::make_unknown_relative_to (const binding_cluster *other,
+ store *out_store,
store_manager *mgr)
{
for (map_t::iterator iter = other->m_map.begin ();
@@ -1309,6 +1311,21 @@ binding_cluster::make_unknown_relative_to (const
binding_cluster *other,
= mgr->get_svalue_manager ()->get_or_create_unknown_svalue
(iter_sval->get_type ());
m_map.put (iter_key, unknown_sval);
+
+ /* For any pointers in OTHER, the merger means that the
+concrete pointer becomes an unknown value, which could
+show up as a false report of a leak when considering what
+pointers are live before vs after.
+Avoid this by marking the base regions they point to as having
+escaped. */
+ if (const region_svalue *region_sval
+ = iter_sval->dyn_cast_region_svalue ())
+ {
+ const region *base_reg
+ = region_sval->get_pointee ()->get_base_region ();
+ binding_cluster *c = out_store->get_or_create_cluster (base_reg);
+ c->mark_as_escaped ();
+ }
}
}
@@ -2092,7 +2109,7 @@ store::can_merge_p (const store *store_a, const store
*store_b,
binding_cluster *out_cluster
= out_store->get_or_create_cluster (base_reg);
if (!binding_cluster::can_merge_p (cluster_a, cluster_b,
-out_cluster, mgr, merger))
+out_cluster, out_store, mgr, merger))
return false;
}
return true;
diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h
index 2f97f4b68a5..366439ce2dd 100644
--- a/gcc/analyzer/store.h
+++ b/gcc/analyzer/store.h
@@ -434,9 +434,11 @@ public:
static bool can_merge_p (const binding_cluster *cluster_a,
const binding_cluster *cluster_b,
binding_cluster *out_cluster,
+ store *out_store,
store_manager *mgr,
model_merger *merger);
void make_unknown_relative_to (const binding_cluster *other_cluster,
+store *out_store,
store_manager *mgr);
void mark_as_escaped
Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause
Hi Philipp:
Could you add zihintpause to -march parser and guard that on the
pattern and builtin like zifencei[1-2]?
And could you sent a PR to
https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md to
mention __builtin_riscv_pause?
Thanks!
[1] march parser change:
https://github.com/gcc-mirror/gcc/commit/b03be74bad08c382da47e048007a78fa3fb4ef49
[2] Default version for ext.:
https://github.com/gcc-mirror/gcc/commit/4b81528241ca682025d92558ff6aeec91dafdca8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void test_pause()
I would suggest you change the function name in the testcase,
otherwise the scan-assembler test will always pass even if you didn't
generate "pause" instruction.
> +{
> + __builtin_riscv_pause ();
> +}
> +
> +/* { dg-final { scan-assembler "pause" } } */
> +
> --
> 2.18.4
>
Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause
I've got a contrary opinion:
Since HINTs are guaranteed to execute as no-ops--e.g., this one is
just a FENCE instruction, which is already a mandatory part of the
base ISA--they don't _need_ to be called out as separate extensions in
the toolchain.
Although there's nothing fundamentally wrong with Kito's suggestion,
it seems like an extra hoop to jump through without commensurate
benefit. I see no reason to restrict the use of __builtin_pause,
since all RISC-V implementations, including old ones, are required to
support it. And, of course, that's the reason we encoded it this way
:)
On Wed, Jan 6, 2021 at 7:35 PM Kito Cheng wrote:
>
> Hi Philipp:
>
> Could you add zihintpause to -march parser and guard that on the
> pattern and builtin like zifencei[1-2]?
>
> And could you sent a PR to
> https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md to
> mention __builtin_riscv_pause?
>
> Thanks!
>
> [1] march parser change:
> https://github.com/gcc-mirror/gcc/commit/b03be74bad08c382da47e048007a78fa3fb4ef49
> [2] Default version for ext.:
> https://github.com/gcc-mirror/gcc/commit/4b81528241ca682025d92558ff6aeec91dafdca8
>
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
> > @@ -0,0 +1,10 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +void test_pause()
>
> I would suggest you change the function name in the testcase,
> otherwise the scan-assembler test will always pass even if you didn't
> generate "pause" instruction.
>
>
> > +{
> > + __builtin_riscv_pause ();
> > +}
> > +
> > +/* { dg-final { scan-assembler "pause" } } */
> > +
> > --
> > 2.18.4
> >
Re: [PATCH] [AVX512] Fix ICE: Convert integer mask to vector in ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp [PR98537]
On Wed, Jan 6, 2021 at 10:39 PM Jakub Jelinek wrote:
>
> On Wed, Jan 06, 2021 at 02:49:13PM +0800, Hongtao Liu wrote:
> > ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp are used by vec_cmpmn
> > for vector comparison to vector mask, but ix86_expand_sse_cmp(which is
> > called in upper 2 functions.) may return integer mask whenever integer
> > mask is available, so convert integer mask back to vector mask if
> > needed.
> >
> > gcc/ChangeLog:
> >
> > PR target/98537
> > * config/i386/i386-expand.c (ix86_expand_fp_vec_cmp):
> > When cmp is integer mask, convert it to vector.
> > (ix86_expand_int_vec_cmp): Ditto.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR target/98537
> > * g++.target/i386/avx512bw-pr98537-1.C: New test.
> > * g++.target/i386/avx512vl-pr98537-1.C: New test.
> > * g++.target/i386/avx512vl-pr98537-2.C: New test.
>
> Do we optimize it then to an AVX/AVX2 comparison if possible?
>
When i'm looking at the code, i find there's other places which
require comparison dest to be vector(i.e. ix86_expand_sse_unpack,
ix86_expand_mul_widen_evenodd). It's a potential bug.
So I fix this bug in another way which won't generate an integer mask
when the comparison dest is required to a vector mask.
Update patch:
ix86_expand_sse_cmp/ix86_expand_int_sse_cmp is used for vector
comparison, considering that avx512 introduces integer mask, but some
original callers require the dest of comparison to be a vector.
So add a new parameter vector_mask_p to control the result
of vector comparison to be vector or not.
regtested/bootstrapped on x86_64-linux-gnu{-m32,}.
gcc/ChangeLog:
PR target/98537
* config/i386/i386-expand.c (ix86_expand_sse_cmp): Add a new
parameter vector_mask_p to control whether the comparison
result should be a vector or not.
(ix86_expand_int_sse_cmp): Ditto.
(ix86_expand_sse_movcc): cmpmode should be MODE_INT.
(ix86_expand_fp_movcc): Allow vector comparison dest as
integer mask.
(ix86_expand_fp_vcond): Ditto.
(ix86_expand_int_vcond): Ditto.
(ix86_expand_fp_vec_cmp): Require vector comparison dest as
vector.
(ix86_expand_int_vec_cmp): Ditto.
(ix86_expand_sse_unpack): Ditto.
(ix86_expand_mul_widen_evenodd): Ditto.
gcc/testsuite/ChangeLog:
PR target/98537
* g++.target/i386/avx512bw-pr98537-1.C: New test.
* g++.target/i386/avx512vl-pr98537-1.C: New test.
* g++.target/i386/avx512vl-pr98537-2.C: New test.
> @@ -4024,8 +4025,18 @@ ix86_expand_fp_vec_cmp (rtx operands[])
> cmp = ix86_expand_sse_cmp (operands[0], code, operands[2], operands[3],
>operands[1], operands[2]);
>
> - if (operands[0] != cmp)
> -emit_move_insn (operands[0], cmp);
> +if (operands[0] != cmp)
> +{
>
> The indentation of the if above looks wrong.
> Otherwise LGTM.
>
> Jakub
>
--
BR,
Hongtao
From bae4500e17f7590a45504c8c9e3ab0fe6200681d Mon Sep 17 00:00:00 2001
From: liuhongt
Date: Thu, 7 Jan 2021 10:15:33 +0800
Subject: [PATCH] Fix ICE: Convert integer mask to vector in
ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp [PR98537]
ix86_expand_sse_cmp/ix86_expand_int_sse_cmp is used for vector
comparison, considering that avx512 introduces integer mask, but some
original callers require the dest of comparison to be a vector.
So add a new parameter vector_mask_p to control the result
of vector comparison to be vector or not.
gcc/ChangeLog:
PR target/98537
* config/i386/i386-expand.c (ix86_expand_sse_cmp): Add a new
parameter vector_mask_p to control whether the comparison
result should be a vector or not.
(ix86_expand_int_sse_cmp): Ditto.
(ix86_expand_sse_movcc): cmpmode should be MODE_INT.
(ix86_expand_fp_movcc): Allow vector comparison dest as
integer mask.
(ix86_expand_fp_vcond): Ditto.
(ix86_expand_int_vcond): Ditto.
(ix86_expand_fp_vec_cmp): Require vector comparison dest as
vector.
(ix86_expand_int_vec_cmp): Ditto.
(ix86_expand_sse_unpack): Ditto.
(ix86_expand_mul_widen_evenodd): Ditto.
gcc/testsuite/ChangeLog:
PR target/98537
* g++.target/i386/avx512bw-pr98537-1.C: New test.
* g++.target/i386/avx512vl-pr98537-1.C: New test.
* g++.target/i386/avx512vl-pr98537-2.C: New test.
---
gcc/config/i386/i386-expand.c | 63 ++-
.../g++.target/i386/avx512bw-pr98537-1.C | 11
.../g++.target/i386/avx512vl-pr98537-1.C | 40
.../g++.target/i386/avx512vl-pr98537-2.C | 8 +++
4 files changed, 93 insertions(+), 29 deletions(-)
create mode 100644 gcc/testsuite/g++.target/i386/avx512bw-pr98537-1.C
create mode 100644 gcc/testsuite/g++.target/i386/avx512vl-pr98537-1.C
create mode 100644 gcc/testsuite/g++.target/i386/avx512vl-pr98537-2.C
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index 6e08fd32726..1e4ef3b9f3f 100644
--- a/gcc/config/i386/i386-expand
Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause
Hi Andrew:
It's safe to execute on old machine, but it is still a new extension not
included on baseline ISA, so I still prefer having -march to guard that,
and then we can track that in the ELF attribute to see what extensions and
which version are used in the executable / object files.
On Thu, Jan 7, 2021 at 11:51 AM Andrew Waterman
wrote:
> I've got a contrary opinion:
>
> Since HINTs are guaranteed to execute as no-ops--e.g., this one is
> just a FENCE instruction, which is already a mandatory part of the
> base ISA--they don't _need_ to be called out as separate extensions in
> the toolchain.
>
> Although there's nothing fundamentally wrong with Kito's suggestion,
> it seems like an extra hoop to jump through without commensurate
> benefit. I see no reason to restrict the use of __builtin_pause,
> since all RISC-V implementations, including old ones, are required to
> support it. And, of course, that's the reason we encoded it this way
> :)
>
>
> On Wed, Jan 6, 2021 at 7:35 PM Kito Cheng wrote:
> >
> > Hi Philipp:
> >
> > Could you add zihintpause to -march parser and guard that on the
> > pattern and builtin like zifencei[1-2]?
> >
> > And could you sent a PR to
> > https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md to
> > mention __builtin_riscv_pause?
> >
> > Thanks!
> >
> > [1] march parser change:
> >
> https://github.com/gcc-mirror/gcc/commit/b03be74bad08c382da47e048007a78fa3fb4ef49
> > [2] Default version for ext.:
> >
> https://github.com/gcc-mirror/gcc/commit/4b81528241ca682025d92558ff6aeec91dafdca8
> >
> >
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
> > > @@ -0,0 +1,10 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2" } */
> > > +
> > > +void test_pause()
> >
> > I would suggest you change the function name in the testcase,
> > otherwise the scan-assembler test will always pass even if you didn't
> > generate "pause" instruction.
> >
> >
> > > +{
> > > + __builtin_riscv_pause ();
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler "pause" } } */
> > > +
> > > --
> > > 2.18.4
> > >
>
Re: [r11-6351 Regression] FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxwq 2 on Linux/x86_64
On Tue, Dec 29, 2020 at 3:01 PM sunil.k.pandey via Gcc-regression
wrote:
>
> On Linux/x86_64,
>
> 12ae2bc70846a2be8255eaa41322cd1a5a7b7350 is the first bad commit
> commit 12ae2bc70846a2be8255eaa41322cd1a5a7b7350
> Author: Hongyu Wang
> Date: Fri Dec 25 09:25:39 2020 +0800
>
> Fix standard name for zero/sign extend expanders
>
> caused
>
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxbd 2
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxbq 2
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxbw 2
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxdq 2
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxwd 2
> FAIL: gcc.target/i386/pr92658-avx512bw-2.c scan-assembler-times pmovsxwq 2
>
> with GCC configured with
>
> ../../gcc/configure
> --prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r11-6351/usr
> --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld
> --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl
> --enable-libmpx x86_64-linux --disable-bootstrap
>
I'm going to checkin this patch as a simple fix for adjusting the testcase.
--
BR,
Hongtao
From 63abcccb7a375bae6dea8a7885de0232f20f175f Mon Sep 17 00:00:00 2001
From: Hongyu Wang
Date: Tue, 29 Dec 2020 15:14:09 +0800
Subject: [PATCH] Adjust testcase for PR 92658
gcc/testsuite/ChangeLog:
* testsuite/gcc.target/i386/pr92658-avx512bw.c: Add
-mprefer-vector-width=512 to avoid impact of different default
mtune which gcc is built with.
* testsuite/gcc.target/i386/pr92658-avx512bw-2.c: Ditto.
---
gcc/testsuite/gcc.target/i386/pr92658-avx512bw-2.c | 2 +-
gcc/testsuite/gcc.target/i386/pr92658-avx512bw.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-2.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-2.c
index 811f21aa917..33eecbf3afa 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-2.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-2.c
@@ -1,6 +1,6 @@
/* PR target/92658 */
/* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -mavx512bw" } */
+/* { dg-options "-O2 -ftree-vectorize -mavx512bw -mprefer-vector-width=512" } */
typedef char v64qi __attribute__((vector_size (64)));
typedef short v32hi __attribute__((vector_size (64)));
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw.c
index b1d54d24a81..2e8978481e1 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw.c
@@ -1,6 +1,6 @@
/* PR target/92658 */
/* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -mavx512bw" } */
+/* { dg-options "-O2 -ftree-vectorize -mavx512bw -mprefer-vector-width=512" } */
typedef unsigned char v64qi __attribute__((vector_size (64)));
typedef unsigned short v32hi __attribute__((vector_size (64)));
--
2.29.2
Re: [PATCH] RISC-V: Zihintpause: add __builtin_riscv_pause
Kito:
We had originally considered to guard this with a -march, but decided
against it
eventually: this instruction will be (among other cases) used in the
cpu_relax() of
the Linux kernel. For cases like that, we should consider this the
baseline (i.e.
either there's no pause—in which case, the encoded fence will not hurt—or
the
Zihintpause extension)... but it all maps back to a single builtin-call.
Note that the Zihintfence will be enabled for all (also older) targets, as
the insn
is supported there as well (as a fence that doesn't do anything)... so
guarding it
will not really change the behavior.
That said, I'll get going on an v2 that will include the -march guard (and
we can
still turn things back to how they are today).
Thanks,
Philipp.
On Thu, 7 Jan 2021 at 06:42, Kito Cheng wrote:
> Hi Andrew:
>
> It's safe to execute on old machine, but it is still a new extension not
> included on baseline ISA, so I still prefer having -march to guard that,
> and then we can track that in the ELF attribute to see what extensions and
> which version are used in the executable / object files.
>
>
> On Thu, Jan 7, 2021 at 11:51 AM Andrew Waterman
> wrote:
>
>> I've got a contrary opinion:
>>
>> Since HINTs are guaranteed to execute as no-ops--e.g., this one is
>> just a FENCE instruction, which is already a mandatory part of the
>> base ISA--they don't _need_ to be called out as separate extensions in
>> the toolchain.
>>
>> Although there's nothing fundamentally wrong with Kito's suggestion,
>> it seems like an extra hoop to jump through without commensurate
>> benefit. I see no reason to restrict the use of __builtin_pause,
>> since all RISC-V implementations, including old ones, are required to
>> support it. And, of course, that's the reason we encoded it this way
>> :)
>>
>>
>> On Wed, Jan 6, 2021 at 7:35 PM Kito Cheng wrote:
>> >
>> > Hi Philipp:
>> >
>> > Could you add zihintpause to -march parser and guard that on the
>> > pattern and builtin like zifencei[1-2]?
>> >
>> > And could you sent a PR to
>> > https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md to
>> > mention __builtin_riscv_pause?
>> >
>> > Thanks!
>> >
>> > [1] march parser change:
>> >
>> https://github.com/gcc-mirror/gcc/commit/b03be74bad08c382da47e048007a78fa3fb4ef49
>> > [2] Default version for ext.:
>> >
>> https://github.com/gcc-mirror/gcc/commit/4b81528241ca682025d92558ff6aeec91dafdca8
>> >
>> >
>> > > --- /dev/null
>> > > +++ b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
>> > > @@ -0,0 +1,10 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-options "-O2" } */
>> > > +
>> > > +void test_pause()
>> >
>> > I would suggest you change the function name in the testcase,
>> > otherwise the scan-assembler test will always pass even if you didn't
>> > generate "pause" instruction.
>> >
>> >
>> > > +{
>> > > + __builtin_riscv_pause ();
>> > > +}
>> > > +
>> > > +/* { dg-final { scan-assembler "pause" } } */
>> > > +
>> > > --
>> > > 2.18.4
>> > >
>>
>
Re: [PATCH] Add line debug info for virtual thunks (PR ipa/97937)
On Wed, 6 Jan 2021, Jakub Jelinek wrote: > On Wed, Jan 06, 2021 at 08:50:43AM +0100, Richard Biener wrote: > > > Theoretically we could exclude the range of the no-loc function > > > from the .debug_ranges, then gdb would not even step into the function. > > > > I'd argue we're failing to emit a .endloc at the end of functions > > (rather than issueing a .noloc at the start of functions with no > > locations). I wonder if using a special file ID and switching to that > > would be an effective workaround? When gas is extended we could use > > file ID zero for this (which gas currently rejects). > > If we had .endloc, emitting it at the end of functions would be > theoretically more correct, but I'd be afraid it would unnecessarily grow > the .debug_line size, because nobody should care about line information in > padding bytes between functions and the .endloc would add there a change > even when it would affect just one byte of padding (probably even when it > wouldn't affect anything, depending on how it is implemented). Sure, but of course gas can optimize this case for bytes it emits because of .align Richard.
Re: [PATCH] gimple-isel: Fall back to using vcond_mask [PR98560]
On Wed, 6 Jan 2021, Richard Sandiford wrote:
> PR98560 is about a case in which the vectoriser initially generates:
>
> mask_1 = a < 0;
> mask_2 = mask_1 & ...;
> res = VEC_COND_EXPR ;
>
> The vectoriser thus expects res to be calculated using vcond_mask.
> However, we later manage to fold mask_2 to mask_1, leaving:
>
> mask_1 = a < 0;
> res = VEC_COND_EXPR ;
>
> gimple-isel then required a combined vcond to exist.
>
> On most targets, it's not too onerous to provide all possible
> (compare x select) combinations. For each data mode, you just
> need to provide unsigned comparisons, signed comparisons, and
> floating-point comparisons, with the data mode and type of
> comparison uniquely determining the mode of the compared values.
> But for targets like SVE that support “unpacked” vectors,
> it's not that simple: the level of unpacking adds another
> degree of freedom.
>
> Rather than insist that the combined versions exist, I think
> we should be prepared to fall back to using separate comparisons
> and vcond_masks. I think that makes more sense on targets like
> AArch64 and AArch32 in which compares and selects are fundementally
> separate operations anyway.
Indeed the mask variants (thus being able to expand the comparison)
are more fundamental. I guess you're running into this path because
we did not consider using vcond_mask because of
if (used_vec_cond_exprs >= 2
&& (get_vcond_mask_icode (mode, TYPE_MODE (op0_type))
!= CODE_FOR_nothing)
&& expand_vec_cmp_expr_p (op0a_type, op0_type, tcode))
{
/* Keep the SSA name and use vcond_mask. */
tcode = TREE_CODE (op0);
}
not triggering? Which also means your patch fails to check/assert
that we can expand_vec_cmp_expr_p the separate compare?
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
It does feel like the function could need some refactoring ...
But OK - preferably with the assertion that we can actually
expand the compare (I suggest to do the expand_vec_cmp_expr_p
above unconditionally and have a 'global' cannot_expand_mask
flag defaulted to false and checked in the new path).
Thanks,
Richard.
> Richard
>
>
> gcc/
> PR tree-optimization/98560
> * gimple-isel.cc (gimple_expand_vec_cond_expr): If we fail to use
> IFN_VCOND{,U,EQ}, fall back on IFN_VCOND_MASK.
>
> gcc/testsuite/
> PR tree-optimization/98560
> * gcc.dg/vect/pr98560-1.c: New test.
> ---
> gcc/gimple-isel.cc| 9 -
> gcc/testsuite/gcc.dg/vect/pr98560-1.c | 17 +
> 2 files changed, 25 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/vect/pr98560-1.c
>
> diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> index d40338ce4a2..9c07d79a86c 100644
> --- a/gcc/gimple-isel.cc
> +++ b/gcc/gimple-isel.cc
> @@ -254,7 +254,14 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi,
> }
> }
>
> - gcc_assert (icode != CODE_FOR_nothing);
> + if (icode == CODE_FOR_nothing)
> +{
> + gcc_assert (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (op0))
> + && (get_vcond_mask_icode (mode, TYPE_MODE (TREE_TYPE (op0)))
> + != CODE_FOR_nothing));
> + return gimple_build_call_internal (IFN_VCOND_MASK, 3, op0, op1, op2);
> +}
> +
>tree tcode_tree = build_int_cst (integer_type_node, tcode);
>return gimple_build_call_internal (unsignedp ? IFN_VCONDU : IFN_VCOND,
>5, op0a, op0b, op1, op2, tcode_tree);
> diff --git a/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> new file mode 100644
> index 000..2583fc48f8a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3 -fno-tree-vrp -fno-tree-fre -fno-tree-pre
> -fno-code-hoisting -fvect-cost-model=dynamic" } */
> +/* { dg-additional-options "-msve-vector-bits=128" { target aarch64_sve } }
> */
> +
> +#include
> +
> +void
> +f (uint16_t *restrict dst, uint32_t *restrict src1, float *restrict src2)
> +{
> + int i = 0;
> + for (int j = 0; j < 4; ++j)
> +{
> + uint16_t tmp = src1[i] >> 1;
> + dst[i] = (uint16_t) (src2[i] < 0 && i < 4 ? tmp : 1);
> + i += 1;
> +}
> +}
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
