Bug 19704

Summary: ICE for tail call of regparm 3 and dllimport
Product: gcc Reporter: Danny Smith <dannysmith>
Component: targetAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: gcc-bugs
Priority: P2 Keywords: ice-on-valid-code, patch
Version: 4.0.0   
Target Milestone: 4.1.0   
Host: i686-pc-mingw32 Target: i686-pc-mingw32
Build: i686-pc-mingw32 Known to work:
Known to fail: Last reconfirmed: 2005-01-30 05:37:07

Description Danny Smith 2005-01-30 04:48:47 UTC
C++ methods that have both dllimport and regparm(3) can result in 
"unable to find a register to spill in class 'CREG'" error message when 
compiled at -O2 or -Os

This causes windows port of Mozilla to fail as reported at:
https://sourceforge.net/tracker/?
func=detail&atid=102435&aid=1112123&group_id=2435

Here is reduced testcase:
// imp_regparm_method.cpp

struct Bar
{ 
  char* data;
  int getlen() const;
  char* getdata() const;
  __declspec(dllimport) void __attribute__ ((regparm(3))) assign(char*, int);
};

struct Foo
{
        Bar foobar;
        void GetData();
};

void Foo::GetData()
{
    foobar.assign(foobar.getdata(), foobar.getlen());
}


g++ -c -Os imp_regparm_method.cpp 

C:\WINNT\Profiles\danny\Desktop>g++-4.0.0  -c -Os imp_regparm_method.cpp 
imp_regparm_method.cpp: In member function 'void Foo::GetData()':
imp_regparm_method.cpp:18: error: unable to find a register to spill in 
class 'CREG'
imp_regparm_method.cpp:18: error: this is the insn:
(call_insn/j:HI 24 23 25 0 (call (mem:QI (reg/f:SI 4 si [62]) [0 S1 A8])
        (const_int 0 [0x0])) 358 {*sibcall_1} (insn_list:REG_DEP_TRUE 20 
(insn_list:REG_DEP_TRUE 21 (insn_list:REG_DEP_TRUE 22 (insn_list:REG_DEP_TRUE 
23 (nil)))))
    (expr_list:REG_DEAD (reg:SI 2 cx [ D.1581 ])
        (expr_list:REG_DEAD (reg:SI 1 dx [ D.1582 ])
            (expr_list:REG_DEAD (reg:SI 0 ax [ D.1580 ])
                (expr_list:REG_DEAD (reg/f:SI 4 si [62])
                    (nil)))))
    (expr_list:REG_DEP_TRUE (use (reg:SI 0 ax [ D.1580 ]))
        (expr_list:REG_DEP_TRUE (use (reg:SI 1 dx [ D.1582 ]))
            (expr_list:REG_DEP_TRUE (use (reg:SI 2 cx [ D.1581 ]))
                (nil)))))
imp_regparm_method.cpp:18: confused by earlier errors, bailing out



Changing regparm(3) to regparm(2) avoids the error.
Removing dllimport attribute  avoids the error.
Making Foo::GetData inline avoids the error.

The failure occurs with GCC 4.0 and 3.4.4.  But not with 3.3.3 nor 2.95.3

Danny
Comment 1 Andrew Pinski 2005-01-30 05:37:07 UTC
Here is the reduced testcase which shows that it has nothing to do with C++ methods at all but just tail 
call and regparm 3 and dllimport:

__declspec(dllimport) void __attribute__ ((regparm(3))) assign(const int *this1, char*, int);
void Foo_GetData(int *this1, char * D1588, int D1587)
{
  assign (this1, D1588, D1587);
}
Comment 2 Andrew Pinski 2005-01-30 19:55:52 UTC
Patch here: <http://gcc.gnu.org/ml/gcc-patches/2005-01/msg02213.html>.
Comment 3 GCC Commits 2005-01-31 05:49:31 UTC
Subject: Bug 19704

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	dannysmith@gcc.gnu.org	2005-01-31 05:49:16

Modified files:
	gcc            : ChangeLog 
	gcc/config/i386: i386.c 

Log message:
	PR target/19704
	* config/i386/i386.c (ix86_function_ok_for_sibcall):  Also check
	that dllimport'd functions do not use all call-clobbered registers
	to pass parameters.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7335&r2=2.7336
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&r1=1.793&r2=1.794

