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

[Maurizio Cimadamore]

Actually, I disagree. Once created, a Layout endianness is part of the 
layout itself. So, the underlying CP representation should just reflect 
whatever is in the layout.

The problem you raise is more related to the fact that an assumption 
made at compile-time (e.g. on endianness) can be invalidated at runtime, 
but that's more an issue that has to be addressed when (and if) javac 
support this. For instance, when investigating intrinsification 
opportunities for String.format a similar problem arose: the Locale used 
to format the string was not guaranteed to be preserved from compile- to 
run- time. The solution was to come up with a BSM which check the 
compile-time assumptions against the run-time ones and fallback to usual 
code when such invariants were violated.

So, there are two issues: one is to be able to embed layout constants in 
the constant pool, which is addressed by this patch. A second (and 
arguably deeper issues) is how to reliably intrinsify layouts, so that 
we don't accidentally depend on conditions that were present at 
compile-time, but are invalidated at run-time. To me these are separate 
issues.

I also think that at some point we will need to address the stance on 
endianness - right now we expose some 'endianness-inferring' factories 
which are, I think, misleading. I think a better approach would be for 
the factories to be always explicit, and then to have a set of constants 
which users can import, which 'do the right thing' (depending on what 
'the right' thing is in a given context).

Also, we have the option of, if we wanted to, keep track if an 
endianness-inferring factory was used and, if so, simply refuse to 
return a ConstantDesc. This means that we would only be able to create 
ConstantDesc for things that have an explicitly provided endianness, 
which would then side-step the issues you mentioned.

Maurizio

On 09/07/2019 18:01, Jorn Vernee wrote:
> 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