Adding icl to compile wrapper script

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Adding icl to compile wrapper script

Peyton, Jonathan L
Hello automake developers,

I have this patch which adds icl (Windows Intel Compiler) to the lib/compile wrapper script.  Icl has a Visual Studio driver interface and supports all the flags that are translated inside the compile script.  It also doesn't support the '-o -c' idiom similar to the Visual Studio compiler so I think it is a good candidate for this wrapper script.

-- Johnny

add-icl-to-compile-script.patch (486 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Adding icl to compile wrapper script

Peter Rosin
Hi Jonathan,

On 2015-11-16 17:44, Peyton, Jonathan L wrote:
> Hello automake developers,
>
> I have this patch which adds icl (Windows Intel Compiler) to the lib/compile wrapper script.  Icl has a Visual Studio driver interface and supports all the flags that are translated inside the compile script.  It also doesn't support the '-o -c' idiom similar to the Visual Studio compiler so I think it is a good candidate for this wrapper script.
>
> -- Johnny

Yes, that sounds usable. However, ideally I think you should plagiarize the test
t/compile4.sh so that we also test the wrapping of the icl compiler when it is
available. You will need to patch the require_tool function in t/ax/am-test-lib.sh
to recognize this "new" icl tool for that to work.

You are also missing a changelog and you need to update the scriptversion at the
top of the compile script.

You could also write the case match as

  cl | *[/\\]cl | cl.exe | *[/\\]cl.exe \
  icl | *[/\\]icl | icl.exe | *[/\\]icl.exe )

in order to not duplicate the code.

Since you send this from an intel account, why is it that -c -o isn't supported?

And another question is if icl supports the -showIncludes option from MSVC? Or
does it have a way of its own to output dependencies? You might want to
support a one-pass mode to find dependencies in the lib/depcomp script.

Cheers,
Peter

Reply | Threaded
Open this post in threaded view
|

RE: Adding icl to compile wrapper script

Peyton, Jonathan L
Peter,

Thanks for all the suggestions!

> However, ideally I think you should plagiarize the test t/compile4.sh so that we also test the wrapping of the icl compiler when it is available. You will need to patch the require_tool function in t/ax/am-test-lib.sh to recognize this "new" icl tool for that to work.
The new patch adds t/compile7.sh which is a copy of t/compile4.sh, but with icl instead of cl.  If there is a better way to avoid the code duplication I'd be more than willing to do it that way.
Also, I've patched t/ax/am-test-lib.sh to recognize icl.

> You are also missing a changelog
* lib/compile: Have icl be treated similarly to cl
* t/ax/am-test-lib.sh: Allow icl in require_tool
* t/compile7.sh: Add new test file for icl
* t/list-of-tests.mk: Add t/compile7.sh

> and you need to update the scriptversion at the top of the compile script.
Done.

> You could also write the case match as...
Done.

> Since you send this from an intel account, why is it that -c -o isn't supported?
Icl is intended to be a drop in replacement for cl.  So it has the same interface as cl.

>And another question is if icl supports the -showIncludes option from MSVC?
It does and works how it is in depcomp.

-- Johnny

-----Original Message-----
From: Peter Rosin [mailto:[hidden email]]
Sent: Tuesday, November 17, 2015 8:07 AM
To: Peyton, Jonathan L; [hidden email]
Subject: Re: Adding icl to compile wrapper script

Hi Jonathan,

On 2015-11-16 17:44, Peyton, Jonathan L wrote:
> Hello automake developers,
>
> I have this patch which adds icl (Windows Intel Compiler) to the lib/compile wrapper script.  Icl has a Visual Studio driver interface and supports all the flags that are translated inside the compile script.  It also doesn't support the '-o -c' idiom similar to the Visual Studio compiler so I think it is a good candidate for this wrapper script.
>
> -- Johnny

Yes, that sounds usable. However, ideally I think you should plagiarize the test t/compile4.sh so that we also test the wrapping of the icl compiler when it is available. You will need to patch the require_tool function in t/ax/am-test-lib.sh to recognize this "new" icl tool for that to work.

You are also missing a changelog and you need to update the scriptversion at the top of the compile script.

