[Mesa-dev] [PATCH] i965/urb: fixes division by zero

[Ardinartsev Nikita]

25.05.2016 00:44, Ben Widawsky пишет:
> On Tue, May 17, 2016 at 11:50:28AM -0700, Matt Turner wrote:
>> On Mon, May 16, 2016 at 4:27 PM, Ardinartsev Nikita <ardinar23 at gmail.com> wrote:
>>> Fixes regression introduced by af5ca43f2676bff7499f93277f908b681cb821d0
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95419
>>
>> Thank you very much for the patch. It is
>>
>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>>
>> I'll commit it shortly.
>
> Great patch.
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
>
>>
>>> ---
>>>   src/mesa/drivers/dri/i965/gen7_urb.c | 24 +++++-------------------
>>>   1 file changed, 5 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c
>>> index a412a42..4194541 100644
>>> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
>>> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
>>> @@ -292,25 +292,11 @@ gen7_upload_urb(struct brw_context *brw)
>>>      if (remaining_space > total_wants)
>>>         remaining_space = total_wants;
>>>      if (remaining_space > 0) {
>>> -      unsigned vs_additional = (unsigned)
>>> -         roundf(vs_wants * (((float) remaining_space) / total_wants));
>>> -      vs_chunks += vs_additional;
>>> -      remaining_space -= vs_additional;
>>> -      total_wants -= vs_wants;
>>> -
>>> -      unsigned hs_additional = (unsigned)
>>> -         round(hs_wants * (((double) remaining_space) / total_wants));
>>> -      hs_chunks += hs_additional;
>>> -      remaining_space -= hs_additional;
>>> -      total_wants -= hs_wants;
>>> -
>>> -      unsigned ds_additional = (unsigned)
>>> -         round(ds_wants * (((double) remaining_space) / total_wants));
>>> -      ds_chunks += ds_additional;
>>> -      remaining_space -= ds_additional;
>>> -      total_wants -= ds_wants;
>>> -
>>> -      gs_chunks += remaining_space;
>>> +      float ratio = ((float) remaining_space) / total_wants;
>>> +      vs_chunks += roundf(vs_wants * ratio);
>>> +      hs_chunks += roundf(hs_wants * ratio);
>>> +      ds_chunks += roundf(ds_wants * ratio);
>>> +      gs_chunks += roundf(gs_wants * ratio);
>>
>> I plan to change these roundf() calls to lroundf().
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

This patch has been already refused as it sometimes allocates more 
chunks than available due to rounding up issues. New variants has been 
proposed on bugzilla https://bugs.freedesktop.org/show_bug.cgi?id=95419. 
One is to always use rounding down and second to simply add zero checks 
in original code, as original code has the virtue of always allocating 
all available chunks.

diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
b/src/mesa/drivers/dri/i965/gen7_urb.c
index a412a42..86dcbd0 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -292,25 +292,10 @@ gen7_upload_urb(struct brw_context *brw)
     if (remaining_space > total_wants)
        remaining_space = total_wants;
     if (remaining_space > 0) {
-      unsigned vs_additional = (unsigned)
-         roundf(vs_wants * (((float) remaining_space) / total_wants));
-      vs_chunks += vs_additional;
-      remaining_space -= vs_additional;
-      total_wants -= vs_wants;
-
-      unsigned hs_additional = (unsigned)
-         round(hs_wants * (((double) remaining_space) / total_wants));
-      hs_chunks += hs_additional;
-      remaining_space -= hs_additional;
-      total_wants -= hs_wants;
-
-      unsigned ds_additional = (unsigned)
-         round(ds_wants * (((double) remaining_space) / total_wants));
-      ds_chunks += ds_additional;
-      remaining_space -= ds_additional;
-      total_wants -= ds_wants;
-
-      gs_chunks += remaining_space;
+      vs_chunks += (vs_wants * remaining_space) / total_wants;
+      hs_chunks += (hs_wants * remaining_space) / total_wants;
+      ds_chunks += (ds_wants * remaining_space) / total_wants;
+      gs_chunks += (gs_wants * remaining_space) / total_wants;
     }

     /* Sanity check that we haven't over-allocated. */

-----------------------------------------------------------------------

diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
b/src/mesa/drivers/dri/i965/gen7_urb.c
index a412a42..87c50c4 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -298,17 +298,21 @@ gen7_upload_urb(struct brw_context *brw)
        remaining_space -= vs_additional;
        total_wants -= vs_wants;

-      unsigned hs_additional = (unsigned)
-         round(hs_wants * (((double) remaining_space) / total_wants));
-      hs_chunks += hs_additional;
-      remaining_space -= hs_additional;
-      total_wants -= hs_wants;
-
-      unsigned ds_additional = (unsigned)
-         round(ds_wants * (((double) remaining_space) / total_wants));
-      ds_chunks += ds_additional;
-      remaining_space -= ds_additional;
-      total_wants -= ds_wants;
+      if(total_wants > 0) {
+          unsigned hs_additional = (unsigned)
+             round(hs_wants * (((double) remaining_space) / total_wants));
+          hs_chunks += hs_additional;
+          remaining_space -= hs_additional;
+          total_wants -= hs_wants;
+      }
+
+      if(total_wants > 0) {
+          unsigned ds_additional = (unsigned)
+             round(ds_wants * (((double) remaining_space) / total_wants));
+          ds_chunks += ds_additional;
+          remaining_space -= ds_additional;
+          total_wants -= ds_wants;
+      }

        gs_chunks += remaining_space;
     }