[TableGen] [CodeGenTarget] Cache the target's instruction namespace

[Paul-C-Anagnostopoulos]

Summary

This revision caches the target machine's instruction namespace in CodeGenTarget. The namespace is requested multiple times by backends and involves a linear scan of the vector of records inheriting from Instruction.

I've separated this revision from https://reviews.llvm.org/D92674 and will remove it from that revision.

[dblaikie]

Is there much data to suggest this is an observable performance issue? (if the number of times it's queried is small, or the number of elements it searches through is small, etc - perhaps it doesn't matter & the simpler code might be better?)

llvm/utils/TableGen/CodeGenTarget.h

63

The = StringRef() is redundant here and should be omitted.

[Paul-C-Anagnostopoulos]

I can't separate the performance increases of the two caching revisions, but I suspect the other one produces most of the improvement (https://reviews.llvm.org/D92674).

At least one backend requests the namespace 22 times, and the search is linear and must skip all the TargetOpcode instructions. There are about 200 of them, I think.

But if you think this is pointless, I'm happy to discard it. On the other hand, it's a total of 3 new lines of code.

[dblaikie]

In D92722#2435708, @Paul-C-Anagnostopoulos wrote:

I can't separate the performance increases of the two caching revisions, but I suspect the other one produces most of the improvement (https://reviews.llvm.org/D92674).

At least one backend requests the namespace 22 times, and the search is linear and must skip all the TargetOpcode instructions. There are about 200 of them, I think.

Yeah, I'd expect that's pretty cheap/not a major performance consideration.

But if you think this is pointless, I'm happy to discard it. On the other hand, it's a total of 3 new lines of code.

Up to you - no strong feelings. Your call.

[Paul-C-Anagnostopoulos]

I'll go for it.

[Paul-C-Anagnostopoulos]

Eliminated explicit InstNamespace initializer.

llvm/utils/TableGen/CodeGenTarget.h

class CodeGenTarget {
   mutable SmallVector<ValueTypeByHwMode, 8> LegalValueTypes;
   CodeGenHwModes CGH;
   void ReadRegAltNameIndices() const;
   void ReadInstructions() const;
   void ReadLegalValueTypes() const;
 
   mutable std::unique_ptr<CodeGenSchedModels> SchedModels;
 
+  mutable StringRef InstNamespace;
   mutable std::vector<const CodeGenInstruction*> InstrsByEnum;
   mutable unsigned NumPseudoInstructions = 0;
 public:
   CodeGenTarget(RecordKeeper &Records);
   ~CodeGenTarget();
 
   Record *getTargetRecord() const { return TargetRec; }
   const StringRef getName() const;

llvm/utils/TableGen/CodeGenTarget.cpp

 
 CodeGenTarget::~CodeGenTarget() {
 }
 
 const StringRef CodeGenTarget::getName() const {
   return TargetRec->getName();
 }
 
+/// getInstNamespace - Find and return the target machine's instruction
+/// namespace. The namespace is cached because it is requested multiple times.
 StringRef CodeGenTarget::getInstNamespace() const {
+  if (InstNamespace.empty()) {
     for (const CodeGenInstruction *Inst : getInstructionsByEnumValue()) {
-    // Make sure not to pick up "TargetOpcode" by accidentally getting
-    // the namespace off the PHI instruction or something.
-    if (Inst->Namespace != "TargetOpcode")
-      return Inst->Namespace;
+      // We are not interested in the "TargetOpcode" namespace.
+      if (Inst->Namespace != "TargetOpcode") {
+        InstNamespace = Inst->Namespace;
+        break;
+      }
+    }
   }
 
-  return "";
+  return InstNamespace;
 }
 
 StringRef CodeGenTarget::getRegNamespace() const {
   auto &RegClasses = RegBank->getRegClasses();
   return RegClasses.size() > 0 ? RegClasses.front().Namespace : "";
 }
 
 Record *CodeGenTarget::getInstructionSet() const {