Richard Biener <rguent...@suse.de> writes: > On Mon, 29 Jul 2024, Prathamesh Kulkarni wrote: > >> Hi Richard, >> Thanks for your suggestions on RFC email, the attached patch adds support >> for streaming of poly_int when it's degree <= accel's NUM_POLY_INT_COEFFS. >> The patch changes streaming of poly_int as follows: >> >> Streaming out poly_int: >> >> degree = poly_int.degree(); >> stream out degree; >> for (i = 0; i < degree; i++) >> stream out poly_int.coeffs[i]; >> >> Streaming in poly_int: >> >> stream in degree; >> if (degree > NUM_POLY_INT_COEFFS) >> fatal_error(); >> stream in coeffs; >> // Set remaining coeffs to zero in case degree < accel's NUM_POLY_INT_COEFFS >> for (i = degree; i < NUM_POLY_INT_COEFFS; i++) >> poly_int.coeffs[i] = 0; >> >> Patch passes bootstrap+test and LTO bootstrap+test on aarch64-linux-gnu. >> LTO bootstrap+test on x86_64-linux-gnu in progress. >> >> I am not quite sure how to test it for offloading since currently it's >> (entirely) broken for aarch64->nvptx. >> I can give a try with x86_64->nvptx offloading if required (altho I guess >> LTO bootstrap should test streaming changes ?) > > + unsigned degree > + = bp_unpack_value (bp, BITS_PER_UNIT * sizeof (unsigned > HOST_WIDE_INT)); > > The NUM_POLY_INT_COEFFS target define doesn't seem to be constrained > to any type it needs to fit into, using HOST_WIDE_INT is arbitrary. > I'd say we should constrain it to a reasonable upper bound, > like 2? Maybe even have MAX_NUM_POLY_INT_COEFFS or > NUM_POLY_INT_COEFFS_BITS in poly-int.h and constrain NUM_POLY_INT_COEFFS. > > The patch looks reasonable over all, but Richard S. should have a say > about the abstraction you chose and the poly-int adjustment.
Sorry if this has been discussed already, but could we instead stream NUM_POLY_INT_COEFFS once per file, rather than once per poly_int? It's a target invariant, and poly_int has wormed its way into lots of things by now :) Thanks, Richard
