Page MenuHomePhabricator

Adding altmacro support in integrated assembler. continue of D10591
AbandonedPublic

Authored by m_zuckerman on Jul 15 2015, 4:41 AM.

Details

Reviewers
grosbach
compnerd
Summary

There was a problem with compere to base I did rebase.
you can find more comments in link [[ URL | http://reviews.llvm.org/D10591 ]]
D10591
The asm parser now support the altmacro and noaltmacro directives.
This is flow the bug in link: https://llvm.org/bugs/show_bug.cgi?id=18918
You can find information about altmacro in this link :https://sourceware.org/binutils/docs/as/Altmacro.html.
In this part of the work we support the three type of string "..." , '...' ,<...>.
You can now use '%' char to write `%expr’ to evaluate the expression expr and use the result as a string.
In the future we will add the local directive and additional string support.

Diff Detail

Event Timeline

m_zuckerman updated this revision to Diff 29769.EditedJul 15 2015, 4:41 AM
m_zuckerman retitled this revision from to Adding altmacro support in integrated assembler. continue of D10591 .
m_zuckerman updated this object.
m_zuckerman changed the visibility from "Public (No Login Required)" to "All Users".

I create a new diff without any extra reindents part.

rafael edited edge metadata.Jul 17 2015, 1:42 PM

Please git-clang-format. The patch has trailing spaces

Don't repeat names in comments.

include/llvm/MC/MCParser/AsmLexer.h
12

spelling/grammar issues:
"must to end"
"no a string"

lib/MC/MCParser/AsmLexer.cpp
36

EOF is not a character.

What exactly is this function doing?

m_zuckerman edited edge metadata.
m_zuckerman changed the visibility from "All Users" to "Public (No Login Required)".
m_zuckerman marked 2 inline comments as done.
m_zuckerman added a comment.EditedJul 31 2015, 3:44 AM

Hi all I want to continue with this review. can you please continue the review.

lib/MC/MCParser/AsmLexer.cpp
36

When there '<' in the start of a token. You don't know if the token is a string or an arithmetic OP.
The function is looking for the char '>'.
If in the end of the statement there is a '>' char.
The function returns string else return not a string

jannau added a subscriber: jannau.Aug 6 2015, 1:05 PM

gnu as seems to evaluate '%' expressions in macro parameters before it instantiate the macro. small example which doesn't work (even segfaults with diff 30357 for me) currently below:

.macro do_test num
    .macro test_\num x
        .long (\num * \x)
    .endm
.endm

.altmacro
        do_test %(10 + 7)
.noaltmacro

        test_17 3

This is the minimizations of Libav's arm PIC in assembler support macros.

compnerd edited edge metadata.Aug 6 2015, 6:18 PM

IIRC, there were some interesting cases that would require pulling the expression parser out of the environment, but I may have incorrectly analyzed what was needed to make all the cases to work.

include/llvm/MC/MCParser/AsmLexer.h
39

Please don't do this AltMacroMode parameter. Switch the entire state of the Lexer. Introduce a Mode enumeration, as there are at least three different modes (normal, which we currently support, Alternate, which we do not, and MRI, which I currently think that we are unlikely to support).

tomatabacu resigned from this revision.Aug 7 2015, 8:16 AM
tomatabacu removed a reviewer: tomatabacu.
m_zuckerman edited edge metadata.
m_zuckerman marked an inline comment as done.

gnu as seems to evaluate '%' expressions in macro parameters before it instantiate the macro. small example which doesn't work (even segfaults with diff 30357 for me) currently below:

.macro do_test num
    .macro test_\num x
        .long (\num * \x)
    .endm
.endm

.altmacro
        do_test %(10 + 7)
.noaltmacro

        test_17 3

This is the minimizations of Libav's arm PIC in assembler support macros.

As I said in the preview .
"This part of the work we support the three type of string "..." , '...' ,<...>. You can now use '%' char to write `%expr’ to evaluate the expression expr and use the result as a string."

In this part of work we support the specific bug: https://llvm.org/bugs/show_bug.cgi?id=18918 . We support directory "altmacro" and the basic feature of the directory.

In the future we will add additional string support.

Hi all I want to continue with this review. can you please continue the review.

grosbach requested changes to this revision.Sep 3 2015, 11:08 AM
grosbach edited edge metadata.

The linked documentation is very vague. There needs to be a lot more clarity on the design before this goes in. For example, what expressions are legal for %expr? It can't be anything, as an expression which requires a relocation would obviously not be reasonable. So what are the constraints? This introduces really nasty layering issues between the parser, lexer, and layout engines, to put it mildly. Before we can realistically talk about the code itself, those design issues need sorted out.

Please start a thread on llvmdev to discuss this.

This revision now requires changes to proceed.Sep 3 2015, 11:08 AM
m_zuckerman abandoned this revision.Aug 1 2017, 11:01 PM