Re: [PATCH] lower-bitint: Lower FLOAT_EXPR from BITINT_TYPE INTEGER_CST [PR112679]

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Nov 24, 2023 at 10:20:01AM +0100, Richard Biener wrote:
> > +      /* Similarly, e.g. with -frounding-math casts from _BitInt INTEGER_CSTs
> > +	 to floating point types need to be rewritten.  */
> > +      else if (SCALAR_FLOAT_TYPE_P (type))
> > +	{
> > +	  gimple *g = SSA_NAME_DEF_STMT (s);
> > +	  if (is_gimple_assign (g) && gimple_assign_rhs_code (g) == FLOAT_EXPR)
> > +	    {
> 
> I think this would combine with the virtual operand code up to the
> is_gimle_assign () check.

Only the gimple *g = SSA_NAME_DEF_STMT (s); if (is_gimple_assign (g) &&
part is the same between the two, but virtual operands will not be seen
on FLOAT_EXPRs and stores won't be seen for the others, so I think it
is cheaper to handle them separately.

> But I also wonder if when you disable enough passes you could
> for example see switch (bit-int-cst) or if (bit-int-cst ...)
> as well?  Given we have PROP_gimple_lbitint couldn't we set that
> optimistically say during gimplification when we didn't see any
> bit-int, making sure to clear the property during inlining when
> the inlined function doesn't have it set?  Or maybe require
> frontends to opt-in into features that require additional
> processing, indicating that with a bit in struct function
> (or a flag on the decl or via a langhook)?  There's other
> passes that we could gate (coroutine stuff, omp stuff?)

I've initially wanted to set a per-function flag somewhere, either
in the FEs or during gimplification, but it turned out that ensuring
the flag is set means I'd need to test it in a lot of places and slow
down processing of everything in there for the checks if any operand
is BITINT_TYPE.
Then I came up with the idea I can just check if any SSA_NAME has non-small
BITINT_TYPE; but of course the INTEGER_CST stores and now the FLOAT_EXPRs
with those ruin that a little bit.

What we could do perhaps cheaply at least for now is set some flag in
build_bitint_type (after all, we have one only, bitint_type_cache != NULL,
except it is static; just probably LTO streaming in bypasses that),
so perhaps we could return early or even gate the pass on no BITINT_TYPEs
created in the IL.  That would help non-LTO with all non-C languages
including C++.  And for C could be also effective, as long as system headers
don't start adding _BitInt types in there...
E.g. there was a risk in one of my attempts that stdbit.h would have one,
but in the end it won't.

	Jakub