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

Unresolved differences between gdc/dmd front ends #2194

Closed
wants to merge 3 commits into from

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Jun 17, 2013

I recall mentioning this in one of the DDMD pulls. FYI @yebblies

This covers all current unresolved differences between gdc and dmd front ends (and acts as a TODO list). An internal goal is to have them 100% shared code bases by release 2.065. This makes a transition from the D front-end implement from C++ to D more of a 'drop in' (and is arguably a blocker and hindrance for adoption of the new implementation), and also relieves a HUGE maintenance burden when it comes round to merge-time. However such a goal requires a bit of tug and pull from both sides of the fence so I'm raising this here to allow discussion with any core devs to iron these out in a way that we can say good riddance to the IN_GCC macro.

The diff can be summarised into the following points:

  • Internal backend types differ
  • Differing _argptr and _arguments front end implementation.
  • Always use Itanium ABI for C++ mangling
  • Support for GDC Extended Assembly in parser.
  • Rejected/not yet pulled fixes for things done in the front end that send invalid/garbage code to the glue part of the front-end.

@yebblies - or anyone else - if you have any questions or suggestions, anything is welcome. Also let me know if you'd rather this broken down... :o)

Will be sending separate pulls for things that can be folded in immediately...

Thanks
Iain.

@yebblies
Copy link
Member

Hmm...

Internal backend types differ

This is ugly, but that is more due to dmd's design than anything else.

Use of fprintf(stderr) instead of printf

Are people relying on the -v output coming over stdout? I seem to remember something about this, for rdmd or something...

Built-in functions are instead CTFE'd in glue part of front-end.

They... what? How does that work?

real_t requires special handling as is not host float.

Can we cram all of the affected code into Port somehow?

PowExp does not depend on std.math.

Hmm.

Differing _argptr, _arguments and va_list front end implementation.

shudder

Always use cppmangle.c for C++ mangling

Will this be fixed by the new mangling stuff?

Support for GDC Extended Assembly in parser.

Urrgh, I guess we don't have much choice here.

mars.c is only used for verror and friends (front end entry point is in glue)

I think we should move these into another file, along with everything else you need from mars.c (global and Loc?)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 17, 2013

Use of fprintf(stderr) instead of printf

Are people relying on the -v output coming over stdout? I seem to remember something about this, for rdmd or something...

Yes, I think rdmd might depend on it. I think rdmd specifically doesn't work with gdc though... that or they've worked around it without telling me. The rationale behind sending all output to stderr is that the gcc backend already uses stdout to pipe assembly to as.

Built-in functions are instead CTFE'd in glue part of front-end.

They... what? How does that work?

There are approximately 580 built-ins that may be subject to being const-folded - without being restricted by host native precision - by the gcc backend. As CTFE has guaranteed that all parameters are known at compile time, I build the call in GCC's IR, and if it gets folded into a constant literal, a new D frontend expression is generated and returned back to CTFE.

real_t requires special handling as is not host float.

Can we cram all of the affected code into Port somehow?

Yes, that is my plan.

PowExp does not depend on std.math.

Hmm.

foo ^^ bar -> __builtin_pow{fl} (foo, bar)

Always use cppmangle.c for C++ mangling

Will this be fixed by the new mangling stuff?

Once I'm convinced that it has done properly, yes removing the CPP_MANGLE macro will sort it just fine.

Support for GDC Extended Assembly in parser.

Urrgh, I guess we don't have much choice here.

This can be the last thing to worry about...

mars.c is only used for verror and friends (front end entry point is in glue)

I think we should move these into another file, along with everything else you need from mars.c (global and Loc?)

Yes please. Although I thought you objected to this last time I mentioned? :o)

@yebblies
Copy link
Member

Yes, I think rdmd might depend on it. I think rdmd specifically doesn't work with gdc though... that or they've worked around it without telling me. The rationale behind sending all output to stderr is that the gcc backend already uses stdout to pipe assembly to as.

Sigh, this probably means we need to bring back stdmsg.

Built-in functions are instead CTFE'd in glue part of front-end.

They... what? How does that work?

There are approximately 580 built-ins that may be subject to being const-folded - without being restricted by host native precision - by the gcc backend. As CTFE has guaranteed that all parameters are known at compile time, I build the call in GCC's IR, and if it gets folded into a constant literal, a new D frontend expression is generated and returned back to CTFE.

Yikes, that's a little scary. I guess there's no more direct way invoke the builtin folders?

PowExp does not depend on std.math.

Hmm.

foo ^^ bar -> __builtin_pow{fl} (foo, bar)

Yeah, but this will need druntime support before dmd can do it properly.

mars.c is only used for verror and friends (front end entry point is in glue)

I think we should move these into another file, along with everything else you need from mars.c (global and Loc?)

Yes please. Although I thought you objected to this last time I mentioned? :o)

I don't think so... or at least, if I did I was wrong. Not sure where to put them exactly...

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 17, 2013

Yes, I think rdmd might depend on it. I think rdmd specifically doesn't work with gdc though... that or they've worked around it without telling me. The rationale behind sending all output to stderr is that the gcc backend already uses stdout to pipe assembly to as.

