[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

probinson created this revision. probinson added a reviewer: MaskRay. Herald added a project: All. probinson requested review of this revision. People use -ffunction-sections to put each function into its own object-file section; this makes linker garbage-collection simpler. However, if there's

[PATCH] D144586: [PS4/PS5] Specify no or

This revision was automatically updated to reflect the committed changes. Closed by commit rG32d441bfb4f3: [PS4/PS5] Specify no complex.h or threads.h (authored by probinson). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144586: [PS4/PS5] Specify no or

probinson added a comment. That's correct, I do see in our SDK. I don't see a need for a release note; we're not actually removing anything that we used to support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144586/new/ https://reviews.llvm.org/D144586

[PATCH] D144586: [PS4/PS5] Specify no or

probinson created this revision. probinson added a reviewer: aaron.ballman. Herald added a project: All. probinson requested review of this revision. We've never provided these headers so set the preprocessor toggles to reflect that. https://reviews.llvm.org/D144586 Files:

[PATCH] D144188: Tighten up a modules test

This revision was automatically updated to reflect the committed changes. Closed by commit rG17a90f1196c1: Tighten up a modules test (authored by probinson). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144188/new/

[PATCH] D144188: Tighten up a modules test

probinson added a comment. I looked at this test only because it caused a merge conflict downstream. While it does work, it will not catch some kinds of mistakes; by being less specific in the "not" checks, it will catch more potential problems. Comment at:

[PATCH] D144188: Tighten up a modules test

probinson created this revision. probinson added a reviewer: ChuanqiXu. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D144188 Files: clang/test/CXX/module/basic/basic.def.odr/p4.cppm Index: clang/test/CXX/module/basic/basic.def.odr/p4.cppm

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

probinson abandoned this revision. probinson added a comment. Discussion on the GCC bug has persuaded me this is not a good idea. I'll solve my user's problem a different way. @MaskRay you can close the GCC bug, it looks like I can't do it myself. CHANGES SINCE LAST ACTION

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

probinson added a comment. I'm not sure I understand the linker's mechanics here. Let me say some things and you can describe my misunderstanding. - If the linker was going to discard all of section foo (in the current scheme), that means it had no reason to retain either f() or g(). In the

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

probinson added a comment. This works in my simple test cases, but I'm not 100% sure it's (a) the best place to do this, or (b) the only place that needs to do this. Really we should guarantee that `comdat any` wins over `comdat nodeduplicate` in all cases. CHANGES SINCE LAST ACTION

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

probinson created this revision. probinson added reviewers: rjmccall, MaskRay. Herald added a project: All. probinson requested review of this revision. People use -ffunction-sections to put each function into its own object-file section; this makes linker garbage-collection simpler. However, if

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

probinson added a subscriber: wolfgangp. probinson added a comment. Basically you're homing a pointer to the parameter, rather than the parameter itself, which makes sense in this context. I suspect the code size/perf impact on -O0 will not be huge, as this sounds like it's only for

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

probinson added a comment. I should expand on my previous comment. Extending the lifetime of these variables by saving the pointers into local variables, that's fine; just make sure it's not conditional on -g. The only difference between -g and not -g should be metadata and dbg.*

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

probinson added a comment. This causes codegen to be different depending on whether debug-info is generated. Please don't do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141381/new/ https://reviews.llvm.org/D141381

[PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

probinson added a comment. Herald added subscribers: jsetoain, JDevlieghere, ormris. Worth trying on MSVC, which is the other more likely place to fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140332/new/ https://reviews.llvm.org/D140332

[PATCH] D138954: [PPC] Convert tests to check 'target='

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG948bb35d7474: [PPC] Convert tests to check target=triple (authored by probinson). Herald added a project: clang. Herald added a subscriber:

[PATCH] D139953: [llvm][DebugInfo] Backport DW_AT_default_value for template args

probinson added a comment. > CHECK-LABEL doesn't have to match a line in the file uniquely. I mean, it's good practice if they do match uniquely; that way you don't get excessively confusing results when the output changes, and things start matching where you didn't expect. But it's not a

[PATCH] D139953: [llvm][DebugInfo] Backport DW_AT_default_value for template args

probinson added a comment. > IIUC the labels have to match a line in the file uniquely, which the DW_TAGs > wouldn't CHECK-LABEL doesn't have to match a line in the file uniquely. What happens is that all the -LABEL directives are processed first, in order, subdividing the input text into

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

probinson added a comment. See D139967 for why `UNSUPPORTED: powerpc` didn't work. That patch will put it back, and also update the lit config so the check will work now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7793e676514b: [ZOS] Convert tests to check target={{.*}}-zos{{.*}} (authored by probinson). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson added a comment. Thanks @Kai and @uweigand, good to know a z/OS bot is in the works. Hope this patch now meets your needs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139444/new/ https://reviews.llvm.org/D139444 ___ cfe-commits

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson updated this revision to Diff 482180. probinson added a comment. Herald added a subscriber: delcypher. Define 'system-zos' and use it in the one test that needs it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139444/new/ https://reviews.llvm.org/D139444 Files:

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson added a comment. I suppose we could temporarily add a test that does ; REQUIRES: target={{.*}}-zos ; RUN: %python -c 'import platform; print(platform.system())' && false and see what gets printed. Searching the buildbot console page for 'zos' turns up nothing; 's390' turns up

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson updated this revision to Diff 481421. probinson added a comment. Add trailing '{{.*}}' as requested. Have not changed the encoding.ll test, waiting on @uweigand about correct value to test in Python. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139444/new/

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson added a comment. In D139444#3978189 , @uweigand wrote: > In D139444#3975182 , @probinson > wrote: > >> The changes in this patch assume that there aren't any possible suffixes >> after the `-zos` part

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. Oh, right, PS4 defaults to C99. It's okay with me if you make those two unsupported for PS4. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138597/new/ https://reviews.llvm.org/D138597

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. LGTM. I agree with the commentary in the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138597/new/ https://reviews.llvm.org/D138597 ___ cfe-commits mailing list

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson added a comment. The changes in this patch assume that there aren't any possible suffixes after the `-zos` part of the triple (no version numbers, like you might find with darwin or macos, and nothing like `-elf` or `-eabi` like some targets have). If there are suffixes, I'll

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

probinson created this revision. probinson added reviewers: uweigand, Kai. Herald added a project: All. probinson requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Part of the project to eliminate special handling for

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}-aix{{.*}}'

probinson added a comment. The clang-ppc64-aix bot ran with these changes here and all results are as expected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137437/new/

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}-aix{{.*}}'

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG64e4d03c681a: [lit][AIX] Convert clang tests to use target={{.*}}-aix{{.*}} (authored by probinson). Changed prior to commit:

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. One not-yet-asked question, does gcc produce the C++17/20 codes? If so, does fstrict-dwarf affect that? The major consumers of this would be lldb (which we control) and gdb, so following gcc's lead here would seem appropriate. I can accept that listing the codes on

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}-aix{{.*}}'

