On 10/31/23 18:05, Vineet Gupta wrote:
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.
Consider any such fixup pre-approved. I was thinking that the 8/16 bit sub-objects should probably be extended one way or another. It wouldn't make much sense not to.

Not as much of a win as I'd hoped.  But I'll take it

jeff

Reply via email to