Discussion:
[m-dev.] argument packing
Zoltan Somogyi
2018-05-15 10:04:09 UTC
Permalink
I committed the diff that enabled the packing of sub-word-sized
integers and dummies in heap cells about a week ago. Have you
guys tried out those features (after turning them on, since they are
OFF by default)?

The reason I ask is that traditionally, we computed the offset of each
argument into the memory cell when we generated code for a unification.
Since repeating this work for *every* unification is wasted work, that diff
added code to compute these offsets just once, when the representation
of each function symbol is decided, but it also left the old code in place,
with assertions checking that the new method yields the exact same offset
as the old method. I would like to delete the code that calculates the offsets
the old way, along with the assertions, before I start work on my next
diff involving argument packing. However, I would not want to take
that step until those assertions have passed your toughest tests.

So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.

Zoltan.
Peter Wang
2018-05-15 13:52:31 UTC
Permalink
Post by Zoltan Somogyi
I committed the diff that enabled the packing of sub-word-sized
integers and dummies in heap cells about a week ago. Have you
guys tried out those features (after turning them on, since they are
OFF by default)?
The reason I ask is that traditionally, we computed the offset of each
argument into the memory cell when we generated code for a unification.
Since repeating this work for *every* unification is wasted work, that diff
added code to compute these offsets just once, when the representation
of each function symbol is decided, but it also left the old code in place,
with assertions checking that the new method yields the exact same offset
as the old method. I would like to delete the code that calculates the offsets
the old way, along with the assertions, before I start work on my next
diff involving argument packing. However, I would not want to take
that step until those assertions have passed your toughest tests.
So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
Hi Zoltan,

I made a quick modification to one of my programs and found a problem
(not an assertion failure though). When constructing a term of type:

:- type thread
---> thread( ... , int32, int32).

the compiler generated this code in hlc.gc:

MR_hl_field(MR_mktag(0), base, 5) = (((uint32_t) Var_32) | ((((uint32_t) Var_33) << (MR_Integer) 32)));

for which gcc reports:

warning: left shift count >= width of type [-Wshift-count-overflow]
warning: assignment makes pointer from integer without a cast [-Wint-conversion]

This is with mmc rotd-2018-05-12. I simply passed --allow-packing-ints
which I realise is wrong.

Peter

PS. YesLogic don't have any code using sub-word-sized integers yet.
Zoltan Somogyi
2018-05-16 13:49:21 UTC
Permalink
Post by Peter Wang
I made a quick modification to one of my programs and found a problem
:- type thread
---> thread( ... , int32, int32).
MR_hl_field(MR_mktag(0), base, 5) = (((uint32_t) Var_32) | ((((uint32_t) Var_33) << (MR_Integer) 32)));
warning: left shift count >= width of type [-Wshift-count-overflow]
warning: assignment makes pointer from integer without a cast [-Wint-conversion]
The attached diff fixes this problem. Thanks for reporting it.
Post by Peter Wang
This is with mmc rotd-2018-05-12. I simply passed --allow-packing-ints
which I realise is wrong.
It was enough to demonstrate this problem :-)
Post by Peter Wang
PS. YesLogic don't have any code using sub-word-sized integers yet.
I intend to add some to the compiler. Specificially, I mean to use uint8s
to represent both primary tags, and the number of bits in arg_pos_widths
(and maybe in other contexts) where the number of bits is guaranteed to be
in the range 0 to 64. This should give us some dogfood.

By the way, Julien: You added arithmetic and logical operations on
sub-word-sized integers to builtin_ops.m. Did you intend to add any of
the conversion operations, such as int8 to int? Did you reject it as requiring
expansion of the expansion of the existing builtin_op framework?
For work on ptags and num_bits, it would be nice if at least the
unchecked conversions (casts) could be builtin.

