Hi Ryan,

> On Jan 20, 2018, at 1:15 AM, Ryan Schmidt <[email protected]> wrote:
> 
> 
> On Jan 20, 2018, at 01:54, Ryan Schmidt wrote:
> 
>> On Jan 12, 2018, at 12:54, Frank Schima wrote:
>> 
>>> Frank Schima (mf2k) pushed a commit to branch master
>>> in repository macports-ports.
>>> 
>>> 
>>> https://github.com/macports/macports-ports/commit/05e7489989c736d2397e10073b8e8edd628d1ac8
>>> 
>>> The following commit(s) were added to refs/heads/master by this push:
>>> 
>>>    new 05e7489  unpaper: Update to version 6.1  - Use recommended compile 
>>> flags
>>> 
>>> 05e7489 is described below
>>> 
>>> 
>>> commit 05e7489989c736d2397e10073b8e8edd628d1ac8
>>> 
>>> Author: Frank Schima
>>> AuthorDate: Fri Jan 12 11:54:56 2018 -0700
>>> 
>>> 
>>>   unpaper: Update to version 6.1
>>>    - Use recommended compile flags
>>> 
>>>   Fixes: https://trac.macports.org/ticket/55660
>> 
>> 
>>> +configure.cflags-append -O2 -march=native -pipe -flto
>> 
>> Why? Recommended by whom?
>> 
>> -O2 is an optimization flag. If you want to set it, do so in 
>> configure.optflags. But an upstream recommendation is not a sufficient 
>> reason to me to change it.
>> 
>> -march=native may cause the binaries produced by our buildbot to be 
>> incompatible with user system, if user systems are not using the same 
>> processor as our buildbot.
>> 
>> -pipe is a flag MacPorts adds for you, if the user requested it in 
>> macports.conf setting; ports should not override it. The MacPorts default is 
>> enabled.
> 
> I see now that the project's README.md recommends using the above flags, only 
> because "it is important to build with optimizations turned on". MacPorts 
> builds with optimizations turned on by default, specifically -Os, so let's 
> leave it at that.
> 
>> I don't know about -flto.
> 
> The README.md says "Even better, if your compiler supports it, is to use 
> Link-Time Optimizations". So we can certainly use this for compilers that 
> support it. Apparently recent clang does. I don't know about old clang. 
> gcc-4.2 which we use on Snow Leopard and earlier does not.
> 
> So I would like to commit the attached (which also fixes #55725). Ok?

It’s fine with me. Thank you!


-Frank

Reply via email to