diff --git a/changelog.d/11817-codegen-hidden-visibility-decl.md b/changelog.d/11817-codegen-hidden-visibility-decl.md new file mode 100644 index 0000000000..1f98c278b7 --- /dev/null +++ b/changelog.d/11817-codegen-hidden-visibility-decl.md @@ -0,0 +1 @@ +- **perry-codegen**: a module split into codegen units no longer panics with "cannot form external declaration for generated global" when one unit references another unit's closure-info record. Those records are emitted `hidden constant`, and the cross-unit declaration builder stripped only linkage keywords, so the visibility keyword made it give up. It now keeps the preemption and visibility on the declaration. Linux/COFF only (Mach-O replicates the globals instead); it aborted the OpenCode v1.18.30 build in effect's `Schema.ts`. diff --git a/crates/perry-codegen/src/module/linkage.rs b/crates/perry-codegen/src/module/linkage.rs index dad9da83ee..983ade120d 100644 --- a/crates/perry-codegen/src/module/linkage.rs +++ b/crates/perry-codegen/src/module/linkage.rs @@ -187,12 +187,20 @@ pub(crate) fn external_decl_for_global(line: &str) -> Option { } let (name, rhs) = line.split_once(" = ")?; let rhs = strip_leading_linkage(rhs.trim_start()); + // After the linkage, LLVM orders a global's prefix as + // [preemption] [visibility] [thread_local] [unnamed_addr] global|constant + // A declaration in another unit must keep the preemption and visibility of + // the definition it names. Closure-info records are emitted + // `hidden constant` (fn_info.rs), and on ELF/COFF, where globals are not + // replicated, every cross-unit reference to one is declared here. + let (preemption, rhs) = take_keyword(rhs, &["dso_local", "dso_preemptable"]); + let (visibility, rhs) = take_keyword(rhs, &["default", "hidden", "protected"]); // #10399: keep the TLS specifier — `@g = external global i8` and // `@g = external thread_local global i8` are different symbols to LLVM. let (tls, rhs) = split_thread_local(rhs); - let (kind, rest) = if let Some(rest) = rhs.strip_prefix("unnamed_addr constant ") { - ("constant", rest) - } else if let Some(rest) = rhs.strip_prefix("constant ") { + // `unnamed_addr` describes the definition only; the declaration drops it. + let (_, rhs) = take_keyword(rhs, &["unnamed_addr", "local_unnamed_addr"]); + let (kind, rest) = if let Some(rest) = rhs.strip_prefix("constant ") { ("constant", rest) } else if let Some(rest) = rhs.strip_prefix("global ") { ("global", rest) @@ -223,12 +231,29 @@ pub(crate) fn external_decl_for_global(line: &str) -> Option { } _ => rest.find(char::is_whitespace).unwrap_or(rest.len()), }; - let tls = if tls.is_empty() { - String::new() - } else { - format!("{tls} ") - }; - Some(format!("{name} = external {tls}{kind} {}", &rest[..ty_end])) + let prefix: String = [preemption, visibility, tls] + .iter() + .filter(|keyword| !keyword.is_empty()) + .map(|keyword| format!("{keyword} ")) + .collect(); + Some(format!( + "{name} = external {prefix}{kind} {}", + &rest[..ty_end] + )) +} + +/// Take one of `keywords` from the front of `s` when it is followed by +/// whitespace, returning `(keyword, rest)`, or `("", s)` when none is there. +fn take_keyword<'a>(s: &'a str, keywords: &[&'static str]) -> (&'static str, &'a str) { + let t = s.trim_start(); + for keyword in keywords { + if let Some(rest) = t.strip_prefix(keyword) { + if rest.starts_with(char::is_whitespace) { + return (keyword, rest.trim_start()); + } + } + } + ("", t) } /// Attribute-group suffix for a runtime-helper `declare` line, keyed by diff --git a/crates/perry-codegen/src/module/tests.rs b/crates/perry-codegen/src/module/tests.rs index 01468e055f..51affe66b8 100644 --- a/crates/perry-codegen/src/module/tests.rs +++ b/crates/perry-codegen/src/module/tests.rs @@ -804,6 +804,33 @@ fn split_unit_declares_local_function_used_as_pointer_argument() { assert!(init_unit.contains(&format!("declare double @{wrapper_name}(i64, double)"))); } +#[test] +fn external_decl_keeps_hidden_visibility_of_closure_info() { + // Closure-info records are emitted `hidden constant` (fn_info.rs). On ELF + // and COFF every cross-unit reference to one is declared through + // `external_decl_for_global`, which only stripped LINKAGE keywords and so + // returned `None` for a VISIBILITY keyword — a panic while splitting + // effect's Schema.ts into codegen units on Linux. + use crate::module::linkage::external_decl_for_global; + assert_eq!( + external_decl_for_global( + "@perry_closure_m__247$info = hidden constant { ptr, i16, i16 } { ptr @perry_closure_m__247, i16 2, i16 0 }" + ) + .as_deref(), + Some("@perry_closure_m__247$info = external hidden constant { ptr, i16, i16 }") + ); + // The preemption specifier precedes visibility and is kept too. + assert_eq!( + external_decl_for_global("@g = dso_local hidden global i32 0").as_deref(), + Some("@g = external dso_local hidden global i32") + ); + // Visibility composes with the TLS specifier in LLVM's order. + assert_eq!( + external_decl_for_global("@t = hidden thread_local global i8 0").as_deref(), + Some("@t = external hidden thread_local global i8") + ); +} + #[test] fn string_constant_escapes_nonprintable() { let mut m = LlModule::new("arm64-apple-macosx15.0.0");