[PATCH] D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 1 11:29:16 PDT 2018


rnk added a comment.

In https://reviews.llvm.org/D49915#1184590, @vsk wrote:

> In https://reviews.llvm.org/D49915#1184574, @rnk wrote:
>
> > In https://reviews.llvm.org/D49915#1184440, @vsk wrote:
> >
> > > A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.
> >
> >
> > Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.
>
>
> Yes, c.f the source-based coverage instrumentation which ended up needing two bits of information from the frontend per segment: http://llvm.org/doxygen/structllvm_1_1coverage_1_1CoverageSegment.html.
>
> (Btw I'm not convinced we have to grow DILocation; I'm not aware of the use case of enabling SamplePGO along with GCOV instrumentation, so I don't see why it's not feasible to borrow 1-2 bits from the discriminator operand. + @xur & @danielcdh for any corrections here)


The discriminator seems to be stored in a separate parent scope, though. I'm not sure that's really the right place to store this bit.


Repository:
  rL LLVM

https://reviews.llvm.org/D49915





More information about the llvm-commits mailing list