[PATCH] D65000: [ARM] Set default alignment to 64bits

2019-07-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. test case missing A8 aside this looks ok to me. Would like to see if there are any comments from the Pacific time zone. Comment at: test/CodeGen/ARM/exception-alignment.cpp:8 +// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16 +#include +

[PATCH] D65000: [ARM] Set default alignment to 64bits

2019-07-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Thanks for the update. Will be worth adding some reviewers from Apple to see if this change should be IsAAPCS only. I've no more further comments myself besides a small nit on style. Comment at: lib/Basic/Targets/ARM.cpp:331 + // but

[PATCH] D65000: [ARM] Set default alignment to 64bits

2019-07-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added reviewers: srhines, danalbert. peter.smith added a comment. I think that this may not apply for Android as AFAIK their ABI still requires 128-bit alignment in some cases. Adding some more reviewers from Android. Comment at: lib/Basic/Targets/ARM.cpp:311

[PATCH] D64415: Consistent types and naming for AArch64 target features (NFC)

2019-07-15 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. FWIW I think this looks reasonable. The ARM equivalent uses bitfields such as unsigned CRC : 1; unsigned Crypto : 1; unsigned DSP : 1; unsigned Unaligned : 1; unsigned DotProd : 1; Which would make more sense than using unsigned for each individual field.

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

2019-04-10 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In D60472#1461336 , @manojgupta wrote: > The motivation for this change is to make "crypto" setting an additive option > e.g. like "-mavx" used in many media packages. Some packages in Chrome want > to enable crypto

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

2019-04-10 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I'm not in favour of adding AArch64 support to -mcrypto -mnocrypto for a few reasons: - Arm are trying to keep the options for controlling target features as consistent as possible with GCC and this option isn't supported in GCC

[PATCH] D58320: [Darwin] Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins library.

2019-03-05 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I've no objections to adding the command line as a Darwin only option. Implementation looks fine to me although I've not got any prior experience with Darwin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58320/new/

[PATCH] D58314: [Driver] Sync ARM behavior between clang-as and gas.

2019-02-21 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Thanks for the update. To summarise I'd like to keep the integrated and non-integrated assembler in synch for Linux like Android, even at the risk of adding an fpu when it isn't needed. I think that given how difficult it is to not use NEON, I think the

[PATCH] D58429: [CodeGen] Enable the complex-math test for arm

2019-02-20 Thread Peter Smith via Phabricator via cfe-commits
peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the fix. My apologies for missing that at the time, it looks like a cut and paste error on my part. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D58314: [Driver] Sync ARM behavior between clang-as and gas.

2019-02-19 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. My main concern is that this changes the default behaviour for arm-linux-gnueabi and arm-linux-gnueabihf targets. I've put some suggestions on what I think the behaviour should be. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:277 + } +

[PATCH] D58320: [Darwin] Introduce a new flag, -flink-builtins-rt that forces linking of the builtins library.

2019-02-19 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. The implementation changes in the Darwin toolchain look fine to me, although with respect to the command line option I think Petr Hosek's message on cfe-dev is interesting: > GCC implements -nolibc which could be used to achieve the same effect when > combined

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I think we need to be very careful here. If I've understood this correctly then if this functionality is used for anything critical then a manually supplied target will be needed when doing cross-linking. For example my host LLD is x86_64, is just called ld.lld and

[PATCH] D55029: set default max-page-size to 4KB in lld for Android Aarch64

2018-11-30 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In D55029#1313120 , @ruiu wrote: > LGTM. Please commit. > > Peter, I wonder if you are fine with the default 64KiB page size with lld, > especially given that lld always round up the text segment size to the > maximum page

[PATCH] D55029: set default max-page-size to 4KB in lld for Android Aarch64

2018-11-29 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Yes this is fine. The effects are entirely within the Android target. The change in LLD to a 64k page size was made in D25079 . The main reason given was that a sufficient number of linux distributions including Redhat had chosen a

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

2018-10-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Added LLVM_FALLTHROUGH; in r344890. Repository: rC Clang https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-19 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Thanks for pointing that out, I'm out of office today, will look at describing the intention to fall through when I get back in on Monday. Comment at: lib/Driver/ToolChains/Gnu.cpp:241 + case llvm::Triple::thumbeb: +IsBigEndian = true; +

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

2018-10-17 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In https://reviews.llvm.org/D45240#1267846, @stefson wrote: > hey there, I've run into problems with stripping static-libs on arm when > using llvm/clang-7. Could you imagine this patch being at fault? > > strip: armv7a-unknown-linux-gnueabihf-strip

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

