This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, arm] Fix gcc.c-torture/execute/930628-1.c -O3 for Thumb
- From: Adam Nemet <anemet at Lnxw dot COM>
- To: Richard dot Earnshaw at arm dot com
- Cc: Hans-Peter Nilsson <hp at bitrange dot com>, gcc-patches at gcc dot gnu dot org, Nick Clifton <nickc at redhat dot com>, rearnsha at arm dot com
- Date: 31 Oct 2002 00:37:30 -0800
- Subject: Re: [PATCH, arm] Fix gcc.c-torture/execute/930628-1.c -O3 for Thumb
- References: <200210301712.g9UHCJJ05490@pc960.cambridge.arm.com>
> No, it just means you are papering over a deficiency elsewhere in reload.
> LEGITIMIZE_RELOAD_ADDRESS is like LEGITIMIZE_ADDRESS in this respect, it
> *should not be needed* to generate correct code; it's too crude an
> instrument.
I can see the point that LEGITIMIZE_RELOAD_ADDRESS is not the best
place to take care of this issue from a design point of view. I was
only looking at the existing facilities to implement the change and
this was the only one that looked reasonable.
Also, I had the impression from the comment before the original
THUMB_LEGITIMIZE_RELOAD_ADDRESS that the code was already _fixing_
addresses by reloading them (the HImode SP + large_offset_addr case).
I tried to find out more about the origin of this code from the late
thumb.h but I couldn't find the corresponding posting on gcc-patches.
> I've been thinking about this particular case, but the fix is quite
> complex and involves introducing a new macro that allows the index reg
> class to be based both on the mode of access and the class of base
> register in use. Unfortunately I don't have a patch available for this
> yet.
While this might be a good idea in general it won't fix the issue I am
seeing. INDEX_REG_CLASS is already defined as LO_REGS. Yet in
the problematic insn:
(insn:HI 551 710 552 13 0x40128370 (set (mem/s:SF (plus:SI (reg:SI 51)
(reg/f:SI 40)) [14 bdm S4 A32])
(reg:SF 150)) 195 {*thumb_movsf_insn} (nil)
(expr_list:REG_EQUAL (const_double:SF -100663296)
pseudo (reg 40) will be assigned to SP. The reason for this is that
(reg 40) corresponds to the address of an array on the stack which
becomes SP after elimination.
Therefore, I think that if we reject the LEGITIMIZE_RELOAD_ADDRESS
idea we will have to refine the REG*_OK_FOR_BASE_P family and
eventually BASE_REG_CLASS to only allow SP as a base register if the
index is constant.
I am thinking of something along this line:
#define THUMB_REGNO_MODE_OK_FOR_BASE_P(REGNO, MODE, INDEX) \
(TEST_REGNO (REGNO, <=, LAST_LO_REGNUM) \
|| (GET_CODE (INDEX) == CONST_INT \
&& GET_MODE_SIZE (MODE) >= 4 \
&& TEST_REGNO (REGNO, ==, STACK_POINTER_REGNUM)))
If you agree with this approach, I can give it a try and come up with
a working version implementing this idea.
Adam