[foreign-memaccess] 8227393: Add Constable support to the layout API

Jorn Vernee jbvernee at xs4all.nl
Tue Jul 9 17:01:38 UTC 2019


Looking at this again, I realized there's a problematic part w.r.t. 
ByteOrder and compile time constant folding (like condy-folding) that we 
might want to do later on. Let's say we compile on an LE system, and 
we're creating a native byte order ValueLayout. javac creates a 
trackable constant value, and then commits this to the constant pool 
using describeConstable, and it's going to be explicitly LE, instead of 
native order. When loading this constant on a system that is Big Endian, 
we are gonna get a ValueLayout that is supposed to be native byte order, 
i.e. BE, but is actually LE.

I think one way to get rid of that discrepancy is to modify 
java.nio.ByteOrder to be Constable also, and to have a flag for tracking 
if it's native order, then take this flag into account when creating a 
ConstantDesc to make sure to call `nativeOrder()` instead of loading the 
LE or BE constant directly.

Another way is to track if it's native byte order in ValueLayout itself, 
and choose a different bootstrap appropriately.

Thoughts on this?

Thanks,
Jorn

On 2019-07-09 17:01, Maurizio Cimadamore wrote:
> Updated webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8227393_v2/
> 
> (addressed all comments, including covariant overrides)
> 
> Thanks
> Maurizio
> 
> On 08/07/2019 16:22, Jorn Vernee wrote:
>> Hi,
>> 
>> Some comments:
>> 
>> - The constants added to AbstractLayout should probably be `final`? 
>> (Also minor naming; these use `XXX_CD` while ConstantDescs uses 
>> `CD_XXX`, might want to follow the latter)
>> - In MemoryHandles.java there are 2 imports added that are not used.
>> - In TestLayoutConstants.java, there seems to be a spurious line: 
>> `MemoryLayout.ofStruct(MemoryLayout.ofFloatingPoint(32), 
>> MemoryLayout.ofPadding(8));`.
>> 
>> Also, what do we want to do with covariant return types for various 
>> `describeConstable` methods? e.g. GroupLayout::describeConstable could 
>> return Optional<DynamicConstantDesc<GroupLayout>>, do we want to go 
>> that route?
>> 
>> Cheers,
>> Jorn
>> 
>> On 2019-07-08 16:17, Maurizio Cimadamore wrote:
>>> As suggested by Brian, there's no reason as to why MemoryLayout
>>> classes could not implement the Constable interface. After all, they
>>> are static description of some memory layout, and a constant pool 
>>> form
>>> for them can be defined (using condy). This patch does just that.
>>> 
>>> Webrev:
>>> 
>>> http://cr.openjdk.java.net/~mcimadamore/panama/8227393/
>>> 
>>> Now, there are some degree of freedom in doing this mapping; from
>>> other cases, I infer that the idiomatic way to implement constable
>>> would be to return a custom DynamicConstantDesc (e.g. enums return an
>>> EnumDesc, VarHandles a VarHandleDesc) - but in this case I'm not sure
>>> the exercise is worth it for two reasons:
>>> 
>>> * I can't see any 'interesting method' that could be added to a
>>> LayoutDesc - seems simpler to just work on the layouts themselves
>>> 
>>> * To canonicalize dynamic constants representing layouts, we'd still
>>> need some behavioral changes in the public SE API - which is a bit
>>> tricky given everything is defined in an incubator module
>>> 
>>> I'd say that what I have implemented is good enough for now, but of
>>> course we should reserve the right to change it later if/when the 
>>> code
>>> ends up in java.base.
>>> 
>>> Similar considerations hold for memory access VarHandles Constable
>>> support - on the one hand we could just add such support - on the
>>> other hand, to do that correctly we'd need some surgery in
>>> VarHandleDesc - which is again a SE API in java.base. I think when 
>>> the
>>> time comes, we'd add a new 'kind' to VarHandleDesc called MEMORY, and
>>> then add a suitable implementation of describeConstable under
>>> VarHandleMemoryAddressBase (as well as canonicalize support).
>>> 
>>> Note: I had to add a 'missing' BSM constant in ConstantDescs - namely
>>> for ConstantBootstraps#getStaticFinal. I hope this change is
>>> innocuous/standalone enough.
>>> 
>>> Cheers
>>> Maurizio


More information about the panama-dev mailing list