Specifically, these: clearBit64 clearBits64WithMask countTrailingZeros64WithoutNullCheck clearBit64 provides a faster way to clear a bit without having to a shift to make a mask first. clearBits64WithMask does the equivalent of and64 with the 1's complement of the provided mask. countTrailingZeros64WithoutNullCheck does the same thing as countTrailingZeros64, except that it assumes that the word in the register it is processing will never be null, and therefore skips the null check. This is useful in code that already does a null check ahead of time. So, there's no need to do a redundant null check.
(In reply to Mark Lam from comment #0) > clearBit64 provides a faster way to clear a bit without having to a shift to > make a mask first. Correction: clearBit64 will emit the shift to make the mask.
Created attachment 400277 [details] proposed patch.
<rdar://problem/63642626>
Comment on attachment 400277 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=400277&action=review > Source/JavaScriptCore/ChangeLog:16 > + clearBits64WithMask does the equivalent of and64 with the 1's complement of the > + provided mask. feels weird to call this a "mask" when we're really using ~mask
Comment on attachment 400277 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=400277&action=review >> Source/JavaScriptCore/ChangeLog:16 >> + provided mask. > > feels weird to call this a "mask" when we're really using ~mask It is a mask: one that defines the bits to clear instead of the bits to keep (as is the case with and64). I'm open to an alternate term if you can think of anything better.
Comment on attachment 400277 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=400277&action=review r=me > Source/JavaScriptCore/ChangeLog:22 > + redundant null check. Do we have any way of easily inserting an assert in the generated code when in debug? It could be useful for these kind of things. > Source/JavaScriptCore/assembler/testmasm.cpp:474 > + auto test = compile([=] (CCallHelpers& jit) { what are you capturing by copy and why?
Comment on attachment 400277 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=400277&action=review >> Source/JavaScriptCore/ChangeLog:22 >> + redundant null check. > > Do we have any way of easily inserting an assert in the generated code when in debug? It could be useful for these kind of things. Hmmm, I'll see if it's easy to do. If not, I'll do it in another patch. >> Source/JavaScriptCore/assembler/testmasm.cpp:474 >> + auto test = compile([=] (CCallHelpers& jit) { > > what are you capturing by copy and why? Copy paste error. I'll remove the '='. > Source/JavaScriptCore/assembler/testmasm.cpp:501 > + auto test = compile([=] (CCallHelpers& jit) { Ditto. > Source/JavaScriptCore/assembler/testmasm.cpp:555 > + auto test = compile([=] (CCallHelpers& jit) { Ditto.
(In reply to Mark Lam from comment #7) > >> Source/JavaScriptCore/ChangeLog:22 > >> + redundant null check. > > > > Do we have any way of easily inserting an assert in the generated code when in debug? It could be useful for these kind of things. > > Hmmm, I'll see if it's easy to do. If not, I'll do it in another patch. I've added an assertion check.
Created attachment 400284 [details] patch for landing. Thanks for the review. Here's the updated patch for landing.
(In reply to Mark Lam from comment #8) > (In reply to Mark Lam from comment #7) > > >> Source/JavaScriptCore/ChangeLog:22 > > >> + redundant null check. > > > > > > Do we have any way of easily inserting an assert in the generated code when in debug? It could be useful for these kind of things. > > > > Hmmm, I'll see if it's easy to do. If not, I'll do it in another patch. > > I've added an assertion check. Great, thanks!
Created attachment 400285 [details] patch for landing. Also added the assertion to MacroAssemblerARM64 as well. ARM64 doesn't really need the check because it's implementation will always have correctly even if the word is 0. However, it's good to have the assertion consistently to help catch logic errors in the client that break this invariant.
Landed in r262168: <http://trac.webkit.org/r262168>.