Hi, On Wed, 2018-01-31 at 09:49 +0900, Yasumasa Suenaga wrote: > Hi Thomas, > > > > looks good to me - however there is another (pre-existing) bug: > > > the > > > shift in that code should be a logical shift, not an arithmetic > > > shift. > > > > > > I.e. ">>" instead of ">>>". > > > > > > I will run the patch through testing and report back in a few > > > hours. > > > Should be okay. > > > > is good. Do you want to fix the issue with the shift operator too > > here, or use another CR? > > Thanks! > > If the use of ">>>" is the bug, I want to fix it in new bug ticket. > I do not think the use of ">>>" is not a bug. > > I g1BiasedArray.hpp, G1BiasedMappedArray::get_by_address() uses ">>" > operator to calculate biased_index: > http://hg.openjdk.java.net/jdk/hs/file/ee513596f3ee/src/hotspot/sha > re/gc/g1/g1BiasedArray.hpp#l134 > > idx_t is defined as size_t. So it is calculated as unsigned value. > In JLS 15.19, ">>>" is for unsigned. If we use ">>", it might remain > MSB in some cases. > https://docs.oracle.com/javase/specs/jls/se9/html/jls-15.html#jls-1 > 5.19 > > Thus I think this is not a bug.
You are right, I mixed them up. I will push the change then, with jgeorge and me as reviewers. Thomas
