[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

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.).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4baae166ce33: [pseudo] Fix pseudo-gen usage when 
cross-compiling (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

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


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # 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 $)
+  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


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # 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 $)
+  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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126397

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126397: [pseudo] Fix pseudo-gen usage when cross-compiling

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: hokein, sammccall.
Herald added a subscriber: mgorny.
Herald added a project: All.
smeenai requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126397

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


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # 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 $)
+  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


Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,25 +1,35 @@
 # 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 $)
+  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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits