Page MenuHomeFreeBSD
Differential D2839

illums compat: use flsl/flsll for highbit/highbit64
ClosedPublic
Actions

Authored by avg on Jun 16 2015, 2:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jan 9, 7:14 PM
Unknown Object (File)
Dec 18 2024, 5:08 AM
Unknown Object (File)
Nov 26 2024, 1:42 PM
Unknown Object (File)
Oct 23 2024, 8:48 AM
Unknown Object (File)
Oct 5 2024, 9:56 PM
Unknown Object (File)
Oct 3 2024, 2:05 AM
Unknown Object (File)
Oct 2 2024, 9:21 PM
Unknown Object (File)
Sep 25 2024, 12:45 PM
View All Files
Subscribers
None

Details

Reviewers
will
gibbs
delphij
mahrens
mav
smh
Commits
rS284591: illums compat: use flsl/flsll for highbit/highbit64
rS284511: illumos compat: use flsl/flsll for highbit/highbit64
Summary

This is a micro optimization.
The upstream code uses the binary search.

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

avg updated this revision to Diff 6236.Jun 16 2015, 2:14 PM
avg retitled this revision from to illums compat: use flsl/flsll for highbit/highbit64.
avg updated this object.
avg edited the test plan for this revision. (Show Details)
avg added reviewers: smh, mav, will, gibbs, delphij.
delphij accepted this revision.Jun 16 2015, 9:29 PM
delphij edited edge metadata.

Looks good to me.

This revision is now accepted and ready to land.Jun 16 2015, 9:29 PM
mav accepted this revision.Jun 17 2015, 1:11 AM
mav edited edge metadata.

It would be good to upstream it. I don't know why it was implemented originally, but it seems that at least on user level Illumos has fsll() and co.

avg added a reviewer: mahrens.Jun 17 2015, 7:12 AM
Closed by commit rS284511: illumos compat: use flsl/flsll for highbit/highbit64 (authored by avg). · Explain WhyJun 17 2015, 12:05 PM
This revision was automatically updated to reflect the committed changes.
avg reopened this revision.Jun 17 2015, 7:52 PM

This was backed out because of build problems on platforms other than amd64.

This revision is now accepted and ready to land.Jun 17 2015, 7:52 PM
avg updated this revision to Diff 6285.Jun 18 2015, 8:32 AM
avg edited edge metadata.

highbit: use only fast flsl/flsll versions

Currently libkern flsl/flsll are even less optimized than the original
code. The same goes for libc implementations on most platforms.

This revision now requires review to proceed.Jun 18 2015, 8:32 AM
avg added a comment.Jun 18 2015, 11:33 AM

Could you please re-review?
I believe that the latest diff should be safe on all platforms.

delphij accepted this revision.Jun 18 2015, 6:57 PM
delphij edited edge metadata.
This revision is now accepted and ready to land.Jun 18 2015, 6:57 PM
mahrens accepted this revision.Jun 18 2015, 7:53 PM
mahrens edited edge metadata.

Looks fine to me. FWIW, illumos appears to have flsll() in userland but not kernel. It's implemented similarly to highbit().

mav added a comment.Jun 18 2015, 10:44 PM

Checking HAVE_INLINE_* define is what I am successfully using in SCHED_ULE for some time now, so it should work. Though I hope somebody implement missing functions on other platforms to reach the parity and avoid this mess.

mav accepted this revision.Jun 18 2015, 10:44 PM
mav edited edge metadata.
Closed by commit rS284591: illums compat: use flsl/flsll for highbit/highbit64 (authored by avg). · Explain WhyJun 19 2015, 6:41 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
cddl/
contrib/
opensolaris/
uts/
common/
sys/
15 lines

Diff 6236

View Options

sys/cddl/contrib/opensolaris/uts/common/sys/sysmacros.h

Loading...