Re: [PATCH] Fix several endless loops in cse_insn for certain targettypes combinations

[Igor Shevlyakov <igor at microunity dot com>]

Ian Lance Taylor wrote:

> Igor Shevlyakov <igor@microunity.com> writes:
>
>  
>
> > I found several places in cse.c with similar code which supposed to be
> > iterating class of type modes:
> >
> >
> >      for (wider_mode = GET_MODE_WIDER_MODE (mode);
> >           GET_MODE_BITSIZE (wider_mode) <= BITS_PER_WORD
> >           && src_related == 0;
> >           wider_mode = GET_MODE_WIDER_MODE (wider_mode))
> >
> > But in a situation when widest mode in a class as wide as
> > BITS_PER_WORD (like in my case registers are 128 bits and largest mode
> > we use is TImode), this loop will never stop. So there should be an
> > extra check that VOIDmode is reached.
> >    
>
> Another approach would be for you to put
>
> INT_MODE (OI, 32);
>
> in your CPU-modes.def file, and to patch genmodes.c to add something
> like
>    gcc_assert (BITS_PER_WORD < GET_MODE_BITSIZE (MAX_MODE_INT));
> to init_adjust_machine_modes.
>
> The main advantage of this, if it works, would be keeping the code
> slightly simpler for the ordinary case.
>  

But that means to introduce fake type, I didn't plan to use. This will 
require to add several new patterns into .md file  (at least movoi), 
handle calling convention for OI type and god knows what else. It will 
affect the performance in other places since it will take one more 
iteration from MIN_TYPE_INT to MAX_TYPE_INT. Plus, building a cross 
compiler to support 256-bit type will be nightmare. Since part of 
constant optimizations are still performed in HOST_WIDE_INT type, even 
128-bit support on 32-bit host is broken. I've avoided this problem by 
moving into AMD64 host, but 256-bit will bring that saga back.

But even in general, to have a code which could loop endlessly and pass 
a knowledge that "in order for compiler to work, you always need to 
define types with 2*BITS_PER_WORD size", looks like not very wise 
maintainance policy to me. Oh, by the way, it's not only OI, 256-bit 
floating point mode will also need to be defined.

If you are concerned about performance of a loops like that

     for (wider_mode = GET_MODE_WIDER_MODE (mode);
          GET_MODE_BITSIZE (wider_mode) <= BITS_PER_WORD;
          wider_mode = GET_MODE_WIDER_MODE (wider_mode))
	{ body; }


than, first of all, this is not best version either, since it's always 
dereffences memory in condition. Faster version would be

     |limit = mode_for_size(BITS_PER_WORD, GET_MODE_CLASS(mode), 0);||
     for (wider_mode = GET_MODE_WIDER_MODE (mode);
          wider_mode <= limit;
          wider_mode = GET_MODE_WIDER_MODE (wider_mode))
	{ body; }
|


But it still suffers from the problem. The following code will be 
fastest and safe:

     |limit = mode_for_size(BITS_PER_WORD, GET_MODE_CLASS(mode), 0);
     wider_mode = mode;
     do {
       wider_mode = ||GET_MODE_WIDER_MODE (mode);
       body;
     } while (wider_mode != limit};

|

Should I redo the patch to use this variant?

Igor