Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
Erik Joelsson
erik.joelsson at oracle.com
Thu Feb 2 08:58:05 UTC 2012
Hello,
I've intentionally left out all VS project files. I'm not sure but I
suspect that CPP is some kind of standard name for the compiler in that
context. I'm happy to hear I didn't mess up the project creation!
/Erik
On 2012-02-02 09:01, Staffan Larsen wrote:
> Those generated files are Visual Studio projects for VS version 6 (I think). Really old stuff. I don't think these are used (nor is VS 6 supported), so we should eventually clean out that code. I wouldn't bother fixing it.
>
> /Staffan
>
>
> On 2 feb 2012, at 08:51, Bengt Rutisson wrote:
>
>> Hi Erik,
>>
>> I have not looked closely at your changes, so don't consider this a review. What I did do was apply your patch and try to create a Visual Studio project with the create.bat script. That still works. Nice!
>>
>> One thing I noticed is that the ProjectCreator tool generates some files for the ADLC builds. These files still use the CPP name. Since it still works to create a project I don't know if this needs to be changed. But maybe it is good to be consistent.
>>
>> Here's where we use the CPP name:
>>
>> src\share\tools\ProjectCreator/WinGammaPlatformVC6.java:
>>
>> 68: printWriter.println("CPP=cl.exe");
>> 145: printWriter.println("# SUBTRACT CPP /YX /Yc /Yu");
>> 149: printWriter.println("# ADD CPP /Yc\"incls/_precompiled.incl\"");
>> 210: rv.add("ADD CPP /nologo /MT /W3 /WX /GX /YX /Fr /FD /c");
>> 217: rv.add("ADD BASE CPP "+Util.prefixed_join(" /I ", includes, true));
>> 218: rv.add("ADD CPP "+Util.prefixed_join(" /I ", includes, true));
>> 219: rv.add("ADD BASE CPP "+Util.prefixed_join(" /D ", defines, true));
>> 220: rv.add("ADD CPP "+Util.prefixed_join(" /D ", defines, true));
>> 221: rv.add("ADD CPP /Yu\"incls/_precompiled.incl\"");
>> 230: rv.add("ADD BASE CPP /MD");
>> 231: rv.add("ADD CPP /MD");
>> 252: rv.add("ADD BASE CPP /Gm /Zi /O"+opt);
>> 272: rv.add("ADD CPP /O"+getOptFlag());
>>
>> And these are the generated files:
>>
>> build\vs-amd64/compiler2/generated/ADLCompiler.dsp
>> build\vs-amd64/tiered/generated/ADLCompiler.dsp
>>
>>
>> Bengt
>>
>> On 2012-02-02 03:33, David Holmes wrote:
>>> Hi Erik,
>>>
>>> On 1/02/2012 7:13 PM, Erik Joelsson wrote:
>>>> http://cr.openjdk.java.net/~erikj/7141242/webrev.00/
>>>> 240 lines changed: 0 ins; 19 del; 221 mod; 6363 unchg
>>>>
>>>> 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
>>> Lots of CCC to CXX too :)
>>>
>>> One compatibility concern: anyone currently setting CPP_FLAGS or LINK_FLAGS etc, externally, will need to change to the new names. Probably worth sending a wider email (jdk8-dev?) when this gets pushed.
>>>
>>> ---
>>>
>>> make/bsd/makefiles/gcc.make
>>>
>>> - CPP = $(CXX)
>>> + CXX = $(CXX)
>>>
>>> infinite recursion or a tautology? :)
>>>
>>> ---
>>>
>>> make/*/makefiles/launcher.make
>>>
>>> Not your doing but this has highlighted some strange rules eg:
>>>
>>> + $(QUIETLY) $(CC) -g -o $@ -c $< -MMD $(LAUNCHERFLAGS) $(CXXFLAGS)
>>>
>>> C++ flags passed to C compiler?
>>>
>>> ---
>>>
>>> make/*/makefiles/rules.make
>>>
>>> -# $(CC) is the c compiler (cc/gcc), $(CCC) is the c++ compiler (CC/g++).
>>> -C_COMPILE = $(CC) $(CPPFLAGS) $(CFLAGS)
>>> -CC_COMPILE = $(CCC) $(CPPFLAGS) $(CFLAGS)
>>> +# $(CC) is the c compiler (cc/gcc), $(CXX) is the c++ compiler (CC/g++).
>>> +C_COMPILE = $(CC) $(CXXFLAGS) $(CFLAGS)
>>> +CC_COMPILE = $(CXX) $(CXXFLAGS) $(CFLAGS)
>>>
>>> The original code is confusing, given that CC is the C compiler it makes no sense that a C++ compile be called CC_COMPILE. Is it worth changing these to CC_COMPILE and CXX_COMPILE? Maybe a secondary cleanup?
>>>
>>> And again C++ flags passed to C compiler :(
>>>
>>> ---
>>>
>>> You missed a couple of scripts on Windows that use LINK_VER:
>>>
>>> windows/get_msc_ver.sh
>>> windows/build_vm_def.sh
>>>
>>> Cheers,
>>> David
>>> -----
>>>
>>>
>>>> The build-infra project is starting to move into jdk8. For the hotspot
>>>> build to stay compatible with the changes, the naming of standard make
>>>> variables, like CC and LD need to be standardized across the build.
>>>> Currently hotspot names the C++ compiler CPP, which is traditionally the
>>>> name of the preprocessor. The windows nmake files name the linker LINK.
>>>> We would like to rename the C++ compiler to CXX and have the linker
>>>> named LD everywhere.
>>>>
>>>> Patch is tested with hsx/hotspot-rt. Testing with jdk7u is in progress.
>>>>
>>>> /Erik
>>>>
>>>>
More information about the build-dev
mailing list