[Bug fortran/91077] [8/9/10 Regression] Wrong indexing when using a pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91077 Paul Thomas changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |pault at gcc dot gnu.org --- Comment #3 from Paul Thomas --- Since I am the guilty party, I had better take it :-) Cheers Paul
Re: [PATCH] Fix PR91069
On Wed, Jul 03, 2019 at 12:49:34PM +0200, Richard Biener wrote:
> 2019-07-03 Richard Biener
>
> PR middle-end/91069
> * match.pd (vec_perm -> bit_insert): Fix element read from
> first vector.
>
> * gcc.dg/pr91069.c: New testcase.
I've noticed the testcase FAILs on ia32, the problem is that long is
32-bit and so v2di type isn't v2di we are expecting, but v4si and mismatches
the v2df in number of elements and element precision.
Fixed thusly, regtested on x86_64-linux and i686-linux, committed to trunk.
Note, still not truly portable, if double and long long have different
sizes on some target, this will still fail.
2019-07-04 Jakub Jelinek
PR middle-end/91069
* gcc.dg/pr91069.c (v2df): Use 2 * sizeof (double) instead of
hardcoded 16 for better portability.
(v2di): Change from long vector to long long vector. Use
2 * sizeof (long long) instead of hardcoded 16.
--- gcc/testsuite/gcc.dg/pr91069.c.jj 2019-07-04 00:18:31.510099301 +0200
+++ gcc/testsuite/gcc.dg/pr91069.c 2019-07-04 07:08:24.944046933 +0200
@@ -1,8 +1,8 @@
/* { dg-do run } */
/* { dg-options "-std=gnu11" } */
-typedef double v2df __attribute__((vector_size(16)));
-typedef long v2di __attribute__((vector_size(16)));
+typedef double v2df __attribute__((vector_size(2 * sizeof (double;
+typedef long long v2di __attribute__((vector_size(2 * sizeof (long long;
void foo (v2df *res, v2df *src)
{
Jakub
[Bug tree-optimization/91069] [10 Regression] Miscompare of 453.povray since r272843
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91069 --- Comment #11 from Jakub Jelinek --- Author: jakub Date: Thu Jul 4 05:10:52 2019 New Revision: 273039 URL: https://gcc.gnu.org/viewcvs?rev=273039=gcc=rev Log: PR middle-end/91069 * gcc.dg/pr91069.c (v2df): Use 2 * sizeof (double) instead of hardcoded 16 for better portability. (v2di): Change from long vector to long long vector. Use 2 * sizeof (long long) instead of hardcoded 16. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/pr91069.c
[committed] Call lower_omp on GIMPLE_OMP_SCAN body
Hi!
For is_simd, we change GIMPLE_OMP_SCAN statements immediately to GIMPLE_NOPs
and emit their bodies after them, so the lower_omp caller keeps iterating
over those statements, but for is_for we don't do that, so we should
lower_omp the bodies.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2019-07-04 Jakub Jelinek
* omp-low.c (lower_omp_scan): Call lower_omp on stmt's body
in worksharing loop scans.
--- gcc/omp-low.c.jj2019-07-03 07:02:16.460989884 +0200
+++ gcc/omp-low.c 2019-07-03 21:02:10.097104445 +0200
@@ -8874,8 +8874,10 @@ lower_omp_scan (gimple_stmt_iterator *gs
gsi_insert_seq_after (gsi_p, gimple_omp_body (stmt), GSI_SAME_STMT);
gsi_insert_seq_after (gsi_p, before, GSI_SAME_STMT);
gsi_replace (gsi_p, gimple_build_nop (), true);
+ return;
}
- else if (before)
+ lower_omp (gimple_omp_body_ptr (stmt), octx);
+ if (before)
{
gimple_stmt_iterator gsi = gsi_start_1 (gimple_omp_body_ptr (stmt));
gsi_insert_seq_before (, before, GSI_SAME_STMT);
Jakub
Re: [PATCH] simplify-rtx.c: Change BITSIZE to UNIT_PRECISION in simplification
On Wed, Jul 03, 2019 at 04:32:54PM -0600, Jeff Law wrote: John, I assume you're doing this for an out of tree port (s12z?)? That is correct. Otherwise it'd also be useful if you could include a test which triggers the assert. Once I have the other aspects of the port sorted out I can create a test case (although it wouldn't be necessary because it happens building libgcc). If you could confirm that Richard's suggestion of using GET_MODE_PRECISION rather than GET_MODE_UNIT_PRECISION works it'd be appreciated. I had wondered about the difference between the two. Unfortunately, neither are mentioned in gccint.texinfo - I ended up deciding upon GET_MOE_UNIT_PRECISION because there is a similar situation a few lines above in simplify-rtx.c which uses it. If GET_MODE_UNIT_PRECISION is inappropriate, then please change it. Regards, John -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key.
[committed] Fix ICE on scan* OpenMP tests on various targets (PR tree-optimization/91074)
Hi! In this case we create temp = .MUL_OVERFLOW (arg1, arg2); temp2 = REALPART_EXPR ; temp3 = IMAGPART_EXPR ; x86 has been happy in the testing even without the following patch, though temp hasn't been rewritten into SSA, but on various other targets the expansion assumes that .???_OVERFLOW result must be a SSA_NAME. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk. 2019-07-04 Jakub Jelinek PR tree-optimization/91074 * omp-low.c (lower_omp_for_scan): Set DECL_GIMPLE_REG_P on cplx temporary. --- gcc/omp-low.c.jj2019-07-03 07:02:16.460989884 +0200 +++ gcc/omp-low.c 2019-07-03 16:59:25.819391742 +0200 @@ -9699,6 +9699,7 @@ lower_omp_for_scan (gimple_seq *body_p, gimple_seq_add_stmt (body_p, g); tree cplx = create_tmp_var (build_complex_type (unsigned_type_node, false)); + DECL_GIMPLE_REG_P (cplx) = 1; g = gimple_build_call_internal (IFN_MUL_OVERFLOW, 2, thread_nump1, twok); gimple_call_set_lhs (g, cplx); gimple_seq_add_stmt (body_p, g); Jakub
[Bug tree-optimization/91074] [10 regression] c-c++-common/gomp/scan-3.c fails with ICE starting with r272958
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91074 --- Comment #4 from Jakub Jelinek --- Author: jakub Date: Thu Jul 4 04:54:52 2019 New Revision: 273037 URL: https://gcc.gnu.org/viewcvs?rev=273037=gcc=rev Log: PR tree-optimization/91074 * omp-low.c (lower_omp_for_scan): Set DECL_GIMPLE_REG_P on cplx temporary. Modified: trunk/gcc/ChangeLog trunk/gcc/omp-low.c
[PATCH] Fix vect_init_vector regression (PR tree-optimization/91063)
Hi!
A recent change in vect_init_vector emits new statements into a gimple_seq
stmts and then calls vect_init_vector_1 on each of those statements.
This doesn't work well, because vect_init_vector_1 places the given
statement into another sequence (body of some bb, on the edge, ...),
and when the caller holds a gimple_stmt_iterator pointing to that statement
across its insertion into another sequence the ->prev/->next bookkeeping may
go wrong, such as on the testcase where stmt == stmt->prev == stmt->next
because of that.
Fixed by first removing the statement from the stmts sequence (thus
gsi_remove already updates the iterator to the next statement).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-07-04 Jakub Jelinek
PR tree-optimization/91063
* tree-vect-stmts.c (vect_init_vector): Call gsi_remove to remove
stmt from stmts sequence before calling vect_init_vector_1.
Formatting fix.
* gcc.dg/gomp/pr91063.c: New test.
--- gcc/tree-vect-stmts.c.jj2019-07-03 10:24:33.463768431 +0200
+++ gcc/tree-vect-stmts.c 2019-07-03 12:35:48.998435660 +0200
@@ -1496,15 +1496,19 @@ vect_init_vector (stmt_vec_info stmt_inf
promotion of invariant/external defs. */
val = gimple_convert (, TREE_TYPE (type), val);
for (gimple_stmt_iterator gsi2 = gsi_start (stmts);
- !gsi_end_p (gsi2); gsi_next ())
- vect_init_vector_1 (stmt_info, gsi_stmt (gsi2), gsi);
+ !gsi_end_p (gsi2); )
+ {
+ init_stmt = gsi_stmt (gsi2);
+ gsi_remove (, false);
+ vect_init_vector_1 (stmt_info, init_stmt, gsi);
+ }
}
}
val = build_vector_from_val (type, val);
}
new_temp = vect_get_new_ssa_name (type, vect_simple_var, "cst_");
- init_stmt = gimple_build_assign (new_temp, val);
+ init_stmt = gimple_build_assign (new_temp, val);
vect_init_vector_1 (stmt_info, init_stmt, gsi);
return new_temp;
}
--- gcc/testsuite/gcc.dg/gomp/pr91063.c.jj 2019-07-03 12:50:55.123799217
+0200
+++ gcc/testsuite/gcc.dg/gomp/pr91063.c 2019-07-03 12:50:43.320989864 +0200
@@ -0,0 +1,17 @@
+/* PR tree-optimization/91063 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fopenmp-simd" } */
+/* { dg-additional-options "-mavx512f" { target { i?86-*-* x86_64-*-* } } } */
+
+struct S { void *s; };
+
+int
+foo (struct S *x)
+{
+ int r = 0;
+ int i;
+#pragma omp simd reduction (+ : r)
+ for (i = 0; i < 64; ++i)
+r += (int) (x->s != 0);
+ return r;
+}
Jakub
[Bug c++/91082] New: Reference to function binds to pointer to function when given a template specialization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91082
Bug ID: 91082
Summary: Reference to function binds to pointer to function
when given a template specialization
Product: gcc
Version: 9.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: david at doublewise dot net
Target Milestone: ---
In the following code, gcc accepts the code in `c`, but rejects the code in
`d`. I believe both should be rejected, because it is attempting to bind a
pointer to a reference.
template
void a();
void b();
void c() {
static_cast();
}
void d() {
static_cast();
}
See it live: https://godbolt.org/z/zguy6D
[Bug rtl-optimization/90756] [7/8/9/10 Regression] g++ ICE in convert_move, at expr.c:218 on i686 and s390x
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90756 --- Comment #12 from Jakub Jelinek --- Author: jakub Date: Thu Jul 4 04:49:22 2019 New Revision: 273036 URL: https://gcc.gnu.org/viewcvs?rev=273036=gcc=rev Log: PR rtl-optimization/90756 * explow.c (promote_ssa_mode): Always use TYPE_MODE, don't bypass it for VECTOR_TYPE_P. * gcc.dg/pr90756.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr90756.c Modified: trunk/gcc/ChangeLog trunk/gcc/explow.c trunk/gcc/testsuite/ChangeLog
[Bug debug/78685] -Og generates too many ""s
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78685 Eric Gallager changed: What|Removed |Added CC||rsandifo at gcc dot gnu.org --- Comment #17 from Eric Gallager --- Richard Sandiford had a series of patches radically overhauling how -Og works in general that might affect this bug: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01392.html cc-ing him so he can comment on if it does in fact affect this bug.
[Bug other/59893] Use LTO for libgcc.a, libstdc++.a, etc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59893 Eric Gallager changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=77278 --- Comment #8 from Eric Gallager --- (In reply to Richard Biener from comment #6) > Confirmed. It was the original intent to allow this, esp. for libgfortran > for example. That's bug 77278
[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38629 --- Comment #11 from Eric Gallager --- (In reply to Federico Fissore from comment #8) > I forgot to say: this result came out of avr-gcc 4.8.1 (packaged by Arduino: > it's a 4.8.1 with two small patches applied [1]). It uses -Os optimization > flag > > [1] https://github.com/arduino/toolchain-avr/tree/master/gcc-patches This link no longer works
Go patch committed: Optimize 0, 1, 2-case select statements
This Go patch by Cherry Zhang optimizes 0,1,2-case select statements.
For a select statement with zero-, one-, or two-case with a default
case, we can generate simpler code instead of calling the generic
selectgo. A zero-case select is just blocking the execution. A
one-case select is mostly just executing the case. A two-case select
with a default case is a non-blocking send or receive. We add these
special cases for lowering a select statement, using libgo code
already written for the gc compiler. 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 273032)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-197b6fdfb861f07bab7365e350b5b855cfccc290
+7a8e10be0ddb8909ce25a264d03b24cee4df60cc
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 273009)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -6262,7 +6262,8 @@ Function_declaration::get_or_make_decl(G
if (this->asm_name_ == "runtime.gopanic"
|| this->asm_name_ == "__go_runtime_error"
- || this->asm_name_ == "runtime.panicdottype")
+ || this->asm_name_ == "runtime.panicdottype"
+ || this->asm_name_ == "runtime.block")
flags |= Backend::function_does_not_return;
}
Index: gcc/go/gofrontend/runtime.def
===
--- gcc/go/gofrontend/runtime.def (revision 272944)
+++ gcc/go/gofrontend/runtime.def (working copy)
@@ -204,6 +204,22 @@ DEF_GO_RUNTIME(CHANRECV2, "runtime.chanr
DEF_GO_RUNTIME(SELECTGO, "runtime.selectgo", P3(POINTER, POINTER, INT),
R2(INT, BOOL))
+// Non-blocking send a value on a channel, used for two-case select
+// statement with a default case.
+DEF_GO_RUNTIME(SELECTNBSEND, "runtime.selectnbsend", P2(CHAN, POINTER),
R1(BOOL))
+
+// Non-blocking receive a value from a channel, used for two-case select
+// statement with a default case.
+DEF_GO_RUNTIME(SELECTNBRECV, "runtime.selectnbrecv", P2(POINTER, CHAN),
R1(BOOL))
+
+// Non-blocking tuple receive from a channel, used for two-case select
+// statement with a default case.
+DEF_GO_RUNTIME(SELECTNBRECV2, "runtime.selectnbrecv2", P3(POINTER, POINTER,
CHAN),
+ R1(BOOL))
+
+// Block execution. Used for zero-case select.
+DEF_GO_RUNTIME(BLOCK, "runtime.block", P0(), R0())
+
// Panic.
DEF_GO_RUNTIME(GOPANIC, "runtime.gopanic", P1(EFACE), R0())
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 273032)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -5665,6 +5665,28 @@ Select_statement::do_lower(Gogo* gogo, N
Block* b = new Block(enclosing, loc);
int ncases = this->clauses_->size();
+
+ // Zero-case select. Just block the execution.
+ if (ncases == 0)
+{
+ Expression* call = Runtime::make_call(Runtime::BLOCK, loc, 0);
+ Statement *s = Statement::make_statement(call, false);
+ b->add_statement(s);
+ this->is_lowered_ = true;
+ return Statement::make_block_statement(b, loc);
+}
+
+ // One-case select. It is mostly just to run the case.
+ if (ncases == 1)
+return this->lower_one_case(b);
+
+ // Two-case select with one default case. It is a non-blocking
+ // send/receive.
+ if (ncases == 2
+ && (this->clauses_->at(0).is_default()
+ || this->clauses_->at(1).is_default()))
+return this->lower_two_case(b);
+
Type* scase_type = Channel_type::select_case_type();
Expression* ncases_expr =
Expression::make_integer_ul(ncases, NULL,
@@ -5733,6 +5755,213 @@ Select_statement::do_lower(Gogo* gogo, N
return Statement::make_block_statement(b, loc);
}
+// Lower a one-case select statement.
+
+Statement*
+Select_statement::lower_one_case(Block* b)
+{
+ Select_clauses::Select_clause& scase = this->clauses_->at(0);
+ Location loc = this->location();
+ Expression* chan = scase.channel();
+ if (chan != NULL)
+{
+ // Lower this to
+ // if chan == nil { block() }; send/recv; body
+ Temporary_statement* chantmp = Statement::make_temporary(NULL, chan,
loc);
+ b->add_statement(chantmp);
+ Expression* chanref = Expression::make_temporary_reference(chantmp, loc);
+
+ Expression* nil = Expression::make_nil(loc);
+ Expression* cond = Expression::make_binary(OPERATOR_EQEQ, chanref, nil,
loc);
+ Block* bnil = new Block(b, loc);
+ Expression* call = Runtime::make_call(Runtime::BLOCK, loc, 0);
+ Statement* s = Statement::make_statement(call, false);
+ bnil->add_statement(s);
+
Go patch committed: Fix indentation of select statement AST dump
This Go frontend patch by Cherry Zhang fixes the indentation of select
statements in the AST dump. 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 273026)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-bf66d40bc7adb438dcfac85d73bfa7b17217eed9
+197b6fdfb861f07bab7365e350b5b855cfccc290
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 272944)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -5766,6 +5766,7 @@ Select_statement::do_dump_statement(Ast_
{
ast_dump_context->ostream() << " {" << dsuffix(location()) << std::endl;
this->clauses_->dump_clauses(ast_dump_context);
+ ast_dump_context->print_indent();
ast_dump_context->ostream() << "}";
}
ast_dump_context->ostream() << std::endl;
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #31 from EML --- (In reply to dave.anglin from comment #26) > On 2019-07-03 6:06 p.m., elowe at elowe dot com wrote: > > If I replace those 3 lines and run the assembler+linker by hand - the > > non-working foo.s will run correctly > So, HAVE_AS_LTOFFX_LDXMOV_RELOCS is probably not defined. The configure > script is > likely not detecting this assembler capability correctly. > > Are you using bash shell? If not, I suggest that you use it instead of HP > shell. This macro only seems to control whether you use ltoffx or ltoff. I can confirm I am using bash, and #define HAVE_AS_LTOFFX_LDXMOV_RELOCS 1 in gcc/config.h I also added in an #error to the code to make sure. It is definitely set.
Contact from SSA expertise
Jeff, Who is the best person to contact for SSA expertise in GCC as I've started trying to figure out if it's possible to multi-thread and parallel the SSA dominator trees including insertion, walking and pushing to hardware registers during RTL allocation. Huge thanks, Nick
Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC
On 07/03/2019 05:31 AM, Richard Biener wrote: On Wed, Jul 3, 2019 at 5:18 AM Jeff Law wrote: On 7/2/19 11:54 AM, Indu Bhagat wrote: Ping. Can someone please review these patches ? We would like to get the support for CTF integrated soon. I'm not sure there's really even consensus that we want CTF support in GCC. Though I think that the changes you've made in the last several weeks do make it somewhat more palatable. But ultimately the first step is to get that consensus. I'd hazard a guess that Jakub in particular isn't on board as he's been pushing to some degree for post-processing or perhaps doing it via a plug in. Richi has been guiding you a bit through how to make the changes easier to integrate, but I haven't seen him state one way or the other his preference on whether or not CTF support is something we want. I'm mostly worried about the lack of a specification and the appearant restriction on a subset of C (the patches have gcc_unreachable () in paths that can be reached by VECTOR_TYPE or COMPLEX_TYPE not to mention FIXED_POINT_TYPE, etc...). RE lack of specification : I cannot agree more; This does need to absolutely exist if we envision CTF support in toolchain to be useful to the community. We plan on getting to this task once the Linker changes are scoped and closer to done (~ a couple of weeks from now). Will this work ? RE subset of C : It is true that CTF format currently does leave out a very small subset of C like FIXED_POINT as you noted ( CTF does have representation for COMPLEX_TYPE, if my code paths culminate to gcc_unreachable () for that, I should fix them ). The end goal is to make it support all of C, and not just a subset. Meanwhile, I intend to make the compiler skip types when a C construct is not supported instead of crashing because of gcc_unreachable (). (You may have also noted stubs with "TBD WARN instead" notes in the patch series I sent.) While CTF might be easy and fast to parse and small I fear it will go the STABS way of being not extensible and bitrotten. FWIW, I can understand this. We will maintain it. And I hope it will also be a community effort thereafter with active consumers, so there is a positive feedback loop. Given it appears to generate only debug info for symbols and no locations or whatnot it should be sufficient to introspect the compilation to generate the CTF info on the side and then merge it in at link-time. Which makes me wonder if this shouldn't be a plugin for now until it is more complete and can be evaluated better (comments in the patches indicate even the on-disk format is in flux?). Adding plugin hook invocations to the three places the CTF info generation hooks off should be easy. Yes, some bits of the on-disk format are being adapted to make it easier to adopt the CTF format across the board. E.g., we recently added CU name in the CTF header. As another example, we added CTF_K_SLICE type because there existed no way in CTF to represent enum bitfields. For the most part though, CTF format has stayed as is. Hmm...a GCC plugin for CTF generation at compile-time may work out for a single compilation unit. But I am not sure how will LTO be supported in that case. Basically, for LTO and -gtLEVEL to work together, I need the lto-wrapper to be aware of the presence of .ctf sections (so I think). I will need to combine the .ctf sections from multiple compilation units into a CTF archive, which the linker can then de-duplicate. Even if I assume that the technical hurdle in the above paragraph is solvable within the purview of a plugin, I fear worse problems of adoption, maintenance and distribution in the long run, if CTF support unfortunately ever remains to be done via a plugin for reasons unforeseen. Going the plugin route for the short term, will continue to suffer similar problems of distribution and support. - Is the plugin infrastructure supported on most platforms ? Also, I see that the plugin infrastructure supports all gcc versions from 4.5 onwards. Can someone confirm ? ( We minimally want the toolchain support with GCC 4.8.5 and GCC 8 and later, for now. ) - How will the plugin be distributed for a variety of platforms and architectures outside of what Oracle Linux commits to support ? Unless you are suggesting that the GCC plugin be distributed within GCC, meanwhile ? Well, that may be acceptable in the short term, depending on how I resolve some points raised above. That said, the patch series isn't ready for integration since it will crash left and right -- did you bootstrap and run the testsuite with -gt? Bootstrap and Testsuite : Yes, I have. On x86_64/linux, sparc64/linux, aarch64/linux. Run testsuite with -gt : Not yet. Believe me, it's on my plate. And I already regret not having done it sooner :) Bootstrap with -gt : Not yet. I should try soon. (I have compiled libdtrace-ctf with -gt and parsed the .ctf
[Bug plugins/90924] lto-plugin/lto-plugin.c heap memory corruption due to insufficient sanitization.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90924
--- Comment #5 from Ren Kimura ---
Yes. I can understand what you want to say. It may annoying for developers to
fix such nitpicky bugs.
But unfortunately these kind of bugs have been reported like, memory corruption
with *crafted* ELF file.
https://www.google.com/search?q=binutils+crafted+elf+cve
>From the perspective of attackers, they can prevent some kind of services by
sending crafted ELF file through network. i.e. Denial of Service.
Please consider our request of fixing.
Thanks
(In reply to Martin Liška from comment #4)
> (In reply to Ren Kimura from comment #3)
> > Hi. Sorry for late. I've just attached more simple one.
> >
> > PoC file for this bug can be created easily, just generating ELF file and
> > edit e_shstrndx in ELF header file to 0.
> >
> > Attached one is built from simple Hello World program.
> >
> > #include
> > int main() {
> > printf("Hello World\n");
> > };
> >
> > gcc -o memcorrupt_nm-2.30_gcc-9.1.0_gold_simple hello_world.c
> >
> > Edit e_shtrndx (offset 0x3E) to 0.
>
> What sense does it make to create a valid ELF container and then corrupt it?
> It's expected that various tools will crash then.
[Bug libfortran/91030] Poor performance of I/O -fconvert=big-endian
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91030 --- Comment #27 from Jerry DeLisle --- (In reply to Thomas Koenig from comment #26) > Jerry, you are working on a Linux box, right? What does > > stat -f -c %b . > > tell you? 13429330 Ryzen 2500U with M.2 SSD Fedora 30, Kernel 5.1.15-300.fc30.x86_64
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #30 from The Written Word --- (In reply to dave.anglin from comment #29) > On 2019-07-03 7:20 p.m., bugzilla-gcc at thewrittenword dot com wrote: > > configure:8057: /opt/build/china/gcc-8.3.0/.obj/./gcc/xgcc > > -B/opt/build/china/gcc-8.3.0/.obj/./gcc/ > > -B/opt/build/gcc8/ia64-hp-hpux11.31/bin/ -B/opt/build/gcc8/i > > a64-hp-hpux11.31/lib/ -isystem /opt/build/gcc8/ia64-hp-hpux11.31/include > > -isystem /opt/build/gcc8/ia64-hp-hpux11.31/sys-include-o conftest -O2 > > -g > > conftest.c >&5 > > configure:8057: $? = 0 > > configure:8057: ./conftest > > /opt/build/china/gcc-8.3.0/libstdc++-v3/configure[20]: 1572 > > Memoryfault(coredum > > p) > > The configure test needs to be debugged in the same manner as the "hello > world" program. Yeah, we've already looked at the difference between 4.9.4/8.3.0 assembly but want to rebuild 8.3.0 with as few patches as possible to ensure the issue isn't some patch we've introduced.
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #29 from dave.anglin at bell dot net --- On 2019-07-03 7:20 p.m., bugzilla-gcc at thewrittenword dot com wrote: > configure:8057: /opt/build/china/gcc-8.3.0/.obj/./gcc/xgcc > -B/opt/build/china/gcc-8.3.0/.obj/./gcc/ > -B/opt/build/gcc8/ia64-hp-hpux11.31/bin/ -B/opt/build/gcc8/i > a64-hp-hpux11.31/lib/ -isystem /opt/build/gcc8/ia64-hp-hpux11.31/include > -isystem /opt/build/gcc8/ia64-hp-hpux11.31/sys-include-o conftest -O2 -g > conftest.c >&5 > configure:8057: $? = 0 > configure:8057: ./conftest > /opt/build/china/gcc-8.3.0/libstdc++-v3/configure[20]: 1572 > Memoryfault(coredum > p) The configure test needs to be debugged in the same manner as the "hello world" program.
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #28 from The Written Word --- (In reply to EML from comment #17) > Note that in certain cases, the MPFR library won't build depending on the > CFLAGS used (in particular the default -g -O2), this is due to problems with > thread local storage. You can work around this by configuring MPFR with > --disable-thread-safe We are building GCC against mpfr-3.1.6 but MPFR wasn't a difficult build on HP-UX/IA. We are building with the HP C compiler though. The MPFR testsuite passed all but one test which was a PASS. And, ./configure shows -DMPFR_USE_THREAD_SAFE=1.
Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC
On 07/02/2019 08:18 PM, Jeff Law wrote: On 7/2/19 11:54 AM, Indu Bhagat wrote: Ping. Can someone please review these patches ? We would like to get the support for CTF integrated soon. I'm not sure there's really even consensus that we want CTF support in GCC. Though I think that the changes you've made in the last several weeks do make it somewhat more palatable. But ultimately the first step is to get that consensus. Thanks for your message. Absolutely, consensus is the first step. We are happy to take all the constructive feedback and answer all the concerns to make certain that CTF support in toolchain will be a useful and worthwhile contribution. I'd hazard a guess that Jakub in particular isn't on board as he's been pushing to some degree for post-processing or perhaps doing it via a plug in. Richi has been guiding you a bit through how to make the changes easier to integrate, but I haven't seen him state one way or the other his preference on whether or not CTF support is something we want. I'm hesitant to add CTF support in GCC, but can understand how it might be useful given the kernel's aversion to everything dwarf. But if the kernel is the primary consumer than I'd lean towards post-processing. Kernel is just *one* of the consumers. There are other applications, external and internal to Oracle, that have shown interest. Not just that, a couple of distro and package maintainers have shown interest in enabling CTF by default. Post-processing in kernel and other internally available large applications has been a deterrent for adoption because of high space and compile-time costs. I answered some of Jakub's concerns in the post here https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00131.html. I would even argue that the usecases will only grow if CTF is properly supported in the toolchain. Thanks
Re: [PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas
On 6/12/19 12:25 PM, Oliver Browne wrote:
> Patch fixes following PRs:
> c++/90816 - -finstrument-functions-exclude-function-list improperly
> handles namespace/class definitions
> c++/90809 - -finstrument-functions-exclude-function-list mishandles
> comma escaping
>
> Fixes as follows:
> At flag_instrument_functions_exclude_p [gimplify.c]
> Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
> class information as part of printable name to allow for
> inclusion of namespace / class specification when passing symbols to
> -finstrument-functions-exclude-function-list. Was
> previously lang_hooks.decl_printable_name (fndecl, 0).
>
> At add_comma_separated_to_vector [opts.c]
> Added writing of a null character to w after primary loop finishes, to
> account for offset between r and w when r reaches end of
> passed string.
>
> from Oliver Browne
> PR c++/90816
> PR c++/90809
> * gimplify.c (flag_instrument_functions_exclude_p): include namespace
>information as part of decl name
> * opts.c (add_comma_separated_to_vector): add null character to correct
>position in last token added to token vector
> Index: gimplify.c
> ===
> --- gimplify.c 2019-06-12 19:07:26.872077000 +0100
> +++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
> @@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
> {
>const char *name;
> - int i;
> + unsigned int i;
>char *s;
>
> - name = lang_hooks.decl_printable_name (fndecl, 0);
> - FOR_EACH_VEC_ELT (*v, i, s)
> + name = lang_hooks.decl_printable_name (fndecl, 1);
> + for(i = 0; i < v->length(); i++){
> + s = (*v)[i];
> + if(strstr(name, s) != NULL){
> + return(true);
> + }
> + }
> +/* FOR_EACH_VEC_ELT (*v, i, s)
> if (strstr (name, s) != NULL)
> - return true;
> + return true;*/
> }
So why did you drop the FOR_EACH_VEC_ELT and open-code the loop? I
don't see that as being a necessary change. Leaving the
FOR_EACH_VEC_ELT in place would also avoid the mis-formatting you've
introduced as well as removing clutter of a commented-out hunk of code.
>
> @@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,
>
>return true;
> -}
> \ No newline at end of file
> +}
> Index: opts.c
> ===
> --- opts.c 2019-06-12 19:10:04.354612000 +0100
> +++ opts.c 2019-06-12 18:53:43.675852000 +0100
> @@ -263,7 +263,8 @@ add_comma_separated_to_vector (void **pv
> *w++ = *r++;
> }
> - if (*token_start != '\0')
> + *w = '\0';
> + if (*token_start != '\0'){
> v->safe_push (token_start);
> -
> + }
>*pvec = v;
So why introduce the unnecessary { } scope? And why do it in a way that
is different than 99% of the other code in GCC (where the { and } will
be on individual lines with 2 spaces of indention?
Jeff
[Bug middle-end/90773] Improve piecewise operation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90773 Jeffrey A. Law changed: What|Removed |Added CC||law at redhat dot com --- Comment #4 from Jeffrey A. Law --- But isn't -Os a better choice if we care about this? It gives more compact code. I guess I'm just not sure optimizing this using an overlapping store is all that important.
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #27 from The Written Word --- (In reply to dave.anglin from comment #26) > On 2019-07-03 6:06 p.m., elowe at elowe dot com wrote: > > If I replace those 3 lines and run the assembler+linker by hand - the > > non-working foo.s will run correctly > So, HAVE_AS_LTOFFX_LDXMOV_RELOCS is probably not defined. The configure > script is > likely not detecting this assembler capability correctly. > > Are you using bash shell? If not, I suggest that you use it instead of HP > shell. It's set to 1 for us. We're getting a segfault while building libstdc++-v3 with 8.3.0: configure:7964: checking for ANSI C header files configure:7984: /opt/build/china/gcc-8.3.0/.obj/./gcc/xgcc -B/opt/build/china/gcc-8.3.0/.obj/./gcc/ -B/opt/build/gcc8/ia64-hp-hpux11.31/bin/ -B/opt/build/gcc8/ia64-hp-hpux11.31/lib/ -isystem /opt/build/gcc8/ia64-hp-hpux11.31/include -isystem /opt/build/gcc8/ia64-hp-hpux11.31/sys-include-c -O2 -g conftest.c >&5 configure:7984: $? = 0 configure:8057: /opt/build/china/gcc-8.3.0/.obj/./gcc/xgcc -B/opt/build/china/gcc-8.3.0/.obj/./gcc/ -B/opt/build/gcc8/ia64-hp-hpux11.31/bin/ -B/opt/build/gcc8/i a64-hp-hpux11.31/lib/ -isystem /opt/build/gcc8/ia64-hp-hpux11.31/include -isystem /opt/build/gcc8/ia64-hp-hpux11.31/sys-include-o conftest -O2 -g conftest.c >&5 configure:8057: $? = 0 configure:8057: ./conftest /opt/build/china/gcc-8.3.0/libstdc++-v3/configure[20]: 1572 Memoryfault(coredum p) configure:8057: $? = 139 Using the stage 1 8.3.0 compiler, we can get a simple "hello world" program to compile.
Re: [PATCH] Add -fprofile-note option.
On 7/2/19 6:37 AM, Martin Liška wrote:
@@ -12403,6 +12403,11 @@ the profile feedback data files. See
@option{-fprofile-dir}.
To optimize the program based on the collected profile information, use
@option{-fprofile-use}. @xref{Optimize Options}, for more information.
+@item -fprofile-note=@var{path}
+@opindex fprofile-note
+
+If @var{path} is specified, GCC saves gcno filename into @var{path} location.
+
@item -fprofile-update=@var{method}
@opindex fprofile-update
"gcno filename" is implementor-speak with no context. In other places
the documentation uses "@file{.gcno} file". Please use that here as
well, and add a @cindex entry on the main definition/discussion of these
things and a cross-reference here.
I assume this option only makes sense with some other profiling options.
What are they?
Can there be more than one of these files per gcc invocation? E.g. if
you specify a command line like
gcc -c foo.c bar.c
?? It looks like the code part of the patch would cause the file to be
overwritten. Maybe this should be like -o and diagnose an error?
-Sandra
Merge from trunk to gccgo branch
I merged trunk revision 273026 to the gccgo branch. Ian
Re: [range-ops] patch 05/04: bonus round!
On 7/1/19 4:24 AM, Aldy Hernandez wrote: > This is completely unrelated to range-ops itself, but may yield better > results in value_range intersections. It's just something I found while > working on VRP, and have been dragging around on our branch. > > If we know the intersection of two ranges is the empty set, there is no > need to conservatively add anything to the result. > > Tested on x86-64 Linux with --enable-languages=all. > > Aldy > > range-ops-intersect-undefined.patch > > commit 4f9aa7bd1066267eee92f622ff29d78534158e20 > Author: Aldy Hernandez > Date: Fri Jun 28 11:34:19 2019 +0200 > > Do not try to further refine a VR_UNDEFINED result when intersecting > value_ranges. > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index 01fb97cedb2..b0d78ee6871 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,9 @@ > +2019-07-01 Aldy Hernandez > + > + * tree-vrp.c (intersect_ranges): If we know the intersection is > + empty, there is no need to conservatively add anything else to > + the set. Do we have a test where this improves the code or at least the computed ranges? jeff
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577 --- Comment #26 from dave.anglin at bell dot net --- On 2019-07-03 6:06 p.m., elowe at elowe dot com wrote: > If I replace those 3 lines and run the assembler+linker by hand - the > non-working foo.s will run correctly So, HAVE_AS_LTOFFX_LDXMOV_RELOCS is probably not defined. The configure script is likely not detecting this assembler capability correctly. Are you using bash shell? If not, I suggest that you use it instead of HP shell.
Re: [PATCH] Fix out-of-ssa with unsupported vector types (PR rtl-optimization/90756)
On 7/3/19 1:59 AM, Jakub Jelinek wrote: > Hi! > > This weird hink has been added by Alex in r228175, it isn't clear why > nor how it ever can be correct. While say for DECL_MODE we have the problem > that for global vars when switching between functions with different ISA > selections the mode might not be ok, TYPE_MODE is stored as a raw vector > mode that a function overrides to BLKmode if that particular vector mode is > not supported. This hunk breaks that assumption and leaks unsupported > vector modes in the IL of the functions which then have no way to handle > that, but even before that happens usually it breaks because we try to > convert_mode between BLKmode and the unsupported vector mode or vice versa > on PHI nodes. > > Alex, do you remember why this has been done? > > Patch has been bootstrapped/regtested on x86_64-linux and i686-linux (the > latter didn't have SSE enabled by default), Jeff said he'll test it on many > crosses. Ok for trunk if that testing succeeds? > > 2019-07-03 Jakub Jelinek > > PR rtl-optimization/90756 > * explow.c (promote_ssa_mode): Always use TYPE_MODE, don't bypass it > for VECTOR_TYPE_P. > > * gcc.dg/pr90756.c: New test. Nothing tripped related to this patch in the various targets in my tester. jeff
[Bug target/91050] -mdejagnu-cpu= does not affect the -m assembler option
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91050 --- Comment #7 from Alan Modra --- Huh, I'd forgotten that gas only reloads the opcode table when the cpu changes. Be aware that .machine isn't a complete solution as it doesn't fix a wrong gas command line for "gcc -c asm.S". You can't insert .machine in that case!
Re: [range-ops] patch 02/04: enforce canonicalization in value_range
On 7/3/19 3:35 AM, Aldy Hernandez wrote:
> On 7/2/19 5:36 PM, Jeff Law wrote:
>
>> I don't see anything inherently concerning here. I do wonder if there's
>> any value in having a debugging function in the class that would iterate
>> over the ranges and check them for proper canonicalization, verify that
>> VR_{VARYING,UNDEFINED} objects do not have equivalences, etc. Thoughts?
>
> In patch 01 we have:
>
>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
>> index 594ee9adc17..97046c22ed1 100644
>> --- a/gcc/tree-vrp.c
>> +++ b/gcc/tree-vrp.c
>> @@ -86,6 +86,8 @@ value_range_base::set (enum value_range_kind kind,
>> tree min, t
>> ree max)
>> void
>> value_range::set_equiv (bitmap equiv)
>> {
>> + if (undefined_p () || varying_p ())
>> + equiv = NULL;
>
> So it shouldn't be possible to attach an equivalence to a VARYING /
> UNDEFINED range. Plus, we already have a sanity checker:
>
>> void
>> value_range::check ()
>> {
>> value_range_base::check ();
>> switch (m_kind)
>> {
>> case VR_UNDEFINED:
>> case VR_VARYING:
>> gcc_assert (!m_equiv || bitmap_empty_p (m_equiv));
>> default:;
>> }
>> }
>
> We have no methods for altering a range, except for changing
> equivalences. So it shouldn't be possible to create a non-canonicalized
> range.
Ah. Cool. Thanks for enlightening me ;-)
jeff
Re: [MIPS][Testsuite] specify msa-fmadd.c abis
On 7/3/19 12:52 AM, Paul Hua wrote: > Hi, > > The msa-fmadd.c fails on abi=64, the attached patch fixed by specify the abis. > > spawn -ignore SIGHUP > /home/xuchenghua/GCC/test/gcc-r272929_obj/gcc/xgcc > -B/home/xuchenghua/GCC/test/gcc-r272929_obj/gcc/ > /home/xuchenghua/GCC/gcc_git_trunk/gcc/testsuite/gcc.target/mips/msa-fmadd.c > -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers > -fdiagnostics-color=never -O1 -DNOMIPS16=__attribute__((nomips16)) > -DNOMICROMIPS=__attribute__((nomicromips)) > -DNOCOMPRESSION=__attribute__((nocompression)) -EL -mhard-float > -mdouble-float -mfp64 -mno-mips16 -modd-spreg -mmsa > -flax-vector-conversions -ffat-lto-objects -fno-ident -S -o > msa-fmadd.s^M > /home/xuchenghua/GCC/gcc_git_trunk/gcc/testsuite/gcc.target/mips/msa-fmadd.c:11:16: > warning: call-clobbered register used for global register variable^M > /home/xuchenghua/GCC/gcc_git_trunk/gcc/testsuite/gcc.target/mips/msa-fmadd.c:12:16: > warning: call-clobbered register used for global register variable^M > FAIL: gcc.target/mips/msa-fmadd.c -O1 (test for excess errors) > Excess errors: > /home/xuchenghua/GCC/gcc_git_trunk/gcc/testsuite/gcc.target/mips/msa-fmadd.c:11:16: > warning: call-clobbered register used for global register variable > /home/xuchenghua/GCC/gcc_git_trunk/gcc/testsuite/gcc.target/mips/msa-fmadd.c:12:16: > warning: call-clobbered register used for global register variable > > > Ok for commit ? > > > 0001-MIPS-Testsuite-specify-msa-fmadd.c-abis.patch > > From 912581f71ad37b415aec06d23210109e1c778296 Mon Sep 17 00:00:00 2001 > From: Chenghua Xu > Date: Mon, 17 Jun 2019 14:36:37 +0800 > Subject: [PATCH] [MIPS][Testsuite] specify msa-fmadd.c abis. > > gcc/testsuite/ > > * gcc.target/mips/mips-fmadd.c: Rename to ... > * gcc.target/mips/mips-fmadd-o32.c: ... Here; add abi=32. > * gcc.target/mips/mips-fmadd-n64.c: New. OK jeff
Re: [PATCH v2 3/5] or1k: Add mrori option, fix option docs
On Wed, Jul 03, 2019 at 05:26:58PM -0500, Segher Boessenkool wrote:
> On Thu, Jul 04, 2019 at 06:49:17AM +0900, Stafford Horne wrote:
> > On Wed, Jul 03, 2019 at 09:49:02AM -0500, Segher Boessenkool wrote:
> > > On Wed, Jul 03, 2019 at 12:33:49PM +0900, Stafford Horne wrote:
> > > > @@ -179,11 +183,11 @@
> > > >[(set (match_operand:SI 0 "register_operand" "=r,r")
> > > > (rotatert:SI (match_operand:SI 1 "register_operand" "r,r")
> > > > (match_operand:SI 2 "reg_or_u6_operand" "r,n")))]
> > > > - "TARGET_ROR"
> > > > + "TARGET_ROR || TARGET_RORI"
> > > >"@
> > > > l.ror\t%0, %1, %2
> > > > l.rori\t%0, %1, %2"
> > > > - [(set_attr "insn_support" "*,shftimm")])
> > > > + [(set_attr "insn_support" "ror,rori")])
> > >
> > > Does this work? If you use -mno-ror -mrori? It will then allow
> > > generating
> > > a reg for the second operand, and ICE later on, as far as I can see?
> >
> > It does seem to work. Why would it produce an internal compiler error?
> >
> > One thing I have is RegectNegative on mror and mrori, so -mno-ror will not
> > be
> > allowed and cause an error.
>
> But both options are off by default, and neither is enabled or disabled
> based on the setting of the other.
>
> > Example:
> >
> > $ cat ./gcc/testsuite/gcc.target/or1k/ror-4.c
> >
> > unsigned int rotate6 (unsigned int a) {
> > return ( a >> 6 ) | ( a << ( 32 - 6 ) );
> > }
>
> That's a fixed distance rotate. My question is will it work if the
> distance is a variable. The other direction should work fine, agreed.
>
> So, does ror-[12].c work with -mrori and no -mror? The predicates say
> this insn pattern is just fine in that case, but the constraints will
> disagree.
OK, yes I see it now. Sorry I mis-understood what you meant by second argument.
I will fix. It's probably going to be easiest to split this to 2 instructions.
-Stafford
Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
On 7/3/19 3:46 AM, Aldy Hernandez wrote:
>
>
> On 7/2/19 4:17 PM, Jeff Law wrote:
>> On 7/1/19 2:52 AM, Aldy Hernandez wrote:
>>> As discussed before, this enforces types on undefined and varying, which
>>> makes everything more regular, and removes some special casing
>>> throughout range handling.
>>>
>>> The min/max fields will contain TYPE_MIN_VALUE and TYPE_MAX_VALUE, which
>>> will make it easy to get at the bounds of a range later on. Since
>>> pointers don't have TYPE_MIN/MAX_VALUE, we are using build_zero_cst()
>>> and wide_int_to_tree(wi::max_value(precision)), for consistency.
>>> UNDEFINED is set similarly, though nobody should ever ask for anything
>>> except type() from it. That is, no one should be asking for the bounds.
>>>
>>> There is one wrinkle, ipa-cp creates VR_VARYING ranges of floats,
>>> presumably to pass around state?? This causes value_range_base::type()
>>> and others to fail, even though I haven't actually seen a case of
>>> someone actually trying to set a VR_RANGE of a float. For now, I've
>>> built a NOP_EXPR wrapper so type() works correctly. The correct
>>> approach would probably be to avoid creating these varying/undefined
>>> ranges in ipa-cp, but I don't have enough ipa-cp-foo to do so.
>>> Suggestions welcome, if you don't like special casing this for ipa-cp.
>>> Or perhaps as a follow up.
>> No idea why we create ranges of floats from ipa-cp. I presume it's
>> coming from propagate_vr_across_jump_function? Or somewhere else?
>
> I believe it was ipcp_vr_lattice::set_to_bottom, while changing an
> UNDEFINED to VARYING. IMO, we shouldn't even have created UNDEFINED
> ranges of floats. It's not like we can do anything with float ranges.
Note that propagate_vr_across_jump_function calls set_to_bottom ;-)
I zero'd in on that one because it did so when presented with something
that wasn't an INTEGRAL_TYPE_P and wasn't POINTER_TYPE_P.
I think digging into where these are coming from would be advisable.
Hell, if you've got types, abort the first time we try to create a range
for something that isn't an integer/pointer, then walk backwards.
I wouldn't be surprised if we find just a couple sites that are
responsible for these problems in ipa-cp.
>>> +/* Allocate a new range from the obstack and set it to VARYING for
>>> TYPE. */
>>> +inline value_range *
>>> +type_range_cache::new_varying (tree type)
>>> +{
>>> + /* Allocate memory. */
>>> + void *p = XOBNEW (_range_obj, value_range);
>>> + /* Invoke the constructors on the memory using placement new. */
>>> + value_range *new_p = new (p) value_range ();
>>> + /* Initialize it to varying. */
>>> + new_p->set_varying (type);
>>> + return new_p;
>>> +}
>> So is placement new C++98/C++03 or C++11?
>>
>> If the former then we can use it, if the latter we probably can't since
>> we haven't stepped forward to C++11.
>
> Google isn't cooperating here to narrow the specific C++ version, but
> I'm seeing some very old references to placement new, from the mid to
> the late 1990s.
>
> FWIW, the above snippet shows no warnings when compiled with -std=c++-98
> -Wall.
Given it compiles in C++-98 mode, let's consider it a non-issue.
jeff
[PATCH] [Ada] Make clockid_t type public on GNU/kFreeBSD
From: James Clarke Monotonic_Clock and RT_Resolution in the recently-added s-tpopmo.adb call clock_gettime/clock_getres with the integral constants from OSC and thus rely on clockid_t being an integral type, so we cannot hide it on GNU/kFreeBSD. Instead, make the definition public to match all the other implementations. gcc/ada * libgnarl/s-osinte__kfreebsd-gnu.ads (clockid_t): Make type definition public. (CLOCK_REALTIME): Make value public. --- gcc/ada/libgnarl/s-osinte__kfreebsd-gnu.ads | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gcc/ada/libgnarl/s-osinte__kfreebsd-gnu.ads b/gcc/ada/libgnarl/s-osinte__kfreebsd-gnu.ads index 408187314..b60ffd2c0 100644 --- a/gcc/ada/libgnarl/s-osinte__kfreebsd-gnu.ads +++ b/gcc/ada/libgnarl/s-osinte__kfreebsd-gnu.ads @@ -206,9 +206,8 @@ package System.OS_Interface is function nanosleep (rqtp, rmtp : access timespec) return int; pragma Import (C, nanosleep, "nanosleep"); - type clockid_t is private; - - CLOCK_REALTIME : constant clockid_t; + type clockid_t is new int; + CLOCK_REALTIME : constant clockid_t := 0; function clock_gettime (clock_id : clockid_t; @@ -607,9 +606,6 @@ private end record; pragma Convention (C, timespec); - type clockid_t is new int; - CLOCK_REALTIME : constant clockid_t := 0; - type pthread_attr_t is record detachstate : int; schedpolicy : int; -- 2.17.1
Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On Thu, Jul 04, 2019 at 07:19:56AM +0900, Stafford Horne wrote:
> On Wed, Jul 03, 2019 at 09:09:51PM +0200, Richard Henderson wrote:
> > On 7/3/19 5:43 PM, Segher Boessenkool wrote:
> > >> @@ -212,6 +214,7 @@ enum reg_class
> > >> #define REG_CLASS_CONTENTS \
> > >> { { 0x, 0x }, \
> > >>{ SIBCALL_REGS_MASK, 0 }, \
> > >> + { 0x7efe, 0x }, \
> > >
> > > Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> > > in GCC register numbers, 0, 8, and 31? You probably should mention r8
> > > somewhere (it's because it is the last arg, this avoid problems, I
> > > guess?),
> > > and the 30/31 thing is confused some way. Maybe it is all just that one
> > > documentation line :-)
> >
> > ... and if r8 is excluded because of arguments, I suspect that this is the
> > wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a
> > pair,
> > at least that I can see.
> >
> > Perhaps function_arg and/or function_arg_advance is the right place for a
> > fix?
> > The calling convention says that 64-bit arguments are not split across
> > registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
>
> I will double check, the mask may be wrong. It should not matter about the
> function args.
>
> I didn't see any issue that caused me to add r8. So I may have just masked
> thw
> rong bit thinking it's r31. Is there something worng with what I did?
>
> The mask is 0x7efe, and names should corresbond to this name list?
>
> #define REGISTER_NAMES {
> "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", # 7e, excl r0
> "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", # ff, excl
> none
> "r17", "r19", "r21", "r23", "r25", "r27", "r29", "r31", # fe, excl
> r31
> "r16", "r18", "r20", "r22", "r24", "r26", "r28", "r30", # fe, excl
> r30
> "?ap", "?fp", "?sr_f" }
>
> Do I have it backwards? With an endian issue?
Yes :-) 0x0001 is reg 0 (r0), 0x8000 is reg 31 (r30).
Segher
Re: [PATCH] simplify-rtx.c: Change BITSIZE to UNIT_PRECISION in simplification
On 7/3/19 2:17 PM, Richard Sandiford wrote:
>> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
>> index 89a46a9..d74a4ba 100644
>> --- a/gcc/simplify-rtx.c
>> +++ b/gcc/simplify-rtx.c
>> @@ -1504,12 +1504,12 @@ simplify_unary_operation_1 (enum rtx_code code,
>> machine_mode mode, rtx op)
>>&& CONST_INT_P (XEXP (op, 1))
>>&& XEXP (XEXP (op, 0), 1) == XEXP (op, 1)
>>&& (op_mode = as_a (GET_MODE (op)),
>> - GET_MODE_BITSIZE (op_mode) > INTVAL (XEXP (op, 1
>> + GET_MODE_UNIT_PRECISION (op_mode) > INTVAL (XEXP (op, 1
>> {
>>scalar_int_mode tmode;
>> - gcc_assert (GET_MODE_BITSIZE (int_mode)
>> - > GET_MODE_BITSIZE (op_mode));
>> - if (int_mode_for_size (GET_MODE_BITSIZE (op_mode)
>> + gcc_assert (GET_MODE_UNIT_PRECISION (int_mode)
>> + > GET_MODE_UNIT_PRECISION (op_mode));
>> + if (int_mode_for_size (GET_MODE_UNIT_PRECISION (op_mode)
>> - INTVAL (XEXP (op, 1)), 1).exists ())
>> {
>>rtx inner =
>
> I think these should be GET_MODE_PRECISION rather than
> GET_MODE_UNIT_PRECISION. They do the same thing in this context,
> but it seems odd to use GET_MODE_UNIT_... when we're specifically
> dealing with scalars.
>
> Looks good otherwise (and small enough not to need a copyright assignment,
> in case you haven't signed one).
John, I assume you're doing this for an out of tree port (s12z?)?
Otherwise it'd also be useful if you could include a test which triggers
the assert.
If you could confirm that Richard's suggestion of using
GET_MODE_PRECISION rather than GET_MODE_UNIT_PRECISION works it'd be
appreciated.
Thanks,
jeff
Go patch committed: Set varargs lowered for imported call expressions
This Go frontend patch by Than McIntosh fixes a Go frontend buglet:
varargs lowering happens before inlinable function bodies are written
out to export data, so set the "varargs lowered" flag on call
expressions that we import. This fixes
https://golang.org/issue/32922. The test case is in
https://golang.org/cl/184918. 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 273009)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-ae7d7e05bce19aefaa27efe2ee797933aafbef06
+bf66d40bc7adb438dcfac85d73bfa7b17217eed9
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 272944)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -18338,6 +18338,7 @@ Expression::import_expression(Import_exp
}
imp->require_c_string(")");
expr = Expression::make_call(expr, args, is_varargs, loc);
+ expr->call_expression()->set_varargs_are_lowered();
}
else if (imp->match_c_string("["))
{
@@ -19389,4 +19390,3 @@ Numeric_constant::hash(unsigned int seed
return (static_cast(val) + seed) * PRIME;
}
-
[Bug c++/83374] [DR1813] Bad std::is_standard_layout with two base class subobjects of the same type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83374 Marek Polacek changed: What|Removed |Added Keywords||accepts-invalid, patch, ||rejects-valid --- Comment #4 from Marek Polacek --- https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00304.html
Re: [PATCH v2 3/5] or1k: Add mrori option, fix option docs
On Thu, Jul 04, 2019 at 06:49:17AM +0900, Stafford Horne wrote:
> On Wed, Jul 03, 2019 at 09:49:02AM -0500, Segher Boessenkool wrote:
> > On Wed, Jul 03, 2019 at 12:33:49PM +0900, Stafford Horne wrote:
> > > @@ -179,11 +183,11 @@
> > >[(set (match_operand:SI 0 "register_operand" "=r,r")
> > > (rotatert:SI (match_operand:SI 1 "register_operand" "r,r")
> > > (match_operand:SI 2 "reg_or_u6_operand" "r,n")))]
> > > - "TARGET_ROR"
> > > + "TARGET_ROR || TARGET_RORI"
> > >"@
> > > l.ror\t%0, %1, %2
> > > l.rori\t%0, %1, %2"
> > > - [(set_attr "insn_support" "*,shftimm")])
> > > + [(set_attr "insn_support" "ror,rori")])
> >
> > Does this work? If you use -mno-ror -mrori? It will then allow generating
> > a reg for the second operand, and ICE later on, as far as I can see?
>
> It does seem to work. Why would it produce an internal compiler error?
>
> One thing I have is RegectNegative on mror and mrori, so -mno-ror will not be
> allowed and cause an error.
But both options are off by default, and neither is enabled or disabled
based on the setting of the other.
> Example:
>
> $ cat ./gcc/testsuite/gcc.target/or1k/ror-4.c
>
> unsigned int rotate6 (unsigned int a) {
> return ( a >> 6 ) | ( a << ( 32 - 6 ) );
> }
That's a fixed distance rotate. My question is will it work if the
distance is a variable. The other direction should work fine, agreed.
So, does ror-[12].c work with -mrori and no -mror? The predicates say
this insn pattern is just fine in that case, but the constraints will
disagree.
Segher
C++ PATCH for DR 1813, c++/83374 - __is_standard_layout wrong for a class with repeated bases.
This resolves DR 1813 which says that for a class to be a standard-layout
class, it may not have two (possibly indirect) base class subobjects of the
same type. I was going to play DFS games but then I noticed we'd already set
CLASSTYPE_REPEATED_BASE_P, making this significantly easier.
There are 3 related DRs which I opened PRs for:
91079 - [DR 1881] Standard-layout classes and unnamed bit-fields
91080 - [DR 1672] Layout compatibility with multiple empty bases
91081 - [DR 2120] Array as first non-static data member in standard-layout class
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-07-03 Marek Polacek
DR 1813
PR c++/83374 - __is_standard_layout wrong for a class with repeated
bases.
* class.c (check_bases): Set CLASSTYPE_NON_STD_LAYOUT for a class if
CLASSTYPE_REPEATED_BASE_P is true.
* g++.dg/ext/is_std_layout3.C: New test.
* g++.dg/ext/is_std_layout4.C: New test.
diff --git gcc/cp/class.c gcc/cp/class.c
index 73291b341fe..f77b7f4834b 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -1715,11 +1715,15 @@ check_bases (tree t,
&& (same_type_ignoring_top_level_qualifiers_p
(TREE_TYPE (field), basetype)))
CLASSTYPE_NON_STD_LAYOUT (t) = 1;
+ /* DR 1813:
+...has at most one base class subobject of any given type... */
+ else if (CLASSTYPE_REPEATED_BASE_P (t))
+ CLASSTYPE_NON_STD_LAYOUT (t) = 1;
else
/* ...either has no non-static data members in the most-derived
class and at most one base class with non-static data
members, or has no base classes with non-static data
- members */
+ members. FIXME This was reworded in DR 1813. */
for (basefield = TYPE_FIELDS (basetype); basefield;
basefield = DECL_CHAIN (basefield))
if (TREE_CODE (basefield) == FIELD_DECL
diff --git gcc/testsuite/g++.dg/ext/is_std_layout3.C
gcc/testsuite/g++.dg/ext/is_std_layout3.C
new file mode 100644
index 000..b0555c8207b
--- /dev/null
+++ gcc/testsuite/g++.dg/ext/is_std_layout3.C
@@ -0,0 +1,18 @@
+// DR 1813
+// PR c++/83374 - __is_standard_layout wrong for a class with repeated bases.
+// { dg-do compile { target c++11 } }
+
+struct B { int i; };// standard-layout class
+struct C : B { }; // standard-layout class
+struct D : C { }; // standard-layout class
+struct E : D { char : 4; }; // not a standard-layout class
+static_assert( __is_standard_layout(B), "" );
+static_assert( __is_standard_layout(C), "" );
+static_assert( __is_standard_layout(D), "" );
+static_assert( ! __is_standard_layout(E), "" );
+
+struct Q {};
+struct S : Q { };
+struct T : Q { };
+struct U : S, T { }; // not a standard-layout class
+static_assert( ! __is_standard_layout(U), "" );
diff --git gcc/testsuite/g++.dg/ext/is_std_layout4.C
gcc/testsuite/g++.dg/ext/is_std_layout4.C
new file mode 100644
index 000..09c0098120d
--- /dev/null
+++ gcc/testsuite/g++.dg/ext/is_std_layout4.C
@@ -0,0 +1,11 @@
+// DR 1813
+// PR c++/83374 - __is_standard_layout wrong for a class with repeated bases.
+// { dg-do compile { target c++11 } }
+
+struct R { };
+struct Q { };
+struct S : R { };
+struct T : Q { };
+struct U : S, T { };
+// No repeated base class subobjects.
+static_assert(__is_standard_layout(U), "");
Call for testers: improve move2add_use_add2_insn for targets with HAVE_POST_MODIFY_DISP and/or clobber-encumbered add
2019-07-03 Joern Rennecke
* postreload.c (rtl-iter.h): Include.
(reg_addr_use_luid, reg_addr_use_insn): New static variables.
(reg_addr_use, move2add_last_jump_luid): Likewise.
(move2add_use_add2_insn): Try to use a POST_MODIFY before and add.
(reload_cse_move2add): Keep new static variables up to date.
(move2add_use_add2_insn): If using add fails, try
to use movstr* or xor.
Index: postreload.c
===
--- postreload.c(revision 272931)
+++ postreload.c(working copy)
@@ -40,6 +40,7 @@ Software Foundation; either version 3, o
#include "cselib.h"
#include "tree-pass.h"
#include "dbgcnt.h"
+#include "rtl-iter.h"
static int reload_cse_noop_set_p (rtx);
static bool reload_cse_simplify (rtx_insn *, rtx);
@@ -1646,6 +1647,12 @@ reload_combine_note_use (rtx *xp, rtx_in
static rtx reg_symbol_ref[FIRST_PSEUDO_REGISTER];
static machine_mode reg_mode[FIRST_PSEUDO_REGISTER];
+/* Note when and where we last saw this register used as a plain
+ register-indirect address. */
+static int reg_addr_use_luid[FIRST_PSEUDO_REGISTER];
+static rtx_insn *reg_addr_use_insn[FIRST_PSEUDO_REGISTER];
+static rtx *reg_addr_use[FIRST_PSEUDO_REGISTER];
+
/* move2add_luid is linearly increased while scanning the instructions
from first to last. It is used to set reg_set_luid in
reload_cse_move2add and move2add_note_store. */
@@ -1654,6 +1661,7 @@ reload_combine_note_use (rtx *xp, rtx_in
/* move2add_last_label_luid is set whenever a label is found. Labels
invalidate all previously collected reg_offset data. */
static int move2add_last_label_luid;
+static int move2add_last_jump_luid;
/* ??? We don't know how zero / sign extension is handled, hence we
can't go from a narrower to a wider mode. */
@@ -1768,6 +1776,18 @@ move2add_use_add2_insn (scalar_int_mode
if (INTVAL (off) == reg_offset [regno])
changed = validate_change (insn, _SRC (pat), reg, 0);
}
+ else if (HAVE_POST_MODIFY_DISP
+ && reg_addr_use_luid[regno] > reg_set_luid[regno]
+ && reg_addr_use_luid[regno] > move2add_last_jump_luid
+ && !reg_used_between_p (reg, reg_addr_use_insn[regno], insn)
+ && validate_change
+ (reg_addr_use_insn[regno], reg_addr_use[regno],
+gen_rtx_POST_MODIFY (mode, reg,
+ gen_rtx_PLUS (mode, reg, new_src)),
+1)
+ && validate_change (insn, _SRC (pat), reg, 1)
+ && apply_change_group ())
+changed = true;
else
{
struct full_rtx_costs oldcst, newcst;
@@ -1779,8 +1799,9 @@ move2add_use_add2_insn (scalar_int_mode
SET_SRC (pat) = src;
if (costs_lt_p (, , speed)
- && have_add2_insn (reg, new_src))
- changed = validate_change (insn, _SRC (pat), tem, 0);
+ && have_add2_insn (reg, new_src)
+ && validate_change (insn, _SRC (pat), tem, 0))
+ changed = true;
else if (sym == NULL_RTX && mode != BImode)
{
scalar_int_mode narrow_mode;
@@ -1807,6 +1828,19 @@ move2add_use_add2_insn (scalar_int_mode
}
}
}
+ /* Some processors clobber some flags for add (hence it won't match
+above), but none (that the compiler models) for xor. */
+ if (!changed)
+ {
+ new_src = gen_int_mode (UINTVAL (off) ^ reg_offset[regno], mode);
+ tem = gen_rtx_XOR (mode, reg, new_src);
+ SET_SRC (pat) = tem;
+ get_full_set_rtx_cost (pat, );
+ SET_SRC (pat) = src;
+ if (costs_lt_p (, , speed)
+ && validate_change (insn, _SRC (pat), tem, 0))
+ changed = true;
+ }
}
}
move2add_record_sym_value (reg, sym, off);
@@ -1908,6 +1942,7 @@ reload_cse_move2add (rtx_insn *first)
for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; i--)
{
reg_set_luid[i] = 0;
+ reg_addr_use_luid[i] = 0;
reg_offset[i] = 0;
reg_base_reg[i] = 0;
reg_symbol_ref[i] = NULL_RTX;
@@ -1915,6 +1950,7 @@ reload_cse_move2add (rtx_insn *first)
}
move2add_last_label_luid = 0;
+ move2add_last_jump_luid = 0;
move2add_luid = 2;
for (insn = first; insn; insn = NEXT_INSN (insn), move2add_luid++)
{
@@ -2104,8 +2140,29 @@ reload_cse_move2add (rtx_insn *first)
}
}
}
+
+ if (HAVE_POST_MODIFY_DISP)
+ {
+ subrtx_var_iterator::array_type array;
+ FOR_EACH_SUBRTX_VAR (iter, array, PATTERN (insn), NONCONST)
+ if (MEM_P (*iter))
+ {
+ rtx addr = XEXP (*iter, 0);
+ if (REG_P (addr))
+ {
+ int regno = REGNO (addr);
+ reg_addr_use_luid[regno] = move2add_luid;
+ reg_addr_use_insn[regno] = insn;
Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On Wed, Jul 03, 2019 at 09:09:51PM +0200, Richard Henderson wrote:
> On 7/3/19 5:43 PM, Segher Boessenkool wrote:
> >> @@ -212,6 +214,7 @@ enum reg_class
> >> #define REG_CLASS_CONTENTS \
> >> { { 0x, 0x }, \
> >>{ SIBCALL_REGS_MASK, 0 }, \
> >> + { 0x7efe, 0x }, \
> >
> > Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> > in GCC register numbers, 0, 8, and 31? You probably should mention r8
> > somewhere (it's because it is the last arg, this avoid problems, I guess?),
> > and the 30/31 thing is confused some way. Maybe it is all just that one
> > documentation line :-)
>
> ... and if r8 is excluded because of arguments, I suspect that this is the
> wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a pair,
> at least that I can see.
>
> Perhaps function_arg and/or function_arg_advance is the right place for a fix?
> The calling convention says that 64-bit arguments are not split across
> registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
I will double check, the mask may be wrong. It should not matter about the
function args.
I didn't see any issue that caused me to add r8. So I may have just masked thw
rong bit thinking it's r31. Is there something worng with what I did?
The mask is 0x7efe, and names should corresbond to this name list?
#define REGISTER_NAMES {
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", # 7e, excl r0
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", # ff, excl none
"r17", "r19", "r21", "r23", "r25", "r27", "r29", "r31", # fe, excl r31
"r16", "r18", "r20", "r22", "r24", "r26", "r28", "r30", # fe, excl r30
"?ap", "?fp", "?sr_f" }
Do I have it backwards? With an endian issue?
-Stafford
[Bug c++/91081] New: [DR 2120] Array as first non-static data member in standard-layout class
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91081
Bug ID: 91081
Summary: [DR 2120] Array as first non-static data member in
standard-layout class
Product: gcc
Version: 10.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: mpolacek at gcc dot gnu.org
Target Milestone: ---
Cf. https://wg21.link/cwg2120
Test:
struct A {};
struct B : A {};
struct C { A a; };
struct D { C c[5]; };
struct E : B { D d; };
static_assert(__is_standard_layout(B), "");
static_assert(__is_standard_layout(D), "");
static_assert(!__is_standard_layout(E), "");
[Bug c++/91080] New: [DR 1672] Layout compatibility with multiple empty bases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91080
Bug ID: 91080
Summary: [DR 1672] Layout compatibility with multiple empty
bases
Product: gcc
Version: 10.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: mpolacek at gcc dot gnu.org
Target Milestone: ---
Cf. https://wg21.link/cwg1672
Test (from the clang testsuite):
struct Empty {};
struct A : Empty {};
struct B { Empty e; };
struct C : A { B b; int n; };
struct D : A { int n; B b; };
static_assert(!__is_standard_layout(C), "");
static_assert(__is_standard_layout(D), "");
struct E { B b; int n; };
struct F { int n; B b; };
union G { B b; int n; };
union H { int n; B b; };
struct X {};
template struct Y : X, A { T t; };
static_assert(!__is_standard_layout(Y), "");
static_assert(__is_standard_layout(Y), "");
static_assert(!__is_standard_layout(Y), "");
static_assert(!__is_standard_layout(Y), "");
static_assert(!__is_standard_layout(Y), "");
Re: [PATCH] PowerPC Prefixed Memory, Patch #4, Add pc-relative reference support
Hi Mike, On Fri, Jun 28, 2019 at 02:50:33PM -0400, Michael Meissner wrote: > --- gcc/config/rs6000/rs6000-logue.c (revision 272714) > +++ gcc/config/rs6000/rs6000-logue.c (working copy) > @@ -1406,23 +1406,13 @@ uses_TOC (void) > } > #endif > > +/* Create a TOC style reference for a symbol. */ > rtx > create_TOC_reference (rtx symbol, rtx largetoc_reg) Does this really belong in this file? It doesn't do anythin *logue, and it isn't called from anywhere in here. > +/* Create either a TOC reference to a locally defined item or a pc-relative > + reference, depending on the ABI. */ > +rtx > +create_data_reference (rtx symbol, rtx largetoc_reg) Same here. What is largetoc_reg? The function comment should say. It also is only relevant for create_TOC_reference (where such a comment is also missing), so could you factor this better please? Probably a create_data_reference with only one argument? Which calls create_TOC_reference with a NULL second arg. It looks like your proposed create_data_reference will not do the right thing if called with a non-null second arg if pcrel. Perhaps that cannot happen, but make that clear then? Just an assert will do, bigger cleanups are better of course. Segher
Re: [PATCH v2 4/5] or1k: Initial support for FPU
On Wed, Jul 03, 2019 at 10:43:01AM -0500, Segher Boessenkool wrote:
> Hi Stafford,
>
> On Wed, Jul 03, 2019 at 12:33:50PM +0900, Stafford Horne wrote:
> > +case 'd':
> > + if (REG_P (x))
> > + if (GET_MODE (x) == DFmode || GET_MODE (x) == DImode)
> > + fprintf (file, "%s,%s", reg_names[REGNO (operand)],
> > + reg_names[REGNO (operand) + 1]);
> > + else
> > + fprintf (file, "%s", reg_names[REGNO (operand)]);
> > + else
>
> The coding conventions says to use braces around nested conditionals.
Right I will fix that. Interestingly the indentation is correct just missing
the braces.
> > @@ -212,6 +214,7 @@ enum reg_class
> > #define REG_CLASS_CONTENTS \
> > { { 0x, 0x }, \
> >{ SIBCALL_REGS_MASK, 0 }, \
> > + { 0x7efe, 0x }, \
>
> Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> in GCC register numbers, 0, 8, and 31? You probably should mention r8
> somewhere (it's because it is the last arg, this avoid problems, I guess?),
> and the 30/31 thing is confused some way. Maybe it is all just that one
> documentation line :-)
>
> > +; d - double pair base registers (excludes r0, r30 and r31 which overflow)
Hmm, maybe I messed up the mask. It should be r0, r30 and r31. Register pairs
can be a base register (rX) with a +1 or +2 offset second register.
Registers not allowed
- r0, because its reserved for hardwired zero and doesn't work as a double
zero when paired with a general register.
- r31, because it cant pair with r32 or r33 (those are overflows)
- r30, because it cant work when paried with r32 (its an overflow), it would
work with r31, but GCC will not generate that pair anyway.
-Stafford
[Bug c++/91079] New: [DR 1881] Standard-layout classes and unnamed bit-fields
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91079
Bug ID: 91079
Summary: [DR 1881] Standard-layout classes and unnamed
bit-fields
Product: gcc
Version: 10.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: mpolacek at gcc dot gnu.org
Target Milestone: ---
Cf. https://wg21.link/cwg1881
Test:
struct A { int a : 4; };
struct B : A { int b : 3; };
static_assert(__is_standard_layout(A), "");
static_assert(!__is_standard_layout(B), "");
struct C { int : 0; };
struct D : C { int : 0; };
static_assert(__is_standard_layout(C), "");
static_assert(!__is_standard_layout(D), "");
[Bug target/61577] [4.9.0] can't compile on hp-ux v3 ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61577
--- Comment #25 from EML ---
I have applied the patch and tried your other suggestions, still the stage1
compiler has the same problems generating executables.
In analyzing the intermediate files between working (gcc 4.93) and not
(bootstrap 8.3), the intermediate files seem similar until the "mach" stage
The problem seems to be in out the compiler decides to reference a string in
the source.
My program is:
#include
int main()
{
printf("Hellos World\n");
return 0;
}
The generated .s file for Working does this:
.LC0:
stringz "Hellos World"
addl r36 = @ltoffx(.LC0), r1
;;
ld8.mov r36 = [r36], .LC0
The non-working .s file does this:
.LC0:
stringz "Hellos World"
movl r36 = @gprel(.LC0)
;;
add r36 = r1, r36
If I replace those 3 lines and run the assembler+linker by hand - the
non-working foo.s will run correctly
Re: [patch] Small improvements to coverage info (2/n)
On 7/3/19 7:35 AM, Eric Botcazou wrote: > Hi, > > this is a series of fixes for the exception handling code, with the same goal > of preventing instructions from inheriting random source location information > in the debug info generated by the compiler. > > Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline? > > > 2019-07-03 Eric Botcazou > > * except.c (emit_to_new_bb_before): Make sure to put a location on SEQ. > * tree-eh.c (replace_goto_queue_1) : Propagate location. > (emit_eh_dispatch): Delete. > (lower_catch): Emit the eh_dispatch manually and set the location of > the first catch statement onto it. > (lower_eh_filter): Emit the eh_dispatch manually and set location. > (lower_eh_dispatch): Propagate location. > * tree-outof-ssa.c (set_location_for_edge): Handle EH edges specially. > (eliminate_build): Likewise. > OK jeff
Re: [patch] Small improvements to coverage info (1/n)
On 7/3/19 4:46 AM, Eric Botcazou wrote: > Hi, > > we have collected a number of small improvements to coverage info generated > by > the compiler over the years. One of the issues is when a new expression or > statement is built without source location information and ends up inheriting > the source location information of the previous instruction in the debug > info, > which can be totally unrelated. > > Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline? > > > 2019-07-03 Eric Botcazou > > * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on > phi nodes if possible. > * tree-scalar-evolution.c (final_value_replacement_loop): Propagate > location info on the newly created statement. > * tree-ssa-loop-manip.c (create_iv): Propagate location info on the > newly created increment if needed. > OK jeff
[Bug fortran/91077] [8/9/10 Regression] Wrong indexing when using a pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91077 Dominique d'Humieres changed: What|Removed |Added Priority|P3 |P4 Status|UNCONFIRMED |NEW Last reconfirmed||2019-07-03 CC||pault at gcc dot gnu.org Known to work||7.4.0 Ever confirmed|0 |1 Known to fail||10.0, 8.3.0, 9.1.0 --- Comment #2 from Dominique d'Humieres --- Likely revision r251949 (r251946 is OK, wrong code at r251980).
Re: [PATCH v2 3/5] or1k: Add mrori option, fix option docs
On Wed, Jul 03, 2019 at 09:49:02AM -0500, Segher Boessenkool wrote:
> On Wed, Jul 03, 2019 at 12:33:49PM +0900, Stafford Horne wrote:
> > @@ -179,11 +183,11 @@
> >[(set (match_operand:SI 0 "register_operand" "=r,r")
> > (rotatert:SI (match_operand:SI 1 "register_operand" "r,r")
> > (match_operand:SI 2 "reg_or_u6_operand" "r,n")))]
> > - "TARGET_ROR"
> > + "TARGET_ROR || TARGET_RORI"
> >"@
> > l.ror\t%0, %1, %2
> > l.rori\t%0, %1, %2"
> > - [(set_attr "insn_support" "*,shftimm")])
> > + [(set_attr "insn_support" "ror,rori")])
>
> Does this work? If you use -mno-ror -mrori? It will then allow generating
> a reg for the second operand, and ICE later on, as far as I can see?
It does seem to work. Why would it produce an internal compiler error?
One thing I have is RegectNegative on mror and mrori, so -mno-ror will not be
allowed and cause an error.
Example:
$ cat ./gcc/testsuite/gcc.target/or1k/ror-4.c
unsigned int rotate6 (unsigned int a) {
return ( a >> 6 ) | ( a << ( 32 - 6 ) );
}
# With rori, direct immediate.
$ or1k-elf-gcc -O2 -c -mrori ./gcc/testsuite/gcc.target/or1k/ror-4.c
$ or1k-elf-objdump -d ror-4.o
ror-4.o: file format elf32-or1k
Disassembly of section .text:
:
0: 44 00 48 00 l.jr r9
4: b9 63 00 c6 l.rori r11,r3,0x6
# With ror, loading immediate to temporary register first.
$ or1k-elf-gcc -O2 -c -mror ./gcc/testsuite/gcc.target/or1k/ror-4.c
$ or1k-elf-objdump -d ror-4.o
ror-4.o: file format elf32-or1k
Disassembly of section .text:
:
0: aa 20 00 06 l.ori r17,r0,0x6
4: 44 00 48 00 l.jr r9
8: e1 63 88 c8 l.ror r11,r3,r17
-Stafford
[Bug lto/91078] [10 regression] All LTO tests FAIL: SIGBUS in lto1 (lto_file_finalize)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91078 Rainer Orth changed: What|Removed |Added Target Milestone|--- |10.0
[Bug lto/91078] New: [10 regression] All LTO tests FAIL: SIGBUS in lto1 (lto_file_finalize)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91078 Bug ID: 91078 Summary: [10 regression] All LTO tests FAIL: SIGBUS in lto1 (lto_file_finalize) Product: gcc Version: 10.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: lto Assignee: unassigned at gcc dot gnu.org Reporter: ro at gcc dot gnu.org CC: marxin at gcc dot gnu.org Target Milestone: --- Host: sparc-sun-solaris2.11 Target: sparc-sun-solaris2.11 Build: sparc-sun-solaris2.11 Between 20190702 (r272944) and 20190703 (r273009), all LTO tests started to FAIL on Solaris/SPARC. It boils down to $ cat /var/tmp//ccqNGG1b 20010124-1.o 20010124-1-lib.o main.o $ lto1 -quiet @/var/tmp//ccqNGG1b -o 20010124-1.s [...] lto1: internal compiler error: Bus Error 0x9d1597 crash_signal /vol/gcc/src/hg/trunk/local/gcc/toplev.c:326 0x3fea74 lto_file_finalize /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2202 0x3fea74 lto_create_files_from_ids /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2225 0x3fea74 lto_file_read /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2279 0x3fea74 read_cgraph_and_symbols(unsigned int, char const**) /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2731 0x3dd10b lto_main() /vol/gcc/src/hg/trunk/local/gcc/lto/lto.c:616 Thread 2 received signal SIGSEGV, Segmentation fault. [Switching to Thread 1 (LWP 1)] lto_file_finalize (file=0x16a6370, file_data=0xfa886050) at /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2202 2202 file_data->lto_section_header = *(const lto_section *)data; (gdb) where #0 lto_file_finalize (file=0x16a6370, file_data=0xfa886050) at /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2202 #1 lto_create_files_from_ids (count=, file_data=0xfa886050, file=0x16a6370) at /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2225 #2 lto_file_read (count=, resolution_file=0x0, file=) at /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2279 #3 read_cgraph_and_symbols (nfiles=, fnames=) at /vol/gcc/src/hg/trunk/local/gcc/lto/lto-common.c:2731 #4 0x003dd10c in lto_main () at /vol/gcc/src/hg/trunk/local/gcc/lto/lto.c:616 #5 0x009d1610 in compile_file () at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:456 #6 0x009d4128 in do_compile () at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:2209 #7 toplev::main (this=0xffbfe4de, argc=, argv=) at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:2344 #8 0x012f11ac in main (argc=27, argv=0xffbfe544) at /vol/gcc/src/hg/trunk/local/gcc/main.c:39 (gdb) p/x data $1 = 0xfb9d8429 gdb showing this as SIGSEGV is a gdb bug. This most likely affects all strict-alignment targets.
[Bug c++/91073] [9/10 Regression] if constexpr no longer works directly with Concepts
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91073 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Known to work||8.3.0 Keywords||rejects-valid Last reconfirmed||2019-07-03 CC||paolo at gcc dot gnu.org Ever confirmed|0 |1 Summary|if constexpr no longer |[9/10 Regression] if |works directly with |constexpr no longer works |Concepts|directly with Concepts Known to fail||10.0, 9.1.0 --- Comment #1 from Jonathan Wakely --- Confirmed as a regression (GCC 8 accepts it, if 'concept HasInit' is replaced with 'concept bool HasInit'). It started to be rejected with r260482: PR c++/84588 * parser.c (cp_parser_maybe_commit_to_declaration, cp_parser_check_condition_declarator): New. (cp_parser_simple_declaration): Use the first above. (cp_parser_condition): Use both the above; enforce [stmt.stmt]/2 about the declarator not specifying a function or an array; improve error-recovery.
[Bug libstdc++/91067] [9/10 Regression] Clang compiler can't link executable if std::filesystem::directory_iterator is encountered
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91067 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #9 from Jonathan Wakely --- Fixed on trunk and for GCC 9.2, thanks for finding and reporting the bug!
[Bug libstdc++/91067] [9/10 Regression] Clang compiler can't link executable if std::filesystem::directory_iterator is encountered
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91067 --- Comment #8 from Jonathan Wakely --- Author: redi Date: Wed Jul 3 21:09:13 2019 New Revision: 273025 URL: https://gcc.gnu.org/viewcvs?rev=273025=gcc=rev Log: PR libstdc++/91067 fix missing exports for filesystem iterators The copy assignment operator for recursive_directory_iterator was not exported despite being needed. The __shared_ptr default constructors are not needed when compiling with GCC but Clang requires them for -O1. PR libstdc++/91067 * acinclude.m4 (libtool_VERSION): Bump to 6:27:0. * configure: Regenerate. * config/abi/pre/gnu.ver (GLIBCXX_3.4.27): Add new version. Export missing symbols. * testsuite/27_io/filesystem/iterators/91067.cc: New test. * testsuite/util/testsuite_abi.cc: Add new symbol version. Added: branches/gcc-9-branch/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc Modified: branches/gcc-9-branch/libstdc++-v3/ChangeLog branches/gcc-9-branch/libstdc++-v3/acinclude.m4 branches/gcc-9-branch/libstdc++-v3/config/abi/pre/gnu.ver branches/gcc-9-branch/libstdc++-v3/configure branches/gcc-9-branch/libstdc++-v3/testsuite/util/testsuite_abi.cc
[Bug libstdc++/91067] [9/10 Regression] Clang compiler can't link executable if std::filesystem::directory_iterator is encountered
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91067 --- Comment #7 from Jonathan Wakely --- Author: redi Date: Wed Jul 3 21:06:25 2019 New Revision: 273023 URL: https://gcc.gnu.org/viewcvs?rev=273023=gcc=rev Log: PR libstdc++/91067 fix missing exports for filesystem iterators The copy assignment operator for recursive_directory_iterator was not exported despite being needed. The __shared_ptr default constructors are not needed when compiling with GCC but Clang requires them for -O1. PR libstdc++/91067 * acinclude.m4 (libtool_VERSION): Bump to 6:27:0. * configure: Regenerate. * config/abi/pre/gnu.ver (GLIBCXX_3.4.27): Add new version. Export missing symbols. * testsuite/27_io/filesystem/iterators/91067.cc: New test. * testsuite/util/testsuite_abi.cc: Add new symbol version. Added: trunk/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/acinclude.m4 trunk/libstdc++-v3/config/abi/pre/gnu.ver trunk/libstdc++-v3/configure trunk/libstdc++-v3/testsuite/util/testsuite_abi.cc
[Bug c++/91064] __is_standard_layout incorrect for a class with multiple bases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91064 --- Comment #4 from Martin Sebor --- Yes, my example in comment #0 is missing the derivation from Q. So the sentence has at most one base class subobject of any given type doesn't mean that a std layout class cannot have multiple base class subobjects, just that none of them can be of a type that's derived, either directly or indirectly, from the same base class. Okay, thanks.
[Bug c++/91076] wrong class-key in mentioned in a diagnostic note
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91076 Martin Sebor changed: What|Removed |Added Keywords||diagnostic Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-07-03 Assignee|unassigned at gcc dot gnu.org |msebor at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Martin Sebor --- I'm testing (what I'm hoping will stay) a simple patch.
Re: [PATCH] S/390: Improve storing asan frame_pc
Segher Boessenkool writes: > On Tue, Jul 02, 2019 at 03:55:56PM +0200, Ilya Leoshkevich wrote: >> > Am 02.07.2019 um 15:39 schrieb Jakub Jelinek : >> > On Tue, Jul 02, 2019 at 03:33:28PM +0200, Ilya Leoshkevich wrote: >> >>> Am 02.07.2019 um 15:19 schrieb Segher Boessenkool >> >>> : >> >>> >> >>> On Tue, Jul 02, 2019 at 08:02:16AM -0500, Segher Boessenkool wrote: >> On Tue, Jul 02, 2019 at 10:51:54AM +0200, Ilya Leoshkevich wrote: >> > +#undef TARGET_INSN_ALIGNMENT >> > +#define TARGET_INSN_ALIGNMENT 16 >> >> There already is FUNCTION_BOUNDARY for something similar, which fits in >> well with STACK_BOUNDARY, PARM_BOUNDARY, many more *_BOUNDARY. I >> realise >> you may prefer a hook, but as long as we aren't getting rid of all the >> other macros, what's the point? >> >>> >> >>> And maybe LABEL_BOUNDARY is bettter for this than INSN_BOUNDARY as well? >> >> >> >> Can’t we just use FUNCTION_BOUNDARY then? >> >> I think .LASANPC is always emitted at the beginning of a function. >> > >> > Isn't e.g. the hotpatch sequence emitted before it? >> >> You are right, with -fpatchable-function-entry it’s moved. >> >> So, I guess I should stick with the current approach. >> I could change TARGET_INSN_ALIGNMENT hook to INSN_BOUNDARY macro if that >> would better match the current design. I would still call it INSN, and >> not LABEL, because LABEL can refer to data. > > On some archs LABEL_BOUNDARY can be bigger than INSN_BOUNDARY (just like > FUNCTION_BOUNDARY can be even bigger, like on 390 :-) ) > > Either will work for your purposes afaics. > > LABEL in RTL is always a CODE_LABEL I think? Maybe CODE_LABEL_BOUNDARY > would make it clearer, it's not like a short name for this is useful > anyway. IIUC the new value is effectively a mandatory/guaranteed minimum value of align_labels/LABEL_ALIGN that applies even in blocks optimized for size. So IMO sticking with *_ALIGNMENT would be better. Thanks, Richard
Re: [PATCH] Fix few build warnings with LLVM toolchain
Jeff Law writes: > On 6/28/19 12:46 PM, Richard Sandiford wrote: >> Segher Boessenkool writes: >>> On Fri, Jun 28, 2019 at 08:55:00AM -0600, Martin Sebor wrote: Jeff reminded me in a code review the other day that GCC does have a guideline for defining POD structs with the keyword "struct" and classes with ctors/dtors using "class": https://gcc.gnu.org/codingconventions.html#Struct_Use I quickly prototyped a warning to see how closely GCC follows this convention. The result shows that out of just under 800 structs and classes defined in GCC sources some 200 use the keyword 'struct' despite having a ctor or dtor, or about 25%. So as is the case with most other conventions, without a tool to help remind us they exist they are unlikely to be followed with enough consistency to be worth putting in place to begin with. >>> >>> The goal is not to have the rules adhered to. The goal is to have >>> a more readable / maintainable / etc. codebase. >> >> IMO it's a shame we don't follow that attitude for changelogs, >> where apparently the number of spaces between the name and the email >> address is of vital importance :-) Too often a new contributor's >> first taste of GCC is a slew of comments about things like that. > :-) Given that I regularly deal with the random one off contributors > and cut-n-paste their author information from their email which is > always one space I find the adherence to two spaces in the ChangeLog > annoying. I still try to fix the, but like all the whitespace stuff, > I'd just assume leave that to commit hooks. There just isn't a lot of > value in having a human looking for whitespace issues. I'd much rather > be spending time on more substantial questions. > > >> >> But surely it's a valid point that we're not following our own >> conventions on the C++ usage. I miss how consistent the codebase >> was in the C days... > Sure, it's a valid point and I fully applaud the effort to add warnings > for things where we can and fix this kind of stuff. > >> >> That can be fixed by changing the conventions if we no longer think >> they're a good idea. But if someone's willing to change the codebase >> to follow (one of) the coventions instead, and willing to add a warning >> to keep things that way, then that sounds like a really good thing. >> Especially when it's a common convention that others might want a >> warning for too. > Likewise -- I'm certainly open to changing conventions where we don't > think they make sense. But I'm not going to unilaterally do that. > > >> >> And I don't think the current state of the struct/class tags is really >> a result of making the codebase more maintainable. I get the feeling >> it just kind-of happened. ISTM we're trying too hard to find a reason >> not to clean this up. > I believe the idea was to distinguish between POD and more complex > types. POD isn't going to do things behind your back, whereas classes > certainly can (and it's often useful, for example RAII constructs). Yeah. And that sounds like a reasonable rule if enforced properly (like Martin says). I'm certainly not objecting to that. :-) But what started this thread was that we currently use inconsistent tags for the same type (struct in one file, class in another). By definition that can't be following the current rules, since a type is either POD or not. And I can't think it was done for readability or maintainability reasons either. It looks completely accidental. What I was objecting to mostly was the pushback against fixing that up and making the tags self-consistent. This has come up before, and the response always seems to be to complain about clang rather than admit that the current state of things isn't great. Thanks, Richard
[Bug libstdc++/90409] std::move[_backward] could be more optimized for deque iterators
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90409 François Dumont changed: What|Removed |Added Status|NEW |ASSIGNED CC||fdumont at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |fdumont at gcc dot gnu.org --- Comment #1 from François Dumont --- Patch awaiting on mailing list: https://gcc.gnu.org/ml/libstdc++/2019-06/msg00097.html
Re: [PATCH] simplify-rtx.c: Change BITSIZE to UNIT_PRECISION in simplification
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index 89a46a9..d74a4ba 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -1504,12 +1504,12 @@ simplify_unary_operation_1 (enum rtx_code code,
> machine_mode mode, rtx op)
> && CONST_INT_P (XEXP (op, 1))
> && XEXP (XEXP (op, 0), 1) == XEXP (op, 1)
> && (op_mode = as_a (GET_MODE (op)),
> - GET_MODE_BITSIZE (op_mode) > INTVAL (XEXP (op, 1
> + GET_MODE_UNIT_PRECISION (op_mode) > INTVAL (XEXP (op, 1
> {
> scalar_int_mode tmode;
> - gcc_assert (GET_MODE_BITSIZE (int_mode)
> - > GET_MODE_BITSIZE (op_mode));
> - if (int_mode_for_size (GET_MODE_BITSIZE (op_mode)
> + gcc_assert (GET_MODE_UNIT_PRECISION (int_mode)
> + > GET_MODE_UNIT_PRECISION (op_mode));
> + if (int_mode_for_size (GET_MODE_UNIT_PRECISION (op_mode)
>- INTVAL (XEXP (op, 1)), 1).exists ())
> {
> rtx inner =
I think these should be GET_MODE_PRECISION rather than
GET_MODE_UNIT_PRECISION. They do the same thing in this context,
but it seems odd to use GET_MODE_UNIT_... when we're specifically
dealing with scalars.
Looks good otherwise (and small enough not to need a copyright assignment,
in case you haven't signed one).
Thanks,
Richard
[Bug c++/91064] __is_standard_layout incorrect for a class with multiple bases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91064 Marek Polacek changed: What|Removed |Added CC||mpolacek at gcc dot gnu.org --- Comment #3 from Marek Polacek --- Note the testcase here is actually wrong; U *is* a standard-layout class. The standard has a different test.
[Bug fortran/91077] [8/9/10 Regression] Wrong indexing when using a pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91077 Thomas Koenig changed: What|Removed |Added Keywords||wrong-code --- Comment #1 from Thomas Koenig --- Note that this works when the variable std is changed to an allocatable.
[Bug fortran/91077] New: [8/9/10 Regression] Wrong indexing when using a pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91077
Bug ID: 91077
Summary: [8/9/10 Regression] Wrong indexing when using a
pointer
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: fortran
Assignee: unassigned at gcc dot gnu.org
Reporter: tkoenig at gcc dot gnu.org
Target Milestone: ---
From a c.l.f. post by ygalkl...@gmail.com (slightly modified):
program test
implicit none
integer, parameter :: length = 9
real(8), dimension(2) :: a, b
integer :: i
type point
real(8) :: x
end type point
type stored
type(point), dimension(:), allocatable :: np
end type stored
type(stored), dimension(:), pointer :: std =>null()
allocate(std(1))
allocate(std(1)%np(length))
std(1)%np(1)%x = 0.3d0
std(1)%np(2)%x = 0.3555d0
std(1)%np(3)%x = 0.26782d0
std(1)%np(4)%x = 0d0
std(1)%np(5)%x = 1.555d0
std(1)%np(6)%x = 7.3d0
std(1)%np(7)%x = 7.8d0
std(1)%np(8)%x = 6.3d0
std(1)%np(9)%x = 5.5d0
do i = 1, 2
write(*, "('std(1)%np(',i1,')%x = ',1e22.14)") i, std(1)%np(i)%x
end do
do i = 1, 2
write(*, "('std(1)%np(1:',i1,') = ',9e22.14)") i, std(1)%np(1:i)%x
end do
a = std(1)%np(1:2)%x
b = [std(1)%np(1)%x, std(1)%np(2)%x]
print *,a
print *,b
if (norm2(a - b) .gt. 1d-3) then
write(*,*) 'failure'
else
write(*, *) 'success'
end if
end program test
yields
std(1)%np(1)%x = 0.30E+00
std(1)%np(2)%x = 0.355500E+00
std(1)%np(1:1) = 0.30E+00
std(1)%np(1:2) = 0.30E+00 0.55E+01
0.25.5000
0.2 0.35548
failure
with gcc-8, gcc-9 and current trunk.
Something is rotten in the state of Denmark here.
[Bug fortran/91077] [8/9/10 Regression] Wrong indexing when using a pointer
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91077 Thomas Koenig changed: What|Removed |Added Target Milestone|--- |8.4
[Bug c++/91076] New: wrong class-key in mentioned in a diagnostic note
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91076
Bug ID: 91076
Summary: wrong class-key in mentioned in a diagnostic note
Product: gcc
Version: 9.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: msebor at gcc dot gnu.org
Target Milestone: ---
When a type defined with one class-key (such as "class") contains a member such
as a pointer to the same type that uses a different class-key (such as
"struct") the C++ front-end replaces the original class-key associated with the
type with the one used in the member. This causes problems in GCC code that
examines the class-key by using CLASSTYPE_DECLARED_CLASS (t) or by calling
class_key_or_enum_as_string (). A test case illustrating this bug is as
follows:
$ cat a.C && gcc -S -Wall -Wextra a.C
class A // uses CLASS
{
public:
struct A *p; // uses STRUCT
}
[[maybe_unused]]; // misleading warning refers to STRUCT
a.C:6:1: warning: attribute ignored in declaration of ‘struct A’ [-Wattributes]
6 | [[maybe_unused]]; // misleading warning refers to STRUCT
| ^
a.C:6:1: note: attribute for ‘struct A’ must follow the ‘struct’ keyword
[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
--- Comment #20 from Evgeniy Dushistov ---
Also if add one line to code `printf("test\n");`
```
struct FooDeleter {
void operator()(FooOpaque *p) {
printf("test\n");
Foo_free(p);
}
};
```
gcc don't report any warning,
and valgrind also can not find any errors.
Re: [PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end
Dennis Zhang writes: > Hi Richard, Thanks for the tips. > > The special exceptions according to TARGET_SECONDARY_RELOAD hook are > revised. Some related patterns still need constraints in order to work > in an expected way in the TARGET_SECONDARY_RELOAD function. > > The updated patch is tested for targets: aarch64_be-linux-gnu, > aarch64_be-none-linux-gnu, aarch64-linux-gnu, and > aarch64-none-linux-gnu. It survives in testsuite regression. > > gcc/ChangeLog: > > 2019-07-03 Dennis Zhang > > * config/aarch64/aarch64.md: Remove redundant constraints from > define_expand but keep some patterns untouched if they are > specially selected by TARGET_SECONDARY_RELOAD hook. > * config/aarch64/aarch64-sve.md: Likewise. > * config/aarch64/atomics.md: Remove redundant constraints from > define_expand. > * config/aarch64/aarch64-simd.md: Likewise. Thanks, applied as r273021. Richard
[Bug c++/80635] std::optional and bogus -Wmaybe-uninitialized warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635
Evgeniy Dushistov changed:
What|Removed |Added
CC||dushistov at mail dot ru
--- Comment #19 from Evgeniy Dushistov ---
I saw problem similar `std::optional` may be unitialized case as desribed here
with boost::variant/boost::optional (c++11) and std::variant/std::optional
(c++17),
but in my case I have not only gcc warning, but valgrind also reports problem,
is it the same problem or gcc code generation bug?
```
//Foo.cpp
#include "Foo.hpp"
struct FooOpaque {};
FooOpaque *Foo_new() {
auto p = new FooOpaque;
return p;
}
void Foo_free(FooOpaque *p) { delete p; }
std::variant, std::string> f_res_opt(int var) {
switch (var) {
case 0:
return {std::optional{Foo{Foo_new()}}};
case 1:
return {std::optional{}};
case 2:
return {std::string{}};
default:
std::abort();
}
}
```
```
//Foo.hpp
#include
#include
#include
struct FooOpaque;
FooOpaque *Foo_new();
void Foo_free(FooOpaque *);
struct FooDeleter {
void operator()(FooOpaque *p) { Foo_free(p); }
};
using Foo = std::unique_ptr;
std::variant, std::string> f_res_opt(int var);
```
```
//main.cpp
#include "Foo.hpp"
int main() {
auto res1 = f_res_opt(0);
auto res1_ok = std::get>(std::move(res1));
printf("step 2\n");
auto res2 = f_res_opt(1);
auto res2_ok = std::get>(std::move(res2));
printf("step 3\n");
auto res3 = f_res_opt(2);
auto res3_ok = std::get(std::move(res3));
}
```
gcc reports:
```
g++ -ggdb -Ofast -Wall -Wextra -std=c++17 -pedantic main.cpp Foo.cpp
In file included from main.cpp:1:
Foo.hpp: In function 'int main()':
Foo.hpp:10:43: warning: 'res2_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' may be used uninitialized in this function
[-Wmaybe-uninitialized]
10 | void operator()(FooOpaque *p) { Foo_free(p); }
| ^~~
main.cpp:12:8: note: 'res2_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' was declared here
12 | auto res2_ok = std::get>(std::move(res2));
|^~~
In file included from main.cpp:1:
Foo.hpp:10:43: warning: 'res1_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' may be used uninitialized in this function
[-Wmaybe-uninitialized]
10 | void operator()(FooOpaque *p) { Foo_free(p); }
| ^~~
main.cpp:6:8: note: 'res1_ok.std::_Head_base<0, FooOpaque*,
false>::_M_head_impl' was declared here
6 | auto res1_ok = std::get>(std::move(res1));
|^~~
```
but valgrind also reports:
valgrind -v ./a.out
```
==7858== Conditional jump or move depends on uninitialised value(s)
==7858==at 0x109374: ~unique_ptr (unique_ptr.h:288)
==7858==by 0x109374: _M_destroy (optional:257)
==7858==by 0x109374: _M_reset (optional:277)
==7858==by 0x109374: ~_Optional_payload (optional:398)
==7858==by 0x109374: ~_Optional_base (optional:471)
==7858==by 0x109374: ~optional (optional:656)
==7858==by 0x109374: main (main.cpp:12)
```
gcc 9.1.0 and valgrind 3.15.0.GIT
[Bug libfortran/91030] Poor performance of I/O -fconvert=big-endian
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91030 --- Comment #26 from Thomas Koenig --- Jerry, you are working on a Linux box, right? What does stat -f -c %b . tell you?
[Bug target/91050] -mdejagnu-cpu= does not affect the -m assembler option
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91050 --- Comment #6 from Peter Bergner --- (In reply to Peter Bergner from comment #5) > Shouldn't we check whether the new .machine is different than the > currently active cpu value before reloading the opcode table? I don't think > it would be too hard to do and would solve stupid user tricks like: Actually, gas' ppc_machine() already ignores .machine directives if it matches the currently active cpu, so no code needed. That means we'll never reload the opcode table unless we really have to.
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Wed, Jul 03, 2019 at 02:36:11PM -0400, Michael Meissner wrote: > On Wed, Jul 03, 2019 at 12:55:41PM -0500, Segher Boessenkool wrote: > > I don't think this is a good idea. You can set "cost" directly, if that > > is the only thing you need this for? > > The trouble is the cost is currently a factor based on type + the cost from > the > cost structure. It really would be hard to set it to a single value in the > insns without having to have complex means for setting the machine dependent > costs. If the numeric RTL attributes could set the value from a function, it > would be simpler, but that isn't currently supported. (set (attr "cost") (symbol_ref "any C expression you want here")) It is supported. The syntax is a bit weird, sure :-) It may well be that some helper attribute can help here, but just num_insns isn't it. Segher
Re: [OpenRISC] [PATCH v2 4/5] or1k: Initial support for FPU
On 7/3/19 5:43 PM, Segher Boessenkool wrote:
>> @@ -212,6 +214,7 @@ enum reg_class
>> #define REG_CLASS_CONTENTS \
>> { { 0x, 0x }, \
>>{ SIBCALL_REGS_MASK, 0 }, \
>> + { 0x7efe, 0x }, \
>
> Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
> in GCC register numbers, 0, 8, and 31? You probably should mention r8
> somewhere (it's because it is the last arg, this avoid problems, I guess?),
> and the 30/31 thing is confused some way. Maybe it is all just that one
> documentation line :-)
... and if r8 is excluded because of arguments, I suspect that this is the
wrong fix, as there's nothing inherently wrong with r7:r8 or r8:r9 as a pair,
at least that I can see.
Perhaps function_arg and/or function_arg_advance is the right place for a fix?
The calling convention says that 64-bit arguments are not split across
registers+stack, so you already shouldn't have seen (r8, [sp+0]) as a pair.
r~
[Darwin] Revise pie,no-pie and rdynamic driver specs.
Processing these in the driver self-specs and pushing the corresponding Xlinker
lines
seemed like a good idea (nominally the Right Place to process driver specs).
However,
it has the effect that the driver then supposes that there are linker inputs,
and causes a
link line to be created even if it is not needed (of course, only if a linker
option is given,
but still not desirable).
The solution is to place these drive specs into the link spec and claim them at
the end
of that. We can’t filter them at a lower level since RDYNAMIC, at least, is
relevant to
kernel and kernel module code.
Tested on i686-darwin9, x86-64-darwin16, x86-64-darwin18
applied to mainline
thanks
Iain
2019-07-03 Iain Sandoe
* config/darwin.h (DRIVER_SELF_SPECS): Remove the linker cases.
(RDYNAMIC): Rename to, DARWIN_RDYNAMIC.
(DARWIN_PIE_SPEC, DARWIN_NOPIE_SPEC): Adjust to remove the Xlinker
clauses.
(LINK_COMMAND_SPEC_A): Add DARWIN_RDYNAMIC, DARWIN_PIE_SPEC and
DARWIN_NOPIE_SPEC.
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 72215ce..e17bc64 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -124,30 +124,25 @@ extern GTY(()) int darwin_ms_struct;
"%{fapple-kext|mkernel:-static}",\
"%{shared:-Zdynamiclib} %= 10.5 mmacosx-version-min= -Xlinker) \
- %:version-compare(>= 10.5 mmacosx-version-min= -pie) }} %= 10.5 mmacosx-version-min= -pie) }} "
#define DARWIN_NOPIE_SPEC \
"%{no-pie|fno-pie|fno-PIE: \
- %:version-compare(>= 10.7 mmacosx-version-min= -Xlinker ) \
- %:version-compare(>= 10.7 mmacosx-version-min= -no_pie) } %= 10.7 mmacosx-version-min= -no_pie) }"
#define DARWIN_CC1_SPEC
\
"%{findirect-virtual-calls: -fapple-kext} %
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Wed, Jul 03, 2019 at 01:06:27PM -0400, Michael Meissner wrote: > On Mon, Jul 01, 2019 at 04:27:05PM -0500, Segher Boessenkool wrote: > > > @@ -7385,8 +7385,8 @@ (define_insn "*mov_softfloat" > > > *, *, *, *") > > > > > > (set_attr "length" > > > - "4, 4, 4, 4, 4, 4, > > > - 4, 4, 8, 4")]) > > > + "*, *, *, *, *, *, > > > + *, *, 8, *")]) > > > > [ That last line should start with a tab as well. ] > > Ok. > > > The entry before the 8 is split as well. Maybe that should be "4", to > > stand out? I don't know what works better; your choice. > > Though the "G" constraint specifically says for SFmode it is a single > instruction. Sure. But that doesn't mean much, does it? You cannot see the actual split insns here, and that the constraint is for "Constant that can be copied into GPR with two insns for DF/DD and one for SF/SD." is pretty weak :-) The default is one insn, that's what * does. You can still use 4 though when that is beneficial, to make things a tiny bit clearer maybe. > > > @@ -7696,8 +7696,8 @@ (define_insn "*mov_softfloat64" > > > *, *, *") > > > > > > (set_attr "length" > > > -"4, 4, 4, 4, 4, 8, > > > - 12, 16, 4")]) > > > +"*, *, *, *, *, 8, > > > + 12, 16, *")]) > > > > Same for the last entry here. > > Well technically that alternative will never fire (destination is "*h" and > source is "0"), and a nop is emitted. If it can never fire we should remove it. But it *can* fire, or I cannot prove it cannot anyway. Can you? This would be a lovely cleanup if we could make it :-/ > I do wish we could never ever load > floating point into SPR registers. I've tried in the reload days to eliminate > it, but there was always some abort if it got eliminated. It should never load anything into CTR, LR, or VRSAVE, period. :-) Maybe we should use those as fixed registers, not allocatable as they are now. Segher
[Darwin.comitted] Some TLC for older Darwin versions.
The command lines and build options for some of the crts for the older PPC
Darwin versions had bit-rotted somewhat. This adjusts the build criteria for
the
crts to avoid newer ld64 versions warnings about mismatches in build and object
versions.
Added some more comments so that it's documented why the specs are as they
are.
Tested across a range of systems, although this has no (intended) changes for
the X86 port. We can test generation of code for older Darwin on
powerpc-darwin9
but self-hosting on <= Darwin8 needs some more work.
applied to mainline
Thanks
Iain
2019-07-03 Iain Sandoe
gcc/
* config/darwin.h (REAL_LIBGCC_SPEC): Adjust for earlier Darwin.
(STARTFILE_SPEC): Split crt3 into a separate spec.
(DARWIN_EXTRA_SPECS): Add crt2 and crt3 spec.
(DARWIN_CRT2_SPEC): New.
(DARWIN_CRT3_SPEC): New.
(MIN_LD64_OMIT_STUBS): Revise to 62.1.
* config/rs6000/darwin.h (DARWIN_CRT2_SPEC): Revise conditions.
(DARWIN_CRT3_SPEC): New.
libgcc/
2019-07-03 Iain Sandoe
* config.host (powerpc-*-darwin*,powerpc64-*-darwin*): Revise crt
list.
* config/rs6000/t-darwin: Build crt3_2 for older systems. Revise
mmacosx-version-min for crts to run across all system versions.
* config/rs6000/t-darwin64 (LIB2ADD): Remove.
* config/t-darwin: Revise mmacosx-version-min for crts to run across
system versions >= 10.4.
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index ae324f1..72215ce 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -189,8 +189,15 @@ extern GTY(()) int darwin_ms_struct;
#define DARWIN_NOCOMPACT_UNWIND \
" %:version-compare(>= 10.6 mmacosx-version-min= -no_compact_unwind) "
-/* This is mostly a clone of the standard LINK_COMMAND_SPEC, plus
- precomp, libtool, and fat build additions.
+/* In Darwin linker specs we can put -lcrt0.o and ld will search the library
+ path for crt0.o or -lcrtx.a and it will search for for libcrtx.a. As for
+ other ports, we can also put xxx.{o,a}%s and get the appropriate complete
+ startfile absolute directory. This latter point is important when we want
+ to override ld's rule of .dylib being found ahead of .a and the user wants
+ the convenience library to be linked. */
+
+/* The LINK_COMMAND spec is mostly a clone of the standard LINK_COMMAND_SPEC,
+ plus precomp, libtool, and fat build additions.
In general, random Darwin linker flags should go into LINK_SPEC
instead of LINK_COMMAND_SPEC. The command spec is better for
@@ -352,43 +359,42 @@ extern GTY(()) int darwin_ms_struct;
/* Support -mmacosx-version-min by supplying different (stub) libgcc_s.dylib
libraries to link against, and by not linking against libgcc_s on
- earlier-than-10.3.9.
+ earlier-than-10.3.9. If we need exceptions, prior to 10.3.9, then we have
+ to link the static eh lib, since there's no shared version on the system.
+
+ Note that by default, except as above, -lgcc_eh is not linked against.
+ This is because,in general, we need to unwind through system libraries that
+ are linked with the shared unwinder in libunwind (or libgcc_s for 10.4/5).
- Note that by default, -lgcc_eh is not linked against! This is
- because in a future version of Darwin the EH frame information may
- be in a new format, or the fallback routine might be changed; if
- you want to explicitly link against the static version of those
- routines, because you know you don't need to unwind through system
- libraries, you need to explicitly say -static-libgcc.
+ The static version of the current libgcc unwinder (which differs from the
+ implementation in libunwind.dylib on systems Darwin10 [10.6]+) can be used
+ by specifying -static-libgcc.
- If it is linked against, it has to be before -lgcc, because it may
+ If libgcc_eh is linked against, it has to be before -lgcc, because it might
need symbols from -lgcc. */
+
#undef REAL_LIBGCC_SPEC
#define REAL_LIBGCC_SPEC \
"%{static-libgcc|static: -lgcc_eh -lgcc; \
- shared-libgcc|fexceptions|fgnu-runtime: \
- %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_s.10.4) \
+ shared-libgcc|fexceptions|fobjc-exceptions|fgnu-runtime:\
+ %:version-compare(!> 10.3.9 mmacosx-version-min= -lgcc_eh) \
+ %:version-compare(>< 10.3.9 10.5 mmacosx-version-min= -lgcc_s.10.4) \
%:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5) \
- %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_ext.10.4) \
+ %:version-compare(>< 10.3.9 10.5 mmacosx-version-min= -lgcc_ext.10.4) \
%:version-compare(>= 10.5 mmacosx-version-min= -lgcc_ext.10.5) \
-lgcc ;\
:%:version-compare(>< 10.3.9 10.5
[Bug tree-optimization/91074] [10 regression] c-c++-common/gomp/scan-3.c fails with ICE starting with r272958
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91074 --- Comment #3 from seurer at gcc dot gnu.org --- I am running a check on the test and ... looks like it works! Problem test cases fixed and no other new issues.
[Bug c++/80785] warning for static definitions inside extern "C"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80785 Andrew Pinski changed: What|Removed |Added Keywords||diagnostic Severity|normal |enhancement
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Wed, Jul 03, 2019 at 12:55:41PM -0500, Segher Boessenkool wrote:
> On Wed, Jul 03, 2019 at 12:50:37PM -0400, Michael Meissner wrote:
> > On Tue, Jul 02, 2019 at 07:09:20PM -0500, Segher Boessenkool wrote:
> > > We'll need to update our insn_cost for prefixed, sure, it currently does
> > > int n = get_attr_length (insn) / 4;
> > > to figure out how many machine instructions a pattern is, and then uses
> > > "n" differently for the different types of insn. We'll need to refine
> > > this a bit for prefixed instructions.
> >
> > Yes, I have some plans with this regard. In particular, I will be
> > introducing
> > a "num_insns" RTL attribute, that if set is the number of instructions that
> > will be emitted.
>
> I don't think this is a good idea. You can set "cost" directly, if that
> is the only thing you need this for?
The trouble is the cost is currently a factor based on type + the cost from the
cost structure. It really would be hard to set it to a single value in the
insns without having to have complex means for setting the machine dependent
costs. If the numeric RTL attributes could set the value from a function, it
would be simpler, but that isn't currently supported.
Here is my current version of rs6000_insn_cost. At the moment, I'm only
setting the "num_insns" in a few places, so the default would normally kick in.
/* How many real instructions are generated for this insn? This is slightly
different from the length attribute, in that the length attribute counts the
number of bytes. With prefixed instructions, we don't want to count a
prefixed instruction (length 12 bytes including possible NOP) as taking 3
instructions, but just one. */
static int
rs6000_num_insns (rtx_insn *insn)
{
/* If the insn provides an override, use it. */
int num = get_attr_num_insns (insn);
if (!num)
{
/* Try to figure it out based on the length and whether there are
prefixed instructions. While prefixed instructions are only 8 bytes,
we have to use 12 as the size of the first prefixed instruction in
case the instruction needs to be aligned. Back to back prefixed
instructions would only take 20 bytes, since it is guaranteed that one
of the prefixed instructions does not need the alignment. */
int length = get_attr_length (insn);
if (length >= 12 && TARGET_PREFIXED_ADDR
&& get_attr_prefixed (insn) == PREFIXED_YES)
{
/* Single prefixed instruction. */
if (length == 12)
return 1;
/* A normal instruction and a prefixed instruction (16) or two back
to back prefixed instructions (20). */
if (length == 16 || length == 20)
return 2;
/* Guess for larger instruction sizes. */
num = 2 + (length - 20) / 4;
}
else
num = length / 4;
}
return num;
}
rs6000_insn_cost (rtx_insn *insn, bool speed)
{
int cost;
if (recog_memoized (insn) < 0)
return 0;
if (!speed)
return get_attr_length (insn);
cost = get_attr_cost (insn);
if (cost > 0)
return cost;
int n = rs6000_num_insns (insn);
enum attr_type type = get_attr_type (insn);
switch (type)
{
case TYPE_LOAD:
case TYPE_FPLOAD:
case TYPE_VECLOAD:
cost = COSTS_N_INSNS (n + 1);
break;
case TYPE_MUL:
switch (get_attr_size (insn))
{
case SIZE_8:
cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const9;
break;
case SIZE_16:
cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi_const;
break;
case SIZE_32:
cost = COSTS_N_INSNS (n - 1) + rs6000_cost->mulsi;
break;
case SIZE_64:
cost = COSTS_N_INSNS (n - 1) + rs6000_cost->muldi;
break;
default:
gcc_unreachable ();
}
break;
// ...
> > What I was talking about is I've found some insns that don't set the length,
> > and are split. Using the insn cost mechanism will mean that these
> > instructions
> > will be thought of being cheaper than they actually are.
>
> Yes. Please split RTL insns as early as possible. It also matters for
> scheduling, and it prevents exponential explosion of the number of
> patterns you need. Only sometimes do you need to split late, usually
> because RA can put some registers in memory and you want to handle that
> optimally, or things depend on what exact register you were allocated
> (cr0 vs. crN for example, but could be GPR vs. VSR).
Generally most of the places I've been modifying with splits need to be handled
after register allocation.
> And again, you can set cost directly; length alone is not usually enough
> for determining the cost of split patterns. But you do need length for
> accurate costs with -Os, hrm.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1
Re: [patch, c++ openmp] Improve diagnostics for unmappable types
On 7/1/19 7:16 AM, Andrew Stubbs wrote: On 28/06/2019 17:21, Jason Merrill wrote: + inform ((decl ? DECL_SOURCE_LOCATION (decl) : UNKNOWN_LOCATION), + "incomplete types are not mappable"); It's better to use input_location as a fallback; essentially no diagnostics should use UNKNOWN_LOCATION. And let's print the type with %qT. + if (notes) + result = false; + else + return false; Returning early when !notes doesn't seem worth the extra lines of code. How is this version? OK, thanks. Jason
Re: C++ PATCH to detect narrowing in case values (PR c++/90805)
On 7/3/19 10:13 AM, Marek Polacek wrote: On Sat, Jun 22, 2019 at 11:28:36PM -0400, Jason Merrill wrote: On 6/13/19 5:03 PM, Marek Polacek wrote: Case values are converted constant expressions, so narrowing conversion is not permitted. This patch adds detecting narrowing to case_conversion; it's a handy spot because we have both the value and the (adjusted) type of the condition. Is there a reason not to use build_converted_constant_expr? The function comment says "Note that if TYPE and VALUE are already integral we don't really do the conversion because the language-independent warning/optimization code will work better that way" so I avoided adding any conversions. What I could do is to, instead of calling check_narrowing, call build_converted_constant_expr (type, value, tf_warning_or_error); and not use its result, but I'm not sure what the benefits would be. I was thinking about using it instead of the current perform_implicit_conversion_flags, so we get the somewhat different constraints on the conversion. And then it becomes simpler to use it unconditionally but throw the result away in the easy case. Jason
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Wed, Jul 03, 2019 at 12:50:37PM -0400, Michael Meissner wrote: > On Tue, Jul 02, 2019 at 07:09:20PM -0500, Segher Boessenkool wrote: > > We'll need to update our insn_cost for prefixed, sure, it currently does > > int n = get_attr_length (insn) / 4; > > to figure out how many machine instructions a pattern is, and then uses > > "n" differently for the different types of insn. We'll need to refine > > this a bit for prefixed instructions. > > Yes, I have some plans with this regard. In particular, I will be introducing > a "num_insns" RTL attribute, that if set is the number of instructions that > will be emitted. I don't think this is a good idea. You can set "cost" directly, if that is the only thing you need this for? > What I was talking about is I've found some insns that don't set the length, > and are split. Using the insn cost mechanism will mean that these > instructions > will be thought of being cheaper than they actually are. Yes. Please split RTL insns as early as possible. It also matters for scheduling, and it prevents exponential explosion of the number of patterns you need. Only sometimes do you need to split late, usually because RA can put some registers in memory and you want to handle that optimally, or things depend on what exact register you were allocated (cr0 vs. crN for example, but could be GPR vs. VSR). And again, you can set cost directly; length alone is not usually enough for determining the cost of split patterns. But you do need length for accurate costs with -Os, hrm. Segher
XOOM Deactivation Request
Dear Customer, Your recent request to update your email (gcc@gcc.gnu.org) associated with your account with Xoom will be processed shortly. If this request was not made by you, you are required to use the button below to stop the request! Cancel Request Why it is Important Helps us protect your security and privacy of our customers. About Us | User Agreement | Privacy Policy | Security | Site Map Copyright © 2018 PayPal.
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Mon, Jul 01, 2019 at 04:27:05PM -0500, Segher Boessenkool wrote: > Hi Mike, > > Sorry I missed this patch :-( > > On Thu, Jun 27, 2019 at 04:18:00PM -0400, Michael Meissner wrote: > > As we discussed off-line earlier, I changed all of the "4" lengths to be > > "*", > > even for instruction alternatives that would not be subject to being > > changed to > > be a prefixed instruction (i.e. the sign extend instruction "extsw"'s > > length is > > set to "*", even though it would not be a prefixed instruction). > > "*" means "use the default for this attribute", which often is nicer to > see than "4". For example, "8" stands out more in a sea of "*"s. > > Usually "*" is the insns defined as "normal" alternatives, and "8" or > "12" etc. are split. > > > @@ -7231,7 +7231,7 @@ (define_insn "*movcc_internal1" > >(const_string "mtjmpr") > >(const_string "load") > >(const_string "store")]) > > - (set_attr "length" "4,4,12,4,4,8,4,4,4,4,4,4")]) > > + (set_attr "length" "*,*,12,*,*,8,*,*,*,*,*,*")]) > > In this case, the "12" and "8" are actually defined as one insn in the > template, with some "\;". Luckily there aren't many of those. > > > @@ -7385,8 +7385,8 @@ (define_insn "*mov_softfloat" > > *, *, *, *") > > > > (set_attr "length" > > - "4, 4, 4, 4, 4, 4, > > - 4, 4, 8, 4")]) > > + "*, *, *, *, *, *, > > + *, *, 8, *")]) > > [ That last line should start with a tab as well. ] Ok. > The entry before the 8 is split as well. Maybe that should be "4", to > stand out? I don't know what works better; your choice. Though the "G" constraint specifically says for SFmode it is a single instruction. > > @@ -7696,8 +7696,8 @@ (define_insn "*mov_softfloat64" > > *, *, *") > > > > (set_attr "length" > > -"4, 4, 4, 4, 4, 8, > > - 12, 16, 4")]) > > +"*, *, *, *, *, 8, > > + 12, 16, *")]) > > Same for the last entry here. Well technically that alternative will never fire (destination is "*h" and source is "0"), and a nop is emitted. I do wish we could never ever load floating point into SPR registers. I've tried in the reload days to eliminate it, but there was always some abort if it got eliminated. > > > @@ -8760,10 +8760,10 @@ (define_insn "*movdi_internal32" > >vecsimple") > > (set_attr "size" "64") > > (set_attr "length" > > - "8, 8, 8, 4, 4, 4, > > - 16,4, 4, 4, 4, 4, > > - 4, 4, 4, 4, 4, 8, > > - 4") > > + "8, 8, 8, *, *, *, > > + 16,*, *, *, *, *, > > + *, *, *, *, *, 8, > > + *") > > And the last here. Well it will be split into a single VSPLTISW instruction. > > > @@ -8853,11 +8853,11 @@ (define_insn "*movdi_internal64" > > mftgpr,mffgpr") > > (set_attr "size" "64") > > (set_attr "length" > > - "4, 4, 4, 4, 4, 20, > > -4, 4, 4, 4, 4, 4, > > -4, 4, 4, 4, 4, 4, > > -4, 8, 4, 4, 4, 4, > > -4, 4") > > + "*, *, *, *, *, 20, > > +*, *, *, *, *, *, > > +*, *, *, *, *, *, > > +*, 8, *, *, *, *, > > +*, *") > > And two of the entries here. Though the second split becomes a single VSPLTISW instruction once again. > > @@ -1150,9 +1150,9 @@ (define_insn "vsx_mov_64bit" > > store, load, store, *, vecsimple, > > vecsimple, > > vecsimple, *, *, vecstore, vecload") > > (set_attr "length" > > - "4, 4, 4, 8, 4, 8, > > -8, 8, 8, 8, 4, 4, > > -4, 20,8, 4, 4") > > + "*, *, *, 8, *, 8, > > +8, 8, 8, 8, *, *, > > +*, 20,8, *, *") > > No idea which ones are split here :-) None of the * and all other would > be nice,
Re: [PATCH] Prepare for prefixed instructions on PowerPC
On Tue, Jul 02, 2019 at 07:09:20PM -0500, Segher Boessenkool wrote: > On Tue, Jul 02, 2019 at 07:36:21PM -0400, Michael Meissner wrote: > > On Mon, Jul 01, 2019 at 04:27:05PM -0500, Segher Boessenkool wrote: > > > The entry before the 8 is split as well. Maybe that should be "4", to > > > stand out? I don't know what works better; your choice. > > > > I'll look into it. Note, the length is used in two places. One at the end > > to > > generate the appropriate branches, but the other is in rs6000_insn_cost > > inside > > rs6000.c. > > We'll need to update our insn_cost for prefixed, sure, it currently does > int n = get_attr_length (insn) / 4; > to figure out how many machine instructions a pattern is, and then uses > "n" differently for the different types of insn. We'll need to refine > this a bit for prefixed instructions. Yes, I have some plans with this regard. In particular, I will be introducing a "num_insns" RTL attribute, that if set is the number of instructions that will be emitted. If "num_insns" is not set, then it will use the length and look at the "prefixed" attribute. > > This occurs before the final passes, so it is important that even > > though the insn will be split, that the length is still set. However, > > things > > are rather inconsistant, in that sometimes the length field is accurate, and > > sometimes not. > > It *has* to be correct; it is allowed to be pessimistic though. This > is used to determine if a B-form branch can reach, for example. You get > ICEs if it isn't correct. Only on very few testcases, of course :-( To be clear. Yes it has to be correct when the label handling is done. What I was talking about is I've found some insns that don't set the length, and are split. Using the insn cost mechanism will mean that these instructions will be thought of being cheaper than they actually are. > > I'm finding that the rs6000_insn_cost issue really muddies things up, > > particularly if a vector load/store insn is done in gpr registers, where it > > can > > be 4 insns. > > Yeah, it doesn't handle vectors specially *at all* currently, not even > for FP in vectors. It is pretty much just a switch over the "type", and > for everything vector it gets to > default: > cost = COSTS_N_INSNS (n); > (with the above "n") which is a pretty coarse approximation to the truth ;-) > > You could try #undef'ing TARGET_INSN_COST (in rs6000.c) for now, and hope > that rs6000_rtx_costs does better for what you need right now. In the end > it will have to be fixed properly, insn_cost is quite important. > > > Segher > -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
[COMMITTED] Fix store merging tests on Arm
Fix the failing store merging test on Arm. Aligning variables fixes a
few cases, otherwise disable the test on Arm. All store merging tests
now pass. Committed as obvious.
ChangeLog:
2019-07-03 Wilco Dijkstra
testsuite/
* gcc.dg/store_merging_27.c: Fix test for Arm.
* gcc.dg/store_merging_28.c: Likewise.
* gcc.dg/store_merging_29.c: Likewise.
* gcc.dg/tree-ssa/dump-6.c: Likewise.
--
diff --git a/gcc/testsuite/gcc.dg/store_merging_27.c
b/gcc/testsuite/gcc.dg/store_merging_27.c
index
a691368ad3f8bad263569378421e699a9c9e8880..d3cd117bdc63442bc0ea4377365f2f83f93af143
100644
--- a/gcc/testsuite/gcc.dg/store_merging_27.c
+++ b/gcc/testsuite/gcc.dg/store_merging_27.c
@@ -18,7 +18,7 @@ bar (struct S *x)
int
main ()
{
- struct S s = {};
+ __attribute__((aligned(8))) struct S s = {};
s.buf[1] = 1;
s.buf[3] = 2;
bar ();
diff --git a/gcc/testsuite/gcc.dg/store_merging_28.c
b/gcc/testsuite/gcc.dg/store_merging_28.c
index
95a082288d73a904d72090fd1cc22d3f027ad23c..2d6cffc46945596566aa4e529b0935df6c33b786
100644
--- a/gcc/testsuite/gcc.dg/store_merging_28.c
+++ b/gcc/testsuite/gcc.dg/store_merging_28.c
@@ -3,7 +3,7 @@
/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-store-merging-details" } */
/* { dg-final { scan-tree-dump-times "New sequence of \[24] stores to replace
old one of 16 stores" 8 "store-merging" { target { i?86-*-* x86_64-*-* } } } }
*/
-/* { dg-final { scan-tree-dump-times "New sequence of \[24] stores to replace
old one of 6 stores" 1 "store-merging" } } */
+/* { dg-final { scan-tree-dump-times "New sequence of \[24] stores to replace
old one of 6 stores" 1 "store-merging" { target { ! arm*-*-* } } } } */
typedef struct S { char data[16]; } S;
void optimize_me (S);
diff --git a/gcc/testsuite/gcc.dg/store_merging_29.c
b/gcc/testsuite/gcc.dg/store_merging_29.c
index
777020fab5a12946e0bc68785394347ae06060ee..6b32aa9b6f973b8ce6daec34870d37f0fe92622d
100644
--- a/gcc/testsuite/gcc.dg/store_merging_29.c
+++ b/gcc/testsuite/gcc.dg/store_merging_29.c
@@ -3,7 +3,7 @@
/* { dg-require-effective-target store_merge } */
/* { dg-options "-O2 -fdump-tree-store-merging-details" } */
/* { dg-final { scan-tree-dump "New sequence of 3 stores to replace old one of
6 stores" "store-merging" { target { le && { ! arm*-*-* } } } } } */
-/* { dg-final { scan-tree-dump "New sequence of \[34] stores to replace old
one of 6 stores" "store-merging" { target { be || { arm*-*-* } } } } } */
+/* { dg-final { scan-tree-dump "New sequence of \[34] stores to replace old
one of 6 stores" "store-merging" { target { be && { ! arm*-*-* } } } } } */
struct T { char a[1024]; };
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
index
3e09668ddc96320e3a1b964a9e60e7c5ba593460..70659c00c0efde5548fa69d4acd234bfde94791a
100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-6.c
@@ -5,7 +5,7 @@
{ dg-require-effective-target store_merge } */
-extern char a2[2];
+extern __attribute__((aligned(2))) char a2[2];
void f2 (void)
{
@@ -13,7 +13,7 @@ void f2 (void)
a2[1] = 0;
}
-extern char a4[4];
+extern __attribute__((aligned(4))) char a4[4];
void f4 (void)
{
@@ -23,7 +23,7 @@ void f4 (void)
a4[3] = 0;
}
-extern char a8[8];
+extern __attribute__((aligned(8))) char a8[8];
void f8 (void)
{
Re: Doubts regarding the _Dependent_ptr keyword
On Wed, Jul 03, 2019 at 05:47:56PM +0200, Richard Biener wrote:
> On July 3, 2019 5:14:58 PM GMT+02:00, "Paul E. McKenney"
> wrote:
> >On Wed, Jul 03, 2019 at 12:39:41AM +0530, Akshat Garg wrote:
> >> On Tue, Jul 2, 2019 at 8:40 PM Paul E. McKenney
> >
> >> wrote:
> >>
> >> > On Tue, Jul 02, 2019 at 02:15:55PM +0100, Ramana Radhakrishnan
> >wrote:
> >> > > On Tue, Jul 2, 2019 at 1:38 PM Paul E. McKenney
> >
> >> > wrote:
> >> > >
> >> > > >
> >> > > > Once a user-created non-dependent pointer is assigned to, it is
> >OK to
> >> > > > break the dependency.
> >> > >
> >> > > Ok, that's good.
> >> > > >
> >> > > > Or am I missing the point here?
> >> > >
> >> > > I was just trying to make sure we were on the same page. I wonder
> >if
> >> > > marking this volatile would be sufficient for prototyping. I
> >suspect
> >> > > we would need another flag somewhere which someone with gimple
> >> > > knowledge might be able to help us with.
> >> >
> >> > I expect that marking it as volatile would do the trick. ;-)
> >> >
> >> > Thanx, Paul
> >> >
> >> So, marking this pointer as volatile will not allow the compiler to
> >> modify/optimize the statements, the pointer is appearing in. And we
> >don't
> >> need to push any other code inside any of the passes. Due to this, we
> >want
> >> to automatically say those dependent pointers are volatile and
> >introduce a
> >> new flag for this. Am I getting you guys correctly? Kindly, let me
> >know?
> >
> >While I suspect that this might work, it would suppress way more
> >optimizations than would be good. For but one example, consider:
> >
> > _Dependent_ptr int *p;
> >
> > p = atomic_load_explicit(gp, memory_order_consume);
> > a = p->a;
> > b = p->b;
> >
> >If "p" is volatile, then the compiler will be prevented from keeping
> >it in a register, which would not make people coding fastpaths at
> >all happy. ;-)
> >
> >Still, use of volatile might be a good technique for prototyping and
> >analysis of _Dependent_ptr.
>
> With this example can you quickly summarize what kind of guarantees
> _Dependent_ptr gives and how a compiler
> Could possibly break those?
First I suppose I should fix the bug in the above code. Or one of the
bugs, at least. :-/
struct foo {
int a;
int b;
};
_Dependent_ptr struct foo *p;
p = atomic_load_explicit(gp, memory_order_consume);
a = p->a;
b = p->b;
And then let me tweak the example a bit. For the first tweak:
struct foo {
int a;
int b;
};
struct foo default_foo = { .a = 42, .b = 43 };
int *gp = _foo;
...
_Dependent_ptr int *p;
p = atomic_load_explicit(gp, memory_order_consume);
a = p->a;
b = p->b;
Suppose that the compiler used feedback-driven optimization, and noticed
that the value of gp was almost always _foo. The compiler might
decide to transform the last three lines as follows:
p = atomic_load_explicit(gp, memory_order_consume);
if (p == _foo) {
a = default_foo.a;
b = default_foo.b;
} else {
a = p->a;
b = p->b;
}
Now, as long as the value of gp had remained _foo for the full
duration of execution, no harm done. But suppose the following code
was executing concurrently with the above transformed code:
struct foo *q;
q = malloc(sizeof(*q));
assert(q);
q->a = 1729;
q->b = 1730;
atomic_store_explicit(gp, q, memory_order_release);
do_something();
default_foo.a = 1;
default_foo.b = 2;
atomic_store_explicit(gp, _foo, memory_order_release);
In this case, if the memory_order_consume() came just after the pointer
was reset to _foo, it is possible that the transformed code
would set "a" to 42 and "b" to 43, which might not be what the guy
writing the code wanted to happen.
One of the purposes of _Dependent_ptr is to prevent this transformation.
This transformation can also happen if the developer's code contained a
comparison to _foo -- an ARM or PowerPC compiler backend, upon
seeing two pointers containing the same bits, would likely consider the
two pointers as being interchangeable, and thus might do the dereferences
using the copy that was not tagged with the hardware dependencies.
There are quite a few other examples. The C++ standards committee
working papers shown below go through a number of them, in case the
above example is not convincing. Or you could tell me what you would
like to see, and I would attempt to find/create a suitable example.
Does that help, or am I missing your point?
Thanx, Paul
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0098r0.pdf
Re: [PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.
On July 3, 2019 4:53:30 PM GMT+02:00, "Martin Liška" wrote: >On 7/2/19 7:15 PM, Marc Glisse wrote: >> On Tue, 2 Jul 2019, Martin Liška wrote: >> >>> After the discussion with Richi and Nathan, I made a place in >tree_function_decl >>> and I rebased the original Dominik's patch on top of that. >> >> So, last time there were some questions about the legality of this >transformation. Did you change the exact set of functions on which this >is applied? > >Yes. I was not included in the original discussion, but I hope the >transformation is valid. >Btw. clang also removes the new/delete pairs and I guess it was the >original motivation of the patch. We also remove malloc/free pairs which the C standard does not explicitly allow (but also doesn't explicitly forbid). I don't think standards need to enumerate everything allowed and I don't know any explicit wording in the C++ standard that forbids this. It's only that users can override the allocation functions (but so can they in C) and it was suggested we need to preserve side effects unknown to the compiler. Richard. >Martin > >Or has there been a clarification in the standard saying that this is >ok? (or were we mistaken the first time to believe that there might be >an issue?) >>
Re: Doubts regarding the _Dependent_ptr keyword
On July 3, 2019 5:14:58 PM GMT+02:00, "Paul E. McKenney" wrote: >On Wed, Jul 03, 2019 at 12:39:41AM +0530, Akshat Garg wrote: >> On Tue, Jul 2, 2019 at 8:40 PM Paul E. McKenney > >> wrote: >> >> > On Tue, Jul 02, 2019 at 02:15:55PM +0100, Ramana Radhakrishnan >wrote: >> > > On Tue, Jul 2, 2019 at 1:38 PM Paul E. McKenney > >> > wrote: >> > > >> > > > >> > > > Once a user-created non-dependent pointer is assigned to, it is >OK to >> > > > break the dependency. >> > > >> > > Ok, that's good. >> > > > >> > > > Or am I missing the point here? >> > > >> > > I was just trying to make sure we were on the same page. I wonder >if >> > > marking this volatile would be sufficient for prototyping. I >suspect >> > > we would need another flag somewhere which someone with gimple >> > > knowledge might be able to help us with. >> > >> > I expect that marking it as volatile would do the trick. ;-) >> > >> > Thanx, Paul >> > >> So, marking this pointer as volatile will not allow the compiler to >> modify/optimize the statements, the pointer is appearing in. And we >don't >> need to push any other code inside any of the passes. Due to this, we >want >> to automatically say those dependent pointers are volatile and >introduce a >> new flag for this. Am I getting you guys correctly? Kindly, let me >know? > >While I suspect that this might work, it would suppress way more >optimizations than would be good. For but one example, consider: > > _Dependent_ptr int *p; > > p = atomic_load_explicit(gp, memory_order_consume); > a = p->a; > b = p->b; > >If "p" is volatile, then the compiler will be prevented from keeping >it in a register, which would not make people coding fastpaths at >all happy. ;-) > >Still, use of volatile might be a good technique for prototyping and >analysis of _Dependent_ptr. With this example can you quickly summarize what kind of guarantees _Dependent_ptr gives and how a compiler Could possibly break those? Richard. > > Thanx, Paul > >> Akshat >> >> > >> > > regards >> > > Ramana >> > > >> > > > >> > > > Thanx, >Paul >> > > > >> > > > > Ramana >> > > > > >> >> > > > > >> >> > > > > >> > Does this sounds like a workable plan for ? Let me know >your >> > thoughts. If this sounds good then, we can do this for all the >> > optimizations that may kill the dependencies at somepoint. >> > > > > >> > >> > > > > >> > -Akshat >> > > > > >> > > > >> > > >> > >> >
Re: [PATCH v2 4/5] or1k: Initial support for FPU
Hi Stafford,
On Wed, Jul 03, 2019 at 12:33:50PM +0900, Stafford Horne wrote:
> +case 'd':
> + if (REG_P (x))
> + if (GET_MODE (x) == DFmode || GET_MODE (x) == DImode)
> + fprintf (file, "%s,%s", reg_names[REGNO (operand)],
> + reg_names[REGNO (operand) + 1]);
> + else
> + fprintf (file, "%s", reg_names[REGNO (operand)]);
> + else
The coding conventions says to use braces around nested conditionals.
> @@ -212,6 +214,7 @@ enum reg_class
> #define REG_CLASS_CONTENTS \
> { { 0x, 0x },\
>{ SIBCALL_REGS_MASK, 0 },\
> + { 0x7efe, 0x },\
Above you said r0, r30, r31 are excluded, but this is r0, r8, r30, or
in GCC register numbers, 0, 8, and 31? You probably should mention r8
somewhere (it's because it is the last arg, this avoid problems, I guess?),
and the 30/31 thing is confused some way. Maybe it is all just that one
documentation line :-)
> +; d - double pair base registers (excludes r0, r30 and r31 which overflow)
Segher
[Bug c++/91075] New: Wrong code generated for static variable with local redeclaration
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91075
Bug ID: 91075
Summary: Wrong code generated for static variable with local
redeclaration
Product: gcc
Version: 9.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: andrey.vihrov at gmail dot com
Target Milestone: ---
Consider the following C++ sample:
static int x;
int main()
{
int x = 1;
{
extern int x;
return x;
}
}
Compiling the above code with "g++ test.cpp" gives:
/usr/bin/ld: /tmp/ccZhQ3uF.o: in function `main':
test.cpp:(.text+0x6): undefined reference to `x'
collect2: error: ld returned 1 exit status
Removing the first "int x;" declaration from main() fixes the problem.
According to Note 3.3.2.11 of [1], the local "extern int x;" declaration should
refer to the static variable from the global namespace.
The problem is reproducible with GCC 9.1.0 and all GCC versions / targets
available on gcc.godbolt.org, while clang and other compilers produce correct
code.
[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4296.pdf
Re: Doubts regarding the _Dependent_ptr keyword
On Wed, Jul 03, 2019 at 12:39:41AM +0530, Akshat Garg wrote: > On Tue, Jul 2, 2019 at 8:40 PM Paul E. McKenney > wrote: > > > On Tue, Jul 02, 2019 at 02:15:55PM +0100, Ramana Radhakrishnan wrote: > > > On Tue, Jul 2, 2019 at 1:38 PM Paul E. McKenney > > wrote: > > > > > > > > > > > Once a user-created non-dependent pointer is assigned to, it is OK to > > > > break the dependency. > > > > > > Ok, that's good. > > > > > > > > Or am I missing the point here? > > > > > > I was just trying to make sure we were on the same page. I wonder if > > > marking this volatile would be sufficient for prototyping. I suspect > > > we would need another flag somewhere which someone with gimple > > > knowledge might be able to help us with. > > > > I expect that marking it as volatile would do the trick. ;-) > > > > Thanx, Paul > > > So, marking this pointer as volatile will not allow the compiler to > modify/optimize the statements, the pointer is appearing in. And we don't > need to push any other code inside any of the passes. Due to this, we want > to automatically say those dependent pointers are volatile and introduce a > new flag for this. Am I getting you guys correctly? Kindly, let me know? While I suspect that this might work, it would suppress way more optimizations than would be good. For but one example, consider: _Dependent_ptr int *p; p = atomic_load_explicit(gp, memory_order_consume); a = p->a; b = p->b; If "p" is volatile, then the compiler will be prevented from keeping it in a register, which would not make people coding fastpaths at all happy. ;-) Still, use of volatile might be a good technique for prototyping and analysis of _Dependent_ptr. Thanx, Paul > Akshat > > > > > > regards > > > Ramana > > > > > > > > > > > Thanx, Paul > > > > > > > > > Ramana > > > > > >> > > > > > >> > > > > > >> > Does this sounds like a workable plan for ? Let me know your > > thoughts. If this sounds good then, we can do this for all the > > optimizations that may kill the dependencies at somepoint. > > > > > >> > > > > > > >> > -Akshat > > > > > > > > > > > > > > > >
Re: Doubts regarding the _Dependent_ptr keyword
On Tue, Jul 02, 2019 at 07:53:20PM +0200, Richard Biener wrote:
> On July 2, 2019 5:36:08 PM GMT+02:00, Jason Merrill wrote:
> >On Mon, Jul 1, 2019 at 8:59 PM Paul E. McKenney
> >wrote:
> >>
> >> On Tue, Jul 02, 2019 at 05:58:48AM +0530, Akshat Garg wrote:
> >> > On Tue, Jun 25, 2019 at 9:49 PM Akshat Garg
> >wrote:
> >> >
> >> > > On Tue, Jun 25, 2019 at 4:04 PM Ramana Radhakrishnan <
> >> > > ramana@googlemail.com> wrote:
> >> > >
> >> > >> On Tue, Jun 25, 2019 at 11:03 AM Akshat Garg
> >wrote:
> >> > >> >
> >> > >> > As we have some working front-end code for _Dependent_ptr,
> >What should
> >> > >> we do next? What I understand, we can start adding the library
> >for
> >> > >> dependent_ptr and its functions for C corresponding to the ones
> >we created
> >> > >> as C++ template library. Then, after that, we can move on to
> >generating the
> >> > >> assembly code part.
> >> > >> >
> >> > >>
> >> > >>
> >> > >> I think the next step is figuring out how to model the Dependent
> >> > >> pointer information in the IR and figuring out what
> >optimizations to
> >> > >> allow or not with that information. At this point , I suspect we
> >need
> >> > >> a plan on record and have the conversation upstream on the
> >lists.
> >> > >>
> >> > >> I think we need to put down a plan on record.
> >> > >>
> >> > >> Ramana
> >> > >
> >> > > [CCing gcc mailing list]
> >> > >
> >> > > So, shall I start looking over the pointer optimizations only and
> >see what
> >> > > information we may be needed on the same examples in the IR
> >itself?
> >> > >
> >> > > - Akshat
> >> > >
> >> > I have coded an example where equality comparison kills dependency
> >from the
> >> > document P0190R4 as shown below :
> >> >
> >> > 1. struct rcutest rt = {1, 2, 3};
> >> > 2. void thread0 ()
> >> > 3. {
> >> > 4. rt.a = -42;
> >> > 5. rt.b = -43;
> >> > 6. rt.c = -44;
> >> > 7. rcu_assign_pointer(gp, );
> >> > 8. }
> >> > 9.
> >> > 10. void thread1 ()
> >> > 11. {
> >> > 12.int i = -1;
> >> > 13.int j = -1;
> >> > 14._Dependent_ptr struct rcutest *p;
> >> > 15.
> >> > 16.p = rcu_dereference(gp);
> >> > 17.j = p->a;
> >> > 18. if (p == )
> >> > 19.i = p->b; /*Dependency breaking point*/
> >> > 20. else if(p)
> >> > 21. i = p->c;
> >> > 22. assert(i<0);
> >> > 23. assert(j<0);
> >> > 24. }
> >> > The gimple unoptimized code produced for lines 17-24 is shown below
> >> >
> >> > 1. if (p_16 == )
> >> > 2. goto ; [INV]
> >> > 3. else
> >> > 4.goto ; [INV]
> >> > 5.
> >> > 6. :
> >> > 7. i_19 = p_16->b;
> >> > 8. goto ; [INV]
> >> > 9.
> >> > 10. :
> >> > 11. if (p_16 != 0B)
> >> > 12.goto ; [INV]
> >> > 13. else
> >> > 14.goto ; [INV]
> >> > 15.
> >> > 16. :
> >> > 17. i_18 = p_16->c;
> >> > 18.
> >> > 19. :
> >> > 20. # i_7 = PHI
> >> > 21. _3 = i_7 < 0;
> >> > 22. _4 = (int) _3;
> >> > 23. assert (_4);
> >> > 24. _5 = j_17 < 0;
> >> > 25. _6 = (int) _5;
> >> > 26. assert (_6);
> >> > 27. return;
> >> >
> >> > The optimized code after -O1 is applied for the same lines is hown
> >below :
> >> >
> >> > 1. if (_2 == )
> >> > 2.goto ; [30.00%]
> >> > 3. else
> >> > 4.goto ; [70.00%]
> >> > 5.
> >> > 6. [local count: 322122547]:
> >> > 7. i_12 = rt.b;
> >> > 8. goto ; [100.00%]
> >> > 9.
> >> > 10. [local count: 751619277]:
> >> > 11. if (_1 != 0)
> >> > 12. goto ; [50.00%]
> >> > 13. else
> >> > 14.goto ; [50.00%]
> >> > 15.
> >> > 16. [local count: 375809638]:
> >> > 17. i_11 = MEM[(dependent_ptr struct rcutest *)_2].c;
> >> > 18.
> >> > 19.[local count: 1073741824]:
> >> > 20. # i_7 = PHI
> >> > 21. _3 = i_7 < 0;
> >> > 22. _4 = (int) _3;
> >> > 23. assert (_4);
> >> > 24. _5 = j_10 < 0;
> >> > 25. _6 = (int) _5;
> >> > 26. assert (_6);
> >> > 27. return;
> >>
> >> Good show on tracing this through!
> >>
> >> > Statement 19 in the program gets converted from i_19 = p_16->b; in
> >line 7
> >> > in unoptimized code to i_12 = rt.b; in line 7 in optimized code
> >which
> >> > breaks the dependency chain. We need to figure out the pass that
> >does that
> >> > and put some handling code in there for the _dependent_ptr
> >qualified
> >> > pointers.
>
> Wtf should this be for? A type qualifier is certainly not going to work.
I might be wrong, but I don't believe that Akshat is claiming that this
is already a complete solution.
But please tell us more. Given what Akshat is trying to do, what else
is missing or otherwise in need of fixing?
Thanx, Paul
> Richard.
>
>
> Passing simply -fipa-pure-const,
> >-fguess-branch-probability or
> >> > any other option alone does not produce the optimized code that
> >breaks the
> >> > dependency. But applying -O1, i.e., allowing all the optimizations
> >does so.
> >> > As passes are applied in a certain order, we need to figure out up
> >to what
> >> > passes, the code remains same and after what
