[StructuralHash] Track global variables

[nikic]

LGTM

[uabelho]

With this patch the following started failing:

opt -passes=strip-dead-prototypes bbi-83491.ll -o /dev/null -verify-analysis-invalidation

It looks like strip-dead-prototypes has removed

@.str = external constant [15 x i16]

Is that what the change aimed to catch? Should I write an issue about it?

bbi-83491.ll789 BDownload

[nikic]

The hashing is skipping function declarations, so maybe it would make sense to skip global declarations as well?

[aeubanks]

yes skipping global declarations makes sense

[uabelho]

In D149209#4417787, @aeubanks wrote:

yes skipping global declarations makes sense

I made https://reviews.llvm.org/D153855 for that.

llvm/lib/IR/StructuralHash.cpp

 //===-- StructuralHash.cpp - IR Hashing -------------------------*- C++ -*-===//
 //
 // 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/IR/Function.h"
+#include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Module.h"
 
 using namespace llvm;
 
 namespace {
 
 // Basic hashing mechanism to detect structural change to the IR, used to verify
 // pass return status consistency with actual change. Loosely copied from
 // llvm/lib/Transforms/Utils/FunctionComparator.cpp
 
 class StructuralHashImpl {
   hash_code Hash;
 
   template <typename T> void hash(const T &V) { Hash = hash_combine(Hash, V); }
 
 public:
   StructuralHashImpl() : Hash(4) {}
 
   void update(const Function &F) {
-    if (F.empty())
+    // Declarations don't affect analyses.
+    if (F.isDeclaration())
       return;
 
+    hash(12345); // Function header
+
     hash(F.isVarArg());
     hash(F.arg_size());
 
     SmallVector<const BasicBlock *, 8> BBs;
     SmallPtrSet<const BasicBlock *, 16> VisitedBBs;
 
     BBs.push_back(&F.getEntryBlock());
     VisitedBBs.insert(BBs[0]);
     while (!BBs.empty()) {
       const BasicBlock *BB = BBs.pop_back_val();
       hash(45798); // Block header
       for (auto &Inst : *BB)
         hash(Inst.getOpcode());
 
       const Instruction *Term = BB->getTerminator();
       for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
         if (!VisitedBBs.insert(Term->getSuccessor(i)).second)
           continue;
         BBs.push_back(Term->getSuccessor(i));
       }
     }
   }
 
+  void update(const GlobalVariable &GV) {
+    // used/compiler.used don't affect analyses.
+    if (GV.getName() == "llvm.compiler.used" || GV.getName() == "llvm.used")
+      return;
+    hash(23456); // Global header
+    hash(GV.getValueType()->getTypeID());
+  }
+
   void update(const Module &M) {
+    for (const GlobalVariable &GV : M.globals())
+      update(GV);
     for (const Function &F : M)
       update(F);
   }
 
   uint64_t getHash() const { return Hash; }
 };
 
 } // namespace