[PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

Vedant Kumar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 24 15:56:16 PDT 2019


vsk added inline comments.


================
Comment at: clang/lib/AST/Mangle.cpp:130
+      return;
+    }
+
----------------
rjmccall wrote:
> This is actually backwards, right?  A literal label is one that doesn't get the global prefix and therefore potentially needs the `\01` prefix to suppress it.
Thank you for the careful and helpful review! Before updating this patch, I'd like to make sure we're on the same page on this point.

What I'm searching for is a way to prevent `mangleName` from adding either a `\01` prefix or the global prefix to the label contained within an AsmLabelAttr. As the result from `mangleName` is passed to a hash function, omitting both prefixes is necessary to fix the lldb expression evaluation failure mentioned in the summary. Based on (probably a bad reading of) earlier feedback, I thought that marking an attr as "Literal" would be the preferred method to suppress all prefixes.

However, it sounds like you believe:
1) An attr marked as "Literal" should have a `\01` prefix applied, but no global prefix.
2) An attr *not* marked as "Literal" should have a global prefix applied (if one is available).

If I've characterized your view correctly, then we really need something else to denote "do not add any kind of prefix". One option is to use `BoolArgument<"IsUnprefixed", /*optional=*/0, /*fake=*/1>`, and set the literal/non-literal distinction aside.

OTOH, if what you really meant was that `mangleName` should omit all prefixes for non-"Literal" labels (and that "Literal" is the right name/distinction for this idea), I can simply invert the condition I've been using.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67774/new/

https://reviews.llvm.org/D67774





More information about the cfe-commits mailing list