| Message ID | 24325532-9484-4f9f-adb3-59d206505d9d@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [committed] Fix MIPS bootstrap | expand |
On Sun, 14 Jan 2024, Jeff Law wrote: > The condition for the pattern looks like this: > > "ISA_HAS_MIPS16E2" > > And if you dig into ISA_HAS_MIPS16E2: > > /* The MIPS16e V2 instructions are available. */ > #define ISA_HAS_MIPS16E2 (TARGET_MIPS16 && TARGET_MIPS16E2 \ > && !TARGET_64BIT) > > > The way the mode iterator is handled is by adding its condition to the > pattern's condition when we expand copies of the pattern resulting in this > condition for one of the two generated patterns: > > (TARGET_MIPS16 && TARGET_MIPS16E2 && !TARGET_64BIT) && TARGET_64BIT > > This can never be true because of the TARGET_64BIT tests. And of course it's the ISA_HAS_MIPS16E2 condition that is wrong, because there's nothing inherent to the MIPS16e2 ASE that prevents it from being implemented on top of a base MIPS64 ISA. The MIPS16e2 specification as published explicitly supports it (even though the only document released to the public has been built from the MIPS32 template). One could argue that a MIPS64 ISA is unlikely to be ever implemented with the MIPS16e2 ASE included, so there is no point in us having support for this combination. The same stands however for the earlier MIPS16e ASE, which to the best of my knowledge has never been included with any MIPS64 ISA silicon (unsure about the legacy MIPS16 ASE), and yet we support that combination, so it'd be consistent if we did the same for the MIPS16e2 ASE as well, and it's minimal maintenance overhead anyway. This would have been caught in the review of course if the patch series weren't fast-tracked instead. I had this macro on my radar in particular. There were reports of breakage posted after the merge too, never addressed until now. > The fix is trivial. Don't use a mode iterator on that pattern. Thank you for handling this somehow anyway. Maciej
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index 17dfcbd6722..b0fb5850a9e 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -3440,16 +3440,16 @@ (define_insn "*ior<mode>3" (set_attr "compression" "micromips,*,*") (set_attr "mode" "<MODE>")]) -(define_insn "*ior<mode>3_mips16_asmacro" - [(set (match_operand:GPR 0 "register_operand" "=d,d") - (ior:GPR (match_operand:GPR 1 "register_operand" "%0,0") - (match_operand:GPR 2 "uns_arith_operand" "d,K")))] +(define_insn "*iorsi3_mips16_asmacro" + [(set (match_operand:SI 0 "register_operand" "=d,d") + (ior:SI (match_operand:SI 1 "register_operand" "%0,0") + (match_operand:SI 2 "uns_arith_operand" "d,K")))] "ISA_HAS_MIPS16E2" "@ or\t%0,%2 ori\t%0,%x2" [(set_attr "alu_type" "or") - (set_attr "mode" "<MODE>") + (set_attr "mode" "SI") (set_attr "extended_mips16" "*,yes")]) (define_insn "*ior<mode>3_mips16"
mips bootstraps have been broken for a while. They've been triggering an error about mutually exclusive equal-tests always being false when building gencondmd. This was ultimately tracked down to the ior<mode>3_mips16_asmacro pattern. The pattern uses the GPR mode iterator which looks like this: (define_mode_iterator GPR [SI (DI "TARGET_64BIT")]) The condition for the pattern looks like this: "ISA_HAS_MIPS16E2" And if you dig into ISA_HAS_MIPS16E2: /* The MIPS16e V2 instructions are available. */ #define ISA_HAS_MIPS16E2 (TARGET_MIPS16 && TARGET_MIPS16E2 \ && !TARGET_64BIT) The way the mode iterator is handled is by adding its condition to the pattern's condition when we expand copies of the pattern resulting in this condition for one of the two generated patterns: (TARGET_MIPS16 && TARGET_MIPS16E2 && !TARGET_64BIT) && TARGET_64BIT This can never be true because of the TARGET_64BIT tests. The fix is trivial. Don't use a mode iterator on that pattern. Bootstrapped on mips64el. I don't have any tests to compare against, so no regression test data. Pushed to the trunk, Jeff commit e927cfa842c16bea902500e69ab4eca2ef15af4e Author: Jeff Law <jlaw@ventanamicro.com> Date: Sun Jan 14 07:53:49 2024 -0700 [committed] Fix MIPS bootstrap mips bootstraps have been broken for a while. They've been triggering an error about mutually exclusive equal-tests always being false when building gencondmd. This was ultimately tracked down to the ior<mode>3_mips16_asmacro pattern. The pattern uses the GPR mode iterator which looks like this: (define_mode_iterator GPR [SI (DI "TARGET_64BIT")]) The condition for the pattern looks like this: "ISA_HAS_MIPS16E2" And if you dig into ISA_HAS_MIPS16E2: /* The MIPS16e V2 instructions are available. */ && !TARGET_64BIT) The way the mode iterator is handled is by adding its condition to the pattern's condition when we expand copies of the pattern resulting in this condition for one of the two generated patterns: (TARGET_MIPS16 && TARGET_MIPS16E2 && !TARGET_64BIT) && TARGET_64BIT This can never be true because of the TARGET_64BIT tests. The fix is trivial. Don't use a mode iterator on that pattern. Bootstrapped on mips64el. I don't have any tests to compare against, so no regression test data. gcc/ * config/mips/mips.md (ior<mode>3_mips16_asmacro): Use SImode, not the GPR iterator. Adjust pattern name and mode attribute accordingly.