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

[Lindenmaier, Goetz]

Hi,

I added an assertion, generated code looks like this now:

  // Remove duplicated operands and inputs which use the same name.
  if (num_opnds() == 4) {
    unsigned num0 = 0;
    unsigned num1 = opnd_array(1)->num_edges(); 	// mem
    unsigned num2 = opnd_array(2)->num_edges(); 	// twentyfour
    unsigned num3 = opnd_array(3)->num_edges(); 	// twentyfour
    unsigned idx0 = oper_input_base();
    unsigned idx1 = idx0 + num0;
    unsigned idx2 = idx1 + num1;
    unsigned idx3 = idx2 + num2;
    unsigned idx4 = idx3 + num3;
    for (int i = idx4 - 1; i >= (int)idx3; i--) {
      del_req(i);
    }
    _num_opnds = 3;
  } else {
    assert(_num_opnds == 3, "There should be either 3 or 4 operands.");
  }

New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8168283-adlc_expand/webrev.02/

Best regards,
  Goetz.

> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Donnerstag, 20. Oktober 2016 19:09
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-compiler-
> dev at openjdk.java.net
> Subject: Re: RFR(S): 8168283: adlc: fix error expanding expanded nodes.
> 
> On 10/19/16 11:16 PM, Lindenmaier, Goetz wrote:
> > 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.
> 
> Yes, num_opnds() >= cur_num_opnds does not make sense. That is why I
> thought to use num_unique_opnds.
> 
> > But I could assert that num_opnds() is either cur_num_opnds or
> num_unique_opnds().
> 
> Would be nice.
> 
> thanks,
> Vladimir
> 
> >
> > 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.
> >>>