Re: r263344 - clang-cl: Add a test for the interaction of /Yc and /showIncludes.

On 13 March 2016 at 02:56, Nico Weber via cfe-commits wrote: > Author: nico > Date: Sat Mar 12 13:55:59 2016 > New Revision: 263344 > > URL: http://llvm.org/viewvc/llvm-project?rev=263344&view=rev > Log: > clang-cl: Add a test for the interaction of /Yc and /showIncludes. > > We almost get this ri

Re: r263464 - CodeGen: Mark functions used in vtables as unnamed_addr.

On 14 March 2016 at 18:42, Peter Collingbourne via cfe-commits wrote: > Author: pcc > Date: Mon Mar 14 13:41:59 2016 > New Revision: 263464 > > URL: http://llvm.org/viewvc/llvm-project?rev=263464&view=rev > Log: > CodeGen: Mark functions used in vtables as unnamed_addr. > > This marks virtual func

Re: r263895 - P0184R0: Allow types of 'begin' and 'end' expressions in range-based for loops to differ.

On 20 March 2016 at 11:33, Richard Smith via cfe-commits wrote: > Author: rsmith > Date: Sun Mar 20 05:33:40 2016 > New Revision: 263895 > > URL: http://llvm.org/viewvc/llvm-project?rev=263895&view=rev > Log: > P0184R0: Allow types of 'begin' and 'end' expressions in range-based for > loops to di

Re: [PATCH] D18311: Clang tests for ARM Cortex-A32 support.

rengolin added inline comments. Comment at: test/Preprocessor/arm-target-features.c:349 @@ -349,1 +348,3 @@ +// Test whether predefines are as expected when targeting ARMv8-A Cortex implementations +// RUN: %clang -target armv8 -mcpu=cortex-a32 -x c -E -dM %s -o - | FileCheck --

Re: [PATCH] D18311: Clang tests for ARM Cortex-A32 support.

rengolin accepted this revision. rengolin added a reviewer: rengolin. rengolin added a comment. This revision is now accepted and ready to land. Hi Sam, The patch looks good, thanks! A few comments on the process: 1. When you send an update, send the whole patch again, not just the new diff. E

Re: [PATCH] D18311: Clang tests for ARM Cortex-A32 support.

rengolin added a comment. Right, if you could update both reviews with the full patch, I could commit them for you, for now. http://reviews.llvm.org/D18311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

Re: [PATCH] D18311: Clang tests for ARM Cortex-A32 support.

rengolin closed this revision. rengolin added a comment. Committed in r263957. http://reviews.llvm.org/D18311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r263957 - [ARM] Clang tests for ARM Cortex-A32 support

Author: rengolin Date: Mon Mar 21 12:29:51 2016 New Revision: 263957 URL: http://llvm.org/viewvc/llvm-project?rev=263957&view=rev Log: [ARM] Clang tests for ARM Cortex-A32 support Patch by Sam Parker. Modified: cfe/trunk/test/CodeGen/arm-target-features.c cfe/trunk/test/Driver/arm-cortex

Re: [PATCH] D18391: Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Nice cleaning! LGTM. Thanks! http://reviews.llvm.org/D18391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

Re: [PATCH] D15883: Add ARM EHABI-related constants to unwind.h.

rengolin added a comment. Tests? http://reviews.llvm.org/D15883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15883: Add ARM EHABI-related constants to unwind.h.

rengolin added a comment. Well, I only saw later that these are propositions from another patch... I don't see why this can't be part of the original patch, but I'm ok with no tests if they're used (and tested) on the final patch. I'll defer to Logan to decide. :) http://reviews.llvm.org/D158

Re: r257754 - PR25910: clang allows two var definitions with the same mangled name

On 14 January 2016 at 10:41, Andrey Bokhanko via cfe-commits wrote: > Author: asbokhan > Date: Thu Jan 14 04:41:16 2016 > New Revision: 257754 > > URL: http://llvm.org/viewvc/llvm-project?rev=257754&view=rev > Log: > PR25910: clang allows two var definitions with the same mangled name Hi Andrey,

Re: r257934 - [CMake] Support generation of linker order files using dtrace

On 15 January 2016 at 21:21, Chris Bieneman via cfe-commits wrote: > Author: cbieneman > Date: Fri Jan 15 15:21:12 2016 > New Revision: 257934 > > URL: http://llvm.org/viewvc/llvm-project?rev=257934&view=rev > Log: > [CMake] Support generation of linker order files using dtrace > > Summary: > This

