Bug 29478 - [4.2 Regression] optimization generates warning for casts
Summary: [4.2 Regression] optimization generates warning for casts
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.2.0
: P2 normal
Target Milestone: 4.2.3
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 30183 30743 32854 33859 34076 36042 (view as bug list)
Depends on: 16876
Blocks: 31959
  Show dependency treegraph
 
Reported: 2006-10-15 14:48 UTC by Martin Koegler
Modified: 2009-01-15 23:03 UTC (History)
14 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.1.1 4.1.2 4.3.0
Known to fail: 4.2.0
Last reconfirmed: 2007-06-23 09:39:37


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Koegler 2006-10-15 14:48:56 UTC
The following is a strip down example of a GDB build problem. 

ada_lookup_symbol_list gets a const struct block* pointer. This pointer is casted to struct pointer* and passed to remove_out_of_scope_renamings. 

This code (as with no optimization) should not issue a warning, as the const qualifier is discarded by the programmer. 

If a optimization level is enabled, a warning is generated (which breaks the GDB build, as it uses -Werror).

y.i:
struct block;

static int
remove_out_of_scope_renamings (struct block *current_block)
{
  return 1;
}
int
ada_lookup_symbol_list (const struct block *block0)
{
  return remove_out_of_scope_renamings ((struct block *) block0);
}

$./cc1 y.i
 remove_out_of_scope_renamings ada_lookup_symbol_list
Execution times (seconds)
 parser                :   0.01 (50%) usr   0.00 ( 0%) sys   0.00 ( 0%) wall      28 kB ( 3%) ggc
 TOTAL                 :   0.02             0.00             0.02                887 kB
Extra diagnostic checks enabled; compiler may run slowly.
Configure with --disable-checking to disable checks.

$./cc1 -O1 y.i
 remove_out_of_scope_renamings ada_lookup_symbol_list
Analyzing compilation unitPerforming interprocedural optimizations
Assembling functions:
 ada_lookup_symbol_list
y.i: In function 'ada_lookup_symbol_list':
y.i:11: warning: passing argument 1 of 'remove_out_of_scope_renamings' discards qualifiers from pointer target type

Execution times (seconds)
 parser                :   0.01 (50%) usr   0.00 ( 0%) sys   0.00 ( 0%) wall      28 kB ( 3%) ggc
 expand                :   0.00 ( 0%) usr   0.00 ( 0%) sys   0.01 (33%) wall       5 kB ( 1%) ggc
 TOTAL                 :   0.02             0.00             0.03                890 kB
Extra diagnostic checks enabled; compiler may run slowly.
Configure with --disable-checking to disable checks.

$/cc1 --version
GNU C version 4.2.0 20061015 (experimental) (i686-pc-linux-gnu)
        compiled by GNU C version 4.2.0 20061009 (experimental) m68hc05.
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Comment 1 Andrew Pinski 2006-10-15 15:25:39 UTC
Confirmed, a regression from 4.1.2.
Comment 2 Andrew Pinski 2006-10-15 15:41:04 UTC
Janis,
  Could you do a regression hunt on this bug?

Thanks,
Andrew
Comment 3 Richard Biener 2006-10-16 13:37:48 UTC
Happens during inlining, so I guess one of the strip-useless-type-conversion patches caused this.

But initialize_inlined_parameters shouldn't call into the frontend again here:

      /* Find the initializer.  */
      value = lang_hooks.tree_inlining.convert_parm_for_inlining
              (p, a ? TREE_VALUE (a) : NULL_TREE, fn, argnum);

I don't see why this should be necessary anymore as we're now inlining gimple?
Comment 4 Richard Biener 2006-10-16 13:46:41 UTC
I'm testing complete removal of the langhook.
Comment 5 Richard Biener 2006-10-16 16:06:28 UTC
Doesn't work out.  It looks like we depend on its side-effects in some testcases:

FAIL: gcc.dg/assign-warn-3.c  (test for warnings, line 9)
FAIL: gcc.dg/assign-warn-3.c  (test for warnings, line 13)
FAIL: gcc.dg/pr29254.c  (test for warnings, line )
FAIL: gcc.dg/pr29254.c  (test for warnings, line 21)
FAIL: gcc.dg/warn-1.c  (test for warnings, line 15)
FAIL: gcc.dg/noncompile/pr16876.c (internal compiler error)
FAIL: gcc.dg/noncompile/pr16876.c  (test for errors, line 10)
...

