https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88810
--- Comment #6 from paul.richard.thomas at gmail dot com <paul.richard.thomas at gmail dot com> --- Hi Steve and Thomas, I plead guilty to creating confusing code... It developed step by step and I didn't go back and consolidate it. If you can simplify it and still obtain the same result, that would be great. Cheers Paul On Tue, 15 Jan 2019 at 15:36, sgk at troutmask dot apl.washington.edu <gcc-bugzi...@gcc.gnu.org> wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88810 > > --- Comment #5 from Steve Kargl <sgk at troutmask dot apl.washington.edu> --- > On Tue, Jan 15, 2019 at 12:39:13PM +0000, tkoenig at gcc dot gnu.org wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88810 > > > > Thomas Koenig <tkoenig at gcc dot gnu.org> changed: > > > > What |Removed |Added > > ---------------------------------------------------------------------------- > > CC| |tkoenig at gcc dot gnu.org > > > > --- Comment #4 from Thomas Koenig <tkoenig at gcc dot gnu.org> --- > > As far as I can see, the duplicated code does not do anything bad, > > and removing the duplicate also would not do anything bad. > > > > A patch removing the duplication is pre-approved, provided it > > passes a regression test. > > > > Are you sure it is duplicate code? > > Here, if reverse[m] == GFC_ENABLE_REVERSE and this_dep == GFC_DEP_BACKWARD, > then reverse[m] is update to GFC_REVERSE_SET; otherwise it is left as-is > which is GFC_ENABLE_REVERSE. > > /* Set reverse if backward dependence and not inhibited. */ > if (reverse && reverse[m] == GFC_ENABLE_REVERSE) > reverse[m] = (this_dep == GFC_DEP_BACKWARD) ? > GFC_REVERSE_SET : reverse[m]; > > Here, if reverse[m] was updated to GFC_REVERSE_SET, the first > conditional fails and reverse[m] is left unchanged. However, if > reverse[m] was left unchanged from above, it is then GFC_ENABLE_REVERSE. > gfortran then checks this_dep == GFC_DEP_FORWARD and updates accordingly. > > /* Set forward if forward dependence and not inhibited. */ > if (reverse && reverse[m] == GFC_ENABLE_REVERSE) > reverse[m] = (this_dep == GFC_DEP_FORWARD) ? > GFC_FORWARD_SET : reverse[m]; > > The code is likely correct as-is, but confusing. Some would have > written > > /* Set reverse if backward dependence and not inhibited. */ > if (reverse && reverse[m] == GFC_ENABLE_REVERSE > && this_dep == GFC_DEP_BACKWARD) > reverse[m] = GFC_REVERSE_SET; > > /* Set forward if forward dependence and not inhibited. */ > if (reverse && reverse[m] == GFC_ENABLE_REVERSE > && this_dep == GFC_DEP_FORWARD) > reverse[m] = GFC_FORWARD_SET; > > or > > if (reverse && reverse[m] == GFC_ENABLE_REVERSE) > { > if (this_dep == GFC_DEP_BACKWARD) > reverse[m] = GFC_REVERSE_SET; > else if (this_dep == GFC_DEP_FORWARD) > reverse[m] = GFC_FORWARD_SET; > } > > If this_dep can only take on the values of GFC_REVERSE_SET and > GFC_DEP_FORWARD, then the above can be collapsed to > > if (reverse && reverse[m] == GFC_ENABLE_REVERSE) > reverse[m] = this_dep == GFC_DEP_BACKWARD > ? GFC_REVERSE_SET : GFC_FORWARD_SET; > > Looks like this PR is a false positive. > > -- > You are receiving this mail because: > You are on the CC list for the bug.
