This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][RFA][P1 PR tree-optimization/83298] Avoid over-optimistic result range for COND_EXPR


So the underlying issue here is quite simple.  Given something like

x = (cond) ? res1 : res2;

EVRP analysis will compute the resultant range using vrp_meet of the
ranges for res1 and res2.  Seems pretty natural.

vrp_meet makes optimistic assumptions if either range is VR_UNDEFINED
and will set the resultant range to the range of the other operand.

Some callers explicitly mention this is the desired behavior (PHI
processing).  Other callers avoid calling vrp_meet when one of the
ranges is VR_UNDEFINED and do something sensible
(extract_range_from_unary_expr, extract_range_from_binary_expr_1).

extract_range_from_cond_expr neither mentions that it wants the
optimistic behavior nor does it avoid calling vrp_meet with a
VR_UNDEFINED range.  It naturally seems to fit in better with the other
extract_range_from_* routines.

I'm not at all familiar with the ipa-cp bits, but from a quick look they
also seems to fall into the extract_* camp.


Anyway, normally in a domwalk the only place where we're going to see
VR_UNDEFINED would be in the PHI nodes.  It's one of the nice properties
of a domwalk :-)

However, for jump threading we look past the dominance frontier;
furthermore, we do not currently record ranges for statements we process
as part of the jump threading.  But we do try to extract the range each
statement generates -- we're primarily looking for cases where the
statement generates a singleton range.

While the plan does include recording ranges as we look past the
dominance frontier, I strongly believe some serious code cleanup in DOM
and jump threading needs to happen first.  So I don't want to go down
that path for gcc-8.

So we're kind-of stuck with the fact that we might query for a resultant
range when one or more input operands may not have recorded range
information.  Thankfully that's easily resolved by making
extract_range_from_cond_expr work like the other range extraction
routines and avoid calling vrp_meet when one or more operands is
VR_UNDEFINED.

Bootstrapped and regression tested on x86_64.  OK for the trunk?

Jeff

	PR tree-optimization/83298
	* vr-values.c (vr_values::extract_range_from_cond_expr): Do not
	call vrp_meet if one of the input operands is VR_UNDEFINED.

	PR tree-optimization/83298
	* gcc.c-torture/execute/pr83298.c: New test.

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr83298.c b/gcc/testsuite/gcc.c-torture/execute/pr83298.c
new file mode 100644
index 00000000000..0e51ababf5c
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr83298.c
@@ -0,0 +1,11 @@
+
+int a, b, c = 1;
+
+int main ()
+{
+  for (; b < 1; b++)
+    ;
+  if (!(c * (a < 1))) 
+    __builtin_abort ();
+  return 0; 
+}
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 9352e120d9d..ee5ae3c6a27 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -912,6 +912,23 @@ vr_values::extract_range_from_cond_expr (value_range *vr, gassign *stmt)
   else
     set_value_range_to_varying (&vr1);
 
+  /* If either range is VR_UNDEFINED, vrp_meet will make the optimistic
+     choice and use the range of the other operand as the result range.
+
+     Other users of vrp_meet either explicitly filter the calls for
+     this case, or they do not care (PHI processing where unexecutable
+     edges are explicitly expected to be ignored).
+
+     Like most other callers, we can not generally tolerate the optimistic
+     choice here.  Consider jump threading where we're looking into a
+     non-dominated block and thus may not necessarily have processed the
+     ranges for statements within that non-dominated block.  */
+  if (vr0.type == VR_UNDEFINED || vr1.type == VR_UNDEFINED)
+    {
+      set_value_range_to_varying (vr);
+      return;
+    }
+
   /* The resulting value range is the union of the operand ranges */
   copy_value_range (vr, &vr0);
   vrp_meet (vr, &vr1);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]