[I added Victor Wagner as co-researcher of this problem]

On 13-01-2018 21:10, Tom Lane wrote:
In the end this might just be an instance of the old saw about
avoiding dot-zero releases.  Have you tried a newer gcc?
(Digging in their bugzilla finds quite a number of __int128 bugs
fixed in 5.4.x, though none look to be specifically about
misaligned data.)

gcc 5.5.0 (from [1]) did not fix the problem..

On 16-01-2018 2:41, Tom Lane wrote:
Marina Polyakova <m.polyak...@postgrespro.ru> writes:
On 13-01-2018 21:10, Tom Lane wrote:
I'm not sure there's much we can do about this.  Dropping the use
of the alignment spec isn't a workable option.  If there were a
simple way for configure to detect that the compiler generates bad
code for that, we could have it do so and reject use of __int128,
but it'd be up to you to come up with a workable test.

I'll think about it..

Attached is a possible test program.  I can confirm it passes on a
machine with working __int128, but I have no idea whether it will
detect the problem on yours.  If not, maybe you can tweak it?

Thank you! Using gcc 5.5.0 it prints that everything is ok. But, investigating the regression diffs, we found out that the error occurs when we pass int128 as not the first argument to the function (perhaps its value is replaced by the value of some address):

-- Use queries from random.sql
SELECT count(*) FROM onek; -- Everything is ok
...
SELECT random, count(random) FROM RANDOM_TBL
  GROUP BY random HAVING count(random) > 3; -- Everything is ok

postgres=# SELECT * FROM RANDOM_TBL ORDER BY random; -- Print current data
 random
--------
     78
     86
     98
     98
(4 rows)

postgres=# SELECT AVG(random) FROM RANDOM_TBL
postgres-#   HAVING AVG(random) NOT BETWEEN 80 AND 120; -- Oops!
              avg
-------------------------------
 79446934848446476698976780288
(1 row)

Debug output from the last query (see attached diff.patch, it is based on commit 9c7d06d60680c7f00d931233873dee81fdb311c6 of master):

makeInt128AggState
int8_avg_accum val 98
int8_avg_accum val_int128 as 2 x int64: 0 98
int8_avg_accum val_int128 bytes: 00000000000000000000000000000062
int8_avg_accum state 100e648d8
int8_avg_accum 1007f2e94
do_int128_accum int128 newval as 2 x int64: 4306826968 0
do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
do_int128_accum state 100e648d8
do_int128_accum 1007f1e30
int8_avg_accum val 86
int8_avg_accum val_int128 as 2 x int64: 0 86
int8_avg_accum val_int128 bytes: 00000000000000000000000000000056
int8_avg_accum state 100e648d8
int8_avg_accum 1007f2e94
do_int128_accum int128 newval as 2 x int64: 4306826968 0
do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
do_int128_accum state 100e648d8
do_int128_accum 1007f1e30
int8_avg_accum val 98
int8_avg_accum val_int128 as 2 x int64: 0 98
int8_avg_accum val_int128 bytes: 00000000000000000000000000000062
int8_avg_accum state 100e648d8
int8_avg_accum 1007f2e94
do_int128_accum int128 newval as 2 x int64: 4306826968 0
do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
do_int128_accum state 100e648d8
do_int128_accum 1007f1e30
int8_avg_accum val 78
int8_avg_accum val_int128 as 2 x int64: 0 78
int8_avg_accum val_int128 bytes: 0000000000000000000000000000004E
int8_avg_accum state 100e648d8
int8_avg_accum 1007f2e94
do_int128_accum int128 newval as 2 x int64: 4306826968 0
do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
do_int128_accum state 100e648d8
do_int128_accum 1007f1e30
numeric_poly_avg
int128_to_numericvar
int128_to_numericvar int128 val as 2 x int64: 17227307872 0
int128_to_numericvar int128 val bytes: 0000000402D3DB600000000000000000

(val_int128 in the function int8_avg_accum is correct, but newval in the function do_int128_accum is not equal to it. val in the function int128_to_numericvar is (4 * 4306826968).)

