[llvm] r184793 - Remove all non-linker oriented compile options from the linker

[Bob Wilson]

On Jun 27, 2013, at 10:50 AM, Eric Christopher <echristo at gmail.com> wrote:

> Hi Bob,
> 
>> At some level, this boils down to an argument about naming conventions, and
>> I don't think there is a clear right or wrong answer.  More importantly, it
>> is fairly common for people to set CFLAGS and CXXFLAGS when building.  Your
>> change broke that for Apple, and it could well affect other LLVM users.
>> What is the motivation for introducing this incompatibility with previous
>> LLVM releases?  If there's a good reason for it, I don't mind fixing our
>> builds to accommodate it, but I don't think it's worth the trouble and risk
>> of breaking other people's builds without a clear benefit to changing
>> things.
>> 
> 
> Basically a lot of this is worked around by having the linker ignore
> arguments that it doesn't understand, however, this isn't guaranteed
> to exist for any given linker argument and if you want to pass an
> argument to the compiler and _not_ have it forwarded to the linker
> then you have no options without removing CXXFLAGS from the linker
> command line.

CXXFLAGS is not being passed to the linker.  It's being used with $CXX, and the compiler knows which options should be passed on to the linker.

If you want to pass an option to the compiler, but only for the compile steps and not the link step, then yes, I see the problem.  Is that really what you're trying to do?  Can you give an example where that is necessary?

> If you're willing to modify the various linkers to
> ignore command line options passed down then I'm happy to have
> CXXFLAGS in there. That it works at all is a lucky side-effect that
> none of the options that are passed through currently cause the linker
> to error.

Again, the "Link" command is not invoking the linker.  Are you building with CXX=ld?

> 
> I was pretty sure the change I made worked for the targets I could
> test. Can you be a bit more clear where the -arch option isn't being
> passed in the correct place for the link step?

We're running:

make CFLAGS="-arch armv7" CXXFLAGS="-arch armv7"

and the link step fails because the -arch option is used when compiling but not when linking.

> 
> -eric
> 
> 
>> On Jun 26, 2013 11:13 PM, "Bob Wilson" <bob.wilson at apple.com> wrote:
>>> 
>>> This broke one of Apple’s buildbots.  I have tried to get it going again
>>> by adding CXXFLAGS back to the link command in r185060.  I still need to
>>> confirm that it actually fixes the buildbot.  Assuming that it does, will
>>> that work for you, Eric?  It wasn’t clear from your commit message why you
>>> changed this, so I don’t know what your constraints are.
>>> 
>>> On Jun 24, 2013, at 4:20 PM, Eric Christopher <echristo at gmail.com> wrote:
>>> 
>>> Author: echristo
>>> Date: Mon Jun 24 18:20:04 2013
>>> New Revision: 184793
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=184793&view=rev
>>> Log:
>>> Remove all non-linker oriented compile options from the linker
>>> command line. Change the darwin universal binary options to
>>> be TargetCommonOpts so that they'll be passed to the linker since
>>> -arch at least is still needed.
>>> 
>>> Someone on darwin with a buildit based build should probably verify
>>> that this doesn't break anything there.
>>> 
>>> Modified:
>>>   llvm/trunk/Makefile.rules
>>> 
>>> Modified: llvm/trunk/Makefile.rules
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/Makefile.rules?rev=184793&r1=184792&r2=184793&view=diff
>>> 
>>> ==============================================================================
>>> --- llvm/trunk/Makefile.rules (original)
>>> +++ llvm/trunk/Makefile.rules Mon Jun 24 18:20:04 2013
>>> @@ -691,9 +691,9 @@ ifdef UNIVERSAL
>>>    UNIVERSAL_ARCH := i386 ppc
>>>  endif
>>>  UNIVERSAL_ARCH_OPTIONS := $(UNIVERSAL_ARCH:%=-arch %)
>>> -  CompileCommonOpts += $(UNIVERSAL_ARCH_OPTIONS)
>>> +  TargetCommonOpts += $(UNIVERSAL_ARCH_OPTIONS)
>>>  ifdef UNIVERSAL_SDK_PATH
>>> -    CompileCommonOpts += -isysroot $(UNIVERSAL_SDK_PATH)
>>> +    TargetCommonOpts += -isysroot $(UNIVERSAL_SDK_PATH)
>>>  endif
>>> 
>>>  # Building universal cannot compute dependencies automatically.
>>> @@ -755,8 +755,7 @@ Preprocess.CXX= $(Compile.Wrapper) \
>>>          $(CXX) $(CPP.Flags) $(TargetCommonOpts) $(CPPFLAGS) \
>>>                $(CompileCommonOpts) $(CXX.Flags) -E
>>> Link          = $(Compile.Wrapper) \
>>> -           $(CXX) $(CPP.Flags) $(CXX.Flags) $(CXXFLAGS) $(LD.Flags) \
>>> -                $(LDFLAGS) $(TargetCommonOpts)  $(CompileCommonOpts)
>>> $(Strip)
>>> +           $(CXX) $(LD.Flags) $(LDFLAGS) $(TargetCommonOpts) $(Strip)
>>> 
>>> BCCompile.C   = $(LLVMCC) $(CPP.Flags) $(C.Flags) $(CFLAGS) $(CPPFLAGS) \
>>>                $(TargetCommonOpts) $(CompileCommonOpts)
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>>