Hello Jakub,
Thank you for the review. I had some questions.
On Tue, Jul 7, 2020 at 4:13 AM Jakub Jelinek <ja...@redhat.com> wrote:
> On Fri, Jul 03, 2020 at 10:43:55PM -0400, y2s1982 via Gcc-patches wrote:
>
> > + switch (id_size)
> > + {
> > + case 1:
> > + *output_id = (_gompd_device_id) *((__UINT8_TYPE__ *) input_id);
> > + break;
> > + case 2:
> > + *output_id = (_gompd_device_id) *((__UINT16_TYPE__ *) input_id);
>
> I have no idea what this function is doing, but e.g. from aliasing point of
> view trying to access something as short/int/long long is dangerous, and
> there might be alignment implications too.
>
This function is used in ompd_device_initialize(). The initializing function
receives a void *id and ompd_size_t sizeof_id. My first attempt tried to
just
preserve both information as is, but I wasn't sure how the void * would
ultimately be read. In the second attempt, I tried to cast the value, based
on
the sizeof_id, and store it in a large enough type. This does assume the
void * is pointing at a numerical value.
What would be the best way to handle the void *id?
>
>
> > --- /dev/null
> > +++ b/libgomp/ompd-types.h
>
> ompd-types.h is an installed header I think, so Makefile.am should install
> it.
>
Is this similar to how omp-tools.h was handled before?
Cheers,
Tony
>
> Jakub
>
>
