PING: Fwd: [PATCH] Refactor range handling of builtins in vr_values and ranger.

[Andrew MacLeod]

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 <aldyh@redhat.com>
> To: GCC patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek 
> <jakub@redhat.com>
> CC: Andrew MacLeod <amacleod@redhat.com>, Aldy Hernandez 
> <aldyh@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