Fix ArrayStack's call to default constructor for <T>.

[sandreenko]

The first commit add IsEmpty(), the second adds BottomRef(), the third fixes the old issue that template argument for ArrayStack had to have a default constructor.

I hit the last issue again in #21395 so decided to fix it.

@mikedn I think you also requested this change, could you please review it?
PTAL @dotnet/jit-contrib

[mikedn]

I think you also requested this change, could you please review it?

Yes, I wanted to do this for a while now but somehow I kept postponing it. Tank you!

[mikedn]

FWIW this is not the ideal solution because we're losing T's alignment. But I'm not aware of anything in the JIT that requires larger than pointer alignment and the array is pointer aligned due to the T* data; that precedes it.

P.S. And anyway I'm not sure what the ideal solution is anyway 😄

[mikedn]

Hold on a bit, PIN seems to show a significant regression on this change. Odd.

[mikedn]

Hmm, false alarm. The regression is small, only 0.08%. Still, I was expecting a small improvement, not a small regression.

[mikedn]

Interesting situation. You'd think that removing the useless initialization of an 8 element array (plus whatever initialization is performed when dynamically allocating the array) would result in a perf win. But:

• a lot of these ArrayStack objects are ArrayStack<GenTree*> or similar, in this case no initialization was performed to begin with. It's an array of primitive types, not an array of class objects.
• something, most like the builtin array reinterpretation, triggers VC++'s /GS checks. I doubt that those a significant impact but here and there they also seem to cause other issues (failure inline some functions apparently).

So, anyone knows a way to declare an array of T that doesn't result in initialization and doesn't require reinterpretation?

[sandreenko]

So, anyone knows a way to declare an array of T that doesn't result in initialization and doesn't require reinterpretation?

I was playing with unions like:

union S
{
  char builtinDataCache[builtinSize * sizeof(T)];
  T* builtinData;
};

but did not measure pin, I think it looks too bad even if we get a tiny perf win.

[mikedn]

I was playing with unions like:

Hmm, I tried a different version - an array of unions of char and T. /GS still doesn't like it.

I think it looks too bad even if we get a tiny perf win.

Yep. Personally I'm fine with the 0.08% regression, considering that I can pull improvements from other places like rabbits from a hat 😄. It's just that I wasn't expecting this.

[sandreenko]

PR was updated with small fixes.

[mikedn]

@sandreenko This should have been T& 😁. Unfortunately I only noticed this now, when you merged and I saw a conflict with my own BottomRef in another PR.

[sandreenko]

Thank you. I will use this function in my next PR and fix it there.