Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add range check test around profiled guard method test #4721

Merged

Conversation

r30shah
Copy link
Contributor

@r30shah r30shah commented Feb 13, 2019

In OpenJ9 we take advantage of the profiled information in inlining method for virtual call. While inlining the method, we generate a guard around inlined code to make sure that the inlined implementation is indeed what we need to execute. For this TR_ProfiledGuard**
kind of guard it either TR_VftTest or TR_MethodTest.

In case of TR_MethodTest, it records the offset of the inlined method in the Virtual Function Table of the class in which that method belongs to and compares the value at the same offset of class of receiver object with the J9Method of inlined method. This comparison introduces a functional bug.
Some classes would have smaller function table and recorded offset w.r.t the class
might point to a garbage value or even might be inaddressible (Out of mapped memory range). This commit adds a range check test around the TR_MethodTest to verify that address that we are going to reference points to valid address.

Signed-off-by: Rahil Shah rahil@ca.ibm.com

@r30shah
Copy link
Contributor Author

r30shah commented Feb 13, 2019

@fjeremic @andrewcraik @jdmpapin Can I please get a review? Currently I am doing a performance evaluation of this change and also I need to add documentation of what this is doing. so marked this as WIP.
Looking at the function where I can possibly add this test in lower trees is https://github.com/eclipse/openj9/blob/99e024af1e763969da4a3030ec72543dd3d4ea88/runtime/compiler/codegen/J9CodeGenerator.cpp#L784.
It is a massive function doing all sorts of things, and it looks like this kind of change needs proper documentation and separate phase generating this test require us to split the block and copy GlRegDeps.
So I have created a separate CodeGen Phase to add the range check test instead of the doing it in lowerTrees. I would like to get all reviewer's view on this.

TR::Compilation *comp = self()->comp();
TR::CFG * cfg = comp->getFlowGraph();
TR::Block *currentBlock = NULL;
for (TR::TreeTop * treeTop = comp->getStartTree(); treeTop; treeTop = treeTop->getNextTreeTop())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are you walking all TreeTops - a profiled guard has to be at the end of a basic block so you can walk the blocks in treetop order checking their exits which will save time...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using one of the Block iterators available in ILWalk.cpp to accomplish this walk.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added change to walk blocks in CFG and checking Exits (36292aa)

