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

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #49082)	Does it need to be non-static ?
985 ↗	(On Diff #49082)	May be pass frame and sa as the arguments. You already calculated them in non-fallback version.
1005 ↗	(On Diff #49082)	Change to nitems() ?
1049 ↗	(On Diff #49082)	Can you replace all three if()s with single or-ed condition and change goto to direct return(fallback()); statement ?
1053 ↗	(On Diff #49082)	Change to nitems() ?
1060 ↗	(On Diff #49082)	Again, can you replace this with return () ?
1063 ↗	(On Diff #49082)	Why 6 and not regcnt ? Or, do you need reg and regcnt in this function at all ?
1069 ↗	(On Diff #49082)	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 ↗	(On Diff #49089)	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 ↗	(On Diff #49089)	This and previous lines, () are excessive.
1052 ↗	(On Diff #49089)	argp is not needed, inline its initializator there.