[Bug middle-end/107642] New: LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[Bug middle-end/107642] New: LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

            Bug ID: 107642
           Summary: LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: documentation, internal-improvement,
                    missed-optimization
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

LOGICAL_OP_NON_SHORT_CIRCUIT is defined as:

@defmac LOGICAL_OP_NON_SHORT_CIRCUIT
Define this macro if a non-short-circuit operation produced by
@samp{fold_range_test ()} is optimal.  This macro defaults to true if
@code{BRANCH_COST} is greater than or equal to the value 2.
@end defmac

Some targets define it to 0 or something else:
apinski@xeond:~/src/upstream-gcc-git/gcc/gcc/config$ git grep
LOGICAL_OP_NON_SHORT_CIRCUIT */*.h
arc/arc.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT \
arm/arm.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT                                 
\
csky/csky.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT \
loongarch/loongarch.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
mips/mips.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
msp430/msp430.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
nds32/nds32.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
pru/pru.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT          0
riscv/riscv.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
rs6000/rs6000.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0
visium/visium.h:#define LOGICAL_OP_NON_SHORT_CIRCUIT 0

arc.h:
#define LOGICAL_OP_NON_SHORT_CIRCUIT \
  (BRANCH_COST (optimize_function_for_speed_p (cfun), \
                false) > 9)

arm.h:
#define LOGICAL_OP_NON_SHORT_CIRCUIT                                    \
  ((optimize_size)                                                      \
   ? (TARGET_THUMB ? false : true)                                      \
   : TARGET_THUMB ? static_cast<bool>
(current_tune->logical_op_non_short_circuit_thumb) \
   : static_cast<bool> (current_tune->logical_op_non_short_circuit_arm))

csky.h (which is the default it seems):
#define LOGICAL_OP_NON_SHORT_CIRCUIT \
  (csky_default_logical_op_non_short_circuit ())
...
bool
csky_default_logical_op_non_short_circuit (void)
{
  return BRANCH_COST (optimize_function_for_speed_p (cfun), false) >= 2;
}

LOGICAL_OP_NON_SHORT_CIRCUIT is used in many few places than fold_range_test
these days too.
fold-const.cc:#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
fold-const.cc:#define LOGICAL_OP_NON_SHORT_CIRCUIT \
fold-const.cc:  bool logical_op_non_short_circuit =
LOGICAL_OP_NON_SHORT_CIRCUIT;
fold-const.cc:  bool logical_op_non_short_circuit =
LOGICAL_OP_NON_SHORT_CIRCUIT;
tree-ssa-ifcombine.cc:#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
tree-ssa-ifcombine.cc:#define LOGICAL_OP_NON_SHORT_CIRCUIT \
tree-ssa-ifcombine.cc:    bool logical_op_non_short_circuit =
LOGICAL_OP_NON_SHORT_CIRCUIT;

fold_truth_andor in fold-const.cc
ifcombine_ifandif in tree-ssa-ifcombine.cc

What LOGICAL_OP_NON_SHORT_CIRCUIT is trying to say is that expansion (from
gimple to RTL) of
things like `a & b` (where a and b are bools) is cheaper than expanding out
using jumps.

Note this is not exactly true as there is code which does that again in
dojump.cc:
      /* High branch cost, expand as the bitwise OR of the conditions.
         Do the same if the RHS has side effects, because we're effectively
         turning a TRUTH_OR_EXPR into a TRUTH_ORIF_EXPR.  */
      if (BRANCH_COST (optimize_insn_for_speed_p (), false) >= 4
          || TREE_SIDE_EFFECTS (TREE_OPERAND (exp, 1)))

[Bug middle-end/107642] LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-11-11
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

[Bug middle-end/107642] LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
A --param option was added in 2018 to override the setting of
LOGICAL_OP_NON_SHORT_CIRCUIT :
https://gcc.gnu.org/legacy-ml/gcc-patches/2018-11/msg02604.html

But the way it is implemented is such that the override needs to happen in each
place that LOGICAL_OP_NON_SHORT_CIRCUIT is used. This is fragil too.

[Bug middle-end/107642] LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
LOGICAL_OP_NON_SHORT_CIRCUIT has to be defined (if not already defined) in each
file that uses it too.
fold-const.cc:#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
tree-ssa-ifcombine.cc:#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT

It would be better if it is defined in one place or better yet changed really
to a target hook.

[Bug middle-end/107642] LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=17107

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
r0-49401-g85e50b6ba8ca22 introduced RANGE_TEST_NON_SHORT_CIRCUIT.
r0-63607-gb8610a53763f72 renamed it to LOGICAL_OP_NON_SHORT_CIRCUIT.

r0-126134-g5d2a9da9a7f7c1 introduced LOGICAL_OP_NON_SHORT_CIRCUIT to
tree-ssa-ifcombine.c to basically do what was done in fold-const.c at the time.

But again as mentioned dojump.c still undid what was done on the gimple level.

[Bug middle-end/107642] LOGICAL_OP_NON_SHORT_CIRCUIT vs BRANCH_COST confusion

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107642

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
At some point one of the ideas was to avoid deciding this "early" and leave it
entirely to RTL expansion what to do to have more similar GIMPLE through the
pipeline.  That eventually means prefering if (a & b) to if (a && b) on GIMPLE,
RTL expansion can only change it in one direction.