[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#639887, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#639874, @probinson wrote: > > > Over the weekend I had a thought: Why is -O0 so special here? That is, > > after going to all this trouble to propagate -O0 to LTO, how

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640046, @probinson wrote: > > > "I don't care" doesn't seem like much of a principle. > > > Long version is: "There is no use-case, no users, so I don't have much > moti

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640170, @probinson wrote: > In my experience, modifying source Note that the source modification consists of adding `#pragma clang optimize off` to the top of the file. It is not a complicated thing. https://reviews.llvm.org/D284

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640170, @probinson wrote: > > > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#640046, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640182, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > > > Also, that's not practicable: what if I have an LTO static library for > > which I don't have the source, now if I build my own file wi

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > I don't follow: IMO if I generate a module with optnone and pipe it to `opt > -O3` I expect no function IR to be touched. If it is not the case it is a bug. Your opinion and expectation are not supporte

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > You just wrote above that " mixing -O0 and LTO " is wrong, *if* I were to > agree with you at some point, then I'd make it a hard error. Yes, I was not clear that I meant that `-O0 -flto` on the same cl

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640284, @probinson wrote: > > > Upfront, it seemed peculiar to handle only one optimization level. After > > more thought, the whole idea of mixing -O0 and LTO seems wr

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640284, @probinson wrote: > > > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#640170, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. Basically, I don't see why having clang always emit a real .o at -O0 would be a problem. I haven't gotten through the other-CFI documentation yet though. https://reviews.llvm.org/D28404 ___ cfe-commits mailing list cfe-co

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640362, @probinson wrote: > In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > > > I don't follow: IMO if I generate a module with optnone and pipe it to `opt > > -O3` I expect no function IR to be touched. If it is not

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640588, @mehdi_amini wrote: > Actually, as mentioned before, I could be fine with making `O0` incompatible > with LTO, however security features like CFI (or other sort of whole-program > analyses/instrumentations) requires LTO. We

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#640682, @mehdi_amini wrote: > > I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds > > welcome) which would set the default for the pragma to 'off'. How is that > > different than what you wanted for `-O0`? It i

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#641078, @chandlerc wrote: > For me, the arguments you're raising against -O0 and -flto don't hold up on > closer inspection: > > - O0 != optnone: correct. But this is only visible in LTO. And in LTO, Os != > optsize, and Oz != minsiz

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote: > As I stand right now, there hasn't been any correction. > I still consider the fact that `optnone` wouldn't produce the "same" result > (modulo corner cases around `merging global variables` for instanc

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote: > If we want to support `-O0 -flto` and `optnone` it the way to convey this to > the optimizer, I don't see the alternative. optsize != -Os (according to Chandler) minsize != -Oz (according to Chandler) o

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. I guess I'm getting irritated because people are trying to tell me what optnone means. I know what it means; I spent probably a whole year pushing to get it adopted. Optnone means: When you are running optimizations, try not to optimize this part, if you can. That'

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#641757, @chandlerc wrote: > % ag OptimizeNone lib/Transforms/IPO > lib/Transforms/IPO/ForceFunctionAttrs.cpp > 47: .Case("optnone", Attribute::OptimizeNone) This is implementing a debugging option, not skipping a pass. >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. @rsmith could you say whether it seems reasonable to have a LangOpts flag that basically means "`pragma clang optimize off` is always in effect." I think it would make the other optnone-related logic simpler. It would not be the only sort-of-codegen-related flag in

[PATCH] D28503: Documentation for the newly added x86 intrinsics.

probinson added inline comments. Comment at: emmintrin.h:1607 +/// +/// This intrinsic corresponds to the VMOVSD / MOVSD instruction. +/// should this be VMOVQ/MOVQ instead? Repository: rL LLVM https://reviews.llvm.org/D28503 ___

[PATCH] D28620: Guard __gnuc_va_list typedef

probinson created this revision. probinson added a reviewer: yaron.keren. probinson added a subscriber: cfe-commits. https://reviews.llvm.org/D28620 Files: lib/Headers/stdarg.h test/Headers/stdarg-gnuc_va_list.c Index: test/Headers/stdarg-gnuc_va_list.c =

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

probinson added subscribers: cfe-commits, probinson. probinson added a comment. Hi David! As this is a Clang patch, you should subscribe cfe-commits rather than llvm-commits. I've done that for you. See also inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +I

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

probinson accepted this revision. probinson added a reviewer: probinson. probinson added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +InitExpr = + DBuilder.createConstantValueExpression(Init.getFloat

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

probinson added a comment. As dblaikie said in email, probably better to make this X86-specific; if long-double varies by OS you can put in a specific triple. FTR we don't rely on Perl being available everywhere, anything that does this kind of scripty stuff uses Python. Com

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

probinson added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:350 + std::string Checksum; + SM.getChecksumMD5(SM.getFileID(Loc), Checksum); + rnk wrote: > We should only do this if `CGM.getCodeGenOpts().EmitCodeView`, or we will > regress compile ti

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27597#621618, @dgross wrote: > So would a Python equivalent of the Perl be acceptable? I think this is an > academic question -- better to explicitly test m

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#675687, @chandlerc wrote: > In https://reviews.llvm.org/D28404#675616, @mehdi_amini wrote: > > > We're still waiting for @rsmith to comment whether it'd be better to `have > > a LangOpts flag that basically means "pragma clang optimiz

[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 membe

[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 pr

[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] 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] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

probinson added a reviewer: jmorse. probinson added a subscriber: jmorse. probinson added a comment. + @jmorse who is better placed than I am to say whether this is what Sony would prefer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106084/new/

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

probinson added a comment. > (hence the renaming of "limited" a long time ago to "standalone-debug" to > create a policy/philosophy around what goes in each category). Sorry, what? I thought -fstandalone-debug meant FullDebugInfo, and AFAICT that's still how the driver handles it? Repository

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

probinson added a comment. > Wouldn't the current "limited" behavior have problems for this shared > libraries situation too? Sounds like in that case -fstandalone-debug should > be used. @jmorse am I remembering correctly, that we require dllimport-style annotations, so "limited" actually inc

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

probinson added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1636 +if (Opts.getDebugInfo() == codegenoptions::DebugInfoConstructor) + Opts.setDebugInfo(codegenoptions::LimitedDebugInfo); No... you want to check both options in

[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 draf

[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 belie

[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 ___ cfe-commits

[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 functions

[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 `-fstandalone-debug

[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 `DebuggerKind::Default

[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 here

[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 or

[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 cm

[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 eventual

[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 nece

[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] 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 an

[PATCH] D145271: [MSVC compatibility][DLLEXPORT/DLLIMPORT] Allow dllexport/dllimport for local classes

probinson added a subscriber: cfe-commits. probinson added a project: clang. probinson added a comment. I've looked at this but I'd like someone more in tune with MSVC behavior to review as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145271/new/ https://reviews.llvm.org/D145271

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

probinson added a comment. See D145173 for a different tactic to solve this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143745/new/ https://reviews.llvm.org/D143745 ___ cfe-commits mailing list cfe-commi

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

probinson abandoned this revision. probinson added a comment. I think the GC behavior with explicit section names is currently a little peculiar. For functions without a section name, -ffunction-sections allows GC to happen at the individual function level. With a section name, GC would happen

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1558 + EmitSeparator = FieldIt->isBitField(); + } + I might not be following this correctly, but it feels like EmitSeparator will end up true if the last field is a bitfield, ev

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

probinson added a comment. Is it possible you need to look only at the immediately preceding field, and not iterate? For example, struct zero_bitfield { char a : 8; char : 0; char b : 8; char c : 8; }; If processing `b` sees the zero-length bitfield and does the needful, the

[PATCH] D146802: [Documentation] improved documentation of diagnostic messages by explaining thier syntax and test of clang by telling which subobject is uninitialized

probinson added a comment. You are combining documentation of the syntax for defining diagnostics, and changes to the content of certain diagnostics. The LLVM project wants to see one topic per patch, so these things need to be done separately. Also, the documentation probably does not want to

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

probinson added subscribers: wolfgangp, probinson. probinson added a comment. This is pretty different from the "always desugar to the canonical type" habit that has always been in place. Sony has done some downstream things to try to work around that in the past. @wolfgangp will remember it bet

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

probinson added a comment. Still one question, and haven't dug into the test in detail yet. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1552 + + auto *PreviousMDEntry = + PreviousFieldsDI.empty() ? nullptr : PreviousFieldsDI.back(); Maybe a comment here

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. One entirely optional suggestion on the test. LGTM. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1563 + + assert(PreviousBitfield->isBitField()); + j

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

probinson added a comment. In D141824#4229953 , @argentite wrote: > Just to confirm, `UNSUPPORTED: target=x86_64-scei-ps4` should be enough, > right? `UNSUPPORTED: target={{.*-(ps4|ps5)}}` please. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

probinson added a comment. In D141824#4231372 , @probinson wrote: > In D141824#4229953 , @argentite > wrote: > >> Just to confirm, `UNSUPPORTED: target=x86_64-scei-ps4` should be enough, >> right? > > `UNSUPPORT

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

probinson added subscribers: debug-info, probinson. probinson added a comment. I think we cannot be 100% sure about source paths in a cross-compile situation. Cross-compiling on platform A targeting platform B does not mean your sources and debugger UI are on platform B. My users keep source and

[PATCH] D147461: [Headers] Add some intrinsic function descriptions to immintrin.h

probinson created this revision. probinson added reviewers: RKSimon, pengfei. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D147461 Files: clang/lib/Headers/immintrin.h Index: clang/lib/Headers/immintrin.h ===

[PATCH] D147461: [Headers] Add some intrinsic function descriptions to immintrin.h

probinson added a comment. FTR, I'll be working my way through a bunch of intrinsics over the next month or so, trying not to do too many at once. Mostly AVX2 but also some others. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147461/new/ https://reviews.llvm.org/D147461 _

[PATCH] D147461: [Headers] Add some intrinsic function descriptions to immintrin.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa82170fa41ca: [Headers] Add some intrinsic function descriptions to immintrin.h. (authored by probinson). Herald added a project: clang. Changed pri

[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

probinson added a comment. An LLVM code change should be testable on its own; this has it tested by Clang. I think you need a new command-line option to set TargetOptions::UseTargetPathSeparator e.g. via llvm-mc. Other TargetOptions are handled this way. Repository: rG LLVM Github Monorepo

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

probinson created this revision. probinson added reviewers: RKSimon, pengfei, goldstein.w.n. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D148021 Files: clang/lib/Headers/fmaintrin.h Index: clang/lib/Headers/fmaintrin.h

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

probinson added inline comments. Comment at: clang/lib/Headers/fmaintrin.h:22 +/// Computes a multiply-add of 128-bit vectors of [4 x float]. +///For each element, computes (__A * __B) + __C . +/// pengfei wrote: > We are using a special format to describute

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0905c567f0c7: [Headers][doc] Add FMA intrinsic descriptions (authored by probinson). Herald added a project: clang. Changed prior to commit: https

[PATCH] D148021: [Headers][doc] Add FMA intrinsic descriptions

probinson added a comment. I chose to leave the "for each element" cases as-is, but I will keep your comments in mind as I go through other intrinsics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148021/new/ https://reviews.llvm.org/D148021 ___

[PATCH] D148653: [Header][doc] Add/revise MONITOR/MWAIT[X] descriptions

probinson created this revision. probinson added reviewers: RKSimon, pengfei, goldstein.w.n, craig.topper. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D148653 Files: clang/lib/Headers/mwaitxintrin.h clang/lib/Headers/pmmintrin.h Index:

[PATCH] D148653: [Header][doc] Add/revise MONITOR/MWAIT[X] descriptions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5ddcef2ad3db: [Headers][doc] Add/revise MONITOR/MWAIT descriptions (authored by probinson). Herald added a project: clang. Repository: rG LLVM Git

[PATCH] D148653: [Header][doc] Add/revise MONITOR/MWAIT[X] descriptions

probinson added inline comments. Comment at: clang/lib/Headers/pmmintrin.h:278 ///the monitor event pending state. Data stored in the monitored address ///range causes the processor to exit the pending state. /// goldstein.w.n wrote: > interrupts too.

[PATCH] D149205: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

probinson created this revision. probinson added reviewers: pengfei, RKSimon, goldstein.w.n, craig.topper. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D149205 Files: clang/lib/Headers/avx2intrin.h Index: clang/lib/Headers/avx2intrin.h

[PATCH] D149205: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

probinson marked 2 inline comments as done. probinson added inline comments. Comment at: clang/lib/Headers/avx2intrin.h:942 +/// +/// \code +/// FOR element := 0 to 1 pengfei wrote: > Use `\code{.operation}` please, the same below. Our internal tool will > recog

[PATCH] D149205: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. probinson marked an inline comment as done. Closed by commit rG039ae62405b6: [Headers][doc] Add "gather" intrinsic descriptions to avx2intrin.h (authored by probinson).

[PATCH] D150114: [Headers][doc] Add "add/sub/mul" intrinsic descriptions to avx2intrin.h

probinson created this revision. probinson added reviewers: pengfei, RKSimon, goldstein.w.n, craig.topper. Herald added a project: All. probinson requested review of this revision. https://reviews.llvm.org/D150114 Files: clang/lib/Headers/avx2intrin.h Index: clang/lib/Headers/avx2intrin.h

[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] 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] 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 https://reviews

[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 https://list

[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 `llvm/lib/Support/H

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson created this revision. probinson added a reviewer: ioeric. probinson requested review of this revision. The preceding EXPECT_EQ was never executed; modifying the test input made that happen. Found by the Rotten Green Tests project. https://reviews.llvm.org/D119040 Files: clang/unit

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson added a comment. By "the EXPECT_EQ was never executed" I mean, replacing it with `assert(false);` does not crash the test. The string `"a::b::Foo"` was never seen by the Visitor. Maybe this indicates some much more subtle, deeper problem; I don't know. This change does cause the EX

[PATCH] D118471: [clang][Lexer] Make raw and normal lexer behave the same for line comments

probinson added inline comments. Comment at: clang/unittests/Lex/LexerTest.cpp:654 + while (!L.LexFromRawLexer(T)) { +ASSERT_TRUE(!ToksView.empty()); +EXPECT_EQ(T.getKind(), ToksView.front().getKind()); @kadircet @sammccall It turns out this while loop

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson added a reviewer: kadircet. probinson added a subscriber: kadircet. probinson added a comment. Ping; +@kadircet CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119040/new/ https://reviews.llvm.org/D119040 ___ cfe-commits mailing list

[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 Mon

[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 https://reviews.llvm.org/D1

[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 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 http

[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 mailing

[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 `-aix`

[PATCH] D103131: support debug info for alias variable

probinson added a comment. In D103131#2789493 , @probinson wrote: >> Mixed feelings - somewhat in favor of "do the thing that's probably already >> fairly tested/known to work" (GCC's thing). But open to the idea that that >> approach has problems, for

[PATCH] D103131: support debug info for alias variable

probinson added a comment. Of course, if it turns out that gdb can't handle the imported_declaration, we might end up having to do this two different ways under the tuning option. I'd *really* prefer not to do that though, and I'd argue it's a gdb bug if it cannot understand imported_declarati

[PATCH] D103131: support debug info for alias variable

probinson added a comment. In D103131#2791881 , @dblaikie wrote: > In D103131#2789493 , @probinson > wrote: > >>> Mixed feelings - somewhat in favor of "do the thing that's probably already >>> fairly tested/kno

[PATCH] D103131: support debug info for alias variable

probinson added a comment. In D103131#2792364 , @dblaikie wrote: > I will say, as the person who implemented DW_TAG_imported_declaration support > in Clang - it's a bit not great/unfortunate (in part because we currently > have to emit them for all usin

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson added a reviewer: hokein. probinson added a subscriber: hokein. probinson added a comment. Herald added a project: All. + @hokein who has done work in the one place where `replaceNestedName` is used. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119040/new/ https://reviews.ll

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

probinson added a comment. In D111587#3381369 , @keith wrote: > Fix tests with dwarf 6 Do you mean dwarf 5 here? There is no v6 yet. Comment at: clang/test/Modules/module-debuginfo-prefix.m:24 -// Dir should always be empty, but on

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

probinson added a comment. In D111587#3382834 , @keith wrote: > I actually mean dwarf 6, which appears to be partially implemented according > to https://lists.llvm.org/pipermail/llvm-dev/2020-April/141055.html > > I discovered the issue from the failed

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson abandoned this revision. probinson added a comment. In D119040#3363406 , @kadircet wrote: > Thanks for the change, but the test is actually checking for rename in > presence of using-decls. I beleive https://reviews.llvm.org/D121103 is the > p

[PATCH] D111587: re-land: [clang] Fix absolute file paths with -fdebug-prefix-map

probinson added a comment. In D111587#3383684 , @keith wrote: > You're right it's version 5 not 4, maybe the issue is that some platforms > (like macOS) are defaulting to 4 intentionally for now? I guess I thought 6 > because passing 6 also reproduces,

[PATCH] D118850: [PS4] Make __BIGGEST_ALIGNMENT__ 32bytes

probinson accepted this revision. probinson added a comment. LGTM, sorry I missed this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118850/new/ https://reviews.llvm.org/D118850 ___ cfe-commits mailing

<    1   2   3   4   5   6   >