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.

Sign up for GitHub

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

versions: add D_REG64, remove D_X32, document X32 #3790

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

versions: add D_REG64, remove D_X32, document X32 #3790

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion spec/version.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ $(H3 $(LEGACY_LNAME2 PredefinedVersions, predefined-versions, Predefined Version
$(TROW $(ARGS $(D CppRuntime_Sun)) , $(ARGS Sun Cpp runtime))
$(TROW $(ARGS $(D X86)) , $(ARGS Intel and AMD 32-bit processors))
$(TROW $(ARGS $(D X86_64)) , $(ARGS Intel and AMD 64-bit processors))
$(TROW $(ARGS $(D X32)) , $(ARGS Set when D_LP64 is not set and X86_64 is not set. Do not use.))
Copy link
Contributor

Choose a reason for hiding this comment

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

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...

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@TurkeyMan TurkeyMan Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@TurkeyMan TurkeyMan Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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...

Copy link
Member

@ibuclaw ibuclaw Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

@TurkeyMan TurkeyMan Mar 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

$(TROW $(ARGS $(D ARM)) , $(ARGS The ARM architecture (32-bit) (AArch32 et al)))
$(TROW $(ARGS $(D ARM_Thumb)) , $(ARGS ARM in any Thumb mode))
$(TROW $(ARGS $(D ARM_SoftFloat)) , $(ARGS The ARM $(D soft) floating point ABI))
Expand Down Expand Up @@ -327,7 +328,7 @@ $(H3 $(LEGACY_LNAME2 PredefinedVersions, predefined-versions, Predefined Version
$(TROW $(ARGS $(D D_InlineAsm_X86_64)) , $(ARGS $(DDLINK spec/iasm, Inline Assembler, Inline assembler) for X86-64 is implemented))
$(TROW $(ARGS $(D D_LP64)) , $(ARGS $(B Pointers) are 64 bits
(command line switch $(DDSUBLINK dmd, switch-m64, $(TT -m64))). (Do not confuse this with C's LP64 model)))
$(TROW $(ARGS $(D D_X32)) , $(ARGS Pointers are 32 bits, but words are still 64 bits (x32 ABI) (This can be defined in parallel to $(D X86_64))))
$(TROW $(ARGS $(D D_REG64)) , $(ARGS Set when the CPU has 64 bit registers))
$(TROW $(ARGS $(D D_HardFloat)) , $(ARGS The target hardware has a floating-point unit))
$(TROW $(ARGS $(D D_SoftFloat)) , $(ARGS The target hardware does not have a floating-point unit))
$(TROW $(ARGS $(D D_PIC)) , $(ARGS Position Independent Code
Expand Down