Comment 4 Danny Smith 2005-01-31 05:55:16 UTC
Fixed on mainline.
Should this also be applied be applied to 3_4-barnch (assuming regtesting is 
OK), where the bug also exists?

Danny
Comment 5 Danny Smith 2005-05-16 07:52:35 UTC
The patch in comment #3 isn't sufficient for cases where C++ class members get 
their dllimport status from attribute applied to class type, rather than from 
explicit attribute on the member.  This still fails with same error as original 
testcase:

// dllimport attribute applied to class type causes non-inline class members to 
// use dllimport indirect reference semantics

struct __declspec(dllimport)  Bar { 
  char* data;
  int getlen() const;
  char* getdata() const;
  void __attribute__ ((regparm(3))) assign(char*, int);  
};

struct Foo
{
        Bar foobar;
        void GetData();
};

void Foo::GetData()
{
    foobar.assign(foobar.getdata(), foobar.getlen());
}

Comment 6 GCC Commits 2005-10-12 20:54:58 UTC
Subject: Bug 19704

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	dannysmith@gcc.gnu.org	2005-10-12 20:54:50

Modified files:
	gcc            : ChangeLog config.gcc tree.c tree.h varasm.c 
	                 target.h target-def.h 
	gcc/doc        : tm.texi 
	gcc/config/i386: i386-protos.h i386.c winnt.c cygming.h 
	                 t-cygming 
	gcc/cp         : ChangeLog class.c 
	gcc/testsuite  : ChangeLog 
	gcc/testsuite/gcc.dg: dll-2.c dll-3.c dll-4.c 
	gcc/testsuite/g++.dg/ext: dllimport1.C dllimport2.C dllimport3.C 
	                          dllimport7.C dllimport8.C dllimport9.C 
Added files:
	gcc/config/i386: winnt-stubs.c winnt-cxx.c 

