_builder.EmitOpCode(ILOpCode.Cpblk, -3);

ECMA-335 says:

cpblk assumes that both destaddr and srcaddr are aligned to the natural size of the machine

Are we ensuring that this requirement is met? #Closed

We emitcpblk in a very similar way as above (underif (elementType.EnumUnderlyingTypeOrSelf().SpecialType.SizeInBytes() == 1)), so I assumed the alignment is fine.

ECMA-335 also says:

All such structures, allocated by the CLI, are naturally aligned for the current platform

and the field has a structure type, so I think it should be aligned properly.

and the field has a structure type, so I think it should be aligned properly.

A blob of bytes is not an instance of a structure type. We are not copying data from a regular field. We are copying data from a blob stored in a module.

I thought we are emitting a special struct (e.g.,__StaticArrayInitTypeSize=12) and a field in<PrivateImplementationDetails> which is of the type of that struct, initialized with a blob of data, and we are using that field - so that field should be naturally aligned as it's of the struct type. PlusCreateSpan can return pointer to a different blob of data with fixed-up endianness, but it should be also naturally aligned as it allocates (and caches) an array in that case.

Here is what I think:

1. We are not working with a regular field. We are working with a field which is mapped into a region of memory inside module's image, so-called an RVA field.
2. Alignment of that region cannot change at runtime, it is the alignment that was used during emit.
3. Some operations have certain alignment requirements. For example, we already established thatCreateSpanAPI has them.
4. A comment forPrivateImplementationDetails.CreateDataField explains how compiler enforces the right alignment for an RVA field. Here is a verbatim quote: "a type is generated of the same size as
   the data, and that type needs its .pack set to the alignment required for the underlying data. While that
   .pack value isn't required by anything else in the compiler (the compiler always aligns RVA fields at 8-byte
   boundaries, which accomodates any element type that's relevant), it is necessary for IL rewriters. Such rewriters
   also need to ensure an appropriate alignment is maintained for the RVA field, and while they could also simplify
   by choosing a worst-case alignment as does the compiler, they may instead use the .pack value as the alignment
   to use for that field, since it's an opaque blob with no other indication as to what kind of data is
   stored and what alignment might be required."
5. It looks like we already addressed theCreateSpan required alignment, which is thenatural boundary of the span's element type.
6. The span produced byCreateSpan points into the same blob when the data are not moved or copied anywhere else. Therefore, still have the original alignment.
7. By comparison toCreateSpan,cpblk command has a different alignment requirement: "aligned tothe natural size of the machine". Does the RVA meet this requirement? The spec doesn't provide any specific number. As of commit 10, we are using different alignment for different element types. It is quite possible that some of the numbers aren't matching "the natural size of the machine". Perhaps we should always use 8-byte alignment. It would be good to get some guidance from the runtime team here.

@stephentoub can you please advise how we ensure we meetcpblk's alignment requirements here? Thanks.

Btw, looks like this code creates an RVA field with alignment 1 and uses it withcpblk, which, if correct, would suggestcpblk doesn't need any special alignment of the field:

You can prefix the cpblk instruction with unaligned in order to avoid the alignment requirement, which IIRC wants the pointers to be aligned to the machine's natural size. It's possible the cited existing code should be using unaligned as well, or maybe the requirement is less about the machine's natural word size and more about the size of the type in use (or maybe this is a place where ECMA and coreclr differ).@jkotas?

If you would like to be compliant with ECMA, emit theunaligned prefix. It is not required to make things work on current runtimes.cpblk implementations in current runtimes do not make any assumptions about the alignment of the inputs.

if (sizeInBytes != 8)

Looking at theunaligned. section of ECMA-335

shall use unaligned. if the alignment is not known at compile time to be 8-byte.

From that, I infer that, if we align the RVA field type at 8 bytes, we will be compliant without theunaligned prefix. Should we use that option instead, rather than having conditional IL?

Are there any downsides to conditional IL? On the other hand, it seems that aligning properly to 1/2/4/8 bytes instead of always to 8 bytes saves space.

Are there any downsides to conditional IL?

IL is bigger. Compiler's code is more complex. Test matrix is bigger, more scenarios need special attention.

it seems that aligning properly to 1/2/4/8 bytes instead of always to 8 bytes saves space.

I do not think it actually does. At least not for the code emitted by our compilers because, I think, compiler aligns all blobs at 8 bytes regardless of the requested alignment, this is mentioned in one of the comments in code. The alignment that we explicitly set on the types is important for IL rewriting scenarios.

The alignment that we explicitly set on the types is important for IL rewriting scenarios.

I think this means it improves size for AOT and similar precompiled code (right?) which seems important as well.

I think this means it improves size for AOT and similar precompiled code (right?) which seems important as well.

To be honest, I do not know the answer to these questions, And, even if that was the case, I am not sure if that is something that we should care about.

I do not think it makes sense to deoptimize the 1-byte case like that.

To make the two cases as similar as possible, you can emitunaligned.1 cpblk in both cases and emit the blob withelementType.EnumUnderlyingTypeOrSelf().SpecialType.SizeInBytes() alignment in both cases.

did you want that to be aligned to 8 bytes as well?

Yes, this is what I would do. I would align both at 8 bytes explicitly (we are aligning blobs at 8 bytes regardless), and would completely get rid of the.unaligned instruction.

I would align both at 8 bytes explicitly (we are aligning blobs at 8 bytes regardless), and would completely get rid of the .unaligned instruction.

It is size de-optimization for trimmer and AOT that are scenarios where we care about the side the most.

It is size de-optimization for trimmer and AOT that are scenarios where we care about the side the most.

Perhaps I misinterpreted you first message on this discussion thread.

If the blob size optimization is important for AOT, then, I think we should keep the.unaligned usage on both code paths. We can get there by reverting the last commit. Sorry for the confusion.

Perhaps I misinterpreted you first message on this discussion thread.

I was commenting on why we set the alignment and why it would not work to omit it on the types. Sorry for the confusion.