Page MenuHomePhabricator

zequanwu (Zequan Wu)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 31 2020, 4:54 PM (25 w, 6 d)

Recent Activity

Yesterday

zequanwu added a reviewer for D88456: [COFF][CG Profile] set undefined symbol to external: aeubanks.
Mon, Sep 28, 6:46 PM · Restricted Project
zequanwu updated the diff for D87811: [CodeGen] emit CG profile for COFF object file.

Split changes on .cg_profile to another patch D88456.

Mon, Sep 28, 5:19 PM · Restricted Project
zequanwu requested review of D88456: [COFF][CG Profile] set undefined symbol to external.
Mon, Sep 28, 5:17 PM · Restricted Project
zequanwu added inline comments to D87811: [CodeGen] emit CG profile for COFF object file.
Mon, Sep 28, 3:09 PM · Restricted Project
zequanwu updated the diff for D87811: [CodeGen] emit CG profile for COFF object file.

For symbols haven't seen, just set them to external not weak external.

Mon, Sep 28, 3:06 PM · Restricted Project
zequanwu reopened D87811: [CodeGen] emit CG profile for COFF object file.
Mon, Sep 28, 3:04 PM · Restricted Project
zequanwu abandoned D87648: [Coverage][NFC] Remove skipped region after added into MappingRegions.
Mon, Sep 28, 10:41 AM · Restricted Project

Thu, Sep 24

zequanwu added a reverting change for rG90242caca207: Revert "[CodeGen] emit CG profile for COFF object file": rG506b6170cb51: Reland [CodeGen] emit CG profile for COFF object file.
Thu, Sep 24, 2:40 PM
zequanwu committed rG506b6170cb51: Reland [CodeGen] emit CG profile for COFF object file (authored by zequanwu).
Reland [CodeGen] emit CG profile for COFF object file
Thu, Sep 24, 2:40 PM
zequanwu closed D87811: [CodeGen] emit CG profile for COFF object file.
Thu, Sep 24, 2:40 PM · Restricted Project

Wed, Sep 23

zequanwu updated the diff for D87811: [CodeGen] emit CG profile for COFF object file.

Update. Don't emit CGProfileEntry if function has dll import storage class.

Wed, Sep 23, 5:09 PM · Restricted Project
zequanwu committed rGf5435399e823: [CGProfile] don't emit cgprofile entry if called function is dllimport (authored by zequanwu).
[CGProfile] don't emit cgprofile entry if called function is dllimport
Wed, Sep 23, 4:57 PM
zequanwu reopened D87811: [CodeGen] emit CG profile for COFF object file.
Wed, Sep 23, 4:57 PM · Restricted Project
zequanwu closed D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.
Wed, Sep 23, 4:57 PM · Restricted Project
zequanwu added a comment to D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.
In D88127#2291320, @rnk wrote:

I'm saying the fix belongs in TargetObjectFileLoweringImpl.cpp, which is the point where we translate from IR to assembly. We can't fix this after producing assembly, because we don't know what symbols are marked dllimport at that point.

Wed, Sep 23, 4:20 PM · Restricted Project
zequanwu added a comment to D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.
In D88127#2291167, @rnk wrote:

However, I would prefer to move the check into CodeGen, so that if we receive some IR that has edges like this, we don't emit an object file that cannot be linked.

Wed, Sep 23, 4:08 PM · Restricted Project

Tue, Sep 22

zequanwu requested review of D88127: [CGProfile] don't emit cgprofile entry if called function is dllimport.
Tue, Sep 22, 6:11 PM · Restricted Project
zequanwu added inline comments to D87811: [CodeGen] emit CG profile for COFF object file.
Tue, Sep 22, 2:43 PM · Restricted Project

Mon, Sep 21

zequanwu committed rG9caa3fbe03f4: [Coverage] Add empty line regions to SkippedRegions (authored by zequanwu).
[Coverage] Add empty line regions to SkippedRegions
Mon, Sep 21, 12:44 PM
zequanwu closed D84988: [Coverage] Add empty line regions to SkippedRegions.
Mon, Sep 21, 12:44 PM · Restricted Project, Restricted Project, Restricted Project

