[PATCH] D46472: [HIP] Support offloading by linker script

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM except for minor suggestions. Comment at: lib/CodeGen/CGCUDANV.cpp:361-373 + if (IsHIP) +FatbinConstantName = ".hip_fatbin"; + else if (RelocatableDeviceCode) /

[PATCH] D38770: AMDGPU: Use stricter bounds for workitem builtins

t-tye added inline comments. Comment at: include/clang/Basic/TargetInfo.h:1060 + /// \returns Maximum device supported OpenCL workgroup size. + virtual unsigned getOpenCLMaxWorkGroupSize(unsigned Dim) const { +return 0; Is this specifically tied to OpenCL o

[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM. Other clean up can be done as a separate patch. Comment at: test/CodeGenOpenCL/address-spaces.cl:37 +// SPIR: i32 addrspace(2)* %arg +// GIZ: i32 addrspace(4)* %arg void

[PATCH] D36802: AMDGPU: Cleanup most of the macros

t-tye requested changes to this revision. t-tye added inline comments. This revision now requires changes to proceed. Comment at: lib/Basic/Targets/AMDGPU.cpp:362 +Builder.defineMacro(Twine("__") + Twine(GPUName)); +Builder.defineMacro(Twine("__") + Twine(GPUName) + Twine

[PATCH] D43094: AMDGPU: Enable PIC by default for amdgcn

t-tye accepted this revision. t-tye added a comment. For now seems reasonable to fix amdgpu as PIC. If/when other clients of amdgpu have tool chains defined then can switch to controling in the toolchain isPICDefault() function. https://reviews.llvm.org/D43094 __

[PATCH] D36802: AMDGPU: Cleanup most of the macros

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D36802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D43340: Clean up AMDGCN tests

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Plus I think there was a test with giz in its name. Should that be renamed? https://reviews.llvm.org/D43340 ___ cfe-commits mailing list cfe-c

[PATCH] D43911: [AMDGPU] Clean up old address space mapping and fix constant address space value

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM except minor comment. Comment at: lib/Basic/Targets/AMDGPU.cpp:41-49 0, // Default 1, // opencl_global 3, // opencl_local 4, // opencl_constant 5,

[PATCH] D53223: AMDGPU: Add sram-ecc feature options

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D53223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D53153: [OpenCL] Mark namespace scope variables and kernel functions with default visibility

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Summary needs updating as now only being done for kernels and not namespace scope variables. https://reviews.llvm.org/D53153 ___ cfe-commits

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

t-tye added a comment. In https://reviews.llvm.org/D53153#1288127, @rjmccall wrote: > In https://reviews.llvm.org/D53153#1288112, @rjmccall wrote: > > > But do you want to support *dynamically* linking object files? Because > > that's what visibility is about. > > > To be specific, if you don't

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Do we know the state of split DWARF and DWARF compression for DWARF 5 (compared to DWARF 2)? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59008/new/ http

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

t-tye added a comment. In D59008#1442014 , @dblaikie wrote: > In D59008#1441903 , @t-tye wrote: > > > LGTM > > > > Do we know the state of split DWARF and DWARF compression for DWARF 5 > > (compared to DWARF 2)? >

[PATCH] D52891: [AMDGPU] Add -fvisibility-amdgpu-non-kernel-functions

t-tye added a comment. Another word commonly used across languages is "offload". https://reviews.llvm.org/D52891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58518: [HIP] change kernel stub name

t-tye added a comment. Yes this relates to supporting the debugger. For the same function being present on both host and device, having the same name is correct as the debugger must set a breakpoint at both places. This is similar to needing to set a breakpoint at every place a function is inli

[PATCH] D58518: [HIP] change kernel stub name

t-tye added a comment. To clarify, I am saying that the stub does have a different name since it is conceptually part of the implementation of doing the call to the device function implementation, and is not in fact the the device function being called itself. However, when we generate code for

[PATCH] D68578: [HIP] Fix device stub name

t-tye added a comment. In D68578#1697898 , @tra wrote: > In D68578#1697851 , @yaxunl wrote: > > > In D68578#1697822 , @tra wrote: > > > > > Could you elaborate on how exactly

[PATCH] D68578: [HIP] Fix device stub name

t-tye added a comment. In D68578#1700652 , @tra wrote: > In D68578#1698864 , @t-tye wrote: > > > From a source language point of view, the device function comprises the > > code that is launched as a grid. We need

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

t-tye created this revision. t-tye added a reviewer: scott.linder. Herald added subscribers: libcxx-commits, mgorny. Herald added a reviewer: bollu. Herald added a reviewer: MaskRay. Herald added a reviewer: sscalpone. Herald added a project: libunwind. Herald added a reviewer: libunwind. t-tye req

[PATCH] D106734: Eliminate clang man page generation warning for missing .rst files

t-tye created this revision. t-tye added reviewers: kzhuravl, scott.linder. Herald added a subscriber: mgorny. t-tye requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Update `clang/docs/CMakeLists.txt` for Sphinx man builder

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

t-tye updated this revision to Diff 361402. t-tye added a comment. Split change for clang makefile to elimnate Sphinx warnings of missing .rst fies when building man pages into D106734 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

t-tye updated this revision to Diff 361407. t-tye added a comment. Factor out documentation and CMake file changes unrelated to adding DOCX support to D106736 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106339/

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

t-tye added a comment. In D106339#2890258 , @ldionne wrote: > What's the benefit of having docx documentation? We generate HTML > documentation, which ends up in the website, and that seems strictly superior > to generating docx. What do you need it for

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

t-tye added inline comments. Comment at: include/clang/Basic/SyncScope.h:23 +enum class SyncScope { + OpenCLWorkItem = 0, + OpenCLWorkGroup = 1, The OpenCL workitem scope is only used for image fences and does not apply to atomic operations so not sure that it

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

t-tye added inline comments. Comment at: docs/LanguageExtensions.rst:1935 +builtin function, and are named with a ``__opencl_`` prefix.) Low-level ARM exclusive memory builtins Should it also say: ``` The macros ``__OPENCL_MEMORY_SCOPE_WORK_ITEM``, ``__OPENC

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

t-tye accepted this revision. t-tye added a comment. LGTM https://reviews.llvm.org/D28691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye added inline comments. Comment at: include/clang/Basic/SyncScope.h:46 + Scopes.push_back(SyncScope::OpenCLSubGroup); + return Scopes; +} Should there be an assert/static_assert in case SyncScope enum grows due to other languages? Comme

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM other than suggested documentation and static_assert/unreachable comments. Comment at: lib/CodeGen/CGAtomic.cpp:696 +if (S != Default) + SI->addCase(Builder.getIn

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye added inline comments. Comment at: include/clang/Basic/SyncScope.h:56 /// enums in opencl-c.h. -enum class SyncScope { - OpenCLWorkGroup = 1, - OpenCLDevice = 2, - OpenCLAllSVMDevices = 3, - OpenCLSubGroup = 4, +enum class OpenCLMemoryScope { + WorkGroup = 1,

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye added inline comments. Comment at: lib/CodeGen/CGAtomic.cpp:678 + auto &Builder = CGF.Builder; + auto Scopes = getAllLanguageSyncScopes(); + llvm::DenseMap BB; yaxunl wrote: > t-tye wrote: > > Should only the scopes that apply to the language be returned

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye accepted this revision. t-tye added a comment. LGTM https://reviews.llvm.org/D36580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

t-tye added inline comments. Comment at: cfe/trunk/lib/Frontend/InitPreprocessor.cpp:581 // Define macros for the OpenCL memory scope. // The values should match clang SyncScope enum. + static_assert( // The values should match clang AtomicScopeOpenCLModel

[PATCH] D36802: AMDGPU: Insert __devicename__ macros

t-tye added inline comments. Comment at: lib/Basic/Targets/AMDGPU.cpp:364-367 + if (GPUName.empty()) +return; + + Builder.defineMacro(Twine("__") + Twine(GPUName) + Twine("__")); Should this be the following since extra macros could be after it in the futur

[PATCH] D36771: AMDGPU: add missing amdgcn processors and tests

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D36771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D87858: [hip] Add HIP scope atomic ops.

t-tye added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:792-799 +ATOMIC_BUILTIN(__hip_atomic_compare_exchange_strong, "v.", "t") +ATOMIC_BUILTIN(__hip_atomic_exchange, "v.", "t") +ATOMIC_BUILTIN(__hip_atomic_fetch_add, "v.", "t") +ATOMIC_BUILTIN(__hip_atom

[PATCH] D87858: [hip] Add HIP scope atomic ops.

t-tye added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:792-799 +ATOMIC_BUILTIN(__hip_atomic_compare_exchange_strong, "v.", "t") +ATOMIC_BUILTIN(__hip_atomic_exchange, "v.", "t") +ATOMIC_BUILTIN(__hip_atomic_fetch_add, "v.", "t") +ATOMIC_BUILTIN(__hip_atom

[PATCH] D93017: [AMDGPU] Add missing targets to amdgpu-features.cl

t-tye created this revision. t-tye added a reviewer: kzhuravl. Herald added subscribers: kerbowa, tpr, dstuttard, yaxunl, nhaehnle, jvesely. t-tye requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Repository: rG LLVM Github Monorepo h

[PATCH] D93018: [AMDGPU] Add missing targets to target-invalid-cpu-note.c

t-tye created this revision. t-tye added a reviewer: kzhuravl. Herald added subscribers: tpr, dstuttard, yaxunl. t-tye requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D930

[PATCH] D93017: [AMDGPU] Add missing targets to amdgpu-features.cl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG92ab6ed6672b: [AMDGPU] Add missing targets to amdgpu-features.cl (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D93018: [AMDGPU] Add missing targets to target-invalid-cpu-note.c

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7beee561e23d: [AMDGPU] Add missing targets to target-invalid-cpu-note.c (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D93181: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

t-tye created this revision. t-tye added reviewers: kzhuravl, ronlieb. Herald added subscribers: dexonsmith, tpr, dstuttard, yaxunl. t-tye requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://r

[PATCH] D93181: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5ad202ce8963: [NFC][AMDGPU] Reformat AMD GPU targets in cuda.cpp (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D93258: [amdgpu] Default to code object v3

t-tye added inline comments. Comment at: llvm/docs/AMDGPUUsage.rst:2874-2876 .. warning:: Code object V3 is not the default code object version emitted by this version of LLVM. Move this to the "Code Object V4 Metadata" section. Repository: rG LLVM Git

[PATCH] D93258: [amdgpu] Default to code object v3

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93258/new/ https://reviews.llvm.org/D93258 ___ cfe

[PATCH] D93398: [NFC] Use regex for code object version in hip tests

t-tye added inline comments. Comment at: clang/test/Driver/hip-code-object-version.hip:56 -// V4: "-mllvm" "--amdhsa-code-object-version=4" -// V4: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906" +// VD: "-mllvm" "--amdhsa-code-object-version=4" +// VD: "-tar

[PATCH] D93398: [NFC] Use regex for code object version in hip tests

t-tye added inline comments. Comment at: clang/test/Driver/hip-code-object-version.hip:56 -// V4: "-mllvm" "--amdhsa-code-object-version=4" -// V4: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa--gfx906" +// VD: "-mllvm" "--amdhsa-code-object-version=4" +// VD: "-tar

[PATCH] D93398: [NFC] Use regex for code object version in hip tests

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93398/new/ https://reviews.llvm.org/D93398 ___ cfe

[PATCH] D93648: Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93648/new/ https://reviews.llvm.org/D93648 ___ cfe

[PATCH] D89484: [AMDGPU][HIP] Switch default DWARF version to 5

t-tye accepted this revision. t-tye added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89484/new/ https://reviews.llvm.org/D89484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D92115: AMDGPU - Add diagnostic for compiling modules with AMD HSA OS type and GFX 6 arch

t-tye requested changes to this revision. t-tye added inline comments. This revision now requires changes to proceed. Comment at: llvm/docs/AMDGPUUsage.rst:2109-2112 +Caution: + AMD HSA Os is not supported in Southern Islands (GFX6) ASICs. + For example: This i

[PATCH] D92115: AMDGPU - Add diagnostic for compiling modules with AMD HSA OS type and GFX 6 arch

t-tye added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:134 + if (isAmdHsaOS() && getGeneration() == AMDGPUSubtarget::SOUTHERN_ISLANDS) { +report_fatal_error("GFX6 (SI) ASICs does not support AMD HSA OS type \n", + false); --

[PATCH] D92441: Add CLangOffloadBundler documentation to Clang index

t-tye created this revision. t-tye added reviewers: kzhuravl, scott.linder, b-sumner, tpr, rampitec, yaxunl, kerbowa. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. t-tye requested review of this revision. Change-Id: I1a35bea10861cb5219e0dd13e14a86df56b38825 Rep

[PATCH] D92441: Add CLangOffloadBundler documentation to Clang index

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa417cb086209: [NFC] Add CLangOffloadBundler documentation to Clang index (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D89487: [AMDGPU] gfx1032 target

t-tye added inline comments. Comment at: llvm/docs/AMDGPUUsage.rst:280 names. + ``gfx1032`` ``amdgcn`` dGPU - xnack *TBA* +

[PATCH] D89636: [AMDGPU] Extend hip-toolchin-features.hip test

t-tye created this revision. t-tye added reviewers: kzhuravl, scott.linder, rampitec. Herald added subscribers: cfe-commits, tpr, dstuttard, yaxunl. Herald added a project: clang. t-tye requested review of this revision. Herald added a subscriber: wdng. - Extend hip-toolchin-features.hip to also c

[PATCH] D89636: [AMDGPU] Extend hip-toolchin-features.hip test

This revision was automatically updated to reflect the committed changes. Closed by commit rG89d71970cb82: [AMDGPU] Extend hip-toolchin-features.hip test (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89636/new/ https://reviews.l

[PATCH] D90212: [AMDGPU] Add missing support for targets

t-tye created this revision. t-tye added reviewers: kzhuravl, scott.linder, tpr. Herald added subscribers: llvm-commits, cfe-commits, kerbowa, rupprecht, hiraditya, dstuttard, yaxunl, nhaehnle, jvesely, emaste. Herald added a reviewer: espindola. Herald added a reviewer: jhenderson. Herald added p

[PATCH] D90212: [AMDGPU] Add missing support for targets

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG598409782389: [AMDGPU] Add missing support for targets (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D90364: [AMDGPU] Update AMD GPU documentation

t-tye created this revision. t-tye added reviewers: kzhuravl, scott.linder. Herald added subscribers: llvm-commits, cfe-commits, dang, kerbowa, s.egerton, simoncook, tpr, dstuttard, yaxunl, nhaehnle, jvesely. Herald added projects: clang, LLVM. t-tye requested review of this revision. Herald added

[PATCH] D90364: [AMDGPU] Update AMD GPU documentation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG661797bd7633: [AMDGPU] Update AMD GPU documentation (authored by t-tye). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D90447: [AMDGPU] Add gfx1033 target

t-tye added inline comments. Comment at: llvm/docs/AMDGPUUsage.rst:317 + names. + ``gfx1033`` ``amdgcn`` dGPU - wavefrontsize64 *TBA* +

[PATCH] D90419: [AMDGPU] Add gfx90c target

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90419/new/ https://reviews.llvm.org/D90419 ___ cfe

[PATCH] D90447: [AMDGPU] Add gfx1033 target

t-tye requested changes to this revision. t-tye added inline comments. This revision now requires changes to proceed. Comment at: llvm/docs/AMDGPUUsage.rst:317 + names. + ``gfx1033``

[PATCH] D90447: [AMDGPU] Add gfx1033 target

t-tye added inline comments. Comment at: llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test:61-64 +# RUN: yaml2obj %s -o %t -DCPU=GFX90C +# RUN: llvm-readobj -h %t | FileCheck %s --match-full-lines -DFILE=%t -DCPU=GFX90C -DFLAGS=0x32 + # RUN: yaml2obj %s -o %t -DCPU=GFX10

[PATCH] D90447: [AMDGPU] Add gfx1033 target

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM provided the other files are updated as other reviews have mentioned. Comment at: llvm/docs/AMDGPUUsage.rst:317 +

[PATCH] D107190: [AMDGPU][HIP] Switch default DWARF version to 5

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107190/new/ https://reviews.llvm.org/D107190 ___ c

[PATCH] D70424: clang/AMDGPU: Fix default for frame-pointer attribute

t-tye added a comment. @scott.linder can answer about the -g question, but I would expect that the CFI is capable of describing the address of the CFA regardless of whether there is a frame pointer by simply knowing the constant offset from the stack pointer. For AMDGPU it seems to me what we r

[PATCH] D73651: [OpenCL][CUDA][HIP][SYCL] Add norecurse

t-tye added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:918 + // + // ToDo: clang does not support CUDA/HIP dynamic parallelism, therefore + // CUDA/HIP kernel can be marked with norecurse. This may change in the tra wrote: > I believe d

[PATCH] D39878: AMDGPU: Add -mxnack/-mno-xnack options that set +/-xnack feature

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D39877: AMDGPU/NFC: Move getAMDGPUTargetFeatures to AMDGPU toolchain

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D40045: AMDGPU/GCN: Bring processors in sync with AMDGPUUsage

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM except for Bonaire. Comment at: lib/Basic/Targets/AMDGPU.cpp:271 .Case("gfx700", GK_GFX7) .Case("bonaire", GK_GFX7) .Case("kaveri", GK_GFX7) -

[PATCH] D31210: [AMDGPU] Add new address space mapping

t-tye requested changes to this revision. t-tye added a comment. This revision now requires changes to proceed. Also please upload as full diff. Comment at: lib/Basic/Targets.cpp:2026-2069 + struct AddrSpace { +unsigned Generic, Global, Local, Constant, Private; +bool

[PATCH] D31210: [AMDGPU] Switch address space mapping by triple environment

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. Just a couple of suggestions, otherwise: LGTM Comment at: lib/Basic/Targets.cpp:2015 -static const unsigned AMDGPUAddrSpaceMap[] = { - 1,// opencl_global - 3,// op

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: lib/AST/ASTContext.cpp:9547-9555 +unsigned ASTContext::getTargetAddressSpace(unsigned AS) const { + // For OpenCL, the address space qualifier is 0 in AST. + if (AS == 0 && LangOpts.OpenCL) +return getTargetInfo().getDataLayout().get

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: lib/AST/ASTContext.cpp:9547-9555 +unsigned ASTContext::getTargetAddressSpace(unsigned AS) const { + // For OpenCL, the address space qualifier is 0 in AST. + if (AS == 0 && LangOpts.OpenCL) +return getTargetInfo().getDataLayout().get

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: lib/AST/ASTContext.cpp:9547-9555 +unsigned ASTContext::getTargetAddressSpace(unsigned AS) const { + // For OpenCL, the address space qualifier is 0 in AST. + if (AS == 0 && LangOpts.OpenCL) +return getTargetInfo().getDataLayout().get

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: include/clang/AST/ASTContext.h:2319 return AddrSpaceMapMangling || - AS < LangAS::Offset || - AS >= LangAS::Offset + LangAS::Count; + AS > LangAS::target_first; } Should this be >= s

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: include/clang/AST/Type.h:339-340 +auto Addr = getAddressSpace(); +if (Addr == 0) + return 0; +return Addr - LangAS::target_first; Since you mention this is only used for `__attribute__((address_space(n)))

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:2055 + << AllocType.getUnqualifiedType() + << AllocType.getQualifiers().getAddressSpacePrintValue(); else if (getLangOpts().ObjCAutoRefCount) { Would suggest renaming getAddressSpacePr

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye added inline comments. Comment at: include/clang/AST/Type.h:339-340 +auto Addr = getAddressSpace(); +if (Addr == 0) + return 0; +return Addr - LangAS::target_first; Anastasia wrote: > yaxunl wrote: > > t-tye wrote: > > > Since you mention t

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D31404 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D31771: [AMDGPU] Temporarily change constant address space from 4 to 2 for the new address space mapping

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. Other than one comment: LGTM Comment at: lib/Basic/Targets.cpp:2083 Local = 3; -Constant = 4; +Constant = 2; Private = 5; ---

[PATCH] D31771: [AMDGPU] Temporarily change constant address space from 4 to 2 for the new address space mapping

t-tye added a comment. LGTM Comment at: lib/Basic/Targets.cpp:2083 Local = 3; -Constant = 4; +Constant = 2; Private = 5; t-tye wrote: > Since Constant is now the same regardless of the GIZ setting, should it be > moved

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

t-tye added inline comments. Comment at: include/clang/Basic/Builtins.def:717 +ATOMIC_BUILTIN(__opencl_atomic_fetch_max, "v.", "t") + #undef ATOMIC_BUILTIN Will the OpenCL 2.0 memory fences also be supported which also have a memory order and memory scope? ==

[PATCH] D42800: Let CUDA toolchain support amdgpu target

t-tye added inline comments. Comment at: include/clang/Basic/Cuda.h:49-57 + GFX700, + GFX701, + GFX800, + GFX801, + GFX802, + GFX803, + GFX810, Should complete list of processors for the amdgcn architecture be included? See https://llvm.org/docs/AMDGPUUsa

[PATCH] D32248: CodeGen: Cast alloca to expected address space

t-tye added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1105-1119 + // Alloca always returns a pointer in alloca address space, which may + // be different from the type defined by the language. For example, + // in C++ the auto variables are in the default address spa

[PATCH] D32248: CodeGen: Cast alloca to expected address space

t-tye added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1115 + if (AddrTy->getAddressSpace() != ExpectedAddrSpace && + Ty.getAddressSpace() != LangAS::opencl_constant) { +address = Address(Builder.CreateAddrSpaceCast(Addr, Anastasia wrote: > Do

[PATCH] D33109: Enhance synchscope representation (clang)

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D32248: CodeGen: Cast alloca to expected address space

t-tye added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1115 + assert(T.getAddressSpace() == LangAS::Default || + T.getQualifiers().hasTargetSpecificAddressSpace()); + auto Addr = getTargetHooks().performAddrSpaceCast(*this, Should allowing spec

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1115-1124 + // Currently defaults to 3 in AMDGPUBaseInfo.cpp + // Using that default lets clang emit IR for amdgcn when llvm has been built + // without that target, provided the user wants this cod

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added a comment. I have no opinion, just making an observation and defer to @kzhuravl . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98746/new/ https://reviews.llvm.org/D98746 ___ cfe-commits mail

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

t-tye added a comment. I vaguely remember that clang needed to know what code object it was going to request as it used that to either validate other options, or change the format of other passed cc1 options. If that is true, then I am not sure the defaulting approach works as clang will not kn

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

t-tye added inline comments. Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:9 + +find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm) +if (NOT ${hsa-runtime64_FOUND}) JonChesterfield wrote: > JonChesterfield wrote: > > gregro

[PATCH] D94338: [Clang][Docs] Fix ambiguity in clang-offload-bundler docs

t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94338/new/ https://reviews.llvm.org/D94338 ___ cfe

[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

t-tye added a comment. In D93525#2493024 , @yaxunl wrote: > can you document this in ClangOffloadBundler.rst ? I think we need a clear > description about how clang-offload-bundler knows which file in the .a file > belongs to which target. How does the

[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

t-tye added a comment. In D93525#2495374 , @saiislam wrote: > In D93525#2493752 , @t-tye wrote: > >> In D93525#2493024 , @yaxunl wrote: >> >>> can you document this in ClangO

[PATCH] D93525: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives

t-tye requested changes to this revision. t-tye added a comment. This revision now requires changes to proceed. In D93525#2509796 , @jdoerfert wrote: >> At the moment this patch defines compatibility as exact string match of >> bundler entry ID. >> [...]