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, 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


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