crt1/crti/crtn and self-relocation for LoongArch.
Signed-off-by: Xiaoqiang Zhao <zhaoxiaoqiang007@gmail.com>
Signed-off-by: yu shan wei <mpysw@vip.163.com>
Details
Details
- Reviewers
kib imp dchagin shipujin.t_gmail.com
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 78871 Build 75754: arc lint + arc unit
Event Timeline
Comment Actions
A few high-level comments.
These files contain $FreeBSD$ which should is long obsolete and should never be present.
As with @imp's comments on D59893, the bringing over the copyrights and acknowledgements is questionable.
Please don't implement crti and crtn. The _init and _fini functions are obsolete and should not be supported in new architectures. IMO it was an error to have added support to RISC-V.
| lib/csu/loongarch/crtn.S | ||
|---|---|---|
| 2 ↗ | (On Diff #187410) | __FBSDID("$Free$") is the old style and is suggested to be removed. |
| 15 ↗ | (On Diff #187410) | This works functionally, but for consistency with the code style of the other architectures, it is suggested to change it to: ret |
| 17 ↗ | (On Diff #187410) | This looks like an FPU ABI flag copied from MIPS, and LoongArch does not need it. Suggested change to: .section .fini,"ax",@progbits |
| lib/csu/loongarch/crt1_s.S | ||
|---|---|---|
| 50–51 | It can be changed more concisely: bl __start | |
Comment Actions
Please don't implement crti and crtn. The _init and _fini functions are obsolete and should not be supported in new architectures. IMO it was an error to have added support to RISC-V.
Is it appropriate to change the clang FreeBSD driver to reflect this ?
I have opened a PR for this： https://github.com/llvm/llvm-project/pull/228689
Comment Actions
More copyright tagging advice.
| lib/csu/loongarch/Makefile | ||
|---|---|---|
| 2 | this line, and the following blank link, need to go. | |
| lib/csu/loongarch/crt.h | ||
| 3 | This should have a new copyright, as described elsewhere. This will also solve the stray $FreeBSD$ issue too. | |
| lib/csu/loongarch/crt1_s.S | ||
| 4 | This is a whole new file, so should have the new, short-form copyright | |
| lib/csu/loongarch/crti.S | ||
| 1 ↗ | (On Diff #187410) | This file needs a copyright. |
| 2 ↗ | (On Diff #187410) | this needs to be deleted. |
| lib/csu/loongarch/crtn.S | ||
| 1 ↗ | (On Diff #187410) | This file needs a copyright. |
| lib/libc/csu/loongarch/Makefile.inc | ||
| 2 | I'd be tempted to delete the first two lines. | |
| lib/libc/csu/loongarch/reloc.c | ||
| 3 | This is effectively a whole new file, so needs a copy short-form copyright. | |
Comment Actions
since clang driver changes has been merged, all changes with be there in next reversion
Comment Actions
update to resolve comments since last reversion
- rebase main
- fix code style issue
- remove crti and crtn
- add missing copyright lines
- misc suggestions from shipujin.t_gmail.com