[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

simon_tatham accepted this revision. simon_tatham added a comment. Yes, this looks good to me too. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158614/new/ https://reviews.llvm.org/D158614 ___ cfe-commits mailing list

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. The change LGTM, and "agree with gcc" seems like a reasonable justification in this case. But I'm curious more generally about what options should / shouldn't be covered by

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

simon_tatham added inline comments. Comment at: clang/lib/Basic/Sanitizers.cpp:134-143 + if ((A && + A->getOption().matches(clang::driver::options::OPT_mexecute_only)) || + (std::find(Features.begin(), Features.end(), "+execute-only") != + Features.end())) { +

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

simon_tatham added a comment. I think this is also true of `-fsanitize=kcfi`, isn't it? That also works by writing a cookie just before the function entry point. If we're diagnosing incompatibilities with execute-only, then perhaps we should do it for both. Repository: rG LLVM Github

[PATCH] D154043: [CodeGen] -fsanitize={function, kcfi}: ensure align 4 if +strict-align

simon_tatham added a comment. The details of this approach look good to me, but is this the best place to solve it? Doing it in clang means that //every// language front end that wants to use either of these sanitizers is responsible for doing this same work: tagging every IR function with

[PATCH] D153885: [Clang][Driver] Change missing multilib error to warning

simon_tatham added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:740 -def err_drv_no_matching_multilib : Error< - "no multilib found matching flags: %0">; +def warn_drv_no_matching_multilib : Warning< + "no multilib found matching flags:

[PATCH] D153292: [Driver][BareMetal] Error if no matching multilib

simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. LGTM. One of the thoughts I mentioned offline before this patch was written was that maybe the error would need to be conditional, via a directive inside `multilib.yaml` itself –

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

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/

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

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/

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

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.

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

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

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

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

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,

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

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] D151438: [NFC][Driver] Change Multilib flag representation

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

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

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] D151308: -fsanitize=function: fix alignment fault on Arm targets.

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

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