Log message:
	PR middle-end/21275
	PR middle-end/21766
	* target.h (struct gcc_target): Add valid_dllimport_attribute_p
	target hook.
	(struct cxx): Add adjust_class_at_definition target hook.
	* target-def.h: (TARGET_VALID_DLLIMPORT_ATTRIBUTE_P): New define,
	defaulting to hook_bool_tree_true. Add to TARGET_INITIALIZER
	(TARGET_CXX_ADJUST_CLASS_AT_DEFINITION): New define, defaulting to
	hook_void_tree. Add to TARGET_CXX.
	* tree.h (struct decl_with_vis): Rename non_addr_const_p field to
	dllimport_flag.
	(DECL_NON_ADDR_CONSTANT_P): Replace with DECL_DLLIMPORT_P macro.
	* tree.c (merge_dllimport_decl_attributes): Check DECL_DLLIMPORT_P
	instead of attribute. Check for dllexport override.  Warn if
	inconsistent dll linkage. Don't lose old dllimport if decl has
	had address referenced.   Tweak lookup of dllimport atribute.
	(handle_dll_attribute): Check targetm.valid_dllimport_attribute_p
	for target specific rules.  Don't add dllimport attribute if
	DECL_DECLARED_INLINE_P.  Set DECL_DLLIMPORT_P when adding
	dllimport attribute.
	(staticp): Replace DECL_NON_ADDR_CONSTANT_P with DECL_DLLIMPORT_P.
	* varasm.c (initializer_constant_valid_p): Replace
	DECL_NON_ADDR_CONSTANT_P with DECL_DLLIMPORT_P
	
	PR target/21801
	PR target/23589
	* config.gcc (i[34567]86-*-cygwin*): Add winnt-cxx.o to
	'cxx_target_objs', winnt-stubs,o to 'extra_objs'.
	(i[34567]86-*-mingw32*): Likewise.
	
	* doc/tm.texi (TARGET_CXX_ADJUST_CLASS_AT_DEFINITION): Document.
	(TARGET_VALID_DLLIMPORT_ATTRIBUTE_P): Document.
	
	* config/i386/winnt.c (i386_pe_dllimport_p): Factor out
	C++-specific code. Change return value to bool.
	(i386_pe_dllimport_p): Likewise.
	(associated_type): Simplify and make language-independent
	(i386_pe_encode_section_info): Replace override of ambiguous
	dllimport symbol refs with a gcc_assert.
	(i386_pe_valid_dllimport_attribute_p): Define.
	* config/i386/winnt-cxx.c: New file. Define C++ versions of
	i386_pe_type_dllimport_p, i386_pe_type_dllexport_p,
	i386_pe_adjust_class_at_definition.
	* config/i386/winnt-stubs.c: New file. Define stub versions of
	lang-specific functions.
	* config/i386/i386-protos.h: Declare winnt-[cxx|stubs].c functions
	i386_pe_type_dllimport_p, i386_pe_type_dllexport_p,
	i386_pe_adjust_class_at_definition.
	(i386_pe_valid_dllimport_attribute_p): Declare.
	* config/i386/cygming.h (TARGET_VALID_DLLIMPORT_ATTRIBUTE_P): Define.
	(TARGET_CXX_ADJUST_CLASS_AT_DEFINITION): Define.
	* config/i386/t-cygming: Add rules for winnt-cxx.o, winnt-stubs.o.
	
	PR target/19704
	* config/i386/i386.c (ix86_function_ok_for_sibcall): Replace test for
	dllimport attribute with test of DECL_DLLIMPORT_P.
	
	cp
	PR target/21801
	PR target/23589
	* class.c (finish_struct_1): Call
	targetm.cxx.adjust_class_at_definition.
	
	testsuite
	* gcc.dg/dll-2.c: Add tests for warnings.
	* gcc.dg/dll-3.c: Likewise.
	* gcc.dg/dll-4.c: Likewise.
	
	* g++.dg/ext/dllimport1.C: Adjust tests for warnings.
	* g++.dg/ext/dllimport2.C: Likewise.
	* g++.dg/ext/dllimport3.C: Likewise.
	* g++.dg/ext/dllimport7.C: Likewise.
	* g++.dg/ext/dllimport8.C: Likewise.
	* g++.dg/ext/dllimport9.C: Likewise.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.10149&r2=2.10150
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config.gcc.diff?cvsroot=gcc&r1=1.568&r2=1.569
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree.c.diff?cvsroot=gcc&r1=1.508&r2=1.509
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree.h.diff?cvsroot=gcc&r1=1.757&r2=1.758
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/varasm.c.diff?cvsroot=gcc&r1=1.532&r2=1.533
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target.h.diff?cvsroot=gcc&r1=1.144&r2=1.145
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/target-def.h.diff?cvsroot=gcc&r1=1.133&r2=1.134
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/doc/tm.texi.diff?cvsroot=gcc&r1=1.446&r2=1.447
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/winnt-stubs.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/winnt-cxx.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386-protos.h.diff?cvsroot=gcc&r1=1.147&r2=1.148
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&r1=1.862&r2=1.863
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/winnt.c.diff?cvsroot=gcc&r1=1.84&r2=1.85
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/cygming.h.diff?cvsroot=gcc&r1=1.32&r2=1.33
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/t-cygming.diff?cvsroot=gcc&r1=1.3&r2=1.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4927&r2=1.4928
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/class.c.diff?cvsroot=gcc&r1=1.734&r2=1.735
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.6183&r2=1.6184
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/dll-2.c.diff?cvsroot=gcc&r1=1.7&r2=1.8
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/dll-3.c.diff?cvsroot=gcc&r1=1.5&r2=1.6
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/dll-4.c.diff?cvsroot=gcc&r1=1.5&r2=1.6
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport1.C.diff?cvsroot=gcc&r1=1.3&r2=1.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport2.C.diff?cvsroot=gcc&r1=1.3&r2=1.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport3.C.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport7.C.diff?cvsroot=gcc&r1=1.1&r2=1.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport8.C.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/dllimport9.C.diff?cvsroot=gcc&r1=1.1&r2=1.2

Comment 7 Danny Smith 2005-10-12 21:03:24 UTC
Now fixed on trunk for C++ too.
http://gcc.gnu.org/ml/gcc-cvs/2005-10/msg00474.html
Danny
Comment 8 Andrew Pinski 2005-10-12 21:09:54 UTC
Fixed so closing as such.