Create non-primitive-constant-data.md #46

davidwrighton · 2018-08-11T01:49:22Z

Runtime level non-primitive constant support

jkotas · 2018-08-11T05:28:01Z

There is existing very similar proposal https://github.com/dotnet/corefx/issues/26948

The differences between the two is:

This proposal is extended to handle arbitrary structs. The older proposal was scoped to primitives and Span because of that's what C# syntax was designed for.
RuntimeFieldHandle vs. field address + generic argument. Is there a good reason why to choose field address and generic argument, and not just follow the existing InitializeArray pattern with RuntimeFieldHandle ?
API names - the API review folks converged to CreateSpan as the preferred API name

jkotas · 2018-08-11T05:32:08Z

cc @VSadov

jkotas · 2018-08-11T05:37:30Z

proposed/non-primitive-constant-data.md

+        return LoadConstantData(ref inData, 1)[0];
+    }
+
+    // General replacement for ArrayInitialize pattern, also handles non-primitive constants.


Alternative design could be to teach the existing InitializeArray API to support more types, and not introduce new API.

InitializeArray requires that there is an array instance. Span-based API feels a bit more flexible.

It might be nice if InitializeArray works with generalized const-able types, but having span based APIs, it would not be strictly necessary. Compiler couild just emit ToArray or something.

Sure, its straightforward to enable InitializeArray API to support more types. I generally prefer to avoid extending existing apis, but that would be reasonable to do.

jaredpar · 2018-08-13T15:43:55Z

proposed/non-primitive-constant-data.md

+
+### Definition of types which can be represented as constants
+- Must be a struct
+- The struct must have sequential layout


What about explicit layout?

I think the requirement should just be that the type has a constant size (so no auto-layout and no pointer or pointer-like types).
If the user opts-for Sequential layout, they are responsible for ensuring that the layout is the same on all systems; or the runtime should throw when the constant size and the actual size are different

I think the requirement should just be that the type has a constant size

That is not sufficient to make this work on bigendian platforms.

Layout is sufficiently variable that it must either support explicit layout only(without overlapping fields), or sequential layout could also be supported. If sequential layout is supported at all, it doesn't make sense to put arbitrary restrictions on it, like must be consistent between platforms, as that's impossible to define.

It also occurs to me that the current proposal would actually also work for auto layout. I'm not sure how I feel about that, but we could certainly make it work.

jaredpar · 2018-08-13T15:44:35Z

proposed/non-primitive-constant-data.md

+- Must be a struct
+- The struct must have sequential layout
+- Without pointers of any form (No object reference, IntPtr, UIntPtr, pointer, function pointer)
+- All fields must be public, or the type must be a primitive type


Why does public matter at all here? I would assume the type must be primitive not any or qualification.

Requiring all fields be public would also block "opaque" types, such as Vector128<T>

Why does public matter at all here?

The field layout has to be public contract for this to work with versioning. The easiest way to guarantee it is to require all fields to be public. Consider the case where the struct is defined in one assembly and used in different assembly.

"opaque" types, such as Vector128<T>

Types like Vector128<T> do not work with this in general. The swizzler would have to special case them.

The field layout has to be public contract for this to work with versioning

I don't believe so. It could work with fields of any accessibility, provided the user doesn't do anything that modifies the layout of the struct later.

At the language level, it would be possible for the user to specially attribute structs that they want constant to work with (which tells the compiler, "I am opting into this behavior, and I won't change the layout later"). If they were to change it later, then it would be a breaking change and would fail at runtime (the same as any number of other breaking changes that are possible for people to make).

At the language level, it would be possible for the user to specially attribute structs that they want constant to work with

I think it's going to be a requirement for these types to be marked in a special way. There are too many requirement,s most outlined in this document, that go against how struct are generated in general.

The reference assemblies and the tooling around them would need to learn how to deal with it.

It would be great if the tooling could just reuse the C# Ref Assembly feature (which already deals with fields, etc)... It would be conceivable, even for partial facades, if the reference definitions were pulled from the general S.P.Corelib reference assembly (as needed), rather than being manually recreated.

