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

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

[PATCH] D124469: [NFC] Cleanup miscellaneous header items

wristow added a comment. Thanks for the review @RKSimon ! Comment at: clang/lib/Headers/__wmmintrin_pclmul.h:25 /// \code -/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// __m128i _mm_clmulepi64_si128(__m128i X, __m128i Y, const int I); ///

[PATCH] D124469: [NFC] Cleanup miscellaneous header items

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdf08b3493869: [NFC] Cleanup miscellaneous header items (authored by wristow). Herald added a project: clang. Repository: rG LLVM Github Monorepo

[PATCH] D124469: [NFC] Cleanup miscellaneous header items

wristow created this revision. wristow added reviewers: craig.topper, RKSimon. Herald added a subscriber: StephenFan. Herald added a project: All. wristow requested review of this revision. - Explain the use of the _MM_SHUFFLE and _MM_SHUFFLE2 macros - Update some doxygen parameter descriptions

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

wristow added a comment. Thanks @zahiraam ! The updated ReleaseNote LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

wristow added a comment. In D107994#3131268 , @zahiraam wrote: > In D107994#3130494 , @wristow wrote: > >> The Release Note change here says: >> >> Floating Point Support in Clang >>

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

wristow added a comment. The Release Note change here says: Floating Point Support in Clang --- - The -ffp-model=precise now implies -ffp-contract=on rather than -ffp-contract=fast, and the documentation of these features has been clarified. Previously,

[PATCH] D67429: Improve code generation for thread_local variables:

wristow added a comment. This commit appears to be the root cause of a run-time crash related to the interaction of global initializers and the wrapper functions to access `thread_local` variables -- reported as PR48030 . Repository: rL LLVM

[PATCH] D86649: Fix for assertion failure on PR46865

wristow added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:3098 // have been value-dependent too), so diagnose that. -assert(!VD->mightBeUsableInConstantExpressions(Info.Ctx)); +assert(!VD->isUsableInConstantExpressions(Info.Ctx)); if

[PATCH] D80952: [FPEnv][Clang][Driver] Disable constrained floating point on targets lacking support.

wristow added inline comments. Comment at: clang/test/CodeGen/fp-strictfp.cpp:1 +// RUN: %clang_cc1 -triple mips64-linux-gnu -frounding-math -ffp-exception-behavior=strict -O2 -verify=rounding,exception -emit-llvm -o - %s | tee /tmp/1 | FileCheck %s +// RUN: %clang_cc1 -triple

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow added a comment. Revisiting this patch. I think that before fixing the `-ffp-contract=off` problem I originally raised here, there are two questions that have come up in this discussion that we should first resolve. Specifically: (1) Do we want to enable FMA transformations by

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow marked 4 inline comments as done. wristow added inline comments. Comment at: clang/test/Driver/fast-math.c:196 +// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s +// wristow wrote:

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow updated this revision to Diff 240689. wristow added a comment. Used the clearer '!off && !on' (rather than '!(off || on)') in the checks. Added more tests that note the current situation that `-ffast-math` enables FMA. overriding an earlier switch that had disabled it (included a "TODO"

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow added a comment. About: >> This is a bit of an oddity in our handling. > > Yes it is! > > This is certainly getting more complicated than I had originally expected. I > feel there isn't a clear spec on what we want in terms of whether FMA should > be enabled "automatically" at (for

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow marked 4 inline comments as done. wristow added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2792 if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && - !TrappingMath) + !TrappingMath && !(FPContract.equals("off") ||

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow updated this revision to Diff 240305. wristow added a comment. Update a comment to remove the discussion about enabling the `__FAST_MATH__` preprocessor macro. The handling of the setting of `__FAST_MATH__` is done in "clang/lib/Frontend/InitPreprocessor.cpp", and once we decide on the

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow added a comment. In D72675#1839662 , @andrew.w.kaylor wrote: > In D72675#1839492 , @wristow wrote: > > > 1. Should we enable FMA "by default" at (for example) '-O2'? > > > We recently introduced a new

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow added a comment. > A separate question is the interaction of `-ffast-math` with > `-ffp-contract=`. Currently, there is no such interaction whatsoever in GCC: > `-ffast-math` does not imply any particular `-ffp-contract=` setting, and > vice versa the `-ffp-contract=` setting is not

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow updated this revision to Diff 238933. wristow added a comment. Updated patch to correct a comment and fix a typo. Regarding the point from @spatel : > This follows the reasoning from the earlier discussion, but after re-reading > the gcc comment in particular, I'm wondering whether

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

wristow updated this revision to Diff 238595. wristow retitled this revision from "Fix -ffast-math/-ffp-contract interaction" to "[Clang][Driver] Fix -ffast-math/-ffp-contract interaction". wristow added a comment. Changing this to address only the Clang driver aspect, as discussed in the

[PATCH] D72675: Fix -ffast-math/-ffp-contract interaction

wristow added a comment. > One commit for the clang changes should be ok; it's a very small diff. But > I'm still not sure if the driver change induces frontend diffs that we should > make visible via tests. The only thing I can think of is that it changes whether/when `__FAST_MATH__` is

[PATCH] D72675: Fix -ffast-math/-ffp-contract interaction

wristow added a comment. This all sounds good to me. So to make sure we're all on the same page, my understanding is that the plan forward is: 1. Make the Clang change first (including adding another pair of tests for `-ffp-contract=on`). 2. Update the LLVM tests illustrating the current

[PATCH] D72675: Fix -ffast-math/-ffp-contract interaction

wristow marked an inline comment as done. wristow added inline comments. Comment at: llvm/test/CodeGen/PowerPC/fmf-propagation.ll:201-203 ; fma(X, 7.0, X * 42.0) --> X * 49.0 -; This is the minimum FMF needed for this transform - the FMA allows reassociation. +; This is the

[PATCH] D72675: Fix -ffast-math/-ffp-contract interaction

wristow updated this revision to Diff 238143. wristow retitled this revision from "ix -ffast-math/-ffp-contract interaction" to "Fix -ffast-math/-ffp-contract interaction". wristow added a comment. Addressed comments from @hfinkel . CHANGES SINCE LAST ACTION

[PATCH] D72675: ix -ffast-math/-ffp-contract interaction

wristow marked 3 inline comments as done. wristow added a comment. Thanks for the quick feedback @hfinkel Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2721 if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && - !TrappingMath) + !TrappingMath

