llvm dev - Sep 2006 - [LLVMdev] gfortran: array constructor problems

Hi, in order to get a handle on the questions in Chris's previous
email, I rebuilt LLVM with debugging info, and then rebuilt gcc4 with
CHECKING_ENABLED.

In the process, I ran into an assertion error when compiling the first
part of libgfortran:

../../src/gcc/llvm-convert.cpp:3871: failed assertion
`(TREE_CONSTANT(exp) || TREE_CODE(exp) == STRING_CST) && "Isn't
a
constant!"'

In this case, TreeConstantToLLVM::Convert() is getting a constant to
convert that fails the test "TREE_CONSTANT(exp)"
(from my gdb session:)
>Breakpoint 2, TreeConstantToLLVM::Convert (exp=0x45e48c60) at
../../src/gcc/llvm-convert.cpp:3868
>2: exp->common.constant_flag = 0
>1: exp->common.code = CONSTRUCTOR
>From the backtrace the problem,  calls ConvertArrayCONSTRUCTOR:
(gdb) bt
#0  0x9004802c in kill ()
#1  0x9012dfb4 in abort ()
#2  0x94af90b0 in __eprintf ()
#3  0x0029450c in TreeConstantToLLVM::Convert (exp=0x0) at
../../src/gcc/llvm-convert.cpp:3870
#4  0x00295d80 in TreeConstantToLLVM::ConvertArrayCONSTRUCTOR
(exp=0x45e48d20) at ../../src/gcc/llvm-convert.cpp:4083
#5  0x0028b624 in emit_global_to_llvm (decl=0x45e4c700) at
../../src/gcc/llvm-backend.cpp:377
#6  0x00284844 in rest_of_decl_compilation (decl=0x45e4c700,
top_level=0, at_end=0) at ../../src/gcc/passes.c:252
#7  0x002a7da8 in TreeToLLVM::EmitBIND_EXPR (this=0xbfffe63c,
exp=0x45e4d5a0, DestLoc=0x0) at ../../src/gcc/llvm-convert.cpp:1119
#8  0x00297ac0 in TreeToLLVM::Emit (this=0xbfffe63c, exp=0x45e4d5a0,
DestLoc=0x0) at ../../src/gcc/llvm-convert.cpp:474
#9  0x002882f0 in llvm_emit_code_for_current_function
(fndecl=0x45e4ae00) at ../../src/gcc/llvm-backend.cpp:309

It is compiling libgfortran/intrinsics/selected_int_kind.f90, which
has only one array constructor, shown below. (this part is actually
generated at build time as selected_int_kind.inc)

 type :: int_info
    integer :: kind
    integer :: range
  end type int_info
  integer, parameter :: c = 4
  type (int_info), parameter :: int_infos(c) = (/ &
    int_info (1, range(0_1)), &
    int_info (2, range(0_2)), &
    int_info (4, range(0_4)), &
    int_info (8, range(0_8)) /)

This is trying to declare a constant array of int_info structures.
range() is an intrinsic function that returns the expoent range of the
argument's type kind. I haven't quite been able to figure out where
that gets evaluated, but I am pretty sure it should be a constant at
compile time.

Here's the output of debug_tree() for the expression that breaks the
assertion:

Breakpoint 2, TreeConstantToLLVM::Convert (exp=0x45e48c60) at
../../src/gcc/llvm-convert.cpp:3868
2: exp->common.constant_flag = 0
1: debug_tree (exp) =  <constructor 0x45e48c60
    type <record_type 0x45e4c180 int_info BLK
        size <integer_cst 0x45e23870 constant invariant 64>
        unit size <integer_cst 0x45e238a0 constant invariant 8>
        align 32 symtab 1183925312 alias set -1
        fields <field_decl 0x45e4c200 kind type <integer_type 0x45e28480
int4>
            asm-frame-size 0 SI file
/Users/mike/Documents/hpcl/LLVM/fortran/gcc4/src/libgfortran/intrinsics/selected_int_kind.f90
line 22
            size <integer_cst 0x45e23600 constant invariant 32>
            unit size <integer_cst 0x45e23120 constant invariant 4>
            align 32 offset_align 128
            offset <integer_cst 0x45e23150 constant invariant 0>
            bit offset <integer_cst 0x45e23c60 constant invariant 0>
context <record_type 0x45e4c180 int_info> arguments <integer_cst
0x45e23150 0>
            LLVM: uint 0 chain <field_decl 0x45e4c280 range>>
        pointer_to_this <pointer_type 0x45e4c580> chain <type_decl
0x45e4c300 D.269>>

    arg 0 <tree_list 0x45e49840 purpose <field_decl 0x45e4c200 kind>
        value <integer_cst 0x45e23db0 constant invariant 1>
        chain <tree_list 0x45e49860 purpose <field_decl 0x45e4c280
range> value <integer_cst 0x45e23db0 1>>>>

I'm having a hard time mapping this output to what I expected from the
structure, but that may be my inexperience with gcc internals.

For how to fix this, it seems like the problem here is in
gfc_conv_array_initializer() (trans-array.c:2905) , where
gfc_conv_structure() is called for EXPR_STRUCTURES in an array
initializer, even if it should be constant, and gfc_conv_structure()
doesn't set the constant_flag of the expression. My first guess is
that we might need to either relax the assertion (not sure what
problems that would cause), find a way to communicate that the
structure is intended to be contained in a constant array, or set the
structure as constant after we get it back in
gfc_conv_array_initializer()...

I'm not entirely sure how to proceed here, so any advice would be
appreciated.

Thanks,
-mike

-- 
Michael McCracken
UCSD CSE PhD Candidate
research: http://www.cse.ucsd.edu/~mmccrack/
misc: http://michael-mccracken.net/wp/

On Tue, 5 Sep 2006, Michael McCracken wrote:
> Hi, in order to get a handle on the questions in Chris's previous
> email, I rebuilt LLVM with debugging info, and then rebuilt gcc4 with
> CHECKING_ENABLED.
>
> In the process, I ran into an assertion error when compiling the first
> part of libgfortran:
ok.
> ../../src/gcc/llvm-convert.cpp:3871: failed assertion
> `(TREE_CONSTANT(exp) || TREE_CODE(exp) == STRING_CST) &&
"Isn't a
> constant!"'
>
> In this case, TreeConstantToLLVM::Convert() is getting a constant to
> convert that fails the test "TREE_CONSTANT(exp)"
> (from my gdb session:)
..
> I'm not entirely sure how to proceed here, so any advice would be
appreciated.
Thanks for the details.

