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
----------------
sandeepkosuri wrote:
> mstorsjo wrote:
> > 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?
> @mstorsjo , I noticed that you have committed this 
> https://github.com/llvm/llvm-project/commit/c2019c416c8d7ec50aec6ac6b82c9aa4e99b0f6f
> 
> Does this solve your problem ?
Yes, that commit fixed the issue.


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

Reply via email to