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

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

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

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

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

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

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

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

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

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

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

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.

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

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

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

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

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

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

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=

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=

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

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

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

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

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

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

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

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

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

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

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=

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=

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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++

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

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.

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.

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

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

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

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

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

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

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

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

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

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

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

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'

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

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'

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'

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'

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'

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'

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

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'

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'

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'

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'

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'

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'

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

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'

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'

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'

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

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

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'

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'

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

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

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

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

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

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

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

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

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.

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

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

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

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   >