[cfe-dev] Small patches to allow fully independent clang/llvm/compiler-rt/libc++

[C Bergström via cfe-dev]

On Wed, Oct 14, 2015 at 11:43 PM, Jonathan Roelofs
<jonathan at codesourcery.com> wrote:
>
>
> On 10/14/15 8:53 AM, C Bergström wrote:
>>
>> On Wed, Oct 14, 2015 at 9:16 PM, Jonathan Roelofs
>> <jonathan at codesourcery.com> wrote:
>>>
>>>
>>>
>>> On 10/14/15 7:52 AM, C Bergström wrote:
>>>>
>>>>
>>>> On Wed, Oct 14, 2015 at 8:47 PM, Jonathan Roelofs
>>>> <jonathan at codesourcery.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 10/14/15 7:11 AM, Vasileios Kalintiris wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Some people may disagree on the principle, even if it doesn't affect
>>>>>>> current builds. But I'll let them voice their concerns.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> IMHO, the "correct" approach for a GNU-free/independent clang package
>>>>>> requires
>>>>>> a new ToolChain class for LLVM-based toolchains. Maybe, this class
>>>>>> could
>>>>>> inherit from the Linux ToolChain and use most of the existing
>>>>>> infrastructure
>>>>>> for paths, options & tools invocations. Otherwise, we could use these
>>>>>> configuration-time options for most of the GNU-dependencies that we
>>>>>> want
>>>>>> to
>>>>>> replace. However, I think that we would be duplicating
>>>>>> effort/functionality
>>>>>> at two different places (cmake & clang source code). Additionally,
>>>>>> other
>>>>>> than
>>>>>> compiler-rt, libcxx, libcxxabi, etc., we'd need a non-GLIBC/GNU C
>>>>>> library
>>>>>> which might have a different set of CRT files with its own naming
>>>>>> scheme
>>>>>> for the
>>>>>> dynamic linker/loader.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Something along these ^ lines is the "correct" way to do it. You'd have
>>>>> to
>>>>> add your own vendor to Triple.h, and then specialize these defaults
>>>>> based
>>>>> on
>>>>> seeing that particular vendor.  Baking these decisions in at
>>>>> compile-time
>>>>> is
>>>>> an anti-pattern driver-wise.
>>>>>
>>>>> If you do it this way, then you'll be able to write testcases for your
>>>>> changes, and those tests will get run by all the buildbots (not just
>>>>> one
>>>>> configured to have one particular set of defaults).  This has the added
>>>>> benefit of not requiring more builders to test more configurations.
>>>>>
>>>>> Please don't commit these patches as-is.
>>>>
>>>>
>>>>
>>>> Can you give precise feedback about what's wrong with the patches
>>>> (as-is)? I'd like to "fix" them. Is it philosophical or something
>>>> wrong?
>>>
>>>
>>>
>>> To re-phrase, I think the "spirit" behind clang-01.diff, clang-02.diff
>>> and
>>> clang-03.diff are contrary to the design goals of the driver. I think you
>>> ought to re-work them so that they don't touch CMakeLists.txt at all,
>>> that
>>> way all of these defaults are set at compiler runtime via the vendor part
>>> of
>>> the triple (or some flag(s)), not at compiler compiletime.
>>>
>>> I'm not sure I'm the best person to review clang-04.diff, but at the very
>>> least it needs testcases.
>>
>>
>> The whole point of the patches is not to touch the default so "driver
>> and flag people" can have it their way, but non-flag people can have a
>> default. This doesn't hurt the driver and anyone using it would be
>> opt-in.
>
>
> I understand that's what you _want_ to do, but I still don't think that's
> the _right_ thing to do.
>
> To name a specific example of undesirable effects of this series of patches,
> consider someone filing a bug report: in order to diagnose what's going on,
> now we'd need more than just the svn revision as printed in the version
> string... we'd need to know how the particular build was configured, which
> isn't something that the binary is able to tell us.

There's lots of things which can impact the compiler beyond just the
flags. I can add flags (optimizations) to gcc and build a version of
clang which won't produce correct code. How would you diagnosis that?
(Similar scenario?) What happens if some rare bug slips in and or some
optimization is turned on and then that compiler builds a buggy libc++
(same?) I get what you're saying, but I don't think you can perfectly
guard against compile time decisions by adding more flags.

I don't know why gcc does it, but gcc -v will give you the configure
line which was used to build the compiler. Doing something similar
when these options are enabled would give the information someone
would need. I'm happy to add that change if it makes these patches
palatable.

Thanks for the tip on the test case. If the other 3 patches get
approval I'll add a test case for the 4th.