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 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

H.J.