Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Deprecate $v* and $fv* aliases in the ELF psABI docs. #27

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Nov 5, 2021

继续 #25 中的讨论之一。

cc @scylaac @ChenghuaXu @yetist @xry111

@xen0n xen0n force-pushed the psabi-deprecate-v-regs branch 2 times, most recently from 4658a5b to 73f90cf Compare November 5, 2021 09:34
@chenhuacai
Copy link

Nack,我支持用别名

@xen0n
Copy link
Contributor Author

xen0n commented Nov 8, 2021

Nack,我支持用别名

请给出清晰的理由。我在 PR 改动中叙述了我做此变更的原因,那么为了反驳,也应该详细列出原因,至少应当做到“如果将该等解释性文字加入文档,将打消读者关于 $vX 别名存在必要的疑虑”。我不认为这是可能的。

@xen0n xen0n mentioned this pull request Nov 8, 2021
@yetist
Copy link
Contributor

yetist commented Nov 9, 2021

我也认为v0、v1这种名字存在的必要性不大,反而容易给人带来困惑。

@xen0n
Copy link
Contributor Author

xen0n commented Nov 9, 2021

另一个不使用 v* 别名的理由:该别名无法 survive round-trip。用户会看到自己写的 v* 反汇编出来都是 a*,因此一定会形成二者严格等价,“写 v* 没用”的印象,因此从一开始就没必要分开。

寄存器 a0 a1 何时应该当作“入参”或“返回值”,是数据流决定的,意思是对于这俩寄存器的使用,当且仅当该使用发生在过程调用返回前的写入,与过程调用返回后对其内容的覆盖之间,名字才叫 v*。(有没有很绕?)显然这种分析几乎是不可能做到 100% 准确的,且需要一整套的数据流分析设施才能基本实现,得不偿失。目前能看到的 LA 反汇编工具没有一个做了这件事,都选择无条件返回 a*,这是具体实现人员的自然选择,也是必然选择。

最后个人经验,人脑做这个事情足够了,心智负担不高。(否则其他那些一开始就没有 v* 写法,还共享返回值寄存器的架构开发者都怎么活过来的)

的寄存器写法:这些名字分别等价于 `$a0` `$a1` `$fa0` `$fa1` 。
这些别名最初是为了兼容 MIPS 的分立传参、返回值寄存器写法而设计的。
由于 LoongArch 实际并没有专门的返回值寄存器,这种写法反而会造成误解,
因而不建议使用。
Copy link
Contributor

Choose a reason for hiding this comment

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

ABI文档是一个规范,解释性的东西是不是不需要写进去。再说了,上面都删了,还不建议使用,都没得用了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ABI文档是一个规范,解释性的东西是不是不需要写进去。

下面单独回复了。

再说了,上面都删了,还不建议使用,都没得用了。

上面那张表我理解只用来描述“标准形式”,但 $vX 是别名,所以叙述上我选择明确分隔开来。为了平滑支持已有的代码迁移,支持肯定不能直接去掉,只能通过规定具体编译器可以不实现,加之规定不建议下游使用,来倒逼下游改代码。这些话是写给未来的开发人员看的,不是那些已经用了 $vX 的过去的开发人员,所以理解成“没得用”其实就符合制定规范的预期了;你读到这种写法知道是啥意思就行了,历史原因,自己不要用。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

至于提了一嘴 MIPS,如果这是个问题的话,去掉也没关系,反正开发者的眼睛是雪亮的,这个信息一定会在坊间被补全。因为近年的主流架构唯一一个有 $vX 还跟龙芯有联系的就它了 😂

Copy link
Contributor

Choose a reason for hiding this comment

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

我建议这里的语句再斟酌一下,英文版本同样。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我建议这里的语句再斟酌一下,英文版本同样。

往哪个方向调整?

Copy link
Contributor

Choose a reason for hiding this comment

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

兼容显然谈不上,比如换成参考/仿照这类说法?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

兼容显然谈不上,比如换成参考/仿照这类说法?

可以

Copy link
Contributor Author

Choose a reason for hiding this comment

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

英文版的相应位置没有使用“兼容”的写法,当时我用了 match 一词表示“匹配/对应”,感觉还算合理,因此这版没改。各位有意见吗?

Copy link
Contributor

