[Bug ipa/114985] [15 regression] internal compiler error: in discriminator_fail during stage2

["aldyh at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

--- Comment #22 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Martin Jambor from comment #20)

Thanks for looking into this.

> The IL we generate the jump function from is:
>   <bb 2>
>   _1 = cclauses_2(D) != 0B;
>   c_parser_omp_all_clauses (_1);
> 
> Which translates to the expected jump function:
>   callsite  void c_parser_omp_teams(int**)/3 -> int*
> c_parser_omp_all_clauses(bool)/1 :
>      param 0: PASS THROUGH: 0, op ne_expr 0B
> 
> so IPA looks like it's doing what it should.

So, is it bad IL?  If so, do you know what created it?

> 
> (In reply to Aldy Hernandez from comment #6)
> > I wonder if something like this would work.
> > 
> > diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> > index 5781f50..ea8a685 100644
> > --- a/gcc/ipa-cp.cc
> > +++ b/gcc/ipa-cp.cc
> > @@ -1730,6 +1730,8 @@ ipa_value_range_from_jfunc (vrange &vr,
> >         }
> >        else
> >         {
> > +         if (TREE_CODE_CLASS (operation) == tcc_comparison)
> > +           vr_type = boolean_type_node;
> >           Value_Range op_res (vr_type);
> >           Value_Range res (vr_type);
> >           tree op = ipa_get_jf_pass_through_operand (jfunc);
> 
> This looks OKish and we also do a similar thing in
> ipa_get_jf_arith_result.
> 
> Also note that the ipa_value_range_from_jfunc already has a parameter
> that tells it what type the result should be.  It is called parm_type,
> which is boolean_type in the case that ICEs.  So we can even bail out
> if we really encounter jump function created from bad IL.
> 
> I was thinking of using use parm_type from the beginning, to
> initialize op_res with it, but there are jump functions representing
> an operation followed by a truncation, for example for:
> 
>   _2 = complain_6(D) & 1;
>   _3 = (int) std_alignof_7(D);
>   cxx_sizeof_or_alignof_type (_3, _2);
> 
> where _r is in fact bool (has smaller size and precision) and trying
> to make ranger do the bit_and_expr directly to bool leads to a failed
> assert in fold_range (the test of m_operator->operand_check_p).

ranger should accept any combination the IL does.  What we try to diagnose with
the operand_check_p's is combinations that would also be incorrect in the IL.

> 
> So doing the operation in the original type - unless it is a
> comparison - and then using ipa_vr_operation_and_type_effects seems to
> be the right thing to do.
> 
> But I am really curious why propagate_vr_across_jump_function does not
> need the same check for tcc_comparison operators and generally why is

In my local tree I have mods to both ipa_value_range_from_jfunc and
propagate_vr_across_jump_function, because I found a failing testcase that
needed it.  So you probably need to tweak both places.

> it so different (in the non-scc case)?  Why is ipa_supports_p (this
> predicate has a really really really bad name BTW and I am completely
> at loss as to what it does and how or why) used there and not in
> ipa_value_range_from_jfunc?

Feel free to change it to whatever you prefer.  I was just trying to avoid
adding "&& prange::supports_type_p()" in a bunch of places.  Although what we
probably want is to verify that the types match with operands_check_p as you do
in ipa_vr_operation_and_type_effects():

  return (handler.operand_check_p (dst_type, src_type, dst_type)
          && handler.fold_range (dst_vr, dst_type, src_vr, varying)
          && !dst_vr.varying_p ()
          && !dst_vr.undefined_p ());

My apologies if I have dirtied ipa-* somewhat.  To be honest, I would much
rather not have to touch the code, as I'm not very familiar with it.  Feel free
to fix it however way you feel is best.  I'll be happy to provide feedback as
needed.

> 
> (I also cannot prevent myself from ranting a little that it would
> really help if all the ranger (helper) classes and functions were
> better documented.)

Andrew has been in documentation and clean-up mode for a few months now, and
should be able to push something out soon.