* [PATCH 1/3] MIPS: add -mcompact-branches=prefer option
@ 2021-02-05 9:53 YunQiang Su
2021-02-05 9:53 ` [PATCH 2/3] MIPS: add builtime option for -mcompact-branches YunQiang Su
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: YunQiang Su @ 2021-02-05 9:53 UTC (permalink / raw)
To: gcc-patches; +Cc: syq, macro, jiaxun.yang, YunQiang Su
For MIPSr6, we may wish to use compact-branches only.
Currently, we have to use `always' option, while it is mark as conflict
with pre-R6.
cc1: error: unsupported combination: ‘mips32r2’ -mcompact-branches=always
It make some trouble for distributions to make -mcompact-branches=always
default for R6 only.
The new added `prefer' option:
just ignored by pre-R6 target.
do the same as `always' for R6+.
---
gcc/config/mips/mips-opts.h | 3 ++-
gcc/config/mips/mips.h | 6 ++++--
gcc/config/mips/mips.opt | 3 +++
gcc/doc/invoke.texi | 6 ++++++
gcc/testsuite/gcc.target/mips/compact-branches-8.c | 10 ++++++++++
gcc/testsuite/gcc.target/mips/compact-branches-9.c | 10 ++++++++++
6 files changed, 35 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-8.c
create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-9.c
diff --git a/gcc/config/mips/mips-opts.h b/gcc/config/mips/mips-opts.h
index 6214849f3e1..f3804b9722b 100644
--- a/gcc/config/mips/mips-opts.h
+++ b/gcc/config/mips/mips-opts.h
@@ -51,6 +51,7 @@ enum mips_r10k_cache_barrier_setting {
enum mips_cb_setting {
MIPS_CB_NEVER,
MIPS_CB_OPTIMAL,
- MIPS_CB_ALWAYS
+ MIPS_CB_ALWAYS,
+ MIPS_CB_PREFER
};
#endif
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index b4a60a55d80..f8762fe6638 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -106,20 +106,22 @@ struct mips_cpu_info {
'never' policy or the 'optimal' policy on a core that lacks
compact branch instructions. */
#define TARGET_CB_NEVER (mips_cb == MIPS_CB_NEVER \
- || (mips_cb == MIPS_CB_OPTIMAL \
+ || ((mips_cb == MIPS_CB_OPTIMAL || mips_cb == MIPS_CB_PREFER) \
&& !ISA_HAS_COMPACT_BRANCHES))
/* Compact branches may be used if the user either selects the
'always' policy or the 'optimal' policy on a core that supports
compact branch instructions. */
#define TARGET_CB_MAYBE (TARGET_CB_ALWAYS \
- || (mips_cb == MIPS_CB_OPTIMAL \
+ || ((mips_cb == MIPS_CB_OPTIMAL || mips_cb == MIPS_CB_PREFER) \
&& ISA_HAS_COMPACT_BRANCHES))
/* Compact branches must always be generated if the user selects
the 'always' policy or the 'optimal' policy om a core that
lacks delay slot branch instructions. */
#define TARGET_CB_ALWAYS (mips_cb == MIPS_CB_ALWAYS \
+ || (mips_cb == MIPS_CB_PREFER \
+ && ISA_HAS_COMPACT_BRANCHES) \
|| (mips_cb == MIPS_CB_OPTIMAL \
&& !ISA_HAS_DELAY_SLOTS))
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 6af8037e9bd..f2d7550e36c 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -464,6 +464,9 @@ Enum(mips_cb_setting) String(optimal) Value(MIPS_CB_OPTIMAL)
EnumValue
Enum(mips_cb_setting) String(always) Value(MIPS_CB_ALWAYS)
+EnumValue
+Enum(mips_cb_setting) String(prefer) Value(MIPS_CB_PREFER)
+
mloongson-mmi
Target Mask(LOONGSON_MMI)
Use Loongson MultiMedia extensions Instructions (MMI) instructions.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3751bc3ac7c..9493c508d5b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -25199,9 +25199,11 @@ and MIPS64 architectures specifically deprecate their use.
@item -mcompact-branches=never
@itemx -mcompact-branches=optimal
@itemx -mcompact-branches=always
+@itemx -mcompact-branches=prefer
@opindex mcompact-branches=never
@opindex mcompact-branches=optimal
@opindex mcompact-branches=always
+@opindex mcompact-branches=prefer
These options control which form of branches will be generated. The
default is @option{-mcompact-branches=optimal}.
@@ -25215,6 +25217,10 @@ used instead.
This option is supported from MIPS Release 6 onwards.
+The @option{-mcompact-branches=prefer} option is same with
+@option{-mcompact-branches=always} for MIPS Release 6 onwards, and
+is same with @option{-mcompact-branches=never} for pre-R6.
+
The @option{-mcompact-branches=optimal} option will cause a delay slot
branch to be used if one is available in the current ISA and the delay
slot is successfully filled. If the delay slot is not filled, a compact
diff --git a/gcc/testsuite/gcc.target/mips/compact-branches-8.c b/gcc/testsuite/gcc.target/mips/compact-branches-8.c
new file mode 100644
index 00000000000..72ffcb49cfc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/compact-branches-8.c
@@ -0,0 +1,10 @@
+/* { dg-options "-mno-abicalls -mcompact-branches=prefer isa_rev<=5" } */
+void bar (int);
+
+void
+foo ()
+{
+ bar (1);
+}
+
+/* { dg-final { scan-assembler "\t(j|jal)\t" } } */
diff --git a/gcc/testsuite/gcc.target/mips/compact-branches-9.c b/gcc/testsuite/gcc.target/mips/compact-branches-9.c
new file mode 100644
index 00000000000..7a46b53d3e4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/compact-branches-9.c
@@ -0,0 +1,10 @@
+/* { dg-options "-mno-abicalls -fno-PIC -mcompact-branches=prefer isa_rev>=6" } */
+void bar (int);
+
+void
+foo ()
+{
+ bar (1);
+}
+
+/* { dg-final { scan-assembler "\t(bc|balc)\t" } } */
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-02-05 9:53 [PATCH 1/3] MIPS: add -mcompact-branches=prefer option YunQiang Su
@ 2021-02-05 9:53 ` YunQiang Su
2021-02-16 18:41 ` Jeff Law
2021-02-05 9:53 ` [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration YunQiang Su
2021-02-16 19:18 ` [PATCH 1/3] MIPS: add -mcompact-branches=prefer option Jeff Law
2 siblings, 1 reply; 11+ messages in thread
From: YunQiang Su @ 2021-02-05 9:53 UTC (permalink / raw)
To: gcc-patches; +Cc: syq, macro, jiaxun.yang, YunQiang Su
For R6+, it allows to configure gcc to use compact branches only.
---
gcc/config.gcc | 18 +++++++++++++++++-
gcc/doc/install.texi | 23 +++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 17fea83b2e4..7d50e7995d2 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4743,7 +4743,7 @@ case "${target}" in
;;
mips*-*-*)
- supported_defaults="abi arch arch_32 arch_64 float fpu nan fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 madd4"
+ supported_defaults="abi arch arch_32 arch_64 float fpu nan fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 madd4 compact-branches"
case ${with_float} in
"" | soft | hard)
@@ -4896,6 +4896,22 @@ case "${target}" in
exit 1
;;
esac
+
+ case ${with_compact_branches} in
+ never | always | optimal | prefer)
+ if test "$with_compact_branches" = "always" -a \
+ "$default_mips_arch" != "mips32r6" -a \
+ "$default_mips_arch" != "mips64r6";then
+ echo "Compact-branch=always is not allowed for pre-R6" 1>&2
+ exit 1
+ fi
+ with_compact_branches=${with_compact_branches}
+ ;;
+ *)
+ echo "Unknown compact-branches policy used in --with-compact-branches" 1>&2
+ exit 1
+ ;;
+ esac
;;
nds32*-*-*)
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 4c38244ae58..6b9520569ba 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1464,6 +1464,29 @@ systems that support conditional traps).
Division by zero checks use the break instruction.
@end table
+@item --with-compact-branches=@var{policy}
+Specify how the compiler should generate code for checking for
+division by zero. This option is only supported on the MIPS target.
+The possibilities for @var{type} are:
+@table @code
+@item optimal
+Cause a delay slot branch to be used if one is available in the
+current ISA and the delay slot is successfully filled. If the delay slot
+is not filled, a compact branch will be chosen if one is available.
+@item never
+Ensures that compact branch instructions will never be generated.
+@item always
+Ensures that a compact branch instruction will be generated if available.
+If a compact branch instruction is not available,
+a delay slot form of the branch will be used instead.
+This option is supported from MIPS Release 6 onwards.
+@item prefer
+Ensures that a compact branch instruction will be generated if available
+on MIPS Release 6 onwards.
+Simliar with @option{always} besides that this option works for pre-R6
+target, on which, this option will just be ignored.
+@end table
+
@c If you make --with-llsc the default for additional targets,
@c update the --with-llsc description in the MIPS section below.
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration
2021-02-05 9:53 [PATCH 1/3] MIPS: add -mcompact-branches=prefer option YunQiang Su
2021-02-05 9:53 ` [PATCH 2/3] MIPS: add builtime option for -mcompact-branches YunQiang Su
@ 2021-02-05 9:53 ` YunQiang Su
2021-02-16 18:38 ` Jeff Law
2021-02-16 19:18 ` [PATCH 1/3] MIPS: add -mcompact-branches=prefer option Jeff Law
2 siblings, 1 reply; 11+ messages in thread
From: YunQiang Su @ 2021-02-05 9:53 UTC (permalink / raw)
To: gcc-patches; +Cc: syq, macro, jiaxun.yang, YunQiang Su
GCC may be configured to use PIC by default, then the test with
-mno-abicall may fail. Just add -fno-PIC option for it.
---
gcc/testsuite/gcc.target/mips/compact-branches-5.c | 2 +-
gcc/testsuite/gcc.target/mips/compact-branches-6.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.target/mips/compact-branches-5.c b/gcc/testsuite/gcc.target/mips/compact-branches-5.c
index 90d312c614d..0189635bf61 100644
--- a/gcc/testsuite/gcc.target/mips/compact-branches-5.c
+++ b/gcc/testsuite/gcc.target/mips/compact-branches-5.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mcompact-branches=never isa_rev>=6" } */
+/* { dg-options "-mno-abicalls -fno-PIC -mcompact-branches=never isa_rev>=6" } */
void bar (int);
void
diff --git a/gcc/testsuite/gcc.target/mips/compact-branches-6.c b/gcc/testsuite/gcc.target/mips/compact-branches-6.c
index dd35a5581bd..36180b0c76c 100644
--- a/gcc/testsuite/gcc.target/mips/compact-branches-6.c
+++ b/gcc/testsuite/gcc.target/mips/compact-branches-6.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mcompact-branches=optimal isa_rev>=6" } */
+/* { dg-options "-mno-abicalls -fno-PIC -mcompact-branches=optimal isa_rev>=6" } */
void bar (int);
void
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration
2021-02-05 9:53 ` [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration YunQiang Su
@ 2021-02-16 18:38 ` Jeff Law
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2021-02-16 18:38 UTC (permalink / raw)
To: YunQiang Su, gcc-patches; +Cc: syq, jiaxun.yang
On 2/5/21 2:53 AM, YunQiang Su wrote:
> GCC may be configured to use PIC by default, then the test with
> -mno-abicall may fail. Just add -fno-PIC option for it.
THanks. Installed on the trunk.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-02-05 9:53 ` [PATCH 2/3] MIPS: add builtime option for -mcompact-branches YunQiang Su
@ 2021-02-16 18:41 ` Jeff Law
2021-02-16 19:35 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2021-02-16 18:41 UTC (permalink / raw)
To: YunQiang Su, gcc-patches; +Cc: syq, jiaxun.yang
On 2/5/21 2:53 AM, YunQiang Su wrote:
> For R6+, it allows to configure gcc to use compact branches only.
> ---
> gcc/config.gcc | 18 +++++++++++++++++-
> gcc/doc/install.texi | 23 +++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 17fea83b2e4..7d50e7995d2 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4743,7 +4743,7 @@ case "${target}" in
> ;;
>
> mips*-*-*)
> - supported_defaults="abi arch arch_32 arch_64 float fpu nan fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 madd4"
> + supported_defaults="abi arch arch_32 arch_64 float fpu nan fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 madd4 compact-branches"
>
> case ${with_float} in
> "" | soft | hard)
> @@ -4896,6 +4896,22 @@ case "${target}" in
> exit 1
> ;;
> esac
> +
> + case ${with_compact_branches} in
> + never | always | optimal | prefer)
> + if test "$with_compact_branches" = "always" -a \
> + "$default_mips_arch" != "mips32r6" -a \
> + "$default_mips_arch" != "mips64r6";then
> + echo "Compact-branch=always is not allowed for pre-R6" 1>&2
> + exit 1
> + fi
> + with_compact_branches=${with_compact_branches}
> + ;;
> + *)
> + echo "Unknown compact-branches policy used in --with-compact-branches" 1>&2
> + exit 1
> + ;;
> + esac
> ;;
>
> nds32*-*-*)
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 4c38244ae58..6b9520569ba 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -1464,6 +1464,29 @@ systems that support conditional traps).
> Division by zero checks use the break instruction.
> @end table
>
> +@item --with-compact-branches=@var{policy}
> +Specify how the compiler should generate code for checking for
> +division by zero. This option is only supported on the MIPS target.
> +The possibilities for @var{type} are:
Is this really correct -- I would expect that the change affects
branches in general, not just checks for division by zero.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] MIPS: add -mcompact-branches=prefer option
2021-02-05 9:53 [PATCH 1/3] MIPS: add -mcompact-branches=prefer option YunQiang Su
2021-02-05 9:53 ` [PATCH 2/3] MIPS: add builtime option for -mcompact-branches YunQiang Su
2021-02-05 9:53 ` [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration YunQiang Su
@ 2021-02-16 19:18 ` Jeff Law
2021-02-16 19:41 ` Maciej W. Rozycki
2 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2021-02-16 19:18 UTC (permalink / raw)
To: YunQiang Su, gcc-patches; +Cc: syq, jiaxun.yang
On 2/5/21 2:53 AM, YunQiang Su wrote:
> For MIPSr6, we may wish to use compact-branches only.
> Currently, we have to use `always' option, while it is mark as conflict
> with pre-R6.
> cc1: error: unsupported combination: ‘mips32r2’ -mcompact-branches=always
>
> It make some trouble for distributions to make -mcompact-branches=always
> default for R6 only.
>
> The new added `prefer' option:
> just ignored by pre-R6 target.
> do the same as `always' for R6+.
> ---
> gcc/config/mips/mips-opts.h | 3 ++-
> gcc/config/mips/mips.h | 6 ++++--
> gcc/config/mips/mips.opt | 3 +++
> gcc/doc/invoke.texi | 6 ++++++
> gcc/testsuite/gcc.target/mips/compact-branches-8.c | 10 ++++++++++
> gcc/testsuite/gcc.target/mips/compact-branches-9.c | 10 ++++++++++
> 6 files changed, 35 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-8.c
> create mode 100644 gcc/testsuite/gcc.target/mips/compact-branches-9.c
I think this will be OK once the wording in patch 2/3 of this series is
fixed.
It would be helpful if you could also include a ChangeLog entry.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-02-16 18:41 ` Jeff Law
@ 2021-02-16 19:35 ` Maciej W. Rozycki
2021-02-19 5:55 ` YunQiang Su
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2021-02-16 19:35 UTC (permalink / raw)
To: Jeff Law; +Cc: YunQiang Su, gcc-patches, syq, jiaxun.yang
On Tue, 16 Feb 2021, Jeff Law via Gcc-patches wrote:
> > diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> > index 4c38244ae58..6b9520569ba 100644
> > --- a/gcc/doc/install.texi
> > +++ b/gcc/doc/install.texi
> > @@ -1464,6 +1464,29 @@ systems that support conditional traps).
> > Division by zero checks use the break instruction.
> > @end table
> >
> > +@item --with-compact-branches=@var{policy}
> > +Specify how the compiler should generate code for checking for
> > +division by zero. This option is only supported on the MIPS target.
> > +The possibilities for @var{type} are:
> Is this really correct -- I would expect that the change affects
> branches in general, not just checks for division by zero.
I wonder if we need to multiply the options here at all. The original
change: <https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01892.html> was
discussed here: <https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00940.html>
in this respect:
On Mon, 17 Aug 2015, Matthew Fortune wrote:
> > > Compact branches are used based on a branch policy. The polices are:
> > >
> > > never: Only use delay slot branches
> > > optimal: Do whatever is best for the current architecture. This will
> > > generally mean that delay slot branches will be used if the delay
> > > slot gets filled but otherwise a compact branch will be used. A
> > > special case here is that JAL and J will not be used in R6 code
> > > regardless of whether the delay slot could be filled.
> > > always: Never emit a delay slot form of a branch if a compact form exists.
> > > This policy cannot apply 100% as FP branches (and MSA branches when
> > > committed) only have delay slot forms.
> > >
> > > These user choices are combined with the features available in the chosen
> > > architecture and, in particular, the optimal form will get handled like
> > > 'never' when there are no compact branches available and will get handled
> > > like 'always' when there are no delay slot branches available.
> > >
> >
> > Why did you choose to make this a user-selectable option? Why not always generated
> > optimal?
> > I don't have a strong opinion about it, but the options seem to complicate things and I'm
> > interested in your rationale.
>
> This is down to micro-architecture decisions that different implementers may make.
> Honestly, I have not absorbed all of the rationale behind choosing one form over
> the other but our arch team have made enough comments about this to mean the support
> in the compiler is worth the extra bit of effort. I can attempt a write-up of some
> of the pipeline possibilities if you would like more detail but I'd probably have to
> refresh my mind on this with our hardware teams.
My understanding therefore is that the original assumption that `optimal'
will serve people best is no longer true.
First, I think it would be good if we knew why. I find proliferating
variants of defaults, especially for the less obvious cases, will cause
user confusion as one won't know what code model to expect, especially as
(please correct me if I am wrong) we don't actually provide a way to dump
the compiler's overridable configuration defaults.
Second, I wonder if it makes sense to just keep things simple, and rather
than adding `prefer' (to stand for "`always' if available"), and possibly
`avoid' (to stand for "`never' if available"), whether we shouldn't just
relax the checks for `always' and `never', and let them through whether
the architecture selected provides for the option chosen or not.
Please note that in the discussion quoted Catherine has already raised a
concern I agree with of adding a complication here, and now we would
complicate it even further by not only adding a fourth choice, but another
overridable configuration default as well.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] MIPS: add -mcompact-branches=prefer option
2021-02-16 19:18 ` [PATCH 1/3] MIPS: add -mcompact-branches=prefer option Jeff Law
@ 2021-02-16 19:41 ` Maciej W. Rozycki
0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2021-02-16 19:41 UTC (permalink / raw)
To: Jeff Law; +Cc: YunQiang Su, gcc-patches, syq, jiaxun.yang
On Tue, 16 Feb 2021, Jeff Law via Gcc-patches wrote:
> I think this will be OK once the wording in patch 2/3 of this series is
> fixed.
As I noted with 2/3 I would like to know what this extra complication is
exactly needed for, and then whether we can't reuse the existing options.
Once settled I think it would best be placed in the change description for
posterity, so that discussions do not have to be chased like I had to with
the original change (when we had a different policy on commit messages).
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-02-16 19:35 ` Maciej W. Rozycki
@ 2021-02-19 5:55 ` YunQiang Su
2021-03-03 23:50 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: YunQiang Su @ 2021-02-19 5:55 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jeff Law, YunQiang Su, gcc-patches, Jiaxun Yang
Maciej W. Rozycki <macro@orcam.me.uk> 于2021年2月17日周三 上午3:45写道:
>
> On Tue, 16 Feb 2021, Jeff Law via Gcc-patches wrote:
>
> > > diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> > > index 4c38244ae58..6b9520569ba 100644
> > > --- a/gcc/doc/install.texi
> > > +++ b/gcc/doc/install.texi
> > > @@ -1464,6 +1464,29 @@ systems that support conditional traps).
> > > Division by zero checks use the break instruction.
> > > @end table
> > >
> > > +@item --with-compact-branches=@var{policy}
> > > +Specify how the compiler should generate code for checking for
> > > +division by zero. This option is only supported on the MIPS target.
> > > +The possibilities for @var{type} are:
> > Is this really correct -- I would expect that the change affects
> > branches in general, not just checks for division by zero.
>
> I wonder if we need to multiply the options here at all. The original
> change: <https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01892.html> was
> discussed here: <https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00940.html>
> in this respect:
>
> On Mon, 17 Aug 2015, Matthew Fortune wrote:
>
> > > > Compact branches are used based on a branch policy. The polices are:
> > > >
> > > > never: Only use delay slot branches
> > > > optimal: Do whatever is best for the current architecture. This will
> > > > generally mean that delay slot branches will be used if the delay
> > > > slot gets filled but otherwise a compact branch will be used. A
> > > > special case here is that JAL and J will not be used in R6 code
> > > > regardless of whether the delay slot could be filled.
> > > > always: Never emit a delay slot form of a branch if a compact form exists.
> > > > This policy cannot apply 100% as FP branches (and MSA branches when
> > > > committed) only have delay slot forms.
> > > >
> > > > These user choices are combined with the features available in the chosen
> > > > architecture and, in particular, the optimal form will get handled like
> > > > 'never' when there are no compact branches available and will get handled
> > > > like 'always' when there are no delay slot branches available.
> > > >
> > >
> > > Why did you choose to make this a user-selectable option? Why not always generated
> > > optimal?
> > > I don't have a strong opinion about it, but the options seem to complicate things and I'm
> > > interested in your rationale.
> >
> > This is down to micro-architecture decisions that different implementers may make.
> > Honestly, I have not absorbed all of the rationale behind choosing one form over
> > the other but our arch team have made enough comments about this to mean the support
> > in the compiler is worth the extra bit of effort. I can attempt a write-up of some
> > of the pipeline possibilities if you would like more detail but I'd probably have to
> > refresh my mind on this with our hardware teams.
>
> My understanding therefore is that the original assumption that `optimal'
> will serve people best is no longer true.
>
I guess that `optimal' can still produce the best performance, while
the delay slot
make MIPS quite differnent with other architectures.
And the hardware engineers seems hate it also.
And we expect that MIPS can have as few as possible differnece delta
with other major architectures,
to ultily all of new framworks of community.
> First, I think it would be good if we knew why. I find proliferating
> variants of defaults, especially for the less obvious cases, will cause
> user confusion as one won't know what code model to expect, especially as
> (please correct me if I am wrong) we don't actually provide a way to dump
> the compiler's overridable configuration defaults.
>
So, should we provide a predefined macro for it?
> Second, I wonder if it makes sense to just keep things simple, and rather
> than adding `prefer' (to stand for "`always' if available"), and possibly
> `avoid' (to stand for "`never' if available"), whether we shouldn't just
> relax the checks for `always' and `never', and let them through whether
> the architecture selected provides for the option chosen or not.
>
relax the `always' is what I would like to do first.
But I was afread to break some complatiable.
> Please note that in the discussion quoted Catherine has already raised a
> concern I agree with of adding a complication here, and now we would
> complicate it even further by not only adding a fourth choice, but another
> overridable configuration default as well.
I am still concern about whether we should just set the `always' as default.
My short team plan is to set it default for Debian r6 Port.
So, at least, I wish that we can provide a buildtime option for other need.
>
> Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-02-19 5:55 ` YunQiang Su
@ 2021-03-03 23:50 ` Maciej W. Rozycki
2021-03-04 3:24 ` YunQiang Su
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2021-03-03 23:50 UTC (permalink / raw)
To: YunQiang Su; +Cc: Jeff Law, YunQiang Su, gcc-patches, Jiaxun Yang
On Fri, 19 Feb 2021, YunQiang Su wrote:
> > My understanding therefore is that the original assumption that `optimal'
> > will serve people best is no longer true.
> >
>
> I guess that `optimal' can still produce the best performance, while
> the delay slot
> make MIPS quite differnent with other architectures.
> And the hardware engineers seems hate it also.
Right, but what does it have to do with compiler defaults? Given what we
have available in hardware we want the best results possible, except for
research or special use cases (such as GAS's `-minsn32' option with
microMIPS code). I would like to understand what the use case is here.
> And we expect that MIPS can have as few as possible differnece delta
> with other major architectures,
> to ultily all of new framworks of community.
Well, machine code is inherently architecture-specific, so you can't
have a single template that fits all. The difference betwen processor
architectures is more than just the bit patterns for individual opcodes
and operand encodings (and the corresponding mnemonics and syntax for
the assembly language).
For example one of the major architectures is ARM, which has conditions
encoded with all the instructions. And you cant mimic it with other ISAs.
Similarly Power has 8 sets of condition codes and dedicated instructions
to make ALU operations between these codes. You can't do those elsewhere
either. Well, the MIPS or RISC-V ISAs do not have condition codes at all.
And x86 is not a load-store architecture at all, so you'll see operations
made directly on memory, as a destination even (let's ignore the even more
arcane original 32-bit instruction set).
These are all considered major architectures nowadays.
So we have got the MIPS ISA and its delay slots. Some subsets/variations
of the ISA have already either greatly reduced their use or eliminated
them completely, but we went into great lengths with GCC to produce good
code making use of these delay slots, so I think it would be a shame to
get this effort wasted on one hand, and MIPS code put at a disadvantage
due to cycles wasted for pipeline stalls that could be avoided if delay
slots were scheduled -- on the other.
> > First, I think it would be good if we knew why. I find proliferating
> > variants of defaults, especially for the less obvious cases, will cause
> > user confusion as one won't know what code model to expect, especially as
> > (please correct me if I am wrong) we don't actually provide a way to dump
> > the compiler's overridable configuration defaults.
> >
>
> So, should we provide a predefined macro for it?
I've been thinking more along `gcc -v --version' dumping the invocation
of `configure' used, but I have to correct myself here in that it already
happens, so nothing to do. I'm not sure why I forgot it and/or could not
have figured it out previously. Sorry about the confusion.
> > Second, I wonder if it makes sense to just keep things simple, and rather
> > than adding `prefer' (to stand for "`always' if available"), and possibly
> > `avoid' (to stand for "`never' if available"), whether we shouldn't just
> > relax the checks for `always' and `never', and let them through whether
> > the architecture selected provides for the option chosen or not.
> >
>
> relax the `always' is what I would like to do first.
> But I was afread to break some complatiable.
Hmm, honestly I don't think there could be any compatibility to care of
here given that the compiler currently refuses to run with such an option
combination. Nobody may have relied on that then and the extra protection
given is in my opinion a bit excessive. Garbage in, garbage out: you get
what you have requested. Our usual policy with irrelevant options has
been to silently ignore them, which helps users override Makefile defaults
by just having CFLAGS, etc. appended to whatever the defaults are.
> > Please note that in the discussion quoted Catherine has already raised a
> > concern I agree with of adding a complication here, and now we would
> > complicate it even further by not only adding a fourth choice, but another
> > overridable configuration default as well.
>
> I am still concern about whether we should just set the `always' as default.
> My short team plan is to set it default for Debian r6 Port.
> So, at least, I wish that we can provide a buildtime option for other need.
You're free with what you do with your distribution, although it seems to
me like it's going to be a performance regression. I suggest that you do
some benchmarking with real code and hardware before you decide. Maybe
you can prove me wrong and there will be no loss of any significance.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches
2021-03-03 23:50 ` Maciej W. Rozycki
@ 2021-03-04 3:24 ` YunQiang Su
0 siblings, 0 replies; 11+ messages in thread
From: YunQiang Su @ 2021-03-04 3:24 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Jeff Law, YunQiang Su, gcc-patches, Jiaxun Yang
Maciej W. Rozycki <macro@orcam.me.uk> 于2021年3月4日周四 上午7:50写道:
>
> On Fri, 19 Feb 2021, YunQiang Su wrote:
>
> > > My understanding therefore is that the original assumption that `optimal'
> > > will serve people best is no longer true.
> > >
> >
> > I guess that `optimal' can still produce the best performance, while
> > the delay slot
> > make MIPS quite differnent with other architectures.
> > And the hardware engineers seems hate it also.
>
> Right, but what does it have to do with compiler defaults? Given what we
> have available in hardware we want the best results possible, except for
> research or special use cases (such as GAS's `-minsn32' option with
> microMIPS code). I would like to understand what the use case is here.
>
I want to give the hardware designer the possibility to remove the
delay slot branch
instructions in future (maybe in a long term).
> > And we expect that MIPS can have as few as possible differnece delta
> > with other major architectures,
> > to ultily all of new framworks of community.
>
> Well, machine code is inherently architecture-specific, so you can't
> have a single template that fits all. The difference betwen processor
> architectures is more than just the bit patterns for individual opcodes
> and operand encodings (and the corresponding mnemonics and syntax for
> the assembly language).
>
Delay slot makes some troubles for newly written binary tools.
Lots of people are not used to delay slot, and then they may not
consider MIPS in some new work.
We cannot expect to much the support from them, so we have to reduce the delta.
> For example one of the major architectures is ARM, which has conditions
> encoded with all the instructions. And you cant mimic it with other ISAs.
> Similarly Power has 8 sets of condition codes and dedicated instructions
> to make ALU operations between these codes. You can't do those elsewhere
> either. Well, the MIPS or RISC-V ISAs do not have condition codes at all.
> And x86 is not a load-store architecture at all, so you'll see operations
> made directly on memory, as a destination even (let's ignore the even more
> arcane original 32-bit instruction set).
>
> These are all considered major architectures nowadays.
Yes. "nowadays".
How about the future?
Since the MIPS is the only architecture which has delay slot, will it
be ”no-major“?
>
> So we have got the MIPS ISA and its delay slots. Some subsets/variations
> of the ISA have already either greatly reduced their use or eliminated
> them completely, but we went into great lengths with GCC to produce good
> code making use of these delay slots, so I think it would be a shame to
Yes. There will not be a problem for GCC itself, while there are more and more
other tools, open source or commercial.
Since MIPS is not the de-facto majority, it is not so easy to ask them
to support MIPS.
> get this effort wasted on one hand, and MIPS code put at a disadvantage
> due to cycles wasted for pipeline stalls that could be avoided if delay
> slots were scheduled -- on the other.
>
I have some tests on I6500. The performance of delay-branch and compact-branch
are almost the same. It will not be a performance regression.
> > > First, I think it would be good if we knew why. I find proliferating
> > > variants of defaults, especially for the less obvious cases, will cause
> > > user confusion as one won't know what code model to expect, especially as
> > > (please correct me if I am wrong) we don't actually provide a way to dump
> > > the compiler's overridable configuration defaults.
> > >
> >
> > So, should we provide a predefined macro for it?
>
> I've been thinking more along `gcc -v --version' dumping the invocation
> of `configure' used, but I have to correct myself here in that it already
> happens, so nothing to do. I'm not sure why I forgot it and/or could not
> have figured it out previously. Sorry about the confusion.
>
nop.
> > > Second, I wonder if it makes sense to just keep things simple, and rather
> > > than adding `prefer' (to stand for "`always' if available"), and possibly
> > > `avoid' (to stand for "`never' if available"), whether we shouldn't just
> > > relax the checks for `always' and `never', and let them through whether
> > > the architecture selected provides for the option chosen or not.
> > >
> >
> > relax the `always' is what I would like to do first.
> > But I was afread to break some complatiable.
>
> Hmm, honestly I don't think there could be any compatibility to care of
> here given that the compiler currently refuses to run with such an option
> combination. Nobody may have relied on that then and the extra protection
> given is in my opinion a bit excessive. Garbage in, garbage out: you get
> what you have requested. Our usual policy with irrelevant options has
> been to silently ignore them, which helps users override Makefile defaults
> by just having CFLAGS, etc. appended to whatever the defaults are.
>
Again, it is not so easy to ask ALL packages to have MIPS dirty
patches just for a CFLAGS.
> > > Please note that in the discussion quoted Catherine has already raised a
> > > concern I agree with of adding a complication here, and now we would
> > > complicate it even further by not only adding a fourth choice, but another
> > > overridable configuration default as well.
> >
> > I am still concern about whether we should just set the `always' as default.
> > My short team plan is to set it default for Debian r6 Port.
> > So, at least, I wish that we can provide a buildtime option for other need.
>
> You're free with what you do with your distribution, although it seems to
> me like it's going to be a performance regression. I suggest that you do
With some tests, there are no performance regression at all on I6500.
> some benchmarking with real code and hardware before you decide. Maybe
> you can prove me wrong and there will be no loss of any significance.
#include "stdio.h"
#include "perf_func.h"
#include <mips/m64c0.h>
int PerfCnt[4];
__attribute__((noinline)) int add_1(int x) {
return x+1;
}
int main(){
int a;
mipsperf_switch_group();
for (int i=0; i<1000000; i++){
a+=add_1(a);
}
PerfCnt[0] = mips32_get_c0(C0_PERFCNT0);
PerfCnt[1] = mips32_get_c0(C0_PERFCNT1);
return a;
}
This is an example of my tests. It has almost the same performance with
-mcompact-branches=always
-mcompact-branches=optimal
I guess you may have more tests.
>
> Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-03-04 3:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 9:53 [PATCH 1/3] MIPS: add -mcompact-branches=prefer option YunQiang Su
2021-02-05 9:53 ` [PATCH 2/3] MIPS: add builtime option for -mcompact-branches YunQiang Su
2021-02-16 18:41 ` Jeff Law
2021-02-16 19:35 ` Maciej W. Rozycki
2021-02-19 5:55 ` YunQiang Su
2021-03-03 23:50 ` Maciej W. Rozycki
2021-03-04 3:24 ` YunQiang Su
2021-02-05 9:53 ` [PATCH 3/3] MIPS: fix compact-branches test FAIL for PIC default configuration YunQiang Su
2021-02-16 18:38 ` Jeff Law
2021-02-16 19:18 ` [PATCH 1/3] MIPS: add -mcompact-branches=prefer option Jeff Law
2021-02-16 19:41 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).