Choose a reason for hiding this comment

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

建议改为“在一些早期的 LoongArch 汇编代码中,您可能会见到形如 $v0 $v1 $fv0 $fv1
的寄存器写法:这些名字分别等价于 $a0 $a1 $fa0 $fa1
由于 LoongArch 实际并没有专门的返回值寄存器,这种写法反而会造成误解,
因而不建议使用,工具链已有代码继续支持一个版本,下个版本将通过警告提醒用户替换,下下个版本会删除形如$v0 $v1 $fv0 $fv1的写法。” //指代、措辞和表达可做调整,大体思路这样即可。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChenghuaXu Done, plz review.

@ChenghuaXu
Copy link
Contributor

如果目前能够解决已有代码中的写法,我也觉得可以删掉了,确实有点多余了。看起来目前最好的办法是加注释,建议别用,直接删掉的话,已有代码会有问题。

@xen0n
Copy link
Contributor Author

xen0n commented Nov 9, 2021

如果目前能够解决已有代码中的写法,我也觉得可以删掉了,确实有点多余了。看起来目前最好的办法是加注释,建议别用,直接删掉的话,已有代码会有问题。

实现上的策略一定是(1)具体编译器实现中继续支持该特性但不写相应文档(面向用户),(2)规范里写明该特性的存在原因和 deprecate 原因(面向工具链开发者),(3)后续渐渐用 warning 等形式 phase out 下游使用。这也是为何我们要写明 deprecate 理由,因为规范归规范,也是给人看的,编译器开发者难免要知道一些背景知识才能做好工作。

@xen0n xen0n force-pushed the psabi-deprecate-v-regs branch from 73f90cf to 4ff46e3 Compare November 10, 2021 03:44
Copy link
Contributor

@yetist yetist left a comment

Choose a reason for hiding this comment

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

我感觉没啥问题了

@xen0n xen0n force-pushed the psabi-deprecate-v-regs branch 2 times, most recently from b05f5f3 to 8345477 Compare November 16, 2021 05:30
@xen0n
Copy link
Contributor Author

xen0n commented Nov 16, 2021

又改了一下,在表格里写清楚了 a0/a1/fa0/fa1 既是传参寄存器也是返回值寄存器。

@xen0n xen0n force-pushed the psabi-deprecate-v-regs branch from 8345477 to 8a12982 Compare November 16, 2021 10:56
@xen0n
Copy link
Contributor Author

xen0n commented Nov 16, 2021

Rebased

@yetist
Copy link
Contributor

yetist commented Nov 18, 2021

@xen0n @ChenghuaXu

这个 PR 现在怎么样了?

@xen0n
Copy link
Contributor Author

xen0n commented Nov 18, 2021

@yetist Waiting for review

@xen0n
Copy link
Contributor Author

xen0n commented Nov 19, 2021

ping @scylaac @ChenghuaXu

@ChenghuaXu ChenghuaXu self-requested a review November 24, 2021 06:59
@xen0n
Copy link
Contributor Author

xen0n commented Nov 25, 2021

现在还差谁再同意就可以合并了?

@scylaac @FreeFlyingSheep @sterling-teng Thoughts?

@yetist yetist requested a review from a team December 1, 2021 01:47
@xen0n
Copy link
Contributor Author

xen0n commented Dec 1, 2021

Ping for status?

@yetist
Copy link
Contributor

yetist commented Dec 1, 2021

之前参与过本 PR 讨论的同学,请尽快给出明确意见,别一直拖着。

@xry111
Copy link
Contributor

xry111 commented Dec 1, 2021

我是同意的,但是我并没有 write access :)

@FreeFlyingSheep
Copy link
Collaborator

我是同意的,但是我并没有 write access :)

Approve 应该不需要 write access,点击 Files changed 页面的 Review changes,里面可以选择。

@yetist
Copy link
Contributor

yetist commented Dec 2, 2021

从最后一个 Approve 至今已过去 8 天,且无人再提出异议,原则上认为此 PR 大家已经达成共识,可以合并了。

@FreeFlyingSheep FreeFlyingSheep merged commit 2f568d8 into loongson:main Dec 2, 2021
@xen0n xen0n deleted the psabi-deprecate-v-regs branch December 2, 2021 02:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants