RFR(S): 8168283: adlc: fix error expanding expanded nodes.

[Lindenmaier, Goetz]

Hi Vladimir, 

thanks for looking at my change.

> Should we use next ?
> if (num_opnds() > %d) {\n", node->num_unique_opnds());

The code within the if accesses the opnd_array up to index 'cur_num_opnds'.
Therefore num_opnds() must be at least that number.
It is opnd_array(cur_num_opnds-1)->num_edges() what is crashing in my case.

It would be possible to check >=, but I don't think that makes sense.
But I could assert that num_opnds() is either cur_num_opnds or num_unique_opnds().

Best regards,
  Goetz.


> -----Original Message-----
> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
> Sent: Donnerstag, 20. Oktober 2016 00:26
> To: hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR(S): 8168283: adlc: fix error expanding expanded nodes.
> 
> Should we use next ?
> 
> if (num_opnds() > %d) {\n", node->num_unique_opnds());
> 
> instead of
> 
> +      fprintf(fp, "  // Remove duplicated operands and inputs which use
> the same name.\n");
> +      fprintf(fp, "  if (num_opnds() == %d) {\n", cur_num_opnds);
> 
> Otherwise seems good.
> 
> Thanks,
> Vladimir
> 
> On 10/19/16 7:44 AM, Lindenmaier, Goetz wrote:
> > Hi,
> >
> > This fixes a small error in the expand methods usually generated from
> expand rules by adlc.
> >
> > Actually all nodes get an expand method generated, as it's used to finish
> construction of nodes after the matcher generated them. E.g., temp nodes
> are added in the expand.
> >
> > The error occurs for a 'real' expand generating several sub nodes. The 'real'
> expand rule calls expand of the newly generated nodes, let's call it sub-
> expand.
> >
> > Unfortunately the matcher sometimes generates superfluous operands
> that are removed in the expand methods. The 'real' expand generating sub-
> nodes does not generate these superfluous operands, therefore running the
> sub-expand crashes when it tries to remove these.
> >
> > This change adds a simple check in the sub-expand to avoid this crash. Also
> it adds setting the correct number of operands in the expand that generates
> the sub node.
> >
> > An example of an adl instruct declaration and the generated code is
> attached to the bug. We see this in a rule for the s390 port.
> >
> > http://cr.openjdk.java.net/~goetz/wr16/8168283-
> adlc_expand/webrev.01/
> > https://bugs.openjdk.java.net/browse/JDK-8168283
> >
> > Please review this change. I please need a sponsor.
> >
> > Best regards,
> >   Goetz.
> >