[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl added a comment. In D151349#4426008 , @jhuber6 wrote: > I've started seeing these errors when compiling for OpenMP targeting AMDGPU: > > $ clang input.c -fopenmp --offload-arch=gfx1030 > warning: feature flag '+image-insts' is ignored since

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

jhuber6 added a comment. I've started seeing these errors when compiling for OpenMP targeting AMDGPU: $ clang input.c -fopenmp --offload-arch=gfx1030 warning: feature flag '+image-insts' is ignored since the feature is read only [-Winvalid-command-line-argument] Any suggestions for what

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc0f0d50653e1: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT` (authored by yaxunl). Herald added a project: clang. Changed prior to commit:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl updated this revision to Diff 529644. yaxunl added a comment. warn if users try to enable/disable image-insts and do not emit image-insts as function attributes in IR CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151349/new/ https://reviews.llvm.org/D151349 Files:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl added a comment. In D151349#4395312 , @arsenm wrote: > In D151349#4381569 , @yaxunl wrote: > >> In D151349#4381471 , @arsenm wrote: >> >>> In D151349#4377792

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

arsenm added a comment. In D151349#4381569 , @yaxunl wrote: > In D151349#4381471 , @arsenm wrote: > >> In D151349#4377792 , @yaxunl wrote: >> >>> using ISA version to

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl added a comment. In D151349#4381471 , @arsenm wrote: > In D151349#4377792 , @yaxunl wrote: > >> using ISA version to determine whether image is supported > > That’s backward. You can track the feature in

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

arsenm added a comment. In D151349#4377792 , @yaxunl wrote: > using ISA version to determine whether image is supported That’s backward. You can track the feature in clang separately and just not emit it. We do that for a few other features. You just

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

tra added inline comments. Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:248 + auto ISAVer = llvm::AMDGPU::getIsaVersion(Opts.CPU); + HasImage = ISAVer.Major != 9 || ISAVer.Minor != 4; } My usual nit about negations: `!(ISAVer.Major == 9 && ISAVer.Minor ==

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl updated this revision to Diff 526263. yaxunl added a comment. using ISA version to determine whether image is supported CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151349/new/ https://reviews.llvm.org/D151349 Files: clang/include/clang/Basic/TargetInfo.h

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl added inline comments. Comment at: clang/test/OpenMP/amdgcn-attributes.cpp:36 // DEFAULT: attributes #0 = { convergent noinline norecurse nounwind optnone "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" } -// CPU:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl added a comment. In D151349#4372695 , @arsenm wrote: > You seem to be defining a new subtarget feature without actually defining the > underlying feature. I thought the issue was specific missing image > instructions, which is already covered by

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

arsenm added inline comments. Comment at: clang/test/OpenMP/amdgcn-attributes.cpp:36 // DEFAULT: attributes #0 = { convergent noinline norecurse nounwind optnone "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" } -// CPU:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

arsenm added a comment. You seem to be defining a new subtarget feature without actually defining the underlying feature. I thought the issue was specific missing image instructions, which is already covered by extended-image-insts Comment at:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl updated this revision to Diff 525637. yaxunl added a comment. Herald added a subscriber: jdoerfert. Herald added a reviewer: kiranchandramohan. update tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151349/new/ https://reviews.llvm.org/D151349 Files:

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

yaxunl created this revision. yaxunl added reviewers: b-sumner, arsenm, tra. Herald added subscribers: kerbowa, hiraditya, jvesely. Herald added a project: All. yaxunl requested review of this revision. Herald added subscribers: llvm-commits, wdng. Herald added a project: LLVM. HIP texture/image