AaronBallman wrote:

> Thanks for the review. I’ve implemented all the requested changes but still 
> have a few questions:
> 
>     * Originally, using `IVTy` to query the iteration variable’s width always 
> returned “32-bit int,” even if the loop was declared as `char` or `short`. As 
> a result, errors only appeared when the factor exceeded 32 bits, and the 
> diagnostic always said “int (32 bits).” However, when the loop variable was 
> `long`, the code compiled successfully, so larger types were recognized. I’m 
> not sure whether this was due to how I was reading the type or something else.
> 
>     * In the current implementation I switched to using `OrigVar->getType()` 
> to get the variable’s `Type`. Now `getTypeSize()` correctly returns 8 bits 
> for `char`, 16 for `short`, 64 for `long`, etc. That means code like
>       ```c++
>       #pragma omp unroll partial(0xFFFFF)
>       for (char i = 0; …) { }
>       ```
>       
>       
>           
>             
>           
>       
>             
>           
>       
>           
>         
>       now rejects, whereas it used to compile.
> 
>     * One remaining concern: the bit-width check treats all iteration 
> variables as unsigned. It doesn’t account for signed types. Should I add a 
> separate signed-range check, as suggested here in this 
> [comment](https://github.com/llvm/llvm-project/issues/139268#issuecomment-2867068205)?

I think I need @alexey-bataev or @shiltian to weigh in on what the OpenMP 
specification expects in these cases. My naive take is that we should be 
rejecting that code which previously compiled and we should be accounting for 
signed types.

https://github.com/llvm/llvm-project/pull/139986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to