On Tue, Oct 20, 2020 at 6:19 PM Andrew MacLeod wrote:
>
> 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
> > To: GCC patches , Jakub Jelinek
> >
> > CC: Andrew MacLeod , Aldy Hernandez
> >
> >
> > 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.
Yes. We had a compare and trap in our original ranger branch, and
then it got lost somewhere in the transition to the staging branch :).
>
> 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.
Will do.
>
> Â 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?
Yes. A week or two would be fine. I think Jeff runs Fedora builds every week.
Pushed.
Aldy
>
> Andrew
>
