Skip to content

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Sep 5, 2025

It is tied to the vocab having had been set. Checking that vector's emtpy is sufficient. Less state to track (for a maintainer)

It is tied to the vocab having had been set. Checking that vector's `emtpy`
is sufficient. Less state to track (for a maintainer)
@mtrofin mtrofin requested a review from svkeerthy September 5, 2025 15:52
@llvmbot llvmbot added mlgo llvm:analysis Includes value tracking, cost tables and constant folding labels Sep 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Mircea Trofin (mtrofin)

Changes

It is tied to the vocab having had been set. Checking that vector's emtpy is sufficient. Less state to track (for a maintainer)


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

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IR2Vec.h (+6-7)
  • (modified) llvm/lib/Analysis/IR2Vec.cpp (+1-8)
diff --git a/llvm/include/llvm/Analysis/IR2Vec.h b/llvm/include/llvm/Analysis/IR2Vec.h
index b7b881999241e..82e2f396388e8 100644
--- a/llvm/include/llvm/Analysis/IR2Vec.h
+++ b/llvm/include/llvm/Analysis/IR2Vec.h
@@ -164,7 +164,6 @@ class Vocabulary {
   friend class llvm::IR2VecVocabAnalysis;
   using VocabVector = std::vector<ir2vec::Embedding>;
   VocabVector Vocab;
-  bool Valid = false;
 
 public:
   // Slot layout:
@@ -210,9 +209,9 @@ class Vocabulary {
       static_cast<unsigned>(OperandKind::MaxOperandKind);
 
   Vocabulary() = default;
-  LLVM_ABI Vocabulary(VocabVector &&Vocab);
+  LLVM_ABI Vocabulary(VocabVector &&Vocab) : Vocab(std::move(Vocab)) {}
 
-  LLVM_ABI bool isValid() const;
+  LLVM_ABI bool isValid() const { return !Vocab.empty(); };
   LLVM_ABI unsigned getDimension() const;
   /// Total number of entries (opcodes + canonicalized types + operand kinds)
   static constexpr size_t getCanonicalSize() { return NumCanonicalEntries; }
@@ -243,22 +242,22 @@ class Vocabulary {
   /// Const Iterator type aliases
   using const_iterator = VocabVector::const_iterator;
   const_iterator begin() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.begin();
   }
 
   const_iterator cbegin() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.cbegin();
   }
 
   const_iterator end() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.end();
   }
 
   const_iterator cend() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.cend();
   }
 
diff --git a/llvm/lib/Analysis/IR2Vec.cpp b/llvm/lib/Analysis/IR2Vec.cpp
index 98849fd922843..99afc0601d523 100644
--- a/llvm/lib/Analysis/IR2Vec.cpp
+++ b/llvm/lib/Analysis/IR2Vec.cpp
@@ -260,15 +260,8 @@ void FlowAwareEmbedder::computeEmbeddings(const BasicBlock &BB) const {
 // Vocabulary
 //===----------------------------------------------------------------------===//
 
-Vocabulary::Vocabulary(VocabVector &&Vocab)
-    : Vocab(std::move(Vocab)), Valid(true) {}
-
-bool Vocabulary::isValid() const {
-  return Vocab.size() == NumCanonicalEntries && Valid;
-}
-
 unsigned Vocabulary::getDimension() const {
-  assert(Valid && "IR2Vec Vocabulary is invalid");
+  assert(isValid() && "IR2Vec Vocabulary is invalid");
   return Vocab[0].size();
 }
 

@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-mlgo

Author: Mircea Trofin (mtrofin)

Changes

It is tied to the vocab having had been set. Checking that vector's emtpy is sufficient. Less state to track (for a maintainer)


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

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IR2Vec.h (+6-7)
  • (modified) llvm/lib/Analysis/IR2Vec.cpp (+1-8)
diff --git a/llvm/include/llvm/Analysis/IR2Vec.h b/llvm/include/llvm/Analysis/IR2Vec.h
index b7b881999241e..82e2f396388e8 100644
--- a/llvm/include/llvm/Analysis/IR2Vec.h
+++ b/llvm/include/llvm/Analysis/IR2Vec.h
@@ -164,7 +164,6 @@ class Vocabulary {
   friend class llvm::IR2VecVocabAnalysis;
   using VocabVector = std::vector<ir2vec::Embedding>;
   VocabVector Vocab;
-  bool Valid = false;
 
 public:
   // Slot layout:
@@ -210,9 +209,9 @@ class Vocabulary {
       static_cast<unsigned>(OperandKind::MaxOperandKind);
 
   Vocabulary() = default;
-  LLVM_ABI Vocabulary(VocabVector &&Vocab);
+  LLVM_ABI Vocabulary(VocabVector &&Vocab) : Vocab(std::move(Vocab)) {}
 
-  LLVM_ABI bool isValid() const;
+  LLVM_ABI bool isValid() const { return !Vocab.empty(); };
   LLVM_ABI unsigned getDimension() const;
   /// Total number of entries (opcodes + canonicalized types + operand kinds)
   static constexpr size_t getCanonicalSize() { return NumCanonicalEntries; }
@@ -243,22 +242,22 @@ class Vocabulary {
   /// Const Iterator type aliases
   using const_iterator = VocabVector::const_iterator;
   const_iterator begin() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.begin();
   }
 
   const_iterator cbegin() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.cbegin();
   }
 
   const_iterator end() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.end();
   }
 
   const_iterator cend() const {
-    assert(Valid && "IR2Vec Vocabulary is invalid");
+    assert(isValid() && "IR2Vec Vocabulary is invalid");
     return Vocab.cend();
   }
 
diff --git a/llvm/lib/Analysis/IR2Vec.cpp b/llvm/lib/Analysis/IR2Vec.cpp
index 98849fd922843..99afc0601d523 100644
--- a/llvm/lib/Analysis/IR2Vec.cpp
+++ b/llvm/lib/Analysis/IR2Vec.cpp
@@ -260,15 +260,8 @@ void FlowAwareEmbedder::computeEmbeddings(const BasicBlock &BB) const {
 // Vocabulary
 //===----------------------------------------------------------------------===//
 
-Vocabulary::Vocabulary(VocabVector &&Vocab)
-    : Vocab(std::move(Vocab)), Valid(true) {}
-
-bool Vocabulary::isValid() const {
-  return Vocab.size() == NumCanonicalEntries && Valid;
-}
-
 unsigned Vocabulary::getDimension() const {
-  assert(Valid && "IR2Vec Vocabulary is invalid");
+  assert(isValid() && "IR2Vec Vocabulary is invalid");
   return Vocab[0].size();
 }
 

Copy link
Contributor

@svkeerthy svkeerthy left a comment

Choose a reason for hiding this comment

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

Yes, makes sense. Thanks!

Copy link
Member Author

mtrofin commented Sep 6, 2025

Merge activity

  • Sep 6, 1:07 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Sep 6, 1:09 AM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Stack merges are not currently supported for forked repositories. Please create a branch in the target repository in order to merge).

@mtrofin mtrofin merged commit 96c1201 into llvm:main Sep 7, 2025
9 checks passed
@mtrofin mtrofin deleted the ir2vec branch September 7, 2025 18:26
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.

4 participants