Thanks for looking at this.
Matthew Fortune <matthew.fort...@imgtec.com> writes:
>> > Hi all,
>> > This caused some testsuite failures on arm:
>> > FAIL: gcc.target/arm/vfp-ldmdbd.c scan-assembler fldmdbd
>> > FAIL: gcc.target/arm/vfp-ldmdbs.c scan-assembler fldmdbs
>> > FAIL: gcc.target/arm/vfp-ldmiad.c scan-assembler fldmiad
>> > FAIL: gcc.target/arm/vfp-ldmias.c scan-assembler fldmias
>> >
>> > From the vfp-ldmdbd.c test this patch changed the codegen from:
>> > fldmdbd r5!, {d7}
>> >
>> > into
>> > sub r5, r5, #8
>> > fldd d7, [r5]
>> >
>> > Which broke the test.
>>
>> Sorry for the breakage. I've reverted the patch for now and will send a
>> fixed version when I have time.
>
> The problem appears to lie with the new satisfies_memory_constraint_p
> which is passing just the address to valid_address_p but the constraint
> is a memory constraint (which wants the mem to be retained).
Yeah.
> One option is to re-construct the MEM using the address_info provided
> to valid_address_p. This has mode, address space and whether it was
> actually a MEM to start with so there is enough information.
We don't want to create garbage rtl though. The substitution happens
in-place, so the routine does temporarily turn the original MEM into the
eliminated form.
My first reaction after seeing the bug was to pass the operand in as
another parameter to valid_address_p. That made the interface a bit
ridiculous though, so I didn't post it and reverted the original patch
instead.
I think a cleaner way of doing it would be to have helper functions
that switch in and out of the eliminated form, storing the old form
in fields of a new structure (either separate from address_info,
or a local inheritance of it). We probably also want to have arrays
of address_infos, one for each operand, so that we don't analyse the
same address too many times during the same insn.
> Another issue noticed while looking at this:
> process_address does not seem to make an attempt to check for
> EXTRA_MEMORY_CONSTRAINT even though it does decompose memory addresses.
> For the lea address case then the address is checked with valid_address_p.
> This seems inconsistent, is it intentional?
Yeah, I think so. Memory constraints are different in two main ways:
(a) it's obvious from the MEM what the mode and address space of the
access actually are. legitimate_address_p is all about the most
general address, so in practice no memory constraint should rely
on accepting more than legitimate_address_p does.
(b) it's useful for one pattern to have several alternatives with
different memory constraints (e.g. "ZS", "ZT" and "m" in the 32-bit
MIPS move patterns). There isn't really anything special about the
first alternative.
IMO it'd be good to get rid of this special handling for extra address
constraints, but I've no idea how easy that would be.
Thanks,
Richard
