<Esmil>
drewfustini: so what are you trying to achieve with those patches? 1) just a performance enhancement where the kernel is allowed to clobber the vector registers on syscalls, but won't spend time doing on every syscall. Or 2) let userspace opt into a new ABI where the kernel is never allowed to clobber the vector registers?
<drewfustini>
The motivation was that always cloberring the vector registers on syscalls increased the duration of a syscall by 25%
<drewfustini>
on the x280 cores
<Esmil>
so that is 1), right?
<drewfustini>
When that change was made 2 years ago, the idea was that it would be revisted once there was performance data
<drewfustini>
Well, that the kernel will not do that current behavior where it always clobbers the vector registers in the syscall path
<Esmil>
Right, but there is a difference between not clobbering them for performance (1) or promising userspace that they will never be clobbered (2)
<drewfustini>
Right now, do_trap_ecall_u() always calls riscv_v_vstate_discard(regs) which clobbers the vector registers
<Esmil>
..and my reading of the comments is that it's unclear if you're trying to achieve 1 or 2
<drewfustini>
It seems unnecessary for the kernel to have this strict clobbering behavior when it does impact performance on some cores. There are other cases in which the vector state is managed like signals. There was once a bug in that: https://lore.kernel.org/linux-riscv/20240403072638.567446-1-bjorn@kernel.org/
<drewfustini>
I think signal handling is different than this strict clobbering in the do_trap_ecall_u() path
<Esmil>
Right. So that sounds like you're trying to achieve 1
<drewfustini>
The simplest thing would be to revert that patch that added riscv_v_vstate_discard(). But Palmer wants to keep that strict behavior for testing at least
<Esmil>
So then it's important to explicitly say in the documentation that the kernel doesn't promise to leave the vector registers intact. Otherwise you're accidentally creating a new ABI variant
<drewfustini>
Okay, good point.
<drewfustini>
Maybe something like this for /proc/sys/abi/riscv_v_vstate_discard ?
<drewfustini>
0: Vector state is not always clobbered in all syscalls
<drewfustini>
1: Mandatory clobbering of vector state in all syscalls
<Esmil>
yeah, that makes it more clear to me
<drewfustini>
Ok, thanks, yeah, I will improve the description in the patch
avg_surfman has joined #riscv
tlwoerner has quit [Ping timeout: 260 seconds]
<dramforever[m]1>
so the thing i was talking about in the thread is whether restoring vector registers is a performance regression for programs that sporadically uses vector instructions
<dramforever[m]1>
although, if we are maintaining in the abi that restoring is not guaranteed, this can be a future non-breaking fix
BootLayer has joined #riscv
vagrantc has joined #riscv
<drewfustini>
I think the only time vector registers are restored is in the case of signals and when the kernel uses the vector unit.
<drewfustini>
I was just trying to allow the choice of going back to the original behavior in do_trap_ecall_u() where the vector registers were not always clobbered
Andre_Z has quit [Quit: Leaving.]
<dramforever[m]1>
wait, not always?
avg_surfman has quit [Ping timeout: 252 seconds]
<drewfustini>
before Bjorn's patch 2 years, do_trap_ecall_u() did not do anything regarding vector registers
<drewfustini>
Bjorn's patch made do_trap_ecall_u() always call riscv_v_vstate_discard()
<drewfustini>
before that, do_trap_ecall_u() didn't touch vector registers
<dramforever[m]1>
so, what i am curious about is whether that could be a performance optimization for syscall heavy programs
<drewfustini>
I believe that the cases where vector registers were touched were in signal handler and if the kernel used the vector unit
<dramforever[m]1>
oh, if we also change it so we don't clobber it when we know it's clobbered
<drewfustini>
like it's a no-op if the kernel knows it was already clobbered in the previous syscall?
<dramforever[m]1>
yeah, like if VS = Initial
<dramforever[m]1>
well, not restoring if it hasn't changed since last time would also help one thread, but i was imagining if we switch to a thread with VS = Initial we can avoid some memory traffic
<dramforever[m]1>
... as well
<dramforever[m]1>
i think that's what i said in the thread
<drewfustini>
Yeah, I think so. You wrote: if (VS == Clean || VS == Dirty) { clobber; VS = Initial; }
<drewfustini>
so once VS=Initial, it would not go down the clobber path
<dramforever[m]1>
there will need to be handling in the thread switching case
<dramforever[m]1>
i forgot about that in my email
<dramforever[m]1>
but yes that's the idea
<drewfustini>
in __switch_to_vector(), right?
<dramforever[m]1>
proabbly
tlwoerner has joined #riscv
avg_surfman has joined #riscv
avg_surfman has quit [Ping timeout: 272 seconds]
ruidx has quit [Quit: WeeChat 3.8]
psydroid2 has joined #riscv
LainIwakura has quit [Ping timeout: 272 seconds]
coldfeet has joined #riscv
LainIwakura has joined #riscv
ruidx has joined #riscv
___nick___ has joined #riscv
jacklsw has joined #riscv
LainIwakura has quit [Ping timeout: 272 seconds]
jobol has quit [Quit: Leaving]
Noisytoot has quit [Remote host closed the connection]
LainIwakura has joined #riscv
jacklsw has quit [Ping timeout: 252 seconds]
LainIwakura has quit [Quit: Client closed]
BootLayer has quit [Quit: Leaving]
LainIwakura has joined #riscv
LainIwakura has quit [Client Quit]
ruidx has quit [Quit: WeeChat 3.8]
undermine has quit [Quit: Bye]
Andre_Z has joined #riscv
Noisytoot has joined #riscv
Noisytoot has quit [Remote host closed the connection]
Noisytoot has joined #riscv
coldfeet has quit [Quit: Lost terminal]
___nick___ has quit [Ping timeout: 276 seconds]
alperak has quit [Quit: Connection closed for inactivity]