probinson added a comment. Lacking an LGTM from someone with AIX knowledge, I see there's a bot "clang-ppc64-aix" where I can verify that the test results haven't changed. For this review the set of tests are mostly UNSUPPORTED, except for three XFAIL. I'll let this sit a little while in case

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. > They're published on the website too: https://dwarfstd.org/LanguagesV6.php (sigh) sorry, distracted by other things today. Yeah, using the v6 codes with the v6 attributes for the new ones WFM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. In D138597#3957356 , @dblaikie wrote: > In D138597#3954269 , @probinson > wrote: > >> The codes have been allocated but AFAICT the website doesn't have the new >> codes listed (I

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}-aix{{.*}}'

probinson updated this revision to Diff 478382. probinson retitled this revision from "[lit][AIX] Convert clang tests to use 'target={{.*}}aix{{.*}}'" to "[lit][AIX] Convert clang tests to use 'target={{.*}}-aix{{.*}}'". probinson added a comment. Be consistent about using `-aix` CHANGES

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

probinson added a comment. Hmmm I might be inclined to emit 17 and 20 only under not-strict-DWARF for v5, although it makes the logic more complicated. The codes have been allocated but AFAICT the website doesn't have the new codes listed (I looked at

[PATCH] D138463: [windows-itanium] Propagate DLL storage class to Initialisation Guard Variables

probinson added a subscriber: cfe-commits. probinson added a comment. +cfe-commits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138463/new/ https://reviews.llvm.org/D138463 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

probinson added a comment. If this is specifically for C++17, I believe Sony doesn't officially support that yet although I am checking. It looks like this is only part of the fix for #58819? The original report also had a `static int n` with an internal-linkage name. Repository: rG LLVM

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}aix{{.*}}'

probinson added inline comments. Comment at: clang/test/ClangScanDeps/modules-no-undeclared-includes.c:3 // section in XCOFF yet. -// UNSUPPORTED: aix +// UNSUPPORTED: target={{.*}}aix{{.*}} hubert.reinterpretcast wrote: > Perhaps we should normalize on

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}aix{{.*}}'

probinson edited subscribers, added: cfe-commits; removed: clang. probinson added a comment. Try again to add the commits list. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137437/new/ https://reviews.llvm.org/D137437 ___ cfe-commits

[PATCH] D137287: [Test] Fix CHECK typo.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM, but you'll want to be ready to jump on any bot failures. That's just the nature of these kinds of changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131919: Move googletest to the third-party directory

probinson accepted this revision. probinson added a comment. Yes, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131919/new/ https://reviews.llvm.org/D131919 ___ cfe-commits mailing list

[PATCH] D137287: [Test] Fix CHECK typo.

probinson added a comment. Nice work! have you verified that all the modified tests pass? naively it looks like you'd need at least 3 different targets to run them all (windows, webassembly, powerpc) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D133998: [HIP][test] Avoid %T

probinson added a comment. I filed #58711 to get the test fixed; in the meantime, removed `clang-driver` and added `XFAIL: *` in rG7af01fe4 Repository: rG LLVM Github

[PATCH] D133998: [HIP][test] Avoid %T

probinson added subscribers: lamb-j, probinson. probinson added a comment. @MaskRay I see that you restored the `clang-driver` keyword in hip-link-bc-to-bc.hip, and of course that keyword is not defined anywhere so the test is never being run. Is there an issue filed to track this? Or is the

[PATCH] D136309: [clang][Toolchains][Gnu] pass -g through to assembler

probinson added inline comments. Comment at: clang/test/Driver/as-options.s:121 +// Test that -g is passed through to GAS. +// RUN: %clang -fno-integrated-as -g %s -### 2>&1 | \ +// RUN: FileCheck --check-prefix=CHECK-DEBUG %s Please use an explicit triple

[PATCH] D136187: [clang][AIX] Omitting Explicit Debugger Tuning Option

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4374 + ? llvm::DebuggerKind::Default + : DebuggerTuning); Seems like you should be able to return

[PATCH] D136188: Update docs for -fuse-ctor-homing

probinson added a subscriber: jmorse. probinson added a comment. +jmorse who is closer to this topic than I am. We've had a few complaints from licensees about ctor homing, and debug-info size in general is something of a sensitive topic. But if we can come to a place where

[PATCH] D136041: [clang][DebugInfo] Emit DISubprogram for extern functions with reserved names

probinson added inline comments. Comment at: llvm/lib/IR/Verifier.cpp:3470 + // (Interposable functions are not inlinable as are functions w/o + // declarations). if (Call.getFunction()->getSubprogram() && Call.getCalledFunction() && (Interposable

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

probinson added subscribers: wristow, probinson. probinson added a comment. +wristow who has been tracking this kind of stuff for Sony. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135097/new/ https://reviews.llvm.org/D135097 ___

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

