[PATCH] D122524: [Driver][AVR] Emit proper warnings for different options

2022-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:451 if (!Args.hasArg(options::OPT_nostdlib) && !Args.hasArg(options::OPT_nodefaultlibs)) { +if (!CPU.empty

[PATCH] D122524: [Driver][AVR] Emit proper warnings for different options

2022-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:477 if (!LinkStdlib) D.Diag(diag::warn_drv_avr_stdlib_not_linked); benshi001 wrote: > We should not merge the above two `if`, due to this check. > > So I will keep my or

[PATCH] D122679: [OpenMP] Use offload bundler when outputting textual formats with new driver

2022-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/openmp-offload-gpu.c:350 + +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvida-cuda -march=sm_70 \ +// RUN: --libomptarget-nvptx-bc-path=%S/Inputs/libompt

[PATCH] D122699: [HLSL] Add Semantic syntax, and SV_GroupIndex

2022-03-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6386 + let Content = [{ +The `SV_GroupIndex` semantic when applied to an input parameter specifies a data +binding to map the group index to the specified parameter. This attribute is -

[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

2022-03-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. To add more why I think the current semantics may need more discussion before we quickly commit to such a driver option: - interaction with --dyld-prefix: --sysroot does not affect the fallback --dyld-prefix. It seems even less appropriate for --overlay-platform-toolcha

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

2022-03-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: StephenFan. @nemanjai mentioned "All the issues I was seeing seem to have been resolved. I can build with CLANG_DEFAULT_PIE_ON_LINUX=on without any failing sanitizer or crt failures." Will recommit. And ppc64 bots have hopefully got better with

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

2022-03-31 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/modules.cpp:79 +// +// RUN: %clang -fcxx-modules -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-CXX-MODULES +// CHECK-CXX-MODULES: "-fcxx-modules" You need to specify a pre-20 -stdc to make the test

[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

2022-04-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Is this a problem with D105462 ? Should -msse4.2 imply -mcrc32? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122789/new/ https://reviews.llvm.org/D122789

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/memtag-attr.cpp:18 +// CHECK-NO-NOT: sanitize_memtag +// CHECK-NO-NEXT: define {{.*}}HasSanitizeMemTag +// CHECK-MEMTAG: Function Attrs: {{.*}} sanitize_memtag By placing a space before `HasSanitizeMem

[PATCH] D122789: [compiler-rt] [scudo] Use -mcrc32 on x86 when available

2022-04-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D122789#3424213 , @pengfei wrote: > In D122789#3423846 , @MaskRay wrote: > >> Is this a problem with D105462 ? Should >> -msse4.2 imply -mcrc32? > > -

[PATCH] D123097: [RISCV] Remove redundant enabling of IAS for Clang

2022-04-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. Herald added a subscriber: StephenFan. It's conventional to add NFC to the subject line to indicate "no functional change". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

2022-04-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D118948#3427401 , @hctim wrote: > In D118948#3427344 , @tschuett > wrote: > >> Is `-fsanitize=memtag-heap` Android specific or target independent? It >> passes Android flags to the li

[PATCH] D130566: [Driver] Default to DWARF 4 on Linux/sparc64

2022-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Consider creating a minimum reproduce and filing a bug on https://sourceware.org/bugzilla/buglist.cgi Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130566/new/ https://reviews.llvm.org/D1305

[PATCH] D130145: [AArch64] Simplify BTI/PAC-RET module flags

2022-07-26 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 rGde1b5c91453f: [AArch64] Simplify BTI/PAC-RET module flags (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D130145?vs=4473

[PATCH] D130569: [Driver] Use libatomic for 32-bit SPARC atomics support on Linux

2022-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:634 + // LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so + // forcibly link with libatomic as a workaround. `// TODO ...` and attach a bug link ===

[PATCH] D130569: [Driver] Use libatomic for 32-bit SPARC atomics support on Linux

2022-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This needs a test. For now you can use `linux-ld.c` which has some sparc tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130569/new/ https://reviews.llvm.org/D130569 ___ cfe

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

2022-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/LTO/PowerPC/data-sections-aix.ll:18-19 + +; CHECK-NOT: (csect: .data) +; CHECK: g O .data {{.*}} var + hubert.reinterpretcast wrote: > Unfortunately (without the change) t

[PATCH] D130566: [Driver] Default to DWARF 4 on Linux/sparc64

2022-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I accepted this because this seems a major showstopper and we want to resolve it for the upcoming major release But I just recall that clang 14.0.0 has defaulted to DWARF v5 for most ELF operating systems on all architectures. > So far, I had only seen it with the GNU l

[PATCH] D130526: [Driver][PowerPC] Support -mtune=

2022-07-28 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 rG1dc26b80b872: [Driver][PowerPC] Support -mtune= (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

2022-07-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Making the behavior different for different Linux distributions is not great, but if it matches the practice, I think it is fine. I don't know Sparc enough to suggest anything for 32-bit, though... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D130516: [llvm] compression classes

2022-07-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!). Let's discuss several use cases. (a) if a use case just calls compress/uncompress

[PATCH] D128757: [Driver][test] Add -fuse-ld= option tests for NetBSD

2022-07-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Windows is very different from all other OSes. Having a fuse-ld-windows.c may be fine. Again, NetBSD shares many characteristics with other OSes and having a test for all ~10 ELF OSes Clang supports seems too much. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

2022-08-01 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/D130531/new/ https://reviews.llvm.org/D130531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D120175: [Driver] Re-run lld with --reproduce when it crashes

2022-08-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM (Note: I think 2.5h from approval to push was too short for others to react on this patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120175/new/ https://reviews.llvm.org/D120175

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. When a module with `"indirect_branch_cs_prefix"` and another without the module flag are merged, what the result should be? If 0, we should use `Min` instead of `Override`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-02 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. Herald added a subscriber: StephenFan. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6350 + if (Args.hasArg(options::OPT_mindirect_branch_cs_prefix)) +

[PATCH] D130516: [llvm] compression classes

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D130516#3688236 , @dblaikie wrote: > In D130516#3688123 , @MaskRay wrote: > >> I'd like to make a few arguments for the current namespace+free function >> design, as opposed to the cla

[PATCH] D130516: [llvm] compression classes

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D130516#3694366 , @dblaikie wrote: > In D130516#3694151 , @MaskRay wrote: > >> In D130516#3688236 , @dblaikie >> wrote: >> >>> In D130516#3688

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6350 + if (Args.hasArg(options::OPT_mindirect_branch_cs_prefix)) +CmdArgs.push_back("-mindirect-branch-cs-prefix"); This is not needed with the TableGen CC1Option change. Re

[PATCH] D130794: [Docs] Add HLSL ResourceType documentation

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. I'll assume that you have done `ninja docs-clang-html`:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130794/new/ https://reviews.llvm.org/D130794 __

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

2022-08-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:5502 MarshallingInfoFlag>; +def fexperimental_sanitize_metadata_covered +: Flag<["-"], "fexperimental-sanitize-metadata-covered">, Let CC1 options use the same spelling as

[PATCH] D131057: [Sema] -Wformat: support C23 printf %b %B

2022-08-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aaron.ballman, dim, enh, erik.pilkington. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Close #56885. %b %

[PATCH] D131057: [Sema] -Wformat: support C23 printf %b %B

2022-08-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131057#3697165 , @aaron.ballman wrote: > Thanks for working on this! Please also add a release note for it. Will add:) > I think there are changes missing for ScanfFormatString.cpp to handle `bArg` > and `BArg` that should

[PATCH] D131057: [Sema] -Wformat: support C23 format specifier %b %B

2022-08-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 449740. MaskRay retitled this revision from "[Sema] -Wformat: support C23 printf %b %B" to "[Sema] -Wformat: support C23 format specifier %b %B". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: krytarowski, arich

[PATCH] D131057: [Sema] -Wformat: support C23 format specifier %b %B

2022-08-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131057#3697265 , @enh wrote: >> I think making scanf in the same patch makes sense. Let me check existing >> tests... > > fwiw, most of the libcs seem to have been doing these separately because > scanf is harder. i hope to

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

2022-08-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6350 + if (Args.hasArg(options::OPT_mindirect_branch_cs_prefix)) +CmdArgs.push_back("-mindirect-branch-cs-prefi

[PATCH] D131057: [Sema] -Wformat: support C23 format specifier %b %B

2022-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 450046. MaskRay marked an inline comment as done. MaskRay added a comment. Thanks for quick reviews! Updated ReleaseNotes.rst Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131057/new/ https://reviews.llvm.org/D

[PATCH] D131057: [Sema] -Wformat: support C23 format specifier %b %B

2022-08-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 rG88501dc74911: [Sema] -Wformat: support C23 format specifier %b %B (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D131217: [clang][WebAssembly] Pass `-Wl,-no-type-check` through to the MC layer

2022-08-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Test? And we should not have `llvm::dbgs() << "Main " << Opts.NoTypeCheck << "\n";` by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131217/new/ https://reviews.llvm.org/D131217 _

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-08-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D130689#3704581 , @dyung wrote: > Your change is causing a build failure on the PS4 linux build bot using GCC > 9.3. Can you take a look? > https://lab.llvm.org/buildbot/#/builders/139/builds/26186 > > FAILED: > tools/clang

[PATCH] D131346: [clang] LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2022-08-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: aaron.ballman. Herald added subscribers: steakhal, kosarev, pmatos, asb, StephenFan, martong, kerbowa, arphaman, kbarton, jgravelle-google, sbc100, jvesely, nemanjai, dschuff. Herald added a reviewer: dang. Herald added a reviewer: NoQ. Her

[PATCH] D131346: [clang] LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131346#3706216 , @nemanjai wrote: > Why? There are many years of precedent for using `LLVM_FALLTHROUGH` and it is > very clear and obvious. What do we gain by getting rid of it? > Don't get me wrong, I am not super opposed to

[PATCH] D131346: [clang] LLVM_FALLTHROUGH => [[fallthrough]]. NFC

2022-08-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 rG3f18f7c0072b: [clang] LLVM_FALLTHROUGH => [[fallthrough]]. NFC (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D131422: [lldb] Remove include/lldb/lldb-private.h

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: JDevlieghere, kastiglione. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. The header from

[PATCH] D131422: [lldb] Remove include/lldb/lldb-private.h

2022-08-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 rGcdeb50c32155: [lldb] Remove include/lldb/lldb-private.h (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @tmsriram Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130531/new/ https://reviews.llvm.org/D130531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

[PATCH] D131455: [Driver] Add -Xclang= as an alias for -Xclang

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: jhuber6, rnk. Herald added subscribers: jeroen.dobbelaere, StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a

[PATCH] D131455: [Driver] Add -Xclang= as an alias for -Xclang

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D131455#3708621 , @jhuber6 wrote: > LG > > `-Xopenmp-target=` is somewhat different. The first joined argument is used > to select a toolchain and the second, separate argument is the argument to > pass. So something like `-X

[PATCH] D131455: [Driver] Add -Xclang= as an alias for -Xclang

2022-08-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 rG8d9d27db4759: [Driver] Add -Xclang= as an alias for -Xclang (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D131455?vs=45

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: clang-language-wg. Herald added subscribers: bzcheeseman, StephenFan, mstorsjo. Herald added a project: All. MaskRay requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Heral

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang-language-wg, probinson, rjmccall. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang's default C++

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

2022-08-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 451044. MaskRay added a comment. simplify Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131465/new/ https://reviews.llvm.org/D131465 Files: clang/docs/ReleaseNotes.rst clang/lib/Basic/LangStandards.cpp c

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D130689#3710291 , @aaron.ballman wrote: > In D130689#3710281 , @royjacobson > wrote: > >> In D130689#3709834 , @thieta wrote: >> >>> In D1306

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

2022-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Posted "C++/ObjC++: switch to gnu++17 as the default dialect" https://discourse.llvm.org/t/c-objc-switch-to-gnu-17-as-the-default-dialect/64360 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131465/new/ https://reviews.llvm

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/AST/ast-dump-openmp-begin-declare-variant_11.c:2 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=c_mode -ast-dump %s | FileCheck %s --check-prefix=C -// RUN: %clang_cc1 -triple x86_64-unknown-unkno

[PATCH] D131225: Driver: Refactor and support per target dirs in baremetal

2022-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Support/Triple.cpp:1938 +return false; + + if (getVendor() != UnknownVendor) The prevailing style doesn't add a blank line for every `if` statement. Commen

[PATCH] D130516: [llvm] compression classes

2022-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I have only taken very brief look at the new version. Having an enum class `CompressionKind` with a parallel `CompressionAlgorithm` seems redundant. `friend CompressionAlgorithm *CompressionKind::operator->() const;` looks magical. I hope that someone insisting on objec

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/AST/ast-dump-openmp-begin-declare-variant_11.c:2 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=c_mode -ast-dump %s | FileCheck %s --check-prefix=C -// RUN: %clang_cc1 -triple x86_64-unknown-unkno

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 451377. MaskRay added a comment. fix some tests in a better way Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131464/new/ https://reviews.llvm.org/D131464 Files: clang/lib/Basic/LangStandards.cpp clang/tes

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 451586. MaskRay marked 5 inline comments as done. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: llvm-commits, delcypher. Herald added a project: LLVM. Add lit substitutions Repository: rG LLVM Github Monore

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. It will take some time to fix all tests properly. Let's have a guideline how to fix them properly. I tried fixing some using several patterns in the last revision. I didn't fix all, because it would likely lead to an unsatisfactory result and I would spend more time :)

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

2021-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Since @glaubitz is here: I want to set `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` to on by default so that the libraries for x86-64 will be in `lib/clang/13.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.profile.a` instead of `lib/clang/13.0.0/lib/linux/libclang_rt.profile-x86

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: lbenes, sthibaul. 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/D106119 Files: clang/lib/Driver/ToolChains/

[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

2021-07-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/clang_f_opts.c:603 +// RUN: %clang -### -fno-call-graph-section %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CALL-GRAPH-SECTION %s +// RUN: %clang -### %s 2>&1 | FileCheck -check-prefix=CHECK-NO-CALL-GRAPH-SECTION %s +/

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106119#2884412 , @lbenes wrote: > Thanks for looking into this. On OpenSUSE i686, the failing tests are now > fixed, but Driver/linux-cross.cpp fails with: > > https://pastebin.com/uHfi3KD1 "This page is no longer available

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 359496. MaskRay added a comment. Add two .keep files to keep two directories Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106119/new/ https://reviews.llvm.org/D106119 Files: clang/lib/Driver/ToolChains/Gnu.

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106119#2884915 , @lbenes wrote: > This one should work. > https://controlc.com/505d58d3 > > Please let me know if there's anything else that could help. Thanks. I forgot to add two `.keep` files to retain two otherwise empty

[PATCH] D105911: [CallGraphSection] Introduce CGSectionFuncComdatCreator pass

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1420 + +if (CodeGenOpts.CallGraphSection) { + MPM.addPass(CGSectionFuncComdatCreatorPass()); https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-stateme

[PATCH] D105907: [CallGraphSection] Add call graph section options and documentation

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. High-level comments: I'd use a bottom-up patch order. llvm patches are before clang patches. The clang driver patch is the last - having the user-facing option requires the functionality to be fully available. llvm-objdump patch is orthogonal to clang patches, so its ord

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 359654. MaskRay added a comment. fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106119/new/ https://reviews.llvm.org/D106119 Files: clang/lib/Driver/ToolChains/Gnu.cpp clang/test/Driver/Inputs/arch

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. If you can accept this, I'll submit it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106119/new/ https://reviews.llvm.org/D106119 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D106119: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian Linux

2021-07-20 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 rG5b899c22f3d2: [Driver] Detect libstdc++ include paths for native gcc on 32-bit non-Debian… (authored by MaskRay). Repository: rG LLVM Github Monor

[PATCH] D105834: [PowerPC] Semachecking for XL compat builtin icbt

2021-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-pwr8.c:10 +// RUN: -target-cpu pwr8 -o - | FileCheck %s -check-prefix=CHECK-PWR8 +// RUN: not %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s \ +// RUN: -target-cpu pwr7 2>&1 | File

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

2021-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This breaks `clang/test/Index/preamble-reparse-changed-module.m` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93769/new/ https://reviews.llvm.org/D93769 ___ cfe-commits mailing

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

2021-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Preprocessor/predefined-flteval-macro.c:332 +// RUN: %clang_cc1 -std=c11 -ffreestanding -triple=riscv64 -fsyntax-only %s +// RUN: %clang_cc1 -std=c11 -ffreestanding -triple=riscv64-unknown-linux -fsyntax-only %s +#ifndef FE

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

2021-07-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D93769#2891549 , @mibintc wrote: > In D93769#2891545 , @MaskRay wrote: > >> This breaks `clang/test/Index/preamble-reparse-changed-module.m` >> >> http://45.33.8.238/linux/51638/step_7.tx

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} tstellar wrote: > MaskRay wrote: > > What does this do? > > > > A hard-coded list cannot

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} MaskRay wrote: > tstellar wrote: > > MaskRay wrote: > > > What does this do? > > > > > >

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @davidxl @vsk 😃 I am happy to postpone this after 13.0.0 is branched - so that we can adjust the format while only bumping the version once. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104556/new/ https://reviews.llvm.o

[PATCH] D106535: [clangd] Use CommandMangler in TestTU

2021-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `[clangd] Get rid of arg adjusters in CommandMangler` can crash `BuildCompilerInvocation.DropsPlugins`. I am testing `ninja check-clangd` and will push the revert shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1065

[PATCH] D106535: [clangd] Use CommandMangler in TestTU

2021-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106535#2900728 , @kadircet wrote: > Can you provide a reproducer or build bot failure for the crash Fangrui ? I already provided an internal bug :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: compnerd, craig.topper, rsmith. Herald added subscribers: ormris, StephenFan, frasercrmck, dexonsmith, dang, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jone

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106701#2901639 , @efriedma wrote: > Can we hook this up to a LLVM IR function attribute, instead of making it a > codegen flag? The current TargetLoweringBase::PrefLoopAlignment is global. I have considered a function attri

[PATCH] D106585: Fix clang debug info irgen of i128 enums

2021-07-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 +llvm::APSInt Value = Enum->getInitVal(); +Value.setIsSigned(IsSigned); +Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); ---

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 361467. MaskRay marked 2 inline comments as done. MaskRay added a comment. comments. add a test/CodeGen test. add HelpText. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106701/new/ https://reviews.llvm.org/D10

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | F

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106701#2902544 , @dblaikie wrote: > In D106701#2901656 , @MaskRay wrote: > >> In D106701#2901639 , @efriedma >> wrote: >> >>> Can we hook thi

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc

[PATCH] D106737: DRAFT - [clang] [hexagon] Add resource include dir

2021-07-24 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. Thanks for the patch. This makes the behavior closer to Linux.cpp . Request changes to clear my queue before a test is added... Comment at: clang/lib/Driver/Tool

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/LibClang/lit.local.cfg:1 + +config.substitutions.append(('%libclang', os.path.join(config.clang_lib_dir, 'libclang.so'))) delete blank line Comment at: clang/test/LibClang/symbols.test:2 +#

[PATCH] D106733: [clang/darwin] Pass libclang_rt.profile last on linker command

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106733/new/ https://reviews.llvm.org/D106733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | F

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

2021-07-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I remember that we even converted some `*.md` to `*.rst` to remove the number of supported documentation formats, so I agree that adding support the even less used `*.docx` may not be the proper direction. Thanks for splitting this off. Repository: rG LLVM Github Mo

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

2021-07-26 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/D105527/new/ https://reviews.llvm.org/D105527 _

[PATCH] D106701: [clang] Add -falign-loops=N where N is a power of 2

2021-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106701#2908679 , @craig.topper wrote: > Does this work with LTO? `-falign-loops=` doesn't affect ld.lld code generation options. `-Wl,-mllvm,--align-loops=128` can be used for now. Repository: rG LLVM Github Monorepo C

[PATCH] D106925: COFF/ELF: Place llvm.global_ctors elements in llvm.used if comdat is used

2021-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: rnk, rsmith. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. On ELF, an SHT_INIT_ARRAY outside a section group is a GC root. The current codegen abuses SHT_INIT_ARRAY in a sect

[PATCH] D60912: MS ABI: handle inline static data member and inline variable as template static data member

2021-07-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: lib/CodeGen/CGDeclCXX.cpp:494 // too. AddGlobalCtor(Fn, 65535, COMDATKey); } else { Confirmed with `lld-link -subsystem:console -opt:ref -debug:symtab a.o b.o c.o -entry:main -out:win.exe` that `__declspec(

[PATCH] D106899: [LLVM][NFC] Remove LLVM_ATTRIBUTE_NORETURN and use [[noreturn]] directly

2021-07-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I migrated all llvm/ files. You need to remove them from the patch. It is a good idea to drop the macro definition in `llvm/include/llvm/Support/Compiler.h` separately. It makes it easy to bring back the definition if some downstream users want extended time for migrat

[PATCH] D106925: COFF/ELF: Place llvm.global_ctors elements in llvm.used if comdat is used

2021-07-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D106925#2910638 , @rnk wrote: > lgtm > > So the main impact here is that, on ELF, linker GC will no longer be able to > GC vague linkage global variables with dynamic initializers. Those are things > like > > - C++17 inline g

<    4   5   6   7   8   9   10   11   12   13   >