2018-10-16 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344597: [ARM][AArch64] Pass through endian flags to assembler and linker. (authored by psmith, committed by ). Changed prior to commit: https://reviews.llvm.org/D52784?vs=169689=169799#toc Repository:

[PATCH] D53272: Add target requirement to profile remap test.

2018-10-16 Thread Peter Smith via Phabricator via cfe-commits
peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. Looks good to me. REQUIRES: x86-registered-target is used in the same directory for tests that depend on specific targets. Repository: rC Clang https://reviews.llvm.org/D53272

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

2018-10-15 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 169689. peter.smith marked 3 inline comments as done. peter.smith added a comment. Updated diff to reflect review comments. Main changes are: - isArmBigEndian always returns false if the target architecture isn't Arm. - Added tests to make sure "--be8"

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

2018-10-15 Thread Peter Smith via Phabricator via cfe-commits
peter.smith marked 7 inline comments as done. peter.smith added a comment. Thanks very much for the comments. I'll post an update shortly. Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364 +const char* EndianFlag = "-EL"; +if (isArmBigEndian(Triple, Args)) { +

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

2018-10-12 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 169386. peter.smith added a comment. I've decided to roll the linker changes in with the assembler ones as the linker use case affects the design. It turns out that only Arm needs to check to see if the -mbig-endian and -mlittle-endian flags override

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

2018-10-12 Thread Peter Smith via Phabricator via cfe-commits
peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. Looks good to me. By generic target I just meant --target=arm-linux-androideabi21 with a -march to specify the revision, which you've got in the test. Repository: rC Clang

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

2018-10-11 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. On reflection after looking at what would be needed for the linker tools::gnutools::Linker::ConstructJob() I think it may be better to move the triple detection into a separate function. I'll work on that and will hopefully post an update soon.

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

2018-10-11 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Thanks for the comments. I agree with you that the -EB and -EL flags need to be passed to the linker as well, I kind of expected that ld.bfd would infer it from the endianness of the first object but it doesn't seem to do that. If it's ok I'll do that in a separate

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

2018-10-11 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. It looks like we might be able to simplify a bit, and I think there probably could be some more test cases but no objections from me. Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:360 + unsigned ArchVersion; + if (ArchName.empty())

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

2018-10-10 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Ping. Does anyone have any changes they'd like me to make? https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-02 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 167955. peter.smith added a comment. Updated diff to add more tests, including some that use -mlittle-endian when the target is big-endian. https://reviews.llvm.org/D52784 Files: lib/Driver/ToolChains/Gnu.cpp test/Driver/linux-as.c Index:

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

2018-10-02 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In https://reviews.llvm.org/D52784#1252569, @rengolin wrote: > 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). I thought

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

2018-10-02 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. peter.smith added reviewers: rengolin, compnerd, olista01. Herald added subscribers: chrib, kristof.beyls, srhines. Herald added a reviewer: javed.absar. The big-endian arm32 Linux builds are currently failing when the -mbig-endian and -fno-use-integrated-as

[PATCH] D52705: First-pass at ARM hard/soft-float multilib driver (NOT WORKING YET)

2018-10-01 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I think that you may be better off posting a RFC to llvm-dev to get discussion on the best approach here. It would also be good to step back a bit and consider the requirements, as it stands it looks like this might be a solution for just one particular multilib

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

