[X86] Reorder how the subtarget map key is created.
authorCraig Topper <craig.topper@intel.com>
Fri, 17 Jul 2020 03:14:20 +0000 (20:14 -0700)
committerCraig Topper <craig.topper@intel.com>
Fri, 17 Jul 2020 04:41:45 +0000 (21:41 -0700)
We use a SmallString<512> and attempted to reserve enough space
for CPU plus Features, but that doesn't account for all the things
that get added to the string.

Reorder the string so the shortest things go first which shouldn't
exceed the small size. Finally add the feature string at the end
which might be long. This should ensure at most one heap allocation
without needing to use reserve.

I don't know if this matters much in practice, but I was looking
into something else that will require more code here and noticed
the odd reserve call.

llvm/lib/Target/X86/X86TargetMachine.cpp

index 7344116..9a9ea24 100644 (file)
@@ -249,25 +249,10 @@ X86TargetMachine::getSubtargetImpl(const Function &F) const {
                      : (StringRef)TargetFS;
 
   SmallString<512> Key;
-  Key.reserve(CPU.size() + FS.size());
-  Key += CPU;
-  Key += FS;
-
-  // FIXME: This is related to the code below to reset the target options,
-  // we need to know whether or not the soft float flag is set on the
-  // function before we can generate a subtarget. We also need to use
-  // it as a key for the subtarget since that can be the only difference
-  // between two functions.
-  bool SoftFloat =
-      F.getFnAttribute("use-soft-float").getValueAsString() == "true";
-  // If the soft float attribute is set on the function turn on the soft float
-  // subtarget feature.
-  if (SoftFloat)
-    Key += FS.empty() ? "+soft-float" : ",+soft-float";
-
-  // Keep track of the key width after all features are added so we can extract
-  // the feature string out later.
-  unsigned CPUFSWidth = Key.size();
+  // The additions here are ordered so that the definitely short strings are
+  // added first so we won't exceed the small size. We append the
+  // much longer FS string at the end so that we only heap allocate at most
+  // one time.
 
   // Extract prefer-vector-width attribute.
   unsigned PreferVectorWidthOverride = 0;
@@ -275,7 +260,7 @@ X86TargetMachine::getSubtargetImpl(const Function &F) const {
     StringRef Val = F.getFnAttribute("prefer-vector-width").getValueAsString();
     unsigned Width;
     if (!Val.getAsInteger(0, Width)) {
-      Key += ",prefer-vector-width=";
+      Key += "prefer-vector-width=";
       Key += Val;
       PreferVectorWidthOverride = Width;
     }
@@ -288,16 +273,35 @@ X86TargetMachine::getSubtargetImpl(const Function &F) const {
         F.getFnAttribute("min-legal-vector-width").getValueAsString();
     unsigned Width;
     if (!Val.getAsInteger(0, Width)) {
-      Key += ",min-legal-vector-width=";
+      Key += "min-legal-vector-width=";
       Key += Val;
       RequiredVectorWidth = Width;
     }
   }
 
-  // Extracted here so that we make sure there is backing for the StringRef. If
-  // we assigned earlier, its possible the SmallString reallocated leaving a
-  // dangling StringRef.
-  FS = Key.slice(CPU.size(), CPUFSWidth);
+  // Add CPU to the Key.
+  Key += CPU;
+
+  // Keep track of the start of the feature portion of the string.
+  unsigned FSStart = Key.size();
+
+  // FIXME: This is related to the code below to reset the target options,
+  // we need to know whether or not the soft float flag is set on the
+  // function before we can generate a subtarget. We also need to use
+  // it as a key for the subtarget since that can be the only difference
+  // between two functions.
+  bool SoftFloat =
+      F.getFnAttribute("use-soft-float").getValueAsString() == "true";
+  // If the soft float attribute is set on the function turn on the soft float
+  // subtarget feature.
+  if (SoftFloat)
+    Key += FS.empty() ? "+soft-float" : "+soft-float,";
+
+  Key += FS;
+
+  // We may have added +soft-float to the features so move the StringRef to
+  // point to the full string in the Key.
+  FS = Key.substr(FSStart);
 
   auto &I = SubtargetMap[Key];
   if (!I) {