Struct promotion should promote 32-byte SIMD on 16-byte alignment

[CarolEidt]

This code in Compiler::StructPromotionHelper:

        if ((fieldInfo.fldOffset % fieldInfo.fldSize) != 0)
        {
            // The code in Compiler::genPushArgList that reconstitutes
            // struct values on the stack from promoted fields expects
            // those fields to be at their natural alignment.
            return false;
        }

Won't allow promotion of a TYP_SIMD32 field unless it's on a 32-byte-aligned offset. However, since the stack is not 32-byte aligned this is not a useful condition.
This should probably either use genTypeAlignments[] or perhaps the minimum of the type size and STACK_ALIGN.

category:cq
theme:structs
skill-level:beginner
cost:small
impact:small

[anthonycanino]

Hi @sandreenko , can you provide a little clarification on this issue if you know the details? I spent some time this morning trying to reproduce a situation where a TYP_SIMD32 field wouldn't get promoted per the code snippet above, but since struct fields are aligned based on the size of the largest field, I can't seem get something like...

public struct Simd32
{
  public Vector128<int> V0;
  public Vector256<int> V1;
  public Vector128<int> V2;
  // ...
}
  
[MethodImpl(MethodImplOptions.NoInlining)]
public static int RunSimd32Stack()
{
  Simd32 s = new Simd32();
  if (s.V0.GetElement(0) < s.V1.GetElement(0))
  {
    return s.V0.GetElement(0) + s.V1.GetElement(0);
  }
  else
  {
    return s.V1.GetElement(0) + s.V2.GetElement(0);
  }
}

to not promote the fields V0, V1, and V2, unless I preference the struct with [StructLayout(LayoutKind.Sequential, Pack=16)].

Moreover, it looks like the function mentioned in the comment above Compiler::genPushArgList is not in the codebase any longer (unless it has been renamed or I have missed it with grep).

[anthonycanino]

Hi @JulieLeeMSFT , do you know if anyone can help me investigate/understand this issue?

[JulieLeeMSFT]

Adding @kunalspathak if he has a quick answer.

Hi @JulieLeeMSFT , do you know if anyone can help me investigate/understand this issue?

[sandreenko]

the code from the header is here:

runtime/src/coreclr/jit/lclvars.cpp

Line 1810 in 99dd33b

if ((fieldInfo.fldOffset % fieldInfo.fldSize) != 0)

to not promote the fields V0, V1, and V2, unless I preference the struct with [StructLayout(LayoutKind.Sequential, Pack=16)].

it is what I did to repro it.

[kunalspathak]

I don't know much about this issue at the top of my head. I will have to spend some time in understanding the surrounding feature logic and I don't think I will get time for at least next few weeks.

[anthonycanino]

@kunalspathak ok, please let me know if you do plan to investigate and I will be happy to help dig further as well.