[Openmp-commits] [PATCH] D74092: Changed omp_get_max_threads() implementation to more closely match spec description.

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

The reasoning looks OK to me and I like the associated test. I don't understand the behaviour change in max_threads though, it looks like:

  #pragma omp target teams map(MaxThreadsL1, MaxThreadsL2) thread_limit(32)      \
      num_teams(1)
    {
      MaxThreadsL1 = omp_get_max_threads();
  // ...
    }
  
    // CHECK: Non-SPMD MaxThreadsL1 = 32                                                                                                                                                                             
    printf("Non-SPMD MaxThreadsL1 = %d\n", MaxThreadsL1);

Which I think is the developer asking for a maximum number of threads of 32 - do you mean both instances of 32 become 64, or just the second one? Agreed that it should be part of this patch, otherwise we'd break the build by committing.

Further, the get_max_threads result might be architecture dependent. Do nvptx and amdgcn return the same value in equivalent contexts? We might need an #ifdef in the test, or a separate instance of the test under amdgcn if they do behave differently here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74092/new/

https://reviews.llvm.org/D74092