[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. In D153156#4600270 , @rupprecht wrote: > of which there's an example in LLVM itself: > https://github.com/llvm/llvm-project/blob/6a0e536ccfef1f7bd64ee4153b4efc0aeecf28d4/clang/test/SemaCXX/cxx11-compat.cpp#L38 Sorry, I don't

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. The other common breakage I'm seeing is code that hasn't yet migrated from the "PRI" format macros, of which there's an example in LLVM itself: https://github.com/llvm/llvm-project/blob/6a0e536ccfef1f7bd64ee4153b4efc0aeecf28d4/clang/test/SemaCXX/cxx11-compat.cpp#L38

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

rupprecht added a comment. In D153156#4599106 , @aaron.ballman wrote: > In D153156#4598988 , @rZhBoYao > wrote: > >> In D153156#4598915 , >> @steelannelida wrote: >>

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

rupprecht added a comment. In D154014#4457883 , @MaskRay wrote: >> This is a breaking change since some SCLs might use .* or (abc|def) which >> are supported regexes but not valid globs. Since we have just cut clang 16.x >> this is a good time to make

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

rupprecht added a comment. In D150226#4401188 , @jyknight wrote: > In D150226#4400782 , @rupprecht > wrote: > >> As a general question/feature request: is there a way to have specific >> warnings apply even for

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

rupprecht added a comment. In D150226#4353907 , @aaron.ballman wrote: > In D150226#4353863 , @jyknight > wrote: > >> When looking for errors in existing codebases, don't forget that this >> diagnostic is

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

rupprecht added a comment. I found one use here: https://github.com/ericniebler/range-v3/blob/48dc2eb666c07e6afc8ec5edf7db9a5c6cde7a56/include/range/v3/functional/invoke.hpp#L51 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150875/new/ https://reviews.llvm.org/D150875

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

rupprecht added a comment. In D150226#4338264 , @shafik wrote: > In D150226#4336755 , @rupprecht > wrote: > >> We're still using `-Wno-enum-constexpr-conversion`, although I'm not sure if >> we need that or if

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

rupprecht added a comment. We're still using `-Wno-enum-constexpr-conversion`, although I'm not sure if we need that or if we just forgot to remove it after doing some cleanup. I'm trying it out now. (Sorry, I'm not sure we were aware that having a way to turn this off was just temporary).

[PATCH] D146987: [Assignment Tracking] Enable by default

rupprecht added a comment. In D146987#4263048 , @MaskRay wrote: > The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d > still > caused some issues to our internal workloads.

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

rupprecht added a comment. In D144285#4163004 , @cor3ntin wrote: > If however we find this change to disruptive, we should inform WG21. Thanks for the explanation, I think I understand the issue now. I got totally thrown off by the title -- it's not

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

rupprecht added a comment. Here's one change this patch causes on "real" code (invalid code, but something a user might try to compile): we see is a static_assert in gmock that now fails to report a useful error message: https://godbolt.org/z/sPr1PYT9d Previously we saw `error: static

[PATCH] D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process

rupprecht added a comment. This has already been reverted, but I found a breakage (not a crash) caused by this: #include class Base { protected: int member; }; template struct Subclass : public Parent { static_assert(std::is_base_of::value, "Parent

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

rupprecht accepted this revision as: rupprecht. rupprecht added a comment. All the things that were broken before are no longer broken, so LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

rupprecht added a comment. In D124351#4102679 , @aaron.ballman wrote: > In D124351#4102634 , @eaeltsin > wrote: > >>> Looks like we fail to enter the appropriate context somewhere (my guess is >>> that it

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

