RFR: 8246152: Improve String concat bootstrapping

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Claes Redestad" <claes.redestad at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Vendredi 29 Mai 2020 18:47:03
> Objet: Re: RFR: 8246152: Improve String concat bootstrapping

> Hi Rémi,
> 
> thanks for looking at this and suggesting some improvements!
> 
> On 2020-05-29 17:51, Remi Forax wrote:
>> Hi Claes,
>> For the code below the comment "Mock the recipe to reuse the concat generator
>> code",
>> i believe you can use String.repeat() that was introduced recently.
> 
> Sure,
> 
>> 
>> The code that parse the the receipe can be in its own method to make the code
>> more readable,
>> this method returns the list and use the StringBuilder internally.
> 
> sure,
> 
>> 
>> In generateMHInlineCopy,
>>    element.get(0) and element.get(1) should be stored in local variables after
>>    "elements.size() == 2"
>>    will make the code more readable
> 
> sure,
> 
>> 
>> In simpleConcat(),
>>    should use a local variable 'mh', like the newString or newArrayWithSuffix
> 
> yes!
> 
> http://cr.openjdk.java.net/~redestad/8246152/open.01/


I don't think you need to check MAX_INDY_CONCAT_ARG_SLOTS anymore, given that now indy is based on invokeWithArguments instead of invoke.

And i think you should keep the null checks of constants upfront to not change the semantics (you are allowing constant not referenced by a recipe to be null). 

The catch should be:
   catch (Error | RuntimeException e) {
 if you want the StringConcatException to be propagated

all the other changes looks good.

> 
> Re-running tier1
> 
> /Claes

Rémi