versions: add D_REG64, remove D_X32, document X32

[WalterBright]

Addresses https://issues.dlang.org/show_bug.cgi?id=24412

@ibuclaw @kinke I need your OK on this, because DMD doesn't support these.

[dlang-bot]

Thanks for your pull request, @WalterBright!

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.

[TurkeyMan]

1. I don't like this name, but I can live with it. I would prefer D_64Bit, or D_64_Bit.
2. That comment is wrong. Register width is not related to pointer width. Pointer width may be related to register width, but register width 'just is'. There is no reason to mention pointer widths in the spec here, and it certainly shouldn't be set according to the condition written here. This should be set if the CPU has 64bit integer registers, period.

[WalterBright]

Ok, I updated the comment.

I used "D_REG64" because D_64Bit implies pointers are 64 bits, as that is far and away commonplace.

[TurkeyMan]

Is this right?
I would expect that if X32 is set, then LP_64 is NOT set, and X86_64 IS set; X32 addressing model doesn't make sense on a non-64 bit architecture.

A predicating question though is how dlang should specify this X32 concept.
It's slightly confusing because X32 tends to refer to a precise ABI specification with respect to the x86 architecture, where 32bit pointers are used on a x86_64 arch.
HOWEVER, the general concept; 32 bit pointers on a 64 bit arch is common, and present for all 64 bit architectures... we need a version specifier for this.
We may choose to use the version token X32 to refer to this concept generally in an architecture independent way and it should be documented as such (I would go this way if it were my call), or we may choose for X32 to only apply to x86 targets specifically, in which case it should not be specified for any non-x86 arch, and we need to nominate another name for the general concept in addition to this.

Your call...

[WalterBright]

It's what is implemented. Whether that is right or not is another thing. The "do not use" is there because X32 is a C memory model, and does not really apply to D. Using it will always be confusing.

[TurkeyMan]

Upon closer look, I see what you did here; you removed D_X32 as a general concept, and added X32 as an architecture.
I think that logic is wrong.

1. X32 is NOT an architecture, and shouldn't be confused for one. It's an ABI for the X86_64 architecture. It would be improper to define it in place of X86, x86_64, it should be defined together with X86_64.
2. The D_X32 identifier you removed is better suited to describe the general architecture-agnostic concept I mention above, and I would leave it named that way and spec if to describe 32 bit pointers on >32bit arch as it did before, but tune the text to invite the arch-agnosticism to the identifier. There are many such X32 platforms in the wild, and this version constant is already well positioned to identify them.

Some common non-x86 cases with hundreds of millions of installed units:
Playstation 2 - MIPS
Playstation 3 - PowerPC
XBox 360 - PowerPC

[WalterBright]

I simply documented what was actually implemented. D_X32 is not implemented, and X32 is implemented. I documented what X32 did (right or wrong). I added "do not use" for it because, as you say, it is confusing. I was afraid if both D_X32 and X32 were predefined as different things, this would be a swamp of confusion for users. Heck, I can't even keep in my head what the distinction is.

I'm not a fan of overlapping version identifiers. There should be an orthogonal set.

[TurkeyMan]

You can't make a linear enum of identifiers when conflating several axiis into one. Architecture, ABI, and bit-ness are 3 different dimensions, and if you squash them into a linear (mutually-exclusive) set the space is cubic.

Can you explain how you see these as overlapping? X32 is NOT among the same set as X86, X86_64, PPC, MIPS, etc. The change you appear to have made here, is to reposition D_X32 into the arch set, where it doesn't belong.

[TurkeyMan]

I simply documented what was actually implemented

I'm not convinced that the spec should be dictated by what is in this case essentially a bug in DMD... especially where this thing barely affects DMD, and almost entirely affects GDC/LDC...

[ibuclaw]

Indeed, dmd should reflect the spec, not the other way around. Especially as gdc already documents and implements this correctly for around a decade now.

https://gcc.gnu.org/onlinedocs/gdc/Target-Predefined-Versions.html

Then there's druntime which already makes use of D_X32 in the correct way since ~2012 too.

https://github.com/dlang/dmd/blob/01418426ece2c2ef484c847748141a04af2ddec7/druntime/src/core/sys/posix/sys/types.d#L43

