[PATCH, i386, Pointer Bounds Checker 33/x] MPX ABI

[Ilya Enkovich]

2014-09-25 1:51 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2014-09-24 23:09 GMT+04:00 Jeff Law <law@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fake-return-values.patch
Type: application/octet-stream
Size: 1164 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20140925/34f61a49/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: generated_message_reflection.ii.gz
Type: application/x-gzip
Size: 143912 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20140925/34f61a49/attachment.bin>