Re: [PATCH RESUBMIT]: add tree-based folding of builtin signbit*

[Roger Sayle <roger at eyesopen dot com>]

On Wed, 2 Mar 2005, Kaveh R. Ghazi wrote:
>  > On Wed, 2 Mar 2005, Kaveh R. Ghazi wrote:
>  > > I'm resubmitting this now that mainline is in stage1 again.
>  > > http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02499.html
>  >
>  > I'm not sure that this is needed any more, now that RTH has
>  > implemented expand_builtin_signbit during RTL expansion, which
>  > performs identically the same transformation after the tree-ssa
>  > passes.  Certainly, your original motivation that linux can't rely on
>  > __builtin_signbit no longer holds.
>
> Er, my recollection was that he implemented builtin copysign, not
> builtin signbit.

My mistake.  You're right of course, RTH's recent patch was to implement
copysign at RTL expansion time.  And indeed the exisiting/current RTL
expander for signbit doesn't handle "long double" types, i.e. signbitl.
The reason for the latter is that when I wrote expand_builtin_signbit
I didn't know how to handle floating point types wider than the widest
integer mode.  The good news is that RTH's expand_copysign both solves
the problem and demonstrates how it should be done in the signbit RTL
expander.


>  > It also isn't clear whether your code to lower the tree during "fold"
>  > actually hurts the quality of code we generate.  For example, I know
>  > that if tree-ssa propagates a floating point constant into a CALL_EXPR
>  > to __builtin_copysign that we'll constant fold it during tree-ssa.  It
>  > isn't clear whether the COMPONENT_REF of an ARRAY_REF that you
>  > generate instead is currently folded by tree-ssa, or instead doesn't
>  > get cleaned up until the RTL optimizers, if at all.  I believe this
>  > issue was the source of RTH's concern about lowering tree's too early.
>
> Can you suggest some C code to test this on?


Could you check the following with your patch:

int foo()
{
  double t = 1.0;
  return __builtin_signbit(t);
}

int bar()
{
  double t = 1.0;
  union { double __d; int __i[2]; } __u;
  __u.__d = t;
  return __u.__i[1] < 0;
}


On CVS mainline at -O2, "foo" is optimized to "return 0;" before the
.00.expand RTL dump, i.e. -dr.  The function "bar", on the other hand,
has initial RTL containing floating point loads from the constant pool
and an SImode lshiftrt by 31.  This is on i686-pc-linux-gnu.  As I'd
expected, the RTL optimizers manage to take up the slack, so by the
time we generate assembly language, both functions generate the same
code.  However, the point is that we fail to optimize through the
union at the tree-level, a potential regression with your patch.



> AFAICT, signbitl isn't RTL expanded.  Certainly relying on the RTL
> expanders becomes more compelling if it actually works. :-)
> Care to implement that?  (RTL is rather opaque to me.)

It should be a "simple" matter of cut-n-pasting the code from RTH's
recent copysign work.  I'll probably get some time to investigate a
patch for it this weekend.


Sorry again for my confusion over signbit vs. copysign.  We now have
RTL expanders for both, but one handles long doubles and the other
doesn't.

Roger
--