loongarch: use -mno-check-zero-division as the default for optimized code

  Hi,

We've made a consensus [1] that not to enable trapping for division by
zero by default for LLVM, and we think GCC should behave similarly.

The main rationales:

1. Division by zero is undefined behavior, so in theory any portable
program shall not depend on it.
2. There are already many targets where both the hardware and GCC port
do nothing to trap on division by zero.  A list taken from
gcc.c-torture/execute/20101011-1.c:  PowerPC, RISC-V, ARM64, MSP430, and
many others.  So in practice any portable program cannot depend on this
trap.
3. As an ICPC assistant coach, I'm well aware that the main disadvantage
not to trap on division by zero is "it breaks expectations of newbies".
So, we keep -mcheck-zero-division defaulted for -O0 and -Og.  For other
optimization levels, it's well known that UBs are already breaking
newbies' expectations [2].
4. GCC is going to optimize more heavily exploiting integer division by
zero [3].  So let's stop encouraging people to rely on any integer
division by zero behavior from now.

[1]: https://reviews.llvm.org/D128572/new/#3612039
[2]: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
[3]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595099.html

Patch content following.  Bootstrapped and regtested on loongarch64-
linux-gnu. Ok for trunk?

-- >8 --

Integer division by zero is undefined behavior anyway, and there are
already many platforms where neither the GCC port and the hardware do
anything to trap on division by zero.  So any portable program shall not
rely on SIGFPE on division by zero, in both theory and practice.  As the
result, there is no real reason to cost two additional instructions just
for the trap on division by zero with a new ISA.

One remaining reason to trap on division by zero may be debugging,
especially while -fsanitize=integer-divide-by-zero is not implemented
for LoongArch yet.  To make debugging easier, keep -mcheck-zero-division
as the default for -O0 and -Og, but use -mno-check-zero-division as the
default for all other optimization levels.

gcc/ChangeLog:

	* config/loongarch/loongarch.cc (loongarch_check_zero_div_p):
	New static function.
	(loongarch_idiv_insns): Use loongarch_check_zero_div_p instead
	of TARGET_CHECK_ZERO_DIV.
	(loongarch_output_division): Likewise.
	* doc/invoke.texi: Update to match the new behavior.

gcc/testsuite/ChangeLog:

	* gcc.c-torture/execute/20101011-1.c (dg-additional-options):
	add -mcheck-zero-division for LoongArch targets.
---
 gcc/config/loongarch/loongarch.cc              | 18 +++++++++++++++---
 gcc/doc/invoke.texi                            |  3 ++-
 .../gcc.c-torture/execute/20101011-1.c         |  1 +
 3 files changed, 18 insertions(+), 4 deletions(-)
  

On Thu, Jun 30, 2022 at 5:00 AM Xi Ruoyao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> We've made a consensus [1] that not to enable trapping for division by
> zero by default for LLVM, and we think GCC should behave similarly.
>
> The main rationales:
>
> 1. Division by zero is undefined behavior, so in theory any portable
> program shall not depend on it.
> 2. There are already many targets where both the hardware and GCC port
> do nothing to trap on division by zero.  A list taken from
> gcc.c-torture/execute/20101011-1.c:  PowerPC, RISC-V, ARM64, MSP430, and
> many others.  So in practice any portable program cannot depend on this
> trap.
> 3. As an ICPC assistant coach, I'm well aware that the main disadvantage
> not to trap on division by zero is "it breaks expectations of newbies".
> So, we keep -mcheck-zero-division defaulted for -O0 and -Og.  For other
> optimization levels, it's well known that UBs are already breaking
> newbies' expectations [2].
> 4. GCC is going to optimize more heavily exploiting integer division by
> zero [3].  So let's stop encouraging people to rely on any integer
> division by zero behavior from now.
>
> [1]: https://reviews.llvm.org/D128572/new/#3612039
> [2]: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
> [3]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595099.html
>
> Patch content following.  Bootstrapped and regtested on loongarch64-
> linux-gnu. Ok for trunk?

