This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Split DecisionForest Evaluate() into one func per tree.
ClosedPublic

Authored by usaxena95 on Sep 30 2020, 12:45 AM.

Details

Summary

This allows us MSAN to instrument this function. Previous version is not
instrumentable due to it shear volume.

Diff Detail

Event Timeline

usaxena95 created this revision.Sep 30 2020, 12:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2020, 12:45 AM
usaxena95 requested review of this revision.Sep 30 2020, 12:45 AM
usaxena95 updated this revision to Diff 295284.Sep 30 2020, 7:49 AM

Minor formatting fixes.

usaxena95 added a subscriber: kbobyrev.
adamcz added inline comments.Sep 30 2020, 10:42 AM
clang-tools-extra/clangd/quality/CompletionModelCodegen.py
168

Why are these member functions? Why not keep them in .cc file only, in anonymous namespace?

I wonder if that will make compiler inline them and then bring back msan issues though.

usaxena95 added inline comments.Sep 30 2020, 11:24 AM
clang-tools-extra/clangd/quality/CompletionModelCodegen.py
168

Why are these member functions? Why not keep them in .cc file only, in anonymous namespace?

We will need to add getters for member variables (hoping they would be inlined).
SG ?
(Or make the members public o_O ?)

I wonder if that will make compiler inline them and then bring back MSAN issues though.

I think we can disable that using LLVM_ATTRIBUTE_NOINLINE

Addressed comments: Move the evaluate functions to anonymous namespace.

usaxena95 marked an inline comment as done.Sep 30 2020, 11:25 PM
adamcz accepted this revision.Oct 1 2020, 8:51 AM
This revision is now accepted and ready to land.Oct 1 2020, 8:51 AM