Re: RFC [1/2] divmod transform

2016-08-13 Thread Prathamesh Kulkarni
On 13 August 2016 at 16:56, Prathamesh Kulkarni
 wrote:
> On 28 July 2016 at 19:05, Prathamesh Kulkarni
>  wrote:
>> On 8 June 2016 at 19:53, Richard Biener  wrote:
>>> On Fri, 3 Jun 2016, Jim Wilson wrote:
>>>
 On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
 > Joseph - do you know sth about why there's not a full set of divmod
 > libfuncs in libgcc?

 Because udivmoddi4 isn't a libfunc, it is a helper function for the
 div and mov libfuncs.  Since we can compute the signed div and mod
 results from udivmoddi4, there was no need to also add a signed
 version of it.  It was given a libfunc style name so that we had the
 option of making it a libfunc in the future, but that never happened.
 There was no support for calling any divmod libfunc until it was added
 as a special case to call an ARM library (not libgcc) function.  This
 happened here

 2004-08-09  Mark Mitchell  

 * config.gcc (arm*-*-eabi*): New target.
 * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
 (TARGET_LIB_INT_CMP_BIASED): Likewise.
 * expmed.c (expand_divmod): Try a two-valued divmod function as a
 last resort.
 ...
 * config/arm/arm.c (arm_init_libfuncs): New function.
 (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
 (TARGET_INIT_LIBFUNCS): Define it.
 ...

 Later, two ports added their own divmod libfuncs, but I don't see any
 evidence that they were ever used, since there is no support for
 calling divmod other than the expand_divmod last resort code that only
 triggers for ARM.

 It is only now that Prathamesh is adding gimple support for divmod
 operations that we need to worry about getting this right, without
 breaking the existing ARM library support or the existing udivmoddi4
 support.
>>>
>>> Ok, so as he is primarily targeting the special arm divmod libcall
>>> I suppose we can live with special-casing libcall handling to
>>> udivmoddi3.  It would be nice to not lie about divmod availablilty
>>> as libcall though... - it looks like the libcall is also guarded
>>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
>>> like on x86).
>>>
>>> So not sure where to go from here.
>> Hi,
>> I have attached patch, which is rebased on trunk.
>> Needed to update divmod-7.c, which now gets transformed to divmod
>> thanks to your code-hoisting patch -;)
>> We still have the issue of optab_libfunc() returning non-existent
>> libcalls. As in previous patch, I am checking
>> explicitly for "__udivmoddi4", with a FIXME note.
>> I hope that's okay for now ?
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu,
>> armv8l-unknown-linux-gnueabihf.
>> Bootstrap+test in progress on i686-linux-gnu.
>> Cross-tested on arm*-*-*.
> Hi Richard,
> I have following two approaches to workaround optab_libfunc issue:
>
> a) Not lie about divmod libfunc availability by setting libcall entry to NULL
> for sdivmod_optab in optabs.def.
> Patch posted for that here:
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
> Although it doesn't cause any regressions with the gcc testsuite,
> I am not sure if this change is correct.
>
> b) Perform the transform only if target-specific divmod is available,
> ie, drop targeting
> __udivmoddi4. I have attached (untested) patch for that.
> When/If we have the optab_libfunc issue resolved, we can later target 
> "generic"
> divmod libfunc.
Oops, small mistake in the previous patch.
We also want to check if target has optab_libfunc set for the given mode.
Corrected in this version.

Thanks,
Prathamesh
>
> Do either of these approaches look reasonable ?
>
> PS: I am on vacation next week, will get back to working on patch
> after returning.
>
> Thanks,
> Prathamesh
>>
>> Thanks,
>> Prathamesh
>>>
>>> Richard.
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index f506a83..618c810 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2012,28 +2012,14 @@ default_max_noce_ifcvt_seq_cost (edge e)
DImode __udivmoddi4 (DImode op0, DImode op1, DImode *rem).  */
 
 void
-default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
-  rtx op0, rtx op1,
-  rtx *quot_p, rtx *rem_p)
+default_expand_divmod_libfunc (bool unsignedp ATTRIBUTE_UNUSED,
+  machine_mode mode ATTRIBUTE_UNUSED,
+  rtx op0 ATTRIBUTE_UNUSED,
+  rtx op1 ATTRIBUTE_UNUSED,
+   rtx *quot_p ATTRIBUTE_UNUSED,
+  rtx *rem_p ATTRIBUTE_UNUSED)
 {
-  gcc_assert (mode == DImode);
-  gcc_assert (unsignedp);
-
-  rtx libfunc = optab_libfunc (udivmod_optab, DImode);
-  gcc_assert (libfunc);

Re: RFC [1/2] divmod transform

2016-08-13 Thread Prathamesh Kulkarni
On 28 July 2016 at 19:05, Prathamesh Kulkarni
 wrote:
> On 8 June 2016 at 19:53, Richard Biener  wrote:
>> On Fri, 3 Jun 2016, Jim Wilson wrote:
>>
>>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
>>> > Joseph - do you know sth about why there's not a full set of divmod
>>> > libfuncs in libgcc?
>>>
>>> Because udivmoddi4 isn't a libfunc, it is a helper function for the
>>> div and mov libfuncs.  Since we can compute the signed div and mod
>>> results from udivmoddi4, there was no need to also add a signed
>>> version of it.  It was given a libfunc style name so that we had the
>>> option of making it a libfunc in the future, but that never happened.
>>> There was no support for calling any divmod libfunc until it was added
>>> as a special case to call an ARM library (not libgcc) function.  This
>>> happened here
>>>
>>> 2004-08-09  Mark Mitchell  
>>>
>>> * config.gcc (arm*-*-eabi*): New target.
>>> * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
>>> (TARGET_LIB_INT_CMP_BIASED): Likewise.
>>> * expmed.c (expand_divmod): Try a two-valued divmod function as a
>>> last resort.
>>> ...
>>> * config/arm/arm.c (arm_init_libfuncs): New function.
>>> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
>>> (TARGET_INIT_LIBFUNCS): Define it.
>>> ...
>>>
>>> Later, two ports added their own divmod libfuncs, but I don't see any
>>> evidence that they were ever used, since there is no support for
>>> calling divmod other than the expand_divmod last resort code that only
>>> triggers for ARM.
>>>
>>> It is only now that Prathamesh is adding gimple support for divmod
>>> operations that we need to worry about getting this right, without
>>> breaking the existing ARM library support or the existing udivmoddi4
>>> support.
>>
>> Ok, so as he is primarily targeting the special arm divmod libcall
>> I suppose we can live with special-casing libcall handling to
>> udivmoddi3.  It would be nice to not lie about divmod availablilty
>> as libcall though... - it looks like the libcall is also guarded
>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
>> like on x86).
>>
>> So not sure where to go from here.
> Hi,
> I have attached patch, which is rebased on trunk.
> Needed to update divmod-7.c, which now gets transformed to divmod
> thanks to your code-hoisting patch -;)
> We still have the issue of optab_libfunc() returning non-existent
> libcalls. As in previous patch, I am checking
> explicitly for "__udivmoddi4", with a FIXME note.
> I hope that's okay for now ?
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu,
> armv8l-unknown-linux-gnueabihf.
> Bootstrap+test in progress on i686-linux-gnu.
> Cross-tested on arm*-*-*.
Hi Richard,
I have following two approaches to workaround optab_libfunc issue:

a) Not lie about divmod libfunc availability by setting libcall entry to NULL
for sdivmod_optab in optabs.def.
Patch posted for that here:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
Although it doesn't cause any regressions with the gcc testsuite,
I am not sure if this change is correct.

b) Perform the transform only if target-specific divmod is available,
ie, drop targeting
__udivmoddi4. I have attached (untested) patch for that.
When/If we have the optab_libfunc issue resolved, we can later target "generic"
divmod libfunc.

Do either of these approaches look reasonable ?

PS: I am on vacation next week, will get back to working on patch
after returning.

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
>>
>> Richard.
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index f506a83..618c810 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2012,28 +2012,14 @@ default_max_noce_ifcvt_seq_cost (edge e)
DImode __udivmoddi4 (DImode op0, DImode op1, DImode *rem).  */
 
 void
-default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
-  rtx op0, rtx op1,
-  rtx *quot_p, rtx *rem_p)
+default_expand_divmod_libfunc (bool unsignedp ATTRIBUTE_UNUSED,
+  machine_mode mode ATTRIBUTE_UNUSED,
+  rtx op0 ATTRIBUTE_UNUSED,
+  rtx op1 ATTRIBUTE_UNUSED,
+   rtx *quot_p ATTRIBUTE_UNUSED,
+  rtx *rem_p ATTRIBUTE_UNUSED)
 {
-  gcc_assert (mode == DImode);
-  gcc_assert (unsignedp);
-
-  rtx libfunc = optab_libfunc (udivmod_optab, DImode);
-  gcc_assert (libfunc);
-  gcc_assert (!strcmp (XSTR (libfunc, 0), "__udivmoddi4"));
-
-  rtx remainder = assign_stack_temp (DImode, GET_MODE_SIZE (DImode));
-  rtx address = XEXP (remainder, 0);
-
-  rtx quotient = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
- DImode, 3,
- op0, GET_MODE (op0),
- 

Re: RFC [1/2] divmod transform

2016-08-09 Thread Prathamesh Kulkarni
ping https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01867.html

Thanks,
Prathamesh

On 28 July 2016 at 19:05, Prathamesh Kulkarni
 wrote:
> On 8 June 2016 at 19:53, Richard Biener  wrote:
>> On Fri, 3 Jun 2016, Jim Wilson wrote:
>>
>>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
>>> > Joseph - do you know sth about why there's not a full set of divmod
>>> > libfuncs in libgcc?
>>>
>>> Because udivmoddi4 isn't a libfunc, it is a helper function for the
>>> div and mov libfuncs.  Since we can compute the signed div and mod
>>> results from udivmoddi4, there was no need to also add a signed
>>> version of it.  It was given a libfunc style name so that we had the
>>> option of making it a libfunc in the future, but that never happened.
>>> There was no support for calling any divmod libfunc until it was added
>>> as a special case to call an ARM library (not libgcc) function.  This
>>> happened here
>>>
>>> 2004-08-09  Mark Mitchell  
>>>
>>> * config.gcc (arm*-*-eabi*): New target.
>>> * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
>>> (TARGET_LIB_INT_CMP_BIASED): Likewise.
>>> * expmed.c (expand_divmod): Try a two-valued divmod function as a
>>> last resort.
>>> ...
>>> * config/arm/arm.c (arm_init_libfuncs): New function.
>>> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
>>> (TARGET_INIT_LIBFUNCS): Define it.
>>> ...
>>>
>>> Later, two ports added their own divmod libfuncs, but I don't see any
>>> evidence that they were ever used, since there is no support for
>>> calling divmod other than the expand_divmod last resort code that only
>>> triggers for ARM.
>>>
>>> It is only now that Prathamesh is adding gimple support for divmod
>>> operations that we need to worry about getting this right, without
>>> breaking the existing ARM library support or the existing udivmoddi4
>>> support.
>>
>> Ok, so as he is primarily targeting the special arm divmod libcall
>> I suppose we can live with special-casing libcall handling to
>> udivmoddi3.  It would be nice to not lie about divmod availablilty
>> as libcall though... - it looks like the libcall is also guarded
>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
>> like on x86).
>>
>> So not sure where to go from here.
> Hi,
> I have attached patch, which is rebased on trunk.
> Needed to update divmod-7.c, which now gets transformed to divmod
> thanks to your code-hoisting patch -;)
> We still have the issue of optab_libfunc() returning non-existent
> libcalls. As in previous patch, I am checking
> explicitly for "__udivmoddi4", with a FIXME note.
> I hope that's okay for now ?
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu,
> armv8l-unknown-linux-gnueabihf.
> Bootstrap+test in progress on i686-linux-gnu.
> Cross-tested on arm*-*-*.
>
> Thanks,
> Prathamesh
>>
>> Richard.