rupprecht added a comment. Hi, me again :) I ran into an interesting build breakage from this, I can't tell if it's a legitimate breakage based on reading P2036R3 and P2579R0 (mostly I'm not a language lawyer). struct StringLiteral { template StringLiteral(const char ()[N])

[PATCH] D142449: [clang] Fix linking to LLVMTestingAnnotations in standalone build

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142449/new/ https://reviews.llvm.org/D142449 ___ cfe-commits mailing list

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3432f4bf86e7: [test] Split out Annotations from `TestingSupport` (authored by rupprecht). Changed prior to commit:

[PATCH] D141175: [test] Split out Annotations from `TestingSupport`

rupprecht updated this revision to Diff 488381. rupprecht added a comment. - Remove redundant comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141175/new/ https://reviews.llvm.org/D141175 Files:

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

rupprecht added a comment. In D141175#4038017 , @GMNGeoffrey wrote: > It seems like the same logic would extend to the CMake build. Could we make > the same change there? The simplest (only?) way to do that is to have it literally in a separate

[PATCH] D141175: [bazel] Split out Annotations from `TestingSupport`

rupprecht updated this revision to Diff 488366. rupprecht added a comment. Herald added subscribers: cfe-commits, kadircet, arphaman, hiraditya. Herald added projects: clang, clang-tools-extra. - Move annotations to a separate package entirely Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4024628 , @rupprecht wrote: > I still see one behavior change (actually it was there before, but I missed > it in the test results), but as far as I can tell, it's a good one? If I > reduce it too much, I get the

[PATCH] D136554: Implement CWG2631

rupprecht accepted this revision. rupprecht added a comment. I still see one behavior change (actually it was there before, but I missed it in the test results), but as far as I can tell, it's a good one? If I reduce it too much, I get the warning with the baseline toolchain, so it's not

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. I threw this at the "test everything" test (some millions of targets) and it found only one breakage, so this is very very close. Without further ado, here is this silly looking thing: File `blah.h`: #include #include template struct Base { virtual

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. I'm not sure what to make of the new failure when I try it out this time. Given a source like this: #include struct Options { std::function identity = [](bool x) -> bool { return x; }; }; struct Wrapper { explicit Wrapper(const Options& options

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4018870 , @rupprecht wrote: > All good now! The latest revision of this patch doesn't seem to break > anything, unless I ran our tests wrong. From my perspective this is OK to > reland now. ... and yep, I was

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. All good now! The latest revision of this patch doesn't seem to break anything, unless I ran our tests wrong. From my perspective this is OK to reland now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. In D136554#4013463 , @rupprecht wrote: > Glad the test case made sense to you, it was convoluted to me :) > > Still seeing one more error, and it's not modules-related so I might be able > to get it reduced today. Generally,

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Glad the test case made sense to you, it was convoluted to me :) Still seeing one more error, and it's not modules-related so I might be able to get it reduced today. Generally, it looks like this: struct Inner { Foo& foo; const std::unique_ptr<...> x =

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Sorry for the delay, I was out on vacation for a bit. I have a repro for this new issue now: F25778542: repro.tar.gz $ CLANG=~/dev/clang ./repro.sh ++ dirname /tmp/repro/repro.sh + DIR=/tmp/repro + : /tmp/D136554 + rm -rf

[PATCH] D139741: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

rupprecht added a comment. In D139741#4000201 , @rupprecht wrote: > I'm not sure what the libcxx failure was that caused you to revert this, but > we also saw a clang crasher as a result of this. `clang/lib/AST/Decl.cpp:4300 > in unsigned int

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. I applied this version of the patch and the crash is now gone  However, now I get this inexplicable error -- I'm not entirely sure it's related, maybe I'm holding it wrong: In module '': foo.h$line:$num: error: 'foo::FooClass' has different definitions in

[PATCH] D140058: [clang-format][NFC] Turn on some code-changing options one by one

rupprecht added a comment. In D140058#4000311 , @rupprecht wrote: > This is causing a test failure: > https://buildkite.com/llvm-project/upstream-bazel/builds/48607#0185190c-43f8-43ff-b8bd-fa8ce0b6e2f5 > (and likewise running `ninja check-clang-unit`

[PATCH] D140058: [clang-format][NFC] Turn on some code-changing options one by one

rupprecht added a comment. This is causing a test failure: https://buildkite.com/llvm-project/upstream-bazel/builds/48607#0185190c-43f8-43ff-b8bd-fa8ce0b6e2f5 (and likewise running `ninja check-clang-unit` locally, but I don't have a buildbot link to that) Comment at:

