lxfind added a comment.

In D98135#2650940 <https://reviews.llvm.org/D98135#2650940>, @ABataev wrote:

> In D98135#2650914 <https://reviews.llvm.org/D98135#2650914>, @lxfind wrote:
>
>> @ABataev, wondering if you have a timeline on this?
>> Missing counters from OMP functions sometimes cause llvm-cov to crash 
>> because of data inconsistency.
>
> Cannot answer right now. It would be much easier to fix this if you could 
> provide additional info about what tests are exactly failed, what are the 
> constructs that do not support it, etc.

Yes the whole pipeline is a bit long and complex, so I don't have an exact 
repro in hand because it requires source code and run it.

But let me try to explain what happened in my observation. There are two 
sections that are related to this issue in the binary, the IPSK_covfun section 
that contains the function records, and the IPSK_name section that contains the 
list of all function names. The issue here is that some OMP functions that are 
found in the IPSK_covfun section are not found in the IPSK_name section.

The records in IPSK_covfun are generated like this:

Whenever CodeGenFunction is generating code for any function, it will first 
call the `CodeGenFunction::GenerateCode()` function, in which it will call 
`PGO.assignRegionCounters(GD, CurFn);`: 
https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/clang/lib/CodeGen/CodeGenFunction.cpp#L1329

From there, it will call `emitCounterRegionMapping`:
https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/clang/lib/CodeGen/CodeGenPGO.cpp#L819

which will then call: `CGM.getCoverageMapping()->addFunctionMappingRecord`:
https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/clang/lib/CodeGen/CodeGenPGO.cpp#L890

which will eventually add this function to a `FunctionRecords`:
https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/clang/lib/CodeGen/CoverageMappingGen.cpp#L1655

So all above is to show that every function will eventually be added to 
`FunctionRecords`, unless in the case where a function is explicitly marked as 
unused. The `FunctionRecords` will eventually be all written into the 
`IPSK_covfun` section in the binary.

The names in IPSK_name section are generated like this:
Within InstrProfiling.cpp 
(https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp),
 it collects all the function names referenced in all instrumentation counter 
increments instructions:
https://github.com/llvm/llvm-project/blob/e5f51fdd650c6d20c81fedb8e856e9858aa10991/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp#L920
Basically for every InstrProfIncrementInst, it adds the function name 
referenced in it to a list `ReferencedNames`.
Then in the end this `ReferencedNames` is written into the IPSK_name section.

Now you can see that for the OMP functions  that don't have any counters, they 
are still added to `FunctionRecords`, but not added to `ReferencedNames`, 
because they are not referenced by any InstrProfIncrementInst.
During the running of llvm-cov, when reading the list of function records, it 
will attempt to look up the name of the function from the function name list:
https://github.com/llvm/llvm-project/blob/b9ff67099ad6da931976e66f1510c5af2558a86e/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp#L560

And it will not be able to find it for the OMP case, so it will return an error.

Overall this is very complex and a bit fragile to me. For instance, we probably 
could have detected the error much earlier during Instrumentation pass in LLVM, 
that some function records' names are not in the name list. Or we could simply 
construct the list of function names based on the function records. But 
currently these two are generated independently.
cc @MaskRay and @vsk, maybe they have thoughts on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98135/new/

https://reviews.llvm.org/D98135

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to