On 10/19/20 6:03 AM, Aldy Hernandez wrote:



-------- Forwarded Message --------
Subject: [PATCH] Refactor range handling of builtins in vr_values and ranger.
Date: Fri,  9 Oct 2020 14:32:05 +0200
From: Aldy Hernandez <al...@redhat.com>
To: GCC patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <ja...@redhat.com> CC: Andrew MacLeod <amacl...@redhat.com>, Aldy Hernandez <al...@redhat.com>

Hi Jakub.

As the last known expert in this area, would you review this, please? :)

This sets things up so we can share range handling of builtins between
vr_values and ranger.  It is meant to refactor the code so that we can
verify that both implementations yield the same results.

First, we abstract out gimple_ranger::range_of_builtin_call into an externally
visible counterpart that can be called from vr_values.  It will take a
range_query since both ranger and vr_values inherit from this base class.

Then we abstract out all the builtin handling in vr_values into a separate
method that is easier to compare against.

Finally, we call the ranger version from vr_values and compare it with the
vr_values version.  Since this proves both versions return the same,
we can remove vr_values::extract_range_builtin in a follow-up patch.

The vr_values::range_of_expr change brings the vr_values version up to par
with the ranger version.  It should've handled non-SSA's.  This was
a small oversight that went unnoticed because the vr_value version isn't
stressed nearly as much as the ranger version.  The change is needed because the ranger code handling builtins calls, may call it for integer arguments
in range_of_builtin_ubsan_call.

There should be no change in functionality.

Tested on x86_64, with aarch64 tests still going.

OK provided aarch64 tests finish this century?




IIRC you basically duplicated the builtin code from vr-values and adapted it, we just never got back to consolidating them.  Until range_query i guess that would have been more difficult.

I think you should also post the followup patch which removes the old builtin range extraction.  There shouldn't be much churn so it's not a waste of time?  It would just be useful to see the other half.

 This is OK,and the plan is to leave the verification code in place for a week or two to allow OS builds and various other things to bounce off it just as a double check?

Andrew

Reply via email to