(Sorry for the delay in posting the re-spin, I was away last week.)
In this version we refactor to introudce a helper class (annotate_saver)
which is much less invasive to the caller (maybe_convert_cond) and
should (at least in theory) be reuseable elsewhere.
This version also relies on the assumption that operands 1 and 2 of
ANNOTATE_EXPRs are INTEGER_CSTs, which simplifies the flag updates
without having to rely on assumptions about the specific changes made
in maybe_convert_cond.
Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk?
Thanks,
Alex
-- >8 --
For the testcase added with this patch, we would end up losing the:
#pragma GCC unroll 4
and emitting "warning: ignoring loop annotation". That warning comes
from tree-cfg.cc:replace_loop_annotate, and means that we failed to
process the ANNOTATE_EXPR in tree-cfg.cc:replace_loop_annotate_in_block.
That function walks backwards over the GIMPLE in an exiting BB for a
loop, skipping over the final gcond, and looks for any ANNOTATE_EXPRS
immediately preceding the gcond.
The function documents the following pre-condition:
/* [...] We assume that the annotations come immediately before the
condition in BB, if any. */
now looking at the exiting BB of the loop, we have:
<bb 8> :
D.4524 = .ANNOTATE (iftmp.1, 1, 4);
retval.0 = D.4524;
if (retval.0 != 0)
goto <bb 3>; [INV]
else
goto <bb 9>; [INV]
and crucially there is an intervening assignment between the gcond and
the preceding .ANNOTATE ifn call. To see where this comes from, we can
look to the IR given by -fdump-tree-original:
if (<<cleanup_point ANNOTATE_EXPR <first != last && !use_find(short
int*)::<lambda(short int)>::operator() (&pred, *first), unroll 4>>>)
goto <D.4518>;
else
goto <D.4516>;
here the problem is that we've wrapped a CLEANUP_POINT_EXPR around the
ANNOTATE_EXPR, meaning the ANNOTATE_EXPR is no longer the outermost
expression in the condition.
The CLEANUP_POINT_EXPR gets added by the following call chain:
finish_while_stmt_cond
-> maybe_convert_cond
-> condition_conversion
-> fold_build_cleanup_point_expr
this patch chooses to fix the issue by first introducing a new helper
class (annotate_saver) to save and restore outer chains of
ANNOTATE_EXPRs and then using it in maybe_convert_cond.
With this patch, we don't get any such warning and the loop gets unrolled as
expected at -O2.
gcc/cp/ChangeLog:
PR libstdc++/116140
* semantics.cc (anotate_saver): New. Use it ...
(maybe_convert_cond): ... here, to ensure any ANNOTATE_EXPRs
remain the outermost expression(s) of the condition.
gcc/testsuite/ChangeLog:
PR libstdc++/116140
* g++.dg/ext/pragma-unroll-lambda.C: New test.
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 5ab2076b673..b1a49b14238 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -951,6 +951,86 @@ maybe_warn_unparenthesized_assignment (tree t, bool
nested_p,
}
}
+/* Helper class for saving/restoring ANNOTATE_EXPRs. For a tree node t, users
+ can construct one of these like so:
+
+ annotate_saver s (&t);
+
+ and t will be updated to have any annotations removed. The user can then
+ transform t, and later restore the ANNOTATE_EXPRs with:
+
+ t = s.restore (t).
+
+ The intent is to ensure that any ANNOTATE_EXPRs remain the outermost
+ expressions following any operations on t. */
+
+class annotate_saver {
+ /* The chain of saved annotations, if there were any. Otherwise null. */
+ tree m_annotations;
+
+ /* If M_ANNOTATIONS is non-null, then M_INNER points to TREE_OPERAND (A, 0)
+ for the innermost annotation A. */
+ tree *m_inner;
+
+public:
+ annotate_saver (tree *);
+ tree restore (tree);
+};
+
+/* If *COND is an ANNOTATE_EXPR, walk through the chain of annotations, and set
+ *COND equal to the first non-ANNOTATE_EXPR (saving a pointer to the
+ original chain of annotations for later use in restore). */
+
+annotate_saver::annotate_saver (tree *cond) : m_annotations (nullptr)
+{
+ tree *t = cond;
+ while (TREE_CODE (*t) == ANNOTATE_EXPR)
+ t = &TREE_OPERAND (*t, 0);
+
+ if (t != cond)
+ {
+ m_annotations = *cond;
+ *cond = *t;
+ m_inner = t;
+ }
+}
+
+/* If we didn't strip any annotations on construction, return NEW_INNER
+ unmodified. Otherwise, wrap the saved annotations around NEW_INNER
(updating
+ the types and flags of the annotations if needed) and return the resulting
+ expression. */
+
+tree
+annotate_saver::restore (tree new_inner)
+{
+ if (!m_annotations)
+ return new_inner;
+
+ /* If the type of the inner expression changed, we need to update the types
+ of all the ANNOTATE_EXPRs. We may need to update the flags too, but we
+ assume they only change if the type of the inner expression changes.
+ The flag update logic assumes that the other operands to the
+ ANNOTATE_EXPRs are always INTEGER_CSTs. */
+ if (TREE_TYPE (new_inner) != TREE_TYPE (*m_inner))
+ {
+ const bool new_readonly
+ = TREE_READONLY (new_inner) || CONSTANT_CLASS_P (new_inner);
+
+ for (tree c = m_annotations; c != *m_inner; c = TREE_OPERAND (c, 0))
+ {
+ gcc_checking_assert (TREE_CODE (c) == ANNOTATE_EXPR
+ && TREE_CODE (TREE_OPERAND (c, 1)) == INTEGER_CST
+ && TREE_CODE (TREE_OPERAND (c, 2)) ==
INTEGER_CST);
+ TREE_TYPE (c) = TREE_TYPE (new_inner);
+ TREE_SIDE_EFFECTS (c) = TREE_SIDE_EFFECTS (new_inner);
+ TREE_READONLY (c) = new_readonly;
+ }
+ }
+
+ *m_inner = new_inner;
+ return m_annotations;
+}
+
/* COND is the condition-expression for an if, while, etc.,
statement. Convert it to a boolean value, if appropriate.
In addition, verify sequence points if -Wsequence-point is enabled. */
@@ -966,6 +1046,9 @@ maybe_convert_cond (tree cond)
if (type_dependent_expression_p (cond))
return cond;
+ /* Strip any ANNOTATE_EXPRs from COND. */
+ annotate_saver annotations (&cond);
+
/* For structured binding used in condition, the conversion needs to be
evaluated before the individual variables are initialized in the
std::tuple_{size,elemenet} case. cp_finish_decomp saved the conversion
@@ -984,7 +1067,10 @@ maybe_convert_cond (tree cond)
/* Do the conversion. */
cond = convert_from_reference (cond);
- return condition_conversion (cond);
+ cond = condition_conversion (cond);
+
+ /* Restore any ANNOTATE_EXPRs around COND. */
+ return annotations.restore (cond);
}
/* Finish an expression-statement, whose EXPRESSION is as indicated. */
diff --git a/gcc/testsuite/g++.dg/ext/pragma-unroll-lambda.C
b/gcc/testsuite/g++.dg/ext/pragma-unroll-lambda.C
new file mode 100644
index 00000000000..f410f6d8d25
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/pragma-unroll-lambda.C
@@ -0,0 +1,17 @@
+// { dg-do compile { target c++11 } }
+
+template<typename Iter, typename Pred>
+inline Iter
+my_find(Iter first, Iter last, Pred pred)
+{
+#pragma GCC unroll 4
+ while (first != last && !pred(*first))
+ ++first;
+ return first;
+}
+
+short *use_find(short *p)
+{
+ auto pred = [](short x) { return x == 42; };
+ return my_find(p, p + 1024, pred);
+}
