[PATCH] Fix questionable optimization of short-circuit operators

[Eric Botcazou]

Hi,

the compiler can swap the operands of the short-circuit operators in some 
cases, which can be problematic for tools like Valgrind that detect uses of 
uninitialized data, and is probably counterproductive in most cases.

The change prevents this from occurring, but also extends the use of jumps for 
conjunctions and disjunctions to Ada for targets where they are cheap.

Tested on x86-64/Linux, OK for the mainline?


2025-06-10  Eric Botcazou  <ebotca...@adacore.com>

        * cfgexpand.cc (has_cheap_jumps_and_no_conditional_compare): New
        predicate.
        (expand_gimple_cond): Call it.  Emit sequences of jumps for all
        boolean types if it returns true.
        (reorder_operands): Do not reorder the operands of expressions
        that expand_gimple_cond can turn into jumps.

-- 
Eric Botcazou
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 8919cc33539..a9564f3417f 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -2853,6 +2853,16 @@ maybe_cleanup_end_of_block (edge e, rtx_insn *last)
     }
 }
 
+/* Return true if jumps are cheap and the target does not support
+   conditional compare.  */
+
+static bool
+has_cheap_jumps_and_no_conditional_compare (void)
+{
+  return BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
+	 && !targetm.have_ccmp ();
+}
+
 /* A subroutine of expand_gimple_basic_block.  Expand one GIMPLE_COND.
    Returns a new basic block if we've terminated the current basic
    block and created a new one.  */
@@ -2900,11 +2910,9 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
 	    }
 	  /* If jumps are cheap and the target does not support conditional
 	     compare, turn some more codes into jumpy sequences.  */
-	  else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4
-		   && !targetm.have_ccmp ())
+	  else if (has_cheap_jumps_and_no_conditional_compare ())
 	    {
 	      if ((code2 == BIT_AND_EXPR
-		   && TYPE_PRECISION (TREE_TYPE (op0)) == 1
 		   && TREE_CODE (gimple_assign_rhs2 (second)) != INTEGER_CST)
 		  || code2 == TRUTH_AND_EXPR)
 		{
@@ -6053,11 +6061,21 @@ reorder_operands (basic_block bb)
 	  lattice[i] += lattice[gimple_uid (def_stmt)];
 	}
       i++;
-      if (!is_gimple_assign (stmt)
-	  || !commutative_tree_code (gimple_assign_rhs_code (stmt)))
+      if (!is_gimple_assign (stmt))
+	continue;
+      enum tree_code code = gimple_assign_rhs_code (stmt);
+      if (!commutative_tree_code (code))
+	continue;
+      /* Do not change operations expand_gimple_cond can turn into jumps.  */
+      if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt))) == BOOLEAN_TYPE
+	  && (code == TRUTH_AND_EXPR
+	      || code == TRUTH_OR_EXPR
+	      || code == BIT_AND_EXPR
+	      || code == BIT_IOR_EXPR)
+	  && has_cheap_jumps_and_no_conditional_compare ())
 	continue;
-      op0 = gimple_op (stmt, 1);
-      op1 = gimple_op (stmt, 2);
+      op0 = gimple_assign_rhs1 (stmt);
+      op1 = gimple_assign_rhs2 (stmt);
       if (TREE_CODE (op0) != SSA_NAME
 	  || TREE_CODE (op1) != SSA_NAME)
 	continue;