[PATCH] D73044: {tablegen] Emit string literals instead of char arrays

[Luke Drummond via Phabricator via llvm-commits]

ldrumm marked an inline comment as done.
ldrumm added a comment.

In D73044#1831436 <https://reviews.llvm.org/D73044#1831436>, @mstojanovic wrote:

> First char of the title: `{` -> `[`.


Thanks. Phabricator doesn't seem to track the git commit message, but I've fixed this is on my tree



================
Comment at: cmake/modules/TableGen.cmake:68
+    # in length. Ensure tablgen works around this when the generator is VS
+    set(tblgen_long_string_literals_flag "--long-string-literals=0")
   else()
----------------
mstorsjo wrote:
> I think this needs to go inside a `if (MSVC)` clause instead, as the generator often can be Ninja even though you're building with MSVC.
fixed


================
Comment at: utils/TableGen/CMakeLists.txt:64
+# [1] https://docs.microsoft.com/en-us/cpp/cpp/compiler-limits?view=vs-2017
+if (NOT CMAKE_CROSSCOMPILING AND NOT MSVC)
+  set(TABLEGEN_EMIT_LONG_STR_LITERALS ON)
----------------
mstorsjo wrote:
> I'm afraid this isn't sufficient; I normally first do a fully native build, then when crosscompiling, I point it at the already built fully native tblgen.
> 
> Also likewise if I don't have an already built tblgen when crosscompiling, cmake is invoked a second time recursively for building these tools, and I'd guess `CMAKE_CROSSCOMPILING` isn't set there, as it's a normal native build for those tools.
> 
> I think the one safe way of handling it would be to add a runtime option to tblgen for making msvc compatible output, and set that when generating tables, if the target compiler is msvc. Then it should work the same with any combination of build configurations. I don't know the tblgen internals if this is doable or what it looks like wrt the tblgen invocations in cmake though...
The logic to do that already includes workarounds for visual studio.
I've added them in the same place:
```diff
diff --git a/cmake/modules/TableGen.cmake b/cmake/modules/TableGen.cmake
index a1c28870c14..4d888696ec7 100644
--- a/cmake/modules/TableGen.cmake
+++ b/cmake/modules/TableGen.cmake
@@ -63,8 +63,13 @@ function(tablegen project ofn)
     # behavior. Since it doesn't do restat optimizations anyway, just don't
     # pass --write-if-changed there.
     set(tblgen_change_flag)
+    # Visual Studio has problems with string literals greater than 65534 bytes
+    # in length. Ensure tablgen works around this when the generator is VS
+    set(tblgen_long_string_literals_flag "--long-string-literals=0")
   else()
     set(tblgen_change_flag "--write-if-changed")
+    # otherwise use the configuration default
+    set(tblgen_long_string_literals_flag)
   endif()
 
   # We need both _TABLEGEN_TARGET and _TABLEGEN_EXE in the  DEPENDS list
@@ -81,6 +86,7 @@ function(tablegen project ofn)
     ${LLVM_TABLEGEN_FLAGS}
     ${LLVM_TARGET_DEFINITIONS_ABSOLUTE}
     ${tblgen_change_flag}
+    ${tblgen_long_string_literals_flag}
     ${additional_cmdline}
     # The file in LLVM_TARGET_DEFINITIONS may be not in the current
     # directory and local_tds may not contain it, so we must



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73044/new/

https://reviews.llvm.org/D73044