Skip to content
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

Fix the representation of C void pointers in LLVM IR #11539

Merged
merged 1 commit into from
Jan 14, 2014

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Jan 14, 2014

Currently, we have c_void defined to be represented as an empty struct,
but LLVM expects C's void* to be represented as i8*. That means we
currently generate code in which LLVM doesn't recognize malloc() and
free() and can't apply certain optimization that would remove calls to
those functions.

@alexcrichton
Copy link
Member

Could you post some before/after code of what this change in types enables?

It's also probably worth adding to the comment on the definition of c_void as to why it's u8 (which probably isn't what one would expect)

@dotdash
Copy link
Contributor Author

dotdash commented Jan 14, 2014

Test code:

fn main() {
    let x = ~3;
}

Compile with rustc --emit-llvm -S -O -Z lto issue11539.rs
Before:

define internal void @main::hd4b0664c300788adah::v0.0({ i64, %tydesc*, i8*, i8*, i8 }* nocapture readnone) unnamed_addr #0 {
"function top level":
  %1 = tail call %"enum.std::libc::types::common::c95::c_void[#1]"* @malloc(i64 8)
  %2 = icmp eq %"enum.std::libc::types::common::c95::c_void[#1]"* %1, null
  br i1 %2, label %_ZN2rt11global_heap10malloc_raw19h000735e5ce40b7f3aR4v0.9E.exit.thread, label %cond.i

_ZN2rt11global_heap10malloc_raw19h000735e5ce40b7f3aR4v0.9E.exit.thread: ; preds = %"function top level"
  tail call void @abort()
  call void @llvm.trap()
  unreachable

cond.i:                                           ; preds = %"function top level"
  %3 = bitcast %"enum.std::libc::types::common::c95::c_void[#1]"* %1 to i64*
  store i64 3, i64* %3, align 8
  tail call void @free(%"enum.std::libc::types::common::c95::c_void[#1]"* %1) #1
  ret void
}

After:

define internal void @main::hd4b0664c300788adah::v0.0({ i64, %tydesc*, i8*, i8*, i8 }* nocapture readnone) unnamed_addr #0 {
"_ZN13_$UP$$LP$$RP$9glue_drop19h82dc7f896d315c35a8E.exit":
  ret void
}

@dotdash
Copy link
Contributor Author

dotdash commented Jan 14, 2014

check-stage1-std TESTNAME=zero_1kb

Before:

run: x86_64-unknown-linux-gnu/stage1/test/stdtest-x86_64-unknown-linux-gnu

running 5 tests
test vec::bench::zero_1kb_fixed_repeat                          ... bench:        74 ns/iter (+/- 4)
test vec::bench::zero_1kb_from_elem                             ... bench:       135 ns/iter (+/- 62)
test vec::bench::zero_1kb_loop_set                              ... bench:       810 ns/iter (+/- 254)
test vec::bench::zero_1kb_mut_iter                              ... bench:        73 ns/iter (+/- 2)
test vec::bench::zero_1kb_set_memory                            ... bench:        74 ns/iter (+/- 1)

metrics saved to: tmp/check-stage1-T-x86_64-unknown-linux-gnu-H-x86_64-unknown-linux-gnu-std-metrics.json
test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured

After:

run: x86_64-unknown-linux-gnu/stage1/test/stdtest-x86_64-unknown-linux-gnu

running 5 tests
test vec::bench::zero_1kb_fixed_repeat                          ... bench:        74 ns/iter (+/- 3)
test vec::bench::zero_1kb_from_elem                             ... bench:        73 ns/iter (+/- 4)
test vec::bench::zero_1kb_loop_set                              ... bench:       550 ns/iter (+/- 3)
test vec::bench::zero_1kb_mut_iter                              ... bench:        72 ns/iter (+/- 3)
test vec::bench::zero_1kb_set_memory                            ... bench:        72 ns/iter (+/- 3)

metrics saved to: tmp/check-stage1-T-x86_64-unknown-linux-gnu-H-x86_64-unknown-linux-gnu-std-metrics.json
test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured

@alexcrichton
Copy link
Member

I played around a bit, and I think we can get the same optimization with

#[repr(u8)]
pub enum c_void {
   priv variant1,
   priv variant2,
}

In order for repr to work, it needs to have at least two variants apparently? A comment could then explain why it looks like this (for optimization purposes), but this should also disallow accidental dereferencing or creation of c_void values.

Currently, we have c_void defined to be represented as an empty struct,
but LLVM expects C's void* to be represented as i8*. That means we
currently generate code in which LLVM doesn't recognize malloc() and
free() and can't apply certain optimization that would remove calls to
those functions.
@dotdash
Copy link
Contributor Author

dotdash commented Jan 14, 2014

The test suite isn't built with LTO. With LTO we get:

Before:

running 5 tests
test zero_1kb_fixed_repeat ... bench:        54 ns/iter (+/- 0)
test zero_1kb_from_elem    ... bench:        54 ns/iter (+/- 0)
test zero_1kb_loop_set     ... bench:       528 ns/iter (+/- 2)
test zero_1kb_mut_iter     ... bench:        54 ns/iter (+/- 0)
test zero_1kb_set_memory   ... bench:        54 ns/iter (+/- 0)

test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured

After:

running 5 tests
test zero_1kb_fixed_repeat ... bench:         0 ns/iter (+/- 0)
test zero_1kb_from_elem    ... bench:         0 ns/iter (+/- 0)
test zero_1kb_loop_set     ... bench:       528 ns/iter (+/- 1)
test zero_1kb_mut_iter     ... bench:         0 ns/iter (+/- 0)
test zero_1kb_set_memory   ... bench:         0 ns/iter (+/- 0)

test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured

The "before" numbers are probably faster than before, because I copied the benchmarks into their own file and we still get better inlining behaviour when there is only one user of a function.

bors added a commit that referenced this pull request Jan 14, 2014
Currently, we have c_void defined to be represented as an empty struct,
but LLVM expects C's void* to be represented as i8*. That means we
currently generate code in which LLVM doesn't recognize malloc() and
free() and can't apply certain optimization that would remove calls to
those functions.
@bors bors closed this Jan 14, 2014
@bors bors merged commit 5902263 into rust-lang:master Jan 14, 2014
@dotdash dotdash deleted the void_type_fixup branch January 18, 2014 13:02
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 21, 2023
…ntri3

Warn missing_enforced_import_renames by default

Similar to rust-lang/rust-clippy#8261 that did the same thing to disallowed_methods & disallowed_types.
This lint is also only triggered if import renames are defined in the `clippy.toml` file.

changelog: Moved [`missing_enforced_import_renames`] to `style` (Now warn-by-default)
[rust-lang#11539](rust-lang/rust-clippy#11539)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants