Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Jeff Law]

On 06/12/14 08:34, Christian Bruel wrote:

On 06/11/2014 02:00 PM, Christian Bruel wrote:

On 06/11/2014 06:17 AM, Joern Rennecke wrote:

Joern, is this new target macro interface OK with you ?

Yes, this interface should allow me to do switches between rounding
and truncating
floating-point modes with an add/subtract immediate.

However, the implentation, as posted, doesn't work - it causes memory
corruption.

It appears to work with the attached amendment patch.


Indeed,  thanks for pointing out the bad reusing of the aux field
between multiple entities.

In fact rereading this part of the implementation, I find the allocation
of aux*n_entities awkward. A simpler setting in the entity loop to carry
the mode directly into eg-aux is possible without array allocation
(which also fixes a memory leak by the way).


Here is the revised version fixing the aforementioned issue found by
Joern on Epiphany. It also simplifies the allocation of the aux edges
field to carry the modes.

Now that everyone agrees on the interface, is this OK for trunk ?

bootstrapped/regtested for X86 and SH4a.

thanks,

Christian







toggle.patch


2014-06-12  Christian Bruelchristian.br...@st.com

* mode-switching.c (struct bb_info): Add mode_out, mode_in caches.
(make_preds_opaque): Delete.
(clear_mode_bit, mode_bit_p, set_mode_bit): New macros.
(commit_mode_sets): New function.
(optimize_mode_switching): Handle current_mode to mode_switching_emit.
Process all modes at once.
* basic-block.h (pre_edge_lcm_avs): Declare.
* lcm.c (pre_edge_lcm_avs): Renamed from pre_edge_lcm.
Call clear_aux_for_edges. Fix comments.
(pre_edge_lcm): New wrapper function to call pre_edge_lcm_avs.
(pre_edge_rev_lcm): Idem.
* config/epiphany/epiphany.c (emit_set_fp_mode): Add prev_mode 
parameter.
* config/epiphany/epiphany-protos.h (emit_set_fp_mode): Idem.
* config/epiphany/resolve-sw-modes.c (pass_resolve_sw_modes::execute): 
Idem.
* config/i386/i386.c (x96_emit_mode_set): Idem.
* config/sh/sh.c (sh_emit_mode_set): Likewise. Handle PR toggle.
* config/sh/sh.md (toggle_pr):  Defined if TARGET_FPU_SINGLE.
(fpscr_toggle) Disallow from delay slot.
* target.def (emit_mode_set): Add prev_mode parameter.
* doc/tm.texi: Regenerate.

2014-06-12  Christian Bruelchristian.br...@st.com

* gcc.target/sh/fpchg.c: New test.

This is fine for the trunk.

Thanks for your patience,
Jeff

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

On 06/11/2014 02:00 PM, Christian Bruel wrote:
 On 06/11/2014 06:17 AM, Joern Rennecke wrote:
 Joern, is this new target macro interface OK with you ?
 Yes, this interface should allow me to do switches between rounding
 and truncating
 floating-point modes with an add/subtract immediate.

 However, the implentation, as posted, doesn't work - it causes memory
 corruption.

 It appears to work with the attached amendment patch.

 Indeed,  thanks for pointing out the bad reusing of the aux field
 between multiple entities.

 In fact rereading this part of the implementation, I find the allocation
 of aux*n_entities awkward. A simpler setting in the entity loop to carry
 the mode directly into eg-aux is possible without array allocation
 (which also fixes a memory leak by the way).


Here is the revised version fixing the aforementioned issue found by
Joern on Epiphany. It also simplifies the allocation of the aux edges
field to carry the modes.

Now that everyone agrees on the interface, is this OK for trunk ?

bootstrapped/regtested for X86 and SH4a.

thanks,

Christian






