On Thu, Oct 16, 2014 at 2:11 PM, Tom de Vries <tom_devr...@mentor.com> wrote: > On 16-10-14 10:14, Richard Biener wrote: >> >> On Thu, Oct 16, 2014 at 9:20 AM, Tom de Vries <tom_devr...@mentor.com> >> wrote: >>> >>> On 08/10/12 11:24, Richard Guenther wrote: >>>> >>>> On Sun, Oct 7, 2012 at 12:44 PM, Tom de Vries <tom_devr...@mentor.com> >>>> wrote: >>>>> >>>>> Richard, >>>>> >>>>> attached patch checks that unlinked uses do not contain ssa-names when >>>>> renaming. >>>>> >>>>> This assert triggers when compiling (without the fix) the PR54735 >>>>> example. >>>>> >>>>> AFAIU, it was due to chance that we caught the PR54735 bug by hitting >>>>> the >>>>> verification failure, because the new vdef introduced by renaming >>>>> happened to be >>>>> the same name as the ssa name referenced in the invalid unlinked use >>>>> (in terms >>>>> of maybe_replace_use: rdef == use). >>>>> >>>>> The assert from this patch catches all cases that an unlinked use >>>>> contains an >>>>> ssa-name. >>>>> >>>>> Bootstrapped and reg-tested on x86_64 (Ada inclusive). >>>>> >>>>> OK for trunk? >>>> >>>> >>>> I don't think that is exactly what we should assert here ... (I thought >>>> about >>>> adding checking myself ...). What we'd want to assert is that before >>>> any new DEF is registered (which may re-allocate an SSA name) that >>>> no uses with SSA_NAME_IN_FREELIST appear. Thus, a light verification >>>> pass would be necessary at the beginning of update_ssa >>>> (which I queued onto my TODO list ...). We'd want that anyway to for >>>> example catch the case where a non-virtual operand is partially renamed. >>>> >>> >>> Richard, >>> >>> while developing a patch, I ran into the same 'no immediate_use list' >>> verification error again, caused by an unlinked use containing an >>> ssa-name. >>> >>> The verification error was caused by an error in my patch, but triggered >>> by >>> chance, by an unrelated change in the patch. >>> >>> I've tried to implement the 'light verification pass' you describe above, >>> and >>> I've checked that the error in my patch is found, also when I remove the >>> trigger >>> for the verification error from my patch. >>> >>> Bootstrapped and reg-tested on x86_64 (with the ENABLE_CHECKING guarding >>> removed, in order to ensure the code is active). >>> >>> OK for trunk? >> >> >> Ok with changing the gcc_assert to >> >> if (SSA_NAME_IN_FREE_LIST (use)) >> { >> error ("statement uses released SSA name"); >> debug_gimple_stmt (stmt); >> err = true; >> } >> >> and after checking all stmts >> >> if (err) >> internal_error ("cannot update SSA form"); >> >> you might want to push/pop TV_TREE_STMT_VERIFY around all this >> as well. >> > > Richard, > > I've implemented the changes listed above, and also made the message a bit > more verbose: > ... > kernels-2.c: In function ‘main’: > kernels-2.c:41:5: error: statement uses released SSA name > for (COUNTERTYPE ii = 0; ii < N; ii++) > ^ > # .MEM_57 = VDEF <.MEM_79> > .omp_data_arr.10 ={v} {CLOBBER}; > The use of .MEM_79 should have been replaced or marked for renaming
^^^ or marked for renaming is not correct, only replacing is > kernels-2.c:41:5: internal compiler error: cannot update SSA from > ... > > I've added mentioning the specific use that has the problem, since it will > not always be evident which is the one with the problem. > > OK for trunk? Ok with ajdusting the message. Thanks RIchard. > If that's too verbose I can also implement instead: > ... > kernels-2.c:41:5: error: statement uses released SSA name .MEM_79 > ... > > Thanks, > - Tom >