This is the mail archive of the 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 to gcc/resource.c for PR target/13256, CRIS, SH, reorg,dbr, strict_low_part.

Hans-Peter Nilsson wrote:
	* resource.c (mark_set_resources) <case STRICT_LOW_PART>: Recurse
	if in_dest, and pass in_dest.

The ZERO_EXTRACT and SIGN_EXTRACT cases are handled the same as STRICT_LOW_PART. If STRICT_LOW_PART is wrong, then they must be also.

My reading of the code is that MARK_DEST is used if we only want resources set by the insn, it does not include anything also used by the insn. A strict_low_part is both used and set, and hence is excluded if MARK_DEST. But we need to search a strict_low_part in all other cases, including all !in_dest cases, just in case it includes something volatile. Thus the test appears to be correct to me. We recur in all cases except when both MARK_DEST and in_dest.

As an aside, there still seems to be a flaw here, in that we should search in the MARK_DEST and in_dest case also just in case there is something volatile inside. We can make that work by passing 0 for in_dest in this case. So the code then becomes
if (mark_type == MARK_DEST)
in_dest = 0;
mark_set_resources (... in_dest ...)
for both the zero_extract/sign_extract and strict_low_part cases.

However, I think a simpler solution here is to just drop the MARK_DEST support. There is no place in the compiler that uses MARK_DEST, so we can get rid of it. Once we do that, resource.c goes back to what it looked in in gcc-2.95 and earlier. The strict_low_part case can be deleted, since now the fall through code at the end handles it correctly. We still need the zero_extract/sign_extract, case, because we need to pass 0 for in_dest for operands 1 and 2, but there is no if statement needed here. Likewise for the SET_SRC code in the SET case.

You are right that in_dest has to be passed recursively for the strict_low_part operand. That is the only real bug I see here. And passing mark_type recursively looks reasonable also, though this should have no practical effect.
Jim Wilson, GNU Tools Support,

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