Skip to content

Commit

Permalink
Allow blockaddress to reference non-locals
Browse files Browse the repository at this point in the history
This commit changes the `GetBlockAddressStatement` to allow it to
reference non-local function pointers via name, rather than requiring
the reference target to always be a local `BlockId`.

The previous implementation of the function pointer compilation was not
able to generate relocatable function pointers to functions that are
declared but not defined in the current translation unit. This commit
changes that to now make this case work properly.

This is the first point at which `alloc` is able to be compiled from
LLVM IR to FLO without any crashes or missing features being
encountered.

It also fixes the building of the transient IR input artifacts for
testing the compiler to allow it to work properly on CI. This was not
caught before as there were no tests yet depending on their presence.

The new approach uses a `build.rs` build script to copy them in, and has
also removed one that had accidentally been committed.
  • Loading branch information
iamrecursion committed Jan 27, 2025
1 parent a69e352 commit bad1da4
Show file tree
Hide file tree
Showing 9 changed files with 240 additions and 141 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ main
crates/compiler/input/compilation/alloc.ll
crates/compiler/input/compilation/compiler_builtins.ll
crates/compiler/input/compilation/core.ll
crates/compiler/input/compilation/hieratika_rust_test_input.ll

# Tooling
.idea/
Expand Down
61 changes: 61 additions & 0 deletions crates/compiler/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use std::{fs, path::Path};

/// The environment variable that we get the input files from.
const SOURCE_FILE_ENV_VAR: &str = "RUST_TEST_INPUTS";

/// The target directory in the project where the artefacts should end up.
const TARGET_DIR: &str = "input/compilation";

/// Executes to copy the compilation inputs into the correct place in the build
/// directory if they do not exist.
#[allow(clippy::permissions_set_readonly_false)] // We do not care if it is world-writable
fn main() {
println!("cargo:rerun-if-changed=build.rs");

// We do want to continue if they are missing, at least for compilation.
let env_val = std::env::var(SOURCE_FILE_ENV_VAR).unwrap_or("".into());
if env_val.is_empty() {
return;
}

// Grab our path
let path = Path::new(&env_val);

// If it is not a directory but is set we just continue.
if !path.is_dir() {
return;
}

// Now we can loop over the entries in the directory.
for entry in path
.read_dir()
.expect("Path known to be a directory was not a directory")
.flatten()
{
let path = entry.path();
let filename = path.file_name().expect("Path known to be a file had no filename");
let target_path = Path::new(TARGET_DIR).join(filename);

// Make it writable if it exists so we can overwrite if needed.
if target_path.exists() {
let mut perms = target_path
.metadata()
.expect("No permissions found for the target path")
.permissions();
perms.set_readonly(false);
fs::set_permissions(&target_path, perms.clone()).expect("Could not set permissions");
}

// Actually copy the file.
fs::copy(path, &target_path).expect("Could not copy file known to exist");

// Make it writable after the copy in case it retained the read-only properties
// from the nix store.
let mut perms = target_path
.metadata()
.expect("No permissions found for the target path")
.permissions();
perms.set_readonly(false);
fs::set_permissions(&target_path, perms.clone()).expect("Could not set permissions");
}
}
3 changes: 3 additions & 0 deletions crates/compiler/input/compilation/constants.ll
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ target triple = "riscv64"
@function_pointer_const = constant ptr @hieratika_test_reference_const
@function_pointer_const_in_struct = constant { i1, ptr } { i1 0, ptr @hieratika_test_reference_const }

declare i64 @test_external_function()

define i64 @hieratika_test_call_function_ptr() unnamed_addr {
start:
%result = call i64 @function_pointer_const()
%result2 = call i64 @test_external_function()
ret i64 %result
}

Expand Down
115 changes: 0 additions & 115 deletions crates/compiler/input/compilation/hieratika_rust_test_input.ll

This file was deleted.

