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

Make backend configuration platform-independent #12137

Closed
wants to merge 5 commits into from

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Jan 19, 2021

If we want the backend to support cross compilation, it needs to receive the target platform from the driver.
So the first step is actually in backconfig. In the process of removing static condition, I removed duplicated functions.
Better reviewed commit-by-commit and with whitespace diff off (it makes the diff rather trivial, except for the util_set{32,64} merge).

This will allow us to move towards a cross compiler.
Removing static condition is one step forward towards cross compilation.
This makes the code a lot more DRY.
The TARGET_OSX clauses were merged with the other POSIX target as support for 32 bits OSX has ended.
Another step towards real cross compilation.
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

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 run digger -- build "master + dmd#12137"

_tyalignsize[TYldouble] = 2;
_tyalignsize[TYildouble] = 2;
_tyalignsize[TYcldouble] = 2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Did you check if dmc still uses this function? If so, it would be worth mentioning in the commit message.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no way to check this, so I wanted to get @WalterBright 's opinion.

Copy link
Member

Choose a reason for hiding this comment

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

DMC is open sourced on the digitalmars github org IIRC, so it can be checked.

Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

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

Please:

  1. These sorts of changes make my life difficult as I have to redo them to ensure they work with the other backend's Digital Mars builds. For example, you removed the 16 bit support. Dead code to DMD may not be dead code for DMC. See https://github.com/DigitalMars/Compiler/tree/master/dm/src/dmc
  2. This PR is way too large. It can be done incrementally with much easier to see increments. I understand it is done with a series of commits, but those get squashed and lost. It is really not a problem to do a series of PRs if you let us know where it's headed.
  3. I was working towards this myself recently with a series (not all in one) PRs already merged. I had to put it on the shelf in order to get some other serious bug reports resolved, and take over things like @SSoulaimane 's abandoned PR. I want to get it done because it will help me in debugging other platforms without needing a setup for those platforms.

@Geod24
Copy link
Member Author

Geod24 commented Jan 20, 2021

These sorts of changes make my life difficult as I have to redo them to ensure they work with the other backend's Digital Mars builds. For example, you removed the 16 bit support. Dead code to DMD may not be dead code for DMC. See https://github.com/DigitalMars/Compiler/tree/master/dm/src/dmc

Thanks, I've been looking for this for a while. I'll see if there's a way to set it up that does not break DMC all the time.

This PR is way too large. It can be done incrementally with much easier to see increments. I understand it is done with a series of commits, but those get squashed and lost. It is really not a problem to do a series of PRs if you let us know where it's headed.

Commits do not get squashed unless someone squash them, which would be a mistake. But point taken, I'll submit them separately.

I was working towards this myself recently with a series (not all in one) PRs already merged. I had to put it on the shelf in order to get some other serious bug reports resolved, and take over things like @SSoulaimane 's abandoned PR. I want to get it done because it will help me in debugging other platforms without needing a setup for those platforms.

Which is why I actually done this, as I imagined it'd be uncontroversial.

@Geod24
Copy link
Member Author

Geod24 commented Jan 20, 2021

@WalterBright : I hope DigitalMars/Compiler#12 can be a good way forward.

@thewilsonator
Copy link
Contributor

platform independence has been achieved by #12508 closing. Please reopen if this adds something that PR doesn't fix

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

Successfully merging this pull request may close these issues.

7 participants