TR_VirtualGuard *vg = comp->findVirtualGuardInfo(node);
if (vg && vg->getTestType() == TR_MethodTest && vg->getIsInterfaceMethodGuard())
{
traceMsg(comp, "Need to add a rangecheck before n%dn in block_%d\n",node->getGlobalIndex(), currentBlock->getNumber());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be a dumpOptDetails rather than just a trace message I would have thought...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in 36292aa

if (vg && vg->getTestType() == TR_MethodTest && vg->getIsInterfaceMethodGuard())
{
traceMsg(comp, "Need to add a rangecheck before n%dn in block_%d\n",node->getGlobalIndex(), currentBlock->getNumber());
TR::Node* vTableSizeOfReceiver = TR::Node::createWithSymRef(TR::iloadi, 1, 1, node->getFirstChild()->getFirstChild(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you guaranteed that the grandchild exists?

@andrewcraik
Copy link
Contributor

I'm not sure on the compile-time implication of a new phase - @fjeremic thoughts? The clarity certainly seems useful rather than weaving it in with a bunch of other code. That being said some more comments wouldn't hurt :)

@r30shah
Copy link
Contributor Author

r30shah commented Feb 13, 2019

OMR Counterpart: eclipse/omr#3564

@fjeremic
Copy link
Contributor

I'm not sure on the compile-time implication of a new phase - @fjeremic thoughts? The clarity certainly seems useful rather than weaving it in with a bunch of other code. That being said some more comments wouldn't hurt :)

We've introduced other codegen phases that do a treetop walk (example [1]) and they have never been visible on a startup libj9jit29.so profile. I'm in support of the logical separation.

[1] https://github.com/eclipse/openj9/blob/35c44ead49a53f8c0ad1ca3e63c18711d3a7b26f/runtime/compiler/z/codegen/CodeGenPhaseToPerform.hpp#L51

else if (node->getOpCode().isIf() && node->isTheVirtualGuardForAGuardedInlinedCall())
{
TR_VirtualGuard *vg = comp->findVirtualGuardInfo(node);
if (vg && vg->getTestType() == TR_MethodTest && vg->getIsInterfaceMethodGuard())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this will create an unnecessary bounds check in cases where vg will be generated using vgnop with runtime assumptions and no runtime test. Should we determine here whether there will be a runtime test? Unfortunately, existing guard APIs don't seem quite right for this purpose:

  • isNopable() probably doesn't guarantee the absence of a runtime test
  • mustBeNoped() will fail to identify most guards that don't do a runtime test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone will need to check but a combination of isNopable and the codegen supporting noping is what I would expect to be the right test.

Copy link
Contributor Author

@r30shah r30shah Feb 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the virtualGuardHelper
https://github.com/eclipse/omr/blob/6e7568d6532997b3de2e41da2560f9af79cd9799/compiler/z/codegen/ControlFlowEvaluator.cpp#L139
I see we have a codegen query to know if VGNOP is supported or not :
cg->getSupportsVirtualGuardNOPing()
Also this query node->isNopableInlineGuard() which checks if it is guard for inlined method and not profiled guard. I think I can add this two query.
One more doubt I have is another query : Let's say Codegen supports the VGNOP and it is Nopable Inline guard, I see next query we have is following

if (!node->isHCRGuard() && !node->isOSRGuard() && !(comp->performVirtualGuardNOPing() &&
         comp->isVirtualGuardNOPingRequired(virtualGuard)) &&
virtualGuard->canBeRemoved())

Assuming we would not have MethodTest for OSR and HCR guard we do not need to check that. So interesting set of queries there would be last which should be covered here.

runtime/compiler/codegen/J9CodeGenerator.cpp Show resolved Hide resolved
TR::Node* vTableSizeOfReceiver = TR::Node::createWithSymRef(TR::iloadi, 1, 1, node->getFirstChild()->getFirstChild(),
comp->getSymRefTab()->findOrCreateVtableEntrySymbolRef(comp->getMethodSymbol(),
sizeof(J9Class)+ offsetof(J9VTableHeader, size)));
TR::Node * rangeCheckTest = TR::Node::createif(TR::ificmplt, vTableSizeOfReceiver, TR::Node::iconst(node, node->getFirstChild()->getSymbolReference()->getOffset()), node->getBranchDestination());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to @andrewcraik's comment above, we need to know that node->getFirstChild() is an aloadi <vtable-entry-symbol>. There needs to be some kind of check that the guard tree is in the expected form

TR::Node* vTableSizeOfReceiver = TR::Node::createWithSymRef(TR::iloadi, 1, 1, node->getFirstChild()->getFirstChild(),
comp->getSymRefTab()->findOrCreateVtableEntrySymbolRef(comp->getMethodSymbol(),
sizeof(J9Class)+ offsetof(J9VTableHeader, size)));
TR::Node * rangeCheckTest = TR::Node::createif(TR::ificmplt, vTableSizeOfReceiver, TR::Node::iconst(node, node->getFirstChild()->getSymbolReference()->getOffset()), node->getBranchDestination());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe J9VTableHeader.size is in entries (measured relative to the first entry), whereas the offset used in the guard tree is in bytes relative to the beginning of the J9Class, so that the offset needs to be converted into an index for comparison, something like this:

(offset - sizeof (J9Class) - sizeof (J9VTableHeader)) / sizeof (uintptrj_t)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. Changed to adjust the constant offset to point to index in table .
36292aa

if (vg && vg->getTestType() == TR_MethodTest && vg->getIsInterfaceMethodGuard())
{
traceMsg(comp, "Need to add a rangecheck before n%dn in block_%d\n",node->getGlobalIndex(), currentBlock->getNumber());
TR::Node* vTableSizeOfReceiver = TR::Node::createWithSymRef(TR::iloadi, 1, 1, node->getFirstChild()->getFirstChild(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J9VTableHeader.size is a UDATA, which is long on 64-bit platforms. I don't know whether extremely large vtables (with at least 2^31 entries) are possible, but regardless, the test that is currently generated will load the high half of size on big-endian 64-bit platforms, which will almost certainly be zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed load to aloadi to make sure, it loads correct size from the J9VTableHeader. 36292aa

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead choose between iloadi/iconst/icmp and lloadi/lconst/lcmp as appropriate? Neither of the values being compared here is really an address, and the ordering acmp opcodes are a bit unusual

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 29a608c

TR::Node* vTableSizeOfReceiver = TR::Node::createWithSymRef(TR::iloadi, 1, 1, node->getFirstChild()->getFirstChild(),
comp->getSymRefTab()->findOrCreateVtableEntrySymbolRef(comp->getMethodSymbol(),
sizeof(J9Class)+ offsetof(J9VTableHeader, size)));
TR::Node * rangeCheckTest = TR::Node::createif(TR::ificmplt, vTableSizeOfReceiver, TR::Node::iconst(node, node->getFirstChild()->getSymbolReference()->getOffset()), node->getBranchDestination());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comparison should not be strict, i.e. it should use if..cmple, because an index equal to size is out of range

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comparison should still change to le from lt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 29a608c

// Because of the fact that it is very unlikely that we will have another aloadi node with same VTable offset of same receiver object, this child would not be commoned out
// and have only single reference in this virtual guard therefore splitting of block will not store it to temp slot.
// Due to the above reasons we have a fatal assert in case the first child of the guard node performing is not aloadi. If there exists any case we need to reconsider our choice for range check test.
TR_ASSERT_FATAL(node->getFirstChild()->getOpCodeValue() == TR::aloadi, "Virtual Guard perfoming TR_MethodTest should have aloadi as first child, to extract the vftPointer of the receiver object");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewcraik @jdmpapin I have added a comment here regarding to why we have the FATAL_ASSERT if child of the guard is not aloadi .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've recently seen PRE hoist an aloadi <vft-symbol> out of a loop, leaving a profiled guard with a VFT test of this form:

ifacmpne --> slow path
  aload <temp slot ...>
  aconst 0x... (isClassPointerConstant)

There might be circumstances where aloadi <vtable-entry-symbol> looks like it will happen unconditionally, so that normally it would be safe to hoist, but because it's part of one of these guards it would now be implicitly conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewcraik I discussed this with @jdmpapin as well, although it is possible to optimization to hoist out aloadi we would still left with the issue where that aloadi is not safe as it is followed by invisible condition. Another solution Devin came up with was , we could have new type of conditional load node in trees which we can identify during codegen phase and generate a range check test. Even in this case, if some optimization hoist id out of loop, we would have difficulty generating a range check test.
It seems if some optimization tries to work with these type of load, it needs to be cautious not to mess up these kind of invisible conditional node.
@andrewcraik I would like to get your view on this. ATM I have strengthen out the ASSERT by also checking the type of symref associated with aloadi which should be of type vTable Entry Symbol in ddbed97

TR_VirtualGuard *vg = comp->findVirtualGuardInfo(node);
// Mainly we need to make sure that virtual guard which performs the TR_MethodTest and can be NOP'd are needed the range check.
if (vg && vg->getTestType() == TR_MethodTest &&
!(comp->performVirtualGuardNOPing() && (node->isNopableInlineGuard() || comp->isVirtualGuardNOPingRequired(vg))))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewcraik @jdmpapin Combining query from CodeGen where it generates the VGNOP. Basically if VGNOP is supported and node or guard requires NOPing then we won't add range check test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ok - @jdmpapin concerns?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this is probably the best we'll get without some mechanism to get codegen to commit in advance to using vgnop. I think it's good for now

@r30shah
Copy link
Contributor Author

r30shah commented Feb 20, 2019

Ran a personal test and I am not seeing any failures. Also finished performance runs with the last changes incorporated.

DayTrader-7 on Linux on Z (4 Core No SMT) results.

  1. Compilation Time Results
Config Avg. Compilation Time % Standard Deviation % Perf
Baseline 156956.7 3.65 100
RangeTest 158405.4 3.72 99.08
  1. Throughput Results
Config Avg. Throughput % Standard Deviation % Perf
Baseline 10309.89 0.49 100
RangeTest 10300.23 0.75 99.90

Spark Results (16 Core SMT-2 Linux On Z)

  1. HiBench/Pagerank Performance
Config Time % Standard Deviation % Perf
Baseline 39.39 1.45 100
RangeTest 39.62 3.93 99.41
  1. HiBench/Terasort Performance
Config Time % Standard Deviation % Perf
Baseline 32.35 7.60 100
RangeTest 31.53 0.83 102.6

In summary I do not see any effect of having these extra test in both compilation time as well as throughput.

Changes should be fine. Removing WIP.

@r30shah r30shah changed the title WIP : Add range check test around profiled guard method test Add range check test around profiled guard method test Feb 20, 2019
// and have only single reference in this virtual guard therefore splitting of block will not store it to temp slot.
// Due to the above reasons we have a fatal assert in case the first child of the guard node performing is not aloadi. If there exists any case we need to reconsider our choice for range check test.
TR::Node *vTableLoad = node->getFirstChild();
TR_ASSERT_FATAL(vTableLoad->getOpCodeValue() == TR::aloadi &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that Devin has seen the aloadi happen I don't think we can tolerate a fatal assert in production - would you crash the JVM? I think the right thing is to abort the compilation... It should be rare but a fatal assert when we have a known example makes me worry that we would generate issues for users needlessly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative to failing the compile is to do a type test or to jump unconiditionally to the failed side of the guard - the latter probably being the easiest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting inclined towards the idea jump unconditionally to prevent JVM for crashing. This would definitely have lesser perf impact then failing compilation but I have one concern with that. In case, some one saves vtable-entry-table symbol in temp slot (hoisting out of the loop or splitting block, etc), that means we would still have a bug present in the compiled body where illegal reference appears well before of the guard. Skipping over the whole guard and inlined body in that case will not prevent the illegal memory reference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative to failing the compile is to do a type test or to jump unconiditionally to the failed side of the guard - the latter probably being the easiest.

I agree. The safe thing to do is always bail to the slow path. We can place a static debug counter here to keep track of how often we need fall back to the slow path if we want such data (in customer scenarios).

@andrewcraik do we have some sort of debug counter name prepend which is meant for "bad performance" things that we're doing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not at present - we were going to look to do that as part of the assert conversion work - we hadn't agreed a convention yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't actually seen aloadi <vtable-entry-symbol> get replaced with a load of a temp, but I have seen the corresponding thing happen to aloadi <vft-symbol>, and I know of a couple of situations in which it would be reasonable to do that transformation with <vtable-entry-symbol> as well, unless we decide that it's illegal to do so.


The first situation is as follows. We have two calls, one after the other, calling the same method of the same object.

treetop
  calli Foo.bar()V
    aloadi <vft-symbol>
      aload obj
    ==>aload obj
treetop
  calli Foo.bar()V
    aloadi <vft-symbol>
      aload obj
    ==>aload obj

We inline both with the same profiled guard with method test, and virtual guard tail splitter causes the second guard to be dominated by the first inlined body. After block extension, local CSE commons the children of the guards:

BBStart
ifacmpne --> slow call 0
  aloadi <vtable-entry-symbol>
    aloadi <vft-symbol>
      aload obj
  aconst 0x...
BBEnd
BBStart (extension)
<...inlined Foo.bar()V 0...>
BBEnd
BBStart (extension)
ifacmpne --> slow call 1
  ==>aloadi <vtable-entry-symbol>
  ==>aconst 0x...
BBEnd
BBStart (extension)
<...inlined Foo.bar()V 1...>
BBEnd

If <...inlined Foo.bar()V 0...> is split and a merge point is inserted, then aloadi <vtable-entry-symbol> will be spilled into a temp.

Assuming this kind of thing is sufficiently unlikely, I believe it's fine to deal with it by unconditionally failing the guard, as mentioned upthread.


The second scenario is code motion, as @r30shah mentioned. To elaborate a bit, the safety requirements for aloadi <vtable-entry-symbol> are stricter than those for <vft-symbol>, and I don't know whether anything checks for those and moves these loads. However, it would be reasonable to hoist any (invariant) load whatsoever if, upon entering the loop, the load would necessarily run, e.g.

do {
    x = obj.baz;  // NB. if loop has been entered, this *always* runs
    ...
} while(...);

can in general be safely rewritten as

tmp = obj.baz;
do {
    x = tmp;
    ...
} while(...);

because if the load is disallowed at the new location, it would also have been disallowed at its original location, no matter what its safety conditions are.

The issue would be that with this change, when aloadi <vtable-entry-symbol> occurs in a profiled guard, the vtable entry isn't necessarily loaded every time we reach the guard. It has an implicit safety condition that is tested at runtime. Because the test is implicit, it might look as though the load can be hoisted, but hoisting it would remove it from the guard, and as a result separate it from the implicit test.

To ensure that we don't hit this kind of problem in the future, I think we have to do one of the following:

  1. decree that no optimization may speculatively load <vtable-entry-symbol>, at least when its original occurrence was under a profiled guard; or
  2. decree that no optimization may speculatively load <vtable-entry-symbol> without first checking the size of the vtable, even if the existing control flow appears to guarantee that the vtable is large enough; or
  3. create a new node that encapsulates the conditional, which would be safe to speculatively evaluate; or
  4. make the safety test explicit in the CFG.

Personally I'm not a fan of restrictions like 1 or 2 (out of which 2 seems less objectionable to me), but I understand if given the alternatives one of them seems to be the best option, or at least the best option for the moment.

If we do want to go with something like 1 or 2, I'm not suggesting that we have to immediately audit all optimizations that could insert problematic speculative loads. This is more of a request to consider and possibly document the direction of the design w.r.t. this sort of issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering everything, It seems like jumping to slower path would not prevent the bug.
As we are checking both opcode of the child as well as type of symbol reference, If we do not have both, it would be safer to fail compilation.
I have changed the code in 0f44e09

@r30shah r30shah force-pushed the fixInterfaceMethodTestOpenJ9 branch 2 times, most recently from f633703 to 24df5d3 Compare February 25, 2019 14:42
Copy link
Contributor

@jdmpapin jdmpapin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved assuming the opcode typo is fixed

// In rare case child of the virtual guard is manipulated then illegal memory reference load would hace occured before the Virtual Guard which
// is already a bug as mentioned in the description of this function and it would be safer to fail compilation.
TR::Node *vTableLoad = node->getFirstChild();
if (!(vTableLoad->getOpCodeValue() == TR::alaodi && comp->getSymRefTab()->isVtableEntrySymbolRef(vTableLoad->getSymbolReference())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TR::aloadi (the a and o are switched)

TR_VirtualGuard *vg = comp->findVirtualGuardInfo(node);
// Mainly we need to make sure that virtual guard which performs the TR_MethodTest and can be NOP'd are needed the range check.
if (vg && vg->getTestType() == TR_MethodTest &&
!(comp->performVirtualGuardNOPing() && (node->isNopableInlineGuard() || comp->isVirtualGuardNOPingRequired(vg))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this is probably the best we'll get without some mechanism to get codegen to commit in advance to using vgnop. I think it's good for now

In OpenJ9 we take advantage of the profiled information in
inlining method for virtual call. While inlining the method,
we generate a guard around inlined code to make sure that the inlined
implementation is indeed what we need to execute. For this TR_ProfiledGuard**
kind of guard it either TR_VftTest or TR_MethodTest.
In case of TR_MethodTest, it records the offset of the inlined method in the
Virtual Function Table of the class in which that method belongs to and
compares the value at the same offset of class of receiver object with the
J9Method of inlined method. This comparison introduces a functional bug.
Some classes would have smaller function table and recorded offset w.r.t the class
might point to a garbage value.
This commit adds a range check test around the TR_MethodTest to verify that address
that we are going to reference points to valid address.

Signed-off-by: Rahil Shah <rahil@ca.ibm.com>
// In rare case child of the virtual guard is manipulated then illegal memory reference load would hace occured before the Virtual Guard which
// is already a bug as mentioned in the description of this function and it would be safer to fail compilation.
TR::Node *vTableLoad = node->getFirstChild();
if (!(vTableLoad->getOpCodeValue() == TR::aloadi && comp->getSymRefTab()->isVtableEntrySymbolRef(vTableLoad->getSymbolReference())))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

@andrewcraik
Copy link
Contributor

Jenkins test sanity xlinux,win,plinux jdk8,jdk11

@r30shah
Copy link
Contributor Author

r30shah commented Feb 25, 2019

@andrewcraik Seems like PPCLE failure is same as #4470. Nothing to do with this change. Can we do the sanity tests again?

@andrewcraik
Copy link
Contributor

Given the failure is the same as #4470 I think this should be good to go.

@andrewcraik andrewcraik merged commit 36b12f0 into eclipse-openj9:master Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants