RFR: 8263164: assert(_base >= VectorA && _base <= VectorZ) failed: Not a Vector while calling StoreVectorNode::memory_size() [v3]

[Vladimir Ivanov]

On Thu, 11 Mar 2021 23:54:17 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Would be nice to see where TOP value is coming from in stepEvent004.java test.
>
> Hi @vnkozlov , @XiaohongGong and @theRealELiu ,
> 
> If I read the code correctly, StoreNode::Idea can't fix this kind of bugs.
> 
> This is because we may see other store nodes with value=top on the IGVN list [1]
> 
> StoreNode::Ideal
>   |
>   `InitializeNode::capture_store
>      |
>      `InitializeNode::stores_are_sane
>         |
>         `st->as_Store()->memory_size()  <-- st's value may be TOP (e.g., when it is on the IGVN list waiting bo be transformed)
> 
> Fixing in StoreNode::Idea can just bypass the top value for the current store node during IGVN optimization.
> But it may still fail when other nodes with top value queuing on the IGVN list.
> 
> So we should support vector's memory_size()/element_size()/length()/length_in_bytes() for that case.
> 
> vect_type() has been improved based on @XiaohongGong 's comments.
> And since StoreNode::Idea won't fix the problem, I suggest to keep StoreNode::Ideal() as it is to lower the risk.
> What do you think?
> 
> Testing:
>   - tier1~tier3 on Linux/x64, no regression
>   - nightly testing of jdk/incubator/vector
> 
> Thanks.
> Best regards,
> Jie
> 
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/memnode.cpp#L4467

Why not simply cache the vector type of value being stored during `StoreVectorNode` construction and use it for subsequent queries? Most of the vector nodes already do that since `VectorNode` subclasses `TypeNode`, but `StoreVectorNode` is a `MemNode`. 

And that's the reason I don't understand why non-`StoreVectorNode`-related changes are needed in your patch.

Nevertheless, there's another case that seems to suffer from the very same problem - `ReductionNode`.

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

PR: https://git.openjdk.java.net/jdk/pull/2867