Re: r257934 - [CMake] Support generation of linker order files using dtrace

It did, thanks! On 15 January 2016 at 23:28, Chris Bieneman wrote: > That should be fixed with r257948. > > -Chris > >> On Jan 15, 2016, at 3:19 PM, Renato Golin wrote: >> >> On 15 January 2016 at 21:21, Chris Bieneman via cfe-commits >> wrote: >>>

Re: r258720 - [MSVC Compat] Only warn for unknown clang-cl arguments

On 25 January 2016 at 21:14, Ehsan Akhgari via cfe-commits wrote: > Author: ehsan > Date: Mon Jan 25 15:14:52 2016 > New Revision: 258720 > > URL: http://llvm.org/viewvc/llvm-project?rev=258720&view=rev > Log: > [MSVC Compat] Only warn for unknown clang-cl arguments Hi Eshan, Not sure you've see

Re: r258720 - [MSVC Compat] Only warn for unknown clang-cl arguments

On 26 January 2016 at 01:56, Ehsan Akhgari wrote: > OK, I reverted the test in r258772. Thanks! Ok, the error code with no output will make for an interesting investigation... :) I'll try to collect as much info as possible. In the meantime, if you have any insights, just let me know. cheers,

Re: [PATCH] D14471: [AArch64] Fix a crash in driver

rengolin added a comment. In http://reviews.llvm.org/D14471#288621, @ahatanak wrote: > In getAArch64TargetCPU, if it finds out the cpu name passed via -mtune or > -mcpu is "native", > > 1. Call llvm::sys::getHostCPUName to get the host CPU name. > 2. Check the host CPU name to see if it is a val

Re: [PATCH] D14662: [ARM] Pass architecture to TargetParser defaulting to cope with API change

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rL LLVM http://reviews.llvm.org/D14662 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

Re: [PATCH] D14570: Handle ARMv6KZ naming

rengolin added a comment. In http://reviews.llvm.org/D14570#288132, @compnerd wrote: > Wow, this is tricky: the code change is in LLVM, and test change in clang :(. > However, this does seem to preserve the features. The problem is that Clang is the most important user of this library, which

Re: [PATCH] D14609: [ARM, AArch64] Fix __rev16l and __rev16ll intrinsics

rengolin accepted this revision. rengolin added a reviewer: rengolin. rengolin added a comment. This revision is now accepted and ready to land. LGTM. The 64-bit rev pattern can be added later. Thanks! Repository: rL LLVM http://reviews.llvm.org/D14609 _

Re: r253269 - Make FP_CONTRACT ON the default.

On 16 November 2015 at 23:09, Stephen Canon via cfe-commits wrote: > Author: scanon > Date: Mon Nov 16 17:09:11 2015 > New Revision: 253269 > > URL: http://llvm.org/viewvc/llvm-project?rev=253269&view=rev > Log: > Make FP_CONTRACT ON the default. > > Differential Revision: D14200 Hi Stephen, It

Re: r253269 - Make FP_CONTRACT ON the default.

On 17 November 2015 at 15:47, Manuel Klimek wrote: > Reverted in r253337. Failing test case in commit message. And the bot is green again! :) Thanks! --renato ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

Re: [PATCH] D14758: Handle ARMv7K as an alias, instead of fake architecture (NFC)

rengolin requested changes to this revision. This revision now requires changes to proceed. Comment at: lib/Driver/Tools.cpp:6298 @@ -6297,1 +6297,3 @@ unsigned ArchKind; + if (Arch == "armv7k" || Arch == "thumbv7k") +return "v7k"; // In other cases, Cortex-A7 should resol

Re: [PATCH] D14756: Handle ARMv6-J as an alias, instead of fake architecture

rengolin added a comment. Same comment as http://reviews.llvm.org/D14755. Whenever that's approved, so it this. http://reviews.llvm.org/D14756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

Re: [PATCH] D14773: [ARM] Support +feature targeting in -mcpu/-march

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. This looks good to me. Thanks! Repository: rL LLVM http://reviews.llvm.org/D14773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D14804: [clang] Disable Unicode in asm files

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. LGTM, too. Thanks! Repository: rL LLVM http://reviews.llvm.org/D14804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

Re: r253859 - Fix calculation of shifted cursor/code positions. Specifically support

