r225958 - Use the integrated assembler by default on 32-bit PowerPC and SPARC.

Brad Smith brad at comstyle.com
Wed Jan 14 19:04:41 PST 2015


On 01/14/15 21:33, Chandler Carruth wrote:
>
> On Wed, Jan 14, 2015 at 6:26 PM, Brad Smith <brad at comstyle.com
> <mailto:brad at comstyle.com>> wrote:
>
>     And you're asking after the fact. I'm not a mind reader to know about
>     this wishy washy process. The previous diffs were ok'd by two other
>     developers without any mentioning of this. In the past when I floated a
>     similar diff and was told that I should be enabling it everywhere. If
>     they knew this is what is expected then they should have said something
>     not to ok the diff making me think its ok to move forward.
>
>
> FWIW, I couldn't find any of these code reviews through simple
> searching. They're probably there and I've just missed them, but I think
> others may have missed them as well and also been confused. It would
> have been nice to let people know this was landing now. Switching
> between the integrated assembler and the system assembler is very often
> a disruptive process. Doing it a few hours before 3.6 branches seems
> like a *really* bad idea under any circumstances. This is the kind of
> thing that should happen right after a release branches rather than
> right before. We should at least make sure that 3.6 doesn't include this
> change.

I switched it because all of the BSD's have already been using this
on the 32-bit PowerPC side of the house for quite awhile now and that's
where more or less all the testing and development has been going on
for 32-bit PowerPC as far as I can see. The SPARC backend is still
quite experimental and has rough edges. I could understand the concern
if either backend was more or less 100% perfect or close to it but
they're not. There is still a lot of work going on to make the relevant
architectures usable and by usable I mean building a whole OS not
just a few userland programs here and there.

>
>
>
>
>
>                  It is especially unacceptable to do so when there are
>         active
>                  problems on
>                  build bots and the tests aren't passing. This wasn't
>         the first
>                  time this
>                  patch caused a problem either, and you are forcing
>         several other
>                  developers to chase down build bot failures.
>
>
>              What build bot failures? I haven't seen any and with the
>         first patch I
>              did.
>
>
>         Duncan cited bot failures when he reverted the patch the first
>         time and
>         Rafael pointed to an openbsd build bot failure. While I don't
>         have the
>         links handy, I don't think they were making them up.
>
>
>     Yes, that was the first revision. The failing tests were fixed mostly by
>     Ulrich when I pointed out that the 64-bit PowerPC integrater assembler
>     was missed being enabled in the LLVM backend by Eric Christopher with
>     the Clang front end bits being commited and the rest by me. The second
>     revision was ok'd with the intent that the tests are passing and they
>     are.
>
>     No tests are failing at the moment even on the PowerPC build bots..
>
>     http://lab.llvm.org:8011/__builders/ppc64le-llvm/builds/__59
>     <http://lab.llvm.org:8011/builders/ppc64le-llvm/builds/59>
>     http://lab.llvm.org:8011/__builders/ppc64le-clang/builds/__40
>     <http://lab.llvm.org:8011/builders/ppc64le-clang/builds/40>
>
>
> Rafael indicated in this thread that there was an openbsd bot still
> failing. I don't know which one, but it seemed really weird to have a
> report that a build bot was failing and *no* reply. Not a "wait, where?"
> if you couldn't find it, or a "I'm on it" if its getting fixed.

Any failures with the OpenBSD build bot at the moment are pre-existing
failures which have no relation to this diff. Some of the tests are
broken using command line parameters not specified by POSIX with sed,
mv, head and some others. The change was run with and without the patch
and the same tests were failing either way. Mentioning this in relation
to this diff when the tests are not failing due to this diff is
irrelevant.

> It would *also* be really nice to get the link to the failing build bot
> if there is one so we don't have to play these guessing games. Rafael?

I'd love to see one too if there really is.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.




More information about the cfe-commits mailing list