Sigh, this probably means we need to bring back stdmsg.

Need not have it defined globally though, eg: add a new function message(...) and vmessage(...) to use instead of printf().

Built-in functions are instead CTFE'd in glue part of front-end.

They... what? How does that work?

There are approximately 580 built-ins that may be subject to being const-folded - without being restricted by host native precision - by the gcc backend. As CTFE has guaranteed that all parameters are known at compile time, I build the call in GCC's IR, and if it gets folded into a constant literal, a new D frontend expression is generated and returned back to CTFE.

Yikes, that's a little scary. I guess there's no more direct way invoke the builtin folders?

Currently it's done just by implementing our own d_gcc_eval_builtin routine - renamed because it takes slightly different arguments than eval_builtin (a FuncDeclaration instead of an enum BUILTIN)

mars.c is only used for verror and friends (front end entry point is in glue)

I think we should move these into another file, along with everything else you need from mars.c (global and Loc?)

Yes please. Although I thought you objected to this last time I mentioned? :o)

I don't think so... or at least, if I did I was wrong. Not sure where to put them exactly...

Might have been in reference to me mentioning that I was going to drop mars.c from gdc's copy of the D front end.

@ghost
Copy link

ghost commented Jun 17, 2013

W.r.t. erorring lets just be careful not to re-introduce this regression: http://d.puremagic.com/issues/show_bug.cgi?id=10050

@yebblies
Copy link
Member

Need not have it defined globally though, eg: add a new function message(...) and vmessage(...) to use instead of printf().

This is much better.

Currently it's done just by implementing our own d_gcc_eval_builtin routine - renamed because it takes slightly different arguments than eval_builtin (a FuncDeclaration instead of an enum BUILTIN)

Interesting. Does DMD really need the BUILTIN enum, or could it use the FuncDeclaration approach too?

Might have been in reference to me mentioning that I was going to drop mars.c from gdc's copy of the D front end.

I actually thought you'd done this already.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 19, 2013

Currently it's done just by implementing our own d_gcc_eval_builtin routine - renamed because it takes slightly different arguments than eval_builtin (a FuncDeclaration instead of an enum BUILTIN)

Interesting. Does DMD really need the BUILTIN enum, or could it use the FuncDeclaration approach too?

Shouldn't make a difference if dmd was to switch... all that you'd use it for is to switch (fd->builtin) - though I think a more pluggable way of adding built-in functions to the front end might be in order...

Might have been in reference to me mentioning that I was going to drop mars.c from gdc's copy of the D front end.

I actually thought you'd done this already.

Not yet... :o)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jul 18, 2013

Rebased to most recent head. Round about half of the initial list of differences raised in this pull have now been resolved.

@yebblies
Copy link
Member

Half way!

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 30, 2014

Gone from - 25 changed files with 387 additions and 70 deletions.
To - 23 files changed, 353 insertions(+), 59 deletions(-)

At the very least, builtin.c, mars.c, and root/async.c have been removed from gdc's frontend which have in part accounted for the small drop over the last 7 months.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 30, 2014

On the bright side, conflicts of interest between implementations are small, and most of the differences are now gdc ehancements.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 26, 2014

@yebblies - btw how much of this do you feel has been sorted out in DMD-HEAD?

@yebblies
Copy link
Member

The diff looks up-to-date. The iasm and mem stuff should probably go into the frontend unconditionally, and the paint_float_int stuff should be in port. I have a patch somewhere to remove the verror workaround.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 27, 2014

The mem changes are only needed to maintain the c++ frontend. The glue doesn't allocate through Mem directly itself.

@yebblies
Copy link
Member

The mem changes are only needed to maintain the c++ frontend. The glue doesn't allocate through Mem directly itself.

Ok, but for now we support the C++ frontend so it needs to be fixed.

@andralex
Copy link
Member

Well so should we close this for now?

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 27, 2015

If only to stop the auto-tester trying to build it.

The idea here is that there should be as close to zero changes as possible. And I would hope that a common shared frontend is a prerequesite for ddmd.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jan 27, 2015

I will need to update this anyway for 2.066/2.067 - doing so with also remind me to raise new PRs for other things (and cross items off the list in the OP)

@@ -580,10 +580,10 @@ class FuncDeclaration : public Declaration
// scopes from having the same name
VarDeclaration *vthis; // 'this' parameter (member and nested)
VarDeclaration *v_arguments; // '_arguments' parameter
#ifdef IN_GCC
#ifdef IN_GCC // %%
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for this change? This same change made it's way in in multiple other places as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make them show up in the diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - upstream need to be aware of any IN_GCC macros - whether or not we can remove these in future is up for discussion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, how do you deal with _argptr, if you don't declare it in the frontend?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yebblies - We use v_argptr to stash the _argptr var, then generate the code in the backend. (va_start and va_end are gcc builtins).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I misread this block as a #ifndef IN_GCC block. I'm considering doing the same in dmd.

@yebblies
Copy link
Member

I wouldn't worry about the autotester, it will only run this after all the more recently updated ones and will fail very quickly.

@ibuclaw
Copy link
Member Author

ibuclaw commented Feb 9, 2015

