public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-9150] bitintlower: Fix .{ADD, SUB}_OVERFLOW lowering [PR114040]
@ 2024-02-23 10:37 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2024-02-23 10:37 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:be1f2bc4522f772184a4d16d8f3fec75baed89cf

commit r14-9150-gbe1f2bc4522f772184a4d16d8f3fec75baed89cf
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Feb 23 11:36:15 2024 +0100

    bitintlower: Fix .{ADD,SUB}_OVERFLOW lowering [PR114040]
    
    The following testcases show 2 bugs in the .{ADD,SUB}_OVERFLOW lowering,
    both related to storing of the REALPART_EXPR part of the result.
    On the first testcase prec is 255, prec_limbs is 4 and for the second limb
    in the loop the store of the REALPART_EXPR of .USUBC (_30) is stored through:
      if (_27 <= 3)
        goto <bb 12>; [80.00%]
      else
        goto <bb 15>; [20.00%]
    
      <bb 12> [local count: 1073741824]:
      if (_27 < 3)
        goto <bb 14>; [80.00%]
      else
        goto <bb 13>; [20.00%]
    
      <bb 13> [local count: 1073741824]:
      bitint.3[_27] = _30;
      goto <bb 15>; [100.00%]
    
      <bb 14> [local count: 858993464]:
      MEM[(unsigned long *)&bitint.3 + 24B] = _30;
    
      <bb 15> [local count: 1073741824]:
    The first check is right, as prec_limbs is 4, we don't want to store
    bitint.3[4] or above at all, those limbs are just computed for the overflow
    checking and nothing else, so _27 > 4 leads to no store.
    But the other condition is exact opposite of what should be done, if
    the current index of the second limb (_27) is < 3, then it should
      bitint.3[_27] = _30;
    and if it is == 3, it should
      MEM[(unsigned long *)&bitint.3 + 24B] = _30;
    and (especially important for the targets which would bitinfo.extended = 1)
    should actually in this case zero extend it from the 63 bits to 64, that is
    the handling of the partial limb.  The if_then_if_then_else helper if
    there are 2 conditions sets m_gsi to be at the start of the
    edge_true_false->dest bb, i.e. when the first condition is true and second
    false, and that is where we store the SSA_NAME indexed limb store, so the
    condition needs to be reversed.
    
    The following patch does that and adds the cast as well, the usual
    assumption that already handle_operand has the partial limb type doesn't
    have to be true here, because the source operand could have much larger
    precision than the REALPART_EXPR of the lhs.
    
    2024-02-23  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/114040
            * gimple-lower-bitint.cc (bitint_large_huge::lower_addsub_overflow):
            Use EQ_EXPR rather than LT_EXPR for g2 condition and change its
            probability from likely to unlikely.  When handling the true true
            store, first cast to limb_access_type and then to l's type.
    
            * gcc.dg/torture/bitint-60.c: New test.
            * gcc.dg/torture/bitint-61.c: New test.

Diff:
---
 gcc/gimple-lower-bitint.cc               | 11 ++++++----
 gcc/testsuite/gcc.dg/torture/bitint-60.c | 24 +++++++++++++++++++++
 gcc/testsuite/gcc.dg/torture/bitint-61.c | 36 ++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
index fb03063f86e4..3552fdd183e9 100644
--- a/gcc/gimple-lower-bitint.cc
+++ b/gcc/gimple-lower-bitint.cc
@@ -4255,12 +4255,12 @@ bitint_large_huge::lower_addsub_overflow (tree obj, gimple *stmt)
 				     NULL_TREE, NULL_TREE);
 	      gimple *g2 = NULL;
 	      if (!single_comparison)
-		g2 = gimple_build_cond (LT_EXPR, idx,
+		g2 = gimple_build_cond (EQ_EXPR, idx,
 					size_int (prec_limbs - 1),
 					NULL_TREE, NULL_TREE);
 	      edge edge_true_true, edge_true_false, edge_false;
 	      if_then_if_then_else (g, g2, profile_probability::likely (),
-				    profile_probability::likely (),
+				    profile_probability::unlikely (),
 				    edge_true_true, edge_true_false,
 				    edge_false);
 	      tree l = limb_access (type, var ? var : obj, idx, true);
@@ -4269,8 +4269,11 @@ bitint_large_huge::lower_addsub_overflow (tree obj, gimple *stmt)
 	      if (!single_comparison)
 		{
 		  m_gsi = gsi_after_labels (edge_true_true->src);
-		  l = limb_access (type, var ? var : obj,
-				   size_int (prec_limbs - 1), true);
+		  tree plm1idx = size_int (prec_limbs - 1);
+		  tree plm1type = limb_access_type (type, plm1idx);
+		  l = limb_access (type, var ? var : obj, plm1idx, true);
+		  if (!useless_type_conversion_p (plm1type, TREE_TYPE (rhs)))
+		    rhs = add_cast (plm1type, rhs);
 		  if (!useless_type_conversion_p (TREE_TYPE (l),
 						  TREE_TYPE (rhs)))
 		    rhs = add_cast (TREE_TYPE (l), rhs);
diff --git a/gcc/testsuite/gcc.dg/torture/bitint-60.c b/gcc/testsuite/gcc.dg/torture/bitint-60.c
new file mode 100644
index 000000000000..d2d27a188b67
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/bitint-60.c
@@ -0,0 +1,24 @@
+/* PR tree-optimization/114040 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+#if __BITINT_MAXWIDTH__ >= 8671
+__attribute__((noipa)) unsigned
+foo (unsigned _BitInt(8671) x, unsigned y, unsigned _BitInt(512) z)
+{
+  unsigned _BitInt (8671) r
+    = x * __builtin_sub_overflow_p (y * z, 0, (unsigned _BitInt(255)) 0);
+  return r;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 8671
+  if (foo (1, 1, 0xfffa46471e7c2dd60000000000000000wb))
+    __builtin_abort ();
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/torture/bitint-61.c b/gcc/testsuite/gcc.dg/torture/bitint-61.c
new file mode 100644
index 000000000000..e5651f10ea46
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/bitint-61.c
@@ -0,0 +1,36 @@
+/* PR tree-optimization/114040 */
+/* { dg-do run { target { bitint && int128 } } } */
+/* { dg-options "-std=c23" } */
+/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+unsigned a;
+signed char b;
+short c;
+long d;
+__int128 e;
+int f;
+
+#if __BITINT_MAXWIDTH__ >= 511
+__attribute__((noinline)) void
+foo (_BitInt(3) x, unsigned _BitInt(511) y, unsigned *z)
+{
+  int g = __builtin_sub_overflow_p (y ^ x, 0, (unsigned _BitInt(255)) 0);
+  unsigned h = y + e, i = h + d;
+  unsigned _BitInt(2) j = i + g;
+  unsigned k = j + c;
+  unsigned l = k + a + f + b;
+  *z = l;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 511
+  unsigned x;
+  foo (0, 0x81e4a5fa7c408f370000000000000000uwb, &x);
+  if (x)
+    __builtin_abort ();
+#endif
+}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-02-23 10:37 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-23 10:37 [gcc r14-9150] bitintlower: Fix .{ADD, SUB}_OVERFLOW lowering [PR114040] Jakub Jelinek

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).