2018-09-28 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343304: [ARM] Alter test to account for change to armv6k default CPU (authored by psmith, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

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

2018-09-28 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343304: [ARM] Alter test to account for change to armv6k default CPU (authored by psmith, committed by ). Repository: rC Clang https://reviews.llvm.org/D52595 Files: test/Driver/arm-cortex-cpus.c

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

2018-09-27 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. peter.smith added reviewers: falstaff84, nickdesaulniers, rengolin, labrinea. Herald added a reviewer: javed.absar. Herald added subscribers: chrib, kristof.beyls. Review https://reviews.llvm.org/D52594 will change the default in llvm for armv6k from the

[PATCH] D49674: [AArch64] Add Tiny Code Model for AArch64

2018-08-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. Looks good to me now that LLVM support has been approved in https://reviews.llvm.org/D49673 https://reviews.llvm.org/D49674 ___

[PATCH] D46932: [AArch64] Correct inline assembly test case for S modifier [NFC]

2018-05-17 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC332606: [AArch64] Correct inline assembly test case for S modifier [NFC] (authored by psmith, committed by ). Repository: rC Clang https://reviews.llvm.org/D46932 Files:

[PATCH] D46932: [AArch64] Correct inline assembly test case for S modifier [NFC]

2018-05-16 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. peter.smith added reviewers: t.p.northover, manojgupta. Herald added subscribers: kristof.beyls, eraman, rengolin. Herald added a reviewer: javed.absar. The existing test for the AArch64 constraint S uses the A and L modifiers. These modifiers were implemented

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

2018-03-29 Thread Peter Smith via Phabricator via cfe-commits
peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. Given that this is important for compiling the Linux kernel with clang I think that it is worth improving compatibility with GCC. LGTM. Repository: rC Clang

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

2018-03-27 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. Thanks for the clarification. With that in mind I'm much less concerned that adding "r" as an alias will make extra warnings more difficult. I agree that there should be at least a warning for register long x asm ("wn") although that is separate from this patch.

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

2018-03-26 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I think that we may have a bit of a conceptual difference with GCC here. As far as I can tell in GCC it doesn't matter if "rn", "wn", "xn" or even "bn" is used, this refers to register 0. The Operand constraint such as %w0 is used to control the substitution so the

[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

2017-11-20 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In https://reviews.llvm.org/D40127#929578, @compnerd wrote: > Would be nice to rename the variable prior to commit. Thanks for the review, I've renamed the variable as suggested. https://reviews.llvm.org/D40127 ___

[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

2017-11-20 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318647: [ARM] For assembler files recognize -Xassembler or -Wa, -mthumb (authored by psmith). Changed prior to commit: https://reviews.llvm.org/D40127?vs=123310=123571#toc Repository: rL LLVM

[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

2017-11-17 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 123310. peter.smith added a comment. Updated diff with an attempt to simplify the check for filetype and mthumb. I've left the existing Args.filtered in expression for now as I couldn't make a better alternative with std::for_any.

[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

2017-11-17 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added inline comments. Comment at: lib/Driver/ToolChain.cpp:549-556 +bool IsIntegratedAssemblerThumb = false; +for (const Arg *A : + Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) { + for (StringRef Value : A->getValues()) { +

[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

2017-11-16 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. Herald added subscribers: kristof.beyls, javed.absar, aemerson. The Unified Arm Assembler Language is designed so that the majority of assembler files can be assembled for both Arm and Thumb with the choice made as a compilation option. The way this is done in

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

2017-10-24 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316424: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu (authored by psmith). Changed prior to commit: https://reviews.llvm.org/D39179?vs=119837=120025#toc Repository: rL LLVM

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

2017-10-23 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. Herald added subscribers: javed.absar, rengolin, aemerson. When -mtune is used on AArch64 the -target-cpu is passed the value of the cpu given to -mtune. As well as setting micro-architectural features of the -mtune cpu, this will also add the architectural

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-27 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL309257: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp (authored by psmith). Changed prior to commit: https://reviews.llvm.org/D35538?vs=108245=108436#toc Repository: rL LLVM

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-26 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I've created https://reviews.llvm.org/D35904 to cover adding a test for LLVMs use of the runtime helper functions. https://reviews.llvm.org/D35538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-26 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 108245. peter.smith added a comment. Thanks both for the comments, I've added the missing punctuation and have expanded the tests so that we anchor the check to the function and check for the operation that will be lowered by the back-end.

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-25 Thread Peter Smith via Phabricator via cfe-commits
peter.smith updated this revision to Diff 108092. peter.smith added a comment. Herald added a subscriber: javed.absar. I've added a clang test that will check that none of the floating point helper functions defined in the Runtime ABI for the ARM Architecture are directly called by clang. Given

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-19 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. In https://reviews.llvm.org/D35538#814008, @compnerd wrote: > Can you please add a test that shows that the AEABI functions are not given > the wrong CC? Also, can you show that if someone also passes `-meabi=gnu` > with a VFP target, that we still annotate the

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

2017-07-18 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. Herald added subscribers: kristof.beyls, aemerson. The ARM Runtime ABI document (IHI0043) defines the AEABI floating point helper functions in section 4.1 Floating-point library. These functions always use the base PCS (soft-fp). However helper functions

[PATCH] D34513: [NFC] Update to account for DiagnosticRenderer use of FullSourceLoc

2017-06-27 Thread Peter Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL306385: [NFC] Update to account for DiagnosticRenderer use of FullSourceLoc (authored by psmith). Changed prior to commit: https://reviews.llvm.org/D34513?vs=103570=104116#toc Repository: rL LLVM

[PATCH] D34513: [NFC] Update to account for DiagnosticRenderer use of FullSourceLoc

2017-06-22 Thread Peter Smith via Phabricator via cfe-commits
peter.smith created this revision. https://reviews.llvm.org/D31709 [NFC] Refactor DiagnosticRenderer to use FullSourceLoc was committed in r305684 and reverted in 305688 as clang-tidy and clang-query failed to build. This change updates the extra tools to use the new interface. With this