It might be worth backporting this behavioral change to the GCC 12 branch
as well (so 12.1 is the only release with different default behavior) and
documenting the change in changes.html

> -- >8 --
>
> Integer division by zero is undefined behavior anyway, and there are
> already many platforms where neither the GCC port and the hardware do
> anything to trap on division by zero.  So any portable program shall not
> rely on SIGFPE on division by zero, in both theory and practice.  As the
> result, there is no real reason to cost two additional instructions just
> for the trap on division by zero with a new ISA.
>
> One remaining reason to trap on division by zero may be debugging,
> especially while -fsanitize=integer-divide-by-zero is not implemented
> for LoongArch yet.  To make debugging easier, keep -mcheck-zero-division
> as the default for -O0 and -Og, but use -mno-check-zero-division as the
> default for all other optimization levels.
>
> gcc/ChangeLog:
>
>         * config/loongarch/loongarch.cc (loongarch_check_zero_div_p):
>         New static function.
>         (loongarch_idiv_insns): Use loongarch_check_zero_div_p instead
>         of TARGET_CHECK_ZERO_DIV.
>         (loongarch_output_division): Likewise.
>         * doc/invoke.texi: Update to match the new behavior.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.c-torture/execute/20101011-1.c (dg-additional-options):
>         add -mcheck-zero-division for LoongArch targets.
> ---
>  gcc/config/loongarch/loongarch.cc              | 18 +++++++++++++++---
>  gcc/doc/invoke.texi                            |  3 ++-
>  .../gcc.c-torture/execute/20101011-1.c         |  1 +
>  3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
> index c8502b0b0f3..f297083c2e9 100644
> --- a/gcc/config/loongarch/loongarch.cc
> +++ b/gcc/config/loongarch/loongarch.cc
> @@ -2101,6 +2101,19 @@ loongarch_load_store_insns (rtx mem, rtx_insn *insn)
>    return loongarch_address_insns (XEXP (mem, 0), mode, might_split_p);
>  }
>
> +/* Return true if we need to trap on division by zero.  */
> +
> +static bool
> +loongarch_check_zero_div_p (void)
> +{
> +  /* if -m[no-]check-zero-division is given explicitly.  */
> +  if (target_flags_explicit & MASK_CHECK_ZERO_DIV)
> +    return TARGET_CHECK_ZERO_DIV;
> +
> +  /* if not, don't trap for optimized code except -Og.  */
> +  return !optimize || optimize_debug;
> +}
> +
>  /* Return the number of instructions needed for an integer division.  */
>
>  int
> @@ -2109,7 +2122,7 @@ loongarch_idiv_insns (machine_mode mode ATTRIBUTE_UNUSED)
>    int count;
>
>    count = 1;
> -  if (TARGET_CHECK_ZERO_DIV)
> +  if (loongarch_check_zero_div_p ())
>      count += 2;
>
>    return count;
> @@ -4050,7 +4063,6 @@ loongarch_do_optimize_block_move_p (void)
>    return !optimize_size;
>  }
>
> -
>  /* Expand a QI or HI mode atomic memory operation.
>
>     GENERATOR contains a pointer to the gen_* function that generates
> @@ -5262,7 +5274,7 @@ loongarch_output_division (const char *division, rtx *operands)
>    const char *s;
>
>    s = division;
> -  if (TARGET_CHECK_ZERO_DIV)
> +  if (loongarch_check_zero_div_p ())
>      {
>        output_asm_insn (s, operands);
>        s = "bne\t%2,%.,1f\n\tbreak\t7\n1:";
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index bde59ff0472..7e2a0b01233 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -24740,7 +24740,8 @@ Set the cost of branches to roughly @var{n} instructions.
>  @itemx -mno-check-zero-divison
>  @opindex -mcheck-zero-division
>  Trap (do not trap) on integer division by zero.  The default is
> -@option{-mcheck-zero-division}.
> +@option{-mcheck-zero-division} for @option{-O0} or @option{-Og}, and
> +@option{-mno-check-zero-division} for other optimization levels.
>
>  @item -mcond-move-int
>  @itemx -mno-cond-move-int
> diff --git a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
> index 649e168e0b1..d2c0f9ab7ec 100644
> --- a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
> +++ b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c
> @@ -1,6 +1,7 @@
>  /* { dg-options "-fnon-call-exceptions" } */
>  /* With -fnon-call-exceptions 0 / 0 should not be eliminated.  */
>  /* { dg-additional-options "-DSIGNAL_SUPPRESS" { target { ! signal } } } */
> +/* { dg-additional-options "-mcheck-zero-division" { target { loongarch*-*-* } } } */
>
>  #ifdef SIGNAL_SUPPRESS
>  # define DO_TEST 0
> --
> 2.36.0
>
>
  

