[foreign] RFR 8219822: Jextract should handle complex macro constants
Jorn Vernee
jbvernee at xs4all.nl
Thu Feb 28 13:01:05 UTC 2019
Hi Maurizio,
Some comments:
* For pointers, I thought casting to (long) would truncate the value on
Windows, since long is only 32 bits there, but in practice this does not
seem to be a problem... Any ways, we should probably make sure that that
case keeps working. Here's the test case I used:
```
diff -r 8f30f51887d9 test/jdk/com/sun/tools/jextract/ConstantsTest.java
--- a/test/jdk/com/sun/tools/jextract/ConstantsTest.java Thu Feb
28 12:34:49 2019 +0100
+++ b/test/jdk/com/sun/tools/jextract/ConstantsTest.java Thu Feb
28 13:29:40 2019 +0100
@@ -101,7 +101,8 @@
{ "CHAR_VALUE", int.class, equalsTo(104) }, //integer
char constants have type int
{ "MULTICHAR_VALUE", int.class, equalsTo(26728) },
//integer char constants have type int
{ "BOOL_VALUE", boolean.class, equalsTo(true) },
- { "ZERO_PTR", Pointer.class, pointerEqualsTo(0L) }
+ { "ZERO_PTR", Pointer.class, pointerEqualsTo(0L) },
+ { "F_PTR", Pointer.class,
pointerEqualsTo(0xFFFF_FFFF_FFFF_FFFFL) }
};
}
diff -r 8f30f51887d9 test/jdk/com/sun/tools/jextract/constants.h
--- a/test/jdk/com/sun/tools/jextract/constants.h Thu Feb 28
12:34:49 2019 +0100
+++ b/test/jdk/com/sun/tools/jextract/constants.h Thu Feb 28
13:29:40 2019 +0100
@@ -65,3 +65,4 @@
#define SUB SUP + 2 //dependency
#define ZERO_PTR (void*)0;
+#define F_PTR (void*) 0xFFFFFFFFFFFFFFFFLL; // all 1s
```
* (I know this was already there); Should the MacroParser.toNumber fast
path use Long.decode instead, to catch more values?
---
>> So we have a choice here:
>>
>> 1) we extract the value of the constant, and we create a plain
>> constant field - e.g.
>>
>> static final int FOO = 1;
>>
>> which will then work as before (as a true compile-time constant)
>>
>> 2) we aim for regularity, and we just translate as
>>
>> static final int FOO = lib.FOO();
My vote is on 1., and when condy eventually becomes available, switch
the more complex constants to that.
---
Also, I tested this out with the winreg.h case I had, e.g.:
```
typedef struct HKEY__ {
int unused;
} *HKEY;
typedef unsigned __int64 ULONG_PTR;
typedef long LONG;
#define HKEY_LOCAL_MACHINE (( HKEY ) (ULONG_PTR)((LONG)0x80000002) )
```
But unfortunately no constant was generated for HKEY_LOCAL_MACHINE.
(Note that __int64 is an MSVC extension, maybe that's the problem?)
---
On a bit of a tangent; I'm wondering if we really need 2 annotations for
this? Could we instead use one annotation with a `Class carrier` and
`String value` instead? i.e. we encode all constant values as Strings in
the annotation and then reparse them when spinning the header impl.
Jorn
Maurizio Cimadamore schreef op 2019-02-28 00:19:
> Hi,
> here's a new revision where I just slightly improved the TestConstants
> test, to be more uniform (instead of passing expected constant values,
> the test now passes checker predicates, which is more scalable).
>
> Webrev:
>
> http://cr.openjdk.java.net/~mcimadamore/panama/8219822_v3/
>
> Cheers
> Maurizio
>
>
> On 27/02/2019 19:02, Maurizio Cimadamore wrote:
>> Hi,
>> this patch regularizes support for constant macros (and enum constants
>> too).
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8219822_v2/
>>
>> The goal of this patch is to stop having jextract generating 'default'
>> methods for constants. Instead, two new annotations are introduced:
>>
>> @NativeNumericConstant
>> @NativeStringConstant
>>
>> The first takes a long value, the second a String value.
>>
>> So, given C code like this:
>>
>> #define FOO 1
>>
>> instead of generating a method like this:
>>
>> default int FOO() {
>> return 1;
>> }
>>
>> We will now generate this:
>>
>> @NativeNumericConstant(1L)
>> int FOO();
>>
>> And then binder will then generate the implementation.
>>
>> Decoupling the generation of the constant methods from jextract has a
>> big advantage: we can now support complex constant types, such as
>> strings (which need to be turned to Pointer<Byte>) and pointer
>> constants (e.g. (void*)0). Note that, whenever we need some 'constant'
>> pointer, we need to allocate, which means we need a scope - which we
>> don't have at extraction time.
>>
>> Efficiency-wise, the binder-generated implementation should be as good
>> as the jextract one - it just ldc a constant (which can even be a
>> pointer constant, via CP patching!) and then returns it.
>>
>> In order to support pointer constants of the kind:
>>
>> #define PTR (void*)0
>>
>> I also tweaked MacroParser, so that now two attempts can be made at
>> evaluating the macro:
>>
>> 1) a first attempt is made using a snippet like this:
>>
>> __auto_type jextract$var = PTR;
>>
>> If this fails (and it does fail in this case, as clang cursor
>> evaluation API doesn't detect this constant), then:
>>
>> 2) we look at the type of the cursor, if the type is a pointer type,
>> then we trigger another snippet parse, this time of:
>>
>> __auto_type jextract$var = (long)PTR;
>>
>> The extra cast will effectively reinterpret the value as a numeric
>> constant. Then we'll create a macro whose type is the pointer type
>> determined at (1) and whose value is the one determined at (2).
>>
>>
>> Note that, since we only execute step (2) when the type of the cursor
>> in (1) is a pointer type, we don't end up executing the second step
>> very often. I also tried some of the examples, and the extraction time
>> is the same as before this patch.
>>
>>
>> There is a slight issue with AsmCodeFactoryExt: ideally, we'd like for
>> constant methods to be turned into static constants here; and we'd
>> like to initialize such constants to the result of the computation of
>> the bound constant method - that is, if the library has a method:
>>
>> @NativeNumericConstant(1L)
>> int FOO();
>>
>> Then, the static wrapper should have the following static field:
>>
>> static final int FOO = lib.FOO();
>>
>> However, if we do so, a test will fail, namely LibEnumTest, as that
>> test uses the constants of the static wrapper as labels in a 'switch'
>> statement; this only works if the constant field has the
>> 'ConstantValue' attribute, and that in turn is only possible if the
>> constant is 'simple' - using a condy-based ConstantValue would be a
>> way out, but that is not something that is supported currently,
>> neither by the VM nor by javac.
>>
>> So we have a choice here:
>>
>> 1) we extract the value of the constant, and we create a plain
>> constant field - e.g.
>>
>> static final int FOO = 1;
>>
>> which will then work as before (as a true compile-time constant)
>>
>> 2) we aim for regularity, and we just translate as
>>
>> static final int FOO = lib.FOO();
>>
>>
>> The patch I've submitted does (1) for simple numeric constants and
>> falls back to (2) for complex constants. I'm open to suggestions as to
>> whether it's better to keep this behavior, or simplify it by always
>> aiming for (2).
>>
>> Maurizio
>>
>>
>>
>>
More information about the panama-dev
mailing list