Re: [PATCH, PR 42025] Really ensuring type compatibilty in IL generated by IPA-SRA

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jan 13, 2010 at 8:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Nov 21, 2009 at 12:37 PM, Martin Jambor <mjambor@suse.cz> wrote:
>> Hi,
>>
>> On Fri, Nov 20, 2009 at 03:59:41PM +0100, Richard Guenther wrote:
>>> On Fri, 20 Nov 2009, Martin Jambor wrote:
>>>
>>> > > This doesn't look correct. ?Above you may end up setting dont_convert
>>> > > for the LHS where you may not fold the VIEW_CONVERT_EXPR you generate
>>> > > (because it might end up being a NOP_EXPR which is invalid on the LHS).
>>> >
>>> > Thanks for the comment, this might indeed probably be a problem.
>>> >
>>> > Is it always invalid on the LHS or only when the argument is not a
>>> > gimple register (for example when it has DECL_GIMPLE_REG_P == 0 which
>>> > complex replacements would currently have)? ?I thought this was only
>>> > problem if the conversion argument was an SSA_NAME.
>>>
>>> A NOP_EXPR on the LHS is always invalid (but was that your question?).
>>> Fold does not distinguish between SSA_NAMEs or bare decls and
>>> happily folds V_C_E<signed int>(unsigned int) to a NOP_EXPR independent
>>> of how the operand looks like (maybe that was it? ;)).
>>
>> Well, I was surprised I could still encounter a NOP_EXPR at this
>> stage in the first place. ?Then I also thought that if I could it would
>> be appropriate wherever a V_C_E would. ?But apparently I was wrong on
>> both accounts. ?I really haven't seen one in months...
>>
>>>
>>> > > A union with a complex signed integer entry and a struct with
>>> > > two unsigned ints and a REALPART_EXPR on it should trigger that
>>> > > (well, if carefully crafted).
>>> >
>>> > The problem may not occur with a union of a complex and a structure
>>> > because the comparison function compare_access_positions we use for
>>> > sorting always prefers gimple register types over aggregates, so the
>>> > selected type of the replacements will always be gimple register type
>>> > (if any is involved and complex numbers are gimple register types).
>>> > Nevertheless, the conversion could still happen if we had a union of
>>> > two incompatible gimple registers of the same size and at least one of
>>> > them was a complex number.
>>> >
>>> > So I was thinking, if conversions of non-gimple registers were ok on
>>> > the left hand side, we could change compare_access_positions to prefer
>>> > complex types over the other gimple types. ?That way, the
>>> > replacement would always have DECL_GIMPLE_REG_P equal to zero.
>>> >
>>> > But I guess it all depends on the first question. ?If conversions on
>>> > the LHS are a problem even for non-gimple registers, I will probably
>>> > disable IPA-SRA for parameters which have an access group with a
>>> > partial lhs access and multiple different types.
>>>
>>> That sounds like overkill. ?Why not avoid folding at all and not keep the
>>> build1 (...) as it was if we process the LHS?
>>>
>>
>> Sure, that is easy to do, a patch (bootstrapped and tested on
>> x86_64-linux) is below.
>>
>> Thanks,
>>
>> Martin
>>
>>
>> 2009-11-20 ?Martin Jambor ?<mjambor@suse.cz>
>>
>> ? ? ? ?PR middle-end/42025
>> ? ? ? ?* tree-sra.c (access_precludes_ipa_sra_p): New function.
>> ? ? ? ?(splice_param_accesses): Check all accesses by calling
>> ? ? ? ?access_precludes_ipa_sra_p.
>> ? ? ? ?(sra_ipa_modify_expr): Rename argument erite to dont_convert and do
>> ? ? ? ?not convert types if it is true.
>> ? ? ? ?(sra_ipa_modify_assign): Convert types in case of mismatch.
>>
>> ? ? ? ?* testsuite/gcc.c-torture/compile/pr42025-1.c: New test.
>> ? ? ? ?* testsuite/gcc.c-torture/compile/pr42025-2.c: New test.
>>
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42714
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46734


-- 
H.J.