On Tue, Jul 1, 2025 at 5:17 PM Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>
> > On Jul 1, 2025, at 03:14, Richard Biener <richard.guent...@gmail.com> wrote:
> >
> > On Mon, Jun 30, 2025 at 10:37 PM Qing Zhao <qing.z...@oracle.com> wrote:
> >>
> >> Hi, David,
> >>
> >> Thank you for the info.
> >>
> >> Yes, this does sound like a general issue in this area.
> >>
> >> Is there any mechanism in GCC currently that records such original source
> >> code information for IRs
> >> after the compiler transformation?
> >>
> >> If not, shall we add such information in the IR for this purpose? Is doing
> >> this very expensive?
> >>
> >>> On Jun 30, 2025, at 12:23, David Malcolm <dmalc...@redhat.com> wrote:
> >>>
> >>> On Mon, 2025-06-30 at 16:47 +0000, Qing Zhao wrote:
> >>>
> >>> [...snip...]
> >>>
> >>>> The output with -fdiagnostics-show-context=1 is:
> >>>>
> >>>> /home/opc/Work/GCC/latest-gcc-
> >>>> write/gcc/testsuite/gcc.dg/pr109071_7.c: In function ‘foo’:
> >>>> /home/opc/Work/GCC/latest-gcc-
> >>>> write/gcc/testsuite/gcc.dg/pr109071_7.c:12:6: warning: array
> >>>> subscript -1 is below array bounds of ‘int[10]’ [-Warray-bounds=]
> >>>> 12 | a[i] = -1; /* { dg-warning "is below array bounds of" }
> >>>> */
> >>>> | ~^~~
> >>>> ‘foo’: events 1-2
> >>>> 11 | if (i == -1)
> >>>> | ^
> >>>> | |
> >>>> | (1) when the condition is evaluated to true
> >>>
> >>> Looks great, but one caution: presumably "true" in this context refers
> >>> to the state of the IR when the warning is emitted, rather than what
> >>> the user wrote. I've run into this in the analyzer;
> >>> see PR analyzer/100116 (which I don't have a good solution for, alas).
> >>>
> >>> Is there a way to get at the original sense of the condition, in terms
> >>> of what the user wrote? I'm guessing that this has been canonicalized
> >>> away long before the middle-end warnings see this.
> >>
> >> As long as such information is lost during transformation, there is no way
> >> to get
> >> the original source code, I guess.
> >
> > We could try to track condition inversions done with a flag on the gcond.
> > For example we canonicalize
> >
> > if (bool_var == 0)
> >
> > to
> >
> > if (bool_var != 0)
> >
> > with the true/false sense of the edges swapped.
> One flag is enough for such transformation.
> >
> > Instead of "when the condition is evaluated to true" we could also
> > print "when execution continues to" and then point to the first
> > stmt location we can find on the path executed.
>
> This is doable, but I guess the information might be confusing to user too.
> >
> > Note we'll also turn
> >
> > if (a)
> > if (b)
> > {
> > ...
> >
> > into
> >
> > tem = a && b;
> > if (tem)
> > {
> > ...
> >
> > where the location of the if retained is usually the outer one and the
> > split out part might or might not have sensible locations. That's also
> > an issue for debugging of course.
>
> However, for such cases, we might need a new data structure to record
> the original condition statements and attach it to the gcond in order to
> get the original source code information.
>
> Each transformation that might change the condition need to be investigated
> and
> maintain such new data structure. This might be quite tedious and
> error-prone.
> >
> > Caret locations and thus "paths" will always have this kind of issues.
> > Another variant might be to collect a predicate expression up to
> > user variables and print "when `a && b' evaluates to true/false"
> > for the above case instead.
>
> Yes, this is a good workaround too.
>
> And there should be other transformations that might change the conditions,
> in addition to the existing compiler transformations, there might be new
> transformations
> that will be added later to change the conditions too.
>
> >
> > I think we need to amend the documentation a bit about these
> > kind of pitfalls.
>
> Yes. This is necessary.
>
> > And fixup locations we put on stmts eventually.
>
> Yes, looks like a big project…
As one says, the perfect is the enemy of the good.
Richard.
>
> Qing
>
>
> >
> > Richard.
> >
> >> Not sure whether the LOCATION_T can include such information?
> >>>
> >>> Or perhaps the message for (1) could say exactly what condition it
> >>> considers to be true e.g.
> >>> (1) when the condition "i == -1" is evaluated to true
> >>> or somesuch, which might give the user a better clue if the sense of
> >>> the conditional has been reversed during canonicalization/optimization.
> >>
> >> This looks like a reasonable workaround based on the current IR.
> >>>
> >>> Caveat: I didn't reread the latest version of your patch, but am just
> >>> assuming it's looking at the cfg edge flags when the warning is
> >>> emitted; sorry if I'm making a false assumption here.
> >>
> >> Yes, that’s right. You made good assumptions here -:).
> >>
> >> thanks.
> >>
> >> Qing
> >>>
> >>>>
> >>> Dave
>
>
