amd64: partially depessimize cpu_fetch_syscall_args and cpu_set_syscall_retval
ClosedPublic
Actions

Authored by mjg on Oct 12 2018, 11:45 PM.

Details

Reviewers

Commits

rS339349: amd64: partially depessimize cpu_fetch_syscall_args and cpu_set_syscall_retval

Summary

cpu_fetch_syscall_args becoms jmp-free for the common case (modulo sv_mask which is going away soon). Bumps getuid rate on Broadwell from 105mln to 107.5mln.

This pessimizes 6 or more arg syscalls, which are a complete minority.

cpu_set_syscall_retval change removes 3 branches in favor of 1 and prevents reloads of ->td_frame.

Note the entire mechanism should be reworked - as it is there is a func call to amd64_syscall, then 3 indirect calls (fetch, syscall itself, set). The initial call can be indirect to a routine which does the fetch/set work and includes inlined amd64_syscall work.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mjg created this revision.Oct 12 2018, 11:45 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 12 2018, 11:45 PM

Harbormaster completed remote builds in B20172: Diff 49082.Oct 12 2018, 11:45 PM

kib added inline comments.Oct 13 2018, 12:09 AM

sys/amd64/amd64/trap.c
973–974	Does it need to be non-static ?
983–984	May be pass frame and sa as the arguments. You already calculated them in non-fallback version.
1002–1003	Change to nitems() ?
1046	Can you replace all three if()s with single or-ed condition and change goto to direct return(fallback()); statement ?
1050	Change to nitems() ?
1057	Again, can you replace this with return () ?
1060	Why 6 and not regcnt ? Or, do you need reg and regcnt in this function at all ?
1066	Then, remove the label.

also depessimize cpu_set_syscall_retval

mjg edited the summary of this revision. (Show Details)Oct 13 2018, 8:44 AM

Commit _retval() change now, I agree with it.

sys/amd64/amd64/trap.c
985	There regcnt is used as a constant. I suggest you to add a define to machine/proc.h e.g. NARGREGS 6, and use it consistently instead of the literal '6' or regcnt, in both functions.
1038	This and previous lines, () are excessive.
1052	argp is not needed, inline its initializator there.