This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Ping: [patch] -m128bit-long-double asserts
- From: Stuart Hastings <stuart at apple dot com>
- To: Geoffrey Keating <geoffk at geoffk dot org>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 1 Apr 2005 00:58:30 -0800
- Subject: Re: Ping: [patch] -m128bit-long-double asserts
- References: <C93ECF02-959D-11D9-A2C9-00039392F57C@apple.com> <66F19BE1-A235-11D9-AA86-00039392F57C@apple.com> <m2r7hvgzr8.fsf@greed.local>
On Mar 31, 2005, at 3:30 PM, Geoffrey Keating wrote:
Hi Stuart,
If you look at subreg_offset_representable_p, you'll see that indeed
it does not work properly if the assert does not hold. For instance,
suppose
ymode == SImode (size 4)
xmode == XCmode (size 32)
nregs_xmode == 6
nregs_ymode == 1
offset == 3 + 12
so
y_offset = 3
mode_multiple = 8
nregs_multiple = 6
You'll see that in fact this is not representable, because there is no
register which actually holds that word, but
[if Stuart deletes the troublesome assert]
the routine will
incorrectly return true, saying that it is representable. I expect
that subreg_regno_offset will fail for many cases where the offset
actually is representable.
(Forgive my insertion; your statement seemed a bit ambiguous to me, so
I inferred the meaning shown above.)
If I understood what you suggested, well, O.K., but it would assert the
same way if invoked with a /valid/ offset. The failing assert fires
due to dis-alignment of modes and hard register multiples, and is not
affected by the value of the 'offset' parameter.
Thus, in order to remove that 'assert', you'd have to modify both
subreg_offset_representable_p and subreg_regno_offset to deal with
such cases correctly. There are probably other bugs you'd find too.
O.K., the assert can stay.
This takes me to the second patch, which seems correct as far as it
goes, but needs the corresponding change to XCmode, to prevent exactly
the case above.
O.K., but I don't understand how this fixes your (contrived, but
excellent) example. I tried the GDB equivalent of "(subreg:SI (reg:XC)
15)", and it asserts on the previous line ("gcc_assert ((offset %
GET_MODE_SIZE (ymode)) == 0);").
I'd be grateful if you could explain why it's better to pretend that an
80-bit XFmode datum that is allocated 128 bits of memory should be
accorded four hard SImode registers. There are really only three
(2.5?) SImode datums there; telling subreg_offset_representable_p
there's a fourth seems like misrepresentation to me (?).
I don't know where you came up with your XCmode example; perhaps you
noticed the XCmode datum in the testcase. That's good, but the
original failure was due to something like (recalling from weak memory)
"(subreg:SI (reg:XF) 12)". The XCmode in the testcase seems to be
necessary to recreate the problem, but only distantly; when the assert
fires, XCmode is not evident.
To insure we're talking about the same patch, here's what I thought you
suggested:
Attachment:
i386.h.diffs.txt
Description: Text document
2005-04-01 Stuart Hastings <stuart@apple.com>
* config/i386/i386.h(HARD_REGNO_NREGS): Aware of
-m128bit-long-double.
(This version hasn't been bootstrapped yet...)
Messing about with the debugger, I validated your concerns about
XCmode; ("(subreg:SI (reg:XC) 12)") asserted too. Adding the change
you suggested (patch above), XCmode no longer asserts. Using the
debugger commandline, I noticed it correctly asserts if you ask for an
odd offset (e.g. "(subreg:SI (reg:XF) 5)"), but it's happy with any
offset that's a multiple of the outer mode size (e.g. "(subreg:SI
(reg:XF) 1280)"). I hope this makes sense to somebody.
I have yet to make a thorough study of this routine, but it looks to me
like it was intended to filter out crazy subregs created in the unholy
crucible :-) of the reload phase, tolerate big-endian and little-endian
targets, and as a place to hang some sanity checking (all those
asserts). It returned TRUE for most of the combinations I tried; I was
able to get FALSE from it by asking for very strange things like
"(subreg:HI (reg:XF) 2)".
I need to give up for the night so I can deal with my children in the
morning. Let me know if you like the patch above; even it doesn't
solve everything, it's arguably better than ICE-ing on the testcase as
we do currently.
stuart