Re: [HACKERS] "may be unused" warnings for gcc
On 2/21/17 22:17, Andres Freund wrote: > I've not run comparisons this year, but late last year I was seeing > 5% > < 10% benefits - that seems plenty enough to care. You mean the 5-minute benchmarks on my laptop are not representative? ;-) Here is a patch that I had lying around that clears the compiler warnings under -O3 for me. It seems that they are a subset of what you are seeing. Plausibly, as compilers are doing more analysis in larger scopes, we can expect to see more of these. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >From 2a73d3ac095435ecbe3aefff7bcecc292675e167 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 22 Feb 2017 09:23:40 -0500 Subject: [PATCH] Silence compiler warnings from gcc -O3 --- src/backend/commands/dbcommands.c | 13 +++-- src/backend/utils/adt/varlena.c | 6 ++ src/backend/utils/misc/guc.c | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 1ebacbc24f..2a23f634c5 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -27,6 +27,7 @@ #include "access/genam.h" #include "access/heapam.h" #include "access/htup_details.h" +#include "access/multixact.h" #include "access/xact.h" #include "access/xloginsert.h" #include "access/xlogutils.h" @@ -103,14 +104,14 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) Relation rel; Oid src_dboid; Oid src_owner; - int src_encoding; - char *src_collate; - char *src_ctype; + int src_encoding = -1; + char *src_collate = NULL; + char *src_ctype = NULL; bool src_istemplate; bool src_allowconn; - Oid src_lastsysoid; - TransactionId src_frozenxid; - MultiXactId src_minmxid; + Oid src_lastsysoid = InvalidOid; + TransactionId src_frozenxid = InvalidTransactionId; + MultiXactId src_minmxid = InvalidMultiXactId; Oid src_deftablespace; volatile Oid dst_deftablespace; Relation pg_database_rel; diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 28b5745ba8..7da7535b3b 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -1129,6 +1129,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state) state->use_wchar = false; state->str1 = VARDATA_ANY(t1); state->str2 = VARDATA_ANY(t2); + state->wstr1 = 0; + state->wstr2 = 0; state->len1 = len1; state->len2 = len2; } @@ -1144,6 +1146,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state) len2 = pg_mb2wchar_with_len(VARDATA_ANY(t2), p2, len2); state->use_wchar = true; + state->str1 = 0; + state->str2 = 0; state->wstr1 = p1; state->wstr2 = p2; state->len1 = len1; @@ -1227,6 +1231,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state) state->skiptable[wstr2[i] & skiptablemask] = last - i; } } + else + state->skiptablemask = 255; } static int diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 24771389c8..107e1f2957 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -9275,7 +9275,7 @@ RestoreGUCState(void *gucstate) char *varname, *varvalue, *varsourcefile; - int varsourceline; + int varsourceline = -1; GucSource varsource; GucContext varscontext; char *srcptr = (char *) gucstate; -- 2.11.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] "may be unused" warnings for gcc
On 2017-02-21 17:20:44 -0500, Peter Eisentraut wrote: > On 2/20/17 09:41, Andres Freund wrote: > > When building with a new-ish gcc (6.3.0 right now, but I've seen this > > for a while) with optimization I get a number of warnings: > > These all look like related to inlining/-O3. > > I have attempted to fix these in the past, but I have found that -O3 > doesn't get any performance improvement, so I haven't bothered lately. I've not run comparisons this year, but late last year I was seeing > 5% < 10% benefits - that seems plenty enough to care. - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] "may be unused" warnings for gcc
On 2/20/17 09:41, Andres Freund wrote: > When building with a new-ish gcc (6.3.0 right now, but I've seen this > for a while) with optimization I get a number of warnings: These all look like related to inlining/-O3. I have attempted to fix these in the past, but I have found that -O3 doesn't get any performance improvement, so I haven't bothered lately. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] "may be unused" warnings for gcc
Hi, When building with a new-ish gcc (6.3.0 right now, but I've seen this for a while) with optimization I get a number of warnings: In file included from /home/andres/src/postgresql/src/include/postgres.h:48:0, from /home/andres/src/postgresql/src/backend/parser/parse_collate.c:41: /home/andres/src/postgresql/src/backend/parser/parse_collate.c: In function ‘select_common_collation’: /home/andres/src/postgresql/src/include/utils/elog.h:107:4: warning: ‘context.location2’ may be used uninitialized in this function [-Wmaybe-uninitialized] errfinish rest; \ ^ /home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: ‘context.location2’ was declared here assign_collations_context context; ^~~ In file included from /home/andres/src/postgresql/src/include/postgres.h:48:0, from /home/andres/src/postgresql/src/backend/parser/parse_collate.c:41: /home/andres/src/postgresql/src/include/utils/elog.h:107:4: warning: ‘context.collation2’ may be used uninitialized in this function [-Wmaybe-uninitialized] errfinish rest; \ ^ /home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: ‘context.collation2’ was declared here assign_collations_context context; ^~~ While I believe these are false positives, I am not surprised that the compiler can't see that. select_common_collation() initializes some fields of assign_collations_context, but not others. There's several branches out of assign_collations_walker that return without setting ocllation2/location2. I think that's currently harmless because it looks like select_common_collation() won't enter the context.strength == COLLATE_CONFLICT branch in that case - but it's certainly hard to see. In file included from /home/andres/src/postgresql/src/backend/commands/dbcommands.c:20:0: /home/andres/src/postgresql/src/backend/commands/dbcommands.c: In function ‘createdb’: /home/andres/src/postgresql/src/include/postgres.h:529:35: warning: ‘src_minmxid’ may be used uninitialized in this function [-Wmaybe-uninitialized] #define TransactionIdGetDatum(X) ((Datum) SET_4_BYTES((X))) ^ /home/andres/src/postgresql/src/backend/commands/dbcommands.c:113:14: note: ‘src_minmxid’ was declared here MultiXactId src_minmxid; ^~~ (and the same for src_frozenxid, src_lastsysoid, ...) It appears that the loop in get_db_info() is too complex for gcc. Replacing the !HeapTupleIsValid(tuple) break; with a heap_close() and direct return fixes those. /home/andres/src/postgresql/src/backend/utils/misc/guc.c: In function ‘RestoreGUCState’: /home/andres/src/postgresql/src/backend/utils/misc/guc.c:6619:21: warning: ‘varsourceline’ may be used uninitialized in this function [-Wmaybe-uninitialized] record->sourceline = sourceline; ~~~^~~~ /home/andres/src/postgresql/src/backend/utils/misc/guc.c:9279:8: note: ‘varsourceline’ was declared here int varsourceline; ^ Not sure what the problem is here - even if the varsourcefile[0] test in RestoreGUCState is stored in a local variable that's also checked before the set_config_sourcefile() branch, it warns. Initializing varsourceline to 0 works and seems reasonable. /home/andres/src/postgresql/src/backend/utils/adt/varlena.c: In function ‘text_position’: /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1358:36: warning: ‘state.skiptablemask’ may be used uninitialized in this function [-Wmaybe-uninitialized] hptr += state->skiptable[*hptr & skiptablemask]; ~~^~~ /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: ‘state.skiptablemask’ was declared here TextPositionState state; ^ /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1344:9: warning: ‘state.wstr2’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (nptr == needle) ^ /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: ‘state.wstr2’ was declared here TextPositionState state; ^ /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: warning: ‘state.wstr1’ may be used uninitialized in this function [-Wmaybe-uninitialized] /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1288:9: warning: ‘state.str2’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (nptr == needle) ^ /home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: ‘state.str2’ was declared here TextPositionState state; ^ No idea what exactly triggers this, but zero-initializing TextPositionState helps. What confuses me is that doing so in text_position() is sufficient - the other uses don't trigger a warning? /home/andres/src/postgre