2014-06-12  Christian Bruel  christian.br...@st.com

	* mode-switching.c (struct bb_info): Add mode_out, mode_in caches.
	(make_preds_opaque): Delete.
	(clear_mode_bit, mode_bit_p, set_mode_bit): New macros.
	(commit_mode_sets): New function.
	(optimize_mode_switching): Handle current_mode to mode_switching_emit.
	Process all modes at once.
	* basic-block.h (pre_edge_lcm_avs): Declare.
	* lcm.c (pre_edge_lcm_avs): Renamed from pre_edge_lcm.
	Call clear_aux_for_edges. Fix comments.
	(pre_edge_lcm): New wrapper function to call pre_edge_lcm_avs.
	(pre_edge_rev_lcm): Idem.
	* config/epiphany/epiphany.c (emit_set_fp_mode): Add prev_mode parameter.
	* config/epiphany/epiphany-protos.h (emit_set_fp_mode): Idem.
	* config/epiphany/resolve-sw-modes.c (pass_resolve_sw_modes::execute): Idem.
	* config/i386/i386.c (x96_emit_mode_set): Idem.
	* config/sh/sh.c (sh_emit_mode_set): Likewise. Handle PR toggle.
	* config/sh/sh.md (toggle_pr): 	Defined if TARGET_FPU_SINGLE.
	(fpscr_toggle) Disallow from delay slot.
	* target.def (emit_mode_set): Add prev_mode parameter.
	* doc/tm.texi: Regenerate.

2014-06-12  Christian Bruel  christian.br...@st.com

	* gcc.target/sh/fpchg.c: New test.

Index: gcc/basic-block.h
===
--- gcc/basic-block.h	(revision 211436)
+++ gcc/basic-block.h	(working copy)
@@ -711,6 +711,9 @@ extern void bitmap_union_of_preds (sbitmap, sbitma
 extern struct edge_list *pre_edge_lcm (int, sbitmap *, sbitmap *,
    sbitmap *, sbitmap *, sbitmap **,
    sbitmap **);
+extern struct edge_list *pre_edge_lcm_avs (int, sbitmap *, sbitmap *,
+	   sbitmap *, sbitmap *, sbitmap *,
+	   sbitmap *, sbitmap **, sbitmap **);
 extern struct edge_list *pre_edge_rev_lcm (int, sbitmap *,
 	   sbitmap *, sbitmap *,
 	   sbitmap *, sbitmap **,
Index: gcc/config/epiphany/epiphany-protos.h
===
--- gcc/config/epiphany/epiphany-protos.h	(revision 211436)
+++ gcc/config/epiphany/epiphany-protos.h	(working copy)
@@ -40,7 +40,8 @@ extern int epiphany_initial_elimination_offset (in
 extern void epiphany_init_expanders (void);
 extern int hard_regno_mode_ok (int regno, enum machine_mode mode);
 #ifdef HARD_CONST
-extern void emit_set_fp_mode (int entity, int mode, HARD_REG_SET regs_live);
+extern void emit_set_fp_mode (int entity, int mode, int prev_mode,
+			  HARD_REG_SET regs_live);
 #endif
 extern void epiphany_insert_mode_switch_use (rtx insn, int, int);
 extern void epiphany_expand_set_fp_mode (rtx *operands);
Index: gcc/config/epiphany/epiphany.c
===
--- gcc/config/epiphany/epiphany.c	(revision 211436)
+++ gcc/config/epiphany/epiphany.c	(working copy)
@@ -2543,7 +2543,8 @@ epiphany_mode_exit (int entity)
 }
 
 void
-emit_set_fp_mode (int entity, int mode, HARD_REG_SET regs_live ATTRIBUTE_UNUSED)
+emit_set_fp_mode (int entity, int mode, int prev_mode ATTRIBUTE_UNUSED,
+		  HARD_REG_SET regs_live ATTRIBUTE_UNUSED)
 {
   rtx save_cc, cc_reg, mask, src, src2;
   enum attr_fp_mode fp_mode;
Index: gcc/config/epiphany/resolve-sw-modes.c
===
--- gcc/config/epiphany/resolve-sw-modes.c	(revision 211436)
+++ gcc/config/epiphany/resolve-sw-modes.c	(working copy)
@@ -170,7 +170,7 @@ pass_resolve_sw_modes::execute (function *fun)
 	}
 	  start_sequence ();
 	  emit_set_fp_mode (EPIPHANY_MSW_ENTITY_ROUND_UNKNOWN,
-			jilted_mode, NULL);
+			jilted_mode, FP_MODE_NONE, NULL);
 	  seq = get_insns ();
 	  end_sequence ();
 	  need_commit = true;
Index: gcc/config/i386/i386.c
===
--- gcc/config/i386/i386.c	(revision 211436)
+++ gcc/config/i386/i386.c	(working copy)
@@ -16447,7 +16447,8 @@ ix86_avx_emit_vzeroupper 

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

On 06/11/2014 06:17 AM, Joern Rennecke wrote:

 Joern, is this new target macro interface OK with you ?
 Yes, this interface should allow me to do switches between rounding
 and truncating
 floating-point modes with an add/subtract immediate.

 However, the implentation, as posted, doesn't work - it causes memory
 corruption.

 It appears to work with the attached amendment patch.


Indeed,  thanks for pointing out the bad reusing of the aux field
between multiple entities.

In fact rereading this part of the implementation, I find the allocation
of aux*n_entities awkward. A simpler setting in the entity loop to carry
the mode directly into eg-aux is possible without array allocation
(which also fixes a memory leak by the way).

cheers,

Christian

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

On 06/10/2014 04:03 PM, Joern Rennecke wrote:
 On 13 May 2014 22:41, Oleg Endo oleg.e...@t-online.de wrote:

 Right.  I was thinking to add FPSCR.SZ mode switching to SH, in order to
 do float vector moves.  For that SZ and PR need to be switched both at
 the same time (only SH4A has both, fpchg and fschg).  So basically I'd
 add another mode entity, which would emit SZ mode changes in addition to
 the PR mode changes.  But then adjacent FPSCR-changing insns could be
 combined ... any idea/suggestion how to accomplish that?
 If they are sufficiently adjacent, you can use a peephole2 pattern for this.

 I see Cristian's patch addresses this in a different way - keeping size and
 precision in the same entity, and emitting toggles as appropriate.

yes, I was only interested to optimize the SH4a case when PR=1 with a
good enough implementation. To cover all the other possibilities a new
entity would be better. But then as you say recombining them might be
difficult.  An alternate hackish way could be to have a singe entity
with 4 modes covering all PR*SZ combinations).

