Expand IsOpcodeDisabled() function to prepare for implementation of re-enabled opcodes in Magnetic update.
AbandonedPublic
Actions

Authored by danconnolly on Jul 25 2018, 10:24.

Details

Reviewers

schancel
jasonbcox

Group Reviewers

Restricted Project

Summary

The purpose of this change is to prepare for the implementation of the re-enabled opcodes in the Magnetic update. The IsOpcodeDisabled() function is altered to add a check for the SCRIPT_ENABLE_MAGNETIC_OPCODES flag and the opcodes that will probably be re-enabled.

This check is structured in such a way that future changes can implement the re-enabled opcodes individually and independently.

The change does not in itself result in any behavioural change. We have verified that the tests in script_tests.json cover these changes and since the tests have not been altered, their continued success verifies that the behaviour of the function has not changed.

Test Plan

make check

Diff Detail

Branch

opcodes_2

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 3038
Build 4168: arc lint + arc unit

Event Timeline

danconnolly created this revision.Jul 25 2018, 10:24

Harbormaster completed remote builds in B2935: Diff 4395.Jul 25 2018, 10:24

danconnolly added a child revision: D1598: OP_MUL implementation.Jul 25 2018, 22:15

danconnolly added a parent revision: D1592: Add SCRIPT_ENABLE_MAGNETIC_OPCODES flag to gate opcodes activation for the Magnetic upgrade..

danconnolly added a child revision: D1606: OP_INVERT implementation.Jul 26 2018, 23:48

danconnolly added a child revision: D1608: OP_2MUL & OP_2DIV implementation.Jul 27 2018, 15:13

danconnolly retitled this revision from expand isdisabledopcode function to enable implementation of magnetic opcodes to Expand IsOpcodeDisabled() function to prepare for implementation of re-enabled opcodes in Magnetic update..Jul 28 2018, 10:10

danconnolly edited the summary of this revision. (Show Details)

danconnolly added a reviewer: Restricted Project.Jul 28 2018, 12:49

schancel requested changes to this revision.Aug 2 2018, 21:12

schancel added a subscriber: schancel.

schancel added inline comments.

src/script/interpreter.cpp
91 ↗	(On Diff #4395)	These aren't currently disabled are they? They're invalid opcodes I thought?

This revision now requires changes to proceed.Aug 2 2018, 21:12

danconnolly added inline comments.Aug 2 2018, 22:13

src/script/interpreter.cpp
91 ↗	(On Diff #4395)	Nope, disabled.

The opcodes were disabled, not invalid. See lines 85-93 in interpreter.cpp

jasonbcox requested changes to this revision.Aug 6 2018, 15:52

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

src/script/interpreter.cpp
93 ↗	(On Diff #4395)	We had discussed removing 2MUL and 2DIV. I don't see value in adding them if we're trying to keep a reduced instruction set.

This revision now requires changes to proceed.Aug 6 2018, 15:52

danconnolly marked an inline comment as done.Aug 8 2018, 19:05

danconnolly added inline comments.

src/script/interpreter.cpp
93 ↗	(On Diff #4395)	Updated spec, will remove them.

specification has been updated to remove OP_2MUL & OP_2DIV

Harbormaster completed remote builds in B3038: Diff 4529.Aug 8 2018, 19:09

@danconnolly You're correct, my bad. This looks good. Let me take a look at the implementations before I sign off on landing this.

danconnolly added a child revision: D1638: OP_LSHIFT & OP_RSHIFT implementation.Aug 9 2018, 19:36

danconnolly abandoned this revision.Sep 4 2018, 09:29

Revision Contents
Changeset List

Path

Size

src/

script/

interpreter.cpp

22 lines

Diff 4529

View Options

src/script/interpreter.cpp

Expand IsOpcodeDisabled() function to prepare for implementation of re-enabled opcodes in Magnetic update.AbandonedPublicActions