The heurstics that was added for PR71016, try to search to see
if the conversion was being moved away from its definition. The problem
is the heurstics would stop if there was a non GIMPLE_ASSIGN (and already 
ignores
debug statements) and in this case we would have a GIMPLE_LABEL that was not
being ignored. So we should need to ignore GIMPLE_NOP, GIMPLE_LABEL and 
GIMPLE_PREDICT.
Note this is now similar to how gimple_empty_block_p behaves.

Note this fixes the wrong code that was reported by moving the VCE (conversion) 
out before
the phiopt/match could convert it into an bit_ior and move the VCE out with the 
VCE being
conditionally valid.

Bootstrapped and tested on x86_64-linux-gnu.
Also built and tested for aarch64-linux-gnu.

        PR tree-optimization/116098

gcc/ChangeLog:

        * tree-ssa-phiopt.cc (factor_out_conditional_operation): Ignore
        nops, labels and predicts for heuristic for conversion with a constant.

gcc/testsuite/ChangeLog:

        * c-c++-common/torture/pr116098-1.c: New test.
        * gcc.target/aarch64/csel-1.c: New test.

Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
---
 .../c-c++-common/torture/pr116098-1.c         | 84 +++++++++++++++++++
 gcc/testsuite/gcc.target/aarch64/csel-1.c     | 28 +++++++
 gcc/tree-ssa-phiopt.cc                        |  9 +-
 3 files changed, 119 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/torture/pr116098-1.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/csel-1.c

diff --git a/gcc/testsuite/c-c++-common/torture/pr116098-1.c 
b/gcc/testsuite/c-c++-common/torture/pr116098-1.c
new file mode 100644
index 00000000000..b9d9a342305
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/pr116098-1.c
@@ -0,0 +1,84 @@
+/* { dg-do run } */
+/* PR tree-optimization/116098 */
+/* truthy was being miscompiled where the VCE was not being pulled out
+   of the if statement by factor_out_conditional_operation before the rest of
+   phiopt would happen which assumed VCE would be correct. */
+/* The unused label was causing truthy to have different code generation than 
truthy_1. */
+
+
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+
+enum ValueType {
+        VALUE_BOOLEAN,
+        VALUE_NUM,
+};
+
+struct Value {
+    enum ValueType type;
+    union {
+        bool boolean;
+        int num;
+    };
+};
+
+static struct Value s_value;
+static bool s_b;
+
+
+bool truthy_1(void) __attribute__((noinline));
+bool
+truthy_1(void)
+{
+    struct Value value = s_value;
+    if (s_b) s_b = 0;
+    enum ValueType t = value.type;
+    if (t != VALUE_BOOLEAN)
+      return 1;
+  return value.boolean;
+}
+bool truthy(void) __attribute__((noinline));
+bool
+truthy(void)
+{
+    struct Value value = s_value;
+    if (s_b) s_b = 0;
+    enum ValueType t = value.type;
+    if (t != VALUE_BOOLEAN)
+      return 1;
+  /* This unused label should not cause any difference in code generation. */
+a: __attribute__((unused));
+  return value.boolean;
+}
+
+int
+main(void)
+{
+    s_b = 0;
+    s_value = (struct Value) {
+        .type = VALUE_NUM,
+        .num = 2,
+    };
+    s_value = (struct Value) {
+        .type = VALUE_BOOLEAN,
+        .boolean = !truthy_1(),
+    };
+    bool b = truthy_1();
+    if (b)
+      __builtin_abort();
+
+    s_b = 0;
+    s_value = (struct Value) {
+        .type = VALUE_NUM,
+        .num = 2,
+    };
+    s_value = (struct Value) {
+        .type = VALUE_BOOLEAN,
+        .boolean = !truthy(),
+    };
+    b = truthy();
+    if (b)
+      __builtin_abort();
+}
+
diff --git a/gcc/testsuite/gcc.target/aarch64/csel-1.c 
b/gcc/testsuite/gcc.target/aarch64/csel-1.c
new file mode 100644
index 00000000000..a20d39ea375
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/csel-1.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* These 2 functions should be the same; even though there is a label in f1. 
+   The label should not make a difference in code generation.
+   There sign extend should be removed as it is not needed. */
+void f(int t, int a, short *b)
+{
+  short t1 = 1;
+  if (a)
+    {
+      t1 = t;
+    }
+  *b = t1;
+}
+
+void f1(int t, int a, short *b)
+{
+  short t1 = 1;
+  if (a)
+    {
+      label1: __attribute__((unused))
+      t1 = t;
+    }
+  *b = t1;
+}
+
+/* { dg-final { scan-assembler-not "sxth\t" } } */
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 9a009e187ee..271a5d51f09 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -324,8 +324,13 @@ factor_out_conditional_operation (edge e0, edge e1, gphi 
*phi,
                  gsi_prev_nondebug (&gsi);
                  if (!gsi_end_p (gsi))
                    {
-                     if (gassign *assign
-                           = dyn_cast <gassign *> (gsi_stmt (gsi)))
+                     gimple *stmt = gsi_stmt (gsi);
+                     /* Ignore nops, predicates and labels. */
+                     if (gimple_code (stmt) == GIMPLE_NOP
+                         || gimple_code (stmt) == GIMPLE_PREDICT
+                         || gimple_code (stmt) == GIMPLE_LABEL)
+                       ;
+                     else if (gassign *assign = dyn_cast <gassign *> (stmt))
                        {
                          tree lhs = gimple_assign_lhs (assign);
                          enum tree_code ass_code
-- 
2.43.0

Reply via email to