Skip to content

[NFC][IR2Vec] Exposing helpers in IR2Vec Vocabulary #147841

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

Open
wants to merge 1 commit into
base: users/svkeerthy/07-07-_nfc_ir2vec_minor_refactoring_of_opcode_access_in_vocabulary
Choose a base branch
from

Conversation

svkeerthy
Copy link
Contributor

@svkeerthy svkeerthy commented Jul 9, 2025

Minor refactoring IR2Vec vocabulary. This would help in upcoming PRs related to the IR2Vec tool.

(Tracking issue - #141817)

@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-mlgo

Author: S. VenkataKeerthy (svkeerthy)

Changes

Minor refactoring IR2Vec vocabulary. This would help in upcoming PRs related to the IR2Vec tool.

(Tracking issue - #141817)


Full diff: https://github.com/llvm/llvm-project/pull/147841.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IR2Vec.h (+8-8)
  • (modified) llvm/lib/Analysis/IR2Vec.cpp (+2-1)
diff --git a/llvm/include/llvm/Analysis/IR2Vec.h b/llvm/include/llvm/Analysis/IR2Vec.h
index 176cdaf7b5378..4209cf8d476b9 100644
--- a/llvm/include/llvm/Analysis/IR2Vec.h
+++ b/llvm/include/llvm/Analysis/IR2Vec.h
@@ -163,6 +163,14 @@ class Vocabulary {
   static constexpr unsigned MaxOperandKinds =
       static_cast<unsigned>(OperandKind::MaxOperandKind);
 
+public:
+  Vocabulary() = default;
+  Vocabulary(VocabVector &&Vocab);
+
+  bool isValid() const;
+  unsigned getDimension() const;
+  size_t size() const;
+
   /// Helper function to get vocabulary key for a given Opcode
   static StringRef getVocabKeyForOpcode(unsigned Opcode);
 
@@ -175,14 +183,6 @@ class Vocabulary {
   /// Helper function to classify an operand into OperandKind
   static OperandKind getOperandKind(const Value *Op);
 
-public:
-  Vocabulary() = default;
-  Vocabulary(VocabVector &&Vocab);
-
-  bool isValid() const;
-  unsigned getDimension() const;
-  size_t size() const;
-
   /// Accessors to get the embedding for a given entity.
   const ir2vec::Embedding &operator[](unsigned Opcode) const;
   const ir2vec::Embedding &operator[](Type::TypeID TypeId) const;
diff --git a/llvm/lib/Analysis/IR2Vec.cpp b/llvm/lib/Analysis/IR2Vec.cpp
index f97644b93a3d4..d1eb709012a57 100644
--- a/llvm/lib/Analysis/IR2Vec.cpp
+++ b/llvm/lib/Analysis/IR2Vec.cpp
@@ -518,7 +518,8 @@ IR2VecVocabAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
   // Otherwise, try to read from the vocabulary file.
   if (VocabFile.empty()) {
     // FIXME: Use default vocabulary
-    Ctx->emitError("IR2Vec vocabulary file path not specified");
+    Ctx->emitError("IR2Vec vocabulary file path not specified; You may need to "
+                   "set it using --ir2vec-vocab-path");
     return Vocabulary(); // Return invalid result
   }
   if (auto Err = readVocabulary()) {

@@ -163,6 +163,14 @@ class Vocabulary {
static constexpr unsigned MaxOperandKinds =
static_cast<unsigned>(OperandKind::MaxOperandKind);

public:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of moving these declarations? Keep them in the same order but also make the getVocabKeyFor* functions part of the public interface?

@@ -518,7 +518,8 @@ IR2VecVocabAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
// Otherwise, try to read from the vocabulary file.
if (VocabFile.empty()) {
// FIXME: Use default vocabulary
Ctx->emitError("IR2Vec vocabulary file path not specified");
Ctx->emitError("IR2Vec vocabulary file path not specified; You may need to "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really have anything to do with the patch (based on the description) and shouldn't really be here, but separating it out is probably too much of a hassle for what it is. Leaving it should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding mlgo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants