[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

scott.linder added a comment. When I went to mark these as static I noticed they use `CGDebugInfo::CreateMemberType` which uses a couple other non-static member functions, and it starts to become difficult to tease things out into nice clean static functions. Comment at: lib

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

scott.linder added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:997 llvm::DINode::DIFlags Flags = llvm::DINode::FlagAppleBlock; unsigned LineNo = 0; echristo wrote: > scott.linder wrote: > > echristo wrote: > > > Just noticed that LineNo is 0.

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

scott.linder updated this revision to Diff 159512. scott.linder added a comment. Address feedback https://reviews.llvm.org/D50099 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h test/CodeGenOpenCL/blocks.cl Index: test/CodeGenOpenCL/blocks.cl

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

This revision was automatically updated to reflect the committed changes. Closed by commit rL339265: [DebugInfo][OpenCL] Address post-commit review for r338299 (authored by scott.linder, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

scott.linder created this revision. scott.linder added a reviewer: klimek. Herald added a subscriber: cfe-commits. See https://reviews.llvm.org/D48144 for the LLVM patch. Repository: rC Clang https://reviews.llvm.org/D51223 Files: unittests/Tooling/DiagnosticsYamlTest.cpp unittests/Tooli

[PATCH] D51434: [HIP] Add -amdgpu-internalize-symbols option to opt

scott.linder added a reviewer: arsenm. scott.linder added a comment. +Matt to confirm, but our executable format is a shared object and we eventually want to support preemptible symbols through the PLT. We already generate GOT entries for globals. Currently we work around the lack of PLT suppor

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

scott.linder created this revision. scott.linder added reviewers: Anastasia, echristo. Herald added subscribers: cfe-commits, JDevlieghere, aprantl, yaxunl. OpenCL block literal structs have different fields which are now correctly identified in the debug info. Repository: rC Clang https://r

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

scott.linder added a comment. Thank you for taking a look @yaxunl. Should I wait for another reviewer or can I commit this? Repository: rC Clang https://reviews.llvm.org/D49930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

This revision was automatically updated to reflect the committed changes. Closed by commit rL338299: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL (authored by scott.linder, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

scott.linder added a comment. Sorry, I didn't see the additional comments until after I committed. I will make those changes; is it OK to update this review, or should I create a new one? As for choosing limited it was just what the function adding the debug info checked for as a minimum; what

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

scott.linder created this revision. scott.linder added reviewers: echristo, Anastasia, yaxunl. Herald added subscribers: cfe-commits, JDevlieghere, aprantl. Refactor collection of member debug info into helper functions and add separate debug-info tests. The reason for `-debug-info-kind=limited`

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

scott.linder created this revision. scott.linder added reviewers: yaxunl, Anastasia, arsenm. Herald added subscribers: cfe-commits, wdng. Ensures the statically sized alloca is not converted to DYNAMIC_STACKALLOC later because it is not in the entry block. I believe it is valid to insert the all

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

scott.linder updated this revision to Diff 158532. scott.linder added a comment. Add comments to explain OpenCL case https://reviews.llvm.org/D50099 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h test/CodeGenOpenCL/blocks.cl Index: test/CodeGenOpenCL/blocks.cl =

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

scott.linder updated this revision to Diff 158545. scott.linder added a comment. Address feedback; I hope I understood correctly what debug info to check for. I don't see where in CreateMemTemp and friends EmitLifetimeStart gets called, and I don't see any lifetime intrinsics in the IR even at -

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

scott.linder updated this revision to Diff 158618. scott.linder added a comment. Update test https://reviews.llvm.org/D50104 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGenOpenCL/cl20-device-side-enqueue.cl test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl Index: test/CodeGenOpenCL/enqu

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

scott.linder added a comment. I still don't quite see what you describe; with that change all of the lifetime.end calls pile up just before the enclosing function returns, not after each call to enqueue_kernel. Looking at https://clang.llvm.org/doxygen/EHScopeStack_8h_source.html#l00078 I don't

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

scott.linder updated this revision to Diff 158816. scott.linder added a comment. Emit lifetime intrinsics for the sizes temp, and update test https://reviews.llvm.org/D50104 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGenOpenCL/cl20-device-side-enqueue.cl test/CodeGenOpenCL/enqueue-kernel-n

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

This revision was automatically updated to reflect the committed changes. Closed by commit rL338899: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin (authored by scott.linder, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews

[PATCH] D43044: [DebugInfo] Update Checksum handling in CGDebugInfo

scott.linder created this revision. scott.linder added reviewers: aprantl, JDevlieghere. Herald added a subscriber: cfe-commits. scott.linder added a dependency: D43043: [DebugInfo] Unify ChecksumKind and Checksum value in DIFile. Update to match new DIFile API. Repository: rC Clang https://

[PATCH] D43044: [DebugInfo] Update Checksum handling in CGDebugInfo

scott.linder closed this revision. scott.linder added a comment. Closed by https://reviews.llvm.org/rC324929 Repository: rC Clang https://reviews.llvm.org/D43044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D42766: [DebugInfo] Support DWARFv5 source code embedding extension

scott.linder added a comment. With https://reviews.llvm.org/D42765 committed I think this is ready for review. Repository: rC Clang https://reviews.llvm.org/D42766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D42766: [DebugInfo] Support DWARFv5 source code embedding extension

scott.linder updated this revision to Diff 135909. scott.linder added a comment. New diff addresses feedback. I committed the refactoring as NFC and rebased, then clarified the extension comment. https://reviews.llvm.org/D42766 Files: docs/ClangCommandLineReference.rst include/clang/Driver

[PATCH] D42766: [DebugInfo] Support DWARFv5 source code embedding extension

This revision was automatically updated to reflect the committed changes. Closed by commit rL326102: [DebugInfo] Support DWARF v5 source code embedding extension (authored by scott.linder, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

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

scott.linder created this revision. scott.linder added reviewers: yaxunl, arsenm, kzhuravl, Anastasia. Herald added subscribers: cfe-commits, wdng. The rationale for this is that OpenCL provides programmatic access to these symbols, but not to e.g. non-kernel functions. These symbols should have

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

scott.linder updated this revision to Diff 169417. scott.linder added a comment. Address feedback https://reviews.llvm.org/D53153 Files: lib/AST/Decl.cpp test/CodeGenOpenCL/visibility.cl Index: test/CodeGenOpenCL/visibility.cl ==

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

scott.linder added a comment. The rationale is that `-fvisibility` only affects the default, and already does not apply in many cases. For example, see the rest of the conditions above the fvisibility check in `getLVForNamespaceScopeDecl`: when `Var->getStorageClass() == SC_Static` the linkage

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

scott.linder added a comment. That's my understanding, at least. Take for example: $ cat foo.c static void foo() { } void bar() { } void baz() { bar(); foo(); } $ clang -fvisibility=protected -O0 -S -emit-llvm foo.c -o - ... ; Function Attrs: noinline nounwind optnone uwtab

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

scott.linder added a subscriber: b-sumner. scott.linder added a comment. In https://reviews.llvm.org/D53153#1263804, @rsmith wrote: > In https://reviews.llvm.org/D53153#1263771, @scott.linder wrote: > > > The rationale is that `-fvisibility` only affects the default, and already > > does not app

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

scott.linder added a comment. In https://reviews.llvm.org/D53153#1263835, @yaxunl wrote: > In https://reviews.llvm.org/D53153#1263810, @scott.linder wrote: > > > In https://reviews.llvm.org/D53153#1263804, @rsmith wrote: > > > > > In https://reviews.llvm.org/D53153#1263771, @scott.linder wrote: >

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

scott.linder added a comment. Beyond constructors/destructors I believe an API which we implement through access to dynamic symbols for global variable is `clGetProgramBuildInfo(..., CL_PROGRAM_BUILD_GLOBAL_VARIABLE_TOTAL_SIZE, ...)` which is defined as "The total amount of storage, in bytes, u

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

scott.linder updated this revision to Diff 170637. scott.linder added a comment. Don't mark namespace-scope global variable declarations in OpenCL with explicit default visibility https://reviews.llvm.org/D53153 Files: lib/AST/Decl.cpp test/CodeGenOpenCL/visibility.cl Index: test/CodeGen

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

scott.linder added a comment. In https://reviews.llvm.org/D53153#1263882, @rsmith wrote: > In https://reviews.llvm.org/D53153#1263848, @scott.linder wrote: > > > Beyond constructors/destructors I believe an API which we implement through > > access to dynamic symbols for global variable is > >

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder created this revision. scott.linder added a reviewer: rsmith. Herald added a subscriber: cfe-commits. An attempt at removing one instance of a hardcoded output stream in `CompilerInstance::ExecuteAction`. There are still other cases of output being hard-coded to standard streams in

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

scott.linder added a comment. In D53153#1355718 , @rjmccall wrote: > In D53153#1353256 , @scott.linder > wrote: > > > In D53153#1317977 , @rjmccall > > wrote: > > > > > I t

[PATCH] D53329: Generate DIFile with main program if source is not available

scott.linder added a comment. In D53329#1356381 , @yonghong-song wrote: > @dblaikie @scott.linder Have you got time to look at this issue? Looks like a > trivial test case will be able to reproduce the problem. I have the simple > example in the previou

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

scott.linder added a comment. It sounds like `-fextern-visiblity=` isn't something we really want, then? I think it would have to be implemented in the abstract linkage computer. Would a patch to add an option to apply the normal visibility calculations to extern decls be reasonable, then? This

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

scott.linder added a comment. I think we need the option to be in the driver, not just -cc1. We are moving towards the clang driver being the authority on how we do offline compilation for AMDGPU, and hiding this in cc1 doesn't help us. I also don't think this is unreasonable to expose in gener

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

scott.linder created this revision. scott.linder added reviewers: b-sumner, arsenm, kzhuravl, t-tye, yaxunl. Herald added subscribers: cfe-commits, tpr, dstuttard, wdng. This allows the global visibility controls to be restrictive while still populating the dynamic symbol table where required. De

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

scott.linder abandoned this revision. scott.linder added a comment. Will be superseded by either https://reviews.llvm.org/D53153 or https://reviews.llvm.org/D56871 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52891/new/ https://reviews.llvm.org/D52891 __

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

scott.linder added a comment. That sounds reasonable to me. I had already posted a patch with the Driver options, but I will update it to only include the -cc1 version. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53153/new/ https://reviews.llvm.org/D53153 _

[PATCH] D56868: Add -fset-visibility-for-decls for -cc1

scott.linder updated this revision to Diff 182390. scott.linder retitled this revision from "Add -f[no-]set-visibility-for-decls" to "Add -fset-visibility-for-decls for -cc1". scott.linder added a comment. Remove driver options CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56868/new/ h

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

scott.linder updated this revision to Diff 182394. scott.linder added a comment. Add missing flag to tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56871/new/ https://reviews.llvm.org/D56871 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/TargetInfo.cpp test/CodeGenOpenCL/v

[PATCH] D56868: Add -fextern-visibility for -cc1

scott.linder updated this revision to Diff 182528. scott.linder retitled this revision from "Add -fset-visibility-for-decls for -cc1" to "Add -fextern-visibility for -cc1". scott.linder added a comment. Update the -cc1 option name and docs; also update the LangOpt and docs. CHANGES SINCE LAST A

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

scott.linder updated this revision to Diff 182532. scott.linder added a comment. Update option name CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56871/new/ https://reviews.llvm.org/D56871 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/TargetInfo.cpp test/CodeGenOpenCL/visibili

[PATCH] D56868: Add -fapply-global-visibility-to-externs for -cc1

scott.linder updated this revision to Diff 182559. scott.linder retitled this revision from "Add -fextern-visibility for -cc1" to "Add -fapply-global-visibility-to-externs for -cc1". scott.linder added a comment. No worries, I agree that we don't gain much with a shorter flag here; explicit seem

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

scott.linder updated this revision to Diff 182561. scott.linder added a comment. Update option name CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56871/new/ https://reviews.llvm.org/D56871 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/TargetInfo.cpp test/CodeGenOpenCL/visibili

[PATCH] D56868: Add -fapply-global-visibility-to-externs for -cc1

This revision was automatically updated to reflect the committed changes. Closed by commit rL352391: Add -fapply-global-visibility-to-externs for -cc1 (authored by scott.linder, committed by ). Changed prior to commit: https://reviews.llvm.org/D56868?vs=182559&id=183886#toc Repository: rL LL

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

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

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

scott.linder updated this revision to Diff 171936. scott.linder added a comment. Rebase @klimek is there anyone I should add to take a look at this? As far as the YAML is concerned I believe this is a non-functional change. https://reviews.llvm.org/D51223 Files: unittests/Tooling/Diagnostic

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D53768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

scott.linder added a comment. Ping https://reviews.llvm.org/D53153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

scott.linder added a comment. I don't believe that is currently the case (the unrestricted linking of OCL code to OCL code via a dynamic linker), but we do have the notion of a static link step, followed by dynamic linking at runtime. The static link step is currently via IR, but we plan to sup

[PATCH] D54489: Implement -frecord-gcc-switches

scott.linder created this revision. Herald added a subscriber: cfe-commits. See the parent revision https://reviews.llvm.org/D54487 for differences between this implementation and the equivalent GCC option. Repository: rC Clang https://reviews.llvm.org/D54489 Files: docs/ClangCommandLineR

[PATCH] D54489: Implement -frecord-gcc-switches

scott.linder added a comment. In https://reviews.llvm.org/D54489#1297504, @troyj wrote: > I realize that you're probably striving for option compatibility with gcc, > but continuing to name it -frecord-gcc-switches when it actually records > Clang switches seems weird to me. It almost sounds l

[PATCH] D48144: [Support] Teach YAMLIO about polymorphic types

This revision was automatically updated to reflect the committed changes. Closed by commit rC346884: [Support] Teach YAMLIO about polymorphic types (authored by scott.linder, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D48144?vs=1719

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

scott.linder added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D53768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder updated this revision to Diff 174104. scott.linder retitled this revision from "Implement -frecord-gcc-switches" to "Implement -frecord-command-line (-frecord-gcc-switches)". scott.linder added a comment. Change canonical option name to -frecord-command-line and add an alias from -f

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder added inline comments. Comment at: docs/ClangCommandLineReference.rst:797 + +Generate a section .LLVM.command.line containing the clang driver command line. + rjmccall wrote: > 1. Is this section always called `.LLVM.command.line`, or does it differ

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder added a comment. Looking a bit further, it seems some other object-file formats have conventions for naming (e.g. Mach-O sections are `_snake_case`) and so `.LLVM.command.line` may not end up being what we want universally. Additionally it seems things like `.ident` are not support

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

scott.linder accepted this revision. scott.linder added a comment. This revision is now accepted and ready to land. r346884 https://reviews.llvm.org/D51223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

scott.linder closed this revision. scott.linder added a comment. r346884 https://reviews.llvm.org/D51223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53329: Generate DIFile with main program if source is not available

scott.linder added a comment. In https://reviews.llvm.org/D53329#1300435, @dblaikie wrote: > Thanks! > > So I can see where/how this fails now - the LLVM backend seems to require > that if any file has embedded source, they all do. Would you be able to/would > you mind adding a debug info verif

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder updated this revision to Diff 174429. scott.linder added a comment. Update documentation for new option and error in the driver when generating for unsupported object-file format. https://reviews.llvm.org/D54489 Files: docs/ClangCommandLineReference.rst include/clang/Driver/CC

[PATCH] D53329: Generate DIFile with main program if source is not available

scott.linder added a comment. See https://reviews.llvm.org/D54953 for the Verifier work. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53329/new/ https://reviews.llvm.org/D53329 ___ cfe-commits mailing list cfe-commi

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder updated this revision to Diff 175540. scott.linder added a comment. Update documentation for new option CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54489/new/ https://reviews.llvm.org/D54489 Files: docs/ClangCommandLineReference.rst include/clang/Driver/CC1Options.td

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

scott.linder added a comment. In D53153#1291348 , @rjmccall wrote: > In D53153#1289380 , @scott.linder > wrote: > > > I don't believe that is currently the case (the unrestricted linking of OCL > > code to OCL cod

[PATCH] D53329: Generate DIFile with main program if source is not available

scott.linder added a comment. The Verifier patch has landed as https://reviews.llvm.org/rL348022 I do not have any more knowledge than the reviewers of the right way to handle providing source for remapped files, but at least compilation will warn and strip debug-info rather than segfault when

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

scott.linder added a comment. In D53153#1314798 , @rjmccall wrote: > You still have the same linkage model for those other languages, right? > Ultimately there's something like a kernel function that has to be declared > specially and which becomes the

[PATCH] D53768: Add VerboseOutputStream to CompilerInstance

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

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

scott.linder added a comment. In D53153#1315109 , @rjmccall wrote: > Okay. So it's still the case that all symbols will be defined within the > linkage unit; it's just that some things might need to get exposed outside of > it. > > LLVM does provide a `

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder added a comment. There may be changes to some details in the LLVM patch; once they are finalized I will update the Clang patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54489/new/ https://reviews.llvm.org/D54489 ___ cfe-c

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

scott.linder created this revision. scott.linder added reviewers: kzhuravl, t-tye. Herald added subscribers: cfe-commits, tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely. Herald added a project: clang. Reverts r337612. The issues that cropped up with the last attempt appear to have gone away.

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

scott.linder added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59008/new/ https://reviews.llvm.org/D59008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

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

scott.linder updated this revision to Diff 192688. scott.linder added a comment. Add a test to confirm split-dwarf is supported for the amdhsa OS in the driver. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59008/new/ https://reviews.llvm.org/D59008 Files: lib/Driver/ToolChains/AMDGP

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC357285: [AMDGPU] Switch default DWARF version to 5 (authored by scott.linder, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59008/new/ https://reviews.llvm

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder created this revision. scott.linder added reviewers: atanasyan, rjmccall, yaxunl. Herald added subscribers: cfe-commits, arichardson, tpr, sdardis. Herald added a project: clang. AMDGPU relies on global properties being set before setTargetProperties is called. Existing targets like

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. In D60967#1475925 , @rjmccall wrote: > It seems reasonable to me for target hooks to run after global hooks, but can > I ask why AMDGPU specifically relies on this? We want to ensure certain symbols have a meaningful visibi

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. In D60967#1476029 , @rjmccall wrote: > Shouldn't it be an error if the user tries to give it hidden visibility? We effectively consider the user explicitly specifying that a symbol is e.g. a `kernel` to also carry with it v

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. In D60967#1476069 , @rjmccall wrote: > In D60967#1476057 , @scott.linder > wrote: > > > In D60967#1476029 , @rjmccall > > wrote: > > > > > Sho

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. In D60967#1476226 , @rjmccall wrote: > I suspect that other OpenCL and CUDA implementations don't care at all about > symbol visibility for device-side code generation, and giving kernel > functions default visibility seems

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

This revision was automatically updated to reflect the committed changes. Closed by commit rC359039: Move setTargetAttributes after setGVProperties in SetFunctionAttributes (authored by scott.linder, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D609

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. @rjmccall Would you expect similar conflicts in explicit visibility to result in diagnostics? For example, marking a `static` variable with an explicit visibility attribute doesn't warn, instead the explicit visibility attribute is silently ignored. GCC 7.3 complai

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

scott.linder created this revision. scott.linder added reviewers: rjmccall, espindola. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC warns on these cases, but we currently just silently ignore the attribute. Repository: rC Clang https://reviews.llvm.org/D61097 Fi

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

scott.linder marked 3 inline comments as done. scott.linder added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:2621 +if (!ND->isExternallyVisible()) { + S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL; + return; aaron.

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

scott.linder updated this revision to Diff 196853. scott.linder added a comment. Clean up use of `auto` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61097/new/ https://reviews.llvm.org/D61097 Files: lib/Sema/SemaDeclAttr.cpp test/Sema/attr-visibility.c test/SemaCXX/ast-print.cpp

[PATCH] D60967: Move setTargetAttributes after setGVProperties in SetFunctionAttributes

scott.linder added a comment. @rjmccall I'm not sure if this is the right place to continue discussing this, but I don't have a patch I am happy with and I would rather not post something half-baked. Currently for AMDGPU we have the behavior that the user can set the visibility of these symbol

[PATCH] D61274: [Sema][AST] Explicit visibility for OpenCL/CUDA kernels/variables

scott.linder created this revision. scott.linder added reviewers: Anastasia, tra, yaxunl, rjmccall. Herald added subscribers: cfe-commits, tpr. Herald added a project: clang. For AMDGPU the visibility of these symbols (OpenCL kernels, CUDA `__global__` functions, and CUDA `__device__` variables)

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

scott.linder updated this revision to Diff 197570. scott.linder added a comment. Use distinct sema diagnostic and do not return early. I am going to bump `DIAG_SIZE_SEMA` in another patch, as it seems like we have hit the current limit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D610

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rL359814: [Sema] Emit warning for visibility attribute on internal-linkage declaration (authored by scott.linder, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

scott.linder added a comment. Richard Smith on cfe-dev pointed out some cases where this patch is incorrect, stemming from trying to calculate the linkage too early; the warning will either have to work without the use of `isExternallyVisible` or will have to be emitted later. This was reverted

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

scott.linder created this revision. scott.linder added reviewers: yaxunl, kzhuravl, arsenm. Herald added subscribers: cfe-commits, Anastasia, tpr, dstuttard, nhaehnle, wdng, jvesely. Controls the visibility of non-kernel functions when compiling for AMDGPU targets. Defaults to the default value

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

scott.linder added a comment. I don't know who else to add as a reviewer; Sam, is there someone else outside of AMD that would be interested in reviewing this? Repository: rC Clang https://reviews.llvm.org/D52891 ___ cfe-commits mailing list cfe

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

scott.linder updated this revision to Diff 168314. scott.linder added a comment. Update docs https://reviews.llvm.org/D52891 Files: docs/ClangCommandLineReference.rst include/clang/Basic/LangOptions.def include/clang/Driver/CC1Options.td include/clang/Driver/Options.td lib/CodeGen/Tar

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

scott.linder added a comment. I will update the patch to modify the HIP toolchain and to add tests for global variables. As far as the semantics are concerned, are we OK with this being AMDGPU only? I do not see a means of determining what is a "kernel" in a language-agnostic way other than ch

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

scott.linder abandoned this revision. scott.linder added a comment. See https://reviews.llvm.org/D56871 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53153/new/ https://reviews.llvm.org/D53153 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

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

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rL353870: [AMDGPU] Require at least protected visibility for certain symbols (authored by scott.linder, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior t

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder updated this revision to Diff 178082. scott.linder added a comment. Update documentation to match section naming change, and to explicitly note that the section format differs from the equivalent GCC format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54489/new/ https://

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder added a comment. In D54489#1330255 , @rjmccall wrote: > So we're using the same command line option as GCC to produce something in > the same section as GCC but formatting that section incompatibly with GCC? > That combination of choices do

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

scott.linder closed this revision. scott.linder added a comment. rL349155 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54489/new/ https://reviews.llvm.org/D54489 ___ cfe-commits mailing list cfe-commits@

  1   2   3   >