[LLVMdev] gfortran: array constructor problems

[Michael McCracken]

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/