在 2022/6/30 下午2:55, Richard Biener 写道:
> On Thu, Jun 30, 2022 at 5:00 AM Xi Ruoyao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> Hi,
>>
>> We've made a consensus [1] that not to enable trapping for division by
>> zero by default for LLVM, and we think GCC should behave similarly.
>>
>> The main rationales:
>>
>> 1. Division by zero is undefined behavior, so in theory any portable
>> program shall not depend on it.
>> 2. There are already many targets where both the hardware and GCC port
>> do nothing to trap on division by zero.  A list taken from
>> gcc.c-torture/execute/20101011-1.c:  PowerPC, RISC-V, ARM64, MSP430, and
>> many others.  So in practice any portable program cannot depend on this
>> trap.
>> 3. As an ICPC assistant coach, I'm well aware that the main disadvantage
>> not to trap on division by zero is "it breaks expectations of newbies".
>> So, we keep -mcheck-zero-division defaulted for -O0 and -Og.  For other
>> optimization levels, it's well known that UBs are already breaking
>> newbies' expectations [2].
>> 4. GCC is going to optimize more heavily exploiting integer division by
>> zero [3].  So let's stop encouraging people to rely on any integer
>> division by zero behavior from now.
>>
>> [1]: https://reviews.llvm.org/D128572/new/#3612039
>> [2]: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
>> [3]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595099.html
>>
>> Patch content following.  Bootstrapped and regtested on loongarch64-
>> linux-gnu. Ok for trunk?
> It might be worth backporting this behavioral change to the GCC 12 branch
> as well (so 12.1 is the only release with different default behavior) and
> documenting the change in changes.html
diff --git a/gcc/common/config/loongarch/loongarch-common.cc 
b/gcc/common/config/loongarch/loongarch-common.cc
index b6cbd84b873..f8b4660fabf 100644
--- a/gcc/common/config/loongarch/loongarch-common.cc
+++ b/gcc/common/config/loongarch/loongarch-common.cc
@@ -38,7 +38,4 @@ static const struct default_options 
loongarch_option_optimization_table[] =
    { OPT_LEVELS_NONE, 0, NULL, 0 }
  };

-#undef TARGET_DEFAULT_TARGET_FLAGS
-#define TARGET_DEFAULT_TARGET_FLAGS    MASK_CHECK_ZERO_DIV
-

I think this modifications are needed, and there is no problem with the 
rest.

Thanks!

Lulu Cheng
  

On Sat, 2022-07-02 at 15:39 +0800, Lulu Cheng wrote:

> diff --git a/gcc/common/config/loongarch/loongarch-common.cc 
> b/gcc/common/config/loongarch/loongarch-common.cc
> index b6cbd84b873..f8b4660fabf 100644
> --- a/gcc/common/config/loongarch/loongarch-common.cc
> +++ b/gcc/common/config/loongarch/loongarch-common.cc
> @@ -38,7 +38,4 @@ static const struct default_options 
> loongarch_option_optimization_table[] =
>     { OPT_LEVELS_NONE, 0, NULL, 0 }
>   };
> 
> -#undef TARGET_DEFAULT_TARGET_FLAGS
> -#define TARGET_DEFAULT_TARGET_FLAGS    MASK_CHECK_ZERO_DIV
> -
> 
> I think this modifications are needed, and there is no problem with the 
> rest.

