Jakub Jelinek <ja...@redhat.com> writes: > On Wed, Nov 19, 2014 at 09:59:45AM +0100, Richard Biener wrote: >> > OImode/XImode on i?86/x86_64 are not <= MAX_BITSIZE_MODE_ANY_INT, because >> > they are never used for integer arithmetics (and there is no way >> > to represent all their values in RTL if not using CONST_WIDE_INT). >> > As the following testcase shows, simplify_immed_subreg can be called >> > with such modes though, e.g. trying to forward propagate a CONST_VECTOR >> > (i?86/x86_64 handles all zeros and all ones as CONST_VECTORs that can >> > appear >> > in the IL directly) into a SUBREG_REG. >> >> So we have (subreg:OI (reg:V4DF ...) ...) for example? What do we >> end doing with that OI mode subreg? (why can't we simply use the >> V4DF one?) > > propagate_rtx_1 is called on > (subreg:OI (reg:V8DI 89) 0) > with old_rtx > (reg:V8DI 89) > and new_rtx > (const_vector:V8DI [ > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > (const_int 0 [0]) > ]) > > Seems we throw away the result in that case though, because > gen_lowpart_common doesn't like to return low parts of VOIDmode constants > larger if the mode is larger than double int: > 1353 innermode = GET_MODE (x); > 1354 if (CONST_INT_P (x) > 1355 && msize * BITS_PER_UNIT <= HOST_BITS_PER_WIDE_INT) > 1356 innermode = mode_for_size (HOST_BITS_PER_WIDE_INT, MODE_INT, 0); > 1357 else if (innermode == VOIDmode) > 1358 innermode = mode_for_size (HOST_BITS_PER_DOUBLE_INT, MODE_INT, 0); > we hit the last if and as mode is wider than innermode, we return NULL later > on. > >> > The following patch instead of ICE handles the most common cases (all 0 >> > and all 1 CONST_VECTORs) and returns NULL otherwise. >> > >> > Before wide-int got merged, the testcase worked, though the code didn't >> > bother checking anything, just created 0 or constm1_rtx for the two cases >> > that could happen and if something else appeared, could just return what >> > matched low TImode (or DImode for -m32). >> > >> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> Looks ok to me. Not sure if the zero/all-ones case really happens that >> much to be worth special-casing - I think you could use >> fixed_wide_int<proper-size> and simply see if the result is representable >> in a CONST_INT/CONST_DOUBLE? Can you try if that works? It looks like >> the patch may be smaller for that. > > So perhaps something like this? Don't know how much more inefficient it is > compared to what it used to do before.
I think the effect is that OImode integers are interesting on x86 if the value fits in a sign-extended TImode, but not otherwise. Is that right? In some ways it seems like an arbitrary distinction. If any nonzero OImode values are potentially interesting then aren't all of them potentially interesting? > Or just keep the existing code and just remove the assert and instead return > NULL whenever outer_submode is wider than MAX_BITSIZE_MODE_ANY_INT? At > least during propagation that will make zero change. Sounds good to me FWIW, especially if there are no known cases where we want a nonnull return for modes wider than MAX_BITSIZE_MODE_ANY_INT. (I assume the assert would have triggered for them too.) Thanks, Richard
