Skip to content

Comments

feat!: make sure constructors don't get optimised out#1540

Merged
ghaith merged 27 commits intomasterfrom
fix_arm_constructors
Feb 23, 2026
Merged

feat!: make sure constructors don't get optimised out#1540
ghaith merged 27 commits intomasterfrom
fix_arm_constructors

Conversation

@ghaith
Copy link
Collaborator

@ghaith ghaith commented Oct 30, 2025

Adds an LLVM C++ Wrapper that provides access to mark the compiled application to use the elf .init_array constructors.
Removes the built in linker scripts and deprecates the --no-linker-script flag.
This is a breaking change as the init functions are no longer visible in the binary.

This PR also activates builds on aarch64 (arm64) and for that we needed to filter out alignmets from all the tests.

@ghaith ghaith force-pushed the fix_arm_constructors branch 3 times, most recently from 18437fb to 750d3e5 Compare November 19, 2025 13:15
@ghaith ghaith force-pushed the fix_arm_constructors branch from 38bae86 to 8b4e085 Compare November 25, 2025 06:13
@ghaith ghaith changed the title fix: make sure constructors don't get optimised out feat!: make sure constructors don't get optimised out Nov 25, 2025
@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

❌ Patch coverage is 75.72816% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.64%. Comparing base (54406fb) to head (e69b8fe).
⚠️ Report is 86 commits behind head on master.

Files with missing lines Patch % Lines
src/codegen/debug.rs 52.00% 12 Missing ⚠️
compiler/plc_llvm/build.rs 74.41% 11 Missing ⚠️
compiler/plc_driver/src/pipelines.rs 50.00% 1 Missing ⚠️
src/codegen/generators/pou_generator.rs 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1540      +/-   ##
==========================================
- Coverage   94.95%   94.64%   -0.31%     
==========================================
  Files         174      187      +13     
  Lines       56307    58487    +2180     
==========================================
+ Hits        53464    55354    +1890     
- Misses       2843     3133     +290     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ghaith ghaith force-pushed the fix_arm_constructors branch from 4615f6b to 943e9fe Compare November 28, 2025 06:26
@ghaith ghaith marked this pull request as ready for review November 28, 2025 08:08
@ghaith ghaith force-pushed the fix_arm_constructors branch 8 times, most recently from 1d77ab9 to 6745bed Compare January 21, 2026 15:56
adds a new plc_llvm crate that wraps llvm c++ apis.
The only api currently provided is to expose the init array flag.
This ensures that the constructors created by the compiler are actually
called in aarch64 architectures.

filter out alignments so we are compatible with aarch64

filter debug align info

add alignment tests
@ghaith ghaith force-pushed the fix_arm_constructors branch from 6745bed to fb001ba Compare January 21, 2026 16:06
@ghaith ghaith requested review from Copilot, mhasel and volsa January 21, 2026 16:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the codebase to ensure constructors are not optimized out by using the ELF .init_array section. It removes built-in linker scripts, deprecates the --no-linker-script flag, and enables builds on aarch64 (arm64) architecture. To support cross-architecture compatibility, alignment values in test snapshots have been filtered out.

Changes:

  • Modified LLVM global constructors to use priority 65535 instead of 0 for the .init_array section
  • Updated all test snapshots to replace specific alignment values with [filtered] placeholders
  • Added a new alignment_tests module to handle architecture-specific alignment testing

Reviewed changes

Copilot reviewed 295 out of 407 changed files in this pull request and generated 2 comments.

File Description
src/codegen/tests/snapshots/*.snap Updated test snapshots to filter out alignment values for cross-architecture compatibility
src/codegen/tests.rs Added new alignment_tests module
Comments suppressed due to low confidence (3)

src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_var_array_added_to_debug_info.snap:1

  • The removal of the snapshot_kind: text metadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---

src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__global_alias_type.snap:1

  • The removal of the snapshot_kind: text metadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---

src/codegen/tests/snapshots/rusty__codegen__tests__debug_tests__dwarf_version_override.snap:1

  • The removal of the snapshot_kind: text metadata line is inconsistent with the PR description which mentions filtering alignments but doesn't mention metadata changes. Verify that this metadata removal is intentional and doesn't affect test execution.
---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ghaith ghaith requested review from Copilot and mhasel February 10, 2026 19:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 295 out of 408 changed files in this pull request and generated 12 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mhasel
mhasel previously approved these changes Feb 19, 2026
@ghaith ghaith requested a review from mhasel February 23, 2026 06:15
@ghaith ghaith merged commit ad317f5 into master Feb 23, 2026
33 checks passed
@ghaith ghaith deleted the fix_arm_constructors branch February 23, 2026 09:39
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.

2 participants