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.
This commit is contained in:
Alex Coplan 2024-08-02 09:52:50 +01:00
parent 6291f25631
commit f97d86242b
2 changed files with 104 additions and 1 deletions

View File

@ -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. */

View File

@ -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);
}