This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

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.



I'm also a bit cautious of the number of calls to build_int_cst with a
type of NULL_TREE.  My understanding is that we're trying to improve
the type safety of the middle-end, and should now try to be using
valid "types" where possible.

>>+	/* ((union {fptype fp; int i[];}) arg).i[offset]  */
>>+	result = fold (build4 (ARRAY_REF, integer_type_node, result,
>>+			       build_int_cst (NULL_TREE, offset),
>>+			       NULL_TREE, NULL_TREE));

Here for example, the first argument to build_int_cst should be
array_idx, the domain of the array.  There's recently been a discussion
on this on the gcc list.

>>+	/* (((union {fptype fp; int i[];}) arg).i[offset]) & mask  */
>>+	result = fold (build2 (BIT_AND_EXPR, integer_type_node, result,
>>+			       build_int_cst (NULL_TREE, mask)));

And here for example, the first argument to build_int_cst should be
integer_type_node to match the type of the BIT_AND_EXPR and "result".



Let me know if you disagree, and still think that signbit needs to be
lowered during fold instead of RTL expansion.  My get feeling is that
it's better to keep at a higher level of abstraction during the tree
passes, so we can optimize "x = fabs(y); z = signbit(x)", and yet still
generate efficient bit manipulation code when we descend to RTL.

Hypothetically, some machines may support a "signbit" instruction/sequence
that we'd like to make use of via an optab, but decomposing too soon
to an aliasing memory bit manipulation would obfuscate things too much.

Thoughts?

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]