[Committed] S/390: Support vector load/store alignment hints
The IBM z14 POP adds an optional alignment operand to the vl, vst,
vlm, and vstm instruction (vector loads and stores). Vectors residing
on 8 or 16 byte boundaries might get loaded or stored faster on some
models given the instruction uses the proper hint operand. A wrong
hint will hurt performance though.
The attached testcase align-1 currently fails due to:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88085
Bootstrapped and regtested on s390x with z13 and z14 defaults.
Committed to mainline
gcc/ChangeLog:
2018-11-21 Andreas Krebbel
* configure.ac: Add check for Binutils to determine whether vector
load/store alignments hints are being supported.
* config.in: Regenerate.
* configure: Regenerate.
* config/s390/s390.c (print_operand): Support new output
modifier A.
* config/s390/s390.md ("movti"): Append alignment hint output
using the new output modifier 'A'.
* config/s390/vector.md ("mov", "*vec_tf_to_v1tf")
("*vec_ti_to_v1ti"): Likewise.
gcc/testsuite/ChangeLog:
2018-11-21 Andreas Krebbel
* gcc.target/s390/vector/align-1.c: New test.
* gcc.target/s390/vector/align-2.c: New test.
---
gcc/config.in | 7 +
gcc/config/s390/s390.c | 13 ++
gcc/config/s390/s390.md| 4 +--
gcc/config/s390/vector.md | 12 -
gcc/configure | 36 ++
gcc/configure.ac | 6 +
gcc/testsuite/gcc.target/s390/vector/align-1.c | 30 +
gcc/testsuite/gcc.target/s390/vector/align-2.c | 29 +
8 files changed, 129 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/s390/vector/align-1.c
create mode 100644 gcc/testsuite/gcc.target/s390/vector/align-2.c
diff --git a/gcc/config.in b/gcc/config.in
index 67a1e6c..999ade4 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -657,6 +657,13 @@
#endif
+/* Define if your assembler supports vl/vst/vlm/vstm with an optional
+ alignment hint argument. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_VECTOR_LOADSTORE_ALIGNMENT_HINTS
+#endif
+
+
/* Define if your assembler supports VSX instructions. */
#ifndef USED_FOR_TARGET
#undef HAVE_AS_VSX
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index ab06ada..277d555 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -7598,6 +7598,8 @@ print_operand_address (FILE *file, rtx addr)
CODE specified the format flag. The following format flags
are recognized:
+'A': On z14 or higher: If operand is a mem print the alignment
+hint usable with vl/vst prefixed by a comma.
'C': print opcode suffix for branch condition.
'D': print opcode suffix for inverse branch condition.
'E': print opcode suffix for branch on index instruction.
@@ -7635,6 +7637,17 @@ print_operand (FILE *file, rtx x, int code)
switch (code)
{
+case 'A':
+#ifdef HAVE_AS_VECTOR_LOADSTORE_ALIGNMENT_HINTS
+ if (TARGET_Z14 && MEM_P (x))
+ {
+ if (MEM_ALIGN (x) >= 128)
+ fprintf (file, ",4");
+ else if (MEM_ALIGN (x) == 64)
+ fprintf (file, ",3");
+ }
+#endif
+ return;
case 'C':
fprintf (file, s390_branch_condition_mnemonic (x, FALSE));
return;
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 30d113f..c6c960f 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -1552,8 +1552,8 @@
vone\t%v0
vlvgp\t%v0,%1,%N1
#
- vl\t%v0,%1
- vst\t%v1,%0
+ vl\t%v0,%1%A1
+ vst\t%v1,%0%A0
#
#
#
diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
index 102313f..f0e4049 100644
--- a/gcc/config/s390/vector.md
+++ b/gcc/config/s390/vector.md
@@ -198,8 +198,8 @@
""
"@
vlr\t%v0,%v1
- vl\t%v0,%1
- vst\t%v1,%0
+ vl\t%v0,%1%A1
+ vst\t%v1,%0%A0
vzero\t%v0
vone\t%v0
vgbm\t%v0,%t1
@@ -551,8 +551,8 @@
"TARGET_VX"
"@
vmrhg\t%v0,%1,%N1
- vl\t%v0,%1
- vst\t%v1,%0
+ vl\t%v0,%1%A1
+ vst\t%v1,%0%A0
vzero\t%v0
vlvgp\t%v0,%1,%N1"
[(set_attr "op_type" "VRR,VRX,VRX,VRI,VRR")])
@@ -563,8 +563,8 @@
"TARGET_VX"
"@
vlr\t%v0,%v1
- vl\t%v0,%1
- vst\t%v1,%0
+ vl\t%v0,%1%A1
+ vst\t%v1,%0%A0
vzero\t%v0
vone\t%v0
vlvgp\t%v0,%1,%N1"
diff --git a/gcc/configure b/gcc/configure
index 8957362..3a20399 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -27520,6 +27520,42 @@ $as_echo "#define HAVE_AS_ARCHITECTURE_MODIFIERS 1"
>>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for vector
load/store alignment hints" >&5
+$as_echo_n "checking assembler for vector load/store alignment hints... " >&6;
}
+if ${gcc_cv_as_s390_vector_loadstore_alignment_hints+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+
Re: Patch ping (Re: [PATCH] Fortran include line fixes and -fdec-include support)
Hi Jakub, I'd like to ping this patch, ok for trunk? OK. Thanks for the patch! Before 9.0 is released, we should also document the flag (and the extension it supports) in the manual, and note it in changes.html and on the Wiki. Would you also do that? Regards Thomas
[Bug c/88126] Need Compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88126 Andrew Pinski changed: What|Removed |Added Keywords||diagnostic --- Comment #1 from Andrew Pinski --- -Wconversion will warn about some things but it might not warn when the sizes are the same in the case of ILP32 where unsigned int and unsigned long are the same size; it will warn on LP64 targets though (not LLP64 targets; like windows).
Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438
On Wed, Nov 21, 2018 at 12:50 AM Jeff Law wrote: > > + /* x86 targets use mode-switching infrastructure to > > + conditionally insert vzeroupper instruction at the exit > > + from the function and there is no need to switch the > > + mode before the return value copy. The vzeroupper insertion > > + pass runs after reload, so use !reload_completed as a > > stand-in > > + for x86 to skip the search for return value copy insn. > Note that the GCN target may well end up needing a late mode switching > pass -- it's got kindof an inverse problem to solve -- where to place > initializations of the exec register which is needed when we want to do > scalar ops in a simd unit. > > I thought the SH used mode switching as well. BUt I can't recall if it > was run before register allocation & reload. SH mode switching ran before reload. Uros.
[Bug c/88126] New: Need Compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88126
Bug ID: 88126
Summary: Need Compiler warning
Product: gcc
Version: 5.3.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: srinivas.sundar at vvdntech dot in
Target Milestone: ---
int main()
{
unsigned long ul=4294967296;
unsigned int var;
var=ul; // how to get warning
}
The value of "var" will be different in 64bit system and 32bit system. Is it
possible to get compilation warning in 64bit system.
Re: Stream TREE_TYPE of TYPE_DECLs again
> PR lto/87957
> * tree.c (fld_decl_context): Break out from ...
> (free_lang_data_in_decl): ... here; free TREE_PUBLIC, TREE_PRIVATE
> DECL_ARTIFICIAL of TYPE_DECL; do not free TREE_TYPE of TYPE_DECL.
> (fld_incomplete_type_of): Build copy of TYP_DECL.
> * ipa-devirt.c (free_enum_values): Rename to ...
> (free_odr_warning_data): ... this one; free also duplicated TYPE_DECLs
> and TREE_TYPEs of TYPE_DECLs.
Hi,
and here are the testcases from the PR which I finally managed to
annotate so they are accepted :)
PR lto/87957
* g++.dg/lto/odr-1_0.C: Extend by mismatched enum.
* g++.dg/lto/odr-1_1.C: Extend by mismatched enum.
* g++.dg/lto/odr-2_0.C: New.
* g++.dg/lto/odr-2_0.C: New.
* g++.dg/lto/odr-3_1.C: New.
* g++.dg/lto/odr-3_1.C: New.
Index: g++.dg/lto/odr-1_0.C
===
--- g++.dg/lto/odr-1_0.C(revision 266333)
+++ g++.dg/lto/odr-1_0.C(working copy)
@@ -1,8 +1,11 @@
// PR c++/82414
// { dg-lto-do link }
+enum vals {aa,cc}; // { dg-lto-warning "6: type 'vals' violates the C\\+\\+
One Definition Rule" }
struct a { // { dg-lto-warning "8: type 'struct a' violates the C\\+\\+ One
Definition Rule" }
struct b *ptr; // { dg-lto-message "13: the first difference of
corresponding definitions is field 'ptr'" }
+ enum vals vals;
};
-void test(struct a *)
+void test(struct a *a)
{
+ a->vals = cc;
}
Index: g++.dg/lto/odr-1_1.C
===
--- g++.dg/lto/odr-1_1.C(revision 266333)
+++ g++.dg/lto/odr-1_1.C(working copy)
@@ -1,12 +1,16 @@
namespace {
- struct b;
+ struct b; // { dg-lto-message "type 'struct b' defined in anonymous
namespace can not match across the translation unit boundary" }
}
-struct a {
- struct b *ptr;
-};
+enum vals {aa,bb,cc}; // { dg-lto-message "an enum with different value name
is defined in another translation unit" }
+struct a { // { dg-lto-message "a different type is defined in another
translation unit" }
+ struct b *ptr; // { dg-lto-message "a field of same name but different type
is defined in another translation unit" }
+ enum vals vals;
+} a;
void test(struct a *);
int
main(void)
{
- test (0);
+ test ();
+ if (a.vals==aa)
+return 1;
}
Index: g++.dg/lto/odr-2_0.C
===
--- g++.dg/lto/odr-2_0.C(nonexistent)
+++ g++.dg/lto/odr-2_0.C(working copy)
@@ -0,0 +1,8 @@
+// { dg-lto-do link }
+// { dg-lto-options { { -O0 -flto } }
+enum a {} b; // { dg-lto-warning "6: type 'a' violates the C\\+\\+ One
Definition Rule" }
+int
+main(void)
+{
+ return 0;
+}
Index: g++.dg/lto/odr-2_1.C
===
--- g++.dg/lto/odr-2_1.C(nonexistent)
+++ g++.dg/lto/odr-2_1.C(working copy)
@@ -0,0 +1,4 @@
+class a { // { dg-lto-message "a different type is defined in another
translation unit" }
+ int *b() const;
+};
+int *a::b() const { return 0; }
Index: g++.dg/lto/odr-3_0.C
===
--- g++.dg/lto/odr-3_0.C(nonexistent)
+++ g++.dg/lto/odr-3_0.C(working copy)
@@ -0,0 +1,12 @@
+// { dg-lto-do link }
+// { dg-lto-options { -O0 -flto } }
+
+typedef struct {
+ int a; // { dg-lto-message "the first difference of corresponding
definitions is field 'a'" }
+} YYSTYPE; // { dg-lto-warning "3: warning: type ‘struct YYSTYPE’ violates the
C\\+\\+ One Definition Rule" }
+union yyalloc { // { dg-lto-warning "7: type ‘union yyalloc’ violates the
C\\+\\+ One Definition Rule" }
+ short yyss;
+ YYSTYPE yyvs; // { dg-lto-message "the first difference of corresponding
definitions is field ‘yyvs’" }
+};
+void b() { yyalloc c; }
+
Index: g++.dg/lto/odr-3_1.C
===
--- g++.dg/lto/odr-3_1.C(nonexistent)
+++ g++.dg/lto/odr-3_1.C(working copy)
@@ -0,0 +1,9 @@
+typedef struct YYSTYPE { // { dg-lto-message ":16 a different type is defined
in another translation unit" }
+} YYSTYPE;
+union yyalloc {
+ short yyss;
+ YYSTYPE yyvs; // { dg-lto-message "the first difference of corresponding
definitions is field ‘yyvs’" }
+
+};
+void a() { yyalloc b; }
+
[Bug c++/85569] [8/9 Regression] is_invocable(F, decltype(objs)...) fails with "not supported by dump_expr#" unless via indirection
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85569 Alexandre Oliva changed: What|Removed |Added CC||aoliva at gcc dot gnu.org --- Comment #6 from Alexandre Oliva --- Created attachment 45051 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45051=edit patch for context, viable work-around but probably not suitable for inclusion The one remaining problem is that a constexpr TARGET_EXPR initializer with an empty CONSTRUCTOR is recreated in cxx_eval_outermost_constant_expr, because adjust_temp_type only returns the original expr when types compare the same pointer-wise (they differ in qualifiers in this case), and then maybe_constant_value fails the assert because the original and evaled exprs compare equal, because CONSTRUCTOR types are compared with same_type_p. I don't like this change, though; if it's ok to disregard the qualifiers in the type of the TARGET_EXPR initializer, then we might as well use same_type_p in adjust_temp_type and avoid creating the CONSTRUCTOR with the different type, though that regresses g++.robertl/eb73.C, probably because of some NULL type. OTOH, we could accept TARGET_EXPRs that compare equal but that are actually different, but I guess they could be anywhere in the expression. Thoughts?
[Bug libstdc++/88125] Erroneous duplicate "basic_stringbuf" symbol entry in libstdc++ gnu.ver file.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88125 Brooks Moses changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #1 from Brooks Moses --- Jakub, looks like you (and Benjamin Kosnik) were responsible for the original patch in question, so even though it's been 13 years I'm hoping you still have context to decide whether this is the right cleanup. :) Thanks much!
[Bug libstdc++/88125] New: Erroneous duplicate "basic_stringbuf" symbol entry in libstdc++ gnu.ver file.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88125 Bug ID: 88125 Summary: Erroneous duplicate "basic_stringbuf" symbol entry in libstdc++ gnu.ver file. Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: libstdc++ Assignee: unassigned at gcc dot gnu.org Reporter: brooks at gcc dot gnu.org Target Milestone: --- We've been working on getting libstdc++ to link correctly with LLD, and came across a problematic duplicate entry in the gnu.ver symbol version file. Normally, one would only expect to find two entries for the same symbol in the gnu.ver symbol file when there are corresponding asm ".symver" aliases in the source files. And, for most of the duplicate entries, there are ".symver" aliases, and the world is fine. (Well, almost fine. Gold has an Internal Error about the matter, but that's https://sourceware.org/bugzilla/show_bug.cgi?id=21215 and not relevant here...) Anyway, there is one exception to that: _ZNKSt15basic_stringbufIwSt11char_traitsIwESaIwEE3strEv on lines 359 and 1152: https://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/config/abi/pre/gnu.ver?revision=266242=markup#l359 https://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/config/abi/pre/gnu.ver?revision=266242=markup#l1152 As far as I can find, there are no corresponding entries for these symbol versions in the sources. So: ld.bfd happily handles these in a "first match wins" way, and only emits the "3.4" version for the symbol. We can see this in the baseline_symbols.txt file from the libstdc++-abi tests -- there's only one entry: https://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/config/abi/post/x86_64-linux-gnu/baseline_symbols.txt?revision=261865=markup#l482 (By contrast, there are of course multiple entries for the other symbols that are properly multi-versioned. See, e.g., lines 268/269.) LLD, however, points out the problem and errors out. We can see that this bug was actually present in the patch that initially added the "3.4.6" version of this symbol: https://gcc.gnu.org/viewcvs/gcc?limit_changes=0=revision=101125 Although this patch adds the entry for this symbol at 3.4.6 in the gnu.ver file (then called linker-map.gnu), it doesn't add any corresponding entries to the baseline_symbols.txt files for it. Thus, I believe the correct solution here is to simply remove the second entry for _ZNKSt15basic_stringbufIwSt11char_traitsIwESaIwEE3strEv; (lines 1151 to 1153) from the gnu.ver file. Since it is being ignored by ld.bfd, this won't cause any ABI or exported-symbol changes.
[Bug c/80793] three signed conversion warnings for the same expression
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80793 --- Comment #6 from Eric Gallager --- (In reply to Manuel López-Ibáñez from comment #5) > (In reply to Martin Sebor from comment #0) > > There are several issues conflated here. > > > t.c: In function ‘f’: > > t.c:3:46: warning: signed and unsigned type in conditional expression > > [-Wsign-compare] > >unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U; > > This warning should be moved to where -Wsign-conversion is handled to avoid > emitting both. > > The reason we cannot simply remove -Wsign-compare is because > -Wsign-conversion is not enabled by neither -Wall nor -Wextra, while > -Wsign-compare is. Only in C++ though, and this bug is for plain C > > The reason -Wsign-conversion is not enabled by neither -Wall nor -Wextra is > because it has annoying false positives like PR40752. Clang doesn't have > those. > > > t.c:3:46: warning: negative integer implicitly converted to unsigned type > > [-Wsign-conversion] > > This is same warning as Clang gives, just less informative than Clang's. > Printing the types should be easy. > > > t.c:3:21: warning: conversion to ‘unsigned char’ alters ‘unsigned int’ > > constant value [-Wconversion] > >unsigned char c = i ? (-__SCHAR_MAX__ - 1) : 1U; > > This warning is a bug in my opinion. Probably the patch in PR40752 silences > it.
[Bug c++/61760] -Wconversion inconsistency between gcc and g++
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61760 Eric Gallager changed: What|Removed |Added Keywords||easyhack --- Comment #3 from Eric Gallager --- (In reply to Manuel López-Ibáñez from comment #2) > I see now that the C FE has a FE optimization (short_shift in > build_binary_op) that the C++ FE doesn't have. The optimization is probably > useless for code generation, but for improving -Wconversion, it would need > to be moved to c-common.c and used in unsafe_conversion_p, like we do with > shorten_binary_op. > > I don't have time to work on this, so any help is welcome. Sounds easy enough, I might give it a try once my checkout is in a state where I can commit from it... (which might not ever happen, but if it does...)
[Bug fortran/88124] Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 --- Comment #5 from Steve Kargl --- On Wed, Nov 21, 2018 at 03:57:05AM +, kargl at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 > > --- Comment #4 from kargl at gcc dot gnu.org --- > (In reply to kargl from comment #3) > > I think you're running into undefined behavior. Even though > > the derive type defined in the include file is obviously > > identical when the main program and subroutine are in > > different files and the file is INCLUDEd and therefore > > compiled separately, these are different types. The > > namespaces for the main program and the subroutine are > > different. > > PS: INCLUDE is not a substitute for MODULE. > PPS: I think the code when in separate files is non-conforming as the types are different and therefore the call to the subroutine in the main program has a type mismatch. gfortran is not required to diagnosis this mismatch as the subroutine has an implicit interface.
[Bug c++/58875] No float to int conversion warning in std::inner_product(x, x_end, y, 0)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58875 --- Comment #2 from Eric Gallager --- (In reply to Jonathan Wakely from comment #1) > Like PR58876 this is another case where you need -Wsystem-headers to get > warnings from within library code. Aren't there others along those lines too? I forget the numbers...
[Bug fortran/88124] Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 --- Comment #4 from kargl at gcc dot gnu.org --- (In reply to kargl from comment #3) > I think you're running into undefined behavior. Even though > the derive type defined in the include file is obviously > identical when the main program and subroutine are in > different files and the file is INCLUDEd and therefore > compiled separately, these are different types. The > namespaces for the main program and the subroutine are > different. PS: INCLUDE is not a substitute for MODULE.
[Bug fortran/88124] Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 kargl at gcc dot gnu.org changed: What|Removed |Added CC||kargl at gcc dot gnu.org --- Comment #3 from kargl at gcc dot gnu.org --- I think you're running into undefined behavior. Even though the derive type defined in the include file is obviously identical when the main program and subroutine are in different files and the file is INCLUDEd and therefore compiled separately, these are different types. The namespaces for the main program and the subroutine are different.
Re: Proposal to add FDO profile quality related diagnostics
Indu Bhagat writes: > Proposal to add diagnostics to know which functions were not run in the > training run in FDO. Don't you think the warning will be very noisy? I assume most programs have a lot of cold error handling functions etc. that are never executed in a normal execution. Like how does it look like for a gcc build for example? I suspect it would need some heuristics to cut it down at least, like if the function ends with exit (perhaps propagated through the call tree) it's not flagged. Or if there is warning for a function don't warn about callees that are not called from somewhere else. -Andi
[Bug fortran/88124] Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 --- Comment #2 from Jerry DeLisle --- See https://gcc.gnu.org/ml/fortran/2018-11/msg00101.html For further explanation of problem. I found that when I placed the content of sub.F into main.f with the contains, that it gave the correct results.
[Bug fortran/88124] Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 --- Comment #1 from Jerry DeLisle --- Created attachment 45050 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45050=edit Include file for example
[Bug fortran/88124] New: Wrong results with procedure in seperate file
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88124 Bug ID: 88124 Summary: Wrong results with procedure in seperate file Product: gcc Version: 8.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: fortran Assignee: unassigned at gcc dot gnu.org Reporter: jvdelisle at gcc dot gnu.org Target Milestone: --- Created attachment 45049 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45049=edit Code to reproduce The attached code works correctly when subroutine is conationed as shown in maim.F. If the subroutine is instead provided in a separate file such as sub.F and compiled with "gfortran main.F sub.F" results are wrong.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #14 from Jan Hubicka --- Author: hubicka Date: Wed Nov 21 02:38:43 2018 New Revision: 266334 URL: https://gcc.gnu.org/viewcvs?rev=266334=gcc=rev Log: PR lto/84044 * ipa-devirt.c (odr_types_equivalent_p): Use operand_equal_p to compare ENUM values. * g++.dg/lto/odr-4_0.C: New testcase. * g++.dg/lto/odr-4_1.C: New testcase. Added: trunk/gcc/testsuite/g++.dg/lto/odr-4_0.C trunk/gcc/testsuite/g++.dg/lto/odr-4_1.C Modified: trunk/gcc/ChangeLog trunk/gcc/ipa-devirt.c trunk/gcc/testsuite/ChangeLog
Re: [PATCH 06/10] GCN back-end config
On 11/16/18 9:28 AM, Andrew Stubbs wrote: > This patch contains the configuration adjustments needed to enable the GCN > back-end. > > The new configure check for dlopen is required to allow building the new > gcn-run tool. This tool uses libdl to load the HSA runtime libraries, which > are required to run programs on the GPU. The tool is disabled if libdl is not > available. > > 2018-11-16 Andrew Stubbs > Kwok Cheung Yeung > Julian Brown > Tom de Vries > Jan Hubicka > Martin Jambor > > * config.sub: Recognize amdgcn*-*-amdhsa. > * configure.ac: Likewise. > * configure: Regenerate. > * contrib/config-list.mk: Add amdgcn-amdhsa. > > gcc/ > * config.gcc: Add amdgcn*-*-amdhsa configuration. > * configure.ac: Check for dlopen. > * configure: Regenerate. With Joseph's issues around config.sub address, this is fine when the rest of the port is approved. jeff
Fix regression introduced by 88069
Richi's recent change to fix 88069 is causing various targets to fail
tree-ssa/20030711-2.c. That test is verifying a variety of
optimizations occur during the first DOM pass.
Prior to Richi's change FRE1 would do some significant cleanups of the
IL and as a result DOM was fully able to optimize the resultant code.
After Richi's change we've got a redundant load in the IL. After
analyzing the CFG and IL it was clear that DOM *should* be able to
remove the redundant load, but simply wasn't.
DOM would discover that it could statically determine the result of a
branch condition. This resulted in one arm of the branch becoming
unreachable. That in turn caused some PHI nodes to become degenerates.
Normally when a PHI node becomes a degenerate we record it as a copy in
the const_and_copies table and *most* of the time we'll propagate the
src value into uses of the dest. But propagation is not guaranteed
(there's a BZ around that issue you can find if you dig into the history
of some of this code).
Anyway, exposing the degenerate PHI *should* have exposed the redundant
load, but we didn't record anything into the const/copies table for the
virtual phi. That's a conscious decision to avoid issues with
overlapping lifetimes of virtual SSA_NAMEs.
While investigating the history here I noticed Richi's little trick
which allows propagation of virtuals if we propagate to all the uses.
Twiddling DOM to use that same trick results in the virtual operand
propagating. That in turn allows DOM to see and remove the redundant load.
Bootstrapped and regression tested on x86_64 where is fixes
20030711-2.c. Also verified that it fixed various other targets where
that test had started failing.
Installing on the trunk.
jeff
commit 1438e5e18df9f927990074dd32572abf924eba86
Author: Jeff Law
Date: Tue Nov 20 18:10:28 2018 -0700
2018-11-20 Jeff Law
PR tree-optimization/88069
* tree-ssa-dom.c (record_equivalences_from_phis): Propagate away
degenerate virtual PHIs.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index aece55980f0..5c7a9509b35 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-11-20 Jeff Law
+
+ PR tree-optimization/88069
+ * tree-ssa-dom.c (record_equivalences_from_phis): Propagate away
+ degenerate virtual PHIs.
+
2018-11-20 Jakub Jelinek
PR tree-optimization/87895
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 7787da8b237..ce840488403 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1106,10 +1106,13 @@ record_equivalences_from_phis (basic_block bb)
{
gphi_iterator gsi;
- for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next ())
+ for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); )
{
gphi *phi = gsi.phi ();
+ /* We might eliminate the PHI, so advance GSI now. */
+ gsi_next ();
+
tree lhs = gimple_phi_result (phi);
tree rhs = NULL;
size_t i;
@@ -1159,9 +1162,26 @@ record_equivalences_from_phis (basic_block bb)
this, since this is a true assignment and not an equivalence
inferred from a comparison. All uses of this ssa name are dominated
by this assignment, so unwinding just costs time and space. */
- if (i == gimple_phi_num_args (phi)
- && may_propagate_copy (lhs, rhs))
- set_ssa_name_value (lhs, rhs);
+ if (i == gimple_phi_num_args (phi))
+ {
+ if (may_propagate_copy (lhs, rhs))
+ set_ssa_name_value (lhs, rhs);
+ else if (virtual_operand_p (lhs))
+ {
+ gimple *use_stmt;
+ imm_use_iterator iter;
+ use_operand_p use_p;
+ /* For virtual operands we have to propagate into all uses as
+otherwise we will create overlapping life-ranges. */
+ FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
+ FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
+ SET_USE (use_p, rhs);
+ if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs))
+ SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs) = 1;
+ gimple_stmt_iterator tmp_gsi = gsi_for_stmt (phi);
+ remove_phi_node (_gsi, true);
+ }
+ }
}
}
[Bug rtl-optimization/85408] ICE in patch_jump_insn, at cfgrtl.c:1271
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85408 --- Comment #3 from Arseny Solokha --- I cannot reproduce it on the trunk anymore.
[Bug rtl-optimization/85426] ICE in patch_jump_insn, at cfgrtl.c:1271
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85426 --- Comment #4 from Arseny Solokha --- As of r266255 on the trunk it fails w/ the following backtrace: during RTL pass: sms t3kxgcui.c: In function 'c8._loopfn.0': t3kxgcui.c:7:3: internal compiler error: in cfg_layout_redirect_edge_and_branch_force, at cfgrtl.c:4482 7 | for (ns = 0; ns < v2; ++ns) | ^ 0x5abba0 cfg_layout_redirect_edge_and_branch_force /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfgrtl.c:4482 0x8bf855 redirect_edge_and_branch_force(edge_def*, basic_block_def*) /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfghooks.c:486 0x8c02b7 make_forwarder_block(basic_block_def*, bool (*)(edge_def*), void (*)(basic_block_def*)) /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfghooks.c:893 0x8c8ff5 merge_latch_edges /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfgloop.c:780 0x8c8ff5 disambiguate_multiple_latches /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfgloop.c:831 0x8c8ff5 disambiguate_loops_with_multiple_latches() /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/cfgloop.c:844 0xb6a554 apply_loop_flags /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/loop-init.c:54 0xb6afa6 loop_optimizer_init(unsigned int) /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/loop-init.c:123 0x14ca409 sms_schedule /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/modulo-sched.c:1354 0x14ccebf execute /var/tmp/portage/cross-powerpc-e300c3-linux-gnu/gcc-9.0.0_alpha20181118/work/gcc-9-20181118/gcc/modulo-sched.c:3337
[Bug go/88060] ../../../gcc-8.2.0/libgo/go/syscall/libcall_linux_utimesnano.go:17:18: error: reference to undefined name ‘_AT_FDCWD’
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88060 Ian Lance Taylor changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #9 from Ian Lance Taylor --- These problems should be fixed. Thanks for the info.
[Bug go/88060] ../../../gcc-8.2.0/libgo/go/syscall/libcall_linux_utimesnano.go:17:18: error: reference to undefined name ‘_AT_FDCWD’
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88060 --- Comment #8 from ian at gcc dot gnu.org --- Author: ian Date: Wed Nov 21 02:16:15 2018 New Revision: 266333 URL: https://gcc.gnu.org/viewcvs?rev=266333=gcc=rev Log: PR go/88060 syscall: always define _AT_FDCWD and IPv6MTUInfo They aren't defined by old versions of glibc, but are required by the code in syscall_linux.go. Reviewed-on: https://go-review.googlesource.com/c/150697 Modified: trunk/gcc/go/gofrontend/MERGE trunk/libgo/mksysinfo.sh
libgo patch committed: Always define _AT_FDCWD and IPv6MTUInfo
This libgo patch alwayss define _AT_FDCWD and IPv6MTUInfo in the
syscall package. They aren't defined by old versions of glibc, but
are required by the code in syscall_linux.go. This should fix GCC PR
88060. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 266324)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-559fae430b81595efe151222385192a07a9fc3c3
+37cb9763cbe8407b8c3a237b05a5272a226f14a0
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/mksysinfo.sh
===
--- libgo/mksysinfo.sh (revision 266324)
+++ libgo/mksysinfo.sh (working copy)
@@ -202,6 +202,11 @@ if ! grep '^const AF_LOCAL ' ${OUT} >/de
fi
fi
+# The syscall package requires _AT_FDCWD, but doesn't export it.
+if ! grep '^const _AT_FDCWD = ' ${OUT} >/dev/null 2>&1; then
+ echo "const _AT_FDCWD = -100" >> ${OUT}
+fi
+
# sysconf constants.
grep '^const __SC' gen-sysinfo.go |
sed -e 's/^\(const \)__\(SC[^= ]*\)\(.*\)$/\1\2 = __\2/' >> ${OUT}
@@ -669,6 +674,14 @@ grep '^type _ip6_mtuinfo ' gen-sysinfo.g
-e 's/ip6m_mtu/Mtu/' \
>> ${OUT}
+# We need IPv6MTUInfo to compile the syscall package.
+if ! grep 'type IPv6MTUInfo ' ${OUT} >/dev/null 2>&1; then
+ echo 'type IPv6MTUInfo struct { Addr RawSockaddrInet6; Mtu uint32; }' >>
${OUT}
+fi
+if ! grep 'const _sizeof_ip6_mtuinfo = ' ${OUT} >/dev/null 2>&1; then
+ echo 'const SizeofIPv6MTUInfo = 32' >> ${OUT}
+fi
+
# Try to guess the type to use for fd_set.
fd_set=`grep '^type _fd_set ' gen-sysinfo.go || true`
fds_bits_type="_C_long"
[Bug tree-optimization/88107] [7/8/9 Regression] ICE in find_outermost_region_in_block, at tree-cfg.c:7157
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88107 Arseny Solokha changed: What|Removed |Added Known to work||5.4.0 Summary|[9 Regression] ICE in |[7/8/9 Regression] ICE in |find_outermost_region_in_bl |find_outermost_region_in_bl |ock, at tree-cfg.c:7157 |ock, at tree-cfg.c:7157 Known to fail||6.3.0, 7.3.0, 8.2.0, 9.0 --- Comment #3 from Arseny Solokha --- It's rather old. gcc 8.2, 7.3, 6.3 all fail the same way compiling gcc/testsuite/gcc.dg/vect/vect-simd-clone-9.c w/ -O1 (-O2, -O3, -Ofast) -fexceptions -floop-nest-optimize -fnon-call-exceptions -fopenmp -ftree-parallelize-loops=2.
[Bug tree-optimization/60707] FAIL: gfortran.dg/pr45636.f90 -O scan-tree-dump-times forwprop2 "memset" 0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60707 John David Anglin changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #7 from John David Anglin --- Fixed.
[Bug target/62247] [7/8/9 Regression] FAIL: g++.dg/abi/anon3.C -std=c++98/9 scan-assembler .weak(_definition)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62247 John David Anglin changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #17 from John David Anglin --- Fixed.
[Bug testsuite/64886] FAIL: gcc.dg/pr64434.c scan-rtl-dump-times expand "Swap operands" 1
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64886 --- Comment #7 from John David Anglin --- Doesn't appear fixed on aarch64 ILP32.
[Bug target/66114] some indirect_jump patterns use operands[] in their condition when they shouldn't
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66114 John David Anglin changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #5 from John David Anglin --- Appears fixed to me.
Re: [maintainer-scipts] Add a bugzilla script
On Tue, 2018-11-20 at 11:08 +0100, Martin Liška wrote: > Hi. > > It's the script that I used to identify potentially resolvable bugs. > That's done > by parsing of comments and seeking for trunk/branch commits. Sample > output looks > as follows: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88084 branches: > trunk fail:w > ork: basic_string_view::copy > doesn't use Traits::copy > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88083 branches: > trunk fail:w > ork: ICE in > find_constant_pool_ref_1, at config/s390/s390.c:8231 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88077 branches: > trunk fail: > 8.2.0 work: > 9.0 [8 Regression] ICE: c:378 > since r256989 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88073 branches: > trunk fail:w > ork: [7/8 Regression] Internal > compiler error compiling WHERE construct with -O or -O2 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88071 branches: > trunk fail: 8.2.0, > 9.0 work: 7.3.0 [8 > Regression] ICE: verify_gimple failed (error: dead STMT in EH table) > > Plus there are generated bugzilla list so that one can go easily > through. > Would you be interested in putting that into maintainer scripts? > > Martin A minor nit re the imports: argparse and json are in the stdlib, whereas requests is third-party, so per PEP 8 they should split into two alphabetized groups as: import argparse import json import requests (with that blank line) Dave
[Bug libbacktrace/88063] Libbacktrace leak on dwarf read failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88063 Ian Lance Taylor changed: What|Removed |Added CC||ian at airs dot com --- Comment #3 from Ian Lance Taylor --- Created attachment 45048 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45048=edit Proposed patch Does this patch look right to you?
compute discriminator info for overrides
In some cases of overriding or resetting locations, we might retain
discriminator info from earlier locations, when we should take
discriminator information from the overriding location or reset it.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
* final.c (compute_discriminator): Declare. Renamed from...
(maybe_set_discriminator): ... this. Set and return a local.
(override_discriminator): New.
(final_scan_insn_1): Set it.
(notice_source_line): Adjust. Always set discriminator.
---
gcc/final.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/gcc/final.c b/gcc/final.c
index 0c1ac625f37a..f707d2fc0bcd 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -128,6 +128,7 @@ static int last_discriminator;
/* Discriminator to be written to assembly for current instruction.
Note: actual usage depends on loc_discriminator_kind setting. */
static int discriminator;
+static inline int compute_discriminator (location_t loc);
/* Discriminator identifying current basic block among others sharing
the same locus. */
@@ -149,6 +150,7 @@ static const char *last_filename;
static const char *override_filename;
static int override_linenum;
static int override_columnnum;
+static int override_discriminator;
/* Whether to force emission of a line note before the next insn. */
static bool force_source_line = false;
@@ -2342,6 +2344,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int
optimize_p ATTRIBUTE_UNUSED,
override_filename = LOCATION_FILE (*locus_ptr);
override_linenum = LOCATION_LINE (*locus_ptr);
override_columnnum = LOCATION_COLUMN (*locus_ptr);
+ override_discriminator = compute_discriminator (*locus_ptr);
}
}
break;
@@ -2379,12 +2382,14 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int
optimize_p ATTRIBUTE_UNUSED,
override_filename = LOCATION_FILE (*locus_ptr);
override_linenum = LOCATION_LINE (*locus_ptr);
override_columnnum = LOCATION_COLUMN (*locus_ptr);
+ override_discriminator = compute_discriminator (*locus_ptr);
}
else
{
override_filename = NULL;
override_linenum = 0;
override_columnnum = 0;
+ override_discriminator = 0;
}
}
break;
@@ -3185,9 +3190,11 @@ map_decl_to_instance (const_tree decl)
/* Set DISCRIMINATOR to the appropriate value, possibly derived from LOC. */
-static inline void
-maybe_set_discriminator (location_t loc)
+static inline int
+compute_discriminator (location_t loc)
{
+ int discriminator;
+
if (!decl_to_instance_map)
discriminator = bb_discriminator;
else
@@ -3209,6 +3216,8 @@ maybe_set_discriminator (location_t loc)
discriminator = map_decl_to_instance (decl);
}
+
+ return discriminator;
}
/* Return whether a source line note needs to be emitted before INSN.
@@ -3234,7 +3243,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- maybe_set_discriminator (loc);
+ discriminator = compute_discriminator (loc);
force_source_line = true;
}
else if (override_filename)
@@ -3242,6 +3251,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = override_filename;
linenum = override_linenum;
columnnum = override_columnnum;
+ discriminator = override_discriminator;
}
else if (INSN_HAS_LOCATION (insn))
{
@@ -3249,13 +3259,14 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
filename = xloc.file;
linenum = xloc.line;
columnnum = xloc.column;
- maybe_set_discriminator (INSN_LOCATION (insn));
+ discriminator = compute_discriminator (INSN_LOCATION (insn));
}
else
{
filename = NULL;
linenum = 0;
columnnum = 0;
+ discriminator = 0;
}
if (filename == NULL)
--
Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
[Bug c++/88114] "virtual ~destructor() = default": Destructor not created
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88114
--- Comment #2 from Tobias Burnus ---
The "=default" destructor function is generated (when it is generated) via:
mark_vtable_entries -> mark_used -> synthesize_method
While the {} destructor is generated via:
cp_parser_late_parsing_for_member ->
cp_parser_function_definition_after_declarator ->
cp_parser_ctor_initializer_opt_and_function_body
Re: [PATCH][libbacktrace] Handle DW_FORM_GNU_strp_alt
On Wed, Nov 14, 2018 at 6:45 AM, Tom de Vries wrote: > On 14-11-18 14:25, Jakub Jelinek wrote: >> On Wed, Nov 14, 2018 at 02:08:05PM +0100, Tom de Vries wrote: +btest_dwz_CFLAGS = $(AM_CFLAGS) -g -O0 >>> >>> Hmm, I already discovered that specifying the -O0 doesn't work, since >>> it's overridden by $(CFLAGS). >>> >>> With a hack like this: >>> ... >>> diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am >>> index 2fec9bbb4b6..8bdf13b3546 100644 >>> --- a/libbacktrace/Makefile.am >>> +++ b/libbacktrace/Makefile.am >>> @@ -99,11 +99,14 @@ check_PROGRAMS += btest >>> if HAVE_DWZ >>> >>> btest_dwz_SOURCES = btest_dwz.c testlib.c >>> -btest_dwz_CFLAGS = $(AM_CFLAGS) -g -O0 >>> +btest_dwz_CFLAGS = $(AM_CFLAGS) -g >>> btest_dwz_LDADD = libbacktrace.la >>> >>> check_PROGRAMS += btest_dwz >>> >>> +btest_dwz-btest_dwz.o: btest_dwz.c >>> + $(AM_V_CC)$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) >>> $(AM_CPPFLAGS) $(CPPFLAGS) $(btest_dwz_CFLAGS) $(CFLAGS) -O0 -c -o >>> btest_dwz-btest_dwz.o `test -f 'btest_dwz.c' || echo >>> '$(srcdir)/'`btest_dwz.c >> >> Can't you instead do something like: >> btest_dwz.o: CFLAGS += -g -O0 >> or something similar > > Hi, > > yes, that works, thanks. > >> (whatever the corresponding goal is)? > > The goal is to run the testcase with a setting lower than -O2, such that > we can successfully run a substantial portion of the test without > needing support for DW_FORM_GNU_ref_alt. > > [ At O2 we get constprop versions of some functions, which have an > abstract origin, which tends to be moved to the common debug file by dwz > -m, after which we need support for DW_FORM_GNU_ref_alt to get to the > name of the function. ] > >> Otherwise, the patch looks generally ok to me, > > Great. > >> but yes, I've been wondering >> how you can get away with DW_FORM_GNU_ref_alt not implemented properly. >> > > Indeed, DW_FORM_GNU_ref_alt support is required to make this work in > general. > > But I observed that implementing just DW_FORM_GNU_strp_alt improves on > the current situation, so I thought it was worthwhile submitting this as > a separate patch. > > Updated patch attached (which also rewrites btest_dwz.c to an include of > btest.c, while disabling the inline tests that require DW_FORM_GNU_ref_alt). Unfortunately the tests don't pass for me. rm -f btest_dwz.debug cp btest_dwz btest_dwz_2 cp btest_dwz btest_dwz_3 dwz -m btest_dwz.debug btest_dwz_2 btest_dwz_3 FAIL: btest_dwz_2 FAIL: btest_dwz_3 > libbacktrace/btest_dwz_2 test1: [0]: missing file name or function name FAIL: backtrace_full noinline test3: [0]: missing file name or function name FAIL: backtrace_simple noinline PASS: backtrace_syminfo variable > libbacktrace/btest_dwz_3 test1: [0]: missing file name or function name FAIL: backtrace_full noinline test3: [0]: missing file name or function name FAIL: backtrace_simple noinline PASS: backtrace_syminfo variable > +#define INLINE_TESTS 0 > +#include "btest.c" Please avoid this kind of #include game. If you need to skip some tests (why?) use a command line option. If you need to compile with different options, use automake features. > +elf_open_debugfile_by_debugaltlink (struct backtrace_state *state, Do we need this function? It seems to be the same as elf_find_debugfile_by_debuglink. Ian
Re: [PATCH 10/10] Port testsuite to GCN
On 11/16/18 9:29 AM, Andrew Stubbs wrote:
> This collection of miscellaneous patches configures the testsuite to run on
> AMD
> GCN in a standalone (i.e. not offloading) configuration. It assumes you have
> your Dejagnu set up to run binaries via the gcn-run tool.
>
> 2018-11-16 Andrew Stubbs
> Kwok Cheung Yeung
> Julian Brown
> Tom de Vries
>
> gcc/testsuite/
> * gcc.dg/20020312-2.c: Add amdgcn support.
> * gcc.dg/Wno-frame-address.c: Disable on amdgcn.
> * gcc.dg/builtin-apply2.c: Likewise.
> * gcc.dg/torture/stackalign/builtin-apply-2.c: Likewise.
> * gcc.dg/gimplefe-28.c: Force -ffast-math.
> * gcc.dg/intermod-1.c: Add -mlocal-symbol-id on amdgcn.
> * gcc.dg/memcmp-1.c: Increase timeout factor.
> * gcc.dg/pr59605-2.c: Addd -DMAX_COPY=1025 on amdgcn.
> * gcc.dg/sibcall-10.c: xfail on amdgcn.
> * gcc.dg/sibcall-9.c: Likewise.
> * gcc.dg/tree-ssa/gen-vect-11c.c: Likewise.
> * gcc.dg/tree-ssa/pr84512.c: Likewise.
> * gcc.dg/tree-ssa/loop-1.c: Adjust expectations for amdgcn.
> * gfortran.dg/bind_c_array_params_2.f90: Likewise.
> * gcc.dg/vect/tree-vect.h: Avoid signal on amdgcn.
> * lib/target-supports.exp (check_effective_target_trampolines):
> Configure amdgcn.
> (check_profiling_available): Likewise.
> (check_effective_target_global_constructor): Likewise.
> (check_effective_target_return_address): Likewise.
> (check_effective_target_fopenacc): Likewise.
> (check_effective_target_fopenmp): Likewise.
> (check_effective_target_vect_int): Likewise.
> (check_effective_target_vect_intfloat_cvt): Likewise.
> (check_effective_target_vect_uintfloat_cvt): Likewise.
> (check_effective_target_vect_floatint_cvt): Likewise.
> (check_effective_target_vect_floatuint_cvt): Likewise.
> (check_effective_target_vect_simd_clones): Likewise.
> (check_effective_target_vect_shift): Likewise.
> (check_effective_target_whole_vector_shift): Likewise.
> (check_effective_target_vect_bswap): Likewise.
> (check_effective_target_vect_shift_char): Likewise.
> (check_effective_target_vect_long): Likewise.
> (check_effective_target_vect_float): Likewise.
> (check_effective_target_vect_double): Likewise.
> (check_effective_target_vect_perm): Likewise.
> (check_effective_target_vect_perm_byte): Likewise.
> (check_effective_target_vect_perm_short): Likewise.
> (check_effective_target_vect_widen_mult_qi_to_hi): Likewise.
> (check_effective_target_vect_widen_mult_hi_to_si): Likewise.
> (check_effective_target_vect_widen_mult_qi_to_hi_pattern): Likewise.
> (check_effective_target_vect_widen_mult_hi_to_si_pattern): Likewise.
> (check_effective_target_vect_natural_alignment): Likewise.
> (check_effective_target_vect_fully_masked): Likewise.
> (check_effective_target_vect_element_align): Likewise.
> (check_effective_target_vect_masked_store): Likewise.
> (check_effective_target_vect_scatter_store): Likewise.
> (check_effective_target_vect_condition): Likewise.
> (check_effective_target_vect_cond_mixed): Likewise.
> (check_effective_target_vect_char_mult): Likewise.
> (check_effective_target_vect_short_mult): Likewise.
> (check_effective_target_vect_int_mult): Likewise.
> (check_effective_target_sqrt_insn): Likewise.
> (check_effective_target_vect_call_sqrtf): Likewise.
> (check_effective_target_vect_call_btrunc): Likewise.
> (check_effective_target_vect_call_btruncf): Likewise.
> (check_effective_target_vect_call_ceil): Likewise.
> (check_effective_target_vect_call_floorf): Likewise.
> (check_effective_target_lto): Likewise.
> (check_vect_support_and_set_flags): Likewise.
> (check_effective_target_vect_stridedN): Enable when fully masked is
> available.
> ---
>
> diff --git a/gcc/testsuite/gcc.dg/gimplefe-28.c
> b/gcc/testsuite/gcc.dg/gimplefe-28.c
> index 467172d..57b6e1f 100644
> --- a/gcc/testsuite/gcc.dg/gimplefe-28.c
> +++ b/gcc/testsuite/gcc.dg/gimplefe-28.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile { target sqrt_insn } } */
> -/* { dg-options "-fgimple -O2" } */
> +/* { dg-options "-fgimple -O2 -ffast-math" } */
So why does the GCN need fast-math here? I'm not aware of any other
target that needs that kind of handling to make this test work.
> diff --git a/gcc/testsuite/gcc.dg/vect/tree-vect.h
> b/gcc/testsuite/gcc.dg/vect/tree-vect.h
> index 69c93ac..2ddfa5e 100644
> --- a/gcc/testsuite/gcc.dg/vect/tree-vect.h
> +++ b/gcc/testsuite/gcc.dg/vect/tree-vect.h
> @@ -1,5 +1,9 @@
> /* Check if system supports SIMD */
> +#ifdef __AMDGCN__
> +#define signal(A,B)
> +#else
> #include
> +#endif
Presumably you don't have signals. Though one could make an argument
that this really shouldn't be exposed in the testsuite. It
Re: [PATCH 03/10] GCN libgcc.
On 11/16/18 9:27 AM, Andrew Stubbs wrote: > This patch contains the GCN port of libgcc. > > Since the previous posting, I've removed gomp_print.c and reduction.c, > as well as addressing some other feedback. > > 2018-11-16 Andrew Stubbs > Kwok Cheung Yeung > Julian Brown > Tom de Vries > > libgcc/ > * config.host: Recognize amdgcn*-*-amdhsa. > * config/gcn/crt0.c: New file. > * config/gcn/lib2-divmod-hi.c: New file. > * config/gcn/lib2-divmod.c: New file. > * config/gcn/lib2-gcn.h: New file. > * config/gcn/sfp-machine.h: New file. > * config/gcn/t-amdgcn: New file. > --- This is fine. However please wait for OK on the rest of the target bits before installing. jeff
Re: Not quoted option names in errors and warnings
On 11/20/2018 12:54 PM, Martin Liška wrote: On 11/20/18 4:24 PM, Martin Sebor wrote: On 11/20/2018 03:10 AM, Martin Liška wrote: On 11/15/18 5:54 PM, Martin Sebor wrote: On 11/15/2018 03:12 AM, Martin Liška wrote: Hi. I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations of option names (~400). Is it something we should fix? How important is that? That's quite a few... I've been fixing these as I notice them, usually as part of related changes. The most onerous part of the cleanup is adjusting tests, especially under the target- specific ones. It's (obviously) not critical but I think it would be nice to make the quoting consistent throughout over time (if not in one go) and then put in place a -Wformat checker to detect the missing quoting as new diagnostics are introduced. Do you think it might be scriptable? Hi. Are you talking about a proper GCC warning that will be triggered once a warning message is missing quotations? I can definitely cook a patch in next stage1 and the testsuite fall out should be easy to come with. Yes, issuing a -Wformat warning for __gcc_diag__ functions is what I'm thinking of. A checker that would look for substrings within the format string that match the "-[Wfm][a-z][a-z_1-9]*" patterns (or anything else that matches an option) and point them out if they're not enclosed in a pair of %< %> (or suggest to use %qs). Martin Sounds good to me. Well, I can imagine doing that for GCC 9 release. When will you find spare cycles for the warning? In can prepare the warning/error messages patch. I don't expect to have the time to work on the warning until sometime in stage 1 (as an enhancement I also wouldn't expect it to be approved, but maybe you can get away with it ;-) Martin
Re: [PATCH 01/10] Fix IRA ICE.
On 11/16/18 9:27 AM, Andrew Stubbs wrote: > > This patch is unchanged from that which was posted before. Discussion > fizzled out there and I was too busy with other patches to restart it > then. This issue needs to be resolved before libgfortran can be > compiled for GCN. > > The IRA pass makes an assumption that any pseudos created after the pass > begins > were created explicitly by the pass itself and therefore will have > corresponding entries in its other tables. > > The GCN back-end, however, often creates additional pseudos, in expand > patterns, to represent the necessary EXEC value, and these break IRA's > assumption and cause ICEs: > > /libgfortran/generated/matmul_r8.c: In function 'matmul_r8': > /libgfortran/generated/matmul_r8.c:3002:1: internal compiler error: in > setup_preferred_alternate_classes_for_new_pseudos, at ira.c:2772 > > This patch simply has IRA skip unknown pseudos, and the problem goes away. > > Presumably, it's not ideal that these registers have not been processed by > IRA, > but it does not appear to do any real harm. > > 2018-11-16 Andrew Stubbs > > gcc/ > * ira.c (setup_preferred_alternate_classes_for_new_pseudos): Skip > pseudos not created by this pass. > (move_unallocated_pseudos): Likewise. This seems like a really gross hack and sets an expectation that generating registers in the target after IRA has started is OK. It is not OK. THe fact that this works is, IMHO, likely an accident. I think this comes back to the fundamental representational issue with the EXEC handling that still needs to be addressed. Jeff
Re: [maintainer-scipts] Add a bugzilla script
On 11/20/2018 03:08 AM, Martin Liška wrote: Hi. It's the script that I used to identify potentially resolvable bugs. That's done by parsing of comments and seeking for trunk/branch commits. Sample output looks as follows: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88084 branches: trunk fail:work: basic_string_view::copy doesn't use Traits::copy https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88083 branches: trunk fail:work: ICE in find_constant_pool_ref_1, at config/s390/s390.c:8231 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88077 branches: trunk fail: 8.2.0 work: 9.0 [8 Regression] ICE: c:378 since r256989 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88073 branches: trunk fail:work: [7/8 Regression] Internal compiler error compiling WHERE construct with -O or -O2 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88071 branches: trunk fail: 8.2.0, 9.0 work: 7.3.0 [8 Regression] ICE: verify_gimple failed (error: dead STMT in EH table) Plus there are generated bugzilla list so that one can go easily through. Would you be interested in putting that into maintainer scripts? It would be nice to add comments explaining how to use it. Maybe an example. (FWIW, I document my own scripts using the man page format at the top.) Martin
Re: [PATCH] avoid error_mark_node in -Wsizeof-pointer-memaccess (PR 88065)
On 11/19/18 5:36 PM, Martin Sebor wrote: > On 11/19/2018 04:10 PM, Jeff Law wrote: >> On 11/17/18 3:45 PM, Martin Sebor wrote: >>> -Wsizeof-pointer-memaccess fails with an ICE when one of >>> the arguments is ill-formed (error_mark_node). To avoid >>> the error the attached patch has the function bail in this >>> case. >>> >>> Martin >>> >>> gcc-88065.diff >>> >>> PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy >>> >>> gcc/c-family/ChangeLog: >>> >>> PR c/88065 >>> * c-warn.c (sizeof_pointer_memaccess_warning): Bail if source >>> or destination is an error. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c/88065 >>> * gcc.dg/Wsizeof-pointer-memaccess2.c: New test. >> This is probably OK. But before final ACK, is there a point earlier >> where we could/should have bailed out? >> >> ie, when does the ERROR_MARK get created and if you were to look at the >> flow from that point to the offending call to >> sizeof_pointer_memaccess_warning is there a better place to bail? > > sizeof_pointer_memaccess_warning() has a big switch statement > with the built-in code as a controlling expression that it > uses to determine which argument is the source, which one is > the destination, and which one is the size/bound. To do > the checking anywhere else up the stack the caller would have > to replicate the same switch statement, so I think this is > the right spot to do it. > > I can move the test if it's important. I don't think it's terribly important. It's just part of the "did we get here in a reasonable way" kind of question I always try to ask myself with these kinds of patches. > > FWIW, I tend to only add rests to c-c++-common if they exercise > different code between the two front-ends. Otherwise it seems > like unnecessarily increasing the already excessive testsuite > runtimes (each of the C++ tests runs once for each C++ revision, > i.e., C++ 98, C++ 11, C++ 14, and C++ 17). I do realize that > if the implementation were to diverge between the two front-ends > having test coverage for both would be a good thing. So it's > a trade-off. While griping about c-c++-common: sometimes (though > not in the case of ICEs), because of subtle differences between > the two front-ends (like missing or inaccurate location > information) it can also be a pain to get the tests to produce > the same output even for a shared implementation of a warning, > let alone for distinct ones. I don't have strong opinions on this issue with this patch, so I'll defer to Jakub on this issue if the test can be easily moved. So moving the test and using error_operand_p rather than testing for ERROR_MARK_NODE and this is OK for the trunk. jeff
Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
+void test_2 (void)
+{
+ takes_int_ptr(ivar); /* { dg-warning "" "" { target c } } */
+ /* { dg-error "" "" { target c++ } .-1 } */
+ /* { dg-message "possible fix: take the address with '&'" "" { target *-*-*
} .-2 } */
+
+ /* Expect an '&' fix-it hint. */
+ /* { dg-begin-multiline-output "" }
+ takes_int_ptr(ivar);
+ ^~~~
+ |
+ int
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ takes_int_ptr(ivar);
+ ^~~~
+ &
I experimented with adding hints to sizeof_pointer_memaccess_warning
over the weekend and ran into a location difference between the two
front-ends. In an attempt to resolve it, rather than using
an "insertion hint" like I think you did above, I used a replacement
hint. And although it started out as a workaround I ended up liking
the result better. What I think the same approach would result in
here is something like:
takes_int_ptr(ivar);
note: possible fix: take the address with '&'
note: takes_int_ptr(ivar);
^~~~
|
int
note: takes_int_ptr(ivar);
^~~~
Have you considered this style, i.e., printing valid expressions
in the hints when possible rather than just operators? It solves
the problem of the lone operators looking a little too terse and
cryptic.
I realize it's not always possible (not every fix-it hint is for
a bad expression) but I'm wondering if it would be worth using
when it is.
Martin
Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
On 11/19/18 2:23 PM, David Malcolm wrote: [ Snip ] > > The above code is in c-family, but same_type_p is specific to C++, > so the change is not quite trivial. > > Here's a v3 of the patch which moves same_type_p from cp/cp-tree.h > to c-family/c-common.h, converting it from a macro to an extern decl, > with implementations for C and for C++. I used: > comptypes (type1, type2) == 1 > for the C implementation of same_type_p. > > The v3 patch uses same_type_p rather than pointer equality, and I added > a test case for a typedef of int vs a int. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > Is this OK for trunk? > > Thanks > Dave > > gcc/c-family/ChangeLog: > PR c++/87850 > * c-common.c: Include "gcc-rich-location.h". > (maybe_emit_indirection_note): New function. > * c-common.h (maybe_emit_indirection_note): New decl. > (same_type_p): New decl > > gcc/c/ChangeLog: > PR c++/87850 > * c-typeck.c (same_type_p): New function. > (convert_for_assignment): Call maybe_emit_indirection_note for > pointer vs non-pointer diagnostics. > > gcc/cp/ChangeLog: > PR c++/87850 > * call.c (convert_like_real): Call > maybe_emit_indirection_note for "invalid conversion" diagnostic. > * cp-tree.h (same_type_p): Delete macro, moving to... > * typeck.c (same_type_p): New function. > > gcc/testsuite/ChangeLog: > PR c++/87850 > * c-c++-common/indirection-fixits.c: New test. OK. jeff
Re: [PATCH, ARM, ping3] PR85434: Prevent spilling of stack protector guard's address on ARM
On 11/16/18 7:56 AM, Thomas Preudhomme wrote: > Ping? I thought I acked the target independent stuff a while back. What's still waiting on review here? jeff
Re: [PATCH 5/6] ifcvt: Only created temporaries as needed.
On 11/15/18 4:38 AM, Robin Dapp wrote: >> This looks pretty reasonable. ISTM it ought to be able to go forward if >> it's tested independently. > > The test suite already passes, any other tests you have in mind? To be > honest I suppose noce_convert_multiple_sets will currently never > successfully return (due to the costing problems I described before), so > a specific test case might be difficult. There can be subtle dependencies on prior patches when you test it as part of a set. If we're going to gate this in independently, then it should be tested independently. Jeff
Re: [PATCH v2, target]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438
On 11/20/18 12:40 PM, Uros Bizjak wrote:
> Hello!
>
> Attached patch is a different approach to the problem of split return
> copies in create_pre_exit. It turns out that for vzeroupper insertion
> pass, we actually don't need to insert a mode switch before the return
> copy, it is enough to split edge to exit block - so we can emit
> vzeroupper at the function exit edge.
>
> Since x86 is the only target that uses optimize mode switching after
> reload, I took the liberty and used !reload_completed for the
> condition when we don't need to search for return copy. Sure, with the
> big comment as evident from the patch.
>
> 2018-11-20 Uros Bizjak
>
> PR target/88070
> * mode-switching.c (create_pre_exit): After reload, always split the
> fallthrough edge to the exit block.
>
> testsuite/ChangeLog:
>
> 2018-11-20 Uros Bizjak
>
> PR target/88070
> * gcc.target/i386/pr88070.c: New test.
>
> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>
> Committed to mainline SVN.
>
> Uros.
>
OK. But note this may have to be revisited for the GCN port.
jeff
Re: Fix PR37916 (unnecessary spilling)
On 11/20/18 6:42 AM, Michael Matz wrote: > Hi, > > this bug report is about cris generating worse code since tree-ssa. The > effect is also visible on x86-64. The symptom is that the work horse of > adler32.c (from zlib) needs spills in the inner loop, while gcc 3 did not, > and those spills go away with -fno-tree-reassoc. > > The underlying reason for the spills is register pressure, which could > either be rectified by the pressure aware scheduling (which cris doesn't > have), or by simply not generating high pressure code to begin with. In > this case it's TER which ultimately causes the register pressure to > increase, and there are many plans in people minds how to fix this (make > TER regpressure aware, do some regpressure scheduling on gimple, or even > more ambitious things), but this patch doesn't tackle this. Instead it > makes reassoc not generate the situation which causes TER to run wild. > > TER increasing register pressure is a long standing problem and so it has > some heuristics to avoid that. One wobbly heuristic is that it doesn't > TER expressions together that have the same base variable as their LHSs. > But reassoc generates only anonymous SSA names for its temporary > subexpressions, so that TER heuristic doesn't apply. In this testcase > it's even the case that reassoc doesn't really change much code (one > addition moves from the end to the beginning of the inner loop), so that > whole rewriting is even pointless. > > In any case, let's use copy_ssa_name instead of make_ssa_name, when we > have an obvious LHS; that leads to TER making the same decisions with and > without -fno-tree-reassoc, leading to the same register pressure and no > spills. > > On x86-64 the effect is: > before patch: 48 bytes stackframe, 24 stack > accesses (most of them in the loops), 576 bytes codesize; > after patch: no stack frame, no stack accesses, 438 bytes codesize > > On cris: > before patch: 64 bytes stack frame, 27 stack access in loops, size of .s > 145 lines > after patch: 20 bytes stack frame (as it uses callee saved regs, which > is also complained about in the bug report), but no stack accesses > in loops, size of .s: 125 lines > > I'm wondering about testcase: should I add an x86-64 specific that tests > for no stack accesses, or would that be too constraining in the future? I think that'd be a fine test. Yea it's constraining, but isn't that the point, to make sure we never generate something dumb for this code. If we get to a point where we have to have a stack reference we can re-evaluate the test. > > Regstrapped on x86-64-linux, no regressions. Okay for trunk? > > > Ciao, > Michael. > > PR tree-optimization/37916 > * tree-ssa-reassoc.c (make_new_ssa_for_def): Use copy_ssa_name. > (rewrite_expr_tree, linearize_expr, negate_value, > repropagate_negates, attempt_builtin_copysign, > reassociate_bb): Likewise. I wonder if this would help 21182. I don't think anyone ever looked to see if reassoc was involved in the issues in that BZ. OK for the trunk. jeff
[Bug c++/88114] "virtual ~destructor() = default": Destructor not created
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88114
Tobias Burnus changed:
What|Removed |Added
CC||jason at gcc dot gnu.org
--- Comment #1 from Tobias Burnus ---
The reason that the presence of a single
virtual ... = 0;
will remove the 'virtual ~One() = default' seems to be the following code in
cp/class.c's build_vtbl_initializer (added in r208845 in 2014):
/* Don't refer to a virtual destructor from a constructor
vtable or a vtable for an abstract class, since destroying
an object under construction is undefined behavior and we
don't want it to be considered a candidate for speculative
devirtualization. But do create the thunk for ABI
compliance. */
if (DECL_DESTRUCTOR_P (fn_original)
&& (CLASSTYPE_PURE_VIRTUALS (DECL_CONTEXT (fn_original))
|| orig_binfo != binfo))
init = size_zero_node;
That affects the code generation via cp/decl2.c's maybe_emit_vtables() which
calls mark_needed() for all non-size_zero_node items.
In principle, that's also called for
inline virtual ~One() {}
such that the vtable seems to contain a NULL -- but the destructor function
itself is emitted. My feeling is that something marks the tree as needed and,
hence, the symbol is produced. [Probably via somehow via parser.c's
cp_parser_save_member_function_body().]
Finally, the reason that the One::~One() works without "#pragma interface" is
not that it is generated when compiling "test.cc" but it will be generated when
compiling "test3.cc".
[Bug libstdc++/87635] backport of cmath patches to gcc 48 for Darwin incomplete cmath
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87635 --- Comment #3 from Riccardo --- Created attachment 45047 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45047=edit Same patches for gcc6 Backport of patches but for gcc 6.5
[Bug libstdc++/87635] backport of cmath patches to gcc 48 for Darwin incomplete cmath
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87635 --- Comment #2 from Riccardo --- Just for the reference, I backported the fix also to gcc 6.5 It applied almost cleanly. Given your comment, I don't think it is worth opening a new bug just for that? So I leave the patch here for the record.
Re: [maintainer-scipts] Add a bugzilla script
On 11/20/18 3:08 AM, Martin Liška wrote: > Hi. > > It's the script that I used to identify potentially resolvable bugs. That's > done > by parsing of comments and seeking for trunk/branch commits. Sample output > looks > as follows: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88084 branches: trunk > fail:work: > basic_string_view::copy doesn't use Traits::copy > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88083 branches: trunk > fail:work: > ICE in find_constant_pool_ref_1, at config/s390/s390.c:8231 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88077 branches: trunk > fail: 8.2.0 work: 9.0 > [8 Regression] ICE: c:378 since r256989 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88073 branches: trunk > fail:work: > [7/8 Regression] Internal compiler error compiling WHERE > construct with -O or -O2 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88071 branches: trunk > fail: 8.2.0, 9.0 work: 7.3.0 > [8 Regression] ICE: verify_gimple failed (error: dead STMT > in EH table) > > Plus there are generated bugzilla list so that one can go easily through. > Would you be interested in putting that into maintainer scripts? > > Martin > If it helps people find stuff that is likely fixed, then absolutely :-) jeff
Stream TREE_TYPE of TYPE_DECLs again
Hi,
this patch recovers location infomration in the ODR warnings.
Because location info is not attached to types but corresponding
TYPE_DECLs, we need to prevent TYPE_DECLs to be merged when
corresponding types are not merged.
To achieve this I no longer clear TREE_TYPE of TYPE_DECLs which
puts them back to the same SCC as the type itself. While making
incomplete type variant we need to produce copy of TYPE_DECL. Becuase
it is possible that TYPE_DECL was not processed by free lang data
we can not do copy_node but build it from scratch (because
the toplevel loops possibly processed all decls). This is not hard
to do, but made me notice few extra flags that are streamed for
TYPE_DECLs and free_lang_data is not seeing them.
I have also extended ipa-devirt to get rid of the duplicated decls
once ODR warnings are done to save ltrans streaming (it actually
added about 10% of ltrans data w/o this change)
I have checked that the patch does not increase number of type
duplicates for cc1 (24), I will also re-do testing for Firefox
which may uncover some extra flags/attributes to care about.
lto-bootstrapped/regtested x86_64-linux OK?
Honza
PR lto/87957
* tree.c (fld_decl_context): Break out from ...
(free_lang_data_in_decl): ... here; free TREE_PUBLIC, TREE_PRIVATE
DECL_ARTIFICIAL of TYPE_DECL; do not free TREE_TYPE of TYPE_DECL.
(fld_incomplete_type_of): Build copy of TYP_DECL.
* ipa-devirt.c (free_enum_values): Rename to ...
(free_odr_warning_data): ... this one; free also duplicated TYPE_DECLs
and TREE_TYPEs of TYPE_DECLs.
Index: tree.c
===
--- tree.c (revision 266325)
+++ tree.c (working copy)
@@ -5206,6 +5206,24 @@ fld_process_array_type (tree t, tree t2,
return array;
}
+/* Return CTX after removal of contexts that are not relevant */
+
+static tree
+fld_decl_context (tree ctx)
+{
+ /* Variably modified types are needed for tree_is_indexable to decide
+ whether the type needs to go to local or global section.
+ This code is semi-broken but for now it is easiest to keep contexts
+ as expected. */
+ if (ctx && TYPE_P (ctx)
+ && !variably_modified_type_p (ctx, NULL_TREE))
+ {
+ while (ctx && TYPE_P (ctx))
+ctx = TYPE_CONTEXT (ctx);
+ }
+ return ctx;
+}
+
/* For T being aggregate type try to turn it into a incomplete variant.
Return T if no simplification is possible. */
@@ -5267,6 +5285,27 @@ fld_incomplete_type_of (tree t, struct f
}
else
TYPE_VALUES (copy) = NULL;
+
+ /* Build copy of TYPE_DECL in TYPE_NAME if necessary.
+This is needed for ODR violation warnings to come out right (we
+want duplicate TYPE_DECLs whenever the type is duplicated because
+of ODR violation. Because lang data in the TYPE_DECL may not
+have been freed yet, rebuild it from scratch and copy relevant
+fields. */
+ TYPE_NAME (copy) = fld_simplified_type_name (copy);
+ tree name = TYPE_NAME (copy);
+
+ if (name && TREE_CODE (name) == TYPE_DECL)
+ {
+ gcc_checking_assert (TREE_TYPE (name) == t);
+ tree name2 = build_decl (DECL_SOURCE_LOCATION (name), TYPE_DECL,
+ DECL_NAME (name), copy);
+ SET_DECL_ASSEMBLER_NAME (name2, DECL_ASSEMBLER_NAME (name));
+ SET_DECL_ALIGN (name2, 0);
+ DECL_CONTEXT (name2) = fld_decl_context
+(DECL_CONTEXT (name));
+ TYPE_NAME (copy) = name2;
+ }
}
return copy;
}
@@ -5649,12 +5688,13 @@ free_lang_data_in_decl (tree decl, struc
{
DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
DECL_VISIBILITY_SPECIFIED (decl) = 0;
- /* TREE_PUBLIC is used to tell if type is anonymous. */
+ TREE_PUBLIC (decl) = 0;
+ TREE_PRIVATE (decl) = 0;
+ DECL_ARTIFICIAL (decl) = 0;
TYPE_DECL_SUPPRESS_DEBUG (decl) = 0;
DECL_INITIAL (decl) = NULL_TREE;
DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
DECL_MODE (decl) = VOIDmode;
- TREE_TYPE (decl) = void_type_node;
SET_DECL_ALIGN (decl, 0);
}
else if (TREE_CODE (decl) == FIELD_DECL)
@@ -5688,20 +5728,7 @@ free_lang_data_in_decl (tree decl, struc
if (TREE_CODE (decl) != FIELD_DECL
&& ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
|| !DECL_VIRTUAL_P (decl)))
-{
- tree ctx = DECL_CONTEXT (decl);
- /* Variably modified types are needed for tree_is_indexable to decide
-whether the type needs to go to local or global section.
-This code is semi-broken but for now it is easiest to keep contexts
-as expected. */
- if (ctx && TYPE_P (ctx)
- && !variably_modified_type_p (ctx, NULL_TREE))
-{
- while
Re: [PATCH][libbacktrace] Factor out read_string
On 11/15/18 9:02 AM, Tom de Vries wrote: > Hi, > > This patch factors out new function read_string in dwarf.c. > > Bootstrapped and reg-tested on x86_64. > > OK for trunk (or, for stage1)? > > Thanks, > - Tom > > [libbacktrace] Factor out read_string > > 2018-11-15 Tom de Vries > > * dwarf.c (read_string): Factor out of ... > (read_attribute, read_line_header, read_line_program): ... here. OK jeff
Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438
On 11/20/18 3:24 AM, Uros Bizjak wrote: > On Tue, Nov 20, 2018 at 8:59 AM Eric Botcazou wrote: >> >>> The blockage was introduced as a fix for PR14381 [1] in r79265 [2]. >>> Later, the blockage was moved after return label as a fix for PR25176 >>> [3] in r107871 [4]. >>> >>> After that, r122626 [5] moves the blockage after the label for the >>> naked return from the function. Relevant posts from gcc-patches@ ML >>> are at [6], [7]. However, in the posts, there are no concrete >>> examples, how scheduler moves instructions from different BB around >>> blockage insn, the posts just show that there is a jump around >>> blockage when __builtin_return is used. I was under impression that >>> scheduler is unable to move instructions over BB boundaries. >> >> The scheduler works on extended basic blocks. The [7] post gives a rather >> convincing explanation and there is a C++ testcase under PR rtl-opt/14381. >> >>> A mystery is the tree-ssa merge [8] that copies back the hunk, moved >>> in r122626 [5] to its original position. From this revision onwards, >>> we emit two blockages. >> >> It's the dataflow merge, not the tree-ssa merge. The additional blockage >> might be needed for DF. >> >> Given that the current PR is totally artificial, I think that we need to be >> quite conservative and only do something on mainline. And even there I'd be >> rather conservative and remove the kludge only for targets that emit unwind >> information in the epilogue (among which there is x86 I presume). > > Hm, I think I'll rather go with somehow target-dependent patch: > > --cut here-- > diff --git a/gcc/mode-switching.c b/gcc/mode-switching.c > index 370a49e90a9c..de75efe2b6c9 100644 > --- a/gcc/mode-switching.c > +++ b/gcc/mode-switching.c > @@ -252,7 +252,21 @@ create_pre_exit (int n_entities, int *entity_map, > const int *num_modes) > if (EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) == 1 > && NONJUMP_INSN_P ((last_insn = BB_END (src_bb))) > && GET_CODE (PATTERN (last_insn)) == USE > - && GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG) > + && GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG > + > + /* x86 targets use mode-switching infrastructure to > + conditionally insert vzeroupper instruction at the exit > + from the function and there is no need to switch the > + mode before the return value copy. The vzeroupper insertion > + pass runs after reload, so use !reload_completed as a stand-in > + for x86 to skip the search for return value copy insn. Note that the GCN target may well end up needing a late mode switching pass -- it's got kindof an inverse problem to solve -- where to place initializations of the exec register which is needed when we want to do scalar ops in a simd unit. I thought the SH used mode switching as well. BUt I can't recall if it was run before register allocation & reload. jeff
Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438
On 11/19/18 12:58 PM, Uros Bizjak wrote:
> Hello!
>
> The assert in create_pre_exit at mode-switching.c expects return copy
> pair with nothing in between. However, the compiler starts mode
> switching pass with the following sequence:
>
> (insn 19 18 16 2 (set (reg:V2SF 21 xmm0)
> (mem/c:V2SF (plus:DI (reg/f:DI 7 sp)
> (const_int -72 [0xffb8])) [0 S8 A64]))
> "pr88070.c":8 1157 {*movv2sf_internal}
> (nil))
> (insn 16 19 20 2 (set (reg:V2SF 0 ax [orig:91 ] [91])
> (reg:V2SF 0 ax [89])) "pr88070.c":8 1157 {*movv2sf_internal}
> (nil))
> (insn 20 16 21 2 (unspec_volatile [
> (const_int 0 [0])
> ] UNSPECV_BLOCKAGE) "pr88070.c":8 710 {blockage}
> (nil))
> (insn 21 20 23 2 (use (reg:V2SF 21 xmm0)) "pr88070.c":8 -1
> (nil))
So I know there's an updated patch. But I thought it might be worth
mentioning that insn 16 here appears to be a nop-move. Removing it
might address this instance of the problem, but I doubt it's general
enough to address any larger issues.
You still might want to investigate why it's still in the IL.
Jeff
Re: [PATCH][driver] Ensure --help=params lines end with period
On 11/20/18 4:51 AM, Tom de Vries wrote: > Hi, > > this patch ensures that gcc --help=params lines end with a period by: > - fixing the help message of param HOT_BB_COUNT_FRACTION, and > - adding a test-case. > > Build and tested on x86_64. > > OK for trunk? > > Thanks, > - Tom > > [driver] Ensure --help=params lines end with period > > 2018-11-20 Tom de Vries > > PR c/79855 > * params.def (HOT_BB_COUNT_FRACTION): Terminate help message with > period. > > * lib/options.exp (check_for_options_with_filter): New proc. > * gcc.misc-tests/help.exp: Check that --help=params lines end with > period. OK jeff
Re: [PATCH] Do not mix -fsanitize=thread and -mabi=ms (PR sanitizer/88017).
On 11/20/18 5:22 AM, Martin Liška wrote: > Hi. > > It's very similar to what I did few days ago for -fsanitize=address and > -mabi=ms. > > Patch survives tests on x86_64-linux-gnu and bootstraps. > > Ready for trunk? > Thanks, > Martin > > gcc/ChangeLog: > > 2018-11-20 Martin Liska > > PR sanitizer/88017 > * config/i386/i386.c (ix86_option_override_internal): > > gcc/testsuite/ChangeLog: > > 2018-11-20 Martin Liska > > PR sanitizer/88017 > * gcc.dg/tsan/pr88017.c: New test. OK jeff
[Bug c++/88123] New: faulty qualified name lookup of overloaded operator within generic lambda via using-directive
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88123
Bug ID: 88123
Summary: faulty qualified name lookup of overloaded operator
within generic lambda via using-directive
Product: gcc
Version: 6.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: logicstuffs at gmail dot com
Target Milestone: ---
Consider this code:
struct bar {};
namespace foo
{
void operator+(bar) {}
} // namespace foo
int main()
{
using namespace foo;
auto l = [](auto x) { +x; };
l(bar());
}
Somewhere between versions 5.5.0 and 6.1.0 (not tested with 6.0), the operator+
(binary too) lookup inside starts to fail, and fails ever since. That is, in
the exact form as posted above.
The code compiles, if we do ANY of the following:
- refactor operator+ into a regular function
- put bar class in foo namespace
- make the lambda non-generic
- make the operator+ argument non-generic, i.e. auto l = [](auto) { +bar(); };
- change using-directive into using-declaration, i.e. using foo::operator+;
- put the using namespace directive inside the lambda's body
The inheritance mechanism of the available names in the scope of the lambda
from the outer scope seems to be broken.
[Bug c++/88122] New: g++ ICE: internal compiler error: Segmentation fault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88122
Bug ID: 88122
Summary: g++ ICE: internal compiler error: Segmentation fault
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: helloqirun at gmail dot com
Target Milestone: ---
It appears to be a recent regression.
$ g++-trunk -v
Using built-in specs.
COLLECT_GCC=g++-trunk
COLLECT_LTO_WRAPPER=/home/absozero/trunk/root-gcc/libexec/gcc/x86_64-pc-linux-gnu/9.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc/configure --prefix=/home/absozero/trunk/root-gcc
--enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 9.0.0 20181120 (experimental) [trunk revision 266315] (GCC)
$ g++-trunk abc.C
abc.C:7:6: error: use of deleted function ‘::(...)
[inherited from a]’
7 | } b{3};
| ^
abc.C:6:13: internal compiler error: Segmentation fault
6 | using a ::a;
| ^
0xf0a5cf crash_signal
../../gcc/gcc/toplev.c:326
0x92bc7b tree_check(tree_node*, char const*, int, char const*, tree_code)
../../gcc/gcc/tree.h:3153
0x92bc7b maybe_explain_implicit_delete(tree_node*)
../../gcc/gcc/cp/method.c:1824
0x8e9cdc mark_used(tree_node*, int)
../../gcc/gcc/cp/decl2.c:5353
0x852af1 build_over_call
../../gcc/gcc/cp/call.c:7917
0x856bda convert_like_real
../../gcc/gcc/cp/call.c:6939
0x856b0b convert_like_real
../../gcc/gcc/cp/call.c:7069
0x852979 build_over_call
../../gcc/gcc/cp/call.c:8181
0x85589f build_new_method_call_1
../../gcc/gcc/cp/call.c:9677
0x85589f build_new_method_call(tree_node*, tree_node*, vec**, tree_node*, int, tree_node**, int)
../../gcc/gcc/cp/call.c:9752
0x8564e9 build_special_member_call(tree_node*, tree_node*, vec**, tree_node*, int, int)
../../gcc/gcc/cp/call.c:9176
0x905312 expand_default_init
../../gcc/gcc/cp/init.c:1968
0x905312 expand_aggr_init_1
../../gcc/gcc/cp/init.c:2083
0x905bfa build_aggr_init(tree_node*, tree_node*, int, int)
../../gcc/gcc/cp/init.c:1817
0x8ba84f build_aggr_init_full_exprs
../../gcc/gcc/cp/decl.c:6290
0x8ba84f check_initializer
../../gcc/gcc/cp/decl.c:6439
0x8d254c cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int)
../../gcc/gcc/cp/decl.c:7162
0x9757a6 cp_parser_init_declarator
../../gcc/gcc/cp/parser.c:20093
0x9599fa cp_parser_simple_declaration
../../gcc/gcc/cp/parser.c:13269
0x97c3e9 cp_parser_declaration
../../gcc/gcc/cp/parser.c:12966
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
$ cat abc.C
struct a {
a(...);
a();
};
struct : a {
using a ::a;
} b{3};
Re: Question about make_extraction() in combine.c
On 11/20/18 11:07 AM, Michael Eager wrote:
> On 11/18/2018 08:14 AM, Jeff Law wrote:
>> On 11/18/18 8:44 AM, Michael Eager wrote:
>>> On 11/16/18 14:50, Segher Boessenkool wrote:
Hi!
On Wed, Nov 14, 2018 at 11:22:58AM -0800, Michael Eager wrote:
> The (mem:SI) is converted to (mem:QI).
>
> The return from make_extract() is
> (zero_extract:SI (mem:QI (reg/v/f:SI 50 [ gp ]))
> (const_int 1 [0x1])
> (const_int 2 [0x2]))
>
> The target has an instruction pattern for zero_extract, but it matches
> SI values, not QI. So the instruction which implements a test of a
> bit
> flag is never generated.
The RTL documentation says:
If @var{loc} is in memory, its mode must be a single-byte integer
mode.
But obviously various ports use other modes, too. I wonder if that
ever
worked well.
>>>
>>> Thanks. I hadn't noticed this.
>>>
>>> Does anyone have any idea why there is a restriction on the mode?
>>> Other instruction patterns don't place arbitrary restriction on the
>>> memory access mode.
>> Not offhand. BUt it's been around for a long time (at least since the
>> early 90s). I stumbled over it several times through the years. It
>> could well be an artifact of the m68k or the vax.
>
> The internal RTL should not be dictating what the target arch can or
> cannot implement. Reload should insert any needed conversions,
> especially ones which narrow the size.
>
> I have a patch which removes this conversion. Works for my target. I
> built and ran 'make check' for x86 with no additional failures. I don't
> have a VAX or M68K sitting around to test. :-)
I can do correctness tests for m68k via qemu. Essentially verifying it
still bootstraps:-) Just pass along a git formatted patch and I can
throw it into the tester.
jeff
Re: [PATCH, libstdc++] Implement P0415 More constexpr for std::complex.
On 11/19/18 6:13 AM, Jonathan Wakely wrote:
On 16/11/18 19:39 -0500, Ed Smith-Rowland wrote:
@@ -322,67 +323,43 @@
//@{
/// Return new complex value @a x plus @a y.
template
- inline complex<_Tp>
+ inline _GLIBCXX20_CONSTEXPR complex<_Tp>
operator+(const complex<_Tp>& __x, const complex<_Tp>& __y)
- {
- complex<_Tp> __r = __x;
- __r += __y;
- return __r;
- }
+ { return complex<_Tp>(__x.real() + __y.real(), __x.imag() +
__y.imag()); }
Is this change (and all the similar ones) really needed?
Doesn't the fact that all the constructors and member operators of
std::complex mean that the original definition is also valid in a
constexpr function?
These changes are rolled back. Sorry.
@@ -1163,50 +1143,43 @@
#endif
template
- complex&
+ _GLIBCXX20_CONSTEXPR complex&
operator=(const complex<_Tp>& __z)
{
- __real__ _M_value = __z.real();
- __imag__ _M_value = __z.imag();
+ _M_value = __z.__rep();
These changes look OK, but I wonder if we shouldn't ask the compiler
to make it possible to use __real__ and __imag__ in constexpr
functions instead.
I assume it doesn't, and that's why you made this change. But if it
Just Worked, and the other changes I commented on above are also
unnecessary, then this patch would *mostly* just be adding
_GLIBCXX20_CONSTEXPR which is OK for stage 3 (as it doesn't affect any
dialects except C++2a).
Yes, this is the issue. I agree that constexpr _real__, __imag__would
be better.
Do you have any idea where this change would be? I grepped around a
little and couldn't figure it out. if you don't I'll look more.
Actually, looking at constexpr.c it looks like the old way ought to work...
OK, plain assignment works but not the others. Interesting.
@@ -1872,7 +1831,7 @@
{ return _Tp(); }
template
- inline typename __gnu_cxx::__promote<_Tp>::__type
+ _GLIBCXX_CONSTEXPR inline typename
__gnu_cxx::__promote<_Tp>::__type
This should be _GLIBCXX20_CONSTEXPR.
Done.
Index:
testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
===
---
testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
(nonexistent)
+++
testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
(working copy)
@@ -0,0 +1,51 @@
+// { dg-do compile { target c++2a } }
All the tests with { target c++2a} should also have:
// { dg-options "-std=gnu++2a" }
Because otherwise they are skipped by default, and only get run when
RUNTESTFLAGS explicitly includes something like
--target_board=unix/-std=gnu++2a
The dg-options needs to come first, or it doesn't apply before the
check for { target c++2a }.
Thank you, done.
Updated patch attached. I'd like to understand why
__real__ _M_value += __z.real();
doesn't work though.
Ed
Index: include/std/complex
===
--- include/std/complex (revision 266251)
+++ include/std/complex (working copy)
@@ -70,10 +70,11 @@
/// Return phase angle of @a z.
template _Tp arg(const complex<_Tp>&);
/// Return @a z magnitude squared.
- template _Tp norm(const complex<_Tp>&);
+ template _Tp _GLIBCXX20_CONSTEXPR norm(const complex<_Tp>&);
/// Return complex conjugate of @a z.
- template complex<_Tp> conj(const complex<_Tp>&);
+ template
+_GLIBCXX20_CONSTEXPR complex<_Tp> conj(const complex<_Tp>&);
/// Return complex with magnitude @a rho and angle @a theta.
template complex<_Tp> polar(const _Tp&, const _Tp& = 0);
@@ -169,18 +170,18 @@
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 387. std::complex over-encapsulated.
- void
+ _GLIBCXX20_CONSTEXPR void
real(_Tp __val) { _M_real = __val; }
- void
+ _GLIBCXX20_CONSTEXPR void
imag(_Tp __val) { _M_imag = __val; }
/// Assign a scalar to this complex number.
- complex<_Tp>& operator=(const _Tp&);
+ _GLIBCXX20_CONSTEXPR complex<_Tp>& operator=(const _Tp&);
/// Add a scalar to this complex number.
// 26.2.5/1
- complex<_Tp>&
+ _GLIBCXX20_CONSTEXPR complex<_Tp>&
operator+=(const _Tp& __t)
{
_M_real += __t;
@@ -189,7 +190,7 @@
/// Subtract a scalar from this complex number.
// 26.2.5/3
- complex<_Tp>&
+ _GLIBCXX20_CONSTEXPR complex<_Tp>&
operator-=(const _Tp& __t)
{
_M_real -= __t;
@@ -197,30 +198,30 @@
}
/// Multiply this complex number by a scalar.
- complex<_Tp>& operator*=(const _Tp&);
+ _GLIBCXX20_CONSTEXPR complex<_Tp>& operator*=(const _Tp&);
/// Divide this complex number by a scalar.
- complex<_Tp>& operator/=(const _Tp&);
+ _GLIBCXX20_CONSTEXPR complex<_Tp>& operator/=(const _Tp&);
// Let the compiler synthesize the copy assignment operator
#if __cplusplus >= 201103L
- complex& operator=(const complex&) =
[Bug lto/88112] [9 regression] ICE in lto1: TYPE_FIELDS defined in incomplete type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88112 --- Comment #5 from Eric Botcazou --- > I am not sure I fully understand the problem here, but > why we end up streaming ungimplified type at first place? Because you cannot gimplify a type declared at file scope.
[Bug lto/88112] [9 regression] ICE in lto1: TYPE_FIELDS defined in incomplete type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88112 --- Comment #4 from Eric Botcazou --- > Index: gcc/tree.c > === > --- gcc/tree.c (revision 266308) > +++ gcc/tree.c (working copy) > @@ -5260,7 +5260,7 @@ free_lang_data_in_one_sizepos (tree *exp > Note this should only happen for abstract copies so setting sizes > to NULL is OK (but we cannot easily assert this). */ >else if (expr && !is_gimple_val (expr)) > -*expr_p = NULL_TREE; > +*expr_p = build0 (PLACEHOLDER_EXPR, TREE_TYPE (expr)); > } > > > fixes the ICE but not sure if that's good. Setting to size_zero_node > also works (so much for "proper" type checking ...). Maybe it's better > to adjust the type-checker somehow? The C++ front-end should be fixed instead, i.e. it should gimplify all the size expressions (like the Ada front-end does).
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #13 from Jan Hubicka --- ... and I can also confirm that the original testcase no longer triggers.
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044 --- Comment #12 from Jan Hubicka --- Actually the issue here is that enum is context that is the type B and it has keyed vtable which makes it unmerged. So we are correct to not merge here provided that we want to keep TYPE_CONTEXT here (which I am not 100% sure we want) I will check in the testcase along the fix.
[Bug tree-optimization/87895] [7/8 Regression] ICE in purge_dead_edges, at cfgrtl.c:3246
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87895 Jakub Jelinek changed: What|Removed |Added Known to work||9.0 Summary|[7/8/9 Regression] ICE in |[7/8 Regression] ICE in |purge_dead_edges, at|purge_dead_edges, at |cfgrtl.c:3246 |cfgrtl.c:3246 Known to fail|9.0 | --- Comment #5 from Jakub Jelinek --- Fixed fir GCC 9+ so far.
[Bug c++/88110] [9 Regression] ICE (segfault) with -std=C++2a in cxx_eval_constant_expression when trying to evaluate nonoverridden "virtual ... = 0" function of a base class
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88110 Jakub Jelinek changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #5 from Jakub Jelinek --- Fixed.
[Bug c++/88118] GCC keeps unnecessary calls to new
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88118 Marc Glisse changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=58483 --- Comment #2 from Marc Glisse --- Or PR 87732 or PR 78104 or ...
[Bug d/88039] gdc.test/compilable/ddoc12.d FAILs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88039
--- Comment #4 from Iain Buclaw ---
(In reply to r...@cebitec.uni-bielefeld.de from comment #2)
> > --- Comment #1 from Iain Buclaw ---
> > (In reply to Rainer Orth from comment #0)
> >>
> >> The problem obviously is that the native assemblers don't support UTF-8 in
> >> identifiers (and I bet there are other non-gas assemblers that don't
> >> either).
> >>
> >
> > Do we have a function that checks if the effective target supports utf-8 in
> > assembler?
>
> we do: there's ucn, used in some of the gcc.dg/ucnid-*.c tests.
>
> However, there's another option: C11, 6.4.2.1 General, n.71 suggests
>
> On systems in which linkers cannot accept extended characters, an
> encoding of the universal character name may be used in forming valid
> external identifiers. For example, some otherwise unused character or
> sequence of characters may be used to encode the \u in a universal
> character name. Extended characters may produce a long external
> identifier
>
> (cited from the n1570.pdf draft). Go, which heavily relies on UTF-8
> identifiers, does something like this:
> cf. gcc/go/gofrontend/go-encode-id.{cc,h}.
>
The following comment echoes the current thoughts on UTF-8 support in
identifiers.
```
When I originally started with D, I thought non-ASCII identifiers with Unicode
was a good idea. I've since slowly become less and less enthusiastic about it.
First off, D source text simply must (and does) fully support Unicode in
comments, characters, and string literals. That's not an issue.
But identifiers? I haven't seen hardly any use of non-ascii identifiers in C,
C++, or D. In fact, I've seen zero use of it outside of test cases. I don't see
much point in expanding the support of it. If people use such identifiers, the
result would most likely be annoyance rather than illumination when people who
don't know that language have to work on the code.
```
I'm not sure given them an encoded mangle in similar vain to Go would be a
desirable direction, so better off relegating the tests.
[Bug c++/88110] [9 Regression] ICE (segfault) with -std=C++2a in cxx_eval_constant_expression when trying to evaluate nonoverridden "virtual ... = 0" function of a base class
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88110 --- Comment #4 from Jakub Jelinek --- Author: jakub Date: Tue Nov 20 22:23:12 2018 New Revision: 266329 URL: https://gcc.gnu.org/viewcvs?rev=266329=gcc=rev Log: PR c++/88110 * constexpr.c (cxx_eval_constant_expression) : Punt if get_base_address of ADDR_EXPR operand is not a DECL_P. * g++.dg/cpp2a/constexpr-virtual13.C: New test. Added: trunk/gcc/testsuite/g++.dg/cpp2a/constexpr-virtual13.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/constexpr.c trunk/gcc/testsuite/ChangeLog
Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
On Tue, 20 Nov 2018, David Malcolm wrote: > Should I do: You should do whatever is appropriate for the warning in question. But if what's appropriate for the warning in question includes types that are compatible but not the same, the comments need to avoid saying it's about the types being the same. -- Joseph S. Myers jos...@codesourcery.com
[Bug middle-end/88097] Missing optimization of endian conversion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88097 --- Comment #5 from joseph at codesourcery dot com --- As of glibc 2.28, glibc uses __builtin_bswap* when available on all architectures. commit 0d40d0ecba3b1e5b8c3b8da01c708fea3948e193 Author: Joseph Myers Date: Tue Feb 6 21:55:08 2018 + Unify and simplify bits/byteswap.h, bits/byteswap-16.h headers (bug 14508, bug 15512, bug 17082, bug 20530).
[Bug tree-optimization/88074] [7/8/9 Regression] g++ hangs on math expression
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88074 --- Comment #19 from joseph at codesourcery dot com --- On Tue, 20 Nov 2018, rguenth at gcc dot gnu.org wrote: > if (fmt->emin < min_exp) > min_exp = fmt->emin - fmt->p + 1; > so somehow the formula fmt->emin - fmt->p + 1 isn't sufficient (that's No, it's sufficient. long double and float128 have the same emin but different precision; you need to use fmt->emin - fmt->p + 1 < min_exp above, or else you'll get the wrong results if the loop looks at the long double format before float128.
Re: [C++ PATCH] Fix ICE in constexpr OBJ_TYPE_REF handling (PR c++/88110)
OK.
On Tue, Nov 20, 2018 at 3:51 PM Jakub Jelinek wrote:
>
> Hi!
>
> The comment in OBJ_TYPE_REF handling code correctly says that we are
> looking for x.D.2103.D.2094, but it is important that x is not an
> INDIRECT_REF or something similar as in the following testcase - we can't
> really devirtualize in that case because we really don't know what it points
> to. The following patch ensures that the argument got evaluated to address
> of some field of (ultimately) a decl, which is all we should get during
> valid constexpr evaluation.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-11-20 Jakub Jelinek
>
> PR c++/88110
> * constexpr.c (cxx_eval_constant_expression) : Punt
> if get_base_address of ADDR_EXPR operand is not a DECL_P.
>
> * g++.dg/cpp2a/constexpr-virtual13.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2018-11-19 14:24:49.0 +0100
> +++ gcc/cp/constexpr.c 2018-11-20 15:03:26.968152935 +0100
> @@ -4815,7 +4815,8 @@ cxx_eval_constant_expression (const cons
> obj = cxx_eval_constant_expression (ctx, obj, lval, non_constant_p,
> overflow_p);
> /* We expect something in the form of get x. */
> - if (TREE_CODE (obj) != ADDR_EXPR)
> + if (TREE_CODE (obj) != ADDR_EXPR
> + || !DECL_P (get_base_address (TREE_OPERAND (obj, 0
> {
> if (!ctx->quiet)
> error_at (cp_expr_loc_or_loc (t, input_location),
> --- gcc/testsuite/g++.dg/cpp2a/constexpr-virtual13.C.jj 2018-11-20
> 15:07:17.558386765 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/constexpr-virtual13.C2018-11-20
> 15:05:30.188140420 +0100
> @@ -0,0 +1,20 @@
> +// PR c++/88110
> +// { dg-do compile }
> +
> +struct A {
> + virtual int foo () const = 0;
> +};
> +struct B {
> + virtual int bar () const = 0;
> + virtual int baz () const = 0;
> +};
> +struct C : public A { };
> +struct D : public C { };
> +struct E : public D, public B { };
> +
> +void
> +qux (const E *x)
> +{
> + if (x->baz ())
> +;
> +}
>
> Jakub
Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)
On Tue, 2018-11-20 at 02:46 +, Joseph Myers wrote:
> On Mon, 19 Nov 2018, David Malcolm wrote:
>
> > +/* C implementation of same_type_p.
> > + Returns true iff TYPE1 and TYPE2 are the same type, in the
> > usual
> > + sense of `same'. */
> > +
> > +bool
> > +same_type_p (tree type1, tree type2)
> > +{
> > + return comptypes (type1, type2) == 1;
> > +}
>
> I don't think "compatible" and "same" are the same concept. Normally
> in C
> you'd be concerned with compatibility; "same type" is only used for
> the
> rule on duplicate typedefs, which uses
> comptypes_check_different_types.
The purpose here is to be able to offer fix-it hints for bogus code
that's missing an '&' or a '*' prefix, and have that code live in c-
common.c
Jason wanted to avoid a pointer-equality test for types by using
same_type_p to look through enums - but same_type_p is C++-specific.
Should I do:
(a) something like this for C:
/* C implementation of same_type_p.
Returns true iff TYPE1 and TYPE2 are the same type, or are
compatible enough to be permitted in C11 typedef redeclarations. */
bool
same_type_p (tree type1, tree type2)
{
bool different_types_p = false;
int result = comptypes_check_different_types (type1, type2,
_types_p);
if (result == 1 && !different_types_p)
return true;
return false;
}
(b) provide a same_type_p for C that e.g. simply does pointer equality,
(d) add a newly named function (e.g. "compatible_types_p", as C++ has a
comptypes, but it has a 3rd param), or
(d) fall back to simply doing pointer equality.
Thanks
Dave
Re: C++ PATCH to implement P1094R2, Nested inline namespaces
On 11/19/18 5:12 PM, Marek Polacek wrote:
On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
2018-11-19 Marek Polacek
Implement P1094R2, Nested inline namespaces.
* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.
* g++.dg/cpp2a/nested-inline-ns3.C: New test.
Just a small testsuite comment.
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
Especially because 2a testing isn't included by default, but also
to make sure it works right even with -std=c++17, wouldn't it be better to
drop the nested-inline-ns3.C test, make this test c++17 or
even better always enabled, add dg-options "-Wpedantic" and
just add dg-warning with c++17_down and c++14_down what should be
warned on the 3 lines (with .-1 for c++14_down)?
Or if you want add some further testcases that will test how
c++17 etc. will dg-error on those with -pedantic-errors etc.
Sure, I've made it { target c++11 } and dropped the third test:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-11-19 Marek Polacek
Implement P1094R2, Nested inline namespaces.
* parser.c (cp_parser_namespace_definition): Parse the optional inline
keyword in a nested-namespace-definition. Adjust push_namespace call.
Formatting fix.
* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 292cce15676..f39e9d753d2 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
cp_ensure_no_oacc_routine (parser);
bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+ const bool topmost_inline_p = is_inline;
if (is_inline)
{
@@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
{
identifier = NULL_TREE;
+ bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+RID_INLINE);
+ if (nested_inline_p && nested_definition_count != 0)
+ {
+ if (cxx_dialect < cxx2a)
+ pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+OPT_Wpedantic, "nested inline namespace definitions only "
+"available with -std=c++2a or -std=gnu++2a");
+ cp_lexer_consume_token (parser->lexer);
+ }
This looks like we won't get any diagnostic in lower conformance modes
if there are multiple namespace scopes before the inline keyword.
if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
{
identifier = cp_parser_identifier (parser);
@@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
}
if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
- break;
+ {
+ /* Don't forget that the innermost namespace might have been
+marked as inline. */
+ is_inline |= nested_inline_p;
This looks wrong: an inline namespace does not make its nested
namespaces inline as well.
Jason
[PATCH 5/6] [og8] Backport parts of upstream declare-allocate patch
This patch adjusts mappings used for some special cases in Fortran
(e.g. allocatable scalars) on og8 to match code that is already upstream,
or that has been submitted but not yet reviewed. Parts taken from
https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01205.html and parts
reverted from https://gcc.gnu.org/ml/gcc-patches/2017-01/msg02188.html.
gcc/fortran/
* trans-openmp.c (gfc_omp_finish_clause): Don't use
GOMP_MAP_FIRSTPRIVATE_POINTER.
(gfc_trans_omp_clauses_1): Adjust handling of allocatable scalars.
gcc/
* gimplify.c (demote_firstprivate_pointer): Remove.
(gimplify_scan_omp_clauses): Remove special handling for OpenACC. Don't
call demote_firstprivate_pointer.
(gimplify_adjust_omp_clauses): Adjust promotion of reduction clauses.
* omp-low.c (lower_omp_target): Remove special handling for Fortran.
gcc/testsuite/
* gfortran.dg/goacc/kernels-alias-3.f95: Revert comment changes and
XFAIL.
libgomp/
* testsuite/libgomp.oacc-fortran/non-scalar-data.f90: Remove XFAIL for
-O2 and -O3 and explanatory comment.
---
gcc/fortran/trans-openmp.c | 22 -
gcc/gimplify.c | 49 ++-
gcc/omp-low.c |3 +-
.../gfortran.dg/goacc/kernels-alias-3.f95 |4 +-
.../libgomp.oacc-fortran/non-scalar-data.f90 |6 +--
5 files changed, 20 insertions(+), 64 deletions(-)
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 98f40d1..71a3ebb 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -1084,7 +1084,7 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p)
return;
tree orig_decl = decl;
c4 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
- OMP_CLAUSE_SET_MAP_KIND (c4, GOMP_MAP_FIRSTPRIVATE_POINTER);
+ OMP_CLAUSE_SET_MAP_KIND (c4, GOMP_MAP_POINTER);
OMP_CLAUSE_DECL (c4) = decl;
OMP_CLAUSE_SIZE (c4) = size_int (0);
decl = build_fold_indirect_ref (decl);
@@ -1100,10 +1100,7 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p)
OMP_CLAUSE_SIZE (c3) = size_int (0);
decl = build_fold_indirect_ref (decl);
OMP_CLAUSE_DECL (c) = decl;
- OMP_CLAUSE_SET_MAP_KIND (c4, GOMP_MAP_POINTER);
}
- if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (decl)))
- OMP_CLAUSE_SET_MAP_KIND (c4, GOMP_MAP_POINTER);
}
if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (decl)))
{
@@ -2168,11 +2165,15 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, gfc_omp_clauses *clauses,
(TREE_TYPE (TREE_TYPE (field)
{
tree orig_decl = decl;
- enum gomp_map_kind gmk = GOMP_MAP_FIRSTPRIVATE_POINTER;
- if (GFC_DECL_GET_SCALAR_ALLOCATABLE (decl)
- && (n->sym->attr.oacc_declare_create)
- && clauses->update_allocatable)
- gmk = ptr_map_kind;
+ enum gomp_map_kind gmk = GOMP_MAP_POINTER;
+ if (GFC_DECL_GET_SCALAR_ALLOCATABLE (field)
+ && n->sym->attr.oacc_declare_create)
+ {
+ if (clauses->update_allocatable)
+ gmk = GOMP_MAP_ALWAYS_POINTER;
+ else
+ gmk = GOMP_MAP_FIRSTPRIVATE_POINTER;
+ }
node4 = build_omp_clause (input_location,
OMP_CLAUSE_MAP);
OMP_CLAUSE_SET_MAP_KIND (node4, gmk);
@@ -2189,10 +2190,7 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, gfc_omp_clauses *clauses,
OMP_CLAUSE_DECL (node3) = decl;
OMP_CLAUSE_SIZE (node3) = size_int (0);
decl = build_fold_indirect_ref (decl);
- OMP_CLAUSE_SET_MAP_KIND (node4, GOMP_MAP_POINTER);
}
- if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (decl)))
- OMP_CLAUSE_SET_MAP_KIND (node4, GOMP_MAP_POINTER);
}
if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (decl))
&& n->u.map_op != OMP_MAP_ATTACH
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 40bf586..7f55cfd 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -7634,37 +7634,6 @@ find_decl_expr (tree *tp, int *walk_subtrees, void *data)
return NULL_TREE;
}
-static void
-demote_firstprivate_pointer (tree decl, gimplify_omp_ctx *ctx)
-{
- if (!lang_GNU_Fortran ())
-return;
-
- while (ctx)
-{
- if (ctx->region_type == ORT_ACC_PARALLEL
- || ctx->region_type == ORT_ACC_KERNELS)
- break;
- ctx = ctx->outer_context;
-}
-
- if (ctx == NULL)
-return;
-
- tree clauses = ctx->clauses;
-
- for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
-{
- if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
- && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
- && OMP_CLAUSE_DECL (c) == decl)
- {
- OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_POINTER);
- return;
- }
-}
-}
-
/* Insert a GOMP_MAP_ALLOC or GOMP_MAP_RELEASE node following a
GOMP_MAP_STRUCT mapping. C is an always_pointer mapping. STRUCT_NODE is
the struct node to insert the new mapping after (when the struct node is
@@ -7843,7 +7812,7 @@
[PATCH 6/6] [og8] OpenACC refcounting refresh
This patch represents a mild overhaul of reference counting for OpenACC
in libgomp. It's been partly automatically checked (using code not yet
quite finished nor submitted upstream), but it's already more precise
than the pre-patch implementation (as demonstrated by adjustments to
previously-erroneous tests, included).
I have a few more changes planned, but those are still tbd.
libgomp/
* libgomp.h (gomp_device_descr): Add GOMP_MAP_VARS_OPENACC_ENTER_DATA.
(gomp_acc_remove_pointer): Update prototype.
(gomp_acc_data_env_remove_tgt): Add prototype.
(gomp_unmap_vars, gomp_map_vars_async): Update prototype.
* oacc-int.h (goacc_async_copyout_unmap_vars): Update prototype.
* oacc-async.c (goacc_async_copyout_unmap_vars): Remove finalize
parameter.
* oacc-init.c (acc_shutdown_1): Remove finalize argument to
gomp_unmap_vars call.
* oacc-mem.c (lookup_dev_1): New helper function.
(lookup_dev): Rewrite in terms of above.
(acc_free): Update calls to lookup_dev.
(acc_map_data): Likewise. Don't add data mapped this way to OpenACC
data environment list.
(gomp_acc_data_env_remove, gomp_acc_data_env_remove_tgt): New functions.
(acc_unmap_data): Rewrite using splay tree functions directly. Don't
call gomp_unmap_vars. Fix refcount handling.
(present_create_copy): Use GOMP_MAP_VARS_OPENACC_ENTER_DATA in
gomp_map_vars_async call. Adjust refcount handling.
(delete_copyout): Remove dubious handling of target_mem_desc refcount.
(gomp_acc_insert_pointer): Use GOMP_MAP_VARS_OPENACC_ENTER_DATA in
gomp_map_vars_async call. Update refcount handling.
(gomp_acc_remove_pointer): Reimplement. Fix detach and refcount
handling.
* oacc-parallel.c (find_pointer): Handle more mapping types. Update
calls to gomp_unmap_vars and goacc_async_copyout_unmap_vars.
(GOACC_enter_exit_data): Update refcount handling.
libgomp/
* target.c (gomp_detach_pointer): Unlock device on error path.
(gomp_map_vars_async): Support GOMP_MAP_VARS_OPENACC_ENTER_DATA and
mapping size fix GOMP_MAP_ATTACH.
(gomp_unmap_tgt): Call gomp_acc_data_env_remove_tgt.
(gomp_unmap_vars): Remove finalize parameter.
(gomp_unmap_vars_async): Likewise. Adjust detach handling.
(GOMP_target, GOMP_target_ext, GOMP_target_end_data)
(gomp_target_task_fn): Update calls to gomp_unmap_vars.
* testsuite/libgomp.oacc-c-c++-common/context-2.c: Use correct API to
unmap data.
* testsuite/libgomp.oacc-c-c++-common/context-4.c: Likewise.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-6.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-7.c: New test.
* testsuite/libgomp.oacc-c-c++-common/deep-copy-8.c: New test.
* testsuite/libgomp.oacc-fortran/data-2.f90: Fix for unmap semantics.
---
libgomp/libgomp.h | 10 +-
libgomp/oacc-async.c |4 +-
libgomp/oacc-init.c|2 +-
libgomp/oacc-int.h |2 +-
libgomp/oacc-mem.c | 387 ++--
libgomp/oacc-parallel.c| 76 +++--
libgomp/target.c | 35 ++-
.../libgomp.oacc-c-c++-common/context-2.c |6 +-
.../libgomp.oacc-c-c++-common/context-4.c |6 +-
.../libgomp.oacc-c-c++-common/deep-copy-6.c| 59 +++
.../libgomp.oacc-c-c++-common/deep-copy-7.c| 42 +++
.../libgomp.oacc-c-c++-common/deep-copy-8.c| 53 +++
libgomp/testsuite/libgomp.oacc-fortran/data-2.f90 | 20 +-
13 files changed, 445 insertions(+), 257 deletions(-)
create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-6.c
create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-7.c
create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-8.c
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 17fe0d3..568e260 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -1002,6 +1002,7 @@ struct gomp_device_descr
enum gomp_map_vars_kind
{
GOMP_MAP_VARS_OPENACC,
+ GOMP_MAP_VARS_OPENACC_ENTER_DATA,
GOMP_MAP_VARS_TARGET,
GOMP_MAP_VARS_DATA,
GOMP_MAP_VARS_ENTER_DATA
@@ -1010,7 +1011,8 @@ enum gomp_map_vars_kind
struct gomp_coalesce_buf;
extern void gomp_acc_insert_pointer (size_t, void **, size_t *, void *, int);
-extern void gomp_acc_remove_pointer (void *, size_t, bool, int, int, int);
+extern void gomp_acc_remove_pointer (void **, size_t *, unsigned short *,
+ int, void *, bool, int);
extern void gomp_acc_declare_allocate (bool, size_t, void **, size_t *,
unsigned short *);
struct gomp_coalesce_buf;
@@ -1039,10 +1041,12 @@ extern struct
[PATCH 3/6] [og8] OpenACC 2.6 manual deep copy support (attach/detach)
Previously posted upstream: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00826.html gcc/c/ * c-parser.c (c_parser_omp_variable_list): Allow deref (->) in variable lists. (c_parser_oacc_all_clauses): Re-alphabetize cases. * c-typeck.c (handle_omp_array_sections_1): Support deref. gcc/cp/ * parser.c (cp_parser_omp_var_list_no_open): Support deref. (cp_parser_oacc_all_clauses): Re-alphabetize cases. * semantics.c (finish_omp_clauses): Allow "this" for OpenACC data clauses. Support deref. gcc/fortran/ * gfortran.h (gfc_omp_map_op): Add OMP_MAP_ATTACH, OMP_MAP_DETACH. * openmp.c (omp_mask2): Add OMP_CLAUSE_ATTACH, OMP_CLAUSE_DETACH. (gfc_match_omp_clauses): Remove allow_derived parameter, infer from clause mask. Support attach and detach. Slight reformatting. (OACC_PARALLEL_CLAUSES, OACC_KERNELS_CLAUSES, OACC_DATA_CLAUSES) (OACC_ENTER_DATA_CLAUSES): Add OMP_CLAUSE_ATTACH. (OACC_EXIT_DATA_CLAUSES): Add OMP_CLAUSE_DETACH. (match_acc): Remove derived_types parameter, and don't pass to gfc_match_omp_clauses. (gfc_match_oacc_update): Don't pass allow_derived argument. (gfc_match_oacc_enter_data): Likewise. (gfc_match_oacc_exit_data): Likewise. (check_symbol_not_pointer): Don't disallow pointer objects of derived type. (resolve_oacc_data_clauses): Don't disallow allocatable derived types. (resolve_omp_clauses): Perform duplicate checking only for non-derived type component accesses (plain variables and arrays or array sections). Support component refs. * trans-openmp.c (gfc_omp_privatize_by_reference): Support component refs. (gfc_trans_omp_clauses_1): Support component refs, attach and detach clauses. gcc/ * gimplify.c (gimplify_omp_var_data): Add GOVD_MAP_HAS_ATTACHMENTS. (insert_struct_component_mapping): Support derived-type member mappings for arrays with descriptors which use GOMP_MAP_TO_PSET. (gimplify_scan_omp_clauses): Rewrite GOMP_MAP_ALWAYS_POINTER to GOMP_MAP_ATTACH for OpenACC struct/derived-type component pointers. Handle pointer mappings that use GOMP_MAP_TO_PSET. Handle attach/detach clauses. (gimplify_adjust_omp_clauses_1): Skip adjustments for explicit attach/detach clauses. (gimplify_omp_target_update): Handle finalize for detach. gcc/testsuite/ * c-c++-common/goacc/mdc-1.c: Update scan tests. * gfortran.dg/goacc/data-clauses.f95: Remove expected errors. * gfortran.dg/goacc/derived-types.f90: Likewise. * gfortran.dg/goacc/enter-exit-data.f95: Likewise. libgomp/ * libgomp.h (struct target_var_desc): Add do_detach flag. (struct splay_tree_key_s): Add attach_count field. (struct gomp_coalesce_buf): Add forward declaration. (gomp_map_val, gomp_attach_pointer, gomp_detach_pointer): Add prototypes. (gomp_unmap_vars): Add finalize parameter. * libgomp.map (OACC_2.6): New section. Add acc_attach, acc_attach_async, acc_detach, acc_detach_async, acc_detach_finalize, acc_detach_finalize_async. * oacc-async.c (goacc_async_copyout_unmap_vars): Add finalize parameter. Pass to gomp_unmap_vars_async. * oacc-init.c (acc_shutdown_1): Update call to gomp_unmap_vars. * oacc-int.h (goacc_async_copyout_unmap_vars): Add finalize parameter. * oacc-mem.c (acc_unmap_data): Update call to gomp_unmap_vars. (present_create_copy): Initialise attach_count. (delete_copyout): Likewise. (gomp_acc_insert_pointer): Likewise. (gomp_acc_remove_pointer): Update calls to gomp_unmap_vars, goacc_async_copyout_unmap_vars. (acc_attach_async, acc_attach, goacc_detach_internal, acc_detach) (acc_detach_async, acc_detach_finalize, acc_detach_finalize_async): New functions. * oacc-parallel.c (find_pointer): Support attach/detach. Make a little more strict. (GOACC_parallel_keyed_internal): Use gomp_map_val to calculate device addresses. Update calls to gomp_unmap_vars, goacc_async_copyout_unmap_vars. (GOACC_data_end): Update call to gomp_unmap_vars. (GOACC_enter_exit_data): Support attach/detach and GOMP_MAP_STRUCT. * openacc.h (acc_attach, acc_attach_async, acc_detach) (acc_detach_async, acc_detach_finalize, acc_detach_finalize_async): Add prototypes. * target.c (limits.h): Include. (gomp_map_vars_existing): Initialise do_detach field of tgt_var_desc. (gomp_attach_pointer, gomp_detach_pointer): New functions. (gomp_map_val): Make global. (gomp_map_vars_async): Support attach and detach. (gomp_remove_var): Free attach count array if present.
[PATCH 0/6] [og8] OpenACC attach/detach
This patch series is a backport of the OpenACC attach/detach support to the openacc-gcc-8-branch branch. It was previously posted upstream here: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00823.html This version of the series has been adjusted to account for features on the branch that are not yet upstream. It also contains improvements to the reference counting behaviour, partially verified using self-checking code (not quite complete, and not yet submitted). Tested (as a series) with offloading to nvptx. I will apply to the openacc-gcc-8-branch shortly. Julian Brown (6): [og8] Host-to-device transfer coalescing & magic offset value self-documentation [og8] Factor out duplicate code in gimplify_scan_omp_clauses [og8] OpenACC 2.6 manual deep copy support (attach/detach) [og8] Interaction of dynamic/multidimensional arrays with attach/detach. [og8] Backport parts of upstream declare-allocate patch [og8] OpenACC refcounting refresh gcc/c/c-parser.c | 15 +- gcc/c/c-typeck.c |4 + gcc/cp/parser.c| 16 +- gcc/cp/semantics.c |6 +- gcc/fortran/gfortran.h |2 + gcc/fortran/openmp.c | 126 -- gcc/fortran/trans-openmp.c | 163 +++- gcc/gimplify.c | 414 ++ gcc/omp-low.c | 13 +- .../c-c++-common/goacc/deep-copy-multidim.c| 32 ++ gcc/testsuite/c-c++-common/goacc/mdc-1.c | 10 +- gcc/testsuite/gfortran.dg/goacc/data-clauses.f95 | 38 +- gcc/testsuite/gfortran.dg/goacc/derived-types.f90 | 23 +- .../gfortran.dg/goacc/enter-exit-data.f95 | 24 +- .../gfortran.dg/goacc/kernels-alias-3.f95 |4 +- libgomp/libgomp.h | 30 ++- libgomp/libgomp.map| 10 + libgomp/oacc-mem.c | 459 libgomp/oacc-parallel.c| 212 -- libgomp/openacc.h |6 + libgomp/target.c | 291 +++-- .../libgomp.oacc-c-c++-common/context-2.c |6 +- .../libgomp.oacc-c-c++-common/context-4.c |6 +- .../libgomp.oacc-c-c++-common/deep-copy-1.c| 24 + .../libgomp.oacc-c-c++-common/deep-copy-2.c| 29 ++ .../libgomp.oacc-c-c++-common/deep-copy-3.c| 34 ++ .../libgomp.oacc-c-c++-common/deep-copy-4.c| 87 .../libgomp.oacc-c-c++-common/deep-copy-5.c| 81 .../libgomp.oacc-c-c++-common/deep-copy-6.c| 59 +++ .../libgomp.oacc-c-c++-common/deep-copy-7.c| 42 ++ .../libgomp.oacc-c-c++-common/deep-copy-8.c| 53 +++ libgomp/testsuite/libgomp.oacc-fortran/data-2.f90 | 20 +- .../testsuite/libgomp.oacc-fortran/deep-copy-1.f90 | 35 ++ .../testsuite/libgomp.oacc-fortran/deep-copy-2.f90 | 33 ++ .../testsuite/libgomp.oacc-fortran/deep-copy-3.f90 | 34 ++ .../testsuite/libgomp.oacc-fortran/deep-copy-4.f90 | 49 ++ .../testsuite/libgomp.oacc-fortran/deep-copy-5.f90 | 57 +++ .../testsuite/libgomp.oacc-fortran/deep-copy-6.f90 | 61 +++ .../testsuite/libgomp.oacc-fortran/deep-copy-7.f90 | 89 .../testsuite/libgomp.oacc-fortran/deep-copy-8.f90 | 41 ++ .../libgomp.oacc-fortran/derived-type-1.f90|6 +- .../libgomp.oacc-fortran/non-scalar-data.f90 |6 +- .../testsuite/libgomp.oacc-fortran/update-2.f90| 44 +- 43 files changed, 2079 insertions(+), 715 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/goacc/deep-copy-multidim.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-1.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-2.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-3.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-4.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-5.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-6.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-7.c create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-8.c create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-1.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-2.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-3.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-4.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-5.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-6.f90 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/deep-copy-7.f90 create
[PATCH 4/6] [og8] Interaction of dynamic/multidimensional arrays with attach/detach.
OpenACC multidimensional (or "dynamic") arrays do not seem to fit very
neatly into the attach/detach mechanism described for OpenACC 2.6,
that is if the user tries to use a multidimensional array as a field
in a struct. This patch disallows that combination, for now at least.
Multidimensional array support in general has been submitted upstream
here but not yet accepted:
https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00937.html
gcc/
* omp-low.c (scan_sharing_clauses): Disallow dynamic (multidimensional)
arrays within structs.
gcc/testsuite/
* c-c++-common/goacc/deep-copy-multidim.c: Add test.
libgomp/
* target.c (gomp_map_vars_async, gomp_load_image_to_device):
Zero-initialise do_detach, dynamic_refcount and attach_count in more
places.
---
gcc/omp-low.c | 10 +-
.../c-c++-common/goacc/deep-copy-multidim.c| 32
libgomp/target.c |6
3 files changed, 47 insertions(+), 1 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/goacc/deep-copy-multidim.c
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index e559211..1726451 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -1481,7 +1481,15 @@ scan_sharing_clauses (tree clauses, omp_context *ctx,
t = TREE_TYPE (t);
}
- install_var_field (da_decl, by_ref, 3, ctx);
+ if (DECL_P (decl))
+ install_var_field (da_decl, by_ref, 3, ctx);
+ else
+ {
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "dynamic arrays cannot be used within structs");
+ break;
+ }
+
tree new_var = install_var_local (da_decl, ctx);
bool existed = ctx->dynamic_arrays->put (new_var, da_dimensions);
diff --git a/gcc/testsuite/c-c++-common/goacc/deep-copy-multidim.c b/gcc/testsuite/c-c++-common/goacc/deep-copy-multidim.c
new file mode 100644
index 000..1696f0c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/deep-copy-multidim.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+
+#include
+#include
+
+struct dc
+{
+ int a;
+ int **b;
+};
+
+int
+main ()
+{
+ int n = 100, i, j;
+ struct dc v = { .a = 3 };
+
+ v.b = (int **) malloc (sizeof (int *) * n);
+ for (i = 0; i < n; i++)
+v.b[i] = (int *) malloc (sizeof (int) * n);
+
+#pragma acc parallel loop copy(v.a, v.b[:n][:n]) /* { dg-error "dynamic arrays cannot be used within structs" } */
+ for (i = 0; i < n; i++)
+for (j = 0; j < n; j++)
+ v.b[i][j] = v.a + i + j;
+
+ for (i = 0; i < n; i++)
+for (j = 0; j < n; j++)
+ assert (v.b[i][j] == v.a + i + j);
+
+ return 0;
+}
diff --git a/libgomp/target.c b/libgomp/target.c
index d9d42eb..da51291 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1484,6 +1484,7 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
set to false here. */
tgt->list[i].copy_from = false;
tgt->list[i].always_copy_from = false;
+ tgt->list[i].do_detach = false;
size_t align = (size_t) 1 << (kind >> rshift);
tgt_size = (tgt_size + align - 1) & ~(align - 1);
@@ -1521,6 +1522,8 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
k->tgt = tgt;
k->refcount = 1;
+ k->dynamic_refcount = 0;
+ k->attach_count = NULL;
k->link_key = NULL;
tgt_size = (tgt_size + align - 1) & ~(align - 1);
target_row_addr = tgt->tgt_start + tgt_size;
@@ -1532,6 +1535,7 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
= GOMP_MAP_COPY_FROM_P (kind & typemask);
row_desc->always_copy_from
= GOMP_MAP_ALWAYS_FROM_P (kind & typemask);
+ row_desc->do_detach = false;
row_desc->offset = 0;
row_desc->length = da->data_row_size;
@@ -1839,6 +1843,7 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
k->tgt = tgt;
k->tgt_offset = target_table[i].start;
k->refcount = REFCOUNT_INFINITY;
+ k->attach_count = NULL;
k->link_key = NULL;
tgt->list[i].key = k;
tgt->refcount++;
@@ -1873,6 +1878,7 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
k->tgt = tgt;
k->tgt_offset = target_var->start;
k->refcount = target_size & link_bit ? REFCOUNT_LINK : REFCOUNT_INFINITY;
+ k->attach_count = NULL;
k->link_key = NULL;
tgt->list[i].key = k;
tgt->refcount++;
[PATCH 1/6] [og8] Host-to-device transfer coalescing & magic offset value self-documentation
Previously posted upstream:
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00825.html
libgomp/
* libgomp.h (OFFSET_INLINED, OFFSET_POINTER, OFFSET_STRUCT): Define.
* target.c (FIELD_TGT_EMPTY): Define.
(gomp_coalesce_chunk): New.
(gomp_coalesce_buf): Use above instead of flat array of size_t pairs.
(gomp_coalesce_buf_add): Adjust for above change.
(gomp_copy_host2dev): Likewise.
(gomp_map_val): Use OFFSET_* macros instead of magic constants. Write
as switch instead of list of ifs.
(gomp_map_vars_async): Adjust for gomp_coalesce_chunk change. Use
OFFSET_* macros.
---
libgomp/libgomp.h |5 +++
libgomp/target.c | 101 +++-
2 files changed, 65 insertions(+), 41 deletions(-)
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 607f4c2..acf7f8f 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -842,6 +842,11 @@ struct target_mem_desc {
artificial pointer to "omp declare target link" object. */
#define REFCOUNT_LINK (~(uintptr_t) 1)
+/* Special offset values. */
+#define OFFSET_INLINED (~(uintptr_t) 0)
+#define OFFSET_POINTER (~(uintptr_t) 1)
+#define OFFSET_STRUCT (~(uintptr_t) 2)
+
struct splay_tree_key_s {
/* Address of the host object. */
uintptr_t host_start;
diff --git a/libgomp/target.c b/libgomp/target.c
index ab17650..7220ac6 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -45,6 +45,8 @@
#include "plugin-suffix.h"
#endif
+#define FIELD_TGT_EMPTY (~(size_t) 0)
+
static void gomp_target_init (void);
/* The whole initialization code for offloading plugins is only run one. */
@@ -206,8 +208,14 @@ goacc_device_copy_async (struct gomp_device_descr *devicep,
}
}
-/* Infrastructure for coalescing adjacent or nearly adjacent (in device addresses)
- host to device memory transfers. */
+/* Infrastructure for coalescing adjacent or nearly adjacent (in device
+ addresses) host to device memory transfers. */
+
+struct gomp_coalesce_chunk
+{
+ /* The starting and ending point of a coalesced chunk of memory. */
+ size_t start, end;
+};
struct gomp_coalesce_buf
{
@@ -215,10 +223,10 @@ struct gomp_coalesce_buf
it will be copied to the device. */
void *buf;
struct target_mem_desc *tgt;
- /* Array with offsets, chunks[2 * i] is the starting offset and
- chunks[2 * i + 1] ending offset relative to tgt->tgt_start device address
+ /* Array with offsets, chunks[i].start is the starting offset and
+ chunks[i].end ending offset relative to tgt->tgt_start device address
of chunks which are to be copied to buf and later copied to device. */
- size_t *chunks;
+ struct gomp_coalesce_chunk *chunks;
/* Number of chunks in chunks array, or -1 if coalesce buffering should not
be performed. */
long chunk_cnt;
@@ -251,14 +259,14 @@ gomp_coalesce_buf_add (struct gomp_coalesce_buf *cbuf, size_t start, size_t len)
{
if (cbuf->chunk_cnt < 0)
return;
- if (start < cbuf->chunks[2 * cbuf->chunk_cnt - 1])
+ if (start < cbuf->chunks[cbuf->chunk_cnt-1].end)
{
cbuf->chunk_cnt = -1;
return;
}
- if (start < cbuf->chunks[2 * cbuf->chunk_cnt - 1] + MAX_COALESCE_BUF_GAP)
+ if (start < cbuf->chunks[cbuf->chunk_cnt-1].end + MAX_COALESCE_BUF_GAP)
{
- cbuf->chunks[2 * cbuf->chunk_cnt - 1] = start + len;
+ cbuf->chunks[cbuf->chunk_cnt-1].end = start + len;
cbuf->use_cnt++;
return;
}
@@ -268,8 +276,8 @@ gomp_coalesce_buf_add (struct gomp_coalesce_buf *cbuf, size_t start, size_t len)
if (cbuf->use_cnt == 1)
cbuf->chunk_cnt--;
}
- cbuf->chunks[2 * cbuf->chunk_cnt] = start;
- cbuf->chunks[2 * cbuf->chunk_cnt + 1] = start + len;
+ cbuf->chunks[cbuf->chunk_cnt].start = start;
+ cbuf->chunks[cbuf->chunk_cnt].end = start + len;
cbuf->chunk_cnt++;
cbuf->use_cnt = 1;
}
@@ -301,20 +309,20 @@ gomp_copy_host2dev (struct gomp_device_descr *devicep,
if (cbuf)
{
uintptr_t doff = (uintptr_t) d - cbuf->tgt->tgt_start;
- if (doff < cbuf->chunks[2 * cbuf->chunk_cnt - 1])
+ if (doff < cbuf->chunks[cbuf->chunk_cnt-1].end)
{
long first = 0;
long last = cbuf->chunk_cnt - 1;
while (first <= last)
{
long middle = (first + last) >> 1;
- if (cbuf->chunks[2 * middle + 1] <= doff)
+ if (cbuf->chunks[middle].end <= doff)
first = middle + 1;
- else if (cbuf->chunks[2 * middle] <= doff)
+ else if (cbuf->chunks[middle].start <= doff)
{
- if (doff + sz > cbuf->chunks[2 * middle + 1])
+ if (doff + sz > cbuf->chunks[middle].end)
gomp_fatal ("internal libgomp cbuf error");
- memcpy ((char *) cbuf->buf + (doff - cbuf->chunks[0]),
+ memcpy ((char *) cbuf->buf + (doff - cbuf->chunks[0].start),
h, sz);
return;
}
@@ -538,17 +546,25 @@ gomp_map_val (struct target_mem_desc *tgt, void **hostaddrs, size_t i)
return tgt->list[i].key->tgt->tgt_start
[PATCH 2/6] [og8] Factor out duplicate code in gimplify_scan_omp_clauses
Previously posted upstream:
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00824.html
gcc/
* gimplify.c (insert_struct_component_mapping)
(check_base_and_compare_lt): New.
(gimplify_scan_omp_clauses): Outline duplicated code into calls to
above two functions.
---
gcc/gimplify.c | 307
1 files changed, 174 insertions(+), 133 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 9be0b70..824e020 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -7661,6 +7661,160 @@ demote_firstprivate_pointer (tree decl, gimplify_omp_ctx *ctx)
}
}
+/* Insert a GOMP_MAP_ALLOC or GOMP_MAP_RELEASE node following a
+ GOMP_MAP_STRUCT mapping. C is an always_pointer mapping. STRUCT_NODE is
+ the struct node to insert the new mapping after (when the struct node is
+ initially created). PREV_NODE is the first of two or three mappings for a
+ pointer, and is either:
+ - the node before C, when a pair of mappings is used, e.g. for a C/C++
+ array section.
+ - not the node before C. This is true when we have a reference-to-pointer
+ type (with a mapping for the reference and for the pointer), or for
+ Fortran derived-type mappings with a GOMP_MAP_TO_PSET.
+ If SCP is non-null, the new node is inserted before *SCP.
+ if SCP is null, the new node is inserted before PREV_NODE.
+ The return type is:
+ - PREV_NODE, if SCP is non-null.
+ - The newly-created ALLOC or RELEASE node, if SCP is null.
+ - The second newly-created ALLOC or RELEASE node, if we are mapping a
+ reference to a pointer. */
+
+static tree
+insert_struct_component_mapping (enum tree_code code, tree c, tree struct_node,
+ tree prev_node, tree *scp)
+{
+ enum gomp_map_kind mkind = (code == OMP_TARGET_EXIT_DATA
+ || code == OACC_EXIT_DATA)
+ ? GOMP_MAP_RELEASE : GOMP_MAP_ALLOC;
+
+ tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
+ tree cl = scp ? prev_node : c2;
+ OMP_CLAUSE_SET_MAP_KIND (c2, mkind);
+ OMP_CLAUSE_DECL (c2) = unshare_expr (OMP_CLAUSE_DECL (c));
+ OMP_CLAUSE_CHAIN (c2) = scp ? *scp : prev_node;
+ OMP_CLAUSE_SIZE (c2) = TYPE_SIZE_UNIT (ptr_type_node);
+ if (struct_node)
+OMP_CLAUSE_CHAIN (struct_node) = c2;
+
+ /* We might need to create an additional mapping if we have a reference to a
+ pointer (in C++). Don't do this if we have something other than a
+ GOMP_MAP_ALWAYS_POINTER though, i.e. a GOMP_MAP_TO_PSET. */
+ if (OMP_CLAUSE_CHAIN (prev_node) != c
+ && OMP_CLAUSE_CODE (OMP_CLAUSE_CHAIN (prev_node)) == OMP_CLAUSE_MAP
+ && (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (prev_node))
+ == GOMP_MAP_ALWAYS_POINTER))
+{
+ tree c4 = OMP_CLAUSE_CHAIN (prev_node);
+ tree c3 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
+ OMP_CLAUSE_SET_MAP_KIND (c3, mkind);
+ OMP_CLAUSE_DECL (c3) = unshare_expr (OMP_CLAUSE_DECL (c4));
+ OMP_CLAUSE_SIZE (c3) = TYPE_SIZE_UNIT (ptr_type_node);
+ OMP_CLAUSE_CHAIN (c3) = prev_node;
+ if (!scp)
+ OMP_CLAUSE_CHAIN (c2) = c3;
+ else
+ cl = c3;
+}
+
+ if (scp)
+*scp = c2;
+
+ return cl;
+}
+
+/* Called initially with ORIG_BASE non-null, sets PREV_BITPOS and PREV_POFFSET
+ to the offset of the field given in BASE. Return type is 1 if BASE is equal
+ to *ORIG_BASE after stripping off ARRAY_REF and INDIRECT_REF nodes and
+ calling get_inner_reference, else 0.
+
+ Called subsequently with ORIG_BASE null, compares the offset of the field
+ given in BASE to PREV_BITPOS, PREV_POFFSET. Returns -1 if the base object
+ has changed, 0 if the new value has a higher bit position than that
+ described by the aforementioned arguments, or 1 if the new value is less
+ than them. Used for (insertion) sorting components after a GOMP_MAP_STRUCT
+ mapping. */
+
+static int
+check_base_and_compare_lt (tree base, tree *orig_base, tree decl,
+ poly_int64 *prev_bitpos,
+ poly_offset_int *prev_poffset)
+{
+ tree offset;
+ poly_int64 bitsize, bitpos;
+ machine_mode mode;
+ int unsignedp, reversep, volatilep = 0;
+ poly_offset_int poffset;
+
+ if (orig_base)
+{
+ while (TREE_CODE (base) == ARRAY_REF)
+ base = TREE_OPERAND (base, 0);
+
+ if (TREE_CODE (base) == INDIRECT_REF)
+ base = TREE_OPERAND (base, 0);
+}
+ else
+{
+ if (TREE_CODE (base) == ARRAY_REF)
+ {
+ while (TREE_CODE (base) == ARRAY_REF)
+ base = TREE_OPERAND (base, 0);
+ if (TREE_CODE (base) != COMPONENT_REF
+ || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE)
+ return -1;
+ }
+ else if (TREE_CODE (base) == INDIRECT_REF
+ && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF
+ && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0)))
+ == REFERENCE_TYPE))
+ base = TREE_OPERAND (base, 0);
+}
+
+ base = get_inner_reference (base, , , , ,
+ , , );
+
+ if (orig_base)
+*orig_base =
Re: [PATCH] handle unusual targets in -Wbuiltin-declaration-mismatch (PR 88098)
By calling builtin_decl_explicit rather than builtin_decl_implicit
the updated patch in the attachment avoids test failures due to
missing warnings on targets with support for long double but whose
libc doesn't support C99 functions like fabsl (such as apparently
aarch64-linux).
Martin
On 11/19/2018 02:37 PM, Martin Sebor wrote:
The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with
the recent -Wbuiltin-declaration-mismatch enhancement to detect
calls with incompatible arguments to built-ins declared without
a prototype fails on a few targets due to incorrect assumptions
hardcoded into the test. Besides removing those assumptions
(or adding appropriate { target } attributes, the attached patch
also adjusts the implementation of the warning to avoid triggering
for enum promotion to int on short_enums targets.
Since the fix is trivial I plan to commit it tomorrow if there
are no concerns.
Tested on x86_64-linux and with an arm-none-eabi cross-compiler.
I also did a little bit of testing with sparc-solaris2.11 cross
compiler but there the test harness fails due to the -m32 option
so the Wbuiltin-declaration-mismatch-4.c still has unexpected
FAILs. I've raised bug 88104 for the outstanding problem on
sparc-solaris2.11.
Martin
PR testsuite/88098 - FAIL: gcc.dg/Wbuiltin-declaration-mismatch-4.c
gcc/c/ChangeLog:
PR testsuite/88098
* c-typeck.c (convert_arguments): Call builtin_decl_explicit instead.
(maybe_warn_builtin_no_proto_arg): Handle short enum to int promotion.
gcc/testsuite/ChangeLog:
PR testsuite/88098
* gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust.
* gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test.
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c (revision 266320)
+++ gcc/c/c-typeck.c (working copy)
@@ -3422,7 +3422,10 @@ convert_arguments (location_t loc, vec
built_in_function code = DECL_FUNCTION_CODE (fundecl);
if (C_DECL_BUILTIN_PROTOTYPE (fundecl))
{
- if (tree bdecl = builtin_decl_implicit (code))
+ /* For a call to a built-in function declared without a prototype
+ use the types of the parameters of the internal built-in to
+ match those of the arguments to. */
+ if (tree bdecl = builtin_decl_explicit (code))
builtin_typelist = TYPE_ARG_TYPES (TREE_TYPE (bdecl));
}
@@ -6461,7 +6464,9 @@ maybe_warn_builtin_no_proto_arg (location_t loc, t
&& TYPE_MODE (parmtype) == TYPE_MODE (argtype))
return;
- if (parmcode == argcode
+ if ((parmcode == argcode
+ || (parmcode == INTEGER_TYPE
+ && argcode == ENUMERAL_TYPE))
&& TYPE_MAIN_VARIANT (parmtype) == TYPE_MAIN_VARIANT (promoted))
return;
Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c
===
--- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (revision 266320)
+++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c (working copy)
@@ -77,9 +77,9 @@ void test_integer_conversion_memset (void *d)
/* Passing a ptrdiff_t where size_t is expected may not be unsafe
but because GCC may emits suboptimal code for such calls warning
for them helps improve efficiency. */
- memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .long int.} where .long unsigned int. is expected" } */
+ memset (d, 0, diffi); /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .\(long \)?int.} where .\(long \)?unsigned int. is expected" } */
- memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where 'long unsigned int' is expected" } */
+ memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where '\(long \)?unsigned int' is expected" } */
/* Verify that the same call as above but to the built-in doesn't
trigger a warning. */
@@ -108,7 +108,8 @@ void test_real_conversion_fabs (void)
/* In C, the type of an enumeration constant is int. */
d = fabs (e0);/* { dg-warning ".fabs. argument 1 type is .int. where .double. is expected in a call to built-in function declared without prototype" } */
- d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" } */
+ d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" "ordinary enum" { target { ! short_enums } } } */
+ /* { dg-warning ".fabs. argument 1 promotes to .int. where .double. is expected in a call to built-in function declared without prototype" "size 1 enum" { target short_enums } .-1 } */
/* No warning here since float is promoted to double. */
d = fabs (f);
[Bug lto/84044] Spurious -Wodr warning with -flto
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84044
--- Comment #11 from Jan Hubicka ---
Warning from #9 is caused by fact that I compare pointers rather than test for
equality so when the enums are not merged, they triggers the warning even if
the values are in fact equivalent.
The following silences the warning but enums should be really merged here, so I
need to look into why.
Concerning the original bug I believe it was fixed a while ago when we cleaned
up the BINFO streaming.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 266325)
+++ ipa-devirt.c(working copy)
@@ -1343,7 +1343,7 @@ odr_types_equivalent_p (tree t1, tree t2
" is defined in another translation unit"));
return false;
}
- if (TREE_VALUE (v1) != TREE_VALUE (v2))
+ if (!operand_equal_p (TREE_VALUE (v1), TREE_VALUE (v2), 0))
{
warn_odr (t1, t2, NULL, NULL, warn, warned,
G_("an enum with different values is defined"
[Bug c++/88118] GCC keeps unnecessary calls to new
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88118 Andrew Pinski changed: What|Removed |Added Depends on||19831 --- Comment #1 from Andrew Pinski --- Related to bug 19831. Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19831 [Bug 19831] Missing DSE/malloc/free optimization
Re: [RFC C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386)
On Tue, 2018-11-20 at 21:57 +0100, Jakub Jelinek wrote:
> Hi!
>
> This PR is complaining about range covering the first token from
> an id-expression:
> pr87386.C:4:15: error: static assertion failed: foo
> 4 | static_assert(foo::test::value, "foo");
> | ^~~
> The following patch adjust that to:
> pr87386.C:4:31: error: static assertion failed: foo
> 4 | static_assert(foo::test::value, "foo");
> | ^
> instead, though as the changes to the testsuite show, not really sure
> if it is a good idea in all cases, because then we sometimes print:
> ... bar is not in foo namespace, did you mean 'baz'
> foo::bar
> ~^~~
> baz
> where the baz is misaligned.
Is the compiler suggesting the use of
(a) "foo::baz" or
(b) "::baz"?
Given the underlining, the fix-it hint would be suggesting the
replacement of "foo::bar" with "baz", which would be wrong if we mean
(a) above.
(c.f. "Fix-it hints should work" in https://gcc.gnu.org/onlinedocs/gcci
nt/Guidelines-for-Diagnostics.html )
FWIW in r265610 I ran into issues like this, which I resolved by
holding off on some fix-it hints for the case where we don't have a
location covering the whole of a qualified name.
> Would it be better to just print
> pr87386.C:4:31: error: static assertion failed: foo
> 4 | static_assert(foo::test::value, "foo");
> | ^
> instead? That would mean dropping the cp_parser_id_expression change
> and readjusting or dropping some testsuite changes.
That might be better... let me look at the affected test cases.
[...snip...]
> /* Parse a template-declaration.
> --- gcc/testsuite/g++.dg/spellcheck-pr79298.C.jj 2018-10-31
> 10:31:13.281572644 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-pr79298.C 2018-11-20
> 19:14:19.208219955 +0100
> @@ -11,7 +11,7 @@ int foo ()
>return M::y; // { dg-error ".y. is not a member of .M." }
>/* { dg-begin-multiline-output "" }
> return M::y;
> - ^
> + ~~~^
> { dg-end-multiline-output "" } */
> }
> @@ -20,7 +20,7 @@ int bar ()
>return O::colour; // { dg-error ".colour. is not a member of .O.;
> did you mean 'color'\\?" }
>/* { dg-begin-multiline-output "" }
> return O::colour;
> - ^~
> - color
> + ~~~^~
> + color
> { dg-end-multiline-output "" } */
> }
This makes the fix-it hint wrong: after the fix-it is applied, it will
become
return color;
(which won't compile), rather than
return O::color;
which will.
(I wish we had a good automated way of verifying that fix-it hints fix
things)
> --- gcc/testsuite/g++.dg/lookup/suggestions2.C.jj 2018-10-31
> 10:31:06.928677642 +0100
> +++ gcc/testsuite/g++.dg/lookup/suggestions2.C2018-11-20
> 19:12:05.281395810 +0100
> @@ -33,8 +33,8 @@ int test_1_long (void) {
>return outer_ns::var_in_inner_ns_a; // { dg-error "did you mean
> 'var_in_outer_ns'" }
>/* { dg-begin-multiline-output "" }
> return outer_ns::var_in_inner_ns_a;
> -^
> -var_in_outer_ns
> + ~~^
> + var_in_outer_ns
> { dg-end-multiline-output "" } */
> }
Again, this makes the fix-it hint wrong: after the fix-it is applied,
it will become
return var_in_outer_ns;
(which won't compile) rather than:
return outer_ns::var_in_outer_ns;
[...snip; I think there are more examples in this test file...]
> --- gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C.jj 20
> 18-10-31 10:31:07.765663807 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C 2018-
> 11-20 19:14:35.698952024 +0100
> @@ -73,7 +73,7 @@ void test_3 ()
>ns3::goo_3 (); // { dg-error "'goo_3' is not a member of 'ns3';
> did you mean 'foo_3'\\?" }
>/* { dg-begin-multiline-output "" }
> ns3::goo_3 ();
> -^
> -foo_3
> + ~^
> + foo_3
> { dg-end-multiline-output "" } */
> }
Again, the fix-it hint becomes wrong, it will become:
foo_3 ();
rather than:
ns3::foo_3 ();
[...snip...]
> --- gcc/testsuite/g++.dg/spellcheck-pr77829.C.jj 2018-10-31
> 10:31:10.213623350 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-pr77829.C 2018-11-20
> 19:13:30.78045 +0100
> @@ -21,8 +21,8 @@ void fn_1_explicit ()
>detail::some_type i; // { dg-error ".some_type. is not a member of
> .detail.; did you mean 'some_typedef'\\?" }
>/* { dg-begin-multiline-output "" }
> detail::some_type i;
> - ^
> - some_typedef
> + ^
> + some_typedef
> { dg-end-multiline-output "" } */
> }
Similar problems here.
[...snip...]
So it looks like the less invasive fix might be better (not that I've
looked at it in detail, though).
Hope this is constructive
Dave
Re: [PATCH] handle unusual targets in -Wbuiltin-declaration-mismatch (PR 88098)
On 11/20/2018 08:56 AM, Christophe Lyon wrote:
On Mon, 19 Nov 2018 at 22:38, Martin Sebor wrote:
The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with
the recent -Wbuiltin-declaration-mismatch enhancement to detect
calls with incompatible arguments to built-ins declared without
a prototype fails on a few targets due to incorrect assumptions
hardcoded into the test. Besides removing those assumptions
(or adding appropriate { target } attributes, the attached patch
also adjusts the implementation of the warning to avoid triggering
for enum promotion to int on short_enums targets.
Since the fix is trivial I plan to commit it tomorrow if there
are no concerns.
Tested on x86_64-linux and with an arm-none-eabi cross-compiler.
I also did a little bit of testing with sparc-solaris2.11 cross
compiler but there the test harness fails due to the -m32 option
so the Wbuiltin-declaration-mismatch-4.c still has unexpected
FAILs. I've raised bug 88104 for the outstanding problem on
sparc-solaris2.11.
Hello,
I tested your patch on arm* and aarch64*. It does the job on arm, but
on aarch64*elf,
I'm seeing new failures:
gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for
warnings, line 121)
gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for
warnings, line 123)
gcc.dg/Wbuiltin-declaration-mismatch-4.c large long double (test for
warnings, line 98)
The failures are due to the target apparently not supporting
the fabsl built-in used by the test.
Martin
[RFC C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386)
Hi!
This PR is complaining about range covering the first token from
an id-expression:
pr87386.C:4:15: error: static assertion failed: foo
4 | static_assert(foo::test::value, "foo");
| ^~~
The following patch adjust that to:
pr87386.C:4:31: error: static assertion failed: foo
4 | static_assert(foo::test::value, "foo");
| ^
instead, though as the changes to the testsuite show, not really sure
if it is a good idea in all cases, because then we sometimes print:
... bar is not in foo namespace, did you mean 'baz'
foo::bar
~^~~
baz
where the baz is misaligned. Would it be better to just print
pr87386.C:4:31: error: static assertion failed: foo
4 | static_assert(foo::test::value, "foo");
| ^
instead? That would mean dropping the cp_parser_id_expression change
and readjusting or dropping some testsuite changes.
2018-11-20 Jakub Jelinek
PR c++/87386
* parser.c (cp_parser_primary_expression): Use
id_expression.get_location () instead of id_expr_token->location.
(cp_parser_id_expression): For nested_name_specifier_p, make a
range covering whole id-expression.
(cp_parser_operator): For operator "" make a range from "" to
the end of the suffix with caret at the start of the id.
gcc/testsuite/
* g++.dg/spellcheck-pr79298.C: Adjust expected diagnostics.
* g++.dg/lookup/suggestions2.C: Likewise.
* g++.dg/spellcheck-single-vs-multiple.C: Likewise.
* g++.dg/parse/error17.C: Likewise.
* g++.dg/spellcheck-pr77829.C: Likewise.
* g++.dg/spellcheck-pr78656.C: Likewise.
* g++.dg/cpp0x/pr51420.C: Likewise.
* g++.dg/cpp0x/udlit-declare-neg.C: Likewise.
* g++.dg/cpp0x/udlit-member-neg.C: Likewise.
libstdc++-v3/
* testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust expected
line.
* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
* testsuite/experimental/propagate_const/requirements2.cc: Likewise.
* testsuite/experimental/propagate_const/requirements3.cc: Likewise.
* testsuite/experimental/propagate_const/requirements4.cc: Likewise.
* testsuite/experimental/propagate_const/requirements5.cc: Likewise.
--- gcc/cp/parser.c.jj 2018-11-20 08:41:28.686923718 +0100
+++ gcc/cp/parser.c 2018-11-20 19:05:22.848941522 +0100
@@ -5602,7 +5602,7 @@ cp_parser_primary_expression (cp_parser
/*is_namespace=*/false,
/*check_dependency=*/true,
_decls,
- id_expr_token->location);
+ id_expression.get_location ());
/* If the lookup was ambiguous, an error will already have
been issued. */
if (ambiguous_decls)
@@ -5673,7 +5673,7 @@ cp_parser_primary_expression (cp_parser
if (parser->local_variables_forbidden_p
&& local_variable_p (decl))
{
- error_at (id_expr_token->location,
+ error_at (id_expression.get_location (),
"local variable %qD may not appear in this context",
decl.get_value ());
return error_mark_node;
@@ -5692,7 +5692,7 @@ cp_parser_primary_expression (cp_parser
id_expression.get_location ()));
if (error_msg)
cp_parser_error (parser, error_msg);
- decl.set_location (id_expr_token->location);
+ decl.set_location (id_expression.get_location ());
return decl;
}
@@ -5758,6 +5758,7 @@ cp_parser_id_expression (cp_parser *pars
{
bool global_scope_p;
bool nested_name_specifier_p;
+ location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
/* Assume the `template' keyword was not used. */
if (template_p)
@@ -5809,6 +5810,7 @@ cp_parser_id_expression (cp_parser *pars
parser->object_scope = saved_object_scope;
parser->qualifying_scope = saved_qualifying_scope;
+ unqualified_id.set_range (start_loc, unqualified_id.get_finish ());
return unqualified_id;
}
/* Otherwise, if we are in global scope, then we are looking at one
@@ -14931,7 +14933,7 @@ cp_literal_operator_id (const char* name
static cp_expr
cp_parser_operator (cp_parser* parser)
{
- tree id = NULL_TREE;
+ cp_expr id = NULL_TREE;
cp_token *token;
bool utf8 = false;
@@ -15219,8 +15221,9 @@ cp_parser_operator (cp_parser* parser)
if (id != error_mark_node)
{
const char *name = IDENTIFIER_POINTER (id);
- id = cp_literal_operator_id (name);
+ *id = cp_literal_operator_id (name);
}
+ id.set_range (start_loc, id.get_finish ());
return id;
}
@@ -15244,7 +15247,8 @@ cp_parser_operator
[Bug c++/87229] [8 Regression] ICE: tree code 'call_expr' is not supported in LTO streams
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87229 --- Comment #6 from Andreas Schwab --- That breaks gnat.dg/lto19.adb on aarch64. Executing on host: /opt/gcc/gcc-20181120/Build/gcc/gnatmake --GCC=/opt/gcc/gcc-20181120/Build/gcc/xgcc --GNATBIND=/opt/gcc/gcc-20181120/Build/gcc/gnatbind --GNATLINK=/opt/gcc/gcc-20181120/Build/gcc/gnatlink -cargs -B/opt/gcc/gcc-20181120/Build/gcc -largs --GCC=/opt/gcc/gcc-20181120/Build/gcc/xgcc\ -B/opt/gcc/gcc-20181120/Build/gcc\ \ -mabi=lp64 -margs --RTS=/opt/gcc/gcc-20181120/Build/aarch64-suse-linux/./libada -q -f /opt/gcc/gcc-20181120/gcc/testsuite/gnat.dg/lto19.adb -mabi=lp64 -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -flto -lm -o ./lto19.exe(timeout = 300) spawn -ignore SIGHUP /opt/gcc/gcc-20181120/Build/gcc/gnatmake --GCC=/opt/gcc/gcc-20181120/Build/gcc/xgcc --GNATBIND=/opt/gcc/gcc-20181120/Build/gcc/gnatbind --GNATLINK=/opt/gcc/gcc-20181120/Build/gcc/gnatlink -cargs -B/opt/gcc/gcc-20181120/Build/gcc -largs --GCC=/opt/gcc/gcc-20181120/Build/gcc/xgcc -B/opt/gcc/gcc-20181120/Build/gcc -mabi=lp64 -margs --RTS=/opt/gcc/gcc-20181120/Build/aarch64-suse-linux/./libada -q -f /opt/gcc/gcc-20181120/gcc/testsuite/gnat.dg/lto19.adb -mabi=lp64 -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -flto -lm -o ./lto19.exe lto1: error: TYPE_FIELDS defined in incomplete type SI size unit-size align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7cd885e8 precision:32 min max context > nonaddressable SI /opt/gcc/gcc-20181120/gcc/testsuite/gnat.dg/lto19_pkg1.ads:9:5 size unit-size align:32 warn_if_not_align:0 offset_align 128 offset bit-offset context chain BLK /opt/gcc/gcc-20181120/gcc/testsuite/gnat.dg/lto19_pkg1.ads:8:5 align:32 warn_if_not_align:0 offset_align 128 offset bit-offset context >> context pointer_to_this reference_to_this > lto1: internal compiler error: verify_type failed 0xdb963f verify_type(tree_node const*) ../../gcc/tree.c:14320 0x5d2d73 lto_fixup_state ../../gcc/lto/lto.c:2706 0x5df75f lto_fixup_decls ../../gcc/lto/lto.c:2737 0x5df75f read_cgraph_and_symbols ../../gcc/lto/lto.c:2971 0x5df75f lto_main() ../../gcc/lto/lto.c:3401 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. lto-wrapper: fatal error: /opt/gcc/gcc-20181120/Build/gcc/xgcc returned 1 exit status compilation terminated. /usr/aarch64-suse-linux/bin/ld: error: lto-wrapper failed collect2: error: ld returned 1 exit status gnatlink: error when calling /opt/gcc/gcc-20181120/Build/gcc/xgcc gnatmake: *** link failed. compiler exited with status 1 FAIL: gnat.dg/lto19.adb (internal compiler error)
[C++ PATCH] Fix ICE in constexpr OBJ_TYPE_REF handling (PR c++/88110)
Hi!
The comment in OBJ_TYPE_REF handling code correctly says that we are
looking for x.D.2103.D.2094, but it is important that x is not an
INDIRECT_REF or something similar as in the following testcase - we can't
really devirtualize in that case because we really don't know what it points
to. The following patch ensures that the argument got evaluated to address
of some field of (ultimately) a decl, which is all we should get during
valid constexpr evaluation.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-11-20 Jakub Jelinek
PR c++/88110
* constexpr.c (cxx_eval_constant_expression) : Punt
if get_base_address of ADDR_EXPR operand is not a DECL_P.
* g++.dg/cpp2a/constexpr-virtual13.C: New test.
--- gcc/cp/constexpr.c.jj 2018-11-19 14:24:49.0 +0100
+++ gcc/cp/constexpr.c 2018-11-20 15:03:26.968152935 +0100
@@ -4815,7 +4815,8 @@ cxx_eval_constant_expression (const cons
obj = cxx_eval_constant_expression (ctx, obj, lval, non_constant_p,
overflow_p);
/* We expect something in the form of get x. */
- if (TREE_CODE (obj) != ADDR_EXPR)
+ if (TREE_CODE (obj) != ADDR_EXPR
+ || !DECL_P (get_base_address (TREE_OPERAND (obj, 0
{
if (!ctx->quiet)
error_at (cp_expr_loc_or_loc (t, input_location),
--- gcc/testsuite/g++.dg/cpp2a/constexpr-virtual13.C.jj 2018-11-20
15:07:17.558386765 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-virtual13.C2018-11-20
15:05:30.188140420 +0100
@@ -0,0 +1,20 @@
+// PR c++/88110
+// { dg-do compile }
+
+struct A {
+ virtual int foo () const = 0;
+};
+struct B {
+ virtual int bar () const = 0;
+ virtual int baz () const = 0;
+};
+struct C : public A { };
+struct D : public C { };
+struct E : public D, public B { };
+
+void
+qux (const E *x)
+{
+ if (x->baz ())
+;
+}
Jakub
[committed] Fix omp simd clone creation for multiple return stmts (PR tree-optimization/87895)
Hi!
In certain cases like the testcases below there are multiple return stmts
in the function for which we create simd clones and the simd clone adjusting
code wasn't handling that case properly, some bbs could end up with
non-fallthru edges to the increment bb even without a gimple_goto at the
end, others could be missed from redirection to incr_bb.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.
2018-11-20 Jakub Jelinek
PR tree-optimization/87895
* omp-simd-clone.c (ipa_simd_modify_function_body): When removing
or replacing GIMPLE_RETURN, set EDGE_FALLTHRU on the edge to EXIT.
(simd_clone_adjust): Don't set EDGE_FALLTHRU here. In a loop that
redirects edges to EXIT to edges to incr_bb, iterate while EXIT
has any preds and always use EDGE_PRED (, 0).
* gcc.dg/gomp/pr87895-1.c: New test.
* gcc.dg/gomp/pr87895-2.c: New test.
* gcc.dg/gomp/pr87895-3.c: New test.
--- gcc/omp-simd-clone.c.jj 2018-11-14 01:01:56.758459348 +0100
+++ gcc/omp-simd-clone.c2018-11-20 13:57:53.902488981 +0100
@@ -994,6 +994,8 @@ ipa_simd_modify_function_body (struct cg
if (greturn *return_stmt = dyn_cast (stmt))
{
tree retval = gimple_return_retval (return_stmt);
+ edge e = find_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun));
+ e->flags |= EDGE_FALLTHRU;
if (!retval)
{
gsi_remove (, true);
@@ -1150,14 +1152,9 @@ simd_clone_adjust (struct cgraph_node *n
incr_bb = create_empty_bb (orig_exit);
incr_bb->count = profile_count::zero ();
add_bb_to_loop (incr_bb, body_bb->loop_father);
- /* The succ of orig_exit was EXIT_BLOCK_PTR_FOR_FN (cfun), with an empty
-flag. Set it now to be a FALLTHRU_EDGE. */
- gcc_assert (EDGE_COUNT (orig_exit->succs) == 1);
- EDGE_SUCC (orig_exit, 0)->flags |= EDGE_FALLTHRU;
- for (unsigned i = 0;
- i < EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds); ++i)
+ while (EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds))
{
- edge e = EDGE_PRED (EXIT_BLOCK_PTR_FOR_FN (cfun), i);
+ edge e = EDGE_PRED (EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
redirect_edge_succ (e, incr_bb);
incr_bb->count += e->count ();
}
--- gcc/testsuite/gcc.dg/gomp/pr87895-1.c.jj2018-11-20 13:18:00.483355400
+0100
+++ gcc/testsuite/gcc.dg/gomp/pr87895-1.c 2018-11-20 13:18:28.792884133
+0100
@@ -0,0 +1,19 @@
+/* PR tree-optimization/87895 */
+/* { dg-do compile } */
+/* { dg-additional-options "-O0" } */
+
+#pragma omp declare simd
+int
+foo (int x)
+{
+ if (x == 0)
+return 0;
+}
+
+#pragma omp declare simd
+int
+bar (int *x, int y)
+{
+ if ((y == 0) ? (*x = 0) : *x)
+return 0;
+}
--- gcc/testsuite/gcc.dg/gomp/pr87895-2.c.jj2018-11-20 13:18:07.780233931
+0100
+++ gcc/testsuite/gcc.dg/gomp/pr87895-2.c 2018-11-20 13:18:57.265410143
+0100
@@ -0,0 +1,5 @@
+/* PR tree-optimization/87895 */
+/* { dg-do compile } */
+/* { dg-additional-options "-O1" } */
+
+#include "pr87895-1.c"
--- gcc/testsuite/gcc.dg/gomp/pr87895-3.c.jj2018-11-20 14:06:23.131004074
+0100
+++ gcc/testsuite/gcc.dg/gomp/pr87895-3.c 2018-11-20 14:06:03.697327933
+0100
@@ -0,0 +1,18 @@
+/* PR tree-optimization/87895 */
+/* { dg-do compile } */
+/* { dg-additional-options "-O2" } */
+
+#pragma omp declare simd
+int foo (int x) __attribute__((noreturn));
+
+#pragma omp declare simd
+int
+bar (int x, int y)
+{
+ if (y == 1)
+foo (x + 2);
+ if (y == 10)
+foo (x + 6);
+ if (y != 25)
+return 4;
+}
Jakub
[Bug tree-optimization/87895] [7/8/9 Regression] ICE in purge_dead_edges, at cfgrtl.c:3246
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87895 --- Comment #4 from Jakub Jelinek --- Author: jakub Date: Tue Nov 20 20:44:38 2018 New Revision: 266328 URL: https://gcc.gnu.org/viewcvs?rev=266328=gcc=rev Log: PR tree-optimization/87895 * omp-simd-clone.c (ipa_simd_modify_function_body): When removing or replacing GIMPLE_RETURN, set EDGE_FALLTHRU on the edge to EXIT. (simd_clone_adjust): Don't set EDGE_FALLTHRU here. In a loop that redirects edges to EXIT to edges to incr_bb, iterate while EXIT has any preds and always use EDGE_PRED (, 0). * gcc.dg/gomp/pr87895-1.c: New test. * gcc.dg/gomp/pr87895-2.c: New test. * gcc.dg/gomp/pr87895-3.c: New test. Added: trunk/gcc/testsuite/gcc.dg/gomp/pr87895-1.c trunk/gcc/testsuite/gcc.dg/gomp/pr87895-2.c trunk/gcc/testsuite/gcc.dg/gomp/pr87895-3.c Modified: trunk/gcc/ChangeLog trunk/gcc/omp-simd-clone.c trunk/gcc/testsuite/ChangeLog
[Bug tree-optimization/88038] ICE: in check, at tree-vrp.c:155: recipe for target 'all-stage3-isl' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88038 Iain Buclaw changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |WORKSFORME --- Comment #3 from Iain Buclaw --- (In reply to Iain Buclaw from comment #2) > Build log is when testing r265361 from a couple weeks back. > > Looking at recent history, the assert has been removed from the function > since then. > > I'll trigger a rebuild to see if its still happening. I can reproduce on r265362, x86_64-linux-gnu. However, the ICE is indeed gone from trunk.
[Bug c++/88121] Nonsensical messages when suggesting names from other namespaces
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88121 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2018-11-20 Assignee|unassigned at gcc dot gnu.org |dmalcolm at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from David Malcolm --- Am working on a fix
[PATCH] Fix up method-nonnull-1.mm testcase on Solaris (PR testsuite/88090)
Hi!
The following testcase fails on Solaris, because it doesn't print there
'size_t', but 'std::size_t', as the type is defined by system headers and
it is not under gcc control how exactly is size_t defined.
The following patch fixes that by using a different typedef which we have
total control over.
Tested on x86_64-linux and i686-linux, ok for trunk?
2018-11-20 Jakub Jelinek
PR testsuite/88090
* obj-c++.dg/attributes/method-nonnull-1.mm (my_size_t): New typedef.
(MyArray::removeObjectAtIndex): Use my_size_t instead of size_t and
expect it in diagnostics.
--- gcc/testsuite/obj-c++.dg/attributes/method-nonnull-1.mm.jj 2018-11-16
10:22:17.817272221 +0100
+++ gcc/testsuite/obj-c++.dg/attributes/method-nonnull-1.mm 2018-11-20
09:10:28.404872788 +0100
@@ -5,6 +5,8 @@
#include
#include
+typedef __SIZE_TYPE__ my_size_t;
+
@interface MyArray
{
Class isa;
@@ -25,8 +27,8 @@
+ (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* {
dg-warning "exceeds the number of function parameters 3" } */
- (void) removeObject: (id)object __attribute__ ((nonnull (2))); /* {
dg-warning "exceeds the number of function parameters 3" } */
-+ (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /*
{ dg-warning "refers to parameter type .size_t." } */
-- (void) removeObjectAtIndex: (size_t)object __attribute__ ((nonnull (1))); /*
{ dg-warning "refers to parameter type .size_t." } */
++ (void) removeObjectAtIndex: (my_size_t)object __attribute__ ((nonnull (1)));
/* { dg-warning "refers to parameter type .my_size_t." } */
+- (void) removeObjectAtIndex: (my_size_t)object __attribute__ ((nonnull (1)));
/* { dg-warning "refers to parameter type .my_size_t." } */
+ (void) removeObject: (id)object __attribute__ ((nonnull (MyArray))); /* {
dg-error "" } */
/* { dg-warning "attribute argument is invalid" "" { target *-*-* } .-1 } */
Jakub
Re: [PATCH] clarify comments for implicit_p flag for built-ins
On 11/20/2018 11:02 AM, Martin Sebor wrote:
Would the updated comments in the attached patch more accurately
describe the purpose of the IMPLICIT_P flag and
the builtin_decl_explicit() and builtin_decl_implicit() functions?
I ended up here while trying to understand the differences between
the functions on different targets and decide which one should be
used to diagnose bugs like the one below:
long double fabsl (); // missing prototype
long double f (int x)
{
return fabsl (x); // want a warning here
}
I think we want the warning regardless of IMPLICIT_P so the warning
code should call builtin_decl_explicit() to obtain fabsl's expected
type, even if the target's runtime doesn't support the function on
the basis of the comment:
When a program uses floorf we may assume that the floorf function
has the expected meaning
Actually, some more testing suggests the comment in builtins.def
(either the original or the patched one) isn't entirely accurate
or helpful to understanding the purpose of the flag:
IMPLICIT specifies condition when the builtin can be produced by
compiler. For instance C90 reserves floorf function, but does not
define it's meaning. When user uses floorf we may assume that the
floorf has the meaning we expect, but we can't produce floorf by
simplifying floor((double)float) since the runtime need not
implement it.
Given the following code:
float floorf ();
void f (void)
{
if (floorf (0.0f))
__builtin_abort ();
}
in C90 mode, BUILT_IN_FLOORF's IMPLICIT flag is clear and GCC
doesn't seem to assume anything about the call to the function,
contrary to the comment ("we may assume the meaning we expect").
The comment also doesn't explain when IMPLICIT may be set.
I've updated the comment a bit more to more accurately describe
when I think the flag is set or clear, and how it's used.
Corrections or further clarification are appreciated.
Thanks
Martin
gcc/ChangeLog:
* builtins.def (DEF_BUILTIN): Update comment..
* tree.h (builtin_decl_explicit): Same.
(builtin_decl_implicit): Same.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 266320)
+++ gcc/tree.h (working copy)
@@ -5220,7 +5220,9 @@ is_lang_specific (const_tree t)
#define BUILTIN_VALID_P(FNCODE) \
(IN_RANGE ((int)FNCODE, ((int)BUILT_IN_NONE) + 1, ((int) END_BUILTINS) - 1))
-/* Return the tree node for an explicit standard builtin function or NULL. */
+/* Return the tree node for the built-in function declaration corresponding
+ to FNCODE or NULL. */
+
static inline tree
builtin_decl_explicit (enum built_in_function fncode)
{
@@ -5229,7 +5231,12 @@ builtin_decl_explicit (enum built_in_function fnco
return builtin_info[(size_t)fncode].decl;
}
-/* Return the tree node for an implicit builtin function or NULL. */
+/* Return the tree node for the built-in function declaration corresponding
+ to FNCODE if its IMPLICIT_P flag has been set or NULL otherwise.
+ IMPLICIT_P is clear for library built-ins that GCC implements but that
+ may not be implemented in the runtime library on the target. See also
+ the DEF_BUILTIN macro in builtins.def. */
+
static inline tree
builtin_decl_implicit (enum built_in_function fncode)
{
Index: gcc/builtins.def
===
--- gcc/builtins.def (revision 266320)
+++ gcc/builtins.def (working copy)
@@ -54,12 +54,18 @@ along with GCC; see the file COPYING3. If not see
ATTRs is an attribute list as defined in builtin-attrs.def that
describes the attributes of this builtin function.
- IMPLICIT specifies condition when the builtin can be produced by
- compiler. For instance C90 reserves floorf function, but does not
- define it's meaning. When user uses floorf we may assume that the
- floorf has the meaning we expect, but we can't produce floorf by
- simplifying floor((double)float) since the runtime need not implement
- it.
+ IMPLICIT specifies the condition when calls to a library builtin
+ may be introduced by GCC. For instance, C90 defines the floor
+ function but only reserves floorf without defining its meaning.
+ Thus, IMPLICIT is set for floor but clear for floorf. GCC can
+ safely substitute calls to floor for equivalent expressions but
+ the most it can do for floorf is assume that explicit calls to
+ it in a program are those to the reserved function. It cannot
+ introduce calls to the function that do not exist in the source
+ code of the program. This prevents trasformations that might be
+ possibe otherwise, such as turning the call floor((double)flt)
+ into one to floorf(flt) because the runtime library can be assumed
+ to implement the latter function.
The builtins is registered only if COND is true. */
[Bug c++/88120] [7/8/9 Regression] ICE in contains_placeholder_p at gcc/tree.c:3710
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88120 Marek Polacek changed: What|Removed |Added Status|NEW |ASSIGNED Known to work||4.8.4 Assignee|unassigned at gcc dot gnu.org |mpolacek at gcc dot gnu.org Target Milestone|--- |7.4 Summary|ICE in |[7/8/9 Regression] ICE in |contains_placeholder_p at |contains_placeholder_p at |gcc/tree.c:3710 |gcc/tree.c:3710 --- Comment #2 from Marek Polacek --- Testing it. G++ 4.8.4 seems to compile the testcase fine, making this a regression.
