[API Proposal]: Make OpCode.StackChange() public

[buyaa-n]

Background and motivation

OpCodes represents MS IL instructions that used for emitting IL with ILGenerator. An OpCode instruction may pop value(s) from stack as an operand of the instruction and also may push value(s) into the stack as a result. This stack change is needed for calculating MaxStack value that is essential for populating method body. I could use the StackBehaviourPop and StackBehaviourPush properties of OpCode to calculate this, but there is an internal method that gives this info directly:

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs

Lines 48 to 49 in 946c524

	internal int StackChange() =>
	m_flags >> StackChangeShift;

Making this public will be useful for calculating MaxStack faster
Related to https://github.com/dotnet/runtime/pull/93244/files#r1353013060

API Proposal

namespace System.Reflection.Emit

public readonly partial struct OpCode : System.IEquatable<System.Reflection.Emit.OpCode>
{
    ...
    public StackBehaviour StackBehaviourPop { get }
    public StackBehaviour StackBehaviourPush { get }
    ...
+   public int EvaluationStackDelta { get }
}

Alternative Designs

Other naming options: EvaluationStackDelta, StackSizeDelta, StackTransitionDelta or StackDepthDelta as it is showing the difference of evaluation stack transition.

API Usage

    private void UpdateStackSize(OpCode opCode)
    {
        _currentStack += opCode.EvaluationStackDelta;
        _maxStackSize = Math.Max(_maxStackSize, _currentStack);
    }

[ghost]

Tagging subscribers to this area: @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

OpCodes represents MS IL instructions that used for emitting IL with ILGenerator. An OpCode instruction may pop value(s) from stack as an operand of the instruction and also may push value(s) into the stack as a result. This stack change is needed for calculating MaxStack value that is essential for populating method body. I could use the StackBehaviourPop and StackBehaviourPush properties of OpCode to calculate this, but there is an internal method that gives this info directly:

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs

Lines 48 to 49 in 946c524

	internal int StackChange() =>
	m_flags >> StackChangeShift;

Making this public will be useful for calculating MaxStack faster
Related to https://github.com/dotnet/runtime/pull/93244/files#r1353013060

API Proposal

namespace System.Reflection.Emit

public readonly partial struct OpCode : System.IEquatable<System.Reflection.Emit.OpCode>
{
    ...
    public StackBehaviour StackBehaviourPop { get }
    public StackBehaviour StackBehaviourPush { get }
    ...
+   public int StackDifference()
}

API Usage

    private void UpdateStackSize(OpCode opCode)
    {
        _currentStack += opCode.StackDifference();
        _maxStackSize = Math.Max(_maxStackSize, _currentStack);
    }

Alternative Designs

It can be a property too

namespace System.Reflection.Emit

public readonly partial struct OpCode : System.IEquatable<System.Reflection.Emit.OpCode>
{
    ...
    public StackBehaviour StackBehaviourPop { get }
    public StackBehaviour StackBehaviourPush { get }
    ...
+   public int StackDifference { get }
}

Risks

No response

Author:	buyaa-n
Assignees:	-
Labels:	api-suggestion, area-System.Reflection.Emit
Milestone:	-

[alexrp]

Seems like this should be a property?

[buyaa-n]

Seems like this should be a property?

Yes, can be a property and that is mentioned in the Alternative Designs part

[terrajobst]

How expensive is the computation?

I assume it's just a switch over the op code as the values are static? In that case I'd be in favor of a property.

I also assume a positive value means the op code net-ads to the evaluation stack (pushing), a negative value means net consumption (popping), and zero means no change or equal amounts of pushing and popping.

If so, I think I'd prefer EvaluationStackDelta.

@dotnet/fxdc?

[buyaa-n]

How expensive is the computation?

The OpCode has this info in the upper 4 bits of m_flags field:

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs

Line 34 in d18295e

internal const int StackChangeShift = 28; // XXXX0000000000000000000000000000

So the computation is just shifting those bits:

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs

Lines 48 to 49 in d18295e

	internal int StackChange() =>
	m_flags >> StackChangeShift;

I also assume a positive value means the op code net-ads to the evaluation stack (pushing), a negative value means net consumption (popping), and zero means no change or equal amounts of pushing and popping.

Yep, that is right

[terrajobst]

Oh wow. I didn't realize the op code value encoded the stack change. In other metadata readers I've always seen people doing switches facepalm

Also, I didn't realize we already have properties called StackBehaviuor(Push|Pop). In that case I rescind my proposal to include the term Evalutation. I think I still prefer the word Delta over Difference though.

Anyone else on @dotnet/fxdc?

namespace System.Reflection.Emit;

public partial struct OpCode
{
    // Existing:
    // public StackBehaviour StackBehaviourPop { get; }
    // public StackBehaviour StackBehaviourPush { get; }

    public int StackDelta { get; }
}

[steveharter]

Slightly more verbose, but I think StackSizeDelta rings a bit better than StackDelta which is somewhat ambiguous.

[buyaa-n]

Another option could be StackTransitionDelta, as it is showing the difference of evaluation stack transition/movement, also could be StackDepthDelta.

[terrajobst]

Either of those names work for me :-)

[bartonjs]

Video

Looks good as proposed.

namespace System.Reflection.Emit

public readonly partial struct OpCode : System.IEquatable<System.Reflection.Emit.OpCode>
{
    public int EvaluationStackDelta { get }
}