[polly] Add diagnostic remark for ReportVariantBasePtr

[David Blaikie]

The current state is that just using -R should do the right thing
(include column info everywhere), but using it combined with any -g
will omit column info unless you request it explicitly
(-gcolumn-info).

We're talking on llvm-dev about turning on column info by default so
that -R + -g results in the same diagnostic quality as -R alone.

On Thu, Jun 26, 2014 at 1:47 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> [adding some debug-info folks...]
>
>  -Hal
>
> ----- Original Message -----
>> From: "Tobias Grosser" <tobias at grosser.es>
>> To: "Andreas Simbuerger" <simbuerg at googlemail.com>
>> Cc: llvm-commits at cs.uiuc.edu, "Andreas Simbuerger" <simbuerg at fim.uni-passau.de>
>> Sent: Thursday, June 26, 2014 3:41:32 PM
>> Subject: Re: [polly] Add diagnostic remark for ReportVariantBasePtr
>>
>> On 26/06/2014 22:12, Andreas Simbuerger wrote:
>> > -----BEGIN PGP SIGNED MESSAGE-----
>> > Hash: SHA1
>> >
>> >
>> >
>> > On 06/26/2014 09:25 PM, Tobias Grosser wrote:
>> >> On 26/06/2014 21:16, Andreas Simbürger wrote:
>> >>> What was the exact output?
>> >>
>> >> polly-clang /tmp/test.c  -c -mllvm -polly -O3  -Rpass-missed=polly
>> >> -mllvm -polly-detect-track-failures -mllvm
>> >> -polly-detect-keep-going -gcolumn-info /tmp/test.c:6:9: remark:
>> >> The
>> >> following errors keep this region from being a Scop.
>> >> [-Rpass-missed=polly-detect] for (int i=0; i<32; i++) ^
>> >> /tmp/test.c:7:6: remark: The base address of this array is not
>> >> invariant inside the loop [-Rpass-missed=polly-detect] A->b[i] =
>> >> A->b[i+p]; ^ /tmp/test.c:7:6: remark: The base address of this
>> >> array is not invariant inside the loop
>> >> [-Rpass-missed=polly-detect]
>> >>
>> >>
>> >>> I would expect the source location reported for the write and
>> >>> then just the message with the line/column info but without
>> >>> source code.
>> >>
>> >> Both remarks give the very same location. I would have expected
>> >> the first one referring to the first memory address, the second
>> >> one
>> >> to the second memory address. (I would also expect to see the
>> >> source line twice. Once with the caret at the first read, once at
>> >> the second).
>> >>
>> >
>> > That would be nice. However, if you have a look at the attached
>> > *.ll
>> > file (your example translated with the command-line you posted),
>> > you
>> > will see that the dbg metadata that is attached to all instructions
>> > inside the loop body is:
>> >
>> > !26 = metadata !{i32 7, i32 5, metadata !18, null}.
>> >
>> > So no matter what instruction I use inside the body, I will only be
>> > able to report line 7 column 5 :o(.
>> > The fact that both errors have the same DebugLoc leads to to effect
>> > that you don't get the same source location printed twice in a row
>> > (I
>> > guess).
>> >
>> > How can we increase the precision of the column info?
>>
>> This clang hack helps:
>>
>> diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
>> index 62ecc73..53c63f4 100644
>> --- a/lib/CodeGen/CGExpr.cpp
>> +++ b/lib/CodeGen/CGExpr.cpp
>> @@ -771,6 +771,9 @@ LValue CodeGenFunction::EmitCheckedLValue(const
>> Expr
>> *E, TypeCheckKi
>>   /// length type, this is not possible.
>>   ///
>>   LValue CodeGenFunction::EmitLValue(const Expr *E) {
>> +  if (CGDebugInfo *DI = getDebugInfo()) {
>> +  DI->EmitLocation(Builder, E->getLocEnd(), true);
>> +  }
>>     switch (E->getStmtClass()) {
>>     default: return EmitUnsupportedLValue(E, "l-value expression");
>>
>>
>> /tmp/test.c:7:16: remark: The base address of this array is not
>> invariant inside the loop [-Rpass-missed=polly-detect]
>>       A->b[i] = A->b[i+p];
>>                 ^
>> /tmp/test.c:7:6: remark: The base address of this array is not
>> invariant
>> inside the loop [-Rpass-missed=polly-detect]
>>       A->b[i] = A->b[i+p];
>>       ^
>> The patch is probably to aggressive, but it may be a start. Could we
>> possibly emit column info for all load instructions? That would be
>> good
>> enough for Polly at least, no? We probably need to understand the
>> overhead and if other people could also use this information.
>>
>> Tobias
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory