classpath-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [cp-patches] RFC: patch for Unicode scalar value to UTF-16conversion


From: Jeroen Frijters
Subject: RE: [cp-patches] RFC: patch for Unicode scalar value to UTF-16conversion
Date: Sun, 8 Jan 2006 12:47:21 +0100

Chris Burdess wrote:
> > > > I also have a mild preference for the bit-shifting and masking
> > > > computation, but this is more minor.
> > > 
> > > The problem is that the bit-shifting method produces the 
> wrong result.
> > 
> > In this case they both produce the same result. I think the 
> problem was
> > that the original code forgot to subtract 0x10000.
> 
> I see. So, is there a good reason to do it this way (bitshifting and
> bitmasking) rather than the arithmetical methods (which presumably get
> compiled to the same thing, but are much more straightforward to
> understand)?

It's a matter of personal preference. The masking and shifting may be a
tiny bit more efficient, but that's hardly significant. I think Tom's
point was (and I agree) that it is a little more common to do these
types of operations using the masking/shifting and that makes it a
little easier to read, but like I said, it all depends on what you're a
used to.

> Should I commit this? Or should it be bitshifting and bitmasking?

Please do. The current code is broken, so it is clearly an improvement.
Once the code is correct we can argue about micro optimizations ;-)

BTW, if you want to understand why shifting is a little more efficient,
take a look at the attached C++ code and resulting assembly. You'll see
that the compiler is clever enough to compile the multiplication and
modulo operations into bit shifting and masking, but it still takes a
little more work because it needs to explicitly handle the case where
the codePoint value is negative (which can't happen in this case, but
the compiler (or JIT) doesn't know that).

Regards,
Jeroen

Attachment: test.cpp
Description: test.cpp

Attachment: test.asm
Description: test.asm


reply via email to

[Prev in Thread] Current Thread [Next in Thread]