Fri, Sep 18

zequanwu added a comment to D84988: [Coverage] Add empty line regions to SkippedRegions.
In D84988#2282849, @vsk wrote:

@zequanwu FTR, I don't have any outstanding concerns (I understand you might be asking for a second reviewer to chime in though).

Fri, Sep 18, 3:34 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added a comment to D84988: [Coverage] Add empty line regions to SkippedRegions.

Friendly ping.

Fri, Sep 18, 11:12 AM · Restricted Project, Restricted Project, Restricted Project
zequanwu committed rG91aed9bf975f: [CodeGen] emit CG profile for COFF object file (authored by zequanwu).
[CodeGen] emit CG profile for COFF object file
Fri, Sep 18, 10:58 AM
zequanwu closed D87811: [CodeGen] emit CG profile for COFF object file.
Fri, Sep 18, 10:58 AM · Restricted Project

Thu, Sep 17

zequanwu updated the summary of D87811: [CodeGen] emit CG profile for COFF object file.
Thu, Sep 17, 6:08 PM · Restricted Project
zequanwu updated the diff for D87811: [CodeGen] emit CG profile for COFF object file.

test case.

Thu, Sep 17, 6:08 PM · Restricted Project

Wed, Sep 16

zequanwu requested review of D87811: [CodeGen] emit CG profile for COFF object file.
Wed, Sep 16, 7:06 PM · Restricted Project
zequanwu committed rG4d437348d24d: fix test no-rtti.cpp (authored by zequanwu).
fix test no-rtti.cpp
Wed, Sep 16, 11:03 AM
zequanwu committed rGebf267b87d4b: [Sema][MSVC] warn at dynamic_cast/typeid when /GR- is given (authored by zequanwu).
[Sema][MSVC] warn at dynamic_cast/typeid when /GR- is given
Wed, Sep 16, 10:39 AM
zequanwu closed D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Wed, Sep 16, 10:39 AM · Restricted Project

Tue, Sep 15

zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

reopen the diff and update.

Tue, Sep 15, 1:45 PM · Restricted Project
zequanwu reopened D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Tue, Sep 15, 1:45 PM · Restricted Project
zequanwu committed rGf975ae4867d1: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time (authored by zequanwu).
[CodeGen][typeid] Emit typeinfo directly if type is known at compile-time
Tue, Sep 15, 12:17 PM
zequanwu closed D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.
Tue, Sep 15, 12:17 PM · Restricted Project
zequanwu added inline comments to D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.
Tue, Sep 15, 11:29 AM · Restricted Project
zequanwu updated the diff for D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.

Address comment.

Tue, Sep 15, 11:29 AM · Restricted Project

Mon, Sep 14

zequanwu requested review of D87648: [Coverage][NFC] Remove skipped region after added into MappingRegions.
Mon, Sep 14, 4:01 PM · Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

Rename option EmptyLineCoverage to EmptyLineCommentCoverage and mark it cl::Hidden.

Mon, Sep 14, 11:40 AM · Restricted Project, Restricted Project, Restricted Project
zequanwu added a comment to D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.

I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong.

If the type of the operand is already most derived, we could just emit the typeinfo of the type. This would match the standard: https://eel.is/c++draft/expr.typeid#2

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

For "optimization in codegen", do you mean optimization after the IR is generated or like I did in CodeGenFunction::EmitCXXTypeidExpr?

Mon, Sep 14, 11:27 AM · Restricted Project
zequanwu updated the diff for D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.

If the operand of CXXTypeidExpr is already most derived object, no need to look up vtable.

Mon, Sep 14, 11:00 AM · Restricted Project

Fri, Sep 11

zequanwu closed D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

committed here https://reviews.llvm.org/rG83286a1a8f059d1664b64341854676a36a85cecd.

