interp: do not unroll loops

[niaow]

This change triggers a revert whenever a basic block runs instructions at runtime twice.
As a result, a loop body with runtime-only instructions will no longer be unrolled.
This should help some extreme cases where loops can be expanded into hundreds or thousands of instructions.

[dgryski]

I feel like there should be a threshold here. There certainly is init code with loops that makes sense to run that incidentally runs an instruction twice: say, a strconv or something that's looping over a string. For testing, it would probably be handy to have a verbosity flag that prints out when an init function is being move to runtime and control how many iterations is considered "looping".

[niaow]

To clarify: this doesn't mean that an instruction being run twice will trigger a revert. This requires the instruction to be forced to runtime both times within the same function invocation.

[niaow]

a strconv or something that's looping over a string

In both of these cases I think a revert is correct.
No useful work is being done in those cases, this is just copying the body of the loop for every character.
If we revert, then we just call the function with the loop at runtime which is safe.

[dgryski]

Odd that the bytes test code, which I would have assumed would trigger on this, doesn't appear to be fixed.

[deadprogram]

Anything stopping this from being merged now?

[niaow]

I do not think so, but I still need a review.

[deadprogram]

All tests passing with latest dev branch. @aykevl can you please review? Thanks!

[aykevl]

Looks good to me. I'm only concerned about one possible slowdown.

[aykevl]

I've noticed that heap allocations are a significant slowdown, so I think it would be a good idea to move the creation of the map to where anything is inserted. Like:

Suggested change

	runtimeBlocks := make(map[int]struct{})
	var runtimeBlocks map[int]struct{}

And then:

				// Flag the last block as having run stuff at runtime.
				if runtimeBlocks == nil {
					runtimeBlocks = make(map[int]struct{})
				}
				runtimeBlocks[lastBB] = struct{}{}

[niaow]

In this case, the map will be used in any situation with multiple basic blocks. It might be more efficient for me to reuse these maps within an interp pass instead.

[aykevl]

Only when there are instructions run at runtime, right? That should not happen very often. Most functions should go through interp without any instructions emitted at runtime.

[niaow]

Oh yeah, was just a bit confused coming back to this after a while.

[niaow]

Done

[niaow]

Rebased and made the requested change

[deadprogram]

Now merging, thank you very much @niaow