but I'm not sure that covering the case where PR=0 SZ=1 worth it, maybe
code size only, ? as the 64 move would be implemented as 2*32 moves anyway,


 The problem get's a bit more interesting if you have some instruction patterns
 that care about one setting but not the other.
 Describing this exactly allows lazy code motion to be a bit more lazy, but 
 OTOH
 it can make it harder to combine mode switching instructions if you
 still want to
 do that.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Oleg Endo]

On 11 Jun 2014, at 14:40, Christian Bruel christian.br...@st.com wrote:

 
 On 06/10/2014 04:03 PM, Joern Rennecke wrote:
 On 13 May 2014 22:41, Oleg Endo oleg.e...@t-online.de wrote:
 
 Right.  I was thinking to add FPSCR.SZ mode switching to SH, in order to
 do float vector moves.  For that SZ and PR need to be switched both at
 the same time (only SH4A has both, fpchg and fschg).  So basically I'd
 add another mode entity, which would emit SZ mode changes in addition to
 the PR mode changes.  But then adjacent FPSCR-changing insns could be
 combined ... any idea/suggestion how to accomplish that?
 If they are sufficiently adjacent, you can use a peephole2 pattern for this.
 
 I see Cristian's patch addresses this in a different way - keeping size and
 precision in the same entity, and emitting toggles as appropriate.
 
 yes, I was only interested to optimize the SH4a case when PR=1 with a
 good enough implementation. To cover all the other possibilities a new
 entity would be better. But then as you say recombining them might be
 difficult.  An alternate hackish way could be to have a singe entity
 with 4 modes covering all PR*SZ combinations).
 
 but I'm not sure that covering the case where PR=0 SZ=1 worth it, maybe
 code size only, ? as the 64 move would be implemented as 2*32 moves anyway,

I was thinking of using PR=0,SZ=1 mode (i.e. fmov.d) for float vector 
loads/stores.  But at the moment that's a pie in the sky.

Cheers,
Oleg

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 13 May 2014 22:41, Oleg Endo oleg.e...@t-online.de wrote:

 Right.  I was thinking to add FPSCR.SZ mode switching to SH, in order to
 do float vector moves.  For that SZ and PR need to be switched both at
 the same time (only SH4A has both, fpchg and fschg).  So basically I'd
 add another mode entity, which would emit SZ mode changes in addition to
 the PR mode changes.  But then adjacent FPSCR-changing insns could be
 combined ... any idea/suggestion how to accomplish that?

If they are sufficiently adjacent, you can use a peephole2 pattern for this.

I see Cristian's patch addresses this in a different way - keeping size and
precision in the same entity, and emitting toggles as appropriate.