Zoltan.
Julien Fischer
2018-05-16 14:10:40 UTC
Permalink
Post by Zoltan Somogyi
Post by Peter Wang
PS. YesLogic don't have any code using sub-word-sized integers yet.
I intend to add some to the compiler. Specificially, I mean to use uint8s
to represent both primary tags, and the number of bits in arg_pos_widths
(and maybe in other contexts) where the number of bits is guaranteed to be
in the range 0 to 64. This should give us some dogfood.
By the way, Julien: You added arithmetic and logical operations on
sub-word-sized integers to builtin_ops.m. Did you intend to add any of
the conversion operations, such as int8 to int? Did you reject it as requiring
expansion of the expansion of the existing builtin_op framework?
No, it was simply something that didn't occur to me as being necessary
at the time I added the others.
Post by Zoltan Somogyi
For work on ptags and num_bits, it would be nice if at least the
unchecked conversions (casts) could be builtin.
I have no objections.

Julien.
Julien Fischer
2018-05-16 00:36:47 UTC
Permalink
Hi Zoltan,
Post by Zoltan Somogyi
I committed the diff that enabled the packing of sub-word-sized
integers and dummies in heap cells about a week ago. Have you
guys tried out those features (after turning them on, since they are
OFF by default)?
The reason I ask is that traditionally, we computed the offset of each
argument into the memory cell when we generated code for a unification.
Since repeating this work for *every* unification is wasted work, that diff
added code to compute these offsets just once, when the representation
of each function symbol is decided, but it also left the old code in place,
with assertions checking that the new method yields the exact same offset
as the old method. I would like to delete the code that calculates the offsets
the old way, along with the assertions, before I start work on my next
diff involving argument packing. However, I would not want to take
that step until those assertions have passed your toughest tests.
The first of those tougher tests would be compiling the standard
library in the csharp grade with the new optimizations turned on:

Making Mercury/css/thread.mvar.cs
Making Mercury/css/thread.semaphore.cs
Making mer_std.dll
** Error making `mer_std.dll'.
Mercury/css/erlang_rtti_implementation.cs(740,113): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
Mercury/css/erlang_rtti_implementation.cs(741,3): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
... <more snipped>
Mercury/css/erlang_rtti_implementation.cs(1445,30): error CS1729: The type
`mercury.erlang_rtti_implementation.Maybe_pseudo_type_info_0.Pseudo_1'
does not contain a constructor that takes `0' arguments
... error log truncated, see `mer_std.err' for the complete log.
gmake[1]: *** [libmer_std.install] Error 1
Post by Zoltan Somogyi
So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
I'll try a bunch of Opturion's stuff with the new optimizations enabled.
What's the best way to determine if the new optimizations have been
applied?

Julien.
Julien Fischer
2018-05-16 02:14:18 UTC
Permalink
Post by Julien Fischer
Post by Zoltan Somogyi
I committed the diff that enabled the packing of sub-word-sized
integers and dummies in heap cells about a week ago. Have you
guys tried out those features (after turning them on, since they are
OFF by default)?
The reason I ask is that traditionally, we computed the offset of each
argument into the memory cell when we generated code for a unification.
Since repeating this work for *every* unification is wasted work, that diff
added code to compute these offsets just once, when the representation
of each function symbol is decided, but it also left the old code in place,
with assertions checking that the new method yields the exact same offset
as the old method. I would like to delete the code that calculates the offsets
the old way, along with the assertions, before I start work on my next
diff involving argument packing. However, I would not want to take
that step until those assertions have passed your toughest tests.
The first of those tougher tests would be compiling the standard
And similarly for the Java grade:

