[PATCH] D88664: [AST] do not error on APFloat invalidOp in default mode

2020-10-01 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. spatel added reviewers: rjmccall, efriedma, sepavloff. Herald added a subscriber: mcrosier. spatel requested review of this revision. If FP exceptions are ignored, we should not error out of compilation just because APFloat indicated an exception. This is required a

[PATCH] D88664: [AST] do not error on APFloat invalidOp in default mode

2020-10-01 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG686eb0d8ded9: [AST] do not error on APFloat invalidOp in default mode (authored by spatel). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D88238: [APFloat] convert SNaN to QNaN in convert() and raise Invalid signal

2020-10-01 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG149f5b573c79: [APFloat] convert SNaN to QNaN in convert() and raise Invalid signal (authored by spatel). Herald added subscribers: cfe-commits, jrtc27. Herald added a project: clang. Repository: rG LLVM

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-06 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88154#2310653 , @venkataramanan.kumar.llvm wrote: > In D88154#2290205 , @abique wrote: > >> Looks good to me. >> Regarding the tests, it seems that you check if auto-vectorization takes

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-12 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88154#2325713 , @venkataramanan.kumar.llvm wrote: > As per review comments from Sanjay, updated the test case to use metadata. > Also autogenerated the checks in the test cases using > llvm/utils/update_test_checks.py. Tha

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-13 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:9 +define void @exp_f32(float* nocapture %varray) { +; CHECK-LABEL: @exp_f32( +; CHECK-NEXT: entry: fpetrogalli wrote: > I think you are over-testing her

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a subscriber: aeubanks. spatel added a comment. In D88154#2328312 , @venkataramanan.kumar.llvm wrote: > I can add few more float type tests with meta data for VF=8. please let me > know your suggestions. I may be missing some subtlety of

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88712#2412874 , @venkataramanan.kumar.llvm wrote: > In D88712#2412366 , @MaskRay wrote: > >> In D88712#2411841 , >> @venkataramanan.kumar.llvm w

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88712#2413877 , @venkataramanan.kumar.llvm wrote: > In D88712#2413688 , @spatel wrote: > >> In D88712#2412874 , >> @venkataramanan.kumar.llvm wr

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. I'm not sure why we need 3 different test files to test the very similar patterns - just programmer preference? Wait a day or so to commit in case anyone else has comments. CHANGES SIN

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-22 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG57cdc52c4df0: Initial support for vectorization using Libmvec (GLIBC vector math library) (authored by venkataramanan.kumar.llvm, committed by spatel). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-10-22 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D87188#2348326 , @nikic wrote: > Reopening this so we don't forget... > > I believe @spatel is working on the cost modelling. I did not have much luck > tracking down the miscompile, at least did not spot anything incriminating

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

2021-05-25 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D101191#2779007 , @fhahn wrote: > Looks like this is causing an infinite loop in instcombine: > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34661 This seems likely to be another partial undef/poison vector problem (

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

2021-05-25 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D101191#2779631 , @spatel wrote: > In D101191#2779007 , @fhahn wrote: > >> Looks like this is causing an infinite loop in instcombine: >> https://bugs.chromium.org/p/oss-fuzz/issues/det

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

2021-05-30 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D101191#2783570 , @rupprecht wrote: > In D101191#2782963 , @rupprecht > wrote: > >> The issue I'm seeing seems more directly caused by SLP vectorization, as it >> goes away with `-fno-

[PATCH] D109925: [AST] add warnings for out-of-bounds FP values when using relaxed FP math compile flags

2021-09-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. spatel added reviewers: sepavloff, jyknight, efriedma, dim, kparzysz, rsmith. Herald added a subscriber: mcrosier. spatel requested review of this revision. There's currently ongoing discussion on the dev lists about how to handle related cases with isnan() / isinf()

[PATCH] D96231: [X86] Always assign reassoc flag for intrinsics *reduce_add/mul_ps/pd.

2021-02-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13829 CGM.getIntrinsic(Intrinsic::vector_reduce_fadd, Ops[1]->getType()); +Builder.getFastMathFlags().setAllowReassoc(true); return Builder.CreateCall(F, {Ops[0], Ops[1]}); ---

[PATCH] D96231: [X86] Always assign reassoc flag for intrinsics *reduce_add/mul_ps/pd.

2021-02-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9352 static __inline__ double __DEFAULT_FN_ATTRS512 _mm512_reduce_add_pd(__m512d __W) { return __builtin_ia32_reduce_fadd_pd512(0.0, __W); } Ah - this is where the +0.0 is specified

[PATCH] D96231: [X86] Always assign reassoc flag for intrinsics *reduce_add/mul_ps/pd.

2021-02-09 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel 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/D96231/new/ https://reviews.llvm.org/D96231 ___ c

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

2021-02-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; If I understand correctl

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

2021-02-14 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/Headers/avx512fintrin.h:9305 + * 1. The elements are reassociable when using fadd/fmul intrinsics; + * 2. There's no nan and signed zero in the elements when using fmin/max + intrinsics; pengfei wrote: > spatel

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

2021-02-14 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. Thanks! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93179/new/ https://reviews.llvm.org/D93179

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

2021-08-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Hard to see through all of the lint noise, but seems like a mechanical fix. Can we add a test like the one in the bug report? https://godbolt.org/z/sPT8e9vx9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107843/new/ https://

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

2021-08-13 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Probably want to address the other cleanups in another patch; the parens fixes and test LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107843/new/ https://reviews.llvm.org/D107843 _

[PATCH] D105268: [X86] AVX512FP16 instructions enabling 5/6

2021-07-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16vl-fma.ll:193-194 +define <8 x half> @stack_fold_fmsub123ph(<8 x half> %a0, <8 x half> %a1, <8 x half> %a2) { + ;check-label: stack_fold_fmsub123ph: + ;check: vfmsub213ph {{-?[0-9]

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-28 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } MatzeB wrote: > I believe this is causing some of our clients tr

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

2022-08-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM These clang tests are just awful, but I don't have the patience to fix them... Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:245 // TODO: Investigate the cost/

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

2022-11-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM based on the timing, results, and alternatives discussed There's some testing in progress according to previous comments, so best to wait for that to finish in case it turns up anything ne

[PATCH] D127460: Rename GCCBuiltin into ClangBuiltin

2022-06-14 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:387 -/// GCCBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this +/// ClangBuiltin - If this intrinsic exactly corresponds to a GCC builtin, this /// specifies the name of the builti

[PATCH] D72467: Remove "mask" operand from shufflevector.

2020-03-27 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Looks like D76649 landed and added another IRBuilder call that needs updating: [2974/5437] Building CXX object lib/Target/X86/CMakeFiles/LLVMX86CodeGen.dir/X86PartialReduction.cpp.o FAILED: lib/Target/X86/CMakeFiles/LLVMX86CodeGen.d

[PATCH] D74500: clang: Treat ieee mode as the default for denormal-fp-math

2020-03-02 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/include/llvm/ADT/FloatingPointMode.h:48 - DenormalMode() = default; - DenormalMode(DenormalModeKind Out, DenormalModeKind In) : + constexpr DenormalMode() = default; + constexpr DenormalMode(DenormalModeKind Out, DenormalModeKi

[PATCH] D69598: Work on cleaning up denormal mode handling

2019-11-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1736-1737 +if (CodeGenOpts.FPSubnormalMode != llvm::SubnormalMode::Invalid) + FuncAttrs.addAttribute("denormal-fp-math", + llvm::subnormalModeName(CodeGenOpts.FPSubnormalM

[PATCH] D69598: Work on cleaning up denormal mode handling

2019-11-06 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1736-1737 +if (CodeGenOpts.FPSubnormalMode != llvm::SubnormalMode::Invalid) + FuncAttrs.addAttribute("denormal-fp-math", + llvm::subnormalModeName(CodeGenOpts.FPSubnormalM

[PATCH] D69598: Work on cleaning up denormal mode handling

2019-11-06 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1736-1737 +if (CodeGenOpts.FPSubnormalMode != llvm::SubnormalMode::Invalid) + FuncAttrs.addAttribute("denormal-fp-math", + llvm::subnormalModeName(CodeGenOpts.FPSubnormalM

[PATCH] D69598: Work on cleaning up denormal mode handling

2019-11-07 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM - see inline for some leftover naming diffs. Comment at: clang/lib/CodeGen/CGCall.cpp:1744-1745 FuncAttrs.addAttribute("null-pointer-is-valid", "true"); -if (

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D69979#1738099 , @craig.topper wrote: > I checked Redhat 7.4 that's on the server I'm using for work. And I had a > coworker check his Ubuntu 18.04 system with this program. And both systems > printed 1f80 as the value of MXCS

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a reviewer: cameron.mcinally. spatel added a comment. Also, I may have missed some discussions. Does this patch series replace the proposal to add instruction-level FMF for denorms? http://lists.llvm.org/pipermail/llvm-dev/2019-September/135183.html Ie, did we decide that a function

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-11-14 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D69979#1740294 , @arsenm wrote: > In D69979#1738099 , @craig.topper > wrote: > > > I checked Redhat 7.4 that's on the server I'm using for work. And I had a > > coworker check his Ubuntu

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-11-18 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D69979#1749198 , @arsenm wrote: > I just posted the test I wrote here: https://github.com/arsenm/subnormal_test Thanks. I tried compiling with gcc (can't trust clang since it doesn't honor #pragma STDC FENV_ACCESS ON?). And r

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

2019-11-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added reviewers: cameron.mcinally, mcberg2017, arsenm. spatel added a comment. Herald added a subscriber: wdng. I like the idea, but I'd be more comfortable reviewing the diffs in stages, so we know that the test coverage for the value tracking calls is good. So I'd prefer if we split thi

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-12-02 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a subscriber: andreadb. spatel added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:580 + /// This checks for presence of the -Ofast, -ffast-math or -funsafe-math flags. + virtual bool FastMathRuntimeIsAvailable( +const llvm::opt::ArgList

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2019-12-11 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: clang/test/Driver/default-denormal-fp-math.c:7 +// crtfastmath enables ftz and daz +// RUN: %clang -### -target x86_64-unknown-linux-gnu -ffast-math --sysroot=%S/Inputs/basic_linux_tree -c %s -v 2>&1 | FileCheck -check-prefix=CHECK-ZERO

[PATCH] D75467: [instcombine] remove fsub to fneg hacks; only emit fneg

2020-03-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/include/llvm/IR/PatternMatch.h:622 inline bind_ty m_Instruction(Instruction *&I) { return I; } /// Match a binary operator, capturing it if we match. +inline bind_ty m_UnOp(UnaryOperator *&I) { return I; } Match a

[PATCH] D75467: [instcombine] remove fsub to fneg hacks; only emit fneg

2020-03-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. LGTM - see inline for 1 more comment nit. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2132 // Subtraction from -0.0 is the canonical form of fneg. + // fsub -0.0, X ==> fneg X Th

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

2020-03-12 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a subscriber: mibintc. spatel added a comment. In D72675#1917844 , @wristow wrote: > 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 > i

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

2020-03-12 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D72675#1919793 , @mibintc wrote: > In D72675#1919685 , @spatel wrote: > > > In D72675#1917844 , @wristow wrote: > > > > > Revisiting this patch. I

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

2020-02-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM - the PS4 behavior was confirmed off-list. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69979/new/ https://reviews.llvm.org/D69979 ___

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

2020-01-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D72675#1823227 , @mcberg2017 wrote: > We crossed that bridge internally at Apple a while ago, meaning I have some > code debt for cleaning up open source for fma formation that uses contract > and reassoc differently than we do

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

2020-01-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D72675#1824659 , @wristow wrote: > 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

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

2020-01-17 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. This follows the reasoning that we discussed in the earlier discussion, but after re-reading the gcc comment in particular, I'm wondering whether this is what we really want to do... If __FAST_MATH__ is purely here for compatibility with gcc, then should we mimic gcc beh

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

2020-01-17 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added reviewers: efriedma, scanon, arsenm, echristo, RKSimon. spatel added a comment. Herald added a subscriber: wdng. Adding potential FP reviewers for question of gcc- (potentially-buggy-) compatibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72675/new/ https://reviews.

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

2020-01-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D72675#1827893 , @wristow wrote: > How to handle this seems like an implementation question. The code here is > just deciding whether or not we intend to pass "-ffast-math" to cc1 (it isn't > directly defining `__FAST_MATH__`)

[PATCH] D72467: Remove "mask" operand from shufflevector.

2020-01-23 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/include/llvm/IR/Constants.h:1220 + /// Assert that this is an shufflevector and return the mask. See class + /// ShuffleVectorInst for a description of the mask representation. an shufflevector -> a shufflevector

[PATCH] D72467: Remove "mask" operand from shufflevector.

2020-01-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM, but should get a 2nd opinion since I'm not familiar with some of the parts. Also, please update the related LangRef text for shufflevector in or alongside this patch. Could add some asc

[PATCH] D72467: Remove "mask" operand from shufflevector.

2020-01-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D72467#1839172 , @efriedma wrote: > In D72467#1838591 , @spatel wrote: > > > LGTM, but should get a 2nd opinion since I'm not familiar with some of the > > parts. > > > Any specific part

[PATCH] D44852: [CodeGen] Mark fma as const for Android

2018-03-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13108-13110 // We make "fma" on GNU or Windows const because we know it does not set // errno in those environments even though it could set errno based on the // C stand

[PATCH] D45421: [X86] Emit native IR for pmuldq/pmuludq builtins.

2018-04-09 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. > The only question I have is whether its ok to emit the v2i32 intermediate > type for the 128-bit version. I wasn't sure of any examples where we use an > illegal type in our intrinsic/builtin handling. At least not a narrower type. > I know pavg uses a wider type. I d

[PATCH] D45421: [X86] Emit native IR for pmuldq/pmuludq builtins.

2018-04-09 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: test/CodeGen/sse2-builtins.c:7 // NOTE: This should match the tests in llvm/test/CodeGen/X86/sse2-intrinsics-fast-isel.ll There should be matching codegen tests for the new IR patterns here or have they moved? Re

[PATCH] D45421: [X86] Emit native IR for pmuldq/pmuludq builtins.

2018-04-09 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D45421#1061875, @craig.topper wrote: > Yes. I'll make the llvm changes before committing this. Just wanted to make > sure this direction was ok first. Ah, seems ok then. But instcombine is going to turn these casts into 'and' or 'shl+ashr',

[PATCH] D45421: [X86] Emit native IR for pmuldq/pmuludq builtins.

2018-04-09 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM (assuming the backend gets this right in all cases and we have tests for that). https://reviews.llvm.org/D45421 ___ cfe-commits mailing lis

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-30 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 120902. spatel added a comment. Patch updated: As suggested, I've morphed this patch to just handle sqrt libcalls based on the updated LLVM intrinsic definition. I was going to include the builtins too, but that exposes another bug (marked here with FIXME) -

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39204#911316, @efriedma wrote: > > I was going to include the builtins too, but that exposes another bug > > (marked here with FIXME) - the builtins are all defined 'const'. > > Probably just need to change c->e in Builtins.def? Yes - at lea

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39204#912084, @efriedma wrote: > Let's do this one step at a time; first this patch, then fix the __builtin_* > functions to use "e", then add all the missing cases > CodeGenFunction::EmitBuiltinExpr. Sounds good. > This patch LGTM, assumi

[PATCH] D39204: [CodeGen] map sqrt libcalls to llvm.sqrt when errno is not set

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL317031: [CodeGen] map sqrt libcalls to llvm.sqrt when errno is not set (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D39204?vs=120902&id=121042#toc Repository: rL LLVM http

[PATCH] D39481: [CodeGen] fix const-ness of builtin equivalents of and functions that might set errno

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. As the description in Builtins.def says: "e = const, but only when -fmath-errno=0". This is step 2 of 3 to fix builtins and math calls as discussed in https://reviews.llvm.org/D39204. I went through the lists at: http://en.cppr

[PATCH] D39481: [CodeGen] fix const-ness of builtin equivalents of and functions that might set errno

2017-11-01 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39481#912378, @efriedma wrote: > Granted, I'm not sure all of those are right... I'm pretty sure, for example, > cbrt can't set errno, and carg can. But I'd prefer to deal with that in a > separate patch. I figured we have to be ultra-cons

[PATCH] D39481: [CodeGen] fix const-ness of builtin equivalents of and functions that might set errno

2017-11-02 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 121344. spatel added a comment. Patch updated: Make const-ness of the builtins match const-ness of their lib function siblings. We're deferring fixing some of these that are obviously wrong to follow-up patches. Hopefully, the bugs are visible in the new test

[PATCH] D39481: [CodeGen] fix const-ness of builtin equivalents of and functions that might set errno

2017-11-02 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL317265: [CodeGen] fix const-ness of builtin equivalents of and … (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D39481?vs=121344&id=121363#toc Repository: rL LLVM https://r

[PATCH] D39611: [CodeGen] make cbrt and fma constant (never set errno); document complex calls as always constant

2017-11-03 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. These are 3 errno-related diffs raised in D39841. 1. Cube root We're going to dismiss POSIX language like this: "On successful completion, cbrt() returns the cube root of x. If x is NaN, cbrt() returns NaN and errno may be set t

[PATCH] D39615: [CodeGen] add remquo to list of recognized library calls

2017-11-03 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. I assume this is just an oversight because we already do recognize __builtin_remquo() with the same signature. http://en.cppreference.com/w/c/numeric/math/remquo http://pubs.opengroup.org/onlinepubs/9699919799/functions/remquo.ht

[PATCH] D39615: [CodeGen] add remquo to list of recognized library calls

2017-11-04 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL317407: [CodeGen] add remquo to list of recognized library calls (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D39615?vs=121536&id=121591#toc Repository: rL LLVM https://re

[PATCH] D39611: [CodeGen] make cbrt and fma constant (never set errno); document complex calls as always constant

2017-11-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39611#915812, @hfinkel wrote: > In the C specification, 7.12 specifies the requirements for functions in > math.h. For those functions, 7.12.1 (Treatment of error conditions) says that > overflows do set ERANGE, and that it's implementation d

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. Splitting cbrt and fma off from https://reviews.llvm.org/D39611, so we can deal with complex.h separately. I've also gone ahead with a 'fix' for the fma case in CodeGenFunction::EmitBuiltinExpr(). But as noted previously, there'

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-07 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 121898. spatel added a comment. Patch updated - no code or functional changes from the last rev. I split the and test files up, so I can update those independently and not incur svn conflicts. https://reviews.llvm.org/D39641 Files: include/clang/Basic/

[PATCH] D39611: [CodeGen] change const-ness of complex calls

2017-11-07 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 121949. spatel retitled this revision from "[CodeGen] make cbrt and fma constant (never set errno); document complex calls as always constant" to "[CodeGen] change const-ness of complex calls". spatel edited the summary of this revision. spatel added a comment

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

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. There are 2 parts to getting the -fassociative-math command-line flag translated to LLVM FMF: 1. In the driver/frontend, we accept the flag and its 'no' inverse and deal with the interactions with other flags like -ffast-math. T

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

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. 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 match that behavior, I need to change this patch to light up 'nsz' and t

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

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39812#919699, @wristow wrote: > Yes, I think we want to match that behavior. But by "light up" 'nsz' and the > no-trapping-math attribute, do you mean automatically turn them on when > '-fassociative-math' is specified? I'd think it should

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

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 122155. spatel added a comment. Patch updated: Match gcc's handling of -fassociative-math with -fno-signed-zeros and -fno-trapping-math. I've created a new codegen option (-mreassociate) that maps directly to LLVM's 'reassoc' flag, so we limit the mix-and-mat

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

2017-11-08 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 122158. spatel added a comment. Updated again - the last upload was missing the driver's test file. https://reviews.llvm.org/D39812 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CodeGenFunction.cpp li

[PATCH] D39611: [CodeGen] change const-ness of complex calls

2017-11-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Thanks for the clarification! If I'm reading this properly, we should make the same kind of change as in https://reviews.llvm.org/D39481 ('c' -> 'e') for most of complex.h. Ie, the standard allows errno-setting, and it's (unfortunately for optimization) even more clearl

[PATCH] D39611: [CodeGen] change const-ness of complex calls

2017-11-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 122474. spatel added a comment. Patch updated: Except for cimag, cproj, conj, and creal, everything in is marked 'e' - the functions are not const if we might set errno. https://reviews.llvm.org/D39611 Files: include/clang/Basic/Builtins.def test/CodeG

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Based on the comments in https://reviews.llvm.org/D39611, I think we have to change this (although I'd be happy to be wrong). 1. cbrt() can underflow and could set errno to ERANGE. 2. fma() can overflow or underflow and set errno to ERANGE. We could still make an excepti

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-10 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 122545. spatel added a comment. Patch updated: cbrt() is always const. fma() is always const with GNU or Win+MSVC. https://reviews.llvm.org/D39641 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp lib/Sema/SemaDecl.cpp test/CodeGen/li

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-11 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 122584. spatel marked an inline comment as done. spatel added a comment. Patch updated: 1. Fix predicate for detecting MSVC - isOSMSVCRT(). 2. Use switch on BuiltinID instead of string matching for "fma". https://reviews.llvm.org/D39641 Files: include/cla

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-13 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318093: [CodeGen] fix const-ness of cbrt and fma (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D39641?vs=122584&id=122727#toc Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics

2017-11-14 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. There are 20 LLVM math intrinsics that correspond to mathlib calls according to the LangRef: http://llvm.org/docs/LangRef.html#standard-c-library-intrinsics We were only converting 3 mathlib calls (sqrt, fma, pow) and 12 builtin

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

2017-11-15 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Ping. https://reviews.llvm.org/D39812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39611: [CodeGen] change const-ness of complex calls

2017-11-17 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Ping. https://reviews.llvm.org/D39611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39611: [CodeGen] change const-ness of complex calls

2017-11-18 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318598: [CodeGen] change const-ness of complex calls (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D39611?vs=122474&id=123467#toc Repository: rL LLVM https://reviews.llvm.o

[PATCH] D40230: Add -mprefer-vector-width driver option and attribute during CodeGen.

2017-11-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a reviewer: hfinkel. spatel added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.h:254 + /// The prefered vector width. + std::string PreferVectorWidth; typo - 'preferred'. Should add a bit more to the explanation. "The prefer

[PATCH] D40226: [CodeGen] Move Reciprocals option from TargetOptions to CodeGenOptions

2017-11-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D40226 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics

2017-11-21 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Ping. https://reviews.llvm.org/D40044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139006: [UpdateTestChecks] Match define for labels

2022-12-13 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added subscribers: arsenm, spatel. spatel added a comment. I agree with the revert - we'd need lots of commits like this: b7232dafe69eb04c14217 (cc @arsenm) Another possibility to fix this with less churn: Add a warnin

[PATCH] D139006: [UpdateTestChecks] Match define for labels

2022-12-16 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D139006#4000992 , @sebastian-ne wrote: > Adding `--function-signature` by default sounds like a good idea to me. > Is there any reason why we wouldn’t want to enable this by default (for new > tests)? No objection from me for

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

2017-03-28 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. This copies the text used in the #define statements to the code comments. The conflicting text comes from AMD manuals, but those are wrong. Sadly, this part has not been updated even after some docs were updated for Zen: http://s

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

2017-03-29 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 93366. spatel added a comment. Patch updated: Standardize the spelling of "non-signaling" and don't abbreviate "unordered". https://reviews.llvm.org/D31428 Files: lib/Headers/avxintrin.h Index: lib/Headers/avxintrin.h =

[PATCH] D23418: [analyzer] Added a reusable constraint system to the CloneDetector

2017-04-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D23418#719139, @NoQ wrote: > Hmm, reverted because i'm seeing crashes on some buildbots (works for me > though). > > It's crashing somewhere in `saveHash`, seems that some `Stmt`s are null. For > instance, > http://lab.llvm.org:8011/builders/

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

2017-04-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Ping. https://reviews.llvm.org/D31428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31428: [x86] fix AVX FP cmp intrinsic documentation (PR28110)

2017-04-12 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL300068: [x86] fix AVX FP cmp intrinsic documentation (PR28110) (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D31428?vs=93366&id=94976#toc Repository: rL LLVM https://review

<    1   2   3   >