[PATCH] D102568: [Driver] Delete -mimplicit-it=

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 19 11:41:28 PDT 2021


MaskRay added a comment.

In D102568#2769237 <https://reviews.llvm.org/D102568#2769237>, @mstorsjo wrote:

> In D102568#2769053 <https://reviews.llvm.org/D102568#2769053>, @nickdesaulniers wrote:
>
>>> But this change did break my build in these places:
>>> https://code.videolan.org/videolan/x264/-/blob/b684ebe04a6f80f8207a57940a1fa00e25274f81/configure#L855
>>> https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1012
>>> https://github.com/cisco/openh264/blob/089d6c6d83ab6e5d451ef18808bb6c46faf553a2/build/platform-mingw_nt.mk#L23
>>
>> Thanks for the links; @MaskRay I think you should file some issues in those projects to help them move to the assembler option before we try to remove it again.
>
> Well I already started doing that, I had made one PR, when CI tests there informed me that the new form of the option didn't work out with the version of Clang in use there (10.0 fwiw).

Thanks for noticing the teams.

>>>> Should be an easy fix either way (replace `-mimplicit-it=always` with `-Wa,-wmimplicit-it=always`).
>>>
>>> No, it's not an easy fix as no existing releases of Clang support it.
>>
>> Well, these projects linked above will need to implement either feature detection for the command line flag options before hard coding them, or compiler version checks.
>
> I really think it's silly to demand multiple projects to implement detection for this option. Just let's revert this change, let both option forms coexist in a couple public stable releases, until it's acceptable to raise the minimum required tool version for those build configurations to Clang 13 (it's a quite niche configuration, but upgrading CI systems always takes some time), and then after that drop the old form (e.g. in Clang 15). Maybe we could add a deprecation warning to the one that we're going to remove in the future?

I don't mind reverting this temporarily.
However, reverting this would break musl build.
musl detects both options and will add both if available: `-Wa,-mimplicit-it=never -mimplicit-it=always` will cause a duplicate option failure.
Can't there be other projects which do similar detection and be broken by having both options?

I think "waiting for a few releases" is too much and doesn't improve things. I can accept "waiting for one major release".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568



More information about the cfe-commits mailing list