[PATCH] D14577: Cull non-standard variants of ARM architectures (NFC)

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 11 13:25:03 PST 2015


rengolin added a comment.

In http://reviews.llvm.org/D14577#287449, @tyomitch wrote:

> The motivation is to still accept them in -march and triples (as aliases), while cleaning them out of the rest of the source code.
>
> The user interface stays unaffected.


No, it doesn't. You're removing more than you're putting back.

Even if all current tests continue to pass, it doesn't mean that the change has no functional changes, just that the tests weren't picking all the changes up.

For example, you're changing v7hl into v7-a, which in itself is "correct" but can have unpredictable side effects on corner cases. Those functions are called all over the place for different reasons, some of them only relevant to the internal logic of the function, like depending on the sub-arch, choose different options. These places are used on Linux, bare-metal, Darwin and Windows, with different results for each one.

The original state was that every place had its own parsing, which was really bad. Now, all of them are using the same parsing, but that raises a lot of bloat, redundancies and non-canonical naming, which is still bad, but not as much.

Removing a lot of legacy like this is bound to break a lot of things we aren't testing for, for no other reason than "just a clean up".

Normally, I'd be supportive of clean ups, but this part is a minefield, and we have been bitten before, every time we assume something was not being used any more. I don't think it's a good idea.


http://reviews.llvm.org/D14577





More information about the llvm-commits mailing list