Fri, Sep 11, 4:24 PM · Restricted Project
zequanwu committed rG83286a1a8f05: [MS ABI] Add mangled type for auto template parameter whose argument kind is… (authored by zequanwu).
[MS ABI] Add mangled type for auto template parameter whose argument kind is…
Fri, Sep 11, 4:20 PM
zequanwu updated the diff for D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

update comment.
add a test case for template function.

Fri, Sep 11, 4:18 PM · Restricted Project
zequanwu updated the diff for D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

Add check for MSVC version 19.14 (removed AutoParmTemplate<false> auto_bool as its mangled name conflicts with AutoParmTemplate<0> auto_int in V19.14).

Fri, Sep 11, 1:55 PM · Restricted Project

Thu, Sep 10

zequanwu added a reviewer for D84988: [Coverage] Add empty line regions to SkippedRegions: rnk.
Thu, Sep 10, 8:08 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.
  • Address comment.
  • Add cl::opt to disable emitting skipped regions for empty lines and comments (used on test case only), and update test cases under CoverageMapping with -mllvm -emptyline-comment-coverage=false.
  • Remove the logic which only emits skipped regions for comments and empty lines parsing function body, because can't tell if it's inside class member function.
  • Always set SR.ColumnStart to 1 in adjustSkippedRange.
Thu, Sep 10, 8:07 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral.

rebase and address comment.

Thu, Sep 10, 6:52 PM · Restricted Project

Wed, Sep 9

zequanwu updated the diff for D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.

Remove unused field.

Wed, Sep 9, 4:33 PM · Restricted Project
zequanwu requested review of D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time.
Wed, Sep 9, 4:30 PM · Restricted Project
zequanwu added a reviewer for D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral: rnk.
Wed, Sep 9, 12:49 PM · Restricted Project
zequanwu added a comment to D84988: [Coverage] Add empty line regions to SkippedRegions.

Thanks for reviewing. One last thing I need to resolve is to handle the coverage test case as skipped regions are emitted for empty lines, and haven't thought a good way other than adding checks for those new skipped regions.

Wed, Sep 9, 12:44 PM · Restricted Project, Restricted Project, Restricted Project

Mon, Sep 7

zequanwu committed rG7907e5516a41: [Sema] fix /gr warning test case (authored by zequanwu).
[Sema] fix /gr warning test case
Mon, Sep 7, 8:55 PM
zequanwu committed rG3e782bf8090c: [Sema][MSVC] warn at dynamic_cast when /GR- is given (authored by zequanwu).
[Sema][MSVC] warn at dynamic_cast when /GR- is given
Mon, Sep 7, 4:52 PM
zequanwu closed D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Mon, Sep 7, 4:52 PM · Restricted Project

Thu, Sep 3

zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

address comment.

Thu, Sep 3, 1:39 PM · Restricted Project
zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Thu, Sep 3, 11:18 AM · Restricted Project
zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

address comment.

Thu, Sep 3, 11:17 AM · Restricted Project

Wed, Sep 2

zequanwu added inline comments to D84988: [Coverage] Add empty line regions to SkippedRegions.
Wed, Sep 2, 6:34 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

address comment.
emit second segment when first segment is skipped and activeregions is not empty.
I don't know who can be added as reviewers for the preprocessor part

Wed, Sep 2, 6:20 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Wed, Sep 2, 5:19 PM · Restricted Project
zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

warn at dynamic_cast<void*> like MSVC in clang-cl, but not in clang.

Wed, Sep 2, 4:59 PM · Restricted Project
zequanwu added inline comments to D84988: [Coverage] Add empty line regions to SkippedRegions.
Wed, Sep 2, 3:11 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

Emit second segment as wrapped segment only when first segment is RegionEntry

Wed, Sep 2, 3:05 PM · Restricted Project, Restricted Project, Restricted Project

Tue, Sep 1

zequanwu added a comment to D85778: More accurately compute the ranges of possible values for +, -, *, &, %..

