[PATCH] D138143: [FPEnv] Enable strict fp for AArch64 in clang

ostannard added inline comments. Comment at: clang/docs/ReleaseNotes.rst:760 +- Strict floating point has been enabled for AArch64, which means that + ``-ftrapping-math``, ``-frounding-math``, ``-ffp-model=strict``, and + ``-ffp-exception-behaviour=`` are now accepted.

[PATCH] D112422: [clang][ARM] emit PACBTI-M feature defines

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM, I assume that the pre-commit test failure is because the bot hasn't applied the parent patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D112421: [clang][ARM] PACBTI-M frontend support

ostannard added a comment. In the commit message: s/armclang/clang/ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1832 // Enable/disable return address signing and indirect branch targets. if (Arg *A = Args.getLastArg(options::OPT_msign_return_address_EQ,

[PATCH] D112420: [clang][ARM] PACBTI-M assembly support

ostannard added inline comments. Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:4083 def : t2InstAlias<"csdb$p", (t2HINT 20, pred:$p), 1>; +def : t2InstAlias<"pacbti$p r12,lr,sp", (t2HINT 13, pred:$p), 1>; +def : t2InstAlias<"bti$p", (t2HINT 15, pred:$p), 1>; --

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

ostannard added a comment. This change is causing a lot of failures in the address sanitiser tests on the 2-stage AArch64 buildbots. For example: https://lab.llvm.org/buildbot/#/builders/179/builds/1326 I can reproduce the failures on another AArch64 machine, they only happen with a 2-stage bu

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

ostannard added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665 + +CmdArgs.push_back("-mllvm"); +if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465)) ostannard wrote: > labrinea wrote: > > ostannard wrote: > > > Are t

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

ostannard added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665 + +CmdArgs.push_back("-mllvm"); +if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465)) labrinea wrote: > ostannard wrote: > > Are these optional also bein

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

ostannard added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665 + +CmdArgs.push_back("-mllvm"); +if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465)) Are these optional also being passed through to the linker when doi

[PATCH] D102090: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

