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

[Richard Biener <rguenther@suse.de>]

On Fri, 24 Nov 2023, Jakub Jelinek wrote:

> 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.

Yeah, wondered about that ...

> 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.

I guess we have that flag already, it's 'bitint_type_chache' itself?

But it's a bit coarse indeed.

Richard.