Skip to content

implement the write_bytes() intrinsic #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
solson merged 5 commits into from
Mar 14, 2017
Merged

implement the write_bytes() intrinsic #148

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f3440d
implement write_bytes intrinsic
Mar 13, 2017
aa2f998
add write-bytes test
Mar 13, 2017
e79ee14
write_bytes(): fix handling of types that are larger than u128
Mar 14, 2017
4cae50c
add write_bytes() test for a larger-than-u128 struct
Mar 14, 2017
41c2aa6
simplify write_bytes() by using memory.write_repeat()
Mar 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/terminator/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,25 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
}
}

"write_bytes" => {
let u8 = self.tcx.types.u8;
let ty = substs.type_at(0);
let size = self.type_size(ty)?.expect("write_bytes() type must be sized");
let val_byte = self.value_to_primval(arg_vals[1], u8)?.to_u128()?;
let mut pattern: u128 = 0;
for ii in 0..size {
pattern |= val_byte << (8 * ii);
}
let val_full = Value::ByVal(PrimVal::from_u128(pattern));
let mut ptr = arg_vals[0].read_ptr(&self.memory)?;

let count = self.value_to_primval(arg_vals[2], usize)?.to_u64()?;
for _ in 0..count {
self.write_value_to_ptr(val_full, ptr, ty)?;
Copy link
Contributor

@solson solson Mar 14, 2017

Choose a reason for hiding this comment

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

I don't think this is going to work properly for non-primitive types, especially when they are larger than u128.

I would rather eliminate all the pattern/val_full code and replace it with a simple loop with size * count iterations, writing a single val_byte each iteration.

Can you also add a test where ty is something like a struct with at least 3 fields (so our ByVal/ByValPair optimizations definitely won't take effect)?

Copy link
Contributor

@solson solson Mar 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather eliminate all the pattern/val_full code and replace it with a simple loop with size * count iterations, writing a single val_byte each iteration.

Actually, I think you could do a lower-level let bytes = self.memory.get_bytes_mut(ptr, size * count, ty_align); and then do a loop simply assigning the bytes directly. This would be much more efficient than repeatedly calling a single-byte write function on Memory which would do a lot of checks. It should be even more efficient than the val_full code.

Copy link
Contributor Author

@dwrensha dwrensha Mar 14, 2017

Choose a reason for hiding this comment

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

Memory::get_bytes_mut() is private and therefore inaccessible here. Shall I make it public?

Copy link
Contributor Author

@dwrensha dwrensha Mar 14, 2017

Choose a reason for hiding this comment

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

I've pushed a commit that updates to a simple loop with size * count iterations.

I shied away from using get_bytes_mut() for now.

Copy link
Contributor

@solson solson Mar 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, true. Just now when I was looking for a public wrapper, I noticed Memory::write_repeat which even does the loop for you.

The only thing I'm unsure about is the alignment requirement. write_repeat assumes no alignment requirement (align = 1), but I'm not sure if this write_bytes intrinsic should require the alignment of the type it's parameterized over.

Perhaps we could do:

self.memory.check_align(ptr, size * count, ty_align);
self.memory.write_repeat(ptr, val_byte, size * count);

Copy link
Contributor Author

@dwrensha dwrensha Mar 14, 2017

Choose a reason for hiding this comment

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

Cool! Done.

ptr = ptr.offset(size);
}
}

name => return Err(EvalError::Unimplemented(format!("unimplemented intrinsic: {}", name))),
}

Expand Down
17 changes: 17 additions & 0 deletions tests/run-pass/write-bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fn main() {
const LENGTH: usize = 10;
let mut v: [u64; LENGTH] = [0; LENGTH];

for idx in 0..LENGTH {
assert_eq!(v[idx], 0);
}

unsafe {
let p = v.as_mut_ptr();
::std::ptr::write_bytes(p, 0xab, LENGTH);
}

for idx in 0..LENGTH {
assert_eq!(v[idx], 0xabababababababab);
}
}