[PATCH] D155756: -fsanitize={function,kcfi}: Switch to xxh3_64bits

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155756#4516501 , @samitolvanen wrote: > Changing the hash function breaks compatibility with the Rust KCFI > implementation >

[PATCH] D155756: -fsanitize={function,kcfi}: Switch to xxh3_64bits

2023-07-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added a reviewer: samitolvanen. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Following recent changes switching from xxh64 to xxh32 for better hashing

[PATCH] D155670: [c-index-test] Suppress -Wcast-qual after D153911

2023-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. commit 0f0c3d45d7d75ba82a955246da654146a7d57a0d did this first Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155670: [c-index-test] Suppress -Wcast-qual after D153911

2023-07-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: AlexM, dblaikie. Herald added a subscriber: arphaman. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. free_remapped_files needs to discard the

[PATCH] D104931: [AArch64] Wire up ILP32 ABI support in Clang

2023-07-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a subscriber: wangpc. We need some tests. https://maskray.me/blog/2021-08-08-toolchain-testing#i-dont-know-whether-a-test-is-needed Adding some to `clang/test/Preprocessor/init-aarch64.c` should cover many changes, but we also need one to cover

[PATCH] D146054: [RISCV] Add --print-supported-extensions support

2023-07-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry for the delay. Comment at: clang/include/clang/Frontend/FrontendOptions.h:286 + /// print the supported extensions for the current target + unsigned PrintSupportedExtensions : 1; Comment at:

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

