[llvm] addbf73 - [X86] Reorder how the subtarget map key is created.

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2020-07-16T21:41:45-07:00
New Revision: addbf732c83b54259d6aa2c6ed982099be0aeeaf

URL: https://github.com/llvm/llvm-project/commit/addbf732c83b54259d6aa2c6ed982099be0aeeaf
DIFF: https://github.com/llvm/llvm-project/commit/addbf732c83b54259d6aa2c6ed982099be0aeeaf.diff

LOG: [X86] Reorder how the subtarget map key is created.

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.

Added: 
    

Modified: 
    llvm/lib/Target/X86/X86TargetMachine.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86TargetMachine.cpp b/llvm/lib/Target/X86/X86TargetMachine.cpp
index 7344116e14af..9a9ea245f702 100644
--- a/llvm/lib/Target/X86/X86TargetMachine.cpp
+++ b/llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -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 
diff erence
-  // 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 
diff erence
+  // 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) {