@tannergooding - Types like Vector128 which are special cased by the runtime are effectively primitives. I'll update the doc to note that Vector128 and Vector256 fall into that category. If we do end up allowing

@jaredpar Generally, non-public fields are not considered to be part of the public contract of a type. I am aware that in some circumstances, the private details of a structure affect various behaviors of the C#, but we generally want to make public contracts impacted by private details.

An alternative approach would be to describe the language compiler generated layout of structures via a set of attributes on the types. This would possibly allow us to describe the special case for VectorXXX as well as make the discussion of public vs private fields moot (at the runtime level). The idea of private fields being part of this sort of a contract makes me uncomfortable, but it works for me if one is talking about constants for types defined in the same module or something.

It would be great if the tooling could just reuse the C# Ref Assembly

I agree that it would be great, but last time I have checked the C# Ref Assembly feature was insufficient for what CoreFX needs.

rather than being manually recreated

The manually maintaned public surface definition lets us ensure that we have same surface between platforms, and compatible surface betwen versions and different runtimes. The C# Ref Assembly feature does not have equivalent for this today.

Generally, non-public fields are not considered to be part of the public contract of a type. I am aware that in some circumstances, the private details of a structure affect various behaviors of the C#, but we generally want to make public contracts impacted by private details.

Struct private fields are very much a part of the c# contract. I think every time people have tried to get cute about them not they get broken. I don't think I'd want to require public fields here in C#. Would detract a bit from the feature.

I agree that it would be great, but last time I have checked the C# Ref Assembly feature was insufficient for what CoreFX needs.

We should follow up again on this as I'm not sure what feature is missing at this point. There shouldn't be any gap at this point, other than changing the infrastructure for calling the compiler.

VSadov · 2018-08-13T16:51:05Z

proposed/non-primitive-constant-data.md

+The proposal is to provide a set of CompilerServices apis implemented as intrinsics (in most cases) which will allow the use of more complex constants. This api will accept as a byref argument a reference to data in a well defined type layout, and return a byref to data in the correct platform layout.
+
+### Definition of types which can be represented as constants
+- Must be a struct


Can it be a generic type? - like int? ?
It feels "why not", but then we must be sure that such types respect sequential layout and have predictable packing.

Well, int? is probably a bad example, since it has private fields, but the question still remains - would generic types be supported?
A better example would be tuples like (byte, byte, byte) - can that be a constant?

I think this could be a generic type, but it only make sense if we were willing to make setting the fields of a nullable a well known detail of the frameworks. Given the history of nullable I think it would be fine, but it would probably require special casing in the runtime to have this knowledge.

And yes, it would be ok to declare the fields of ValueTuple to be a public contract as well. With regards to layout of generic structures, by allowing the layout to differ between the PE file and the actual runtime, support for a variety of capabilities pops out. I hadn't thought of generics, but they should just work.

davidwrighton · 2018-08-13T17:38:55Z

With regards to the proposal by @VSadov, that proposal might be a better api to access this functionality. Initially, I had been thinking that avoiding the RuntimeFieldHandle and using just the ref parameter would be simpler, but as I dug into putting together the handling to allow proper interning of the data and only allowing this in safe ways, the design has gotten more complex. Support for the ref T returning variant is quite important, as it allows for individual constants to be more efficiently encoded, but we could easily swap to using the RuntimeFieldHandle to get at the data, and obviously the names can be swapped around.

VSadov · 2018-08-13T18:07:37Z

Yes, we went through exactly the same reasoning. Referring to RVA constants symbolically - via RuntimeFieldHandle is sufficient for scenarios that matter.
It is also safer since in the handle case we can know the size of the data.

- allow auto and explicit layout - explicitly declare that generics are supported - Add comment about special types (Vector128<T>, Vector256<T>)

…s of api, but still retaining the handling of arbitrary types.

davidwrighton · 2018-08-28T20:50:38Z

@jaredpar @VSadov I've updated the proposal.

