public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <burnus@net-b.de>
To: Dominique Dhumieres <dominiq@lps.ens.fr>
Cc: fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org, pault@gcc.gnu.org
Subject: Re: [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
Date: Mon, 07 Jan 2013 18:23:00 -0000	[thread overview]
Message-ID: <50EB12B3.3090205@net-b.de> (raw)
In-Reply-To: <20130107162204.DBBE43BC70@mailhost.lps.ens.fr>

Dominique Dhumieres wrote:
> (1) Is there any hint of -fno-whole-file misbehavior (as suggested by the
> second point of the "Reasoning")?

The middle end is simply not prepared for having multiple declaration 
for a single file ("translation unit"), which leads to wrong 
optimizations and thus to wrong code.

I think the bug which triggeded my decision to fix all remaining known 
-fno-whole-file issues and then to enable it by default  was PR 44945. 
See also http://gcc.gnu.org/ml/fortran/2010-07/msg00321.html

But there are surely more. Using multiple declarations was simply a 
design bug in the original implementation of gfortran, which was only 
realized rather late - and then deferred. Paul was that brave to fix 
this by implementing -fwhole-file.

> As said in my comment below, if the answer is yes, then IMO this should be
> investigated and fixed before removal.

We did the investigation when switching to -fwhole-file by default. And 
no, I do not want to investigate why the middle end "misbehaves" with 
multiple declarations in a single translation unit - nor do I want to 
fix it. I am content if everything works with -fwhole-file. There is 
really not any advantage of having multiple declarations in a single TU.

> (2) Is the -fwhole-file status ZKB (zero known bug)?
> I am not 100% confident that this the case (see at least PR 45128).

I am not aware of any bug since 4.6. I am rather certain that we fixed 
all known bugs in this area. Neither having remaining mult-decls nor 
issues which wouldn't occur with -fno-whole-file. By now it simply makes 
more sense to fix a bug, ignoring whether it worked by chance with 
-fno-whole-file or not. All recent regressions seem to be unrelated. 
Thus, for users it makes more sense to use an older version or wait for 
the patch than to try -fno-while-file. Given the potential to generate 
wrong-code bugs, I wouldn't trust -fno-whole-file for any code which 
goes beyond a hello-world example.

PR45128 requires the new array descriptor; we have a kludge which allows 
sub-type pointer in some cases. Seemingly, that kludge works in one case 
with -fno-whole-file where it fails with -fwhole-file. But I will 
actively ignore all subpointer PRs until the new array descriptor is 
ready, which will be the right fix.

> (3) What is the confidence that there is no rampant bug in -fwhole-file?

Well, even if there is a bug which doesn't affect -fno-whole-file, using 
-fno-whole-file is no solution, given that -fno-whole-file has many 
known bugs.

And while I am willing to fix -fwhole-file bugs, I will ignore any 
-fno-whole-file ones.

> (4) Is there an easy replacement for -fno-whole-file?

Yes: -fwhole-file :-)  Really, you shouldn't think of -fno-whole-file 
anymore.

> Here the answer is clearly yes: put the different TUs in different files.

Or doing that ;-)

> My understanding of -fno-whole-file as well as -fno-realloc-lhs,
> -fno-frontend-optimize, or any of the -fno-* options is two fold:
>
> (i) to provide a workaround to user hit by a bug in the corresponding area,
> (ii) to help the maintainers to locate the bug or to change the default in
> case of a too severe bug.

Well, the main point of -fno-realloc-lhs is to disable the performance 
penalty of RHS. That is also helped to find some bugs was a side effect. 
For -fno-frontend-optimize I agree that its purpose was indeed to toggle 
the optimization separately from only using -O0 vs -O1.

But there is a difference between -frealloc-lhs, -ffrontend-optimize and 
-fwhole-file: The former two enable a specific feature or optimization 
and valid programs can (should) be produced with either version. By 
contrast, in terms of middle-end assumptions -fno-whole-file is always 
wrong.

I think (ii) does not really apply. We never changed the defaults when a 
serious bug popped up, we always fixed it. [Though we might have 
suggested a command-line option as workaround, cf. (i).] For 
-fno-whole-file, it also didn't help much with localizing the bug. For 
-f(no-)frontend-optimize I agree that it helped several times to isolate 
the bug.

>> * There are many wrong-code issues and probably also some ICEs with
>> -fno-whole file.
> I think the wording is misleading. I have checked bugzilla for open PR
> containing -fno-whole-file and I have found only 2 (48939 and 52621).
> None of them are related to wrong-code issue due to -fno-whole-file.

You have to search for mult-decl issues. -fwhole-file was the cure of 
that problem; as it had a couple of issues before July 2010 and wasn't 
enabled by default, almost no one used it. Hence, you don't find it in 
the log. And after -fwhole-file became the default, no one cared anymore 
about issues which occurred only with -fno-whole-file.

> My understanding of the -fno-whole-file option is that it is (should be)
> strictly equivalent to put all the TUs in separate files. If it exists an
> example for which this is not true, I think it would be a bug

But that's not how the middle end works. It expects that in a single 
file (translation unit), no multiple declarations for the same object exist.

We do not want to change the middle end. Besides, having a single decl 
per object allows for inlining and similar optimization. There is simply 
no reason for -fno-whole-file except that gfortran until 4.5 had it by 
default.

In principle, we could have done the switch without adding 
-f(no-)whole-file; having the flag was only useful because: (i) It 
allowed Paul to sensible put the patch into the trunk even though it was 
not yet completely working. (ii) It allowed to find out that PR44945 was 
only failing with -fno-whole-file. (iii) Mainly in 2010-07, it made it 
easier to find regressions in -fwhole-file. After 2010-07, most issues 
were solved, rendering -f(no-)whole-file less useful.

>> * The generated code of -fwhole-file is faster as it allows for inlining.
> This has nothing to do with the removal of -fno-whole-file.

No but with the addition of -fwhole-file. Without, inlining was 
essentially not possible.

> AFAICT most of the speeddup is achieved with -fwhole-program.

But that only works with a single file/TU, unless you use LTO. 
Additionally, -flto and -fwhole-program both enable -fwhole-file. Due to 
the multiple declarations issue, -fno-whole-file -fwhole-program would 
optimize many procedures away, which were actually used - leading to 
linking errors.

> OK, but no time table has been released so far.

In terms of the bugs, I was even considering to remove it for 4.6. I 
think by 2010-08 we had fixed nearly all -fwhole-file specific bugs - 
and no one wanted to go back to -fno-whole-file.

> Compiling PR 45128 with -fwhole-file gives an ICE: Segmentation fault, 
> while with -fno-whole-file it gives a linking error Undefined symbols: 
> "_span.0", referenced from: ... as it does if the two TUs are put is 
> different files. Dominique 

As said above: I will actively ignore subpointer bugs! Actually, before 
I thought that -fno-whole-file worked for that file, but as the linker 
error indicates, -fno-whole-file also doesn't work!

Tobias

  reply	other threads:[~2013-01-07 18:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-07 16:22 Dominique Dhumieres
2013-01-07 18:23 ` Tobias Burnus [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-01-04 23:31 Tobias Burnus
2013-01-07 14:14 ` Tobias Burnus
2013-02-13 18:20 ` Tobias Burnus
2013-02-13 19:29   ` Steve Kargl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50EB12B3.3090205@net-b.de \
    --to=burnus@net-b.de \
    --cc=dominiq@lps.ens.fr \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=pault@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).