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

[Vladimir Kozlov]

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.
>>>