https://gcc.gnu.org/g:783e7e1fd46d7c16a80597ed54ea9ec4fc463e29
commit r16-7954-g783e7e1fd46d7c16a80597ed54ea9ec4fc463e29 Author: Richard Sandiford <[email protected]> Date: Mon Mar 9 08:38:31 2026 +0000 Revert two changes in r16-7265-ga9e48eca3a6eef [PR118608] Sorry to be awkward, but I'd like to revert the rtlanal.cc and config/mips/mips.md parts of r16-7265-ga9e48eca3a6eef. I think the expr.cc part of that patch is enough to fix the bug. The other parts seem unnecessary and are likely to regress code quality on MIPS compared to previous releases. (See the testing below for examples.) The rtlanal.cc part added the following code to truncated_to_mode: /* This explicit TRUNCATE may be needed on targets that require MODE to be suitably extended when stored in X. Targets such as mips64 use (sign_extend:DI (truncate:SI (reg:DI x))) to perform an explicit extension, avoiding use of (subreg:SI (reg:DI x)) which is assumed to already be extended. */ scalar_int_mode imode, omode; if (is_a <scalar_int_mode> (mode, &imode) && is_a <scalar_int_mode> (GET_MODE (x), &omode) && targetm.mode_rep_extended (imode, omode) != UNKNOWN) return false; I think this has two problems. The first is that mode_rep_extended describes a canonical form that is obtained by correctly honouring TARGET_TRULY_NOOP_TRUNCATION. It is not an independent restriction on what RTL optimisers can do. If we need to disable an optimisation on MIPS-like targets, the restrictions should be based on TARGET_TRULY_NOOP_TRUNCATION instead. The second problem is that, although the comment treats MIPS-like DI->SI truncation as a special case, truncated_to_mode is specifically written for such cases. The comment above the function says: /* Suppose that truncation from the machine mode of X to MODE is not a no-op. See if there is anything special about X so that we can assume it already contains a truncated value of MODE. */ Thus we're already in the realm of MIPS-like truncations that need TRUNCATE rather than SUBREG (and that in turn guarantee sign-extension in some cases). It's the caller that checks for that condition: && (TRULY_NOOP_TRUNCATION_MODES_P (mode, GET_MODE (op)) || truncated_to_mode (mode, op))) So I think the patch has the effect of disabling exactly the kind of optimisation that truncated_to_mode is supposed to provide. truncated_to_mode makes an implicit assumption that sign-extension is enough to allow a SUBREG to be used in place of a TRUNCATE. This is true for MIPS and was true for the old SH64 port. I don't know whether it's true for gcn and nvptx, although I assume that it must be, since no-one seems to have complained. However, it would not be true for a port that required zero rather than sign extension (which AFAIK we've never had). It's probably worth noting that this assumption is in the opposite direction from what mode_rep_extended describes. mode_rep_extended says that "proper" truncation leads to a guarantee of sign extension. truncated_for_mode assumes that sign extension avoids the need for "proper" truncation. On MIPS, the former is only true for truncation from 64 bits to 32 bits, whereas the latter is true for all cases (such as 64 bits to 16 bits). And that feeds into the mips.md change in r16-7265-ga9e48eca3a6eef. The change was: (define_insn_and_split "*extenddi_truncate<mode>" [(set (match_operand:DI 0 "register_operand" "=d") (sign_extend:DI - (truncate:SHORT (match_operand:DI 1 "register_operand" "d"))))] + (truncate:SUBDI (match_operand:DI 1 "register_operand" "d"))))] "TARGET_64BIT && !TARGET_MIPS16 && !ISA_HAS_EXTS" The old :SHORT pattern existed because QI and HI values are only guaranteed to be sign-extensions of bit 31 of the register, not bits 7 or 15 (respectively). Thus we have the worst of both worlds: (1) truncation from DI is not a nop. It requires a left shift by at least 32 bits and a right shift by the same amount. (2) sign extension to DI is not a nop. It requires a left shift and a right shift in the normal way (by 56 bits for QI and 48 bits for HI). So a separate truncation and extension would yield four shifts. The pattern above exists to reduce this to two shifts, since (2) subsumes (1). But the :SI case is different: (1) truncation from DI is not a nop. It requires a left shift by 32 and a right shift by 32, as above. (2) sign extension from SI to DI is a nop. (2) is implemented by: ;; When TARGET_64BIT, all SImode integer and accumulator registers ;; should already be in sign-extended form (see TARGET_TRULY_NOOP_TRUNCATION ;; and truncdisi2). We can therefore get rid of register->register ;; instructions if we constrain the source to be in the same register as ;; the destination. ;; ;; Only the pre-reload scheduler sees the type of the register alternatives; ;; we split them into nothing before the post-reload scheduler runs. ;; These alternatives therefore have type "move" in order to reflect ;; what happens if the two pre-reload operands cannot be tied, and are ;; instead allocated two separate GPRs. We don't distinguish between ;; the GPR and LO cases because we don't usually know during pre-reload ;; scheduling whether an operand will be LO or not. (define_insn_and_split "extendsidi2" [(set (match_operand:DI 0 "register_operand" "=d,l,d") (sign_extend:DI (match_operand:SI 1 "nonimmediate_operand" "0,0,m")))] "TARGET_64BIT" "@ # # lw\t%0,%1" "&& reload_completed && register_operand (operands[1], VOIDmode)" [(const_int 0)] { emit_note (NOTE_INSN_DELETED); DONE; } [(set_attr "move_type" "move,move,load") (set_attr "mode" "DI")]) So extending the first pattern above from :SHORT to :SUBDI is not really an optimisation, in the sense that it doesn't add new information. Not providing the combination allows the truncation or sign-extension to be optimised with surrounding code. I suppose the argument in favour of going from :SHORT to :SUBDI is that it might avoid a move in some cases. But (a) I think that would need to be measured further, (b) it might instead mean that the extendsidi2 pattern needs to be tweaked for modern RA choices, and (c) it doesn't really feel like stage 4 material. I can understand where the changes came from. The output of combine was clearly wrong before r16-7265-ga9e48eca3a6eef. And what combine did looked bad. But I don't think combine itself did anything wrong. IMO, all it did was expose the problems in the existing RTL. Expand dropped a necessary sign-extension and the rest flowed from there. In particular, the old decisions based on truncated_to_mode seemed correct. The thing that the truncated_to_mode patch changed was the assumption that a 64-bit register containing a "u16 lower" parameter could be truncated with a SUBREG. And that's true, since it's guaranteed by the ABI. The parameter is zero-extended from bit 16 and so the register contains a sign extension of bit 16 (i.e. 0). And that was the information that truncated_to_mode was using. I tested the patch on mips64-linux-gnu (all 3 ABIs). The patch fixes regressions in: - gcc.target/mips/octeon-exts-7.c (n32 & 64) - gcc.target/mips/truncate-1.c (n32 & 64) - gcc.target/mips/truncate-2.c (n32) - gcc.target/mips/truncate-6.c (64) gcc/ PR middle-end/118608 * rtlanal.cc (truncated_to_mode): Revert a change made on 2026-02-03. * config/mips/mips.md (*extenddi_truncate<mode>): Likewise. Diff: --- gcc/config/mips/mips.md | 2 +- gcc/rtlanal.cc | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index 6a1b63b00d2e..85e7d67901f7 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -3952,7 +3952,7 @@ (define_insn_and_split "*extenddi_truncate<mode>" [(set (match_operand:DI 0 "register_operand" "=d") (sign_extend:DI - (truncate:SUBDI (match_operand:DI 1 "register_operand" "d"))))] + (truncate:SHORT (match_operand:DI 1 "register_operand" "d"))))] "TARGET_64BIT && !TARGET_MIPS16 && !ISA_HAS_EXTS" "#" "&& reload_completed" diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc index c5062ab7715b..88561a54e5a0 100644 --- a/gcc/rtlanal.cc +++ b/gcc/rtlanal.cc @@ -6201,17 +6201,6 @@ truncated_to_mode (machine_mode mode, const_rtx x) if (REG_P (x) && rtl_hooks.reg_truncated_to_mode (mode, x)) return true; - /* This explicit TRUNCATE may be needed on targets that require - MODE to be suitably extended when stored in X. Targets such as - mips64 use (sign_extend:DI (truncate:SI (reg:DI x))) to perform - an explicit extension, avoiding use of (subreg:SI (reg:DI x)) - which is assumed to already be extended. */ - scalar_int_mode imode, omode; - if (is_a <scalar_int_mode> (mode, &imode) - && is_a <scalar_int_mode> (GET_MODE (x), &omode) - && targetm.mode_rep_extended (imode, omode) != UNKNOWN) - return false; - /* See if we already satisfy the requirements of MODE. If yes we can just switch to MODE. */ if (num_sign_bit_copies_in_rep[GET_MODE (x)][mode]
