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

2022-11-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2021-10-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-10-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2021-10-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2021-10-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-05-13 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-03-29 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-03-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2021-02-05 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2021-02-04 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-12-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-12-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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=

2020-12-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-11-19 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-10-12 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-10-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-09-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-09-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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,

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

2020-07-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-07-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-07-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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/

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

2020-07-02 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-06-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-06-12 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-06-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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:

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

2020-06-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-06-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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:

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

2020-06-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-05-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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.

2020-05-26 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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:

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

2020-05-20 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-04-20 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-04-09 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-03-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-03-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-03-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-03-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-03-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-03-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-02-20 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-02-20 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-02-11 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-01-27 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-01-20 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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,

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

2020-01-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2020-01-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2020-01-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 :

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

2019-10-29 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-10-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-10-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-10-11 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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=224568#toc Repository: rG LLVM Github Monorepo CHANGES

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

2019-10-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-10-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-23 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-19 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-16 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-09-12 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-11 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-09-05 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-04 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-09-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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.

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

2019-09-02 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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/

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

2019-09-02 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-09-02 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-08-01 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-08-01 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-31 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-30 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-30 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-22 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-19 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-07-10 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

[PATCH] D64169: ARM MTE stack sanitizer.

2019-07-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 };

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

2019-07-08 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 -

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

2019-07-04 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 -

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

2019-07-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-07-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 -

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

2019-07-01 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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 ___

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

2019-06-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-06-28 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-06-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-06-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-06-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-06-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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,

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

2019-06-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-06-07 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-06-05 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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/

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

2019-06-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-05-31 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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.

2019-05-03 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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:

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

2019-04-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-04-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

2019-04-15 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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

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

2019-04-11 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
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