From: Erik Lindahl Date: Mon, 11 Aug 2014 13:27:53 +0000 (+0200) Subject: Fix Portland compiler warnings X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=f5ca20c4e2f0dcfd13cd3a4b1a1f3d44cd892796;p=alexxy%2Fgromacs.git Fix Portland compiler warnings The PGI compiler finds several unreachable code parts, mainly due to multiple return statements being used. For most places this is just a cosmetic fix to get rid of warnings, but functions that are performance-sensitive should only have a single return statement since the return instruction causes a pipeline stall on many architectures. This patch also fixes a warning about an unused variable. Change-Id: Ibf1c9e9dd1cdf29fc59c84afa4348e02bed270e6 --- diff --git a/src/gromacs/analysisdata/tests/histogram.cpp b/src/gromacs/analysisdata/tests/histogram.cpp index b3a2634fb5..18ec9ce1b9 100644 --- a/src/gromacs/analysisdata/tests/histogram.cpp +++ b/src/gromacs/analysisdata/tests/histogram.cpp @@ -488,7 +488,6 @@ class AverageInputData class MockAverageHistogram : public gmx::AbstractAverageHistogram { public: - MockAverageHistogram() {} //! Creates a histogram module with defined bin parameters. explicit MockAverageHistogram(const gmx::AnalysisHistogramSettings &settings) : AbstractAverageHistogram(settings) diff --git a/src/gromacs/fft/fft5d.cpp b/src/gromacs/fft/fft5d.cpp index 302955022d..94fde316da 100644 --- a/src/gromacs/fft/fft5d.cpp +++ b/src/gromacs/fft/fft5d.cpp @@ -89,19 +89,18 @@ static tMPI::mutex big_fftw_mutex; #define FFTW_UNLOCK try { big_fftw_mutex.unlock(); } GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR #endif /* GMX_FFT_FFTW3 */ +#ifdef GMX_MPI /* largest factor smaller than sqrt */ static int lfactor(int z) { - int i; - for (i = static_cast(sqrt(static_cast(z)));; i--) + int i = static_cast(sqrt(static_cast(z))); + while (z%i != 0) { - if (z%i == 0) - { - return i; - } + i--; } - return 1; + return i; } +#endif /* largest factor */ static int l2factor(int z) @@ -109,16 +108,17 @@ static int l2factor(int z) int i; if (z == 1) { - return 1; + i = 1; } - for (i = z/2;; i--) + else { - if (z%i == 0) + i = z/2; + while (z%i != 0) { - return i; + i--; } } - return 1; + return i; } /* largest prime factor: WARNING: slow recursion, only use for small numbers */ diff --git a/src/gromacs/fileio/tngio.cpp b/src/gromacs/fileio/tngio.cpp index 6cef6532d0..1fea9473e7 100644 --- a/src/gromacs/fileio/tngio.cpp +++ b/src/gromacs/fileio/tngio.cpp @@ -58,21 +58,24 @@ static const char *modeToVerb(char mode) { + const char *p; switch (mode) { case 'r': - return "reading"; + p = "reading"; break; case 'w': - return "writing"; + p = "writing"; break; case 'a': - return "appending"; + p = "appending"; break; default: gmx_fatal(FARGS, "Invalid file opening mode %c", mode); - return ""; + p = ""; + break; } + return p; } void gmx_tng_open(const char *filename, diff --git a/src/gromacs/legacyheaders/types/simple.h b/src/gromacs/legacyheaders/types/simple.h index 9a69a3af6b..bf1a5216af 100644 --- a/src/gromacs/legacyheaders/types/simple.h +++ b/src/gromacs/legacyheaders/types/simple.h @@ -222,6 +222,9 @@ typedef uint64_t gmx_uint64_t; #elif (defined(__INTEL_COMPILER) || defined(__ECC)) && !defined(_MSC_VER) /* ICC on *nix */ # define gmx_unused __attribute__ ((unused)) +#elif defined(__PGI) +/* Portland group compilers */ +# define gmx_unused __attribute__ ((unused)) #elif defined _MSC_VER /* MSVC */ # define gmx_unused /*@unused@*/ diff --git a/src/gromacs/selection/parser.cpp b/src/gromacs/selection/parser.cpp index f570eddb0b..bf326b2516 100644 --- a/src/gromacs/selection/parser.cpp +++ b/src/gromacs/selection/parser.cpp @@ -963,7 +963,7 @@ yy_symbol_value_print (yyoutput, yytype, yyvaluep, scanner) void *scanner; #endif { - FILE *yyo = yyoutput; + FILE *yyo gmx_unused = yyoutput; YYUSE (yyo); if (!yyvaluep) return; diff --git a/src/gromacs/selection/parser.patch b/src/gromacs/selection/parser.patch new file mode 100644 index 0000000000..940535e271 --- /dev/null +++ b/src/gromacs/selection/parser.patch @@ -0,0 +1,11 @@ +--- parser.cpp 2014-08-11 22:24:31.000000000 +0200 ++++ parser.cpp 2014-08-11 22:24:40.000000000 +0200 +@@ -963,7 +963,7 @@ + void *scanner; + #endif + { +- FILE *yyo = yyoutput; ++ FILE *yyo gmx_unused = yyoutput; + YYUSE (yyo); + if (!yyvaluep) + return; diff --git a/src/gromacs/selection/regenerate_parser.sh b/src/gromacs/selection/regenerate_parser.sh index 3e2792199e..66324c737c 100755 --- a/src/gromacs/selection/regenerate_parser.sh +++ b/src/gromacs/selection/regenerate_parser.sh @@ -6,6 +6,10 @@ # The commands are run only if the generated files are older than the # Bison/Flex input files, or if a '-f' flag is provided. +# Note: You can check parser.cpp/scanner.cpp for the exact versions of +# bison/flex that were used in the generation. Some OSs have older versions +# of these tools installed. + FORCE= if [ "x$1" == "x-f" ] ; then FORCE=1 @@ -25,5 +29,7 @@ if [[ -f $dirname/parser.y && -f $dirname/scanner.l ]] ; then cd $dirname fi -[[ $FORCE || parser.y -nt parser.cpp ]] && $BISON -t -o parser.cpp --defines=parser.h parser.y -[[ $FORCE || scanner.l -nt scanner.cpp ]] && $FLEX -o scanner.cpp scanner.l +# We apply some trivial patches to the output to avoid warnings for PGI +# (and maybe other) compilers +[[ $FORCE || parser.y -nt parser.cpp ]] && $BISON -t -o parser.cpp --defines=parser.h parser.y && patch -p0 < parser.patch && rm -f parser.cpp.orig +[[ $FORCE || scanner.l -nt scanner.cpp ]] && $FLEX -o scanner.cpp scanner.l && patch -p0 < scanner.patch && rm -f scanner.cpp.orig diff --git a/src/gromacs/selection/scanner.cpp b/src/gromacs/selection/scanner.cpp index 84eadc60b3..ae8dfac457 100644 --- a/src/gromacs/selection/scanner.cpp +++ b/src/gromacs/selection/scanner.cpp @@ -331,7 +331,7 @@ void _gmx_sel_yyfree (void * ,yyscan_t yyscanner ); #define YY_AT_BOL() (YY_CURRENT_BUFFER_LVALUE->yy_at_bol) -#define _gmx_sel_yywrap(yyscanner) 1 +static inline int _gmx_sel_yywrap(yyscan_t yyscanner) { return 1; } #define YY_SKIP_YYWRAP typedef unsigned char YY_CHAR; @@ -548,11 +548,15 @@ static yyconst flex_int16_t yy_chk[151] = // this call. #define ADD_TOKEN _gmx_sel_lexer_add_token(yytext, yyleng, state) +// Set YY_BREAK to an empty value to avoid warnings (for the PGI compiler) +// when we have return statements followed by break. Instead, we add breaks +// manually. +#define YY_BREAK #define YY_NO_UNISTD_H 1 -#line 556 "scanner.cpp" +#line 560 "scanner.cpp" #define INITIAL 0 #define matchof 1 @@ -780,7 +784,7 @@ YY_DECL register int yy_act; struct yyguts_t * yyg = (struct yyguts_t*)yyscanner; -#line 90 "scanner.l" +#line 94 "scanner.l" @@ -814,7 +818,7 @@ YY_DECL } -#line 818 "scanner.cpp" +#line 822 "scanner.cpp" if ( !yyg->yy_init ) { @@ -895,34 +899,34 @@ do_action: /* This label is used only to access EOF actions. */ case 1: YY_RULE_SETUP -#line 123 "scanner.l" - +#line 127 "scanner.l" +break; YY_BREAK case 2: YY_RULE_SETUP -#line 124 "scanner.l" +#line 128 "scanner.l" { yylval->i = strtol(yytext, NULL, 10); ADD_TOKEN; return TOK_INT; } YY_BREAK case 3: YY_RULE_SETUP -#line 125 "scanner.l" +#line 129 "scanner.l" { yylval->r = strtod(yytext, NULL); ADD_TOKEN; return TOK_REAL; } YY_BREAK case 4: YY_RULE_SETUP -#line 126 "scanner.l" +#line 130 "scanner.l" { yylval->str = gmx_strndup(yytext+1, yyleng-2); ADD_TOKEN; return STR; } YY_BREAK case 5: /* rule 5 can match eol */ YY_RULE_SETUP -#line 128 "scanner.l" -{ _gmx_sel_lexer_add_token(" ", 1, state); } +#line 132 "scanner.l" +{ _gmx_sel_lexer_add_token(" ", 1, state); break; } YY_BREAK case 6: /* rule 6 can match eol */ YY_RULE_SETUP -#line 129 "scanner.l" +#line 133 "scanner.l" { if (yytext[0] == ';' || state->bInteractive) { @@ -934,97 +938,98 @@ YY_RULE_SETUP { _gmx_sel_lexer_add_token(" ", 1, state); } + break; } YY_BREAK case YY_STATE_EOF(cmdstart): -#line 142 "scanner.l" +#line 147 "scanner.l" { state->bCmdStart = true; yyterminate(); } YY_BREAK case YY_STATE_EOF(INITIAL): case YY_STATE_EOF(matchof): case YY_STATE_EOF(matchbool): -#line 143 "scanner.l" +#line 148 "scanner.l" { state->bCmdStart = true; return CMD_SEP; } YY_BREAK case 7: YY_RULE_SETUP -#line 146 "scanner.l" +#line 151 "scanner.l" { ADD_TOKEN; yylval->i = 1; return TOK_INT; } YY_BREAK case 8: YY_RULE_SETUP -#line 147 "scanner.l" +#line 152 "scanner.l" { ADD_TOKEN; yylval->i = 0; return TOK_INT; } YY_BREAK case 9: YY_RULE_SETUP -#line 149 "scanner.l" +#line 154 "scanner.l" { ADD_TOKEN; return GROUP; } YY_BREAK case 10: YY_RULE_SETUP -#line 150 "scanner.l" +#line 155 "scanner.l" { ADD_TOKEN; return TO; } YY_BREAK case 11: YY_RULE_SETUP -#line 151 "scanner.l" +#line 156 "scanner.l" { ADD_TOKEN; BEGIN(0); return OF; } YY_BREAK case 12: YY_RULE_SETUP -#line 152 "scanner.l" +#line 157 "scanner.l" { ADD_TOKEN; return AND; } YY_BREAK case 13: YY_RULE_SETUP -#line 153 "scanner.l" +#line 158 "scanner.l" { ADD_TOKEN; return OR; } YY_BREAK case 14: YY_RULE_SETUP -#line 154 "scanner.l" +#line 159 "scanner.l" { ADD_TOKEN; return XOR; } YY_BREAK case 15: YY_RULE_SETUP -#line 155 "scanner.l" +#line 160 "scanner.l" { ADD_TOKEN; return NOT; } YY_BREAK case 16: YY_RULE_SETUP -#line 156 "scanner.l" +#line 161 "scanner.l" { yylval->str = gmx_strndup(yytext, yyleng); ADD_TOKEN; return CMP_OP; } YY_BREAK case 17: YY_RULE_SETUP -#line 158 "scanner.l" +#line 163 "scanner.l" { return _gmx_sel_lexer_process_identifier(yylval, yytext, yyleng, state); } YY_BREAK case 18: /* rule 18 can match eol */ YY_RULE_SETUP -#line 160 "scanner.l" -{ _gmx_sel_lexer_add_token(" ", 1, state); } +#line 165 "scanner.l" +{ _gmx_sel_lexer_add_token(" ", 1, state); break; } YY_BREAK case 19: YY_RULE_SETUP -#line 161 "scanner.l" +#line 166 "scanner.l" { yylval->str = gmx_strndup(yytext, yyleng); ADD_TOKEN; return STR; } YY_BREAK case 20: YY_RULE_SETUP -#line 162 "scanner.l" +#line 167 "scanner.l" { ADD_TOKEN; return yytext[0]; } YY_BREAK case 21: YY_RULE_SETUP -#line 163 "scanner.l" +#line 168 "scanner.l" YY_FATAL_ERROR( "flex scanner jammed" ); YY_BREAK -#line 1028 "scanner.cpp" +#line 1033 "scanner.cpp" case YY_END_OF_BUFFER: { @@ -2175,4 +2180,4 @@ void _gmx_sel_yyfree (void * ptr , yyscan_t yyscanner) #define YYTABLES_NAME "yytables" -#line 163 "scanner.l" +#line 168 "scanner.l" diff --git a/src/gromacs/selection/scanner.l b/src/gromacs/selection/scanner.l index df784001f5..2ba3ccd0f3 100644 --- a/src/gromacs/selection/scanner.l +++ b/src/gromacs/selection/scanner.l @@ -63,6 +63,10 @@ // this call. #define ADD_TOKEN _gmx_sel_lexer_add_token(yytext, yyleng, state) +// Set YY_BREAK to an empty value to avoid warnings (for the PGI compiler) +// when we have return statements followed by break. Instead, we add breaks +// manually. +#define YY_BREAK %} INTEGER [[:digit:]]+ @@ -120,12 +124,12 @@ COMMENT (#.*) } %} -{COMMENT} +{COMMENT} break; {INTEGER} { yylval->i = strtol(yytext, NULL, 10); ADD_TOKEN; return TOK_INT; } {REAL} { yylval->r = strtod(yytext, NULL); ADD_TOKEN; return TOK_REAL; } {STRING} { yylval->str = gmx_strndup(yytext+1, yyleng-2); ADD_TOKEN; return STR; } -\\\n { _gmx_sel_lexer_add_token(" ", 1, state); } +\\\n { _gmx_sel_lexer_add_token(" ", 1, state); break; } ";"|\n { if (yytext[0] == ';' || state->bInteractive) { @@ -137,6 +141,7 @@ COMMENT (#.*) { _gmx_sel_lexer_add_token(" ", 1, state); } + break; } <> { state->bCmdStart = true; yyterminate(); } @@ -157,6 +162,6 @@ not|"!" { ADD_TOKEN; return NOT; } {IDENTIFIER} { return _gmx_sel_lexer_process_identifier(yylval, yytext, yyleng, state); } -[[:space:]]+ { _gmx_sel_lexer_add_token(" ", 1, state); } +[[:space:]]+ { _gmx_sel_lexer_add_token(" ", 1, state); break; } [_[:alnum:]]+ { yylval->str = gmx_strndup(yytext, yyleng); ADD_TOKEN; return STR; } . { ADD_TOKEN; return yytext[0]; } diff --git a/src/gromacs/selection/scanner.patch b/src/gromacs/selection/scanner.patch new file mode 100644 index 0000000000..8bf42639c2 --- /dev/null +++ b/src/gromacs/selection/scanner.patch @@ -0,0 +1,20 @@ +--- scanner.cpp 2014-08-12 22:12:01.000000000 +0300 ++++ scanner.cpp 2014-08-12 22:17:03.000000000 +0300 +@@ -331,7 +331,7 @@ + + #define YY_AT_BOL() (YY_CURRENT_BUFFER_LVALUE->yy_at_bol) + +-#define _gmx_sel_yywrap(yyscanner) 1 ++static inline int _gmx_sel_yywrap(yyscan_t yyscanner) { return 1; } + #define YY_SKIP_YYWRAP + + typedef unsigned char YY_CHAR; +@@ -1807,7 +1807,7 @@ + YY_BUFFER_STATE b; + char *buf; + yy_size_t n; +- int i; ++ yy_size_t i; + + /* Get memory for full buffer, including space for trailing EOB's. */ + n = _yybytes_len + 2; diff --git a/src/gromacs/selection/scanner_flex.h b/src/gromacs/selection/scanner_flex.h index 89dabd919a..6dc819f879 100644 --- a/src/gromacs/selection/scanner_flex.h +++ b/src/gromacs/selection/scanner_flex.h @@ -340,7 +340,7 @@ extern int _gmx_sel_yylex (yyscan_t yyscanner); #undef YY_DECL #endif -#line 163 "scanner.l" +#line 168 "scanner.l" #line 346 "scanner_flex.h" #undef _gmx_sel_yyIN_HEADER diff --git a/src/gromacs/selection/scanner_internal.cpp b/src/gromacs/selection/scanner_internal.cpp index 0f0457121a..fc5ce92313 100644 --- a/src/gromacs/selection/scanner_internal.cpp +++ b/src/gromacs/selection/scanner_internal.cpp @@ -324,8 +324,7 @@ _gmx_sel_lexer_process_identifier(YYSTYPE *yylval, char *yytext, size_t yyleng, GMX_THROW(gmx::InternalError("Unsupported variable type")); return INVALID; } - delete yylval->sel; - return INVALID; /* Should not be reached. */ + /* This position should not be reached. */ } /* For method symbols, return the correct type */ if (symtype == gmx::SelectionParserSymbol::MethodSymbol) diff --git a/src/gromacs/selection/selelem.cpp b/src/gromacs/selection/selelem.cpp index a10d5a5802..c667739d5e 100644 --- a/src/gromacs/selection/selelem.cpp +++ b/src/gromacs/selection/selelem.cpp @@ -66,19 +66,22 @@ const char * _gmx_selelem_type_str(const gmx::SelectionTreeElement &sel) { + const char *p = NULL; switch (sel.type) { - case SEL_CONST: return "CONST"; - case SEL_EXPRESSION: return "EXPR"; - case SEL_BOOLEAN: return "BOOL"; - case SEL_ARITHMETIC: return "ARITH"; - case SEL_ROOT: return "ROOT"; - case SEL_SUBEXPR: return "SUBEXPR"; - case SEL_SUBEXPRREF: return "REF"; - case SEL_GROUPREF: return "GROUPREF"; - case SEL_MODIFIER: return "MODIFIER"; - } - return NULL; + case SEL_CONST: p = "CONST"; break; + case SEL_EXPRESSION: p = "EXPR"; break; + case SEL_BOOLEAN: p = "BOOL"; break; + case SEL_ARITHMETIC: p = "ARITH"; break; + case SEL_ROOT: p = "ROOT"; break; + case SEL_SUBEXPR: p = "SUBEXPR"; break; + case SEL_SUBEXPRREF: p = "REF"; break; + case SEL_GROUPREF: p = "GROUPREF"; break; + case SEL_MODIFIER: p = "MODIFIER"; break; + // No default clause so we intentionally get compiler errors + // if new selection choices are added later. + } + return p; } /*! @@ -91,30 +94,36 @@ _gmx_selelem_type_str(const gmx::SelectionTreeElement &sel) const char * _gmx_sel_value_type_str(const gmx_ana_selvalue_t *val) { + const char *p = NULL; switch (val->type) { - case NO_VALUE: return "NONE"; - case INT_VALUE: return "INT"; - case REAL_VALUE: return "REAL"; - case STR_VALUE: return "STR"; - case POS_VALUE: return "VEC"; - case GROUP_VALUE: return "GROUP"; + case NO_VALUE: p = "NONE"; break; + case INT_VALUE: p = "INT"; break; + case REAL_VALUE: p = "REAL"; break; + case STR_VALUE: p = "STR"; break; + case POS_VALUE: p = "VEC"; break; + case GROUP_VALUE: p = "GROUP"; break; + // No default clause so we intentionally get compiler errors + // if new selection choices are added later. } - return NULL; + return p; } /*! \copydoc _gmx_selelem_type_str() */ const char * _gmx_selelem_boolean_type_str(const gmx::SelectionTreeElement &sel) { + const char *p = NULL; switch (sel.u.boolt) { - case BOOL_NOT: return "NOT"; break; - case BOOL_AND: return "AND"; break; - case BOOL_OR: return "OR"; break; - case BOOL_XOR: return "XOR"; break; + case BOOL_NOT: p = "NOT"; break; + case BOOL_AND: p = "AND"; break; + case BOOL_OR: p = "OR"; break; + case BOOL_XOR: p = "XOR"; break; + // No default clause so we intentionally get compiler errors + // if new selection choices are added later. } - return NULL; + return p; } diff --git a/src/gromacs/selection/sm_compare.cpp b/src/gromacs/selection/sm_compare.cpp index 7c12dd9a5f..605bf1cf2d 100644 --- a/src/gromacs/selection/sm_compare.cpp +++ b/src/gromacs/selection/sm_compare.cpp @@ -197,17 +197,20 @@ comparison_type(char *str) static const char * comparison_type_str(e_comparison_t cmpt) { + const char *p = NULL; switch (cmpt) { - case CMP_INVALID: return "INVALID"; break; - case CMP_LESS: return "<"; break; - case CMP_LEQ: return "<="; break; - case CMP_GTR: return ">"; break; - case CMP_GEQ: return ">="; break; - case CMP_EQUAL: return "=="; break; - case CMP_NEQ: return "!="; break; + case CMP_INVALID: p = "INVALID"; break; + case CMP_LESS: p = "<"; break; + case CMP_LEQ: p = "<="; break; + case CMP_GTR: p = ">"; break; + case CMP_GEQ: p = ">="; break; + case CMP_EQUAL: p = "=="; break; + case CMP_NEQ: p = "!="; break; + // No default clause so we intentionally get compiler errors + // if new selection choices are added later. } - return NULL; + return p; } /*! diff --git a/src/gromacs/selection/tests/nbsearch.cpp b/src/gromacs/selection/tests/nbsearch.cpp index 0ecf04b6e1..177913fad1 100644 --- a/src/gromacs/selection/tests/nbsearch.cpp +++ b/src/gromacs/selection/tests/nbsearch.cpp @@ -73,10 +73,6 @@ class NeighborhoodSearchTestData public: struct TestPosition { - TestPosition() : refMinDist(0.0), refNearestPoint(-1) - { - clear_rvec(x); - } explicit TestPosition(const rvec x) : refMinDist(0.0), refNearestPoint(-1) { diff --git a/src/programs/mdrun/tests/moduletest.cpp b/src/programs/mdrun/tests/moduletest.cpp index 4bab454016..c0f12bdb49 100644 --- a/src/programs/mdrun/tests/moduletest.cpp +++ b/src/programs/mdrun/tests/moduletest.cpp @@ -68,11 +68,14 @@ namespace test namespace { +#if defined(GMX_THREAD_MPI) || defined(DOXYGEN) //! Number of tMPI threads for child mdrun call. -int gmx_unused g_numThreads = 1; +int g_numThreads = 1; +#endif +#if defined(GMX_OPENMP) || defined(DOXYGEN) //! Number of OpenMP threads for child mdrun call. -int gmx_unused g_numOpenMPThreads = 1; - +int g_numOpenMPThreads = 1; +#endif //! \cond GMX_TEST_OPTIONS(MdrunTestOptions, options) {