[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Adrian - what do you think about Reid's point regarding not changing the C API unless needed? I'm fairly OK with that, but not sure. Seems like it could be a surprising "gotcha" where someone thinks they're doing the right things to produce CodeView but they're missing this because the API isn't exposed/suggesting that usage.

> It would be more efficient to put, e.g., a NamedMDNode into the module so this information can be shared between the CUs.

I assume the string data itself is shared by the bitcode format? But I don't really know.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:674-679
+  for (unsigned i = 0; i != Str.size(); ++i) {
+    if (Str[i] == '\\' && i != Str.size() - 1) {
+      if (Str[i + 1] == '\\')
+        Str.erase(Str.begin() + i + 1);
+    }
+  }
----------------
This is O(N^2) (erase is O(N)) & could be made O(N) (using a technique similar to the erase+remove idiom - using literal erase+remove probably isn't practical due to the stateful nature of the walk).

Is there no other logic for this already elsewhere in LLVM?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:692-693
+  MCContext &Context = MMI->getContext();
+  MCSymbol *ObjBegin = Context.createTempSymbol(),
+           *ObjEnd = Context.createTempSymbol();
+  OS.AddComment("Record length");
----------------
Probably best just to use another/separate declaration here? (I think such a thing might be specified in the style guide, but I'm not sure)


https://reviews.llvm.org/D43002