This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Fwd: Re: [PATCH] Add powi-to-multiply expansion to cse_sincos pass]
On Tue, May 24, 2011 at 5:28 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
>
> On Tue, 2011-05-24 at 16:44 +0200, Richard Guenther wrote:
>> On Tue, May 24, 2011 at 3:38 PM, William J. Schmidt
>> <wschmidt@linux.vnet.ibm.com> wrote:
>> >
>> > On Tue, 2011-05-24 at 15:11 +0200, Richard Guenther wrote:
>> >> On Tue, May 24, 2011 at 2:26 PM, William J. Schmidt
>> >> <wschmidt@linux.vnet.ibm.com> wrote:
>> >> > Sure, I'll give that a try this morning. ?Much obliged.
>> >>
>> >> Seems it won't work that way without some major changes elsewhere.
>> >> Instead follow what update_call_from_tree () does before calling
>> >> gsi_replace.
>> >
>> > Bother. ?This won't work, unfortunately. ?I can't use gimple_set_vuse ()
>> > and gimple_set_vdef () on statements without memory ops.
>>
>> You should do
>>
>> ? if (!gimple_has_mem_ops (new_stmt))
>> ? ? unlink_stmt_vdef (old_stmt);
>
> OK, thanks, that's the interface I was struggling to find.
>
> That solved the issue I ran into. ?The handling for powi now looks like
> this:
>
> ? ? ? ? ? ? ? ?CASE_FLT_FN (BUILT_IN_POWI):
> ? ? ? ? ? ? ? ? ?arg0 = gimple_call_arg (stmt, 0);
> ? ? ? ? ? ? ? ? ?arg1 = gimple_call_arg (stmt, 1);
> ? ? ? ? ? ? ? ? ?if (!host_integerp (arg1, 0))
> ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ? ?n = TREE_INT_CST_LOW (arg1);
> ? ? ? ? ? ? ? ? ?loc = gimple_location (stmt);
> ? ? ? ? ? ? ? ? ?result = gimple_expand_builtin_powi (&gsi, loc, arg0, n);
>
> ? ? ? ? ? ? ? ? ?if (result)
> ? ? ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ? ? ?tree lhs = gimple_get_lhs (stmt);
> ? ? ? ? ? ? ? ? ? ? ?gimple new_stmt = gimple_build_assign (lhs, result);
> ? ? ? ? ? ? ? ? ? ? ?gimple_set_location (new_stmt, loc);
> ? ? ? ? ? ? ? ? ? ? ?move_ssa_defining_stmt_for_defs (new_stmt, stmt);
>
> ? ? ? ? ? ? ? ? ? ? ?if (gimple_vdef (stmt))
> ? ? ? ? ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ? ? ? ? ?gcc_assert (!gimple_has_mem_ops (new_stmt));
> ? ? ? ? ? ? ? ? ? ? ? ? ?unlink_stmt_vdef (stmt);
> ? ? ? ? ? ? ? ? ? ? ? ?}
As you say the new stmt will not have mem-ops, so
move_ssa_defining_stmt_for_defs is not necessary. Likewise
you can simply unconditionally call unlink_stmt_vdef.
> ? ? ? ? ? ? ? ? ? ? ?gsi_replace (&gsi, new_stmt, true);
> ? ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ? ?break;
>
> gimple_has_mem_ops (new_stmt) will always return false. ?The assert is
> in place in case we add other powi transforms in the future.
>
> The call to move_ssa_defining_stmt_for_defs requires adding the header
> file tree-ssa-propagate.h, and the corresponding dependency in
> Makefile.in.
>
> Currently regression testing the "final" fix. ?Let me know if you want
> to see it one more time before commit.
It's ok with the above two changes.
Thanks,
Richard.
> Thanks as always for your help!
>
> Bill
>
>>
>> > Bill
>> >
>> >>
>> >> Richard.
>> >>
>> >
>> >
>> >
>
>