On Mon, Apr 25, 2016 at 8:10 AM, Uros Bizjak <[email protected]> wrote:
> On Mon, Apr 25, 2016 at 4:47 PM, H.J. Lu <[email protected]> wrote:
>> On Mon, Apr 25, 2016 at 7:18 AM, Uros Bizjak <[email protected]> wrote:
>>> On Mon, Apr 25, 2016 at 2:51 PM, H.J. Lu <[email protected]> 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)))
> 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.
