From c75074914c3bd5622eb0932993f518d691b8c6d3 Mon Sep 17 00:00:00 2001 From: ampli Date: Wed, 29 May 2024 22:40:23 +0300 Subject: [PATCH 1/9] MSVC: Remove the vcxproj.filters files We don't need any special file organization, so remove them to ease maintenance. --- Makefile.am | 4 - msvc/LinkGrammar.vcxproj.filters | 377 --------------------------- msvc/LinkGrammarExe.vcxproj.filters | 42 --- msvc/LinkGrammarJava.vcxproj.filters | 27 -- msvc/Python3.vcxproj.filters | 28 -- 5 files changed, 478 deletions(-) delete mode 100644 msvc/LinkGrammar.vcxproj.filters delete mode 100644 msvc/LinkGrammarExe.vcxproj.filters delete mode 100644 msvc/LinkGrammarJava.vcxproj.filters delete mode 100644 msvc/Python3.vcxproj.filters diff --git a/Makefile.am b/Makefile.am index a6e9542d15..26929151aa 100644 --- a/Makefile.am +++ b/Makefile.am @@ -46,18 +46,14 @@ EXTRA_DIST = \ msvc/LGlib-features.props \ msvc/LinkGenerator.vcxproj \ msvc/LinkGrammarExe.vcxproj \ - msvc/LinkGrammarExe.vcxproj.filters \ msvc/LinkGrammarJava.vcxproj \ - msvc/LinkGrammarJava.vcxproj.filters \ msvc/LinkGrammar.sln \ msvc/LinkGrammar.vcxproj \ - msvc/LinkGrammar.vcxproj.filters \ msvc/Local.props \ msvc/confvar.bat \ msvc/MSVC-common.props \ msvc/post-build.bat \ msvc/Python3.vcxproj \ - msvc/Python3.vcxproj.filters \ msvc/README.md \ msvc/make-check.py \ TODO diff --git a/msvc/LinkGrammar.vcxproj.filters b/msvc/LinkGrammar.vcxproj.filters deleted file mode 100644 index d70cda19c8..0000000000 --- a/msvc/LinkGrammar.vcxproj.filters +++ /dev/null @@ -1,377 +0,0 @@ - - - - - {93995380-89BD-4b04-88EB-625FBE52EBFB} - h;hpp;hxx;hm;inl;inc;xsd - - - {67DA6AB6-F800-4c08-8B7A-83BB121AAD01} - rc;ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe;resx - - - {4FC737F1-C7A5-4376-A066-2A32D752A2FF} - cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx - - - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - Header Files - - - - - - - - Source Files - - - \ No newline at end of file diff --git a/msvc/LinkGrammarExe.vcxproj.filters b/msvc/LinkGrammarExe.vcxproj.filters deleted file mode 100644 index 6cbab54d3a..0000000000 --- a/msvc/LinkGrammarExe.vcxproj.filters +++ /dev/null @@ -1,42 +0,0 @@ - - - - - {4FC737F1-C7A5-4376-A066-2A32D752A2FF} - cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx - - - {93995380-89BD-4b04-88EB-625FBE52EBFB} - h;hpp;hxx;hm;inl;inc;xsd - - - {67DA6AB6-F800-4c08-8B7A-83BB121AAD01} - rc;ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe;resx;tiff;tif;png;wav - - - - - Source Files - - - Source Files - - - Source Files - - - Source Files - - - - - Header Files - - - Header Files - - - Header Files - - - \ No newline at end of file diff --git a/msvc/LinkGrammarJava.vcxproj.filters b/msvc/LinkGrammarJava.vcxproj.filters deleted file mode 100644 index 7b9b930520..0000000000 --- a/msvc/LinkGrammarJava.vcxproj.filters +++ /dev/null @@ -1,27 +0,0 @@ - - - - - {93995380-89BD-4b04-88EB-625FBE52EBFB} - h;hpp;hxx;hm;inl;inc;xsd - - - {67DA6AB6-F800-4c08-8B7A-83BB121AAD01} - rc;ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe;resx - - - {4FC737F1-C7A5-4376-A066-2A32D752A2FF} - cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx - - - - - Source Files - - - - - Header Files - - - diff --git a/msvc/Python3.vcxproj.filters b/msvc/Python3.vcxproj.filters deleted file mode 100644 index b19bd20b59..0000000000 --- a/msvc/Python3.vcxproj.filters +++ /dev/null @@ -1,28 +0,0 @@ - - - - - {4FC737F1-C7A5-4376-A066-2A32D752A2FF} - cpp;c;cc;cxx;def;odl;idl;hpj;bat;asm;asmx - - - {93995380-89BD-4b04-88EB-625FBE52EBFB} - h;hh;hpp;hxx;hm;inl;inc;xsd - - - {67DA6AB6-F800-4c08-8B7A-83BB121AAD01} - rc;ico;cur;bmp;dlg;rc2;rct;bin;rgs;gif;jpg;jpeg;jpe;resx;tiff;tif;png;wav;mfcribbon-ms - - - - - Source Files - - - - - - Source Files - - - \ No newline at end of file From fdf053a882f44e238d96aaceb4c1e4c1db4683d3 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 01:08:12 +0300 Subject: [PATCH 2/9] LinkGrammar.vcxproj: Update the list of source file names --- msvc/LinkGrammar.vcxproj | 42 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/msvc/LinkGrammar.vcxproj b/msvc/LinkGrammar.vcxproj index 9e0cf31b46..ce45ec113e 100644 --- a/msvc/LinkGrammar.vcxproj +++ b/msvc/LinkGrammar.vcxproj @@ -235,14 +235,15 @@ - - + + + @@ -254,6 +255,10 @@ + + + + @@ -272,18 +277,18 @@ - + - + - + @@ -295,13 +300,13 @@ - + - - + - + + @@ -312,12 +317,20 @@ CompileAsCpp CompileAsCpp - + + + + + + + + + @@ -327,6 +340,7 @@ + @@ -341,13 +355,15 @@ - + - + + + @@ -394,4 +410,4 @@ - \ No newline at end of file + From f0c591ec11edf31b04f003ced012c31858fd6235 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 02:14:55 +0300 Subject: [PATCH 3/9] table_count(): Bugfix !USE_TABLE_TRACON Fix bug in tracon table debug when USE_TABLE_TRACON is disabled, introduced in commit 2112df2 --- link-grammar/parse/count.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index 8f90fe44fc..2617b62f47 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -957,7 +957,7 @@ static Count_bin table_count(count_context_t * ctxt, int lw, int rw, Connector *le, Connector *re, unsigned int null_count) { - if (!USE_TABLE_TRACON) return true; + if (!USE_TABLE_TRACON) return count_unknown; /* This check is not necessary for correctness, but it saves CPU time. * If a cross link would result, we know that the count would be 0. From 870393811b0902e1301aa8c3e48f904a4402c585 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 02:39:02 +0300 Subject: [PATCH 4/9] table_lookup(): Remove redundant check in assert() --- link-grammar/parse/count.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index 2617b62f47..aa393bc5a3 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -576,9 +576,9 @@ static Count_bin table_store(count_context_t *ctxt, Count_bin *e = table_lookup(ctxt, lw, rw, le, re, null_count, NULL); if (e != NULL) { - assert((e == NULL) || (hist_total(&c) == hist_total(e)), "Inconsistent count for w(%d,%d) tracon_id(%d,%d)", lw, rw, l_id, r_id); + assert((hist_total(&c) == hist_total(e)), return c; } From ed0359515d6f41e0dfcf7f0a4f904a74c664a7b4 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 02:41:00 +0300 Subject: [PATCH 5/9] table_lookup(): Print conflicting values in assert() --- link-grammar/parse/count.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index aa393bc5a3..6e22e2f93c 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -576,9 +576,9 @@ static Count_bin table_store(count_context_t *ctxt, Count_bin *e = table_lookup(ctxt, lw, rw, le, re, null_count, NULL); if (e != NULL) { - "Inconsistent count for w(%d,%d) tracon_id(%d,%d)", - lw, rw, l_id, r_id); assert((hist_total(&c) == hist_total(e)), + "Inconsistent count for w(%d,%d) tracon_id(%d,%d): %zd != %zd", + lw, rw, l_id, r_id, (ssize_t)hist_total(&c), (ssize_t)hist_total(e)); return c; } From dc9426b815cd0dd92673a3e09d188d995129ec97 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 02:42:41 +0300 Subject: [PATCH 6/9] table_lookup(): On !USE_TABLE_TRACON, return Count_bin type The value of the w_count_bin type argument (c) is already clamed, so the table value should be the same. But using the count_bin type value (*e) avoids a warning of possible loss of data (64 -32 bit conversion) that is enabled by default on Windows. --- link-grammar/parse/count.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index 6e22e2f93c..1c636f6720 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -579,7 +579,7 @@ static Count_bin table_store(count_context_t *ctxt, assert((hist_total(&c) == hist_total(e)), "Inconsistent count for w(%d,%d) tracon_id(%d,%d): %zd != %zd", lw, rw, l_id, r_id, (ssize_t)hist_total(&c), (ssize_t)hist_total(e)); - return c; + return *e; } // The count is still stored, for the above consistency check From 267dadabefc41dd9e6c7a2758e7edef2568f6b18 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 02:59:35 +0300 Subject: [PATCH 7/9] estimate_log2_table_size(): Avoid warning on conversion to/from double (I would like to clean up implicit conversions.) --- link-grammar/parse/count.c | 2 +- link-grammar/parse/extract-links.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index 1c636f6720..27c49246fe 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -40,7 +40,7 @@ typedef uint8_t WordIdx_m; /* Storage representation of word index */ const bool ENABLE_WORD_SKIP_VECTOR = true; const bool ENABLE_MATCH_LIST_CACHE = true; const bool ENABLE_TABLE_LRCNT = true; // Also controls the above two caches. -const bool USE_TABLE_TRACON = true; // The table is always maintained. +const bool USE_TABLE_TRACON = false; // The table is always maintained. const bool USE_PSEUDOCOUNT = true; // Controls only the non-cyclic solutions. typedef struct Table_tracon_s Table_tracon; diff --git a/link-grammar/parse/extract-links.c b/link-grammar/parse/extract-links.c index 98c186dbd6..0e098f1113 100644 --- a/link-grammar/parse/extract-links.c +++ b/link-grammar/parse/extract-links.c @@ -157,7 +157,8 @@ static void record_choice( static int estimate_log2_table_size(Sentence sent) { /* Size estimate based on measurements (see #1402) */ - double lscale = log2(sent->num_disjuncts + 1.0) - 0.5 * log2(sent->length); + double lscale = log2((double)sent->num_disjuncts + 1.0) - + 0.5 * log2((double)sent->length); double lo_est = lscale + 4.0; double hi_est = 1.5 * lscale; double dj_est = fmax(lo_est, hi_est); @@ -166,10 +167,10 @@ static int estimate_log2_table_size(Sentence sent) * pex->Pset_bucket_pool is almost exactly equal to the num elts * issued for sent->Table_tracon_pool. This provides a better * estimate when parsing with MST, when the above is too low. */ - double ntracon = pool_num_elements_issued(sent->Table_tracon_pool); + double ntracon = (double)pool_num_elements_issued(sent->Table_tracon_pool); double ltra = log2(ntracon) + 1.0; // + 1.0 because floor() - int log2_table_size = floor(fmax(dj_est, ltra)); + int log2_table_size = (int)floor(fmax(dj_est, ltra)); // Enforce min and max sizes. if (log2_table_size < 4) log2_table_size = 4; From b4f22e4f0f8f77c5eabbaf146be994d3b7526c97 Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 03:31:24 +0300 Subject: [PATCH 8/9] Change GNUC_NORETURN to NORETURN and define for MSVC too This prevents MSVC warnings after assert(0, ...) at end of functions that returns a value only at their middle. --- link-grammar/error.h | 5 +++-- link-grammar/parse/count.c | 2 +- link-grammar/utilities.h | 11 +++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/link-grammar/error.h b/link-grammar/error.h index 9114b182dc..b81e4f57d7 100644 --- a/link-grammar/error.h +++ b/link-grammar/error.h @@ -15,7 +15,7 @@ #include "link-includes.h" #include "externs.h" // verbosity -#include "utilities.h" // GNUC_NORETURN, STRINGIFY +#include "utilities.h" // NORETURN, STRINGIFY /* User verbosity levels are 1-4, to be used for user info/debug. * For now hard-coded numbers are still used instead of D_USER_BASIC/TIMES. */ @@ -97,8 +97,9 @@ void lg_lib_failure(void); extern void (* assert_failure_trap)(void); #define FILELINE __FILE__ ":" STRINGIFY(__LINE__) +NORETURN void assert_failure(const char[], const char[], const char *, const char *, ...) - GNUC_PRINTF(4,5) GNUC_NORETURN; + GNUC_PRINTF(4,5); /* Define a private version of assert() with a printf-like error * message. The C one is not used. */ diff --git a/link-grammar/parse/count.c b/link-grammar/parse/count.c index 27c49246fe..1c636f6720 100644 --- a/link-grammar/parse/count.c +++ b/link-grammar/parse/count.c @@ -40,7 +40,7 @@ typedef uint8_t WordIdx_m; /* Storage representation of word index */ const bool ENABLE_WORD_SKIP_VECTOR = true; const bool ENABLE_MATCH_LIST_CACHE = true; const bool ENABLE_TABLE_LRCNT = true; // Also controls the above two caches. -const bool USE_TABLE_TRACON = false; // The table is always maintained. +const bool USE_TABLE_TRACON = true; // The table is always maintained. const bool USE_PSEUDOCOUNT = true; // Controls only the non-cyclic solutions. typedef struct Table_tracon_s Table_tracon; diff --git a/link-grammar/utilities.h b/link-grammar/utilities.h index dceed6d1b8..116c3d6252 100644 --- a/link-grammar/utilities.h +++ b/link-grammar/utilities.h @@ -253,13 +253,13 @@ static inline char *_strndupa3(char *new_s, const char *s, size_t n) * support C11. So it already supports all the features below. */ /* Optimizations etc. that only gcc understands */ -/* FIXME: Change to ATTR_* and define also for MSVC. */ +/* FIXME: Define also for MSVC. */ #if __GNUC__ #define GCC_DIAGNOSTIC #define UNREACHABLE(x) (__extension__ ({if (x) __builtin_unreachable();})) #define GNUC_MALLOC __attribute__ ((__malloc__)) #define GNUC_UNUSED __attribute__ ((__unused__)) -#define GNUC_NORETURN __attribute__ ((__noreturn__)) +#define NORETURN __attribute__ ((__noreturn__)) #define ATTR_PURE __attribute__ ((__pure__)) #define NO_SAN __attribute__ ((no_sanitize_address, no_sanitize_undefined)) @@ -271,7 +271,6 @@ static inline char *_strndupa3(char *new_s, const char *s, size_t n) #else #define NO_SAN_DICT #endif - #ifndef DONT_EXPECT #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0) @@ -281,7 +280,7 @@ static inline char *_strndupa3(char *new_s, const char *s, size_t n) #define UNREACHABLE(x) #define GNUC_MALLOC #define GNUC_UNUSED -#define GNUC_NORETURN +#define NORETURN #define ATTR_PURE #define NO_SAN_DICT @@ -289,6 +288,10 @@ static inline char *_strndupa3(char *new_s, const char *s, size_t n) #define unlikely(x) x #endif +#ifdef _MSC_VER +#undef NORETURN +#define NORETURN __declspec(noreturn) +#endif /* Apply a pragma to a specific code section only. * XXX According to the GCC docs, we cannot use here something like From 3c23a1c944970c27401f0f24afe49d4303fe322d Mon Sep 17 00:00:00 2001 From: ampli Date: Thu, 30 May 2024 04:37:41 +0300 Subject: [PATCH 9/9] form_match_list(): Don't use _Atomic There is no need to ensure the "id" variable is incremented in an atomic way, because we don't care if two threads will use the same value. The crash that got debugged on commit 9f73b372 was due to another reason: not using "id" variable if it is 0. This has been fixed. Removing it gets rid of some small overhead, and also allows MSVC compilation. --- link-grammar/parse/fast-match.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/link-grammar/parse/fast-match.c b/link-grammar/parse/fast-match.c index d59d7dec12..a6e9d0e7f9 100644 --- a/link-grammar/parse/fast-match.c +++ b/link-grammar/parse/fast-match.c @@ -595,8 +595,8 @@ form_match_list(fast_matcher_t *ctxt, int w, } #ifdef VERIFY_MATCH_LIST - static _Atomic(uint16_t) id = 0; - uint16_t lid = ++id; /* A local copy, for multi-threading support. */ + static uint16_t id = 0; + uint16_t lid = ++id; /* A stable local copy, for multi-threading support. */ #else const uint16_t lid = 0; #endif