Page MenuHomePhabricator

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

Authored by smeenai on May 25 2022, 10:24 AM.

Details

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.

Diff Detail

Event Timeline

smeenai created this revision.May 25 2022, 10:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 10:24 AM
Herald added a subscriber: mgorny. · View Herald Transcript
smeenai requested review of this revision.May 25 2022, 10:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 10:24 AM
sammccall accepted this revision.May 25 2022, 10:44 AM

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

This revision is now accepted and ready to land.May 25 2022, 10:44 AM

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!

This revision was automatically updated to reflect the committed changes.

Thanks for the fix!

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 added a subscriber: hans.May 30 2022, 7:10 AM

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 added a subscriber: thakis.May 31 2022, 5:31 AM

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

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.

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.

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