RFR: 8201543: Modularize C1 GC barriers (original) (raw)

Erik Österlund erik.osterlund at oracle.com
Thu Apr 26 12:33:46 UTC 2018

Hi Per,

Thank you for the review. I have incorporated your suggested improvements.

/Erik

On 2018-04-26 12:44, Per Liden wrote:

On 04/26/2018 12:38 PM, Per Liden wrote: [...]

Webrev: http://cr.openjdk.java.net/~eosterlund/8201543/webrev.00/ Looks good overall, just some minor comments. src/hotspot/cpu/x86/gc/g1/g1BarrierSetAssemblerx86.hpp ------------------------------------------------------- 59 void geng1prebarrierstub(LIRAssembler* ce, G1PreBarrierStub* stub); 60 void geng1postbarrierstub(LIRAssembler* ce, G1PostBarrierStub* stub); It seems superfluous to have "g1" in the function names given the context ;) src/hotspot/cpu/x86/gc/g1/g1BarrierSetAssemblerx86.cpp ------------------------------------------------------- The C1-related includes and functions should be under #ifdef COMPILER1. src/hotspot/share/gc/shared/c1/barrierSetC1.hpp ----------------------------------------------- 96 CodeEmitInfo*& patchinfo() { return patchinfo; } 97 CodeEmitInfo*& accessemitinfo() { return accessemitinfo; } Should we make this "patchinfo" and "accessinfo"? Or maybe "patchemitinfo" and "accessemitinfo"? I think I prefer the former. 122 virtual LIROpr atomiccmpxchgresolved(LIRAccess& access, LIRItem& cmpvalue, LIRItem& newvalue); 123 124 virtual LIROpr atomicxchgresolved(LIRAccess& access, LIRItem& value); ... 133 virtual LIROpr atomicxchg(LIRAccess& access, LIRItem& value); The methods above are missing an "at" in their name. src/hotspot/share/gc/shared/c1/barrierSetC1.cpp ----------------------------------------------- 73 void BarrierSetC1::storeat(LIRAccess& access,LIROpr value) { Missing space in argument list. src/hotspot/share/runtime/sharedRuntime.cpp ------------------------------------------- The only change in this file is a single (unnecessary) line delete. Btw, I don't need to see a new webrev. /Per

More information about the hotspot-dev mailing list