RE: r303798 - For Microsoft compatibility, set fno_operator_names

Thanks for the feedback, working on it… From: Keane, Erich Sent: Wednesday, May 24, 2017 3:47 PM To: Nico Weber ; Blower, Melanie Cc: cfe-commits ; rnk Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names Adding Melanie, the author of the patch. From: tha...@google.com

RE: r303798 - For Microsoft compatibility, set fno_operator_names

Sometimes it’s possible to reason with the open source community, then they change their code and make modifications which accommodate the various compilers. E.g. we could ask chromium team if they could use the operator symbol instead of ‘and’. From: Nico Weber [mailto:tha...@google.com] Sent:

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

Thanks I'll fix that. -Original Message- From: Fedor Sergeev via Phabricator [mailto:revi...@reviews.llvm.org] Sent: Tuesday, June 13, 2017 3:30 PM To: Blower, Melanie ; zhangsheng...@huawei.com; olivier...@gmail.com; kalinichev.s...@gmail.com; kf...@kde.org; m...@milianw.de; cfe-commi

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

Yes the IR looks a bit odd. I should use a simpler case with a global _Atomic int a instead of the parameter. The store is the parameter value a, if I move that to a global then it won't be as confusing. > -Original Message- > From: JF Bastien via Phabricator [mailto:revi...@reviews.llv

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

Sorry I formatted my reply badly, there are responses inline in the previous message > -Original Message- > From: Blower, Melanie > Sent: Wednesday, March 13, 2019 2:06 PM > To: 'reviews+d59307+public+153a08d52e63c...@reviews.llvm.org' > ; cfe- > comm...@lists.llvm.org; Keane, Erich ; > r

RE: [PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

> -Original Message- > From: Joerg Sonnenberger via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Tuesday, September 12, 2017 3:24 PM > To: Blower, Melanie ; olivier...@gmail.com; > kalinichev.s...@gmail.com; kf...@kde.org; m...@milianw.de; Keane, Erich > ; mgo...@gentoo.org; fedo

RE: [PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

Thanks for your review. I put some quick replies below. I'll work on an update to this. > -Original Message- > From: Kevin P. Neal via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Thursday, July 25, 2019 1:09 PM > To: Blower, Melanie ; chandl...@gmail.com > Cc: mgo...@gentoo.or

RE: [PATCH] D34158: For standards compatibility, preinclude if the file is available

fedor.sergeev added a comment. Hmm... I tried this patch and now the following worries me: - it passes -finclude-if-exists stdc-predef.h on all platforms (say, including my Solaris platform that has no system stdc-predef.h) - it searches all the paths, not just "system include" ones That ess

RE: [PATCH] D34158: For standards compatibility, preinclude if the file is available

jyknight added a comment. In https://reviews.llvm.org/D34158#823316, @fedor.sergeev wrote: > Hmm... I tried this patch and now the following worries me: > > - it passes -finclude-if-exists stdc-predef.h on all platforms (say, > including my Solaris platform that has no system stdc-predef.h)

RE: [PATCH] D34158: For standards compatibility, preinclude if the file is available

fedor.sergeev added inline comments. Comment at: test/Driver/stdc-predef.c:15 + /* In this test, the file stdc-predef.h is missing from the +installation */ #if _STDC_PREDEF_H + #error "stdc-predef.h should not be included" I would rather see a real check on

RE: [PATCH] D34158: For standards compatibility, preinclude if the file is available

joerg added a comment. I had a long discussion with James about this on IRC without reaching a clear consensus. I consider forcing this behavior on all targets to be a major bug. It should be opt-in and opt-in only: (1) The header name is not mandated by any standard. It is not in any namespa

RE: [PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

fedor.sergeev added a comment. In https://reviews.llvm.org/D34158#834298, @mibintc wrote: > In fact I did have trouble writing the new test case to pass with the > gnu/Linux toolchain. In the file lib/Driver/ToolChains/Linux.cpp function > AddGnuIncludeArgs checks if GCCInstallation.isValid()

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

Hahnfeld added a comment. Some comments inline. In general you should consider posting an RFC on cfe-dev because this change will basically affect all compilations on GNU/Linux if the file is present. >> Thank you Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

Hahnfeld added a subscriber: cfe-commits. Hahnfeld edited reviewers, added: rsmith, rengolin; removed: cfe-commits. Hahnfeld added a comment. Some comments inline. In general you should consider posting an RFC on cfe-dev because this change will basically affect all compilations on GNU/Linux if

RE: [PATCH] D34158: For standards compatibility, preinclude if the file is available

@jyknight I've removed the check on nostdinc. AFAIK, I've addressed all the feedback which I've received on the patch, is it OK to commit? --Melanie -Original Message- From: James Y Knight via Phabricator [mailto:revi...@reviews.llvm.org] Sent: Monday, July 10, 2017 11:57 AM To: Blower

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

> -Original Message- > From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Thursday, January 11, 2018 6:13 AM > To: Blower, Melanie ; Keane, Erich > ; sca...@apple.com; roger.ferreriba...@arm.com; > sjoerd.mei...@arm.com; jle...@google.com; > hubert.reinterpretc.

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

> -Original Message- > From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Thursday, January 11, 2018 6:25 AM > To: Blower, Melanie ; Keane, Erich > ; sca...@apple.com; roger.ferreriba...@arm.com; > sjoerd.mei...@arm.com; jle...@google.com; > hubert.reinterpretc.

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

> -Original Message- > From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Friday, January 12, 2018 3:18 PM > > nsz added a comment. > > > it is not clear to me from the original bug report what "fedora 27 workloads" > break because of the lack of _Float128

RE: D26636: patch for llvm/clang bug 25965, suppress warning diagnostic on float-to-bool conversion when in condition context

Thanks for your question The bug was originally posted in llvm bugs database, see 25965, there is discussion between Richard Smith and the submitter Gregory Pakosz which concludes "maybe we should suppress the diagnostic entirely". Evidently there is fairly usage to guard against divide by zero