@yebblies @andralex - By the way, I intend on releasing a GDC stubs repository sometime this week, which takes the current dmd-master sources, then provides the minimum required stubs in order to tie the frontend to GCC. So this will include parsing and semantic processing, but no codegen.

This is for a few reasons:

  1. Trialling DDMD
  2. Catch problems in the front-end before 2.067 release.
  3. Allows a possible clean-room rewrite of GDC in a manner that takes advantage of GCC's new coding style for C++, and lowers the entrypoint for potential contributors.

@yebblies
Copy link
Member

yebblies commented Feb 9, 2015

I like the sound of that!

@yebblies
Copy link
Member

A lot of these differences seem to be fixed already. I'd be very interested to see a current diff.

I've opened #4472 to look at merging in the ExtAsmStatement code.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 12, 2015

@yebblies rebased: (edit: rebased again)
Was 23 files changed 353 insertions(+) 59 deletions(-)
Now 19 files changed, 360 insertions(+), 56 deletions(-)

Some progress, hidden by the fact that parseExtAsm has been re-implemented / grown in size.

@yebblies
Copy link
Member

What do you think about the approach in #4569 for getting rid of the backend type references?

@dlang dlang deleted a comment from dlang-bot Sep 27, 2017
@dlang dlang deleted a comment from dlang-bot Sep 27, 2017
@dlang dlang deleted a comment from dlang-bot Sep 27, 2017
@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 27, 2017

Granted that most comments here are just me saying "bump, bump, bump..." - I certainly wouldn't miss them. :-)

@dlang-bot
Copy link
Contributor

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.

@dlang-bot
Copy link
Contributor

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.

@dlang-bot
Copy link
Contributor

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.

@dlang-bot
Copy link
Contributor

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.

@dlang-bot
Copy link
Contributor

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.

@wilzbach wilzbach changed the base branch from stable to master December 29, 2017 13:14
@dlang-bot
Copy link
Contributor

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.

@wilzbach
Copy link
Member

PRs pointing at stable shouldn't get stalled. It should be dead simple: either it's a regression fix, then merge or target master instead. I will look into implementing a warning at dlang-bot for all pulls to stable which are older than three days.
I changed the base here because PRs to stable have a priority in the auto-tester queue and thus take up quite a bit of resources.

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 30, 2017

@wilzbach - What about a label such as Don't test or dlang-bot ignore?

@wilzbach
Copy link
Member

Are you referring to the bot commenting here too often? That's due to it not doing paging at the moment and not finding its comment on the first page. I will look into adding this to it, but I think this is one of the last PRs with so many comments that have been opened before the bot's existence. So if you are annoyed by this, it might be easier to open a new PR.

If you are referring to the CIs testing this. There's not much what we can do easily. There is an API for Travis and CircleCi, so we might be able to cancel these builds, but not for the other CIs anyhow as GitHub triggers the CIs directly. Though we could make it easy to disable the CIs from being run, by e.g. checking for the existence of a stub file and then "aborting successfully". Though I think this is the first time we have encountered this request.

Or do you just want the bot to recognize the prefix and auto-label the PR like it does e.g. for WIP?

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 30, 2017

Any or all of the above.

I'll have a run through this PR again though, I think that all changes listed here are in other PRs either merged or pending, except for ExtAsmStatement and this ABI hack that depends on dmd's bespoke calling convention that nobody supports (this might fix it also, on second look).

@ibuclaw
Copy link
Member Author

ibuclaw commented Dec 31, 2017

From top to bottom:

clone.d: dlang/druntime#2025 / #7561
cppmangle.d: #7563
vector fixes: #7065
genCmain: #7534
gcc-style assembler: #7562
gluelayer.d: #7560

That concludes everything that is modified in the D front-end for GDC.

@ibuclaw ibuclaw closed this Dec 31, 2017
@ibuclaw ibuclaw deleted the gdcfe branch December 31, 2017 01:41
wilzbach pushed a commit to wilzbach/dmd that referenced this pull request Jul 31, 2018
…compiling with -betterC

The problem is not actually with PR 2194 per se, but rather with a fundamental design flaw in D: There is no way to determine which code is needed at compile-time and
which code that is needed at runtime.

Users of -betterC may wish to employ features of D at compile-time that are not available in the -betterC subset of the language at runtime. An obvious example is
pragma(msg, typeid(size_t));. That code, as ridiculous as it is, is currently possible in -betterC despite the fact that it requires TypeInfo which is a feature not
available in -betterC at runtime. If users are doing such things in -betterC, then dlang#2194 will introduce regressions. dlang#2194 should have actually been implemented at the
time -betterC was first introduced, but we're past that point now.

To resolve this issue in a more prudent manner, we either need to decide that compile-time -betterC and runtime -betterC must have feature parity, or we need to add a
version(CTFE) feature to the language to segregate compile-time features and runtime features. This is actually a general cross-compilation issue: Users may want to
have features of D available at compile-time that are not supported by their runtime target. But that's not a battle I'm prepared to fight at this time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GDC Gnu D Compiler WIP Work In Progress - not ready for review or pulling
Projects
None yet
8 participants