Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

[spec] New instructions#52

Merged
rossberg merged 3 commits intomasterfrom
spec2
Sep 17, 2021
Merged

[spec] New instructions#52
rossberg merged 3 commits intomasterfrom
spec2

Conversation

@rossberg
Copy link
Member

@rossberg rossberg commented Aug 9, 2021

Add the (non-controversial ones of the) new instructions to the spec.

This is the same as #50, but rebased on master.

@conrad-watt, PTAL. For some strange reason, GH doesn't let me add you as a reviewer this time.

This was referenced Aug 9, 2021
@rossberg
Copy link
Member Author

rossberg commented Sep 1, 2021

@conrad-watt, friendly ping.

@conrad-watt
Copy link
Contributor

on this now, sorry

Instruction(None, r'\hex{D4}'),
Instruction(r'\REFASNONNULL', r'\hex{D3}', r'[(\REF~\NULL~\X{ht})] \to [(\REF~\X{ht})]', r'valid-ref.as_non_null', r'exec-ref.as_non_null'),
Instruction(r'\BRONNULL~l', r'\hex{D4}', r'[t^\ast~(\REF~\NULL~\X{ht})] \to [t^\ast~(\REF~\X{ht})]', r'valid-br_on_null', r'exec-br_on_null'),
Instruction(None, r'\hex{D5}'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason br_on_null isn't next to br_on_non_null in the index space?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D5 was already assigned to ref.eq at the time we added br_on_non_null.

@@ -188,11 +189,13 @@ Instructions in this group are concerned with accessing :ref:`references <syntax
\production{instruction} & \instr &::=&
\dots \\&&|&
\REFNULL~\reftype \\&&|&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\reftype here should be \heaptype - sorry if I missed this previously.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member Author

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review.

Instruction(None, r'\hex{D4}'),
Instruction(r'\REFASNONNULL', r'\hex{D3}', r'[(\REF~\NULL~\X{ht})] \to [(\REF~\X{ht})]', r'valid-ref.as_non_null', r'exec-ref.as_non_null'),
Instruction(r'\BRONNULL~l', r'\hex{D4}', r'[t^\ast~(\REF~\NULL~\X{ht})] \to [t^\ast~(\REF~\X{ht})]', r'valid-br_on_null', r'exec-br_on_null'),
Instruction(None, r'\hex{D5}'),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D5 was already assigned to ref.eq at the time we added br_on_non_null.

@@ -188,11 +189,13 @@ Instructions in this group are concerned with accessing :ref:`references <syntax
\production{instruction} & \instr &::=&
\dots \\&&|&
\REFNULL~\reftype \\&&|&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

@conrad-watt conrad-watt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rossberg rossberg merged commit b7065d8 into master Sep 17, 2021
@rossberg rossberg deleted the spec2 branch September 17, 2021 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants