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

Jump to bottom

Fix several issues with CreateSpan support #65919

Merged
merged 3 commits into from
Dec 12, 2022
Merged

Fix several issues with CreateSpan support #65919

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
604bead
Fix several issues with CreateSpan support
stephentoub Dec 10, 2022
0804eab
Address PR feedback
stephentoub Dec 10, 2022
f90cce2
Address PR feedback and fix test
stephentoub Dec 10, 2022
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
18 changes: 2 additions & 16 deletions src/Compilers/CSharp/Portable/CodeGen/EmitArrayInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,7 @@ private void EmitArrayInitializers(ArrayTypeSymbol arrayType, BoundArrayInitiali
{
ImmutableArray<byte> data = this.GetRawData(initExprs);

// Emit the call to RuntimeHelpers.InitializeArray, creating the necessary metadata blob if there isn't
// already one for this data. Note that this specifies an alignment of 1. This is valid regardless of
// the kind of data stored in the array, as it's never accessed directly in the blob; rather, InitializeArray
// copies out the data as bytes. The upside to keeping this as 1 is it means no special alignment is required.
// Although the compiler currently always aligns the metadata fields at an 8-byte boundary, the .pack field
// is appropriately set to the alignment value, and a rewriter (e.g. illink) may respect that. If the alignment
// value were to be increased to match the actual alignment requirements of the element type, that could cause
// such rewritten binaries to regress in size due to the extra padding necessary for aligning. The downside
// to keeping this as 1 is that this data won't unify with any blobs created for spans (e.g. RuntimeHelpers.CreateSpan).
// Code typically does directly read from the blobs via spans, and as such alignment there is required to be
// at least what the element type requires. That means if the same data/element type is used with an array
// and separately with a span, the data will exist duplicated in two different blobs. If it turns out that's
// very common, this can be revised in the future to specify the element type's alignment.
_builder.EmitArrayBlockInitializer(data, alignment: 1, inits.Syntax, _diagnostics.DiagnosticBag);
_builder.EmitArrayBlockInitializer(data, inits.Syntax, _diagnostics.DiagnosticBag);

if (initializationStyle == ArrayInitializerStyle.Mixed)
{
Expand Down Expand Up @@ -675,7 +662,6 @@ private bool TryEmitReadonlySpanAsBlobWrapper(NamedTypeSymbol spanType, BoundExp
// ensure deterministic behavior.
arrayType = arrayType.WithElementType(TypeWithAnnotations.Create(elementType.EnumUnderlyingTypeOrSelf()));

ushort alignment = (ushort)specialElementType.SizeInBytes();
var cachingField = _builder.module.GetArrayCachingFieldForData(data, _module.Translate(arrayType), wrappedExpression.Syntax, _diagnostics.DiagnosticBag);
var arrayNotNullLabel = new object();

Expand All @@ -693,7 +679,7 @@ private bool TryEmitReadonlySpanAsBlobWrapper(NamedTypeSymbol spanType, BoundExp
_builder.EmitIntConstant(elementCount);
_builder.EmitOpCode(ILOpCode.Newarr);
EmitSymbolToken(arrayType.ElementType, wrappedExpression.Syntax);
_builder.EmitArrayBlockInitializer(data, alignment, wrappedExpression.Syntax, _diagnostics.DiagnosticBag);
_builder.EmitArrayBlockInitializer(data, wrappedExpression.Syntax, _diagnostics.DiagnosticBag);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
_builder.EmitOpCode(ILOpCode.Dup);
_builder.EmitOpCode(ILOpCode.Stsfld);
_builder.EmitToken(cachingField, wrappedExpression.Syntax, _diagnostics.DiagnosticBag);
Expand Down
Loading