probinson added a comment. dblaikie: > In the same way that an assert says "This condition should never be false" - > I use "should" in the same sense in both unreachable and assert, and I > believe that's the prevailing opinion of LLVM developers/the LLVM style guide. aaronballman: > I

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

probinson added inline comments. Comment at: clang/test/Frontend/frame-diags.c:1 +// TODO: This is not really a functional test yet, and needs to be rewritten. +// currently its just a copy of backend-diagnostics.c, and all of the run/test I know this is a

[PATCH] D135115: [clang-format] update --files help description

probinson added a comment. In D135115#3839547 , @HazardyKnusperkeks wrote: > It's too long ago for me. Does the `--files` option take multiple file names > on the command line, or a file containing the file names? If the latter the > patch generally

[PATCH] D135115: [clang-format] update --files help description

probinson added a comment. The help text in ClangFormat.cpp for the `--files` option needs to be updated the same way. Note using `cl::value_desc("filename")` is what you need to change the meta-variable in the help output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135115: [clang-format] update --files help description

probinson added inline comments. Comment at: clang/docs/ClangFormat.rst:72-73 Used only with --dry-run or -n ---files= - Provide a list of files to run clang-format +--files= - Accept a list of response

[PATCH] D135115: [clang-format] update --files help description

probinson added a comment. The option handling in clang-format itself would need to be updated as well. Comment at: clang/docs/ClangFormat.rst:29 USAGE: clang-format [options] [ ...] MyDeveloperDay wrote: > if response files are so common place why not

[PATCH] D111000: [clang-format] allow clang-format to be passed a file of filenames so we can add a regression suite of "clean clang-formatted files" from LLVM

probinson added a comment. Response files should work for all our tools, I think? Keeping the option is fine with me, but the description needs some improvement. @ychen Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111000/new/

[PATCH] D111000: [clang-format] allow clang-format to be passed a file of filenames so we can add a regression suite of "clean clang-formatted files" from LLVM

probinson added a comment. In D111000#3822354 , @ychen wrote: > It is possible to just use `clang-format @response.txt`? That should also > invoke the executable once. It's true that whatever file you're handing to `--files` could just as easily be

[PATCH] D134880: [clang] [test] Use %clang_cc1 substitution consistently

probinson added inline comments. Comment at: clang/test/Driver/lit.local.cfg:8 -('%clang_cc1', - """*** Do not use 'clang -cc1' in Driver tests. ***""") ) But we _don't_ want to use `%clang_cc1` in Driver tests; if we do, then we're not testing the

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. In D134507#3818765 , @ychen wrote: > In D134507#3817928 , @probinson > wrote: > >> It feels odd to use a ClangABI check for something that is affecting what >> source is accepted, but

[PATCH] D134673: [Driver][PS4] pass -fcrash-diagnostics-dir to LTO

probinson accepted this revision. probinson 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/D134673/new/ https://reviews.llvm.org/D134673

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a subscriber: rjmccall. probinson added a comment. It feels odd to use a ClangABI check for something that is affecting what source is accepted, but this is not my area of expertise. @aaron.ballman or @rjmccall would probably be the right people to weigh in on this.

[PATCH] D134673: [Driver][PS4] pass -fcrash-diagnostics-dir to LTO

probinson added a comment. I'm guessing the non-PS4 targets would also want this, so a similar change is needed in tools::addLTOOptions. This can be a separate patch if you wish. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134673/new/

[PATCH] D134544: [clang-cl] Implement /ZH: flag

probinson added inline comments. Comment at: clang/test/CodeGen/debug-info-file-checksum.c:3 +// RUN: %S/Inputs/debug-info-file-checksum.c -o - | FileCheck %s +// RUN: %clang -emit-llvm -S -g -gcodeview -gsrc-hash-algorithm=md5 -x c \ +// RUN:

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

probinson added a comment. > The number of people likely to notice that PS4 is different from the other > targets seems far smaller than the number of people noticing the same for > Windows. But I've got no data to back that up, either. heh. There's a PS4 bot that notices. But the PS4