The problem get's a bit more interesting if you have some instruction patterns
that care about one setting but not the other.
Describing this exactly allows lazy code motion to be a bit more lazy, but OTOH
it can make it harder to combine mode switching instructions if you
still want to
do that.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 2 June 2014 13:34, Christian Bruel christian.br...@st.com wrote:
 Hello,

 Any feedback for this ? I'd like to commit only when OK for Epiphany.

 Joern, is this new target macro interface OK with you ?

Yes, this interface should allow me to do switches between rounding
and truncating
floating-point modes with an add/subtract immediate.

However, the implentation, as posted, doesn't work - it causes memory
corruption.

It appears to work with the attached amendment patch.

=== gcc Summary ===

# of expected passes82184
# of unexpected failures41
# of unexpected successes   1
# of expected failures  90
# of unresolved testcases   2
# of unsupported tests  1585
/ssd/adapteva/bld-epiphany/gcc/xgcc  version 4.10.0 20140608
(experimental) (Epiphany toolchain (built 20140610))

This is the same as before applying the patch(es).


tmp
Description: Binary data

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

Hello,

Any feedback for this ? I'd like to commit only when OK for Epiphany.

many thanks,

Christian

On 05/26/2014 05:32 PM, Christian Bruel wrote:
 On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html
 Sorry, I only saw the first part and thought I' d need to wait till I
 see the second part - and I somehow missed that.

 I think the previous known mode should be passed to the
 TARGET_MODE_EMIT hook - no need to have extra hooks
 for toggling, and, as I mentioned earlier, fixating on the toggle is
 actually an SH artifact - other ports have multi-way
 modes settings that can benefit from knowing the previous mode.
 OK I'll change the bool toggle parameter by the previous mode in
 TARGET_MODE_EMIT and remove the bool XOR hooks in the SH description.

 Hello,

 This is the interface for targets that could use the previous mode for
 switching. TARGET_MODE_EMIT now takes a new prev_mode parameter. If the
 previous mode cannot be determined, MODE_NONE (value depends on  the
 entity) is used.

 The implementation is less trivial than just supporting a boolean toggle
 bit, as the previous modes information have to be carried along the
 edges. For this I recycle the auxiliary edge field that is made
 unnecessary by the removal of make_pred_opaque and a change in the
 implementation to call LCM for every modes from every identity
 simultaneously. This idea was suggested by Joern in PR29349.  Another
 speed improvement is that we process the modes to no_mode instead of
 max_num_modes for each entity.
 Thanks to all this, the only additional data to support prev_mode is
 that for each BB, the avin/avout lcm computation are cached inside the
 bb_info mode_in/mode_out fields,  the xor toggle bit handling  could
 have been removed.

 bootstrapped/regtested for x86 and sh4, sh4a, sh4a-single,
 epiphany build is OK. testsuite not ran.

 Joern, is this new target macro interface OK with you ? Jeff, (or other
 RTL maintainer) since this is a new implementation for
 optimize_mode_switching I suppose your previous approval doesn't held
 anymore... is this new one OK for trunk as well ?
 No change for x86/sh4/2a interfaces.

 Many thanks

 Christian

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

 On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html
 Sorry, I only saw the first part and thought I' d need to wait till I
 see the second part - and I somehow missed that.

 I think the previous known mode should be passed to the
 TARGET_MODE_EMIT hook - no need to have extra hooks
 for toggling, and, as I mentioned earlier, fixating on the toggle is
 actually an SH artifact - other ports have multi-way
 modes settings that can benefit from knowing the previous mode.
 OK I'll change the bool toggle parameter by the previous mode in
 TARGET_MODE_EMIT and remove the bool XOR hooks in the SH description.

Hello,

This is the interface for targets that could use the previous mode for
switching. TARGET_MODE_EMIT now takes a new prev_mode parameter. If the
previous mode cannot be determined, MODE_NONE (value depends on  the
entity) is used.

The implementation is less trivial than just supporting a boolean toggle
bit, as the previous modes information have to be carried along the
edges. For this I recycle the auxiliary edge field that is made
unnecessary by the removal of make_pred_opaque and a change in the
implementation to call LCM for every modes from every identity
simultaneously. This idea was suggested by Joern in PR29349.  Another
speed improvement is that we process the modes to no_mode instead of
max_num_modes for each entity.
Thanks to all this, the only additional data to support prev_mode is
that for each BB, the avin/avout lcm computation are cached inside the
bb_info mode_in/mode_out fields,  the xor toggle bit handling  could
have been removed.

