public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 3/3] Change policy of XLog read-buffer allocation 31+ messages / 10 participants [nested] [flat]
* [PATCH v5 3/3] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 18 ------------------ src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 929da4eef2..9ec88b35ef 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1392,6 +1392,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1421,6 +1422,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 96e2115aad..425f7a12bd 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6351,6 +6351,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7722,6 +7723,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 66bd9eb8d7..26f6834b8f 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -79,27 +79,11 @@ XLogReaderAllocate(int wal_segment_size) state->max_block_id = -1; - /* - * Permanently allocate readBuf. We do it this way, rather than just - * making a static array, for two reasons: (1) no need to waste the - * storage in most instantiations of the backend; (2) a static char array - * isn't guaranteed to have any particular alignment, whereas - * palloc_extended() will provide MAXALIGN'd storage. - */ - state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ, - MCXT_ALLOC_NO_OOM); - if (!state->readBuf) - { - pfree(state); - return NULL; - } - state->wal_segment_size = wal_segment_size; state->errormsg_buf = palloc_extended(MAX_ERRORMSG_LEN + 1, MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -112,7 +96,6 @@ XLogReaderAllocate(int wal_segment_size) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -136,7 +119,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 11e52e4c01..ea027caa69 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index ff26b30f82..c60267e87e 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 56e2f8b0b0..bdf3c81b03 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1108,6 +1108,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1190,6 +1191,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.16.3 ----Next_Part(Fri_Sep_06_16_33_18_2019_198)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v12 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index f519a5b6a2..e72d5c7ccb 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1365,6 +1366,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 364b1948e4..df1e9a5830 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6352,6 +6352,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7736,6 +7737,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 71984f00f7..81a95623a2 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index d3a92044b5..59671f99f0 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -519,6 +520,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 4236ea1d12..ee380b25e1 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -90,6 +91,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -113,6 +115,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -131,6 +134,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -172,6 +176,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -220,6 +225,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 16ee68efbe..4a6491007f 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1036,6 +1036,7 @@ main(int argc, char **argv) if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1116,6 +1117,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.23.0 ----Next_Part(Fri_Nov_29_17_14_21_2019_330)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v11 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index b6c7125aa1..e69b6abae7 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1365,6 +1366,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 364b1948e4..df1e9a5830 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6352,6 +6352,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7736,6 +7737,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 9acc8d03e2..cb47aa239a 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index d3a92044b5..59671f99f0 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -519,6 +520,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 4236ea1d12..ee380b25e1 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -90,6 +91,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -113,6 +115,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -131,6 +134,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -172,6 +176,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -220,6 +225,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index c7c05edcda..5a40047733 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1035,6 +1035,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz, waldir); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1115,6 +1116,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.23.0 ----Next_Part(Wed_Nov_27_12_09_23_2019_240)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v7 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index f5f6278880..6f5e6e5e56 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); XLogBeginRead(xlogreader, lsn); while (XLogReadRecord(xlogreader, &record, &errormsg) == @@ -1366,6 +1367,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a3eded30ad..3f7f3e4d4f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6411,6 +6411,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7813,6 +7814,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index ad85f50c77..6b9287d68a 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 7782e3ad2f..858c537558 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -518,6 +519,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 55337b65f1..48502151a1 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); XLogBeginRead(xlogreader, startpoint); do @@ -86,6 +87,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -109,6 +111,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); XLogBeginRead(xlogreader, ptr); while (XLogReadRecord(xlogreader, &record, &errormsg) == @@ -128,6 +131,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -169,6 +173,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -218,6 +223,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 0cd0d132af..ac6740f21d 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1033,6 +1033,7 @@ main(int argc, char **argv) if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1109,6 +1110,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.18.2 ----Next_Part(Tue_Mar_24_18_24_13_2020_275)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v9 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index ced11bbdae..9e80a2cdb1 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e8a4c7916b..0bc7549b07 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6355,6 +6355,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7739,6 +7740,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index ce7fd55496..12a097c3eb 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -104,7 +104,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -117,7 +116,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -141,7 +139,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index db3e8f9dc0..99cb9bcc54 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index c5e5d75a87..d0e408c0a8 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index ee49712830..e17e66e688 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1094,6 +1094,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz, waldir); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1174,6 +1175,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.23.0 ----Next_Part(Thu_Oct_24_14_51_01_2019_720)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v8 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index ced11bbdae..9e80a2cdb1 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 49e8ca486e..4cbb6de1bb 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6349,6 +6349,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7721,6 +7722,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 9cab82e76d..80434aed2e 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -105,7 +105,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -118,7 +117,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -142,7 +140,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index db3e8f9dc0..99cb9bcc54 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index c5e5d75a87..d0e408c0a8 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index ee49712830..e17e66e688 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1094,6 +1094,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz, waldir); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1174,6 +1175,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.16.3 ----Next_Part(Fri_Sep_27_12_07_26_2019_308)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v7 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 3 --- src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index ced11bbdae..9e80a2cdb1 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 49e8ca486e..4cbb6de1bb 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6349,6 +6349,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7721,6 +7722,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 9932ba3882..b8ad4ffcd7 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -105,7 +105,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -118,7 +117,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -142,7 +140,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index db3e8f9dc0..99cb9bcc54 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index c5e5d75a87..d0e408c0a8 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz, datadir); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index ee49712830..e17e66e688 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1094,6 +1094,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz, waldir); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1174,6 +1175,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.16.3 ----Next_Part(Wed_Sep_25_15_50_32_2019_576)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v6 4/4] Change policy of XLog read-buffer allocation @ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw) Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but actually it'd be the responsibility to the callers of XLogReadRecord, which now actually reads in pages. This patch does that. --- src/backend/access/transam/twophase.c | 2 ++ src/backend/access/transam/xlog.c | 2 ++ src/backend/access/transam/xlogreader.c | 18 ------------------ src/backend/replication/logical/logical.c | 2 ++ src/bin/pg_rewind/parsexlog.c | 6 ++++++ src/bin/pg_waldump/pg_waldump.c | 2 ++ 6 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index a736504d62..358951c232 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1392,6 +1392,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == XLREAD_NEED_DATA) @@ -1421,6 +1422,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) *buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader)); memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader)); + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index c20d3b1418..a199600399 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6349,6 +6349,7 @@ StartupXLOG(void) (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); + xlogreader->readBuf = palloc(XLOG_BLCKSZ); xlogreader->system_identifier = ControlFile->system_identifier; /* @@ -7721,6 +7722,7 @@ StartupXLOG(void) close(readFile); readFile = -1; } + pfree(xlogreader->readBuf); XLogReaderFree(xlogreader); /* diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index e00fa270ae..1f98dd0168 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -80,27 +80,11 @@ XLogReaderAllocate(int wal_segment_size) state->max_block_id = -1; - /* - * Permanently allocate readBuf. We do it this way, rather than just - * making a static array, for two reasons: (1) no need to waste the - * storage in most instantiations of the backend; (2) a static char array - * isn't guaranteed to have any particular alignment, whereas - * palloc_extended() will provide MAXALIGN'd storage. - */ - state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ, - MCXT_ALLOC_NO_OOM); - if (!state->readBuf) - { - pfree(state); - return NULL; - } - state->wal_segment_size = wal_segment_size; state->errormsg_buf = palloc_extended(MAX_ERRORMSG_LEN + 1, MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -113,7 +97,6 @@ XLogReaderAllocate(int wal_segment_size) if (!allocate_recordbuf(state, 0)) { pfree(state->errormsg_buf); - pfree(state->readBuf); pfree(state); return NULL; } @@ -137,7 +120,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 11e52e4c01..ea027caa69 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options, ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); + ctx->reader->readBuf = palloc(XLOG_BLCKSZ); ctx->read_page = read_page; ctx->reorder = ReorderBufferAllocate(); @@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx) ReorderBufferFree(ctx->reorder); FreeSnapshotBuilder(ctx->snapshot_builder); + pfree(ctx->reader->readBuf); XLogReaderFree(ctx->reader); MemoryContextDelete(ctx->context); } diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index ff26b30f82..c60267e87e 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); do { @@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, } while (xlogreader->ReadRecPtr != endpoint); + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == XLREAD_NEED_DATA) @@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) } endptr = xlogreader->EndRecPtr; + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { @@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, xlogreader = XLogReaderAllocate(WalSegSz); if (xlogreader == NULL) pg_fatal("out of memory"); + xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ); searchptr = forkptr; for (;;) @@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, searchptr = record->xl_prev; } + pg_free(xlogreader->readBuf); XLogReaderFree(xlogreader); if (xlogreadfd != -1) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index a61a5a91cb..5eba802720 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1108,6 +1108,7 @@ main(int argc, char **argv) xlogreader_state = XLogReaderAllocate(WalSegSz); if (!xlogreader_state) fatal_error("out of memory"); + xlogreader_state->readBuf = palloc(XLOG_BLCKSZ); /* first find a valid recptr to start from */ first_record = XLogFindNextRecord(xlogreader_state, private.startptr, @@ -1188,6 +1189,7 @@ main(int argc, char **argv) (uint32) xlogreader_state->ReadRecPtr, errormsg); + pfree(xlogreader_state->readBuf); XLogReaderFree(xlogreader_state); return EXIT_SUCCESS; -- 2.16.3 ----Next_Part(Tue_Sep_10_17_40_54_2019_177)---- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH v2 1/1] remove db_user_namespace @ 2023-06-30 19:46 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw) --- doc/src/sgml/client-auth.sgml | 5 -- doc/src/sgml/config.sgml | 52 ------------------- src/backend/libpq/auth.c | 5 -- src/backend/libpq/hba.c | 12 ----- src/backend/postmaster/postmaster.c | 19 ------- src/backend/utils/misc/guc_tables.c | 9 ---- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/libpq/pqcomm.h | 2 - 8 files changed, 105 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 204d09df67..6c95f0df1e 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1253,11 +1253,6 @@ omicron bryanh guest1 attacks. </para> - <para> - The <literal>md5</literal> method cannot be used with - the <xref linkend="guc-db-user-namespace"/> feature. - </para> - <para> To ease transition from the <literal>md5</literal> method to the newer SCRAM method, if <literal>md5</literal> is specified as a method diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6262cb7bb2..e6cea8ddfc 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1188,58 +1188,6 @@ include_dir 'conf.d' </para> </listitem> </varlistentry> - - <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace"> - <term><varname>db_user_namespace</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>db_user_namespace</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - This parameter enables per-database user names. It is off by default. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - - <para> - If this is on, you should create users as <replaceable>username@dbname</replaceable>. - When <replaceable>username</replaceable> is passed by a connecting client, - <literal>@</literal> and the database name are appended to the user - name and that database-specific user name is looked up by the - server. Note that when you create users with names containing - <literal>@</literal> within the SQL environment, you will need to - quote the user name. - </para> - - <para> - With this parameter enabled, you can still create ordinary global - users. Simply append <literal>@</literal> when specifying the user - name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal> - will be stripped off before the user name is looked up by the - server. - </para> - - <para> - <varname>db_user_namespace</varname> causes the client's and - server's user name representation to differ. - Authentication checks are always done with the server's user name - so authentication methods must be configured for the - server's user name, not the client's. Because - <literal>md5</literal> uses the user name as salt on both the - client and server, <literal>md5</literal> cannot be used with - <varname>db_user_namespace</varname>. - </para> - - <note> - <para> - This feature is intended as a temporary measure until a - complete solution is found. At that time, this option will - be removed. - </para> - </note> - </listitem> - </varlistentry> </variablelist> </sect2> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a98b934a8e..65d452f099 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail) char *passwd; int result; - if (Db_user_namespace) - ereport(FATAL, - (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"))); - /* include the salt to use for computing the response */ if (!pg_strong_random(md5Salt, 4)) { diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f89f138f3c..5d4ddbb04d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) else if (strcmp(token->string, "reject") == 0) parsedline->auth_method = uaReject; else if (strcmp(token->string, "md5") == 0) - { - if (Db_user_namespace) - { - ereport(elevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), - errcontext("line %d of configuration file \"%s\"", - line_num, file_name))); - *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; - return NULL; - } parsedline->auth_method = uaMD5; - } else if (strcmp(token->string, "scram-sha-256") == 0) parsedline->auth_method = uaSCRAM; else if (strcmp(token->string, "pam") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 4c49393fc5..33a13fdf32 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -236,7 +236,6 @@ int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ bool Log_connections = false; -bool Db_user_namespace = false; bool enable_bonjour = false; char *bonjour_name; @@ -2272,24 +2271,6 @@ retry1: if (port->database_name == NULL || port->database_name[0] == '\0') port->database_name = pstrdup(port->user_name); - if (Db_user_namespace) - { - /* - * If user@, it is a global user, remove '@'. We only want to do this - * if there is an '@' at the end and no earlier in the user string or - * they may fake as a local user of another database attaching to this - * database. - */ - if (strchr(port->user_name, '@') == - port->user_name + strlen(port->user_name) - 1) - *strchr(port->user_name, '@') = '\0'; - else - { - /* Append '@' and dbname */ - port->user_name = psprintf("%s@%s", port->user_name, port->database_name); - } - } - /* * Truncate given database and user names to length of a Postgres name. * This avoids lookup failures when overlength names are given. diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 71e27f8eb0..25d9008bb6 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, - { - {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH, - gettext_noop("Enables per-database user names."), - NULL - }, - &Db_user_namespace, - false, - NULL, NULL, NULL - }, { {"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the default read-only status of new transactions."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e4c0269fa3..c768af9a73 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -96,7 +96,6 @@ #authentication_timeout = 1min # 1s-600s #password_encryption = scram-sha-256 # scram-sha-256 or md5 #scram_iterations = 4096 -#db_user_namespace = off # GSSAPI using Kerberos #krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab' diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index c85090259d..3da00f7983 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType; typedef uint32 PacketLen; -extern PGDLLIMPORT bool Db_user_namespace; - /* * In protocol 3.0 and later, the startup packet length is not fixed, but * we set an arbitrary limit on it anyway. This is just to prevent simple -- 2.25.1 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-04-11 02:16 Thomas Munro <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Thomas Munro @ 2024-04-11 02:16 UTC (permalink / raw) To: pgsql-hackers On Wed, Apr 10, 2024 at 1:38 PM Thomas Munro <[email protected]> wrote: > Therefore, some time after the tree re-opens for hacking, we could rip > out a bunch of support code for LLVM 10-13, and then rip out support > for pre-opaque-pointer mode. Please see attached. ... or of course closer to the end of the cycle if that's what people prefer for some reason, I don't mind too much as long as it happens. I added this to the commitfest app, and it promptly failed for cfbot. That's expected: CI is still using Debian 11 "bullseye", which only has LLVM 11. It became what Debian calls "oldstable" last year, and reaches the end of oldstable in a couple of months from now. Debian 12 "bookworm" is the current stable release, and it has LLVM 14, so we should probably go and update those CI images... ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-04-23 23:43 Thomas Munro <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Thomas Munro @ 2024-04-23 23:43 UTC (permalink / raw) To: pgsql-hackers Rebased over ca89db5f. I looked into whether we could drop the "old pass manager" code too[1]. Almost, but nope, even the C++ API lacks a way to set the inline threshold before LLVM 16, so that would cause a regression. Although we just hard-code the threshold to 512 with a comment that sounds like it's pretty arbitrary, a change to the default (225?) would be unjustifiable just for code cleanup. Oh well. [1] https://github.com/macdice/postgres/commit/0d40abdf1feb75210c3a3d2a35e3d6146185974c Attachments: [application/octet-stream] v2-0001-jit-Require-at-least-LLVM-14-if-enabled.patch (16.0K, ../../CA+hUKGLFNFMOPPME6mpkGeve0+6CRrZLMTKs+MmBGJp4vLcJ=A@mail.gmail.com/2-v2-0001-jit-Require-at-least-LLVM-14-if-enabled.patch) download | inline diff: From aabc71af13f4a6cfab662d74f9663f6129721195 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 19 Oct 2023 04:45:46 +1300 Subject: [PATCH v2 1/2] jit: Require at least LLVM 14, if enabled. Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- config/llvm.m4 | 8 +- configure | 8 +- doc/src/sgml/installation.sgml | 4 +- meson.build | 2 +- src/backend/jit/llvm/llvmjit.c | 101 ------------------------ src/backend/jit/llvm/llvmjit_error.cpp | 25 ------ src/backend/jit/llvm/llvmjit_inline.cpp | 13 --- src/backend/jit/llvm/llvmjit_wrap.cpp | 4 - 8 files changed, 11 insertions(+), 154 deletions(-) diff --git a/config/llvm.m4 b/config/llvm.m4 index 44769d819a0..6ca088b5a45 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -13,7 +13,7 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_REQUIRE([AC_PROG_AWK]) AC_ARG_VAR(LLVM_CONFIG, [path to llvm-config command]) - PGAC_PATH_PROGS(LLVM_CONFIG, llvm-config llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14 llvm-config-13 llvm-config-12 llvm-config-11 llvm-config-10) + PGAC_PATH_PROGS(LLVM_CONFIG, llvm-config llvm-config-18 llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14) # no point continuing if llvm wasn't found if test -z "$LLVM_CONFIG"; then @@ -25,14 +25,14 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_MSG_ERROR([$LLVM_CONFIG does not work]) fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 10) exit 1; else exit 0;}';then - AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required]) + if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 14) exit 1; else exit 0;}';then + AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required]) fi AC_MSG_NOTICE([using llvm $pgac_llvm_version]) # need clang to create some bitcode files AC_ARG_VAR(CLANG, [path to clang compiler to generate bitcode]) - PGAC_PATH_PROGS(CLANG, clang clang-17 clang-16 clang-15 clang-14 clang-13 clang-12 clang-11 clang-10) + PGAC_PATH_PROGS(CLANG, clang clang-18 clang-17 clang-16 clang-15 clang-14) if test -z "$CLANG"; then AC_MSG_ERROR([clang not found, but required when compiling --with-llvm, specify with CLANG=]) fi diff --git a/configure b/configure index cfbd2a096f4..ba4ce5a5282 100755 --- a/configure +++ b/configure @@ -5065,7 +5065,7 @@ if test "$with_llvm" = yes; then : if test -z "$LLVM_CONFIG"; then - for ac_prog in llvm-config llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14 llvm-config-13 llvm-config-12 llvm-config-11 llvm-config-10 + for ac_prog in llvm-config llvm-config-18 llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14 do # Extract the first word of "$ac_prog", so it can be a program name with args. set dummy $ac_prog; ac_word=$2 @@ -5129,8 +5129,8 @@ fi as_fn_error $? "$LLVM_CONFIG does not work" "$LINENO" 5 fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 10) exit 1; else exit 0;}';then - as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required" "$LINENO" 5 + if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 14) exit 1; else exit 0;}';then + as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required" "$LINENO" 5 fi { $as_echo "$as_me:${as_lineno-$LINENO}: using llvm $pgac_llvm_version" >&5 $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} @@ -5138,7 +5138,7 @@ $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} # need clang to create some bitcode files if test -z "$CLANG"; then - for ac_prog in clang clang-17 clang-16 clang-15 clang-14 clang-13 clang-12 clang-11 clang-10 + for ac_prog in clang clang-18 clang-17 clang-16 clang-15 clang-14 do # Extract the first word of "$ac_prog", so it can be a program name with args. set dummy $ac_prog; ac_word=$2 diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index a453f804cd6..826cb5e4935 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -936,7 +936,7 @@ build-postgresql: <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of <productname>LLVM</productname> is - currently 10. + currently 14. </para> <para> <command>llvm-config</command><indexterm><primary>llvm-config</primary></indexterm> @@ -2424,7 +2424,7 @@ ninja install <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of - <productname>LLVM</productname> is currently 10. Disabled by + <productname>LLVM</productname> is currently 14. Disabled by default. </para> diff --git a/meson.build b/meson.build index cdfd31377d1..9a58845f665 100644 --- a/meson.build +++ b/meson.build @@ -748,7 +748,7 @@ endif llvmopt = get_option('llvm') llvm = not_found_dep if add_languages('cpp', required: llvmopt, native: false) - llvm = dependency('llvm', version: '>=10', method: 'config-tool', required: llvmopt) + llvm = dependency('llvm', version: '>=14', method: 'config-tool', required: llvmopt) if llvm.found() diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 1d439f24554..8f9c77eedc1 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -21,13 +21,9 @@ #if LLVM_VERSION_MAJOR > 16 #include <llvm-c/Transforms/PassBuilder.h> #endif -#if LLVM_VERSION_MAJOR > 11 #include <llvm-c/Orc.h> #include <llvm-c/OrcEE.h> #include <llvm-c/LLJIT.h> -#else -#include <llvm-c/OrcBindings.h> -#endif #include <llvm-c/Support.h> #include <llvm-c/Target.h> #if LLVM_VERSION_MAJOR < 17 @@ -50,13 +46,8 @@ /* Handle of a module emitted via ORC JIT */ typedef struct LLVMJitHandle { -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef lljit; LLVMOrcResourceTrackerRef resource_tracker; -#else - LLVMOrcJITStackRef stack; - LLVMOrcModuleHandle orc_handle; -#endif } LLVMJitHandle; @@ -103,14 +94,9 @@ static LLVMContextRef llvm_context; static LLVMTargetRef llvm_targetref; -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcThreadSafeContextRef llvm_ts_context; static LLVMOrcLLJITRef llvm_opt0_orc; static LLVMOrcLLJITRef llvm_opt3_orc; -#else /* LLVM_VERSION_MAJOR > 11 */ -static LLVMOrcJITStackRef llvm_opt0_orc; -static LLVMOrcJITStackRef llvm_opt3_orc; -#endif /* LLVM_VERSION_MAJOR > 11 */ static void llvm_release_context(JitContext *context); @@ -124,10 +110,8 @@ static void llvm_set_target(void); static void llvm_recreate_llvm_context(void); static uint64_t llvm_resolve_symbol(const char *name, void *ctx); -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm); static char *llvm_error_message(LLVMErrorRef error); -#endif /* LLVM_VERSION_MAJOR > 11 */ /* ResourceOwner callbacks to hold JitContexts */ static void ResOwnerReleaseJitContext(Datum res); @@ -292,7 +276,6 @@ llvm_release_context(JitContext *context) { LLVMJitHandle *jit_handle = (LLVMJitHandle *) lfirst(lc); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcExecutionSessionRef ee; LLVMOrcSymbolStringPoolRef sp; @@ -310,11 +293,6 @@ llvm_release_context(JitContext *context) sp = LLVMOrcExecutionSessionGetSymbolStringPool(ee); LLVMOrcSymbolStringPoolClearDeadEntries(sp); } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - LLVMOrcRemoveModule(jit_handle->stack, jit_handle->orc_handle); - } -#endif /* LLVM_VERSION_MAJOR > 11 */ pfree(jit_handle); } @@ -397,7 +375,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) * to mangle here. */ -#if LLVM_VERSION_MAJOR > 11 foreach(lc, context->handles) { LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); @@ -427,19 +404,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) if (addr) return (void *) (uintptr_t) addr; } -#else - foreach(lc, context->handles) - { - LLVMOrcTargetAddress addr; - LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); - - addr = 0; - if (LLVMOrcGetSymbolAddressIn(handle->stack, &addr, handle->orc_handle, funcname)) - elog(ERROR, "failed to look up symbol \"%s\"", funcname); - if (addr) - return (void *) (uintptr_t) addr; - } -#endif elog(ERROR, "failed to JIT: %s", funcname); @@ -735,11 +699,7 @@ llvm_compile_module(LLVMJitContext *context) MemoryContext oldcontext; instr_time starttime; instr_time endtime; -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef compile_orc; -#else - LLVMOrcJITStackRef compile_orc; -#endif if (context->base.flags & PGJIT_OPT3) compile_orc = llvm_opt3_orc; @@ -796,7 +756,6 @@ llvm_compile_module(LLVMJitContext *context) * faster instruction selection mechanism is used. */ INSTR_TIME_SET_CURRENT(starttime); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcThreadSafeModuleRef ts_module; LLVMErrorRef error; @@ -824,16 +783,6 @@ llvm_compile_module(LLVMJitContext *context) /* LLVMOrcLLJITAddLLVMIRModuleWithRT takes ownership of the module */ } -#else - { - handle->stack = compile_orc; - if (LLVMOrcAddEagerlyCompiledIR(compile_orc, &handle->orc_handle, context->module, - llvm_resolve_symbol, NULL)) - elog(ERROR, "failed to JIT module"); - - /* LLVMOrcAddEagerlyCompiledIR takes ownership of the module */ - } -#endif INSTR_TIME_SET_CURRENT(endtime); INSTR_TIME_ACCUM_DIFF(context->base.instr.emission_counter, @@ -945,7 +894,6 @@ llvm_session_initialize(void) /* force symbols in main binary to be loaded */ LLVMLoadLibraryPermanently(NULL); -#if LLVM_VERSION_MAJOR > 11 { llvm_ts_context = LLVMOrcCreateNewThreadSafeContext(); @@ -955,31 +903,6 @@ llvm_session_initialize(void) llvm_opt3_orc = llvm_create_jit_instance(opt3_tm); opt3_tm = 0; } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - llvm_opt0_orc = LLVMOrcCreateInstance(opt0_tm); - llvm_opt3_orc = LLVMOrcCreateInstance(opt3_tm); - -#if defined(HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER) && HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER - if (jit_debugging_support) - { - LLVMJITEventListenerRef l = LLVMCreateGDBRegistrationListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif -#if defined(HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER) && HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER - if (jit_profiling_support) - { - LLVMJITEventListenerRef l = LLVMCreatePerfJITEventListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif - } -#endif /* LLVM_VERSION_MAJOR > 11 */ on_proc_exit(llvm_shutdown, 0); @@ -1009,7 +932,6 @@ llvm_shutdown(int code, Datum arg) elog(PANIC, "LLVMJitContext in use count not 0 at exit (is %zu)", llvm_jit_context_in_use_count); -#if LLVM_VERSION_MAJOR > 11 { if (llvm_opt3_orc) { @@ -1027,23 +949,6 @@ llvm_shutdown(int code, Datum arg) llvm_ts_context = NULL; } } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - /* unregister profiling support, needs to be flushed to be useful */ - - if (llvm_opt3_orc) - { - LLVMOrcDisposeInstance(llvm_opt3_orc); - llvm_opt3_orc = NULL; - } - - if (llvm_opt0_orc) - { - LLVMOrcDisposeInstance(llvm_opt0_orc); - llvm_opt0_orc = NULL; - } - } -#endif /* LLVM_VERSION_MAJOR > 11 */ } /* helper for llvm_create_types, returning a function's return type */ @@ -1213,8 +1118,6 @@ llvm_resolve_symbol(const char *symname, void *ctx) return (uint64_t) addr; } -#if LLVM_VERSION_MAJOR > 11 - static LLVMErrorRef llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, LLVMOrcLookupStateRef *LookupState, LLVMOrcLookupKind Kind, @@ -1233,9 +1136,7 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, { const char *name = LLVMOrcSymbolStringPoolEntryStr(LookupSet[i].Name); -#if LLVM_VERSION_MAJOR > 12 LLVMOrcRetainSymbolStringPoolEntry(LookupSet[i].Name); -#endif symbols[i].Name = LookupSet[i].Name; symbols[i].Sym.Address = llvm_resolve_symbol(name, NULL); symbols[i].Sym.Flags.GenericFlags = LLVMJITSymbolGenericFlagsExported; @@ -1364,8 +1265,6 @@ llvm_error_message(LLVMErrorRef error) return msg; } -#endif /* LLVM_VERSION_MAJOR > 11 */ - /* * ResourceOwner callbacks */ diff --git a/src/backend/jit/llvm/llvmjit_error.cpp b/src/backend/jit/llvm/llvmjit_error.cpp index ebe2f1baa10..351354c30bc 100644 --- a/src/backend/jit/llvm/llvmjit_error.cpp +++ b/src/backend/jit/llvm/llvmjit_error.cpp @@ -30,13 +30,7 @@ static std::new_handler old_new_handler = NULL; static void fatal_system_new_handler(void); static void fatal_llvm_new_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_new_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif static void fatal_llvm_error_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_error_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif /* @@ -135,15 +129,6 @@ fatal_llvm_new_handler(void *user_data, errmsg("out of memory"), errdetail("While in LLVM: %s", reason))); } -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_new_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_new_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif static void fatal_llvm_error_handler(void *user_data, @@ -154,13 +139,3 @@ fatal_llvm_error_handler(void *user_data, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("fatal llvm error: %s", reason))); } - -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_error_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_error_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp index 2007eb523c9..23a8053311d 100644 --- a/src/backend/jit/llvm/llvmjit_inline.cpp +++ b/src/backend/jit/llvm/llvmjit_inline.cpp @@ -594,10 +594,6 @@ function_inlinable(llvm::Function &F, if (F.materialize()) elog(FATAL, "failed to materialize metadata"); -#if LLVM_VERSION_MAJOR < 14 -#define hasFnAttr hasFnAttribute -#endif - if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline)) { ilog(DEBUG1, "ineligibile to import %s due to noinline", @@ -858,9 +854,6 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, llvm::Function *AF; llvm::BasicBlock *BB; llvm::CallInst *fwdcall; -#if LLVM_VERSION_MAJOR < 14 - llvm::Attribute inlineAttribute; -#endif AF = llvm::Function::Create(F->getFunctionType(), LinkageTypes::AvailableExternallyLinkage, @@ -869,13 +862,7 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, Builder.SetInsertPoint(BB); fwdcall = Builder.CreateCall(F, &*AF->arg_begin()); -#if LLVM_VERSION_MAJOR < 14 - inlineAttribute = llvm::Attribute::get(Context, - llvm::Attribute::AlwaysInline); - fwdcall->addAttribute(~0U, inlineAttribute); -#else fwdcall->addFnAttr(llvm::Attribute::AlwaysInline); -#endif Builder.CreateRet(fwdcall); return AF; diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 641c8841ca3..7f7623dac64 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -17,10 +17,6 @@ extern "C" } #include <llvm-c/Core.h> - -/* Avoid macro clash with LLVM's C++ headers */ -#undef Min - #include <llvm/IR/Function.h> #include "jit/llvmjit.h" -- 2.44.0 [application/octet-stream] v2-0002-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch (8.1K, ../../CA+hUKGLFNFMOPPME6mpkGeve0+6CRrZLMTKs+MmBGJp4vLcJ=A@mail.gmail.com/3-v2-0002-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch) download | inline diff: From 83834919ebb245f4d7988dffea2233748bc78801 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 20 Oct 2023 15:13:26 +1300 Subject: [PATCH v2 2/2] jit: Use opaque pointers in all supported LLVM versions. LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- configure | 89 -------------------------------- configure.ac | 3 -- src/backend/jit/llvm/llvmjit.c | 13 ----- src/backend/jit/llvm/meson.build | 3 -- src/include/jit/llvmjit_emit.h | 16 ------ 5 files changed, 124 deletions(-) diff --git a/configure b/configure index ba4ce5a5282..cb4d14f68c3 100755 --- a/configure +++ b/configure @@ -7290,95 +7290,6 @@ if test x"$pgac_cv_prog_CLANGXX_cxxflags__fexcess_precision_standard" = x"yes"; fi - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS" >&5 -$as_echo_n "checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS... " >&6; } -if ${pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CFLAGS=$CFLAGS -pgac_save_CC=$CC -CC=${CLANG} -CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -ac_save_c_werror_flag=$ac_c_werror_flag -ac_c_werror_flag=yes -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_c_werror_flag=$ac_save_c_werror_flag -CFLAGS="$pgac_save_CFLAGS" -CC="$pgac_save_CC" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -fi - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS" >&5 -$as_echo_n "checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS... " >&6; } -if ${pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CXXFLAGS=$CXXFLAGS -pgac_save_CXX=$CXX -CXX=${CLANGXX} -CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -ac_save_cxx_werror_flag=$ac_cxx_werror_flag -ac_cxx_werror_flag=yes -ac_ext=cpp -ac_cpp='$CXXCPP $CPPFLAGS' -ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_cxx_compiler_gnu - -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_cxx_try_compile "$LINENO"; then : - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_ext=c -ac_cpp='$CPP $CPPFLAGS' -ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_c_compiler_gnu - -ac_cxx_werror_flag=$ac_save_cxx_werror_flag -CXXFLAGS="$pgac_save_CXXFLAGS" -CXX="$pgac_save_CXX" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -fi - - NOT_THE_CFLAGS="" { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS" >&5 $as_echo_n "checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS... " >&6; } diff --git a/configure.ac b/configure.ac index 67e738d92b1..b82ec09c8f5 100644 --- a/configure.ac +++ b/configure.ac @@ -634,9 +634,6 @@ if test "$with_llvm" = yes ; then PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-fexcess-precision=standard]) PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-fexcess-precision=standard]) - PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-Xclang -no-opaque-pointers]) - PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-Xclang -no-opaque-pointers]) - NOT_THE_CFLAGS="" PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, NOT_THE_CFLAGS, [-Wunused-command-line-argument]) if test -n "$NOT_THE_CFLAGS"; then diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 8f9c77eedc1..9960a0239c8 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -835,19 +835,6 @@ llvm_session_initialize(void) llvm_llvm_context_reuse_count = 0; } - /* - * When targeting LLVM 15, turn off opaque pointers for the context we - * build our code in. We don't need to do so for other contexts (e.g. - * llvm_ts_context). Once the IR is generated, it carries the necessary - * information. - * - * For 16 and above, opaque pointers must be used, and we have special - * code for that. - */ -#if LLVM_VERSION_MAJOR == 15 - LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false); -#endif - /* * Synchronize types early, as that also includes inferring the target * triple. diff --git a/src/backend/jit/llvm/meson.build b/src/backend/jit/llvm/meson.build index 41c759f73c5..88b4a212bd1 100644 --- a/src/backend/jit/llvm/meson.build +++ b/src/backend/jit/llvm/meson.build @@ -60,9 +60,6 @@ endif # XXX: Need to determine proper version of the function cflags for clang bitcode_cflags = ['-fno-strict-aliasing', '-fwrapv'] -if llvm.version().version_compare('=15.0') - bitcode_cflags += ['-Xclang', '-no-opaque-pointers'] -endif bitcode_cflags += cppflags # XXX: Worth improving on the logic to find directories here diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h index 4f35f3dca13..0a04c85d9b9 100644 --- a/src/include/jit/llvmjit_emit.h +++ b/src/include/jit/llvmjit_emit.h @@ -107,41 +107,25 @@ l_pbool_const(bool i) static inline LLVMValueRef l_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildStructGEP(b, v, idx, ""); -#else return LLVMBuildStructGEP2(b, t, v, idx, ""); -#endif } static inline LLVMValueRef l_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef *indices, int32 nindices, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildGEP(b, v, indices, nindices, name); -#else return LLVMBuildGEP2(b, t, v, indices, nindices, name); -#endif } static inline LLVMValueRef l_load(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildLoad(b, v, name); -#else return LLVMBuildLoad2(b, t, v, name); -#endif } static inline LLVMValueRef l_call(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef fn, LLVMValueRef *args, int32 nargs, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildCall(b, fn, args, nargs, name); -#else return LLVMBuildCall2(b, t, fn, args, nargs, name); -#endif } /* -- 2.44.0 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-12 14:32 Peter Eisentraut <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Peter Eisentraut @ 2024-05-12 14:32 UTC (permalink / raw) To: Thomas Munro <[email protected]>; pgsql-hackers On 24.04.24 01:43, Thomas Munro wrote: > Rebased over ca89db5f. These patches look fine to me. The new cut-off makes sense, and it does save quite a bit of code. We do need to get the Cirrus CI Debian images updated first, as you had already written. As part of this patch, you also sneak in support for LLVM 18 (llvm-config-18, clang-18 in configure). Should this be a separate patch? And as I'm looking up how this was previously handled, I notice that this list of clang-NN versions was last updated equally sneakily as part of your patch to trim off LLVM <10 (820b5af73dc). I wonder if the original intention of that configure code was that maintaining the versioned list above clang-7/llvm-config-7 was not needed, because the unversioning programs could be used, or maybe because pkg-config could be used. It would be nice if we could get rid of having to update that. ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-15 04:21 Thomas Munro <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Thomas Munro @ 2024-05-15 04:21 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers On Mon, May 13, 2024 at 2:33 AM Peter Eisentraut <[email protected]> wrote: > These patches look fine to me. The new cut-off makes sense, and it does > save quite a bit of code. We do need to get the Cirrus CI Debian images > updated first, as you had already written. Thanks for looking! > As part of this patch, you also sneak in support for LLVM 18 > (llvm-config-18, clang-18 in configure). Should this be a separate patch? Yeah, right, I didn't really think too hard about why we have that, and now that you question it... > And as I'm looking up how this was previously handled, I notice that > this list of clang-NN versions was last updated equally sneakily as part > of your patch to trim off LLVM <10 (820b5af73dc). I wonder if the > original intention of that configure code was that maintaining the > versioned list above clang-7/llvm-config-7 was not needed, because the > unversioning programs could be used, or maybe because pkg-config could > be used. It would be nice if we could get rid of having to update that. I probably misunderstood why we were doing that, perhaps something to do with the way some distro (Debian?) was doing things with older versions, and yeah I see that we went a long time after 7 without touching it and nobody cared. Yeah, it would be nice to get rid of it. Here's a patch. Meson didn't have that. Attachments: [text/x-patch] v3-0001-jit-Remove-configure-probes-for-lvm-config-clang-.patch (2.7K, ../../CA+hUKGL3EJ37xuj0813AfgwtxBSmfM_pwE8pSGGE5BeKSxCiZw@mail.gmail.com/2-v3-0001-jit-Remove-configure-probes-for-lvm-config-clang-.patch) download | inline diff: From 025f8b0106821b5b6f2ab7992da388404e3e406c Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Wed, 15 May 2024 15:51:28 +1200 Subject: [PATCH v3 1/3] jit: Remove configure probes for {lvm-config,clang}-N. Previously we searched for llvm-config-N and clang-N as well as the unversioned names, and maintained a list of expected values of N. There doesn't seem to be any reason to think that the default llvm-config and clang won't be good enough, and if they aren't, they can be overridden with LLVM_CONFIG and CLANG, so let's stop maintaining that list. The Meson build system didn't do that, so this brings them into sync. Suggested-by: Peter Eisentraut <[email protected]> Discussion: https://postgr.es/m/CA%2BhUKG%2BSOP-aR%3DYF_n0dtXGWeCy6x%2BCn-RMWURU5ySQdmeKW1Q%40mail.gmail.com --- config/llvm.m4 | 4 ++-- configure | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/llvm.m4 b/config/llvm.m4 index 44769d819a0..c6cf8858f64 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -13,7 +13,7 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_REQUIRE([AC_PROG_AWK]) AC_ARG_VAR(LLVM_CONFIG, [path to llvm-config command]) - PGAC_PATH_PROGS(LLVM_CONFIG, llvm-config llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14 llvm-config-13 llvm-config-12 llvm-config-11 llvm-config-10) + PGAC_PATH_PROGS(LLVM_CONFIG, llvm-config) # no point continuing if llvm wasn't found if test -z "$LLVM_CONFIG"; then @@ -32,7 +32,7 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], # need clang to create some bitcode files AC_ARG_VAR(CLANG, [path to clang compiler to generate bitcode]) - PGAC_PATH_PROGS(CLANG, clang clang-17 clang-16 clang-15 clang-14 clang-13 clang-12 clang-11 clang-10) + PGAC_PATH_PROGS(CLANG, clang) if test -z "$CLANG"; then AC_MSG_ERROR([clang not found, but required when compiling --with-llvm, specify with CLANG=]) fi diff --git a/configure b/configure index 89644f2249e..8e7704d54bd 100755 --- a/configure +++ b/configure @@ -5065,7 +5065,7 @@ if test "$with_llvm" = yes; then : if test -z "$LLVM_CONFIG"; then - for ac_prog in llvm-config llvm-config-17 llvm-config-16 llvm-config-15 llvm-config-14 llvm-config-13 llvm-config-12 llvm-config-11 llvm-config-10 + for ac_prog in llvm-config do # Extract the first word of "$ac_prog", so it can be a program name with args. set dummy $ac_prog; ac_word=$2 @@ -5138,7 +5138,7 @@ $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} # need clang to create some bitcode files if test -z "$CLANG"; then - for ac_prog in clang clang-17 clang-16 clang-15 clang-14 clang-13 clang-12 clang-11 clang-10 + for ac_prog in clang do # Extract the first word of "$ac_prog", so it can be a program name with args. set dummy $ac_prog; ac_word=$2 -- 2.39.2 [text/x-patch] v3-0002-jit-Require-at-least-LLVM-14-if-enabled.patch (14.3K, ../../CA+hUKGL3EJ37xuj0813AfgwtxBSmfM_pwE8pSGGE5BeKSxCiZw@mail.gmail.com/3-v3-0002-jit-Require-at-least-LLVM-14-if-enabled.patch) download | inline diff: From ef1c38dbb1ec8a6a762c88ee50c55a87693d44f6 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 19 Oct 2023 04:45:46 +1300 Subject: [PATCH v3 2/3] jit: Require at least LLVM 14, if enabled. Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Reviewed-by: Peter Eisentraut <[email protected]> Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- config/llvm.m4 | 4 +- configure | 4 +- doc/src/sgml/installation.sgml | 4 +- meson.build | 2 +- src/backend/jit/llvm/llvmjit.c | 101 ------------------------ src/backend/jit/llvm/llvmjit_error.cpp | 25 ------ src/backend/jit/llvm/llvmjit_inline.cpp | 13 --- src/backend/jit/llvm/llvmjit_wrap.cpp | 4 - 8 files changed, 7 insertions(+), 150 deletions(-) diff --git a/config/llvm.m4 b/config/llvm.m4 index c6cf8858f64..fa4bedd9370 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -25,8 +25,8 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_MSG_ERROR([$LLVM_CONFIG does not work]) fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 10) exit 1; else exit 0;}';then - AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required]) + if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 14) exit 1; else exit 0;}';then + AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required]) fi AC_MSG_NOTICE([using llvm $pgac_llvm_version]) diff --git a/configure b/configure index 8e7704d54bd..4b72a0d9f6a 100755 --- a/configure +++ b/configure @@ -5129,8 +5129,8 @@ fi as_fn_error $? "$LLVM_CONFIG does not work" "$LINENO" 5 fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 10) exit 1; else exit 0;}';then - as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required" "$LINENO" 5 + if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 14) exit 1; else exit 0;}';then + as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required" "$LINENO" 5 fi { $as_echo "$as_me:${as_lineno-$LINENO}: using llvm $pgac_llvm_version" >&5 $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 1b32d5ca62c..91dadc10fbb 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -936,7 +936,7 @@ build-postgresql: <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of <productname>LLVM</productname> is - currently 10. + currently 14. </para> <para> <command>llvm-config</command><indexterm><primary>llvm-config</primary></indexterm> @@ -2424,7 +2424,7 @@ ninja install <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of - <productname>LLVM</productname> is currently 10. Disabled by + <productname>LLVM</productname> is currently 14. Disabled by default. </para> diff --git a/meson.build b/meson.build index 1c0579d5a6b..1b845203780 100644 --- a/meson.build +++ b/meson.build @@ -748,7 +748,7 @@ endif llvmopt = get_option('llvm') llvm = not_found_dep if add_languages('cpp', required: llvmopt, native: false) - llvm = dependency('llvm', version: '>=10', method: 'config-tool', required: llvmopt) + llvm = dependency('llvm', version: '>=14', method: 'config-tool', required: llvmopt) if llvm.found() diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 1d439f24554..8f9c77eedc1 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -21,13 +21,9 @@ #if LLVM_VERSION_MAJOR > 16 #include <llvm-c/Transforms/PassBuilder.h> #endif -#if LLVM_VERSION_MAJOR > 11 #include <llvm-c/Orc.h> #include <llvm-c/OrcEE.h> #include <llvm-c/LLJIT.h> -#else -#include <llvm-c/OrcBindings.h> -#endif #include <llvm-c/Support.h> #include <llvm-c/Target.h> #if LLVM_VERSION_MAJOR < 17 @@ -50,13 +46,8 @@ /* Handle of a module emitted via ORC JIT */ typedef struct LLVMJitHandle { -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef lljit; LLVMOrcResourceTrackerRef resource_tracker; -#else - LLVMOrcJITStackRef stack; - LLVMOrcModuleHandle orc_handle; -#endif } LLVMJitHandle; @@ -103,14 +94,9 @@ static LLVMContextRef llvm_context; static LLVMTargetRef llvm_targetref; -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcThreadSafeContextRef llvm_ts_context; static LLVMOrcLLJITRef llvm_opt0_orc; static LLVMOrcLLJITRef llvm_opt3_orc; -#else /* LLVM_VERSION_MAJOR > 11 */ -static LLVMOrcJITStackRef llvm_opt0_orc; -static LLVMOrcJITStackRef llvm_opt3_orc; -#endif /* LLVM_VERSION_MAJOR > 11 */ static void llvm_release_context(JitContext *context); @@ -124,10 +110,8 @@ static void llvm_set_target(void); static void llvm_recreate_llvm_context(void); static uint64_t llvm_resolve_symbol(const char *name, void *ctx); -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm); static char *llvm_error_message(LLVMErrorRef error); -#endif /* LLVM_VERSION_MAJOR > 11 */ /* ResourceOwner callbacks to hold JitContexts */ static void ResOwnerReleaseJitContext(Datum res); @@ -292,7 +276,6 @@ llvm_release_context(JitContext *context) { LLVMJitHandle *jit_handle = (LLVMJitHandle *) lfirst(lc); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcExecutionSessionRef ee; LLVMOrcSymbolStringPoolRef sp; @@ -310,11 +293,6 @@ llvm_release_context(JitContext *context) sp = LLVMOrcExecutionSessionGetSymbolStringPool(ee); LLVMOrcSymbolStringPoolClearDeadEntries(sp); } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - LLVMOrcRemoveModule(jit_handle->stack, jit_handle->orc_handle); - } -#endif /* LLVM_VERSION_MAJOR > 11 */ pfree(jit_handle); } @@ -397,7 +375,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) * to mangle here. */ -#if LLVM_VERSION_MAJOR > 11 foreach(lc, context->handles) { LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); @@ -427,19 +404,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) if (addr) return (void *) (uintptr_t) addr; } -#else - foreach(lc, context->handles) - { - LLVMOrcTargetAddress addr; - LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); - - addr = 0; - if (LLVMOrcGetSymbolAddressIn(handle->stack, &addr, handle->orc_handle, funcname)) - elog(ERROR, "failed to look up symbol \"%s\"", funcname); - if (addr) - return (void *) (uintptr_t) addr; - } -#endif elog(ERROR, "failed to JIT: %s", funcname); @@ -735,11 +699,7 @@ llvm_compile_module(LLVMJitContext *context) MemoryContext oldcontext; instr_time starttime; instr_time endtime; -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef compile_orc; -#else - LLVMOrcJITStackRef compile_orc; -#endif if (context->base.flags & PGJIT_OPT3) compile_orc = llvm_opt3_orc; @@ -796,7 +756,6 @@ llvm_compile_module(LLVMJitContext *context) * faster instruction selection mechanism is used. */ INSTR_TIME_SET_CURRENT(starttime); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcThreadSafeModuleRef ts_module; LLVMErrorRef error; @@ -824,16 +783,6 @@ llvm_compile_module(LLVMJitContext *context) /* LLVMOrcLLJITAddLLVMIRModuleWithRT takes ownership of the module */ } -#else - { - handle->stack = compile_orc; - if (LLVMOrcAddEagerlyCompiledIR(compile_orc, &handle->orc_handle, context->module, - llvm_resolve_symbol, NULL)) - elog(ERROR, "failed to JIT module"); - - /* LLVMOrcAddEagerlyCompiledIR takes ownership of the module */ - } -#endif INSTR_TIME_SET_CURRENT(endtime); INSTR_TIME_ACCUM_DIFF(context->base.instr.emission_counter, @@ -945,7 +894,6 @@ llvm_session_initialize(void) /* force symbols in main binary to be loaded */ LLVMLoadLibraryPermanently(NULL); -#if LLVM_VERSION_MAJOR > 11 { llvm_ts_context = LLVMOrcCreateNewThreadSafeContext(); @@ -955,31 +903,6 @@ llvm_session_initialize(void) llvm_opt3_orc = llvm_create_jit_instance(opt3_tm); opt3_tm = 0; } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - llvm_opt0_orc = LLVMOrcCreateInstance(opt0_tm); - llvm_opt3_orc = LLVMOrcCreateInstance(opt3_tm); - -#if defined(HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER) && HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER - if (jit_debugging_support) - { - LLVMJITEventListenerRef l = LLVMCreateGDBRegistrationListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif -#if defined(HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER) && HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER - if (jit_profiling_support) - { - LLVMJITEventListenerRef l = LLVMCreatePerfJITEventListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif - } -#endif /* LLVM_VERSION_MAJOR > 11 */ on_proc_exit(llvm_shutdown, 0); @@ -1009,7 +932,6 @@ llvm_shutdown(int code, Datum arg) elog(PANIC, "LLVMJitContext in use count not 0 at exit (is %zu)", llvm_jit_context_in_use_count); -#if LLVM_VERSION_MAJOR > 11 { if (llvm_opt3_orc) { @@ -1027,23 +949,6 @@ llvm_shutdown(int code, Datum arg) llvm_ts_context = NULL; } } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - /* unregister profiling support, needs to be flushed to be useful */ - - if (llvm_opt3_orc) - { - LLVMOrcDisposeInstance(llvm_opt3_orc); - llvm_opt3_orc = NULL; - } - - if (llvm_opt0_orc) - { - LLVMOrcDisposeInstance(llvm_opt0_orc); - llvm_opt0_orc = NULL; - } - } -#endif /* LLVM_VERSION_MAJOR > 11 */ } /* helper for llvm_create_types, returning a function's return type */ @@ -1213,8 +1118,6 @@ llvm_resolve_symbol(const char *symname, void *ctx) return (uint64_t) addr; } -#if LLVM_VERSION_MAJOR > 11 - static LLVMErrorRef llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, LLVMOrcLookupStateRef *LookupState, LLVMOrcLookupKind Kind, @@ -1233,9 +1136,7 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, { const char *name = LLVMOrcSymbolStringPoolEntryStr(LookupSet[i].Name); -#if LLVM_VERSION_MAJOR > 12 LLVMOrcRetainSymbolStringPoolEntry(LookupSet[i].Name); -#endif symbols[i].Name = LookupSet[i].Name; symbols[i].Sym.Address = llvm_resolve_symbol(name, NULL); symbols[i].Sym.Flags.GenericFlags = LLVMJITSymbolGenericFlagsExported; @@ -1364,8 +1265,6 @@ llvm_error_message(LLVMErrorRef error) return msg; } -#endif /* LLVM_VERSION_MAJOR > 11 */ - /* * ResourceOwner callbacks */ diff --git a/src/backend/jit/llvm/llvmjit_error.cpp b/src/backend/jit/llvm/llvmjit_error.cpp index ebe2f1baa10..351354c30bc 100644 --- a/src/backend/jit/llvm/llvmjit_error.cpp +++ b/src/backend/jit/llvm/llvmjit_error.cpp @@ -30,13 +30,7 @@ static std::new_handler old_new_handler = NULL; static void fatal_system_new_handler(void); static void fatal_llvm_new_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_new_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif static void fatal_llvm_error_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_error_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif /* @@ -135,15 +129,6 @@ fatal_llvm_new_handler(void *user_data, errmsg("out of memory"), errdetail("While in LLVM: %s", reason))); } -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_new_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_new_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif static void fatal_llvm_error_handler(void *user_data, @@ -154,13 +139,3 @@ fatal_llvm_error_handler(void *user_data, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("fatal llvm error: %s", reason))); } - -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_error_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_error_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp index 2007eb523c9..23a8053311d 100644 --- a/src/backend/jit/llvm/llvmjit_inline.cpp +++ b/src/backend/jit/llvm/llvmjit_inline.cpp @@ -594,10 +594,6 @@ function_inlinable(llvm::Function &F, if (F.materialize()) elog(FATAL, "failed to materialize metadata"); -#if LLVM_VERSION_MAJOR < 14 -#define hasFnAttr hasFnAttribute -#endif - if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline)) { ilog(DEBUG1, "ineligibile to import %s due to noinline", @@ -858,9 +854,6 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, llvm::Function *AF; llvm::BasicBlock *BB; llvm::CallInst *fwdcall; -#if LLVM_VERSION_MAJOR < 14 - llvm::Attribute inlineAttribute; -#endif AF = llvm::Function::Create(F->getFunctionType(), LinkageTypes::AvailableExternallyLinkage, @@ -869,13 +862,7 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, Builder.SetInsertPoint(BB); fwdcall = Builder.CreateCall(F, &*AF->arg_begin()); -#if LLVM_VERSION_MAJOR < 14 - inlineAttribute = llvm::Attribute::get(Context, - llvm::Attribute::AlwaysInline); - fwdcall->addAttribute(~0U, inlineAttribute); -#else fwdcall->addFnAttr(llvm::Attribute::AlwaysInline); -#endif Builder.CreateRet(fwdcall); return AF; diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 641c8841ca3..7f7623dac64 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -17,10 +17,6 @@ extern "C" } #include <llvm-c/Core.h> - -/* Avoid macro clash with LLVM's C++ headers */ -#undef Min - #include <llvm/IR/Function.h> #include "jit/llvmjit.h" -- 2.39.2 [text/x-patch] v3-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch (8.2K, ../../CA+hUKGL3EJ37xuj0813AfgwtxBSmfM_pwE8pSGGE5BeKSxCiZw@mail.gmail.com/4-v3-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch) download | inline diff: From 6607ad80e858257a589c7bf25464e027a4b310af Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 20 Oct 2023 15:13:26 +1300 Subject: [PATCH v3 3/3] jit: Use opaque pointers in all supported LLVM versions. LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Reviewed-by: Peter Eisentraut <[email protected]> Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- configure | 89 -------------------------------- configure.ac | 3 -- src/backend/jit/llvm/llvmjit.c | 13 ----- src/backend/jit/llvm/meson.build | 3 -- src/include/jit/llvmjit_emit.h | 16 ------ 5 files changed, 124 deletions(-) diff --git a/configure b/configure index 4b72a0d9f6a..abad7553f8b 100755 --- a/configure +++ b/configure @@ -7290,95 +7290,6 @@ if test x"$pgac_cv_prog_CLANGXX_cxxflags__fexcess_precision_standard" = x"yes"; fi - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS" >&5 -$as_echo_n "checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS... " >&6; } -if ${pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CFLAGS=$CFLAGS -pgac_save_CC=$CC -CC=${CLANG} -CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -ac_save_c_werror_flag=$ac_c_werror_flag -ac_c_werror_flag=yes -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_c_werror_flag=$ac_save_c_werror_flag -CFLAGS="$pgac_save_CFLAGS" -CC="$pgac_save_CC" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -fi - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS" >&5 -$as_echo_n "checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS... " >&6; } -if ${pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CXXFLAGS=$CXXFLAGS -pgac_save_CXX=$CXX -CXX=${CLANGXX} -CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -ac_save_cxx_werror_flag=$ac_cxx_werror_flag -ac_cxx_werror_flag=yes -ac_ext=cpp -ac_cpp='$CXXCPP $CPPFLAGS' -ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_cxx_compiler_gnu - -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_cxx_try_compile "$LINENO"; then : - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_ext=c -ac_cpp='$CPP $CPPFLAGS' -ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_c_compiler_gnu - -ac_cxx_werror_flag=$ac_save_cxx_werror_flag -CXXFLAGS="$pgac_save_CXXFLAGS" -CXX="$pgac_save_CXX" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -fi - - NOT_THE_CFLAGS="" { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS" >&5 $as_echo_n "checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS... " >&6; } diff --git a/configure.ac b/configure.ac index c7322e292cc..86961b90eff 100644 --- a/configure.ac +++ b/configure.ac @@ -634,9 +634,6 @@ if test "$with_llvm" = yes ; then PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-fexcess-precision=standard]) PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-fexcess-precision=standard]) - PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-Xclang -no-opaque-pointers]) - PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-Xclang -no-opaque-pointers]) - NOT_THE_CFLAGS="" PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, NOT_THE_CFLAGS, [-Wunused-command-line-argument]) if test -n "$NOT_THE_CFLAGS"; then diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 8f9c77eedc1..9960a0239c8 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -835,19 +835,6 @@ llvm_session_initialize(void) llvm_llvm_context_reuse_count = 0; } - /* - * When targeting LLVM 15, turn off opaque pointers for the context we - * build our code in. We don't need to do so for other contexts (e.g. - * llvm_ts_context). Once the IR is generated, it carries the necessary - * information. - * - * For 16 and above, opaque pointers must be used, and we have special - * code for that. - */ -#if LLVM_VERSION_MAJOR == 15 - LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false); -#endif - /* * Synchronize types early, as that also includes inferring the target * triple. diff --git a/src/backend/jit/llvm/meson.build b/src/backend/jit/llvm/meson.build index 41c759f73c5..88b4a212bd1 100644 --- a/src/backend/jit/llvm/meson.build +++ b/src/backend/jit/llvm/meson.build @@ -60,9 +60,6 @@ endif # XXX: Need to determine proper version of the function cflags for clang bitcode_cflags = ['-fno-strict-aliasing', '-fwrapv'] -if llvm.version().version_compare('=15.0') - bitcode_cflags += ['-Xclang', '-no-opaque-pointers'] -endif bitcode_cflags += cppflags # XXX: Worth improving on the logic to find directories here diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h index 4f35f3dca13..0a04c85d9b9 100644 --- a/src/include/jit/llvmjit_emit.h +++ b/src/include/jit/llvmjit_emit.h @@ -107,41 +107,25 @@ l_pbool_const(bool i) static inline LLVMValueRef l_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildStructGEP(b, v, idx, ""); -#else return LLVMBuildStructGEP2(b, t, v, idx, ""); -#endif } static inline LLVMValueRef l_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef *indices, int32 nindices, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildGEP(b, v, indices, nindices, name); -#else return LLVMBuildGEP2(b, t, v, indices, nindices, name); -#endif } static inline LLVMValueRef l_load(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildLoad(b, v, name); -#else return LLVMBuildLoad2(b, t, v, name); -#endif } static inline LLVMValueRef l_call(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef fn, LLVMValueRef *args, int32 nargs, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildCall(b, fn, args, nargs, name); -#else return LLVMBuildCall2(b, t, fn, args, nargs, name); -#endif } /* -- 2.39.2 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-15 05:20 Peter Eisentraut <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 2 replies; 31+ messages in thread From: Peter Eisentraut @ 2024-05-15 05:20 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers On 15.05.24 06:21, Thomas Munro wrote: >> And as I'm looking up how this was previously handled, I notice that >> this list of clang-NN versions was last updated equally sneakily as part >> of your patch to trim off LLVM <10 (820b5af73dc). I wonder if the >> original intention of that configure code was that maintaining the >> versioned list above clang-7/llvm-config-7 was not needed, because the >> unversioning programs could be used, or maybe because pkg-config could >> be used. It would be nice if we could get rid of having to update that. > I probably misunderstood why we were doing that, perhaps something to > do with the way some distro (Debian?) was doing things with older > versions, and yeah I see that we went a long time after 7 without > touching it and nobody cared. Yeah, it would be nice to get rid of > it. Here's a patch. Meson didn't have that. Yes, let's get that v3-0001 patch into PG17. ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-16 02:33 Thomas Munro <[email protected]> parent: Peter Eisentraut <[email protected]> 1 sibling, 1 reply; 31+ messages in thread From: Thomas Munro @ 2024-05-16 02:33 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers On Wed, May 15, 2024 at 5:20 PM Peter Eisentraut <[email protected]> wrote: > Yes, let's get that v3-0001 patch into PG17. Done. Bilal recently created the CI images for Debian Bookworm[1]. You can try them with s/bullseye/bookworm/ in .cirrus.tasks.yml, but it looks like he is still wrestling with a perl installation problem[2] in the 32 bit build, so here is a temporary patch to do that and also delete the 32 bit tests for now. This way cfbot should succeed with the remaining patches. Parked here for v18. [1] https://github.com/anarazel/pg-vm-images/commit/685ca7ccb7b3adecb11d948ac677d54cd9599e6c [2] https://cirrus-ci.com/task/5459439048720384 Attachments: [text/x-patch] v4-0001-XXX-CI-kludge-bullseye-bookworm.patch (3.2K, ../../CA+hUKGLuziOt_X6e92Hf+VoAYZjk2DG-ARuZtHA6ZaxvusXS1w@mail.gmail.com/2-v4-0001-XXX-CI-kludge-bullseye-bookworm.patch) download | inline diff: From c0a05c2929e03558c730b148bdeb5d301dbc4312 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 16 May 2024 14:10:09 +1200 Subject: [PATCH v4 1/3] XXX CI kludge: bullseye->bookworm Temporarily removed 32 bit tests, as the CI image is not fully baked yet. --- .cirrus.tasks.yml | 37 +++++-------------------------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index a2388cd5036..5ff6b6a0556 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -65,7 +65,7 @@ task: CPUS: 4 BUILD_JOBS: 8 TEST_JOBS: 8 - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm CCACHE_DIR: ${CIRRUS_WORKING_DIR}/ccache_dir # no options enabled, should be small CCACHE_MAXSIZE: "150M" @@ -243,7 +243,7 @@ task: CPUS: 4 BUILD_JOBS: 4 TEST_JOBS: 8 # experimentally derived to be a decent choice - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm CCACHE_DIR: /tmp/ccache_dir DEBUGINFOD_URLS: "https://debuginfod.debian.net" @@ -314,7 +314,7 @@ task: #DEBIAN_FRONTEND=noninteractive apt-get -y install ... matrix: - - name: Linux - Debian Bullseye - Autoconf + - name: Linux - Debian Bookworm - Autoconf env: SANITIZER_FLAGS: -fsanitize=address @@ -348,7 +348,7 @@ task: on_failure: <<: *on_failure_ac - - name: Linux - Debian Bullseye - Meson + - name: Linux - Debian Bookworm - Meson env: CCACHE_MAXSIZE: "400M" # tests two different builds @@ -364,24 +364,7 @@ task: build EOF - # Also build & test in a 32bit build - it's gotten rare to test that - # locally. - configure_32_script: | - su postgres <<-EOF - export CC='ccache gcc -m32' - meson setup \ - --buildtype=debug \ - -Dcassert=true -Dinjection_points=true \ - ${LINUX_MESON_FEATURES} \ - -Dllvm=disabled \ - --pkg-config-path /usr/lib/i386-linux-gnu/pkgconfig/ \ - -DPERL=perl5.32-i386-linux-gnu \ - -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \ - build-32 - EOF - build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}' - build_32_script: su postgres -c 'ninja -C build-32 -j${BUILD_JOBS}' upload_caches: ccache @@ -393,16 +376,6 @@ task: # so that we don't upload 64bit logs if 32bit fails rm -rf build/ - # There's currently no coverage of icu with LANG=C in the buildfarm. We - # can easily provide some here by running one of the sets of tests that - # way. Newer versions of python insist on changing the LC_CTYPE away - # from C, prevent that with PYTHONCOERCECLOCALE. - test_world_32_script: | - su postgres <<-EOF - ulimit -c unlimited - PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS} - EOF - on_failure: <<: *on_failure_meson @@ -652,7 +625,7 @@ task: env: CPUS: 4 BUILD_JOBS: 4 - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm # Use larger ccache cache, as this task compiles with multiple compilers / # flag combinations -- 2.39.2 [text/x-patch] v4-0002-jit-Require-at-least-LLVM-14-if-enabled.patch (14.3K, ../../CA+hUKGLuziOt_X6e92Hf+VoAYZjk2DG-ARuZtHA6ZaxvusXS1w@mail.gmail.com/3-v4-0002-jit-Require-at-least-LLVM-14-if-enabled.patch) download | inline diff: From bcdb3453b50cd37077ad71f012d7f92c9842495f Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 19 Oct 2023 04:45:46 +1300 Subject: [PATCH v4 2/3] jit: Require at least LLVM 14, if enabled. Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Reviewed-by: Peter Eisentraut <[email protected]> Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- config/llvm.m4 | 4 +- configure | 4 +- doc/src/sgml/installation.sgml | 4 +- meson.build | 2 +- src/backend/jit/llvm/llvmjit.c | 101 ------------------------ src/backend/jit/llvm/llvmjit_error.cpp | 25 ------ src/backend/jit/llvm/llvmjit_inline.cpp | 13 --- src/backend/jit/llvm/llvmjit_wrap.cpp | 4 - 8 files changed, 7 insertions(+), 150 deletions(-) diff --git a/config/llvm.m4 b/config/llvm.m4 index c6cf8858f64..fa4bedd9370 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -25,8 +25,8 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_MSG_ERROR([$LLVM_CONFIG does not work]) fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 10) exit 1; else exit 0;}';then - AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required]) + if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 14) exit 1; else exit 0;}';then + AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required]) fi AC_MSG_NOTICE([using llvm $pgac_llvm_version]) diff --git a/configure b/configure index 8e7704d54bd..4b72a0d9f6a 100755 --- a/configure +++ b/configure @@ -5129,8 +5129,8 @@ fi as_fn_error $? "$LLVM_CONFIG does not work" "$LINENO" 5 fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 10) exit 1; else exit 0;}';then - as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required" "$LINENO" 5 + if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 14) exit 1; else exit 0;}';then + as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required" "$LINENO" 5 fi { $as_echo "$as_me:${as_lineno-$LINENO}: using llvm $pgac_llvm_version" >&5 $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 1b32d5ca62c..91dadc10fbb 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -936,7 +936,7 @@ build-postgresql: <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of <productname>LLVM</productname> is - currently 10. + currently 14. </para> <para> <command>llvm-config</command><indexterm><primary>llvm-config</primary></indexterm> @@ -2424,7 +2424,7 @@ ninja install <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of - <productname>LLVM</productname> is currently 10. Disabled by + <productname>LLVM</productname> is currently 14. Disabled by default. </para> diff --git a/meson.build b/meson.build index 1c0579d5a6b..1b845203780 100644 --- a/meson.build +++ b/meson.build @@ -748,7 +748,7 @@ endif llvmopt = get_option('llvm') llvm = not_found_dep if add_languages('cpp', required: llvmopt, native: false) - llvm = dependency('llvm', version: '>=10', method: 'config-tool', required: llvmopt) + llvm = dependency('llvm', version: '>=14', method: 'config-tool', required: llvmopt) if llvm.found() diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 1d439f24554..8f9c77eedc1 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -21,13 +21,9 @@ #if LLVM_VERSION_MAJOR > 16 #include <llvm-c/Transforms/PassBuilder.h> #endif -#if LLVM_VERSION_MAJOR > 11 #include <llvm-c/Orc.h> #include <llvm-c/OrcEE.h> #include <llvm-c/LLJIT.h> -#else -#include <llvm-c/OrcBindings.h> -#endif #include <llvm-c/Support.h> #include <llvm-c/Target.h> #if LLVM_VERSION_MAJOR < 17 @@ -50,13 +46,8 @@ /* Handle of a module emitted via ORC JIT */ typedef struct LLVMJitHandle { -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef lljit; LLVMOrcResourceTrackerRef resource_tracker; -#else - LLVMOrcJITStackRef stack; - LLVMOrcModuleHandle orc_handle; -#endif } LLVMJitHandle; @@ -103,14 +94,9 @@ static LLVMContextRef llvm_context; static LLVMTargetRef llvm_targetref; -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcThreadSafeContextRef llvm_ts_context; static LLVMOrcLLJITRef llvm_opt0_orc; static LLVMOrcLLJITRef llvm_opt3_orc; -#else /* LLVM_VERSION_MAJOR > 11 */ -static LLVMOrcJITStackRef llvm_opt0_orc; -static LLVMOrcJITStackRef llvm_opt3_orc; -#endif /* LLVM_VERSION_MAJOR > 11 */ static void llvm_release_context(JitContext *context); @@ -124,10 +110,8 @@ static void llvm_set_target(void); static void llvm_recreate_llvm_context(void); static uint64_t llvm_resolve_symbol(const char *name, void *ctx); -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm); static char *llvm_error_message(LLVMErrorRef error); -#endif /* LLVM_VERSION_MAJOR > 11 */ /* ResourceOwner callbacks to hold JitContexts */ static void ResOwnerReleaseJitContext(Datum res); @@ -292,7 +276,6 @@ llvm_release_context(JitContext *context) { LLVMJitHandle *jit_handle = (LLVMJitHandle *) lfirst(lc); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcExecutionSessionRef ee; LLVMOrcSymbolStringPoolRef sp; @@ -310,11 +293,6 @@ llvm_release_context(JitContext *context) sp = LLVMOrcExecutionSessionGetSymbolStringPool(ee); LLVMOrcSymbolStringPoolClearDeadEntries(sp); } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - LLVMOrcRemoveModule(jit_handle->stack, jit_handle->orc_handle); - } -#endif /* LLVM_VERSION_MAJOR > 11 */ pfree(jit_handle); } @@ -397,7 +375,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) * to mangle here. */ -#if LLVM_VERSION_MAJOR > 11 foreach(lc, context->handles) { LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); @@ -427,19 +404,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) if (addr) return (void *) (uintptr_t) addr; } -#else - foreach(lc, context->handles) - { - LLVMOrcTargetAddress addr; - LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); - - addr = 0; - if (LLVMOrcGetSymbolAddressIn(handle->stack, &addr, handle->orc_handle, funcname)) - elog(ERROR, "failed to look up symbol \"%s\"", funcname); - if (addr) - return (void *) (uintptr_t) addr; - } -#endif elog(ERROR, "failed to JIT: %s", funcname); @@ -735,11 +699,7 @@ llvm_compile_module(LLVMJitContext *context) MemoryContext oldcontext; instr_time starttime; instr_time endtime; -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef compile_orc; -#else - LLVMOrcJITStackRef compile_orc; -#endif if (context->base.flags & PGJIT_OPT3) compile_orc = llvm_opt3_orc; @@ -796,7 +756,6 @@ llvm_compile_module(LLVMJitContext *context) * faster instruction selection mechanism is used. */ INSTR_TIME_SET_CURRENT(starttime); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcThreadSafeModuleRef ts_module; LLVMErrorRef error; @@ -824,16 +783,6 @@ llvm_compile_module(LLVMJitContext *context) /* LLVMOrcLLJITAddLLVMIRModuleWithRT takes ownership of the module */ } -#else - { - handle->stack = compile_orc; - if (LLVMOrcAddEagerlyCompiledIR(compile_orc, &handle->orc_handle, context->module, - llvm_resolve_symbol, NULL)) - elog(ERROR, "failed to JIT module"); - - /* LLVMOrcAddEagerlyCompiledIR takes ownership of the module */ - } -#endif INSTR_TIME_SET_CURRENT(endtime); INSTR_TIME_ACCUM_DIFF(context->base.instr.emission_counter, @@ -945,7 +894,6 @@ llvm_session_initialize(void) /* force symbols in main binary to be loaded */ LLVMLoadLibraryPermanently(NULL); -#if LLVM_VERSION_MAJOR > 11 { llvm_ts_context = LLVMOrcCreateNewThreadSafeContext(); @@ -955,31 +903,6 @@ llvm_session_initialize(void) llvm_opt3_orc = llvm_create_jit_instance(opt3_tm); opt3_tm = 0; } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - llvm_opt0_orc = LLVMOrcCreateInstance(opt0_tm); - llvm_opt3_orc = LLVMOrcCreateInstance(opt3_tm); - -#if defined(HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER) && HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER - if (jit_debugging_support) - { - LLVMJITEventListenerRef l = LLVMCreateGDBRegistrationListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif -#if defined(HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER) && HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER - if (jit_profiling_support) - { - LLVMJITEventListenerRef l = LLVMCreatePerfJITEventListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif - } -#endif /* LLVM_VERSION_MAJOR > 11 */ on_proc_exit(llvm_shutdown, 0); @@ -1009,7 +932,6 @@ llvm_shutdown(int code, Datum arg) elog(PANIC, "LLVMJitContext in use count not 0 at exit (is %zu)", llvm_jit_context_in_use_count); -#if LLVM_VERSION_MAJOR > 11 { if (llvm_opt3_orc) { @@ -1027,23 +949,6 @@ llvm_shutdown(int code, Datum arg) llvm_ts_context = NULL; } } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - /* unregister profiling support, needs to be flushed to be useful */ - - if (llvm_opt3_orc) - { - LLVMOrcDisposeInstance(llvm_opt3_orc); - llvm_opt3_orc = NULL; - } - - if (llvm_opt0_orc) - { - LLVMOrcDisposeInstance(llvm_opt0_orc); - llvm_opt0_orc = NULL; - } - } -#endif /* LLVM_VERSION_MAJOR > 11 */ } /* helper for llvm_create_types, returning a function's return type */ @@ -1213,8 +1118,6 @@ llvm_resolve_symbol(const char *symname, void *ctx) return (uint64_t) addr; } -#if LLVM_VERSION_MAJOR > 11 - static LLVMErrorRef llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, LLVMOrcLookupStateRef *LookupState, LLVMOrcLookupKind Kind, @@ -1233,9 +1136,7 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, { const char *name = LLVMOrcSymbolStringPoolEntryStr(LookupSet[i].Name); -#if LLVM_VERSION_MAJOR > 12 LLVMOrcRetainSymbolStringPoolEntry(LookupSet[i].Name); -#endif symbols[i].Name = LookupSet[i].Name; symbols[i].Sym.Address = llvm_resolve_symbol(name, NULL); symbols[i].Sym.Flags.GenericFlags = LLVMJITSymbolGenericFlagsExported; @@ -1364,8 +1265,6 @@ llvm_error_message(LLVMErrorRef error) return msg; } -#endif /* LLVM_VERSION_MAJOR > 11 */ - /* * ResourceOwner callbacks */ diff --git a/src/backend/jit/llvm/llvmjit_error.cpp b/src/backend/jit/llvm/llvmjit_error.cpp index ebe2f1baa10..351354c30bc 100644 --- a/src/backend/jit/llvm/llvmjit_error.cpp +++ b/src/backend/jit/llvm/llvmjit_error.cpp @@ -30,13 +30,7 @@ static std::new_handler old_new_handler = NULL; static void fatal_system_new_handler(void); static void fatal_llvm_new_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_new_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif static void fatal_llvm_error_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_error_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif /* @@ -135,15 +129,6 @@ fatal_llvm_new_handler(void *user_data, errmsg("out of memory"), errdetail("While in LLVM: %s", reason))); } -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_new_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_new_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif static void fatal_llvm_error_handler(void *user_data, @@ -154,13 +139,3 @@ fatal_llvm_error_handler(void *user_data, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("fatal llvm error: %s", reason))); } - -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_error_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_error_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp index 2007eb523c9..23a8053311d 100644 --- a/src/backend/jit/llvm/llvmjit_inline.cpp +++ b/src/backend/jit/llvm/llvmjit_inline.cpp @@ -594,10 +594,6 @@ function_inlinable(llvm::Function &F, if (F.materialize()) elog(FATAL, "failed to materialize metadata"); -#if LLVM_VERSION_MAJOR < 14 -#define hasFnAttr hasFnAttribute -#endif - if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline)) { ilog(DEBUG1, "ineligibile to import %s due to noinline", @@ -858,9 +854,6 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, llvm::Function *AF; llvm::BasicBlock *BB; llvm::CallInst *fwdcall; -#if LLVM_VERSION_MAJOR < 14 - llvm::Attribute inlineAttribute; -#endif AF = llvm::Function::Create(F->getFunctionType(), LinkageTypes::AvailableExternallyLinkage, @@ -869,13 +862,7 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, Builder.SetInsertPoint(BB); fwdcall = Builder.CreateCall(F, &*AF->arg_begin()); -#if LLVM_VERSION_MAJOR < 14 - inlineAttribute = llvm::Attribute::get(Context, - llvm::Attribute::AlwaysInline); - fwdcall->addAttribute(~0U, inlineAttribute); -#else fwdcall->addFnAttr(llvm::Attribute::AlwaysInline); -#endif Builder.CreateRet(fwdcall); return AF; diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 641c8841ca3..7f7623dac64 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -17,10 +17,6 @@ extern "C" } #include <llvm-c/Core.h> - -/* Avoid macro clash with LLVM's C++ headers */ -#undef Min - #include <llvm/IR/Function.h> #include "jit/llvmjit.h" -- 2.39.2 [text/x-patch] v4-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch (8.2K, ../../CA+hUKGLuziOt_X6e92Hf+VoAYZjk2DG-ARuZtHA6ZaxvusXS1w@mail.gmail.com/4-v4-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch) download | inline diff: From c4d4fa7a3e016c5a5b2e59eb23d56ca53eadfa41 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 20 Oct 2023 15:13:26 +1300 Subject: [PATCH v4 3/3] jit: Use opaque pointers in all supported LLVM versions. LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Reviewed-by: Peter Eisentraut <[email protected]> Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- configure | 89 -------------------------------- configure.ac | 3 -- src/backend/jit/llvm/llvmjit.c | 13 ----- src/backend/jit/llvm/meson.build | 3 -- src/include/jit/llvmjit_emit.h | 16 ------ 5 files changed, 124 deletions(-) diff --git a/configure b/configure index 4b72a0d9f6a..abad7553f8b 100755 --- a/configure +++ b/configure @@ -7290,95 +7290,6 @@ if test x"$pgac_cv_prog_CLANGXX_cxxflags__fexcess_precision_standard" = x"yes"; fi - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS" >&5 -$as_echo_n "checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS... " >&6; } -if ${pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CFLAGS=$CFLAGS -pgac_save_CC=$CC -CC=${CLANG} -CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -ac_save_c_werror_flag=$ac_c_werror_flag -ac_c_werror_flag=yes -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_c_werror_flag=$ac_save_c_werror_flag -CFLAGS="$pgac_save_CFLAGS" -CC="$pgac_save_CC" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -fi - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS" >&5 -$as_echo_n "checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS... " >&6; } -if ${pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CXXFLAGS=$CXXFLAGS -pgac_save_CXX=$CXX -CXX=${CLANGXX} -CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -ac_save_cxx_werror_flag=$ac_cxx_werror_flag -ac_cxx_werror_flag=yes -ac_ext=cpp -ac_cpp='$CXXCPP $CPPFLAGS' -ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_cxx_compiler_gnu - -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_cxx_try_compile "$LINENO"; then : - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_ext=c -ac_cpp='$CPP $CPPFLAGS' -ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_c_compiler_gnu - -ac_cxx_werror_flag=$ac_save_cxx_werror_flag -CXXFLAGS="$pgac_save_CXXFLAGS" -CXX="$pgac_save_CXX" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -fi - - NOT_THE_CFLAGS="" { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS" >&5 $as_echo_n "checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS... " >&6; } diff --git a/configure.ac b/configure.ac index c7322e292cc..86961b90eff 100644 --- a/configure.ac +++ b/configure.ac @@ -634,9 +634,6 @@ if test "$with_llvm" = yes ; then PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-fexcess-precision=standard]) PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-fexcess-precision=standard]) - PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-Xclang -no-opaque-pointers]) - PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-Xclang -no-opaque-pointers]) - NOT_THE_CFLAGS="" PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, NOT_THE_CFLAGS, [-Wunused-command-line-argument]) if test -n "$NOT_THE_CFLAGS"; then diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 8f9c77eedc1..9960a0239c8 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -835,19 +835,6 @@ llvm_session_initialize(void) llvm_llvm_context_reuse_count = 0; } - /* - * When targeting LLVM 15, turn off opaque pointers for the context we - * build our code in. We don't need to do so for other contexts (e.g. - * llvm_ts_context). Once the IR is generated, it carries the necessary - * information. - * - * For 16 and above, opaque pointers must be used, and we have special - * code for that. - */ -#if LLVM_VERSION_MAJOR == 15 - LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false); -#endif - /* * Synchronize types early, as that also includes inferring the target * triple. diff --git a/src/backend/jit/llvm/meson.build b/src/backend/jit/llvm/meson.build index 41c759f73c5..88b4a212bd1 100644 --- a/src/backend/jit/llvm/meson.build +++ b/src/backend/jit/llvm/meson.build @@ -60,9 +60,6 @@ endif # XXX: Need to determine proper version of the function cflags for clang bitcode_cflags = ['-fno-strict-aliasing', '-fwrapv'] -if llvm.version().version_compare('=15.0') - bitcode_cflags += ['-Xclang', '-no-opaque-pointers'] -endif bitcode_cflags += cppflags # XXX: Worth improving on the logic to find directories here diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h index 4f35f3dca13..0a04c85d9b9 100644 --- a/src/include/jit/llvmjit_emit.h +++ b/src/include/jit/llvmjit_emit.h @@ -107,41 +107,25 @@ l_pbool_const(bool i) static inline LLVMValueRef l_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildStructGEP(b, v, idx, ""); -#else return LLVMBuildStructGEP2(b, t, v, idx, ""); -#endif } static inline LLVMValueRef l_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef *indices, int32 nindices, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildGEP(b, v, indices, nindices, name); -#else return LLVMBuildGEP2(b, t, v, indices, nindices, name); -#endif } static inline LLVMValueRef l_load(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildLoad(b, v, name); -#else return LLVMBuildLoad2(b, t, v, name); -#endif } static inline LLVMValueRef l_call(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef fn, LLVMValueRef *args, int32 nargs, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildCall(b, fn, args, nargs, name); -#else return LLVMBuildCall2(b, t, fn, args, nargs, name); -#endif } /* -- 2.39.2 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-16 15:17 Nazir Bilal Yavuz <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Nazir Bilal Yavuz @ 2024-05-16 15:17 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers Hi, On Thu, 16 May 2024 at 05:34, Thomas Munro <[email protected]> wrote: > > On Wed, May 15, 2024 at 5:20 PM Peter Eisentraut <[email protected]> wrote: > > Yes, let's get that v3-0001 patch into PG17. > > Done. > > Bilal recently created the CI images for Debian Bookworm[1]. You can > try them with s/bullseye/bookworm/ in .cirrus.tasks.yml, but it looks > like he is still wrestling with a perl installation problem[2] in the > 32 bit build, so here is a temporary patch to do that and also delete > the 32 bit tests for now. This way cfbot should succeed with the > remaining patches. Parked here for v18. Actually, 32 bit builds are working but the Perl version needs to be updated to 'perl5.36-i386-linux-gnu' in .cirrus.tasks.yml. I changed 0001 with the working version of 32 bit builds [1] and the rest is the same. All tests pass now [2]. [1] postgr.es/m/CAN55FZ0fY5EFHXLKCO_=p4pwFmHRoVom_qSE_7B48gpchfAqzw@mail.gmail.com [2] https://cirrus-ci.com/task/4969910856581120 -- Regards, Nazir Bilal Yavuz Microsoft Attachments: [text/x-patch] v5-0001-Upgrade-Debian-CI-images-to-Bookworm.patch (2.3K, ../../CAN55FZ3c3PEQqSvofF1e8XcesA4HesBdiA3OwPzEXyTwfU74Vg@mail.gmail.com/2-v5-0001-Upgrade-Debian-CI-images-to-Bookworm.patch) download | inline diff: From 976d2c7ad0e470b24875ee27171359f54078a761 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz <[email protected]> Date: Mon, 13 May 2024 10:56:28 +0300 Subject: [PATCH v5 1/3] Upgrade Debian CI images to Bookworm New Debian version, namely Bookworm, is released. Use these new images in CI tasks. Perl version is upgraded in the Bookworm images, so update Perl version at 'Linux - Debian Bookworm - Meson' task as well. Upgrading Debian CI images to Bookworm PR: https://github.com/anarazel/pg-vm-images/pull/91 --- .cirrus.tasks.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index a2388cd5036..47a60aa7c6f 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -65,7 +65,7 @@ task: CPUS: 4 BUILD_JOBS: 8 TEST_JOBS: 8 - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm CCACHE_DIR: ${CIRRUS_WORKING_DIR}/ccache_dir # no options enabled, should be small CCACHE_MAXSIZE: "150M" @@ -243,7 +243,7 @@ task: CPUS: 4 BUILD_JOBS: 4 TEST_JOBS: 8 # experimentally derived to be a decent choice - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm CCACHE_DIR: /tmp/ccache_dir DEBUGINFOD_URLS: "https://debuginfod.debian.net" @@ -314,7 +314,7 @@ task: #DEBIAN_FRONTEND=noninteractive apt-get -y install ... matrix: - - name: Linux - Debian Bullseye - Autoconf + - name: Linux - Debian Bookworm - Autoconf env: SANITIZER_FLAGS: -fsanitize=address @@ -348,7 +348,7 @@ task: on_failure: <<: *on_failure_ac - - name: Linux - Debian Bullseye - Meson + - name: Linux - Debian Bookworm - Meson env: CCACHE_MAXSIZE: "400M" # tests two different builds @@ -375,7 +375,7 @@ task: ${LINUX_MESON_FEATURES} \ -Dllvm=disabled \ --pkg-config-path /usr/lib/i386-linux-gnu/pkgconfig/ \ - -DPERL=perl5.32-i386-linux-gnu \ + -DPERL=perl5.36-i386-linux-gnu \ -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \ build-32 EOF @@ -652,7 +652,7 @@ task: env: CPUS: 4 BUILD_JOBS: 4 - IMAGE_FAMILY: pg-ci-bullseye + IMAGE_FAMILY: pg-ci-bookworm # Use larger ccache cache, as this task compiles with multiple compilers / # flag combinations -- 2.43.0 [text/x-patch] v5-0002-jit-Require-at-least-LLVM-14-if-enabled.patch (14.3K, ../../CAN55FZ3c3PEQqSvofF1e8XcesA4HesBdiA3OwPzEXyTwfU74Vg@mail.gmail.com/3-v5-0002-jit-Require-at-least-LLVM-14-if-enabled.patch) download | inline diff: From a18120ace64bcde41c2ed23a050eb86f9b8772e0 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 19 Oct 2023 04:45:46 +1300 Subject: [PATCH v5 2/3] jit: Require at least LLVM 14, if enabled. Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Reviewed-by: Peter Eisentraut <[email protected]> Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 101 ------------------------ src/backend/jit/llvm/llvmjit_error.cpp | 25 ------ src/backend/jit/llvm/llvmjit_inline.cpp | 13 --- src/backend/jit/llvm/llvmjit_wrap.cpp | 4 - config/llvm.m4 | 4 +- doc/src/sgml/installation.sgml | 4 +- configure | 4 +- meson.build | 2 +- 8 files changed, 7 insertions(+), 150 deletions(-) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 1d439f24554..8f9c77eedc1 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -21,13 +21,9 @@ #if LLVM_VERSION_MAJOR > 16 #include <llvm-c/Transforms/PassBuilder.h> #endif -#if LLVM_VERSION_MAJOR > 11 #include <llvm-c/Orc.h> #include <llvm-c/OrcEE.h> #include <llvm-c/LLJIT.h> -#else -#include <llvm-c/OrcBindings.h> -#endif #include <llvm-c/Support.h> #include <llvm-c/Target.h> #if LLVM_VERSION_MAJOR < 17 @@ -50,13 +46,8 @@ /* Handle of a module emitted via ORC JIT */ typedef struct LLVMJitHandle { -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef lljit; LLVMOrcResourceTrackerRef resource_tracker; -#else - LLVMOrcJITStackRef stack; - LLVMOrcModuleHandle orc_handle; -#endif } LLVMJitHandle; @@ -103,14 +94,9 @@ static LLVMContextRef llvm_context; static LLVMTargetRef llvm_targetref; -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcThreadSafeContextRef llvm_ts_context; static LLVMOrcLLJITRef llvm_opt0_orc; static LLVMOrcLLJITRef llvm_opt3_orc; -#else /* LLVM_VERSION_MAJOR > 11 */ -static LLVMOrcJITStackRef llvm_opt0_orc; -static LLVMOrcJITStackRef llvm_opt3_orc; -#endif /* LLVM_VERSION_MAJOR > 11 */ static void llvm_release_context(JitContext *context); @@ -124,10 +110,8 @@ static void llvm_set_target(void); static void llvm_recreate_llvm_context(void); static uint64_t llvm_resolve_symbol(const char *name, void *ctx); -#if LLVM_VERSION_MAJOR > 11 static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm); static char *llvm_error_message(LLVMErrorRef error); -#endif /* LLVM_VERSION_MAJOR > 11 */ /* ResourceOwner callbacks to hold JitContexts */ static void ResOwnerReleaseJitContext(Datum res); @@ -292,7 +276,6 @@ llvm_release_context(JitContext *context) { LLVMJitHandle *jit_handle = (LLVMJitHandle *) lfirst(lc); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcExecutionSessionRef ee; LLVMOrcSymbolStringPoolRef sp; @@ -310,11 +293,6 @@ llvm_release_context(JitContext *context) sp = LLVMOrcExecutionSessionGetSymbolStringPool(ee); LLVMOrcSymbolStringPoolClearDeadEntries(sp); } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - LLVMOrcRemoveModule(jit_handle->stack, jit_handle->orc_handle); - } -#endif /* LLVM_VERSION_MAJOR > 11 */ pfree(jit_handle); } @@ -397,7 +375,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) * to mangle here. */ -#if LLVM_VERSION_MAJOR > 11 foreach(lc, context->handles) { LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); @@ -427,19 +404,6 @@ llvm_get_function(LLVMJitContext *context, const char *funcname) if (addr) return (void *) (uintptr_t) addr; } -#else - foreach(lc, context->handles) - { - LLVMOrcTargetAddress addr; - LLVMJitHandle *handle = (LLVMJitHandle *) lfirst(lc); - - addr = 0; - if (LLVMOrcGetSymbolAddressIn(handle->stack, &addr, handle->orc_handle, funcname)) - elog(ERROR, "failed to look up symbol \"%s\"", funcname); - if (addr) - return (void *) (uintptr_t) addr; - } -#endif elog(ERROR, "failed to JIT: %s", funcname); @@ -735,11 +699,7 @@ llvm_compile_module(LLVMJitContext *context) MemoryContext oldcontext; instr_time starttime; instr_time endtime; -#if LLVM_VERSION_MAJOR > 11 LLVMOrcLLJITRef compile_orc; -#else - LLVMOrcJITStackRef compile_orc; -#endif if (context->base.flags & PGJIT_OPT3) compile_orc = llvm_opt3_orc; @@ -796,7 +756,6 @@ llvm_compile_module(LLVMJitContext *context) * faster instruction selection mechanism is used. */ INSTR_TIME_SET_CURRENT(starttime); -#if LLVM_VERSION_MAJOR > 11 { LLVMOrcThreadSafeModuleRef ts_module; LLVMErrorRef error; @@ -824,16 +783,6 @@ llvm_compile_module(LLVMJitContext *context) /* LLVMOrcLLJITAddLLVMIRModuleWithRT takes ownership of the module */ } -#else - { - handle->stack = compile_orc; - if (LLVMOrcAddEagerlyCompiledIR(compile_orc, &handle->orc_handle, context->module, - llvm_resolve_symbol, NULL)) - elog(ERROR, "failed to JIT module"); - - /* LLVMOrcAddEagerlyCompiledIR takes ownership of the module */ - } -#endif INSTR_TIME_SET_CURRENT(endtime); INSTR_TIME_ACCUM_DIFF(context->base.instr.emission_counter, @@ -945,7 +894,6 @@ llvm_session_initialize(void) /* force symbols in main binary to be loaded */ LLVMLoadLibraryPermanently(NULL); -#if LLVM_VERSION_MAJOR > 11 { llvm_ts_context = LLVMOrcCreateNewThreadSafeContext(); @@ -955,31 +903,6 @@ llvm_session_initialize(void) llvm_opt3_orc = llvm_create_jit_instance(opt3_tm); opt3_tm = 0; } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - llvm_opt0_orc = LLVMOrcCreateInstance(opt0_tm); - llvm_opt3_orc = LLVMOrcCreateInstance(opt3_tm); - -#if defined(HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER) && HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER - if (jit_debugging_support) - { - LLVMJITEventListenerRef l = LLVMCreateGDBRegistrationListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif -#if defined(HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER) && HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER - if (jit_profiling_support) - { - LLVMJITEventListenerRef l = LLVMCreatePerfJITEventListener(); - - LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); - LLVMOrcRegisterJITEventListener(llvm_opt3_orc, l); - } -#endif - } -#endif /* LLVM_VERSION_MAJOR > 11 */ on_proc_exit(llvm_shutdown, 0); @@ -1009,7 +932,6 @@ llvm_shutdown(int code, Datum arg) elog(PANIC, "LLVMJitContext in use count not 0 at exit (is %zu)", llvm_jit_context_in_use_count); -#if LLVM_VERSION_MAJOR > 11 { if (llvm_opt3_orc) { @@ -1027,23 +949,6 @@ llvm_shutdown(int code, Datum arg) llvm_ts_context = NULL; } } -#else /* LLVM_VERSION_MAJOR > 11 */ - { - /* unregister profiling support, needs to be flushed to be useful */ - - if (llvm_opt3_orc) - { - LLVMOrcDisposeInstance(llvm_opt3_orc); - llvm_opt3_orc = NULL; - } - - if (llvm_opt0_orc) - { - LLVMOrcDisposeInstance(llvm_opt0_orc); - llvm_opt0_orc = NULL; - } - } -#endif /* LLVM_VERSION_MAJOR > 11 */ } /* helper for llvm_create_types, returning a function's return type */ @@ -1213,8 +1118,6 @@ llvm_resolve_symbol(const char *symname, void *ctx) return (uint64_t) addr; } -#if LLVM_VERSION_MAJOR > 11 - static LLVMErrorRef llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, LLVMOrcLookupStateRef *LookupState, LLVMOrcLookupKind Kind, @@ -1233,9 +1136,7 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, { const char *name = LLVMOrcSymbolStringPoolEntryStr(LookupSet[i].Name); -#if LLVM_VERSION_MAJOR > 12 LLVMOrcRetainSymbolStringPoolEntry(LookupSet[i].Name); -#endif symbols[i].Name = LookupSet[i].Name; symbols[i].Sym.Address = llvm_resolve_symbol(name, NULL); symbols[i].Sym.Flags.GenericFlags = LLVMJITSymbolGenericFlagsExported; @@ -1364,8 +1265,6 @@ llvm_error_message(LLVMErrorRef error) return msg; } -#endif /* LLVM_VERSION_MAJOR > 11 */ - /* * ResourceOwner callbacks */ diff --git a/src/backend/jit/llvm/llvmjit_error.cpp b/src/backend/jit/llvm/llvmjit_error.cpp index ebe2f1baa10..351354c30bc 100644 --- a/src/backend/jit/llvm/llvmjit_error.cpp +++ b/src/backend/jit/llvm/llvmjit_error.cpp @@ -30,13 +30,7 @@ static std::new_handler old_new_handler = NULL; static void fatal_system_new_handler(void); static void fatal_llvm_new_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_new_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif static void fatal_llvm_error_handler(void *user_data, const char *reason, bool gen_crash_diag); -#if LLVM_VERSION_MAJOR < 14 -static void fatal_llvm_error_handler(void *user_data, const std::string& reason, bool gen_crash_diag); -#endif /* @@ -135,15 +129,6 @@ fatal_llvm_new_handler(void *user_data, errmsg("out of memory"), errdetail("While in LLVM: %s", reason))); } -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_new_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_new_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif static void fatal_llvm_error_handler(void *user_data, @@ -154,13 +139,3 @@ fatal_llvm_error_handler(void *user_data, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("fatal llvm error: %s", reason))); } - -#if LLVM_VERSION_MAJOR < 14 -static void -fatal_llvm_error_handler(void *user_data, - const std::string& reason, - bool gen_crash_diag) -{ - fatal_llvm_error_handler(user_data, reason.c_str(), gen_crash_diag); -} -#endif diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp index 2007eb523c9..23a8053311d 100644 --- a/src/backend/jit/llvm/llvmjit_inline.cpp +++ b/src/backend/jit/llvm/llvmjit_inline.cpp @@ -594,10 +594,6 @@ function_inlinable(llvm::Function &F, if (F.materialize()) elog(FATAL, "failed to materialize metadata"); -#if LLVM_VERSION_MAJOR < 14 -#define hasFnAttr hasFnAttribute -#endif - if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline)) { ilog(DEBUG1, "ineligibile to import %s due to noinline", @@ -858,9 +854,6 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, llvm::Function *AF; llvm::BasicBlock *BB; llvm::CallInst *fwdcall; -#if LLVM_VERSION_MAJOR < 14 - llvm::Attribute inlineAttribute; -#endif AF = llvm::Function::Create(F->getFunctionType(), LinkageTypes::AvailableExternallyLinkage, @@ -869,13 +862,7 @@ create_redirection_function(std::unique_ptr<llvm::Module> &importMod, Builder.SetInsertPoint(BB); fwdcall = Builder.CreateCall(F, &*AF->arg_begin()); -#if LLVM_VERSION_MAJOR < 14 - inlineAttribute = llvm::Attribute::get(Context, - llvm::Attribute::AlwaysInline); - fwdcall->addAttribute(~0U, inlineAttribute); -#else fwdcall->addFnAttr(llvm::Attribute::AlwaysInline); -#endif Builder.CreateRet(fwdcall); return AF; diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 641c8841ca3..7f7623dac64 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -17,10 +17,6 @@ extern "C" } #include <llvm-c/Core.h> - -/* Avoid macro clash with LLVM's C++ headers */ -#undef Min - #include <llvm/IR/Function.h> #include "jit/llvmjit.h" diff --git a/config/llvm.m4 b/config/llvm.m4 index c6cf8858f64..fa4bedd9370 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -25,8 +25,8 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], AC_MSG_ERROR([$LLVM_CONFIG does not work]) fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 10) exit 1; else exit 0;}';then - AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required]) + if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 14) exit 1; else exit 0;}';then + AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required]) fi AC_MSG_NOTICE([using llvm $pgac_llvm_version]) diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 1b32d5ca62c..91dadc10fbb 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -936,7 +936,7 @@ build-postgresql: <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of <productname>LLVM</productname> is - currently 10. + currently 14. </para> <para> <command>llvm-config</command><indexterm><primary>llvm-config</primary></indexterm> @@ -2424,7 +2424,7 @@ ninja install <acronym>JIT</acronym> compilation (see <xref linkend="jit"/>). This requires the <productname>LLVM</productname> library to be installed. The minimum required version of - <productname>LLVM</productname> is currently 10. Disabled by + <productname>LLVM</productname> is currently 14. Disabled by default. </para> diff --git a/configure b/configure index 8e7704d54bd..4b72a0d9f6a 100755 --- a/configure +++ b/configure @@ -5129,8 +5129,8 @@ fi as_fn_error $? "$LLVM_CONFIG does not work" "$LINENO" 5 fi # and whether the version is supported - if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 10) exit 1; else exit 0;}';then - as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 10 is required" "$LINENO" 5 + if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 14) exit 1; else exit 0;}';then + as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 14 is required" "$LINENO" 5 fi { $as_echo "$as_me:${as_lineno-$LINENO}: using llvm $pgac_llvm_version" >&5 $as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} diff --git a/meson.build b/meson.build index 1c0579d5a6b..1b845203780 100644 --- a/meson.build +++ b/meson.build @@ -748,7 +748,7 @@ endif llvmopt = get_option('llvm') llvm = not_found_dep if add_languages('cpp', required: llvmopt, native: false) - llvm = dependency('llvm', version: '>=10', method: 'config-tool', required: llvmopt) + llvm = dependency('llvm', version: '>=14', method: 'config-tool', required: llvmopt) if llvm.found() -- 2.43.0 [text/x-patch] v5-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch (8.2K, ../../CAN55FZ3c3PEQqSvofF1e8XcesA4HesBdiA3OwPzEXyTwfU74Vg@mail.gmail.com/4-v5-0003-jit-Use-opaque-pointers-in-all-supported-LLVM-ver.patch) download | inline diff: From e570133b50361777a369fd385e186385d03b79fa Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 20 Oct 2023 15:13:26 +1300 Subject: [PATCH v5 3/3] jit: Use opaque pointers in all supported LLVM versions. LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Reviewed-by: Peter Eisentraut <[email protected]> Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com --- src/include/jit/llvmjit_emit.h | 16 ------ src/backend/jit/llvm/llvmjit.c | 13 ----- src/backend/jit/llvm/meson.build | 3 -- configure | 89 -------------------------------- configure.ac | 3 -- 5 files changed, 124 deletions(-) diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h index 4f35f3dca13..0a04c85d9b9 100644 --- a/src/include/jit/llvmjit_emit.h +++ b/src/include/jit/llvmjit_emit.h @@ -107,41 +107,25 @@ l_pbool_const(bool i) static inline LLVMValueRef l_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildStructGEP(b, v, idx, ""); -#else return LLVMBuildStructGEP2(b, t, v, idx, ""); -#endif } static inline LLVMValueRef l_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef *indices, int32 nindices, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildGEP(b, v, indices, nindices, name); -#else return LLVMBuildGEP2(b, t, v, indices, nindices, name); -#endif } static inline LLVMValueRef l_load(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildLoad(b, v, name); -#else return LLVMBuildLoad2(b, t, v, name); -#endif } static inline LLVMValueRef l_call(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef fn, LLVMValueRef *args, int32 nargs, const char *name) { -#if LLVM_VERSION_MAJOR < 16 - return LLVMBuildCall(b, fn, args, nargs, name); -#else return LLVMBuildCall2(b, t, fn, args, nargs, name); -#endif } /* diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 8f9c77eedc1..9960a0239c8 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -835,19 +835,6 @@ llvm_session_initialize(void) llvm_llvm_context_reuse_count = 0; } - /* - * When targeting LLVM 15, turn off opaque pointers for the context we - * build our code in. We don't need to do so for other contexts (e.g. - * llvm_ts_context). Once the IR is generated, it carries the necessary - * information. - * - * For 16 and above, opaque pointers must be used, and we have special - * code for that. - */ -#if LLVM_VERSION_MAJOR == 15 - LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false); -#endif - /* * Synchronize types early, as that also includes inferring the target * triple. diff --git a/src/backend/jit/llvm/meson.build b/src/backend/jit/llvm/meson.build index 41c759f73c5..88b4a212bd1 100644 --- a/src/backend/jit/llvm/meson.build +++ b/src/backend/jit/llvm/meson.build @@ -60,9 +60,6 @@ endif # XXX: Need to determine proper version of the function cflags for clang bitcode_cflags = ['-fno-strict-aliasing', '-fwrapv'] -if llvm.version().version_compare('=15.0') - bitcode_cflags += ['-Xclang', '-no-opaque-pointers'] -endif bitcode_cflags += cppflags # XXX: Worth improving on the logic to find directories here diff --git a/configure b/configure index 4b72a0d9f6a..abad7553f8b 100755 --- a/configure +++ b/configure @@ -7290,95 +7290,6 @@ if test x"$pgac_cv_prog_CLANGXX_cxxflags__fexcess_precision_standard" = x"yes"; fi - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS" >&5 -$as_echo_n "checking whether ${CLANG} supports -Xclang -no-opaque-pointers, for BITCODE_CFLAGS... " >&6; } -if ${pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CFLAGS=$CFLAGS -pgac_save_CC=$CC -CC=${CLANG} -CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -ac_save_c_werror_flag=$ac_c_werror_flag -ac_c_werror_flag=yes -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_c_werror_flag=$ac_save_c_werror_flag -CFLAGS="$pgac_save_CFLAGS" -CC="$pgac_save_CC" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANG_cflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CFLAGS="${BITCODE_CFLAGS} -Xclang -no-opaque-pointers" -fi - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS" >&5 -$as_echo_n "checking whether ${CLANGXX} supports -Xclang -no-opaque-pointers, for BITCODE_CXXFLAGS... " >&6; } -if ${pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CXXFLAGS=$CXXFLAGS -pgac_save_CXX=$CXX -CXX=${CLANGXX} -CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -ac_save_cxx_werror_flag=$ac_cxx_werror_flag -ac_cxx_werror_flag=yes -ac_ext=cpp -ac_cpp='$CXXCPP $CPPFLAGS' -ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_cxx_compiler_gnu - -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_cxx_try_compile "$LINENO"; then : - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=yes -else - pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_ext=c -ac_cpp='$CPP $CPPFLAGS' -ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' -ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' -ac_compiler_gnu=$ac_cv_c_compiler_gnu - -ac_cxx_werror_flag=$ac_save_cxx_werror_flag -CXXFLAGS="$pgac_save_CXXFLAGS" -CXX="$pgac_save_CXX" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&5 -$as_echo "$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" >&6; } -if test x"$pgac_cv_prog_CLANGXX_cxxflags__Xclang__no_opaque_pointers" = x"yes"; then - BITCODE_CXXFLAGS="${BITCODE_CXXFLAGS} -Xclang -no-opaque-pointers" -fi - - NOT_THE_CFLAGS="" { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS" >&5 $as_echo_n "checking whether ${CLANG} supports -Wunused-command-line-argument, for NOT_THE_CFLAGS... " >&6; } diff --git a/configure.ac b/configure.ac index c7322e292cc..86961b90eff 100644 --- a/configure.ac +++ b/configure.ac @@ -634,9 +634,6 @@ if test "$with_llvm" = yes ; then PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-fexcess-precision=standard]) PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-fexcess-precision=standard]) - PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, BITCODE_CFLAGS, [-Xclang -no-opaque-pointers]) - PGAC_PROG_VARCXX_VARFLAGS_OPT(CLANGXX, BITCODE_CXXFLAGS, [-Xclang -no-opaque-pointers]) - NOT_THE_CFLAGS="" PGAC_PROG_VARCC_VARFLAGS_OPT(CLANG, NOT_THE_CFLAGS, [-Wunused-command-line-argument]) if test -n "$NOT_THE_CFLAGS"; then -- 2.43.0 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-16 22:54 Thomas Munro <[email protected]> parent: Nazir Bilal Yavuz <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Thomas Munro @ 2024-05-16 22:54 UTC (permalink / raw) To: Nazir Bilal Yavuz <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers On Fri, May 17, 2024 at 3:17 AM Nazir Bilal Yavuz <[email protected]> wrote: > Actually, 32 bit builds are working but the Perl version needs to be > updated to 'perl5.36-i386-linux-gnu' in .cirrus.tasks.yml. I changed > 0001 with the working version of 32 bit builds [1] and the rest is the > same. All tests pass now [2]. Ahh, right, thanks! I will look at committing your CI/fixup patches. ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-18 22:46 Ole Peder Brandtzæg <[email protected]> parent: Peter Eisentraut <[email protected]> 1 sibling, 2 replies; 31+ messages in thread From: Ole Peder Brandtzæg @ 2024-05-18 22:46 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers On Wed, May 15, 2024 at 07:20:09AM +0200, Peter Eisentraut wrote: > Yes, let's get that v3-0001 patch into PG17. Upon seeing this get committed in 4dd29b6833, I noticed that the docs still advertise the llvm-config-$version search dance. That's still correct for Meson-based builds since we use their config-tool machinery, but no longer holds for configure-based builds. The attached patch updates the docs accordingly. -- Ole Peder Brandtzæg In any case, these nights just ain't getting any easier And who could judge us For seeking comfort in the hazy counterfeit land of memory Attachments: [text/x-diff] 0001-doc-remove-llvm-config-search-from-configure-doc.patch (2.1K, ../../[email protected]/2-0001-doc-remove-llvm-config-search-from-configure-doc.patch) download | inline diff: From 61dfbf5a252b53697cce17cd4885ecddb7665814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Peder=20Brandtz=C3=A6g?= <[email protected]> Date: Sun, 19 May 2024 00:29:09 +0200 Subject: [PATCH] doc: remove llvm-config search from configure documentation As of 4dd29b6833, we no longer attempt to locate any other llvm-config variant than plain llvm-config in configure-based builds; update the documentation accordingly. (For Meson-based builds, we still use Meson's LLVMDependencyConfigTool [0], which runs through a set of possible suffixes [1], so no need to update the documentation there.) [0]: https://github.com/mesonbuild/meson/blob/7d28ff29396f9d7043204de8ddc52226b9903811/mesonbuild/dependencies/dev.py#L184 [1]: https://github.com/mesonbuild/meson/blob/7d28ff29396f9d7043204de8ddc52226b9903811/mesonbuild/environment.py#L183 --- doc/src/sgml/installation.sgml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 1b32d5ca62..19abec2c34 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -941,12 +941,9 @@ build-postgresql: <para> <command>llvm-config</command><indexterm><primary>llvm-config</primary></indexterm> will be used to find the required compilation options. - <command>llvm-config</command>, and then - <command>llvm-config-$major-$minor</command> for all supported - versions, will be searched for in your <envar>PATH</envar>. If - that would not yield the desired program, - use <envar>LLVM_CONFIG</envar> to specify a path to the - correct <command>llvm-config</command>. For example + <command>llvm-config</command> will be searched for in your <envar>PATH</envar>. + If that would not yield the desired program, use <envar>LLVM_CONFIG</envar> to + specify a path to the correct <command>llvm-config</command>. For example <programlisting> ./configure ... --with-llvm LLVM_CONFIG='/path/to/llvm/bin/llvm-config' </programlisting> -- 2.43.0 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-18 23:05 Thomas Munro <[email protected]> parent: Ole Peder Brandtzæg <[email protected]> 1 sibling, 2 replies; 31+ messages in thread From: Thomas Munro @ 2024-05-18 23:05 UTC (permalink / raw) To: Ole Peder Brandtzæg <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers On Sun, May 19, 2024 at 10:46 AM Ole Peder Brandtzæg <[email protected]> wrote: > On Wed, May 15, 2024 at 07:20:09AM +0200, Peter Eisentraut wrote: > > Yes, let's get that v3-0001 patch into PG17. > > Upon seeing this get committed in 4dd29b6833, I noticed that the docs > still advertise the llvm-config-$version search dance. That's still > correct for Meson-based builds since we use their config-tool machinery, > but no longer holds for configure-based builds. The attached patch > updates the docs accordingly. Oops, right I didn't know we had that documented. Thanks. Will hold off doing anything until the thaw. Hmm, I also didn't know that Meson had its own list like our just-removed one: https://github.com/mesonbuild/meson/blob/master/mesonbuild/environment.py#L183 Unsurprisingly, it suffers from maintenance lag, priority issues etc (new major versions pop out every 6 months): https://github.com/mesonbuild/meson/issues/10483 ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-18 23:16 Ole Peder Brandtzæg <[email protected]> parent: Thomas Munro <[email protected]> 1 sibling, 0 replies; 31+ messages in thread From: Ole Peder Brandtzæg @ 2024-05-18 23:16 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers On Sun, May 19, 2024 at 11:05:49AM +1200, Thomas Munro wrote: > Oops, right I didn't know we had that documented. Thanks. Will hold > off doing anything until the thaw. No worries, thanks! > Hmm, I also didn't know that Meson had its own list like our just-removed one: > > https://github.com/mesonbuild/meson/blob/master/mesonbuild/environment.py#L183 I didn't either before writing the doc patch, which led me to investigate why it *just works* when doing meson setup and then I saw the 40 odd "Trying a default llvm-config fallback…" lines in meson-log.txt =) -- Ole Peder Brandtzæg It's raining triple sec in Tchula and the radio plays "Crazy Train" ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-05-18 23:16 Tom Lane <[email protected]> parent: Thomas Munro <[email protected]> 1 sibling, 0 replies; 31+ messages in thread From: Tom Lane @ 2024-05-18 23:16 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Ole Peder Brandtzæg <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers Thomas Munro <[email protected]> writes: > Oops, right I didn't know we had that documented. Thanks. Will hold > off doing anything until the thaw. FWIW, I don't think the release freeze precludes docs-only fixes. But if you prefer to sit on this, that's fine too. regards, tom lane ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-08-21 13:19 Peter Eisentraut <[email protected]> parent: Ole Peder Brandtzæg <[email protected]> 1 sibling, 0 replies; 31+ messages in thread From: Peter Eisentraut @ 2024-08-21 13:19 UTC (permalink / raw) To: Ole Peder Brandtzæg <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers On 19.05.24 00:46, Ole Peder Brandtzæg wrote: > On Wed, May 15, 2024 at 07:20:09AM +0200, Peter Eisentraut wrote: >> Yes, let's get that v3-0001 patch into PG17. > > Upon seeing this get committed in 4dd29b6833, I noticed that the docs > still advertise the llvm-config-$version search dance. That's still > correct for Meson-based builds since we use their config-tool machinery, > but no longer holds for configure-based builds. The attached patch > updates the docs accordingly. committed ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-08-21 14:00 Peter Eisentraut <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Peter Eisentraut @ 2024-08-21 14:00 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers; Nazir Bilal Yavuz <[email protected]> On 17.05.24 00:54, Thomas Munro wrote: > On Fri, May 17, 2024 at 3:17 AM Nazir Bilal Yavuz <[email protected]> wrote: >> Actually, 32 bit builds are working but the Perl version needs to be >> updated to 'perl5.36-i386-linux-gnu' in .cirrus.tasks.yml. I changed >> 0001 with the working version of 32 bit builds [1] and the rest is the >> same. All tests pass now [2]. > > Ahh, right, thanks! I will look at committing your CI/fixup patches. The CI images have been updated, so this should be ready to go now. I gave the remaining two patches a try on CI, and it all looks okay to me. (needed some gentle rebasing) ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-10-01 10:23 Peter Eisentraut <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Peter Eisentraut @ 2024-10-01 10:23 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers; Nazir Bilal Yavuz <[email protected]> On 21.08.24 16:00, Peter Eisentraut wrote: > On 17.05.24 00:54, Thomas Munro wrote: >> On Fri, May 17, 2024 at 3:17 AM Nazir Bilal Yavuz <[email protected]> >> wrote: >>> Actually, 32 bit builds are working but the Perl version needs to be >>> updated to 'perl5.36-i386-linux-gnu' in .cirrus.tasks.yml. I changed >>> 0001 with the working version of 32 bit builds [1] and the rest is the >>> same. All tests pass now [2]. >> >> Ahh, right, thanks! I will look at committing your CI/fixup patches. > > The CI images have been updated, so this should be ready to go now. I > gave the remaining two patches a try on CI, and it all looks okay to me. > (needed some gentle rebasing) I have committed the two remaining patches. I'll go nudge any affected buildfarm members to upgrade as needed. ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Requiring LLVM 14+ in PostgreSQL 18 @ 2024-10-02 02:36 Thomas Munro <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Thomas Munro @ 2024-10-02 02:36 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers; Nazir Bilal Yavuz <[email protected]> On Tue, Oct 1, 2024 at 11:23 PM Peter Eisentraut <[email protected]> wrote: > I have committed the two remaining patches. I'll go nudge any affected > buildfarm members to upgrade as needed. Thanks! FWIW next on my list of LLVM maintenance work are: fix ARM crash in the next week or so (CF #5220), and figure out which patches need to go where for JITLink, a small required API change for LLVM 20, unless someone else is interested in working on that soon (there was a patch[1] but the thread has gone quiet). [1] https://www.postgresql.org/message-id/flat/20220829074622.2474104-1-alex.fan.q%40gmail.com ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c) @ 2025-10-10 16:41 Ranier Vilela <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Ranier Vilela @ 2025-10-10 16:41 UTC (permalink / raw) To: Alvaro Herrera <[email protected]>; +Cc: pgsql-hackers Hi. Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]> escreveu: > Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera < > [email protected]> escreveu: > >> On 2024-Nov-12, Ranier Vilela wrote: >> >> > Per Coverity. >> > >> > The function *determineNotNullFlags* has a little oversight. >> > The struct field *notnull_islocal* is an array. >> > >> > I think this is a simple typo. >> > Fix using array notation access. >> >> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like >> to construct a test case that tickles that code, because it's currently >> uncovered *shudder* >> > Thanks for taking care of this. > Ping. best regards, Ranier Vilela ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c) @ 2025-10-11 05:24 Chao Li <[email protected]> parent: Ranier Vilela <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Chao Li @ 2025-10-11 05:24 UTC (permalink / raw) To: Ranier Vilela <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers On Oct 11, 2025, at 00:41, Ranier Vilela <[email protected]> wrote: Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]> escreveu: > Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera < > [email protected]> escreveu: > >> On 2024-Nov-12, Ranier Vilela wrote: >> >> > Per Coverity. >> > >> > The function *determineNotNullFlags* has a little oversight. >> > The struct field *notnull_islocal* is an array. >> > >> > I think this is a simple typo. >> > Fix using array notation access. >> >> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like >> to construct a test case that tickles that code, because it's currently >> uncovered *shudder* >> > Thanks for taking care of this. > Ping. I tried to debug this bug, and it looks like this bug can never be triggered. To do the test, I created two tables: ``` evantest=# CREATE TABLE parent_nn(a int NOT NULL, b int); CREATE TABLE evantest=# CREATE TABLE child_nn() INHERITS (parent_nn); CREATE TABLE evantest=# ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL; ALTER TABLE ``` Then let’s look at the code: /* * In binary upgrade of inheritance child tables, must have a * constraint name that we can UPDATE later; same if there's a * comment on the constraint. */ if ((dopt->binary_upgrade && !tbinfo->ispartition && !tbinfo->notnull_islocal) || !PQgetisnull(res, r, i_notnull_comment)) // A { const char *val = PQgetvalue(res, r, i_notnull_name); // B tbinfo->notnull_constrs[j] = pstrdup(val); } else { char *default_name; const char *val = PQgetvalue(res, r, i_notnull_name); /* XXX should match ChooseConstraintName better */ default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name, tbinfo->attnames[j]); if (strcmp(default_name, // C val) == 0) tbinfo->notnull_constrs[j] = ""; else { tbinfo->notnull_constrs[j] = // D pstrdup(val); } free(default_name); } Notice that I marked four lines with A/B/C/D. For child table’s column “b”, when it reaches line A, it hits the bug, as tbinfo->notnull_islocal[j] should be false, but tbinfo->notnull_islocal is always true. If the bug is fixed, as tbinfo->notnull_islocal[j] is false, it will enter the “if” clause, in line B, PGgetvalue() will return “parent_nn_a_not_null”. With this bug, if will go the the “else” clause, run strcmp() in line C. Here, “default_name” is built from the table name, its value is “child_nn_a_not_null”, while PGgetvalue() is “parent_nn_a_not_null”, thus it won’t meet the “if” of “strcmp”, instead it goes to line D, that runs the same assignment as in line B. So, Ranier, please let me know if you have an example that generates the wrong result, then I can verify the fix with your test case. But I believe we should still fix the bug: 1) otherwise the code is confusing 2) after fixing, when tbinfo->notnull_islocal[j] is false, the execution path is shorter 3) to make Coverity happy. I also added a TAP test with test cases for determining NULL: ``` chaol@ChaodeMacBook-Air pg_dump % make check PROVE_TESTS='t/ 011_dump_determine_null.pl' # +++ tap check in src/bin/pg_dump +++ t/011_dump_determine_null.pl .. ok All tests successful. Files=1, Tests=5, 2 wallclock secs ( 0.00 usr 0.01 sys + 0.08 cusr 0.31 csys = 0.40 CPU) Result: PASS ``` Please see the attached patch. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v1-0001-Fixed-a-bug-in-pg_dump-about-determining-NotNull.patch (3.9K, ../../CAEoWx2nJXLDb_gZuGcAE4rCfwUX=ndtkcBx55ynNSyFPOgXpEQ@mail.gmail.com/3-v1-0001-Fixed-a-bug-in-pg_dump-about-determining-NotNull.patch) download | inline diff: From 92608d497d779000077d13dae6c2064549c3785f Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Sat, 11 Oct 2025 13:13:32 +0800 Subject: [PATCH v1] Fixed a bug in pg_dump about determining NotNull In determineNotNullFlags(), a check should be done against tbinfo->notnull_islocal[j], but "[j]" was missing. However, this bug cannot be actually fired, see the discussion. This commit fixes the bug and adds a TAP test of determining NotNull for pg_dump. Author: Chao Li <[email protected]> Discussion: https://www.postgresql.org/message-id/CAEudQAo7ah%3D4TDheuEjtb0dsv6bHoK7uBNqv53Tsub2h-xBSJw%40mail.gmail.com --- src/bin/pg_dump/meson.build | 1 + src/bin/pg_dump/pg_dump.c | 2 +- src/bin/pg_dump/t/011_dump_determine_null.pl | 59 ++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/bin/pg_dump/t/011_dump_determine_null.pl diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build index a2233b0a1b4..47225b818da 100644 --- a/src/bin/pg_dump/meson.build +++ b/src/bin/pg_dump/meson.build @@ -103,6 +103,7 @@ tests += { 't/004_pg_dump_parallel.pl', 't/005_pg_dump_filterfile.pl', 't/010_dump_connstr.pl', + 't/011_dump_determine_null.pl', ], }, } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 641bece12c7..9dc9396a34d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -10074,7 +10074,7 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r, */ if ((dopt->binary_upgrade && !tbinfo->ispartition && - !tbinfo->notnull_islocal) || + !tbinfo->notnull_islocal[j]) || !PQgetisnull(res, r, i_notnull_comment)) { tbinfo->notnull_constrs[j] = diff --git a/src/bin/pg_dump/t/011_dump_determine_null.pl b/src/bin/pg_dump/t/011_dump_determine_null.pl new file mode 100644 index 00000000000..11d93135dd8 --- /dev/null +++ b/src/bin/pg_dump/t/011_dump_determine_null.pl @@ -0,0 +1,59 @@ +# Copyright (c) 2021-2025, PostgreSQL Global Development Group + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $tempdir = PostgreSQL::Test::Utils::tempdir; +my $node = PostgreSQL::Test::Cluster->new('main'); +my $port = $node->port; +my $backupdir = $node->backup_dir; +my $plainfile = "$backupdir/plain.sql"; + +# Init & start the node (no extra args; follow conventions used by other tests) +$node->init; +$node->start; + +# Setup parent/child table for binary upgrade NOT NULL check +$node->safe_psql('postgres', "CREATE TABLE parent_nn(a int NOT NULL, b int)"); +$node->safe_psql('postgres', "CREATE TABLE child_nn() INHERITS (parent_nn)"); + +# Add a local NOT NULL to child column 'b' +$node->safe_psql('postgres', "ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL"); + +# Do a schema-only pg_dump through the tap helper +command_ok( + [ + 'pg_dump', + '--file' => $plainfile, + '--schema-only', + '--binary-upgrade', + '--port' => $port, + 'postgres' + ], + 'pg_dump schema-only succeeds' +); + +# Read dump and assert expected NOT NULL markings +my $dump = slurp_file($plainfile); + +# Check parent table that NOT NULL is there +ok($dump =~ qr/CREATE TABLE public\.parent_nn\s*\(\s*a integer NOT NULL/m, + "parent NOT NULL column 'a' is there"); + +# Check parent table that NULL-able is there +ok($dump =~ qr/CREATE TABLE public\.parent_nn\s*\(\s*a.*,\s*b integer/m, + "parent NULL-able column 'b' is there"); + +# Check child table that inherited NOT NULL is preserved +ok($dump =~ qr/CREATE TABLE public\.child_nn\s*\(\s*a integer CONSTRAINT parent_nn_a_not_null NOT NULL/m, + "inherited NOT NULL column 'a' preserved"); + +# Check child table that locally added NOT NULL is preserved +ok($dump =~ qr/CREATE TABLE public\.child_nn\s*\(\s*a.*,\s*b integer NOT NULL/m, + "child local NOT NULL column 'b' preserved"); + +done_testing(); -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c) @ 2025-10-11 10:10 Ranier Vilela <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 31+ messages in thread From: Ranier Vilela @ 2025-10-11 10:10 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers Em sáb., 11 de out. de 2025 às 02:24, Chao Li <[email protected]> escreveu: > > On Oct 11, 2025, at 00:41, Ranier Vilela <[email protected]> wrote: > > Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]> > escreveu: > >> Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera < >> [email protected]> escreveu: >> >>> On 2024-Nov-12, Ranier Vilela wrote: >>> >>> > Per Coverity. >>> > >>> > The function *determineNotNullFlags* has a little oversight. >>> > The struct field *notnull_islocal* is an array. >>> > >>> > I think this is a simple typo. >>> > Fix using array notation access. >>> >>> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like >>> to construct a test case that tickles that code, because it's currently >>> uncovered *shudder* >>> >> Thanks for taking care of this. >> > Ping. > > > I tried to debug this bug, and it looks like this bug can never be > triggered. > > To do the test, I created two tables: > ``` > evantest=# CREATE TABLE parent_nn(a int NOT NULL, b int); > CREATE TABLE > evantest=# CREATE TABLE child_nn() INHERITS (parent_nn); > CREATE TABLE > evantest=# ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL; > ALTER TABLE > ``` > > Then let’s look at the code: > > /* > * In binary upgrade of inheritance child tables, must have a > * constraint name that we can UPDATE later; same if there's a > * comment on the constraint. > */ > if ((dopt->binary_upgrade && > !tbinfo->ispartition && > !tbinfo->notnull_islocal) || > !PQgetisnull(res, r, i_notnull_comment)) // A > { > const char *val = PQgetvalue(res, r, i_notnull_name); // B > tbinfo->notnull_constrs[j] = > pstrdup(val); > } > else > { > char *default_name; > const char *val = PQgetvalue(res, r, i_notnull_name); > > /* XXX should match ChooseConstraintName better */ > default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name, > tbinfo->attnames[j]); > if (strcmp(default_name, // C > val) == 0) > tbinfo->notnull_constrs[j] = ""; > else > { > tbinfo->notnull_constrs[j] = // D > pstrdup(val); > } > free(default_name); > } > > Notice that I marked four lines with A/B/C/D. > > For child table’s column “b”, when it reaches line A, it hits the bug, > as tbinfo->notnull_islocal[j] should be false, but tbinfo->notnull_islocal > is always true. > > If the bug is fixed, as tbinfo->notnull_islocal[j] is false, it will enter > the “if” clause, in line B, PGgetvalue() will return “parent_nn_a_not_null”. > > With this bug, if will go the the “else” clause, run strcmp() in line C. > Here, “default_name” is built from the table name, its value is > “child_nn_a_not_null”, while PGgetvalue() is “parent_nn_a_not_null”, thus > it won’t meet the “if” of “strcmp”, instead it goes to line D, that runs > the same assignment as in line B. > > So, Ranier, please let me know if you have an example that generates the > wrong result, then I can verify the fix with your test case. > > But I believe we should still fix the bug: 1) otherwise the code is > confusing 2) after fixing, when tbinfo->notnull_islocal[j] is false, the > execution path is shorter 3) to make Coverity happy. > > I also added a TAP test with test cases for determining NULL: > > ``` > chaol@ChaodeMacBook-Air pg_dump % make check PROVE_TESTS='t/ > 011_dump_determine_null.pl' > # +++ tap check in src/bin/pg_dump +++ > t/011_dump_determine_null.pl .. ok > All tests successful. > Files=1, Tests=5, 2 wallclock secs ( 0.00 usr 0.01 sys + 0.08 cusr > 0.31 csys = 0.40 CPU) > Result: PASS > ``` > > Please see the attached patch. > Did you read the entire thread? I think it's very rude and inelegant to suggest a patch while taking advantage of another patch. Best regards, Ranier Vilela ^ permalink raw reply [nested|flat] 31+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c) @ 2025-10-11 11:59 Chao Li <[email protected]> parent: Ranier Vilela <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Chao Li @ 2025-10-11 11:59 UTC (permalink / raw) To: Ranier Vilela <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers > On Oct 11, 2025, at 18:10, Ranier Vilela <[email protected]> wrote: > > > Did you read the entire thread? > I think it's very rude and inelegant to suggest a patch while taking advantage of another patch. > > Best regards, > Ranier Vilela No, I guess I didn’t read the entire thread. I just saw ping and thought to help. If you don’t like, just ignore my previous response. ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH] Reproduce filtering issue. @ 2026-03-23 11:50 Antonin Houska <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Antonin Houska @ 2026-03-23 11:50 UTC (permalink / raw) --- contrib/test_decoding/expected/filtering.out | 74 ++++++++++++++ contrib/test_decoding/specs/filtering.spec | 101 +++++++++++++++++++ src/backend/executor/nodeModifyTable.c | 2 + src/backend/replication/logical/snapbuild.c | 3 + src/test/isolation/isolationtester.c | 9 +- 5 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 contrib/test_decoding/expected/filtering.out create mode 100644 contrib/test_decoding/specs/filtering.spec diff --git a/contrib/test_decoding/expected/filtering.out b/contrib/test_decoding/expected/filtering.out new file mode 100644 index 00000000000..6ba9690509f --- /dev/null +++ b/contrib/test_decoding/expected/filtering.out @@ -0,0 +1,74 @@ +Parsed test spec with 5 sessions + +starting permutation: s1_assign_xid s3_repack s2_assign_xid s1_rollback s4_insert s5_wakeup_snapbuild s2_rollback s5_wakeup_insert_speculative s4_insert_commit s5_wakeup_repack +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step s1_assign_xid: + BEGIN; + CREATE TABLE c(i int); + +step s3_repack: + REPACK (CONCURRENTLY) a; + <waiting ...> +step s2_assign_xid: + BEGIN; + CREATE TABLE d(i int); + +step s1_rollback: + ROLLBACK; + +step s4_insert: + BEGIN; + INSERT INTO t(i) + SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i; + <waiting ...> +step s5_wakeup_snapbuild: + SELECT injection_points_wakeup('snapbuild-full'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_rollback: + ROLLBACK; + +step s5_wakeup_insert_speculative: + SELECT injection_points_wakeup('insert-speculative-before-confirm'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s4_insert: <... completed> +step s4_insert_commit: + COMMIT; + +step s5_wakeup_repack: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s3_repack: <... completed> +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/contrib/test_decoding/specs/filtering.spec b/contrib/test_decoding/specs/filtering.spec new file mode 100644 index 00000000000..a02c9f8facb --- /dev/null +++ b/contrib/test_decoding/specs/filtering.spec @@ -0,0 +1,101 @@ +setup +{ + CREATE TABLE a(i int primary key, j int) WITH (autovacuum_enabled = off); + INSERT INTO a(i, j) VALUES (1, 1), (2, 2); + CREATE TABLE t(i int primary key); + INSERT INTO t(i) VALUES (1); + CREATE EXTENSION injection_points; +} + +session s1 +step s1_assign_xid +{ + BEGIN; + CREATE TABLE c(i int); +} +step s1_rollback +{ + ROLLBACK; +} + +session s2 +step s2_assign_xid +{ + BEGIN; + CREATE TABLE d(i int); +} +step s2_rollback +{ + ROLLBACK; +} + +session s3 +setup +{ + SELECT injection_points_attach('snapbuild-full', 'wait'); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +step s3_repack +{ + REPACK (CONCURRENTLY) a; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); + SELECT injection_points_detach('snapbuild-full'); +} + +session s4 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('insert-speculative-before-confirm', 'wait'); +} +step s4_insert +{ + BEGIN; + INSERT INTO t(i) + SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i; +} +step s4_insert_commit +{ + COMMIT; +} +teardown +{ + SELECT injection_points_detach('insert-speculative-before-confirm'); +} + +session s5 +step s5_wakeup_snapbuild +{ + SELECT injection_points_wakeup('snapbuild-full'); +} +step s5_wakeup_insert_speculative +{ + SELECT injection_points_wakeup('insert-speculative-before-confirm'); +} +step s5_wakeup_repack +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +permutation +# Bring the snapshot builder to the FULL_SNAPSHOT state. +s1_assign_xid +s3_repack +s2_assign_xid +s1_rollback +# Perform the speculative insert, but no confirmation so far. The snapshot +# builder should decode it. +s4_insert +# Let the snapshout builder achieve CONSISTENT state and finish the setup. +s5_wakeup_snapbuild +s2_rollback +# While REPACK is waiting on repack-concurrently-before-lock, let the insert +# get confirmed. Relation filtering is now enabled. +s5_wakeup_insert_speculative +s4_insert_commit +# REPACK should now decode the speculative insert and decode the speculative +# insert (with the confirmation record filtered out). +s5_wakeup_repack diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 680c29f35d5..6d5482e5746 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1232,6 +1232,8 @@ ExecInsert(ModifyTableContext *context, slot, arbiterIndexes, &specConflict); + INJECTION_POINT("insert-speculative-before-confirm", NULL); + /* adjust the tuple's state accordingly */ table_tuple_complete_speculative(resultRelationDesc, slot, specToken, !specConflict); diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index fbdd4600a2b..883e5b6e18f 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -141,6 +141,7 @@ #include "storage/procarray.h" #include "storage/standby.h" #include "utils/builtins.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #include "utils/snapmgr.h" #include "utils/snapshot.h" @@ -1390,6 +1391,8 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn builder->state = SNAPBUILD_FULL_SNAPSHOT; builder->next_phase_at = running->nextXid; + INJECTION_POINT("snapbuild-full", NULL); + ereport(LOG, errmsg("logical decoding found initial consistent point at %X/%08X", LSN_FORMAT_ARGS(lsn)), diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 440c875b8ac..8f17ee412c9 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -216,15 +216,22 @@ main(int argc, char **argv) * exactly expect concurrent use of test tables. However, autovacuum will * occasionally take AccessExclusiveLock to truncate a table, and we must * ignore that transient wait. + * + * If the session's backend is blocked, and if its background worker is + * waiting on an injection point, we assume that the injection point is + * the reason for the backend to be blocked. That's what we check in the + * second query of the UNION. XXX Should we use a separate query for that? */ initPQExpBuffer(&wait_query); appendPQExpBufferStr(&wait_query, + "WITH blocking(res) AS (" "SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{"); /* The spec syntax requires at least one session; assume that here. */ appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str); for (i = 2; i < nconns; i++) appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str); - appendPQExpBufferStr(&wait_query, "}')"); + appendPQExpBufferStr(&wait_query, "}') UNION " + "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking"); res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL); if (PQresultStatus(res) != PGRES_COMMAND_OK) -- 2.47.3 --=-=-=-- ^ permalink raw reply [nested|flat] 31+ messages in thread
* [PATCH] Reproduce filtering issue. @ 2026-03-23 11:50 Antonin Houska <[email protected]> 0 siblings, 0 replies; 31+ messages in thread From: Antonin Houska @ 2026-03-23 11:50 UTC (permalink / raw) --- contrib/test_decoding/expected/filtering.out | 74 ++++++++++++++ contrib/test_decoding/specs/filtering.spec | 101 +++++++++++++++++++ src/backend/executor/nodeModifyTable.c | 2 + src/backend/replication/logical/snapbuild.c | 3 + src/test/isolation/isolationtester.c | 9 +- 5 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 contrib/test_decoding/expected/filtering.out create mode 100644 contrib/test_decoding/specs/filtering.spec diff --git a/contrib/test_decoding/expected/filtering.out b/contrib/test_decoding/expected/filtering.out new file mode 100644 index 00000000000..6ba9690509f --- /dev/null +++ b/contrib/test_decoding/expected/filtering.out @@ -0,0 +1,74 @@ +Parsed test spec with 5 sessions + +starting permutation: s1_assign_xid s3_repack s2_assign_xid s1_rollback s4_insert s5_wakeup_snapbuild s2_rollback s5_wakeup_insert_speculative s4_insert_commit s5_wakeup_repack +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step s1_assign_xid: + BEGIN; + CREATE TABLE c(i int); + +step s3_repack: + REPACK (CONCURRENTLY) a; + <waiting ...> +step s2_assign_xid: + BEGIN; + CREATE TABLE d(i int); + +step s1_rollback: + ROLLBACK; + +step s4_insert: + BEGIN; + INSERT INTO t(i) + SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i; + <waiting ...> +step s5_wakeup_snapbuild: + SELECT injection_points_wakeup('snapbuild-full'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s2_rollback: + ROLLBACK; + +step s5_wakeup_insert_speculative: + SELECT injection_points_wakeup('insert-speculative-before-confirm'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s4_insert: <... completed> +step s4_insert_commit: + COMMIT; + +step s5_wakeup_repack: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s3_repack: <... completed> +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/contrib/test_decoding/specs/filtering.spec b/contrib/test_decoding/specs/filtering.spec new file mode 100644 index 00000000000..a02c9f8facb --- /dev/null +++ b/contrib/test_decoding/specs/filtering.spec @@ -0,0 +1,101 @@ +setup +{ + CREATE TABLE a(i int primary key, j int) WITH (autovacuum_enabled = off); + INSERT INTO a(i, j) VALUES (1, 1), (2, 2); + CREATE TABLE t(i int primary key); + INSERT INTO t(i) VALUES (1); + CREATE EXTENSION injection_points; +} + +session s1 +step s1_assign_xid +{ + BEGIN; + CREATE TABLE c(i int); +} +step s1_rollback +{ + ROLLBACK; +} + +session s2 +step s2_assign_xid +{ + BEGIN; + CREATE TABLE d(i int); +} +step s2_rollback +{ + ROLLBACK; +} + +session s3 +setup +{ + SELECT injection_points_attach('snapbuild-full', 'wait'); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +step s3_repack +{ + REPACK (CONCURRENTLY) a; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); + SELECT injection_points_detach('snapbuild-full'); +} + +session s4 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('insert-speculative-before-confirm', 'wait'); +} +step s4_insert +{ + BEGIN; + INSERT INTO t(i) + SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i; +} +step s4_insert_commit +{ + COMMIT; +} +teardown +{ + SELECT injection_points_detach('insert-speculative-before-confirm'); +} + +session s5 +step s5_wakeup_snapbuild +{ + SELECT injection_points_wakeup('snapbuild-full'); +} +step s5_wakeup_insert_speculative +{ + SELECT injection_points_wakeup('insert-speculative-before-confirm'); +} +step s5_wakeup_repack +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +permutation +# Bring the snapshot builder to the FULL_SNAPSHOT state. +s1_assign_xid +s3_repack +s2_assign_xid +s1_rollback +# Perform the speculative insert, but no confirmation so far. The snapshot +# builder should decode it. +s4_insert +# Let the snapshout builder achieve CONSISTENT state and finish the setup. +s5_wakeup_snapbuild +s2_rollback +# While REPACK is waiting on repack-concurrently-before-lock, let the insert +# get confirmed. Relation filtering is now enabled. +s5_wakeup_insert_speculative +s4_insert_commit +# REPACK should now decode the speculative insert and decode the speculative +# insert (with the confirmation record filtered out). +s5_wakeup_repack diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 680c29f35d5..6d5482e5746 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1232,6 +1232,8 @@ ExecInsert(ModifyTableContext *context, slot, arbiterIndexes, &specConflict); + INJECTION_POINT("insert-speculative-before-confirm", NULL); + /* adjust the tuple's state accordingly */ table_tuple_complete_speculative(resultRelationDesc, slot, specToken, !specConflict); diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index fbdd4600a2b..883e5b6e18f 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -141,6 +141,7 @@ #include "storage/procarray.h" #include "storage/standby.h" #include "utils/builtins.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #include "utils/snapmgr.h" #include "utils/snapshot.h" @@ -1390,6 +1391,8 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn builder->state = SNAPBUILD_FULL_SNAPSHOT; builder->next_phase_at = running->nextXid; + INJECTION_POINT("snapbuild-full", NULL); + ereport(LOG, errmsg("logical decoding found initial consistent point at %X/%08X", LSN_FORMAT_ARGS(lsn)), diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 440c875b8ac..8f17ee412c9 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -216,15 +216,22 @@ main(int argc, char **argv) * exactly expect concurrent use of test tables. However, autovacuum will * occasionally take AccessExclusiveLock to truncate a table, and we must * ignore that transient wait. + * + * If the session's backend is blocked, and if its background worker is + * waiting on an injection point, we assume that the injection point is + * the reason for the backend to be blocked. That's what we check in the + * second query of the UNION. XXX Should we use a separate query for that? */ initPQExpBuffer(&wait_query); appendPQExpBufferStr(&wait_query, + "WITH blocking(res) AS (" "SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{"); /* The spec syntax requires at least one session; assume that here. */ appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str); for (i = 2; i < nconns; i++) appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str); - appendPQExpBufferStr(&wait_query, "}')"); + appendPQExpBufferStr(&wait_query, "}') UNION " + "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking"); res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL); if (PQresultStatus(res) != PGRES_COMMAND_OK) -- 2.47.3 --=-=-=-- ^ permalink raw reply [nested|flat] 31+ messages in thread
end of thread, other threads:[~2026-03-23 11:50 UTC | newest] Thread overview: 31+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-09-05 12:29 [PATCH v7 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v6 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v5 3/3] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v12 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v8 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v11 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v7 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2019-09-05 12:29 [PATCH v9 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]> 2023-06-30 19:46 [PATCH v2 1/1] remove db_user_namespace Nathan Bossart <[email protected]> 2024-04-11 02:16 Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-04-23 23:43 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-05-12 14:32 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Peter Eisentraut <[email protected]> 2024-05-15 04:21 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-05-15 05:20 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Peter Eisentraut <[email protected]> 2024-05-16 02:33 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-05-16 15:17 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Nazir Bilal Yavuz <[email protected]> 2024-05-16 22:54 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-08-21 14:00 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Peter Eisentraut <[email protected]> 2024-10-01 10:23 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Peter Eisentraut <[email protected]> 2024-10-02 02:36 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-05-18 22:46 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Ole Peder Brandtzæg <[email protected]> 2024-05-18 23:05 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Thomas Munro <[email protected]> 2024-05-18 23:16 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Ole Peder Brandtzæg <[email protected]> 2024-05-18 23:16 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Tom Lane <[email protected]> 2024-08-21 13:19 ` Re: Requiring LLVM 14+ in PostgreSQL 18 Peter Eisentraut <[email protected]> 2025-10-10 16:41 Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]> 2025-10-11 05:24 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]> 2025-10-11 10:10 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]> 2025-10-11 11:59 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]> 2026-03-23 11:50 [PATCH] Reproduce filtering issue. Antonin Houska <[email protected]> 2026-03-23 11:50 [PATCH] Reproduce filtering issue. Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox