Preincrement improvements

[ds5678]

Problem

#3468

[ds5678]

Hi @siegfriedpammer, I'm running into a small problem with this pull request. My tests pass, except for Roslyn 1.3, which compiles the pre-increment like this:

{Block IL_0000 (incoming: 1) {
	stloc V_0(binary.add.i4(ldloc value, ldc.i4 1))
	stloc value(ldloc V_0)
	leave IL_0000 (call ToString(ldloca V_0))
} at IL_0000}

I want to write a separate transform to make the IL into this:

{Block IL_0000 (incoming: 1) {
	stloc value(binary.add.i4(ldloc value, ldc.i4 1))
	stloc V_0(ldloc value)
	leave IL_0000 (call ToString(ldloca V_0))
} at IL_0000}

In other words, I want the second transform to switch the position of value and V_0, so that other transforms can reduce it further:

{Block IL_0000 (incoming: 1) {
	stloc V_0(stloc value(binary.add.i4(ldloc value, ldc.i4 1)))
	leave IL_0000 (call ToString(ldloca V_0))
} at IL_0000}

My current pattern matching would work on this final transformed IL.

However, I'm concerned about unintended consequences and unsure of how this should be implemented. In particular:

• What rules should I use when deciding whether or not to do this transformation?
• When should the second transform be run?
• Does the second transform violate semantics in some way that might affect the use of ILSpy in other tools like Visual Studio?

In regards to rules, my idea was:

• There are two instructions, stloc x1(expression) followed by stloc x2(ldloc x1).
• expression references x2 but not x1.
  • Or maybe it references only x2 and no other variables?
• x1 and x2 have the same type.

What are your thoughts?

[dgrunwald]

I think your order-swapping only makes sense if you can guarantee that it'll be combined into a preincrement.
If this doesn't happen, it's confusing to show the code in a different order than it's actually executed. If there's multiple statements in the "wrong" order, that might also be problematic for the generated sequence points. But if the preincrement transform can combine it into an inline assignment, then there's only one statement and the sequence points won't be an issue.

So I think this doesn't make sense as a separate statement-swapping transform, but should be a transform that directly creates the inline assignment.

[dgrunwald]

But if it only impacts Roslyn 1.3, then maybe we don't need to support preincrements for that particular version?
You could just use #if in the testcase to avoid testing with that Roslyn version.

[ds5678]

But if it only impacts Roslyn 1.3, then maybe we don't need to support preincrements for that particular version? You could just use #if in the testcase to avoid testing with that Roslyn version.

I ended up doing this. I'll come back and implement a second pull request if I discover that 1.3 support is necessary for my use case.

[christophwille]

@ds5678 so it is no longer WIP (PR title)?

[ds5678]

Fixed