bootstrapped/regtested for x86 and sh4, sh4a, sh4a-single,
epiphany build is OK. testsuite not ran.

Joern, is this new target macro interface OK with you ? Jeff, (or other
RTL maintainer) since this is a new implementation for
optimize_mode_switching I suppose your previous approval doesn't held
anymore... is this new one OK for trunk as well ?
No change for x86/sh4/2a interfaces.

Many thanks

Christian


2014-05-23  Christian Bruel  christian.br...@st.com

	* mode-switching.c (struct bb_info): Add mode_out, mode_in caches.
	(make_preds_opaque): Delete function.
	(clear_mode_bit, mode_bit_p, set_mode_bit): New macros.
	(add_mode_set, get_mode_set, alloc_mode_aux, free_modes_edges): New functions.
	(commit_mode_sets): New function.
	(optimize_mode_switching): Handle current_mode to mode_switching_emit.
	Process all modes at once. 
	* basic-block.h (pre_edge_lcm_avs): Declare.
	* lcm.c (pre_edge_lcm_avs): Renamed from pre_edge_lcm.
	Call clear_aux_for_edges. Fix comments.
	(pre_edge_lcm): New wrapper function to call pre_edge_lcm_avs.
	(pre_edge_rev_lcm): Idem.
	* config/epiphany/epiphany.c (emit_set_fp_mode): Add prev_mode parameter.
	* config/epiphany/epiphany-protos.h (emit_set_fp_mode): Idem.
	* config/epiphany/resolve-sw-modes.c (pass_resolve_sw_modes::execute): Idem.
	* config/i386/i386.c (x96_emit_mode_set): Idem.
	* config/sh/sh.c (sh_emit_mode_set): Likewise. Handle PR toggle.
	* config/sh/sh.md (toggle_pr): 	Defined if TARGET_FPU_SINGLE.
	(fpscr_toggle) Disallow from delay slot.
	* target.def (emit_mode_set): Add prev_mode parameter.
	* doc/tm.texi: Regenerate.

2014-05-19  Christian Bruel  christian.br...@st.com

	* gcc.target/sh/fpchg.c: New test.

