[Bug target/99718] [11 regression] ICE in new test case gcc.target/powerpc/pr98914.c for 32 bits

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99718

--- Comment #15 from luoxhu at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #14)
> You still have:
>   if (VECTOR_MEM_VSX_P (mode))
>     {
>       if (!CONST_INT_P (elt_rtx))
>         {
>           if ((TARGET_P9_VECTOR && TARGET_POWERPC64) || width == 8)
>             return ..._p9 (...);
>           else if (TARGET_P8_VECTOR)
>             return ..._p8 (...);
>         }
> 
>       if (mode == V2DFmode)
>         insn = gen_vsx_set_v2df (target, target, val, elt_rtx);
> 
>       else if (mode == V2DImode)
>         insn = gen_vsx_set_v2di (target, target, val, elt_rtx);
> 
>       else if (TARGET_P9_VECTOR && TARGET_POWERPC64)
>         {
>           ...
>         }
>       if (insn)
>         return;
>     }
> 
>   gcc_assert (CONST_INT_P (elt_rtx));
> 
> while the vector.md condition is VECTOR_MEM_ALTIVEC_OR_VSX_P (<MODE>mode),
> i.e. true for TARGET_ALTIVEC for many modes already (V4SI, V8HI, V16QI, V4SF
> and
> for TARGET_VSX also V2DF and V2DI, right).
> I somehow don't see how this can work properly.
> Looking at vsx_set_v2df and vsx_set_v2di, neither of them will handle
> non-constant elt_rtx (it ICEs on anything but const0_rtx and const1_rtx).
> 
> So, questions:
> 1) does the rs6000_expand_vector_set_var_p9 routine for width == 8 (i.e.
> V2DImode or V2DFmode?)
> handle everything, even when TARGET_P9_VECTOR or TARGET_POWERPC64 is not
> true, plain old VSX?

Yes. V2DI/V2DF for P8 {BE,LE} {m32,m64} will call
rs6000_expand_vector_set_var_p9 instead of xxx_p8. 

Do you mean Power7 for the plain old VSX? I verified the pr98914.c on Power7,
it exactly ICEs on "gcc_assert (CONST_INT_P (elt_rtx));" for both m64 and m32. 
This is still not fixed by the patch in #c11 yet.

For builtin call in rs6000-c.c:altivec_build_resolved_builtin, it is guarded by
TARGET_P8_VECTOR, so Power7 doesn't generate IFN VEC_INSERT before. This ICE
also comes from internal optimization gimple-isel.c:gimple_expand_vec_set_expr,
can_vec_set_var_idx_p doesn't return false due to VECTOR_MEM_ALTIVEC_OR_VSX_P
is true when Power7 VSX, change the "if (VECTOR_MEM_VSX_P (mode))" to "if
(VECTOR_MEM_ALTIVEC_OR_VSX_P (mode))" in rs6000.c:rs6000_expand_vector_set and
remove TARGET_P8_VECTOR in the else branch could fix the ICE on P7 {m32,64}, so
this means even P7 VSX could benefit from this optimization, which is different
from what discussed before.


> 2) what happens if TARGET_P8_VECTOR is false and TARGET_VSX is true and mode
> is other than V2DI/V2DF? If I read the code right, it will fall through to
> gcc_assert (CONST_INT_P (elt_rtx));

Same like 1)?

> 3) what happens if !TARGET_VSX (more specifically, when VECTOR_MEM_VSX_P
> (mode) is false.
> I see there just the assertion that would fail right away.
> Perhaps I'm missing something obvious and those cases are impossible, but if
> that is the case, it would still be better to add further assertion at least
> to the if (...) else if (...) as else gcc_assert ...

Thanks for pointing out, the "gcc_assert (CONST_INT_P (elt_rtx));" should be
moved into the "if (!CONST_INT_P (elt_rtx))" condition like you said. 
gen_vsx_set_v2df and gen_vsx_set_v2di are supposed to handle only const
elt_rtx.