Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renames LDC modules from .di to .d #4730

Closed
wants to merge 1 commit into from

Conversation

denizzzka
Copy link
Contributor

@denizzzka denizzzka commented Aug 11, 2024

Renames few modules from .di to .d because it is need to compile-in these modules contents into druntime to avoid undefined ModuleInfo symbols. This only matters when linking a D static library which uses these modules with static druntime.

Modules named *.di are ignored by CMake build script

@kinke
Copy link
Member

kinke commented Aug 11, 2024

This shouldn't be required (otherwise somebody would have surely complained already), as the modules are basically standalone, only ldc.simd importing core.simd, which is more or less standalone itself. No module ctors which would drag in a ModuleInfo dependency.

So I assume you have modified these files and/or core.simd.

@denizzzka
Copy link
Contributor Author

This shouldn't be required (otherwise somebody would have surely complained already)

That's why I posted this. I think no one ever tried this

So I assume you have modified these files and/or core.simd.

Definitely not, these files are same as in ldc-developers/ldc upstream

@kinke
Copy link
Member

kinke commented Aug 11, 2024

That's why I posted this. I think no one ever tried this

There are various regular .d druntime and Phobos modules that import these intrinsics, llvmasm and simd .di headers, and we don't see any linker errors for the whole testsuite.

There used to be a frontend bug wrt. wrong module(info) dependency graphs, not sure whether that has been fixed or whether you just happen to trigger it with different import graphs.

@kinke
Copy link
Member

kinke commented Aug 11, 2024

If you post the exact linker error(s), we should at least see which ModuleInfo (wrongly) depends on one of these .di ModuleInfos.

@denizzzka
Copy link
Contributor Author

This is output when I try to link druntime.a with another D lib using Clang as linker.
It only appears if I try to link druntime with -Wl,--whole-archive option - I need it because I want to fill up ELF .init_array section by D modules ctors.
(And - yes, this is my fork of ldc version of druntime, but I think it doesn't matter in this case)

/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(bitop.o):(.data._D4core5bitop12__ModuleInfoZ+0x10): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(builtins.o):(.data._D4core8builtins12__ModuleInfoZ+0x10): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(checkedint.o):(.data._D4core10checkedint12__ModuleInfoZ+0x14): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(atomic.o):(.data._D4core8internal6atomic12__ModuleInfoZ+0x14): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(bits.o):(.data._D4core8internal2gc4bits12__ModuleInfoZ+0x24): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(gc.o):(.data._D4core8internal2gc4impl12conservativeQw12__ModuleInfoZ+0x64): more undefined references to `_D3ldc10intrinsics12__ModuleInfoZ' follow
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(simd.o):(.data._D4core4simd12__ModuleInfoZ+0x10): undefined reference to `_D3ldc4simd12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(common.o):(.data._D4core6thread6common12__ModuleInfoZ+0x1c): undefined reference to `_D3ldc7llvmasm12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(common.o):(.data._D4core6thread6common12__ModuleInfoZ+0x20): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(base.o):(.data._D4core6thread5fiber4base12__ModuleInfoZ+0x2c): undefined reference to `_D3ldc7llvmasm12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(arraycat.o):(.data._D2rt8arraycat12__ModuleInfoZ+0x18): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(critical_.o):(.data._D2rt9critical_12__ModuleInfoZ+0x18): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(dmain2.o):(.data._D2rt6dmain212__ModuleInfoZ+0x34): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(monitor_.o):(.data._D2rt8monitor_12__ModuleInfoZ+0x20): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(trace.o):(.data._D2rt5trace12__ModuleInfoZ+0x30): undefined reference to `_D3ldc10intrinsics12__ModuleInfoZ'
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(types.o):(.data._D4core6thread5types12__ModuleInfoZ+0x24): more undefined references to `_D3ldc10intrinsics12__ModuleInfoZ' follow
/home/denizzz/.espressif/tools/esp-clang/esp-17.0.1_20240419/esp-clang/bin/riscv32-esp-elf-clang-ld: /home/denizzz/Dev/dfruntime/install_freertos_riscv32/lib/libdruntime-ldc-debug.a(osthread.o):(.data._D4core6thread8osthread12__ModuleInfoZ+0x48): undefined reference to `_D3ldc7llvmasm12__ModuleInfoZ'
clang++: error: ld command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

With this patch these errors gone

@denizzzka
Copy link
Contributor Author

I need it because I want to fill up ELF .init_array section by D modules ctors.

This need of linking with a -Wl,--whole-archive was caused by the fact that I thoughtlessly added a shared ctor to rt.sections module. Now I removed it and this necessity disappeared.

But I not sure: ability to link whole druntime should be supported or not in common case?

@kinke
Copy link
Member

kinke commented Aug 12, 2024

But I not sure: ability to link whole druntime should be supported or not in common case?

This isn't the problem, it's the ModuleInfo dependencies graph in your case. E.g., rt.dmain2 is linked into every executable, as well as core.internal.gc.impl.conservative, but isn't a problem for upstream here.

@JohanEngelen
Copy link
Member

We had something similar at weka: #4442 . But there never was a good test case for it, the case is complex...

I agree with @kinke that renaming .di to .d is not the solution.

@denizzzka
Copy link
Contributor Author

E.g., rt.dmain2 is linked into every executable

I am worried about static libraries

Ok, there is no issues at the moment, so closing

@denizzzka denizzzka closed this Aug 12, 2024
@kinke
Copy link
Member

kinke commented Aug 12, 2024

If you could come up with a dustmited test case, e.g., by checking that the rt.dmain2 object file wrongly references the _D3ldc10intrinsics12__ModuleInfoZ symbol, then that would be greatly appreciated. This should be a duplicate of #4442 indeed (thx Johan for the ref), and a frontend problem (as the ModuleInfo deps are generated there).

Renaming to .d would just be a workaround, not a fix.

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.

3 participants