Struct DU emit redundant constructors with an unused parameter, initalization and zero-comparison can be improved

[abelbraaksma]

Found this while investigating the performance issues with @cmeeren's implementation of Array.tryPickV, see #6781 (comment).

As an example, take the following code:

type MyNormalDU =
    | Case1
    | Case2
    | Case3
    | Case4
    | Case5
    | Case6
    | Case7
    | Case8
    | Case9

This compiles into the following constructors (I removed attributes for ease of reading):

            internal MyStructDU(int _tag, bool flag)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, byte num)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, sbyte num)
            {
                this.Tag = _tag;
            }
            internal MyStructDU(int _tag, char chr)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, short num)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, int num)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, ushort num)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, uint num, bool flag)
            {
                this.Tag = _tag;
            }

            internal MyStructDU(int _tag, uint num, byte num)
            {
                this.Tag = _tag;
            }

There wasn't anything about this implementation detail in the relevant RFC, but my guess is this is to disambiguate constructors for same-type DU fields, as there's a clear "countable" use of all the build-in types up until 32 bit.

Optimization ideas for struct DUs

These redundant parameters lead to overhead in constructing the types, as the extra param needs to be put on the stack, even though it is always zero (false, '\0', 0uy etc). There used to be other optimization issues with overloaded constructors in structs w.r.t. JIT optimizations, but I haven't been able to verify that that is still the case.

Either way, I propose we improve this, which would consequently improve ValueOption as well:

• These constructors are only ever called from a single place, we can emit simple struct-init code there instead with the proper value(s), if any
• For no-value multi-case struct-DUs, only the _tag needs to be set, ctor overloads are not needed
• Any top-case initialization without a value can be optimized to just init the struct to zero (i.e. ValueNone, or Case1 above)

If this is made possible, voption can be improved to issue the same null-check (i.e. br.false or br.true) as for option to check for ValueNone, which dramatically speeds up the current implementation which calls get_Tag() and compares it with 0.

This would also open the way to implement overlapping DU cases in an overlapping internal struct, which dramatically reduces space occupied by struct DUs.

[zpodlovics]

Please also take a look at this as it could allows simplify struct du construction and potentially enable (>50) large number of cases:
#5342 (comment)

[abelbraaksma]

@zpodlovics, thanks for that. It appears that the redundant constructors are used to prevent extra copies of the structs, but as @dsyme mentioned there, it's a temporary fix that should be improved upon, quote (from #2811, back in 2017):

I tried to code some improvements to simply remove the constructors at all and store into the struct union fields directly. . However this is considerably harder to implement than the fix here as it involves allocating temporaries (to remove the temporary implied by the call to the constructor).

However it's important to simply get this feature working at all, so this fix is ok for now. The constructor is private so we can adjust its codegen at a late point.

And comment:

Correct, it's just a fix. It's not the greatest fix, and I'd like to revisit this, especially in combination with issues like #2688 , but it's the fix I would have used as a late fix for VS2017

When/if we tackle this, we should take into account that these continue to work: #2647, #2463, #2688 (the latter was fixed in #5148). This gives some insight in why & where the things are the way they are currently.