This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Fix modes in ctz libcall notes
Thanks for the review btw. Forgot to say that earlier.
Paolo Bonzini <bonzini@gnu.org> writes:
>>>>>> - emit_libcall_block (insns, target, value,
>>>>>> - gen_rtx_fmt_e (unoptab->code, outmode, op0));
>>>>>> + eq_value = gen_rtx_fmt_e (unoptab->code, mode, op0);
>>>>>> + if (GET_MODE_SIZE (outmode) < GET_MODE_SIZE (mode))
>>>>>> + eq_value = gen_rtx_TRUNCATE (outmode, eq_value);
>>>>> Why not simplifying this so that a SUBREG is created instead?
>>>> Having SUBREGs of anything other than MEMs and REGs seems less
>>>> than ideal. (For reference, we use TRUNCATEs for a similar
>>>> purpose when doing high-part multiplications.)
>>> My thought was that fwprop or CSE will probably create it anyway
>>> (because they simplify) and I just wondered whether having a more
>>> canonical representation would be better. However, you're right that
>>> the more canonical representation may be less ideal.
>>
>> So you'd like to see rtl_hooks.gen_lowpart_no_emit instead?
>
> Yes, I was thinking simplify_gen_unary but they would have the same
> result. Anyway the patch is fine with me as is, I was just trying to
> understand if there was a particular reason and the similarity with
> multiplication makes sense.
OK, I had a prod. On MIPS, simplify_gen_unary produces a TRUNCATE
anyway because of TRULY_NOOP_TRUNCATION, which isn't really interesting
for the purposes of this discussion. If you hide that, s_g_u still
produces a TRUNCATE because CTZ_DEFINED_AT_ZERO is false.
(This is probably going to be true for most targets that need
libcalls for ctz.) If I force a subreg to be created, we're then
unable to constant-fold the libcall. This is because cse.c:fold_rtx
only folds subregs to constants if equiv_constant recognises it as
a constant (that is, if either (a) the subreg was assigned a
constant or (b) the subreg is of a reg that was assigned a constant).
Things work if we remove the current fold_rtx "case SUBREG:" and replace
it with:
case SUBREG:
new = equiv_constant (x);
if (new != NULL_RTX)
return new;
folded_arg0 = fold_rtx (SUBREG_REG (x), insn);
if (folded_arg0 != SUBREG_REG (x))
{
new = simplify_gen_subreg (GET_MODE (x), folded_arg0,
GET_MODE (SUBREG_REG (x)),
SUBREG_BYTE (x));
if (new != NULL_RTX)
return new;
}
return x;
but I haven't really thought about whether that's the right thing.
It's probably too late for 4.3 anyway.
However, that's more a question about whether using SUBREGs like this
is a good thing or not. I agree we should use canonical forms,
and since it doesn't make any difference for MIPS. I'm happy to do
that in isolation. Is the patch below OK, if it survives testing?
Richard
gcc/
* optabs.c (expand_unop): In libcall notes, give ffs, clz, ctz,
popcount and parity rtxes the same mode as their operand.
Truncate or extend the result to the return value's mode
if necessary.
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c 2008-01-23 13:19:49.000000000 +0000
+++ gcc/optabs.c 2008-01-24 10:30:01.000000000 +0000
@@ -3273,6 +3273,7 @@ expand_unop (enum machine_mode mode, opt
{
rtx insns;
rtx value;
+ rtx eq_value;
enum machine_mode outmode = mode;
/* All of these functions return small values. Thus we choose to
@@ -3292,8 +3293,12 @@ expand_unop (enum machine_mode mode, opt
end_sequence ();
target = gen_reg_rtx (outmode);
- emit_libcall_block (insns, target, value,
- gen_rtx_fmt_e (unoptab->code, outmode, op0));
+ eq_value = gen_rtx_fmt_e (unoptab->code, mode, op0);
+ if (GET_MODE_SIZE (outmode) < GET_MODE_SIZE (mode))
+ eq_value = simplify_gen_unary (TRUNCATE, outmode, eq_value, mode);
+ else if (GET_MODE_SIZE (outmode) > GET_MODE_SIZE (mode))
+ eq_value = simplify_gen_unary (ZERO_EXTEND, outmode, eq_value, mode);
+ emit_libcall_block (insns, target, value, eq_value);
return target;
}