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.
