RFC [2/3] divmod transform v2 - override expand_divmod_libfunc for ARM port

Message ID CAAgBjMnE8Ek5=bK=i5gHvfZMBX94v5BjVF+=+rDKEY9iseLmFg@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Oct. 16, 2016, 6 a.m.
Hi,
This patch overrides expand_divmod_libfunc hook for ARM port.
I separated the SImode tests into separate file from DImode tests
because certain arm configs (cortex-15) have hardware div insn for
SImode but not for DImode, and for that config we want SImode tests to
be disabled but not DImode tests. The patch therefore has two
target-effective checks: divmod and divmod_simode.
Cross-tested on arm*-*-*.
OK to commit ?

Thanks,
Prathamesh
2016-10-15  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>
	    Kugan Vivekanandarajah  <kuganv@linaro.org>
	    Jim Wilson  <jim.wilson@linaro.org>

	* config/arm/arm.c (arm_expand_divmod_libfunc): Override hook
	TARGET_EXPAND_DIVMOD_LIBFUNC.
	* doc/sourcebuild.texi: Add items for arm_divmod_simode, divmod,
	divmod_simode.

testsuite/
	* lib/target-supports.exp (check_effective_target_divmod): New.
	(check_effective_target_divmod_simode): Likewise. 
	(check_effective_target_arm_divmod_simode): Likewise.

Comments

Prathamesh Kulkarni Oct. 24, 2016, 3:37 p.m. | #1
On 16 October 2016 at 11:30, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> Hi,

> This patch overrides expand_divmod_libfunc hook for ARM port.

> I separated the SImode tests into separate file from DImode tests

> because certain arm configs (cortex-15) have hardware div insn for

> SImode but not for DImode, and for that config we want SImode tests to

> be disabled but not DImode tests. The patch therefore has two

> target-effective checks: divmod and divmod_simode.

> Cross-tested on arm*-*-*.

> OK to commit ?

ping https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01240.html

Thanks,
Prathamesh
>

> Thanks,

> Prathamesh
Kyrill Tkachov Oct. 26, 2016, 1:21 p.m. | #2
On 16/10/16 07:00, Prathamesh Kulkarni wrote:
> Hi,

> This patch overrides expand_divmod_libfunc hook for ARM port.

> I separated the SImode tests into separate file from DImode tests

> because certain arm configs (cortex-15) have hardware div insn for

> SImode but not for DImode, and for that config we want SImode tests to

> be disabled but not DImode tests. The patch therefore has two

> target-effective checks: divmod and divmod_simode.

> Cross-tested on arm*-*-*.

> OK to commit ?


Looks ok to me, the implementation of the hook is straightforward though
I have a question.
arm_expand_divmod_libfunc is not supposed to ever be called for SImode TARGET_IDIV.
It asserts it rather than just failing the expansion in some way.
How does the midend know not to call TARGET_EXPAND_DIVMOD_LIBFUNC in that case, does it
just check if the relevant sdiv optab is not available?

If so, this is ok for trunk assuming a bootstrap and test run on arm-none-linux-gnueabihf
shows no issues. Would be good to try one for --with-cpu=cortex-a15 and one with a !TARGET_IDIV
target, say --with-cpu=cortex-a9.

Sorry for the delay.

Thanks,
Kyrill


> Thanks,

> Prathamesh
Prathamesh Kulkarni Oct. 27, 2016, 1:31 p.m. | #3
On 26 October 2016 at 18:51, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
>

> On 16/10/16 07:00, Prathamesh Kulkarni wrote:

>>

>> Hi,

>> This patch overrides expand_divmod_libfunc hook for ARM port.

>> I separated the SImode tests into separate file from DImode tests

>> because certain arm configs (cortex-15) have hardware div insn for

>> SImode but not for DImode, and for that config we want SImode tests to

>> be disabled but not DImode tests. The patch therefore has two

>> target-effective checks: divmod and divmod_simode.

>> Cross-tested on arm*-*-*.

>> OK to commit ?

