dmd.mars: Always produce COFF when targeting Windows#12825
dmd.mars: Always produce COFF when targeting Windows#12825ibuclaw wants to merge 1 commit intodlang:masterfrom
Conversation
ibuclaw
requested review from
Geod24,
MartinNowak,
UplinkCoder,
WalterBright and
rainers
as code owners
|
Thanks for your pull request, @ibuclaw! Bugzilla referencesYour 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 locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#12825" |
MoonlightSentinel
added
the
Review:Needs Changelog
|
Should probably come with a way to revert to OMF if anybody actually wants/needs to use it (maybe |
|
Thanks, just trialing things at the moment (I guess I will probably end with removing the OMF pipelines?). Tah @thewilsonator - looks like what @ManuEvans has previously done would be a good baseline to begin from. |
|
CC @TurkeyMan |
ibuclaw
force-pushed
the
disable-omf
branch
3 times, most recently
from
531c3ca to
cd0c687
Compare
|
Only had a cursory blast through the codebase, I have no means of testing, so just waiting for the CI results. |
|
This can be split up into two logical PRs (well, three to remove the dependency on target variables). No point doing that unless together they all work. |
| global.params.libfiles.push(phobosLibname.xarraydup.ptr); | ||
|
|
||
| if (target.mscoff) | ||
| if (target.omfobj) |
There was a problem hiding this comment.
I would prefer if you just negated the condition and left the associated branches as they are to reduce unnecessary diff.
Yes, unless they are still supposed to be tested with |
I better add |
WalterBright
left a comment
WalterBright
left a comment
There was a problem hiding this comment.
Changing 18 files and 200 lines just to change a default? DMD is not ready to be packagized. But I digress.
I've hitched my wagon to Microsoft Link before. The troubles are:
- Versions. Every customer has a different version of that linker. Some would work, some wouldn't. I couldn't just ship someone with a non-working MS-Link a linker that did work.
- Bugs. Microsoft placed no priority whatsoever on fixing bugs. I was faced with being dead in the water for indeterminate times. Obviously, this was untenable.
- The compiler didn't work out of the box. I had to tell customers they had to buy some MS dev system to get a linker.
- When MS-Link didn't work, or crashed, etc., I always got blamed.
Having a linker I had control over resolved these issues.
Nothing has changed with those points. It's important that DMD work out of the box. I know that we rely on MS-Link for 64 bits, but at least they can get their code working by default out of the box.
We can fix Optlink bugs if we need to. I have fixed many. We cannot do jack about MS-Link bugs, except desperately search for some workaround. No attempt has been made even to figure which line of code triggered the Optlink error. Maybe it is trivial, maybe it isn't. That's not really a reason to just abandon it.
I oppose this PR.
I have counted 3 issues in the just as many weeks related to OMF pipeline failures. One of which caused the contributor to spend almost a month going round in circles trying to isolate and work around it. This is not an acceptable use of anyone's time, irrespective of whether they did this on their company time or not. |
So even with bugs fixed, IIUC, the main problem is that OMF is not used by other toolchains and users are surprised. (Or are there other UX problems with OMF ?)
Partial answers: I believe that the multiple toolchains (and the two object file formats) in Windows have been a driving force behind COM, and that COM has proven useful not just for fixing compatibility (e.g.: it is useful for fine-grained versioning, including in open-source projects, not just for Windows -- 7zip and, until recently anyway, Mozilla). Also, are we not glad, as users, that there are two competitive open-source C++ toolchains ? And that Visual C++ is also available and competitive ? The opposite of a monopoly (such as Chrome's engine in the world of browsers). @WalterBright : If OPTLINK is significantly faster than ld, gold and lld, than fixing bugs and making it scalable could encourage both users and contributors. Some projects suffer from long link times and high RAM usage. Currently, I just wish to ask whether it is indeed significantly faster. If so, is that still attributed to it being written in Assembler ? Or is there something in OMF which is not in COFF which makes it faster ? |
AFAIK that has no relevance to Win32.
AFAIK it was just assumed to be an optlink bug, because it didn't present with mscoff. But there are other differences between mscoff and omf code generation which could very well be the problem (i.e. a codegen bug). There could be other sources of the problem, too, like the system import libs for OMF were built wrong. I suspect that because the very small number of lines of code in that project were using unusual Windows API calls. It just seems peculiar to me that adding a hundred lines of code to dmd suddenly causes Optlink failures, when we add hundreds of lines of code all the time to dmd and there are no such failures. So what's different about it? The different Windows API calls. Another common source of strange errors when changing the toolchain is inadvertent use of undefined behavior, like uninitialized stack variables (though that shouldn't happen with D). I've certainly encountered many such coding bugs in the past, and one reason why supporting multiple tool chains is a good idea is it exposes latent bugs. It's too easy to dismiss it as Optlink, as the actual problem was never found. Now, if Optlink does produce the Unexpected Termination display, that is definitely an Optlink bug. |
I filed one of them. Were bugzilla issues filed for the other two? |
|
I would like to stress again that we ship |
Fraying 32-bit support in general is blocking meaningful progress on all platforms. 32-bit DMD Linux releases have had the compiler segfaulting for a few years now (unless you are running on an EOL system). The fix is easy, but PIC bugs in the DMD backend are blocking it.
Another reason to sunset OMF. With COFF, if a problem occurs in DMD pipelines, you can try compiling the same code with either GDC or LDC to see if the same crash occurs. If it doesn't, you know the fault is DMD. Because neither GCC or LLVM supports OMF we have no way to track down, verify, or debug problems.
People have been complaining for years in bugzilla, on forums, in other D projects repos. It would be harder to dismiss if this was just a one-off, or a rare occurrence. But in reality, the backlog of evidence is too damn high, and first-time contributors continually run into heisenbugs that manifest only on DMD 32-bit. |
Can such unacceptable spending of time and effort not be avoided (or at least reduced) by having the DMC/OPTLINK/OMF check temporarily be marked as not required for merging of checked dlang/druntime/phobos PRs (but still performed by our Github pipelines) ? |
We release OMF binaries, so unless that stops... |
For me personally, it is my own laziness and lack of ability which is blocking me from learning to work on OPTLINK. If I had been able to fix the bug (edit: or at least understand it) I am sure I could convince the maintainer to merge that fix. No, what I really want is support for this toolchain, but at the expense of effort made by others. Why do I want it ? I have tried explaining, but got "How many others want support for DMC/OPTLINK/OMF ?". It is difficult for me to answer that. |
IIUC, you want support for OMF to be eliminated completely, and you have been using the argument of time and effort needed to investigate fails of OMF checks. I am proposing, as a compromise, from the point-of-view of a non-contributor and of possibly a weird user, that such failures are simply not investigated. I feel this leaves at least some form of support for OMF. Regarding changes of default target from OMF to COFF, FWMOIW (for whatever my opinion is worth): I would vote against, but if you feel strongly for that, I can live with that just fine (edit: plus I am not important). Why have I dared answer to this conversation ? I felt that the following question was being asked: "Is anyone still using OPTLINK or OMF [or interoperating with DMC] [edit: or Embarcadero] [or happy for small size of toolchain and independence] [or appreciative of testing with multiple toolchain versions] ?" So I have answered. Have I been desconsiderate of other people's time and effort ? Maybe. I apologize if so. Until today, I had not been aware that OPTLINK had bugs. Even so, dropping it feels too harsh to me, especially when a compromise of ignoring that pipeline (until OPTLINK is fixed, if ever) can IMO be made (sad as it may seem to me) (but, then again, I am too lazy to learn OPTLINK). |
gold is faster than ld, but is it faster than lld or vice-versa ? |
I can't remember the last time I or anyone else recommended anyone to make a 32bit build on Discord. Straight to 64bit. Ever since dub switched to coff by default for 32bit, I don't remember having to question this at all. |
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
This rebases dlang#12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues.
* Produce MS Coff by default when targetting windows This rebases #12825 but doesn't attempt to rename phobos libraries and doesn't change the build infra where possible(and doesn't have a large diff in `link.d`). See if this fixes the CI issues. * remove OMF piplines * Update glue.d * Update target.d * Update target.d * Update frontend.h
|
Reading through this discussion invokes an overwhelming and visceral sense of PTSD. |
@TurkeyMan at least optlink is now bowing out and coff is the default for 32bit now too. :-) |
|
@TurkeyMan this was closed because I rebased it and that got merged. |
11 participants
Testing whether Azure pipelines will pass. Optlink is causing issues, again...