[IR] Ignore globals with the `llvm.` prefix when calculating module hash

[paulkirth]

Summary

This came up in This came up in
https://reviews.llvm.org/D146776#inline-1489091 and is slightly related
to https://reviews.llvm.org/D153855. In both patches, there is the
observation that some modifications of the module should not invalidate
analysis, such as when adding a declaration or some metadata the won't
be used when compiling the current module.

This patch implements the suggestion that we should ignore globals that have
the llvm. prefix when calculating the module hash.

Fixes https://github.com/llvm/llvm-project/issues/63590

[aeubanks]

llvm/unittests/IR/PassManagerTest.cpp

1090 ↗

(On Diff #535566)

I've been meaning to write some unittests that specifically just check (in)equality of llvm::StructuralHash of two modules so we can avoid doing that. Going through a pass manager shouldn't be necessary. I can do that, or if you have time you can start that :)

[paulkirth]

llvm/unittests/IR/PassManagerTest.cpp

1090 ↗

(On Diff #535566)

I'm going on vacation starting tomorrow,so I'm not sure I'll have time to get to it until after that.

But, just in case I do scrounge the time, are there other important cases you know of of the top of your head?

[aeubanks]

llvm/unittests/IR/PassManagerTest.cpp

1090 ↗

(On Diff #535566)

it's basically just all the cases in StructuralHashImpl, there aren't that many right now. it would be good to get some tests out so it's easy to test future changes

[paulkirth]

Rebase. I will update the test code after D154308 lands.

[paulkirth]

As discussed I will rebase this on top of D154308, and update the tests accordingly.

[paulkirth]

Rebase and update test.

[aeubanks]

lgtm

llvm/lib/IR/StructuralHash.cpp

while (!BBs.empty()) {
           continue;
         BBs.push_back(Term->getSuccessor(i));
       }
     }
   }
 
   void update(const GlobalVariable &GV) {
     // Declarations and used/compiler.used don't affect analyses.
-    // Same for llvm.embedded.object, which is always a metadata section.
-    if (GV.isDeclaration() ||
-        GV.getName() == "llvm.compiler.used" || GV.getName() == "llvm.used" ||
-        GV.getName() == "llvm.embedded.object")
+    // Since there are several `llvm.*` metadata, like `llvm.embedded.object`,
+    // we ignore anything with the `.llvm` prefix
+    if (GV.isDeclaration() || GV.getName().starts_with("llvm."))
       return;
     hash(23456); // Global header
     hash(GV.getValueType()->getTypeID());
   }
 
   void update(const Module &M) {
     for (const GlobalVariable &GV : M.globals())
       update(GV);

llvm/unittests/IR/StructuralHashTest.cpp

 //===- llvm/unittest/IR/StructuralHashTest.cpp ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/StructuralHash.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
+#include <memory>
+
 using namespace llvm;
 
 namespace {
 
 std::unique_ptr<Module> parseIR(LLVMContext &Context, const char *IR) {
   SMDiagnostic Err;
   std::unique_ptr<Module> M = parseAssemblyString(IR, Err, Context);
   if (!M)
TEST(StructuralHashTest, InstructionType) {
   std::unique_ptr<Module> M2 = parseIR(Ctx, "define void @f(ptr %p) {\n"
                                             "  %a = load i64, ptr %p\n"
                                             "  ret void\n"
                                             "}\n");
   // FIXME: should be different
   EXPECT_EQ(StructuralHash(*M1), StructuralHash(*M2));
 }
 
+TEST(StructuralHashTest, IgnoredMetadata) {
+  LLVMContext Ctx;
+  std::unique_ptr<Module> M1 = parseIR(Ctx, "@a = global i32 1\n");
+  // clang-format off
+  std::unique_ptr<Module> M2 = parseIR(
+      Ctx, R"(
+        @a = global i32 1
+        @llvm.embedded.object = private constant [4 x i8] c"BC\C0\00", section ".llvm.lto", align 1, !exclude !0
+        @llvm.compiler.used = appending global [1 x ptr] [ptr @llvm.embedded.object], section "llvm.metadata"
+
+        !llvm.embedded.objects = !{!1}
+
+        !0 = !{}
+        !1 = !{ptr @llvm.embedded.object, !".llvm.lto"}
+        )");
+  EXPECT_EQ(StructuralHash(*M1), StructuralHash(*M2));
+}
 } // end anonymous namespace