[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3e649f8ef187: [openmp][nfc] Simplify macros guarding math complex headers (authored by JonChesterfield). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

fodinabor accepted this revision. fodinabor added a comment. This revision is now accepted and ready to land. LGTM as well :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 __

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a comment. @fodinabor? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

jdoerfert accepted this revision. jdoerfert added a comment. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added inline comments. Comment at: clang/lib/Headers/openmp_wrappers/complex:21 #define __OPENMP_NVPTX__ #include <__clang_cuda_complex_builtins.h> #undef __OPENMP_NVPTX__ ^ this header does not look for a macro called __CUDA__ or include any o

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

pdhaliwal added a comment. Looks ok to me. Regression tests and runtime tests went fine. Tested a simple cuda and openmp kernel with `sin` function on sm_61, didn't see any issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://r

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a comment. Cut down to only dropping the cuda define, which is sufficient to resolve D104904 . Haven't build/tested this diff yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ ht

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield updated this revision to Diff 358250. JonChesterfield added a comment. - reduce patch to only dropping cuda define v2, now with missing save Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield updated this revision to Diff 358248. JonChesterfield added a comment. - reduce patch to only dropping cuda define Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 Files: clang/lib/Header

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a comment. I think the openmp_wrappers are only used when compiling device code, which would explain why setting a macro in one of them is a proxy for detecting compilation for the device. Attempting to verify that, it looks like: trunk-nvptx includes openmp_wrappers on de

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

jdoerfert requested changes to this revision. jdoerfert added a comment. We need a macro for OPENMP and one for OPENMP_OFFLOAD, we can use a single one for the latter and avoid _NVPTX, _AMDGCN, ... but we need both as described by @fodinabor. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

fodinabor added a comment. I added a pretty simple regression that should make testing this -x cuda -fopenmp issue simpler: https://reviews.llvm.org/D105322 I guess a similar test for -x hip -fopenmp could be added, but it hasn't been an issue so far as HIP and OpenMP AMDGCN seem to use the same

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

fodinabor requested changes to this revision. fodinabor added a comment. This revision now requires changes to proceed. citing from https://reviews.llvm.org/rG7f1e6fcff9427adfa8efa3bfeeeac801da788b87: > Due to recent changes we cannot use OpenMP in CUDA files anymore (PR45533) as > the math hand

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a comment. That's interesting. I don't see how there is a semantic change here - _openmp is defined already and the builtins file ignores the cuda define - but I also haven't tried openmp+cuda in combination. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

fodinabor added a comment. Looks pretty much like a revert of https://reviews.llvm.org/D90415 which was necessary to allow building with `-x cuda -fopenmp`. Won't this break that again? I fear there's no test covering that case and I either wasn't sure where to add such a test.. (also `-x hip -

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a comment. Yeah, it probably should be. I should also check the blame list for the file to see who else should be on the reviewer list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D10522

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

pdhaliwal added a comment. Should the name of file be changed as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 ___ cfe-commits mailing list cfe-commits@

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield added a reviewer: ronlieb. JonChesterfield added a comment. this unblocks the hazard I am concerned about for D104904 , namely it stops us defining `__CUDA__` when compiling amdgcn code that includes complex.h Comment at: clan

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

JonChesterfield created this revision. JonChesterfield added reviewers: jdoerfert, tianshilei1992, pdhaliwal. Herald added subscribers: guansong, kristof.beyls, tpr, yaxunl. JonChesterfield requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: c