On 01/14/15 21:33, Chandler Carruth wrote:
On Wed, Jan 14, 2015 at 6:26 PM, Brad Smith <[email protected]
<mailto:[email protected]>> 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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
