RFR: JDK-8218074 - Update Graal
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Mar 12 03:21:56 UTC 2019
Igor confirmed that we don't need to patch StandardGraphBuilderPlugins.java.
Vladimir
On 3/11/19 5:44 PM, Vladimir Kozlov wrote:
> I looked through all changes done in OpenJDK since last 'Graal Update'.
>
> I found only few issues - see following. The rest changes in overwritten-diffs.txt are not overwritten, they are kept -
> we don't need to re-apply them.
>
> -----------------------------------------------------------------------
> New Igor's fix for 8196568 overwrote fix for 8217678 in StandardGraphBuilderPlugins.java file
>
> http://hg.openjdk.java.net/jdk/jdk/rev/b693b0d2053d
>
> Igor, please confirm that your changes also fix 8217678 and we don't need to apply it again.
>
> -----------------------------------------------------------------------
> Next changes were reverted: JDK-8217289
>
> http://hg.openjdk.java.net/jdk/jdk/rev/0040f89feb78
>
> Jesper, I would suggest to revert your changes to corresponding file:
>
> hg revert
> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/BigIntegerIntrinsicsTest.java
>
>
> I asked Patric to file PR for it month ago. Does anyone known if it was filed? I sent e-mail to him.
>
> ----------------------------------------------------------------------
> Several files has incorrect Copyright years change from 2019 back to 2018. For example:
>
> http://cr.openjdk.java.net/~jwilhelm/8218074/webrev.00/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.lir.amd64/src/org/graalvm/compiler/lir/amd64/AMD64ArrayIndexOfOp.java.udiff.html
>
>
> Please Fix.
>
> Doug, we need PR to update Copyright years in master repo so they match.
>
> Thanks,
> Vladimir
>
> On 3/11/19 1:55 PM, jesper.wilhelmsson at oracle.com wrote:
>>> On 11 Mar 2019, at 18:56, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>
>>> I am fine doing small manual labor to find what is missing and what is overwritten. For that I need some information.
>>>
>>> First, how overwritten-diffs.txt was created? If it is created by using 'hg' to find all changes done in OpenJDK
>>> since last 'Update Graal' then I would understand why changes are in this file.
>>>
>>> Next, can you attach to bug report original patch created by updategraalinopenjdk before applying it to OpenJDK
>>> sources? Do we have it? This way I can compare overwritten-diffs.txt with it and find if there duplicate and I don't
>>> need to re-apply changes.
>>
>> The mx script applies the patch automatically so I don't have it as a separate patch. But the only changes that my
>> script does after the mx script has applied the patch is in the test directory, so the patch available in the webrev
>> should be what the mx script created if you ignore anything in test.
>>
>>> Some changes were pushed into OpenJDK because we did not want to wait a fix for months - last updated was done 3
>>> months ago!!! If we do more frequent updates overwritten-diffs.txt will be empty or very small. Manual work should
>>> not take a lot of time in such case.
>>
>> Yes, the intention is to do this more often, hopefully on a weekly basis going forward, and that should minimize the
>> need to push to the OpenJDK as you say. I hope this "first" integration is a one-off in that respect. Obviously how
>> often we can integrate depends on how much time the review takes ;-)
>> /Jesper
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 3/11/19 2:45 AM, Doug Simon wrote:
>>>>> On 11 Mar 2019, at 01:56, jesper.wilhelmsson at oracle.com wrote:
>>>>>
>>>>> Hi Vladimir,
>>>>>
>>>>> I followed the given instructions and used mx to create the diff:
>>>>>
>>>>> mx --java-home=$JAVA_HOME updategraalinopenjdk cleanjdk/open 12
>>>>>
>>>>> I assume JDK-8215322 is in there because it was actually pushed to OpenJDK and now it was overwritten (with the
>>>>> same diff). It seems the mx script do not realize that it's the same cahange.
>>>> Correct. To detect that we would have to add logic for diffs of diffs and the complexity didn’t seem worth it. I’d
>>>> more more than happy to review a smart person’s attempt at adding it though ;-)
>>>> -Doug
>>>>> The diff and patch and all changes made was done by the new script, so no extra human labor has been put into it at
>>>>> this point. If a human is required to modify the overwritten-diff I suggest that is part of the review step (as
>>>>> now) and that a new patch is simply added to the subtask.
>>>>>
>>>>> I have removed JDK-8215322 from the diff now.
>>>>>
>>>>> Thanks,
>>>>> /Jesper
>>>>>
>>>>>> On 9 Mar 2019, at 18:42, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>>
>>>>>> Hi Jesper,
>>>>>>
>>>>>> Thank you for doing this.
>>>>>>
>>>>>> I looked on 8220387 to see what changes your webrev does not have and it is strange. You should not have such big
>>>>>> difference. How you prepared overwritten-diffs.txt?
>>>>>>
>>>>>> For example, JAOTC changes "8215322: add @file support to jaotc" are in overwritten-diffs.txt. But changeset is
>>>>>> listed in the 8218074 Graal changelog list [1].
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> [1] 8433917 Wed Dec 19 16:20:00 2018 -0800 Igor Ignatyev [GR-13142] Add at-file support to jaotc.
>>>>>>
>>>>>> On 3/9/19 12:46 AM, jesper.wilhelmsson at oracle.com wrote:
>>>>>>> Hi,
>>>>>>> Please review the patch to integrate the latest Graal changes into OpenJDK.
>>>>>>> Graal tip to integrate: db79f81716886b7883370cd6ea1bbf5c42966fa5
>>>>>>> JBS duplicates fixed by this integration:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8217161
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218698
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218859
>>>>>>> JBS duplicates deferred to the next integration:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8214947
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8218074
>>>>>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8218074/webrev.00/
>>>>>>> This integration did overwrite changes already in place in OpenJDK. A subtask was created to hold the diff:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220387
>>>>>>> Thanks,
>>>>>>> /Jesper
>>>>>
>>
More information about the hotspot-compiler-dev
mailing list