On 10/20/2016 03:32 AM, Richard Biener wrote:
Starting with some high level design comments. If these conflict with
comments from others, let me know and we'll work through the issues.
I don't really like introducing code conditional on the target capabilities
this early in the gimple optimization pipeline.
It's basically done right before RTL expansion
(pass_optimize_widening_mul).
I didn't look at the placement of the pass as it hadn't changed.
Looking at that now, I see it's fairly late in the pipeline, so I won't
object to its placement.
Would it be possible to always do the transformation to divmod in the gimple
optimizers, regardless of the target capabilities. Then in the gimple->RTL
expanders make a final decision about divmod insn, libcall, or using div/mod
insns?
The issue is that it hoists one or both the division or the modulo and
if we don't do the transform we'd want to undo that code motion.
But don't we only hoist when we find a suitable div/mod pair to turn
into a divmod? Maybe I'm mis-understanding how the identification works.
[ snip ]
*/
+
+ FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, op1)
+ {
+ if (is_gimple_assign (use_stmt)
+ && (gimple_assign_rhs_code (use_stmt) == TRUNC_DIV_EXPR
+ || gimple_assign_rhs_code (use_stmt) == TRUNC_MOD_EXPR)
+ && operand_equal_p (op1, gimple_assign_rhs1 (use_stmt), 0)
+ && operand_equal_p (op2, gimple_assign_rhs2 (use_stmt), 0))
So why check for TRUNC_MOD_EXPR here? ISTM that stmt is always going to be
TRUNC_MOD_EXPR and thus you're only interested in looking for a matching
TRUNC_DIV_EXPR statement.
The only way I could see TRUNC_MOD_EXPR being useful here would be if there is
a redundant TRUNC_MOD_EXPR in the IL, which would be a sign that some other
optimizer hasn't done its job. Have you seen this to be useful in practice?
Note that I've reviewed this parts already and we restructured things
in this way, requiring to look for TRUNC_MOD_EXPR to properly handle
finding a dominating trunc _or_ div and interatively doing so
correctly if we have more than one pair.
But doesn't having more than one pair essentially mean that the other
optimziers have left redundant junk in the IL? I'm clearly missing
something here.
+
+ gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
+ gimple_assign_set_rhs_from_tree (&gsi, new_rhs);
+ update_stmt (use_stmt);
+
+ if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt))
+ cfg_changed = true;
Does this ever happen given how you filter out throwing statements earlier?
We filter out throwing stmts that are not "top". The top one we replace
with the divmod call and we can preserve EH for that (and thus handle
the case where the original div/mod at this place may throw).
OK. It just struck me as a bit odd. No worries if y'all have already
worked through this.
Jeff
