-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[IR2Vec] Add embeddings mode to llvm-ir2vec tool #147844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IR2Vec] Add embeddings mode to llvm-ir2vec tool #147844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Premerge failures here also look relevant.
7c4d86d
to
5f1f3fe
Compare
bf757c0
to
684d298
Compare
✅ With the latest revision this PR passed the Python code formatter. |
2d88b38
to
6fd2dca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style nits, otherwise LGTM.
@@ -34,7 +42,7 @@ | |||
#include "llvm/Support/raw_ostream.h" | |||
|
|||
using namespace llvm; | |||
using namespace ir2vec; | |||
using namespace llvm::ir2vec; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using
statements, it might be better to wrap everything outside of main
in an anonymous namespace inside the llvm::ir2vec
namespace. I'm not sure what the coding standards are, but that's the pattern I see in other tools like llvm-exegesis
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
// Generate embeddings based on the specified level | ||
switch (Level) { | ||
case FunctionLevel: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does clang-format
not let you indent here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wierdly yes!
6fd2dca
to
4e92c2b
Compare
5f1f3fe
to
744b38b
Compare
4e92c2b
to
f975249
Compare
744b38b
to
51b0120
Compare
ab12375
to
a3b518b
Compare
51b0120
to
e931cf1
Compare
e931cf1
to
0f1720f
Compare
f2498dc
to
7b801df
Compare
0f1720f
to
52ec5db
Compare
52ec5db
to
36fe251
Compare
7b801df
to
df6bdef
Compare
36fe251
to
47d402c
Compare
df6bdef
to
0ee74a8
Compare
f4181fd
to
7f45a74
Compare
0ee74a8
to
c0360c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Merge activity
|
7f45a74
to
74e3b78
Compare
) Add a new LLVM tool `llvm-ir2vec`. This tool is primarily intended to generate triplets for training the vocabulary (#141834) and to potentially generate the embeddings in a stand alone manner. This PR introduces the tool with triplet generation functionality. In the upcoming PRs I'll add scripts under `utils/mlgo` to complete the vocabulary tooling. #147844 adds embedding generation logic to the tool. (Tracking issue - #141817)
c0360c7
to
537495c
Compare
Add embedding generation functionality to the llvm-ir2vec tool, complementing the existing triplet generation mode.
This change completes the IR2Vec tool by adding the embedding generation functionality, which was previously mentioned as a TODO item. The tool now supports both triplet generation for vocabulary training and embedding generation using a trained vocabulary.