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

scott.linder added a comment. In D53153#1317977 , @rjmccall wrote: > I think `-fvisibility=hidden` isn't good enough because you want to infer > hidden visibility even for external symbol references, and that's just not > how global visibility works. Bu

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder updated this revision to Diff 220369. scott.linder edited the summary of this revision. scott.linder added reviewers: ddunbar, aprantl, cfang, dblaikie. scott.linder added a comment. Rebase and ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.ll

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder updated this revision to Diff 220396. scott.linder added a comment. Actually rebase this time CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.llvm.org/D53768 Files: include/clang/Frontend/CompilerInstance.h lib/Frontend/CompilerInstance.cpp u

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder marked an inline comment as done. scott.linder added inline comments. Comment at: include/clang/Frontend/CompilerInstance.h:362-363 + /// If not set, this stream defaults to \c llvm::errs(). + void setVerboseOutputStream(raw_ostream &Value, +

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder updated this revision to Diff 221075. scott.linder added a comment. After reading again I think I understand the ask now. Is this closer to what you had in mind? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.llvm.org/D53768 Files: include/clan

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder updated this revision to Diff 221089. scott.linder added a comment. Same patch, this time with a working test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.llvm.org/D53768 Files: include/clang/Frontend/CompilerInstance.h lib/Frontend/Compile

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.llvm.org/D53768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53768/new/ https://reviews.llvm.org/D53768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

This revision was automatically updated to reflect the committed changes. Closed by commit rG87cb734c04be: [Clang] Add VerboseOutputStream to CompilerInstance (authored by scott.linder). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D53768?vs=221089&id=225934#

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

scott.linder added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:1945 + if (NOT touch_head_result EQUAL 0) +return() + endif() This seems to implement the behavior that when the log is not present and is not writable

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: llvm/cmake/modules/AddLLVM.cmake:1916 endif() - if(EXISTS "${path}/.svn") -set(svn_files Nit: can this be in a

[PATCH] D77923: OpenCL: Fix some missing predefined macros

scott.linder added a comment. https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/preprocessorDirectives.html describes `__OPENCL_VERSION__` as "an integer reflecting the version number of the OpenCL supported by the OpenCL device." as contrasted with `__OPENCL_C_VERSION__` which is

[PATCH] D77923: OpenCL: Fix some missing predefined macros

scott.linder added a comment. In D77923#1978261 , @arsenm wrote: > In D77923#1978172 , @scott.linder > wrote: > > > https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/preprocessorDirectives.html > > de

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

scott.linder added a comment. Can you provide more context in the diff? Looking at the current `master` locally it seems like this patch changes the behavior of the `llvm_vcsrevision_h` target when `logs/HEAD` isn't found, such that it only depends on the generation script. I.e. if you configur

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

scott.linder added a comment. Thank you for the explanation; I'm also still lost as to why `.git/logs/HEAD` is referenced at all then. I would propose removing `find_first_existing_vc_file` entirely, as it seems to be completely redundant if there is another mechanism for ensuring the VCS heade

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

scott.linder added a comment. I'd be interested in the answer concerning why we need to avoid `git rev-parse HEAD`; it seems like the cleanest solution is to just always check if `git rev-parse HEAD` changes to determine whether to regenerate the header. If that is not feasible for some reason,

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added a comment. I will let others comment, but I think this is a perfectly reasonable alternative, and I much prefer using indentation over the delimiter-remark approach. LGTM, assuming nobody else objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

scott.linder updated this revision to Diff 418135. scott.linder retitled this revision from "[AMDGPU] Only warn when mixing printf and hostcall" to "[AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5". scott.linder edited the summary of this revision. scott.linder added a co

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

scott.linder added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:9381 + M.getTarget().getTargetOpts().CodeObjectVersion != 500) { +F->addFnAttr("amdgpu-no-hostcall-ptr"); + } arsenm wrote: > sameerds wrote: > > The frontend does not need

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

scott.linder updated this revision to Diff 418635. scott.linder added a comment. Do not add the no-hostcall attribute for "COV_None" modules, to allow for the OpenCL implementation of hostcall in the device-libs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

scott.linder added a comment. @yaxunl Does excluding device-libs via COV_None make sense? @sameerds Would you still rather we just not add this attribute in the frontend at all? I'm OK with it if that is the consensus Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D122669: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