2023-07-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Reverse ping @philnik :) The `release/17.x` branch will be created soon (https://discourse.llvm.org/t/llvm-17-0-0-release-planning-and-update/71762). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151683/new/

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

2023-07-17 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/D154774/new/ https://reviews.llvm.org/D154774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D155490: [clang][docs] Fix tag target name in link

2023-07-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I find that `See patch` is used elsewhere. Does this change fix a sphinx bot? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155490/new/ https://reviews.llvm.org/D155490 ___

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Thank you for the update. I think this Clang patch is good enough to land even if the lld part is still in review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146777/new/

[PATCH] D155339: Enable zba and zbs for RISCV64 Android

2023-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/riscv-features.c:13 +// ANDROID: "-target-feature" "+zba" // ANDROID: "-target-feature" "+zbb" If the features are adjacent, test them on one line. `// ANDROID: "-target-feature" "+zba"

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154923#4491673 , @MaskRay wrote: > In D154923#4491411 , @efriedma > wrote: > >> If I follow correctly, this is basically undoing the splitting that was done >> by the command that

[PATCH] D155336: [clang][JumpDiagnostics] bring VerifyIndirectOrAsmJumps to C++17

2023-07-14 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/D155336/new/ https://reviews.llvm.org/D155336

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4500241 , @cwalther wrote: > Thank you! > > So I guess next time I submit a patch here (nothing planned at the moment), I > should put the proposed commit message into the summary field and the > backstory and start

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/CodeGen/embed-fat-lto-objects.c:3 +// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -emit-llvm < %s | FileCheck %s --check-prefixes=FULL,SPLIT + +// RUN: %clang -cc1 -triple

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thanks for the update. Can you add a comment for the `-funified-lto` combination? It's unclear what it does... `clang -flto=thin -ffat-lto-objects -funified-lto -fuse-ld=lld foo.c` I've left some comments about missing test coverage. Comment at:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Changing the default improves GCC compatibility (`Enabled by default under -std=c++14 and above.`) and I think is the right direction. @wangpc You can use your new username to commandeer this revision? To other reviewers, what's still blocked now that the Apple target

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226 + } else { +Args.claimAllArgs(options::OPT_fhip_uniform_block, + options::OPT_fno_hip_uniform_block); yaxunl wrote: > MaskRay wrote: > > Why is the

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 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 rGd986462b5053: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain (authored by cwalther, committed by MaskRay). Repository: rG LLVM

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226 + } else { +Args.claimAllArgs(options::OPT_fhip_uniform_block, + options::OPT_fno_hip_uniform_block); Why is the -Wunused-command-line-argument

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4497973 , @cwalther wrote: > Okay. No, I don’t have write access (as far as I know) and would appreciate > if you (or any other maintainer) could land the patch, with any commit > message you deem appropriate. Happy

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4496247 , @cwalther wrote: > That summary wasn’t intended to be a commit message at all, but a description > of the problem and start of the discussion. My commit message was part of the > uploaded patch (since I

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D154357#4492625 , @cwalther wrote: > I am unable to provoke any problem with my tests by putting various things in > `/usr/lib/gcc/`, and I also

[PATCH] D155123: [Driver] Warn about -mios-version-min instead of erroring out when targeting MachO embedded architectures

2023-07-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/macho-embedded.c:16 +// CHECK-MACHO-EMBEDDED-DIAG: clang: warning: argument unused during compilation: '-mios-version-min=5' // CHECK-MACHO-EMBEDDED: "-triple" "{{thumbv[67]e?m}}-apple-unknown-macho"

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:268 +- ``-ffat-lto-objects`` can now be used to emit object files with both object + code and LLVM bitcode. Previously this flag was ignored for GCC compatibility. + You can insert the

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. One nit about the test. Comment at: llvm/test/CodeGen/LoongArch/xray-attribute-instrumentation.ll:24 +; CHECK-NEXT: .Lxray_sleds_start0: +; CHECK-NEXT: .Ltmp0: +;

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D146777#4491842 , @paulkirth wrote: > Rebase and try to accomodate Unified LTO changes. > > Based on the Unified LTO patches, I think this is the correct handling for > FatLTO, but I'd like to get a second opinion before

[PATCH] D155123: [Driver] Warn about -mios-version-min instead of erroring out when targeting MachO embedded architectures

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Driver/macho-embedded.c:9 +// RUN: %clang -arch armv7m -target thumbv7-apple-ios -mios-version-min=5 -### -c %s 2>&1 | FileCheck %s

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

2023-07-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152604#4494975 , @rnk wrote: > It sounds like two users have hit this now: Chromium and Rust folks. This is > a flag flip, so it's pretty small and safe to rollback, and IMO we should > consider that while we debug the

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:623 +if (Args.hasArg(options::OPT_ffat_lto_objects)) + CmdArgs.push_back("-fat-lto-objects"); } New lld long options only allow `--` form. So use two dashes.

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154923#4491411 , @efriedma wrote: > If I follow correctly, this is basically undoing the splitting that was done > by the command that produced the bitcode file? Yes, undoing

[PATCH] D154602: [clang][clangd] Don't crash/assert on -gsplit-dwarf=single without output

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/unittests/Driver/ToolChainTest.cpp:371 +TEST(CompilerInvocation, SplitSwarfSingleCrash) { + static constexpr const char *Args[] = {"clang", "-gdwarf-4", "-gsplit-dwarf=single", "-c", "foo.cpp"}; +

[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as` on AIX

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Some nits about testing, otherwise LG In D152924#4490950 , @qiongsiwu1 wrote: > In D152924#4490581 , @MaskRay wrote: > >> We have

[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as` on AIX

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. We have `TargetOptions::DisableIntegratedAS` (your llc.cpp change). Do you know why it is not feasible for the places you want to check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152924/new/

[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as` on AIX

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/lto-aix.c:76 // CSPGO: "-bplugin_opt:-cs-profile-generate" "-bplugin_opt:-cs-profile-path=default_%m.profraw" +// +// Test integrated assembler options This `^//$` line is not useful. Some tests use

[PATCH] D154602: [clang][clangd] Don't crash/assert on -gsplit-dwarf=single without output

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/unittests/Driver/ToolChainTest.cpp:371 +TEST(CompilerInvocation, SplitSwarfSingleCrash) { + static constexpr const char *Args[] = {"clang", "-gdwarf-4", "-gsplit-dwarf=single", "-c", "foo.cpp"}; + CreateInvocationOptions

[PATCH] D154602: [clang][clangd] Don't crash/assert on -gsplit-dwarf=single without output

2023-07-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. https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer I think there is a minor "The test checks too little" issue as we can utilize the test to check a few

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The code change looks good but the driver test will cause an issue. I think we need a fake sysroot tree under `Inputs/`. Otherwise if we have a system cross gcc at `/usr/lib/gcc{,-cross}/powerpc64le-unknown-eabi/12`, Clang Driver will pick up these files. This is really

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_loongarch64.cpp:30 +// are 2RI12-type and 2RI16-type. +inline static uint32_t +encodeInstruction2RIx(uint32_t Opcode, uint32_t Rd, uint32_t Rj, Early xray code unfortunately does not respect

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-07-11 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: compiler-rt/lib/xray/xray_trampoline_loongarch64.S:20 + .type __xray_FunctionEntry,@function +__xray_FunctionEntry: + .cfi_startproc

[PATCH] D154786: [Clang][Driver] Pass through the --be8 endian flag to linker in BareMetal driver For Arm.

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:40 +bool arm::isArmBigEndian(const llvm::Triple , const ArgList ) { + bool IsBigEndian = false; + We do not need this variable. After an early return, we can just `return

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/baremetal.cpp:348 +// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PPCEABI %s Without a sysroot, we may pick up powerpc-unknown-eabi-gcc (if

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4487896 , @cwalther wrote: > [...] There is a PowerPC EABI: > https://www.nxp.com/docs/en/application-note/PPCEABI.pdf . I wouldn’t know if > Clang/LLD obey it, but it works for us… Good to know! I read a newer ABI

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4487896 , @cwalther wrote: > Thanks for the comments. Yes, `powerpc-*-eabi` should be valid (sorry if I > was unclear about that – my parenthetical was only about AArch64). There is a > PowerPC EABI:

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

2023-07-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: ormris, efriedma, rjmccall, aaron.ballman. Herald added subscribers: steven_wu, hiraditya. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

2023-07-10 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. > [clang] Allow disassembly of multi-module bitcode files The subject confused me as I did not recognize what disassembly means :) > Clang currently exits with an error message

[PATCH] D152391: [Clang] Allow bitcode linking when the input is LLVM-IR

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM. CC1 -mlink-bitcode-file is an interesting option from 2011: f1d76db466b2a50781c0754b86ac994dd07b5041 I wonder what the original use case is... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152391/new/

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Frontend/split-lto-ir-support.cpp:1 +// RUN: %clang -c -flto=thin %s -o %t0.bc +// RUN: mkdir %t1.d Without -fsanitize=cfi or -fwhole-program-vtables, -fsplit-lto-unit is not the default. You need to specify

[PATCH] D154902: [Clang] Default ToolChain::IsIntegratedAssemblerDefault to true

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `[Driver]` seems more specific than `[Clang]` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154902/new/ https://reviews.llvm.org/D154902 ___ cfe-commits mailing list

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D151683#4486321 , @aaron.ballman wrote: > Aside from the failing precommit CI test case in C, I think this LGTM. I've > added @MaskRay as the code owner for the command line option changes in case > he's got concerns

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154357#4485881 , @jroelofs wrote: > gcc docs seem to indicate that these are valid triples: > > https://gcc.gnu.org/install/specific.html#powerpc-x-eabi Thanks, good to know. `powerpc-*-eabi` indicates that the eabi

[PATCH] D154797: [CUDA][HIP] Rename and fix `-fcuda-approx-transcendentals`

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. Some nits about testing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154797/new/ https://reviews.llvm.org/D154797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154797: [CUDA][HIP] Rename and fix `-fcuda-approx-transcendentals`

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7228 + } else { +Args.ClaimAllArgs(options::OPT_fgpu_approx_transcendentals); +Args.ClaimAllArgs(options::OPT_fno_gpu_approx_transcendentals); You can use

[PATCH] D154797: [CUDA][HIP] Rename and fix `-fcuda-approx-transcendentals`

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/hip-options.hip:179 +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib -fgpu-approx-transcendentals \ +// RUN: --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefixes=APPROX %s +

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

2023-07-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > At Indel, we have been building bare-metal embedded applications that run on > custom PowerPC and ARM systems with Clang and LLD for a couple of years now, > using target triples powerpc-indel-eabi, powerpc-indel-eabi750, > arm-indel-eabi, aarch64-indel-eabi (which I

[PATCH] D154736: [Driver][ARM] Warn about -mabi= for assembler input with -fno-integrated-as

2023-07-10 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 rG26718d931fee: [Driver][ARM] Warn about -mabi= for assembler input with -fno-integrated-as (authored by MaskRay). Repository: rG LLVM Github

[PATCH] D154602: [clang][clangd] Don't crash/assert on -gsplit-dwarf=single without output

2023-07-09 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! But I share the concern that the test is added to a wrong layer (https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer).

[PATCH] D154786: [Clang][Driver] Pass through the --be8 endian flag to linker in BareMetal driver For Arm.

2023-07-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:48 + case llvm::Triple::thumb: +if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian, + options::OPT_mbig_endian)) We can check the

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-07-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D153835#4481816 , @rjmccall wrote: > I agree that the change to test51 is the right result, yes. The explicit > visibility attribute on the template declaration should take precedence over > the visibility of the types in

[PATCH] D154774: [Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

2023-07-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, aaron.ballman, efriedma, rjmccall, smeenai. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In CodeGenCXX/visibility.cpp, Match GCC for

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85 + auto linkerIs = [Exec](const char *name) { +return llvm::sys::path::filename(Exec).equals_insensitive(name) || + llvm::sys::path::stem(Exec).equals_insensitive(name);

[PATCH] D150013: [Clang] Respect `-L` options when compiling directly for AMDGPU

2023-07-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry, but I just realized that updating CommonArgs.cpp is wrong... I have fixed it with commit 681cb54a54bb60dea9034b4b0b45ccc392875b9a . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-07-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: erichkeane. MaskRay added a comment. In D153835#4481462 , @rjmccall wrote: > We made a decision ten years or so ago to use a slightly different > interpretation of the visibility attributes, so differences from GCC are not >

[PATCH] D154736: [Driver][ARM] Warn about -mabi= for assembler input with -fno-integrated-as

2023-07-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: michaelplatings, peter.smith, simon_tatham, nathanchance, nickdesaulniers. Herald added a subscriber: kristof.beyls. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a

[PATCH] D154543: [llvm] Move StringExtras.h include from Error.h to Error.cpp

2023-07-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > [llvm] Move StringExtras.h include from Error.h to Error.cpp If you land the #include changes separately, I think the tag can be changed to `[Support] `. `Support` is sufficient to refer to llvm-project/llvm/ CHANGES SINCE LAST ACTION

[PATCH] D154543: [llvm] Move StringExtras.h include from Error.h to Error.cpp

2023-07-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. I have spot checked a few files and the additional #include looks great! As mentioned in the other patch, it would be better landing the #include adjustment part separately so that the

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-07-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D153835#4478700 , @efriedma wrote: > Can you write the complete rule we're trying to follow here? Like, if you > have a free function template, prioritize attributes in the following order > ..., if you have a member

[PATCH] D153229: [llvm] Move StringExtras.h include from Error.h to Error.cpp

2023-07-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I've used something like `for i in llvm clang clang-tools flang lld lldb bolt mlir; do LIT_OPTS='--filter=xxx --allow-empty-runs' ninja -C /tmp/Rel check-$i` to ensure I have built everything, beside a few projects I run `check-$i`. It's always good to make the

[PATCH] D153229: [llvm] Move StringExtras.h include from Error.h to Error.cpp

2023-07-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Glad that you landed the first change separately:) The Error.cpp change was reverted by b3c8554f28a95063e406924c25336e0da7efb4fd which was more focused. Repository: rG LLVM Github Monorepo

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

2023-07-06 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa9f5119a78aa: [DebugInfo] Enable debug info emission for extern variables in C++ (authored by chiyuze, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_loongarch64.cpp:23 +enum PatchOpcodes : uint32_t { + PO_ADDID = 0x02c0, // addi.d rd, rj, imm + PO_SD = 0x29c0, // st.d rd, base, offset I think the `PO_` style actually harms

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_loongarch64.cpp:153 + const XRaySledEntry ) XRAY_NEVER_INSTRUMENT { + // FIXME: In the future we'd need to distinguish between non-tail exits and + // tail exits for better

[PATCH] D153600: Implement -frecord-command-line for XCOFF

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/MC/MCAsmStreamer.cpp:972 +void MCAsmStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) { + const char *InfoDirective = "\t.info "; + const char *Separator = ", "; Repository: rG LLVM Github

[PATCH] D154531: [AMDGPU] Support -mcpu=native for OpenCL

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Seems reasonable. I assume that there isn't a way to add a portable test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154531/new/ https://reviews.llvm.org/D154531 ___ cfe-commits mailing list

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-07-05 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/D153835/new/ https://reviews.llvm.org/D153835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D145227: [LLVM][OHOS] Clang toolchain and targets

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Herald added a subscriber: wangpc. Comment at: clang/lib/Driver/ToolChains/OHOS.h:38 + bool isPICDefaultForced() const override { return false; } + bool useRelaxRelocations() const override { return false; } + UnwindLibType

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85 + auto linkerIs = [Exec](const char *name) { +return llvm::sys::path::filename(Exec).equals_insensitive(name) || + llvm::sys::path::stem(Exec).equals_insensitive(name);

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Upstreaming support requires rigid testing, otherwise it's fairly easy for other contributors to break your code while refactoring. https://maskray.me/blog/2021-08-08-toolchain-testing "I don't know whether a test is needed" `clang/test/Driver` has many tests that

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I don't think this belongs to the upstream, either. Note: musl's TLS implementation is very clean and may be a good reference. https://maskray.me/blog/2021-02-14-all-about-thread-local-storage > glibc ld.so reserves some space in static TLS blocks and allows dlopen on

[PATCH] D154388: Don't pass -ibuiltininc, which is used only by the driver, to CC1

2023-07-05 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:1309 +} else if (A->getOption().matches(options::OPT_ibuiltininc)) { + // This is used only by the driver.

[PATCH] D154043: [CodeGen] -fsanitize={function, kcfi}: ensure align 4 if +strict-align

2023-06-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. Obsoleted by D154125 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154043/new/ https://reviews.llvm.org/D154043

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

2023-06-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154014#4461076 , @rupprecht wrote: > In D154014#4457883 , @MaskRay wrote: > >>> This is a breaking change since some SCLs might use .* or (abc|def) which >>> are supported regexes

[PATCH] D140727: [XRay] Add initial support for loongarch64

2023-06-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/test/CodeGen/LoongArch/xray-attribute-instrumentation.ll:1 +; RUN: llc --mtriple=loongarch64 %s -o - | FileCheck %s +; RUN: llc --mtriple=loongarch64 -filetype=obj %s -o %t I think you want to read my recent

[PATCH] D154070: [Driver][lld/COFF] Support DWARF fission when using LTO on Windows

2023-06-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a subscriber: mstorsjo. MaskRay added a comment. This revision is now accepted and ready to land. I'll be away for a few days and back on July 5. Clicked "Accept" if you keep just the lld/COFF part for this patch and create another patch for

[PATCH] D154070: [Driver][lld/COFF] Support DWARF fission when using LTO on Windows

2023-06-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Normally clang and lld patches are separate. You can `git log -- lld/COFF` to find who usually reviews lld/COFF code and add these folks as reviewers. Not many people set subscription rules but some may appreciate if you let know that there is such a change. Though, I

[PATCH] D154043: [CodeGen] -fsanitize={function, kcfi}: ensure align 4 if +strict-align

2023-06-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D154043#4459446 , @simon_tatham wrote: > The details of this approach look good to me, but is this the best place to > solve it? Doing it in clang means that //every// language front end that > wants to use either of these

[PATCH] D154043: [CodeGen] -fsanitize={function, kcfi}: ensure align 4 if +strict-align

2023-06-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: efriedma, rjmccall, simon_tatham, samitolvanen. Herald added a subscriber: kristof.beyls. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-06-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D146777#4457839 , @paulkirth wrote: > In D146777#4457209 , @MaskRay wrote: > >> As mentioned, you may consider landing llvm patch then wait a bit so that >> (a) people can experiment

[PATCH] D154014: [SpecialCaseList] Use Globs instead of Regex

2023-06-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This is a breaking change since some SCLs might use .* or (abc|def) which are > supported regexes but not valid globs. Since we have just cut clang 16.x this > is a good time to make this change. My user has some ignore lists, but there is no `^[a-z]+:.*\(` or

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

2023-06-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > This patch is required so that we can still use kconfig in such BPF programs > compiled from C++. Assuming that you mean https://www.kernel.org/doc/html/next/kbuild/kconfig-language.html#kconfig-language , how is Kconfig relevant here? Clang BPF supports

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

2023-06-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I wonder whether you have ready-to-use instructions to test this for folks who are not familiar with Linux kernel/eBPF. (I happened to start to read eBPF one week ago, but I guess it would take some time for me to be more familiar with it, even if I contributed some

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-06-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. As mentioned, you may consider landing llvm patch then wait a bit so that (a) people can experiment with the clang patch better (b) prevent the llvm/clang patches to be both reverted, if some issue has been identified with the llvm patch. (Don't worry that a feature

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

2023-06-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you for implementing this warning. Side note: it would be better to (a) fix the tests separately from (b) implementing the warning and (c) adding tests to demonstrate the warning. This way the patch is more focused. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 534842. MaskRay edited the summary of this revision. MaskRay added a comment. remove a misleading comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153835/new/ https://reviews.llvm.org/D153835 Files:

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: clang, aaron.ballman, efriedma, rjmccall, smeenai. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix

[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:20 + +void copyRecord(AggregateStorageLocation , AggregateStorageLocation , +Environment ) { See

[PATCH] D153691: [Driver][ARM] Warn about -mabi= for assembler input

2023-06-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 rGfe5bab537270: [Driver][ARM] Warn about -mabi= for assembler input (authored by MaskRay). Changed prior to commit:

[PATCH] D153691: [Driver][ARM] Warn about -mabi= for assembler input

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 534668. MaskRay marked an inline comment as done. MaskRay added a comment. Thanks for the comments. Improved comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153691/new/ https://reviews.llvm.org/D153691

[PATCH] D153582: [SystemZ][z/OS] Add required options/macro/etc for z/os compilation step

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/zos-comp.c:3 + +// RUN: %clang -c -### %s 2>&1 \ +// RUN:--target=s390x-ibm-zos \ wrap too quickly. The whole thing can be placed on one line. Comment at:

[PATCH] D153582: [SystemZ][z/OS] Add required options/macro/etc for z/os compilation step

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:3707 HelpText<"Enable stack probes">; +def mzos_sys_include_EQ : Joined<["-"], "mzos-sys-include=">, Flags<[NoXarchOption]>, MetaVarName<"">, +HelpText<"Path to system headers on z/OS">;

<    1   2   3   4   5   6   7   8   9   10   >