Making Java class files
Mercury/javas/jmercury/erlang_rtti_implementation.java:808: error:
incompatible types: int cannot be converted to DuArgLocn
erlang_rtti_implementation__field_locns_evaluated_pseudo_type_info_thunk_0_2[0]
= new jmercury.runtime.DuArgLocn[] {-1,
^
Mercury/javas/jmercury/erlang_rtti_implementation.java:809: error:
incompatible types: int cannot be converted to DuArgLocn
0,
^
Mercury/javas/jmercury/erlang_rtti_implementation.java:810: error:
incompatible types: int cannot be converted to DuArgLocn
-10};
^
Mercury/javas/jmercury/erlang_rtti_implementation.java:808: error:
incompatible types: DuArgLocn[] cannot be converted to DuArgLocn
erlang_rtti_implementation__field_locns_evaluated_pseudo_type_info_thunk_0_2[0]
= new jmercury.runtime.DuArgLocn[] {-1,
^
Mercury/javas/jmercury/erlang_rtti_implementation.java:1002: error:
incompatible types: int cannot be converted to DuArgLocn
erlang_rtti_implementation__field_locns_maybe_pseudo_type_info_0_0[0]
= new jmercury.runtime.DuArgLocn[] {-1,
^
... error log truncated, see `mer_std.err' for the complete log.
gmake[1]: *** [libmer_std] Error 1
gmake[1]: Leaving directory
`/mnt/opturion/jfischer/mercury-6.git/library'
gmake: *** [library] Error 2

Julien.
Zoltan Somogyi
2018-05-16 23:26:10 UTC
Permalink
Post by Julien Fischer
The first of those tougher tests would be compiling the standard
The new optimizations are not applicable to the C# and Java grades.
This is because they require working with a single addressable word
containing two or more arguments, but the C# and Java grades,
along with hl.gc, use the high level data representation, which
requires the fields of memory cells to be identified by argument names,
not by offset. A word containing two or more arguments has no name.
Post by Julien Fischer
Making Mercury/css/thread.mvar.cs
Making Mercury/css/thread.semaphore.cs
Making mer_std.dll
** Error making `mer_std.dll'.
Mercury/css/erlang_rtti_implementation.cs(740,113): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
Mercury/css/erlang_rtti_implementation.cs(741,3): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
... <more snipped>
Mercury/css/erlang_rtti_implementation.cs(1445,30): error CS1729: The type
`mercury.erlang_rtti_implementation.Maybe_pseudo_type_info_0.Pseudo_1'
does not contain a constructor that takes `0' arguments
... error log truncated, see `mer_std.err' for the complete log.
gmake[1]: *** [libmer_std.install] Error 1
That, and its Java equivalent in your following email, was almost certainly
caused by the optimization accidentally being left *on*. Please try the
attached diff, which turns them off in high-level-data grades. For me,
it generates the same code for tools/make_java_csharp_diff
as a compiler I installed on 2018 may 2, but that is not as though
a test as a bootcheck. I intend to commit it sometime tomorrow
(minus the change to options.m) if it passes further bootchecks.
Post by Julien Fischer
So if you have compiled the Mercury programs you are working on
Post by Zoltan Somogyi
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
I'll try a bunch of Opturion's stuff with the new optimizations enabled.
What's the best way to determine if the new optimizations have been
applied?
There is no really simple way. You have to look at some code that
constructs or deconstructs a term that contains two or more adjacent
arguments whose type is either dummy, or a sub-word-sized integer,
and then check whether it packs or unpacks those arguments. Finding
the right code to check is much easier if you compare the generated code
to the code generated without --allow-packing-ints and --allow-packing-dummies.

Zoltan.
Julien Fischer
2018-05-17 02:11:28 UTC
Permalink
Post by Zoltan Somogyi
Post by Julien Fischer
Making Mercury/css/thread.mvar.cs
Making Mercury/css/thread.semaphore.cs
Making mer_std.dll
** Error making `mer_std.dll'.
Mercury/css/erlang_rtti_implementation.cs(740,113): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
Mercury/css/erlang_rtti_implementation.cs(741,3): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
... <more snipped>
Mercury/css/erlang_rtti_implementation.cs(1445,30): error CS1729: The type
`mercury.erlang_rtti_implementation.Maybe_pseudo_type_info_0.Pseudo_1'
does not contain a constructor that takes `0' arguments
... error log truncated, see `mer_std.err' for the complete log.
gmake[1]: *** [libmer_std.install] Error 1
That, and its Java equivalent in your following email, was almost certainly
caused by the optimization accidentally being left *on*. Please try the
attached diff, which turns them off in high-level-data grades. For me,
it generates the same code for tools/make_java_csharp_diff
as a compiler I installed on 2018 may 2, but that is not as though
a test as a bootcheck. I intend to commit it sometime tomorrow
(minus the change to options.m) if it passes further bootchecks.
I've just bootchecked the Java grade with the attached diff applied.
(I'll do the C# one shortly, but I expect that it should now work too.)

Julien.
Julien Fischer
2018-05-17 15:17:05 UTC
Permalink
Post by Julien Fischer
On Wed, 16 May 2018 10:36:47 +1000 (AEST), Julien Fischer
Post by Julien Fischer
Making Mercury/css/thread.mvar.cs
Making Mercury/css/thread.semaphore.cs
Making mer_std.dll
** Error making `mer_std.dll'.
Mercury/css/erlang_rtti_implementation.cs(740,113): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
Mercury/css/erlang_rtti_implementation.cs(741,3): error CS0029: Cannot
implicitly convert type `int' to `mercury.runtime.DuArgLocn'
... <more snipped>
Mercury/css/erlang_rtti_implementation.cs(1445,30): error CS1729: The type
`mercury.erlang_rtti_implementation.Maybe_pseudo_type_info_0.Pseudo_1'
does not contain a constructor that takes `0' arguments
... error log truncated, see `mer_std.err' for the complete log.
gmake[1]: *** [libmer_std.install] Error 1
That, and its Java equivalent in your following email, was almost certainly
caused by the optimization accidentally being left *on*. Please try the
attached diff, which turns them off in high-level-data grades. For me,
it generates the same code for tools/make_java_csharp_diff
as a compiler I installed on 2018 may 2, but that is not as though
a test as a bootcheck. I intend to commit it sometime tomorrow
(minus the change to options.m) if it passes further bootchecks.
I've just bootchecked the Java grade with the attached diff applied.
(I'll do the C# one shortly, but I expect that it should now work too.)
The C# grade also bootchecks with the diff applied.

Julien.
Zoltan Somogyi
2018-05-22 11:47:13 UTC
Permalink
Post by Julien Fischer
The C# grade also bootchecks with the diff applied.
Here is the updated diff which I am committing. It addresses
both of your concerns, and another I found myself, which is that
num_ptag_bits must *not* be set to zero for hl.gc.

Sorry for the delay; my laptop lost its builtin wifi in the heat. It has
done so several times in the past, but it looks to be permanent this time.
It took me time to buy and install an external wifi adapter.

Zoltan.

Peter Wang
2018-05-17 00:57:33 UTC
Permalink
diff --git a/compiler/handle_options.m b/compiler/handle_options.m
index dce8cd8..6695702 100644
--- a/compiler/handle_options.m
+++ b/compiler/handle_options.m
@@ -1344,14 +1344,17 @@ convert_options_to_globals(OptionTable0, OpMode, Target,
),
% Argument packing only works on C back-ends with low-level data.
- % In the future, we may want to use C bit-field syntax for high-level data.
- % For other back-ends, any RTTI code will need to be updated to cope with
- % packed arguments.
- %
- % Only C targets may store a constructor argument across two words.
- option_implies(highlevel_data, arg_pack_bits, int(0), !Globals),
- (
+ % In the future, we may want to use C bit-field syntax on C backends
+ % with high-level data. For the other target languages, implementing
+ % argument packing will not just a lot of work on RTTI, but also
+ % generalizing field addressing, to allow both single fields and
+ % a group of adjacent fields packed into a single word to be
+ % addressed via a mechanism other than an argument's name.
+ globals.lookup_bool_option(!.Globals, highlevel_data, HighLevelData),
+ ( if
Target = target_c,
+ HighLevelData = yes
+ then
globals.lookup_int_option(!.Globals, arg_pack_bits, ArgPackBits0),
globals.lookup_int_option(!.Globals, bits_per_word, BitsPerWord),
% If --arg-pack-bits is negative then it means use all word bits.
HighLevelData = no

Peter
Julien Fischer
2018-05-17 02:12:44 UTC
Permalink
diff --git a/compiler/handle_options.m b/compiler/handle_options.m
index dce8cd8..6695702 100644
--- a/compiler/handle_options.m
+++ b/compiler/handle_options.m
@@ -1344,14 +1344,17 @@ convert_options_to_globals(OptionTable0, OpMode, Target,
),
% Argument packing only works on C back-ends with low-level data.
- % In the future, we may want to use C bit-field syntax for high-level data.
- % For other back-ends, any RTTI code will need to be updated to cope with
- % packed arguments.
- %
- % Only C targets may store a constructor argument across two words.
- option_implies(highlevel_data, arg_pack_bits, int(0), !Globals),
- (
+ % In the future, we may want to use C bit-field syntax on C backends
+ % with high-level data. For the other target languages, implementing
+ % argument packing will not just a lot of work on RTTI, but also
The wording of that comment is awry too.
+ % generalizing field addressing, to allow both single fields and
+ % a group of adjacent fields packed into a single word to be
+ % addressed via a mechanism other than an argument's name.
Julien.
Julien Fischer
2018-05-16 07:53:28 UTC
Permalink
Post by Zoltan Somogyi
So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
I have tested the new options with the following:

* my JSON library [passes in asm_fast.gc]
* my CSV library [passes in asm_fast.gc]
* the Zinc compiler [passes in hlc.gc]
* Opturion's runtime system [passes in hlc.gc.trseg]
* Two of our transport optimisers

At least one of the latter is now giving me different results that
differ from a system built with rotd-2018-05-10. They're not incorrect
as such, just marginally different. (There may be factors other than
the Mercury compiler in play, I'll have to investigate further.)

Note that none of the above use sub-word sized ints (yet), the
only packing I observed was for enumeration arguments.

Julien.
Julien Fischer
2018-05-16 15:16:51 UTC
Permalink
Post by Julien Fischer
Post by Zoltan Somogyi
So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
* my JSON library [passes in asm_fast.gc]
* my CSV library [passes in asm_fast.gc]
* the Zinc compiler [passes in hlc.gc]
* Opturion's runtime system [passes in hlc.gc.trseg]
* Two of our transport optimisers
At least one of the latter is now giving me different results that
differ from a system built with rotd-2018-05-10. They're not incorrect
as such, just marginally different. (There may be factors other than
the Mercury compiler in play, I'll have to investigate further.)
I've had a further look at this, whatever the issue is it's not caused
by the new optimisation; with the current repository head I get
identical results with the optimisation disabled and enabled.

Julien.
Dirk Ziegemeyer
2018-05-16 09:57:46 UTC
Permalink
Hello Zoltan,

I’m using sub-word-sized integers in this type:

:- type date_without_time
---> date_without_time(
date_year :: int16,
date_month :: uint8,
date_day :: uint8
).

There are some conversions between int, uint8, … types in the application and I compare date_without_time with the builtin inequalities @<, @>, @=< and @>=.

Please let me know if I can contribute by testing the application with a recent rotd.

Dirk
Post by Zoltan Somogyi
I committed the diff that enabled the packing of sub-word-sized
integers and dummies in heap cells about a week ago. Have you
guys tried out those features (after turning them on, since they are
OFF by default)?
The reason I ask is that traditionally, we computed the offset of each
argument into the memory cell when we generated code for a unification.
Since repeating this work for *every* unification is wasted work, that diff
added code to compute these offsets just once, when the representation
of each function symbol is decided, but it also left the old code in place,
with assertions checking that the new method yields the exact same offset
as the old method. I would like to delete the code that calculates the offsets
the old way, along with the assertions, before I start work on my next
diff involving argument packing. However, I would not want to take
that step until those assertions have passed your toughest tests.
So if you have compiled the Mercury programs you are working on
with an installed compiler which has had its default values of
allow_packing_ints and allow_packing_dummies set to `yes',
and had those programs pass all their tests, please tell me.
If you haven't, then please do so and tell me.
And if you had any failures, please tell me as well.
Zoltan.
_______________________________________________
developers mailing list
https://lists.mercurylang.org/listinfo/developers
--
Dirk Ziegemeyer
Selbständiger Fachberater und Projektleiter
Regulierung | Risikomanagement | Meldewesen
Mobil +49 177 2593336
E-Mail ***@ziegemeyer.de | Web http://ziegemeyer.de
Loading...