Index: gcc/basic-block.h
===
--- gcc/basic-block.h	(revision 210845)
+++ gcc/basic-block.h	(working copy)
@@ -711,6 +711,9 @@ extern void bitmap_union_of_preds (sbitmap, sbitma
 extern struct edge_list *pre_edge_lcm (int, sbitmap *, sbitmap *,
    sbitmap *, sbitmap *, sbitmap **,
    sbitmap **);
+extern struct edge_list *pre_edge_lcm_avs (int, sbitmap *, sbitmap *,
+	   sbitmap *, sbitmap *, sbitmap *,
+	   sbitmap *, sbitmap **, sbitmap **);
 extern struct edge_list *pre_edge_rev_lcm (int, sbitmap *,
 	   sbitmap *, sbitmap *,
 	   sbitmap *, sbitmap **,
Index: gcc/config/epiphany/epiphany-protos.h
===
--- gcc/config/epiphany/epiphany-protos.h	(revision 210845)
+++ gcc/config/epiphany/epiphany-protos.h	(working copy)
@@ -40,7 +40,8 @@ extern int epiphany_initial_elimination_offset (in
 extern void epiphany_init_expanders (void);
 extern int hard_regno_mode_ok (int regno, enum machine_mode mode);
 #ifdef HARD_CONST
-extern void emit_set_fp_mode (int entity, int mode, HARD_REG_SET regs_live);
+extern void emit_set_fp_mode (int entity, int mode, int prev_mode,
+			  HARD_REG_SET regs_live);
 #endif
 extern void epiphany_insert_mode_switch_use (rtx insn, int, int);
 extern void epiphany_expand_set_fp_mode (rtx *operands);
Index: gcc/config/epiphany/epiphany.c

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 12 May 2014 23:39, Oleg Endo oleg.e...@t-online.de wrote:

 This is the same as changing/setting the FP modes (PR, SZ) on SH while
 preserving the other FPSCR bits, or did I miss something?

It's more like if you have to control multiple bits at once to get a
specific mode.
Say you have to turn SZ off and PR on.  You you knew that only one bit needs
changing, you can do with one less arithmetic operation.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Oleg Endo]

On Tue, 2014-05-13 at 09:10 +0100, Joern Rennecke wrote:
 On 12 May 2014 23:39, Oleg Endo oleg.e...@t-online.de wrote:
 
  This is the same as changing/setting the FP modes (PR, SZ) on SH while
  preserving the other FPSCR bits, or did I miss something?
 
 It's more like if you have to control multiple bits at once to get a
 specific mode.
 Say you have to turn SZ off and PR on.  You you knew that only one bit needs
 changing, you can do with one less arithmetic operation.

Right.  I was thinking to add FPSCR.SZ mode switching to SH, in order to
do float vector moves.  For that SZ and PR need to be switched both at
the same time (only SH4A has both, fpchg and fschg).  So basically I'd
add another mode entity, which would emit SZ mode changes in addition to
the PR mode changes.  But then adjacent FPSCR-changing insns could be
combined ... any idea/suggestion how to accomplish that?

Cheers,
Oleg

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

Hello,

I'd still wish to ping for the following set of patches. Those changes
does not impact other targets than SH4 but, as suggested by Joern, I
have hooked the macros and moved the SH4A specific support to the target
parts (so a different target can eventually implement other models than
dual mode).

Patch2 only does very little restructuring  but if is not interesting
enough for all targets, patch 1 should not be that intrusive.

For RTL middle end and (X86, SH, Epiphany) target reviewers,

Many thanks,

Christian

On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html

 Many thanks

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

Just saw the Jeff's approval for the RTL part. Sorry for the crossed answers

remains the target maintainers.  Joern, Kaz ?

Many thanks.

Christian

On 05/12/2014 10:44 AM, Christian Bruel wrote:
 Hello,

 I'd still wish to ping for the following set of patches. Those changes
 does not impact other targets than SH4 but, as suggested by Joern, I
 have hooked the macros and moved the SH4A specific support to the target
 parts (so a different target can eventually implement other models than
 dual mode).

 Patch2 only does very little restructuring  but if is not interesting
 enough for all targets, patch 1 should not be that intrusive.

 For RTL middle end and (X86, SH, Epiphany) target reviewers,

 Many thanks,

 Christian

 On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html

 Many thanks

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Kaz Kojima]

[I'd like to add Oleg to CC list.]

Christian Bruel christian.br...@st.com wrote:
 Just saw the Jeff's approval for the RTL part. Sorry for the crossed answers
 
 remains the target maintainers.  Joern, Kaz ?

SH specific part looks OK to me.  Oleg, could you comment on
the patch?

Regards,
kaz

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 12 May 2014 10:06, Christian Bruel christian.br...@st.com wrote:
 Just saw the Jeff's approval for the RTL part. Sorry for the crossed answers

 remains the target maintainers.  Joern, Kaz ?

 Many thanks.

 Christian

 On 05/12/2014 10:44 AM, Christian Bruel wrote:
 Hello,

 I'd still wish to ping for the following set of patches. Those changes
 does not impact other targets than SH4 but, as suggested by Joern, I
 have hooked the macros and moved the SH4A specific support to the target
 parts (so a different target can eventually implement other models than
 dual mode).

 Patch2 only does very little restructuring  but if is not interesting
 enough for all targets, patch 1 should not be that intrusive.

 For RTL middle end and (X86, SH, Epiphany) target reviewers,

 Many thanks,

 Christian

 On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html

Sorry, I only saw the first part and thought I' d need to wait till I
see the second part - and I somehow missed that.

I think the previous known mode should be passed to the
TARGET_MODE_EMIT hook - no need to have extra hooks
for toggling, and, as I mentioned earlier, fixating on the toggle is
actually an SH artifact - other ports have multi-way
modes settings that can benefit from knowing the previous mode.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Christian Bruel]

On 04/28/2014 10:08 AM, Christian Bruel wrote:
 Hello,

 I'd like to ping the following patches

 [Hookize mode-switching]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01003.html

 [Add new hooks to support toggle and SH4A fpchg instruction]
 http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01005.html
 Sorry, I only saw the first part and thought I' d need to wait till I
 see the second part - and I somehow missed that.

 I think the previous known mode should be passed to the
 TARGET_MODE_EMIT hook - no need to have extra hooks
 for toggling, and, as I mentioned earlier, fixating on the toggle is
 actually an SH artifact - other ports have multi-way
 modes settings that can benefit from knowing the previous mode.

OK I'll change the bool toggle parameter by the previous mode in
TARGET_MODE_EMIT and remove the bool XOR hooks in the SH description.
Just for my curiosity, which other targets have multi-way toggling
support ?

I'll commit the first patch as approved and re post the second one.

Many thanks,

Christian

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 12 May 2014 13:16, Christian Bruel christian.br...@st.com wrote:

 Just for my curiosity, which other targets have multi-way toggling
 support ?

The epiphany has, sort of: you read a control register, AND and/or OR
some mask(s) to the value,
and write it back.
If we knew the previous mode, we might elide and AND or an OR.

I think this is actually quite a common issue.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Joern Rennecke]