Re: RFC [1/2] divmod transform

2016-07-28 Thread Prathamesh Kulkarni
On 8 June 2016 at 19:53, Richard Biener  wrote:
> On Fri, 3 Jun 2016, Jim Wilson wrote:
>
>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
>> > Joseph - do you know sth about why there's not a full set of divmod
>> > libfuncs in libgcc?
>>
>> Because udivmoddi4 isn't a libfunc, it is a helper function for the
>> div and mov libfuncs.  Since we can compute the signed div and mod
>> results from udivmoddi4, there was no need to also add a signed
>> version of it.  It was given a libfunc style name so that we had the
>> option of making it a libfunc in the future, but that never happened.
>> There was no support for calling any divmod libfunc until it was added
>> as a special case to call an ARM library (not libgcc) function.  This
>> happened here
>>
>> 2004-08-09  Mark Mitchell  
>>
>> * config.gcc (arm*-*-eabi*): New target.
>> * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
>> (TARGET_LIB_INT_CMP_BIASED): Likewise.
>> * expmed.c (expand_divmod): Try a two-valued divmod function as a
>> last resort.
>> ...
>> * config/arm/arm.c (arm_init_libfuncs): New function.
>> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
>> (TARGET_INIT_LIBFUNCS): Define it.
>> ...
>>
>> Later, two ports added their own divmod libfuncs, but I don't see any
>> evidence that they were ever used, since there is no support for
>> calling divmod other than the expand_divmod last resort code that only
>> triggers for ARM.
>>
>> It is only now that Prathamesh is adding gimple support for divmod
>> operations that we need to worry about getting this right, without
>> breaking the existing ARM library support or the existing udivmoddi4
>> support.
>
> Ok, so as he is primarily targeting the special arm divmod libcall
> I suppose we can live with special-casing libcall handling to
> udivmoddi3.  It would be nice to not lie about divmod availablilty
> as libcall though... - it looks like the libcall is also guarded
> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
> like on x86).
>
> So not sure where to go from here.
Hi,
I have attached patch, which is rebased on trunk.
Needed to update divmod-7.c, which now gets transformed to divmod
thanks to your code-hoisting patch -;)
We still have the issue of optab_libfunc() returning non-existent
libcalls. As in previous patch, I am checking
explicitly for "__udivmoddi4", with a FIXME note.
I hope that's okay for now ?

Bootstrapped and tested on x86_64-unknown-linux-gnu,
armv8l-unknown-linux-gnueabihf.
Bootstrap+test in progress on i686-linux-gnu.
Cross-tested on arm*-*-*.

Thanks,
Prathamesh
>
> Richard.
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 83bd9ab..e4815cf 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7010,6 +7010,12 @@ This is firstly introduced on ARM/AArch64 targets, 
please refer to
 the hook implementation for how different fusion types are supported.
 @end deftypefn

+@deftypefn {Target Hook} void TARGET_EXPAND_DIVMOD_LIBFUNC (bool 
@var{unsignedp}, machine_mode @var{mode}, @var{rtx}, @var{rtx}, rtx 
*@var{quot}, rtx *@var{rem})
+Define this hook if the port does not have hardware div and divmod insn for
+the given mode but has divmod libfunc, which is incompatible
+with libgcc2.c:__udivmoddi4
+@end deftypefn
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index a72c3d8..3efaf4d 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4864,6 +4864,8 @@ them: try the first ones in this list first.

 @hook TARGET_SCHED_FUSION_PRIORITY

