]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
phiopt: Fix VCE moving by rewriting it into cast [PR116098]
authorAndrew Pinski <quic_apinski@quicinc.com>
Tue, 1 Oct 2024 18:34:00 +0000 (18:34 +0000)
committerAndrew Pinski <quic_apinski@quicinc.com>
Wed, 2 Oct 2024 17:16:01 +0000 (17:16 +0000)
Phiopt match_and_simplify might move a well defined VCE assign statement
from being conditional to being uncondtitional; that VCE might no longer
being defined. It will need a rewrite into a cast instead.

This adds the rewriting code to move_stmt for the VCE case.
This is enough to fix the issue at hand. It should also be using rewrite_to_defined_overflow
but first I need to move the check to see a rewrite is needed into its own function
and that is causing issues (see https://gcc.gnu.org/pipermail/gcc-patches/2024-September/663938.html).
Plus this version is easiest to backport.

Bootstrapped and tested on x86_64-linux-gnu.

PR tree-optimization/116098

gcc/ChangeLog:

* tree-ssa-phiopt.cc (move_stmt): Rewrite VCEs from integer to integer
types to case.

gcc/testsuite/ChangeLog:

* c-c++-common/torture/pr116098-2.c: New test.
* g++.dg/torture/pr116098-1.C: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
gcc/testsuite/c-c++-common/torture/pr116098-2.c [new file with mode: 0644]
gcc/testsuite/g++.dg/torture/pr116098-1.C [new file with mode: 0644]
gcc/tree-ssa-phiopt.cc

diff --git a/gcc/testsuite/c-c++-common/torture/pr116098-2.c b/gcc/testsuite/c-c++-common/torture/pr116098-2.c
new file mode 100644 (file)
index 0000000..614ed04
--- /dev/null
@@ -0,0 +1,46 @@
+/* { dg-do run } */
+/* PR tree-optimization/116098 */
+
+
+#include <stdbool.h>
+
+struct Value {
+    int type;
+    union {
+        bool boolean;
+        long long t;
+    };
+};
+
+static struct Value s_item_mem;
+
+/* truthy was being miscompiled for the value.type==2 case,
+   because we would have a VCE from unsigned char to bool
+   that went from being conditional in the value.type==1 case
+   to unconditional when `value.type!=0`.
+   The move of the VCE from conditional to unconditional,
+   needs to changed into a convert (NOP_EXPR). */
+static bool truthy(void) __attribute__((noipa));
+static bool
+truthy(void)
+{
+    struct Value value = s_item_mem;
+    if (value.type == 0)
+      return 0;
+    if (value.type == 1)
+      return value.boolean;
+    return 1;
+}
+
+int
+main(void)
+{
+    s_item_mem.type = 2;
+    s_item_mem.t = -1;
+    bool b1 = !truthy();
+    s_item_mem.type = 1;
+    s_item_mem.boolean = b1;
+    bool b = truthy();
+    if (b1 != b)  __builtin_abort();
+    if (b) __builtin_abort();
+}
diff --git a/gcc/testsuite/g++.dg/torture/pr116098-1.C b/gcc/testsuite/g++.dg/torture/pr116098-1.C
new file mode 100644 (file)
index 0000000..90e44a6
--- /dev/null
@@ -0,0 +1,33 @@
+// { dg-do run }
+/* PR tree-optimization/116098 */
+
+
+static bool truthy(int type, unsigned char data) __attribute__((noipa));
+/* truthy was being miscompiled for the type==2 case,
+   because we would have a VCE from unsigned char to bool
+   that went from being conditional in the type==1 case
+   to unconditional when `type!=0`.
+   The move of the VCE from conditional to unconditional,
+   needs to changed into a convert (NOP_EXPR). */
+
+static bool truthy(void) __attribute__((noipa));
+static bool
+truthy(int type, unsigned char data)
+{
+    if (type == 0)
+      return 0;
+    if (type == 1)
+      /* Emulate what SRA does, so this can be
+        tested without depending on SRA. */
+      return __builtin_bit_cast (bool, data);
+    return 1;
+}
+
+int
+main(void)
+{
+    bool b1 = !truthy(2, -1);
+    bool b = truthy(1, b1);
+    if (b1 != b)  __builtin_abort();
+    if (b) __builtin_abort();
+}
index bd7f9607eb9a37f773a0e4fa3f57e5fa9679f19b..43b65b362a3964f2aa9905d087871c5f93cb25ec 100644 (file)
@@ -742,7 +742,8 @@ empty_bb_or_one_feeding_into_p (basic_block bb,
 }
 
 /* Move STMT to before GSI and insert its defining
-   name into INSERTED_EXPRS bitmap. */
+   name into INSERTED_EXPRS bitmap.
+   Also rewrite its if it might be undefined when unconditionalized.  */
 static void
 move_stmt (gimple *stmt, gimple_stmt_iterator *gsi, auto_bitmap &inserted_exprs)
 {
@@ -761,6 +762,31 @@ move_stmt (gimple *stmt, gimple_stmt_iterator *gsi, auto_bitmap &inserted_exprs)
   gimple_stmt_iterator gsi1 = gsi_for_stmt (stmt);
   gsi_move_before (&gsi1, gsi);
   reset_flow_sensitive_info (name);
+
+  /* Rewrite some code which might be undefined when
+     unconditionalized. */
+  if (gimple_assign_single_p (stmt))
+    {
+      tree rhs = gimple_assign_rhs1 (stmt);
+      /* VCE from integral types to another integral types but with
+        different precisions need to be changed into casts
+        to be well defined when unconditional. */
+      if (gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR
+         && INTEGRAL_TYPE_P (TREE_TYPE (name))
+         && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (rhs, 0))))
+       {
+         if (dump_file && (dump_flags & TDF_DETAILS))
+           {
+             fprintf (dump_file, "rewriting stmt with maybe undefined VCE ");
+             print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+           }
+         tree new_rhs = TREE_OPERAND (rhs, 0);
+         gcc_assert (is_gimple_val (new_rhs));
+         gimple_assign_set_rhs_code (stmt, NOP_EXPR);
+         gimple_assign_set_rhs1 (stmt, new_rhs);
+         update_stmt (stmt);
+       }
+    }
 }
 
 /* RAII style class to temporarily remove flow sensitive