-
-
Notifications
You must be signed in to change notification settings - Fork 380
Update predefined versions #243
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
Conversation
cc @dawgfoto |
You could just say it only affects the pointer size and nothing else. |
The problem is that But probably I should really formulate it in a shorter way. Regarding the definitions in The only place where we need to know about LLP64 LP64 ILP64 and SILP64 in D are those definitions in |
I'd rather like to see a transition to X86_32. |
Strange Also updated the |
You made it up. version (X86) version = AnyX86;
version (X86_64) version = AnyX86; I don't think we should repeat this for every architecture we're adding. |
Um I agree but I don't know what would be a clean solution. Having the versions without I'd personally continue defining AnyMIPS manually and lobby for version expressions. Sooner or later we'll need those anyway. Or maybe we can sneak in a static if(isVersion!"MIPS" || isVersion!"MIPS64")
{
} |
It's ought to be for all architectures. Even if we have recombinations the fundamental issue remains. A version block performs an intersection so we'll often need the complete set to start with. If we don't have that, one has to list all architectures, always. Lots of useless repetition for the most common case.
We have a proved solution to this. |
The only argument I can come up with to favor combination is the one Walter uses to chose |
Using X86 is traditional for 32 bit code on the x86 processors. Using X86_64 is traditional for, you guessed it, 64 bit code on the x86 processors. I don't see any compelling reason to change these. |
A compelling reason would be too keep version identifiers consistent, so If There's not really a traditional meaning, every C compiler has a different predefined macro for X86*. But in C you can at least do ** http://sourceforge.net/p/predef/wiki/Architectures/ Deprecating Maybe we need "complete sets" in general, but we could make an exception for architectures as there are only two choices. You could always get the complete set with Or we add |
I think enough use cases for Boolean expressions in |
Walter you are right about the historical meaning of X86, maybe I am off the track here and focused too much on consistency. I think we will benefit from staying close to C tradition with this. Indeed we would not lead this discussion if version identifiers were recombinable and could be imported. We would simply add a D module that does the mapping, DRY. If we do not solve this enums will supersede version identifiers at some point. I just noticed I already used this myself to get importability. In the meantime AnyX would be helpful. |
This is true IF you are trying to use version blocks in the same way one does in C. However, that is the stylistically wrong way to do it in D. The recent attempt to replace in the DMD source code a bunch of the operating system predefines with POSIX was a failure, and the reason for the failure was a fine example of why the C approach is inferior and why it isn't supported in D. (The Boehm GC source code is a extreme, horrific example of how the C approach goes terribly wrong.) The evil happens when you add an architecture to the A=B||C||D and then blithely assume that all the version(A)'s will now be correct. They never are. A far more robust and organized approach is to figure out just what you're trying to abstract away in these version blocks, and then abstract it away as a struct, class, function, or import. Then, version (B) This is clear, simple, organized, and forces the programmer adding E to check each one of those, instead of assuming they will work. Note: feature() can also be a struct, a class, an import, etc. If your code is a mess of version declarations, strongly consider rethinking and refactoring. (One of my goals is to remove all the #if's from the dmd source code using this technique.) |
You are talking about platforms. We are talking about architectures. The latter are much less likely to break when generalizations are made because all relevant ISA designs from the last 20 or so years are extremely focused on backwards compatibility. It is a simple fact that a lot of inline assembly written for a 32-bit architecture will run perfectly fine on the 64-bit equivalent, for instance. It is also worth noting that version identifiers can be set on the command line by users of the language. We cannot make questionable decisions about how people must use them just because of short-sightedness. |
DRY was mentioned. In this context, I believe it is a false god. Operating systems do not change in lock step with each other, despite purporting to present the same interface, They never do, exactly, and things have to be exact. Making adjustments for one system must not break other systems that use the same version, and the person doing those changes rarely (or frankly, never) checks them all, causing bit rot. Time to throw the C style of versioning under the bus. |
You're right that code is likely to run, but it definitely does not behave the same in terms of performance. Different CPUs in the same line suggest different instruction mixes, sometimes very different mixes. And yes, there are sometimes subtle differences in the execution results. Trying to save a few bytes, or even many K bytes, of source code is not a worthy goal. I know that D users will continue to try and write C style version blocks, but I will continue to try and show a better way. I also am painfully aware that druntime/phobos have turned to C style versioning, despite my exhortations. It should be fixed, not embraced. |
Please let's organize the discussion. The version expression stuff should go to Bugzilla 7417, maybe we should archive the side discussion. |
This pull request affects mapping these architectures+x86+x86_64 onto this kind of problem. |
So about this pull request: Shall we wait for results of the bugzilla discussion or shall I update this pull request with |
It's all about MIPS for kernel constants, they managed to use those consistently across all asm code. |
@dawgfoto: I don't have a strong opinion on the topic and will merge any patches to make LDC conform to the official list. That being said, I do think that we should come up with a consistent, well-designed set of identifiers for the various systems. And do it soon, because there is actually work going on concerning platform support – @jpf91 is (was?) working on ARM support for GDC, @redstar is porting LDC to Linux/PPC64, … In short, I think we should put effort into working out a good design now, because changes are still relatively painless, but they might no longer be soon. By the way, the »correct« way of introducing GDC-specific versions would be using a vendor prefix ( |
$(TR $(TD $(D ARM_Thumb)) $(TD ARM in Thumb mode (AArch32:T32))) | ||
$(TR $(TD $(D ARM_Soft)) $(TD The ARM $(D soft) floating point ABI)) | ||
$(TR $(TD $(D ARM)) $(TD The ARM architecture (32-bit) (AArch32 et al))) | ||
$(TR $(TD $(D ARM_Thumb)) $(TD ARM in any Thumb mode)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really familiar with ARM, so please pardon my ignorance, but: Is this actually needed? What kind of code would be specific to Thumb, but work on both versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thumb1 is a strict subset of Thumb2 so any Thumb1 code would work in both variants. I do not really care about Thumb mode right now though and even if we supported Thumb mode, Thumb1 should be good enough to implement almost everything. The problem is how do we mark ARM non-Thumb assembly correctly (and it shouldn't break even if a Thumb3 is introduced at some point). But I guess if we make it clear that ARM_Thumb
is set for every Thumb variant we won't need ARM_Thumb1 ARM_Thumb2 for now.
version(ARM_Thumb)
else version(ARM)
EDIT: (I didn't know that though when I initially wrote this pull request so I'll update it and remove Thumb1/Thumb2)
And @dawgfoto is working on MIPS ;-) Regarding ARM work I'll continue that soon. I wanted to get the unit tests running on gdc-4.7 first though. I think LDC is in better shape in that regard, gdc still doesn't pass all unit tests or the test suite. Regarding vendor specific versions: @dawgfoto I think the The real problem is the I don't really have a strong opinion on how to implement this but I'd really like all version identifiers to have a consistent and clear meaning. |
Probably not because as Walter correctly stated it has a traditional meaning outside of D.
I think we should go with AnyMIPS/AnyX86/AnyARM then as it already emerged in existing Code.
|
I've added the |
Should be |
thx |
So this is okay to merge regardless of the side discussions I reckon. Is that right? |
No, @jpf91 still needs to make a few changes |
Changed |
I do not understand what the AnyX86 is for. I don't see any use at all for it. |
OK, let's see a real world example:
These declarations enum O_CREAT = 0x0100;
enum O_EXCL = 0x0400;
enum O_NOCTTY = 0x0800;
enum O_TRUNC = 0x0200;
enum O_APPEND = 0x0008;
enum O_DSYNC = O_SYNC;
enum O_NONBLOCK = 0x0080;
enum O_RSYNC = O_SYNC;
enum O_SYNC = 0x0010; are identical on Now I challenge you to write the above declarations so that they are:
This is not an artificial problem. Anyone porting druntime/D to a new architecture will face it and it is very annoying. You keep telling us that we're using Note that the solution you stated above ("abstract it away") is not really possible in this case. I also have to doubt this statement:
This may be true in some cases and of course everyone should carefully check all affected code before adding an architecture in such a definition. But in that case the C approach Your solution is exchanging the problem of wrongly added versions by programmer sloppiness with code duplication and a maintenance mess. I doubt the maintenance mess is better. In druntime/phobos/dmd code we can enforce this but 3rd party libraries will just use (And this is only about combining versions, the fact that versions can not be imported deserves an extra post. Even gdc cheats because of that and the only reason version statements work for architectures is because they're defined by the compiler. Manually defining a custom version is basically useless/impossible right now and even outright dangerous if the version statement affects the ABI) |
Here's one way to do it: module foo; version (linux) |
@walter - That's ugly, and it hides the fact that most of these values are exactly the same on all systems. With my proposal from 7417 it would be: version (AnyMIPS) { ... } |
The salient point is "most". Not all. I've found time and again that different systems have the same names, but one or two of the values are different. It's maddening, and it causes subtle and terrible problems when someone decides that SystemA declarations can be merged with SystemB's without going through them line-by-line. Worse, when SystemA and SystemB share the declarations code, and Fred issues a pull request to fix things for SystemA, then SystemB unwittingly gets broken. Instead of AnyMIPS, all you would need is:
No, I don't think that is ugly (though of course aesthetic taste varies), all the versioning is done in one import and is not embedded in the various modules themselves, and the expert on OSX can maintain osx.foo without worrying about its impact on any other system. I've been drifting towards this style for years (you can see it in D1 Phobos in an early form) and I think it works well. I'm sorry that the D2 library has gone back to the C way. |
Walter - you're basically advocating copy-and-paste. I've done what you suggest several times, and without fail, it has led to a maintenance nightmare. |
@WalterBright your solution is flawed: It does work in this case as there are only declarations, but as soon as there is code which must be compiled and not only imported (e.g. a small function replacing a C macro) you have to compile Now how do you make sure BTW: Your solution is the "glibc" way. Glibc also outsources some declarations into arch directories. But maintaining that is a mess. A similar way would be to use git and have one generic branch, then keep architectures as branches in git. This at least allows easily merging common changes without copy & paste. |
OK, so what should I do to get this pull request accepted? If I removed the @dawgfoto is that OK with you? This would mean you'd have to declare |
@jpf91 I won't argue that my solution is perfect. I'm arguing it's better, and that it's more tasteful. To solve your issue with mips.foo.d, I'd add the following code into it:
No, it ain't beautiful, but it gets the job done with minimal hassle and without throwing out all the other benefits of the approach I suggested. Don, it's hard for me to respond to your case where things led to a maintenance nightmare without more specifics. It's possible that I have badly explained the idea and you've implemented something else. I can tell you that I've had maintenance nightmares from the old way. Just recently, there was a pull request where all the system macros in dmd were pulled into a POSIX macro, and that was used instead. Well, it bombed in the test suite. Nobody went to figure out why, and it was reverted. That was an easy one, but it was hardly the only problem I've encountered with attempts to save keystrokes by combining declarations from different systems into a common source base. I've repeatedly run into issues where different systems had one of the values for a list of constants be slightly different. Just enough to superficially look the same, so it wound up in the common section, and yet it would fail in mysterious ways. I freakin' hated debugging those. |
If we can't reach consensus then leave them out. Most of the other things look good.
Don't overstress this please. |
I don't care that much about upper/lower case, what I want to avoid is having two version identifiers which sound like the same but have a different meaning. I looked up the other architectures and the casing doesn't match most of the time: I would just leave those as is, but I don't mind changing them. |
Is there any reason we don't have |
I only can find mentions of MEABI in gcc-3.3 docs. In more recent releases (http://gcc.gnu.org/onlinedocs/gcc-3.4.1/gcc/MIPS-Options.html http://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/MIPS-Options.html) MEABI isn't mentioned anymore. I have no clue what happened to it though. |
Related question: Since the EABI (the one that hasn't mysteriously disappeared) has 32-bit and 64-bit variants, should we have |
I guess the intent with the MIPS ABIs was to follow gcc which defines |
Anything that does not map 1-to-1 to the ABIs and C defines is a mistake in my opinion. I think O64 is ILP32 btw. |
OK, can we merge this then? Any further comments? |
* Consistent naming: SoftFP --> SoftFloat (on ARM SoftFP != SoftFloat) HardFP --> HardFloat * DOC: ARM_Thumb means any Thumb version * ARM/ARM64 fix reference regarding AARCH (AARCH is an arm8 term but ARM is also defined for ARM7,6,5...) * Rename ARM64 to AArch64 which is the official name * Remove MIPS: MIPS is the same as MIPS32, MIPS32 is the official name * Remove MIPS_NoFloat: This should be a generic version D_NoFloat or it shouldn't be defined at all. * D_X32: Add a note that defining both D_X32 and X86_64 is valid/expected * D_LP64: We only talk about pointers, this is not the C LP64 data model!
Please merge |
There wasn't much feedback on the newsgroup so I've implemented the suggested changes. Please give some feedback if those changes are OK.
HardFP --> HardFloat
but ARM is also defined for ARM7,6,5...)
or it shouldn't be defined at all.
valid/expected
model!