> 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…
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
