[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. When you recommit, consider adding the header to the description. The original message started with "The wrong placement of add pass with optimizations led to -funique-internal-linkage-names being disabled." The header was missing. Repository: rG LLVM Github Monorepo

[PATCH] D87143: Check whether Gentoo-specific configuration directory exists

2020-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2538 const SmallVectorImpl &CandidateBiarchTriples) { + if (!D.getVFS().exists(D.SysRoot + GentooConfigDir)) +return false; Thanks. This should reduce the number of stat sysc

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 appears to be very different from the review. I guess next time your probably can upload the diff again if it is very diffiere

[PATCH] D87953: [xray] Function coverage groups

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D87953#2293024 , @kyulee wrote: > I think it looks good to me. @MaskRay Any further feedback on this? This looks good from my viewpoint. One thing remains unanswered (https://reviews.llvm.org/D87953#2284071) is how the overhea

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D81865#2293066 , @dblaikie wrote: > In D81865#2293059 , @MaskRay wrote: > >> @froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 >>

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D81865#2293158 , @froydnj wrote: > In D81865#2293146 , @MaskRay wrote: > >> In D81865#2293066 , @dblaikie wrote: >> >>> In D81865#2293059

[PATCH] D87953: [xray] Function coverage groups

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D87953#2293162 , @ianlevesque wrote: > Thanks @MaskRay - I tried to answer that question in > https://reviews.llvm.org/D87953#2286430. At present we are deploying > instrumentation to an arbitrary subset of our application usi

[PATCH] D87953: [xray] Function coverage groups

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I probably have asked too much... but it'd be nice if you also add some documentation to `llvm/docs/XRay*.rst` That helps users :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87953/new/ https://reviews.llvm.org/D87953 _

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

2020-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 294232. MaskRay marked an inline comment as done. MaskRay added a comment. Thanks for review! I will wait until next week. Fixed a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87737/new/ https://review

[PATCH] D65880: [Driver] Move LIBRARY_PATH before user inputs

2020-09-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D65880#2294871 , @protze.joachim wrote: > I still see some clang-specific and system link directories listed in the > linker line before the directories from `LIBRARY_PATH`: > > $ LIBRARY_PATH=test1 /usr/local/clang/bin/clan

[PATCH] D87717: [docs] Update ControlFlowIntegrity.rst.

2020-09-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ControlFlowIntegrity.rst:372 apply across DSO boundaries. As in the regular CFI, each DSO must be -built with ``-flto``. +built with ``-flto`` or ``flto=thin``. typo: `-flto=thin` Repository: rG LLVM Gi

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

2020-09-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Without the patch % strace -fc -e file /tmp/RelA/bin/clang '-###' a.cc ... % time seconds usecs/call callserrors syscall -- --- --- - - 74.570.002809 8 331 312 opena

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

2020-09-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1964 for (StringRef Candidate : CandidateBiarchTripleAliases) ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, /*NeedsBiarchSuffix=*/ true);

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

2020-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2469 } Suffixes[] = { - // This is the normal place. - {"gcc/" + CandidateTriple.str(), "../..", true}, + // This is the normal place if Clang is installed alongside with GCC, +

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

2020-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1964 for (StringRef Candidate : CandidateBiarchTripleAliases) ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, Candidate, /*NeedsBiarchSuffix=*/ true);

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:743 +Builder.defineMacro("__BFP__"); +// FIXME: __BOOL__ should be defined under strict -std=c89. +Builder.defineMacro("__BOOL__"); What is strict -std=c89? `!Opts.C99` ?

[PATCH] D78902: [Driver] Add output file to properties of Command

2020-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. What's the advantage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78902/new/ https://reviews.llvm.org/D78902 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D86277: [NFC][compiler-rt] Factor out __mulv[sdt]i3 implementations to .inc file

2020-08-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/builtins/int_mulv_impl.inc:1 -//===-- mulvdi3.c - Implement __mulvdi3 ---===// +//===-- int_mulv_impl.inc - Implement __mulv[sdt]i3 ---===// // `---*

[PATCH] D86289: [NFC][compiler-rt] Factor out __mulo[sdt]i3 implementations to .inc file

2020-08-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. LGTM, but let's give others a chance to comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86289/new/ https://reviews.llvm.org/D86289 ___

[PATCH] D86277: [NFC][compiler-rt] Factor out __mulv[sdt]i3 implementations to .inc file

2020-08-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. LGTM, but let's give others a chance to comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86277/new/ https://reviews.llvm.org/D86277 ___

[PATCH] D86400: [NFC][compiler-rt] Factor out __div[sdt]i3 and __mod[dt]i3 implementations