[PATCH] D72675: ix -ffast-math/-ffp-contract interaction

wristow created this revision. wristow added reviewers: spatel, mcberg2017. Herald added subscribers: jsji, hiraditya, nemanjai. Herald added a project: LLVM. Fused Multiply Add (FMA) was not always being disabled when the switch `-ffp-contract=off` was used. More specifically, FMA is enabled

[PATCH] D71718: [X86] Mark various pointer arguments in builtins as const

wristow added a comment. Thanks for the quick review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71718/new/ https://reviews.llvm.org/D71718 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71718: [X86] Mark various pointer arguments in builtins as const

This revision was automatically updated to reflect the committed changes. Closed by commit rG7fcd9e3f7083: [X86] Mark various pointer arguments in builtins as const (authored by wristow). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71718: [X86] Mark various pointer arguments in builtins as const

wristow created this revision. wristow added a reviewer: craig.topper. Enabling `-Wcast-qual` identified many casts in various system headers that were dropping the `const` qualifier. Fixing those missing qualifiers pointed out that a few of the definitions of the builtins did not properly

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

wristow added a comment. >> Given the performance improvements here, I'd like to develop this patch >> further. > > In D69732#1784290 , @ormris wrote: > //Ping// @tejohnson @ormris, I think that since @tejohnson originally suggested that someone with

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

wristow added inline comments. Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:614 // during ThinLTO and perform the rest of the optimizations afterward. if (PrepareForThinLTO) { // Ensure we perform any last passes, but do so before renaming anonymous

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

wristow added a comment. > This probably needs to be taken over by someone who cares about full LTO > performance We at PlayStation are definitely interested in full LTO performance, so we're looking into this. We certainly agree with the rationale that if suppressing some optimizations is

[PATCH] D64780: Disallow most calling convention attributes on PS4.

wristow accepted this revision. wristow added a comment. In D64780#1593624 , @aaron.ballman wrote: > LGTM aside from a nit. You should give other reviewers a chance to sign off > in case they have additional comments. LGTM too, once the line-length is

[PATCH] D64780: Disallow most calling convention attributes on PS4.

wristow added reviewers: rnk, rjmccall. wristow added inline comments. Comment at: include/clang/Basic/TargetInfo.h:1271 CCCR_Ignore, +CCCR_Error, }; rnk wrote: > I feel like perhaps a cleaner way of implementing this would be to have the > driver

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

wristow added a comment. > Do we need to keep the old behavior on platforms where clang is the de facto > compiler? I know we (PlayStation) will want to keep the old behavior. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64672/new/ https://reviews.llvm.org/D64672

[PATCH] D47291: Proposal to make rtti errors more generic.

wristow added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; probinson wrote: >

[PATCH] D47291: Proposal to make rtti errors more generic.

wristow added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; Sunil_Srivastava wrote: >

[PATCH] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

wristow added a comment. In https://reviews.llvm.org/D39812#919687, @spatel wrote: > I just reviewed the gcc docs: > https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html > > "[-fassociative-math] requires that both -fno-signed-zeros and > -fno-trapping-math be in effect." > > If we want to

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

This revision was automatically updated to reflect the committed changes. wristow marked an inline comment as done. Closed by commit rL293911: Prevent ICE in dllexport class with _Atomic data member (authored by wristow). Changed prior to commit:

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

wristow marked 2 inline comments as done. wristow added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1135 MemberExpr *ME2 = dyn_cast(RHS); -if (dyn_cast(ME2->getMemberDecl()) != Field) +if (!ME2 || dyn_cast(ME2->getMemberDecl()) != Field)

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

wristow updated this revision to Diff 86767. wristow added a comment. Code restructured. https://reviews.llvm.org/D29208 Files: lib/CodeGen/CGClass.cpp test/CodeGenCXX/atomic-dllexport.cpp Index: test/CodeGenCXX/atomic-dllexport.cpp

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

wristow added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1135 MemberExpr *ME2 = dyn_cast(RHS); -if (dyn_cast(ME2->getMemberDecl()) != Field) +if (!ME2 || dyn_cast(ME2->getMemberDecl()) != Field) return nullptr;

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

wristow added a comment. When a class that has been tagged as dllexport (for an MSVC target) contains an atomic data member via the C11 '_Atomic' approach, the front end crashes with a null pointer dereference. This patch fixes it by guarding the null dereference with the approach used by

[PATCH] D29208: Prevent ICE in dllexport class with _Atomic() data member

wristow created this revision. Guard against a null pointer dereference that caused Clang to crash when processing a class containing an _Atomic() data member, and that is tagged with 'dllexport'. https://reviews.llvm.org/D29208 Files: lib/CodeGen/CGClass.cpp