Re: Enable EBX for x86 in 32bits PIC code

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2014-08-28 12:28 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2014-08-28 0:19 GMT+04:00 Vladimir Makarov <vmakarov@redhat.com>:
>> On 2014-08-26 5:42 PM, Ilya Enkovich wrote:
>>>
>>> Hi,
>>>
>>> Here is a patch I tried.  I apply it over revision 214215.  Unfortunately
>>> I do not have a small reproducer but the problem can be easily reproduced on
>>> SPEC2000 benchmark 175.vpr.  The problem is in read_arch.c:701 where float
>>> value is compared with float constant 1.0.  It is inlined into read_arch
>>> function and can be easily found in RTL dump of function read_arch as a
>>> float comparison with 1.0 after the first call to strtod function.
>>>
>>> Here is a compilation string I use:
>>>
>>> gcc -m32 -mno-movbe -g3 -fdump-rtl-all-details -O2 -ffast-math
>>> -mfpmath=sse -m32  -march=slm -fPIE -pie -c -o read_arch.o
>>> -DSPEC_CPU2000        read_arch.c
>>>
>>> In my final assembler comparison with 1.0 looks like:
>>>
>>> comiss  .LC11@GOTOFF(%ebp), %xmm0       # 1101  *cmpisf_sse     [length =
>>> 7]
>>>
>>> and %ebp here doesn't have a proper value.
>>>
>>> I'll try to make a smaller reproducer if these instructions don't help.
>>
>>
>> I've managed to reproduce it.  Although it would be better to send the patch
>> as an attachment.
>>
>> The problem is actually in IRA not LRA.  IRA splits pseudo used for PIC.
>> Then in a region when a *new* pseudo used as PIC we rematerialize a constant
>> which transformed in memory addressed through *original* PIC pseudo.
>>
>> To solve the problem we should prevent such splitting and guarantee that PIC
>> pseudo allocnos in different region gets the same hard reg.
>>
>> The following patch should solve the problem.
>>
>
> Thanks for the patch! I'll try it and be back with results.

Seems your patch doesn't cover all cases.  Attached is a modified
patch (with your changes included) and a test where double constant is
wrongly rematerialized.  I also see in ira dump that there is still a
copy of PIC reg created:

Initialization of original PIC reg:
(insn 23 22 24 2 (set (reg:SI 127)
        (reg:SI 3 bx)) test.cc:42 90 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 3 bx)
        (nil)))
...
Copy is created:
(insn 135 37 25 3 (set (reg:SI 138 [127])
        (reg:SI 127)) 90 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 127)
        (nil)))
...
Copy is used:
(insn 119 25 122 3 (set (reg:DF 134)
        (mem/u/c:DF (plus:SI (reg:SI 138 [127])
                (const:SI (unspec:SI [
                            (symbol_ref/u:SI ("*.LC0") [flags 0x2])
                        ] UNSPEC_GOTOFF))) [5  S8 A64])) 128 {*movdf_internal}
     (expr_list:REG_EQUIV (const_double:DF
2.9999999999999997371893933895137251965934410691261292e-4
[0x0.9d495182a99308p-11])
        (nil)))

After reload we have new usage of r127 which is allocated to ecx which
actually does not have any definition in this function at all.

(insn 151 42 44 4 (set (reg:SI 0 ax [147])
        (plus:SI (reg:SI 2 cx [127])
            (const:SI (unspec:SI [
                        (symbol_ref/u:SI ("*.LC0") [flags 0x2])
                    ] UNSPEC_GOTOFF)))) test.cc:44 213 {*leasi}
     (expr_list:REG_EQUAL (symbol_ref/u:SI ("*.LC0") [flags 0x2])
        (nil)))
(insn 44 151 45 4 (set (reg:DF 21 xmm0 [orig:129 D.2450 ] [129])
        (mult:DF (reg:DF 21 xmm0 [orig:128 D.2450 ] [128])
            (mem/u/c:DF (reg:SI 0 ax [147]) [5  S8 A64]))) test.cc:44
790 {*fop_df_comm_sse}
     (expr_list:REG_EQUAL (mult:DF (reg:DF 21 xmm0 [orig:128 D.2450 ] [128])
            (const_double:DF
2.9999999999999997371893933895137251965934410691261292e-4
[0x0.9d495182a99308p-11]))
        (nil)))

Compilation string: g++ -m32 -O2 -mfpmath=sse -fPIE -S test.cc

Thanks,
Ilya

>
> Ilya
>>

Attachment: pie-2014-08-28.patch
Description: Binary data

Attachment: test.cc
Description: Binary data