>

>

> Looks ok to me, the implementation of the hook is straightforward though

> I have a question.

> arm_expand_divmod_libfunc is not supposed to ever be called for SImode

> TARGET_IDIV.

> It asserts it rather than just failing the expansion in some way.

> How does the midend know not to call TARGET_EXPAND_DIVMOD_LIBFUNC in that

> case, does it

> just check if the relevant sdiv optab is not available?

Yes. The divmod transform isn't enabled if target supports hardware
div in the same
or wider mode even if divmod libfunc is available for the given mode.
>

> If so, this is ok for trunk assuming a bootstrap and test run on

> arm-none-linux-gnueabihf

> shows no issues. Would be good to try one for --with-cpu=cortex-a15 and one

> with a !TARGET_IDIV

> target, say --with-cpu=cortex-a9.

Bootstrap+tested on arm-linux-gnueabihf --with-cpu=cortex-a15 and
--with-cpu=cortex-a9.
Also cross-tested on arm*-*-*.
OK to commit ?

Thanks,
Prathamesh
>

> Sorry for the delay.

>

> Thanks,

> Kyrill

>

>

>> Thanks,

>> Prathamesh

>

>
Kyrill Tkachov Oct. 28, 2016, 7:52 a.m. | #4
On 27/10/16 14:31, Prathamesh Kulkarni wrote:
> On 26 October 2016 at 18:51, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:

>> On 16/10/16 07:00, Prathamesh Kulkarni wrote:

>>> Hi,

>>> This patch overrides expand_divmod_libfunc hook for ARM port.

>>> I separated the SImode tests into separate file from DImode tests

>>> because certain arm configs (cortex-15) have hardware div insn for

>>> SImode but not for DImode, and for that config we want SImode tests to

>>> be disabled but not DImode tests. The patch therefore has two

>>> target-effective checks: divmod and divmod_simode.

>>> Cross-tested on arm*-*-*.

>>> OK to commit ?

>>

>> Looks ok to me, the implementation of the hook is straightforward though

>> I have a question.

>> arm_expand_divmod_libfunc is not supposed to ever be called for SImode

>> TARGET_IDIV.

>> It asserts it rather than just failing the expansion in some way.

>> How does the midend know not to call TARGET_EXPAND_DIVMOD_LIBFUNC in that

>> case, does it

>> just check if the relevant sdiv optab is not available?

> Yes. The divmod transform isn't enabled if target supports hardware

> div in the same

> or wider mode even if divmod libfunc is available for the given mode.

>> If so, this is ok for trunk assuming a bootstrap and test run on

>> arm-none-linux-gnueabihf

>> shows no issues. Would be good to try one for --with-cpu=cortex-a15 and one

>> with a !TARGET_IDIV

>> target, say --with-cpu=cortex-a9.

> Bootstrap+tested on arm-linux-gnueabihf --with-cpu=cortex-a15 and

> --with-cpu=cortex-a9.

> Also cross-tested on arm*-*-*.

> OK to commit ?


Yes, thanks.
Kyrill

> Thanks,

> Prathamesh

>> Sorry for the delay.

>>

>> Thanks,

>> Kyrill

>>

>>

>>> Thanks,

>>> Prathamesh

>>

Patch hide | download patch | download mbox

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 39e3aa8..b1a6aeb 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -62,6 +62,7 @@ 
 #include "builtins.h"
 #include "tm-constrs.h"
 #include "rtl-iter.h"
+#include "optabs-libfuncs.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -304,6 +305,7 @@  static section *arm_function_section (tree, enum node_frequency, bool, bool);
 static bool arm_asm_elf_flags_numeric (unsigned int flags, unsigned int *num);
 static unsigned int arm_elf_section_type_flags (tree decl, const char *name,
 						int reloc);
+static void arm_expand_divmod_libfunc (rtx, machine_mode, rtx, rtx, rtx *, rtx *);
 
 /* Table of machine attributes.  */
 static const struct attribute_spec arm_attribute_table[] =
