Skip to content

Commit 348d2b9

Browse files
XrXrXrXr
committed
ZJIT: Post-process bindings to use i32 for offset type, removing casts
As you may know, `as` casts can be dangerous since they may be lossy, and with type inference, it's hard to tell what kind of conversion is happening. This commit removes the safe `as i32` casts from members of `jit_bindgen_constants` enum. This builds, so we know all the constants fit in i32, with this, they match what `Assembler` expects without the need to cast.
1 parent 613949a commit 348d2b9

6 files changed

Lines changed: 41 additions & 30 deletions

File tree

‎zjit/bindgen/src/main.rs‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,12 +457,23 @@ fn main() {
457457
// Unwrap the Result and panic on failure.
458458
.expect("Unable to generate bindings");
459459

460+
// Write to a Vec for post-processing
461+
let mut bindings_string = Vec::new();
462+
bindings.write(Box::new(&mut bindings_string)).expect("Couldn't write bindings!");
463+
464+
// Use i32 for this type since that's what the assembler APIs expect
465+
const JIT_CONSTANTS_NEEDLE: &str = "\npub type jit_bindgen_constants = u32;\n";
466+
const JIT_CONSTANTS_REPLACEMENT: &[u8] = b"\npub type jit_bindgen_constants = i32;\n";
467+
// Yes, this search and replace could be faster, but it's a small file.
468+
let bindings_str = str::from_utf8(bindings_string.as_ref()).expect("bindings should be in UTF-8");
469+
let type_needle_start = bindings_str.find(JIT_CONSTANTS_NEEDLE).expect("bindings should have jit_bindgen_constants");
470+
(&mut bindings_string[type_needle_start..type_needle_start + JIT_CONSTANTS_NEEDLE.len()])
471+
.copy_from_slice(JIT_CONSTANTS_REPLACEMENT);
472+
473+
// Write out to file
460474
let mut out_path: PathBuf = src_root;
461475
out_path.push(jit_name);
462476
out_path.push("src");
463477
out_path.push("cruby_bindings.inc.rs");
464-
465-
bindings
466-
.write_to_file(out_path)
467-
.expect("Couldn't write bindings!");
478+
std::fs::write(out_path, bindings_string).expect("file output failed");
468479
}

‎zjit/src/backend/tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ fn test_jcc_ptr()
182182
let (mut asm, mut cb) = setup_asm();
183183

184184
let side_exit = Target::CodePtr(cb.get_write_ptr().add_bytes(4));
185-
let not_mask = asm.not(Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_MASK as i32));
185+
let not_mask = asm.not(Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_MASK));
186186
asm.test(
187-
Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_FLAG as i32),
187+
Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_FLAG),
188188
not_mask,
189189
);
190190
asm.push_insn(Insn::Jnz(side_exit));

‎zjit/src/codegen.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ fn gen_ccall_with_frame(
10771077
asm_comment!(asm, "switch to new CFP");
10781078
let new_cfp = asm.sub(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
10791079
asm.mov(CFP, new_cfp);
1080-
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
1080+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
10811081

10821082
let mut cfunc_args = vec![recv];
10831083
cfunc_args.extend(args);
@@ -1087,7 +1087,7 @@ fn gen_ccall_with_frame(
10871087
asm_comment!(asm, "pop C frame");
10881088
let new_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
10891089
asm.mov(CFP, new_cfp);
1090-
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
1090+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
10911091

10921092
asm_comment!(asm, "restore SP register for the caller");
10931093
let new_sp = asm.sub(SP, sp_offset.into());
@@ -1166,7 +1166,7 @@ fn gen_ccall_variadic(
11661166
asm_comment!(asm, "switch to new CFP");
11671167
let new_cfp = asm.sub(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
11681168
asm.mov(CFP, new_cfp);
1169-
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
1169+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
11701170

11711171
let argv_ptr = gen_push_opnds(jit, asm, &args);
11721172
asm.count_call_to(&name.contents_lossy());
@@ -1175,7 +1175,7 @@ fn gen_ccall_variadic(
11751175
asm_comment!(asm, "pop C frame");
11761176
let new_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
11771177
asm.mov(CFP, new_cfp);
1178-
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
1178+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
11791179

11801180
asm_comment!(asm, "restore SP register for the caller");
11811181
let new_sp = asm.sub(SP, sp_offset.into());
@@ -1300,7 +1300,7 @@ fn gen_check_interrupts(jit: &mut JITState, asm: &mut Assembler, state: &FrameSt
13001300
asm_comment!(asm, "RUBY_VM_CHECK_INTS(ec)");
13011301
// Not checking interrupt_mask since it's zero outside finalize_deferred_heap_pages,
13021302
// signal_exec, or rb_postponed_job_flush.
1303-
let interrupt_flag = asm.load(Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_FLAG as i32));
1303+
let interrupt_flag = asm.load(Opnd::mem(32, EC, RUBY_OFFSET_EC_INTERRUPT_FLAG));
13041304
asm.test(interrupt_flag, interrupt_flag);
13051305
asm.jnz(jit, side_exit(jit, state, SideExitReason::Interrupt));
13061306
}
@@ -1623,7 +1623,7 @@ fn gen_send_iseq_direct(
16231623
asm_comment!(asm, "switch to new CFP");
16241624
let new_cfp = asm.sub(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
16251625
asm.mov(CFP, new_cfp);
1626-
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
1626+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
16271627

16281628
let params = unsafe { iseq.params() };
16291629

@@ -2231,7 +2231,7 @@ fn gen_return(asm: &mut Assembler, val: lir::Opnd) {
22312231
asm_comment!(asm, "pop stack frame");
22322232
let incr_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
22332233
asm.mov(CFP, incr_cfp);
2234-
asm.mov(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP);
2234+
asm.mov(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
22352235

22362236
// Order here is important. Because we're about to tear down the frame,
22372237
// we need to load the return value, which might be part of the frame.

‎zjit/src/cruby_bindings.inc.rs‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zjit/src/cruby_methods.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,13 @@ fn inline_thread_current(fun: &mut hir::Function, block: hir::BlockId, _recv: hi
321321
let thread_ptr = fun.push_insn(block, hir::Insn::LoadField {
322322
recv: ec,
323323
id: FieldName::thread_ptr,
324-
offset: RUBY_OFFSET_EC_THREAD_PTR as i32,
324+
offset: RUBY_OFFSET_EC_THREAD_PTR,
325325
return_type: types::CPtr,
326326
});
327327
let thread_self = fun.push_insn(block, hir::Insn::LoadField {
328328
recv: thread_ptr,
329329
id: FieldName::SelfParam,
330-
offset: RUBY_OFFSET_THREAD_SELF as i32,
330+
offset: RUBY_OFFSET_THREAD_SELF,
331331
// TODO(max): Add Thread type. But Thread.current is not guaranteed to be an exact Thread.
332332
// You can make subclasses...
333333
return_type: types::BasicObject,
@@ -461,7 +461,7 @@ fn inline_string_bytesize(fun: &mut hir::Function, block: hir::BlockId, recv: hi
461461
let len = fun.push_insn(block, hir::Insn::LoadField {
462462
recv,
463463
id: FieldName::len,
464-
offset: RUBY_OFFSET_RSTRING_LEN as i32,
464+
offset: RUBY_OFFSET_RSTRING_LEN,
465465
return_type: types::CInt64,
466466
});
467467

@@ -485,7 +485,7 @@ fn inline_string_getbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir
485485
let len = fun.push_insn(block, hir::Insn::LoadField {
486486
recv,
487487
id: FieldName::len,
488-
offset: RUBY_OFFSET_RSTRING_LEN as i32,
488+
offset: RUBY_OFFSET_RSTRING_LEN,
489489
return_type: types::CInt64,
490490
});
491491
// TODO(max): Find a way to mark these guards as not needed for correctness... as in, once
@@ -513,7 +513,7 @@ fn inline_string_setbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir
513513
let len = fun.push_insn(block, hir::Insn::LoadField {
514514
recv,
515515
id: FieldName::len,
516-
offset: RUBY_OFFSET_RSTRING_LEN as i32,
516+
offset: RUBY_OFFSET_RSTRING_LEN,
517517
return_type: types::CInt64,
518518
});
519519
let unboxed_index = fun.push_insn(block, hir::Insn::GuardLess { left: unboxed_index, right: len, reason: SideExitReason::GuardLess, state });
@@ -536,7 +536,7 @@ fn inline_string_empty_p(fun: &mut hir::Function, block: hir::BlockId, recv: hir
536536
let len = fun.push_insn(block, hir::Insn::LoadField {
537537
recv,
538538
id: FieldName::len,
539-
offset: RUBY_OFFSET_RSTRING_LEN as i32,
539+
offset: RUBY_OFFSET_RSTRING_LEN,
540540
return_type: types::CInt64,
541541
});
542542
let zero = fun.push_insn(block, hir::Insn::Const { val: hir::Const::CInt64(0) });

‎zjit/src/hir.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4374,7 +4374,7 @@ impl Function {
43744374
// See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h
43754375
let ptr = self.push_insn(block, Insn::LoadField {
43764376
recv, id: FieldName::as_heap,
4377-
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS as i32,
4377+
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS,
43784378
return_type: types::CPtr,
43794379
});
43804380
let offset = SIZEOF_VALUE_I32 * ivar_index as i32;
@@ -4386,7 +4386,7 @@ impl Function {
43864386

43874387
fn load_ivar_embedded(&mut self, block: BlockId, recv: InsnId, id: ID, ivar_index: attr_index_t) -> InsnId {
43884388
// See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h
4389-
let offset = ROBJECT_OFFSET_AS_ARY as i32
4389+
let offset = ROBJECT_OFFSET_AS_ARY
43904390
+ (SIZEOF_VALUE * ivar_index.to_usize()) as i32;
43914391
self.push_insn(block, Insn::LoadField {
43924392
recv, id: id.into(), offset,
@@ -4416,9 +4416,9 @@ impl Function {
44164416
match layout {
44174417
ShapeLayout::RClass | ShapeLayout::RData => {
44184418
let offset = if layout == ShapeLayout::RClass {
4419-
RCLASS_OFFSET_PRIME_FIELDS_OBJ as i32
4419+
RCLASS_OFFSET_PRIME_FIELDS_OBJ
44204420
} else {
4421-
TDATA_OFFSET_FIELDS_OBJ as i32
4421+
TDATA_OFFSET_FIELDS_OBJ
44224422
};
44234423

44244424
let fields_obj = self.push_insn(block, Insn::LoadField {
@@ -4597,10 +4597,10 @@ impl Function {
45974597
// Current shape contains this ivar
45984598
let (ivar_storage, offset) = if recv_type.flags().is_embedded() {
45994599
// See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h
4600-
let offset = ROBJECT_OFFSET_AS_ARY as i32 + (SIZEOF_VALUE * ivar_index.to_usize()) as i32;
4600+
let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * ivar_index.to_usize()) as i32;
46014601
(self_val, offset)
46024602
} else {
4603-
let as_heap = self.push_insn(block, Insn::LoadField { recv: self_val, id: FieldName::as_heap, offset: ROBJECT_OFFSET_AS_HEAP_FIELDS as i32, return_type: types::CPtr });
4603+
let as_heap = self.push_insn(block, Insn::LoadField { recv: self_val, id: FieldName::as_heap, offset: ROBJECT_OFFSET_AS_HEAP_FIELDS, return_type: types::CPtr });
46044604
let offset = SIZEOF_VALUE_I32 * ivar_index as i32;
46054605
(as_heap, offset)
46064606
};
@@ -9385,13 +9385,13 @@ mod validation_tests {
93859385
function.push_insn(entry, Insn::LoadField {
93869386
recv,
93879387
id: FieldName::as_heap,
9388-
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS as i32,
9388+
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS,
93899389
return_type: types::CPtr,
93909390
});
93919391
let ivar = function.push_insn(entry, Insn::LoadField {
93929392
recv,
93939393
id: FieldName::Id(ID(1)),
9394-
offset: ROBJECT_OFFSET_AS_ARY as i32,
9394+
offset: ROBJECT_OFFSET_AS_ARY,
93959395
return_type: types::BasicObject,
93969396
});
93979397
function.push_insn(entry, Insn::Return { val: ivar });
@@ -9418,13 +9418,13 @@ mod validation_tests {
94189418
function.push_insn(entry, Insn::LoadField {
94199419
recv,
94209420
id: FieldName::as_heap,
9421-
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS as i32,
9421+
offset: ROBJECT_OFFSET_AS_HEAP_FIELDS,
94229422
return_type: types::BasicObject,
94239423
});
94249424
let ivar = function.push_insn(entry, Insn::LoadField {
94259425
recv,
94269426
id: FieldName::Id(ID(1)),
9427-
offset: ROBJECT_OFFSET_AS_ARY as i32,
9427+
offset: ROBJECT_OFFSET_AS_ARY,
94289428
return_type: types::Array,
94299429
});
94309430
function.push_insn(entry, Insn::Return { val: ivar });

0 commit comments

Comments
 (0)