+@hook TARGET_EXPAND_DIVMOD_LIBFUNC
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 49f3495..18876ce 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -2326,6 +2326,48 @@ multi_vector_optab_supported_p (convert_optab optab, 
tree_pair types,
 #define direct_mask_store_optab_supported_p direct_optab_supported_p
 #define direct_store_lanes_optab_supported_p multi_vector_optab_supported_p

+/* Expand DIVMOD() using:
+ a) optab handler for udivmod/sdivmod if it is available.
+ b) If optab_handler doesn't exist, Generate call to
+optab_libfunc for udivmod/sdivmod.  */
+
+static void
+expand_DIVMOD (internal_fn, gcall *stmt)
+{
+  tree lhs = gimple_call_lhs (stmt);
+  tree arg0 = gimple_call_arg (stmt, 0);
+  tree arg1 = gimple_call_arg (stmt, 1);
+
+  gcc_assert (TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE);
+  tree type = TREE_TYPE (TREE_TYPE (lhs));
+  machine_mode mode = TYPE_MODE (type);
+  bool unsignedp = TYPE_UNSIGNED (type);
+  optab tab = (unsignedp) ? udivmod_optab : sdivmod_optab;
+
+  rtx op0 = expand_normal 

Re: RFC [1/2] divmod transform

2016-06-08 Thread Richard Biener
On Fri, 3 Jun 2016, Jim Wilson wrote:

> On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
> > Joseph - do you know sth about why there's not a full set of divmod
> > libfuncs in libgcc?
> 
> Because udivmoddi4 isn't a libfunc, it is a helper function for the
> div and mov libfuncs.  Since we can compute the signed div and mod
> results from udivmoddi4, there was no need to also add a signed
> version of it.  It was given a libfunc style name so that we had the
> option of making it a libfunc in the future, but that never happened.
> There was no support for calling any divmod libfunc until it was added
> as a special case to call an ARM library (not libgcc) function.  This
> happened here
> 
> 2004-08-09  Mark Mitchell  
> 
> * config.gcc (arm*-*-eabi*): New target.
> * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
> (TARGET_LIB_INT_CMP_BIASED): Likewise.
> * expmed.c (expand_divmod): Try a two-valued divmod function as a
> last resort.
> ...
> * config/arm/arm.c (arm_init_libfuncs): New function.
> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
> (TARGET_INIT_LIBFUNCS): Define it.
> ...
> 
> Later, two ports added their own divmod libfuncs, but I don't see any
> evidence that they were ever used, since there is no support for
> calling divmod other than the expand_divmod last resort code that only
> triggers for ARM.
> 
> It is only now that Prathamesh is adding gimple support for divmod
> operations that we need to worry about getting this right, without
> breaking the existing ARM library support or the existing udivmoddi4
> support.

Ok, so as he is primarily targeting the special arm divmod libcall
I suppose we can live with special-casing libcall handling to
udivmoddi3.  It would be nice to not lie about divmod availablilty
as libcall though... - it looks like the libcall is also guarded
on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
like on x86).

So not sure where to go from here.

Richard.


Re: RFC [1/2] divmod transform

2016-06-03 Thread Jim Wilson
On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
> Joseph - do you know sth about why there's not a full set of divmod
> libfuncs in libgcc?

Because udivmoddi4 isn't a libfunc, it is a helper function for the
div and mov libfuncs.  Since we can compute the signed div and mod
results from udivmoddi4, there was no need to also add a signed
version of it.  It was given a libfunc style name so that we had the
option of making it a libfunc in the future, but that never happened.
There was no support for calling any divmod libfunc until it was added
as a special case to call an ARM library (not libgcc) function.  This
happened here

2004-08-09  Mark Mitchell  

* config.gcc (arm*-*-eabi*): New target.
* defaults.h (TARGET_LIBGCC_FUNCS): New macro.
(TARGET_LIB_INT_CMP_BIASED): Likewise.
* expmed.c (expand_divmod): Try a two-valued divmod function as a
last resort.
...
* config/arm/arm.c (arm_init_libfuncs): New function.
(arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
(TARGET_INIT_LIBFUNCS): Define it.
...

Later, two ports added their own divmod libfuncs, but I don't see any
evidence that they were ever used, since there is no support for
calling divmod other than the expand_divmod last resort code that only
triggers for ARM.

It is only now that Prathamesh is adding gimple support for divmod
operations that we need to worry about getting this right, without
breaking the existing ARM library support or the existing udivmoddi4
support.

Jim


Re: RFC [1/2] divmod transform

2016-06-03 Thread Joseph Myers
On Mon, 30 May 2016, Richard Biener wrote:

> Joseph - do you know sth about why there's not a full set of divmod
> libfuncs in libgcc?

I'm not familiar with the choice of divmod libfuncs.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: RFC [1/2] divmod transform

2016-06-01 Thread Richard Biener
On Tue, 31 May 2016, Prathamesh Kulkarni wrote:

> On 30 May 2016 at 13:15, Richard Biener  wrote:
> > On Mon, 30 May 2016, Prathamesh Kulkarni wrote:
> >
> >> The attached patch ICE's during bootstrap for x86_64, and is reproducible 
> >> with
> >> following case with -m32 -O2:
> >>
> >> typedef long long type;
> >>
> >> type f(type x, type y)
> >> {
> >>   type q = x / y;
> >>   type r = x % y;
> >>   return q + r;
> >> }
> >>
> >> The ICE happens because the test-case hits
> >> gcc_assert (unsignedp);
> >> in default_expand_divmod_libfunc ().
> >
> > That's of course your function (and ICE).
> >
> >> Surprisingly, optab_libfunc (sdivmod_optab, DImode) returns optab_libfunc
> >> with name "__divmoddi4" although __divmoddi4() is nowhere defined in
> >> libgcc for x86.
> >> (I verified that by forcing the patch to generate call to __divmoddi4,
> >> which results in undefined reference to __divmoddi4).
> >>
> >> This happens because in optabs.def we have:
> >> OPTAB_NL(sdivmod_optab, "divmod$a4", UNKNOWN, "divmod", '4', 
> >> gen_int_libfunc)
> >>
> >> and gen_int_libfunc generates "__divmoddi4" on first call to optab_libfunc
> >> and sets optab_libfunc (sdivmod_optab, DImode) to "__divmoddi4".
> >> I wonder if we should remove gen_int_libfunc entry in optabs.def for
> >> sdivmod_optab ?
> >
> > Hum, not sure - you might want to look at expand_divmod (though that
> > always just computes one part of the result in the end).
> As a workaround, would it be OK to check if libfunc is __udivmoddi4
> if expand_divmod_libfunc is default, as in attached patch ?

Humm.  I suppose it is because until now we never expanded divmod
directly but only the modulo and division libfuncs are all 
implemented in terms of udivmoddi4.

This means that the signed divmod libfunc expander needs to use
that as well - not sure if that's efficiently possible though.
Which would mean to simply never consider divmoddi4 as available.

Richard.

> This prevents ICE for the above test-case.
> Bootstrap+test on x86_64 in progress.
> 
> Thanks,
> Prathamesh
> >
> > Joseph - do you know sth about why there's not a full set of divmod
> > libfuncs in libgcc?
> >
> > Thanks,
> > Richard.
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: RFC [1/2] divmod transform

2016-05-31 Thread Prathamesh Kulkarni
On 30 May 2016 at 13:15, Richard Biener  wrote:
> On Mon, 30 May 2016, Prathamesh Kulkarni wrote:
>
>> The attached patch ICE's during bootstrap for x86_64, and is reproducible 
>> with
>> following case with -m32 -O2:
>>
>> typedef long long type;
>>
>> type f(type x, type y)
>> {
>>   type q = x / y;
>>   type r = x % y;
>>   return q + r;
>> }
>>
>> The ICE happens because the test-case hits
>> gcc_assert (unsignedp);
>> in default_expand_divmod_libfunc ().
>
> That's of course your function (and ICE).
>
>> Surprisingly, optab_libfunc (sdivmod_optab, DImode) returns optab_libfunc
>> with name "__divmoddi4" although __divmoddi4() is nowhere defined in
>> libgcc for x86.
>> (I verified that by forcing the patch to generate call to __divmoddi4,
>> which results in undefined reference to __divmoddi4).
>>
>> This happens because in optabs.def we have:
>> OPTAB_NL(sdivmod_optab, "divmod$a4", UNKNOWN, "divmod", '4', gen_int_libfunc)
>>
>> and gen_int_libfunc generates "__divmoddi4" on first call to optab_libfunc
>> and sets optab_libfunc (sdivmod_optab, DImode) to "__divmoddi4".
>> I wonder if we should remove gen_int_libfunc entry in optabs.def for
>> sdivmod_optab ?
>
> Hum, not sure - you might want to look at expand_divmod (though that
> always just computes one part of the result in the end).
As a workaround, would it be OK to check if libfunc is __udivmoddi4
if expand_divmod_libfunc is default, as in attached patch ?
This prevents ICE for the above test-case.
Bootstrap+test on x86_64 in progress.

Thanks,
Prathamesh
>
> Joseph - do you know sth about why there's not a full set of divmod
> libfuncs in libgcc?
>
> Thanks,
> Richard.
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 8c7f2a1..111f19f 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6963,6 +6963,12 @@ This is firstly introduced on ARM/AArch64 targets, 
please refer to
 the hook implementation for how different fusion types are supported.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_EXPAND_DIVMOD_LIBFUNC (bool 
@var{unsignedp}, machine_mode @var{mode}, @var{rtx}, @var{rtx}, rtx 
*@var{quot}, rtx *@var{rem})
+Define this hook if the port does not have hardware div and divmod insn for
+the given mode but has divmod libfunc, which is incompatible
+with libgcc2.c:__udivmoddi4
+@end deftypefn
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index f963a58..2c9a800 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4848,6 +4848,8 @@ them: try the first ones in this list first.
 
 @hook TARGET_SCHED_FUSION_PRIORITY
 
+@hook TARGET_EXPAND_DIVMOD_LIBFUNC
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index c867ddc..0cb59f7 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -2276,6 +2276,48 @@ multi_vector_optab_supported_p (convert_optab optab, 
tree_pair types,
 #define direct_mask_store_optab_supported_p direct_optab_supported_p
 #define direct_store_lanes_optab_supported_p multi_vector_optab_supported_p
 
+/* Expand DIVMOD() using:
+ a) optab handler for udivmod/sdivmod if it is available.
+ b) If optab_handler doesn't exist, Generate call to
+optab_libfunc for udivmod/sdivmod.  */
+
+static void 
+expand_DIVMOD (internal_fn, gcall *stmt)
+{
+  tree lhs = gimple_call_lhs (stmt);
+  tree arg0 = gimple_call_arg (stmt, 0);
+  tree arg1 = gimple_call_arg (stmt, 1);
+
+  gcc_assert (TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE);
+  tree type = TREE_TYPE (TREE_TYPE (lhs));
+  machine_mode mode = TYPE_MODE (type);
+  bool unsignedp = TYPE_UNSIGNED (type);
+  optab tab = (unsignedp) ? udivmod_optab : sdivmod_optab;
+
+  rtx op0 = expand_normal (arg0);
+  rtx op1 = expand_normal (arg1);
+  rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
+
+  rtx quotient, remainder;
+
+  /* Check if optab handler exists for [u]divmod.  */
+  if (optab_handler (tab, mode) != CODE_FOR_nothing)
+{
+  quotient = gen_reg_rtx (mode);
+  remainder = gen_reg_rtx (mode);
+  expand_twoval_binop (tab, op0, op1, quotient, remainder, unsignedp);
+}
+  else
+targetm.expand_divmod_libfunc (unsignedp, mode, op0, op1,
+  , );
+
+  /* Wrap the return value (quotient, remainder) within COMPLEX_EXPR.  */
+  expand_expr (build2 (COMPLEX_EXPR, TREE_TYPE (lhs),
+  make_tree (TREE_TYPE (arg0), quotient),
+  make_tree (TREE_TYPE (arg1), remainder)),
+  target, VOIDmode, EXPAND_NORMAL);
+}
+
 /* Return true if FN is supported for the types in TYPES when the
optimization type is OPT_TYPE.  The types are those associated with
the "type0" and "type1" fields of FN's direct_internal_fn_info
diff 

Re: RFC [1/2] divmod transform

2016-05-30 Thread Richard Biener
On Mon, 30 May 2016, Prathamesh Kulkarni wrote:

> The attached patch ICE's during bootstrap for x86_64, and is reproducible with
> following case with -m32 -O2:
> 
> typedef long long type;
> 
> type f(type x, type y)
> {
>   type q = x / y;
>   type r = x % y;
>   return q + r;
> }
> 
> The ICE happens because the test-case hits
> gcc_assert (unsignedp);
> in default_expand_divmod_libfunc ().

That's of course your function (and ICE).

> Surprisingly, optab_libfunc (sdivmod_optab, DImode) returns optab_libfunc
> with name "__divmoddi4" although __divmoddi4() is nowhere defined in
> libgcc for x86.
> (I verified that by forcing the patch to generate call to __divmoddi4,
> which results in undefined reference to __divmoddi4).
> 
> This happens because in optabs.def we have:
> OPTAB_NL(sdivmod_optab, "divmod$a4", UNKNOWN, "divmod", '4', gen_int_libfunc)
> 
> and gen_int_libfunc generates "__divmoddi4" on first call to optab_libfunc
> and sets optab_libfunc (sdivmod_optab, DImode) to "__divmoddi4".
> I wonder if we should remove gen_int_libfunc entry in optabs.def for
> sdivmod_optab ?

Hum, not sure - you might want to look at expand_divmod (though that
always just computes one part of the result in the end).

Joseph - do you know sth about why there's not a full set of divmod
libfuncs in libgcc?

Thanks,
Richard.


Re: RFC [1/2] divmod transform

2016-05-29 Thread Prathamesh Kulkarni
On 27 May 2016 at 17:31, Richard Biener  wrote:
> On Fri, 27 May 2016, Prathamesh Kulkarni wrote:
>
>> On 27 May 2016 at 15:45, Richard Biener  wrote:
>> > On Wed, 25 May 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 25 May 2016 at 12:52, Richard Biener  wrote:
>> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >
>> >> >> On 24 May 2016 at 19:39, Richard Biener  wrote:
>> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >> >
>> >> >> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
>> >> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >> >> >
>> >> >> >> >> On 23 May 2016 at 17:35, Richard Biener 
>> >> >> >> >>  wrote:
>> >> >> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>> >> >> >> >> >  wrote:
>> >> >> >> >> >> Hi,
>> >> >> >> >> >> I have updated my patch for divmod (attached), which was 
>> >> >> >> >> >> originally
>> >> >> >> >> >> based on Kugan's patch.
>> >> >> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
>> >> >> >> >> >> TRUNC_MOD_EXPR
>> >> >> >> >> >> having same operands to divmod representation, so we can cse 
>> >> >> >> >> >> computation of mod.
>> >> >> >> >> >>
>> >> >> >> >> >> t1 = a TRUNC_DIV_EXPR b;
>> >> >> >> >> >> t2 = a TRUNC_MOD_EXPR b
>> >> >> >> >> >> is transformed to:
>> >> >> >> >> >> complex_tmp = DIVMOD (a, b);
>> >> >> >> >> >> t1 = REALPART_EXPR (complex_tmp);
>> >> >> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
>> >> >> >> >> >>
>> >> >> >> >> >> * New hook divmod_expand_libfunc
>> >> >> >> >> >> The rationale for introducing the hook is that different 
>> >> >> >> >> >> targets have
>> >> >> >> >> >> incompatible calling conventions for divmod libfunc.
>> >> >> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
>> >> >> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> >> >> >> >> >> return quotient and store remainder in argument passed as 
>> >> >> >> >> >> pointer,
>> >> >> >> >> >> while the arm version takes two arguments and returns both
>> >> >> >> >> >> quotient and remainder having mode double the size of the 
>> >> >> >> >> >> operand mode.
>> >> >> >> >> >> The port should hence override the hook expand_divmod_libfunc
>> >> >> >> >> >> to generate call to target-specific divmod.
>> >> >> >> >> >> Ports should define this hook if:
>> >> >> >> >> >> a) The port does not have divmod or div insn for the given 
>> >> >> >> >> >> mode.
>> >> >> >> >> >> b) The port defines divmod libfunc for the given mode.
>> >> >> >> >> >> The default hook default_expand_divmod_libfunc() generates 
>> >> >> >> >> >> call
>> >> >> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned 
>> >> >> >> >> >> and
>> >> >> >> >> >> are of DImode.
>> >> >> >> >> >>
>> >> >> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> >> >> >> >> >> cross-tested on arm*-*-*.
>> >> >> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
>> >> >> >> >> >> Does this patch look OK ?
>> >> >> >> >> >
>> >> >> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> >> >> >> >> > index 6b4601b..e4a021a 100644
>> >> >> >> >> > --- a/gcc/targhooks.c
>> >> >> >> >> > +++ b/gcc/targhooks.c
>> >> >> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, 
>> >> >> >> >> > machine_mode,
>> >> >> >> >> > machine_mode, optimization_type)
>> >> >> >> >> >return true;
>> >> >> >> >> >  }
>> >> >> >> >> >
>> >> >> >> >> > +void
>> >> >> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode 
>> >> >> >> >> > mode,
>> >> >> >> >> > +  rtx op0, rtx op1,
>> >> >> >> >> > +  rtx *quot_p, rtx *rem_p)
>> >> >> >> >> >
>> >> >> >> >> > functions need a comment.
>> >> >> >> >> >
>> >> >> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 
>> >> >> >> >> > style?  In that
>> >> >> >> >> > case we could avoid the target hook.
>> >> >> >> >> Well I would prefer adding the hook because that's more easier 
>> >> >> >> >> -;)
>> >> >> >> >> Would it be ok for now to go with the hook ?
>> >> >> >> >> >
>> >> >> >> >> > +  /* If target overrides expand_divmod_libfunc hook
>> >> >> >> >> > +then perform divmod by generating call to the 
>> >> >> >> >> > target-specifc divmod
>> >> >> >> >> > libfunc.  */
>> >> >> >> >> > +  if (targetm.expand_divmod_libfunc != 
>> >> >> >> >> > default_expand_divmod_libfunc)
>> >> >> >> >> > +   return true;
>> >> >> >> >> > +
>> >> >> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
>> >> >> >> >> > +  return (mode == DImode && unsignedp);
>> >> >> >> >> >
>> >> >> >> >> > I don't understand this - we know optab_libfunc returns 
>> >> >> >> >> > non-NULL for 'mode'
>> >> >> >> >> > but still restrict this to DImode && unsigned?  Also if
>> 

Re: RFC [1/2] divmod transform

2016-05-27 Thread Richard Biener
On Fri, 27 May 2016, Prathamesh Kulkarni wrote:

> On 27 May 2016 at 15:45, Richard Biener  wrote:
> > On Wed, 25 May 2016, Prathamesh Kulkarni wrote:
> >
> >> On 25 May 2016 at 12:52, Richard Biener  wrote:
> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 24 May 2016 at 19:39, Richard Biener  wrote:
> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >> >
> >> >> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
> >> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >> >> >
> >> >> >> >> On 23 May 2016 at 17:35, Richard Biener 
> >> >> >> >>  wrote:
> >> >> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
> >> >> >> >> >  wrote:
> >> >> >> >> >> Hi,
> >> >> >> >> >> I have updated my patch for divmod (attached), which was 
> >> >> >> >> >> originally
> >> >> >> >> >> based on Kugan's patch.
> >> >> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
> >> >> >> >> >> TRUNC_MOD_EXPR
> >> >> >> >> >> having same operands to divmod representation, so we can cse 
> >> >> >> >> >> computation of mod.
> >> >> >> >> >>
> >> >> >> >> >> t1 = a TRUNC_DIV_EXPR b;
> >> >> >> >> >> t2 = a TRUNC_MOD_EXPR b
> >> >> >> >> >> is transformed to:
> >> >> >> >> >> complex_tmp = DIVMOD (a, b);
> >> >> >> >> >> t1 = REALPART_EXPR (complex_tmp);
> >> >> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
> >> >> >> >> >>
> >> >> >> >> >> * New hook divmod_expand_libfunc
> >> >> >> >> >> The rationale for introducing the hook is that different 
> >> >> >> >> >> targets have
> >> >> >> >> >> incompatible calling conventions for divmod libfunc.
> >> >> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
> >> >> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> >> >> >> >> >> return quotient and store remainder in argument passed as 
> >> >> >> >> >> pointer,
> >> >> >> >> >> while the arm version takes two arguments and returns both
> >> >> >> >> >> quotient and remainder having mode double the size of the 
> >> >> >> >> >> operand mode.
> >> >> >> >> >> The port should hence override the hook expand_divmod_libfunc
> >> >> >> >> >> to generate call to target-specific divmod.
> >> >> >> >> >> Ports should define this hook if:
> >> >> >> >> >> a) The port does not have divmod or div insn for the given 
> >> >> >> >> >> mode.
> >> >> >> >> >> b) The port defines divmod libfunc for the given mode.
> >> >> >> >> >> The default hook default_expand_divmod_libfunc() generates call
> >> >> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned 
> >> >> >> >> >> and
> >> >> >> >> >> are of DImode.
> >> >> >> >> >>
> >> >> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> >> >> >> >> >> cross-tested on arm*-*-*.
> >> >> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
> >> >> >> >> >> Does this patch look OK ?
> >> >> >> >> >
> >> >> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> >> >> >> >> > index 6b4601b..e4a021a 100644
> >> >> >> >> > --- a/gcc/targhooks.c
> >> >> >> >> > +++ b/gcc/targhooks.c
> >> >> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, 
> >> >> >> >> > machine_mode,
> >> >> >> >> > machine_mode, optimization_type)
> >> >> >> >> >return true;
> >> >> >> >> >  }
> >> >> >> >> >
> >> >> >> >> > +void
> >> >> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode 
> >> >> >> >> > mode,
> >> >> >> >> > +  rtx op0, rtx op1,
> >> >> >> >> > +  rtx *quot_p, rtx *rem_p)
> >> >> >> >> >
> >> >> >> >> > functions need a comment.
> >> >> >> >> >
> >> >> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 
> >> >> >> >> > style?  In that
> >> >> >> >> > case we could avoid the target hook.
> >> >> >> >> Well I would prefer adding the hook because that's more easier -;)
> >> >> >> >> Would it be ok for now to go with the hook ?
> >> >> >> >> >
> >> >> >> >> > +  /* If target overrides expand_divmod_libfunc hook
> >> >> >> >> > +then perform divmod by generating call to the 
> >> >> >> >> > target-specifc divmod
> >> >> >> >> > libfunc.  */
> >> >> >> >> > +  if (targetm.expand_divmod_libfunc != 
> >> >> >> >> > default_expand_divmod_libfunc)
> >> >> >> >> > +   return true;
> >> >> >> >> > +
> >> >> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> >> >> >> >> > +  return (mode == DImode && unsignedp);
> >> >> >> >> >
> >> >> >> >> > I don't understand this - we know optab_libfunc returns 
> >> >> >> >> > non-NULL for 'mode'
> >> >> >> >> > but still restrict this to DImode && unsigned?  Also if
> >> >> >> >> > targetm.expand_divmod_libfunc
> >> >> >> >> > is not the default we expect the target to handle all modes?
> >> >> >> >> Ah indeed, the check for DImode is unnecessary.
> >> >> >> >> 

