lib/csu: add LoongArch support
Needs ReviewPublic
Actions

Authored by zhaoxiaoqiang007_gmail.com on Tue, Sep 22, 2:23 PM.

Details

Reviewers

kib
imp
dchagin

Summary

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>

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77172
Build 74055: arc lint + arc unit

Event Timeline

zhaoxiaoqiang007_gmail.com created this revision.Tue, Sep 22, 2:23 PM

Herald added a subscriber: imp. · View Herald TranscriptTue, Sep 22, 2:23 PM

zhaoxiaoqiang007_gmail.com requested review of this revision.Tue, Sep 22, 2:23 PM

Harbormaster completed remote builds in B77172: Diff 187410.Tue, Sep 22, 2:23 PM

zhaoxiaoqiang007_gmail.com added a parent revision: D59891: build: add LoongArch build infrastructure.Tue, Sep 22, 2:23 PM

zhaoxiaoqiang007_gmail.com added a child revision: D59893: libc: add LoongArch support.

zishun.yi.dev_gmail.com added a subscriber: zishun.yi.dev_gmail.com.Tue, Sep 22, 2:41 PM

gehaowu_bitmoe.com added a subscriber: gehaowu_bitmoe.com.Tue, Sep 22, 8:47 PM

zhaoxiaoqiang007_gmail.com added reviewers: kib, imp, dchagin.Wed, Sep 23, 8:11 AM

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.

In D59892#1375242, @brooks wrote:

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.

shipujin.t_gmail.com added a subscriber: shipujin.t_gmail.com.Thu, Sep 24, 9:07 AM

shipujin.t_gmail.com added inline comments.

lib/csu/loongarch/crtn.S
2	`__FBSDID("$Free$")` is the old style and is suggested to be removed.
15	This works functionally, but for consistency with the code style of the other architectures, it is suggested to change it to: ret
17	This looks like an FPU ABI flag copied from MIPS, and LoongArch does not need it. Suggested change to: .section .fini,"ax",@progbits

shipujin.t_gmail.com added inline comments.Thu, Sep 24, 9:31 AM

lib/csu/loongarch/crt1_s.S
49–50	It can be changed more concisely: bl __start

zhaoxiaoqiang007_gmail.com added inline comments.Thu, Sep 24, 2:11 PM

lib/csu/loongarch/crt1_s.S
49–50	yes
lib/csu/loongarch/crtn.S
2	understand
15	looks better
17	thx

In D59892#1375242, @brooks wrote:

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

More copyright tagging advice.

lib/csu/loongarch/Makefile
1	this line, and the following blank link, need to go.
lib/csu/loongarch/crt.h
2	This should have a new copyright, as described elsewhere. This will also solve the stray $FreeBSD$ issue too.
lib/csu/loongarch/crt1_s.S
3	This is a whole new file, so should have the new, short-form copyright
lib/csu/loongarch/crti.S
1	This file needs a copyright.
2	this needs to be deleted.
lib/csu/loongarch/crtn.S
1	This file needs a copyright.
lib/libc/csu/loongarch/Makefile.inc
1	I'd be tempted to delete the first two lines.
lib/libc/csu/loongarch/reloc.c
2	This is effectively a whole new file, so needs a copy short-form copyright.