Hi, this change seems like hits a false positive case in chromium build: https://bugs.chromium.org/p/chromium/issues/detail?id=1124085

That's not a false positive. The code is (simplified):

int RoundDown(int a, long b) { return a & -b; }

... which is implicitly converting an expression of type long to int, losing precision. For example, RoundDown(-1, 0x1'0000'0000) is -4294967296 prior to the implicit conversion from long to int. Given that the truncation is presumably intentional (0 at least seems like the least-bad answer for rounding down 0 to a multiple of 2^32 as a 32-bit integer), you can suppress the warning with a cast.

Tue, Sep 1, 5:58 PM · Restricted Project
zequanwu added a comment to D85778: More accurately compute the ranges of possible values for +, -, *, &, %..

Hi, this change seems like hits a false positive case in chromium build: https://bugs.chromium.org/p/chromium/issues/detail?id=1124085

Tue, Sep 1, 5:42 PM · Restricted Project

Mon, Aug 31

zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

update.

Mon, Aug 31, 4:52 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added a comment to D84988: [Coverage] Add empty line regions to SkippedRegions.

Friendly ping.

Mon, Aug 31, 1:18 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Mon, Aug 31, 11:54 AM · Restricted Project

Aug 28 2020

zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Aug 28 2020, 11:15 AM · Restricted Project

Aug 27 2020

zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

address comments.

Aug 27 2020, 10:07 AM · Restricted Project
zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Aug 27 2020, 9:24 AM · Restricted Project
zequanwu added a comment to D85176: [Coverage] Enable emitting gap area between macros.

We started seeing assertion failure after rolling a toolchain that contains this change and git bisect identified this change. I have filed a bug with a reproducer as PR47324.

Aug 27 2020, 9:16 AM · Restricted Project

Aug 26 2020

zequanwu reopened D85176: [Coverage] Enable emitting gap area between macros.

Here is a repro of crash caused by this change.

int k, l;
#define m(e) e##e
void p() {
  int kk,ll;
  if (k)
    m(k);
  else
    l = m(l);
}

SM.getExpansLoc(AfterLoc) for m(k) gives location pointing to m(l), which caused the crash.

Aug 26 2020, 5:04 PM · Restricted Project
zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Aug 26 2020, 11:57 AM · Restricted Project
zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

Address cmments.

Aug 26 2020, 11:57 AM · Restricted Project

Aug 25 2020

zequanwu updated the diff for D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.

address comments.

Aug 25 2020, 11:33 PM · Restricted Project
zequanwu added a reverting change for rGa31c89c1b7a0: [Coverage] Enable emitting gap area between macros: rG9500a7209163: Revert "[Coverage] Enable emitting gap area between macros".
Aug 25 2020, 3:29 PM
zequanwu committed rG9500a7209163: Revert "[Coverage] Enable emitting gap area between macros" (authored by zequanwu).
Revert "[Coverage] Enable emitting gap area between macros"
Aug 25 2020, 3:29 PM
zequanwu added a reverting change for D85176: [Coverage] Enable emitting gap area between macros: rG9500a7209163: Revert "[Coverage] Enable emitting gap area between macros".
Aug 25 2020, 3:29 PM · Restricted Project

Aug 24 2020

zequanwu added inline comments to D86170: PrintStackTrace: don't symbolize if LLVM_DISABLE_SYMBOLIZATION is set.
Aug 24 2020, 7:05 PM · Restricted Project
zequanwu accepted D86496: [not][test] Fix disable-symbolization.test when 'printenv' is not available.

LGTM.

Aug 24 2020, 5:06 PM · Restricted Project
zequanwu added inline comments to D86170: PrintStackTrace: don't symbolize if LLVM_DISABLE_SYMBOLIZATION is set.
Aug 24 2020, 4:55 PM · Restricted Project
zequanwu added inline comments to D86170: PrintStackTrace: don't symbolize if LLVM_DISABLE_SYMBOLIZATION is set.
Aug 24 2020, 4:45 PM · Restricted Project
zequanwu added inline comments to D86170: PrintStackTrace: don't symbolize if LLVM_DISABLE_SYMBOLIZATION is set.
Aug 24 2020, 4:29 PM · Restricted Project