Re: RFC [1/2] divmod transform

2016-05-27 Thread Prathamesh Kulkarni
On 27 May 2016 at 15:45, Richard Biener  wrote:
> On Wed, 25 May 2016, Prathamesh Kulkarni wrote:
>
>> On 25 May 2016 at 12:52, Richard Biener  wrote:
>> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 24 May 2016 at 19:39, Richard Biener  wrote:
>> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >
>> >> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
>> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >> >
>> >> >> >> On 23 May 2016 at 17:35, Richard Biener 
>> >> >> >>  wrote:
>> >> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>> >> >> >> >  wrote:
>> >> >> >> >> Hi,
>> >> >> >> >> I have updated my patch for divmod (attached), which was 
>> >> >> >> >> originally
>> >> >> >> >> based on Kugan's patch.
>> >> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
>> >> >> >> >> TRUNC_MOD_EXPR
>> >> >> >> >> having same operands to divmod representation, so we can cse 
>> >> >> >> >> computation of mod.
>> >> >> >> >>
>> >> >> >> >> t1 = a TRUNC_DIV_EXPR b;
>> >> >> >> >> t2 = a TRUNC_MOD_EXPR b
>> >> >> >> >> is transformed to:
>> >> >> >> >> complex_tmp = DIVMOD (a, b);
>> >> >> >> >> t1 = REALPART_EXPR (complex_tmp);
>> >> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
>> >> >> >> >>
>> >> >> >> >> * New hook divmod_expand_libfunc
>> >> >> >> >> The rationale for introducing the hook is that different targets 
>> >> >> >> >> have
>> >> >> >> >> incompatible calling conventions for divmod libfunc.
>> >> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
>> >> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> >> >> >> >> return quotient and store remainder in argument passed as 
>> >> >> >> >> pointer,
>> >> >> >> >> while the arm version takes two arguments and returns both
>> >> >> >> >> quotient and remainder having mode double the size of the 
>> >> >> >> >> operand mode.
>> >> >> >> >> The port should hence override the hook expand_divmod_libfunc
>> >> >> >> >> to generate call to target-specific divmod.
>> >> >> >> >> Ports should define this hook if:
>> >> >> >> >> a) The port does not have divmod or div insn for the given mode.
>> >> >> >> >> b) The port defines divmod libfunc for the given mode.
>> >> >> >> >> The default hook default_expand_divmod_libfunc() generates call
>> >> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
>> >> >> >> >> are of DImode.
>> >> >> >> >>
>> >> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> >> >> >> >> cross-tested on arm*-*-*.
>> >> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
>> >> >> >> >> Does this patch look OK ?
>> >> >> >> >
>> >> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> >> >> >> > index 6b4601b..e4a021a 100644
>> >> >> >> > --- a/gcc/targhooks.c
>> >> >> >> > +++ b/gcc/targhooks.c
>> >> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, 
>> >> >> >> > machine_mode,
>> >> >> >> > machine_mode, optimization_type)
>> >> >> >> >return true;
>> >> >> >> >  }
>> >> >> >> >
>> >> >> >> > +void
>> >> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
>> >> >> >> > +  rtx op0, rtx op1,
>> >> >> >> > +  rtx *quot_p, rtx *rem_p)
>> >> >> >> >
>> >> >> >> > functions need a comment.
>> >> >> >> >
>> >> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 
>> >> >> >> > style?  In that
>> >> >> >> > case we could avoid the target hook.
>> >> >> >> Well I would prefer adding the hook because that's more easier -;)
>> >> >> >> Would it be ok for now to go with the hook ?
>> >> >> >> >
>> >> >> >> > +  /* If target overrides expand_divmod_libfunc hook
>> >> >> >> > +then perform divmod by generating call to the 
>> >> >> >> > target-specifc divmod
>> >> >> >> > libfunc.  */
>> >> >> >> > +  if (targetm.expand_divmod_libfunc != 
>> >> >> >> > default_expand_divmod_libfunc)
>> >> >> >> > +   return true;
>> >> >> >> > +
>> >> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
>> >> >> >> > +  return (mode == DImode && unsignedp);
>> >> >> >> >
>> >> >> >> > I don't understand this - we know optab_libfunc returns non-NULL 
>> >> >> >> > for 'mode'
>> >> >> >> > but still restrict this to DImode && unsigned?  Also if
>> >> >> >> > targetm.expand_divmod_libfunc
>> >> >> >> > is not the default we expect the target to handle all modes?
>> >> >> >> Ah indeed, the check for DImode is unnecessary.
>> >> >> >> However I suppose the check for unsignedp should be there,
>> >> >> >> since we want to generate call to __udivmoddi4 only if operand is 
>> >> >> >> unsigned ?
>> >> >> >
>> >> >> > The optab libfunc for sdivmod should be NULL in that case.
>> >> >> Ah indeed, thanks.
>> >> >> >
>> >> >> >> >
>> >> >> 

Re: RFC [1/2] divmod transform

2016-05-27 Thread Richard Biener
On Wed, 25 May 2016, Prathamesh Kulkarni wrote:

> On 25 May 2016 at 12:52, Richard Biener  wrote:
> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >
> >> On 24 May 2016 at 19:39, Richard Biener  wrote:
> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
> >> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >> >
> >> >> >> On 23 May 2016 at 17:35, Richard Biener  
> >> >> >> wrote:
> >> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
> >> >> >> >  wrote:
> >> >> >> >> Hi,
> >> >> >> >> I have updated my patch for divmod (attached), which was 
> >> >> >> >> originally
> >> >> >> >> based on Kugan's patch.
> >> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
> >> >> >> >> TRUNC_MOD_EXPR
> >> >> >> >> having same operands to divmod representation, so we can cse 
> >> >> >> >> computation of mod.
> >> >> >> >>
> >> >> >> >> t1 = a TRUNC_DIV_EXPR b;
> >> >> >> >> t2 = a TRUNC_MOD_EXPR b
> >> >> >> >> is transformed to:
> >> >> >> >> complex_tmp = DIVMOD (a, b);
> >> >> >> >> t1 = REALPART_EXPR (complex_tmp);
> >> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
> >> >> >> >>
> >> >> >> >> * New hook divmod_expand_libfunc
> >> >> >> >> The rationale for introducing the hook is that different targets 
> >> >> >> >> have
> >> >> >> >> incompatible calling conventions for divmod libfunc.
> >> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
> >> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> >> >> >> >> return quotient and store remainder in argument passed as pointer,
> >> >> >> >> while the arm version takes two arguments and returns both
> >> >> >> >> quotient and remainder having mode double the size of the operand 
> >> >> >> >> mode.
> >> >> >> >> The port should hence override the hook expand_divmod_libfunc
> >> >> >> >> to generate call to target-specific divmod.
> >> >> >> >> Ports should define this hook if:
> >> >> >> >> a) The port does not have divmod or div insn for the given mode.
> >> >> >> >> b) The port defines divmod libfunc for the given mode.
> >> >> >> >> The default hook default_expand_divmod_libfunc() generates call
> >> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
> >> >> >> >> are of DImode.
> >> >> >> >>
> >> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> >> >> >> >> cross-tested on arm*-*-*.
> >> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
> >> >> >> >> Does this patch look OK ?
> >> >> >> >
> >> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> >> >> >> > index 6b4601b..e4a021a 100644
> >> >> >> > --- a/gcc/targhooks.c
> >> >> >> > +++ b/gcc/targhooks.c
> >> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, 
> >> >> >> > machine_mode,
> >> >> >> > machine_mode, optimization_type)
> >> >> >> >return true;
> >> >> >> >  }
> >> >> >> >
> >> >> >> > +void
> >> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
> >> >> >> > +  rtx op0, rtx op1,
> >> >> >> > +  rtx *quot_p, rtx *rem_p)
> >> >> >> >
> >> >> >> > functions need a comment.
> >> >> >> >
> >> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 
> >> >> >> > style?  In that
> >> >> >> > case we could avoid the target hook.
> >> >> >> Well I would prefer adding the hook because that's more easier -;)
> >> >> >> Would it be ok for now to go with the hook ?
> >> >> >> >
> >> >> >> > +  /* If target overrides expand_divmod_libfunc hook
> >> >> >> > +then perform divmod by generating call to the 
> >> >> >> > target-specifc divmod
> >> >> >> > libfunc.  */
> >> >> >> > +  if (targetm.expand_divmod_libfunc != 
> >> >> >> > default_expand_divmod_libfunc)
> >> >> >> > +   return true;
> >> >> >> > +
> >> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> >> >> >> > +  return (mode == DImode && unsignedp);
> >> >> >> >
> >> >> >> > I don't understand this - we know optab_libfunc returns non-NULL 
> >> >> >> > for 'mode'
> >> >> >> > but still restrict this to DImode && unsigned?  Also if
> >> >> >> > targetm.expand_divmod_libfunc
> >> >> >> > is not the default we expect the target to handle all modes?
> >> >> >> Ah indeed, the check for DImode is unnecessary.
> >> >> >> However I suppose the check for unsignedp should be there,
> >> >> >> since we want to generate call to __udivmoddi4 only if operand is 
> >> >> >> unsigned ?
> >> >> >
> >> >> > The optab libfunc for sdivmod should be NULL in that case.
> >> >> Ah indeed, thanks.
> >> >> >
> >> >> >> >
> >> >> >> > That said - I expected the above piece to be simply a 'return 
> >> >> >> > true;' ;)
> >> >> >> >
> >> >> >> > Usually we use some can_expand_XXX helper in optabs.c to query if 
> >> 

Re: RFC [1/2] divmod transform

2016-05-25 Thread Prathamesh Kulkarni
On 25 May 2016 at 12:52, Richard Biener  wrote:
> On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>
>> On 24 May 2016 at 19:39, Richard Biener  wrote:
>> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
>> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >> >
>> >> >> On 23 May 2016 at 17:35, Richard Biener  
>> >> >> wrote:
>> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>> >> >> >  wrote:
>> >> >> >> Hi,
>> >> >> >> I have updated my patch for divmod (attached), which was originally
>> >> >> >> based on Kugan's patch.
>> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
>> >> >> >> TRUNC_MOD_EXPR
>> >> >> >> having same operands to divmod representation, so we can cse 
>> >> >> >> computation of mod.
>> >> >> >>
>> >> >> >> t1 = a TRUNC_DIV_EXPR b;
>> >> >> >> t2 = a TRUNC_MOD_EXPR b
>> >> >> >> is transformed to:
>> >> >> >> complex_tmp = DIVMOD (a, b);
>> >> >> >> t1 = REALPART_EXPR (complex_tmp);
>> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
>> >> >> >>
>> >> >> >> * New hook divmod_expand_libfunc
>> >> >> >> The rationale for introducing the hook is that different targets 
>> >> >> >> have
>> >> >> >> incompatible calling conventions for divmod libfunc.
>> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
>> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> >> >> >> return quotient and store remainder in argument passed as pointer,
>> >> >> >> while the arm version takes two arguments and returns both
>> >> >> >> quotient and remainder having mode double the size of the operand 
>> >> >> >> mode.
>> >> >> >> The port should hence override the hook expand_divmod_libfunc
>> >> >> >> to generate call to target-specific divmod.
>> >> >> >> Ports should define this hook if:
>> >> >> >> a) The port does not have divmod or div insn for the given mode.
>> >> >> >> b) The port defines divmod libfunc for the given mode.
>> >> >> >> The default hook default_expand_divmod_libfunc() generates call
>> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
>> >> >> >> are of DImode.
>> >> >> >>
>> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> >> >> >> cross-tested on arm*-*-*.
>> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
>> >> >> >> Does this patch look OK ?
>> >> >> >
>> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> >> >> > index 6b4601b..e4a021a 100644
>> >> >> > --- a/gcc/targhooks.c
>> >> >> > +++ b/gcc/targhooks.c
>> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
>> >> >> > machine_mode, optimization_type)
>> >> >> >return true;
>> >> >> >  }
>> >> >> >
>> >> >> > +void
>> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
>> >> >> > +  rtx op0, rtx op1,
>> >> >> > +  rtx *quot_p, rtx *rem_p)
>> >> >> >
>> >> >> > functions need a comment.
>> >> >> >
>> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 
>> >> >> > style?  In that
>> >> >> > case we could avoid the target hook.
>> >> >> Well I would prefer adding the hook because that's more easier -;)
>> >> >> Would it be ok for now to go with the hook ?
>> >> >> >
>> >> >> > +  /* If target overrides expand_divmod_libfunc hook
>> >> >> > +then perform divmod by generating call to the 
>> >> >> > target-specifc divmod
>> >> >> > libfunc.  */
>> >> >> > +  if (targetm.expand_divmod_libfunc != 
>> >> >> > default_expand_divmod_libfunc)
>> >> >> > +   return true;
>> >> >> > +
>> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
>> >> >> > +  return (mode == DImode && unsignedp);
>> >> >> >
>> >> >> > I don't understand this - we know optab_libfunc returns non-NULL for 
>> >> >> > 'mode'
>> >> >> > but still restrict this to DImode && unsigned?  Also if
>> >> >> > targetm.expand_divmod_libfunc
>> >> >> > is not the default we expect the target to handle all modes?
>> >> >> Ah indeed, the check for DImode is unnecessary.
>> >> >> However I suppose the check for unsignedp should be there,
>> >> >> since we want to generate call to __udivmoddi4 only if operand is 
>> >> >> unsigned ?
>> >> >
>> >> > The optab libfunc for sdivmod should be NULL in that case.
>> >> Ah indeed, thanks.
>> >> >
>> >> >> >
>> >> >> > That said - I expected the above piece to be simply a 'return true;' 
>> >> >> > ;)
>> >> >> >
>> >> >> > Usually we use some can_expand_XXX helper in optabs.c to query if 
>> >> >> > the target
>> >> >> > supports a specific operation (for example SImode divmod would use 
>> >> >> > DImode
>> >> >> > divmod by means of widening operands - for the unsigned case of 
>> >> >> > course).
>> >> >> Thanks for pointing out. So if a target does not support divmod

Re: RFC [1/2] divmod transform

2016-05-25 Thread Richard Biener
On Tue, 24 May 2016, Prathamesh Kulkarni wrote:

> On 24 May 2016 at 19:39, Richard Biener  wrote:
> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >
> >> On 24 May 2016 at 17:42, Richard Biener  wrote:
> >> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >> >
> >> >> On 23 May 2016 at 17:35, Richard Biener  
> >> >> wrote:
> >> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
> >> >> >  wrote:
> >> >> >> Hi,
> >> >> >> I have updated my patch for divmod (attached), which was originally
> >> >> >> based on Kugan's patch.
> >> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and 
> >> >> >> TRUNC_MOD_EXPR
> >> >> >> having same operands to divmod representation, so we can cse 
> >> >> >> computation of mod.
> >> >> >>
> >> >> >> t1 = a TRUNC_DIV_EXPR b;
> >> >> >> t2 = a TRUNC_MOD_EXPR b
> >> >> >> is transformed to:
> >> >> >> complex_tmp = DIVMOD (a, b);
> >> >> >> t1 = REALPART_EXPR (complex_tmp);
> >> >> >> t2 = IMAGPART_EXPR (complex_tmp);
> >> >> >>
> >> >> >> * New hook divmod_expand_libfunc
> >> >> >> The rationale for introducing the hook is that different targets have
> >> >> >> incompatible calling conventions for divmod libfunc.
> >> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
> >> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> >> >> >> return quotient and store remainder in argument passed as pointer,
> >> >> >> while the arm version takes two arguments and returns both
> >> >> >> quotient and remainder having mode double the size of the operand 
> >> >> >> mode.
> >> >> >> The port should hence override the hook expand_divmod_libfunc
> >> >> >> to generate call to target-specific divmod.
> >> >> >> Ports should define this hook if:
> >> >> >> a) The port does not have divmod or div insn for the given mode.
> >> >> >> b) The port defines divmod libfunc for the given mode.
> >> >> >> The default hook default_expand_divmod_libfunc() generates call
> >> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
> >> >> >> are of DImode.
> >> >> >>
> >> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> >> >> >> cross-tested on arm*-*-*.
> >> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
> >> >> >> Does this patch look OK ?
> >> >> >
> >> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> >> >> > index 6b4601b..e4a021a 100644
> >> >> > --- a/gcc/targhooks.c
> >> >> > +++ b/gcc/targhooks.c
> >> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
> >> >> > machine_mode, optimization_type)
> >> >> >return true;
> >> >> >  }
> >> >> >
> >> >> > +void
> >> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
> >> >> > +  rtx op0, rtx op1,
> >> >> > +  rtx *quot_p, rtx *rem_p)
> >> >> >
> >> >> > functions need a comment.
> >> >> >
> >> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style? 
> >> >> >  In that
> >> >> > case we could avoid the target hook.
> >> >> Well I would prefer adding the hook because that's more easier -;)
> >> >> Would it be ok for now to go with the hook ?
> >> >> >
> >> >> > +  /* If target overrides expand_divmod_libfunc hook
> >> >> > +then perform divmod by generating call to the target-specifc 
> >> >> > divmod
> >> >> > libfunc.  */
> >> >> > +  if (targetm.expand_divmod_libfunc != 
> >> >> > default_expand_divmod_libfunc)
> >> >> > +   return true;
> >> >> > +
> >> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> >> >> > +  return (mode == DImode && unsignedp);
> >> >> >
> >> >> > I don't understand this - we know optab_libfunc returns non-NULL for 
> >> >> > 'mode'
> >> >> > but still restrict this to DImode && unsigned?  Also if
> >> >> > targetm.expand_divmod_libfunc
> >> >> > is not the default we expect the target to handle all modes?
> >> >> Ah indeed, the check for DImode is unnecessary.
> >> >> However I suppose the check for unsignedp should be there,
> >> >> since we want to generate call to __udivmoddi4 only if operand is 
> >> >> unsigned ?
> >> >
> >> > The optab libfunc for sdivmod should be NULL in that case.
> >> Ah indeed, thanks.
> >> >
> >> >> >
> >> >> > That said - I expected the above piece to be simply a 'return true;' 
> >> >> > ;)
> >> >> >
> >> >> > Usually we use some can_expand_XXX helper in optabs.c to query if the 
> >> >> > target
> >> >> > supports a specific operation (for example SImode divmod would use 
> >> >> > DImode
> >> >> > divmod by means of widening operands - for the unsigned case of 
> >> >> > course).
> >> >> Thanks for pointing out. So if a target does not support divmod
> >> >> libfunc for a mode
> >> >> but for a wider mode, then we could zero-extend operands to the 
> >> >> wider-mode,
> >> >> perform divmod on the wider-mode, and then cast result back 

Re: RFC [1/2] divmod transform

2016-05-24 Thread Prathamesh Kulkarni
On 24 May 2016 at 19:39, Richard Biener  wrote:
> On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>
>> On 24 May 2016 at 17:42, Richard Biener  wrote:
>> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 23 May 2016 at 17:35, Richard Biener  
>> >> wrote:
>> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>> >> >  wrote:
>> >> >> Hi,
>> >> >> I have updated my patch for divmod (attached), which was originally
>> >> >> based on Kugan's patch.
>> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
>> >> >> having same operands to divmod representation, so we can cse 
>> >> >> computation of mod.
>> >> >>
>> >> >> t1 = a TRUNC_DIV_EXPR b;
>> >> >> t2 = a TRUNC_MOD_EXPR b
>> >> >> is transformed to:
>> >> >> complex_tmp = DIVMOD (a, b);
>> >> >> t1 = REALPART_EXPR (complex_tmp);
>> >> >> t2 = IMAGPART_EXPR (complex_tmp);
>> >> >>
>> >> >> * New hook divmod_expand_libfunc
>> >> >> The rationale for introducing the hook is that different targets have
>> >> >> incompatible calling conventions for divmod libfunc.
>> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
>> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> >> >> return quotient and store remainder in argument passed as pointer,
>> >> >> while the arm version takes two arguments and returns both
>> >> >> quotient and remainder having mode double the size of the operand mode.
>> >> >> The port should hence override the hook expand_divmod_libfunc
>> >> >> to generate call to target-specific divmod.
>> >> >> Ports should define this hook if:
>> >> >> a) The port does not have divmod or div insn for the given mode.
>> >> >> b) The port defines divmod libfunc for the given mode.
>> >> >> The default hook default_expand_divmod_libfunc() generates call
>> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
>> >> >> are of DImode.
>> >> >>
>> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> >> >> cross-tested on arm*-*-*.
>> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
>> >> >> Does this patch look OK ?
>> >> >
>> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> >> > index 6b4601b..e4a021a 100644
>> >> > --- a/gcc/targhooks.c
>> >> > +++ b/gcc/targhooks.c
>> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
>> >> > machine_mode, optimization_type)
>> >> >return true;
>> >> >  }
>> >> >
>> >> > +void
>> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
>> >> > +  rtx op0, rtx op1,
>> >> > +  rtx *quot_p, rtx *rem_p)
>> >> >
>> >> > functions need a comment.
>> >> >
>> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  
>> >> > In that
>> >> > case we could avoid the target hook.
>> >> Well I would prefer adding the hook because that's more easier -;)
>> >> Would it be ok for now to go with the hook ?
>> >> >
>> >> > +  /* If target overrides expand_divmod_libfunc hook
>> >> > +then perform divmod by generating call to the target-specifc 
>> >> > divmod
>> >> > libfunc.  */
>> >> > +  if (targetm.expand_divmod_libfunc != 
>> >> > default_expand_divmod_libfunc)
>> >> > +   return true;
>> >> > +
>> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
>> >> > +  return (mode == DImode && unsignedp);
>> >> >
>> >> > I don't understand this - we know optab_libfunc returns non-NULL for 
>> >> > 'mode'
>> >> > but still restrict this to DImode && unsigned?  Also if
>> >> > targetm.expand_divmod_libfunc
>> >> > is not the default we expect the target to handle all modes?
>> >> Ah indeed, the check for DImode is unnecessary.
>> >> However I suppose the check for unsignedp should be there,
>> >> since we want to generate call to __udivmoddi4 only if operand is 
>> >> unsigned ?
>> >
>> > The optab libfunc for sdivmod should be NULL in that case.
>> Ah indeed, thanks.
>> >
>> >> >
>> >> > That said - I expected the above piece to be simply a 'return true;' ;)
>> >> >
>> >> > Usually we use some can_expand_XXX helper in optabs.c to query if the 
>> >> > target
>> >> > supports a specific operation (for example SImode divmod would use 
>> >> > DImode
>> >> > divmod by means of widening operands - for the unsigned case of course).
>> >> Thanks for pointing out. So if a target does not support divmod
>> >> libfunc for a mode
>> >> but for a wider mode, then we could zero-extend operands to the 
>> >> wider-mode,
>> >> perform divmod on the wider-mode, and then cast result back to the
>> >> original mode.
>> >> I haven't done that in this patch, would it be OK to do that as a follow 
>> >> up ?
>> >
>> > I think that you should conservatively handle the div_optab query, thus if
>> > the target has a HW division in a wider mode don't use the divmod IFN.
>> > You'd simply iterate 

Re: RFC [1/2] divmod transform

2016-05-24 Thread Richard Biener
On Tue, 24 May 2016, Prathamesh Kulkarni wrote:

