Kim, You are right. Here is an updated webrev:
http://cr.openjdk.java.net/~avoitylov/webrev.8231612.05/ -Aleksei On 11/12/2019 03:03, Kim Barrett wrote: >> On Dec 10, 2019, at 9:05 AM, Aleksei Voitylov <aleksei.voity...@bell-sw.com> >> wrote: >> >> Kim, >> >> thank you for the suggestions. Here is an updated webrev: >> >> http://cr.openjdk.java.net/~avoitylov/webrev.8231612.04/ >> >> The assembly generated with inline functions is equivalent to that with >> the macros, and the testing passed as well. > This is mostly good. > > However, the new function names don't follow usual HotSpot style, > which is lower case with underscore word separators. (Yes, there > are *many* counterexamples.) > > Also, the minimal parenthesizing in SetByteInInt requires one to think > about operator precedence order more than I would like. What's there > is correct, but would be easier to read as > > 855 return (n & ~(0xff << bitsIdx)) | (b << bitsIdx); > > (We have some functions in globalDefinitions.hpp to supposedly help > with these kinds of calculations, but the supported types are often > wrong (as here), and there are missing operations (again, as here).) >
