I think you can just predicate the emission of these stubs for !UseTLAB, and
not mess with the CPU-specific code. What do you think?
diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp
b/src/hotspot/share/c1/c1_LIRGenerator.cpp
--- a/src/hotspot/share/c1/c1_LIRGenerator.cpp
+++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp
@@ -674,7 +674,7 @@
void LIRGenerator::new_instance(LIR_Opr dst, ciInstanceKlass* klass, bool
is_unresolved, LIR_Opr scratch1, LIR_Opr scratch2, LIR_Opr scratch3, LIR_Opr
scratch4, LIR_Opr klass_reg, CodeEmitInfo* info) {
klass2reg_with_patching(klass_reg, klass, info, is_unresolved);
// If klass is not loaded we do not know if the klass has finalizers:
- if (UseFastNewInstance && klass->is_loaded()
+ if (UseFastNewInstance && !UseTLAB && klass->is_loaded()
&& !Klass::layout_helper_needs_slow_path(klass->layout_helper())) {
Runtime1::StubID stub_id = klass->is_initialized() ?
Runtime1::fast_new_instance_id : Runtime1::fast_new_instance_init_check_id;
igor
> On Jul 20, 2018, at 12:37 PM, JC Beyler <jcbey...@google.com> wrote:
>
> Yes that is right, this is the latest:
> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.03/
>
> I apologize for the multiple threads and confusion,
> Jc
>
> On Fri, Jul 20, 2018 at 11:22 AM serguei.spit...@oracle.com <
> serguei.spit...@oracle.com> wrote:
>
>> Thank you a lot, Vladimir!
>> Yes, the webrev.03 is the latest.
>> Jc, will correct us if it is not right.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 7/20/18 10:52, Vladimir Kozlov wrote:
>>> I asked Igor V. to look.
>>>
>>> Seems like review is done in an other thread which does not have bug
>>> id in subject. Currently webrev.03
>>>
>>> Vladimir
>>>
>>> On 7/19/18 4:32 PM, serguei.spit...@oracle.com wrote:
>>>> Thanks, Rahul!
>>>> In fact, there no good experts for this area in the serviceability team.
>>>> It would be much better if anyone from the Compiler team could do it.
>>>>
>>>> Vladimir K.,
>>>>
>>>> Is there anyone from the Compiler team available to review this?
>>>> Otherwise, I could try to review it but am not sure about my review
>>>> quality.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 7/19/18 00:48, Rahul Raghavan wrote:
>>>>> RFR (S) 8207252: C1 still does eden allocations when TLAB is enabled
>>>>>
>>>>> (just adding + hotspot-compiler-dev also)
>>>>>
>>>>>
>>>>> On Wednesday 18 July 2018 09:51 PM, JC Beyler wrote:
>>>>> Subject Was:
>>>>> Re: RFR (S): C1 still does eden allocations when TLAB is enabled
>>>>>
>>>>> + serviceability-dev
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Could anyone else give me a review of this webrev and check/test the
>>>>> various architecture changes?
>>>>>
>>>>> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.02/
>>>>>
>>>>>
>>>>> Thanks for all your help!
>>>>> Jc
>>>>>
>>>>>
>>>>>> On Mon, Jul 16, 2018 at 2:58 PM JC Beyler <jcbey...@google.com>
>> wrote:
>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Here is a webrev that does all the architectures in the same way:
>>>>>>> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.02/
>>>>>>>
>>>>>>> Could anyone review the other architectures and test?
>>>>>>> - arm, sparc & aarch64 are also modified now to follow the same
>>>>>>> "if no
>>>>>>> tlab, then consider eden space allocation" logic.
>>>>>>>
>>>>>>> Thanks for your help!
>>>>>>> Jc
>>>>>>>
>>>>>>> On Fri, Jul 13, 2018 at 9:16 PM JC Beyler <jcbey...@google.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Kim,
>>>>>>>>
>>>>>>>> I opened this bug
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8190862
>>>>>>>>
>>>>>>>> and now I've done an update:
>>>>>>>> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.01/
>>>>>>>>
>>>>>>>> I basically have done your nits but also removed the try_eden (it
>>>>>>>> was
>>>>>>>> used to bind a label but was not used). I updated the comments to
>>>>>>>> use the
>>>>>>>> one you preferred.
>>>>>>>>
>>>>>>>> I still have to do the other architectures though but at least we
>>>>>>>> seem to
>>>>>>>> have a consensus on this architecture, correct?
>>>>>>>>
>>>>>>>> Thanks for the review,
>>>>>>>> Jc
>>>>>>>>
>>>>>>>> On Fri, Jul 13, 2018 at 5:17 PM Kim Barrett <kim.barr...@oracle.com
>>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>>> On Jul 13, 2018, at 4:54 PM, JC Beyler <jcbey...@google.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Yes, you are right, I did those changes due to:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8194084
>>>>>>>>>>
>>>>>>>>>> If Robbin agrees to this change, and if no one sees an issue,
>>>>>>>>>> I'll go
>>>>>>>>> ahead
>>>>>>>>>> and propagate the change across architectures.
>>>>>>>>>>
>>>>>>>>>> Thanks for the review, I'll wait for Robbin (or anyone else's
>>>>>>>>>> comment
>>>>>>>>> and
>>>>>>>>>> review) :)
>>>>>>>>>> Jc
>>>>>>>>>>
>>>>>>>>>> On Fri, Jul 13, 2018 at 1:08 PM John Rose <john.r.r...@oracle.com
>>>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Jul 13, 2018, at 10:23 AM, JC Beyler <jcbey...@google.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I'm not sure if we had left this case intentionally or not
>>>>>>>>>>> but, if we
>>>>>>>>> want
>>>>>>>>>>> it all to be consistent, we should perhaps fix it.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Well, you put in that logic last February, so unless somebody
>>>>>>>>>>> speaks
>>>>>>>>> up
>>>>>>>>>>> quickly, I support your adjusting it to be the way you want it.
>>>>>>>>>>>
>>>>>>>>>>> Doing "hg grep -u supports_inline_contig_alloc -I
>>>>>>>>>>> src/hotspot/share"
>>>>>>>>>>> suggests that the GC group is most active in touching this
>>>>>>>>>>> feature.
>>>>>>>>>>> If Robbin is OK with it, there's your reviewer.
>>>>>>>>>>>
>>>>>>>>>>> FWIW, you can use me as a reviewer, but I'd get one other person
>>>>>>>>>>> working on the GC to OK it.
>>>>>>>>>>>
>>>>>>>>>>> — John
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Jc
>>>>>>>>>
>>>>>>>>> Robbin is on vacation; you might not hear from him for a while.
>>>>>>>>>
>>>>>>>>> I'm assuming you'll open a new bug for this?
>>>>>>>>>
>>>>>>>>> Except for a few minor nits (below), this looks okay to me.
>>>>>>>>>
>>>>>>>>> The comment at line 1052 needs updating.
>>>>>>>>>
>>>>>>>>> pre-existing: The retry_tlab label declared on line 1054 is unused.
>>>>>>>>>
>>>>>>>>> pre-existing: The try_eden label declared on line 1054 is bound at
>>>>>>>>> line 1058, but unreferenced.
>>>>>>>>>
>>>>>>>>> I like the wording of the comment at 1139 better than the
>>>>>>>>> wording at
>>>>>>>>> 1016.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jc
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jc
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>
>
> --
>
> Thanks,
> Jc
