On Wed, Sep 18, 2019 at 10:13 AM Kyle Evans <kev...@freebsd.org> wrote:
>
> On Wed, Sep 18, 2019 at 10:04 AM Enji Cooper <yaneurab...@gmail.com> wrote:
> >
> >
> > > On Sep 18, 2019, at 07:58, Kyle Evans <kev...@freebsd.org> wrote:
> > >
> > >> On Wed, Sep 18, 2019 at 9:46 AM Enji Cooper <yaneurab...@gmail.com> 
> > >> wrote:
> > >>
> > >>
> > >>> On Sep 18, 2019, at 07:33, Enji Cooper <yaneurab...@gmail.com> wrote:
> > >>>
> > >>>
> > >>>>> On Sep 18, 2019, at 05:40, Kyle Evans <kev...@freebsd.org> wrote:
> > >>>>>
> > >>>>> On Wed, Sep 18, 2019 at 7:34 AM Enji Cooper <yaneurab...@gmail.com> 
> > >>>>> wrote:
> > >>>>>
> > >>>>>
> > >>>>>> On Sep 17, 2019, at 18:58, Kyle Evans <kev...@freebsd.org> wrote:
> > >>>>>>
> > >>>>>> Author: kevans
> > >>>>>> Date: Wed Sep 18 01:58:56 2019
> > >>>>>> New Revision: 352465
> > >>>>>> URL: https://svnweb.freebsd.org/changeset/base/352465
> > >>>>>>
> > >>>>>> Log:
> > >>>>>> googletest: default-disable on all of MIPS for now
> > >>>>>>
> > >>>>>> Parts of the fusefs tests trigger a bug in current versions of llvm: 
> > >>>>>> IR
> > >>>>>> representation of some routine for the MIPS targets is a function 
> > >>>>>> with a
> > >>>>>> large number of arguments. This then leads the compiler on an hour+ 
> > >>>>>> long
> > >>>>>> goose chase, which is OK if you build the current tree but less-so 
> > >>>>>> if you're
> > >>>>>> trying external toolchain or doing a universe build involving mips 
> > >>>>>> when it
> > >>>>>> eventually gets switched over to LLVM.
> > >>>>>>
> > >>>>>> Better, accurate details can be found in LLVM PR43263.
> > >>>>>
> > >>>>> Uhhhhh... why not do this in tests/sys/... instead?
> > >>>>
> > >>>> Because there's still value in being able to easily enable these for
> > >>>> building/running the complete set of tests through standard build
> > >>>> infrastructure, but it's not worth adding a knob specifically for the
> > >>>> fusefs tests. I also prefer the communication of it being an
> > >>>> off-by-default option and easily deduced from src.conf(5) that this
> > >>>> part of the build is default-disabled on mips/mips.
> > >>>
> > >>> Let me rephrase things a bit: is googlemock broken for all of mips, or 
> > >>> is it just the tests? If the latter, the tests should be blacklisted 
> > >>> for mips with a justification. If the former, I agree your method of 
> > >>> dealing with the situation is ok, but more investigation needs to be 
> > >>> done to see whether or not the port (in general) is broken and mark it 
> > >>> broken if need be.
> > >>
> > >> It looks like the latter case, based on the PR, and it’s a build 
> > >> performance issue... Is this impacting CI pipelines?
> > >>
> > >
> > > It is the latter, and I do not want to *blacklist* them because as far
> > > as I can tell, the tests aren't necessarily broken. I want to
> > > workaround them for default by now.
> > >
> > >>> The problem with src.opts.mk’s per-architecture options, is that it can 
> > >>> be very heavy handed enabling/disabling features. I’m not sure that 
> > >>> everything in there warrants disabling at that level.
> > >>
> > >> My investigation suggests that the course of action was overly heavy 
> > >> handed. While I’m not asking for a revert, it would be really nice if 
> > >> whole features weren’t disabled, unless there’s an issue with the 
> > >> feature.
> > >>
> > >
> > > We do not have a lighter method for dealing with this that I can tell,
> > > because as I said above: I do not want to blacklist them or completely
> > > kill them off. I still want the option to build and test them, but as
> > > I aim to switch mips over to llvm I do not want to subject CI and the
> > > rest of the world to an extra 1.5+ hour build time for this during
> > > tinderbox runs.
> > >
> > > Given that it's mips, so already tier-high, and I'm one of few people
> > > that care about it (and I only care about it for the time being), I
> > > intend to leave it as-is since it's still a default in the rest of the
> > > world.
> >
> > Ok, valid straw man argument: in this particular case, should llvm / c++ 
> > support be disabled instead, since it’s the real underlying issue? I’m 
> > guessing (non-ancient) g++ doesn’t have this issue.
> >
> > Again, disabling a framework because of a single issue in the tests doesn’t 
> > make sense. Unless you have proof that the build times for all of 
> > googletest/googlemock with llvm is an issue, this seems like the wrong 
> > remediation to perform.
> >
> > -Enji
> >
> > PS A heads up to asomers and myself would have been nice. I don’t like 
> > post-commit nitpicking, since the issue could have been discussed/reviewed 
> > before commit.
>
> If this was any less than a temporary workaround that will get
> reverted in due time, I would sympathize with your argument
> completely. I had no intention of wasting your time or asomers' time
> with this tier-2 problem that had already been diagnosed as an
> LLVM/mips bug.
>
> The unfortunate reality is that no one (including CI) is running tests
> on FreeBSD/mips, and no one will feel the fallout of this decision.

Sorry, this was supposed to read: "this decision, and anyone else will
simply flip it back on for MIPS in the meantime."
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to