> On 24 May 2016 at 17:42, Richard Biener  wrote:
> > On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
> >
> >> On 23 May 2016 at 17:35, Richard Biener  wrote:
> >> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
> >> >  wrote:
> >> >> Hi,
> >> >> I have updated my patch for divmod (attached), which was originally
> >> >> based on Kugan's patch.
> >> >> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
> >> >> having same operands to divmod representation, so we can cse 
> >> >> computation of mod.
> >> >>
> >> >> t1 = a TRUNC_DIV_EXPR b;
> >> >> t2 = a TRUNC_MOD_EXPR b
> >> >> is transformed to:
> >> >> complex_tmp = DIVMOD (a, b);
> >> >> t1 = REALPART_EXPR (complex_tmp);
> >> >> t2 = IMAGPART_EXPR (complex_tmp);
> >> >>
> >> >> * New hook divmod_expand_libfunc
> >> >> The rationale for introducing the hook is that different targets have
> >> >> incompatible calling conventions for divmod libfunc.
> >> >> Currently three ports define divmod libfunc: c6x, spu and arm.
> >> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> >> >> return quotient and store remainder in argument passed as pointer,
> >> >> while the arm version takes two arguments and returns both
> >> >> quotient and remainder having mode double the size of the operand mode.
> >> >> The port should hence override the hook expand_divmod_libfunc
> >> >> to generate call to target-specific divmod.
> >> >> Ports should define this hook if:
> >> >> a) The port does not have divmod or div insn for the given mode.
> >> >> b) The port defines divmod libfunc for the given mode.
> >> >> The default hook default_expand_divmod_libfunc() generates call
> >> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
> >> >> are of DImode.
> >> >>
> >> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> >> >> cross-tested on arm*-*-*.
> >> >> Bootstrap+test in progress on arm-linux-gnueabihf.
> >> >> Does this patch look OK ?
> >> >
> >> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> >> > index 6b4601b..e4a021a 100644
> >> > --- a/gcc/targhooks.c
> >> > +++ b/gcc/targhooks.c
> >> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
> >> > machine_mode, optimization_type)
> >> >return true;
> >> >  }
> >> >
> >> > +void
> >> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
> >> > +  rtx op0, rtx op1,
> >> > +  rtx *quot_p, rtx *rem_p)
> >> >
> >> > functions need a comment.
> >> >
> >> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  
> >> > In that
> >> > case we could avoid the target hook.
> >> Well I would prefer adding the hook because that's more easier -;)
> >> Would it be ok for now to go with the hook ?
> >> >
> >> > +  /* If target overrides expand_divmod_libfunc hook
> >> > +then perform divmod by generating call to the target-specifc 
> >> > divmod
> >> > libfunc.  */
> >> > +  if (targetm.expand_divmod_libfunc != 
> >> > default_expand_divmod_libfunc)
> >> > +   return true;
> >> > +
> >> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> >> > +  return (mode == DImode && unsignedp);
> >> >
> >> > I don't understand this - we know optab_libfunc returns non-NULL for 
> >> > 'mode'
> >> > but still restrict this to DImode && unsigned?  Also if
> >> > targetm.expand_divmod_libfunc
> >> > is not the default we expect the target to handle all modes?
> >> Ah indeed, the check for DImode is unnecessary.
> >> However I suppose the check for unsignedp should be there,
> >> since we want to generate call to __udivmoddi4 only if operand is unsigned 
> >> ?
> >
> > The optab libfunc for sdivmod should be NULL in that case.
> Ah indeed, thanks.
> >
> >> >
> >> > That said - I expected the above piece to be simply a 'return true;' ;)
> >> >
> >> > Usually we use some can_expand_XXX helper in optabs.c to query if the 
> >> > target
> >> > supports a specific operation (for example SImode divmod would use DImode
> >> > divmod by means of widening operands - for the unsigned case of course).
> >> Thanks for pointing out. So if a target does not support divmod
> >> libfunc for a mode
> >> but for a wider mode, then we could zero-extend operands to the wider-mode,
> >> perform divmod on the wider-mode, and then cast result back to the
> >> original mode.
> >> I haven't done that in this patch, would it be OK to do that as a follow 
> >> up ?
> >
> > I think that you should conservatively handle the div_optab query, thus if
> > the target has a HW division in a wider mode don't use the divmod IFN.
> > You'd simply iterate over GET_MODE_WIDER_MODE and repeat the
> > if (optab_handler (div_optab, mode) != CODE_FOR_nothing) check, bailing
> > out if that is available.
> Done.
> >
> >> > +  /* Disable the transform if 

Re: RFC [1/2] divmod transform

2016-05-24 Thread Prathamesh Kulkarni
On 24 May 2016 at 17:42, Richard Biener  wrote:
> On Tue, 24 May 2016, Prathamesh Kulkarni wrote:
>
>> On 23 May 2016 at 17:35, Richard Biener  wrote:
>> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>> >  wrote:
>> >> Hi,
>> >> I have updated my patch for divmod (attached), which was originally
>> >> based on Kugan's patch.
>> >> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
>> >> having same operands to divmod representation, so we can cse computation 
>> >> of mod.
>> >>
>> >> t1 = a TRUNC_DIV_EXPR b;
>> >> t2 = a TRUNC_MOD_EXPR b
>> >> is transformed to:
>> >> complex_tmp = DIVMOD (a, b);
>> >> t1 = REALPART_EXPR (complex_tmp);
>> >> t2 = IMAGPART_EXPR (complex_tmp);
>> >>
>> >> * New hook divmod_expand_libfunc
>> >> The rationale for introducing the hook is that different targets have
>> >> incompatible calling conventions for divmod libfunc.
>> >> Currently three ports define divmod libfunc: c6x, spu and arm.
>> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> >> return quotient and store remainder in argument passed as pointer,
>> >> while the arm version takes two arguments and returns both
>> >> quotient and remainder having mode double the size of the operand mode.
>> >> The port should hence override the hook expand_divmod_libfunc
>> >> to generate call to target-specific divmod.
>> >> Ports should define this hook if:
>> >> a) The port does not have divmod or div insn for the given mode.
>> >> b) The port defines divmod libfunc for the given mode.
>> >> The default hook default_expand_divmod_libfunc() generates call
>> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
>> >> are of DImode.
>> >>
>> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> >> cross-tested on arm*-*-*.
>> >> Bootstrap+test in progress on arm-linux-gnueabihf.
>> >> Does this patch look OK ?
>> >
>> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> > index 6b4601b..e4a021a 100644
>> > --- a/gcc/targhooks.c
>> > +++ b/gcc/targhooks.c
>> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
>> > machine_mode, optimization_type)
>> >return true;
>> >  }
>> >
>> > +void
>> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
>> > +  rtx op0, rtx op1,
>> > +  rtx *quot_p, rtx *rem_p)
>> >
>> > functions need a comment.
>> >
>> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  In 
>> > that
>> > case we could avoid the target hook.
>> Well I would prefer adding the hook because that's more easier -;)
>> Would it be ok for now to go with the hook ?
>> >
>> > +  /* If target overrides expand_divmod_libfunc hook
>> > +then perform divmod by generating call to the target-specifc 
>> > divmod
>> > libfunc.  */
>> > +  if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc)
>> > +   return true;
>> > +
>> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
>> > +  return (mode == DImode && unsignedp);
>> >
>> > I don't understand this - we know optab_libfunc returns non-NULL for 'mode'
>> > but still restrict this to DImode && unsigned?  Also if
>> > targetm.expand_divmod_libfunc
>> > is not the default we expect the target to handle all modes?
>> Ah indeed, the check for DImode is unnecessary.
>> However I suppose the check for unsignedp should be there,
>> since we want to generate call to __udivmoddi4 only if operand is unsigned ?
>
> The optab libfunc for sdivmod should be NULL in that case.
Ah indeed, thanks.
>
>> >
>> > That said - I expected the above piece to be simply a 'return true;' ;)
>> >
>> > Usually we use some can_expand_XXX helper in optabs.c to query if the 
>> > target
>> > supports a specific operation (for example SImode divmod would use DImode
>> > divmod by means of widening operands - for the unsigned case of course).
>> Thanks for pointing out. So if a target does not support divmod
>> libfunc for a mode
>> but for a wider mode, then we could zero-extend operands to the wider-mode,
>> perform divmod on the wider-mode, and then cast result back to the
>> original mode.
>> I haven't done that in this patch, would it be OK to do that as a follow up ?
>
> I think that you should conservatively handle the div_optab query, thus if
> the target has a HW division in a wider mode don't use the divmod IFN.
> You'd simply iterate over GET_MODE_WIDER_MODE and repeat the
> if (optab_handler (div_optab, mode) != CODE_FOR_nothing) check, bailing
> out if that is available.
Done.
>
>> > +  /* Disable the transform if either is a constant, since
>> > division-by-constant
>> > + may have specialized expansion.  */
>> > +  if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2))
>> > +return false;
>> >
>> > please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2)
>> >
>> > +  if (TYPE_OVERFLOW_TRAPS 

Re: RFC [1/2] divmod transform

2016-05-24 Thread Richard Biener
On Tue, 24 May 2016, Prathamesh Kulkarni wrote:

> On 23 May 2016 at 17:35, Richard Biener  wrote:
> > On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
> >  wrote:
> >> Hi,
> >> I have updated my patch for divmod (attached), which was originally
> >> based on Kugan's patch.
> >> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
> >> having same operands to divmod representation, so we can cse computation 
> >> of mod.
> >>
> >> t1 = a TRUNC_DIV_EXPR b;
> >> t2 = a TRUNC_MOD_EXPR b
> >> is transformed to:
> >> complex_tmp = DIVMOD (a, b);
> >> t1 = REALPART_EXPR (complex_tmp);
> >> t2 = IMAGPART_EXPR (complex_tmp);
> >>
> >> * New hook divmod_expand_libfunc
> >> The rationale for introducing the hook is that different targets have
> >> incompatible calling conventions for divmod libfunc.
> >> Currently three ports define divmod libfunc: c6x, spu and arm.
> >> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> >> return quotient and store remainder in argument passed as pointer,
> >> while the arm version takes two arguments and returns both
> >> quotient and remainder having mode double the size of the operand mode.
> >> The port should hence override the hook expand_divmod_libfunc
> >> to generate call to target-specific divmod.
> >> Ports should define this hook if:
> >> a) The port does not have divmod or div insn for the given mode.
> >> b) The port defines divmod libfunc for the given mode.
> >> The default hook default_expand_divmod_libfunc() generates call
> >> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
> >> are of DImode.
> >>
> >> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> >> cross-tested on arm*-*-*.
> >> Bootstrap+test in progress on arm-linux-gnueabihf.
> >> Does this patch look OK ?
> >
> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> > index 6b4601b..e4a021a 100644
> > --- a/gcc/targhooks.c
> > +++ b/gcc/targhooks.c
> > @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
> > machine_mode, optimization_type)
> >return true;
> >  }
> >
> > +void
> > +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
> > +  rtx op0, rtx op1,
> > +  rtx *quot_p, rtx *rem_p)
> >
> > functions need a comment.
> >
> > ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  In 
> > that
> > case we could avoid the target hook.
> Well I would prefer adding the hook because that's more easier -;)
> Would it be ok for now to go with the hook ?
> >
> > +  /* If target overrides expand_divmod_libfunc hook
> > +then perform divmod by generating call to the target-specifc divmod
> > libfunc.  */
> > +  if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc)
> > +   return true;
> > +
> > +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> > +  return (mode == DImode && unsignedp);
> >
> > I don't understand this - we know optab_libfunc returns non-NULL for 'mode'
> > but still restrict this to DImode && unsigned?  Also if
> > targetm.expand_divmod_libfunc
> > is not the default we expect the target to handle all modes?
> Ah indeed, the check for DImode is unnecessary.
> However I suppose the check for unsignedp should be there,
> since we want to generate call to __udivmoddi4 only if operand is unsigned ?

The optab libfunc for sdivmod should be NULL in that case.

> >
> > That said - I expected the above piece to be simply a 'return true;' ;)
> >
> > Usually we use some can_expand_XXX helper in optabs.c to query if the target
> > supports a specific operation (for example SImode divmod would use DImode
> > divmod by means of widening operands - for the unsigned case of course).
> Thanks for pointing out. So if a target does not support divmod
> libfunc for a mode
> but for a wider mode, then we could zero-extend operands to the wider-mode,
> perform divmod on the wider-mode, and then cast result back to the
> original mode.
> I haven't done that in this patch, would it be OK to do that as a follow up ?

I think that you should conservatively handle the div_optab query, thus if
the target has a HW division in a wider mode don't use the divmod IFN.
You'd simply iterate over GET_MODE_WIDER_MODE and repeat the
if (optab_handler (div_optab, mode) != CODE_FOR_nothing) check, bailing
out if that is available.

> > +  /* Disable the transform if either is a constant, since
> > division-by-constant
> > + may have specialized expansion.  */
> > +  if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2))
> > +return false;
> >
> > please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2)
> >
> > +  if (TYPE_OVERFLOW_TRAPS (type))
> > +return false;
> >
> > why's that?  Generally please first test cheap things (trapping, 
> > constant-ness)
> > before checking expensive stuff (target_supports_divmod_p).
> I added TYPE_OVERFLOW_TRAPS (type) 

Re: RFC [1/2] divmod transform

