[PATCH] D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc

spatel added a comment. About the bit hacking: I don't think clang should be in the optimization business. We should be able to take the most obvious/simple representation for this builtin and reduce it as needed (either in instcombine or the backend). So it would be better to use the version w

[PATCH] D50924: [CodeGen] add rotate builtins

spatel created this revision. spatel added reviewers: fabiang, craig.topper, rnk, thakis. Herald added a subscriber: mcrosier. This exposes the LLVM funnel shift intrinsics as more familiar bit rotation functions in clang (when both halves of a funnel shift are the same value, it's a rotate). I

[PATCH] D50924: [CodeGen] add rotate builtins

spatel updated this revision to Diff 161325. spatel added a comment. Patch updated: Fixed a docs typo. https://reviews.llvm.org/D50924 Files: docs/LanguageExtensions.rst include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp test/CodeGen/builtin-rotate.c Index: test/CodeGen/builtin

[PATCH] D50924: [CodeGen] add rotate builtins

spatel added a comment. In https://reviews.llvm.org/D50924#1204772, @rnk wrote: > Do you mind updating the _rotl* and _rotr* intrinsics to use the same > codegen? They're right above in the switch. Sure - I didn't know about those. https://reviews.llvm.org/D50924 _

[PATCH] D50924: [CodeGen] add rotate builtins

spatel updated this revision to Diff 161346. spatel added a comment. Patch updated: Update the existing Microsoft rotate builtins to also use the LLVM funnel shift intrinsics. https://reviews.llvm.org/D50924 Files: docs/LanguageExtensions.rst include/clang/Basic/Builtins.def lib/CodeGen/

[PATCH] D50924: [CodeGen] add rotate builtins

This revision was automatically updated to reflect the committed changes. Closed by commit rC340135: [CodeGen] add rotate builtins (authored by spatel, committed by ). Herald added a subscriber: kristina. Repository: rC Clang https://reviews.llvm.org/D50924 Files: docs/LanguageExtensions.rs

[PATCH] D50924: [CodeGen] add rotate builtins

spatel reopened this revision. spatel added a comment. This revision is now accepted and ready to land. Reopening because I reverted at https://reviews.llvm.org/rL340136. Not sure yet what is causing the problem on those bots. Repository: rC Clang https://reviews.llvm.org/D50924 _

[PATCH] D50924: [CodeGen] add rotate builtins

spatel added a comment. In https://reviews.llvm.org/D50924#1205310, @spatel wrote: > Reopening because I reverted at https://reviews.llvm.org/rL340136. Not sure > yet what is causing the problem on those bots. The common trait for those failures appears to be that the host compiler is: gcc ver

[PATCH] D50924: [CodeGen] add rotate builtins

This revision was automatically updated to reflect the committed changes. Closed by commit rL340137: [CodeGen] add/fix rotate builtins that map to LLVM funnel shift (retry) (authored by spatel, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llv

[PATCH] D50924: [CodeGen] add rotate builtins

spatel reopened this revision. spatel added a comment. This revision is now accepted and ready to land. Reopening again because I reverted this again for the same reason at https://reviews.llvm.org/rL340138. Repository: rL LLVM https://reviews.llvm.org/D50924 _

[PATCH] D50924: [CodeGen] add rotate builtins

This revision was automatically updated to reflect the committed changes. Closed by commit rL340141: [CodeGen] add rotate builtins that map to LLVM funnel shift (authored by spatel, committed by ). Changed prior to commit: https://reviews.llvm.org/D50924?vs=161398&id=161402#toc Repository:

[PATCH] D50924: [CodeGen] add rotate builtins

spatel added a subscriber: lei. spatel added a comment. And the 3rd time is the charm... I have no explanation for why this works, but I removed the microsoft diffs, and now gcc doesn't crash. cc'ing @lei as owner of the bots that have this problem: http://lab.llvm.org:8011/builders/clang-ppc64be

[PATCH] D46656: [Builtins] Improve the IR emitted for MSVC compatible rotr/rotl builtins to match what the middle and backends understand

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM - thanks! https://reviews.llvm.org/D46656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D47202: [CodeGen] use nsw negation for abs

spatel created this revision. spatel added reviewers: craig.topper, efriedma, lebedev.ri. Herald added a subscriber: mcrosier. The clang builtins have the same semantics as the stdlib functions. The stdlib functions are defined in section 7.20.6.1 of the C standard with: "If the result cannot be

[PATCH] D47202: [CodeGen] use nsw negation for abs

spatel added a comment. In https://reviews.llvm.org/D47202#1108016, @lebedev.ri wrote: > That is what happens for "hand-rolled" `abs`, https://godbolt.org/g/6dbgXD > I *think* this makes sense, since **IIRC** LLVM only supports > twos-complement platforms. > But all these attributes are curren

[PATCH] D47202: [CodeGen] use nsw negation for abs

This revision was automatically updated to reflect the committed changes. Closed by commit rL333038: [CodeGen] use nsw negation for builtin abs (authored by spatel, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47202?vs=148020&id=148

[PATCH] D47202: [CodeGen] use nsw negation for abs

This revision was automatically updated to reflect the committed changes. Closed by commit rC333038: [CodeGen] use nsw negation for builtin abs (authored by spatel, committed by ). Repository: rC Clang https://reviews.llvm.org/D47202 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/builtin-a

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

spatel added a subscriber: cfe-commits. spatel added a comment. Oops - I wrongly made llvm-commits a subscriber rather than cfe-commits. Let me know if I should reopen under a new thread to get the patch to hit the right mailing list. https://reviews.llvm.org/D39160

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

spatel created this revision. Herald added a subscriber: mcrosier. This patch is intended to answer a question raised in PR27435: https://bugs.llvm.org/show_bug.cgi?id=27435 Is a programmer using __builtin_sqrt() invoking the compiler's intrinsic definition of sqrt or the mathlib definition of s

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

spatel abandoned this revision. spatel added a comment. Reincarnated as https://reviews.llvm.org/D39204. https://reviews.llvm.org/D39160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

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

spatel added a comment. In https://reviews.llvm.org/D39204#904312, @efriedma wrote: > The gcc documentation says "GCC includes built-in versions of many of the > functions in the standard C library. These functions come in two forms: one > whose names start with the `__builtin_` prefix, and the

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

spatel added a comment. Working my way through the stack: does the sqrt LangRef change mean we can revert https://reviews.llvm.org/rL265521? What allows us to transform any of those libm calls that set errno to vectors in the first place? I'm even more confused than usual, but if I can find a w

[PATCH] D49606: [ms] Add __shiftleft128 / __shiftright128 intrinsics

spatel added a comment. In https://reviews.llvm.org/D49606#1170278, @craig.topper wrote: > Here are the IR patterns for this that work. Not sure if we can do this > directly in C, we need a 128 bit type, but maybe we can emit it from > CGBuiltin.cpp? > > define i64 @__shiftleft128(i64 %x, i64

[PATCH] D49606: [ms] Add __shiftleft128 / __shiftright128 intrinsics

spatel added a comment. In https://reviews.llvm.org/D49606#1170448, @thakis wrote: > Isn't implementing this in plain old C the nicest approach anyhow, even once > funnel shift exists? No, the primary reason for creating the intrinsic is that we can’t guarantee that we’ll recognize the patter

[PATCH] D33406: PR28129 expand vector oparation to an IR constant.

spatel added subscribers: scanon, andrew.w.kaylor. spatel added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:7932 break; case X86::BI__builtin_ia32_cmppd256: ID = Intrinsic::x86_avx_cmp_pd_256; dtemirbulatov wrote: > spatel wrote: > >

[PATCH] D33406: PR28129 expand vector oparation to an IR constant.

spatel added a comment. Functionally, I think this is correct and complete now. See inline for some nits. Comment at: lib/CodeGen/CGBuiltin.cpp:7925 // We can't handle 8-31 immediates with native IR, use the intrinsic. Intrinsic::ID ID; Fix comment

[PATCH] D33406: PR28129 expand vector oparation to an IR constant.

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D33406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL320920: [Driver, CodeGen] pass through and apply -fassociative-math (authored by spatel, committed by ). Changed prior to commit: https://reviews.llvm.org/D39812?vs=122158&id=127245#toc Repository: r

[PATCH] D45202: [X86] Replacing X86-specific floor and ceil vector intrinsics with generic LLVM intrinsics

spatel added a comment. In https://reviews.llvm.org/D45202#1126616, @craig.topper wrote: > I'm not sure whether we should be doing this here or in InstCombine. @spatel, > what do you think? It's been a while since I looked at these. Last memory I have is for the conversion from x86 masked ops

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

spatel added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10090-10100 +// _CMP_TRUE_UQ, _CMP_TRUE_US produce -1,-1... vector +// on any input and _CMP_FALSE_OQ, _CMP_FALSE_OS produce 0, 0... +if (CC == 0xf || CC == 0xb || CC == 0x1b || CC == 0x1f) { +

[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)

spatel created this revision. spatel added reviewers: gfalcon, lebedev.ri. Herald added a subscriber: mcrosier. https://bugs.llvm.org/show_bug.cgi?id=37778 ...shows a miscompile resulting from marking nan builtins as 'const'. The nan libcalls/builtins take a pointer argument: http://www.cplusplus

[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)

This revision was automatically updated to reflect the committed changes. Closed by commit rL334628: [CodeGen] make nan builtins pure rather than const (PR37778) (authored by spatel, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4813

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

spatel added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10107-10112 +case 0x0b: // FALSE_OQ +case 0x1b: // FALSE_OS + return llvm::Constant::getNullValue(ConvertType(E->getType())); +case 0x0f: // TRUE_UQ +case 0x1f: // TRUE_US + return llvm:

[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)

spatel added a comment. In https://reviews.llvm.org/D48134#1131626, @rsmith wrote: > Can we mark these as `argmemonly`? I wasn't aware of that one, but it sounds accurate for nan() and friends: argmemonly This attribute indicates that the only memory accesses inside function are loads a

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

spatel added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10107-10112 +case 0x0b: // FALSE_OQ +case 0x1b: // FALSE_OS + return llvm::Constant::getNullValue(ConvertType(E->getType())); +case 0x0f: // TRUE_UQ +case 0x1f: // TRUE_US + return llvm:

[PATCH] D60016: [InstCombine] canonicalize select shuffles by commuting

This revision was automatically updated to reflect the committed changes. Closed by commit rC357366: [InstCombine] canonicalize select shuffles by commuting (authored by spatel, committed by ). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https

[PATCH] D52586: [X86] Add the movbe instruction intrinsics from icc.

spatel added a comment. The struct hack isn't obvious to me. Without that, we would produce a load with default alignment based on the size of the load (i132 -> align 4, etc)? But we want to force align 1 regardless of the load size, so the __packed__ attribute on the struct gets us that IIUC.

[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c

spatel added a comment. In https://reviews.llvm.org/D30806#1248575, @efriedma wrote: > This was merged in https://reviews.llvm.org/rC298491 . But it got reverted soon after due to miscompiles: https://reviews.llvm.org/rL298695 And the functionality hasn't been reimplemented AFAICT. https://r

[PATCH] D52586: [X86] Add the movbe instruction intrinsics from icc.

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D52586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

spatel added a comment. I skimmed D63174 but haven't applied either of these patches to test locally, so I may not have the full picture. IMO, we do not want clang regression tests running -instcombine/-instsimplify. That can cause clang tests to break when an

[PATCH] D63793: Treat the range of representable values of floating-point types as [-inf, +inf] not as [-max, +max].

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. I don't have much experience with the front-end and have no experience with the sanitizers, but the changes match my understanding for this type of cast/conversion, so LGTM. ===

[PATCH] D61472: [X86FixupLEAs] Turn optIncDec into a generic two address LEA optimizer. Support LEA64_32r properly.

spatel added a comment. I haven't looked closely at the series of transforms that gets us here, so let me ask: would it be more efficient to produce the add/inc/dec machine instructions directly rather than LEA? Or do we do this because the 3-address opportunity helps register allocation, so th

[PATCH] D61472: [X86FixupLEAs] Turn optIncDec into a generic two address LEA optimizer. Support LEA64_32r properly.

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. In D61472#1516642 , @craig.topper wrote: > In D61472#1515994 , @spatel wrote: > > > I haven't looked closely a

[PATCH] D55879: [X86][SSE] Auto upgrade PADDUS/PSUBUS intrinsics to UADD_SAT/USUB_SAT generic intrinsics (clang)

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM - I haven't looked in a while to see if it works, but I thought we have a script to auto-generate the CHECK lines for C-->IR tests, so we wouldn't need to 'CHECK-NOT' in these functions.

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel created this revision. spatel added reviewers: jgorbe, chandlerc, scanon, hans, echristo. Herald added a subscriber: mcrosier. As discussed in the post-commit thread for: https://reviews.llvm.org/rL330437 ( http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180423/545906.html ) We

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel added inline comments. Comment at: test/CodeGen/no-junk-ftrunc.c:1 +// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s + lebedev.ri wrote: > For a good measure, i'd add one more `RUN` line to test that it is currently >

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel added inline comments. Comment at: docs/UsersManual.rst:1260-1265 + Enable or disable a code generation optimization that may convert a + cast of a floating-point value to integer and back to floating-point + into the equivalent of the math libary's 'trunc()' functio

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel updated this revision to Diff 144200. spatel added a comment. Patch updated: 1. Improve the documentation language - more suggestions welcome! 2. Change the default setting so the work-around is 'off' (ie, by default assume source is compliant and optimize accordingly). 3. Remove the 'no'

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel updated this revision to Diff 144226. spatel added a comment. Patch upated: 1. Restore the 'no' option to allow toggling. 2. Add a RUN to the codegen test to show that the function attribute is not appended by default. https://reviews.llvm.org/D46135 Files: docs/UsersManual.rst inc

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

This revision was automatically updated to reflect the committed changes. Closed by commit rC331041: [Driver, CodeGen] add options to enable/disable an FP cast optimization (authored by spatel, committed by ). Changed prior to commit: https://reviews.llvm.org/D46135?vs=144226&id=144342#toc Rep

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel added a comment. Should I add a bullet for this new flag/attribute to the clang release notes, the LLVM release, both? Or do we view this as temporary and not making it to the release? Repository: rC Clang https://reviews.llvm.org/D46135 ___

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

spatel added a comment. In https://reviews.llvm.org/D46135#1081193, @lebedev.ri wrote: > (Not everyone is always using trunk snapshots) > I'd guess it would be present for at least one release, so **I** would > expect to see that in both the clang and llvm release notes. Thanks - sounds right

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

spatel created this revision. spatel added reviewers: rsmith, chandlerc, scanon, hans, echristo, jgorbe, lebedev.ri. Herald added a subscriber: mcrosier. As suggested in the post-commit thread for https://reviews.llvm.org/rL331056, we should match these clang options with the established vocabul

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

spatel updated this revision to Diff 144481. spatel added a comment. Patch updated: I missed a spot in the release notes - forgot to delete 'workaround' when listing the new options. https://reviews.llvm.org/D46236 Files: docs/ReleaseNotes.rst docs/UsersManual.rst include/clang/Driver/Op

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

spatel updated this revision to Diff 144487. spatel marked 6 inline comments as done. spatel added a comment. Patch updated: 1. Improved language in docs. 2. Added help text for clang options. 3. Added driver test with -fstrict-float-cast-overflow specified explicitly. I'll leave this up for fur

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

spatel updated this revision to Diff 144551. spatel added a comment. Patch updated: Added "By default" to the description in the user manual and release notes to make it clearer how this behaves. https://reviews.llvm.org/D46236 Files: docs/ReleaseNotes.rst docs/UsersManual.rst include/cl

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

This revision was automatically updated to reflect the committed changes. Closed by commit rL331209: [Driver, CodeGen] rename options to disable an FP cast optimization (authored by spatel, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

This revision was automatically updated to reflect the committed changes. Closed by commit rC331209: [Driver, CodeGen] rename options to disable an FP cast optimization (authored by spatel, committed by ). Repository: rL LLVM https://reviews.llvm.org/D46236 Files: docs/ReleaseNotes.rst do

[PATCH] D46328: [X86] Mark all x86 specific builtins as nothrow.

spatel added a comment. Seems right, but I don't know much about nothrow. Does some other attribute on these intrinsics suppress the possibility of throwing an exception? Ie, is there some way to test this? I can't seem to produce catch/landingpad IR even after putting an intrinsic in a 'try'

[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates

spatel added inline comments. Comment at: test/CodeGen/avx-builtins.c:1423 - -__m256 test_mm256_cmp_ps_true(__m256 a, __m256 b) { - // CHECK-LABEL: @test_mm256_cmp_ps_true Why are we deleting tests instead of replacing the CHECK lines with the new output? Rep

[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates

spatel added inline comments. Comment at: test/CodeGen/avx-builtins.c:1423 - -__m256 test_mm256_cmp_ps_true(__m256 a, __m256 b) { - // CHECK-LABEL: @test_mm256_cmp_ps_true GBuella wrote: > spatel wrote: > > Why are we deleting tests instead of replacing the CHEC

[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D48715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D80251: [X86] Update some av512 shift intrinsics to use "unsigned int" parameter instead of int to match Intel documentaiton

spatel added a comment. Is it possible to fix those other 5 in the Intel docs for consistency, or is there some functional reason that those are different? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80251/new/ https://reviews.llvm.org/D80251 __

[PATCH] D80584: [utils] change update_test_checks.py use of 'TMP' value names

This revision was automatically updated to reflect the committed changes. Closed by commit rGe5b877275673: [utils] change default nameless value to "TMP" (authored by spatel). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D80584: [utils] change update_test_checks.py use of 'TMP' value names

spatel reopened this revision. spatel added a comment. This revision is now accepted and ready to land. Effectively reverted (but still have the name conflict warning at least) with: rGe5b877275673 If we can change instnamer,

[PATCH] D80584: [utils] change update_test_checks.py use of 'TMP' value names

spatel closed this revision. spatel added a comment. In D80584#2066319 , @jdoerfert wrote: > First, I think the warning is strictly a good thing. Thanks for keeping that > in! Sure - we've been missing that for a long time. > I don't think the options

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

spatel added a comment. In D78478#2128631 , @xbolva00 wrote: > >> UTC_ARGS (I can't help associating it with Coordinated Universal Time). > > Me too. Any suggestions for new name? TCU_ARGS? I also reflexively think of universal time when reading "UTC". I

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

spatel added a comment. In D85787#2214038 , @lebedev.ri wrote: > @ reviewers - i'm not so much interested in deep code/algo review, > but more like in the general direction disscussion, like, is this okay for > instcombine? :) The test diffs look great,

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. This seems similar in spirit to the recursive element tracking that we do in collectShuffleElements() in this file or foldIdentityShuffles() in InstSimplify, but a bit more complicated (becaus

[PATCH] D79472: [X86] Remove support some inline assembly constraints that are no longer supported by gcc.

spatel added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:154-156 +BasicBlock::iterator It(New); +while (isa(*It) || isa(*It)) + ++It; Unrelated diff? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D40230 ___ 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

spatel added a comment. Ping * 2. 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] D40594: [InstCombine] miscompile of __builtin_fmod

spatel added reviewers: efriedma, hfinkel. spatel added a comment. I don't know the history of the frem instruction in IR, and the description in http://llvm.org/docs/LangRef.html#frem-instruction is vague. But based on the existing code, I think this is working as intended. If the instruction

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

spatel added a comment. Ping * 2. 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] D40594: [InstCombine] miscompile of __builtin_fmod

spatel added a comment. Side note: I think there is a different bug here in clang because from what I can tell, we convert the builtin or libcall to 'frem' even when errno could be set by the call. https://reviews.llvm.org/D40044 doesn't address this case because frem is an LLVM instruction rat

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

spatel added a comment. Added a blurb to the LangRef with https://reviews.llvm.org/rL319437, so I think we can abandon this patch. https://reviews.llvm.org/D40594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL319593: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics (authored by spatel). Changed prior to commit: https://reviews.llvm.org/D40044?vs=122881&id=125230#toc Repository: rL LL

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC319593: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics (authored by spatel). Repository: rC Clang https://reviews.llvm.org/D40044 Files: lib/CodeGen/CGBuiltin.cpp test/Code

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

spatel added a comment. Ping * 3. 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] D39812: [Driver, CodeGen] pass through and apply -fassociative-math

spatel added a comment. Ping * 4. 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] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. A few changes for tests suggested inline. There might be some generalization of ctpop analysis that we can make as a follow-up patch. For example, I was looking at a "wrong predicate" combination and noticed that we miss possible optimizations like this: https://alive2.l

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. Thank you for the making the test changes. I pushed the baseline tests on your behalf here: ebaa28e0750b Please rebase and update the patch here in Phabricator - it should only show changes in the CHE

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. In D122077#3413565 , @xbolva00 wrote: > In D122077#3413550 , @joerg wrote: > >> Why is this fold preferable to `(X & (X-1)) == 0`? At least on all >> architectures without native populatio

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915 +/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and +/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2). +static Value *foldIsPowe

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

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/D122077/new/ https://reviews.llvm.org/D122077 ___

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa3cffc11509b: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2 (authored by hkmatsumoto, committed by spatel). Repository: rG LLVM

[PATCH] D130374: [Passes] add a tail-call-elim pass near the end of the opt pipeline

spatel added a comment. In D130374#3679550 , @hiraditya wrote: > In D130374#3675677 , @vdsered wrote: > >> Just JFYI :) >> Yes, probably not worth it > > that is interesting. do we know why? Based on this data:

[PATCH] D121306: [Sema] add warning for tautological FP compare with literal

This revision was automatically updated to reflect the committed changes. Closed by commit rGab982eace6e4: [Sema] add warning for tautological FP compare with literal (authored by spatel). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:916 +/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2). +static Value *foldOrOfCtpop(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd, +I

[PATCH] D124551: [Driver] Add f16 support to -mrecip parsing.

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM. AFAIK, these FP reciprocal options have never been officially documented. Would that go under here: https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation ? Repositor

[PATCH] D130374: [Passes] add a tail-call-elim pass near the end of the opt pipeline

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbfb9b8e075ee: [Passes] add a tail-call-elim pass near the end of the opt pipeline (authored by spatel). Herald added subscribers: cfe-commits, pmatos

[PATCH] D72028: [CodeGen] Emit conj/conjf/confjl libcalls as fneg instructions if possible.

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. Code change LGTM - see inline for a question about the testing. Comment at: clang/test/CodeGen/complex-libcalls.c:115-118 +// NO__ERRNO-NOT: .conj +// NO__ERRNO-NOT: @conj +/

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

spatel 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 minimum FMF needed for this transform - the 'c

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

spatel added a comment. In D87188#2281957 , @sanwou01 wrote: > I know this has already been reverted but just FYI that I've bisected a ~2% > regression in SPEC2017 x264_r on AArch64 to this commit. Presumably this is > due to the extra unrolling / cost m

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM - I think the odds of code that is using intrinsic vector abs inter-mixing with auto-vectorized code using the abs idiom are small. I noticed one other LLVM codegen test that might want t

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

spatel added a comment. In D87188#2445506 , @lebedev.ri wrote: > Partial rebase (without updating test coverage) Thanks for reducing! If I'm seeing it correctly, the codegen looks fine - the difference is in the IR icmp predicate changing from `slt` to `

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

spatel added a comment. If we're going by existing behavior/compatibility, gcc/icc use packed ops too: https://godbolt.org/z/9jEhaW ...so there's an implicit 'nnan nsz' in these intrinsics (and that should be documented in the header file (and file a bug for Intel's page at https://software.inte

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

spatel added reviewers: nlopes, regehr, RKSimon, zhengyangl, nikic, hfinkel. spatel added a comment. Thank you for working on this! Looking back at the bug comments (and adding reviewers based on those comments), this is a step towards killing undef that has been discussed for a long time now. :

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

spatel added a comment. In D93586#2467248 , @aqjune wrote: >> There are 792 files in llvm/test, most of which are in test/Transform (119) >> and test/CodeGen(655). >> The transition will be swiftly done (if there's no other issue hopefully) by >> the nex

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

spatel added a comment. In D87188#2348343 , @spatel wrote: > 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

  1   2   3   >