[pseudo] Fix pseudo-gen usage when cross-compiling

[smeenai]

Summary

Use the LLVM build system's cross-compilation support for the tool, so
that the build works for both host and cross-compilation scenarios.

[sammccall]

Thank you! I have been banging my head against this, I'm not entirely sure why this works and my version doesn't.

[smeenai]

In D126397#3537843, @sammccall wrote:

Thank you! I have been banging my head against this, I'm not entirely sure why this works and my version doesn't.

Ah, sorry for the duplicated work :( Thanks for the review!

[hokein]

Thanks for the fix!

[mstorsjo]

Regarding building native tools while cross compiling - for other tools so far, we’ve also had the option to pass an option like -DLLVM_TABLEGEN=path/to/llvm-tblgen. If all the needed native tools are provided that way, the build doesn’t need to set up the nested native cmake build while cross compiling. Should we prove a way to do that for this build time generation tool too?

[hans]

This didn't fix the mac/arm64 llvm builds for Chromium. I think it's because we don't set CMAKE_CROSSCOMPILING, so I'll try doing that as a work-around. (crbug.com/1330304).

[thakis]

Even after this, you still have to explicitly set -DCMAKE_SYSTEM_NAME=Darwin when building clang for mac/arm on mac/intel. Given that the host and target systems are both mac, that's a bit weird. llvm-tblgen doesn't need this, it just works as long as you set LLVM_USE_HOST_TOOLS=ON. Should pseudo-gen honor LLVM_USE_HOST_TOOLS too? It looks like it's basically the same situation.

(It's only the 2nd compiled binary in all of llvm that needs to run as part of the build (if you count the various tblgen binaries as a single binary). Maybe it'd make sense to make this a tblgen-based tool? Then this would've Just Worked.).

[sammccall]

In D126397#3547060, @thakis wrote:

Should pseudo-gen honor LLVM_USE_HOST_TOOLS too? It looks like it's basically the same situation.

Yes, I agree, it should. I'll send a patch.
One caveat here: that there are (too) many possible configurations here, and little documentation or buildbot coverage, so we're kind of fixing this blind.

Maybe it'd make sense to make this a tblgen-based tool? Then this would've Just Worked.).

There are a few reasons it didn't seem like a good fit for tblgen:

• it necessarily has a dependency on a separate library clangPseudoGrammar (for now it's conceivably possible to copy into the tablegen backend, but soon this will include the LR automaton generator etc). I didn't see any precedent for this.
• tablegen isn't the right format for the input data, and so isn't adding any value except build system bits
• this isn't part of clang, and there isn't a clang-tools-extra-tblgen, so we'd need a new clang-pseudo-tblgen in any case. This would solve *some* problems, but not all.

Regarding building native tools while cross compiling - for other tools so far, we’ve also had the option to pass an option like -DLLVM_TABLEGEN=path/to/llvm-tblgen. If all the needed native tools are provided that way, the build doesn’t need to set up the nested native cmake build while cross compiling. Should we prove a way to do that for this build time generation tool too?

Yeah, it seems reasonable to spend a little bit more cmake complexity for this.
I hope it can be done in a few lines (digging into it) esp given how hard it is to maintain non-bot-covered cmake configs.

[smeenai]

In D126397#3547060, @thakis wrote:

Even after this, you still have to explicitly set -DCMAKE_SYSTEM_NAME=Darwin when building clang for mac/arm on mac/intel. Given that the host and target systems are both mac, that's a bit weird. llvm-tblgen doesn't need this, it just works as long as you set LLVM_USE_HOST_TOOLS=ON. Should pseudo-gen honor LLVM_USE_HOST_TOOLS too? It looks like it's basically the same situation.

(It's only the 2nd compiled binary in all of llvm that needs to run as part of the build (if you count the various tblgen binaries as a single binary). Maybe it'd make sense to make this a tblgen-based tool? Then this would've Just Worked.).

I was checking for CMAKE_CROSSCOMPILING instead of LLVM_USE_HOST_TOOLS because I didn't want this to fire for LLVM_OPTIMIZED_TABLEGEN, but I guess LLVM_OPTIMIZED_TABLEGEN could be interpreted as a generic "optimized host tools" option in that case, given that the NATIVE build is always Release.

[sammccall]

In D126397#3547438, @sammccall wrote:

In D126397#3547060, @thakis wrote:

Should pseudo-gen honor LLVM_USE_HOST_TOOLS too? It looks like it's basically the same situation.

Yes, I agree, it should. I'll send a patch.

Committed in 9d991da60df492a191b34aa3e75484ddd27e8930

Regarding building native tools while cross compiling - for other tools so far, we’ve also had the option to pass an option like -DLLVM_TABLEGEN=path/to/llvm-tblgen. If all the needed native tools are provided that way, the build doesn’t need to set up the nested native cmake build while cross compiling. Should we prove a way to do that for this build time generation tool too?

Yeah, it seems reasonable to spend a little bit more cmake complexity for this.
I hope it can be done in a few lines (digging into it) esp given how hard it is to maintain non-bot-covered cmake configs.

Sent D126717 (I verified this locally, but want to make sure I'm not misunderstanding what we want here).

clang-tools-extra/pseudo/include/CMakeLists.txt

 # The cxx.bnf grammar file
 set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx.bnf)
 
+# Using CMAKE_CROSSCOMPILING and not LLVM_USE_HOST_TOOLS because the latter is
+# also set for LLVM_OPTIMIZED_TABLEGEN, which we don't care about here.
+if(CMAKE_CROSSCOMPILING)
+  build_native_tool(pseudo-gen pseudo_gen)
+  set(pseudo_gen_target "${pseudo_gen}")
+else()
+  set(pseudo_gen $<TARGET_FILE:pseudo-gen>)
+  set(pseudo_gen_target pseudo-gen)
+endif()
+
 # Generate inc files.
 set(cxx_symbols_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXSymbols.inc)
 add_custom_command(OUTPUT ${cxx_symbols_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
      --grammar ${cxx_bnf}
      --emit-symbol-list
      -o ${cxx_symbols_inc}
    COMMENT "Generating nonterminal symbol file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
    VERBATIM)
 
 set(cxx_bnf_inc ${CMAKE_CURRENT_BINARY_DIR}/CXXBNF.inc)
 add_custom_command(OUTPUT ${cxx_bnf_inc}
-   COMMAND "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/pseudo-gen"
+   COMMAND "${pseudo_gen}"
      --grammar ${cxx_bnf}
      --emit-grammar-content
      -o ${cxx_bnf_inc}
    COMMENT "Generating bnf string file for cxx grammar..."
-   DEPENDS pseudo-gen
+   DEPENDS ${pseudo_gen_target}
    VERBATIM)
 
 # add_custom_command does not create a new target, we need to deine a target
 # explicitly, so that other targets can depend on it.
 add_custom_target(cxx_gen
     DEPENDS ${cxx_symbols_inc} ${cxx_bnf_inc}
     VERBATIM)