On Tue, Oct 20, 2020 at 6:19 PM Andrew MacLeod <amacl...@redhat.com> 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 <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.
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 >