Based on this, we modified the test program (see attached). Here is its output on Solaris 10 for different alignments requirements for int128 (on my machine where make check-world passes everything is OK) (ALIGNOF_PG_INT128_TYPE is 16 on Solaris 10):

$ gcc -D PG_ALIGN_128=16 -m64 -o int128test2 int128test2.c
$ ./int128test2
basic aritmetic OK
pass int 16 OK
pass uint 16 OK
pass int 32 OK
pass int 64 OK
pass int 128 OK
$ gcc -D PG_ALIGN_128=8 -m64 -o int128test2 int128test2.c
$ ./int128test2
basic aritmetic OK
pass int 16 FAILED
pass uint 16 FAILED
pass int 32 FAILED
pass int 64 FAILED
pass int 128 OK

Maybe some pass test from int128test2.c can be used to test __int128?

P.S. I suppose, g.b should be 97656250 to get 400000000005:

struct glob128
{
        __int128        start;
        char            pad;
        int128a         a;
        int128a         b;
        int128a         c;
        int128a         d;
} g = {0, 'p', 48828125, 97656255, 0, 0};
...
g.b = (g.b << 12) + 5;            /* 400000000005 */

[1] https://www.opencsw.org

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
#include <stddef.h>
#include <stdio.h>


#ifndef PG_ALIGN_128
#define PG_ALIGN_128 8
#endif

/* GCC, Sunpro and XLC support aligned */
#if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
#define pg_attribute_aligned(a) __attribute__((aligned(a)))
#endif

typedef __int128 int128a
#if defined(pg_attribute_aligned)
pg_attribute_aligned(PG_ALIGN_128)
#endif
;

/*
 * These are globals to discourage the compiler from folding all the
 * arithmetic tests down to compile-time constants.  We do not have
 * convenient support for 128bit literals at this point...
 */
struct glob128
{
	__int128	start;
	char		pad;
	int128a		a;
	int128a		b;
	int128a		c;
	int128a		d;
} g = {0, 'p', 48828125, 97656250, 0, 0};

int128a holder;

void pass_by_val(char *buffer, int128a par) {
   holder = par;
}

int
main()
{
	char *message = "Everything is fine";
	short int i16 = 31000;
	unsigned short int u16 = 49000;
	int i32 = 48828125;
	long int i64 = 97656225L << 12;
	int128a q;
	int failures = 0;

	/* Basic arithmetic tests */

	g.a = (g.a << 12) + 1;		/* 200000000001 */
	g.b = (g.b << 12) + 5;		/* 400000000005 */
	/* use the most relevant arithmetic ops */
	g.c = g.a * g.b;
	g.d = (g.c + g.b) / g.b;
	/* return different values, to prevent optimizations */
	if (g.d != g.a + 1)
	{
		printf("wrong arithmetic result\n");
		failures++;
	}
	else
	{
		printf("basic aritmetic OK\n");
	}

	pass_by_val(message, (int128a) i16);
	q = (int128a) i16;
	printf("pass int 16 %s\n", (q == holder) ? "OK" : "FAILED");
	if (q != holder)
		failures++;

	pass_by_val(message, (int128a) u16);
	q = (int128a) u16;
	printf("pass uint 16 %s\n", (q == holder) ? "OK" : "FAILED");
	if (q != holder)
		failures++;

	pass_by_val(message, (int128a) i32);
	q = (int128a) i32;
	printf("pass int 32 %s\n", (q == holder) ? "OK" : "FAILED");
	if (q != holder)
		failures++;

	pass_by_val(message, (int128a) i64);
	q = (int128a) i64;
	printf("pass int 64 %s\n", (q == holder) ? "OK" : "FAILED");
	if (q != holder)
		failures++;

	q = (int128a) (i64 << 33);
	pass_by_val(message, q);
	printf("pass int 128 %s\n", (q == holder) ? "OK" : "FAILED");
	if (q != holder)
		failures++;

	return failures;
}

Reply via email to