On 12 May 2014 13:51, Joern Rennecke joern.renne...@embecosm.com wrote:
 On 12 May 2014 13:16, Christian Bruel christian.br...@st.com wrote:

 Just for my curiosity, which other targets have multi-way toggling
 support ?

 The epiphany has, sort of: you read a control register, AND and/or OR
 some mask(s) to the value,
 and write it back.
 If we knew the previous mode, we might elide and AND or an OR.

 I think this is actually quite a common issue.

P.S.: In some cases, multiple modes input could still be handled if we
knew that certain other modes
don't appear in the input, so a more powerful interface than providing
the previous mode - if known,
is to provide a set of potential predecessor modes.
The case where we don't know anything then obviously is represented as
the full base set.
In the mode switching infrastructure, you can just calculate the union
of the incoming (potential) mode(s) from each incoming edge.

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Oleg Endo]

On Mon, 2014-05-12 at 19:32 +0900, Kaz Kojima wrote:
 [I'd like to add Oleg to CC list.]
 
 Christian Bruel christian.br...@st.com wrote:
  Just saw the Jeff's approval for the RTL part. Sorry for the crossed answers
  
  remains the target maintainers.  Joern, Kaz ?
 
 SH specific part looks OK to me.  Oleg, could you comment on
 the patch?

Looks OK to me with the 'previous mode vs. toggle' change mentioned by
Jörn here http://gcc.gnu.org/ml/gcc-patches/2014-05/msg00754.html

Except one naming thing (please rename, also the existing ones):
sh4_mode_exit - sh_mode_exit
sh4_toggle_init - sh_mode_toggle_init
sh4_toggle_destroy - sh_mode_toggle_destroy
sh4_toggle_set - sh_mode_toggle_set
sh4_toggle_test - sh_mode_toggle_test

Other SH variants also have modes.  E.g. SH2A FPU mode switching is also
done through the sh4_* functions, which is a bit confusing when reading.


A few cosmetic nits:

sh.c:
+static sbitmap *mode_in_flip;  /* flip in mode status for each basic blocks.  
*/
+static sbitmap *mode_out_flip; /* flip out mode status for each basic blocks.  
*/
^
'basic block'


mode-switching.c:
+   if (mode != num_modes[e] 
+   mode != targetm.mode_switching.exit (e))

The '' should go to the beginning of the 2nd line, AFAIK.

+  gcc_assert ((targetm.mode_switching.entry  targetm.mode_switching.exit) ||
+ (!targetm.mode_switching.entry  !targetm.mode_switching.exit));

Likewise.


tm.texi:
+@deftypefn {Target Hook} void TARGET_MODE_TOGGLE_SET (sbitmap *@var{avin}, 
sbitmap *@var{avout})
+Hook called by the mode switching pass to record the modes needed for each 
entities in entry and exit of each basic block.
+@end deftypefn

'for each entity'

Re: [PING*2][PATCH] Extend mode-switching to support toggle (1/2)

[Oleg Endo]

On Mon, 2014-05-12 at 13:51 +0100, Joern Rennecke wrote:
 On 12 May 2014 13:16, Christian Bruel christian.br...@st.com wrote:
 
  Just for my curiosity, which other targets have multi-way toggling
  support ?
 
 The epiphany has, sort of: you read a control register, AND and/or OR
 some mask(s) to the value,
 and write it back.
 If we knew the previous mode, we might elide and AND or an OR.
 
 I think this is actually quite a common issue.

This is the same as changing/setting the FP modes (PR, SZ) on SH while
preserving the other FPSCR bits, or did I miss something?

Cheers,
Oleg