Handle monitor enter/exit on value based instances in Z #11677

a7ehuo · 2021-01-18T15:06:24Z

If the monitor object type is value based class, VM will either issue a warning or throw an exception based on -XX:ValueBasedClassCheck option.

If the monitor object type is unkonwn, insert a runtime memory check in monitor enter/exit on the class flag J9_CLASS_DISALLOWS_LOCKING_FLAGS.

If the monitor object type is not value based class, proceed as how it is handled today.

Related to JEP390

Signed-off-by: Annabelle Huo Annabelle.Huo@ibm.com

fjeremic

@r30shah could you please review given your expertise in the area?

runtime/compiler/z/codegen/J9TreeEvaluator.cpp

fjeremic · 2021-01-18T17:07:25Z

I'd also like to see a better commit comment than we have currently. We should describe roughly the change being made and why it is being made.

a7ehuo · 2021-01-18T18:13:38Z

I'd also like to see a better commit comment than we have currently. We should describe roughly the change being made and why it is being made.

The PR is currently in a draft state. I'll definitely add detailed description on this change in next commit along with other changes required.

runtime/compiler/z/codegen/J9TreeEvaluator.cpp

a7ehuo · 2021-01-20T15:45:34Z

@r30shah @fjeremic All comments addressed. Ready for another review. Thanks!

r30shah

Minor nitpick, changes looks Good to me. Also through chat, @a7ehuo has provided a sequence of instruction generated for the node from the log file and it seems ok.

runtime/compiler/z/codegen/J9TreeEvaluator.cpp

If the monitor object type is value based class, VM will either issue a warning or throw an exception based on -XX:ValueBasedClassCheck option. If the monitor object type is unkonwn, insert a runtime memory check in monitor enter/exit on the class flag J9_CLASS_DISALLOWS_LOCKING_FLAGS. If the monitor object type is not value based class, proceed as how it is handled today. Related to JEP390 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>

fjeremic · 2021-01-20T19:40:26Z

Also through chat, @a7ehuo has provided a sequence of instruction generated for the node from the log file and it seems ok.

@r30shah could we post that here in the PR for archival purposes in case someone looks at this PR in the future?

r30shah · 2021-01-20T19:57:05Z

Sure, a sample instructions generated for moment node,

