Thanks Jeff and Richard for suggestion and reviewing.
Have another try in phiopt to do the convert from PHI to stmt = cond ? a : b.
It can perform the convert from PHI to stmt = cond ? a : b successfully, and
then
the widen-mul is able to do the recog to .SAT_ADD.
For now, to limit the risck, the above convert from PHI to stmt = cond ? a : b
only be performed when matched,
as well as the backend support the usadd standard name. Unfortunately, I am
stuck in the case that when the lhs
is not matched, we need to clean up something like created stmt in previous, or
we will have ICE for missing definition.
sat_add.c: In function ‘sat_add_u_3_uint8_t’:
sat_add.c:69:1: error: missing definition
69 | SAT_ADD_U_3(uint8_t);
| ^~~~~~~~~~~
for SSA_NAME: _6 in statement:
# VUSE <.MEM_14(D)>
return _6;
during GIMPLE pass: phiopt
dump file: sat_add.c.046t.phiopt1
sat_add.c:69:1: internal compiler error: verify_ssa failed
0x1db41ba verify_ssa(bool, bool
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/tree-ssa.cc:1203
0x18e3075 execute_function_todo
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/passes.cc:2096
0x18e1c52 do_per_function
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/passes.cc:1688
0x18e3222 execute_todo
I bet the reason is that we created new stmt like stmt_cond and stmt_val but we
don't insert it.
Thus, there will be orphan nodes somewhere and we need something like rollback
to recover the
gimple up to a point. I tried sorts of release_xx or likewise but seems not
working.
So is there any suggest to take care of such gimple rollback or another
solution for this? Below are
The function to perform the convert from PHI to stmt = cond ? a : b for
reference, thanks a lot.
Pan
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 918cf50b589..7982b65bac4 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -486,6 +486,88 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
}
}
+extern bool gimple_unsigned_integer_sat_add (tree, tree*, tree (*)(tree));
+
+/* Try to match the phi expr to the gimple cond. Return true if we can
+ perform the convert or return false. There will be some restrictions
+ or such kind of conversion, aka:
+
+ 1. Only selected pattern will try this convert.
+ 2. The generated gassign matched the selected IFN pattern.
+ 3. The backend has implement the standard name.
+
+ From:
+ <bb 2> :
+ _1 = x_3(D) + y_4(D);
+ if (_1 >= x_3(D))
+ goto <bb 3>; [INV]
+ else
+ goto <bb 4>; [INV]
+
+ <bb 3> :
+
+ <bb 4> :
+ # _2 = PHI <255(2), _1(3)>
+
+ To:
+ <bb 2> :
+ _1 = x_3(D) + y_4(D);
+ phi_cond_6 = _1 >= x_3(D);
+ _2 = phi_cond_6 ? _1 : 255; */
+
+static bool
+match_phi_to_gimple_cond (basic_block cond_bb, gphi *phi, tree arg0, tree arg1)
+{
+ gcond *cond = as_a <gcond *> (*gsi_last_bb (cond_bb));
+
+ if (!cond)
+ return false;
+
+ enum tree_code code = gimple_cond_code (cond);
+ tree phi_result = gimple_phi_result (phi);
+ tree cond_tree = make_temp_ssa_name (boolean_type_node, NULL, "phi_cond");
+ tree cmp_tree = build2 (code, boolean_type_node, gimple_cond_lhs (cond),
+ gimple_cond_rhs (cond));
+ tree rhs = build3 (COND_EXPR, TREE_TYPE (phi_result), cond_tree, arg0, arg1);
+
+ gassign *stmt_cond = gimple_build_assign (cond_tree, cmp_tree);
+ gassign *stmt_val = gimple_build_assign (phi_result, rhs);
+
+ tree ops[2];
+ tree lhs = gimple_assign_lhs (stmt_val);
+ bool matched_p = (gimple_unsigned_integer_sat_add (lhs, ops, NULL)
+ && direct_internal_fn_supported_p (IFN_SAT_ADD, TREE_TYPE (lhs),
+ OPTIMIZE_FOR_BOTH));
+
+ if (matched_p)
+ {
+ gimple_stmt_iterator gsi = gsi_last_bb (cond_bb);
+ gimple_stmt_iterator psi = gsi_for_stmt (phi);
+
+ gsi_insert_before (&gsi, stmt_cond, GSI_SAME_STMT);
+ gsi_insert_before (&gsi, stmt_val, GSI_SAME_STMT);
+ remove_phi_node (&psi, false);
+ }
+ else
+ {
+ // Clean up the stmt created, but non of blow works well.
+ // gsi = gsi_for_stmt (stmt_val);
+ // gsi_remove (&gsi, true);
+ // release_defs (stmt_val);
+ // ggc_free (stmt_val);
+
+ // gsi = gsi_for_stmt (stmt_cond);
+ // gsi_remove (&gsi, true);
+ // release_defs (stmt_cond);
+ // ggc_free (stmt_cond);
+
+ // release_defs (stmt_cond);
+ // release_defs (stmt_val);
+ release_ssa_name (cond_tree);
+ }
+
+ return matched_p;
+}
+
/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT.
Return NULL if nothing can be simplified or the resulting simplified value
with parts pushed if EARLY_P was true. Also rejects non allowed tree code
@@ -826,6 +908,9 @@ match_simplify_replacement (basic_block cond_bb,
basic_block middle_bb,
So, given the condition COND, and the two PHI arguments, match and
simplify
can happen on (COND) ? arg0 : arg1. */
+ if (match_phi_to_gimple_cond (cond_bb, phi, arg0, arg1))
+ return true;
+
stmt = last_nondebug_stmt (cond_bb);
/* We need to know which is the true edge and which is the false
-----Original Message-----
From: Jeff Law <jeffreya...@gmail.com>
Sent: Thursday, May 23, 2024 10:59 PM
To: Richard Biener <richard.guent...@gmail.com>; Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com;
tamar.christ...@arm.com; pins...@gmail.com
Subject: Re: [PATCH v2] Match: Support __builtin_add_overflow branch form for
unsigned SAT_ADD
On 5/23/24 6:14 AM, Richard Biener wrote:
> On Thu, May 23, 2024 at 1:08 PM Li, Pan2 <pan2...@intel.com> wrote:
>>
>> I have a try to convert the PHI from Part-A to Part-B, aka PHI to _2 =
>> phi_cond ? _1 : 255.
>> And then we can do the matching on COND_EXPR in the underlying widen-mul
>> pass.
>>
>> Unfortunately, meet some ICE when verify_gimple_phi in sccopy1 pass =>
>> sat_add.c:66:1: internal compiler error: tree check: expected class ‘type’,
>> have ‘exceptional’ (error_mark) in useless_type_conversion_p, at
>> gimple-expr.cc:86
>
> Likely you have released _2, more comments below on your previous mail.
You can be sure by calling debug_tree () on the SSA_NAME node in
question. If it reports "in-free-list", then that's definitive that the
SSA_NAME was released back to the SSA_NAME manager. If that SSA_NAME is
still in the IL, then that's very bad.
jeff
