Re: PATCH: RFA reload.c patch

[Eric Christopher <echristo at redhat dot com>]

> This patch doesn't make any sense to me.  If you search for 
> reg_equiv_memory_loc at the top of reload1.c, you will find a comment 
> that says it may not be a valid MEM, and that it goes into either 
> reg_equiv_mem or reg_equiv_address depending on whether it is valid.
> 
> If you look about a hundred lines after the point that you patched, you 
> will see code that does exactly what the comment says.  It passes 
> reg_equiv_memory_loc through strict_memory_address_p, and puts it in 
> reg_equiv_mem if valid, in reg_equiv_address if it looks like an address 
> that reload can fix, and clears it otherwise.
> 

I see where you're talking about now.

> So your patch that clears it earlier on if it doesn't pass 
> memory_address_p doesn't make any sense.
> 

I think I can see this now. However, a couple of things: 1) we check
this up above where I did as well, and 2) we end up using the invalid
address later on in reload.

> Are you sure you aren't trying to cover up a bug in the frv port?  That 
> what it looks like to me.
> 

Pretty sure. I'll explain what's going on here.

> You didn't provide a testcase, and apparently can't because you can't 
> reproduce this in FSF sources, which makes this even more suspect.
> 

Oh I probably could, if I had the new development work we're doing. The
reason that I can't provide the testcase is that it may be encumbered
because it's part of a proprietary conformance testsuite.

> What later pass is assuming that addresses in reg_equiv_memory_loc are 
> legitimate?

In reload.

We start off with:

(set (reg xx)
     (mem:SF (reg yy)))

with a REG_EQUIV on yy to (const_double:SF ....)

and find_reloads_toplev checks reg_equiv_address and
reg_equiv_memory_loc at that location.

Since your mail I did some checking and it looks that reg_equiv_address
isn't verified using strict_address_p, only reg_equiv_mem is.

Anyhow, we then replace

(mem:SF (reg yy))

with

(mem:SF (symbol_ref/u ..LC0))

later in find_reloads_address after a lot of if/else if conditions we
notice that the address isn't valid and send it down through
find_reloads_address_part which immediately tries to force the it via
force_const_mem and since it's a symbol_ref we don't allow that to
happen and end up returning null and segfaulting there.

About the only other thing I can think of is replacing the use of
reg_equiv_address with reg_equiv_mem which seems like it might work, but
I'm not really as sure there.

-eric

-- 
Eric Christopher <echristo@redhat.com>