This looks like a typical case of "gcc being sloppy with the
'tree's it is
generating".  I hit another case like this in the C++ front-end, which I 
fixed there (and committed a patch to mainline GCC).  It's preferable to 
find the offending code in the fortran frontend and add the 
'TREE_CONSTANT(exp) = 1' line where appropriate.  I don't know if
you feel
up to that, but it is the preferred solution.  It is possible that this is 
fixed in the GCC mainline fortran frontend, checking there might be a 
start.  It's also quite possible it isn't fixed, as GCC never does the 
equivalent assert so there may have been no reason for them to 
discover/fix this.

If you don't feel up to that, I'd be ok relaxing the assert, assuming
that
the generated code is in fact correct.

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

On 9/6/06, Chris Lattner <sabre at nondot.org>
wrote:
> On Tue, 5 Sep 2006, Michael McCracken wrote:
[snip]
> > ../../src/gcc/llvm-convert.cpp:3871: failed assertion
> > `(TREE_CONSTANT(exp) || TREE_CODE(exp) == STRING_CST) &&
"Isn't a
> > constant!"'
> >
> > In this case, TreeConstantToLLVM::Convert() is getting a constant to
> > convert that fails the test "TREE_CONSTANT(exp)"
> > (from my gdb session:)
> ..
> > I'm not entirely sure how to proceed here, so any advice would be
appreciated.
>
> Thanks for the details.
>
> This looks like a typical case of "gcc being sloppy with the
'tree's it is
> generating".  I hit another case like this in the C++ front-end, which
I
> fixed there (and committed a patch to mainline GCC).  It's preferable
to
> find the offending code in the fortran frontend and add the
> 'TREE_CONSTANT(exp) = 1' line where appropriate.  I don't know
if you feel
> up to that, but it is the preferred solution.  It is possible that this is
> fixed in the GCC mainline fortran frontend, checking there might be a
> start.  It's also quite possible it isn't fixed, as GCC never does
the
> equivalent assert so there may have been no reason for them to
> discover/fix this.
>
> If you don't feel up to that, I'd be ok relaxing the assert,
assuming that
> the generated code is in fact correct.
Here's a patch that fixes the error - if a constant array was being
initialized as a list of structures, the 'tree's generated for the
structures weren't being set as constant, even though the overall tree
was. This caused a problem when being translated into LLVM.

This works on my test case, which looked like this:

type :: foo
integer :: a
integer :: b
end type foo
type (foo), parameter :: A(2) = (/ foo(1,2), foo(2,4) /)

There is at least one other point in gfc_conv_array_initializer()
where they build tree lists of structures using gfc_conv_structure().
That part should probably set each tree's constant flag as well,
probably on line 2843. However, I couldn't test that change, since the
only way I could think of to get to that code path will crash gfortran
somewhere else, and I didn't want to work on that just yet.

To crash gfortran like that (it crashes a stock install gfortran on my
linux box also), do this, with the same type foo:

type (foo), parameter :: A(2) = foo(1,2)

This should (and does with the IBM xlF compiler) generate an array
with two identical structures, but actually just runs forever on its
own and crashes under gdb.

I've included two patches - the first one only fixes the case I could
test, and the second one fixes both locations, because I'm pretty sure
that's going to be necessary eventually. I'm not sure when I'll get
to
testing the structure-copying construction.


Index: gcc/fortran/trans-array.c
==================================================================---
gcc/fortran/trans-array.c (revision 160)
+++ gcc/fortran/trans-array.c (working copy)
@@ -2903,6 +2903,7 @@

       case EXPR_STRUCTURE:
               gfc_conv_structure (&se, c->expr, 1);
+        TREE_CONSTANT (se.expr) = 1;
               list = tree_cons (index, se.expr, list);
         break;



Here's the second patch:

Index: gcc/fortran/trans-array.c
==================================================================---
gcc/fortran/trans-array.c (revision 160)
+++ gcc/fortran/trans-array.c (working copy)
@@ -2840,6 +2840,7 @@
       /* This will probably eat buckets of memory for large arrays.  */
       while (hi != 0 || lo != 0)
         {
+    TREE_CONSTANT(se.expr) = 1;
           list = tree_cons (NULL_TREE, se.expr, list);
           if (lo == 0)
             hi--;
@@ -2903,6 +2904,7 @@

       case EXPR_STRUCTURE:
               gfc_conv_structure (&se, c->expr, 1);
+        TREE_CONSTANT (se.expr) = 1;
               list = tree_cons (index, se.expr, list);
         break;



-- 
Michael McCracken
UCSD CSE PhD Candidate
research: http://www.cse.ucsd.edu/~mmccrack/
misc: http://michael-mccracken.net/wp/