Page MenuHomePhabricator

Remove the home-grown android toolchain file and all references to it
ClosedPublic

Authored by labath on Apr 24 2017, 10:12 AM.

Details

Summary

The toolchain file has been deprecated in favor of the "official"
toolchain file present in the Android NDK. Also update the web
build instructions to reflect this.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Apr 24 2017, 10:12 AM
tberghammer added inline comments.Apr 24 2017, 11:41 AM
www/build.html
474–475 ↗(On Diff #96417)

Can you mention the minimum version of the NDK supported by this approach?

488–493 ↗(On Diff #96417)

Can you add some instructions about the supported ANDROID_ABI and LLVM_HOST_TRIPLE pairs? I would find it a bit hard to figure them out on my own (especially the correct triple)

488–493 ↗(On Diff #96417)

Shouldn't we specify LLVM_TARGETS_TO_BUILD as well to reduce the size of the executable?

488–493 ↗(On Diff #96417)

Previously we specified LLVM_TARGET_ARCH as well. Is it not needed anymore?

498 ↗(On Diff #96417)

is *not* supported

eugene accepted this revision.Apr 24 2017, 8:21 PM
This revision is now accepted and ready to land.Apr 24 2017, 8:21 PM
labath marked 2 inline comments as done.Apr 25 2017, 12:53 AM
labath added inline comments.
www/build.html
488–493 ↗(On Diff #96417)

The LLVM_HOST_TRIPLE thingy is described in the generic section above as

<dd>The triple of the system that lldb (or lldb-server) will run on. Not setting
this (or setting it incorrectly) can cause a lot of issues with remote debugging
as a lot of the choices lldb makes depend on the triple reported by the remote
platform.</dd>

LLVM_TARGET_ARCH is computed from the host triple, so it's not necessary if you set the latter properly.

As for TARGETS_TO_BUILD, I chose to omit that as I just want to present a minimal working configuration. If we go the optimization route, then there are many other variables you might want to set (LLVM_BUILD_STATIC, LLVM_USE_LINKER=gold to enable dead stripping, ...), but I don't think that's the purpose of this example.

labath updated this revision to Diff 96514.Apr 25 2017, 12:53 AM

Address Tamas's comments.

tberghammer accepted this revision.Apr 25 2017, 5:39 AM

Looks good

This revision was automatically updated to reflect the committed changes.