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 middle-end/43671: [4.4/4.5/4.6 Regression] -fsched2-use-superblocks -m32 produces wrong code with vectorization


On Tue, May 4, 2010 at 4:29 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Mon, May 3, 2010 at 6:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, May 3, 2010 at 12:15 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Sun, May 02, 2010 at 08:59:23PM -0700, H.J. Lu wrote:
>>>> H.J.
>>>> ----
>>>> gcc/
>>>>
>>>> 2010-05-02 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>>
>>>> ? ? ? PR middle-end/43671
>>>> ? ? ? * (get_addr): Don't return an expression if references_value_p
>>>> ? ? ? returns true.
>>>
>>> I don't think this is the right thing to do to fix this bug.
>>> Of course get_addr callers could be smarter and look at both addresses
>>> and don't call get_addr blindly if the same VALUE appears on both sides.
>>> get_addr in the end returns v->locs->loc anyway, and that can of course also
>>> include stuff referencing VALUEs. ?Your change merely makes the problem
>>> latent, nothing else.
>>>
>>> In the single problematic true_dependence call it is better if
>>> get_addr wouldn't be called on both sides, as both sides originally contain
>>> the same VALUE, but there are many other cases where it is desirable and
>>
>> How about this patch?
>
> That looks like a good idea.
>
> - ?x_addr = get_addr (XEXP (x, 0));
> - ?mem_addr = get_addr (XEXP (mem, 0));
> + ?if ((GET_CODE (XEXP (x, 0)) == VALUE
> + ? ? ? && GET_CODE (XEXP (mem, 0)) != VALUE
> + ? ? ? && reg_mentioned_p (XEXP (x, 0), XEXP (mem, 0)))
> + ? ? ?|| (GET_CODE (XEXP (x, 0)) != VALUE
> + ? ? ? ? && GET_CODE (XEXP (mem, 0)) == VALUE
> + ? ? ? ? && reg_mentioned_p (XEXP (mem, 0), XEXP (x, 0))))
> + ? ?{
> + ? ? ?x_addr = XEXP (x, 0);
> + ? ? ?mem_addr = XEXP (mem, 0);
> + ? ?}
> + ?else
> + ? ?{
> + ? ? ?x_addr = get_addr (XEXP (x, 0));
> + ? ? ?mem_addr = get_addr (XEXP (mem, 0));
> + ? ?}
>
> Can you re-structure this as
>
> ?x_addr = XEXP (x, 0);
> ?mem_addr = XEXP (mem, 0);
> ?if (!(GET_CODE (x_addr) == VALUE
> ? ? ?....
> ? {
> ? ? ?x_addr = get_addr (x_addr);
> ? ? ?mem_addr = get_addr (mem_addr);
> ? }
>
> also fix the other callers
> (canon_true_dependence and write_dependence_p)?
>
> Thanks,
> Richard.
>

I am testing this on Linux/x86-64.  OK to install on trunk/4.4/4.5
if there are no regressions?

Thanks.


-- 
H.J.
--gcc/

2010-05-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/43671
	* alias.c (true_dependence): Handle the same VALUE in x and mem.
	(canon_true_dependence): Likewise.
	(write_dependence_p): Likewise.

gcc/testsuite/

2010-05-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/43671
	* gcc.target/i386/pr43671.c: New.

Attachment: gcc-pr43671-3.patch
Description: Text document


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