Re: How to add 256 byte sized opaque mode in gcc

On Wed, 2026-08-19 at 08:50 +0200, Richard Biener via Gcc wrote:
> On Wed, 19 Aug 2026, Avinash Jayakar wrote:
> 
> > Hi Jakub/Richard,
> > 
> > Thank you for these suggestions and review.
> > 
> > I have attached the proposed patch based on your input to
> > dynamically
> > size the mode_unit_size array based on maximum bytesize declared in
> > machine modes.
> > 
> > Please do let me know if any tests are required for this before
> > sending
> > to gcc-patches?
> 
> +  unsigned int max_type_size = 0;
> +  for_all_modes (c, m)
> +    {
> +      if (max_type_size < m->bytesize)
> +       max_type_size = m->bytesize;
> +    }
> +  if (max_type_size < (1 << 8))
> +    puts ("#define MODE_UNIT_SIZE_TYPE unsigned char\n");
> +  else if (max_type_size < (1 << 16))
> +    puts ("#define MODE_UNIT_SIZE_TYPE unsigned short\n");
> +  else
> +    puts ("#define MODE_UNIT_SIZE_TYPE unsigned int\n");
> +
> 
> I think you need to account for bits_per_unit here, meaning
> you want to track max_unit_size which would be
> ((size_t)m->bytesize * 8) / bits_per_unit
> 
Yes, I think I missed this part. But had a doubt here in the following
function
static void
emit_mode_unit_size (void)
{ 
  int c;
  struct mode_data *m;
  
  print_maybe_const_decl ("%sMODE_UNIT_SIZE_TYPE", "mode_unit_size",
                          "NUM_MACHINE_MODES", adj_bytesize);

  for_all_modes (c, m)
    tagged_printf ("%u",
                   c != MODE_PARTIAL_INT && m->component
                   ? m->component->bytesize : m->bytesize, m->name);
  
  print_closer ();
}  

we print the component->bytesize if component is not null, else just
the bytesize. I do not see bits_per_unit accounted for here. Could this
be a potential issue?
Can I directly use whatever number is being printed here to calculate
the max, since we need to size the type according to what numbers we
print here?

Thanks,
Avinash

> Richard.
> 
> > Also as you mentioned these lookups happen quite often in
> > compilation,
> > so introducing this new mode could potentially increase compile
> > times
> > for powerpc64le right?
> > 
> > Thanks and regards,
> > Avinash Jayakar
> > 
> > On Tue, 2026-08-18 at 13:06 +0200, Jakub Jelinek via Gcc wrote:
> > > On Tue, Aug 18, 2026 at 12:48:55PM +0200, Richard Biener via Gcc
> > > wrote:
> > > > I'd use unsigned short, it's important to not use an overly
> > > > large
> > > > data
> > > > structure for those common lookups to reduce cache footprint. 
> > > > We
> > > > could possibly even dynamically size the component from
> > > > genmodes?
> > > 
> > > Yeah, I'd prefer that, emit a typedef/macro in insn-modes.h, say
> > > MODE_UNIT_SIZE_TYPE, unsigned char or unsigned short depending in
> > > whether all modes fit or don't fit into 255 bytes and use it for
> > > the
> > > generated array too.  Similar to how e.g. the CONST_MODE_* macros
> > > are defined.
> > > 
> > > > > @@ -1884,6 +1885,11 @@ rs6000_hard_regno_mode_ok_uncached
> > > > > (int
> > > > > regno, machine_mode mode)
> > > > >        else
> > > > >  	return 0;
> > > > >      }
> > > > > +  /* QDOmode needs even/odd DMR register pairs.  */
> > > > > +  if (mode == QDOmode)
> > > > > +    {
> > > > > +      return (TARGET_DMF && DMR_REGNO_P (regno) && (regno &
> > > > > 1)
> > > > > == 0);
> > > > > +    }
> > > 
> > > The GCC coding style here is to avoid the {}s around a single
> > > statement
> > > body.
> > Sure I will update this.
> > > 
> > > 	Jakub
> >