Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
Thanks, with #1455, the build succeeds if openmp < 4.5 and --{en,dis}able-omp
is not provided. I'll update the patch sent to buildroot. I'm closing this PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-738164966___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
Closed #1433. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#event-4068788837___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
Fixed here: https://github.com/rpm-software-management/rpm/pull/1455 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-738028700___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
Thank you. This is indeed a bug in the configure script. We shouldn't apply the `OPENMP_CFLAGS` macro if we just evaluated that the required version of OpenMP is not available. Let me fix that quickly. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-736674484___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
The second point is wrong. If `ENABLE_OPENMP` is not defined and `omp.h` is not included, build will still fail on: ``` pack.c: In function 'packageBinaries': pack.c:769:26: error: expected '#pragma omp' clause before 'priority' #pragma omp task untied priority(i) ^ ``` Following your first feedback on this PR, I sent a patch to disable openmp on buildroot side: https://patchwork.ozlabs.org/project/buildroot/patch/20201112184600.427081-1-fontaine.fabr...@gmail.com/ I'm still waiting the feedback of the other buildroot members but I think this is the only viable option as we can't 'know' the openmp version of every toolchain. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-736035545___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
All that being said, I wonder if making OpenMP's `priority` support itself optional (which is the reason for mandating version 4.5 in the first place) wouldn't be better after all, especially considering that this is not the first issue reported after the OpenMP version [bump](https://github.com/rpm-software-management/rpm/pull/1325#issuecomment-684517393). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-735857748___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
I can see two aspects being discussed here: 1) We don't want to error out if OpenMP is older than expected. This is what happens at the moment, though - we only error out if `--enable-openmp` is issued, but not otherwise. 2) We want to allow builds without OpenMP support. This is already supported, too - the `ENABLE_OPENMP` preprocessor macro is left undefined when either OpenMP is too old (see above), or when it is manually disabled with the `--disable-openmp` configure switch. Note that there's no need to conditionalize the individual `#pragma omp` usages - they're ignored by the compiler when the `omp.h` header is not included (which is controlled by the `ENABLE_OPENMP` macro). @ffontaine Is the above right, or am I missing something? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-735850989___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
NAK for littering the source with #ifdef's around every #pragma we may ever add. The job of a configure script is to figure out a way how to make the software run on a system and bail out if it can't, but automatically disabling OpenMP is only when it's not explicitly enabled with --enable-openmp. I just don't think fiddling with the configure logic to get it *just so* with all possible cases is worth anybody's time... It's quite clear people expect to build the latest rpm in all manner of strange configurations that we're not necessarily prepared to support. Having to manually disable something intentionally sends a certain message - openmp might be optional for now but might not always be that way. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-726053611___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
I think we want to generally be certain this is enabled or disabled, since this is a fairly significant behavioral change for `rpmbuild`. @pmatilai, do you think we should allow this to pass through with failure? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433#issuecomment-726040326___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] don't error out if OpenMP is too old (#1433)
Dont error out if OpenMP is too old to be used with rpm but instead display a warning and dont use any pragma omp Signed-off-by: Fabrice Fontaine fontaine.fabr...@gmail.com You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1433 -- Commit Summary -- * dont error out if OpenMP is too old -- File Changes -- M build/pack.c (10) M configure.ac (2) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1433.patch https://github.com/rpm-software-management/rpm/pull/1433.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1433 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
