Patch to fix -fsyntax-only

craig@jcb-sc.com craig@jcb-sc.com
Fri Feb 19 07:00:00 GMT 1999


>  In message < 19990218221323.13314.qmail@deer >you write:
>  > Thu Feb 18 17:05:11 1999  Craig Burley  <craig@jcb-sc.com>
>  > 
>  > 	Fix -fsyntax-only (fix ICEs, decrease spurious warnings):
>  > 	* stmt.c (expand_expr_stmt): Expand expr even when -fsyntax-only.
>Can you comment a little more on this change?  It looks like we explicitly
>avoided expanding this stuff with -fsyntax-only, and how we're allowing it
>to be expanded. 

Yes, that's the case.  The *idea* of the original code is to avoid
doing (some) needless RTL generation.  As I discovered when fixing
ICEs resulting from g77 invocations, RTL generation happens anyway,
though not from that particular place, due to all the other entry
points in stmt.c, the ones relating to jumps, loops, case statements,
and so on.

So, I *tried* fixing all of those, but there were too many dependencies
on some crumbs of RTL generation in stmt.c (and perhaps elsewhere) to
make a go of it.

(This was near the end of an iterative process of debugging ICEs: I
had a list of ICEs for g77 tests using -fsyntax-only; picked the top
item; debugged it; added code to fix it; verified the fix on that
one case; reran the tests; and repeated.)

So, I backed out all my stmt.c changes and re-debugged the most
recent ICE test case.  That yielded a change to one line of varasm.c
(the `else' to which I added ` if (...)'), which fixed that case, and
left no ICEs in the re-generated list.

Nice, but there were nagging concerns:

  -  The code testing flag_syntax_only in stmt.c had been broken in
     the past due to other code relying on RTL-generation "crumbs"
     (you noted one of my fixes below).

  -  RTL generation was, in fact, not inhibited, and likely *could*
     not be easily inhibited, by flag_syntax_only.  (That is, without
     lots of destabilizing code changes.)  It happens in lots of other
     places in stmt.c, which had only that *one* check of the flag,
     and didn't (easily) support new code for all the checks that seemed
     (to me) appropriate.

  -  The (few) tests I'd introduced to fix all the ICEs seemed to
     be in one coherent class of avoiding calling *output* routines
     involving assembly-code generation.

  -  Those few tests represented a fairly small "choke point" on
     restricting output.  I *love* small choke points in code,
     especially a large project -- because they usually indicate
     less need for new chunks of code above and below that point
     to accommodate the feature (flag_syntax_only here) without
     breaking the system, and can (though not so much in this case)
     make it much easier to see the need for accommodating it when
     modifying code right around that choke point.

So, since I'd noticed I had to fix expand_expr to avoid output
when flag_syntax_only *anyway*, due to it getting called from
paths *other* than expand_expr_stmt (which, at this point, I
considered "odd man out" for checking flag_syntax_only in stmt.c),
I decided to see what'd happened if I removed this extra test.

Voila, not only did everything still work (no ICEs, `make check'
ran fine), but lots of spurious diagnostics of unused variables
went away in my private testing (which shows me changes in both
stderr and stdout output).

(I believe the spurious diagnostics could be fixed, wholesale,
by having flag_syntax_only govern the turning off of the
warning flags we know we can't properly handle.  But I wanted to
put off thinking about that.)

I reviewed the remaining instances of flag_syntax_only, and they
seemed to mostly involve avoiding optimization phases, though there
are one or two (in toplev.c, IIRC) that might themselves be
removable in line with my "small choke point" tendencies.

But I felt this would be a worthwhile start, at least of a discussion,
regarding the required vs. desired vs. undesired behavior, and
long-term maintability, of flag_syntax_only.

In essence, I'm proposing we apply the elimination rule: if we can't
see a *requirement* for eliminating this test (and I'm saying
"avoid RTL generation" isn't it, because this test *alone* does
not avoid that, so it's incomplete to keep it as is), we eliminate it.

I don't have a strong opinion about this, or about whether RTL
generation should happen (though some of it seems intertwined with
diagnosing some kinds of errors some people might call "syntactic"
or, at least, "semantic", such as disagreements between the
return type of a function and whether the `return' statements in
it actually return a value).

But, I think it'd be best to eliminate all tests of flag_syntax_only
in stmt.c, and leave it up to whoever really *wants* to disable
all RTL generation accordingly, throughout stmt.c, to add *all*
the tests in a comprehensive fashion.  (I don't know for sure
that stmt.c represents all of the pertinent front-end interface,
beyond varasm.c routines such as assemble_zeros, which g77 uses,
for example.  But I think it's the major player.)

In summary: my proposed changes leave a fairly small, perhaps
almost minimal, set of tests of flag_syntax_only in the code
to support *no* output of code to asm_out_file.  Avoiding RTL
generation is a whole 'nother thing, requiring *lots* of tests
and, perhaps, reworking of code in stmt.c and elsewhere -- the
one test in stmt.c I propose we eliminate is nowhere near the
complete picture, and has probably misled some people into
thinking it was.

>Also note that we tweaked this code about a year ago to handle the case
>where expr_statement_for_value was true:
>
>  revision 1.29
>  date: 1998/03/28 00:38:46;  author: law;  state: Exp;  lines: +1 -1
>          * stmt.c (expand_expr_stmt): Must generate code for
>          statements within an expression (gcc's `({ ... )}')
>          even if -fsyntax-only.
>  Patch from Craig.

Yes, then I had to patch it again later, because I missed cases
of crumb-usage I'd forgotten to consider (basic programming
mistake on my part).

>  > 	* varasm.c (assemble_zeros, assemble_variable,
>  > 	output_constant_def): Do nothing when -fsyntax-only.
>This is OK.

I'll commit that part of it sometime soon.

        tq vm, (burley)


More information about the Gcc-patches mailing list