[PATCH] D52149: add support functions for hard/soft float multilib distinction

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 1 04:40:08 PDT 2018


rengolin added a comment.

In https://reviews.llvm.org/D52149#1250772, @kristina wrote:

> There's no intent to revert


Sorry, bad wording. Ignore "intent", I meant "effect", see below.

> there's intent to make it easier for Clang reviewers to not have to apply separate patchsets from LLVMSupport and from the Clang diff.

That may be easier for Clang developers, but it creates a serious issue for LLVM developers.

I know we're all supposed to be "the same crowd", but LLVM really is an API, and as such, it must be consistent, cohesive, uncoupled.

> The intent is to allow it to stay until the Clang diff is reviewed and **then** if it's not accepted, revert the now-unused chunk of code.

The intent may be "help clang devs" but the actual *effect* is to add ephemeral duplicated code to LLVM, which is bad.

This has been done somewhat liberally in LLVM, with post-commit review patches, for example in LLD, and has caused a good amount of trouble to other developers that are trying to create more complex (and slower paced) features.

We really shouldn't be adding code that will later be reverted just for the sake of testing other parts of the project.

If enough of us do that on enough parts of the code, it will be impossible to use trunk for anything and we may end up derailing the whole project.

> Anyway, once it's fixed, I'm personally in favor and think this does belong in `Triple`, but it looks like the consensus is going to be against.

If that's the case, then I strongly recommend you make the case with an actual move of the code, showing how much better it will be. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D52149





More information about the llvm-commits mailing list