Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(JP) codegen.cが生成するコードとPostgreSQLの演算系コードで差異 #5

Closed
taiki-k opened this issue Apr 23, 2014 · 8 comments

Comments

@taiki-k
Copy link
Contributor

taiki-k commented Apr 23, 2014

codegen.cが生成するOpenCLコードと、PostgreSQLの数値演算・比較系の部分の例外処理の差異を確認したところ、以下の差異を検出。

  • a % bを求める式で、b == -1の時に、強制的に0を返す特別な処理が入っている。
  • a % 0を求める式で、0除算エラーを返す処理が入っている。
  • float系の四則演算で、isnan()、isinf()のチェックがない。
  • float系の比較演算は、NaNを特別扱いしている。
    NaNは非NaNよりも大きいとし、NaN == NaNは真としている。
  • pow, dpowは、pow(0, n) (n < 0)をエラー(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION)としている。
  • pow, dpowは、pow(a, b) (a < 0, bは整数ではない実数)をエラー(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION)としている。
  • pow, dpowは、計算結果がNaNとなった場合に、特別な処理を行っている。

特に、float系の比較演算におけるNaNの扱いは、PostgreSQL側と合わせないと予期しない動作をすると考える。

@kaigai
Copy link
Contributor

kaigai commented Dec 25, 2014

dfebc81 で、codegen.c にかなり強烈な修正を
加えて上記の特殊ケースのハンドリングを追加しています。

エンバグしていても全く不思議ではないので、作成中のテストケースを流して問題を報告して
もらえると助かります。

それと、GpuScanの条件句の中で、上記のような変なデータを食うようなパターンを作っておき、
きちんとCpuReCheckで本体側再評価 → エラーという流れになる事を確認できるような
テストケースの追加が望ましいです。
網羅的である必要はありませんが、0除算やオーバーフローのケースなどは少なくとも。

@onlyindreams
Copy link
Contributor

regress=# create table orz(x integer);
CREATE TABLE
regress=# select avg(x) from orz;
NOTICE:  Bug? unknown devfunc template: 'ac:'
The connection to the server was lost. Attempting reset: Failed.
!> 

これだけで落ちてしまうようになったのですが...

@onlyindreams
Copy link
Contributor

  • バックトレース
(gdb) bt
#0  0x00007f439717a03b in codegen_expression_walker (node=0x1f95a58, context=0x7fff677039f0) at codegen.c:1158
#1  0x00007f439717b474 in pgstrom_codegen_expression (expr=0x1f95a58, context=0x7fff67703d00) at codegen.c:1418
#2  0x00007f43971a1613 in gpupreagg_codegen_projection (gpreagg=0x1f56d70, context=0x7fff67703d00) at gpupreagg.c:1603
#3  0x00007f43971a2811 in gpupreagg_codegen (gpreagg=0x1f56d70, context=0x7fff67703d00) at gpupreagg.c:1927
#4  0x00007f43971a3065 in pgstrom_try_insert_gpupreagg (pstmt=0x1f95468, agg=0x1f94d80) at gpupreagg.c:2145
#5  0x00007f4397180b9c in grafter_try_replace_recurse (pstmt=0x1f95468, plan=0x1f94d80) at grafter.c:42
#6  0x00007f4397181116 in pgstrom_grafter_entrypoint (parse=0x1f56700, cursorOptions=0, boundParams=0x0) at grafter.c:136
#7  0x00000000008483e6 in planner (parse=0x1f56700, cursorOptions=0, boundParams=0x0) at planner.c:137
#8  0x000000000096c814 in pg_plan_query (querytree=0x1f56700, cursorOptions=0, boundParams=0x0) at postgres.c:750
#9  0x000000000096c95a in pg_plan_queries (querytrees=0x1f576f0, cursorOptions=0, boundParams=0x0) at postgres.c:809
#10 0x000000000096cf2d in exec_simple_query (query_string=0x1f55790 "select avg(x) from orz;") at postgres.c:974
#11 0x0000000000975687 in PostgresMain (argc=1, argv=0x1ef5ff0, dbname=0x1ef5ea8 "regress", username=0x1ef5e90 "nanzai") at postgres.c:4010
#12 0x00000000008b5307 in BackendRun (port=0x1f1e360) at postmaster.c:4118
#13 0x00000000008b43cb in BackendStartup (port=0x1f1e360) at postmaster.c:3793
#14 0x00000000008ad713 in ServerLoop () at postmaster.c:1572
#15 0x00000000008ac664 in PostmasterMain (argc=1, argv=0x1ef4ef0) at postmaster.c:1219
#16 0x00000000007a7063 in main (argc=1, argv=0x1ef4ef0) at main.c:220

@taiki-k
Copy link
Contributor Author

taiki-k commented Dec 25, 2014

codegen.cの1156行目は if (!func)ではなく、if (!dfunc)ではないでしょうか?
でないと、dfuncがNULLとなった場合に、SEGVする気がします。

@onlyindreams
Copy link
Contributor

taiki-kさんの修正をいれたらSEGVを回避できました。

以下のクエリでもエラーになるので色々とおかしくなっているようです...

contrib_regression=# select sum(0);
ERROR:  PG-Strom: OpenCL execution error (build program failure)

