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

[Eric Christopher]

> 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.
>

Agreed.

> 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?
>

Debug options were the main reason and they were being passed on to
the linker... at least last I checked. I've tried to duplicate after
updating and can't anymore so I'm left with a fun yet confusing
problem but not anything urgent :)

>> 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?
>

Well, it is invoking the linker, but using the compiler driver to do
it. As is proper :)

And no, not using CXX=ld. That's even worse ;)

>>
>> 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.
>

*nod* This is the case I thought I'd fixed, but I guess not for
command line options being passed in - so just the buildit case.

As long as last one wins with all of the various linkers and options
are passed down in the order they're in on the command line then it
probably won't be too much of a problem in practice.

I.e. something like

make CXXFLAGS="-macosx-version-min=10.5" LDFLAGS="-Wl,-macosx-version-min=10.7"

might get you in trouble, if you wanted to compile .o files for 10.5
and link assuming 10.7 features. It's a bit of a contrived example,
but specific. In the territory of "don't do that". Less specific, but
more troublesome, would be different opt flags for compilers/linkers
that can accept opt flags.

Arguably contrived again and I'm not blocked on this so go ahead and
keep this in.

Thanks for the discussion and sorry for the breakage. Having a public
bot that tests this code path would probably be a good thing since
this type of option is the only one I can come up with that works this
way.

-eric

>>
>> -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
>>>>
>>>>
>>>
>