Skip to content

Commit

Permalink
Auto merge of #113430 - Zalathar:hash, r=b-naber
Browse files Browse the repository at this point in the history
Remove `LLVMRustCoverageHashCString`

Coverage has two FFI functions for computing the hash of a byte string. One takes a ptr/len pair (`LLVMRustCoverageHashByteArray`), and the other takes a NUL-terminated C string (`LLVMRustCoverageHashCString`).

But on closer inspection, the C string version is unnecessary. The calling-side code converts a Rust `&str` into a `CString`, and the C++ code then immediately turns it back into a ptr/len string before actually hashing it. So we can just call the ptr/len version directly instead.

---

This PR also fixes a bug in the C++ declaration of `LLVMRustCoverageHashByteArray`. It should be `size_t`, since that's what is declared and passed on the Rust side, and it's what `StrRef`'s constructor expects to receive on the callee side.
  • Loading branch information
bors committed Jul 16, 2023
2 parents 2c718d1 + 352d031 commit ffb9b61
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 17 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) {
let mut function_data = Vec::new();
for (instance, function_coverage) in function_coverage_map {
debug!("Generate function coverage for {}, {:?}", cx.codegen_unit.name(), instance);
let mangled_function_name = tcx.symbol_name(instance).to_string();
let mangled_function_name = tcx.symbol_name(instance).name;
let source_hash = function_coverage.source_hash();
let is_used = function_coverage.is_used();
let (expressions, counter_regions) =
Expand Down Expand Up @@ -95,7 +95,7 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) {

let filenames_size = filenames_buffer.len();
let filenames_val = cx.const_bytes(&filenames_buffer);
let filenames_ref = coverageinfo::hash_bytes(filenames_buffer);
let filenames_ref = coverageinfo::hash_bytes(&filenames_buffer);

// Generate the LLVM IR representation of the coverage map and store it in a well-known global
let cov_data_val = mapgen.generate_coverage_map(cx, version, filenames_size, filenames_val);
Expand Down Expand Up @@ -228,7 +228,7 @@ impl CoverageMapGenerator {
/// specific, well-known section and name.
fn save_function_record(
cx: &CodegenCx<'_, '_>,
mangled_function_name: String,
mangled_function_name: &str,
source_hash: u64,
filenames_ref: u64,
coverage_mapping_buffer: Vec<u8>,
Expand All @@ -238,7 +238,7 @@ fn save_function_record(
let coverage_mapping_size = coverage_mapping_buffer.len();
let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer);

let func_name_hash = coverageinfo::hash_str(&mangled_function_name);
let func_name_hash = coverageinfo::hash_bytes(mangled_function_name.as_bytes());
let func_name_hash_val = cx.const_u64(func_name_hash);
let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32);
let source_hash_val = cx.const_u64(source_hash);
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,7 @@ pub(crate) fn write_mapping_to_buffer(
}
}

pub(crate) fn hash_str(strval: &str) -> u64 {
let strval = CString::new(strval).expect("null error converting hashable str to C string");
unsafe { llvm::LLVMRustCoverageHashCString(strval.as_ptr()) }
}

pub(crate) fn hash_bytes(bytes: Vec<u8>) -> u64 {
pub(crate) fn hash_bytes(bytes: &[u8]) -> u64 {
unsafe { llvm::LLVMRustCoverageHashByteArray(bytes.as_ptr().cast(), bytes.len()) }
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1916,7 +1916,6 @@ extern "C" {
);

pub fn LLVMRustCoverageCreatePGOFuncNameVar(F: &Value, FuncName: *const c_char) -> &Value;
pub fn LLVMRustCoverageHashCString(StrVal: *const c_char) -> u64;
pub fn LLVMRustCoverageHashByteArray(Bytes: *const c_char, NumBytes: size_t) -> u64;

#[allow(improper_ctypes)]
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,9 @@ extern "C" LLVMValueRef LLVMRustCoverageCreatePGOFuncNameVar(LLVMValueRef F, con
return wrap(createPGOFuncNameVar(*cast<Function>(unwrap(F)), FuncNameRef));
}

extern "C" uint64_t LLVMRustCoverageHashCString(const char *StrVal) {
StringRef StrRef(StrVal);
return IndexedInstrProf::ComputeHash(StrRef);
}

extern "C" uint64_t LLVMRustCoverageHashByteArray(
const char *Bytes,
unsigned NumBytes) {
size_t NumBytes) {
StringRef StrRef(Bytes, NumBytes);
return IndexedInstrProf::ComputeHash(StrRef);
}
Expand Down

0 comments on commit ffb9b61

Please sign in to comment.