[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

manojgupta added a comment. In D150226#4342516 , @aaron.ballman wrote: > In D150226#4340166 , @manojgupta > wrote: > >> https://github.com/bminor/binutils-gdb/blob/master/include/diagnostics.h >> >> gdb only sup

[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

manojgupta added a comment. https://github.com/bminor/binutils-gdb/blob/master/include/diagnostics.h gdb only suppresses the warning. So this patch will likely break gdb. As per commit: https://github.com/bminor/binutils-gdb/commit/ae61525fcf456ab395d55c45492a106d1275873a Since the current c

[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

manojgupta added a comment. In D150226#4338738 , @manojgupta wrote: >> I was under the impression from https://github.com/boostorg/mpl/issues/69 >> that this was fixed but there are a number of issues off of the main one and >> maybe I am confused. > >

[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

manojgupta added a comment. > I was under the impression from https://github.com/boostorg/mpl/issues/69 > that this was fixed but there are a number of issues off of the main one and > maybe I am confused. Seems like boost 1.81 has the mentioned fix. I can try it and see if the warning still f

[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

manojgupta added a comment. We also use Wno-enum-constexpr-conversion in ChromeOS. There are many packages that break with this warning. One of them is boost which is used in many other packages. The errors in boost were: ./boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 i

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

manojgupta added a comment. Thanks, sent a patch to gdb at https://sourceware.org/pipermail/gdb-patches/2023-April/198870.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144331/new/ https://reviews.llvm.org/D144331

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

manojgupta added a comment. Hmm, looking deeper, gdb (actually binutils), is doing something weird. https://github.com/bminor/binutils-gdb/blob/master/include/safe-ctype.h /* Prevent the users of safe-ctype.h from accidently using the routines from ctype.h. Initially, the approach was to

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

manojgupta added a comment. I have opened a bug at issuetracker.google.com/issues/277967395 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144331/new/ https://reviews.llvm.org/D144331 ___ cfe-commits mail

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

manojgupta added a comment. Just a heads up, with this change, we are hitting issues in building gdb. Appreciate any ideas on what is wrong. aarch64-cros-linux-gnu-clang++ -x c++-I. -I. -I./config -DLOCALEDIR="\"/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I../bfd -I./..

[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

manojgupta added a comment. This is firing even in checked length codes, is that expected? example: https://godbolt.org/z/Todje76ao std::optional result; bool ReadDevice(uint8_t* data, size_t len) { if (!result) return false; memset(data, 0, len); if (len > 0) data[0] =

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

manojgupta added a subscriber: rpbeltran. manojgupta added a comment. > If ChromeOS needs time for migration, I think -Xcompiler can be temporarily > ignored. If you can wait for a few weeks, that'd be great. We are already fighting with a large number of ToT issues. And this change makes our b

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

manojgupta added a comment. Here are a few instances of Xcompiler usage for a non-exhaustive search (I can't look inside package tarballs if they are using it ): https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/main/net-vpn/openvpn/openvpn-2.4.4.ebuild#74 https://chromi

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

manojgupta added a comment. Without -Xcompiler, ChromeOS code will break. It may not be supported by GCC but it is supported in some other compilers like Cuda and a few others if you search. Also being supported by libtool makes it more important to keep it working. Repository: rG LLVM Gith

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

manojgupta added a comment. Xlinker still works. Xcompiler is failing. A google search will show that Xcompiler is a wide-spread option used by many packages. Whether or not GCC supports it is not relevant. Please do not remove options just because you do not use them. Repository: rG LLVM G

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

manojgupta added a comment. We use -Xcompiler and -Xlinker which are passed in programs and they raise error now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139717/new/ https://reviews.llvm.org/D139717 _

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

manojgupta added a comment. The removal is also breaking ChromeOS builds which use -Xpattern in some cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139717/new/ https://reviews.llvm.org/D139717 ___

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2497d5aa7716: Define _GNU_SOURCE for arm baremetal in C++ mode. (authored by manojgupta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136712/new/ https://

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta updated this revision to Diff 473004. manojgupta added a comment. Updated test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136712/new/ https://reviews.llvm.org/D136712 Files: clang/lib/Basic/Targets/ARM.cpp clang/test/Preprocesso

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta updated this revision to Diff 472962. manojgupta added a comment. Restore back to C++ only. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136712/new/ https://reviews.llvm.org/D136712 Files: clang/lib/Basic/Targets/ARM.cpp clang/test

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta updated this revision to Diff 472961. manojgupta added a comment. restore back to C++ only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136712/new/ https://reviews.llvm.org/D136712 Files: clang/lib/Basic/Targets/ARM.cpp clang/test/

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta added a comment. @tomhughes has more details on this but if we do not define it in clang itself, we'll need to define it for every package manually at build time since the usage goes deep inside newlib headers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta added reviewers: MaskRay, efriedma. manojgupta added subscribers: MaskRay, efriedma. manojgupta added a comment. Herald added a subscriber: StephenFan. I am not sure who is a good reviewer for this. Starting with @MaskRay and @efriedma Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta updated this revision to Diff 470611. manojgupta added a comment. Removed c++ limitation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136712/new/ https://reviews.llvm.org/D136712 Files: clang/lib/Basic/Targets/ARM.cpp clang/test/D

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

manojgupta created this revision. manojgupta added reviewers: abidh, tomhughes. Herald added subscribers: kristof.beyls, ki.stfu, dschuff. Herald added a project: All. manojgupta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This matches

[PATCH] D134478: BareMetal: detect usr/include/c++/v1 path in sysroot

This revision was automatically updated to reflect the committed changes. Closed by commit rG5e5d21462d1e: BareMetal: detect usr/include/c++/v1 path in sysroot (authored by manojgupta). Changed prior to commit: https://reviews.llvm.org/D134478?vs=465647&id=466250#toc Repository: rG LLVM Gith

[PATCH] D134478: BareMetal: detect usr/include/c++/v1 path in sysroot

manojgupta updated this revision to Diff 465647. manojgupta added a comment. address comments and check CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134478/new/ https://reviews.llvm.org/D134478 Files: clang/lib/Driver/ToolChains/BareMetal.c

[PATCH] D134478: BareMetal: detect usr/include/c++/v1 path in sysroot

manojgupta added a comment. friendly ping for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134478/new/ https://reviews.llvm.org/D134478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D134478: BareMetal: detect usr/include/c++/v1 path in sysroot

manojgupta created this revision. manojgupta added reviewers: MaskRay, barannikov88, abidh. Herald added subscribers: StephenFan, ki.stfu. Herald added a project: All. manojgupta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently bar

[PATCH] D134454: [Driver][Distro] Fix ArchLinux triplet and sysroot detection

manojgupta added a comment. Thanks for the patch. Can you please post a full diff (git diff -U). Adding @MaskRay as a reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134454/new/ https://reviews.llvm.org/D134454 ___

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

This revision was automatically updated to reflect the committed changes. Closed by commit rG06fc5a771462: Driver: Refactor and support per target dirs in baremetal (authored by manojgupta). Changed prior to commit: https://reviews.llvm.org/D131225?vs=451352&id=451517#toc Repository: rG LLVM

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 451352. manojgupta added a comment. Going back to older handlesTarget() style in Baremetal. Apparently RISCV can be both Baremetal or RISCVToolchain for the same tuple. I do not know of the nuances so trying not to disturb that for now. Repository: rG L

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 451325. manojgupta added a comment. Fix clang-format complains. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 Files: clang/lib/Driver/Driver.cpp clang/lib/Dr

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 451222. manojgupta added a comment. Address maskray comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 Files: clang/lib/Driver/Driver.cpp clang/lib/Driv

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta added inline comments. Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:228-237 + case ToolChain::RLT_CompilerRT: { +const std::string fileName = getCompilerRT(Args, "builtins"); +std::string baseName = llvm::sys::path::filename(fileName).str(); +llvm

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 450910. manojgupta added a comment. Address more style lints. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 Files: clang/lib/Driver/Driver.cpp clang/lib/Driv

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 450908. manojgupta added a comment. Address style nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 Files: clang/lib/Driver/Driver.cpp clang/lib/Driver/Too

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 450898. manojgupta added a comment. Herald added a subscriber: hiraditya. Moved defs to Triple.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 Files: clang/l

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:388 + /// IsRISCVBareMetal - Does this tool chain is a riscv baremetal target. + static bool IsRISCVBareMetal(const llvm::Triple &); + barannikov88 wrote: > barannikov88 wrote:

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta updated this revision to Diff 450894. manojgupta added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Moved Baremetal triple related code to Triple.h. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:384 + /// IsBareMetal - Does this tool chain is a baremetal target. + static bool IsBareMetal(const llvm::Triple &); barannikov88 wrote: > Is this a correct sentence? (My Eng

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

manojgupta created this revision. manojgupta added reviewers: MaskRay, abidh, kristof.beyls. Herald added subscribers: luke957, StephenFan, s.egerton, simoncook, ki.stfu. Herald added a project: All. manojgupta requested review of this revision. Herald added subscribers: cfe-commits, pcwang-thead.

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

manojgupta added a comment. Hmm, the commit message says that Wno-error should work but this is not really the case :(. > (they can disable the warning or use -Wno-error to downgrade the > error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Following behavior is also surprising: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Tried locally but I still see the warning with -fno-knr-functions. It also says that the argument is unused. bin/clang --version clang version 15.0.0 (https://github.com/llvm/llvm-project.git a9d68a5524dea113cace5983697786599cbdce9a

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

manojgupta added a comment. We are finding a lot of failures in our ToT builds with this change. here is an example for a configure script: $ cat tent.c int main () { tgetent(0,0); return 0; } $ bin/clang -c tent.c -Wno-error tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 a

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Unless I probably mis-interpreted something, -fno-knr-functions does not suppress the warning: https://godbolt.org/z/rbEfbbb33 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Basically, I'm wondering if you'd be able to enable -fno-knr-function? Thanks. this looks promising. Any ideas when -fno-knr-function support was added? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ http

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Is disabling the pedantic warning an option for your users? Disabling it wholesale is not an option since they actually want this warning (the older version). But we agreed to disable it specifically for the code where the warning was getting fired. One instance i

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

manojgupta added a comment. Some of our users are not very happy with the churn probably caused by this change where the declaration has the "void" argument but the later definition does not have explicit "void". void foo(void); void foo() { } GCC does not warn about this usage: h

[PATCH] D123300: [Clang] Enable opaque pointers by default

manojgupta added a comment. We noticed a new crash that still repros at head. clang -target armv7a-linux-gnueabihf -march=armv8a -mthumb -c -O2 file.cc llvm-project/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10135: llvm::Value *llvm::VPTransformState::get(llvm::VPValue *, unsigned int): As

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

manojgupta added a comment. For the background, we had hit this in Chrome OS when building bluetooth code. This is the one of structs hitting the issue where the warning got promoted to an error: typedef struct { private: static std::string AppendCapability(std::string& result, bool append,

[PATCH] D114312: libfuzzer: Disable broken tests for arm

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e67276d984d: libfuzzer: Disable broken tests for arm (authored by manojgupta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114312/new/ https://reviews.ll

[PATCH] D114312: libfuzzer: Disable broken tests for arm

manojgupta created this revision. manojgupta added reviewers: morehouse, metzman. Herald added a subscriber: kristof.beyls. manojgupta requested review of this revision. Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. libfuzzer was recently enabled for Arm32 in D112091

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

manojgupta added a comment. > More subjective: for most users this whole -march business is abstracted away > in build systems, so they won't have to deal with this, that's why this isn't > so much of an improvement. > If we want a better user experience set options, there are probably other

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

This revision was automatically updated to reflect the committed changes. Closed by commit rG2782cb8da0b3: libfuzzer: All building libfuzzer for ARM32 (authored by manojgupta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112091/new/ https://review

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

manojgupta added a reviewer: morehouse. manojgupta added a comment. Matt, do you think you can review this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112091/new/ https://reviews.llvm.org/D112091 ___

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

manojgupta added a comment. Yes, the current approach of "-march=+feature" is terrible and does not work with developers who want flexibility of features. This being pitched as a feature imo is akin to promoting a design bug as a feature. Any additive or subtractive alternative is welcome. Re

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

manojgupta added a comment. @kcc do you have any concerns? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112091/new/ https://reviews.llvm.org/D112091 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

manojgupta created this revision. manojgupta added a reviewer: metzman. Herald added subscribers: kristof.beyls, mgorny. manojgupta requested review of this revision. Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. We need libfuzzer libraries on Arm32 so that we can fuzz

[PATCH] D112059: Fix inline builtin handling in case of redefinition

manojgupta added a comment. thanks, I can verify that it fixes the crash we were seeing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112059/new/ https://reviews.llvm.org/D112059 ___ cfe-commits mailing

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

manojgupta added a comment. I am noticing a clang crash with ToT after this change. - testcase -- long a; char b, d; extern inline __attribute__((always_inline)) __attribute__((gnu_inline)) unsigned long strlen() { return a; } c(void) { strlen(&b); return 0; } unsig

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

manojgupta added a comment. Please also see https://bugs.llvm.org/show_bug.cgi?id=51416 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 ___ cfe-commits mailing li

[PATCH] D52524: Add -Wpoison-system-directories warning

manojgupta added a comment. An earlier version did check for library directories [1]. I am not exactly sure why was it removed, maybe it didn't work. So if anyone is willing to test that, please apply the diff and try. [1] Diff https://reviews.llvm.org/D52524?id=215958 CHANGES SINCE LAST ACTI

[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

manojgupta accepted this revision. manojgupta added a comment. @MaskRay I have verified that Chrome OS builds are not affected by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97894/new/ https://reviews.llvm.org/D97894 ___

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

manojgupta added a subscriber: tstellar. manojgupta added a comment. Thanks for the clarification. I do not have any objections but I feel that am not the right person to approve this change. @tstellar can you please review it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

manojgupta added a comment. Another concern is people generally want clang to work out of box without any special arguments. As a user, after installing clang's distro package or building from source, I expect that basic compilation should work out-of-box which includes gcc detection. i.e. "cl

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

manojgupta added a comment. Thanks for explaining that it only affects "-B". While I believe that this change won't affect us in Chrome OS, I think it should be reviewed and approved by a few Linux distro contributors since there is already known reliance e.g. Android on the current behavior.

[PATCH] D97902: [Driver] Clarify --gcc-toolchain

manojgupta added a comment. thanks, this is much needed documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97902/new/ https://reviews.llvm.org/D97902 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

manojgupta added a comment. I am not sure of the rationale or upside of this change and why do we want to drop gcc detection? GCC does not need to do the GCC detection because it has the needed information at configure time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

manojgupta added a comment. > With the special rule, it is: if --gcc-toolchain is $sysroot/usr, suppress > sysroot GCC detection as well. > > Clang -B and --gcc-toolchain have some weird behaviors. Hope you can share > your opinions on > https://lists.llvm.org/pipermail/cfe-dev/2021-March/06782

[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

manojgupta added a comment. In Chrome OS, we currently both "-B" and "--prefix". "-B" points to binutils bin directory and the "--prefix" has the binutils directory + target suffix. Should we drop "-B" and still get the same behavior? Sample invocation: '/usr/bin/clang++' '--sysroot=/usr/x86_64

[PATCH] D97894: [Driver] Drop $sysroot/usr special case from Gentoo gcc-config detection

manojgupta added a comment. Thanks @MaskRay for this clean up. I can't speak for all of Gentoo but please give me a couple of days to at least test this on Chrome OS. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97894/new/ https://reviews.llvm.or

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

manojgupta added a comment. This CL has caused two issues in Chrome OS : Compilation fail with FORTIFY: https://bugs.chromium.org/p/chromium/issues/detail?id=1168199 Runtime failures: https://bugs.chromium.org/p/chromium/issues/detail?id=1167504 I have requested George to take a look but will it

[PATCH] D92176: Don't use sysroot/include when sysroot is empty.

manojgupta added a comment. lgtm but not an expert in this area. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92176/new/ https://reviews.llvm.org/D92176 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D89327: fixes compiler-rt bug when printing libgcc for baremetal

This revision was automatically updated to reflect the committed changes. Closed by commit rG18432bea7648: [Driver]: fix compiler-rt path when printing libgcc for baremetal (authored by cjdb, committed by manojgupta). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

manojgupta added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1949 + // Maybe filter out /gcc and /gcc-cross. + GCCDirExists = D.getVFS().exists(LibDir + "/gcc"); + GCCCrossDirExists = D.getVFS().exists(LibDir + "/gcc-cross"); S

[PATCH] D87143: Check whether Gentoo-specific configuration directory exists

manojgupta accepted this revision. manojgupta added a comment. This revision is now accepted and ready to land. Looks ok to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87143/new/ https://reviews.llvm.org/D87143 __

[PATCH] D87143: Check whether Gentoo-specific configuration directory exists

manojgupta added a reviewer: mgorny. manojgupta added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2537-2538 const SmallVectorImpl &CandidateBiarchTriples) { + if (!D.getVFS().exists(GentooConfigDir)) +return false; + I think it shou

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

manojgupta added a comment. @nikic Thanks for the work. In D78862#2003684 , @arsenm wrote: > FWIW I think this attribute should be replaced with a data layout property, > so this would eventually be removed @arsenm Is there any work planned on moving

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added a comment. I opened https://github.com/ClangBuiltLinux/linux/issues/979 to see if we can fix this in Linux kernel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71082/new/ https://reviews.llvm.org/D71082 ___

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added a comment. Yes, it'd be nice if all of the FORTIFY handling can be improved. For a simple call like memcpy of 8 bytes in the example, there is no reason to emit all these stack/range checks since they'd degrade memcpy performance. I still think this change should be reverted if

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added a comment. Unfortunately, cherry-picking the kernel patches didn't work including latest memcpy for x86 (https://github.com/torvalds/linux/commit/170d13ca3a2fdaaa0283399247631b76b441cca2 and https://github.com/torvalds/linux/commit/c228d294f2040c3a5f5965ff04d4947d0bf6e7da ).

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added subscribers: nickdesaulniers, llozano, srhines. manojgupta added a comment. I was able to reduce to following: typedef unsigned int u32; typedef unsigned long long u64; typedef unsigned long size_t; void fortify_panic(const char *name) __attribute__((noreturn)) ; voi

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added a comment. Sure, I am trying to root cause the issue. Will report back hopefully soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71082/new/ https://reviews.llvm.org/D71082 ___ cfe-

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

manojgupta added a comment. We believe this change (relanded as https://reviews.llvm.org/rGd437fba8ef626b6d8b7928540f630163a9b04021) is causing a mis-compile in Linux kernel 4.4 builds resulting in local test failures. Chrome OS bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1066638

[PATCH] D62627: [NFC] Do not run CGProfilePass when -fno-integrated-as is on

manojgupta added a comment. Thanks, Noticed a few typos. Rest lgtm but deferring to other reviewers for now for approval. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1110 PTO.SLPVectorization = CodeGenOpts.VectorizeSLP; + PTO.CallGrpahProfile = CodeGenOpts.CallGraphProf

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

manojgupta added a comment. Thanks for the quick fix, verified that the crash is fixed on trunk. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

manojgupta added a comment. Still causing a crash using a previously supplied test https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c7. Any reason this was not tested with a previous repro? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. Thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. Friendly ping @pcc and @ldionne . We have been carrying this patch I think for too long now. Also, we have not discovered any LTO issues elsewhere so can't tell from our side if there are other places in libc++ that need visibility annotations. Repository: rCXX l

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

manojgupta added a comment. Hi, I see another crash with this change when building gdb. Reduced test case: struct type *a(type *, type *, long, long); enum b {}; static int empty_array(type *, int c) { type *d = a(__null, d, c, c - 1); } long e; b f() { empty_array(0, e); } Repros with: clang -

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

manojgupta added a comment. Thanks, Verified that this fixes the kernel warnings in my local builds with https://gist.github.com/nathanchance/767cccf4d093c1342e1994083518815e! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75758/new/ https://review

[PATCH] D52524: Add -Wpoison-system-directories warning

manojgupta closed this revision. manojgupta added a comment. Submitted as https://reviews.llvm.org/rL371785. Thanks for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/new/ https://reviews.llvm.org/D52524 ___ cfe-commits mail

[PATCH] D52524: Add -Wno-poison-system-directories flag

manojgupta added reviewers: rtrieu, aaron.ballman. manojgupta added a comment. Thanks, Adding a few more reviewers since I am not very familiar with this part of clang. Please also update the patch description as suggested by @thakis CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52524/

[PATCH] D52524: Add -Wno-poison-system-directories flag

manojgupta added inline comments. Comment at: clang/test/Frontend/warning-poison-system-directories.c:12 +// Missing target but included sysroot still causes the warning. +// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tr

[PATCH] D52524: Add -Wno-poison-system-directories flag

manojgupta added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1071 +// cross-compiling. +def PoisonSystemDirectories : DiagGroup<"poison-system-directories">; + Please verify that the warning is not enabled by default. CHANGES SINC

[PATCH] D52524: Add -Wno-poison-system-directories flag

manojgupta added a comment. In D52524#1630767 , @thakis wrote: > Wouldn't those projects just move to also disabling the warning by passing > -Wno-poison-system-directories? If there are projects that are actively > adding -I/usr/include, that means they

[PATCH] D52524: Add -Wno-poison-system-directories flag

manojgupta added a comment. In D52524#1621468 , @thakis wrote: > Couldn't cross build users just pass -nostdsysteminc to tell clang to not > look in system header locations? My understanding is "-nostdsysteminc " does not block users from passing inclu

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. @pcc can you please submit this patch if there are no objections? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list c

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

manojgupta added a comment. @dankm are you still working on this patch? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49466/new/ https://reviews.llvm.org/D49466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

  1   2   >