Yes, this hook is unneeded now.  Though the removal is not strictly
needed, it's good to remove irrelevant code (CWE-1164 says we shouldn't
keep any irrelevant code).

I'll commit the patch with the hook removed after another regtest on
loongarch64-linux-gnu.  I just rebuilt the entire system on my 3A5000,
so I need some time to set it up.  Expectation time to commit is today
evening or tomorrow morning.

BTW I've included this patch building my system, no bad things has
happened so far.
  

在 2022/7/2 下午4:24, Xi Ruoyao 写道:
> On Sat, 2022-07-02 at 15:39 +0800, Lulu Cheng wrote:
>
>> diff --git a/gcc/common/config/loongarch/loongarch-common.cc
>> b/gcc/common/config/loongarch/loongarch-common.cc
>> index b6cbd84b873..f8b4660fabf 100644
>> --- a/gcc/common/config/loongarch/loongarch-common.cc
>> +++ b/gcc/common/config/loongarch/loongarch-common.cc
>> @@ -38,7 +38,4 @@ static const struct default_options
>> loongarch_option_optimization_table[] =
>>      { OPT_LEVELS_NONE, 0, NULL, 0 }
>>    };
>>
>> -#undef TARGET_DEFAULT_TARGET_FLAGS
>> -#define TARGET_DEFAULT_TARGET_FLAGS    MASK_CHECK_ZERO_DIV
>> -
>>
>> I think this modifications are needed, and there is no problem with the
>> rest.
> Yes, this hook is unneeded now.  Though the removal is not strictly
> needed, it's good to remove irrelevant code (CWE-1164 says we shouldn't
> keep any irrelevant code).
>
> I'll commit the patch with the hook removed after another regtest on
> loongarch64-linux-gnu.  I just rebuilt the entire system on my 3A5000,
> so I need some time to set it up.  Expectation time to commit is today
> evening or tomorrow morning.
>
> BTW I've included this patch building my system, no bad things has
> happened so far.
Ok,Thanks!:-)
  

On Sat, 2022-07-02 at 16:35 +0800, Lulu Cheng wrote:
> 在 2022/7/2 下午4:24, Xi Ruoyao 写道:
> > 
> > I'll commit the patch with the hook removed after another regtest on
> > loongarch64-linux-gnu.  I just rebuilt the entire system on my
> > 3A5000,
> > so I need some time to set it up.  Expectation time to commit is
> > today
> > evening or tomorrow morning.
> > 
> > BTW I've included this patch building my system, no bad things has
> > happened so far.
> Ok,Thanks!:-)

Pushed as r13-1410.

How do you think about the suggestion from Richard about a backport into
gcc-12 branch?  Normally we don't backport behavior changes, but making
12.1 the only exception seems compelling.
  

在 2022/7/3 上午11:06, Xi Ruoyao 写道:
> On Sat, 2022-07-02 at 16:35 +0800, Lulu Cheng wrote:
>> 在 2022/7/2 下午4:24, Xi Ruoyao 写道:
>>> I'll commit the patch with the hook removed after another regtest on
>>> loongarch64-linux-gnu.  I just rebuilt the entire system on my
>>> 3A5000,
>>> so I need some time to set it up.  Expectation time to commit is
>>> today
>>> evening or tomorrow morning.
>>>
>>> BTW I've included this patch building my system, no bad things has
>>> happened so far.
>> Ok,Thanks!:-)
> Pushed as r13-1410.
>
> How do you think about the suggestion from Richard about a backport into
> gcc-12 branch?  Normally we don't backport behavior changes, but making
> 12.1 the only exception seems compelling.

I agree with you and Richard.

Thanks!
  

On Mon, 2022-07-04 at 14:25 +0800, Lulu Cheng wrote:

> > How do you think about the suggestion from Richard about a backport into
> > gcc-12 branch?  Normally we don't backport behavior changes, but making
> > 12.1 the only exception seems compelling.
> 
> I agree with you and Richard.
> 
> Thanks!

Pushed r12-8546.  wwwdocs patch preparing.