Skip to content

Commit

Permalink
Use a SHORT_OPERAND_VALUE_SIZE constant instead of a magic number.
Browse files Browse the repository at this point in the history
  • Loading branch information
@vext01
vext01 committed Dec 20, 2023
1 parent 6cd71d2 commit a34b500
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion ykrt/src/compile/jitc_yk/jit_ir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const SHORT_INSTR_MAX_OPERANDS: u64 = 3;
const SHORT_OPERAND_SIZE: u64 = 18;
const SHORT_OPERAND_KIND_SIZE: u64 = 3;
const SHORT_OPERAND_KIND_MASK: u64 = 7;
const SHORT_OPERAND_VALUE_SIZE: u64 = 15;

This comment has been minimized.

Sign in to view

Copy link
@ltratt

ltratt Dec 20, 2023

Contributor

It's very unclear what this is a size of (bytes? bits? megabytes?) -- really it's a max value in disguise. I'm not a big fan of punting this to a pow later. Personally I tend to say "SHORT_OPERAND_MASK: u64 b 0b111111" or whatever and then do a ... & ... != 0 later as I think that best signals the underlying intent.

This comment has been minimized.

Sign in to view

Copy link
@vext01

vext01 Dec 20, 2023

Author Contributor

Its the size of the sub-field in bits, not a mask.
const SHORT_OPERAND_MASK: u64 = 0x3ffff;

/// Bit fiddling for instructions.
Expand Down Expand Up @@ -88,7 +89,7 @@ pub enum Operand {

impl Operand {
pub(crate) fn new(kind: OpKind, val: u64) -> Self {
if val < 2u64.pow(15) {
if val < 2u64.pow(u32::try_from(SHORT_OPERAND_VALUE_SIZE).unwrap()) {
Self::Short(ShortOperand::new(kind, val))
} else {
todo!()
Expand Down

0 comments on commit a34b500

Please sign in to comment.