Page MenuHomeFreeBSD

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

Authored by avg on Jun 16 2015, 2:14 PM.

Details

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
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.
This revision was automatically updated to reflect the committed changes.