Jared, do you think that there is a path to getting non-primitive constants into the language?

jkotas · 2018-08-28T20:57:25Z

proposed/non-primitive-constant-data.md

+class RuntimeHelpers
+{
+    // Placing this attribute on a type indicates that it can be stored and loaded using the CreateSpan<T>, LoadConstant<T> and InitializeArray helper functions
+    public WellKnownLayoutTypeAttribute : Attribute


We never have attributes as nested types.

Shouldn't this attribute be synthetic and encoded as 0x18 in the type flags?

kumpera · 2018-08-28T21:09:26Z

proposed/non-primitive-constant-data.md

+
+The proposal is to provide a set of CompilerServices apis implemented as intrinsics (in most cases) which will allow the use of more complex constants. The intention is to permit more complex constant forms for ReadOnlySpan<T> access, constant access, and through IL compiler magic, add new forms of array initialization that are not currently supported through the InitializeArray pattern.
+
+### Definition of types which can be represented as constants


The list of banned types should include mono's size-variadic types nint and nfloat as they don't fit on any of those categories.

kumpera · 2018-08-28T21:16:03Z

proposed/non-primitive-constant-data.md

+- All fields must also be of a type which can be represented as a constant
+
+### Well known type layout
+Given that IL binaries may be loaded on many different platforms with different type layout rules, constant data must be represented in a consistent manner across all of them. For consistent type layout, types shall be laid out in precisely a sequential manner as described in metadata, with *no* packing between any fields. For instance, this struct will utilize 9 bytes when stored. In addition, if the type uses explicit layout, even if there are overlapping fields, those fields are not to be overlapped in the WellKnownLayout, the layout of the final form will be based on copying out of the data stored in the PE file in metadata order into the runtime layout. An empty structure will consume 0 bytes of storage, and thus while it may be used as a field within a constant, is not itself permitted to be used to instantiate CreateSpan<T> or LoadConstant<T>


Mono supports platforms that doesn't support unaligned loads, meaning all WellKnownLayout types and its fields must be accessed with unaligned. prefix.

kumpera · 2018-08-28T21:18:41Z

proposed/non-primitive-constant-data.md

+class RuntimeHelpers
+{
+    // Placing this attribute on a type indicates that it can be stored and loaded using the CreateSpan<T>, LoadConstant<T> and InitializeArray helper functions
+    public WellKnownLayoutTypeAttribute : Attribute


Shouldn't this attribute be synthetic and encoded as 0x18 in the type flags?

jaredpar · 2018-08-29T22:26:42Z

@davidwrighton

Jared, do you think that there is a path to getting non-primitive constants into the language?

By non-primitive do you mean not the CLR primitives (int, string, double, etc ...)?

davidwrighton · 2018-08-30T22:28:41Z

Yes

And about naming... the naming on the attributes I've whipped up is very suspect. These would need more formal review before I would consider the naming to be at all finished.

tannergooding · 2018-09-11T16:32:35Z

@davidwrighton. How much work would it be to add a RuntimeHelpers.CreateReadOnlySpan method that just supports the 12 primitive types listed here: http://source.roslyn.io/#Microsoft.CodeAnalysis/SpecialTypeExtensions.cs,69087839d91b67fd

Currently, adding the support for constant user-defined structs is likely out of scope for C# 8.0

However, updating C# to directly call RuntimeHelpers.CreateReadOnlySpan instead of RuntimeHelpers.InitializeArray is likely a compiler only change and is would be "in scope" (since it is basically just a bug fix/perf improvement).

davidwrighton · 2018-09-26T00:38:36Z

That would be rather less work. If we don't have any real need for the more complex stuff, it would be much simpler to do that the rather large pile of stuff I suggest in this proposal.

terrajobst · 2020-03-13T21:39:51Z

What's the status of this?

davidwrighton · 2020-03-17T05:19:29Z

Probably not going to happen any time soon, there doesn't seem to be a great need for this at this time.

terrajobst · 2020-03-27T20:13:45Z

Should this be closed then?

pentp · 2020-03-27T21:50:25Z

No, there's still need for this, it's just lower priority than having constant data for primitive types.

richlander · 2020-04-20T15:47:26Z

If we're not doing anything with them, can be merge it into "proposed"? We should only use open PRs for active proposals (that we're actively working on, as a proposal).

davidwrighton · 2020-04-27T18:22:25Z

@richlander, I'm unfamiliar with the merge policies of this repo. If you believe that the it is appropriate to merge, please do so.

Create non-primitive-constant-data.md

756cfd9

davidwrighton requested review from kumpera, jaredpar, vargaz and jkotas August 11, 2018 01:50

jkotas reviewed Aug 11, 2018

View reviewed changes

jaredpar reviewed Aug 13, 2018

View reviewed changes

VSadov reviewed Aug 13, 2018

View reviewed changes

davidwrighton added 2 commits August 13, 2018 11:47

Update definition of what types are permitted to be constants

9061c70

- allow auto and explicit layout - explicitly declare that generics are supported - Add comment about special types (Vector128<T>, Vector256<T>)

Respond to feedback to make this more like the older proposal in term…

539419a

…s of api, but still retaining the handling of arbitrary types.

jkotas reviewed Aug 28, 2018

View reviewed changes

kumpera reviewed Aug 28, 2018

View reviewed changes

Move attributes out of RuntimeHelpers

57ae296

tannergooding mentioned this pull request Jan 31, 2019

Fixing a GC hole in Number.BigInteger dotnet/coreclr#22334

Merged

terrajobst marked this pull request as draft April 9, 2020 23:44

davidwrighton marked this pull request as ready for review April 27, 2020 18:21

huoyaoyuan mentioned this pull request Aug 24, 2020

Nullable structs inside attributes that are stored inside of Assemblies. dotnet/roslyn#47069

Open

microsoft-github-updates bot changed the base branch from master to main September 11, 2020 16:26

davidwrighton closed this May 17, 2021

jkotas mentioned this pull request Oct 29, 2021

[API Proposal]: ReadOnlySpan<T> CreateSpan<T>(RuntimeFieldHandle) dotnet/runtime#60948

Closed

jkotas mentioned this pull request Jun 15, 2022

Provide alignment guarantees for embedded binary data dotnet/roslyn#61259

Open


		The proposal is to provide a set of CompilerServices apis implemented as intrinsics (in most cases) which will allow the use of more complex constants. The intention is to permit more complex constant forms for ReadOnlySpan<T> access, constant access, and through IL compiler magic, add new forms of array initialization that are not currently supported through the InitializeArray pattern.

		### Definition of types which can be represented as constants

Create non-primitive-constant-data.md #46

Create non-primitive-constant-data.md #46

Conversation

davidwrighton commented Aug 11, 2018

jkotas commented Aug 11, 2018 • edited Loading

jkotas commented Aug 11, 2018

jkotas Aug 11, 2018 • edited Loading

Choose a reason for hiding this comment

VSadov Aug 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwrighton Aug 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidwrighton commented Aug 13, 2018 • edited Loading

VSadov commented Aug 13, 2018 • edited Loading

davidwrighton commented Aug 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kumpera Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredpar commented Aug 29, 2018

davidwrighton commented Aug 30, 2018

tannergooding commented Sep 11, 2018

davidwrighton commented Sep 26, 2018

terrajobst commented Mar 13, 2020

davidwrighton commented Mar 17, 2020

terrajobst commented Mar 27, 2020

pentp commented Mar 27, 2020

richlander commented Apr 20, 2020

davidwrighton commented Apr 27, 2020

jkotas commented Aug 11, 2018 •

edited

Loading

jkotas Aug 11, 2018 •

edited

Loading

VSadov Aug 13, 2018 •

edited

Loading

davidwrighton Aug 13, 2018 •

edited

Loading

davidwrighton commented Aug 13, 2018 •

edited

Loading

VSadov commented Aug 13, 2018 •

edited

Loading

kumpera Aug 28, 2018 •

edited

Loading