[PATCH] D139741: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields

rupprecht added a comment. I'm not sure what the libcxx failure was that caused you to revert this, but we also saw a clang crasher as a result of this. `clang/lib/AST/Decl.cpp:4300 in unsigned int clang::FieldDecl::getBitWidthValue(const ASTContext &) const: isBitField() && "not a bitfield"`.

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Here's a well-formed reproducer: struct MyStringView { MyStringView(const char *); }; struct SourceLocation { static SourceLocation current(const char * = __builtin_FILE(), unsigned int = __builtin_LINE()); };

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Actually, that assertion failure is pre-existing. However, this is newly failing in a no-asserts clang, so I wonder if something about this patch is just surfacing an existing bug in clang. Anyway, I hope to have a better repro by EOD. Repository: rG LLVM Github

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Ugh, I left cvise running overnight and forgot to include the validity check by building with a previous clang, so my reduction is invalid. I'm going to run it again, but here's the invalid crasher in the meantime: struct SourceLocation { static SourceLocation

[PATCH] D136554: Implement CWG2631

rupprecht added a comment. Looks like the latest reland still has some issue remaining. With asserts enabled, I get: `assert.h assertion failed at clang/include/clang/AST/Type.h:752 in const ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const: !isNull() && "Cannot retrieve a NULL

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. In D129755#3869081 , @aaronpuchert wrote: > In D129755#3866887 , @rupprecht > wrote: > >> I might have a better answer in a day or two of how widespread this is >> beyond just the

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. In D129755#3866213 , @aaronpuchert wrote: > Are you seeing warnings because of the different treatment of copy-elided > construction, or because we've started to consider `CXXConstructorCall`s > outside of the initializer of

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

rupprecht added a comment. I'm seeing a fair number of breakages from this patch (not really sure how many we truly have, I've hit ~5-10 so far in widely used libraries, but I suspect we have far more in the long tail). They're all valid (most are just adding missing thread safety annotations,

[PATCH] D112732: [ASan] Process functions in Asan module pass

rupprecht added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1189 +MPM.addPass(ModuleAddressSanitizerPass( +CompileKernel, Recover, ModuleUseAfterScope, UseOdrIndicator, +DestructorKind, UseAfterScope, UseAfterReturn));

[PATCH] D112732: [ASan] Process functions in Asan module pass

rupprecht added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1189 +MPM.addPass(ModuleAddressSanitizerPass( +CompileKernel, Recover, ModuleUseAfterScope, UseOdrIndicator, +DestructorKind, UseAfterScope, UseAfterReturn));

[PATCH] D104261: Thread safety analysis: Always warn when dropping locks on back edges

rupprecht added a comment. In D104261#2844636 , @aaronpuchert wrote: > In D104261#2841356 , @delesley > wrote: > >> since it's restricted to relockable managed locks, I'm not too worried... > > Not quite, it

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2788596 , @spatel wrote: > In D101191#2783570 , @rupprecht > wrote: > >> In D101191#2782963 , @rupprecht >> wrote: >> >>> The

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2782963 , @rupprecht wrote: > The issue I'm seeing seems more directly caused by SLP vectorization, as it > goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad > optimization AFAICT. Filed as

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. The issue I'm seeing seems more directly caused by SLP vectorization, as it goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad optimization AFAICT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. FYI, I'm seeing what I think is a miscompile that bisects to this patch. Greatly simplified, the problematic snippet is this: struct Stats { int a; int b; int a_or_b; }; bool x = ... bool y = ... Stats stats; stats.a = x ? 1 : 0; stats.b =

[PATCH] D100776: [clang/Basic] Make TargetInfo.h not use DataLayout again

rupprecht added a comment. Landed 44e2247dcd04f3421164b085094eb575270564ba to fix LLDB. If you decide to go in a different direction again, please adjust that fix accordingly. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D99414: [clang][tooling] Create SourceManager for DiagnosticsEngine before command-line parsing

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Thanks, the error message is much better now! When applying this patch locally and also undoing the fix in our internal tool (i.e. change it to `-ferror-limit=-1`), the error I get is:

[PATCH] D98980: [CodeGen] Don't crash on for loops with cond variables and no increment

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98980/new/ https://reviews.llvm.org/D98980

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

rupprecht added a comment. I tried creating an IR repro, but: running `-S -emit-llvm` with the old PM crashes before it can generate anything, and running with the new PM seems to generate invalid IR (branches to %for.inc, which does not exist). I suspect this is not an optimizer bug, but

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

rupprecht added a comment. The error message I posted earlier was when using O1 + new PM, but it crashes with the old one & no optimizations: $ cat /tmp/repro.cc void a() { for (; int b = 0;) continue; } $ clang -c /tmp/repro.cc -o

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

rupprecht added a comment. We're seeing a crash in the optimizer after this patch, with the following logged in assert builds: `assert.h assertion failed at llvm/include/llvm/Support/Casting.h:104 in static bool llvm::isa_impl_cl::doit(const From *) [To = llvm::InvokeInst, From = const

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

This revision was automatically updated to reflect the committed changes. Closed by commit rG1a368ae3b78d: [CUDA] fix builtin constraints for PTX 7.2 (authored by tra, committed by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97009/new/

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

rupprecht added a comment. This fixes a build error we're seeing, so I'd like to land this in a bit if that's OK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97009/new/ https://reviews.llvm.org/D97009

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

rupprecht added a comment. I ran into this commit when integrating commits today (specifically, 97100646d1b4526de1eac3aacdb0b098739c6ec9 ) -- there's nothing wrong with this patch AFAICT, but I'm wondering if the error

[PATCH] D94468: [test] Demonstrate bad error message

rupprecht created this revision. rupprecht requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This should not be committed, but demonstrates an issue with cascading errors after D84673 . Repository: rG

[PATCH] D87194: Thread safety analysis: Use access specifiers to decide about scope

rupprecht added a comment. In D87194#2369485 , @aaronpuchert wrote: > @rupprecht, maybe you can try it again? Some more interesting errors this time :) The ones I originally saw look correct now (i.e. it's flagging the things that are valid, but not

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. In D84604#2364568 , @aaronpuchert wrote: > In D84604#2363768 , @rupprecht wrote: > >> I applied D87194 locally and rebuilt the >> original source, and

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. In D84604#2363445 , @aaronpuchert wrote: > Pushed a fix in rGbbed8cfe80cd27d3a47d877c7608d9be4e487d97 > . For > now we just consider all static members as

[PATCH] D84604: Thread safety analysis: Consider global variables in scope

rupprecht added a comment. I'm seeing failures which I think are due to this patch -- I don't have a nice godbolt repro yet, but it's something like: foo.h: class Foo { public: static void DoStuff(); // Grabs mu_ private: static std::vector blah_ GUARDED_BY(mu_);

[PATCH] D88640: [Format] Don't treat compound extension headers (foo.proto.h) as foo.cc main file.

rupprecht added a comment. It looks like this fix caused a different regression in not accepting `name..h` as the main header for `name..cc`, e.g.: $ cat /tmp/foo.bar.cc #include "a.h" #include "z.h" #include "foo.bar.h" $ clang-format /tmp/foo.bar.cc # Before #include

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

rupprecht added a comment. Thanks for the revert. In addition to the one eabi test, we saw widespread msan use-of-uninitialized-value errors from here: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/ARM/ARMTargetMachine.cpp#L229. I think it would explain the eabi test

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

This revision was automatically updated to reflect the committed changes. Closed by commit rG60a8a504f16d: [llvm-objdump] Print file format in lowercase to match GNU output. (authored by rupprecht). Changed prior to commit: https://reviews.llvm.org/D74433?vs=243991=244189#toc Repository: rG

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

rupprecht updated this revision to Diff 243991. rupprecht marked an inline comment as done. rupprecht added a comment. - Use StringRef::lower() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74433/new/ https://reviews.llvm.org/D74433 Files:

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

rupprecht added a comment. In D74433#1870647 , @MaskRay wrote: > Wait. I wonder whether we can change llvm-readobj to use lower case names as > well. The following should be updated: > > StringRef ELFObjectFile::getFileFormatName() const { > bool

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

rupprecht created this revision. rupprecht added reviewers: MaskRay, jhenderson. Herald added subscribers: llvm-commits, cfe-commits, kerbowa, nhaehnle, jvesely. Herald added a reviewer: alexshap. Herald added projects: clang, LLVM. GNU objdump prints the file format in lowercase, e.g.

[PATCH] D74070: [Clang] Don't let gen crash diagnostics fail when '#pragma clang __debug crash' is used

rupprecht added a comment. FYI, temporarily reverted this in fafddbd956dbe439787f6d717c247e648bb07ff5 since it was causing failures in `Clang :: Driver/crash-report.c`. I didn't see the failure on all buildbots (otherwise I

[PATCH] D74076: [Clang][Driver] Remove -M group options before generating crash diagnostics

rupprecht added a comment. FYI - reverted this too since I had conflicts when also reverting D74070 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74076/new/ https://reviews.llvm.org/D74076

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

rupprecht added a comment. In D71554#1789360 , @arichardson wrote: > Also handle -h/-v as short options. Does the adjusted test look okay? Sorry, didn't have time to take a second look before the holiday break -- yep, looks good, I didn't anticipate so

[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

rupprecht abandoned this revision. rupprecht added a comment. In D71635#1790611 , @MaskRay wrote: > Before: > > % clang -frounding-math -fsyntax-only -x c /dev/null > clang-10: warning: Support for floating point control option frounding-math > is

[PATCH] D71671: [clang] Remove -Wexperimental-float-control.

This revision was automatically updated to reflect the committed changes. Closed by commit rG553a727f5f64: [clang] Remove -Wexperimental-float-control. (authored by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71671/new/

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added a comment. In D62731#1789122 , @rupprecht wrote: > In D62731#1788902 , @andrew.w.kaylor > wrote: > > > In D62731#1788838 , @rupprecht > > wrote: > > > > >

[PATCH] D71671: [clang] Remove -Wexperimental-float-control.

rupprecht created this revision. rupprecht added reviewers: mibintc, chandlerc, echristo, rjmccall, kpn, erichkeane, rsmith, andrew.w.kaylor. Herald added a project: clang. Herald added a subscriber: cfe-commits. Per D62731 , the behavior of clang with

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht closed this revision. rupprecht added a comment. In D62731#1788902 , @andrew.w.kaylor wrote: > In D62731#1788838 , @rupprecht wrote: > > > It seems the discussion of whether or not this is incomplete

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added a comment. It seems the discussion of whether or not this is incomplete died out -- I'd prefer to assume it is incomplete if there is no consensus. Mailed D71635 to rename `-frounding-math` to `-fexperimental-rounding-math`. Alternatively we

[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

rupprecht created this revision. rupprecht added reviewers: mibintc, chandlerc, echristo, rjmccall, kpn, erichkeane, rsmith, andrew.w.kaylor. Herald added a project: clang. Herald added a subscriber: cfe-commits. D62731 implements an incomplete/experimental

[PATCH] D71554: [llvm-ranlib] Handle -D and -U command line flag

rupprecht accepted this revision. rupprecht added a comment. This revision is now accepted and ready to land. Looks good for D/U, but looks like --help and --version options are also supported as combined short args; do you mind adding those too while you're here? Thanks for the patch!

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">,

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444 +def warn_drv_experimental_fp_control_incomplete_opt : Warning< + "Support for floating point control option %0 is incomplete and experimental">, mibintc

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht marked an inline comment as done. rupprecht added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13047 + F->setUsesFPIntrin(true); + printf("Enclosing function uses fp intrinsics\n"); +} rupprecht wrote: > Looks like this is

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

rupprecht added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13047 + F->setUsesFPIntrin(true); + printf("Enclosing function uses fp intrinsics\n"); +} Looks like this is leftover debugging? I'm seeing log spam compiling some files -- this

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. Just wanna say huge +1 for this patch. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67122/new/ https://reviews.llvm.org/D67122 ___ cfe-commits mailing list

[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

rupprecht planned changes to this revision. rupprecht added a comment. Note: herald added reviewers, but this patch is just to provide context. I'll send the real patches for review in the coming days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

rupprecht created this revision. Herald added subscribers: llvm-commits, cfe-commits, seiya, arphaman, jakehehrlich, aheejin, arichardson, sbc100, emaste. Herald added a reviewer: espindola. Herald added a reviewer: alexshap. Herald added a reviewer: jhenderson. Herald added projects: clang,

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. There's definitely a lot of new findings this creates, but it's hard to say exactly how many root causes there are due to the way test failures are (not) grouped well in the way I'm testing. So far they all seem like true positives, so this would be good to submit.

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. > But TLDR, either the fix in https://github.com/google/filament/pull/1566 > is incorrect and the actually-bad code is elsewhere, > or you have some other unsanitized UB elsewhere. Could be both :S My money is on "both" :p I tested a random sample of a couple

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

rupprecht added a comment. > Still think this looks good. Have you tried running this on the llvm test > suite, or some other interesting corpus? Would be curious to see any pre/post > patch numbers. I finally had time this morning to patch this in and give it a shot. (Sorry for the delay...

[PATCH] D66490: [NewPM] Enable the New Pass Manager by Default in Clang

rupprecht added a comment. We already know that we don't want this enabled for tsan builds due to https://bugs.llvm.org/show_bug.cgi?id=42877, but I don't even know if anyone else will hit it (it's only when building one particular library). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

This revision was automatically updated to reflect the committed changes. Closed by commit rG73986707bd57: [CodeGen][test] Use FileCheck variable matchers for better test support (authored by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht added a comment. Sounds good, changed to use variable matching instead. This passes w/ either `-fno-discard-value-names` or `-fdiscard-value-names` used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63625/new/

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht updated this revision to Diff 205912. rupprecht added a comment. - Use filecheck variable matching instead of an explicit -fno-discard-value-names option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63625/new/

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

rupprecht created this revision. rupprecht added reviewers: rnk, akhuang, aprantl. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depending on how clang is built, it may discard the IR names and use names like `%2` instead of `%result.ptr`, causing tests that rely on the

[PATCH] D60974: Clang IFSO driver action.

rupprecht added a reviewer: rupprecht. rupprecht added a comment. Can you upload this patch with context? Either use arc or upload w/ -U9 I seem to have a lot of comments, but they're all nits -- my major concern being the `llvm_unreachable`s should be errors instead (i.e. should be

[PATCH] D61689: Change -gz and -Wa,--compress-debug-sections to use gABI compression (SHF_COMPRESSED)

rupprecht added a comment. So, while I think this is an //entirely// reasonable assumption in most cases, it's also not one that provides any kind of workaround for the few cases where it's not universally true. - As mentioned in the patch, this effectively changes the default from

[PATCH] D60974: Clang IFSO driver action.

rupprecht added a comment. I didn't follow the technical details, but I don't see anything wrong with moving forward on this patch. I think this seems like an interesting idea worth experimenting with. In D60974#1488563 , @jakehehrlich wrote: > >

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

This revision was automatically updated to reflect the committed changes. Closed by commit rC356614: [clang][OpenMP] Fix build when using libgomp (authored by rupprecht, committed by ). Changed prior to commit: https://reviews.llvm.org/D59609?vs=191567=191577#toc Repository: rC Clang

[PATCH] D59609: [clang][OpenMP] Fix build when using libgomp

rupprecht added a comment. In D59609#1436975 , @lebedev.ri wrote: > Interesting. > What happens if libomp is not being built? > What happens if libomp is not present? I'm far from an omp expert (only running across this because some tests failed),

  1   2   >