ABataev added a comment. In D98135#2651193 <https://reviews.llvm.org/D98135#2651193>, @lxfind wrote:
> 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. Ok, now I see. I think you can restore this patch, it should be enough to fix the problem. Just check that `S` is not `nullptr` before calling `CGF.incrementProfileCounter(S);`. 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
