Hi Frederik! On 2020-04-21T14:13:33+0200, Frederik Harwath <frede...@codesourcery.com> wrote: > Thomas Schwinge <tho...@codesourcery.com> writes: >> Via <https://gcc.gnu.org/PR94629> "10 issues located by the PVS-studio >> static analyzer" (so please reference that one on any patch submission), >> on <https://habr.com/en/company/pvs-studio/blog/497640/> in "Fragment N3, >> Assigning a variable to itself", we find this latter assignment qualified >> as "very strange to assign a variable to itself".
> [...] the original intention must have been [...] >> then does the current algorith still work despite this error? > > [...] Thanks for this analysis! > Hence I found it preferrable to remove the assignment to the > "outer_reduction_clauses" field and the "local_reduction_clauses" field > from "new_omp_context" completely. (The fields are still zero intialized > by the allocation of the struct which uses XCNEW.) That way the whole > logic regarding the fields is now contained in "scan_omp_for". > > I have executed "make check" (on x86_64-linux-gnu) to verify that the > change causes no regressions. Ok to push the commit to master? ACK, thanks! To record the review effort, please include "Reviewed-by: Thomas Schwinge <tho...@codesourcery.com>" in the commit log, see <https://gcc.gnu.org/wiki/Reviewed-by>. Grüße Thomas > From 2d60b374a44b212ff97c8b1fd6f8c39e478dc70f Mon Sep 17 00:00:00 2001 > From: Frederik Harwath <frede...@codesourcery.com> > Date: Tue, 21 Apr 2020 12:36:14 +0200 > Subject: [PATCH] Remove fishy self-assignment in omp-low.c [PR94629] > > The PR noticed that omp-low.c contains a self-assignment in the > function new_omp_context: > > if (outer_ctx) { > ... > ctx->outer_reduction_clauses = ctx->outer_reduction_clauses; > > This is obviously useless. The original intention might have been > to copy the field from the outer_ctx to ctx. Since this is done > (properly) in the only function where this field is actually used > (in function scan_omp_for) and the field is being initialized to zero > during the struct allocation, there is no need to attempt to do > anything to this field in new_omp_context. Thus this commit > removes any assignment to the field from new_omp_context. > > 2020-04-21 Frederik Harwath <frede...@codesourcery.com> > > PR other/94629 > * gcc/omp-low.c (new_omp_context): Remove assignments to > ctx->outer_reduction_clauses and ctx->local_reduction_clauses. > --- > gcc/omp-low.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/gcc/omp-low.c b/gcc/omp-low.c > index 67565d61400..88f23e60d34 100644 > --- a/gcc/omp-low.c > +++ b/gcc/omp-low.c > @@ -128,10 +128,16 @@ struct omp_context > corresponding tracking loop iteration variables. */ > hash_map<tree, tree> *lastprivate_conditional_map; > > - /* A tree_list of the reduction clauses in this context. */ > + /* A tree_list of the reduction clauses in this context. This is > + only used for checking the consistency of OpenACC reduction > + clauses in scan_omp_for and is not guaranteed to contain a valid > + value outside of this function. */ > tree local_reduction_clauses; > > - /* A tree_list of the reduction clauses in outer contexts. */ > + /* A tree_list of the reduction clauses in outer contexts. This is > + only used for checking the consistency of OpenACC reduction > + clauses in scan_omp_for and is not guaranteed to contain a valid > + value outside of this function. */ > tree outer_reduction_clauses; > > /* Nesting depth of this context. Used to beautify error messages re > @@ -931,8 +937,6 @@ new_omp_context (gimple *stmt, omp_context *outer_ctx) > ctx->outer = outer_ctx; > ctx->cb = outer_ctx->cb; > ctx->cb.block = NULL; > - ctx->local_reduction_clauses = NULL; > - ctx->outer_reduction_clauses = ctx->outer_reduction_clauses; > ctx->depth = outer_ctx->depth + 1; > } > else > @@ -948,8 +952,6 @@ new_omp_context (gimple *stmt, omp_context *outer_ctx) > ctx->cb.transform_call_graph_edges = CB_CGE_MOVE; > ctx->cb.adjust_array_error_bounds = true; > ctx->cb.dont_remap_vla_if_no_change = true; > - ctx->local_reduction_clauses = NULL; > - ctx->outer_reduction_clauses = NULL; > ctx->depth = 1; > } > ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
