Bug 12953 - tree inline bug and fix
Summary: tree inline bug and fix
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.3.2
: P2 normal
Target Milestone: 3.3.3
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization, patch
Depends on:
Blocks:
 
Reported: 2003-11-07 22:01 UTC by Alexey Starovoytov
Modified: 2003-11-12 18:57 UTC (History)
1 user (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2003-11-07 22:50:58


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Starovoytov 2003-11-07 22:01:55 UTC
Hi,

I believe there is a bug in the function "inline_forbidden_p"
in the file "c-objc-common.c":

  switch (TREE_CODE (node))
    {
    case CALL_EXPR:
      t = get_callee_fndecl (node);

      if (! t)
        break;

      /* We cannot inline functions that call setjmp.  */
      if (setjmp_call_p (t))
        return node;

      switch (DECL_FUNCTION_CODE (t))
        {
          /* We cannot inline functions that take a variable number of
             arguments.  */
        case BUILT_IN_VA_START:
        case BUILT_IN_STDARG_START:

It is possible that function declaration 't' has a non-zero
DECL_FUNCTION_CODE (t) field and 't' is not a BUILT_IN function.
(In other words t->decl.u1.f != 0 and t->decl.built_in_class == 0)

Therefore during tree inline optimization some regular functions may not
be inlined due to incorrect check for BUILT_IN_*
So the result of inline optimization can be different depending on what
platform you running and what flags were used to build GCC compiler,
though generated code will be correct.

The possible fix could be:
  switch (TREE_CODE (node))
    {
    case CALL_EXPR:
      t = get_callee_fndecl (node);

      if (! t)
        break;

      /* We cannot inline functions that call setjmp.  */
      if (setjmp_call_p (t))
        return node;

      if (DECL_BUILT_IN (t))
        switch (DECL_FUNCTION_CODE (t))
          {
            /* We cannot inline functions that take a variable number of
               arguments.  */
          case BUILT_IN_VA_START:
          case BUILT_IN_STDARG_START:

Regards,
Alexey.


*** c-objc-common.c	Fri May  2 12:52:02 2003
--- c-objc-common.c.new	Mon Nov  3 18:44:28 2003
*************** inline_forbidden_p (nodep, walk_subtrees
*** 88,109 ****
        if (setjmp_call_p (t))
  	return node;

!       switch (DECL_FUNCTION_CODE (t))
! 	{
! 	  /* We cannot inline functions that take a variable number of
! 	     arguments.  */
! 	case BUILT_IN_VA_START:
! 	case BUILT_IN_STDARG_START:
  #if 0
! 	  /* Functions that need information about the address of the
!              caller can't (shouldn't?) be inlined.  */
! 	case BUILT_IN_RETURN_ADDRESS:
  #endif
! 	  return node;

! 	default:
! 	  break;
! 	}

        break;

--- 88,110 ----
        if (setjmp_call_p (t))
  	return node;

!       if (DECL_BUILT_IN (t))
!         switch (DECL_FUNCTION_CODE (t))
! 	  {
! 	    /* We cannot inline functions that take a variable number of
! 	       arguments.  */
! 	  case BUILT_IN_VA_START:
! 	  case BUILT_IN_STDARG_START:
  #if 0
! 	    /* Functions that need information about the address of the
!                caller can't (shouldn't?) be inlined.  */
! 	  case BUILT_IN_RETURN_ADDRESS:
  #endif
! 	    return node;

! 	  default:
! 	    break;
! 	  }

        break;
Comment 1 Andrew Pinski 2003-11-07 22:50:55 UTC
Patch was posted here: <http://gcc.gnu.org/ml/gcc-patches/2003-11/msg00199.html>.
Now note that the the i member of the union is used in the mainline (3.4) for estimating the 
number of insns that function has so that the value of DECL_FUNCTION_CODE will be the higher (or 
lower if little endian) (if HOST_WIDE_INT is 64 bits and "enum built_in_function" is 32 bits) so it still 
depends on the host machine.  I want to say this is an obovious fix, and I will apply it if noone else 
does.
Comment 2 GCC Commits 2003-11-12 18:13:02 UTC
Subject: Bug 12953

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	sayle@gcc.gnu.org	2003-11-12 18:12:58

Modified files:
	gcc            : ChangeLog tree-inline.c 

Log message:
	2003-11-12  Alexey Starovoytov  <alexey.starovoytov@sun.com>
	Roger Sayle  <roger@eyesopen.com>
	
	PR optimization/12953
	* tree-inline.c (inline_forbidden_p_1): Added check for BUILT_IN
	before switch by FUNCTION_CODE.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.1725&r2=2.1726
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-inline.c.diff?cvsroot=gcc&r1=1.85&r2=1.86

Comment 3 Andrew Pinski 2003-11-12 18:20:48 UTC
Fixed for 3.4.
Comment 4 GCC Commits 2003-11-12 18:33:30 UTC
Subject: Bug 12953

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	sayle@gcc.gnu.org	2003-11-12 18:33:24

Modified files:
	gcc            : ChangeLog c-objc-common.c 

Log message:
	2003-11-12  Alexey Starovoytov  <alexey.starovoytov@sun.com>
	
	PR optimization/12953
	* c-objc-common.c (inline_forbidden_p): Added check for BUILT_IN
	before switch by FUNCTION_CODE.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.803&r2=1.16114.2.804
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-objc-common.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.18.4.2&r2=1.18.4.3

Comment 5 Andrew Pinski 2003-11-12 18:38:38 UTC
Fixed in 3.3.3 also.
Comment 6 Andrew Pinski 2003-11-12 18:57:42 UTC
forgot to close as fixed.