[gmx-developers] hidden compiler flags in 4.6?

Tue Feb 5 20:58:12 CET 2013

For all developers out there: RFC.

I hope you do realize that the issues we are discussing are not if-s and
when-s and you'll face them as soon as you want to add/remove/change a
compiler flag!

On Tue, Feb 5, 2013 at 8:30 PM, Roland Schulz <roland at utk.edu> wrote:

>
>
>
> On Tue, Feb 5, 2013 at 1:48 PM, Szilárd Páll <szilard.pall at cbr.su.se>wrote:
>
>>
>> Hi,
>>
>>  Let me state this early enough:
>> - the concerns I raised are related to *usability* and not some standard
>> common practice defines the current compiler flag setup and behaviour;
>> - I consider the current setup safe and consistent for anyone who dos not
>> care  about compiler flags, but extremely confusing, a potential
>> performance concern, and error prone as well for those who want to do
>> *anything* with compiler flags. Just think through how will you explain to
>> an average, rather inexperienced GROMACS user to change a default flag to
>> something else.
>>
>> On Sun, Feb 3, 2013 at 1:53 AM, Roland Schulz <roland at utk.edu> wrote:
>>
>>>
>>>
>>>
>>>  On Sat, Feb 2, 2013 at 5:42 PM, Szilárd Páll <szilard.pall at cbr.su.se>wrote:
>>>
>>>>  Hi,
>>>>
>>>>  With the compiler flag handling changed between beta3 and final, now
>>>> it is completely impossible to check or modify compiler flags we generate.
>>>>
>>>>  I might have missed the discussion on this during code review, but I
>>>> simply consider this a bug. It is pretty much impossible to know what
>>>> compiler flags are being used - at least not at build time, not even by
>>>> looking at the cache, and not without first compiling a binary and checking
>>>> the "-version" output.
>>>>
>>>>  Not only that this is a rather annoying, making it impossible to
>>>> change the default compiler options is a very naive behavior.
>>>>
>>>  You need to set GMX_SKIP_DEFAULT_CFLAGS. This lets you overwrite and
>>> see what would be appended.
>>>
>>
>>  Sorry, but that does not answer my questions/concerns. Moreover,
>> GMX_SKIP_DEFAULT_CFLAGS seems to me an awkward workaround. First of all,
>> there is no intuitive way to figure out how to do what many moderately
>> advanced user will want accomplish at some point: simply reviewing and
>> changing the compiler flags.
>> Secondly, the name of the variable suggests nothing about the fact that
>> this is the only way to review/edit the flags and even if some user happens
>> to magically figure it out (which I highly doubt), a cluttered dump of the
>> flags "skipped" is even worse than providing a "configure-compatible"
>> behaviour, i.e with configure everything is in the config.log while we have
>> only the CMake defaults (=inferior), with configure you override
>> everything, in our case some flags get overridden other don't.
>>
>>
>>>
>>>
>>>   Additionally, there is another issue with the bove example: manually
>>>> passing CMAKE_C_FAGLS_RELEASE will overwrite the default release flags (-O3
>>>> -DNDEBUG). This leads to another issue: passing any additional release flag
>>>> will lead to -O2 being used as this is the default with most compilers (and
>>>> -O3 get overridden). Consequently, if one wants to *append* and
>>>> optimization flag, this can only be done post-configure by modifying the
>>>> cache, but not during configure which goes against the principle we tried
>>>> to adhere to that users should be able to flexibly configure GROMACS by
>>>> using a single command.
>>>>
>>>
>>>  O3 is set by cmake not by us - and it behaves as cmake developers
>>> think it should behave. Look at the redmine issues for the details.
>>>
>>
>>  If you consider this for a second you will probably agree: it's pretty
>> much irrelevant (from the user's point of view) whether a flag is CMake
>> default or custom GROMACS one. In fact, as we are very much concerned about
>> performance and many of the flags we set serve this goal, I would argue
>> that the optimization level should perhaps be controlled by us and not
>> CMake - especially that CMake happily overwrites the non-hidden part of
>> CMAKE_C_FAGLS_RELEASE thus throwing out the *very important* -O3. This will
>> result in falling back to -O2 (the default with most compilers) which will
>> lead to significant performance loss.
>>
>>
>>>
>>>   I think the above issues need fixing asap, preferably by 4.6.1, but
>>>> I'm not familiar with the code, so I can't suggest a concrete solution.
>>>>
>>>  I disagree. Everything works the way as has been discussed in redmine.
>>> We shoudn't change the behaviour after a release.
>>>
>>
>>  Sorry, but that's not entirely true. #1038 was mostly concerned about
>> the CPU acceleration detection not working consistently and about
>> monolithic vs modular CFLAGS which is not what I am talking about.
>> Unfortunately I did not have time neither to comment on 1040 nor to review
>> the change before it got merged (rather hastily, after only 12h by the
>> author himself). Note that 1040 which was talking about usability and
>> consistency and while the latter has improved, the former has IMO gotten
>> worse.
>>
>>
>> To conclude, I would be OK with the current behaviour if certain thing
>> were possible or less awkward to do, e.g.:
>>  - reviewing compiler flags used at *configure-time*;
>>  - adding compiler flags;
>>  - removing/overriding compiler flags;
>>
> Look at redmine for the details why any implementation, which allows
> overwrite and be consistent between Gromacs and cmake flags, would have
> been too complicated tol make it into 4.6.
>

I'll have a look at the discussion, but I'll need quite some time for that
as it very lengthy and contains a long, slightly unrelated, debate.

Note that I'm not talking about some magic override, but simply the ability
to go to the flags and remove e.g. the "-ip" icc flag which I know that it
can in some cases cause performance regression.

In my eyes, reviewing and changing flags is much more important than the
consistency of acceleration flags across cmake reruns, that's why I find it
unfortunate that the build system got crippled in an attempt to fix that
issue. Of course, I am biased, but putting aside the fact that I will
suffer from this build system "behaviour regression", I think many
sysadmins and other advanced users will also be scratching their heads.

>
>
>>  and adding or overriding should be possible through a single
>> command-line invocation.
>>
>> More concretely IMO what we need is:
>> - A summary printed or at least some read-only advanced variables so that
>> one does not need to run e.g "mdrun -version" to know what flags are used;
>>
> I was (and still am) all for the summary. I really liked Teemu's start of
> it. But it got cut because of time. Not sure it would be a good idea to
> print just the flags without having a full summary. I think having
> read-only variables is very confusing.
>

I see your point, but you have yet to reply to my criticism on the fact
that *it is impossible to at least check what flags are being used* and
that is a major flaw.

>
>
>> - An intuitive and consistent way to add/change the flags, what the user
>> wants is simple, but currently requires a set of strange steps: enable
>> skipping GROMACS flags, copy the printed flags, remove cache, re-run cmake
>> with the copied flags.
>>
> No need to remove the cache.
>

Fine, remove that step, but you've to add two more instead:
- check default CMake flags;
- re-add those when setting flags manually

Again, I would very much appreciate of you or others developers would
actually comment on the issue in question: the usability concerns around
this rather long, complicated, and error-prone set of steps required just
to change a flag -Foo to -Bar.

Cheers,
--
Szilárd

>
> Roland
>
>
>>  It might be a good idea to override everything or nothing regardless
>> whether it's GROMACS or CMake flags.
>>
>>  Cheers,
>> Szilard
>>
>>
>>>  Roland
>>>
>>>
>>>
>>>>
>>>>  Cheers,
>>>>  --
>>>> Szilárd
>>>>
>>>
>>>
>>>
>>>  --
>>> ORNL/UT Center for Molecular Biophysics cmb.ornl.gov
>>> 865-241-1537, ORNL PO BOX 2008 MS6309
>>>
>>> --
>>> gmx-developers mailing list
>>> gmx-developers at gromacs.org
>>> http://lists.gromacs.org/mailman/listinfo/gmx-developers
>>> Please don't post (un)subscribe requests to the list. Use the
>>> www interface or send it to gmx-developers-request at gromacs.org.
>>>
>>
>>
>
>
> --
> ORNL/UT Center for Molecular Biophysics cmb.ornl.gov
> 865-241-1537, ORNL PO BOX 2008 MS6309
>
> --
> gmx-developers mailing list
> gmx-developers at gromacs.org
> http://lists.gromacs.org/mailman/listinfo/gmx-developers
> Please don't post (un)subscribe requests to the list. Use the
> www interface or send it to gmx-developers-request at gromacs.org.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20130205/1d402227/attachment.html>