scott.linder created this revision. Herald added subscribers: hsmhsm, foad, Naghasan, ldrumm, kerbowa, hiraditya, t-tye, Anastasia, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl, arsenm. Herald added a project: All. scott.linder requested review of this revision. Herald added subscribers: ll

[PATCH] D121951: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

scott.linder updated this revision to Diff 418922. scott.linder retitled this revision from "[AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5" to "[AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic". scott.linder edited the summary of this revision. scott.linder adde

[PATCH] D121951: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

scott.linder added a comment. In D121951#3414271 , @sameerds wrote: > In D121951#3411856 , @scott.linder > wrote: > >> @yaxunl Does excluding device-libs via COV_None make sense? >> >> @sameerds Would you still r

[PATCH] D121951: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG09f33a430b72: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic (authored by scott.linder). Changed prior to commit: https://reviews.llvm.o

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

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. LGTM, I see no reason why this shouldn't apply for all generators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106734/new/ https:/

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

scott.linder created this revision. Herald added subscribers: foad, kerbowa, hiraditya, t-tye, Anastasia, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl, arsenm. Herald added a project: All. scott.linder requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, w

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

scott.linder added reviewers: sameerds, arsenm, rampitec, b-sumner, kzhuravl, yaxunl. scott.linder added a comment. For reference, the error I'm changing to a warning was added as part of https://reviews.llvm.org/D70038, and the issue that made me consider this approach appears when building OC

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

