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

Chandler Carruth chandlerc at google.com
Wed Jan 14 18:33:28 PST 2015


On Wed, Jan 14, 2015 at 6:26 PM, Brad Smith <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.


>
>
>>
>>         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-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.

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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150114/a0c9644a/attachment.html>


More information about the cfe-commits mailing list