[PATCH] D134468: [Driver] Ignore -fmsc-version= -fms-compatibility-version= values smaller than 19

probinson added a comment. These kinds of compatibility changes generally should get a Discourse post for better visibility. Comment at: clang/docs/ReleaseNotes.rst:85 + a version smaller than ``19.0`` is now unsupported. + Or words to that effect. "19.0"

[PATCH] D98438: Clang: Allow selecting the hash algorithm for file checksums in debug info.

probinson added a comment. So this one can be marked abandoned? You might need to commandeer it first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98438/new/ https://reviews.llvm.org/D98438 ___

[PATCH] D134544: [clang-cl] Implement /ZH: flag

probinson added a project: debug-info. probinson added a comment. Probably we should reject the -gsrc... option if we're emitting DWARF, which requires MD5 (at least so far). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134544/new/ https://reviews.llvm.org/D134544

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. Hi @ychen I saw you updated the patch description, thanks. IIUC, the change causes some difference in the interpretation of particular template stuff. Would this be clearer if the BEFORE-15/AFTER-15 test cases used the same template construct? Currently they are

[PATCH] D134507: [Clang] add missing ClangABICompat check

probinson added a comment. Anytime there's a Clang ABI version check, I have to wonder whether PS4/PS5 need an exception. (And I'm sorry I missed the earlier patches in this series.) We are pretty much locked to however things worked for Clang 3.2. Is this specific to C++20 (or later)? In

[PATCH] D133875: [clang] fix generation of .debug_aranges with LTO (resubmit)

probinson added inline comments. Comment at: clang/test/Driver/debug-options-aranges.c:7 +// RUN: %clang -### -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=PLUGIN_GARANGE %s +// GARANGE: -generate-arange-section +//

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

probinson added a comment. > we don't use addLTOOptions() for either PS4 or PS5, so this patch doesn't > affect (or help) them. I'm okay with saying we'll need to deal with that case > separately. FTR, I've raised an internal ticket about this, and someone from Sony will handle it.

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:515 +// +// But note, this solution is far from perfert, better to encode it into IR +// metadata, but this may not worth it, since looks like aranges on the way

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

probinson added a comment. I'm happy if @dblaikie is happy. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:515 +// +// And as @dblaikie noted, this solution is far from perfert, better to +// encode it into IR metadata, but this may not worth it, since

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

probinson added a reviewer: probinson. probinson added a comment. > Yeah, but it seems that it is not possible to use lld for PS4 It's correct that PS4 does not use lld. PS5 does use lld (under a different name). But, we don't use addLTOOptions() for either PS4 or PS5, so this patch doesn't

[PATCH] D133191: Driver test: remove `REQUIRES: x86-registered-target` and set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`.

probinson closed this revision. probinson added a comment. Closing this for @MaggieYi the commit message didn't cite the Phabricator review. https://github.com/llvm/llvm-project/commit/5de4d97a00b2a5d710892e96d77810784fd2cd5c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM. Checking the tests over by eye, it looks like all run the clang driver with `-###` or `-E` so only the driver itself is invoked, and no backend dependency exists. Repository:

[PATCH] D131820: [PS4][driver] make -fjmc work with LTO driver linking stage

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM with one comment on the test. Comment at: clang/test/Driver/ps4-ps5-linker-jmc.c:1 +// REQUIRES: x86-registered-target + This REQUIRES is not

[PATCH] D131919: Move googletest to the third-party directory

probinson added a comment. In D131919#3749850 , @Meinersbur wrote: > Semi-OT: `polly\lib\External` has 3 more third-party libraries. Two of them > have been heavily modified in-tree, the third has just a custom > CMakeLists.txt. > Should these

[PATCH] D131919: Move googletest to the third-party directory

probinson added inline comments. Comment at: clang/CMakeLists.txt:106 AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt) add_subdirectory(${UNITTEST_DIR} utils/unittest) endif() Should this be `third-party/unittest` ? Looking at how the lldb