You could also write the case match as

  cl | *[/\\]cl | cl.exe | *[/\\]cl.exe \
  icl | *[/\\]icl | icl.exe | *[/\\]icl.exe )

in order to not duplicate the code.

Since you send this from an intel account, why is it that -c -o isn't supported?

And another question is if icl supports the -showIncludes option from MSVC? Or does it have a way of its own to output dependencies? You might want to support a one-pass mode to find dependencies in the lib/depcomp script.

Cheers,
Peter

add-icl-to-compile-script-v2.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Adding icl to compile wrapper script

Peter Rosin
Hi Jonathan,

On 2015-11-18 22:05, Peyton, Jonathan L wrote:

> Peter,
>
> Thanks for all the suggestions!
>
>> However, ideally I think you should plagiarize the test t/compile4.sh so that we also test the wrapping of the icl compiler when it is available. You will need to patch the require_tool function in t/ax/am-test-lib.sh to recognize this "new" icl tool for that to work.
> The new patch adds t/compile7.sh which is a copy of t/compile4.sh, but with icl instead of cl.  If there is a better way to avoid the code duplication I'd be more than willing to do it that way.
> Also, I've patched t/ax/am-test-lib.sh to recognize icl.
>
>> You are also missing a changelog
> * lib/compile: Have icl be treated similarly to cl
> * t/ax/am-test-lib.sh: Allow icl in require_tool
> * t/compile7.sh: Add new test file for icl
> * t/list-of-tests.mk: Add t/compile7.sh
>
>> and you need to update the scriptversion at the top of the compile script.
> Done.
>
>> You could also write the case match as...
> Done.
>
>> Since you send this from an intel account, why is it that -c -o isn't supported?
> Icl is intended to be a drop in replacement for cl.  So it has the same interface as cl.
>
>> And another question is if icl supports the -showIncludes option from MSVC?
> It does and works how it is in depcomp.
>
> -- Johnny
>
> -----Original Message-----
> From: Peter Rosin [mailto:[hidden email]]
> Sent: Tuesday, November 17, 2015 8:07 AM
> To: Peyton, Jonathan L; [hidden email]
> Subject: Re: Adding icl to compile wrapper script
>
> Hi Jonathan,
>
> On 2015-11-16 17:44, Peyton, Jonathan L wrote:
>> Hello automake developers,
>>
>> I have this patch which adds icl (Windows Intel Compiler) to the lib/compile wrapper script.  Icl has a Visual Studio driver interface and supports all the flags that are translated inside the compile script.  It also doesn't support the '-o -c' idiom similar to the Visual Studio compiler so I think it is a good candidate for this wrapper script.
>>
>> -- Johnny
> Yes, that sounds usable. However, ideally I think you should plagiarize the test t/compile4.sh so that we also test the wrapping of the icl compiler when it is available. You will need to patch the require_tool function in t/ax/am-test-lib.sh to recognize this "new" icl tool for that to work.
>
> You are also missing a changelog and you need to update the scriptversion at the top of the compile script.
>
> You could also write the case match as
>
>   cl | *[/\\]cl | cl.exe | *[/\\]cl.exe \
>   icl | *[/\\]icl | icl.exe | *[/\\]icl.exe )
>
> in order to not duplicate the code.
>
> Since you send this from an intel account, why is it that -c -o isn't supported?
>
> And another question is if icl supports the -showIncludes option from MSVC? Or does it have a way of its own to output dependencies? You might want to support a one-pass mode to find dependencies in the lib/depcomp script.
>
> Cheers,
> Peter
I pushed this out on the micro branch since it is a pain to have different versions of
the support scripts in flight. I also added a NEWS entry with a reference to the non-
existent version 1.15.1, which will be the version that is released from the micro
branch, but I have no way of knowing if such a release will materialize or not. But,
since Stefano has stepped down, I imagine that the future maintainer will want to
do a bugfix release with as little changes as possible as his/her first release, to test
the waters, so it felt right to start up the 1.15.1 release in the NEWS file. Easy enough
to clean that up if the 1.16 release is the next release...

I also merged the patch to the minor and master branches.

Cheers,
Peter