[PATCH] D73937: [Driver] Change -fmax-tokens $arg to -fmax-tokens=$arg

2020-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: hans, rnk, rsmith, thakis. Herald added subscribers: cfe-commits, usaxena95, kadircet, ilya-biryukov. Herald added a project: clang. `-foption $arg` is very uncommon in clangDriver (especially a Separate without a corresponding Joined). Chan

[PATCH] D73937: [Driver] Change -fmax-tokens $arg to -fmax-tokens=$arg

2020-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 242236. MaskRay added a comment. Update documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73937/new/ https://reviews.llvm.org/D73937 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/inc

[PATCH] D73865: [CodeGenModule] Assume dso_local for -fpic -fno-semantic-interposition

2020-02-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay reopened this revision. MaskRay added a comment. Accidentally committed. Sorry.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73865/new/ https://reviews.llvm.org/D73865 ___ cfe-commits mailing

[PATCH] D73937: [Driver] Change -fmax-tokens $arg to -fmax-tokens=$arg

2020-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 242359. MaskRay added a comment. Rename to fmax_tokens_EQ as a convention Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73937/new/ https://reviews.llvm.org/D73937 Files: clang/include/clang/Basic/DiagnosticG

[PATCH] D73937: [Driver] Change -fmax-tokens $arg to -fmax-tokens=$arg