[PATCH] D128934: [X86] Add RDPRU instruction

probinson added a comment. In D128934#3630800 , @probinson wrote: > In D128934#3629653 , @pengfei wrote: > >> > > Done, thanks for the reminder! > >>> 2. Update `clang/lib/Headers/cpuid.h` and

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

probinson added a comment. What Aaron said. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131465/new/ https://reviews.llvm.org/D131465 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D120175: [Driver] Re-run lld with --reproduce when it crashes

probinson added a comment. > there didn't already exist a PS4 lit feature that I could mark as unsupported. ? I believe `UNSUPPORTED: ps4` should work, because UNSUPPORTED will look at the default target triple. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120175: [Driver] Re-run lld with --reproduce when it crashes

probinson added a comment. > lld has tests for --reproduce already. Even if I added an environment > variable to crash lld, the test would depend on lld being built, adding a big > dependency to clang's tests. Do we think that's worth it? There is now cross-project-tests which would seem to be

[PATCH] D130786: [clang-repl] Disable execution unittests on unsupported platforms.

probinson added a comment. If you're going to post a patch for review, you really should wait for someone to review it before you land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130786/new/ https://reviews.llvm.org/D130786

[PATCH] D130493: Disable stack-sizes section by default for PS4.

probinson accepted this revision. probinson 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/D130493/new/ https://reviews.llvm.org/D130493

[PATCH] D129404: Change default C dialect for PS5 to gnu17/gnu18.

probinson accepted this revision. probinson 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/D129404/new/ https://reviews.llvm.org/D129404

[PATCH] D128934: [X86] Add RDPRU instruction

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG08e4fe6c6196: [X86] Add RDPRU instruction (authored by probinson). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed

[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

probinson added a comment. Seems like for an "auto"-returning function/lambda with a definition, the front-end should have deduced the return type, and so we should emit that in the DWARF, even if we end up emitting DWARF with both a declaration and a separate definition. I accept that a

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

probinson added a comment. I'm in the middle of upstreaming PS5 support and I still didn't think of this... doh! Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5588 + bool DefaultedSMFArePOD = !RawTriple.isPS4() && !RawTriple.isOSDarwin(); + `isPS4()`

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + dblaikie wrote: > probinson wrote: > > dblaikie wrote: > > > probinson wrote: > > > > Does this mean `-fclang-abi-compat` will

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + dblaikie wrote: > probinson wrote: > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special > > case? I think

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

probinson added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + Does this mean `-fclang-abi-compat` will override the PS4/Darwin special case? I think we don't want to do that. Repository:

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. I see some unrelated whitespace changes, we generally don't like mixing those with "real" changes. But the description seems fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. In D126224#3532769 , @dblaikie wrote: > While the string would be deduplicated, the offset in the DIEs (or index in > DWARFv5, plus offset in .debug_str_offsets) for each of these strings would > not. But perhaps that would

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. Unnamed variables are an oddity, sure; we've had to patch a downstream test or two that wasn't being careful enough. But not providing a name is entirely defensible, and consumers should be willing to cope with DWARF that doesn't fully meet their expectations.

[PATCH] D114115: [Driver] Support for compressed debug info on Fuchsia

probinson added a comment. Lit is aware of zlib's presence or absence, you could use `REQUIRES: zlib` (or you might factor that bit out into its own test with the REQUIRES). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114115/new/

[PATCH] D121299: [NVPTX] Disable DWARF .file directory for PTX

probinson added a comment. I'm okay with doing it this way. My impression over the years (as a debug-info guy, not as a ptxas user) is that ptxas evolves slowly and this is not really "busy work." Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

probinson added a comment. > What's the DW_AT_type referring to? (why is there a type called ".str"?) I'd expect it to point to a base type, in this case character string. A typeless variable with a location would be pretty weird. But I agree the name/linkage_name would be unnecessary.

<    1   2   3   4   5   6   >