Aug 21 2020

zequanwu added inline comments to D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Aug 21 2020, 1:55 PM · Restricted Project
zequanwu requested review of D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given.
Aug 21 2020, 1:42 PM · Restricted Project

Aug 19 2020

zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

minor fix.

Aug 19 2020, 5:03 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added inline comments to D84988: [Coverage] Add empty line regions to SkippedRegions.
Aug 19 2020, 11:41 AM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

Add a wrapped segment at location of zero-length segment with last pushed region count.

Aug 19 2020, 11:38 AM · Restricted Project, Restricted Project, Restricted Project

Aug 18 2020

zequanwu committed rG84fffa672831: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file… (authored by zequanwu).
[Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file…
Aug 18 2020, 1:27 PM
zequanwu closed D86116: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file as regions' {start, end}Loc.
Aug 18 2020, 1:26 PM · Restricted Project
zequanwu updated the diff for D86116: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file as regions' {start, end}Loc.

Minor fix on test case.

Aug 18 2020, 1:25 PM · Restricted Project
zequanwu added a comment to D85036: [llvm-cov] reset executation count to 0 after wrapped segment.
In D85036#2224279, @vsk wrote:
In D85036#2222873, @vsk wrote:

Hi @zequanwu, we've started seeing a regression due to this change in swift (https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/7818/consoleText, see the bit about 'coverage_smoke.swift'). Here's the failure we see:

           165:  165| 2| throw CustomError.Err // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
check:167'0            X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
check:167'1                                                                          with "@LINE" equal to "167"
check:167'2                                                                   ?      possible intended match
           166:  166| 2| } catch {
check:167'0     ~~~~~~~~~~~~~~~~~~
           167:  167| 1| if b { // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
check:167'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           168:  168| 1| return 1 // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}1
check:167'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           169:  169| 1| }

The issue is that line 167 ("if b {") has an execution count of 1, even though it's actually reached twice. Before this change, the wrapped count from the catch block starting on line 166 would apply on line 167, all the way through the IfStmt condition (giving an execution count of 2). But with this change, seeing as the IfStmt condition doesn't get its own region, the line execution count gets picked up from the "Then" side of the IfStmt.

I'm concerned that this nested statements generated in clang as well as swift. Do you have some idea about how to address this?

I wonder whether we can make use of gap regions to fix the clang bugs you were looking at. I noticed that this fixed an issue in instrprof-comdat.h, but it's possible that that was caused by a bug in clang that has since been fixed (per the note).

Hi, I don't know how swift generate the regions. Shouldn't IfStmt gets its own regions covering from if keyword to thenStmt? Is that intended not to do so in swift?

Thanks for taking a look. Clang and Swift both handle IfStmt in the same way, i.e. they both extend the parent region of the IfStmt through the condition. In clang, this is:

void VisitIfStmt(const IfStmt *S) {
  extendRegion(S);
  if (S->getInit())
    Visit(S->getInit());

  // Extend into the condition before we propagate through it below - this is
  // needed to handle macros that generate the "if" but not the condition.
  extendRegion(S->getCond());
  ...

I think this behavior is reasonable (it makes sense to say that the condition of an IfStmt is executed exactly as many times as the parent region containing the IfStmt).

My original thought about this is for one line if there is any new line segment which is start of region, we should ignore the wrapped line segment, and the new line segment count should be the accurate one. Do you have dumping result of segments (-debug-only=coverage-mapping) so we can take a deeper look?

Right, I thought I was on the same page as you about this. After paging a bit more of this in, I think a problem with ignoring the wrapped line segment is that it must have originated from a region that hasn't actually ended. If a line contains a new start-of-region line segment, it may well begin very late (e.g. at the end of the line), in which case it seems appropriate to consider using the count from the parent region (the wrapped count).

Here is the swift example in more detail:

func catchError2(_ b: Bool) -> Int {
  do {
    throw CustomError.Err // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
  } catch { // <<< Line 165 >>>
    if b {                // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
      return 1            // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}1
    }
  }
  return 0                // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}1
}
let _ = catchError2(true)
let _ = catchError2(false)
...
Counter in file 0 165:11 -> 169:4, #1
Counter in file 0 166:10 -> 168:6, #2
Counter in file 0 168:6 -> 169:4, (#1 - #2)
...
  163:6 -> 165:4 (count=2)
  165:11 -> 169:4 (count=2)
  166:10 -> 168:6 (count=1)
  168:6 -> 169:4 (count=1)

So, the "catch" on line 165 gets count=2. We then go to line 166 with wrapped-count=2. But we end up applying the start-of-region count on line 166, 166:10 -> 168:6 (count=1), which seems off.

Aug 18 2020, 10:44 AM · Restricted Project

Aug 17 2020

zequanwu added a comment to D85036: [llvm-cov] reset executation count to 0 after wrapped segment.
In D85036#2222873, @vsk wrote:

Hi @zequanwu, we've started seeing a regression due to this change in swift (https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/7818/consoleText, see the bit about 'coverage_smoke.swift'). Here's the failure we see:

           165:  165| 2| throw CustomError.Err // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
check:167'0            X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
check:167'1                                                                          with "@LINE" equal to "167"
check:167'2                                                                   ?      possible intended match
           166:  166| 2| } catch {
check:167'0     ~~~~~~~~~~~~~~~~~~
           167:  167| 1| if b { // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}2
check:167'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           168:  168| 1| return 1 // CHECK-COV: {{ *}}[[@LINE]]|{{ *}}1
check:167'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           169:  169| 1| }

The issue is that line 167 ("if b {") has an execution count of 1, even though it's actually reached twice. Before this change, the wrapped count from the catch block starting on line 166 would apply on line 167, all the way through the IfStmt condition (giving an execution count of 2). But with this change, seeing as the IfStmt condition doesn't get its own region, the line execution count gets picked up from the "Then" side of the IfStmt.

I'm concerned that this nested statements generated in clang as well as swift. Do you have some idea about how to address this?

I wonder whether we can make use of gap regions to fix the clang bugs you were looking at. I noticed that this fixed an issue in instrprof-comdat.h, but it's possible that that was caused by a bug in clang that has since been fixed (per the note).

Aug 17 2020, 5:56 PM · Restricted Project
zequanwu updated the diff for D86116: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file as regions' {start, end}Loc.

Update test.
The bug is when start or end location skipped regions has the same spelling line number as PrevTokLoc or NextTokLoc but in different files.
In the test case, end location of skipped regions is in line 6 and PreTokLoc is also in line 6, but since they are in different files, it shouldn't be shrinked.

Aug 17 2020, 3:59 PM · Restricted Project
zequanwu requested review of D86116: [Coverage] Adjust skipped regions only if {Prev,Next}TokLoc is in the same file as regions' {start, end}Loc.
Aug 17 2020, 3:54 PM · Restricted Project
zequanwu added inline comments to D84988: [Coverage] Add empty line regions to SkippedRegions.
Aug 17 2020, 1:12 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added inline comments to D84988: [Coverage] Add empty line regions to SkippedRegions.
Aug 17 2020, 1:09 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu updated the diff for D84988: [Coverage] Add empty line regions to SkippedRegions.

Address comments.

Aug 17 2020, 1:09 PM · Restricted Project, Restricted Project, Restricted Project
zequanwu added a comment to D84988: [Coverage] Add empty line regions to SkippedRegions.
In D84988#2221805, @vsk wrote:

Hi @zequanwu, are you looking for review for this patch? I wasn't sure because of the WIP label.

Aug 17 2020, 10:09 AM · Restricted Project, Restricted Project, Restricted Project