Sorry for the slow response, was out last week.
Christophe Lyon via Gcc-patches <[email protected]> writes:
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index feeee16d320..5f559f8fd93 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -6239,9 +6239,14 @@ init_emit_once (void)
>
> /* For BImode, 1 and -1 are unsigned and signed interpretations
> of the same value. */
> - const_tiny_rtx[0][(int) BImode] = const0_rtx;
> - const_tiny_rtx[1][(int) BImode] = const_true_rtx;
> - const_tiny_rtx[3][(int) BImode] = const_true_rtx;
> + for (mode = MIN_MODE_BOOL;
> + mode <= MAX_MODE_BOOL;
> + mode = (machine_mode)((int)(mode) + 1))
> + {
> + const_tiny_rtx[0][(int) mode] = const0_rtx;
> + const_tiny_rtx[1][(int) mode] = const_true_rtx;
> + const_tiny_rtx[3][(int) mode] = const_true_rtx;
> + }
>
> for (mode = MIN_MODE_PARTIAL_INT;
> mode <= MAX_MODE_PARTIAL_INT;
Does this do the right thing for:
gen_int_mode (-1, B2Imode)
(which is used e.g. in native_decode_vector_rtx)? It looks like it
would give 0b01 rather than 0b11.
Maybe for non-BImode we should use const1_rtx and constm1_rtx, like with
MODE_INT.
> @@ -1298,9 +1315,21 @@ enum machine_mode\n{");
> /* Don't use BImode for MIN_MODE_INT, since otherwise the middle
> end will try to use it for bitfields in structures and the
> like, which we do not want. Only the target md file should
> - generate BImode widgets. */
> - if (first && first->precision == 1 && c == MODE_INT)
> - first = first->next;
> + generate BImode widgets. Since some targets such as ARM/MVE
> + define boolean modes with multiple bits, handle those too. */
> + if (first && first->boolean)
> + {
> + struct mode_data *last_bool = first;
> + printf (" MIN_MODE_BOOL = E_%smode,\n", first->name);
> +
> + while (first && first->boolean)
> + {
> + last_bool = first;
> + first = first->next;
> + }
> +
> + printf (" MAX_MODE_BOOL = E_%smode,\n\n", last_bool->name);
> + }
>
> if (first && last)
> printf (" MIN_%s = E_%smode,\n MAX_%s = E_%smode,\n\n",
For the record: this means that MIN_MODE_BOOL and MAX_MODE_BOOL are
in principle only conditionally available, whereas:
/* For BImode, 1 and -1 are unsigned and signed interpretations
of the same value. */
- const_tiny_rtx[0][(int) BImode] = const0_rtx;
- const_tiny_rtx[1][(int) BImode] = const_true_rtx;
- const_tiny_rtx[3][(int) BImode] = const_true_rtx;
+ for (mode = MIN_MODE_BOOL;
+ mode <= MAX_MODE_BOOL;
+ mode = (machine_mode)((int)(mode) + 1))
+ {
+ const_tiny_rtx[0][(int) mode] = const0_rtx;
+ const_tiny_rtx[1][(int) mode] = const_true_rtx;
+ const_tiny_rtx[3][(int) mode] = const_true_rtx;
+ }
assumes that they are unconditionally available. In some ways it
might be clearer if we assert that first->boolean is true and
emit the MIN/MAX stuff unconditionally.
However, that would make the generator less robust against malformed
input, and it would probably be inconsistent with the current generator
code, so I agree that the patch's version is better on balance.
> @@ -1679,15 +1708,25 @@ emit_class_narrowest_mode (void)
> print_decl ("unsigned char", "class_narrowest_mode", "MAX_MODE_CLASS");
>
> for (c = 0; c < MAX_MODE_CLASS; c++)
> - /* Bleah, all this to get the comment right for MIN_MODE_INT. */
> - tagged_printf ("MIN_%s", mode_class_names[c],
> - modes[c]
> - ? ((c != MODE_INT || modes[c]->precision != 1)
> - ? modes[c]->name
> - : (modes[c]->next
> - ? modes[c]->next->name
> - : void_mode->name))
> - : void_mode->name);
> + {
> + /* Bleah, all this to get the comment right for MIN_MODE_INT. */
> + const char *comment_name = void_mode->name;
> +
> + if (modes[c])
> + if (c != MODE_INT || !modes[c]->boolean)
> + comment_name = modes[c]->name;
> + else
> + {
> + struct mode_data *m = modes[c];
> + while (m->boolean)
> + m = m->next;
> + if (m)
> + comment_name = m->name;
> + else
> + comment_name = void_mode->name;
> + }
Have you tried bootstrapping the patch on a host of your choice?
I would expect a warning/Werror about an ambiguous else here.
I guess this reduces to:
struct mode_data *m = modes[c];
while (m && m->boolean)
m = m->next;
const char *comment_name = (m ? m : void_mode)->name;
but I don't know if that's more readable.
LGTM otherwise.
Thanks,
Richard
