[llvm-commits] llvm-gcc4: gimplifier fixes

[Duncan Sands]

> > Backported from gcc-4.3.  These should only effect the Ada front-end.
>... 
> This was part of bigger C++ patch. I am hesitated to bring in partial  
> patch without further testing.
> 
> Please bring in test case and also add "APPLE LOCAL llvm" markers  
> appropriately. Please let us know how you have verified that there is  
> no C++ regression caused by this patch. And why other parts of same  
> bug fix are not required. If this particular part of the patch is  
> necessary for Ada then I'd prefer to bring in entire C++ bug fix after  
> appropriate C++ testing is done.

Hi Devang, you are right that this effects C++ also: in rare cases C++
produces expressions of type address_of_static_constructor.  This currently
causes llvm-gcc4 to abort, at least with the testcase below.  With my patch
it no longer aborts, so the danger is that wrong code may be produced.  Here
is the testcase from gcc PR c++/23171:

p.c:
int *p = (int*)(int[1]){0};

Without my patch:

$ g++ -c -O p.c
llvm-backend.cpp:491: void emit_global_to_llvm(tree_node*): Assertion `((__extension__ ({ const tree __t = ((__extension__ ({ const tree __t = (decl); if (tree_code_type[(int) (((enum tree_code) (__t)->common.code))] != (tcc_declaration)) tree_class_check_failed (__t, (tcc_declaration), "../../gcc.llvm.master/gcc/llvm-backend.cpp", 491, __FUNCTION__); __t; })->decl.initial)); char const __c = tree_code_type[(int) (((enum tree_code) (__t)->common.code))]; if (!((__c) != tcc_type)) tree_class_check_failed (__t, tcc_type, "../../gcc.llvm.master/gcc/llvm-backend.cpp", 491, __FUNCTION__); __t; })->common.constant_flag) || ((enum tree_code) ((__extension__ ({ const tree __t = (decl); if (tree_code_type[(int) (((enum tree_code) (__t)->common.code))] != (tcc_declaration)) tree_class_check_failed (__t, (tcc_declaration), "../../gcc.llvm.master/gcc/llvm-backend.cpp", 491, __FUNCTION__); __t; })->decl.initial))->common.code) == STRING_CST) && "Global initializer should be constant!"' failed.
p.c:0: internal compiler error: Aborted

With my patch this produces the following LLVM:

; ModuleID = 'p.c'
target datalayout = "e-p:32:32"
target endian = little
target pointersize = 32
target triple = "i686-pc-linux-gnu"
%p = global i32* null		; <i32**> [#uses=0]
%llvm.global_ctors = appending global [0 x { i32, void ()* }] zeroinitializer		; <[0 x { i32, void ()* }]*> [#uses=0]

implementation   ; Functions:

I think this is wrong, because the initializer is not static as it should be.
This was a problem with the original gcc patch too (svn revision 109035; equivalent
to the patch I sent), leading to a subsequent much larger patch (svn revision 109075).

So if you consider this bug worth fixing in C++, then both patches need to be backported
(only the patch I sent is needed for Ada, see comments later).

To put some perspective on this, I put a noisy abort statement in this code path and
compiled as much C++ as I had easily available: I bootstrapped the compiler, built LLVM,
ran the testsuite, and built the boost libraries.  The abort was never triggered, so it
seems that this kind of code is rare in C++.

However this construct is often produced by the Ada front-end.  In fact it triggered the
LLVM assert in the very first Ada file compiled when bootstrapping!  So this patch is
needed if you want to get anywhere with Ada.  Historically, additional changes were needed
for the Ada front-end too, however I am working with a backport of the Ada front-end from
gcc 4.3, so those fixes are already in there.  (I'm not using the 4.0 Ada f-e because it
was notoriously weak, producing all kinds of bogus trees).

My preference is to backport the complete patch set.  I have some questions about this
though:
(1) where should the testcase go?  Presumably in the llvm copy of the testsuite;
(2) should the gcc changelog entries be backported too?
(3) how to test?  I have no C++ code that exercises these code paths except for the
testcase above.  I have plenty of Ada but that's useless for the moment since Ada
doesn't build.  I propose that I check that the testcase produces appropriate LLVM,
and check that the changes don't produce any new build failures in the collection of
C++ I build before.

What do you think?

All the best,

Duncan.