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

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: dberris, kristof.beyls, nickdesaulniers, rnk, rsmith, void. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay added a parent revision: D72221: Support function attribute patchable_function_entry. MaskRay added su

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236221. MaskRay added a comment. Move a line from D7 to D72221 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72221/new/ https://reviews.l

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

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236222. MaskRay added a comment. Forgot to add test/Driver/fpatchable-function-entry.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7/new/ https://reviews.llvm.org/D7 Files: clang/include/clang/Basi

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236225. MaskRay added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72221/new/ https://reviews.llvm.org/D72221 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Att

[PATCH] D69758: [Gnu toolchain] Look at standard GCC paths for libstdcxx by default

2020-01-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Patch by sthibaul (Samuel Thibault) @kristina As a belated advice... since LLVM has moved from svn to git, we can ask contributors's names and emails, and then do `git commit --amend --author 'name '` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72247: Add Triple::isX86()

2020-01-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: craig.topper, jyknight, reames, skan. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, hiraditya. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72247 Files: clang/lib/A

[PATCH] D72236: [Clang] Force rtlib=platform in test to avoid fails with CLANG_DEFAULT_RTLIB

2020-01-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72236/new/ https://reviews.llvm.org/D72236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D72227: Add options for clang to align branches within 32B boundary

2020-01-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2010 +static void AlignBranchesOptions(const Driver &D, const ArgList &Args, + ArgStringList &CmdArgs) { The convention is `camelCase`. This file is

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236468. MaskRay marked 13 inline comments as done. MaskRay added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72221/new/ https://reviews.llvm.org/D72221 Files: clang/inclu

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:686 +def PatchableFunctionEntry : InheritableAttr { + let Spellings = [GCC<"patchable_function_entry">]; aaron.ballman wrote: > Should this be inheriting from `TargetSpecificAttr` as

[PATCH] D72247: Add Triple::isX86()

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @craig.topper Good to go? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72247/new/ https://reviews.llvm.org/D72247 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D72247: Add Triple::isX86()

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6904cd948674: Add Triple::isX86() (authored by MaskRay). Herald added subscribers: fedor.sergeev, emaste. Changed prior to commit: https://reviews.llvm.org/D72247?vs=236300&id=236477#toc Repository: r

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:389 +if (Triple.getEnvironment() != llvm::Triple::UnknownEnvironment) { + switch (Triple.getEnvironment()){ +case llvm::Triple::ELFv1: Which ABI does `powerpc64le-linux-gnu`

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Are you still using -target powerpc64-unknown-freebsd12.0-elfv2 or -target powerpc64-unknown-freebsd12.0-elfv1 added in https://reviews.llvm.org/rL361355 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72306/new/ https://r

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the patch. Will take some time to review. Without this, adding -Wrange-loop-analysis to -Wall will make lots of projects fail to build if they have -Werror. For example, `for (const absl::string_review : ...)` or `const std::string_view` is common. Reposito

[PATCH] D72352: [Triple] Use elfv2 on freebsd>=13 and linux-musl

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: Bdragon28, dim, adalava. Herald added subscribers: llvm-commits, cfe-commits, steven.zhang, jsji, dexonsmith, kbarton, hiraditya, krytarowski, arichardson, nemanjai, emaste. Herald added projects: clang, LLVM. The environments "elfv1" and "e

[PATCH] D72352: [Triple] Use elfv2 on freebsd>=13 and linux-musl

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236640. MaskRay added a comment. Simplify Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72352/new/ https://reviews.llvm.org/D72352 Files: clang/lib/Basic/Targets/PPC.h clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D72352: [PowerPC][Triple] Use elfv2 on freebsd>=13 and linux-musl

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8edf759ca7e1: [PowerPC][Triple] Use elfv2 on freebsd>=13 and linux-musl (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72352/new/ http

[PATCH] D72352: [PowerPC][Triple] Use elfv2 on freebsd>=13 and linux-musl

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236647. MaskRay added a comment. Add powerpc64-unknown-freebsd11 (its expected EOL is September 30, 2021) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72352/new/ https://reviews.llvm.org/D72352 Files: clang

[PATCH] D72352: [PowerPC][Triple] Use elfv2 on freebsd>=13 and linux-musl

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236649. MaskRay retitled this revision from "[Triple] Use elfv2 on freebsd>=13 and linux-musl" to "[PowerPC][Triple] Use elfv2 on freebsd>=13 and linux-musl". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a subscriber: shch

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Obsoleted by D72352 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72306/new/ https://reviews.llvm.org/D72306 ___ cfe-commits mailing list cf

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp:6 + struct Record { +volatile int a; +int b; `test_POD` can be dropped. It does not add test coverage. "Struct with a volatile member no longer

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Commit eec0240f97180ea876193dcfa3cb03cb652d9fe3 "Adds -Wrange-loop-analysis to -Wall" was premature before this fix, as

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/SemaCXX/warn-range-loop-analysis.cpp:22 struct Bar { + // The type is too large to suppress the warning for trivially copyable types. + char s[128]; `too large to suppress` means it does not suppress the w

[PATCH] D72363: [PowerPC] Default ppc64 linux-gnu/freebsd to -fno-PIC

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: PowerPC, adalava, Bdragon28, sfertile, stefanp. Herald added subscribers: cfe-commits, steven.zhang, shchenz, jsji, kbarton, krytarowski, arichardson, nemanjai, emaste. Herald added a project: clang. According to D53384

[PATCH] D72363: [PowerPC] Default ppc64 linux-gnu/freebsd to -fno-PIC

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG907a0cadb2c8: [PowerPC] Default ppc64 linux-gnu/freebsd to -fno-PIC (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72363/new/ https://

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

2020-01-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236730. MaskRay marked 2 inline comments as done. MaskRay retitled this revision from "[Driver][CodeGen] Add -fpatchable-function-entry=N[,M]" to "[Driver][CodeGen] Add -fpatchable-function-entry=N[,0]". MaskRay added reviewers: ostannard, peter.smith. MaskRa

[PATCH] D72363: [PowerPC] Default ppc64 linux-gnu/freebsd to -fno-PIC

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72363#1810036 , @sfertile wrote: > LGTM: I really didn't like setting PIC as the default just to work around > some codegen bugs. I believe there are PowerPC codegen tests that will fail > with this change though (or perhaps

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

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay planned changes to this revision. MaskRay added a comment. @ostannard @nickdesaulniers @peter.smith Unfortunately we will have to get buy-in from GCC before committing to do an incompatible design. A promise that they will implement a new approach to overcome the disadvantages and an ag

[PATCH] D72227: Add options for clang to align branches within 32B boundary

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2200 + "from being across or against the boundary of specified size. " + "-x86-align-branch-boundary=0 doesn't align branches.">; +def malign_branch_EQ > -x86-align

[PATCH] D72433: [PowerPC] Change SubArchType if -mspe is specified

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: PowerPC, Bdragon28, jhibbits. Herald added subscribers: cfe-commits, shchenz, jsji, kbarton, nemanjai. Herald added a project: clang. Follow-up of D72014 . Repository: rG LLVM Github Monorepo https://revi

[PATCH] D72433: [Driver][PowerPC] Move powerpcspe logic from cc1 to Driver

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236961. MaskRay retitled this revision from "[PowerPC] Change SubArchType if -mspe is specified" to "[Driver][PowerPC] Move powerpcspe logic from cc1 to Driver". MaskRay edited the summary of this revision. MaskRay removed subscribers: wuzish, merge_guards_bot

[PATCH] D72433: [Driver][PowerPC] Move powerpcspe logic from cc1 to Driver

2020-01-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 236962. MaskRay added a comment. Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72433/new/ https://reviews.llvm.org/D72433 Files: clang/lib/Basic/Targets/PPC.cpp clang/lib/Driver/ToolChains/Ar

[PATCH] D72227: Add options for clang to align branches within 32B boundary

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2196 +: Joined<["-"], "malign-branch-boundary=">, + Group, + Flags<[DriverOption, HelpHidden]>, `Group` Comment at: clang/include/clang/Driver/Option

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: skan, craig.topper, LuoYuanke, annita.zhang, jyknight, reames, chandlerc, tstellar. Herald added a project: clang. Herald added a subscriber: cfe-commits. Based on D72227 . Add -mbranches-within-32B-boundari

[PATCH] D72227: Add options for clang to align branches within 32B boundary

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Created D72463 (`-mbranches-within-32B-boundaries` can be overridden by `-malign-branch*`; added test coverage) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 __

[PATCH] D72227: Add options for clang to align branches within 32B boundary

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045 + + if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, + options::OPT_mno_branches_within_32B_boundaries, false)) { LuoYuanke wrote: > MaskRay wro

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

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Some targets don't have a PC relative relocation of pointer size, e.g. `R_MIPS_PC64` (MIPS only has `R_MIPS_PC32`, which is a GNU extension, sigh https://sourceware.org/git/?p=binutils-gdb.git;a=commit;f=include/elf/mips.h;h=092dcd755dcdcf

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @skan Does this look good to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72463/new/ https://reviews.llvm.org/D72463 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 237251. MaskRay added a comment. -malign-branch-boundary= is the primary option. So swap the roles Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72463/new/ https://reviews.llvm.org/D72463 Files: clang/includ

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 237253. MaskRay marked 5 inline comments as done. MaskRay added a comment. Delete a `AlignBranch = StringRef()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72463/new/ https://reviews.llvm.org/D72463 Files:

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2040 +if (Value.getAsInteger(10, AlignBranchBoundary) || +AlignBranchBoundary < 16 || !llvm::isPowerOf2_64(AlignBranchBoundary)) { + D.Diag(diag::err_drv_invalid_argument_to_option)

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added a comment. The commutative property (independence of options) makes option composition easier. clangDriver makes heavy use of `getLastArg` and `hasArg`. Without the commutative property, it would now be able to shuffle code around. The `-m

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72463#1813658 , @skan wrote: > Add revison D72225 as parent? `-x86-align-branch-prefix-size=` does not work without D72225 , but I think the two patches can

[PATCH] D72221: Support function attribute patchable_function_entry

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa44c434b68e5: Support function attribute patchable_function_entry (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72221/new/ https://re

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

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf17ae668a96e: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0] (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7/new/ https:/

[PATCH] D72433: [Driver][PowerPC] Move powerpcspe logic from cc1 to Driver

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGba91dffafe4d: [Driver][PowerPC] Move powerpcspe logic from cc1 to Driver (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72433/new/ htt

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 237476. MaskRay edited the summary of this revision. MaskRay added a comment. Delete -mno-branches-within-32B-boundaries (GNU as does not support it) Use comma joined list instead of +. CommaJoined is more conventional. For example, `-mrecip=` and `-fsanitize

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 5 inline comments as done. MaskRay added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:254 +def err_drv_invalid_malign_branch_EQ : Error< + "invalid argument '%0' to -malign-branch=; must be one of: %1">; + skan w

[PATCH] D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time

2020-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I am still confused why you need the special rule for `__atomic_is_lock_free` (GCC/clang) and `__c11_atomic_is_lock_free` (clang). https://github.com/gcc-mirror/gcc/blob/master/gcc/builtins.c#L7300-L7314 GCC `__atomic_is_lock_free` expands to either 1 or a library call

[PATCH] D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

2020-01-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: adalava, dim, nemanjai, jfb, rsmith. Herald added subscribers: cfe-commits, steven.zhang, dexonsmith, krytarowski, arichardson, emaste. Herald added a project: clang. MaxAtomicPromoteWidth is defined as "the maximum width lock-free atomic op

[PATCH] D72591: [clang] [test] Fix riscv-toolchain-extra to be less picky about paths

2020-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Build llvm with `-DCLANG_RESOURCE_DIR=../lib/clang/99` Failing Tests (4): Clang :: Driver/modules.m Clang :: Driver/nostdlibinc.c Clang :: Driver/riscv32-toolchain-extra.c

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

2020-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D70919#1818355 , @nathanchance wrote: > I am seeing the same failure that @leonardchan reported above. This is > related to `-DCLANG_DEFAULT_LINKER=lld`: > > $ cmake -G Ninja \ > -Wno-dev \ > -DCMAKE_BUILD_TY

[PATCH] D72668: [Driver][test] Fix Driver/hexagon-toolchain-elf.c for -DCLANG_DEFAULT_LINKER=lld builds

2020-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: leonardchan, nathanchance, sidney, bcain, kparzysz. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72668 Files: clang/test/Driver/hexagon-toolchain-el

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

2020-01-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. https://reviews.llvm.org/D72668 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70919/new/ https://reviews.llvm.org/D70919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D65616#1819581 , @serge-sans-paille wrote: > (back on that one) The default in clang is implicitly > `-fno-semantic-interposition`. I think we can safely support it, and either > warn or error on `-fsemantic-interposition`.

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: hfinkel, tstellar, Romain-Geissler-1A, serge-sans-paille. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fedora wants to build projects with -fno-semantic-interposition (e.g. https://fedoraproject.org/wiki/Changes/Pyt

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 238065. MaskRay added a comment. Use Flag<> instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72724/new/ https://reviews.llvm.org/D72724 Files: clang/include/clang/Driver/Options.td clang/test/Driver/c

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72724#1820362 , @serge-sans-paille wrote: > LGTM I don't follow Fedora news. It is just that a friend informed me of this -fno-semantic-interposition movement two days ago and I postulated that it was your motivation to pi

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5d1b3ba68769: [Driver] Ignore -fno-semantic-interposition (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72724/new/ https://reviews.ll

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72724#1820392 , @serge-sans-paille wrote: > @MaskRay: is that okay with you if I start implementing > -fsemantic-interposition based on `dso_preemptable`? I haven't done anything in this area. Go ahead:) I'd like to help or

[PATCH] D72724: [Driver] Ignore -fno-semantic-interposition

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ah, I'll want to figure out how to emit local aliases to dso_local as a follow-up of D72197 , but that should be orthogonal to what you want to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D72668: [Driver][test] Fix Driver/hexagon-toolchain-elf.c for -DCLANG_DEFAULT_LINKER=lld builds

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1ca51c06729d: [Driver][test] Fix Driver/hexagon-toolchain-elf.c for… (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D72668?vs=237816&id=238085#toc Repository: rG LLVM Github

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 238120. MaskRay marked 2 inline comments as done. MaskRay added a comment. Make driver a thin wrapper since we are going to add a master option in MC (D72738 ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

2020-01-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5ca24d09aefa: [Driver][X86] Add -malign-branch* and -mbranches-within-32B-boundaries (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D72463?vs=238120&id=238167#toc Repository:

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:1992 +def fsplit_machine_functions : Flag <["-"], "fsplit-machine-functions">, + Group, Flags<[CC1Option]

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2131 +.. option:: -fsplit-machine-functions, -fno-split-machine-functions + You don't need to edit this file. It is auto-generated from Options.td Comment at:

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/compress.c:21 -// RUN: %clang -### -fintegrated-as -gz=none -x assembler -c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_NONE %s -// RUN: %clang -### -fintegrated-as -gz=none -c %s 2>&1 | FileCheck -check-prefi

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:1 +; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s + You can drop `-unknown-linux

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hip-gz-options.hip:1 +// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target + clang-driver is for some legacy cygwin stuff. It should not be needed. Does --offload-arch=gfx906 have

[PATCH] D87321: Fix -gz=zlib options for linker

2020-09-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/hip-gz-options.hip:14 +// CHECK-DAG: {{".*clang.*" .* "--compress-debug-sections=zlib"}} +// CHECK: {{"--compress-debug-sections=zlib"}} -

[PATCH] D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency (NFC)

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LG. (Nit: technically this is not NFC: it affects command line options and runtime function names (ABI changes).) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87622/new/ https://reviews.llvm.org/D87622 __

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/fsplit-machine-functions.c:3 +// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-split-mach

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/fsplit-machine-functions.c:3 +// RUN: %clang -### -target x86_64 -fprofile-use=default.profdata -fsplit-machine-functions -fno-

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:518 + if (CodeGenOpts.SplitMachineFunctions) { +if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone) I did not pay attention. Such compatibility checks should be added

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2000 +defm split_machine_functions: OptInFFlag<"split-machine-functions", + "Enable", "Disable", " late function splitting using profile information (x86-elf only)">; + If this can

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:2000 +defm split_machine_functions: OptInFFlag<"split-machine-functions", + "Enable", "Disable", " late function spli

[PATCH] D87737: Add -fprofile-update={atomic,prefer-atomic,single}

2020-09-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: davidxl, vsk, xur. Herald added subscribers: cfe-commits, dang, jfb. Herald added a project: clang. MaskRay requested review of this revision. GCC 7 introduced -fprofile-update={atomic,prefer-atomic} (prefer-atomic is for best efforts (some t

[PATCH] D87737: Add -fprofile-update={atomic,prefer-atomic,single}

2020-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I think an interesting extension (not in GCC) is `-fprofile-update=set`, a boolean counter like `-covermode set` in go: sometimes we don't need the numbers at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87737/new/

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

2020-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. > This patch avoid the issue by defaulting to -mstackrealign, just like gcc. This sentence from the description should be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ ht

[PATCH] D87837: [Driver] Remove the deprecation warning for -fuse-ld=/abs/path

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: chandlerc, hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay requested review of this revision. The feature has been used in the widespread build system Bazel https://github.com/bazelbuild/bazel/commit/cdd0c

[PATCH] D87791: [CUDA][HIP] Fix -gsplit-dwarf option

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (Note that ideally -gsplit-dwarf should not imply -g2 but it currents does so. And Clang and GCC have not agreed whether we should add a new flag like -fsplit-dwarf. /For -gsplit-dwarf builds, it is the best to ensure -g is also specified/.) CHANGES SINCE LAST ACTION

[PATCH] D87791: [CUDA][HIP] Fix -gsplit-dwarf option

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:918 if (FinalOutput && Args.hasArg(options::OPT_c)) { -SmallString<128> T(FinalOutput->getValue()); +SmallString<128> T(llvm::sys::path::stem(FinalOutput->getValue())); +AddPostfi

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Several previous comments are FreeBSD specific. To we clang developers, the concrete request is > Given that GCC will inline at -O, at least these days, ... right? I think this makes sense, especially when `inline` is explicitly specified... This appears to be related

[PATCH] D58531: [clang] Specify type of pthread_create builtin

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2058 +/// Missing a type from +GE_Missing_pthread }; You can append a comma so that next time the list gets appended the diff does not have to touch this line. Reposito

[PATCH] D85810: [clang] Pass-through remarks options to linker

2020-09-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:90 + } else { +if (Output.isFilename()) + F = Output.getFilename(); The output selection logic is untested. Comment at: clang/test/Driver/opt-record

[PATCH] D85810: [clang] Pass-through remarks options to linker

2020-09-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:95 +if (F.empty()) + F = llvm::sys::path::stem(Input.getBaseInput()); + } Is the `F.empty()` case unreachable? Please delete the code or add an assert ==

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 292973. MaskRay retitled this revision from "[Driver] Remove the deprecation warning for -fuse-ld=/abs/path" to "[Driver] Add disabled-by-default -Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path". MaskRay edited the summary of this revisio

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 292989. MaskRay retitled this revision from "[Driver] Add disabled-by-default -Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path" to "[Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path". MaskRa

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9087209314ca: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

2020-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I have tested this patch internally and seen gains and losses. On one document search related benchmark 3~5% improvement. One zippy (snappy) there is 3~5% regression. Perhaps we do need a conditional extra SROA run. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D87953: [xray] Function coverage groups

2020-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The idea seems fine. > By selecting different groups over time you could cover the entire > application incrementally with lower overhead than instrumenting the entire > application at once. How large the overhead is? This is somewhat surprising to me. =

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @rnk @amccarth Do you have more comments? ☺️ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

2020-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The `(aWidth - 1) - clzsi(a)` change is correct, but why is the ctz change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86547/new/ https://reviews.llvm.org/D86547 ___ cfe-commi

[PATCH] D87953: [xray] Function coverage groups

2020-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:814 +auto FuncGroups = CGM.getCodeGenOpts().XRayTotalFunctionGroups; +if (FuncGroups > 1) { + auto FuncName = ArrayRef(CurFn->getName().bytes_begin(), kyulee wrote: > Ma

[PATCH] D88021: [clang] Fix a misleading variable name. NFC.

2020-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! (I did notice this but I forgot to update it) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88021/new/ https://reviews.llvm.org/D

[PATCH] D87737: Add -fprofile-update={atomic,prefer-atomic,single}

2020-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87737/new/ https://reviews.llvm.org/D87737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D85810: [clang] Pass-through remarks options to linker

2020-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:104 + CmdArgs.push_back(Args.MakeArgString( + Twine("--plugin-opt=opt-remarks-format=") + Format.data()));

[PATCH] D87921: Fix -funique-internal-linkage-names to work with -O2 and new pass manager

2020-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D87921#2288395 , @Conanap wrote: > It looks like this commit is causing a few failures on nearly all PPC bots > and a sanitizer bot; would it be possible to revert this commit for now until > the issue is resolved? It would b

[PATCH] D87953: [xray] Function coverage groups

2020-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1136 + if (Opts.XRayTotalFunctionGroups < 1) { +const Arg *A = Args.getLastArg(OPT_fxray_function_groups); +Diags.Report(diag::err_drv_invalid_value) Errors here are not

<    8   9   10   11   12   13   14   15   16   17   >