static bool
gpupreagg_qual_eval(__private cl_int *errcode,
                    __global kern_parambuf *kparams,
                    __global kern_data_store *kds,
                    __global kern_data_store *ktoast,
                    size_t kds_index)
{
  return true;
}

static cl_int
gpupreagg_keycomp(__private int *errcode,
                  __global kern_data_store *kds,
                  __global kern_data_store *ktoast,
                  size_t x_index,
                  size_t y_index)
{
  pg_int4_t comp;

  return 0;
}

static void
gpupreagg_aggcalc(__private cl_int *errcode,
                  cl_int resno,
                  __local pagg_datum *accum,
                  __local pagg_datum *newval)
{
  switch (resno)
  {
  case 0:
    GPUPREAGG_AGGCALC_PSUM_LONG(errcode,accum,newval);
    break;
  default:
    break;
  }
}

static void
gpupreagg_projection(__private cl_int *errcode,
            __global kern_parambuf *kparams,
            __global kern_data_store *kds_in,
            __global kern_data_store *kds_src,
            __global void *ktoast,
            size_t rowidx_in, size_t rowidx_out)
{

  /* projection for resource 0 */
  pg_int8_vstore(kds_src,kds_in,errcode,0,rowidx_out,(null));
}


DETAIL:  :2680:55: error: use of undeclared identifier 'null'
  pg_int8_vstore(kds_src,kds_in,errcode,0,rowidx_out,(null));
                                                      ^

contrib_regression=# explain verbose select sum(0);
                           QUERY PLAN                           
----------------------------------------------------------------
 Aggregate  (cost=500.02..500.03 rows=1 width=0)
   Output: pgstrom.sum((pgstrom.psum((0)::bigint)))
   ->  Custom (GpuPreAgg)  (cost=500.00..500.01 rows=1 width=8)
         Output: pgstrom.psum((0)::bigint)
         Bulkload: Off
         ->  Result  (cost=0.00..0.01 rows=1 width=0)
 Planning time: 0.120 ms
(7 rows)

contrib_regression=# select sum(0);
ERROR:  PG-Strom: OpenCL execution error (build program failure)

static bool
gpupreagg_qual_eval(__private cl_int *errcode,
                    __global kern_parambuf *kparams,
                    __global kern_data_store *kds,
                    __global kern_data_store *ktoast,
                    size_t kds_index)
{
  return true;
}

static cl_int
gpupreagg_keycomp(__private int *errcode,
                  __global kern_data_store *kds,
                  __global kern_data_store *ktoast,
                  size_t x_index,
                  size_t y_index)
{
  pg_int4_t comp;

  return 0;
}

static void
gpupreagg_aggcalc(__private cl_int *errcode,
                  cl_int resno,
                  __local pagg_datum *accum,
                  __local pagg_datum *newval)
{
  switch (resno)
  {
  case 0:
    GPUPREAGG_AGGCALC_PSUM_LONG(errcode,accum,newval);
    break;
  default:
    break;
  }
}

static void
gpupreagg_projection(__private cl_int *errcode,
            __global kern_parambuf *kparams,
            __global kern_data_store *kds_in,
            __global kern_data_store *kds_src,
            __global void *ktoast,
            size_t rowidx_in, size_t rowidx_out)
{

  /* projection for resource 0 */
  pg_int8_vstore(kds_src,kds_in,errcode,0,rowidx_out,(null));
}


DETAIL:  :2680:55: error: use of undeclared identifier 'null'
  pg_int8_vstore(kds_src,kds_in,errcode,0,rowidx_out,(null));
                                                      ^

@taiki-k
Copy link
Contributor Author

taiki-k commented Dec 25, 2014

pgstrom_devfunc_lookup_by_name() 内でテンプレートに / がない場合の動作を考えなければならないと思います。

現在のコードでは、テンプレート内に / がないと942行目の end がNULLとなってしまい、
先頭の a のチェックが行われないまま c: などのチェックに進んでしまいます。
結果として、 ac: と一致しないと判断されるため、988行目のメッセージ出力のところに入ってしまっているようです。

@taiki-k
Copy link
Contributor Author

taiki-k commented Dec 25, 2014

コードのコメントを読んで気づきましたが、 ac: ではなく、 a/c: が正しいテンプレートになるでしょうか?
そうなると、 pgstrom_devfunc_lookup_by_name() の問題ではなく、
devfunc_common_catalog[] のテンプレートの修正漏れが原因になりそうです。

@kaigai
Copy link
Contributor

kaigai commented Dec 25, 2014

すいません。カタログの記載を直した時のミスですね。

ac:とあるのは、本来a/c:とあるべきエントリでした。

これから直しますので、ちょっと今日はこの辺で切り上げてください。。。

kaigai added a commit that referenced this issue Dec 25, 2014
according to the research at #5, codegen.c was reworked.
arithmetrics operators (+-*/) on int/float were moved to pre-declared
functions to handler overflow/zero-division check correctly.
kaigai added a commit that referenced this issue Dec 25, 2014
It was reported by Kondo Taiki, during investigation of bug #5
@kaigai kaigai closed this as completed Apr 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants