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

Add parser and some semantic support for gdc extended asm statements #7562

Closed
wants to merge 1 commit into from

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Dec 31, 2017

You'll never accept this, as it's not used anywhere except in gdc.

But, is a revival nonetheless of #4472 which came from #2194 originally.

@dlang-bot
Copy link
Contributor

dlang-bot commented Dec 31, 2017

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#7562"

@PetarKirov
Copy link
Member

I wonder if there exists a common subset between the GDC and LDC (non-IR) inline asm (https://wiki.dlang.org/LDC_inline_assembly_expressions), which we can agree on and add to the language, though I guess the main difficulty would be integration with dmd's backend.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 1, 2018

I wonder if there exists a common subset between the GDC and LDC (non-IR) inline asm

Changing syntax would break existing code though.

main difficulty would be integration with dmd's backend.

It's just a glorified printf, with some more bells and whistles.

@jacob-carlborg
Copy link
Contributor

I'm not sure which syntax LDC and GDC are using but Clang is compatible with GCC [1].

[1] https://clang.llvm.org/compatibility.html#inline-asm

@JinShil
Copy link
Contributor

JinShil commented Jan 2, 2018

Forgive my ignorance, but I don't understand the motivation for this PR other than to elicit discussion.

It appears that this PR has only a parser and partial semantic support, but ultimately does nothing. Furthermore it's only ever used in GCC. Eventually, do we want to merge code like this so there is one code base shared between the 3 main compilers?

Personally, I would love to have one rich ASM syntax to use for all 3 compilers, but that doesn't seem to be the objective of this PR

You'll never accept this, as it's not used anywhere except in gdc.

I suspect that is an honest comment; not facetious.

As I've been reviving PRs, my goal has been to give those PRs a quick path to acceptance or a quick path to closure. What's the goal of this PR?

@ibuclaw ibuclaw added the Compiler:GDC Gnu D Compiler label Jan 7, 2018
@wilzbach
Copy link
Member

As I've been reviving PRs, my goal has been to give those PRs a quick path to acceptance or a quick path to closure. What's the goal of this PR?

@ibuclaw why did you revive this?

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 25, 2018

I split the mega gdc diff pr into separate patches. From a maintenance standpoint there should be no difference between gdc and dmd frontends... But there is.

This is a problem, and dmd needs to be fixed or improved to allow support for these discrepancies to be implemented without touching the frontend.

@JinShil
Copy link
Contributor

JinShil commented Jan 26, 2018

This is a problem, and dmd needs to be fixed or improved to allow support for these discrepancies to be implemented without touching the frontend.

If the plan is to have a single front-end code-base with compiler differences versioned, that's fine with me, but I'd like to hear from @WalterBright to ensure he's on board with that, and also @kinke @redstar @klickverbot and whoever else is on the LDC team.

@WalterBright
Copy link
Member

You'll never accept this, but the dmd inline asm code is all Boost licensed now, so you can use it!! :-)

@WalterBright
Copy link
Member

If the plan is to have a single front-end code-base with compiler differences versioned, that's fine with me, but I'd like to hear from @WalterBright to ensure he's on board with that, and also @kinke @redstar @klickverbot and whoever else is on the LDC team.

I'm on board with it, provided it is properly encapsulated. When support for it is distributed across numerous files, it is not encapsulated and will be an ongoing problem. There should be 4 files:

iasm.d         // the generic interface
iasmdmd.d  // the dmd version
iasmgcc.d    // the gcc version
iasmldc.d    // the ldc version

There should be minimal to no need for version if iasm.d is set up right.

As an example of what I mean, take a look at scanelf.d, scanomf.d, scanmach.d, and scanmscoff.d. Another example is libelf.d, libomf.d, libmach.d, libmscoff.d. Look, ma, no versions! I am pretty happy with how these have worked out, there has been little trouble with them, and the extra work in coming up with a generic interface turned out to be far less work in the end.

As an example of failure, look at cgobj.c, machobj.c, elfobj.c, mscoffobj.c. Well, sort of failure. I've been slowly moving to a generic interface, and it's getting better.

If we can do it with these files, we can do it with the inline assembler.

@jacob-carlborg
Copy link
Contributor

There should be minimal to no need for version if iasm.d is set up right.

I'm not sure if it's possible, but I would prefer a single check in code over special casing the makefile.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 26, 2018

There should be minimal to no need for version if iasm.d is set up right.

As an example of what I mean, take a look at scanelf.d, scanomf.d, scanmach.d, and scanmscoff.d. Another example is libelf.d, libomf.d, libmach.d, libmscoff.d. Look, ma, no versions! I am pretty happy with how these have worked out, there has been little trouble with them, and the extra work in coming up with a generic interface turned out to be far less work in the end

Ahem, packages (iasm/, scan/, lib/ :-)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 26, 2018

I'm not sure if it's possible, but I would prefer a single check in code over special casing the makefile.

Well preferably, I, as a user of dmd frontend, should be able to omit including dmd and ldc iasm implementations in my local mirror without any compiler or build problems.

Runtime checks won't satisfy this requirement unless unsightly stubs are added.

@wilzbach
Copy link
Member

Ahem, packages

I absolutely agree, but Walter doesn't like packages. I tried:

Though everyone else noticing these PRs was in favor of those changes...

@wilzbach
Copy link
Member

wilzbach commented Jan 26, 2018

As an example of what I mean, take a look at scanelf.d, scanomf.d, scanmach.d, and scanmscoff.d. Another example is libelf.d, libomf.d, libmach.d, libmscoff.d. Look, ma, no versions! I am pretty happy with how these have worked out, there has been little trouble with them, and the extra work in coming up with a generic interface turned out to be far less work in the end.

To be fair these files use a version at the top as this makes it a lot easier for the various build setups (Dub, Ddox build, Ddoc build, Rdmd build, ...).

Well preferably, I, as a user of dmd frontend, should be able to omit including dmd and ldc iasm implementations in my local mirror without any compiler or build problems.

Yes. The following already works

rdmd myfrontendtool.d -I<dlang/dmd>/src -version=NoBackend -version=GC -version=NoMain

See #7782 and it would be cool if that continues to be the case ;-)

@wilzbach
Copy link
Member

Looks like this was accidentally closed?

@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 14, 2018

Nope, closing was deliberate. The PR in #8538 replaced this.

@ibuclaw ibuclaw closed this Aug 14, 2018
@ibuclaw ibuclaw deleted the gccextasm branch August 14, 2018 17:13
@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 14, 2018

If I'm right, the diff between gdc and dmd frontend is now no more than 12 lines!

...

Ignoring the modules that I do not include in gdc (inline.d, builtins.d, target.d, etc...)

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

Successfully merging this pull request may close these issues.

7 participants