ostannard added a comment. There is also a test failure on the aarch64 2-stage bot (https://lab.llvm.org/buildbot/#/builders/7/builds/2720) which I've bisected to this change, so I'll revert it for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D98146: OpaquePtr: Turn inalloca into a type attribute

ostannard added a comment. I've reverted this (07e46367bae ) because it was causing Bindings/Go/go.test to fail on the buoldbots. Example failure at http://lab.llvm.org:8011/#/builders/107/builds/6075. CHANGES SINCE LAST ACT

[PATCH] D98253: [clang][ARM] Refactor ComputeLLVMTriple code for ARM

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98253/new/ https://reviews.llvm.org/D98253 ___

[PATCH] D96007: [AArch64] Enable stack clash protection for AArch64 linux in clang

ostannard updated this revision to Diff 321689. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96007/new/ https://reviews.llvm.org/D96007 Files: clang/lib/CodeGen/CodeGenModule.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/stack-clash-protection.c clang/test/Driver/

[PATCH] D96007: [AArch64] Enable stack clash protection for AArch64 linux in clang

ostannard created this revision. ostannard added reviewers: serge-sans-paille, jnspaulsson, bzEq, tnfchris. Herald added subscribers: danielkiss, kristof.beyls. ostannard requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This allows the -fsta

[PATCH] D93221: [ARM] Add clang command line support for -mharden-sls=

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. Ok, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93221/new/ https://reviews.llvm.org/D93221 ___

[PATCH] D93231: [ARM] Adding v8.7-A command-line support for the ARM target

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93231/new/ https://reviews.llvm.org/D93231 ___

[PATCH] D93221: [ARM] Add clang command line support for -mharden-sls=

ostannard added a comment. Why is this restricted to v7-A or later? The DSB and ISB instructions have existed since v6T2 and v6M. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93221/new/ https://reviews.llvm.org/D93221 ___

[PATCH] D91776: [ARM][AAarch64] Initial command-line support for v8.7-A

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91776/new/ https://reviews.llvm.org/D91776 ___

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard accepted this revision. ostannard added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72932/new/ https://reviews.llvm.org/D72932 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added inline comments. Comment at: clang/test/CodeGen/volatile.c:1 -// RUN: %clang_cc1 -triple=%itanium_abi_triple -emit-llvm < %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-IT +// RUN: %clang_cc1 -triple=%itanium_abi_triple -emit-llvm < %s | FileCheck %s

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72932/new/ https://reviews.llvm.org/D72932 ___

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:396 +/// according to the field declaring type width. +CODEGENOPT(ForceNoAAPCSBitfieldWidth, 1, 0) + dnsampaio wrote: > ostannard wrote: > > Why is this a negative option, w

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added a comment. You haven't addressed my earlier inline comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72932/new/ https://reviews.llvm.org/D72932 ___ cfe-commits mailing list cfe-co

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGe80b81d1cbf8: [Support] Fix formatted_raw_ostream for UTF-8 (authored by ostannard). Repository: rG LLVM Github Monorep

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

This revision was automatically updated to reflect the committed changes. Closed by commit rGe80b81d1cbf8: [Support] Fix formatted_raw_ostream for UTF-8 (authored by ostannard). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://review

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard updated this revision to Diff 275029. ostannard marked 5 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://reviews.llvm.org/D76291 Files: clang/test/Analysis/checker-plugins.c llvm/include/llvm/S

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. I don't think it makes sense to make `f16` legal for targets which don't have any arithmetic operations on it, since that would be contrary to the definition of "legal". I'd also expect

[PATCH] D81428: [ARM] Moving CMSE handling of half arguments and return to the backend

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81428/new/ https://reviews.llvm.org/D81428 _

[PATCH] D81428: [ARM] Moving CMSE handling of half arguments and return to the backend

ostannard added a comment. Is this expected to work for the soft-float calling convention, or is clang still passing half-precision values as integer types for that? If the former, then this needs some tests for that case. Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:2

[PATCH] D81451: [ARM][Clang] Removing lowering of half-precision FP arguments and returns from Clang's CodeGen

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM, thanks for working on this patch series, this looks much clearer than doing it in clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM, inline comment could be done as a follow-up patch given that this is time-sensitive (recently published security vulnerability). Comment at: clang/lib/Driver/Too

[PATCH] D80928: [BFloat] Add convert/copy instrinsic support

ostannard added inline comments. Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:789 +def int_arm_neon_vcvtfp2bf +: Intrinsic<[llvm_anyvector_ty], [llvm_anyvector_ty], [IntrNoMem]>; I only see this being used for f32 -> bf16 conversion, so could this have

[PATCH] D79711: [ARM] Add poly64_t on AArch32.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. Ok, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79711/new/ https://reviews.llvm.org/D79711

[PATCH] D79711: [ARM] Add poly64_t on AArch32.

ostannard added a comment. Should `poly128_t` be available on AArch32 too? I don't see anything in the ACLE version you linked restricting it to AArch64 only, and the intrinsics reference has a number of intrinsics available for both ISAs using it. Comment at: clang/include/c

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79721/new/ https://reviews.llvm.org/D79721 _

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard added a reviewer: kristof.beyls. ostannard added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://reviews.llvm.org/D76291 ___ cfe-commits mailing list cfe-com

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://reviews.llvm.org/D76291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard updated this revision to Diff 251105. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://reviews.llvm.org/D76291 Files: clang/test/Analysis/checker-plugins.c llvm/include/llvm/Support/FormattedStream.h llvm/lib/Support/

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. I agree with @dnsampaio here, it's better to match the existing style, and avoid irrelevant churn in the git history. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard updated this revision to Diff 251030. ostannard added a comment. - I managed to reproduce the lsl-zero.s test failure on windows, this turned out to be because the test merges stdout and stderr, which can result in them being interleaved in ways which breaks tests. The test doesn't che

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard marked 16 inline comments as done. ostannard added inline comments. Comment at: llvm/unittests/Support/formatted_raw_ostream_test.cpp:88 + +TEST(formatted_raw_ostreamTest, Test_UTF8) { + SmallString<128> A; hubert.reinterpretcast wrote: > Should there

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

ostannard created this revision. ostannard added reviewers: MaskRay, thakis, hoyFB, benlangmuir. Herald added subscribers: cfe-commits, dexonsmith, hiraditya. Herald added a project: clang. - The getLine and getColumn functions need to update the position, or they will return stale data for buffe

[PATCH] D75903: [AArch64][CodeGen] Fixing stack alignment of HFA arguments on AArch64 PCS

ostannard added a comment. I've not looked at the code in detail yet, but I think this still gets the ABI wrong for this example: typedef struct { __attribute__ ((__aligned__(32))) double v[4]; } TYPE1; double func(double d0, double d1, double d2, double d3, double d4,

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

ostannard added a comment. Reproducer for that crash: P8198 P8199 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___ cfe-commits ma

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

ostannard added a comment. There's a failure on an ARM 2-stage buildbot which looks related to this: http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/3920/steps/build%20stage%202/logs/stdio I can reproduce the crash on the buildbot machine, so hopefully I'll have a standalon

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added a comment. I think I've spotted a bug in the ABI spec, which you've faithfully implemented here. I don't know of any other compiler which has implemented this ABI change yet, so it's probably worth seeing if we can get the spec fixed. The intention of the ABI is to avoid conflic

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added a comment. > It will require changing all possible initializations, with a sensible value. Where are you seeing multiple initializations? It looks like all of the logic for struct layout happens in `CGRecordLowering::lower`, we might need to add a pass there to calculate the val

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

ostannard added a comment. Why are you doing this in CodeGen, rather than adjusting the existing layout code in CGRecordLowering? Doing it this way will result in AdjustAAPCSBitfieldLValue being called for every access to the bitfield, rather than just once. This is probably more fragile too, b

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7/new/ https://reviews.llvm.org/D7 _

[PATCH] D72221: Support function attribute patchable_function_entry

ostannard added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3995 +``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs +before the function entry and N-M NOPs after the function entry. This attributes +takes precedence over command

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,M]

ostannard added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:77 "invalid unwind library name in argument '%0'">; +def err_drv_incompatible_options : Error<"'%0' and '%1' cannot be used together">; def err_drv_incompatible_unwindlib : Error<

[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67216/new/ https://reviews.llvm.org/D67216 ___ cfe-commits mailing list cfe-com

[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

ostannard added a comment. How about `-f[no-]force-dwarf-frame`, which I think matches the spelling and behaviour of the existing `-fforce-enable-int128` and `-fforce-emit-vtables` options? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67216/new/ https://reviews.llvm.org/D67216 ___

[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

ostannard added a comment. I don't like the idea of adding a `-gno-dwarf-frame` option which doesn't always disable emission of `.debug_frame`. Since it doesn't make sense to emit other debug info without `.debug_frame`, maybe we should just not have a negative option? CHANGES SINCE LAST ACTI

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

This revision was automatically updated to reflect the committed changes. Closed by commit rG9f6a873268e1: Dead Virtual Function Elimination (authored by ostannard). Changed prior to commit: https://reviews.llvm.org/D63932?vs=218363&id=224568#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. > This makes the IR self-contained, which is good, but it also make the > interpretation of the IR modal, which isn't great. It means that suddenly the > rules of interpretation of what is valid to do or not changes according to > this module flag. I think the origin

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D67467: [ARM] Update clang for removal of vfp2d16 and vfp2d16sp

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dmgreen. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67467/new/ https://reviews.llvm.org/D67467

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67160/new/ https://reviews.llvm.org/D67160 _

[PATCH] D67216: [cfi] Add flag to always generate call frame information

ostannard requested changes to this revision. ostannard added a comment. This revision now requires changes to proceed. Does the name of the `-falways-need-cfi` option match any existing compiler (google doesn't find anything)? If not, I think it would make more sense as a `-g` option, as it's s

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

ostannard requested changes to this revision. ostannard added a comment. This revision now requires changes to proceed. Test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67160/new/ https://reviews.llvm.org/D67160 __

[PATCH] D65863: [ARM] Add support for the s,j,x,N,O inline asm constraints

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM with one small change. Comment at: clang/test/Sema/arm_inline_asm_constraints.c:47 +// I: An immediate integer valid for a data-processing instruction. (ARM/Thumb

[PATCH] D66588: [ARM NEON] Avoid duplicated decarations

ostannard added a comment. LGTM with one small nit. Comment at: clang/utils/TableGen/NeonEmitter.cpp:1906 std::string Intrinsic::generate() { + // Avoid duplicated code for big and small endians + if (isBigEndianSafe()) { s/small endians/little endian/ Rep

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard marked 4 inline comments as done. ostannard added a comment. > It isn't that common, but it seems worth doing if it can be done easily. > That said, I note that it does appear that your implementation will end up > preserving the pointer in the vtable in this case because you're rely

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard updated this revision to Diff 218363. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/Options.td clang/lib/CodeGe

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard updated this revision to Diff 212833. ostannard added a comment. - Add LTOPostLink metadata, instead of internalising vcall visibility at LTO time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. > Partial linking will indeed prevent dropping the virtual functions, but it > should not prevent clearing the pointer to the virtual function in the > vtable. The linker should then be able to drop the virtual function body as > part of `--gc-sections` during the fin

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. > - Take the example from my earlier message, give the "main executable" and > "DSO" hidden visibility, build the "main executable" with LTO and the "DSO" > without LTO, and statically link them both into the same executable. We run > into the same problem where the P

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard updated this revision to Diff 212370. ostannard marked 2 inline comments as done. ostannard added a comment. - Rebase - Don't emit llvm.assume when not necessary (we already weren't checking for it's presence in GlobalDCE) - s/"public"/"default"/ in IR docs Repository: rG LLVM Githu

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. In that example, with everything having default ELF visibility, all of the vtables will get vcall_visibility public, which can't be optimised by VFE, and won't ever be relaxed to one of the tighter visibilities. The cases which can be optimised are (assuming "visibili

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D64416: [AArch64] Add support for Transactional Memory Extension (TME)

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64416/new/ https://reviews.llvm.org/D64416 ___ cfe-commits mailing list cfe-com

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63932/new/ https://reviews.llvm.org/D63932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D64169: ARM MTE stack sanitizer.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM with one minor nit. Comment at: llvm/include/llvm/Bitcode/LLVMBitCodes.h:632 ATTR_KIND_WILLRETURN = 61, + ATTR_KIND_SANITIZE_MEMTAG = 62 }; P

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

ostannard added a comment. There are a number of buildbot failures which look related to this, e.g. - http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/6730/steps/ninja%20check%201/logs/FAIL%3A%20UBSan-AddressSanitizer-armhf%3A%3Adiv-zero.cpp - http://lab.llvm.org:8011/builders/cl

[PATCH] D63936: [clang][Driver][ARM] Favor -mfpu over default CPU features

ostannard added inline comments. Comment at: llvm/lib/Support/ARMTargetParser.cpp:412 - if (Extensions & AEK_CRC) -Features.push_back("+crc"); - else -Features.push_back("-crc"); - - if (Extensions & AEK_DSP) -Features.push_back("+dsp"); - else -Features.pus

[PATCH] D64048: [TargetParser][ARM] Account dependencies when processing target features

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64048/new/ https://reviews.llvm.org/D64048 _

[PATCH] D63936: [clang][Driver][ARM] Favor -mfpu over default CPU features

ostannard added inline comments. Comment at: llvm/lib/Support/ARMTargetParser.cpp:412 - if (Extensions & AEK_CRC) -Features.push_back("+crc"); - else -Features.push_back("-crc"); - - if (Extensions & AEK_DSP) -Features.push_back("+dsp"); - else -Features.pus

[PATCH] D63936: [ARM] Minor fixes in command line option parsing

ostannard added a comment. > The second change this patch makes Could this be spilt into two patches? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63936/new/ https://reviews.llvm.org/D63936 ___ cfe-c

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard added a comment. Without switching to `llvm.type.checked.load`, the optimisation would trigger in some cases where it shouldn't trigger, thus miscompiling the code. This is because it needs to know about _every_ virtual call site, if we lose the information about some of them then it

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

ostannard created this revision. ostannard added reviewers: mehdi_amini, pcc, majnemer, tejohnson. Herald added subscribers: dang, dexonsmith, steven_wu, hiraditya, kristof.beyls, Prazek, javed.absar. Herald added projects: clang, LLVM. Currently, it is hard for the compiler to remove unused C++

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. Ah, right. In that case, this LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63437/new/ https://reviews.llvm.org/D63437 __

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

ostannard added a comment. This doesn't look like the right pace to fix this - the backend can handle vectors of i8 and i16, which are also not legal types, so why can't it correctly handle vectors of f16? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60710/new/ https://reviews.llvm.org/D60710 ___ cfe-commits mailing list cfe-com

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

ostannard added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:465 + MVE |= MVE_INT | MVE_FP; + HW_FP |= HW_FP_SP | HW_FP_HP; } SjoerdMeijer wrote: > ostannard wrote: > > Does this also need to set FPU and HasLegalHalfType? > Yep, tha

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

ostannard added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:465 + MVE |= MVE_INT | MVE_FP; + HW_FP |= HW_FP_SP | HW_FP_HP; } Does this also need to set FPU and HasLegalHalfType? CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D62998: [ARM] Fix bugs introduced by the fp64/d32 rework.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. LGTM with one change. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:294 + std::vector FeaturesAfter; + You explained it in the commit message

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. Fair enough, I don't think we currently try to diagnose any other invalid combinations of features. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60697/new/ https://revie

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

ostannard added inline comments. Comment at: llvm/lib/Support/ARMTargetParser.cpp:551 + + // If the default FPU already supports double-precision, or if + // there is no double-prec FPU that extends it, then "fp.dp" Could you also add tests for the erro

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

ostannard requested changes to this revision. ostannard added a comment. This revision now requires changes to proceed. This still needs tests adding. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60697/new/ https://reviews.llvm.org/D60697

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

ostannard added inline comments. Comment at: clang/test/Driver/arm-mfpu.c:112 +// CHECK-VFP3XD-NOT: "-target-feature" "+fp64" +// CHECK-VFP3XD-NOT: "-target-feature" "+32" // CHECK-VFP3XD: "-target-feature" "+vfp3" "+d32" ? Comment at: llvm/li

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

ostannard edited reviewers, added: ostannard; removed: olista01. ostannard added a comment. For the auto-upgrader, these limited FPUs only exist for microcontrollers, where I doubt any users are keeping bitcode files around for a long time, so I'd be fine with not doing this. I've had a skim thr

[PATCH] D60828: [ARM] Fix armv8 features tree and add fp16fml

ostannard added a reviewer: simon_tatham. ostannard added inline comments. Comment at: lib/Basic/Targets/ARM.cpp:443 HasLegalHalfType = true; + HW_FP |= HW_FP_SP | HW_FP_DP | HW_FP_HP; + FPU |= VFP4FPU; Is it always correct to set HW_FP_DP here,

[PATCH] D60721: [ARM] Check codegen of v8.2a intrinsics

ostannard requested changes to this revision. ostannard added a comment. This revision now requires changes to proceed. Clang tests should just cover the C->IR translation, and not depend on the LLVM backends. This should instead be an IR->asm test in the LLVM repository. Repository: rC Clang

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

ostannard requested changes to this revision. ostannard added a comment. This revision now requires changes to proceed. The document you linked in the LLVM change (https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=vs-2019#return-values) says that small POD types are r