This is an archive of the discontinued LLVM Phabricator instance.

[LLVM][inline-asm] Altmacro string escape character '!'
ClosedPublic

Authored by m_zuckerman on May 3 2017, 4:05 AM.

Details

Summary

This patch is the fourth patch in a series of reviews for the Altmacro feature. This patch introduces a new escape character '!' and it depends on D32701.

according to https://sourceware.org/binutils/docs/as/Altmacro.html:
"single-character string escape
To include any single character literally in a string (even if the character would otherwise have some special meaning), you can prefix the character with !' (an exclamation mark). For example, you can write <4.3 !> 5.4!!>' to get the literal text `4.3 > 5.4!'. "

Diff Detail

Repository
rL LLVM

Event Timeline

m_zuckerman created this revision.May 3 2017, 4:05 AM
rengolin added inline comments.May 5 2017, 3:22 AM
lib/MC/MCParser/AsmParser.cpp
1207 ↗(On Diff #97588)

This could be:

while ( ((...) && (*CharPtr != '\0')) || (*CharPtr == '!')) {
  CharPtr++;
1219 ↗(On Diff #97588)

This should really use isAltmacroString to make sure it really is a string, or it could have the same problems you detect on the other cases. But doing that, the same logic would need to be implemented twice.

A better approach would be if isAltmacroString was actually just altMacroString and, if the result is not empty, it "is an alt-macro string".

test/MC/AsmParser/altmacro_string_escape.s
3 ↗(On Diff #97588)

put the check lines near where they make sense, for instance, near the macro definition, or its use.

m_zuckerman updated this revision to Diff 98091.May 7 2017, 1:17 AM
m_zuckerman marked an inline comment as done.May 7 2017, 1:22 AM
m_zuckerman added inline comments.
lib/MC/MCParser/AsmParser.cpp
1207 ↗(On Diff #97588)

It will not work since you need double promotion.
Consider the following code <5!>4> you want to jump into the '4' char. For that, you will need double promotion.

1219 ↗(On Diff #97588)

This function called after "isAltMacroString" validates the string D32701.
How the flow works:

  1. One of the argument contains the '<' sign (parseMacroArguments).
  2. This argument is then validated by the second "else if" in the "parseMacroArguments" function(D32701)
  3. If the string begins with '<' and ends with '>' the function(parseMacroArguments) marks him as a string.
  4. Only token that begins with '<' and marked as a string can enter to this function.

The problem with your approach is that we need to create a new string that not contain the '!'. This we can't do unless we will create a new global string in the begin of the main function "handleMacroEntry". Since the Stringref only contains the reference. We can't modify him and only mark him.

What you think, Do you prefer to create a new global string in the begin of the function and then use it?

rengolin accepted this revision.May 10 2017, 3:06 AM

As parsers go, simplicity and elegance are usually mutually exclusive.

I agree with you that, in this case, simplicity is more important, even if that generates a bit of redundancy, which isn't really that much.

LGTM. Thanks!

lib/MC/MCParser/AsmParser.cpp
1207 ↗(On Diff #97588)

of course!

1219 ↗(On Diff #97588)

Dang, I hate parsers! They're always full of chickens and eggs.

This revision is now accepted and ready to land.May 10 2017, 3:06 AM

Thanks a lot :)

This revision was automatically updated to reflect the committed changes.