c++: Ensure ANNOTATE_EXPRs remain outermost expressions in conditions [PR116140]

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 3e117c216da52b961f30fc1d5b79c48af8cac12b..63212afafb3b75aae66dfd34a511c8febcdf9fc8 100644 (file)
--- 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 (file)
index 0000000..f410f6d
--- /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);
+}