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

On Tue, 25 Aug 2026, Avinash Jayakar wrote:

> On Thu, 2026-08-20 at 10:39 +0200, Richard Biener via Gcc wrote:
> > On Thu, 20 Aug 2026, Avinash Jayakar wrote:
> > 
> > > 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?
> > 
> > So it looks like mode_size[] contains byte size and mode_unit_size[]
> > contains unit size.  I assumed that m->bytesize is really bytesize.
> > 
> > The difficulty is, of course, that we have no target with
> > BITS_PER_UNIT
> > != 8 and that support might have bit-rotted quite a bit.
> > 
> > That said, until we officially declare BITS_PER_UNIT == 8 forever
> > we should see to honor != 8 values correctly.
> > 
> Sure, in that case to calculate the maximum value I will use
> c != MODE_PARTIAL_INT && m->component
>                    ? m->component->bytesize : m->bytesize;
> c = (c * 8) / bits_per_unit
> 
> Also this is required change as we add the sha builtins which I was
> working for past 2 weeks. I plan to split that as a sequence of 4
> patches. 3 of which will be rs6000 specific and 1 will be this change
> in genmodes.cc (this will be 2nd in the sequence and patch #3 and #4
> will depend on it).
> 
> Should I send this non-rs6000 patch as a separate patch? Or would it be
> ok to send it as a sequence with the rs6000 patch?

Sure, you can send it as part of a series.

Richard.

> Thanks,
> Avinash Jayakar
> 
> 
> > Richard.
> > 
> > > 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
> > > > > 
> > > 
> 

-- 
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)