[WalterBright]

The gdc documentation linked to says:

D_X32
X86
X86_64
Versions relating to the x86-32 and x86-64 family of processors.

which says nothing about what D_X32 means.

But anyway, it sounds like D_X32 is what you, @TurkeyMan, want?

[TurkeyMan]

Are we talking about my issue on the bug tracker? If so, I'm not sure how many times I can repeat; my bug has ABSOLUTELY NOTHING TO DO WITH POINTERS.
And what I mean by that, is that it literally has nothing to do with pointers!
I just want to know if the target arch is 64bit. I think that's an important thing to know. There are several algorithms I often use which are 64-bit only, and a different implementation is required for 32bit.

Pointer width is an ABI/OS thing. This isn't an OS/ABI question.

That said, if you're asking on the X32 tangent which has become a larger conversation than the issue you're trying to fix, then my opinion is that the X32 change in this PR is wrong; it's incorrect to reposition X32 beside the architecture tokens, because it's not an arch.
But just once again, that's not at all related to this bug report, and I think you should remove the X32 stuff from this PR, it doesn't belong here, and evidently we're spending all of our energy talking about that totally unrelated thing.

[ibuclaw]

which says nothing about what D_X32 means.

But anyway, it sounds like D_X32 is what you, @TurkeyMan, want?

It's a version relating to x86, not an abstract concept.

[TurkeyMan]

This PR seems to be about adding D_REG64, but the other X32 change feels unrelated and shouldn't be in this PR, otherwise we're going to argue about that instead of the actual thing.

I still prefer D_64Bit to D_REG64...

[WalterBright]

I understand, but D_64Bit will inevitably imply 64 bit pointers and registers.

[TurkeyMan]

I understand, but D_64Bit will inevitably imply 64 bit pointers and registers.

I'm convinced by that argument. I'm not sold on the X32 stuff though, and I think it should be removed from this PR.

[kinke]

LLVM offers a getLargestLegalIntTypeSizeInBits(); 'legal' meaning 'a native integer type supported by the CPU' (see https://llvm.org/doxygen/classllvm_1_1DataLayout.html#a4865a13a415f04576388b969d04d1541).

We could expose this value (probably in bytes, not bits) via something like __traits(getTargetInfo, "largestNativeIntegerSize") instead of defining a bunch of versions.

[TurkeyMan]

@kinke Yeah that's a valid solution. What's important is that it's in the spec and supported for all compilers.
I've never really been able to resolve whether as a principle we should be using version or static if; approaching the concept from the angle you propose, it kinda feels like we should present all associated information that way (rule of least surprise and all that), like a big targetinfo struct full of details as a manifest constant... but there are already numerous version tokens in place.
Ultimately, I don't really care how this is solved, just that it's needed in a reliable and implementation agnostic way ideally.

[kinke]

I've never really been able to resolve whether as a principle we should be using version or static if

IMHO, it depends. static ifs might run into analysis-order issues, especially at module-level. But such a manifest constant is much better suited for non-binary values. Just like LLVM, we could define a value of 0 if that size is unknown for the specific target, instead of defining no version at all (and then possibly having to detect that by checking all versions). And some __traits(getTargetInfo, "largestNativeIntegerSize") >= 8 will continue to work as soon as we have targets with 128-bit native ints etc., without having to revise all blocks of the form:

version (D_REG32)  version = atleast_REG32;
version (D_REG64)  version = atleast_REG32;
version (D_REG128) version = atleast_REG32;

[TurkeyMan]

Well in this hypothetical case where a system is created with 128bit int regs (PS3 SPU has this), I would expect all 3 versions to be defined (not just the one that is the biggest one), and then my algorithms gated on version(D_REG64) would work naturally.
Ie, if 64bit machine, then your hypothetical D_REG32 and D_REG64 are both defined because both natively supported, but not D_REG128.
I agree version wrangling is bad, and if there's a possibility of that, it should be expressed some other way. But this particular case would seen to be a case that I see as being 'compatible' with versions... even though I'm not sure I ever really bought into the version doctrine.
Anyway, whatever works, just go with whatever feels more natural and precedented.
It was actually me who introduced getTargetInfo years ago for this kind of data, so I obviously don't object to it being there! :)