On 2/3/25 2:09 AM, Richard Sandiford wrote:
So yeah, I think the first question is why ira_build_conflicts isn't
kicking in for this register or (if it is) why we still get register 0.
So pulling on this thread leads me into the code that sets up
ALLOCNO_WMODE in create_insn_allocnos:
if ((a = ira_curr_regno_allocno_map[regno]) == NULL)
{
a = ira_create_allocno (regno, false, ira_curr_loop_tree_node);
if (outer != NULL && GET_CODE (outer) == SUBREG)
{
machine_mode wmode = GET_MODE (outer);
if (partial_subreg_p (ALLOCNO_WMODE (a), wmode))
ALLOCNO_WMODE (a) = wmode;
}
}
Note how we only set ALLOCNO_MODE only at allocno creation, so it'll
work as intended if and only if the first reference is via a SUBREG.
Huh, yeah, I agree that that looks wrong.
ISTM the fix here is to always do the check and set ALLOCNO_WMODE.
The other bug I see is that we may potentially have paradoxicals in
different modes. ie, on a 32 bit target, we could in theory have a
paradoxical in DI and another in TI. So in addition to pulling that
code out of the conditional so that it executes every time, the
assignment would look like
if (partial_subreg_p (ALLCONO_WMODE (a), wmode)
&& wmode > ALLOCNO_WMODE (a))
ALLOCNO_WMODE (a) = wmode;
Or something along those lines.
Not sure about this part though. The construct:
if (partial_subreg_p (ALLCONO_WMODE (a), wmode))
ALLOCNO_WMODE (a) = wmode;
is effectively:
ALLOCNO_WMODE (a) = MAX_SIZE (ALLOCNO_WMODE (a), wmode);
You're right.
and so already picks the single widest mode, if there is one.
For things like DI vs DF, it will use the existing mode as a tie-breaker.
So ISTM that moving the code out of the "if (... == NULL)" should be
enough on its own.
It is and that's actually the patch that's been in my tester for the
last week or so. The other (non)issue wasn't necessary to fix the
problem, just something that looked odd/wrong as I was preparing the
email and I included it in the discussion.
Jeff