2016-05-24 Thread Prathamesh Kulkarni
On 23 May 2016 at 17:35, Richard Biener  wrote:
> On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
>  wrote:
>> Hi,
>> I have updated my patch for divmod (attached), which was originally
>> based on Kugan's patch.
>> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
>> having same operands to divmod representation, so we can cse computation of 
>> mod.
>>
>> t1 = a TRUNC_DIV_EXPR b;
>> t2 = a TRUNC_MOD_EXPR b
>> is transformed to:
>> complex_tmp = DIVMOD (a, b);
>> t1 = REALPART_EXPR (complex_tmp);
>> t2 = IMAGPART_EXPR (complex_tmp);
>>
>> * New hook divmod_expand_libfunc
>> The rationale for introducing the hook is that different targets have
>> incompatible calling conventions for divmod libfunc.
>> Currently three ports define divmod libfunc: c6x, spu and arm.
>> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
>> return quotient and store remainder in argument passed as pointer,
>> while the arm version takes two arguments and returns both
>> quotient and remainder having mode double the size of the operand mode.
>> The port should hence override the hook expand_divmod_libfunc
>> to generate call to target-specific divmod.
>> Ports should define this hook if:
>> a) The port does not have divmod or div insn for the given mode.
>> b) The port defines divmod libfunc for the given mode.
>> The default hook default_expand_divmod_libfunc() generates call
>> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
>> are of DImode.
>>
>> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
>> cross-tested on arm*-*-*.
>> Bootstrap+test in progress on arm-linux-gnueabihf.
>> Does this patch look OK ?
>
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 6b4601b..e4a021a 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
> machine_mode, optimization_type)
>return true;
>  }
>
> +void
> +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
> +  rtx op0, rtx op1,
> +  rtx *quot_p, rtx *rem_p)
>
> functions need a comment.
>
> ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  In that
> case we could avoid the target hook.
Well I would prefer adding the hook because that's more easier -;)
Would it be ok for now to go with the hook ?
>
> +  /* If target overrides expand_divmod_libfunc hook
> +then perform divmod by generating call to the target-specifc divmod
> libfunc.  */
> +  if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc)
> +   return true;
> +
> +  /* Fall back to using libgcc2.c:__udivmoddi4.  */
> +  return (mode == DImode && unsignedp);
>
> I don't understand this - we know optab_libfunc returns non-NULL for 'mode'
> but still restrict this to DImode && unsigned?  Also if
> targetm.expand_divmod_libfunc
> is not the default we expect the target to handle all modes?
Ah indeed, the check for DImode is unnecessary.
However I suppose the check for unsignedp should be there,
since we want to generate call to __udivmoddi4 only if operand is unsigned ?
>
> That said - I expected the above piece to be simply a 'return true;' ;)
>
> Usually we use some can_expand_XXX helper in optabs.c to query if the target
> supports a specific operation (for example SImode divmod would use DImode
> divmod by means of widening operands - for the unsigned case of course).
Thanks for pointing out. So if a target does not support divmod
libfunc for a mode
but for a wider mode, then we could zero-extend operands to the wider-mode,
perform divmod on the wider-mode, and then cast result back to the
original mode.
I haven't done that in this patch, would it be OK to do that as a follow up ?
>
> +  /* Disable the transform if either is a constant, since
> division-by-constant
> + may have specialized expansion.  */
> +  if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2))
> +return false;
>
> please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2)
>
> +  if (TYPE_OVERFLOW_TRAPS (type))
> +return false;
>
> why's that?  Generally please first test cheap things (trapping, 
> constant-ness)
> before checking expensive stuff (target_supports_divmod_p).
I added TYPE_OVERFLOW_TRAPS (type) based on your suggestion in:
https://www.mail-archive.com/gcc@gcc.gnu.org/msg78534.html
"When looking at TRUNC_DIV_EXPR you should also exclude
the case where TYPE_OVERFLOW_TRAPS (type) as that should
expand using the [su]divv optabs (no trapping overflow
divmod optab exists)."
>
> +static bool
> +convert_to_divmod (gassign *stmt)
> +{
> +  if (!divmod_candidate_p (stmt))
> +return false;
> +
> +  tree op1 = gimple_assign_rhs1 (stmt);
> +  tree op2 = gimple_assign_rhs2 (stmt);
> +
> +  vec stmts = vNULL;
>
> use an auto_vec  - you currently leak it in at least one place.
>
> +  if (maybe_clean_or_replace_eh_stmt 

Re: RFC [1/2] divmod transform

2016-05-23 Thread Richard Biener
On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni
 wrote:
> Hi,
> I have updated my patch for divmod (attached), which was originally
> based on Kugan's patch.
> The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
> having same operands to divmod representation, so we can cse computation of 
> mod.
>
> t1 = a TRUNC_DIV_EXPR b;
> t2 = a TRUNC_MOD_EXPR b
> is transformed to:
> complex_tmp = DIVMOD (a, b);
> t1 = REALPART_EXPR (complex_tmp);
> t2 = IMAGPART_EXPR (complex_tmp);
>
> * New hook divmod_expand_libfunc
> The rationale for introducing the hook is that different targets have
> incompatible calling conventions for divmod libfunc.
> Currently three ports define divmod libfunc: c6x, spu and arm.
> c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
> return quotient and store remainder in argument passed as pointer,
> while the arm version takes two arguments and returns both
> quotient and remainder having mode double the size of the operand mode.
> The port should hence override the hook expand_divmod_libfunc
> to generate call to target-specific divmod.
> Ports should define this hook if:
> a) The port does not have divmod or div insn for the given mode.
> b) The port defines divmod libfunc for the given mode.
> The default hook default_expand_divmod_libfunc() generates call
> to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
> are of DImode.
>
> Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
> cross-tested on arm*-*-*.
> Bootstrap+test in progress on arm-linux-gnueabihf.
> Does this patch look OK ?

diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 6b4601b..e4a021a 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode,
machine_mode, optimization_type)
   return true;
 }

+void
+default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
+  rtx op0, rtx op1,
+  rtx *quot_p, rtx *rem_p)

functions need a comment.

ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style?  In that
case we could avoid the target hook.

+  /* If target overrides expand_divmod_libfunc hook
+then perform divmod by generating call to the target-specifc divmod
libfunc.  */
+  if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc)
+   return true;
+
+  /* Fall back to using libgcc2.c:__udivmoddi4.  */
+  return (mode == DImode && unsignedp);

I don't understand this - we know optab_libfunc returns non-NULL for 'mode'
but still restrict this to DImode && unsigned?  Also if
targetm.expand_divmod_libfunc
is not the default we expect the target to handle all modes?

That said - I expected the above piece to be simply a 'return true;' ;)

Usually we use some can_expand_XXX helper in optabs.c to query if the target
supports a specific operation (for example SImode divmod would use DImode
divmod by means of widening operands - for the unsigned case of course).

+  /* Disable the transform if either is a constant, since
division-by-constant
+ may have specialized expansion.  */
+  if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2))
+return false;

please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2)

+  if (TYPE_OVERFLOW_TRAPS (type))
+return false;

why's that?  Generally please first test cheap things (trapping, constant-ness)
before checking expensive stuff (target_supports_divmod_p).

+static bool
+convert_to_divmod (gassign *stmt)
+{
+  if (!divmod_candidate_p (stmt))
+return false;
+
+  tree op1 = gimple_assign_rhs1 (stmt);
+  tree op2 = gimple_assign_rhs2 (stmt);
+
+  vec stmts = vNULL;

use an auto_vec  - you currently leak it in at least one place.

+  if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
+   cfg_changed = true;

note that this suggests you should check whether any of the stmts may throw
internally as you don't update / transfer EH info correctly.  So for 'stmt' and
all 'use_stmt' check stmt_can_throw_internal and if so do not add it to
the list of stmts to modify.

Btw, I think you should not add 'stmt' immediately but when iterating over
all uses also gather uses in TRUNC_MOD_EXPR.

Otherwise looks ok.

Thanks,
Richard.

> Thanks,
> Prathamesh


RFC [1/2] divmod transform

2016-05-23 Thread Prathamesh Kulkarni
Hi,
I have updated my patch for divmod (attached), which was originally
based on Kugan's patch.
The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR
having same operands to divmod representation, so we can cse computation of mod.

t1 = a TRUNC_DIV_EXPR b;
t2 = a TRUNC_MOD_EXPR b
is transformed to:
complex_tmp = DIVMOD (a, b);
t1 = REALPART_EXPR (complex_tmp);
t2 = IMAGPART_EXPR (complex_tmp);

* New hook divmod_expand_libfunc
The rationale for introducing the hook is that different targets have
incompatible calling conventions for divmod libfunc.
Currently three ports define divmod libfunc: c6x, spu and arm.
c6x and spu follow the convention of libgcc2.c:__udivmoddi4:
return quotient and store remainder in argument passed as pointer,
while the arm version takes two arguments and returns both
quotient and remainder having mode double the size of the operand mode.
The port should hence override the hook expand_divmod_libfunc
to generate call to target-specific divmod.
Ports should define this hook if:
a) The port does not have divmod or div insn for the given mode.
b) The port defines divmod libfunc for the given mode.
The default hook default_expand_divmod_libfunc() generates call
to libgcc2.c:__udivmoddi4 provided the operands are unsigned and
are of DImode.

Patch passes bootstrap+test on x86_64-unknown-linux-gnu and
cross-tested on arm*-*-*.
Bootstrap+test in progress on arm-linux-gnueabihf.
Does this patch look OK ?

Thanks,
Prathamesh
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 8c7f2a1..111f19f 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6963,6 +6963,12 @@ This is firstly introduced on ARM/AArch64 targets, 
please refer to
 the hook implementation for how different fusion types are supported.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_EXPAND_DIVMOD_LIBFUNC (bool 
@var{unsignedp}, machine_mode @var{mode}, @var{rtx}, @var{rtx}, rtx 
*@var{quot}, rtx *@var{rem})
+Define this hook if the port does not have hardware div and divmod insn for
+the given mode but has divmod libfunc, which is incompatible
+with libgcc2.c:__udivmoddi4
+@end deftypefn
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index f963a58..2c9a800 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4848,6 +4848,8 @@ them: try the first ones in this list first.
 
 @hook TARGET_SCHED_FUSION_PRIORITY
 
+@hook TARGET_EXPAND_DIVMOD_LIBFUNC
+
 @node Sections
 @section Dividing the Output into Sections (Texts, Data, @dots{})
 @c the above section title is WAY too long.  maybe cut the part between
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index c867ddc..0cb59f7 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -2276,6 +2276,48 @@ multi_vector_optab_supported_p (convert_optab optab, 
tree_pair types,
 #define direct_mask_store_optab_supported_p direct_optab_supported_p
 #define direct_store_lanes_optab_supported_p multi_vector_optab_supported_p
 
+/* Expand DIVMOD() using:
+ a) optab handler for udivmod/sdivmod if it is available.
+ b) If optab_handler doesn't exist, Generate call to
+optab_libfunc for udivmod/sdivmod.  */
+
+static void 
+expand_DIVMOD (internal_fn, gcall *stmt)
+{
+  tree lhs = gimple_call_lhs (stmt);
+  tree arg0 = gimple_call_arg (stmt, 0);
+  tree arg1 = gimple_call_arg (stmt, 1);
+
+  gcc_assert (TREE_CODE (TREE_TYPE (lhs)) == COMPLEX_TYPE);
+  tree type = TREE_TYPE (TREE_TYPE (lhs));
+  machine_mode mode = TYPE_MODE (type);
+  bool unsignedp = TYPE_UNSIGNED (type);
+  optab tab = (unsignedp) ? udivmod_optab : sdivmod_optab;
+
+  rtx op0 = expand_normal (arg0);
+  rtx op1 = expand_normal (arg1);
+  rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
+
+  rtx quotient, remainder;
+
+  /* Check if optab handler exists for [u]divmod.  */
+  if (optab_handler (tab, mode) != CODE_FOR_nothing)
+{
+  quotient = gen_reg_rtx (mode);
+  remainder = gen_reg_rtx (mode);
+  expand_twoval_binop (tab, op0, op1, quotient, remainder, unsignedp);
+}
+  else
+targetm.expand_divmod_libfunc (unsignedp, mode, op0, op1,
+  , );
+
+  /* Wrap the return value (quotient, remainder) within COMPLEX_EXPR.  */
+  expand_expr (build2 (COMPLEX_EXPR, TREE_TYPE (lhs),
+  make_tree (TREE_TYPE (arg0), quotient),
+  make_tree (TREE_TYPE (arg1), remainder)),
+  target, VOIDmode, EXPAND_NORMAL);
+}
+
 /* Return true if FN is supported for the types in TYPES when the
optimization type is OPT_TYPE.  The types are those associated with
the "type0" and "type1" fields of FN's direct_internal_fn_info
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index e729d85..56a80f1 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -194,6 +194,9 @@ DEF_INTERNAL_FN (ATOMIC_BIT_TEST_AND_SET,