[PATCH] D43117: [zorg] Cleanup unnecessary options for ARM and AArch64 bots

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: buildbot/osuosl/master/config/builders.py:287
                       runTestSuite=True,
                       testsuite_flags=['--cppflags', '-mcpu=cortex-a57',
                                        '--threads=8', '--build-threads=8'],
----------------
maxim-kuvyrkov wrote:
> rengolin wrote:
> > maxim-kuvyrkov wrote:
> > > rengolin wrote:
> > > > You're not changing it for LNT, better to do that for both.
> > > > 
> > > > I'd also try mcup=native instead...
> > > > You're not changing it for LNT, better to do that for both.
> > > 
> > > What do you mean?  There is no aarch64 LNT bot [yet].
> > > 
> > > > I'd also try mcup=native instead...
> > > 
> > > Yeah, I plan to switch bots (or add extra variants of bots) for -mcpu=native.  That's gonna be a future change.
> > I meant testsuite (using LNT), if you look at the line above you'll see what I mean. :)
> My logic is as follows, let me know if there are mistakes in it:
> <cut>
> The intention of this bot is to run testsuite on a code path for a popular CPU -- cortex-a57.  To achieve this we do a single-stage build of llvm with system compiler (we don't care what tuning system compiler uses as long as ISA is compatible with current CPU), and then build testsuite with stage1 llvm.  We care about exercising -mcpu=cortex-a57 code path in llvm, so we add these flags to testsuite.
> </cut>
> 
> If the above is correct then this change is NFC, which is what I intended it to be.
> 
> Now, regarding setting -mcpu=, -mthumb/-marm and other architecture options. 
>  It appears that a cleaner solution would be setting LLVM_TARGET_TRIPLE and/or other configuration options at cmake stage.  Make LLVM generate cortex-a57 (or cortex-a15, ...) code by default and stop bothering about flags.  WDYT?
Right, this is the test-suite, your assumptions are correct.


================
Comment at: buildbot/osuosl/master/config/builders.py:1290
          'factory': LibcxxAndAbiBuilder.getLibcxxAndAbiBuilder(
             cmake_extra_opts={'LIBCXXABI_USE_LLVM_UNWINDER': 'ON',
                               'LLVM_PARALLEL_LINK_JOBS': '2'})},
----------------
maxim-kuvyrkov wrote:
> rengolin wrote:
> > maxim-kuvyrkov wrote:
> > > rengolin wrote:
> > > > why disabling marm/mthumb here?
> > > This is a single-stage build, so CMAKE_C_FLAGS and CMAKE_CXX_FLAGS are passed only to host compiler.  AFAICT, they do not affect results of libcxx testsuites.
> > The host compiler will compile libcxx with arm/thumb and therefore generate different library code, no? Same for test code.
> Yes and no.  Library and test code will be different bit-wise, but (unless there are "#ifdef __arm__ / #ifdef __thumb__" !) should be semantically equivalent.
> If there are indeed different code paths for ARM / Thumb modes or other architectural features, then, "yes", we should differentiate here.
I don't know, but I wouldn't assume so. I think we can leave that change for a different patch, after asking the libc++ folks.


Repository:
  rL LLVM

https://reviews.llvm.org/D43117