RFR(M): 8027422: assert(_gvn.type(obj)->higher_equal(tjp)) failed: cast_up is no longer needed (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Jan 10 09:36:17 PST 2014

compile.cpp: new verification code (under ASSERT) at the end of remove_speculative_types() checks only the root node - nothing else is pushed on worklist. Also add comment to this new code.

Passing 'true' parameter is not very informative. You can use local variable:

bool include_speculative = true; t->filter(_type, include_speculative);

An other way to make code more informative is to add a comment to parameter:

t->filter(_type, true /* include_speculative */);

Either way is fine.

Thanks, Vladimir

On 1/10/14 1:46 AM, Roland Westrelin wrote:

Hi Vladimir,

Here is a new webrev for this: http://cr.openjdk.java.net/~roland/8027422/webrev.01/ I fixed the issues you mentioned in your review. I added a call to removespeculative() to the ConNode constructor. When a node becomes constant, its speculative part can be not null. The IGVN doesn’t kill ConNodes so without a call to removespeculative() a ConNode with a speculative part can sneak past the call to Compile::removespeculativetypes(). I also added a verification method: NodeHash::checkspeculativetypes() to check that no TypeNode with a speculative type is still in the IGVN hash table after Compile::removespeculativetypes() Roland. On Nov 19, 2013, at 8:49 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

On 11/19/13 11:45 AM, Roland Westrelin wrote: Thanks for reviewing this, Vladimir.

On Nov 19, 2013, at 1:34 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

Next 2 places in type.cpp pass 'true' to meet() unconditionally:

1929 return TypeAry::make(elem->meet(a->elem, true), 3812 const TypeAry *tary = ary->meet(tap->ary, true)->isary(); Should TypeAryPtr::removespeculative() also clean speculative in element's type? You’re right. It probably should. So I need to add a removespeculative() method to TypeAry. Then the 2 places where true is passed to meet() for TypeAry don’t matter anymore because removespeculative() is called from meet() and removespeculative now has an effect on TypeAry, right? Right, passing 'true' will work in all cases then. Thanks, Vladimir

Could you make printing code with 'thist' aligned again in Type::meet()? Sure. Roland.

thanks, Vladimir On 11/18/13 1:15 PM, Roland Westrelin wrote: The root of the problem is that during the null check when the type of obj is improved in GraphKit::castnotnull(): const Type *tnotnull = t->join(TypePtr::NOTNULL, true); The join with TypePtr::NOTNULL is not applied to the speculative part. In fact, no meet between a TypeOopPtr and a TypePtr modifies the speculative part. One way to fix it would be to apply the meet with a TypePtr to the speculative part as well as the standard part of the type which I tried: then we need to move the speculative field up in TypePtr and modify all operations on TypePtr to operate on speculative so that the type system remains symmetric. In many places where we mix a TypePtr with a TypeOopPtr we actually don’t care about the speculative part. I changed the following operations on Type: higherequal() meet() join() filter() so that by default they don’t return a result that include the speculative part of the type. Where we need the speculative part of the type, we have to explicitly request it. I also fixed a problem with Type nodes with a type of TypeNarrowOop that wouldn’t drop the speculative part of the type during Compile::removespeculativetypes(). I included small clean ups that Mikael suggested privately (dropped the duplicate check for res->isaoopptr() in TypeOopPtr::meet, make removespeculative not go through the exercise of creating a new type if speculative is NULL). http://cr.openjdk.java.net/~roland/8027422/webrev.00/ Roland.

More information about the hotspot-compiler-dev mailing list