[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive
From:       Martin_Storsjö_via_Phabricator via cfe-commits <cfe-commits () lis
Date:       2023-08-31 21:08:31
Message-ID: Zm47HoRUQ8CNj77Q735VIg () geopod-ismtpd-10
[Download RAW message or body]

mstorsjo added inline comments.


================
Comment at: openmp/runtime/test/target/target_thread_limit.cpp:28
+// OMP51: target: parallel
+// OMP51: target: parallel
+// OMP51-NOT: target: parallel
----------------
mstorsjo wrote:
> This test fails when running (on Windows) on GitHub Actions runners - see \
> https://github.com/mstorsjo/llvm-mingw/actions/runs/6019088705/job/16342540379. 
> I believe that this bit of the test has got a hidden assumption that it is running \
> in an environment with 4 or more cores. By setting `#pragma omp target \
> thread_limit(tl)` (with `tl=4`) and running a line in parallel with `#pragma omp \
> parallel`, it expects that we'll get 4 printouts - while in practice, we'll get \
> anywhere between 1 and 4 printouts depending on the number of cores. 
> Is there something that can be done to make this test work in such an environment \
> too?
Can someone involved in this patch take on fixing it so that it works on machines \
with fewer than 4 cores? I'm not sure what's the most appropriate path forward here, \
as it breaks clearly in such configs (even if it might not be hit by one of the \
official llvm buildbots, but it shows up as breakage in my nightly builds every day \
now) - reverting seems a bit harsh. I guess I could just rip out this part of the \
test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152054

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic