aboutsummaryrefslogtreecommitdiff
path: root/gcc
diff options
context:
space:
mode:
authorAndrew Pinski <quic_apinski@quicinc.com>2024-08-30 10:36:24 -0700
committerAndrew Pinski <quic_apinski@quicinc.com>2024-08-31 09:16:40 -0700
commitceda727dafba6e05b510b5f8f4ccacfb507dc023 (patch)
tree9896f3538c5f7be3db4a9936faba20aa967e35fe /gcc
parent457805cf5969c8a9a04f0c6e626946d952163929 (diff)
downloadgcc-ceda727dafba6e05b510b5f8f4ccacfb507dc023.zip
gcc-ceda727dafba6e05b510b5f8f4ccacfb507dc023.tar.gz
gcc-ceda727dafba6e05b510b5f8f4ccacfb507dc023.tar.bz2
phiopt: Ignore some nop statements in heursics [PR116098]
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_apinski@quicinc.com>
Diffstat (limited to 'gcc')
-rw-r--r--gcc/testsuite/c-c++-common/torture/pr116098-1.c84
-rw-r--r--gcc/testsuite/gcc.target/aarch64/csel-1.c28
-rw-r--r--gcc/tree-ssa-phiopt.cc9
3 files changed, 119 insertions, 2 deletions
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 0000000..b9d9a34
--- /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 0000000..a20d39e
--- /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 9a009e1..271a5d5 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