Skip to content

Skip needless calls to get_align in some cases. #718

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 20 additions & 13 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,22 +938,29 @@
fn load(&mut self, pointee_ty: Type<'gcc>, ptr: RValue<'gcc>, align: Align) -> RValue<'gcc> {
let block = self.llbb();
let function = block.get_function();
// FractalFir: In some cases, we *should* skip the call to get_aligned.
// For example, calling `get_aligned` on a i8 is pointless(since it can only be 1 aligned)
// Calling get_aligned on a `u128`/`i128` leads to issues(TODO: ask Anton what *kind* of issues,
// describe them in detali).

Check warning on line 944 in src/builder.rs

View workflow job for this annotation

GitHub Actions / spell_check

Unknown word (detali)
// So, we skip the call to `get_aligned` in such a case. *Ideally*, we could do this for all the types,
// but the GCC APIs to facilitate this just aren't quite there yet.

// This checks that we only skip `get_aligned` on 128 bit ints if they have the correct aligement.

Check warning on line 948 in src/builder.rs

View workflow job for this annotation

GitHub Actions / spell_check

Unknown word (aligement)
// Otherwise, this may be an under-aligned load, so we will still call get_aligned.
let mut can_skip_align = (pointee_ty == self.cx.u128_type
|| pointee_ty == self.cx.i128_type)
&& align == self.int128_align;
// We can skip the call to `get_aligned` for byte-sized types with aligement of 1.

Check warning on line 953 in src/builder.rs

View workflow job for this annotation

GitHub Actions / spell_check

Unknown word (aligement)
can_skip_align |=
(pointee_ty == self.cx.u8_type || pointee_ty == self.cx.i8_type) && align.bytes() == 1;
// Skip the call to `get_aligned` when possible.
let aligned_type =
if can_skip_align { pointee_ty } else { pointee_ty.get_aligned(align.bytes()) };

let ptr = self.context.new_cast(self.location, ptr, aligned_type.make_pointer());
// NOTE: instead of returning the dereference here, we have to assign it to a variable in
// the current basic block. Otherwise, it could be used in another basic block, causing a
// dereference after a drop, for instance.
// FIXME(antoyo): this check that we don't call get_aligned() a second time on a type.
// Ideally, we shouldn't need to do this check.
// FractalFir: the `align == self.int128_align` check ensures we *do* call `get_aligned` if
// the alignment of a `u128`/`i128` is not the one mandated by the ABI. This ensures we handle
// under-aligned loads correctly.
let aligned_type = if (pointee_ty == self.cx.u128_type || pointee_ty == self.cx.i128_type)
&& align == self.int128_align
{
pointee_ty
} else {
pointee_ty.get_aligned(align.bytes())
};
let ptr = self.context.new_cast(self.location, ptr, aligned_type.make_pointer());
let deref = ptr.dereference(self.location).to_rvalue();
let loaded_value = function.new_local(
self.location,
Expand Down
7 changes: 7 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@
.layout_of(ty::TypingEnv::fully_monomorphized().as_query_input(rust_type))
.unwrap();
let align = layout.align.abi.bytes();
// For types with size 1, the aligement can be 1 and only 1

Check warning on line 160 in src/context.rs

View workflow job for this annotation

GitHub Actions / spell_check

Unknown word (aligement)
// So, we can skip the call to ``get_aligned`.
// In the future, we can add a GCC API to query the type align,
// and call `get_aligned` if and only if that differs from Rust's expectations.
if layout.size.bytes() == 1 {
return context.new_c_type(ctype);
}
#[cfg(feature = "master")]
{
context.new_c_type(ctype).get_aligned(align)
Expand Down
Loading