diff mbox

RFC [1/2] divmod transform

Message ID CAAgBjMnuUt+wJPGZJwAhRhVmXT8jDk-0Ee3aBv=azzrX2qNh3w@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Aug. 13, 2016, 11:43 a.m. UTC 
On 13 August 2016 at 16:56, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 28 July 2016 at 19:05, Prathamesh Kulkarni

> <prathamesh.kulkarni@linaro.org> wrote:

>> On 8 June 2016 at 19:53, Richard Biener <rguenther@suse.de> wrote:

>>> On Fri, 3 Jun 2016, Jim Wilson wrote:

>>>

>>>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener <rguenther@suse.de> 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  <mark@codesourcery.com>

>>>>

>>>>         * 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 mbox

Patch

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),
-                                         op1, GET_MODE (op1),
-                                         address, GET_MODE (address));
-
-  *quot_p = quotient;
-  *rem_p = remainder;
+  gcc_unreachable (); 
 }
 
 
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index ad32744..933db67 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -3808,9 +3808,13 @@  target_supports_divmod_p (optab divmod_optab, optab div_optab, machine_mode mode
   if (optab_handler (divmod_optab, mode) != CODE_FOR_nothing)
     return true;
 
-  /* Check if libfunc for divmod is available.  */
-  rtx libfunc = optab_libfunc (divmod_optab, mode);
-  if (libfunc != NULL_RTX)
+  /* Check if target-specific divmod libfunc is available.
+     If target overrides expand_divmod_libfunc, then it *has to*
+     set_optab_libfunc (divmod_optab, mode) to target-specific divmod
+     libfunc or NULL for unsupported modes.  */ 
+  
+  if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc
+      && optab_libfunc (divmod_optab, mode))
     {
       /* If optab_handler exists for div_optab, perhaps in a wider mode,
 	 we don't want to use the libfunc even if it exists for given mode.  */ 
@@ -3820,12 +3824,7 @@  target_supports_divmod_p (optab divmod_optab, optab div_optab, machine_mode mode
 	if (optab_handler (div_optab, div_mode) != CODE_FOR_nothing)
 	  return false;
 
-      /* FIXME: This is a hack to workaround an issue with optab_libfunc().
-	 optab_libfunc (sdivmod_optab, DImode) returns libfunc "__divmoddi4",
-	 although __divmoddi4() does not exist in libgcc. For now, enable the
-	 transform only if libfunc is guaranteed to be __udivmoddi4.  */
-      return (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc
-	     || !strcmp (XSTR (libfunc, 0), "__udivmoddi4"));
+      return true;
     }
 
   return false;