simon_tatham added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:151 + const std::vector ) { + if (llvm::find(Features, "-fpregs") == Features.end()) +return; This whole patch hinges on the unspoken

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG20d6dee40d50: -fsanitize=function: fix alignment fault on Arm targets. (authored by simon_tatham). Changed prior to commit: https://reviews.llvm.org/D151308?vs=525111=525479#toc Repository: rG LLVM

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

simon_tatham added a comment. > `-fsanitize=kcfi` only supports aarch64 and x86-64 now. riscv64 is on the > plan. > > % fclang -fsanitize=kcfi --traget=armv7-linux-gnueabi -c a.c > clang: error: unsupported option '--traget=armv7-linux-gnueabi' Sorry to contradict, but that error message

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

simon_tatham updated this revision to Diff 525111. simon_tatham added a comment. Clarify mask construction as @michaelplatings suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151308/new/ https://reviews.llvm.org/D151308 Files:

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

simon_tatham updated this revision to Diff 525084. simon_tatham added a comment. How embarrassing. _Really_ upload the clang-formatted version this time. Sorry for the noise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151308/new/

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

simon_tatham updated this revision to Diff 525065. simon_tatham added a comment. (oops, forgot to clang-format) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151308/new/ https://reviews.llvm.org/D151308 Files: clang/lib/CodeGen/CGExpr.cpp

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

simon_tatham added a comment. I think this began going wrong as a result of D148573 , which enabled `-fsanitize=function` on all targets, where previously it hadn't been running on Arm at all. But I'd rather make it work than turn it off again! Repository:

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

simon_tatham created this revision. simon_tatham added reviewers: peter.smith, MaskRay, dmgreen. Herald added a subscriber: kristof.beyls. Herald added a project: All. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Function

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

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

[PATCH] D145781: [AArch64] Don't #define __ARM_FP when there's no FPU.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5fba4c4d08bd: [AArch64] Dont #define __ARM_FP when theres no FPU. (authored by simon_tatham). Changed prior to commit:

[PATCH] D145781: [AArch64] Don't #define __ARM_FP when there's no FPU.

simon_tatham created this revision. simon_tatham added reviewers: lenary, tmatheson, DavidSpickett, efriedma. Herald added a subscriber: kristof.beyls. Herald added a project: All. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D145567: [Driver] Rename multilib flags to tags

simon_tatham added inline comments. Comment at: clang/docs/Multilib.rst:71 + arguments into a standard set of simpler "tags". In many cases these tags will look like a command line argument with the leading ``-`` stripped off, + but where a suitable form for the tag

[PATCH] D145567: [Driver] Rename multilib flags to tags

simon_tatham added a comment. But it was useful to have it separate at least for review purposes, since it made it much easier when Michael asked me to proofread the change from 'flags' to 'tags'! Comment at: clang/docs/Multilib.rst:66 ``--target=armv7m-none-eabi`` are

[PATCH] D142703: [ARM] Allow selecting hard-float ABI in integer-only MVE.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG60ea6f35a270: [ARM] Allow selecting hard-float ABI in integer-only MVE. (authored by simon_tatham). Changed prior to commit:

[PATCH] D142703: [ARM] Allow selecting hard-float ABI in integer-only MVE.

simon_tatham retitled this revision from "[ARM] Allow selecting the hard float ABI in integer-only MVE." to "[ARM] Allow selecting hard-float ABI in integer-only MVE.". simon_tatham edited the summary of this revision. simon_tatham updated this revision to Diff 492722. simon_tatham added a

[PATCH] D140959: RFC: Multilib prototype

simon_tatham added a comment. "Make difficult things possible": perhaps it might be useful to make sure it's at least //possible// to express a complex boolean function of the basic predicates, even if it's cumbersome? (So that, for example, you could make an attribute conditional on "this cc1

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

simon_tatham added a comment. The clang code change looks reasonable to me, but I'm not the most expert in that area. The intended result certainly seems sensible, because the C90-compatible version of that code //without// a constant literal is happy to do double→float conversion at compile

[PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

simon_tatham added a comment. I don't know if "silence implies no objection" is a reasonable assumption in this community, so for the sake of not treading on toes, I've put my revised version of the patch in a new location instead: D129842 . CHANGES SINCE

[PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

simon_tatham added a comment. Herald added a project: All. @EricWF, I have an updated version of this patch that applies against current `main`. Do you mind if I commandeer this revision and post my updated patch on it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12669/new/

[PATCH] D127197: [ARM] Fix how size-0 bitfields affect homogeneous aggregates.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGceb21fa4e49d: [ARM] Fix how size-0 bitfields affect homogeneous aggregates. (authored by simon_tatham). Repository: rG LLVM Github Monorepo

[PATCH] D127197: [ARM] Fix how size-0 bitfields affect homogeneous aggregates.

simon_tatham updated this revision to Diff 434838. simon_tatham added a comment. Added tests with `extern "C"`, at @lenary's (offline) suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127197/new/ https://reviews.llvm.org/D127197 Files:

[PATCH] D127197: [ARM] Fix how size-0 bitfields affect homogeneous aggregates.

simon_tatham created this revision. simon_tatham added reviewers: asl, rsmith, lenary, john.brawn. Herald added a subscriber: kristof.beyls. Herald added a project: All. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. By both

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

simon_tatham accepted this revision. simon_tatham added a comment. Thanks, that explanation looks fine. (And I agree that re-paragraphing it made more sense than my version) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

simon_tatham added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19 +// The intention is this: +// - Any 128-bit or 64-bit writes to the neon input register of an AES fused +// pair are safe (the inputs are to the AESE/AESD instruction).

[PATCH] D122046: [clang] Remove Address::deprecated from MveEmitter

simon_tatham added inline comments. Comment at: clang/utils/TableGen/MveEmitter.cpp:1197 +const Type *Ty = nullptr; +if (auto *DI = dyn_cast(D->getArg(0))->getOperator()) + if (auto *PTy = dyn_cast(getType(DI, Param))) dblaikie wrote: > nikic wrote:

[PATCH] D122046: [clang] Remove Address::deprecated from MveEmitter

simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. Whereas I'm familiar with this code but not with the opaque-pointers effort, so I had to look up the //other// half of what was going on :-) If I've understood this correctly, the

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

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] D121093: [Driver][AArch64] Split up aarch64-cpus.c test further

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 //

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

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] D120875: [Driver] Split up huge aarch64-cpus.c test.

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] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

simon_tatham accepted this revision. simon_tatham added a comment. This revision is now accepted and ready to land. LGTM this time. Thanks very much for the rework! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119591/new/

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

simon_tatham added a comment. I ran another test of this patch, by trying the cross product of the following cmake configuration choices (on a build targeting Windows, with `LLVM_ENABLE_PROJECTS=clang`): - `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS` set to each of `ON` and `OFF` - `CLANG_BUILD_EXAMPLES`

[PATCH] D119591: clang-analyzer plugins require LLVM_ENABLE_PLUGINS also

simon_tatham added a comment. I gave this patch a test against our downstream code, and found that it doesn't stop me needing the change I suggested in D119199 : in `clang/tools/driver/CMakeLists.txt`, I had to change if(CLANG_PLUGIN_SUPPORT)

[PATCH] D119199: replace clang LLVM_ENABLE_PLUGINS -> CLANG_PLUGIN_SUPPORT in tests

simon_tatham added inline comments. Comment at: clang/tools/driver/CMakeLists.txt:53 # Support plugins. if(CLANG_PLUGIN_SUPPORT) export_executable_symbols_for_plugins(clang) I think we've managed to fix our build by changing this line so that it tests

[PATCH] D119199: replace clang LLVM_ENABLE_PLUGINS -> CLANG_PLUGIN_SUPPORT in tests

simon_tatham added a comment. Hi. This commit seems to have broken the following cmake command, on Windows (with HEAD pointing at 76cad51ba700233d6e3492eddcbb466b6adbc2eb ): cmake -DLLVM_ENABLE_PROJECTS=clang

[PATCH] D115014: [clang] RFC: NFC: simplify macro tokens assignment

simon_tatham added a comment. Ah, now I see what you mean – I didn't look far enough! I don't know this code well (in fact I'm not sure why you picked me as a reviewer), but off the top of my head: the question is not just whether `tokens_iterator` happens to be the same thing as `const Token

[PATCH] D115014: [clang] RFC: NFC: simplify macro tokens assignment

simon_tatham added a comment. I don't think so, I'm afraid. If you look at the definition of `MacroInfo::tokens_begin()` in `clang/include/clang/Lex/MacroInfo.h`, you see that it doesn't return a raw `Token *`: it returns a C++ iterator object. using tokens_iterator =

[PATCH] D105495: [clang] Make negative getLocWithOffset widening-safe.

simon_tatham added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:236 if (Loc.isMacroID()) - return Loc.getLocWithOffset(-SM.getFileOffset(Loc)); + return Loc.getLocWithOffset(0, SM.getFileOffset(Loc)); return

[PATCH] D105495: [clang] Make negative getLocWithOffset widening-safe.

simon_tatham updated this revision to Diff 361957. simon_tatham edited the summary of this revision. simon_tatham added a comment. In D105495#2882628 , @tmatheson wrote: > I also think it's the caller's responsibility to make sure what they pass in > is

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. (But I've pushed this patch anyway, because I'm reasonably confident of the answer; if it turns out that there's some case along those lines that I should have taken care of, I'll fix or revert.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. simon_tatham marked an inline comment as done. Closed by commit rGbd41136746a0: [clang] Use i64 for the !srcloc metadata on asm IR nodes. (authored by simon_tatham).

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. @lattner, thanks for the help. In this case, the real question is whether there's any use case for `!srcloc` that involves writing it out into a bitcode or IR file and then having a separate instance of clang load it back in again. I think that no such case can

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a subscriber: lattner. simon_tatham added a comment. In D105491#2891860 , @dexonsmith wrote: > although it'd be good to get someone more involved in lib/CodeGen to take a > quick look / sign off (ideally someone that knows the use

[PATCH] D105492: [clang] Introduce SourceLocation::[U]IntTy typedefs.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21401a72629c: [clang] Introduce SourceLocation::[U]IntTy typedefs. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:208 const ParmVarDecl *const *getParams() const { -return reinterpret_cast(ParamsAndSelLocs); +return const_cast(Params); } simon_tatham wrote: > tmatheson wrote: >

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham updated this revision to Diff 360123. simon_tatham added a comment. Addressed two nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105498/new/ https://reviews.llvm.org/D105498 Files: clang/include/clang/AST/DeclObjC.h

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:208 const ParmVarDecl *const *getParams() const { -return reinterpret_cast(ParamsAndSelLocs); +return const_cast(Params); } tmatheson wrote: > I don't think you need

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham added inline comments. Comment at: clang/lib/AST/DeclObjC.cpp:31 #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/MathExtras.h" #include "llvm/Support/raw_ostream.h" Oops, before anyone else points it out, this `#include` is also

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham updated this revision to Diff 360104. simon_tatham added a comment. ... and removed an unused function from the previous version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105498/new/ https://reviews.llvm.org/D105498 Files:

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham updated this revision to Diff 360101. simon_tatham edited the summary of this revision. simon_tatham added a comment. Split up the allocations as suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105498/new/

[PATCH] D105492: [clang] Introduce SourceLocation::[U]IntTy typedefs.

simon_tatham updated this revision to Diff 360099. simon_tatham retitled this revision from "[clang] Introduce SourceLocation::[U]IntType typedefs." to "[clang] Introduce SourceLocation::[U]IntTy typedefs.". simon_tatham added a comment. Renamed types to [U]IntTy. This will affect some of the

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham updated this revision to Diff 360098. simon_tatham edited the summary of this revision. simon_tatham added a comment. Added an i64 `!srcloc` to the only existing test of them I could find. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham added a comment. I looked into this yesterday, and realised that I don't actually know what the use case //is// for emitting `!srcloc` metadata in an IR file. It's more or less ignored by llc, as far as I can see: if there's a late-breaking error in the inline asm string, you just

[PATCH] D105493: [clang] Change set type used for SourceLocation.

This revision was automatically updated to reflect the committed changes. simon_tatham marked an inline comment as done. Closed by commit rGcef56d58dbbb: [clang] Change set type used for SourceLocation. (authored by simon_tatham). Changed prior to commit:

[PATCH] D105490: Remove unused parameter from parseMSInlineAsm.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe49985bb6065: Remove unused parameter from parseMSInlineAsm. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. This patch doesn't seem to have attracted much review attention in the last couple of weeks. On the theory that perhaps it's just too big and monolithic to review in one go, I've started to break it up into smaller pieces. So far I've only covered the preparation

[PATCH] D105498: [clang] Remove assumption about SourceLocation alignment.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of a patch series working towards the ability to make SourceLocation

[PATCH] D105497: [clang] Serialize source locations as 64-bit in PCH.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of a patch series working towards the ability to make SourceLocation

[PATCH] D105495: [clang] Make negative getLocWithOffset widening-safe.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added subscribers: dexonsmith, martong. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of a patch series

[PATCH] D105494: [clang] Introduce a union inside ProgramPoint.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of a patch series working towards the ability to make SourceLocation

[PATCH] D105493: [clang] Change set type used for SourceLocation.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added a subscriber: dexonsmith. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of a patch series working

[PATCH] D105492: [clang] Introduce SourceLocation::[U]IntType typedefs.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added subscribers: dexonsmith, arphaman, kbarton, nemanjai. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is part of

[PATCH] D105491: [clang] Use i64 for the !srcloc metadata on asm IR nodes.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added subscribers: dexonsmith, hiraditya. simon_tatham requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This is part of

[PATCH] D105490: Remove unused parameter from parseMSInlineAsm.

simon_tatham created this revision. simon_tatham added reviewers: rsmith, lebedev.ri, akyrtzi. Herald added a subscriber: hiraditya. simon_tatham requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. No implementation uses the

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. Hmmm. Two people have pointed out to me that my strategy of having a 32-bit `SourceLocations::LowBits` and an 0- or 32-bit `SourceLocations::OptionalHighBits` doesn't actually work, because an empty struct still takes at least 1 byte. So this version of the patch

[PATCH] D104442: [libclang] Fix error handler in translateSourceLocation.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd569a11b585: [libclang] Fix error handler in translateSourceLocation. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104442/new/

[PATCH] D97204: [RFC] Clang 64-bit source locations

simon_tatham added a comment. Thanks to @miyuki for repeating the previous benchmark with this version of the patch (and on the same machine as before, which was better than I could have done). The revised results now have the memory usage increase (compared to 32-bit SourceLocation) in the

[PATCH] D104442: [libclang] Fix error handler in translateSourceLocation.

simon_tatham created this revision. simon_tatham added reviewers: akyrtzi, rsmith. simon_tatham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Given an invalid SourceLocation, translateSourceLocation will call clang_getNullLocation, and

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

simon_tatham added inline comments. Comment at: clang/include/clang/Basic/arm_mve.td:1532 // // So this foldl expression implements what you'd write in Python as // [srctype for srctype in T.All if srctype != desttype] If you've removed my unreadable

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

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] D102238: [TableGen] [Clang] Clean up arm_mve.td file

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] D100937: [ARM][Driver][Windows] Allow command-line upgrade to Armv8.

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

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

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:

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

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

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

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.

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

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

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

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

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

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

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1d782942500b: [Sema][BFloat] Forbid arithmetic on vectors of bfloat. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85009/new/

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

simon_tatham added a comment. This discussion seems to have wound down. I'll land this patch tomorrow on the strength of @LukeGeeson's review, unless you have strong objections, @jfb? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85009/new/

[PATCH] D85010: [clang][ARM] Add name-mangling test for direct __fp16 arguments.

This revision was automatically updated to reflect the committed changes. Closed by commit rGed0e4c70c99d: [clang][ARM] Add name-mangling test for direct __fp16 arguments. (authored by simon_tatham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

simon_tatham added a comment. In D85009#2187643 , @jfb wrote: > Language-wise I think https://wg21.link/p1467 is where C++ is going, and C is > taking a similar approach. That doesn't seem to mention vectors at all. As I said on Friday, I wouldn't

[PATCH] D85009: [Sema][BFloat] Forbid arithmetic on vectors of bfloat.

simon_tatham added a comment. In D85009#2187621 , @jfb wrote: > You mean: only aarch64 backend supports lowering bfloat16 vectors at the > moment? Yes, sorry – I should have said that Arm has the only implementation //in an LLVM target//. I meant the

  1   2   3   4   >