pr29254 is artificial - it tests for the bogus warning, otherwise the warning
should be done in the frontend.
Comment 6 Janis Johnson 2006-10-16 19:21:56 UTC
A regression hunt identified the following patch:

    http://gcc.gnu.org/viewcvs?view=rev&rev=116424

    r116424 | amylaar | 2006-08-25 18:51:57 +0000 (Fri, 25 Aug 2006)
Comment 7 Jorn Wolfgang Rennecke 2006-10-23 17:30:21 UTC
(In reply to comment #6)
> A regression hunt identified the following patch:
> 
>     http://gcc.gnu.org/viewcvs?view=rev&rev=116424
> 
>     r116424 | amylaar | 2006-08-25 18:51:57 +0000 (Fri, 25 Aug 2006)
> 

When I modify the testcase to have the function defintion after the use:

struct block;

static int remove_out_of_scope_renamings ();

int
ada_lookup_symbol_list (const struct block *block0)
{
  return remove_out_of_scope_renamings ((struct block *) block0);
}

static int
remove_out_of_scope_renamings (current_block)
  struct block *current_block;
{
  return 1;
}

I still see that the cast has been stripped at the point where
c_convert_parm_for_inlining is called:

(gdb) call debug_tree(value)
 <parm_decl 0xb593c000 block0
    type <pointer_type 0xb59315c0
        type <record_type 0xb5931564 block readonly VOID
...

OTOH, if I further change the passed parameter to a plain 0, value contains an unadorned integer constant.   

(gdb) call debug_tree(value)
 <integer_cst 0xb58aa978 type <integer_type 0xb58bb284 int> constant invariant 0>

I think it is wrong that we are optimizing the expression before converting
it to the required type.
Moreover, why is it right to drop NOP_EXPRESSIONS there?  If for some reason
the compiler finds it needs to put the initializer in a temporary variable,
and uses the type of the value for that variable, we'll get an alias set
problem.
Comment 8 pinskia@gmail.com 2006-10-23 21:37:02 UTC
Subject: Re:  [4.2 Regression] optmization generates warning for casts

On 23 Oct 2006 17:30:22 -0000, amylaar at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
> I think it is wrong that we are optimizing the expression before converting
> it to the required type.
> Moreover, why is it right to drop NOP_EXPRESSIONS there?  If for some reason
> the compiler finds it needs to put the initializer in a temporary variable,
> and uses the type of the value for that variable, we'll get an alias set
> problem.

Hmm, This is one of the language hooks which really need to go away
for LTO as I understand it.  Also I the real problem comes from the
inliner or the gimplifier (I am betting the gimplifier) removing the
cast as it is useless in our type system.
Comment 9 Jorn Wolfgang Rennecke 2006-10-24 13:50:52 UTC
(In reply to comment #8)
> Hmm, This is one of the language hooks which really need to go away
> for LTO as I understand it.  Also I the real problem comes from the
> inliner or the gimplifier (I am betting the gimplifier) removing the
> cast as it is useless in our type system.

How is LTO supposed to handle argument passing for inlining?  As I see it,
we have basically two choices:
1.: We pass a actual parameter with a type and a value of that type,
    to a formal parameter of a possibly different type.
    This can require conversions, which would have to be either done by
    frontend hooks (this is the way the intra-module inlining works at
    the moment), or by some frontend-independent mechanism that the
    frontends anticipate when they generate the parameter encodings for the
    call site.
2.: We pass an actual parameter as bit pattern in registers and/or a stack
    slot.  The inliner will have to figure out where its formal parameter
    is coming in, and will need to re-build abstractions that the optimizers
    can work with.
Comment 10 Andrew Pinski 2007-02-09 14:41:23 UTC
*** Bug 30743 has been marked as a duplicate of this bug. ***
Comment 11 Martin Michlmayr 2007-03-18 09:16:00 UTC
This bug hasn't seen any activity in several months.  Any idea who should work on resolving it?
Comment 12 Mark Mitchell 2007-05-14 22:26:11 UTC
Will not be fixed in 4.2.0; retargeting at 4.2.1.
Comment 13 Richard Biener 2007-06-23 09:39:37 UTC
Mine, patch posted.
Comment 14 Richard Biener 2007-06-23 18:18:11 UTC
Subject: Bug 29478

Author: rguenth
Date: Sat Jun 23 18:17:57 2007
New Revision: 125974

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125974
Log:
2007-06-23  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/16876
	PR middle-end/29478
	* tree.h (CALL_CANNOT_INLINE_P): New macro to access static_flag
	for CALL_EXPRs.
	* tree-inline.c (initialize_inlined_parameters): Do not call
	lang_hooks.tree_inlining.convert_parm_for_inlining.
	* cgraphbuild.c (initialize_inline_failed): Set inline failed
	reason for mismatched types.
	* gimplify.c (gimplify_call_expr): Verify the call expression
	arguments match the called function type signature.  Otherwise
	mark the call expression to be not considered for inlining
	using CALL_CANNOT_INLINE_P flag.
	* ipa-inline.c (cgraph_mark_inline): Honor CALL_CANNOT_INLINE_P on the
	edges call expression.
	(cgraph_decide_inlining_of_small_function): Likewise.
	(cgraph_decide_inlining): Likewise.
	* c-objc-common.h (LANG_HOOKS_TREE_INLINING_CONVERT_PARM_FOR_INLINING):
	Remove define.
	* c-tree.h (c_convert_parm_for_inlining): Remove declaration.
	* c-typeck.c (c_convert_parm_for_inlining): Remove.
	* langhooks-def.h (lhd_tree_inlining_convert_parm_for_inlining):
	Remove declaration.
	(LANG_HOOKS_TREE_INLINING_CONVERT_PARM_FOR_INLINING): Remove define.
	* langhooks.c (lhd_tree_inlining_convert_parm_for_inlining):
	Remove.
	* langhooks.h (struct lang_hooks_for_tree_inlining): Remove
	convert_parm_for_inlining member.

	* gcc.dg/pr29254.c: The warning is bogus.
	* gcc.dg/warn-1.c: Likewise.
	* gcc.dg/assign-warn-3.c: Likewise.
	* gcc.dg/noncompile/pr16876.c: The testcase is bogus, remove.

Removed:
    trunk/gcc/testsuite/gcc.dg/noncompile/pr16876.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-objc-common.h
    trunk/gcc/c-tree.h
    trunk/gcc/c-typeck.c
    trunk/gcc/cgraphbuild.c
    trunk/gcc/gimplify.c
    trunk/gcc/ipa-inline.c
    trunk/gcc/langhooks-def.h
    trunk/gcc/langhooks.c
    trunk/gcc/langhooks.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/assign-warn-3.c
    trunk/gcc/testsuite/gcc.dg/pr29254.c
    trunk/gcc/testsuite/gcc.dg/warn-1.c
    trunk/gcc/tree-inline.c
    trunk/gcc/tree.h

Comment 15 Richard Biener 2007-06-23 18:18:35 UTC
Fixed on the mainline.
Comment 16 Andrew Pinski 2007-07-22 19:31:49 UTC
*** Bug 32854 has been marked as a duplicate of this bug. ***
Comment 17 Richard Biener 2007-07-26 15:02:07 UTC
Unassigning.
Comment 18 Martin Michlmayr 2007-08-29 11:47:04 UTC
(In reply to comment #17)
> Unassigning.

So you don't intend to fix this for 4.2?  I guess the patch you commited to trunk
is too big for 4.2 but do you think there's some kind of workaround that could
be applied to 4.2?  This bug is quite annoying.
Comment 19 Richard Biener 2007-08-29 12:55:28 UTC
Fixing of the trunk was a side-effect of some major re-structuring.  This won't
go on the branch for sure, no easy way to fix this another way either.
Comment 20 Mark Mitchell 2007-10-09 19:21:13 UTC
Change target milestone to 4.2.3, as 4.2.2 has been released.
Comment 21 Richard Biener 2007-10-22 15:36:55 UTC
*** Bug 33859 has been marked as a duplicate of this bug. ***
Comment 22 Andrew Pinski 2007-11-12 19:47:31 UTC
*** Bug 34076 has been marked as a duplicate of this bug. ***
Comment 23 Alexandre Pereira Nunes 2007-11-12 20:11:27 UTC
(In reply to comment #20)
> Change target milestone to 4.2.3, as 4.2.2 has been released.
> 

Does this means it'll get fixed by 4.2.3, or the 4.2.x series will stay bugged indefinitely?
Comment 24 David Fang 2007-11-12 20:19:48 UTC
Subject: Re:  [4.2 Regression] optimization generates
 warning for casts

> ------- Comment #23 from alexandre dot nunes at gmail dot com  2007-11-12 20:11 -------
> (In reply to comment #20)
>> Change target milestone to 4.2.3, as 4.2.2 has been released.
>>
>
> Does this means it'll get fixed by 4.2.3, or the 4.2.x series will stay bugged
> indefinitely?

Just means the current *intent* is to fix by 4.2.3 (or target milestone). 
A nudge might encourage someone to do it sooner on the 4.2 branch before 
the next release (since 4.2.2 was just out recently).
Comment 25 David Fang 2007-11-12 20:22:57 UTC
Subject: Re:  [4.2 Regression] optimization generates
 warning for casts

> Just means the current *intent* is to fix by 4.2.3 (or target milestone).
> A nudge might encourage someone to do it sooner on the 4.2 branch before
> the next release (since 4.2.2 was just out recently).

... but the audit trail suggests otherwise.  :S
Comment 26 Alexandre Pereira Nunes 2007-11-12 20:40:09 UTC
(In reply to comment #25)
> 
> ... but the audit trail suggests otherwise.  :S
> 

Ok, now I'm confused :-)
Comment 27 David Fang 2007-11-12 20:59:39 UTC
Subject: Re:  [4.2 Regression] optimization generates
 warning for casts

> Ok, now I'm confused :-)

Sorry, see comment #19.
Comment 28 Manuel López-Ibáñez 2007-11-13 01:15:50 UTC
(In reply to comment #26)
> (In reply to comment #25)
> > 
> > ... but the audit trail suggests otherwise.  :S
> > 
> 
> Ok, now I'm confused :-)
> 

Mark does not actually read the full list of messages when changing the target milestone. I think this should be closed as WONTFIX since there is no easy way to fix this for 4.2 but it is fixed in 4.3. As a workaround, you should be able to use -Wno-cast-qual to avoid the warning. 

I don't remember if 4.2 supports -Wno-error=cast-qual. If it does, you can still get the warning as a warning even when using -Werror.
Comment 29 Alexandre Pereira Nunes 2007-11-13 17:35:34 UTC
(In reply to comment #28)
> (In reply to comment #26)
> > (In reply to comment #25)
> [cut]
> 
> Mark does not actually read the full list of messages when changing the target
> milestone. I think this should be closed as WONTFIX since there is no easy way
> to fix this for 4.2 but it is fixed in 4.3. As a workaround, you should be able
> to use -Wno-cast-qual to avoid the warning. 
> 
> I don't remember if 4.2 supports -Wno-error=cast-qual. If it does, you can
> still get the warning as a warning even when using -Werror.
> 

It does, thanks for the tip. I added this to my makefile:

check_gcc = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
# Work-around for stupid gcc 4.2 bug
ifneq ($(findstring Werror,$(WARN)),)
        WARN += $(call check_gcc,-Wno-error=cast-qual,)
endif

... now it compiles fine with -Werror both on gcc 4.1 and 4.2.

It would be nice if this bug would get somehow documented on gcc manual or the main site before gcc 4.2.3 release.
Comment 30 Manuel López-Ibáñez 2007-11-13 17:55:51 UTC
(In reply to comment #29)
> 
> It would be nice if this bug would get somehow documented on gcc manual or the
> main site before gcc 4.2.3 release.

I agree. But I am not sure how this is typically done (I am too newbie). I think if you write a patch [*] for http://gcc.gnu.org/gcc-4.2/changes.html (a new item under Caveats) pointing to http://gcc.gnu.org/PR29478 with you would like (as an user) to know, that may do. Send it to gcc-patches@gcc.gnu.org

If you have any doubts, ask me. Thanks.

[*] Getting the webpages: http://gcc.gnu.org/cvs.html#wwwdocs
    And a bit more of info: http://gcc.gnu.org/contribute.html#webchanges
Comment 31 Andrew Pinski 2008-04-25 05:09:38 UTC
*** Bug 36042 has been marked as a duplicate of this bug. ***
Comment 32 Steve Ellcey 2009-01-15 23:03:55 UTC
*** Bug 30183 has been marked as a duplicate of this bug. ***