[PATCH] D150902: [ARM][Driver] Warn if -mhard-float is incompatible

2023-05-25 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:151 + const std::vector &Features) { + if (llvm::find(Features, "-fpregs") == Features.end()) +return; This whole patch hinges on the unspoke

[PATCH] D151308: -fsanitize=function: fix alignment fault on Arm targets.

2023-05-25 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. In D151308#4367704 , @peter.smith wrote: > I note that this is also broken in -fsanitize=kcfi [*] > (https://reviews.llvm.org/D135411) although fixing that is a separate patch. > Would you be able to raise a github issue t

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

2023-05-02 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. Do LLVM's current portability goals include the constraint that you can only build LLVM for a platform it can also target? If not, then there surely still needs to be //some// kind of escape hatch so that you can avoid needing `llvm-nm` to already support the objec

[PATCH] D151437: [NFC][Driver] Change MultilibBuilder interface

2023-06-05 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. LGTM: I agree that the new notation is easier to understand for anyone not already used to the old one. However, please wait at least a day for other people and time zones to have

[PATCH] D151438: [NFC][Driver] Change Multilib flag representation

2023-06-05 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. LGTM, but please wait at least a day for other people to have last-minute thoughts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

[PATCH] D150902: [ARM][Driver] Warn if -mhard-float is incompatible

2023-06-05 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. Thanks, that's much clearer. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150902/new/ https://reviews.llvm.org/D150902 _

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

2023-06-08 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham created this revision. simon_tatham added reviewers: nickdesaulniers, peter.smith, kristof.beyls, t.p.northover, rengolin. Herald added a subscriber: hiraditya. Herald added a project: All. simon_tatham requested review of this revision. Herald added subscribers: llvm-commits, cfe-com

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

2023-06-08 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. In D152433#4405428 , @rengolin wrote: > 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. Yes. I wasn't sure how much th

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

2023-06-12 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham updated this revision to Diff 530544. simon_tatham added a comment. Fixed the isReadTPSoft predicate (sorry – I always get confused by when Tablegen autogenerates those predicates and when it doesn't), and reorganised the tests. I couldn't find any existing test file, so I made a n

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

2023-06-13 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham updated this revision to Diff 530830. simon_tatham added a comment. Rebased past rG34d7acd444b8 (which conflicted with it, though trivially) and attempted to fix the clang-format complaint in pre-merge checks.

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

2023-06-13 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. Right, this seems to be passing tests now, so I think @nickdesaulniers's issue is fixed, and I've also split up the tests as @MaskRay suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152433/new/ https://review

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

2023-06-15 Thread Simon Tatham via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG10e42281144e: [ARM,AArch64] Add a full set of -mtp= options. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152433/new/ https://r

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

2017-07-12 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: docs/LanguageExtensions.rst:448 +``__fp16`` is a storage and interchange format only. This means that values of +``__fp16`` promote to (at least) float when used in artimethic operations. There are +two ``__fp16`` formats. Clang su

[PATCH] D35295: [docs] Add section 'Half-Precision Floating Point'

2017-07-12 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: docs/LanguageExtensions.rst:457 +that arithmetic on ``_Float16`` is performed in half-precision, thus it is not +a storage-only format. It is recommended that portable code use the +``_Float16`` type. I think the us

[PATCH] D120875: [Driver] Split up huge aarch64-cpus.c test.

2022-03-03 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. > While splitting up the test file is not ideal [...] Actually I'm not so sure. I'd almost rather go further, and split it up into lots of //much// smaller files, each with some kind of reasonable theme, like "all the basically v8.1-A stuff" or "all the v8M". The

[PATCH] D121093: [Driver][AArch64] Split up aarch64-cpus.c test further

2022-03-07 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: clang/test/Driver/aarch64-cpus.c:2 +// Check target CPUs are correctly passed. +// TODO: The files should be split up by categories, e.g. by architecture versions, to avoid excessive test // times for large single test files.

[PATCH] D121093: [Driver][AArch64] Split up aarch64-cpus.c test further

2022-03-07 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham accepted this revision. simon_tatham added inline comments. Comment at: clang/test/Driver/aarch64-cpus.c:2 +// Check target CPUs are correctly passed. +// TODO: The files should be split up by categories, e.g. by architecture versions, to avoid excessive test // ti

[PATCH] D121093: [Driver][AArch64] Split up aarch64-cpus.c test further

2022-03-07 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. I like this version! This definitely says to me "nobody is going to just thoughtlessly append to an existing file". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121093/new/ https://reviews.llvm.org/D121093 _

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

2021-05-11 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. For this kind of pure refactoring on a `.td` file, my usual approach to testing it is to run the file through Tablegen without any output option (i.e. just in the default `-print-records` mode), before and after the change. In this case, I'd think, you ought to be

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

2021-05-11 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. For example, that's how I tested the refactoring in D72690 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102238/new/ https://reviews.llvm.org/D102238 ___

[PATCH] D100937: [ARM][Driver][Windows] Allow command-line upgrade to Armv8.

2021-04-21 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham created this revision. simon_tatham added reviewers: mstorsjo, thakis. Herald added subscribers: dexonsmith, danielkiss, hiraditya, kristof.beyls. simon_tatham requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. I

[PATCH] D100937: [ARM][Driver][Windows] Allow command-line upgrade to Armv8.

2021-04-21 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment. Yes, it looks easy enough to add something in `llvm/unittests/ADT/TripleTest.cpp` to directly test `getARMCPUForArch`. I'd mildly prefer to do that //as well// as having the test here, because the call site in the clang driver is quite complicated. My real aim is t

[PATCH] D100937: [ARM][Driver][Windows] Allow command-line upgrade to Armv8.

2021-04-21 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham updated this revision to Diff 339161. simon_tatham added a comment. Added a unit test on the LLVM side. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100937/new/ https://reviews.llvm.org/D100937 Files: clang/test/Driver/woa-crypto.c

[PATCH] D100937: [ARM][Driver][Windows] Allow command-line upgrade to Armv8.

2021-04-21 Thread Simon Tatham via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG77e170db8678: [ARM][Driver][Windows] Allow command-line upgrade to Armv8. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D94599: [clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI

2021-01-19 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:81 + union { +SourceRange SingleRange; +unsigned NumRanges; This surely relies on `SourceRange` having no destructor (or rather, a trivial

[PATCH] D94599: [clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI

2021-01-19 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:81 + union { +SourceRange SingleRange; +unsigned NumRanges; simon_tatham wrote: > This surely relies on `SourceRange` having no destructor

[PATCH] D94599: [clang][Tooling] Get rid of a hack in SymbolOccurrences, NFCI

2021-01-20 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham accepted this revision. simon_tatham added a comment. Thanks. LGTM now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94599/new/ https://reviews.llvm.org/D94599 ___ cfe-commits mailing list

<    1   2   3   4