Request for reviews (M): 7192963: assert(_in[req-1] == this) failed: Must pass arg count to 'new'

[Christian Thalinger]

The changes look good.  I just have two small requests:

src/share/vm/opto/superword.cpp:

+     Node* pi = p->at(i);
+     Node* def = pi->in(idx);
+     if (p0_def != def)

Could you rename def to pi_def?

src/share/vm/opto/vectornode.cpp:

While we are already touching almost all occurences of PackNode::binaryTreePack could we rename it to PackNode::binary_tree_pack?

-- Chris

On Aug 22, 2012, at 10:32 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> http://cr.openjdk.java.net/~kvn/7192963/webrev
> 
> 7192963: assert(_in[req-1] == this) failed: Must pass arg count to 'new'
> 
> An other variant of shifts with loop variant counts: "a[i]=1<<i;". C2 tried to create PackINode for this case and failed because that code was broken. Pack nodes are not supported (there are no Mach nodes) so the code was not tested before. There is code in SuperWord::profitable() method which check for cases where Pack node is needed and skip such vectorization. But it does not check it for shift node count edge since it does not expect vector input for it.
> 
> The fix is to not vectorize shift instructions if count is not the same for all shifts and if count is vector. Note, I removed the changes for previous 7192964 fix since it was not precise and current fix cover 7192964 case (I verified).
> Also fixed Pack node generation, number of inputs should be 2 at the creation otherwise it gives the bug's assert.
> 
> I did some refactoring. Moved and renamed SuperWord::vector_opd_range() to VectorNode::vector_operands(). Opcode checks of supported nodes are done in vectornode.cpp. Also this method was incorrect - it had only one LShiftI opcode listed. And I added other supported nodes.
> 
> Thanks,
> Vladimir