============================================================
; Live regs: GPR=3 FPR=0 VRF=0 {&GPR_0145, &GPR_0049, GPR_0048}
------------------------------
 n41n     (  0)  NULLCHK on n36n [#32]                                                                [     0x3ff7cd03590] bci=[-1,25,15] rc=0 vc=430 vn=- li=4 udi=- nc=1
 n40n     (  1)    monent  jitMonitorEntry[#112  helper Method] [flags 0x400 0x0 ]                    [     0x3ff7cd03540] bci=[-1,25,15] rc=1 vc=430 vn=- li=4 udi=- nc=1
 n36n     (  3)      ==>l2a (in &GPR_0145) (unneededConv sharedMemory )
------------------------------
Instruction 000003FF7CE4D2C0 throws an implicit NPE, node: 000003FF7CD03540 NPE node: 000003FF7CD03400
monent: Internal Control Flow in OOL : true
ReturnType = NoType
monent: Internal Control Flow in OOL : false
ReturnType = NoType
------------------------------
 n41n     (  0)  NULLCHK on n36n [#32]                                                                [     0x3ff7cd03590] bci=[-1,25,15] rc=0 vc=430 vn=- li=4 udi=- nc=1
 n40n     (  0)    monent  jitMonitorEntry[#112  helper Method] [flags 0x400 0x0 ]                    [     0x3ff7cd03540] bci=[-1,25,15] rc=0 vc=430 vn=- li=4 udi=- nc=1
 n36n     (  2)      ==>l2a (in &GPR_0145) (X!=0 unneededConv sharedMemory )
------------------------------

 [     0x3ff7ce4d2c0]	                       LLZRGF  GPR_0148,#405 0(&GPR_0145) ; Throws Implicit Null Pointer Exception
 [     0x3ff7ce4d4d0]	                       IILF    GPR_0149,263168	
 [     0x3ff7ce4d5e0]	                       N       GPR_0149,#406 36(GPR_0148)
 [     0x3ff7ce4d6c0]	                       BRC     BRZ(0x7), Outlined Label L0042	
 [     0x3ff7ce4dad0]	                       LTG     GPR_0150,#407 216(GPR_0148)
 [     0x3ff7ce4dbb0]	                       BRC     BNL(0xc), Outlined Label L0043	 ; Branch to OOL monent monitorLookupCache
 [     0x3ff7ce4dd60]	                       LA      GPR_0150,#408 0(GPR_0150,&GPR_0145)
 [     0x3ff7ce92cc0]	                       XR      GPR_0147,GPR_0147	
 [     0x3ff7ce92e60]	                       CS      GPR_0147,GPR13,#412 0(GPR_0150)
 [     0x3ff7ce92f40]	                       BRC     BM(0x4), Outlined Label L0042	
 [     0x3ff7ce967f0]	                       ASSOCREGS
 [     0x3ff7ce96230]	                       Label L0041:	
 POST:
 {AssignAny:&GPR_0145:R} {AssignAny:GPR_0147:R} {AssignAny:GPR_0148:R}

------------------------------

 [     0x3ff7ce930e0]	                       Outlined Label L0042:	 ; Denotes start of OOL monent sequence
 [     0x3ff7ce94400]	                       LGR     GPR_0230,GPR13	
 [     0x3ff7ce94550]	                       LGR     &GPR_0231,&GPR_0145	 ; LR=Clobber_eval
 [     0x3ff7ce95000]	                       ASSOCREGS
 [     0x3ff7ce959c0]	                       ASSOCREGS
  PRE:
 {GPR2:GPR_0230:R} {GPR3:&GPR_0231:R}
 [     0x3ff7ce94a20]	                       BRASL   GPR_0197,0x0000000000000000	
 POST:
 {GPR0:D_GPR_0192:D}* {GPR1:D_GPR_0193:D}* {GPR2:D_GPR_0194:D}* {GPR3:D_GPR_0195:D}* {GPR4:D_GPR_0196:D}* {GPR14:GPR_0197:D} {FPR0:D_FPR_0198:D}* {FPR1:D_FPR_0199:D}*
 {FPR2:D_FPR_0200:D}* {FPR3:D_FPR_0201:D}* {FPR4:D_FPR_0202:D}* {FPR5:D_FPR_0203:D}* {FPR6:D_FPR_0204:D}* {FPR7:D_FPR_0205:D}* {FPR8:D_FPR_0206:D}* {FPR9:D_FPR_0207:D}*
 {FPR10:D_FPR_0208:D}* {FPR11:D_FPR_0209:D}* {FPR12:D_FPR_0210:D}* {FPR13:D_FPR_0211:D}* {FPR14:D_FPR_0212:D}* {FPR15:D_FPR_0213:D}* {VRF16:D_VRF_0214:D}* {VRF17:D_VRF_0215:D}*
 {VRF18:D_VRF_0216:D}* {VRF19:D_VRF_0217:D}* {VRF20:D_VRF_0218:D}* {VRF21:D_VRF_0219:D}* {VRF22:D_VRF_0220:D}* {VRF23:D_VRF_0221:D}* {VRF24:D_VRF_0222:D}* {VRF25:D_VRF_0223:D}*
 {VRF26:D_VRF_0224:D}* {VRF27:D_VRF_0225:D}* {VRF28:D_VRF_0226:D}* {VRF29:D_VRF_0227:D}* {VRF30:D_VRF_0228:D}* {VRF31:D_VRF_0229:D}*
 [     0x3ff7ce96050]	                       ASSOCREGS
 [     0x3ff7ce95a90]	                       Label L0046:	
 POST:
 {AssignAny:&GPR_0145:R} {AssignAny:GPR_0147:R} {AssignAny:GPR_0148:R}
 [     0x3ff7ce96120]	                       BRC     NOP(0xf), Label L0041	 ; Denotes end of OOL monent: return to mainline
 [     0x3ff7ce8e000]	                       Outlined Label L0043:	 ; Denotes start of OOL monent monitorLookupCache
 [     0x3ff7ce8e1a0]	                       RISBGN  GPR_0151,&GPR_0145,57,189,255	
 [     0x3ff7ce8e360]	                       LLGF    GPR_0150,#409 2112(GPR_0151,GPR13)
 [     0x3ff7ce8e440]	                       CGIJ    GPR_0150,Label L0044,0,BH(mask=0x8), 	
 [     0x3ff7ce8e600]	                       CG      &GPR_0145,#410 0(GPR_0150)
 [     0x3ff7ce8e6e0]	                       BRC     BNH(0x6), Label L0044	
 [     0x3ff7ce8e7d0]	                       XR      GPR_0147,GPR_0147	
 [     0x3ff7ce8e970]	                       CS      GPR_0147,GPR13,#411 24(GPR_0150)
 [     0x3ff7ce8ea50]	                       BRC     BH(0x8), Label L0045	
 [     0x3ff7ce8eb40]	                       Label L0044:	
 [     0x3ff7ce90f80]	                       LGR     GPR_0190,GPR13	
 [     0x3ff7ce91110]	                       LGR     &GPR_0191,&GPR_0145	 ; LR=Clobber_eval
 [     0x3ff7ce91fc0]	                       ASSOCREGS
  PRE:
 {GPR2:GPR_0190:R} {GPR3:&GPR_0191:R}
 [     0x3ff7ce919e0]	                       BRASL   GPR_0157,0x0000000000000000	
 POST:
 [     0x3ff7ce92ae0]	                       ASSOCREGS
 [     0x3ff7ce92520]	                       Label L0045:	
 POST:
 {AssignAny:&GPR_0145:R} {AssignAny:GPR_0147:R} {AssignAny:GPR_0150:R} {AssignAny:GPR_0151:R} {GPR0:D_GPR_0152:D}* {GPR1:D_GPR_0153:D}* {GPR2:D_GPR_0154:D}* {GPR3:D_GPR_0155:D}*
 {GPR4:D_GPR_0156:D}* {GPR14:GPR_0157:D} {FPR0:D_FPR_0158:D}* {FPR1:D_FPR_0159:D}* {FPR2:D_FPR_0160:D}* {FPR3:D_FPR_0161:D}* {FPR4:D_FPR_0162:D}* {FPR5:D_FPR_0163:D}*
 {FPR6:D_FPR_0164:D}* {FPR7:D_FPR_0165:D}* {FPR8:D_FPR_0166:D}* {FPR9:D_FPR_0167:D}* {FPR10:D_FPR_0168:D}* {FPR11:D_FPR_0169:D}* {FPR12:D_FPR_0170:D}* {FPR13:D_FPR_0171:D}*
 {FPR14:D_FPR_0172:D}* {FPR15:D_FPR_0173:D}* {VRF16:D_VRF_0174:D}* {VRF17:D_VRF_0175:D}* {VRF18:D_VRF_0176:D}* {VRF19:D_VRF_0177:D}* {VRF20:D_VRF_0178:D}* {VRF21:D_VRF_0179:D}*
 {VRF22:D_VRF_0180:D}* {VRF23:D_VRF_0181:D}* {VRF24:D_VRF_0182:D}* {VRF25:D_VRF_0183:D}* {VRF26:D_VRF_0184:D}* {VRF27:D_VRF_0185:D}* {VRF28:D_VRF_0186:D}* {VRF29:D_VRF_0187:D}*
 {VRF30:D_VRF_0188:D}* {VRF31:D_VRF_0189:D}*
 [     0x3ff7ce92bb0]	                       BRC     NOP(0xf), Label L0041	 ; Denotes end of OOL monent monitorCacheLookup: return to mainline
 [     0x3ff7ce930e0]	                       Outlined Label L0042:	 ; Denotes start of OOL monent sequence
 [     0x3ff7ce94400]	                       LGR     GPR_0230,GPR13	
 [     0x3ff7ce94550]	                       LGR     &GPR_0231,&GPR_0145	 ; LR=Clobber_eval
 [     0x3ff7ce95000]	                       ASSOCREGS
 [     0x3ff7ce959c0]	                       ASSOCREGS
  PRE:
 {GPR2:GPR_0230:R} {GPR3:&GPR_0231:R}
 [     0x3ff7ce94a20]	                       BRASL   GPR_0197,0x0000000000000000	
 POST:
 {GPR0:D_GPR_0192:D}* {GPR1:D_GPR_0193:D}* {GPR2:D_GPR_0194:D}* {GPR3:D_GPR_0195:D}* {GPR4:D_GPR_0196:D}* {GPR14:GPR_0197:D} {FPR0:D_FPR_0198:D}* {FPR1:D_FPR_0199:D}*
 {FPR2:D_FPR_0200:D}* {FPR3:D_FPR_0201:D}* {FPR4:D_FPR_0202:D}* {FPR5:D_FPR_0203:D}* {FPR6:D_FPR_0204:D}* {FPR7:D_FPR_0205:D}* {FPR8:D_FPR_0206:D}* {FPR9:D_FPR_0207:D}*
 {FPR10:D_FPR_0208:D}* {FPR11:D_FPR_0209:D}* {FPR12:D_FPR_0210:D}* {FPR13:D_FPR_0211:D}* {FPR14:D_FPR_0212:D}* {FPR15:D_FPR_0213:D}* {VRF16:D_VRF_0214:D}* {VRF17:D_VRF_0215:D}*
 {VRF18:D_VRF_0216:D}* {VRF19:D_VRF_0217:D}* {VRF20:D_VRF_0218:D}* {VRF21:D_VRF_0219:D}* {VRF22:D_VRF_0220:D}* {VRF23:D_VRF_0221:D}* {VRF24:D_VRF_0222:D}* {VRF25:D_VRF_0223:D}*
 {VRF26:D_VRF_0224:D}* {VRF27:D_VRF_0225:D}* {VRF28:D_VRF_0226:D}* {VRF29:D_VRF_0227:D}* {VRF30:D_VRF_0228:D}* {VRF31:D_VRF_0229:D}*
 [     0x3ff7ce96050]	                       ASSOCREGS
 [     0x3ff7ce95a90]	                       Label L0046:	
 POST:
 {AssignAny:&GPR_0145:R} {AssignAny:GPR_0147:R} {AssignAny:GPR_0148:R}
 [     0x3ff7ce96120]	                       BRC     NOP(0xf), Label L0041	 ; Denotes end of OOL monent: return to mainline

fjeremic · 2021-01-20T20:46:12Z

Jenkins test sanity.functional,extended.system zlinux jdk8,jdk11

a7ehuo · 2021-01-21T22:03:08Z

@fjeremic All PR tests have passed. Thanks!

fjeremic reviewed Jan 18, 2021

View reviewed changes

fjeremic added arch:z comp:jit labels Jan 18, 2021

r30shah requested changes Jan 18, 2021

View reviewed changes

runtime/compiler/z/codegen/J9TreeEvaluator.cpp Outdated Show resolved Hide resolved

runtime/compiler/z/codegen/J9TreeEvaluator.cpp Outdated Show resolved Hide resolved

a7ehuo force-pushed the jep390-valuedbased-z branch from 331d116 to 1da4c7e Compare January 20, 2021 15:42

a7ehuo changed the title ~~WIP: Handle monitor enter/exit on value based instances in Z~~ Handle monitor enter/exit on value based instances in Z Jan 20, 2021

a7ehuo marked this pull request as ready for review January 20, 2021 15:44

fjeremic self-assigned this Jan 20, 2021

r30shah approved these changes Jan 20, 2021

View reviewed changes

runtime/compiler/z/codegen/J9TreeEvaluator.cpp Outdated Show resolved Hide resolved

a7ehuo force-pushed the jep390-valuedbased-z branch from 1da4c7e to 2bdf5db Compare January 20, 2021 19:38

fjeremic approved these changes Jan 20, 2021

View reviewed changes

fjeremic merged commit a2ae593 into eclipse-openj9:master Jan 21, 2021

a7ehuo mentioned this pull request Jan 22, 2021

Simplify isMonitorValueBasedOrValueType #11747

Merged

a7ehuo deleted the jep390-valuedbased-z branch February 4, 2021 19:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle monitor enter/exit on value based instances in Z #11677

Handle monitor enter/exit on value based instances in Z #11677

a7ehuo commented Jan 18, 2021 •

edited

Loading

fjeremic left a comment

fjeremic commented Jan 18, 2021

a7ehuo commented Jan 18, 2021

a7ehuo commented Jan 20, 2021

r30shah left a comment

fjeremic commented Jan 20, 2021

r30shah commented Jan 20, 2021

fjeremic commented Jan 20, 2021

a7ehuo commented Jan 21, 2021

Handle monitor enter/exit on value based instances in Z #11677

Handle monitor enter/exit on value based instances in Z #11677

Conversation

a7ehuo commented Jan 18, 2021 • edited Loading

fjeremic left a comment

Choose a reason for hiding this comment

fjeremic commented Jan 18, 2021

a7ehuo commented Jan 18, 2021

a7ehuo commented Jan 20, 2021

r30shah left a comment

Choose a reason for hiding this comment

fjeremic commented Jan 20, 2021

r30shah commented Jan 20, 2021

fjeremic commented Jan 20, 2021

a7ehuo commented Jan 21, 2021

a7ehuo commented Jan 18, 2021 •

edited

Loading