Omr: Z code generator can generate invalid snippets

[andrewcraik]

When privatized inliner argument rematerialization is re-enabled the z code generator trips assertions in outlined data snippet code. The optimization is disabled on line 1925 of Inliner.cpp until this issue is resolved.

[fjeremic]

@doubletea Could you please take a look at this and investigate? The relevant PR is #428 which exposes the issue. You will have to re-enable it for z.

[doubletea]

Backtracking from the assertion, the issue stems from _dataReferenceInstruction field not being set when a memory reference is created with an unresolved snippet. I discovered that in the constructor for S390MemMemInstruction the TR::MemoryReference *mf2 argument will not check and set its _dataReferenceInstruction field; whereas the TR::MemoryReference *mf1 argument will (due to its constructor). Other instruction constructors do this if it has a TR::MemoryReference argument.

The S390MemMemInstruction constructor is called by for example generateSS1Instruction inside directMemoryStoreHelper.

Testing a build using master with just adding the same set and check for mf2 passes all test on z now.

I don't know exactly how PR #428 connects to this issue other than exposing what appears to me like a preexisting problem on z.

[doubletea]

Taking a look at the tracelog, the unresolved snippet causing the assertion is made by directMemoryStoreHelper , which tries to perform a memory-memory copy while evaluating a store node with a pattern like this:

 n291n    (  0)  istore  <temp slot 10>[#632  Auto] [flags 0x20000003 0x0 ] (privatizedInlinerArg )   [0x000003ffc44f4e38] bci=[-1,73,475] rc=0 vc=405 vn=- li=78 udi=- nc=1 flg=0x2000
 n292n    (  1)    iloadi  java/util/zip/ZipFile$ZipFileInflaterInputStream.len I[#602  unresolved notAccessed volatile Shadow] [flags 0x2603 0x0 ] (cannotOverflow )  [0x000003ffc44f4e80] bci=[-1,73,475] rc=1 vc=405 vn=- li=- udi=- nc=1 flg=0x1000
 n337n    (  1)      ==>aRegLoad (in &GPR_0688) (X!=0 X>=0 SeenRealReference sharedMemory )

directMemoryStoreHelper creates a MVC instruction which has the form D1(L,B1),D2(B2)

In this case, the memref for the istore will have its offset be in D1 and the memref for the iloadi will have its offset will be in D2

However, unresolved runtime patching cannot handle patching the D2 offset, as it will only look for and patch D1. Therefore, in this case the transformation should not work because iloadi is unresolved.

The solution in my previous comment will not work because mf2 should not have an unresolved snippet in the first place. The new solution is to add an extra guard against directMemoryStoreHelper by conditioning with !valueNode->hasUnresolvedSymbolReference()

Currently testing on https://github.com/eclipse/omr/compare/master...doubletea:snippetFix, debugging one intermittent failure on a NOP test.

[doubletea]

The failing test is caused by the loopVersioner optimization transforming a node with inlineNonoverriddenGuard . It seems the exact same issue was also recently uncovered by the optimizer team, who are now tracking this defect.

The changes pass all other tests on z when privatized inliner argument rematerialization is re-enabled, so PR #471 is ready to merge

[fjeremic]

This fix for this issue (#471) has now been merged. @andrewcraik please verify and close this issue when time permits.

[andrewcraik]

Verification will occur once pull request 485 is merged.

[mstoodle]

Seems to have been resolved. If I'm mistaken please reopen and explain my mistake.