On 23 November 2015 at 08:33, Daniel Jasper via cfe-commits wrote: > Author: djasper > Date: Mon Nov 23 02:33:48 2015 > New Revision: 253859 > > URL: http://llvm.org/viewvc/llvm-project?rev=253859&view=rev > Log: > Fix calculation of shifted cursor/code positions. Specifically support > the case w

Re: r253859 - Fix calculation of shifted cursor/code positions. Specifically support

On 23 November 2015 at 10:41, Daniel Jasper wrote: > Seen and fixed, I think.. Still looks broken... :) http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15/builds/7684 http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/2616 http://lab.llvm.org:8011/builders/clang-cmake-aa

Re: r253859 - Fix calculation of shifted cursor/code positions. Specifically support

On 23 November 2015 at 22:31, Daniel Jasper wrote: > Fixed in r253929. Thanks! ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 11:44, James Molloy via cfe-commits wrote: > In summary, I agree with you that we need tests for both Clang and LLVM > separately. However I also think the full-trip tests add significant value > and wouldn't like to see them removed, and there's significant prior art in > th

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 17:23, James Molloy via cfe-commits wrote: > This isn't just a NEON intrinsics thing, and this isn't just an ARM/AArch64 > thing. There needs to be some way to test the compiler from start to finish. > Not being able to do so leaves serious coverage holes. Just for the sake

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 18:29, David Blaikie wrote: > Are they things the test-suite couldn't (either technically or > philosophically) cover, or only that it doesn't cover it at the moment, but > could do so? IMO, it's a philosophical issue. The test-suite is a whole-program execution, and all tha

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 19:09, David Blaikie wrote: > Not sure I follow - I'm not suggesting adding objdump/instruction checking > to existing large programs in the test-suite, but adding other tests to the > test-suite that do this and have appropriate input for it to be a > reliable/useful form of

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 19:42, James Molloy wrote: > Why do you think it would be non trivial? Some simple lit tests aren't > exactly arduous on most targets. I mean having more points in the testing matrix. Clang check-all is cheaper than running the test-suite, but if we start moving more tests

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 19:45, David Blaikie wrote: > These cases I don't understand - if they're features LLVM supports we'd just > test them straight up. If you run our test suite against another compiler it > should tell you if it's compatible with our compiler (does it offer the same > functiona

Re: r254251 - ARM v8.1a adds Advanced SIMD instructions for Rounding Double Multiply

On 1 December 2015 at 19:53, James Molloy wrote: > I think I'd be OK with demoting these tests to the test-suite, and dealing > with the slightly lower amount of testing that comes with it if it means we > can keep the clang tests in a nice shape. If the tests are independent of sub-architecture

Re: [PATCH] D15142: Teaches clang about Cortex-A35.

rengolin accepted this revision. rengolin added a reviewer: rengolin. rengolin added a comment. This revision is now accepted and ready to land. Isn't it time we move AArch64 to the target parser, too? Anyway, as it is, LGTM, for the time being. Thanks! Repository: rL LLVM http://reviews.llv

Re: [llvm-dev] Phabricator Creator Pulling the Plug

On Wed, 18 Aug 2021 at 18:17, MyDeveloper Day via llvm-dev < llvm-...@lists.llvm.org> wrote: > But unless I missed this, was there any discussion regarding the recent > "Winding Down" announcement of Phabricator? and what it might mean for us > in LLVM > I think we have our own self-hosted versio

Re: [llvm-dev] Phabricator Creator Pulling the Plug

On Mon, 23 Aug 2021 at 18:56, James Y Knight wrote: > If phabricator/phorge do turn out to be non-viable in the future, I think > we may want to reopen the option of moving to Gerrit for the primary > code-review platform. > > I'll note that the Golang folks are using Gerrit as their review platf

Re: [llvm-dev] Phabricator Creator Pulling the Plug

On Tue, 24 Aug 2021 at 12:49, Aaron Ballman wrote: > > A minor issue is that the messages Gerrit sends to Github are a bit > pointless "Message from PersonA: (1 comment)". It would be better if the > integration either works (like adding comments to a specific line or > updating the commits) or n

Re: [llvm-dev] Phabricator Creator Pulling the Plug

On Tue, 24 Aug 2021 at 17:30, James Y Knight wrote: > Yes, the Gerrit hosting which Go uses ("googlesource.com") only permits a > google-account login as a matter of policy. But that's not a restriction of > Gerrit itself -- it can perfectly well be configured to use a github login. > Ah, awesom

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

rengolin added a comment. In https://reviews.llvm.org/D38479#886587, @efriedma wrote: > 1. We don't correctly ignore inline asm clobbers for registers which aren't > allocatable (https://bugs.llvm.org/show_bug.cgi?id=30792) This looks like a different (but related) issue. That should be fixed

[PATCH] D52595: [ARM] Alter test to account for change to armv6k default CPU

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Thanks Peter. LGTM! https://reviews.llvm.org/D52595 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler

rengolin added a comment. Hi Peter, Looks ok. Why did you need to change all cmdlines to have -EL? I imagine you just need one for each case, everything else remains the default (which should still work). Also, it would be interesting to know what happens on cases like "aarch64_be -EL" et al,

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

rengolin requested changes to this revision. rengolin added a comment. This revision now requires changes to proceed. Since the previous version was approved already, I'm "requesting changes" so that we can look at it again, together with D53927 to make sure it'

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

rengolin added a comment. @hfinkel There are changes in this patch, as well as its LLVM counterpart D53927 . Please, re-review. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53928/new/ https://reviews.llvm.org/D53928 _

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

rengolin added a comment. Herald added a subscriber: jdoerfert. Funny thing is, SVML is also only supported, AFAIK, for Intel. I agree that we should emit errors, but we should also emit a similar error on SVML. I know it's not entirely relevant to this patch, but we should keep the behaviour c

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53928/new/ https://reviews.llvm.org/D53928 ___ c

[PATCH] D23610: [ARM] Add pre-defined macros for ROPI and RWPI

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Sorry for the delay, this fell out of my radar and just saw the ping now. Given it's in ACLE and there are only mechanical (obvious) changes, LGTM. I'm assuming those two parameters are al

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

rengolin added a comment. I understand (and agree with) the reasoning of this patch, but wouldn't this also make it harder to test the current behaviour? I mean, LLVM doesn't support officially SVE entirely, so there's no point in expecting anything to work for now. What is the coverage of the

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. I see, makes sense. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65404/new/ https://reviews.llvm.org/D65404 ___ cfe-c

[PATCH] D65572: Fix static linking failure with --unwindlib=libunwind

rengolin added reviewers: compnerd, mstorsjo, jroelofs, theraven. rengolin added a comment. This is a tricky one which may vary depending on the libraries available on different systems. Which toolchain is this? Can you add more context? Repository: rC Clang CHANGES SINCE LAST ACTION https

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

rengolin added reviewers: rsmith, chandlerc, rnk, ABataev. rengolin added a comment. Adding clang/omp developers for proper review. Please feel free to add more. FYI, there are four main ongoing discussions on the LLVM thread (D53927 ): 1. There is collusion bet

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

rengolin added a comment. +1 to Chris, Mehdi and John comments. I think I get the idea, but the text is certainly not conveying that, for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77683/new/ https://reviews.llvm.org/D77683 _

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

rengolin accepted this revision. rengolin added a comment. Agree. Even 10 years ago we made the concerted effort not to care about pre-v4, so I'd be a little surprised if people are actually using modern clang to target those platforms. Projects that rely on it can work in the same way as gcc a

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

rengolin added a comment. I think we can safely say that we care less about jazelle than we care about armv1/2/3, so feel free to ignore that, too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133109/new/ https://reviews.llvm.org/D133109 ___

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

rengolin added a comment. I just applied this to a recent HEAD and got a few warnings. Please make sure there are no new warnings on changes / new files. /home/rengolin/devel/llvm-project/llvm/lib/Target/CSKY/CSKYInstrFormats.td:658:62: warning: unused template argument: R_Z_2:pattern clas

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. In D121445#3418195 , @zixuan-wu wrote: > I have met this before because the downloading of patch will ignore empty > files. You can have a check t

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

rengolin added a comment. I don't know enough about your toolchain requirements, but this looks good to me. Please check the clang-format warnings. If you did pass clang-format, perhaps you need to upgrade to a newer one? I won't approve just yet, to let other people review it also. ===

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

rengolin added inline comments. Comment at: clang/test/Driver/csky-arch-error.c:1 +// RUN: %clang -target csky-unknown-elf -march=csky -### %s \ +// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=CSKY %s This will error out and fail the test. You need to add a

[PATCH] D121445: [Clang][CSKY] Add the CSKY target and compiler driver

rengolin added a comment. I'm surprised these tests are passing for you. Perhaps you're not building or running them all. To make sure you're running your tests, you need to build both clang and llvm (`-DLLVM_ENABLE_PROJECTS=clang`) and run ninja/make `check-all`. You can also run `lit` direct

[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

rengolin added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_w

[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

rengolin added inline comments. Comment at: clang/test/Sema/builtin-alloca-with-align.c:32 void test8(void) { +#if defined(__csky__) __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_w

[PATCH] D130255: [Clang][LoongArch] Add initial LoongArch target and driver support

rengolin added a comment. This looks great, thanks! Exciting that we can finally go all the way from C source to LoongArch binary. The changes look good to me, other than a few nits. But please wait for a day or two for other people to review on their own time. Comment at: c

[PATCH] D35826: [Driver] Error if ARM mode was selected explicitly for M-profile CPUs.

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. This makes sense to me. LGTM. Thanks! https://reviews.llvm.org/D35826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 support

rengolin added a comment. This change seems to have nothing to do with adding core support but exposing features from CPU names. If this was required to "support" the new cores in Clang, why wasn't it needed before? If this is a new, more generic way, of getting support, shouldn't you remove

[PATCH] D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 tests

rengolin added a comment. I'm happy with the patch, but I'll let @SjoerdMeijer approve. https://reviews.llvm.org/D36731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

rengolin added inline comments. Comment at: clang/include/clang/Driver/Options.td:1564 def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>, -HelpText<"Use the given vector functions library">, Values<"Accelerate,MASSV,SVML,none">; +HelpText<"Use the given

[PATCH] D87338: [Driver][PGO] Driver support for de-Optimizing cold functions (PATCH 2/2)

rengolin added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2021 + +Adds ``optnone`` attributes to functions whose instrumentation-based PGO profiling counts are equal to zero (i.e. "cold"). + Following discussions in the mailing list, I

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

rengolin accepted this revision. rengolin added a comment. LGTM too, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88394/new/ https://reviews.llvm.org/D88394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

rengolin added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2089 + static const char *const M68kTriples[] = { + "m68k-linux-gnu", "m68k-unknown-linux-gnu", "m68k-suse-linux"}; + jrtc27 wrote: > rengolin wrote: > > The front-end supports

[PATCH] D90956: [clang][SVE] Additional macros implied by `-msve-vector-bits=`.

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Simple and straightforward, with documentation! :) LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90956/new/ https://reviews.l

[PATCH] D90956: [clang][SVE] Activate macro `__ARM_FEATURE_SVE_VECTOR_OPERATORS`.

rengolin accepted this revision. rengolin added a comment. Hi @fpetrogalli, the document is so dense that it took me a while to check the macros and I was still wrong. Either I'm losing my skill to read Arm documents or folks are getting worse at writing them. Giving this is a change that only

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

rengolin added inline comments. Comment at: clang/lib/Basic/Targets.cpp:314 +default: + return new M68kTargetInfo(Triple, Opts); +} No support for bare-metal? Comment at: clang/lib/Basic/Targets/M68k.cpp:123 +"d0", "d1", "d2",

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

rengolin added inline comments. Comment at: clang/include/clang/Driver/Options.td:3125 +foreach i = {0-4} in + def m680#i#0 : Flag<["-"], "m680"#i#"0">, Group; Same question as @RKSimon had below: Shouldn't this cover all models the back-end recognises? ===

[PATCH] D87981: [X86] AMX programming model.

rengolin added a comment. You should at least get @craig.topper's feedback, given this is a significant change in the x86 backend. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87981/new/ https://reviews.llvm.org/D87981 __

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

rengolin added a comment. Thanks for the changes. This looks good to me but I'll let others check again and approve. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88393/new/ https://reviews.llvm.org/D88393 ___ cfe-commits mailing list cfe-co

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

rengolin added inline comments. Comment at: clang/include/clang/Driver/Options.td:164 +def m_m68k_Features_Group: OptionGroup<"">, + Group, DocName<"M68k">; def m_mips_Features_Group : OptionGroup<"">, craig.topper wrote: > bruno wrot

[PATCH] D83088: Introduce CfgTraits abstraction

rengolin added a comment. I have read all of the comments in this review and have looked at all the other pending reviews because of this and I still see strong disagreement on the design and implementation. Unfortunately, I can't contribute with the technical discussion because there's a lot

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

rengolin accepted this revision. rengolin added a comment. W00t! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132550/new/ https://reviews.llvm.org/D132550 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D132285: [Clang][LoongArch] Implement ABI lowering

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. I can't vouch for your ABI correctness, but you seem to have a lot of tests covering what you claim to have implemented, so looks good to me. Thanks for opening an issue for RISC-V. Clang

[PATCH] D134157: [Clang][LoongArch] Add inline asm support for constraints f/l/I/K

rengolin added inline comments. Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:1958 + // constraints while the official register name is prefixed with a '$'. + // So we manually select general purpose registers here. + // For now, no need to support ABI names

[PATCH] D134157: [Clang][LoongArch] Add inline asm support for constraints f/l/I/K

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Thanks, looks good to me! I wouldn't split this in two commits. One of the benefits of having a monorepo is that we don't have to do that anymore. :) Comment at: llvm/l

[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

rengolin accepted this revision. rengolin added a comment. Looks good, with some nits. Thanks! Comment at: llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp:76 + // TODO: handle extra code. + if (!ExtraCode) { +const MachineOperand &BaseMO = MI->getOperand(OpNo);

[PATCH] D141750: [docs] Add llvm & clang release notes for LoongArch

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Nice! Thanks for the detailed reporting. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141750/new/ https://reviews.llvm.org/D141750 _

[PATCH] D126451: [Clang][CSKY] Add support about CSKYABIInfo

rengolin added a comment. This looks good to me, but wait to make sure others see it, too. My reasons are: it is largely similar to RISCV implementation, it seems to follow what I expected of the ABI (which is similar to other targets) and has a large corpus of tests. I can't comment on the sp

[PATCH] D126451: [Clang][CSKY] Add support about CSKYABIInfo

rengolin added a comment. In D126451#3541590 , @zixuan-wu wrote: > I don't think it is largely similar to RISCV implementation except for the > code structure and test reuse. And the test result is big different. Sorry, that's what I meant. There are no

[PATCH] D45240: [ARM] Compute a target feature which corresponds to the ARM version.

rengolin added a subscriber: joerg. rengolin added a comment. LTO is something I never considered before in the context of the target parser, but I understand the issues are similar to what the build attributes were trying to solve, so adding more info shouldn't make it worse. However, this wil

[PATCH] D40256: [ARM] disable FPU features when using soft floating point.

rengolin added a comment. Wasn't that the mess about -mfpu=softfp? https://reviews.llvm.org/D40256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145833: Switch ABI references to env/environment

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. IIRC, "abi" used to be what Arm called, but "env" is equally good. And it it's consistent with `Triple.h`, even better. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D149946: [LoongArch] Define `ual` feature and override `allowsMisalignedMemoryAccesses`

rengolin added a reviewer: peter.smith. rengolin added a comment. Rationale and implementation make sense to me. I'll let this one sit so that other folks, including Arm, can have a look, too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149946/ne

[PATCH] D152433: [ARM,AArch64] Add a full set of -mtp= options.

rengolin added a comment. The only minor visible difference is the removal of `read-tp-hard` option from the LLVM side, which could be used by other downstream implementations. I personally don't think this is a big deal. First, we don't promise stability on that layer, and second, it would be

[PATCH] D152433: [ARM,AArch64] Add a full set of -mtp= options.

rengolin added a subscriber: olista01. rengolin added inline comments. Comment at: clang/include/clang/Driver/Options.td:3525 + "For AArch32: 'soft' uses a function call, or 'tpidrurw', 'tpidruro' or 'tpidrprw' use the three CP15 registers. 'cp15' is an alias for 'tpi

[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. My floating-point-fu is lacking, so I'll trust you guys that this makes sense. :) I didn't see anything egregious, so LGTM with the nit inline. cheers, --renato Commen

[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

rengolin added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ mgorny wrote: > r

[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

rengolin added inline comments. Comment at: lib/builtins/floatuntitf.c:73 +long_double_bits fb; +fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */ + | ((a >> 64) & 0xLL); /* mantissa */ rengolin wrote: >

[PATCH] D28078: [compiler-rt] [tests] Add missing "int_lib.h" includes and extend guards

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

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

rengolin added a comment. Please, add tests to test/Driver. https://reviews.llvm.org/D28238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

rengolin added inline comments. Comment at: test/Driver/linux-ld.c:623 +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=arm64-unknown-linux-gnu \ +// RUN: --gcc-toolchain="" \ shouldn't you have used your triple? https://r

[PATCH] D28238: [Driver] Add openSuse AArch64 Triple

rengolin added inline comments. Comment at: test/Driver/linux-ld.c:623 +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=arm64-unknown-linux-gnu \ +// RUN: --gcc-toolchain="" \ cryptoad wrote: > rengolin wrote: > > shouldn't

<    1   2   3   4   5   >