Extract common code in copyin/copyout to local routine

[mhorne063_gmail.com]

Summary

On RISC-V the copyin(9) and copyout(9) routines contain nearly identical logic, aside
from some argument checks. Move this code into a local copycommon routine and
have the existing routines jump to there.

[markj]

Elsewhere something similar is done with a macro, but I don't see much of a benefit either way.

sys/riscv/riscv/copyinout.S

67 ↗

(On Diff #52872)

The label can be renamed to "1".

[markj]

In D18850#402925, @markj wrote:

Elsewhere something similar is done with a macro, but I don't see much of a benefit either way.

Once these changes are in I think we'll want to modify copy{in,instr,out}() to save a copy of the frame pointer. In this case, we'd also have to modify copycommon() to do the same, so the use of a subroutine would be adding an extra pair of stack operations plus the unconditional jump. This makes me slightly inclined towards using a macro, but I'm not sure if anyone else finds it convincing. :)

[mhorne063_gmail.com]

In D18850#402944, @markj wrote:

In D18850#402925, @markj wrote:

Elsewhere something similar is done with a macro, but I don't see much of a benefit either way.

Once these changes are in I think we'll want to modify copy{in,instr,out}() to save a copy of the frame pointer. In this case, we'd also have to modify copycommon() to do the same, so the use of a subroutine would be adding an extra pair of stack operations plus the unconditional jump. This makes me slightly inclined towards using a macro, but I'm not sure if anyone else finds it convincing. :)

I get the feeling the performance differences would be too small to measure, but in general I'd take a few extra bytes of code over the extra instructions, so I can switch it to use the macro.
Only reason I can think of not to is if we expect copycommon to grow again substantially in the future.

EDIT: Or if having them call the same routine would benefit the cache, but again it is only a few bytes so it may not.

[mhorne063_gmail.com]

Convert copycommon to a macro.

[markj]

In D18850#402953, @mhorne063_gmail.com wrote:

In D18850#402944, @markj wrote:

In D18850#402925, @markj wrote:

Elsewhere something similar is done with a macro, but I don't see much of a benefit either way.

Once these changes are in I think we'll want to modify copy{in,instr,out}() to save a copy of the frame pointer. In this case, we'd also have to modify copycommon() to do the same, so the use of a subroutine would be adding an extra pair of stack operations plus the unconditional jump. This makes me slightly inclined towards using a macro, but I'm not sure if anyone else finds it convincing. :)

I get the feeling the performance differences would be too small to measure, but in general I'd take a few extra bytes of code over the extra instructions, so I can switch it to use the macro.
Only reason I can think of not to is if we expect copycommon to grow again substantially in the future.

Indeed, I doubt there is a measurable difference. To me it's more important that copyin()/copyout() be easily instrumentable since they show up a lot in profiles. With the subroutine, unwinding through copyin/out is a bit more involved.

EDIT: Or if having them call the same routine would benefit the cache, but again it is only a few bytes so it may not.

Right, with the subroutine we're saving a couple of i-cache lines at most.