[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

2021-02-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: ABataev. MaskRay added a comment. Perhaps we should just drop `llvm/IR/Verifier.cpp:1075` (which is not tested): if (N.getTag() == dwarf::DW_TAG_class_type || N.getTag() == dwarf::DW_TAG_union_type) { AssertDI(N.getFile() && !N.getFile()->getFilename().empt

[PATCH] D109727: [Driver] Remove unneeded *-suse-* triples

2021-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D109727#3003498 , @luismarques wrote: > Regarding D74399 , cmake on a fedora RISC-V > host still detects a generic triple: > > -- LLVM host triple: riscv64-unknown-linux-gnu > -- LLVM defa

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/target-data.c:256 +// RUN: %clang_cc1 -triple s390x-none-zos -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=ZOS If you add so many RUN lines at once, please use unittests instead. This wo

[PATCH] D109635: [OpenMP] Support construct trait set for Clang

2021-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:741 +llvm::omp::TraitProperty Top = ConstructTraits.pop_back_val(); +assert(Top == Trait && "Something left a trait on the stack!"); + } I fixed -Wunused-variable in c5f

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/target-data.c:256 +// RUN: %clang_cc1 -triple s390x-none-zos -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=ZOS anirudhp wrote: > MaskRay wrote: > > If you add so many RUN lines at once,

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/target-data.c:256 +// RUN: %clang_cc1 -triple s390x-none-zos -o - -emit-llvm %s | \ +// RUN: FileCheck %s -check-prefix=ZOS MaskRay wrote: > anirudhp wrote: > > MaskRay wrote: > > > If you add so man

[PATCH] D110142: [clang][Driver] Correct runtime path for Arm hard float targets

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Do you know why `--target=armv8l-unknown-linux-gnueabihf` picks `armhf-unknown-linux-gnueabihf` instead of `armv8l-unknown-linux-gnueabihf`? (For new tests, prefer `--target=` to `-target `; space separated driver options are not the convention.) The direction the Clang

[PATCH] D110142: [clang][Driver] Correct runtime path for Arm hard float targets

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/arm-float-abi-runtime-path.c:7 +// RUN:-resource-dir=%S/Inputs/arm_float_abi_runtime_path 2>&1 | FileCheck -check-prefix=ARMHF %s +// RUN: %clang %s -target armv7-unknown-linux-gnueabihf -print-runtime-dir \ +/

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM. I looked at this in 2019 but did not change because GNU ld 2.25 (2014) introduced --push-state. (gold added it in 2016 but I think we can have higher version requirement for gold.) In 2021, binutils 2.24 (2013) compatibility should be irrelevant now...

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/linux-ld.c:499 +// RUN: %clangxx -no-canonical-prefixes -x c++ %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-unknown-linux-gnu \ `-no-canonical-prefixes` is only useful when the CHECK lines inspe

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Thanks for cleaning up RUN lines :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109362/new/ https://reviews.llvm.org/D109362 ___ cfe-commits mai

[PATCH] D110209: [CSSPGO] Set PseudoProbeInserter as a default pass.

2021-09-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. lld/test/ELF/lto/pseudo-probe-lto.ll needs update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110209/new/ https://reviews.llvm.org/D110209 ___ cfe-commits mailing list cfe-comm

[PATCH] D110142: [clang][Driver] Correct runtime path for Arm hard float targets

2021-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110142#3015482 , @DavidSpickett wrote: > So would you expect to see libraries in > `lib/clang/14.0.0/lib/armv8l-unknown-linux-gnueabihf` and not change clang's > logic? I'd hope that it works this way (`armv8l-*`) :) > Th

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-22 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/D110128/new/ https://reviews.llvm.org/D110128 __

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Please mention GNU ld 2.25 (2014) in the description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110128/new/ https://reviews.llvm.org/D110128 ___ cfe-commits mailing list cfe-

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:581 + CmdArgs.push_back("--push-state"); + CmdArgs.push_back("--as-needed"); if (OnlyLibstdcxxStatic) MaskRay wrote: > The --as-needed change should be dropped from th

[PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

2021-09-23 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/D109975/new/ https://reviews.llvm.org/D109975 _

[PATCH] D110128: [Driver] Correctly handle static C++ standard library

2021-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:144 +} else { + CmdArgs.push_back("--as-needed"); +} as-needed needs push-state as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: pirama, srhines. Herald added subscribers: danielkiss, kristof.beyls. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. These values allow, for example, `--target=aarch64` and `-

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. You may need to check whether the Android clang has correct `clang -dumpmachine`. If not, specify `LLVM_HOST_TRIPLE` explicitly in CMake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110379/new/ https://reviews.llvm.org/D

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 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: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:869 + // symbol, so we can not ensure the correctness of the relative CounterPtr, so + /

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:978 if (NS == 0 && !(DataReferencedByCode && NeedComdat && !Renamed) && - (TT.isOSBinFormatELF() || + (TT.isOSBinFormatELF() || TT.isOSBinFormatXCOFF() || (!Data

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > we can NOT guarantee that the relocations get resolved to the intended weak > symbol, so we can not ensure the correctness of the relative CounterPtr, so > we have to use private linkage for counter and data symbols. In other binary formats the first weak definition i

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110422#3021283 , @jsji wrote: >> In other binary formats the first weak definition is selected and other weak >> definitions are discarded. >> Do you mean that AIX ld doesn't pick the first weak definition? > > No. I think th

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The description is still unclear. Say a.o has a weak `foo, __profc_foo, __profd_foo`, b.o has a weak `foo, __profc_foo, __profd_foo`. The linker picks the definitions from `a.o`. In the PGO implementation, it doesn't whether the non-discarded b.o `__profd_foo` has garba

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110422#3021572 , @jsji wrote: > In D110422#3021535 , @MaskRay wrote: > >> The description is still unclear. >> >> Say a.o has a weak `foo, __profc_foo, __profd_foo`, b.o has a weak `fo

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The description isn't clear why the previous `weak` symbol usage does not work. Have you verified that it does not work? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110422/new/ https://reviews.llvm.org/D110422 _

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry, I still don't understand this response: > We have 3 sets weak symbols here: weak_func, profc_weak_foo, profd_weak_func. > We can't ensure that binder always choose 3 of them from same object. So this part of the description isn't clear to me: > However, on AIX,

[PATCH] D109727: [Driver] Remove unneeded *-suse-* triples

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @marxin Can you check this from Suse perspective? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109727/new/ https://reviews.llvm.org/D109727 ___ cfe-commits mailing list cfe-c

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Supplement: Even if you omit the vendor part in `-DLLVM_DEFAULT_TARGET_TRIPLE` (if unspecified, use `-DLLVM_HOST_TRIPLE`), `x86_64-linux-gnu`, you will see `Target: x86_64-unknown-linux-gnu` in `clang -v` output (and similarly in `-dumpmachine`/`--print-target-triple`).

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375383. MaskRay edited the summary of this revision. MaskRay added a reverted change: D53463: [Driver] allow Android triples to alias for non Android targets. MaskRay added a comment. revert D53463 Repository: rG LLVM Git

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 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 rG75f0194d3d25: [Driver] Remove confusing *-linux-android detection with non-android --target= (authored by MaskRay). Repository: rG LLVM Github Mon

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > However, on AIX, the current binder can NOT discard the weak symbols if we > put all of them into the same csect, as binder can NOT discard only part of a > csect. "as binder can NOT discard only part of a csect." this part caused confusion to me... > CountersOffset

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110422#3025911 , @jsji wrote: >> "as binder can NOT discard only part of a csect." this part caused confusion >> to me... > > OK, yeah, csect is special to XCOFF, so this is not the same concept of > sections in ELF. "as bi

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:865 + // Due to the current limitation of binder, the duplicate weak symbols in the + // same csect won't be discarded. When there are duplicate weak symbols, "D

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I still think the description can be rephrased to be shorter and clearer, and am not very sure this workaround should be added. But I will take a vacation and be back after one week and don't want to appear that I am blocking this change. So LGTM once you decrease the n

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: phosek, sylvestre.ledru. Herald added a subscriber: pengfei. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is needed when config.guess is removed (D109837

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375722. MaskRay added a comment. add missing file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110663/new/ https://reviews.llvm.org/D110663 Files: clang/lib/Driver/ToolChain.cpp clang/test/Driver/Inputs/

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110663#3028811 , @sylvestre.ledru wrote: > just to make sure I understand, it will also update the install or "just" the > dumpmachine option ? If `LLVM_DEFAULT_TARGET_TRIPLE` is `x86_64-linux-gnu, `clang -dumpmachine` will

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375751. MaskRay marked an inline comment as done. MaskRay retitled this revision from "[Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path" to "[Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtim

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375753. MaskRay added a comment. fix getRuntimePath test libclang_rt.builtins.a Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110663/new/ https://reviews.llvm.org/D110663 Files: clang/lib/Driver/ToolChain.cp

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110663#3029088 , @phosek wrote: > The reason I removed this behavior in D101194 > , aside from extra overhead introduced by > the extra checks, is that we've seen cases where people would h

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I'd actually want to get rid of the function `Linux::getMultiarchTriple`, when all Debian Clang use `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=*-linux-gnu` instead of `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=*-{unknown,pc}-linux-gnu` In D110663#3029131

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D110663#3029146 , @phosek wrote: > In D110663#3029124 , @MaskRay wrote: > >> In D110663#3029088 , @phosek wrote: >> >>> The reason I removed th

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > You mean when you build runtimes with the host compiler and then use them > with Clang (i.e. by using -DLLVM_ENABLE_PROJECTS="clang;lld;libcx")? This has > been discussed several times on llvm-dev in the past, ideally nobody should > be using this mode because there's

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. On top on this Diff, with the following patch, `ninja check-clang-driver` passes. We may consider dropping the normalized triple code. --- i/clang/lib/Driver/ToolChain.cpp +++ w/clang/lib/Driver/ToolChain.cpp @@ -494,6 +494,7 @@ std::string ToolChain::getRuntimePat

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-09-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375778. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: luismarques, s.egerton, PkmX, atanasyan, simoncook, arichardson, sdardis. Support Debian -m32 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

2021-10-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a reviewer: ctetreau. In D110663#3029577 , @phosek wrote: > This is going to break Fuchsia as implemented. We assume that > `{x86_64,aarch64}-unknown-fuchsia` and `{x86_64,aarch64}-fuchsia` behave > identically and

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

2021-02-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 rG87dbdd2e3bb6: [FileCheck] Default --allow-unused-prefixes to false (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D90457: [clang][driver] Set LTO mode based on input files

2021-02-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D90457#2551698 , @tbaeder wrote: > Hey @MaskRay, have you thought about this some more? Or is it too much magic > and you'd rather reject it altogether? Yes, it should be rejected. I've seen cases where people want `clang -c a

[PATCH] D96360: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

2021-02-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: beanz, phosek, smeenai. Herald added a subscriber: mgorny. Herald added a reviewer: alexshap. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Repository: rG

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

2021-02-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Making -stdlib=libc++ not affect linker option for `-static` and `-static-pie` looks good to me, for the GNU toolchain. They are expected to pass `-lc++ -lc++abi` if they use separate `libc++abi.a`. Just specifying `-lc++abi` does not work with GNU ld and gold anyway. It

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

2021-02-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D96203#2548529 , @echristo wrote: > In D96203#2548495 , @aaron.ballman > wrote: > >> In D96203#2548471 , @mibintc wrote: >> >>> In D96203#2546856

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

2021-02-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping @abidh Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93023/new/ https://reviews.llvm.org/D93023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

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

2021-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. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93023/new/ https://reviews.llvm.org/D93023 ___ cfe-commits mailing list cfe-commits@

[PATCH] D96360: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

2021-02-15 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 rG02413b097e72: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D95753: Store compilation dir separately in coverage mapping

2021-02-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. (I haven't read through the discussion.) -fprofile-prefix-map has been added. Does this option has overlap with that option? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95753/new/ https://reviews.llvm.org/D95753 __

[PATCH] D95753: Store compilation dir separately in coverage mapping

2021-02-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1131 NegFlag, BothFlags<[CoreOption]>>; +def fcoverage_compilation_dir : Separate<["-"], "fcoverage-compilation-dir">, +Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, Us

[PATCH] D96838: CodeGen: Set !retain metadata on UsedAttr definitions for Linux/FreeBSD

2021-02-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. Herald added subscribers: krytarowski, arichardson, emaste. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC 11 will set SHF_GNU_RETAIN on the section of a `__attribute__((used))` function/variable,

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

2021-02-17 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/debug-options-as.c:65 +// Check that -gdwarf-N can be placed before other options of the "-g" group. +// RUN: %clang -### -c -g -gdwarf-3

[PATCH] D96886: [Driver] Clean up some Separate form options

2021-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: phosek, thakis. Herald added subscribers: jansvoboda11, dang. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Drop the `Separate` form of `-fmodule-name X`, `-fprofile-remappin

[PATCH] D96886: [Driver] Clean up some Separate form options

2021-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 324390. MaskRay added a comment. Move HelpText to EQ. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96886/new/ https://reviews.llvm.org/D96886 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/

[PATCH] D95753: [Coverage] Store compilation dir separately in coverage mapping

2021-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1131 NegFlag, BothFlags<[CoreOption]>>; +def fcoverage_compilation_dir : Separate<["-"], "fcoverage-compilation-dir">, +Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, ph

[PATCH] D96886: [Driver] Clean up some Separate form options

2021-02-17 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 rG0c2bb6b446c5: [Driver] Clean up some Separate form options (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96865: [Driver] Honor "-gdwarf-N" at any position for assembler sources

2021-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/debug-options-as.c:65 +// Check that -gdwarf-N can be placed before other options of the "-g" group. +// RUN: %clang -### -c -g -gdwarf-3 -integrated-as -x assembler %s 2>&1 \ +// RUN: | FileCheck -check-prefix=DWARF3

[PATCH] D96838: CodeGen: Set !retain metadata on UsedAttr definitions for FreeBSD/Fuchsia/Linux

2021-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 324504. MaskRay retitled this revision from "CodeGen: Set !retain metadata on UsedAttr definitions for Linux/FreeBSD" to "CodeGen: Set !retain metadata on UsedAttr definitions for FreeBSD/Fuchsia/Linux". MaskRay added a comment. Add Fuchsia Repository: r

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 325233. MaskRay retitled this revision from "CodeGen: Set !retain metadata on UsedAttr definitions for FreeBSD/Fuchsia/Linux" to "Add GNU attribute 'retain'". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: aaron.

[PATCH] D97161: Improve diagnostic for ignored GNU 'used' attribute

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: aaron.ballman. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D97161 Files: clang/include/clang/Basic/Diagn

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; aaron.ballman wrote: > Should this be a target-specific attribute as it only has effects on ELF > targets? As

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 325336. MaskRay marked 5 inline comments as done. MaskRay edited the summary of this revision. MaskRay added a comment. Comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96838/new/ https://reviews.llvm.org

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D96838#2578073 , @rjmccall wrote: > GCC 11 hasn't been released yet, so can we still engage with GCC about the > semantics of this attribute? This is a very low-level attribute, and I don't > understand why it was made separa

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D96838#2578097 , @rjmccall wrote: > In D96838#2578083 , @MaskRay wrote: > >> In D96838#2578073 , @rjmccall wrote: >> >>> GCC 11 hasn't been releas

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D96838#2578127 , @rjmccall wrote: > In D96838#2578101 , @MaskRay wrote: > >> In D96838#2578097 , @rjmccall wrote: >> >>> In D96838#2578083

[PATCH] D97161: Improve diagnostic for ignored GNU 'used' attribute

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbccdf6b232f6: Improve diagnostic for ignored GNU 'used' attribute (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97161/new/ https://re

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 325486. MaskRay marked 2 inline comments as done. MaskRay added a comment. incorporate rjmccall's doc suggestion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96838/new/ https://reviews.llvm.org/D96838 Files:

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; aaron.ballman wrote: > MaskRay wrote: > > aaron.ballman wrote: > > > Should this be a target-specific attribute

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 325499. MaskRay added a comment. Add tests that Sema discarded unused functions are warned (nor different from the case without 'retain') Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96838/new/ https://revi

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; aaron.ballman wrote: > MaskRay wrote: > > aaron.ballman wrote: > > > MaskRay wrote: > > > > aaron.ballman wrote

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: compnerd. MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; rjmccall wrote: > MaskRay wrote: > > aaron.ballman wrote: > > > MaskRay w

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2655 + +def Retain : InheritableAttr { + let Spellings = [GCC<"retain">]; MaskRay wrote: > aaron.ballman wrote: > > rjmccall wrote: > > > MaskR

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

2021-02-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay commandeered this revision. MaskRay edited reviewers, added: Romain-Geissler-1A; removed: MaskRay. MaskRay added a comment. This revision is now accepted and ready to land. Herald added subscribers: jansvoboda11, dang. Implemented by 2820a2ca3a0e69c3f301845420e00672251b

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:98 +This support is available in GNU ``ld`` and ``gold`` as of binutils 2.36, as +well as in ``ld.lld`` 13. + }]; probinson wrote: > As a u

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D96838#2582965 , @probinson wrote: >> For ELF, `used` does not retain the entity, regardless of this patch. > > That statement does not match my experience. You probably used C identifier name sections with `__attribute__((used

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. We have two implementation choices. 1. Lift the source language restriction (it is currently discouraged) on llvm.compiler.used, let llvm.used use `SHF_GNU_RETAIN` on ELF, and change `__attribute__((used))` to use llvm.compiler.used on ELF. 2. Don't touch the backend se

[PATCH] D97433: [Driver] Create -ffile-compilation-dir alias

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2021-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, rjmccall, rnk, rsmith. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. An global value in the `llvm.used` list does not have GC root semantics on ELF targets. T

[PATCH] D97447: Add GNU attribute 'retain'

2021-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, rjmccall, rnk, rsmith. Herald added a subscriber: pengfei. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For ELF targets, GCC 11 will set SHF_GNU_RETAIN on the

[PATCH] D96838: Add GNU attribute 'retain'

2021-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. Thanks folks for review. Folks are more happy with approach 1 on https://lists.llvm.org/pipermail/llvm-dev/2021-February/148760.html , so I am abandoning this. I have copied the documentation and tests to D97447

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2021-02-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D97446#2587806 , @probinson wrote: >> GNU ld has a rule "start_/stop_ references from a live input section retain >> the associated C identifier name sections", > > which LLD may change in the future (D96914

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 349915. MaskRay added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Re-upload after revert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103372/new/ https://reviews.llvm.org/

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @vsk Do you mind investigating macOS `Posix/instrprof-dynamic-one-shared.test` and `Posix/instrprof-dynamic-two-shared.test` failures on https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8845381350758503248/+/steps/package_clang/0/stdout ? :

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 349953. MaskRay edited the summary of this revision. MaskRay added a comment. Exclude Mach-O. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103372/new/ https://reviews.llvm.org/D103372 Files: llvm/lib/Transf

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 349956. MaskRay added a comment. better description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103372/new/ https://reviews.llvm.org/D103372 Files: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 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 rG9e51d1f348b9: [InstrProfiling] If no value profiling, make data variable private and (for… (authored by MaskRay). Repository: rG LLVM Github Monor

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:935 + // data variables can be private. This optimization applies on COFF and ELF. + if (!DataReferencedByCode && !TT.isOSBinFormatMachO()) { +Linkage = GlobalValue::PrivateLin

[PATCH] D103372: [InstrProfiling] If no value profiling, make data variable private and (for Windows) use one comdat

2021-06-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I measured a stage 2 `-DLLVM_TARGETS_TO_BUILD=X86 -DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD_INSTRUMENTED_COVERAGE=on` build on Linux x86-64. % ls -1 /tmp/out/s3-custom/**/*.o(.) | wc -l 2174 % stat -c %s /tmp/out/s3-custom/**/*.o(.) | awk '{s+=$1}END{print s}' 6

[PATCH] D103777: [X32] Add Triple::isX32(), use it.

2021-06-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. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103777/new/ https://reviews.llvm.org/D103777

[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for tagging us. glibc has supported DT_INIT_ARRAY/DT_FINI_ARRAY since 1999. GCC 4.7 defaulted to .init_array/.fini_array on glibc >= 2.4 and most Linux distributions have thus made the switch for many years. Actually, most ELF operating systems have switched to

[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D103495#2804254 , @efriedma wrote: > Are there any existing optimizations that might be affected by this? In > particular, I think GlobalOpt implicitly reorders functions in global_ctors. Thanks for mentioning llvm::optimize

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

2021-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D101873#2804669 , @lanza wrote: > Hey Fangrui, is there any reason this couldn't extend to armv7? @lanza Always happy when more folks are interested in such kind of stuff:) This needs backend work. See D101872

<    12   13   14   15   16   17   18   19   20   21   >