Add MSP430, D_P16 version identifiers documentation

[luismarques]

The MSP430X should match with version(MSP430), since it's a small set of backwards compatible ISA extensions (with no mode change), and an MSP430X can be used as an MSP430. In fact, you can use the GCC options -mlarge and -msmall to change between 16-bit and 20-bit pointers (16-bit size_t, 32-bit size_t). The MSP430 and MSP430X proper can be diferentiated when really necessary by using version(D_P16).

[dlang-bot]

Thanks for your pull request, @luismarques! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

[JohanEngelen]

I have no opinion on this. I believe we added the D_P16 version identifier because there is also D_LP64 (which according to dlang.org means pointers are 64bit and does not mean the LP64 ABI).

[luismarques]

Same as in https://github.com/dlang/dmd/pull/6994: @MartinNowak D_LP64 is a 64-bit version identifier flag. It does not indicate the LP64 ABI, despite the misleading name. There are several uses of D_LP64 in druntime as 64/32 bit toggle. For instance, one of the first things in object.d is a version(D_LP64) ... else ... to set the types of size_t and ptrdiff_t. So there is precedent for using a version statement instead of a static if. Furthermore, in cases like that one, where we need to introduce an else version(D_P16) to cover the new 16-bit case, having to change the whole block to a static if would introduce further disruption.

[jpf91]

The MSP430X should match with version(MSP430), since it's a small set of backwards compatible ISA extensions (with no mode change), and an MSP430X can be used as an MSP430.

👍 MSP430X should match MPS430 version, but a distinct MSP430X version can still be added. This is what GCC does so it's familiar to C programmers:

      builtin_define ("NO_TRAMPOLINES");        \
      builtin_define ("__MSP430__"); 		\
      builtin_define (msp430_mcu_name ());	\
      if (msp430x)				\
	{					\
	  builtin_define ("__MSP430X__");	\
	  builtin_assert ("cpu=MSP430X");	\
	  if (TARGET_LARGE)			\
	    builtin_define ("__MSP430X_LARGE__");	\
	}					\
      else					\
	builtin_assert ("cpu=MSP430"); 		\

[MartinNowak]

You can easily mix version and static if, and you might already need to distinguish between 16 and 20-bit inside of version (MSP430).

version (D_LP64)
{
}
else static if ((void*).sizeof == 2)
{
}

Lots of druntime is still X86 centric hence D_LP64 is used to detect 64-bit pointers (and maybe there are some wrong usages). In phobos D_LP64 isn't used at all, but you'll find quite of (void*).sizeof and size_t.sizeof. Defining size_t and ptrdiff_t depending on D_LP64 and D_ILP32 (not currently implementd) is exactly what constitutes those ABIs, no? I'm not strictly against adding this, but it seems to be a precedent with no equivalence in GCC/clang.

[MartinNowak]

Using MSP430, MSP430X, and MSP430X_LARGE indeed seems like a better fit, in particular when porting headers.

[luismarques]

In general I find it aesthetically strange having a version identifier flag for 64-bitness and then using a different mechanism for detecting 32 vs 16 bits, but it's certainly not the end of the world.

Defining size_t and ptrdiff_t depending on D_LP64 and D_ILP32 (not currently implementd) is exactly what constitutes those ABIs, no?

This discussion is becoming confusing, because D_LP64 means only 64-bit pointers (despite the name), while your hypothetical D_ILP32 supposedly would mean the ILP32 ABI. So I'm not quite sure what to make of your point here.

I would prefer seeing a coherent mechanism going forward (if we aren't going to have version identifiers to distinguish between all of the possible pointer sizes, why even use D_LP64?). I don't really have strong opinions on the version vs static if (although I prefer the version() because it's easier to read) but this mismatch seems a bit unprincipled to me (these are the kinds of things that probably don't matter too much individually, but together tend of add confusion, conceptual complexity, etc.).

[JohanEngelen]

I think @MartinNowak perhaps meant that some (most?) of the version(D_LP64) in druntime should be replaced with static if((void*).sizeof == 8)? (it's more verbose and you have to do the calculation 8*8 yourself; but it is clearer in meaning and Phobos seems to use it)

[luismarques]

That would be fine by me.