Re: Small fix for walking constructors

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Sep 22, 2014 at 11:00 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Sep 22, 2014 at 10:58 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Thu, Sep 18, 2014 at 10:38 PM, Jeff Law <law@redhat.com> wrote:
>>> On 09/18/14 13:01, Bernd Schmidt wrote:
>>>>
>>>> This fixes an issue on ptx where we fail to output a declaration for a
>>>> variable. The testcase is c-torture/compile/pr34856.c, and the cause of
>>>> the problem is that the variable g is never inserted into the varpool,
>>>> which is where a future patch will look for references to variables not
>>>> defined in the current translation unit (ptx assembly requires
>>>> declarations for these too).
>>>>
>>>> Bootstrapped and tested on x86_64-linux, ok?
>>>>
>>>>
>>>> Bernd
>>>>
>>>> walk-more.diff
>>>>
>>>>
>>>> commit 968a508fdd5c413147b9c26d37633bf7ab7a7e65
>>>> Author: Bernd Schmidt<bernds@codesourcery.com>
>>>> Date:   Thu Sep 11 14:35:01 2014 +0200
>>>>
>>>>      Fix handling of CONSTRUCTORs in gimple-walk.
>>>>
>>>>         * gimple-walk.c (walk_stmt_load_store_addr_ops): Look past casts
>>>> when
>>>>         dealing with CONSTRUCTORs.
>>>
>>> OK.
>>
>> Errr - certainly not.
>>
>> It seems to me that walk_stmt_load_store_addr_ops is called on
>> bogus input.  The function is supposed to be called on GIMPLE
>> stmts and in GIMPLE stmts CONSTRUCTORs may _not_ have
>> conversions in their elements.
>>
>> Please revert if you have applied already.
>
> For the testcase I can indeed see
>
>
>   <bb 2>:
>   pin_3 = {(unsigned int) (long int) &g[16]};
>
> but that's invalid GIMPLE, unfortunately not caught by out checker.
>
> Please fix the root cause and add checking to verify_gimple_assign_single.

I'm on it.  The reason for the invalid GIMPLE is the gimplifier which
says "well, looks like a constant for the target!" and doesn't gimplify
at all then.  Oops (but only for vectors?!).

Introduced by r118747 and only semi-fixed by r129739.  The original
rev. is also just a bugfix for an ICE.

Thus I am testing the following.

2014-09-22  Richard Biener  <rguenther@suse.de>

        * gimplify.c (gimplify_init_constructor): Do not leave
        non-GIMPLE vector constructors around.
        * tree-cfg.c (verify_gimple_assign_single): Verify that
        CONSTRUCTORs have gimple elements.


Richard.






> Thanks,
> Richard.
>
>
>
>>
>> Thanks,
>> Richard.
>>
>>> Jeff
>>>

Attachment: p
Description: Binary data