Hi Alice, >> x18 should be call-preserved too (we have no reason not to). > > This specification comes directly from the (Beta) ABI spec: > https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#815__arm_get_current_vg
x18 is special - basically it can be a global register, a caller-save or a callee-save depending on the platform ABI. Hence saying it is one of those or marking it as corrupted after calls is incorrect. This is easily fixed with a separate patch. > Both here and in patch 2/2 I've followed the approach already in use for > __arm_sme_state, so your comments may be equally applicable there. When > Szabolcs > added __arm_sme_state, he wrote (in the commit message) that: > > The routines are in shared libgcc and static libgcc eh, even though > they are not related to exception handling. This is to avoid linking > a copy of the routines into dynamic linked binaries, because TPIDR2_EL0 > block can be extended in the future which is better to handle in a > single place per process. This is very hard to parse... I guess it says that the functions are statically linked into the binary but dynamically linked in a .so. So it's still called via PLT, hence the variant_pcs is essential for correctness. >> This is OK with a comment here that variant_pcs ensures the PLT gets >> resolved at load-time so that the vector registers are corrupted... > > If you think this comment would be helpful, perhaps it would be worth adding > it > to the all of the affected files (as a separate patch, of course)? Yes, including clarifying that it is dynamically linked only in .so's. The patch is OK as is since it doesn't create new issues. Cheers, Wilco