40 changes: 35 additions & 5 deletions crates/compiler/src/obj_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,18 @@ impl ObjectGenerator {
// error, so we crash loudly.
let module_map = self.get_module_map();

// TODO External symbols that are DECLARED but not DEFINED can also be
// referenced as function pointers. We need to be able to deal with this.
//
// - The first option is to create a stub block that is used as a reference to
// be relocated.
// - The second option is to change the `GetBlockAddress` operation to instead
// take a `BlockRef` which can be resolved at link time.
//
// It _probably_ makes more sense to change how GetBlockAddress works, and
// handle this at the linking stage, than to try and marry up two disparate
// blocks via a new symbol table.

// Globals are referenced in function definitions, but may also reference
// function names. To that end, we start by registering variables of the
// appropriate type for each global.
Expand Down Expand Up @@ -370,8 +382,13 @@ impl ObjectGenerator {
///
/// - [`Error`], if the function pointer variable cannot be generated for
/// any reason.
///
/// # Panics
///
/// - If a function is known to be internal in the module map but has no
/// associated block in the Object Map.
pub fn generate_function_pointer_vars(&self, data: &mut ObjectContext) -> Result<()> {
for (function_name, block_id) in data.map.module_functions.clone() {
for (function_name, function_data) in &self.get_module_map().functions {
// We start by creating a variable to represent the function pointer, which is
// unconditionally set to be local as these are synthetic constructs.
let function_pointer_var = Variable {
Expand All @@ -387,7 +404,9 @@ impl ObjectGenerator {

// We can then insert this variable in to the object map as a
// global, which is a not-inaccurate way to think about it.
data.map.module_globals.insert(function_name, function_pointer);
data.map
.module_globals
.insert(function_name.clone(), function_pointer);

// This is not sufficient, however, as we also need to _initialize_
// this variable to a sensible function pointer representation.
Expand All @@ -409,7 +428,18 @@ impl ObjectGenerator {
// We use the blockaddress mechanism that already exists here to mark it as
// relocatable, writing it into the already-declared variable for the function
// pointer.
bb.simple_get_block_address(function_pointer, block_id);
match function_data.kind {
TopLevelEntryKind::Definition => {
let block_id =
data.map.module_functions.get(function_name).unwrap_or_else(|| {
panic!("No block found for local function {function_name}")
});
bb.simple_get_internal_block_address(function_pointer, *block_id);
}
TopLevelEntryKind::Declaration => {
bb.simple_get_external_block_address(function_pointer, function_name);
}
}

// All that is left to do is return nothing from the initializer.
bb.end_with_return(Vec::new());
Expand Down Expand Up @@ -3800,7 +3830,7 @@ impl ObjectGenerator {
if value.is_poison() || value.is_undef() || value.is_null() {
// In the case that it is a poison value, we can set it to any pointer. We
// choose the null block for this module.
bb.simple_get_block_address(variable, func_ctx.poison_block());
bb.simple_get_internal_block_address(variable, func_ctx.poison_block());

Ok(variable)
} else if let Some(const_ptr) = func_ctx.lookup_variable(value_name) {
Expand Down Expand Up @@ -4023,7 +4053,7 @@ impl ObjectGenerator {
))
})?;

bb.simple_get_block_address(variable, *block_id);
bb.simple_get_internal_block_address(variable, *block_id);

Ok(variable)
}
Expand Down
11 changes: 5 additions & 6 deletions crates/compiler/tests/compilation_alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ fn compiles_alloc() -> miette::Result<()> {

// Currently known to fail due to missing features and bugs.

// let compiler =
// common::default_compiler_from_path("input/compilation/alloc.ll")?;
// let flo = compiler.run()?;
//
// // There should be a single function in the context.
// assert_eq!(common::count_functions(&flo, false), 1);
let compiler = common::default_compiler_from_path("input/compilation/alloc.ll")?;
let flo = compiler.run()?;

// There should be a single function in the context.
assert_eq!(common::count_functions(&flo, false), 778);

Ok(())
}
Loading

0 comments on commit bad1da4

Please sign in to comment.