M-Labs/artiq-zynq
10
1
Fork 19
Code Issues 12 Pull Requests Releases Wiki Activity

disable LTO for libunwind #297

Closed
srenblad wants to merge 1 commits from srenblad/artiq-zynq:fix_linking_issue into master
pull from: srenblad/artiq-zynq:fix_linking_issue
merge into: M-Labs:master
Conversation 2 Commits 1 Files Changed 1 -1
srenblad commented 2024-05-14 14:24:35 +08:00
Contributor
Copy Link

Disable LTO in libunwind/build.rs

LTO causes unresolved symbol errors when building firmware. Appears to be caused by Clang/LLVM v14 upgrade.

LTO flag is not set for other libraries (libc, libksupport etc).

Fixes #245.

Testing:

  • Firmware build completes successfully with nix build .#build_soc-demo-firmware.
  • Smoke tested on firmware.

Performance:

Based on the #161 PR I recreated the micro benchmark (1000 nested try-excepts) and compared with an old firmware build both using the demo.json variant.

Mean time (s), n=10:

Old firmware (35ea0ed2ca): 0.005817
This commit: 0.006256

Note the ~7.5% slowdown in performance for this benchmark. Is this acceptable @sb10q?

I have attached the benchmark code I used for reference.

### Disable LTO in libunwind/build.rs LTO causes unresolved symbol errors when building firmware. Appears to be caused by Clang/LLVM v14 upgrade. LTO flag is not set for other libraries (libc, libksupport etc). Fixes #245. ### Testing: + Firmware build completes successfully with `nix build .#build_soc-demo-firmware`. + Smoke tested on firmware. ### Performance: Based on the #161 PR I recreated the micro benchmark (1000 nested try-excepts) and compared with an old firmware build both using the `demo.json` variant. Mean time (s), n=10: Old firmware (35ea0ed2ca): 0.005817 This commit: 0.006256 Note the ~7.5% slowdown in performance for this benchmark. Is this acceptable @sb10q? I have attached the benchmark code I used for reference.
perf_checker.py
2.1 KiB
srenblad added 1 commit 2024-05-14 14:24:35 +08:00
srenblad commented 2024-05-14 15:17:17 +08:00
Author
Contributor
Copy Link

LTO was added in #161 with the justification:

Enabling LTO improves the performance a bit, I thought this is a separate thing to do so I separated it into another commit.

LTO was added in #161 with the justification: > Enabling LTO improves the performance a bit, I thought this is a separate thing to do so I separated it into another commit.
sb10q commented 2024-05-20 19:37:57 +08:00
Owner
Copy Link

Solution is keep LLVM to the same version used by rustc.

Solution is keep LLVM to the same version used by rustc.
👍 1
sb10q closed this pull request 2024-05-20 19:37:58 +08:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
priority:critical

   
priority:high

   
priority:low

   
priority:medium
No Label
priority:critical
priority:high
priority:low
priority:medium
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#297
No description provided.
Delete Branch "srenblad/artiq-zynq:fix_linking_issue"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?