[PATCH] D29050: [zorg] Add AOSP builder

[Pengxuan Zheng via Phabricator via llvm-commits]

pzheng added inline comments.


================
Comment at: zorg/buildbot/builders/AOSPBuilder.py:101
+    if target_clang:
+        command += " TARGET_CLANG=%s" % target_clang
+    if target_flags:
----------------
srhines wrote:
> pzheng wrote:
> > srhines wrote:
> > > pzheng wrote:
> > > > pzheng wrote:
> > > > > srhines wrote:
> > > > > > pzheng wrote:
> > > > > > > srhines wrote:
> > > > > > > > You can switch from TARGET_CLANG to LLVM_PREBUILTS_BASE along with LLVM_PREBUILTS_VERSION. You might also need to set/adjust LLVM_RELEASE_VERSION (currently 4.0, but obviously upstream will move past that).
> > > > > > > Thanks for your suggestion. Have a couple of questions.
> > > > > > > 1) TARGET_CLANG only switches the Clang used for target build. Host build still uses the AOSP's default Clang. Can I have the same effect by using LLVM_PREBUILTS_BASE & LLVM_RELEASE_VERSION?
> > > > > > > 2) Can you please give an example of how to use LLVM_PREBUILTS_BASE & LLVM_RELEASE_VERSION to switch the Clang for build AOSP? Let's assume the Clang we want to switch to is located at /local/aosp/llvm.inst/bin/clang(++).
> > > > > > 1. I don't think it is wise to mix/match the Clang's used for the host and the target. Why not build the host components using the new Clang as well? We don't offer the ability to switch these independently in Android either, so this is just a weird configuration.
> > > > > > 
> > > > > > 2. The Android build system expects the BUILD_OS to be present as well, so you ideally should transform the installation path of your tools to include a "linux-x86" or "darwin-x86" component. In general, the paths are used like this:
> > > > > > 
> > > > > > $(LLVM_PREBUILTS_BASE)/$(BUILD_OS)-x86/$(LLVM_PREBUILTS_VERSION)/bin/clang(++)
> > > > > > 
> > > > > > So, let's say we update your path to look more like:
> > > > > > /local/aosp/linux-x86/llvm.inst/bin/clang(++)
> > > > > > 
> > > > > > LLVM_PREBUILTS_BASE=/local/aosp/
> > > > > > LLVM_PREBUILTS_VERSION=llvm.inst/
> > > > > > LLVM_RELEASE_VERSION=4.0 # This has to change when LLVM upstream changes their numbering too.
> > > > > > 
> > > > > > You can pass those env vars to the build command directly for AOSP, and it should use your new compiler for all host/device compilation.
> > > > > I checked out android-7.1.1_r11 and tried running
> > > > > "make LLVM_PREBUILTS_BASE=/local/aosp/ LLVM_PREBUILTS_VERSION=llvm.inst/ LLVM_RELEASE_VERSION=4.0",
> > > > > but got the following error
> > > > > "ninja: error: 'prebuilts/clang/host/linux-x86/llvm.inst/lib64/clang/3.8/lib/linux/libclang_rt.asan-aarch64-android.so', needed by 'out/target/product/angler/obj/SHARED_LIBRARIES/libclang_rt.asan-aarch64-android_intermediates/LINKED/libclang_rt.asan-aarch64-android.so', missing and no known rule to make it".
> > > > > 
> > > > > Is this because I used the variables incorrectly, or something else?
> > > > > 
> > > > > 
> > > > > 
> > > > It seems that the variable LLVM_PREBUILTS_BASE I set did not take effect and "prebuilts/clang/host" is still used by AOSP.
> > > Can you try again with the master branch? I think that some variables were not able to be overridden in the past, and it is possible that the 7.1.1_r11 branch is too old (as compared to TOT aosp/master).
> > Just synced the master branch and tried the command again. Got a similar error
> > "ninja: error: 'prebuilts/clang/host/linux-x86/llvm.inst/test/arm/bin/asan_test', needed by 'out/target/product/angler/obj_arm/EXECUTABLES/asan-test_intermediates/asan-test', missing and no known rule to make it"
> Ugh, this will require some substantial number of changes to Android's build rules. Perhaps your way of hacking the build/ project is better for the short term, but honestly there are still changes needed to upstream LLVM so that it can properly build a multi-target compiler-rt. That is where these files are being pulled in from, since our toolchain build creates these multiple libraries, and then bundles those in prebuilts/clang/host/linux-x86/* (you can look at the Android.mk file there to see what I mean about what it is looking up).
> 
> One other alternative is to create a symlink in prebuilts/clang/host/linux-x86/ so that you can point to that with LLVM_PREBUILTS_VERSION. This, of course, would still require you to package the proper compiler-rt pieces in your new toolchain. I'm not sure if that is any easier to do.
> 
> A lot of this infrastructure for supporting better use of the upstream Clang toolchain is on my team's roadmap for this year, but unfortunately, it is mostly planned for much later in the year, rather than something we are working on in the short term. I can try to push some parts forward, but you most likely will still need some workarounds until that is complete.
I see. Looks like we still need my hack of the build system for now. Really looking forward to seeing a better infrastructure for supporting upstream Clang toolchain.


https://reviews.llvm.org/D29050