2020-08-24 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. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86400/new/ https://reviews.llvm.org/D86400 ___ c

[PATCH] D86299: [compiler-rt][builtins] Factor out some common bit manipulations

2020-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/builtins/int_mulo_impl.inc:22 +// themselves indefinitely when compiled with UBSan. +#if defined(__clang__) +#define NO_UBSAN __attribute__((no_sanitize("undefined"))) AFAIK LLVM_USE_SANITIZER does not in

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

2020-08-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Note that Harbormaster actually reported the check-clang-codegencxx issues B69369 .. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86491/new/ https://reviews.llvm.org/D86491 __

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

2020-08-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/driver/driver.cpp:537 + // When running in integrated-cc1 mode, the CrashRecoveryContext returns + // the same code as if the program crashed. On Unix, that is codes >128. + IsCrash |= CommandRes > 128; ---

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

2020-08-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/LTOVisibility.rst:39 +During the LTO link, all classes with public LTO visibility will be refined +to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker +option is applied (``-plugin-opt=whole-program-

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

2020-08-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/freebsd.c:32 +// CHECK-PPC64LE: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s"

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

2020-08-25 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. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73425/new/ https://reviews.llvm.org/D73425

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Preprocessor/init-zos.c:4 + +// S390X-ZOS-GNUXX:#define _EXT 1 +// S390X-ZOS:#define _LONG_LONG 1 --match-full-lines is different from --match-full-lines --strict-whitespace. You can freely add leading and t

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

2020-08-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:563 +return false; + if (!ClDebugFunc.empty() && ClDebugFunc == F.getName()) +return false; `ClDebugFunc != F.getName()` Rep

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

2020-08-26 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/D85474/new/ https://reviews.llvm.org/D85474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

2020-08-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 288382. MaskRay marked an inline comment as done. MaskRay added a comment. Reword a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85474/new/ https://reviews.llvm.org/D85474 Files: clang/docs/ReleaseN

[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-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/UsersManual.rst:1703 - Controls whether Clang emits a label for each basic block. Further, with - values "all" and "list=arg", each basic block or a subset of basic blocks - can be placed in its own unique section. + Co

[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-08-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:78 } else { + auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix(); CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" + Expand `auto` https://llvm.org

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

2020-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: lld/docs/ReleaseNotes.rst:29 (`D77231 `_) -* ``--lto-whole-program-visibility`` is added to support LTO whole-program devirtualization. +* ``--lto-whole-program-visibility`` is added to specify that c

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

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Looks great! 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] 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-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: rsmith. MaskRay added inline comments. Comment at: clang/test/CodeGen/basic-block-sections.c:31 // BB_LABELS: world: -// BB_LABELS: a.BB.world: -// BB_LABELS: aa.BB.world: -// BB_LABELS: a.BB.another: +// BB_LABELS: .Lfunc_begin0: +// BB_LABELS: .LBB_

[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-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. I am still reading the patch, but I have noticed one thing worth discussing. `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the memory image). An absolut

[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-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D85408#2253073 , @tmsriram wrote: > In D85408#2253055 , @MaskRay wrote: > >> I am still reading the patch, but I have noticed one thing worth discussing. >> `.bb_addr_map` is a non-SHF_A

[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-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks, I have followed the flow. The logic looks good. But I have got some nits and a question about the `clang/test/` assembly test. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1033 +/// * 2nd bit: set if it is a block ending with a tail

[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: dblaikie, nemanjai. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay requested review of this revision. Both tests operate on `%T/testbin`. If the two tests run concurrently, one may fail. This is likely the roo

[PATCH] D87103: [test] Use %t instead of %T to remove race conditions between config-file3.c and target-override.c

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6e09722b27ed: [test] Use %t instead of %T to remove race conditions between config-file3.c… (authored by MaskRay). Repository: rG LLVM Github Mono

[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-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047 + getObjFileLowering().getBBAddrMapSection(*MF.getSection()); + if (!BBAddrMapSection) +return; rahmanl wrote: > MaskRay wrote: > > BBAddrMapSection is always non

[PATCH] D87162: [Driver] Allow -specs and -nostartfiles to be forwarded to GCC

2020-09-04 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. LGTM. Comment at: clang/test/Driver/gcc_forward.c:4 // RUN: 2>&1 | FileCheck --check-prefix=FORWARD %s -// FORWARD: gcc{{[^"]*}}" "--coverage" "-fuse-ld=lld" "--ld-path=

[PATCH] D87162: [Driver] Allow -specs and -nostartfiles to be forwarded to GCC

2020-09-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Can D83648 be closed now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87162/new/ https://reviews.llvm.org/D87162 ___ cfe-commits mailing list

[PATCH] D87162: [Driver] Allow -specs and -nostartfiles to be forwarded to GCC

2020-09-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM. > It's probably not used very common, but we do have some baremetal use cases > still relying on these flags to be propagated to GCC. Thanks for the explanation! (Though I think in these cases calling `ld` or `ld.lld` directly might be better?) And apologies abo

[PATCH] D87218: [builtins] Inline __paritysi2 into __paritydi2 and inline __paritydi2 into __parityti2.

2020-09-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I think this is correct, but does their performance matter? llvm/IR/RuntimeLibcalls.def do not define them (they cannot be produced by llvm). Targets either emit popcount & 1 or detect the idiom and emit an optimized parity (x86 after PR46954) Repository: rG LLVM Gi

[PATCH] D87218: [builtins] Inline __paritysi2 into __paritydi2 and inline __paritydi2 into __parityti2.

2020-09-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. LG. Please wait a bit for @efriedma's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87218/new/ https://reviews.llvm.org/D87218

[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-08 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. I like the simplification:) However, I still feel puzzled by the large chunk of assembly test in `clang/test/CodeGen/basic-block-sections.c`. I hope another reviewer or the code owner can sa

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

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/gz.c:1 +// RUN: %clang -### -target x86_64-unknown-linux-gnu -g -gz=none %s 2>&1 | \ +// RUN:FileCheck -check-prefix=NONE %s This can be merged into `compress.c` You may delete some `-c` from compre

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

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hip-gz-options.hip:6 +// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \ +// RUN: -ggdb -gz=zlib 2>&1 | FileCheck %s + MaskRay wrote: > -ggdb is unrelated Sorry it is related. CHANGE

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

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hip-gz-options.hip:6 +// RUN: %clang -### --offload-arch=gfx906 %s -nogpulib -nogpuinc \ +// RUN: -ggdb -gz=zlib 2>&1 | FileCheck %s + -ggdb is unrelated CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D87426: Disallow fbasic-block-sections on non-ELF, non-x86 targets.

2020-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/fbasic-block-sections.c:1 -// RUN: %clang -### -fbasic-block-sections=none %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-NONE %s -// RUN: %clang -### -fbasic-block-sections=all %s -S 2>&1 | FileCheck -check-prefix=CH

[PATCH] D87426: Disallow fbasic-block-sections on non-ELF, non-x86 targets.

2020-09-09 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/fbasic-block-sections.c:12 // CHECK-OPT-LABELS: "-fbasic-block-sections=labels" +// CHECK-TRIPLE-NOT: "-fbasic-block-sections=a

[PATCH] D87426: Disallow fbasic-block-sections on non-ELF, non-x86 targets.

2020-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4892 +} else { + D.Diag(diag::warn_drv_unsupported_opt_for_target) + << A->getAsString(Args) << TripleStr; This should be an error Comment at: cl

[PATCH] D87426: Disallow fbasic-block-sections on non-ELF, non-x86 targets.

2020-09-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4892 +} else { + D.Diag(diag::warn_drv_unsupported_opt_for_target) + << A->getAsString(Args) << TripleStr; snehasish wrote: > MaskRay wrote: > > This should be an e

[PATCH] D87426: Disallow fbasic-block-sections on non-ELF, non-x86 targets.

2020-09-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. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87426/new/ https://reviews.llvm.org/D87426 ___

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

2020-09-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: hans. MaskRay added a comment. In D73425#2266446 , @Bdragon28 wrote: > Any chance of a backport to 11? CC the release manager @hans Perhaps there is still time http://lists.llvm.org/pipermail/llvm-dev/2020-September/144961.ht

[PATCH] D92809: [Driver] Add -gno-split-dwarf which can disable debug fission

2020-12-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: debug-info, dblaikie. Herald added a subscriber: dang. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently when -gsplit-dwarf is specified (could be buried in a build sys

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: shchenz. Gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92054/new/ https://reviews.llvm.org/D92054 ___ cfe-commits mailing list cfe-commits@

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D80391#2438447 , @MaskRay wrote: > In D80391#2437926 , @dblaikie wrote: > >> Looks like GCC has changed the semantics here despite the backwards >> compatibility break - as much as I'd r

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I have also notified ChromeOS/Android NDK/Bazel maintainers since they use -gsplit-dwarf (though might not be affected). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80391/new/ https://reviews.llvm.org/D80391 ___

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG843f2dbf003f: [Driver] Don't make -gsplit-dwarf imply -g2 (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D92809: [Driver] Add -gno-split-dwarf which can disable debug fission

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 310331. MaskRay added a comment. Test there is no -ggnu-pubnames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92809/new/ https://reviews.llvm.org/D92809 Files: clang/include/clang/Driver/Options.td clang/

[PATCH] D92809: [Driver] Add -gno-split-dwarf which can disable debug fission

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG85c18d3521e8: [Driver] Add -gno-split-dwarf which can disable debug fission (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

2020-12-08 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/D85474/new/ https://reviews.llvm.org/D85474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D92915: [Driver] Add -f[no-]new-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aeubanks, reames, rsmith. Herald added subscribers: dang, steven_wu, hiraditya. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The new PM is considered stable and many downstr

[PATCH] D92917: [Driver] Pass -plugin-opt=no-new-pass-manager to the linker

2020-12-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aeubanks, asbirlea, ychen. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. So that if -fno-new-pass-manager is requested for an -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=on build,

[PATCH] D92915: [Driver] Add -f[no-]new-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 310438. MaskRay added a comment. Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92915/new/ https://reviews.llvm.org/D92915 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolCha

[PATCH] D92917: [Driver] Pass -plugin-opt=no-new-pass-manager to the linker

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 310439. MaskRay edited the summary of this revision. MaskRay added a comment. Improve Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92917/new/ https://reviews.llvm.org/D92917 Files: clang/lib/Driver/ToolChai

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 310654. MaskRay retitled this revision from "[Driver] Add -f[no-]new-pass-manager to supersede -f[no-]experimental-new-pass-manager" to "[Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager". MaskRay edited the summary of

[PATCH] D92662: [Clang][Coroutine] Drop const attribute on pthread_self when coroutine is enabled

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. It is worth requesting clarification on GCC `__attribute__((const))`: (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66512) If the attribute is not suitable, glibc should drop it. The compiler can add `readnone`/`readonly` if appropriate. Repository: rG LLVM Github M

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks! @rsmith highlit the coroutine problem https://bugs.llvm.org/show_bug.cgi?id=48190 (" ... it breaks support for a C++20 language feature (coroutines)") Perhaps this means that "EXPERIMENTAL" in `ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER` should not be removed until

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 310720. MaskRay edited the summary of this revision. MaskRay added a comment. -plugin-opt=no-new-pass-manager => -plugin-opt=legacy-pass-manager Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92915/new/ https://

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf9c0d1b0562c: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new… (authored by MaskRay). Repository: rG LLVM Github Mono

[PATCH] D92917: [Driver] Pass -plugin-opt=no-new-pass-manager to the linker

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. I just merged this into D92915 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92917/new/ https://reviews.llvm.org/D92917 ___

[PATCH] D92915: [Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1354-1356 +defm legacy_pass_manager : BooleanMarshalledFFlag<"legacy-pass-manager", "CodeGenOpts.LegacyPassManager", + "static_cast(LLVM_ENABLE_NEW_PASS_MA

[PATCH] D92386: [VE] Add standard include path and library path for C++

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ve-toolchain.cpp:13 + +// RUN: %clangxx -### -target ve %s 2>&1 | FileCheck -check-prefix=DYNLINKER %s +// DYNLINKER: nld{{.*}} "-dynamic-linker" "/opt/nec/ve/lib/ld-linux-ve.so.1" Many linker options t

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This change has been failing on my machine for the past two weeks. `riscv32-toolchain-extra.c` and `riscv64-toolchain-extra.c` are not hermit. I have installed `/usr/lib/gcc-cross/riscv64-linux-gnu/10` (gcc-10-riscv64-linux-gnu and its dependencies) on my machine and th

[PATCH] D92662: [Clang][Coroutine] Drop const attribute on pthread_self when coroutine is enabled

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D92662#2444196 , @lxfind wrote: > In D92662#2443970 , @MaskRay wrote: > >> If the attribute is not suitable, glibc should drop it. The compiler can add >> `readnone`/`readonly` if approp

[PATCH] D92996: [VE] Remove -faddrsig and -fnoaddrsig tests

2020-12-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ve-toolchain.cpp:72 // RUN: %clangxx -### -target ve %s 2>&1 | FileCheck -check-prefix=DEFINITARRAY %s // DEFINITARRAY: clang{{.*}} "-cc1" Most newer toolchains default to -fuse-init-array. There i

[PATCH] D93023: Replace deprecated %T in 2 tests.

2020-12-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. If you do this, `testroot-riscv64-baremetal-nogcc/` is probably also not useful (or you can replace it with a shorter string`. Can you fix the tests when gcc-riscv64-linux-gnu is installed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D92996: [VE] Remove -faddrsig and -fnoaddrsig tests

2020-12-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. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92996/new/ https://reviews.llvm.org/D92996 __

[PATCH] D93023: Replace deprecated %T in 2 tests.

2020-12-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. My build directory is `/tmp/RelA`. After `sudo apt install gcc-10-riscv64-linux-gnu`, % /tmp/RelA/bin/llvm-lit -vv riscv32-toolchain-extra.c ... Exit Code: 1 Command Output (stderr): -- /home/ray/llvm/clang/test/Driver/riscv32-toolchain-extra.c:28:34: erro

[PATCH] D93084: [VE] Optimize toolchain regression test

2020-12-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LG. I don't have a system VE toolchain and the tests still pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93084/new/ https://reviews.llvm.org/D93084 ___ cfe-commits mailing

[PATCH] D93084: [VE] Optimize toolchain regression test

2020-12-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ve-toolchain.cpp:115 +// DEF-SAME: "-z" "max-page-size=0x400" +// DEF-SAME: "crtbegin.o" +// DEF-SAME: "-lc++" "-lc++abi" "-lunwind" "-lpthread" "-ldl" You probably should add an empty file in `basi

[PATCH] D93104: [clang][cli] Revert accidental access-control flag rename

2020-12-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Thanks! Sorry for the breakage. Actually, neither `-fno-access-control` nor `-faccess-control` has any test.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93104/new/ https://reviews.llvm.or

[PATCH] D93084: [VE] Optimize toolchain regression test

2020-12-12 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. Thanks. The tests still work fine on my machine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93084/new/ https://reviews.llvm.org/D93084 ___

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

2020-12-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Gentle ping.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92054/new/ https://reviews.llvm.org/D92054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

2020-12-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4033 +dwarfArgs->render(Args, CmdArgs); + } + Delete unneeded braces http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loo

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

2020-12-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/M68k.h:38 +} // end namespace m68k +} // namespace tools +} // end namespace driver mismatching `end` and no `end`? clang-format seems to drop `end`. I th

[PATCH] D78938: Make LLVM build in C++20 mode

2020-12-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @BRevzin Please share your name and email if you want someone to commit it for you https://llvm.org/docs/DeveloperPolicy.html#commit-messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.or

[PATCH] D83979: [clang][cli] Port LangOpts option flags to new option parsing system

2020-12-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1001 + "LangOpts->AccessControl", DefaultsToTrue, + ChangedBy, + ResetBy>; `ChangedBy` are a bit verbose. How about a shorthand for this most common form? Repository: rG LLVM

[PATCH] D92445: [PowerPC] Add powerpcle target.

2020-12-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:358 resetDataLayout("E-m:a-p:32:32-i64:64-n32"); +else if ((Triple.getArch() == llvm::Triple::ppcle)) + resetDataLayout("e-m:e-p:32:32-i64:64-n32"); redundant `()`

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

2020-12-18 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/D92054/new/ https://reviews.llvm.org/D92054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D93452: [clangd] Trim memory periodically

2020-12-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Support/Unix/Process.inc:122 +#if defined(HAVE_MALLOC_TRIM) + return malloc_trim(Pad); +#else If the user uses jemalloc/tcmalloc with glibc, malloc_trim may exist while the function may do nothing. Repositor

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

2020-12-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Should this have some tests? Even if guarded by `REQUIRES:` if some feature is needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90159/new/ https://reviews.llvm.org/D90159 _

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

2020-12-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 313208. MaskRay edited the summary of this revision. MaskRay added a comment. Improve BE test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92054/new/ https://reviews.llvm.org/D92054 Files: clang/docs/Releas

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

2020-12-21 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6bbb04a732cd: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables (authored by MaskRay). Repository: rG LLVM Github Mon

[PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64

2020-12-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/debug-options.c:379 +// VALIDT: x86_64 +// RUN: %clang -### -c -gdwarf-5 -gdwarf64 -target VALIDT %s 2>&1 | FileCheck -check-prefix=GDWARF64_ON %s +// RUN: %clang -### -c -gdwarf-4 -gdwarf64 -target VALIDT %s 2>&1 | Fi

[PATCH] D89617: Prepend "uniq" to symbol names hash with -funique-internal-linkage-names

2020-12-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `.L` prefixed symbols will be discarded by the linker in the default mode (none of `--discard-none/--discard-locals/--discard-all` is specified). So if you use `.L` (called PrivateGlobalPrefix in MC), this will work without any linker change (if possible I would hope we

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