Support the immediate values for u32 bitwise instructions
#1362
Conversation
implement binary parser for lexer
Fumuran
force-pushed
the
andrew-imm-values-for-u32-bitwise-instrs
branch
from
0dc7693
to
c0ce2a2
Compare
Fumuran
force-pushed
the
andrew-imm-values-for-u32-bitwise-instrs
branch
from
83ddbfc
to
9d3a1c0
Compare
|
It turned out that with our lalrpop parser we can just emulate instructions with immediate values with |
There was a problem hiding this comment.
Looks good! Thank you! I left a couple comments inline (both related to using only u32 values), but also didn't review .lalrpop file in detail. @bitwalker could you take a look at it?
Also, we should update the changelog and documentation.
assembly/src/parser/lexer.rs
Outdated
| loop { | ||
| // If we hit a non-binary digit, we're done | ||
| let c1 = self.read(); | ||
| if !c1.is_ascii_hexdigit() { |
There was a problem hiding this comment.
This should be !is_ascii_binary(c1) no?
| } | ||
| } | ||
|
|
||
| fn is_ascii_binary(c: char) -> bool { |
There was a problem hiding this comment.
Suggestion: Mark this #[inline(always)] to ensure that it gets inlined, otherwise it goes from a very cheap comparison operation to a quite expensive function call if it isn't inlined. While Rust may inline functions like this automatically, it is based on a heuristic, and is sensitive to what is happening in the caller, so in cases where you know it is important, it's always worth providing the inline annotation
There was a problem hiding this comment.
Looks good! Thank you! I left one comment inline, also I think #1362 (comment) still needs to be addressed. After these, we can merge.
This PR adds support for the immediate values for
u32bitwise operations:u32and,u32or,u32xorandu32not.TODO: