This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch, mips] Fix for PR 54619, GCC aborting with -O -mips16
Steve Ellcey <sellcey@mips.com> writes:
> On Wed, 2012-09-19 at 18:42 +0100, Richard Sandiford wrote:
>> But the documentation says:
>>
>> This hook is never called with an invalid address.
>>
>> Since VOIDmode MEMs aren't valid, I think that should mean it's invalid
>> to call this hook (and rtlanal.c:address_cost) with VOIDmode. I never
>> got time to look at that though. (The culprit in the case I saw was
>> tree-ssa-loop-ivopts.c. Sandra had some improvements in this area,
>> so maybe they would fix this too.)
>>
>> Loading or storing BLKmode doesn't map to any instruction, so I don't
>> think returning true for zero is any better than what we do now.
>>
>> Richard
>
> It looks like address_cost does get called with invalid addresses though
Right, that's what I meant. The hook (and address_cost) are being passed
VOIDmode, even though VOIDmode isn't a legitimate mode for a MEM.
And that's a bug IMO.
> and I think that happens on other platforms then MIPS, it is just that
> MIPS is the only one where the invalid address causes a compile time
> failure. For example, if you compile my test program without -mips16
> the compiler does not abort but address_cost is still called with a
> VOIDmode MEM. What do you think about my patch minus the initial check
> for offset == 0? That would avoid the compile time abort. If we didn't
> want to do that it seems like address_cost should include a check for
> valid addresses before it calls the hook. I am sure that would cause
> failures on many platforms and get some attention.
address_cost already checks memory_address_addr_space_p, and returns
a large value if it's false.
We could avoid the assert by making memory_address_addr_space_p return
false for VOIDmode, but an assert seems better (which I think is
what you're suggesting). Returning 1000 for VOIDmode addresses --
as per the former option -- would certainly avoid the assert,
but wouldn't be very helpful. I think tree-ssa-loop-ivopts is simply
asking for the wrong thing, and needs to be changed. As I say,
Sandra had some fixes in this area.
Richard