[IR] Don't verify module flags on every access (llvm#102153)

8b4306c introduced validity checks for every module flag access,
because the auto-upgrader uses named metadata before verifying the
module.

This causes overhead for all other accesses, and the check is, in fact,
only need at that single place. Change the upgrader to be careful when
accessing module flags before the module is verified and remove the
checks on all other occasions.

There are two tangential optimizations included: first, when querying a
specific flag, don't enumerate all other flags into a vector as well.
Second, don't use a Twine for getNamedMetadata(), which has
materialization overhead -- all call sites use simple strings that can
be implicitly converted to a StringRef.

(cherry picked from commit b7cd564)

To minimize the diff I just copied the "checked" version of
getDebugMetadataVersionFromModule that was inlined into UpgradeDebugInfo
in the upstream patch. One could also delay the checks that rely on the
version in the Verifier until after we've already verified the module
flags metadata, but that seemed needlessly complicated.

Change-Id: Id98ec158770d5b3e839e3de1c558f507c28cc82c

Author: aengelke

llvm/include/llvm/IR/Module.h

@@ -158,11 +158,6 @@ class LLVM_EXTERNAL_VISIBILITY Module {
   /// converted result in MFB.
   static bool isValidModFlagBehavior(Metadata *MD, ModFlagBehavior &MFB);
 
-  /// Check if the given module flag metadata represents a valid module flag,
-  /// and store the flag behavior, the key string and the value metadata.
-  static bool isValidModuleFlag(const MDNode &ModFlag, ModFlagBehavior &MFB,
-                                MDString *&Key, Metadata *&Val);
-
   struct ModuleFlagEntry {
     ModFlagBehavior Behavior;
     MDString *Key;
@@ -505,7 +500,7 @@ class LLVM_EXTERNAL_VISIBILITY Module {
 
   /// Return the first NamedMDNode in the module with the specified name. This
   /// method returns null if a NamedMDNode with the specified name is not found.
-  NamedMDNode *getNamedMetadata(const Twine &Name) const;
+  NamedMDNode *getNamedMetadata(StringRef Name) const;
 
   /// Return the named MDNode in the module with the specified name. This method
   /// returns a new NamedMDNode if a NamedMDNode with the specified name is not

llvm/lib/IR/AutoUpgrade.cpp

@@ -4900,7 +4900,25 @@ bool llvm::UpgradeDebugInfo(Module &M) {
   if (DisableAutoUpgradeDebugInfo)
     return false;
 
-  unsigned Version = getDebugMetadataVersionFromModule(M);
+  // We need to get metadata before the module is verified (i.e., getModuleFlag
+  // makes assumptions that we haven't verified yet). Carefully extract the flag
+  // from the metadata.
+  unsigned Version = 0;
+  if (NamedMDNode *ModFlags = M.getModuleFlagsMetadata()) {
+    auto OpIt = find_if(ModFlags->operands(), [](const MDNode *Flag) {
+      if (Flag->getNumOperands() < 3)
+        return false;
+      if (MDString *K = dyn_cast_or_null<MDString>(Flag->getOperand(1)))
+        return K->getString() == "Debug Info Version";
+      return false;
+    });
+    if (OpIt != ModFlags->op_end()) {
+      const MDOperand &ValOp = (*OpIt)->getOperand(2);
+      if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(ValOp))
+        Version = CI->getZExtValue();
+    }
+  }
+
   bool VersionSupported = Version == DEBUG_METADATA_VERSION ||
                           Version == DEBUG_METADATA_VERSION_HETEROGENEOUS_DWARF;
   if (VersionSupported) {

llvm/lib/IR/Module.cpp

@@ -259,10 +259,8 @@ GlobalIFunc *Module::getNamedIFunc(StringRef Name) const {
 /// getNamedMetadata - Return the first NamedMDNode in the module with the
 /// specified name. This method returns null if a NamedMDNode with the
 /// specified name is not found.
-NamedMDNode *Module::getNamedMetadata(const Twine &Name) const {
-  SmallString<256> NameData;
-  StringRef NameRef = Name.toStringRef(NameData);
-  return NamedMDSymTab.lookup(NameRef);
+NamedMDNode *Module::getNamedMetadata(StringRef Name) const {
+  return NamedMDSymTab.lookup(Name);
 }
 
 /// getOrInsertNamedMetadata - Return the first named MDNode in the module
@@ -300,46 +298,31 @@ bool Module::isValidModFlagBehavior(Metadata *MD, ModFlagBehavior &MFB) {
   return false;
 }
 
-bool Module::isValidModuleFlag(const MDNode &ModFlag, ModFlagBehavior &MFB,
-                               MDString *&Key, Metadata *&Val) {
-  if (ModFlag.getNumOperands() < 3)
-    return false;
-  if (!isValidModFlagBehavior(ModFlag.getOperand(0), MFB))
-    return false;
-  MDString *K = dyn_cast_or_null<MDString>(ModFlag.getOperand(1));
-  if (!K)
-    return false;
-  Key = K;
-  Val = ModFlag.getOperand(2);
-  return true;
-}
-
 /// getModuleFlagsMetadata - Returns the module flags in the provided vector.
 void Module::
 getModuleFlagsMetadata(SmallVectorImpl<ModuleFlagEntry> &Flags) const {
   const NamedMDNode *ModFlags = getModuleFlagsMetadata();
   if (!ModFlags) return;
 
   for (const MDNode *Flag : ModFlags->operands()) {
-    ModFlagBehavior MFB;
-    MDString *Key = nullptr;
-    Metadata *Val = nullptr;
-    if (isValidModuleFlag(*Flag, MFB, Key, Val)) {
-      // Check the operands of the MDNode before accessing the operands.
-      // The verifier will actually catch these failures.
-      Flags.push_back(ModuleFlagEntry(MFB, Key, Val));
-    }
+    // The verifier will catch errors, so no need to check them here.
+    auto *MFBConstant = mdconst::extract<ConstantInt>(Flag->getOperand(0));
+    auto MFB = static_cast<ModFlagBehavior>(MFBConstant->getLimitedValue());
+    MDString *Key = cast<MDString>(Flag->getOperand(1));
+    Metadata *Val = Flag->getOperand(2);
+    Flags.push_back(ModuleFlagEntry(MFB, Key, Val));
   }
 }
 
 /// Return the corresponding value if Key appears in module flags, otherwise
 /// return null.
 Metadata *Module::getModuleFlag(StringRef Key) const {
-  SmallVector<Module::ModuleFlagEntry, 8> ModuleFlags;
-  getModuleFlagsMetadata(ModuleFlags);
-  for (const ModuleFlagEntry &MFE : ModuleFlags) {
-    if (Key == MFE.Key->getString())
-      return MFE.Val;
+  const NamedMDNode *ModFlags = getModuleFlagsMetadata();
+  if (!ModFlags)
+    return nullptr;
+  for (const MDNode *Flag : ModFlags->operands()) {
+    if (Key == cast<MDString>(Flag->getOperand(1))->getString())
+      return Flag->getOperand(2);
   }
   return nullptr;
 }
@@ -387,10 +370,7 @@ void Module::setModuleFlag(ModFlagBehavior Behavior, StringRef Key,
   NamedMDNode *ModFlags = getOrInsertModuleFlagsMetadata();
   // Replace the flag if it already exists.
   for (MDNode *Flag : ModFlags->operands()) {
-    ModFlagBehavior MFB;
-    MDString *K = nullptr;
-    Metadata *V = nullptr;
-    if (isValidModuleFlag(*Flag, MFB, K, V) && K->getString() == Key) {
+    if (cast<MDString>(Flag->getOperand(1))->getString() == Key) {
       Flag->replaceOperandWith(2, Val);
       return;
     }

llvm/lib/IR/Verifier.cpp

@@ -402,8 +402,20 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
       : VerifierSupport(OS, M), LandingPadResultTy(nullptr),
         SawFrameEscape(false), TBAAVerifyHelper(this) {
     TreatBrokenDebugInfoAsError = ShouldTreatBrokenDebugInfoAsError;
-    if (unsigned V = getDebugMetadataVersionFromModule(M))
-      DebugInfoVersion = V;
+    if (NamedMDNode *ModFlags = M.getModuleFlagsMetadata()) {
+      auto OpIt = find_if(ModFlags->operands(), [](const MDNode *Flag) {
+        if (Flag->getNumOperands() < 3)
+          return false;
+        if (MDString *K = dyn_cast_or_null<MDString>(Flag->getOperand(1)))
+          return K->getString() == "Debug Info Version";
+        return false;
+      });
+      if (OpIt != ModFlags->op_end()) {
+        const MDOperand &ValOp = (*OpIt)->getOperand(2);
+        if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(ValOp))
+          DebugInfoVersion = CI->getZExtValue();
+      }
+    }
   }
 
   bool hasBrokenDebugInfo() const { return BrokenDebugInfo; }