Adding Vladimir.
Ilya
2014-09-25 13:46 GMT+04:00 Ilya Enkovich <enkovich....@gmail.com>:
> 2014-09-25 1:51 GMT+04:00 Ilya Enkovich <enkovich....@gmail.com>:
>> 2014-09-24 23:09 GMT+04:00 Jeff Law <l...@redhat.com>:
>>> On 09/24/14 07:13, Ilya Enkovich wrote:
>>>>
>>>> I tried to generate PARALLEL with all regs set by call. Here is a
>>>> memset call I got:
>>>>
>>>> (call_insn 23 22 24 2 (set (parallel [
>>>> (expr_list:REG_DEP_TRUE (reg:DI 0 ax)
>>>> (const_int 0 [0]))
>>>> (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>>>> (const_int 0 [0]))
>>>> (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>>>> (const_int 0 [0]))
>>>> ])
>>>> (call/j (mem:QI (symbol_ref:DI ("memset") [flags 0x41]
>>>
>>> [ snip ]
>>> Looks good. This is the approved way to handle multiple results of a call.
>>>
>>>>
>>>> During register allocation LRA generated a weird move instruction:
>>>>
>>>> (insn 63 0 0 (set (reg/f:DI 100)
>>>> (parallel [
>>>> (expr_list:REG_DEP_TRUE (reg:DI 0 ax)
>>>> (const_int 0 [0]))
>>>> (expr_list:REG_DEP_TRUE (reg:BND64 77 bnd0)
>>>> (const_int 0 [0]))
>>>> (expr_list:REG_DEP_TRUE (reg:BND64 78 bnd1)
>>>> (const_int 0 [0]))
>>>> ])) -1
>>>> (nil))
>>>>
>>>> Which caused ICE later in LRA. This move happens because of
>>>> REG_RETURNED (reg/f:DI 100) (see condition in inherit_in_ebb at
>>>> lra-constraints.c:5312). Thus this code in LRA doesn't accept
>>>> PARALLEL dest for calls.
>>>
>>> This is a bug in LRA then. Multiple return values aren't heavily used, so
>>> I'm not surprised that its handling was missed in LRA.
>>>
>>> The question now is how to bundle things together in such a way as to make
>>> it easy for Vlad to reproduce and fix this in LRA.
>>>
>>> Jeff
>>
>> I suppose it should be easy to reproduce using the same test case I
>> use and some speudo patch which adds fake return values (e.g. xmm6 and
>> xmm7) to calls. Will try to make some minimal patch and test Vlad
>> could work with.
>>
>> Ilya
>
> I couldn't reproduce the problem on a small test but chrome build
> shows a lot of errors. Due to the nature of the problem test's size
> shouldn't matter, so I attach patch which emulates situation with
> bounds regs (but uses xmm5 and xmm6 instead of bnd0 and bnd1) with a
> preprocessed chrome file.
>
> I apply patch to revision 215580.
>
> Command to reproduce:
>
>>g++ -O2 -c generated_message_reflection.ii
> ../../third_party/protobuf/src/google/protobuf/generated_message_reflection.cc:
> In member function 'virtual void
> google::protobuf::internal::GeneratedMessageReflection::AddBool(google::protobuf::Message*,
> const google::protobuf::FieldDescriptor*, bool) const':
> ../../third_party/protobuf/src/google/protobuf/generated_message_reflection.cc:726:3910:
> internal compiler error: in lra_set_insn_recog_data, at lra.c:941
> 0xc7b969 lra_set_insn_recog_data(rtx_insn*)
> ../../gcc-pl/gcc/lra.c:939
> 0xc79822 lra_get_insn_recog_data
> ../../gcc-pl/gcc/lra-int.h:473
> 0xc7d426 lra_update_insn_regno_info(rtx_insn*)
> ../../gcc-pl/gcc/lra.c:1600
> 0xc7d690 lra_push_insn_1
> ../../gcc-pl/gcc/lra.c:1653
> 0xc7d6c0 lra_push_insn(rtx_insn*)
> ../../gcc-pl/gcc/lra.c:1661
> 0xc7d7bf push_insns
> ../../gcc-pl/gcc/lra.c:1704
> 0xc7da47 lra_process_new_insns(rtx_insn*, rtx_insn*, rtx_insn*, char const*)
> ../../gcc-pl/gcc/lra.c:1758
> 0xc94c80 inherit_in_ebb
> ../../gcc-pl/gcc/lra-constraints.c:5356
> 0xc9599c lra_inheritance()
> ../../gcc-pl/gcc/lra-constraints.c:5560
> 0xc7e86c lra(_IO_FILE*)
> ../../gcc-pl/gcc/lra.c:2223
> 0xc2eab8 do_reload
> ../../gcc-pl/gcc/ira.c:5311
> 0xc2edfe execute
> ../../gcc-pl/gcc/ira.c:5470
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
> The problem as I see it is that in lra-constraints.c:5352 we do not
> check call dest is actually a register. But probably REG_RETURNED
> shouldn't be applied to such call because it is not clear to which
> return value it applies.
>
> Thanks,
> Ilya
