-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[IR2Vec] Adding tests to check multiple invocations of getFunctionVector()
and getInstVecMap()
return same results
#162365
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
Conversation
@llvm/pr-subscribers-mlgo @llvm/pr-subscribers-llvm-analysis Author: S. VenkataKeerthy (svkeerthy) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162365.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/IR2Vec.cpp b/llvm/lib/Analysis/IR2Vec.cpp
index 295b6d33525d9..688535161d4b9 100644
--- a/llvm/lib/Analysis/IR2Vec.cpp
+++ b/llvm/lib/Analysis/IR2Vec.cpp
@@ -200,6 +200,8 @@ void Embedder::computeEmbeddings() const {
if (F.isDeclaration())
return;
+ FuncVector = Embedding(Dimension, 0.0);
+
// Consider only the basic blocks that are reachable from entry
for (const BasicBlock *BB : depth_first(&F)) {
computeEmbeddings(*BB);
diff --git a/llvm/unittests/Analysis/IR2VecTest.cpp b/llvm/unittests/Analysis/IR2VecTest.cpp
index d136cb6a316b1..ee14596bcb8ca 100644
--- a/llvm/unittests/Analysis/IR2VecTest.cpp
+++ b/llvm/unittests/Analysis/IR2VecTest.cpp
@@ -430,6 +430,60 @@ TEST_F(IR2VecTestFixture, GetFunctionVector_FlowAware) {
EXPECT_TRUE(FuncVec.approximatelyEquals(Embedding(2, 58.1)));
}
+TEST_F(IR2VecTestFixture, MultipleComputeEmbeddingsConsistency_Symbolic) {
+ auto Emb = Embedder::create(IR2VecKind::Symbolic, *F, *V);
+ ASSERT_TRUE(static_cast<bool>(Emb));
+
+ // Get initial function vector
+ const auto &FuncVec1 = Emb->getFunctionVector();
+
+ // Compute embeddings again by calling getFunctionVector multiple times
+ const auto &FuncVec2 = Emb->getFunctionVector();
+ const auto &FuncVec3 = Emb->getFunctionVector();
+
+ // All function vectors should be identical
+ EXPECT_TRUE(FuncVec1.approximatelyEquals(FuncVec2));
+ EXPECT_TRUE(FuncVec1.approximatelyEquals(FuncVec3));
+ EXPECT_TRUE(FuncVec2.approximatelyEquals(FuncVec3));
+
+ // Also check that instruction vectors remain consistent
+ const auto &InstMap1 = Emb->getInstVecMap();
+ const auto &InstMap2 = Emb->getInstVecMap();
+
+ EXPECT_EQ(InstMap1.size(), InstMap2.size());
+ for (const auto &[Inst, Vec1] : InstMap1) {
+ ASSERT_TRUE(InstMap2.count(Inst));
+ EXPECT_TRUE(Vec1.approximatelyEquals(InstMap2.at(Inst)));
+ }
+}
+
+TEST_F(IR2VecTestFixture, MultipleComputeEmbeddingsConsistency_FlowAware) {
+ auto Emb = Embedder::create(IR2VecKind::FlowAware, *F, *V);
+ ASSERT_TRUE(static_cast<bool>(Emb));
+
+ // Get initial function vector
+ const auto &FuncVec1 = Emb->getFunctionVector();
+
+ // Compute embeddings again by calling getFunctionVector multiple times
+ const auto &FuncVec2 = Emb->getFunctionVector();
+ const auto &FuncVec3 = Emb->getFunctionVector();
+
+ // All function vectors should be identical
+ EXPECT_TRUE(FuncVec1.approximatelyEquals(FuncVec2));
+ EXPECT_TRUE(FuncVec1.approximatelyEquals(FuncVec3));
+ EXPECT_TRUE(FuncVec2.approximatelyEquals(FuncVec3));
+
+ // Also check that instruction vectors remain consistent
+ const auto &InstMap1 = Emb->getInstVecMap();
+ const auto &InstMap2 = Emb->getInstVecMap();
+
+ EXPECT_EQ(InstMap1.size(), InstMap2.size());
+ for (const auto &[Inst, Vec1] : InstMap1) {
+ ASSERT_TRUE(InstMap2.count(Inst));
+ EXPECT_TRUE(Vec1.approximatelyEquals(InstMap2.at(Inst)));
+ }
+}
+
static constexpr unsigned MaxOpcodes = Vocabulary::MaxOpcodes;
[[maybe_unused]]
static constexpr unsigned MaxTypeIDs = Vocabulary::MaxTypeIDs;
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
Given the tests only differ by a single enum value, this seems like a good case for use of a value-parameterized test. https://google.github.io/googletest/advanced.html#value-parameterized-tests
Thanks. Will do it for all the applicable tests together in a separate patch. There are many such instances in IR2VecTest. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/16890 Here is the relevant piece of the build log for the reference
|
Tests for #162165. Missed it earlier.