This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR target/70155: Use SSE for TImode load/store


On Mon, Apr 25, 2016 at 8:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Apr 25, 2016 at 8:10 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Apr 25, 2016 at 4:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Mon, Apr 25, 2016 at 7:18 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> On Mon, Apr 25, 2016 at 2:51 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>> Tested on Linux/x86-64.  OK for trunk?
>>>>
>>>>> +  /* FIXME: Since the CSE pass may change dominance info, which isn't
>>>>> +     expected by the fwprop pass, call free_dominance_info to
>>>>> +     invalidate dominance info.  Otherwise, the fwprop pass may crash
>>>>> +     when dominance info is changed.  */
>>>>> +  if (TARGET_64BIT)
>>>>> +    free_dominance_info (CDI_DOMINATORS);
>>>>> +
>>>>
>>>> Please resolve the above problem first, target-dependent sources are
>>>> not the place to apply band-aids for middle-end problems. The thread
>>>> with the proposed fix died in [1].
>>>>
>>>> [1] https://gcc.gnu.org/ml/gcc/2016-03/msg00143.html
>>>
>>> free_dominance_info (CDI_DOMINATORS) has been called in other
>>> places to avoid this middle-end issue.   I don't know when the middle-end
>>> will be fixed.  I don't think this target optimization should be penalized by
>>> the middle-end issue.
>>
>> Let's ask Richard if he is OK with the workaround...
>>
>> One more thing:
>>
>> @@ -3551,6 +3874,7 @@ convert_scalars_to_vector ()
>>    basic_block bb;
>>    bitmap candidates;
>>    int converted_insns = 0;
>> +  rtx zero, minus_one;
>>
>>    bitmap_obstack_initialize (NULL);
>>    candidates = BITMAP_ALLOC (NULL);
>> @@ -3585,22 +3909,40 @@ convert_scalars_to_vector ()
>>      if (dump_file)
>>        fprintf (dump_file, "There are no candidates for optimization.\n");
>>
>> +  if (TARGET_64BIT)
>> +    {
>> +      zero = gen_reg_rtx (V1TImode);
>> +      minus_one = gen_reg_rtx (V1TImode);
>> +    }
>> +  else
>> +    {
>> +      zero = NULL_RTX;
>> +      minus_one = NULL_RTX;
>> +    }
>> +
>>
>> Do we *really* need to crate registers here? They are only used as a
>> temporary in scalar_chain_64::convert_insn, and I think that any CSE
>> worth its name should find out when the same immediate is loaded to
>> different temporaries.
>
> I will double check. Last time when I tried,  CSE didn't work on this for a
> reason.  For
>
> [hjl@gnu-6 pr67400]$ cat z.i
> extern void bar (void);
>
> void *
> foo (void)
> {
>   return &bar;
> }
> [hjl@gnu-6 pr67400]$ gcc -S -fPIC -O2 z.i
> [hjl@gnu-6 pr67400]$
>
> CSE sees:
>
> (insn 5 2 6 2 (set (reg:DI 89)
>         (mem/u/c:DI (const:DI (unspec:DI [
>                         (symbol_ref:DI ("bar") [flags 0x41]
> <function_decl 0x7f24362151b0 bar>)
>                     ] UNSPEC_GOTPCREL)) [1  S8 A8])) z.i:6 89 {*movdi_internal}
>      (nil))
> (insn 6 5 10 2 (set (reg:DI 87 [ <retval> ])
>         (reg:DI 89)) z.i:6 89 {*movdi_internal}
>      (expr_list:REG_EQUAL (symbol_ref:DI ("bar") [flags 0x41]
> <function_decl 0x7f24362151b0 bar>)
>         (nil)))
>
> CSE will not change it to
>
> (insn 6 5 10 2 (set (reg:DI 89 [ <retval> ])
>         (reg:DI 89)) z.i:6 89 {*movdi_internal}
>      (expr_list:REG_EQUAL (symbol_ref:DI ("bar") [flags 0x41]
> <function_decl 0x7f24362151b0 bar>)
>         (nil)))

I meant CSE won't change it to

insn 5 2 9 2 (set (reg:DI 87 [ <retval> ])
        (symbol_ref:DI ("bar") [flags 0x41]  <function_decl
0x7f7a734971b0 bar>)) z.i:6 89 {*movdi_internal}
     (nil))

>> BTW: I'd really like if Ilya can review the functionality of the
>> patch, he is an expert in this conversion stuff.
>>
>> Uros.
>
> Ilya, can you take a look?
>
> Thanks.
>
> --
> H.J.



-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]