@@ -739,6 +741,9 @@  static const struct attribute_spec arm_attribute_table[] =
 #undef TARGET_SECTION_TYPE_FLAGS
 #define TARGET_SECTION_TYPE_FLAGS arm_elf_section_type_flags
 
+#undef TARGET_EXPAND_DIVMOD_LIBFUNC
+#define TARGET_EXPAND_DIVMOD_LIBFUNC arm_expand_divmod_libfunc
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 /* Obstack for minipool constant handling.  */
@@ -30776,4 +30781,33 @@  arm_elf_section_type_flags (tree decl, const char *name, int reloc)
   return flags;
 }
 
+/* Generate call to __aeabi_[mode]divmod (op0, op1).  */
+
+static void
+arm_expand_divmod_libfunc (rtx libfunc, machine_mode mode,
+			   rtx op0, rtx op1,
+			   rtx *quot_p, rtx *rem_p)
+{
+  if (mode == SImode)
+    gcc_assert (!TARGET_IDIV);
+
+  machine_mode libval_mode = smallest_mode_for_size (2 * GET_MODE_BITSIZE (mode),
+						     MODE_INT);
+
+  rtx libval = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
+					libval_mode, 2,
+					op0, GET_MODE (op0),
+					op1, GET_MODE (op1));
+
+  rtx quotient = simplify_gen_subreg (mode, libval, libval_mode, 0);
+  rtx remainder = simplify_gen_subreg (mode, libval, libval_mode,
+				       GET_MODE_SIZE (mode));
+
+  gcc_assert (quotient);
+  gcc_assert (remainder);
+
+  *quot_p = quotient;
+  *rem_p = remainder;
+}
+
 #include "gt-arm.h"
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 07c75e2..39de0ff 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1675,6 +1675,10 @@  and @code{MOVT} instructions available.
 ARM target generates Thumb-1 code for @code{-mthumb} with
 @code{CBZ} and @code{CBNZ} instructions available.
 
+@item arm_divmod_simode
+ARM target for which divmod transform is disabled, if it supports hardware
+div instruction.
+
 @end table
 
 @subsubsection AArch64-specific attributes
@@ -1848,6 +1852,13 @@  Target requires a command line argument to enable a SIMD instruction set.
 
 @item pie_copyreloc
 The x86-64 target linker supports PIE with copy reloc.
+
+@item divmod
+Target supporting hardware divmod insn or divmod libcall.
+
+@item divmod_simode
+Target supporting hardware divmod insn or divmod libcall for SImode.
+
 @end table
 
 @subsubsection Environment attributes
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 201ed4b..fc5e37f 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8070,3 +8070,41 @@  proc check_effective_target_profile_update_atomic {} {
 	int main (void) { return 0; }
     } "-fprofile-update=atomic -fprofile-generate"]
 }
+
+#For versions of ARM architectures that have hardware div insn,
+#disable the divmod transform
+
+proc check_effective_target_arm_divmod_simode { } {
+    return [check_no_compiler_messages arm_divmod assembly {
+	#ifdef __ARM_ARCH_EXT_IDIV__
+	#error has div insn
+	#endif
+	int i;
+    }]
+}
+
+# Return 1 if target supports divmod hardware insn or divmod libcall.
+
+proc check_effective_target_divmod { } {
+    #TODO: Add checks for all targets that have either hardware divmod insn
+    # or define libfunc for divmod.
+    if { [istarget arm*-*-*]
+	 || [istarget x86_64-*-*] } {
+	return 1
+    }
+    return 0
+}
+
+# Return 1 if target supports divmod for SImode. The reason for
+# separating this from check_effective_target_divmod is that
+# some versions of ARM architecture define div instruction
+# only for simode, and for these archs, we do not want to enable
+# divmod transform for simode.
+
+proc check_effective_target_divmod_simode { } {
+    if { [istarget arm*-*-*] } {
+	return [check_effective_target_arm_divmod_simode]
+    }
+
+    return [check_effective_target_divmod]
+}