search

riscv-non-isa / riscv-asm-manual

RISC-V Assembly Programmer's Manual
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
1.41k stars 235 forks source link

Change scratch register of tail to t2 when Zicfilp enabled. #93

Closed yetingk closed 1 month ago

yetingk commented 8 months ago

This change is to make tail conform with software guarded jump of Zicfilp. The reason to not choose t1 as the label register is that t1 is also as .got.plt offset of _dl_runtime_resolve in PLT.

kito-cheng commented 8 months ago

cc @ved-rivos @jrtc27 @asb

kito-cheng commented 8 months ago

@aswaterman The reason I suggest keep both is I don't want make older tool become non-conformance with this asm manual, but maybe we could using different way/wording to prevent that?

jrtc27 commented 8 months ago

I still believe the extension should have been defined in a way that doesn't break existing practices, and it's pretty weird to have tail change which register it uses based on what extensions you happen to have enabled. That sounds like asking for trouble if people are assuming the register that tail clobbers (thinking about custom calling conventions when branching to a non-preemptible symbol), too, given it has been explicitly specified as being t1...

jrtc27 commented 8 months ago

Without thinking through all the implications, my instincts are that it is much better to change the PLT implementation (which is technically also specified today, but really shouldn't be, the code should just be an example of a possible implementation, as application code shouldn't know/care about it) than to change the semantics of pseudo-instructions.

ved-rivos commented 8 months ago

Looks good to me.

yetingk commented 8 months ago

Whatever change is made here should also be reflected in the table.

Done.

kito-cheng commented 4 months ago

NOTE: public review period for zicfiss and zicfilp are ended, waiting for ratification flow and then we will merge this.

cmuellner commented 1 month ago

Zicfiss, Zicfilp were ratified in June.

yetingk commented 1 month ago

Rebased to fix conflicts.