On 10/30/23 13:33, Jeff Law wrote:
+/* Helper function for riscv_extend_comparands to Sign-extend the OP.
+ However if the OP is SI subreg promoted with an inner DI, such as
+ (subreg/s/v:SI (reg/v:DI) 0
+ just peel off the SUBREG to get DI, avoiding extraneous
extension. */
+
+static void
+riscv_sign_extend_if_not_subreg_prom (rtx *op)
+{
+ if (GET_MODE (*op) == SImode
So I may have been partially wrong about v2 patch being wrong and
needing this fixup ;-) [1]
It seems we don't have to limit this to SImode. I re-read the calling
convention doc [2] and it says this
"When passed in registers or on the stack, integer scalars narrower than XLEN
bits are widened according to the sign of their type up to 32 bits, then
sign-extended to XLEN bits."
This essentially means signed short, and signed char will be already
sign-extended at caller site and need not be done in callee: Palmer
mention in internal slack that unadorned char is unsigned on RISC-V
hence we don't see the compiler extra work for say
gcc.dg/torture/pr75964.c. If the test is however tweaked to use signed
chars (or short), it seems caller is doing the work (adjusting the
constant being passed to be a sign-extended variant).
This further validates Jeff's comment about checking for
SUBREG_PROMOTED_SIGNED_P (it was anyhow the right thing to begin with
anyways).
At this point I feel like I'm into splitting hairs (in vain) territory,
as fixing this might not matter much in practice ....
I'd suppose we go ahead with the v3 with changes Jeff asked for and
maybe do a later fixup to relax SI.
+ && GET_CODE (*op) == SUBREG
+ && SUBREG_PROMOTED_VAR_P (*op)
+ && GET_MODE_SIZE (GET_MODE (XEXP (*op, 0))).to_constant ()
+ == GET_MODE_SIZE (word_mode))
+ *op = XEXP (*op, 0);
+ else
+ *op = gen_rtx_SIGN_EXTEND (word_mode, *op);
So for the wrapped test GET_MODE_SIZE stuff), add parenthesis and
indent the "==" clause. ie
&& (GET_MODE_SIZE (GET_MODE (XEXP (*op), 0))).to_constant ()
== GET_MODE_SIZE (word_mode))
Don't you also need to verify that the subreg was sign extended? The
PROMOTED_VAR_P just notes that it was promoted, not *how* it was
promoted. I think you just need to add a test like this:
&& SUBREG_PROMOTED_SIGNED_P (*op)
[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634327.html
[2]
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc
<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc>
