Patchwork Enhance ifcombine to recover non short circuit branches

login
register
mail settings
Submitter Steven Bosscher
Date Nov. 8, 2013, 5:41 p.m.
Message ID <CABu31nMkjb7SWFKPVGp1RaJYqjON36GL-AWb_O1Zd=PmvCkuNg@mail.gmail.com>
Download mbox | patch
Permalink /patch/289898/
State New
Headers show

Comments

Steven Bosscher - Nov. 8, 2013, 5:41 p.m.
On Fri, Nov 8, 2013 at 6:20 PM, Steven Bosscher wrote:
> On Wed, Oct 30, 2013 at 5:03 AM, Andrew Pinski wrote:
>> Here is what I applied in the end; Jeff told me just to remove the
>> testcase.  I added the comment trying to explain why it was the
>> opposite order of PHI-opt.
>>
>> Thanks,
>> Andrew Pinski
>>
>> ChangeLog:
>> * tree-ssa-ifcombine.c: Include rtl.h and tm_p.h.
>
> Eh, why???
>
> The file has this comment:
>
> 25    /* rtl is needed only because arm back-end requires it for
> 26       BRANCH_COST.  */
> 27    #include "rtl.h"
> 28    #include "tm_p.h"
>
> Can you please clarify why this is not something to be fixed in the
> ARM back end?


Could be fixed like attached. Can you please have a look and
foster-parent it if you like it?

Thanks,

Ciao!
Steven
* config/arm/arm-protos.h (arm_branch_cost,
	arm_logical_op_non_short_circuit): New prototypes.
	* config/arm/arm.c (arm_branch_cost): Implement it.
	(arm_logical_op_non_short_circuit): Likewise.
	* config/arm/arm.h (BRANCH_COST): Don't expose ARM back-end
	internals to the RTL middle end.
	(LOGICAL_OP_NON_SHORT_CIRCUIT): Likewise.
	* tree-ssa-ifcombine.c: Don't include rtl.h here.
	* tree-ssa-reassoc.c: Don't include rtl.h here either.
Richard Guenther - Nov. 11, 2013, 11:41 a.m.
On Fri, Nov 8, 2013 at 6:41 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Fri, Nov 8, 2013 at 6:20 PM, Steven Bosscher wrote:
>> On Wed, Oct 30, 2013 at 5:03 AM, Andrew Pinski wrote:
>>> Here is what I applied in the end; Jeff told me just to remove the
>>> testcase.  I added the comment trying to explain why it was the
>>> opposite order of PHI-opt.
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>> ChangeLog:
>>> * tree-ssa-ifcombine.c: Include rtl.h and tm_p.h.
>>
>> Eh, why???
>>
>> The file has this comment:
>>
>> 25    /* rtl is needed only because arm back-end requires it for
>> 26       BRANCH_COST.  */
>> 27    #include "rtl.h"
>> 28    #include "tm_p.h"
>>
>> Can you please clarify why this is not something to be fixed in the
>> ARM back end?
>
>
> Could be fixed like attached. Can you please have a look and
> foster-parent it if you like it?

Looks good to me - Richard, can you pick it up?

Thanks,
Richard.

> Thanks,
>
> Ciao!
> Steven

Patch

Index: config/arm/arm-protos.h
===================================================================
--- config/arm/arm-protos.h	(revision 204565)
+++ config/arm/arm-protos.h	(working copy)
@@ -288,4 +288,7 @@  extern bool arm_autoinc_modes_ok_p (enum machine_m
 
 extern void arm_emit_eabi_attribute (const char *, int, int);
 
+extern bool arm_branch_cost (bool, bool);
+extern bool arm_logical_op_non_short_circuit (void);
+
 #endif /* ! GCC_ARM_PROTOS_H */
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 204565)
+++ config/arm/arm.c	(working copy)
@@ -30351,4 +30351,19 @@  arm_asan_shadow_offset (void)
   return (unsigned HOST_WIDE_INT) 1 << 29;
 }
 
+/* Implement BRANCH_COST as a function, to hide tune_params from the
+   rest of the compiler.  */
+bool
+arm_branch_cost (bool speed_p, bool predictable_p)
+{
+  return current_tune->branch_cost (speed_p, predictable_p);
+}
+
+/* Likewise for LOGICAL_OP_NON_SHORT_CIRCUIT.  */
+bool
+arm_logical_op_non_short_circuit (void)
+{
+  return (current_tune->logical_op_non_short_circuit[TARGET_ARM]);
+}
+
 #include "gt-arm.h"
Index: config/arm/arm.h
===================================================================
--- config/arm/arm.h	(revision 204565)
+++ config/arm/arm.h	(working copy)
@@ -2042,14 +2042,13 @@  enum arm_auto_incmodes
 /* Try to generate sequences that don't involve branches, we can then use
    conditional instructions.  */
 #define BRANCH_COST(speed_p, predictable_p) \
-  (current_tune->branch_cost (speed_p, predictable_p))
+  arm_branch_cost (speed_p, predictable_p)
 
 /* False if short circuit operation is preferred.  */
 #define LOGICAL_OP_NON_SHORT_CIRCUIT				\
   ((optimize_size)						\
    ? (TARGET_THUMB ? false : true)				\
-   : (current_tune->logical_op_non_short_circuit[TARGET_ARM]))
-
+   : arm_logical_op_non_short_circuit ()
 
 /* Position Independent Code.  */
 /* We decide which register to use based on the compilation options and
Index: tree-ssa-ifcombine.c
===================================================================
--- tree-ssa-ifcombine.c	(revision 204565)
+++ tree-ssa-ifcombine.c	(working copy)
@@ -22,9 +22,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
-/* rtl is needed only because arm back-end requires it for
-   BRANCH_COST.  */
-#include "rtl.h"
 #include "tm_p.h"
 #include "tree.h"
 #include "basic-block.h"
Index: tree-ssa-reassoc.c
===================================================================
--- tree-ssa-reassoc.c	(revision 204565)
+++ tree-ssa-reassoc.c	(working copy)
@@ -23,7 +23,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "hash-table.h"
 #include "tm.h"
-#include "rtl.h"
 #include "tm_p.h"
 #include "tree.h"
 #include "basic-block.h"