RFR: JDK-8205336: Modularize allocations in assembler

[Erik Österlund]

Hi Roman,

On 2018-06-21 11:16, Roman Kennke wrote:
> Am 21.06.2018 um 11:14 schrieb Erik Österlund:
>> Hi Roman,
>>
>> I'm looking at the x86 code. I noticed that We used to always call
>> incr_allocated_bytes after calling eden_allocate. However, now you moved
>> incr_allocated_bytes into eden_allocate, which makes sense, and I like
>> that. But it is only incremented if inline contig alloc is supported,
>> otherwise it calls the slowpath without incrementing allocation bytes.
>> So that is seemingly changing the behaviour from before. Is that a bug
>> in your code, a bug in the current code, or am I misunderstanding this
>> code?
> It did generate the code for incr_allocated_bytes(), but always
> unconditionally jumped over it if inline contig alloc is not supported.
> I wouldn't call it a bug, but it's bogus code.

I see. So we previously generated dead accounting code that we always 
jumped over. Eww.

>   It looks to me that your code is fixing a bug in the case when not
>> using TLAB and not using contig allocation. Today it seems to me that we
>> will always call the slowpath that always increments allocated bytes,
>> and then always account it one more time in the interpreter. There might
>> be more subtle differences as well that I have not considered yet.
>> Thoughts?
> As far as I can tell, the incr_allocated_bytes() issue is the only
> difference. And it's not really a bug, just a slight inefficiency in
> that it generates some dead instructions.

Yeah, your code makes more sense. It is very confusing to have incorrect 
accounting code generated, and jump over it, and I'd rather not generate 
that code.

>> Otherwise, I think this looks reasonable.
> Ok, thanks for reviewing. Can you verify that what I said above is
> correct? I'll push it through Mach5 in the meantime.

Confirmed. Looks good.

Thanks,
/Erik

> Thanks,
> Roman
>
>
>> Thanks,
>> /Erik
>>
>> Other than that, it looks reasonable.
>>
>> On 2018-06-19 21:46, Roman Kennke wrote:
>>> Am 19.06.2018 um 19:56 schrieb Andrew Haley:
>>>> On 06/19/2018 06:14 PM, Roman Kennke wrote:
>>>>> Can I please get reviews?
>>>> AArch64 looks OK, but it makes no sense for Register thread to be an
>>>> argument to eden_allocate: it's rthread.  Otherwise fine.
>>>>
>>>> Unless you've messed up copying some of the code, but it'd be hard
>>>> to check all that by hand without half a day to spare...  :-(
>>> Right. It was pre-existing (probably modeled after x86), and I wanted to
>>> make it a 1:1 copy/refactoring job as much as possible, but yeah this is
>>> pointless in aarch64. Let's remove it:
>>>
>>> Incremental:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8205336/webrev.01.diff/
>>> Full:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8205336/webrev.01/
>>>
>>> Good now?
>>>
>>> Thanks for reviewing!
>>> Roman
>>>
>