Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 1829 - Allow labels as inline asm operands #3483

Merged
1 commit merged into from
Dec 6, 2014

Conversation

Orvid
Copy link
Contributor

@Orvid Orvid commented Apr 21, 2014

https://issues.dlang.org/show_bug.cgi?id=1829

This PR allows labels to be used as an operand for any op-code that would normally accept a memory location. The backend is able to handle it perfectly fine, but for some reason the frontend only lets you use labels as an operand to branching instructions. The use of this is quite simple: It's currently impossible to create a jump-table within inline-asm without creating a huge amount of boilerplate code resulting in a separate function for each and every case, as well as the default case, and the code after the default case, which is not, in any form of the word, maintainable. This PR makes the default segment for labels being resolved CS, however it does still allow you to manually override this segment, just as you would with a normal memory operand. Tests are included.

@yebblies
Copy link
Member

This does not currently include a test for this syntax, as I would normally have put it in an inlineasm test file, but that won't be added until PR 3469 is merged.

What?

@Orvid
Copy link
Contributor Author

Orvid commented Apr 22, 2014

That's just me being weird yebblies, instead I'm going to change the name of the test file in that PR, and name the one I'll add for this one inlineasmlabels.

@Orvid
Copy link
Contributor Author

Orvid commented Apr 23, 2014

Just updated the PR again, and corrected segment prefix generation to error when trying to use a segment other than CS with branching instructions, because that is not valid generated code. The CS and DS segment prefixes actually use the same prefixes that the hint branch taken and hint branch not taken prefixes do.

@Orvid
Copy link
Contributor Author

Orvid commented Apr 24, 2014

Alright, provided the tests pass, then this is now ready to be reviewed & merged, as it is now tested correctly.

@Orvid
Copy link
Contributor Author

Orvid commented Apr 28, 2014

So, apparently this fixes the 6-year old issue 1829 which I didn't know existed until it was pointed out to me. I've updated the title and summary to reflect this.

@Orvid Orvid changed the title Allow labels as inline asm operands Issue 1829 - Allow labels as inline asm operands Apr 28, 2014
@adamdruppe
Copy link
Contributor

This looks cool to me, I want it.

@p0nce
Copy link
Contributor

p0nce commented Jun 27, 2014

Very cool, this allows jump-tables!

@yebblies
Copy link
Member

yebblies commented Sep 1, 2014

This looks good, but I don't know the iasm code that well.

@ghost
Copy link

ghost commented Dec 6, 2014

Since most of you ASM folks seem to want this, I think we're a go.

@ghost
Copy link

ghost commented Dec 6, 2014

Auto-merge toggled on

ghost pushed a commit that referenced this pull request Dec 6, 2014
Issue 1829 - Allow labels as inline asm operands
@ghost ghost merged commit c94f831 into dlang:master Dec 6, 2014
@JohanEngelen
Copy link
Contributor

@Orvid Why are there all these NOPs placed in the fail12635 testcase?

@Orvid
Copy link
Contributor Author

Orvid commented Jan 31, 2018

It looks like it was copy-pasta from https://github.com/dlang/dmd/blob/75bccf2ab2cdcdcc9ce63be62a8511ea304c8403/test/fail_compilation/fail353.d

Eliminating them entirely, or switching them to a couple of actual nops would be a valid solution. fail353 needs them because it's specifically trying to get an overflow for a rel8 encoded instruction, but fail12635 has no such constraints.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants