[PATCH] D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs.

[Gheorghe-Teodor Bercea via Phabricator via llvm-commits]

gtbercea added a comment.

In https://reviews.llvm.org/D14254#971108, @Hahnfeld wrote:

> In https://reviews.llvm.org/D14254#970948, @gtbercea wrote:
>
> > In https://reviews.llvm.org/D14254#969452, @Hahnfeld wrote:
> >
> > > Two global remarks:
> > >
> > > 1. I think we agreed on having `<thread id> % <warp size>` instead of bit operations.
> >
> >
> > What's wrong with bit-wise operations as long as they are documented? I think we should keep them and then comment what it is that they do.
>
>
> Please see my discussion with Samuel: The code means to do a modulo and the bit operation is an optimization that the compiler can do.


Good point. Will this optimization be applied by the compiler regardless of kernel size, placement in the code etc?

> 
> 
>>> 2. Somewhat related, there are currently at least four different "symbols" for the warp size: `warpSize`, `WARPSIZE`, `DS_Max_Worker_Warp_Size`, and `DS_Max_Warp_Number`. This needs to be exactly //one// that is used throughout the runtime.
>> 
>> I agree about the collapsing of the first two, maybe the 3rd. The 4th is the maximum number of warps per team AFAICT not the warp size (sure, it happens to be 32).
> 
> You are right, `DS_Max_Warp_Number` means something else and only accidentally has the same value.


Repository:
  rL LLVM

https://reviews.llvm.org/D14254