2020-02-04 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2513118afa38: [Driver] Change -fmax-tokens $arg to -fmax-tokens=$arg (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73937/new/ https:/

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

2020-02-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1856587 , @hans wrote: > In D7#1849326 , @hans wrote: > > > > I created D73680 to place the patch > > > label after BTI. > > > > > > @hans I

[PATCH] D69868: Allow "callbr" to return non-void values

2020-02-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. I think this is fine, but want to hear from @jyknight and @rnk. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:1039 + auto &Last = BB->back(); + for (unsigned i = 0; i < Last.getNumOperands();

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. If you don't mind, I can push a Diff to this Differential which will address these review comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:436 + + std::ifstream fin(FunctionsListFile); + if (!fin.good()) { `MemoryBuffer::getFi

[PATCH] D74298: Honor -finline-functions and -finline-hint-functions at -O0

2020-02-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Why you say regression, do you have a case where fcd33149b48885ab8e4ca4ffb6977bce5be2e623 actually regressed things? -O0 implies optnone. For `-Xclang -disable-O0-optnone -O0 -finline-functions`, the patch can indeed make a difference. `-disable-O0-optnone` is currently

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > In D68049#1865967 , @MaskRay wrote: > If you don't mind, I can push a Diff to this Differential which will address > these review comments. I can't because I can't figure out the patch relationship... First, this patch does no

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

2020-02-10 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. Such changes can be risky. Hope someone can verify the build on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74384/new/ https://revi

[PATCH] D74384: Use std::foo_t rather than std::foo in LLVM.

2020-02-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. You may join https://reviews.llvm.org/project/view/78/ and let the bot test Linux for you.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74384/new/ https://reviews.llvm.org/D74384 __

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

2020-02-11 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. Verified that coff, elf, and mach-o are printed as lower cases. I believe bpf is similar. Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2176 +outs() << ":\tfile

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

2020-02-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: grimar. MaskRay added a comment. Wait. I wonder whether we can change llvm-readobj to use lower case names as well. The following should be updated: StringRef ELFObjectFile::getFileFormatName() const { bool IsLittleEndian = ELFT::TargetEndianness == support::lit

[PATCH] D74433: [llvm-objdump] Print file format in lowercase to match GNU output.

2020-02-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I think the patch is good to go on its own, after the `outs() << ":\tfile format " << O->getFileFormatName().lower() << "\n\n";` change. If the consensus is to also change llvm-readobj output, we can teach `getFileFormatName()` to use lower case names and remove `.lower

[PATCH] D73904: [clang] stop baremetal driver to append .a to lib

2020-02-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. A probably better approach is to use something like `TC.getCompilerRT(Args, "profile")`. The function returns a full path which can avoid `-L` problems. std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,

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

2020-02-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. From https://gcc.gnu.org/ml/gcc/2020-02/msg00116.html > We discussed this on IRC in #gcc. There was no motivation to change GCC. The > platform that wants to avoid the libatomic dependency doesn't use GCC anyway. > Relying on not getting the libatomic dependency seems f

[PATCH] D74456: Reland "[Support] make report_fatal_error `abort` instead of `exit`"

2020-02-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `ninja check-llvm check-clang` passed on a powerpc64le machine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74456/new/ https://reviews.llvm.org/D74456 ___ cfe-commits mailing

[PATCH] D74051: Move update_cc_test_checks.py tests to clang

2020-02-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. @rsmith Does `clang/test/utils/` (a new directory) look appropriate to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74051/new/ https://reviews.llvm.org/D74051 ___

[PATCH] D74591: [Driver] Rename AddGoldPlugin to addLTOOptions. NFC

2020-02-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: evgeny777, tejohnson. Herald added subscribers: cfe-commits, dexonsmith, inglorion, emaste. Herald added a project: clang. AddGoldPlugin does more than adding `-plugin path/to/LLVMgold.so`. It works with lld and GNU ld, and adds other LTO opt

[PATCH] D74591: [Driver] Rename AddGoldPlugin to addLTOOptions. NFC

2020-02-14 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG597dfb3bd56c: [Driver] Rename AddGoldPlugin to addLTOOptions. NFC (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74591/new/ https://re

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

2020-02-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `test/CodeGen/opt-record-1.c` triggers an assertion failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing li

[PATCH] D73865: [CodeGenModule] Assume dso_local for -fpic -fno-semantic-interposition

2020-02-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay planned changes to this revision. MaskRay added a comment. I am thinking whether we may need fine-grained `-fno-semantic-interposition-{data,function}`. I've noticed one problem with an internal target `_multiarray_umath.so` `npy_ma_str_array_finalize`. I need to make more investigation

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/Sema.cpp:14 +#include "UsedDeclVisitor.h" #include "clang/AST/ASTContext.h" hliao wrote: > this file is missing and breaks the build Fixed by c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4 Repository: rG

[PATCH] D74698: [CodeGen] -pg shouldn't add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. When -mfentry is specified, why should frame pointers be disabled? Is that because the Linux kernel has assumption about the exact code sequence? `call __fentry__` is the first instruction. Isn't that sufficient? (There is another difference. GCC emits `call *__fentry__

[PATCH] D74698: [CodeGen] -pg shouldn't add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 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. In D74698#188 , @nickdesaulniers wrote: > In D74698#1881034 , @MaskRay wrote: > > > When -mfentry is spe

[PATCH] D74704: Support -fuse-ld=lld for riscv

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The problem may be `-DCLANG_DEFAULT_LINKER=lld`. (FWIW I really don't like supporting numerous -D configurations and ask authors to revert because of some weird -D configurations.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D74698: [CodeGen] -pg shouldn't unconditionally add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/fentry.c:6 +// RUN: %clang -pg -mfentry -O0 -emit-llvm -S -o - %s | FileCheck -check-prefix=FP --implicit-check-not='"frame-pointer"="none"' %s +// RUN: %clang -pg -mfentry -O2 -fno-omit-frame-pointer -emit-llvm -S -o

[PATCH] D74698: [CodeGen] -pg shouldn't unconditionally add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/mfentry.c:5 +// RUN: %clang -pg -mfentry -O0 -emit-llvm -S -o - %s | FileCheck -check-prefix=FP --implicit-check-not='"frame-pointer"="none"' %s +// RUN: %clang -pg -mfentry -O2 -fno-omit-frame-pointer -emit-llvm -S -o

[PATCH] D74698: [CodeGen] -pg shouldn't unconditionally add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/mfentry.c:5 +// RUN: %clang -pg -mfentry -O0 -emit-llvm -S -o - %s | FileCheck -check-prefix=FP --implicit-check-not='"frame-pointer"="none"' %s +// RUN: %clang -pg -mfentry -O2 -fno-omit-frame-pointer -emit-llvm -S -o

[PATCH] D74698: [CodeGen] -pg shouldn't unconditionally add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The title should be changed from CodeGen to Driver And `fn attr` is no longer appropriate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74698/new/ https://reviews.llvm.org/D74698 ___

[PATCH] D72825: [NFC] Fix options name typo

2020-01-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Can we just delete the options? The misspelled options were added in rC214906 to work around https://clang.debian.net/status.php?version=3.4.2&key=UNKNOWN_ARG . Actually the `-ftree*` options were not used. Apparently our option name

[PATCH] D72825: [NFC] Fix options name typo

2020-01-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > These three options may be used in other projects compiled by gcc. We can add them on demand. I don't see a rationale to add them preemptively. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72825/new/ https://reviews.llv

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

2020-01-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72463#1826499 , @annita.zhang wrote: > @MaskRay Did you merge it to LLVM 10 branch? It is included in the release branch. `git branch origin/release/10.x --contains 5ca24d09aefaedf8e4148c7fce4b4ab0c4ecc72a # suceeded` R

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

2020-01-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: dexonsmith. I think we should not rely too much on the driver emitting object files. There can be many subtle differences (D68897 ). When the -emit-obj output is feed to llvm-nm, the clang cc1 command line sho

[PATCH] D73072: [Driver][CodeGen] Support -fpatchable-function-entry=N,M where M>0

2020-01-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, craig.topper, nickdesaulniers, nsz, ostannard, peter.smith. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay added a parent revision: D73071: [X86] Support -fpatchable-function-entry=N,M where M>0

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/IR/Globals.cpp:101 +return true; + return isInterposableLinkage(getLinkage()); +} MaskRay wrote: > Checking `isInterposableLinkage(getLinkage())` first may be more efficient. `if (!isInterposableLinkage(get

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The logic looks good to me. Linkages which were not interposable before and can be interposable now: available_externally, linkonce_odr, weak_odr, external, and appending. `isDefinitionExact` returned true for external before, and can return false if SemanticInterposit

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/IR/Globals.cpp:101 +return true; + return isInterposableLinkage(getLinkage()); +} MaskRay wrote: > MaskRay wrote: > > Checking `isInterposableLinkage(getLinkage())` first may be more efficient. > `if (!isIn

[PATCH] D73072: [Driver][CodeGen] Support -fpatchable-function-entry=N,M where M>0

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 239400. MaskRay added a comment. Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73072/new/ https://reviews.llvm.org/D73072 Files: clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basi

[PATCH] D69868: Allow "callbr" to return non-void values

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D69868#1832559 , @void wrote: > @jyknight Do you think you'll have time to review this patch this week? I'd > like to get it into the 10.0 release if possible. :-) I volunteer as a reviewer:) Repository: rG LLVM Github Mo

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72829#1831017 , @serge-sans-paille wrote: > > Linkages which were not interposable before and can be interposable now: > > available_externally, linkonce_odr, weak_odr, external, and appending. > > @MaskRay I understand the m

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-22 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: llvm/include/llvm/IR/GlobalValue.h:287 + bool isDSOPreemptable() const { return !IsDSOLocal; } + MaskRay wrote: > It seems that this uti

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72829#1835933 , @serge-sans-paille wrote: > Add Release note and doc. @MaskRay can you confirm current state is ok? I feel that we are still behind a complete `-fsemantic-interposition`... I don't know whether we should men

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: hans. MaskRay added a comment. In D7#1836409 , @peter.smith wrote: > Although this particular commit will not be at fault, it is the option that > enables the feature which is the earliest I can bisect the fault to. There

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1836645 , @nickdesaulniers wrote: > In D7#1836610 , @MaskRay wrote: > > > In D7#1836409 , @peter.smith > > wrote: > > > > > Altho

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1836669 , @hans wrote: > In D7#1836643 , @peter.smith > wrote: > > > >> If the patchable functions is intended for clang-10 we'll need to make > > >> sure any fix is merged t

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1836942 , @nickdesaulniers wrote: > In D7#1836687 , @MaskRay wrote: > > > (I really dislike how the feature was developed on the GCC side. Yet > > another Linux-kernel specif

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:488 + if (Context.getLangOpts().SemanticInterposition && + (Context.getLangOpts().PICLevel && !Context.getLangOpts().PIE)) +// Require various optimization to

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1836409 , @peter.smith wrote: > Although this particular commit will not be at fault, it is the option that > enables the feature which is the earliest I can bisect the fault to. There > are 3 files in linux that assert

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

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1837358 , @nickdesaulniers wrote: > In D7#1837184 , @MaskRay wrote: > > > @peter.smith The build was smooth. Do I need other options to reproduce? > > > I was able to reproduc

[PATCH] D73072: [Driver][CodeGen] Support -fpatchable-function-entry=N,M and __attribute__((patchable_function_entry(N,M))) where M>0

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 240064. MaskRay retitled this revision from "[Driver][CodeGen] Support -fpatchable-function-entry=N,M where M>0" to "[Driver][CodeGen] Support -fpatchable-function-entry=N,M and __attribute__((patchable_function_entry(N,M))) where M>0". MaskRay added a comme

[PATCH] D73072: [Driver][CodeGen] Support -fpatchable-function-entry=N,M and __attribute__((patchable_function_entry(N,M))) where M>0

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

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-01-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp: + this->getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage) { +std::string UniqueSuffix = getUniqueModuleId(&getModule(), true); +if (!UniqueSuffix.empty()) { --

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2685 + if (Args.hasArg(OPT_fsemantic_interposition)) +Opts.SemanticInterposition = 1; + ` Opts.SemanticInterposition = Args.hasArg(OPT_fsemantic_interposition);` =

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

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1838988 , @mrutland wrote: > In D7#1837536 , @MaskRay wrote: > > > > > > > > > For `BTI c` issue, GCC has several releases that do not work with > > -mbranch-protection=bti. T

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1959 +def fno_unique_internal_funcnames : Flag <["-"], "fno-unique-internal-funcnames">, + Group, Flags<[CC1Option]>; + `, Flags<[CC1Option]>` can be deleted. Co

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The code change seems fine, but the test requires some changes. I haven't followed Propeller development, but hope someone with profile experience can confirm InternalLinkage is the only linkage we need to care about (otherwise the option will be a misnomer if we ever e

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-01-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D73307#1839880 , @mtrofin wrote: > In D73307#1839829 , @MaskRay wrote: > > > The code change seems fine, but the test requires some changes. I haven't > > followed Propeller development,

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

2020-01-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: wuzish. At a minimum, `test/Driver/freebsd.c` and `test/Preprocessor/init-ppc64.c` should be updated for `powerpcle-unknown-freebsd` tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73425/new/

[PATCH] D69868: Allow "callbr" to return non-void values

2020-01-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. It seems that callbr (with output) will now be similar to a catchpad. It can set live-in physical register information. (See `test/CodeGen/X86/{seh-catch-all.ll,seh-exception-code.ll,wineh-coreclr.ll,wineh-exceptionpointer.ll}`) Is there any caveat doing this? Add @rnk

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:488 + if (Context.getLangOpts().SemanticInterposition && + (Context.getLangOpts().PICLevel && !Context.getLangOpts().PIE)) +// Require various optimization to respect semantic interposition.

[PATCH] D73319: Add extension 'gnu_asm_goto_with_outputs'

2020-01-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1273 + int y; + asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err); + return y; Is the canonical spelling `%l2`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D69876: Allow output constraints on "asm goto"

2020-01-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Does this depend on D69868 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 ___ cfe-commits mailing l

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D72829#1844050 , @serge-sans-paille wrote: > @MaskRay should we add a verifier step to check that > pie/pic/semanticinterposition module flags are consistent, or leave that to > clang ? I suppose you mean whether "SemanticI

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3011 + if (Args.hasArg(OPT_fsemantic_interposition) && Opts.PICLevel && !Opts.PIE) +Opts.SemanticInterposition = 1; + `Opts.SemanticInterposition` is initialized to 0 and is

[PATCH] D72829: Implement -fsemantic-interposition

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. In D72829#1845396 , @serge-sans-paille wrote: > > I suppose you mean whether "SemanticInterposition" should be invalidated > > when "PIC Level" does not exist or "PIE Level" exists. I am a bit incl

[PATCH] D73500: [driver] Add a -builtininc flag that lets Darwin driver include Clang builtin headers even with -nostdinc

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Should they use -nostdlibinc instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73500/new/ https://reviews.llvm.org/D73500 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D73500: [driver] Add a -builtininc flag that lets Darwin driver include Clang builtin headers even with -nostdinc

2020-01-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D73500#1846036 , @arphaman wrote: > In D73500#1846003 , @MaskRay wrote: > > > Should they use -nostdlibinc instead? > > > Unfortunately they can't control the flag in this specific case,

[PATCH] D69868: Allow "callbr" to return non-void values

2020-01-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @rnk Thoughts on passing live-in formation (a bit similar to WinEH catchpad) to `callbr` at the SelectionDAG stage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69868/new/ https://reviews.llvm.org/D69868 __

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

2020-01-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D7#1847167 , @mrutland wrote: > In D7#1846796 , @mrutland wrote: > > > In D7#1839207 , @MaskRay wrote: > > > > > I shall also mention

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: sammccall. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, klimek. Also rename `matchBonus` and `skipPenalty` to uniform `{match,miss}Score` in addition form. Repository: rCTE Clang Tools Extra https://rev

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Brings some goodies from https://github.com/cquery-project/cquery/blob/master/src/fuzzy_match.cc (what I plagiarized from clangd) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 ___ cfe-commits mailin

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 139313. MaskRay added a comment. Update summary Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h ==

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 139314. MaskRay edited the summary of this revision. MaskRay added a comment. Update summary Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h =

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:93 + for (int I = PatN; I <= WordN; I++) +Best = std::max(Best, Scores[PatN][I][Match].Score); if (isAwful(Best)) sammccall wrote: > this looks like a behavior change - why? This is a behavi

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.h:61 bool allowMatch(int P, int W) const; - int skipPenalty(int W, Action Last) const; - int matchBonus(int P, int W, Action Last) const; + int missScore(int W, Action Last) const; + int matchScore(int P, int W,

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:96 return None; return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best)); } I also don't understand why it clamps the value to zero here. Negative values are also meaningful to

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.h:61 bool allowMatch(int P, int W) const; - int skipPenalty(int W, Action Last) const; - int matchBonus(int P, int W, Action Last) const; + int missScore(int W, Action Last) const; + int matchScore(int P, int W,

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Have you considered also allowing '.' and ' ' (space) as separators in the > request? Having additional separators doesn't really hurt complexity of the > implementation, but allows to switch between tools for different languages > easier. I would suggest allowing pa

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/tool/ClangdMain.cpp:105 +static llvm::cl::opt LimitWorkspaceSymbolResult( +"workspace-symbol-limit", malaperle wrote: > sammccall wrote: > > the -completion-limit was mostly to control rollout, I'm not sure

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D42893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Friendly ping.. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:230 void FuzzyMatcher::buildGraph() { + Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss}; for (int W = 0; W < WordN; ++W) { sammccall wrote: > why this change? > this has also been moved fr

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 140265. MaskRay edited the summary of this revision. MaskRay added a comment. Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h ===

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 140268. MaskRay added a comment. Add comment // Find the optimal prefix of Word to match Pattern. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h =

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:93 + for (int I = PatN; I <= WordN; I++) +Best = std::max(Best, Scores[PatN][I][Match].Score); if (isAwful(Best)) sammccall wrote: > MaskRay wrote: > > sammccall wrote: > > > this looks like

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:96 return None; return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best)); } sammccall wrote: > MaskRay wrote: > > I also don't understand why it clamps the value to zero here. N

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FindSymbols.cpp:31 + + if (supportedSymbolKinds && + std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(), This std::find loop adds some overhead to each candidate... In my experience the cl

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 140297. MaskRay added a comment. missScore -> missPenalty Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 4 inline comments as done. MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:230 void FuzzyMatcher::buildGraph() { + Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss}; for (int W = 0; W < WordN; ++W) { sammccall wrote: > Mask

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:230 void FuzzyMatcher::buildGraph() { + Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss}; for (int W = 0; W < WordN; ++W) { MaskRay wrote: > sammccall wrote: > > MaskRay wrote: > > > sammcc

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:340 + A[WordN] = Miss; + for (int I = WordN, P = PatN; I > 0; I--) { +if (I == W) sammccall wrote: > MaskRay wrote: > > sammccall wrote: > > > sammccall wrote: > > > > sammccall wrote: > > > >

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 140306. MaskRay added a comment. nit picking Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd/FuzzyMatch.h =

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:93 + for (int I = PatN; I <= WordN; I++) +Best = std::max(Best, Scores[PatN][I][Match].Score); if (isAwful(Best)) sammccall wrote: > MaskRay wrote: > > sammccall wrote: > > > MaskRay wrote:

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In `buildGraph`, the nested loop for (int P = 0; P < PatN; ++P) { // Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss}; for (int W = P; W < WordN; ++W) { is interpreted as: we are calculating the cell `Scores[P+1][W+1][*]`, using the ch

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 140311. MaskRay added a comment. Don't rename anything. I just want to have this revision reviewed as soon as possible Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44720 Files: clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h Index: clangd

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FuzzyMatch.cpp:209 std::copy(NewWord.begin(), NewWord.begin() + WordN, Word); - if (PatN == 0) -return true; sammccall wrote: > similarly this one. > (ideally we wouldn't do the work above, it's just there

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FindSymbols.cpp:31 + + if (supportedSymbolKinds && + std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(), malaperle wrote: > MaskRay wrote: > > This std::find loop adds some overhead to each

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/FindSymbols.cpp:31 + + if (supportedSymbolKinds && + std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(), MaskRay wrote: > malaperle wrote: > > MaskRay wrote: > > > This std::find loop adds

<    25   26   27   28   29   30   31   32   33   34   >