scott.linder added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:567 - if (auto HostcallFunction = M.getFunction("__ockl_hostcall_internal")) { + if (auto *HostcallFunction = M.getFunction("__ockl_hostcall_internal")) { for (auto &U :

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

scott.linder added a comment. In D121951#3391472 , @sameerds wrote: > The check for "__ockl_hostcall_internal" is not longer relevant with the new > hostcall attribute. Can we simply remove this check? What is the situation > where the warning is still

[PATCH] D121951: [AMDGPU] Only warn when mixing printf and hostcall

scott.linder added a comment. In D121951#3393928 , @sameerds wrote: > In D121951#3392470 , @scott.linder > wrote: > >> In D121951#3391472 , @sameerds >> wrote: >> >>> Th

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1178-1266 + +void AMDGPUAsmPrinter::emitResourceUsageRemarks( +const MachineFunction &MF, const SIProgramInfo &CurrentProgramInfo, +bool isModuleEntryFunction, bool hasMAIInsts) {

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added a comment. Even with newlines forced via extra remarks, I'm not a big fan of the "---" remark; it doesn't interact well with other random remarks in the output, for example when I enable all remarks using the pattern '.*' I see: remark: foo.cl:27:0: AMD

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:1207 + auto EmitResourceUsageRemark = [&](StringRef RemarkName, + StringRef RemarkLabel, auto &&Argument) { +// Add an indent for every line beside

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added a comment. In D123878#3506500 , @afanfa wrote: > If possible, I would like to keep some kind of delimiter. I like the idea of > having it at the beginning and at the end of the section. The best option > would be to convince clang to

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

scott.linder added a comment. In D123878#3507378 , @vangthao wrote: > I am not sure if allowing clang to accept newlines is a good idea. It seems > like clang wants to know what type of message is being outputted. For example > whether this is a remark,

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

scott.linder created this revision. Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. scott.linder requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. This reverts commit c4d10e7e9bb47b77fad43d

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGffba47df7646: Revert "[AMDGPU][HIP] Switch default DWARF version to 5" (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93648/new/

[PATCH] D93721: [NFC][AMDGPU] Do not override GetDefaultDwarfVersion

scott.linder created this revision. Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. scott.linder requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Use the implementation inhereted from `Too

[PATCH] D93721: [NFC][AMDGPU] Do not override GetDefaultDwarfVersion

scott.linder added reviewers: t-tye, laurentm0, yaxunl. scott.linder added a comment. No other target overrides `GetDefaultDwarfVersion` with the same implementation, and in the future when we make changes we can add them at the points in the hierarchy where they make sense semantically. Repos

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

scott.linder created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. scott.linder requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D88754 Files: clang/test/CodeGen/debug-info-block-expr.c Index: clang/test

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

scott.linder updated this revision to Diff 296558. scott.linder added a comment. Don't hardcode x86_64-specific offsets Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88754/new/ https://reviews.llvm.org/D88754 Files: clang/test/CodeGen/debug-info

[PATCH] D88976: [clang] Use correct address space for global variable debug info

scott.linder created this revision. Herald added subscribers: cfe-commits, jholewinski. Herald added a project: clang. scott.linder requested review of this revision. The target needs to be queried here, but previously we seemed to only duplicate CUDA's (and so HIP's) behavior, and only partially.

[PATCH] D88976: [clang] Use correct address space for global variable debug info

scott.linder added a comment. I'm not certain I fully understand NVPTX's relationship with its debugger, but from https://reviews.llvm.org/D57162 I gather the "default" address space in the debugger is global, and so the frontend omits it rather than explicitly mentioning it. I think it would b

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

scott.linder created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. scott.linder requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. A dbg.declare for a local/parameter describes the hardware location of

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

scott.linder added a comment. I need to add more tests, but I wanted to float the idea of the change and get feedback first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88978/new/ https://reviews.llvm.org/D88978

[PATCH] D88754: [clang] Add a test for CGDebugInfo treatment of blocks

This revision was automatically updated to reflect the committed changes. Closed by commit rG40cef5a00eb8: [clang] Add a test for CGDebugInfo treatment of blocks (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88754/new/ ht

[PATCH] D88976: [clang] Use correct address space for global variable debug info

scott.linder updated this revision to Diff 297309. scott.linder added a comment. Replace uses of CHECK-DAG, use more meaningful names in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88976/new/ https://reviews.llvm.org/D88976 Files: clang/l

[PATCH] D88976: [clang] Use correct address space for global variable debug info

scott.linder added inline comments. Comment at: clang/test/CodeGenHIP/debug-info-address-class.hip:8 + +// CHECK-DAG: ![[FILEVAR0:[0-9]+]] = distinct !DIGlobalVariable(name: "FileVar0", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}, isLocal: false,

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

scott.linder added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1579 if (EmitDebugInfo && HaveInsertPoint()) { -Address DebugAddr = address; +Address DebugAddr = AllocaAddr.isValid() ? AllocaAddr : address; bool UsePointerValue = NRVO && ReturnValuePo

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

scott.linder added a comment. @ABataev Sorry if I'm pulling you in without enough context/work on my end, but I wanted to ask how the Clang codegen for OpenMP locals works at a high level? Is the idea that instead of an `alloc` the frontend can insert calls into the runtime in some cases, like

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

scott.linder added a comment. In D88978#2325991 , @ABataev wrote: > In D88978#2325982 , @scott.linder > wrote: > >> @ABataev Sorry if I'm pulling you in without enough context/work on my end, >> but I wanted to as

[PATCH] D88976: [clang] Use correct address space for global variable debug info

scott.linder added inline comments. Comment at: clang/lib/Basic/Targets/NVPTX.h:47 -1, // Default, opencl_private or opencl_generic - not defined -5, // opencl_global +-1, // opencl_global -1, Does anyone have any thoughts on this change specif

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

scott.linder created this revision. Herald added subscribers: cfe-commits, kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. Herald added a project: clang. scott.linder requested review of this revision. Herald added a subscriber: wdng. Another attempt at this, see D59008

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

scott.linder added inline comments. Comment at: clang/test/Driver/debug-options.c:364-366 +// GEMBED_2: warning: debug information option '-gembed-source' is not supported for target // NOGEMBED_5-NOT: "-gembed-source" +// NOGEMBED_2-NOT: warning: debug information option '-

[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

scott.linder added inline comments. Comment at: clang/test/Driver/debug-options.c:364-366 +// GEMBED_2: warning: debug information option '-gembed-source' is not supported for target // NOGEMBED_5-NOT: "-gembed-source" +// NOGEMBED_2-NOT: warning: debug information option '-

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4d10e7e9bb4: [AMDGPU][HIP] Switch default DWARF version to 5 (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D90419: [AMDGPU] Add gfx90c target

scott.linder added inline comments. Comment at: llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml:161 # RUN: yaml2obj --docnum=41 %s > %t.o.41 # RUN: llvm-readobj -s -file-headers %t.o.41 | FileCheck --check-prefixes=ELF-ALL,ELF-GFX1031 %s Heads up, I just

[PATCH] D100671: [ADT] Factor out in_place_t and expose in Optional ctor

scott.linder updated this revision to Diff 345475. scott.linder added a comment. Add constexpr convenience variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100671/new/ https://reviews.llvm.org/D100671 Files: clang/include/clang/Basic/Dire

[PATCH] D100671: [ADT] Factor out in_place_t and expose in Optional ctor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaf5247c9347d: [ADT] Factor out in_place_t and expose in Optional ctor (authored by scott.linder). Changed prior to commit: https://reviews.llvm.or

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

scott.linder added a comment. Is it possible to use soft links instead of copies? It would still be good to clean up afterwards, but if the host won't allow that cleanup in some cases at least we aren't leaving a big file around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

scott.linder created this revision. Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. scott.linder requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Another attempt at changing this default,

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG635c5ba45bae: [AMDGPU][HIP] Switch default DWARF version to 5 (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107190/new/ https://

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

scott.linder accepted this revision. scott.linder added a comment. LGTM, thank you! I have some pending changes which would add C++20 "compat" types, but removing it until those land sounds fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder created this revision. Herald added subscribers: kosarev, inglorion, tpr, dstuttard, yaxunl, kzhuravl. Herald added a project: All. scott.linder requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, wdng. Herald added a project: clang. For AMDGCN default t

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder added reviewers: arsenm, yaxunl, scchan, pcc, MaskRay, mehdi_amini, respindola, ruiu, aheejin, sbc100. scott.linder added a comment. (Just adding everyone from the parent review, feel free to remove yourself as reviewer if you aren't interested!) It isn't ideal, but as only AMDGCN

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder added a comment. In D142499#4078177 , @arsenm wrote: > Making this target specific doesn’t make sense I don't know how else to resolve others wanting the default to be the `CGOptLevel = clamp(ltoOptLevel, 2, 3)` behavior and AMDGPU wanting

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder updated this revision to Diff 492205. scott.linder added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142499/new/ https://reviews.llvm.org/D142499 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driv

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder updated this revision to Diff 494435. scott.linder added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142499/new/ https://reviews.llvm.org/D142499 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/lib/Driv

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

scott.linder added a comment. Are there any thoughts on whether this is too ugly to live? It will be awkward to teach users the current default behavior without this change, but if we can accept it as a historical quirk that may be OK. The primary driver for wanting the 1:1 mapping is an odd in

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder created this revision. Herald added subscribers: ormris, steven_wu, hiraditya. Herald added projects: Flang, All. scott.linder requested review of this revision. Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, jdoerfert. Herald added projects: clang, OpenMP, LLVM.

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. I wasn't sure whether to include `getID` and unit-test it, or just drop it until it is used. If anyone has a preference let me know and I'll update the review. Repository: rG LLVM Github Mo

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 This is ABI breaking, so maybe we don't want/need to define the underlying type

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder updated this revision to Diff 490321. scott.linder marked 2 inline comments as done. scott.linder added a comment. - Fix return type of `getID` - Fix mistakenly updated option help text - Add back in missing `assert`s Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 barannikov88 wrote: > arsenm wrote: > > scott.linder wrote: > > > This is ABI b

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG25c0ea2a5370: [NFC] Consolidate llvm::CodeGenOpt::Level handling (authored by scott.linder). Changed prior to commit: https://reviews.llvm.org/D14

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:67 + inline std::optional getLevel(IDType ID) { +if (ID < 0 || ID > 3) + return std::nullopt; mstorsjo wrote: > scott.linder wrote: > > barannikov88 wrote: > > > As I ca

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added a comment. I also ended up changing the underlying type to `int` to resolve https://lab.llvm.org/buildbot/#/builders/61/builds/38754 , and I removed the `getID` function (with the implicit conversions it would never get used anyway). Repository: rG LLVM Github Monorepo CH

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:590 + std::optional OptLevelOrNone = + CodeGenOpt::getLevel(CGOpts.OptimizationLevel); + assert(OptLevelOrNone && "Invalid optimization level!"); aaronpuchert wrote: > D

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

scott.linder added a comment. Thank you for the patch! I will leave it to others with more knowledge of the history of diagnostics to comment on whether the premise is acceptable, but I have a few technical comments Comment at: clang/lib/Frontend/TextDiagnosticPrinter.cpp:119

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

scott.linder added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:99-103 + size_t NewlinePos = PD->getShortDescription().find_first_of('\n'); + if (NewlinePos != std::string::npos) +OutStr = PD->getShortDescription().str().insert(

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:993 + // Metadata needs to be padded out to an even word size. + uint32_t PaddedSize = alignTo(std::max((int)MetadataSize, 1), 4); + uint32_t PaddingSize = PaddedSize - MetadataSize;

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. Thank you for the patch and the changes! This LGTM, and AFAICT all of the comments have been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

scott.linder added a comment. This LGTM, but could you add a simple test with a command-line that needs the escaping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74811/new/ https://reviews.llvm.org/D74811 _

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74811/new/ https://reviews.llvm.org/D74811 __

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

This revision was automatically updated to reflect the committed changes. Closed by commit rG6123074d0c0d: [Driver] Escape the program path for -frecord-command-line (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74811/new/

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

scott.linder added a comment. Multiple build bots were failing with the patch applied, see e.g. http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/4890/steps/test-check-clang/logs/FAIL%3A%20Clang%3A%3Aclang_f_opts.c Seems like on platforms with a backslash for a directory separato

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

scott.linder added a comment. Sorry for the delay; I can just commit this, no need to update the review. I will hopefully be back to a terminal to push it a bit later tonight. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74811/new/ https://review

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

scott.linder added a comment. Recommitted as 340feac6721e840f88c1e77dd79931eea5eaccf3 , hopefully this version sticks, cheers! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74811

[PATCH] D70191: [AMDGPU][HIP] Change default DWARF version to 4

scott.linder created this revision. Herald added subscribers: cfe-commits, t-tye, tpr, dstuttard, aprantl, yaxunl, nhaehnle, wdng, jvesely, kzhuravl. Herald added a project: clang. Tooling around DWARF 5 is still not mature enough for this to be a sane default, and the AMDGPU and HIP toolchains s

[PATCH] D70191: [AMDGPU][HIP] Change default DWARF version to 4

This revision was automatically updated to reflect the committed changes. Closed by commit rGc9de002a2cf0: [AMDGPU][HIP] Change default DWARF version to 4 (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70191/new/ https://r

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

scott.linder added a comment. In D70424#1759379 , @t-tye wrote: > @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

[PATCH] D70987: [HIP] Improve opt-level handling

scott.linder created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The HIP toolchain invokes `llc` without an explicit opt-level, meaning it always uses the default (-O2). This makes it impossible to use -O1, for example. The HIP toolchain also coerces -Os/-

[PATCH] D70987: [AMDGPU][HIP] Improve opt-level handling

scott.linder updated this revision to Diff 232346. scott.linder added a comment. Update other toolchain tests I had missed, and address feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70987/new/ https://reviews.llvm.org/D70987 Files: cla

[PATCH] D70987: [AMDGPU][HIP] Improve opt-level handling

This revision was automatically updated to reflect the committed changes. Closed by commit rGd96ea47c75fd: [AMDGPU][HIP] Improve opt-level handling (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70987/new/ https://reviews.

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

scott.linder added a comment. Is there a corresponding document update to go along with this? If consumers of the bundles will begin to rely on this it should be documented that it is guaranteed by the producer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D153600: Implement -frecord-command-line for XCOFF

scott.linder added a comment. Sorry for the delay in review, I am not too familiar with XCOFF so I was hoping someone else would take a look first. If my understanding of the COFF docs I could find is correct then this LGTM save for some nits/suggestions Comment at: llvm/lib

[PATCH] D147279: [HeterogeneousDWARF] Implement AMDGPU CFI, DebugInfo

scott.linder created this revision. Herald added subscribers: kosarev, foad, kerbowa, arphaman, hiraditya, tpr, dstuttard, yaxunl, jvesely, kzhuravl, emaste, arsenm, qcolombet, MatzeB. Herald added a project: All. scott.linder requested review of this revision. Herald added subscribers: llvm-commi

[PATCH] D146023: [AMDGPU] Remove Code Object V2

scott.linder added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:520 - assert(Func.getCallingConv() == CallingConv::AMDGPU_KERNEL || - Func.getCallingConv() == CallingConv::SPIR_KERNEL); + if (Func.getCallingConv() != CallingConv::A

[PATCH] D142499: [Clang][AMDGPU] Set LTO CG opt level based on Clang option

This revision was automatically updated to reflect the committed changes. Closed by commit rG97ba3c2bec48: [Clang][AMDGPU] Set LTO CG opt level based on Clang option (authored by scott.linder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142499/new

<    1   2   3   >