-
Notifications
You must be signed in to change notification settings - Fork 71
Implement volatile loads and stores #167
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks!
let loaded_value = function.new_local(None, value_type, &format!("loadedValue{}", unsafe { RETURN_VALUE_COUNT })); | ||
block.add_assignment(None, loaded_value, deref); | ||
loaded_value.to_rvalue() | ||
// TODO: handle non-natural alignments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rust repo requires that TODO should be annotated with a name. If you don't plan to fix this later, just put my username there.
modified_destination_type = modified_destination_type.make_volatile(); | ||
} | ||
|
||
// FIXME: The type checking in `add_assignment` removes only one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for FIXME
.
let modified_ptr = self.cx.context.new_bitcast(None, ptr, modified_destination_type.make_pointer()); | ||
let modified_destination = modified_ptr.dereference(None); | ||
self.llbb().add_assignment(None, modified_destination, val); | ||
// TODO: handle `MemFlags::NONTEMPORAL`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
let value_type = deref.get_type(); | ||
|
||
// If `is_volatile == true`, convert `ptr` to `volatile value_type *` | ||
// and try dereference again. (libgccjit doesn't provide a way to get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if that's what you're talking about, but an alternative is to do:
ptr.get_type().get_pointee()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that's exactly what I was looking for.
The CI failed for libgccjit12, btw. Not sure if it's normal. |
Not normal, I think. Also, I found that volatile reads are still being eliminated by p_count.write_volatile(0);
STORAGE.add(0).read_volatile();
STORAGE.add(PAGE_SIZE).read_volatile();
STORAGE.add(0).write_volatile(1);
|
I just got bit by this bug. Is there any plan to fix it ? |
@kellda: I will try to take a look in the upcomming weeks. |
@kellda: This was implemented in #572 and rust-lang/gcc@50d1270. Please tell me if that works for you. |
Yes it works, thank you |
Fixes #166.