[PATCH] D128235: [RISCV] Add support for the Zawrs extension

MaskRay added inline comments. Comment at: llvm/test/MC/RISCV/zawrs-invalid.s:4 +# WRS doesn't take immediates +wrs 1 # CHECK: :[[@LINE]]:5: error: invalid operand for instruction + `[[@LINE]]` is a deprecated FileCheck feature. Use `[[#@LINE]]` Note: you can p

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. Magically deciding a default value for --unwindlib or --rtlib is not nice. You may emit a warning if the selected default happens to be incompatible with HIP. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127142/new/ https://reviews.llvm.org/D127142 __

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. In D127142#3601810 , @yaxunl wrote: > In D127142#3600809 , @MaskRay wrote: > >> Magically deciding a default value for --unwindlib or --rtlib is not nice. >> You may emit a warning if the

[PATCH] D128333: [clang][flang] Disable defaulting to `-fpie` for LLVM Flang

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. gfortran defaults to PIE as well. > We can revisit this once support for -fpie and -fpic is available in LLVM > Flang. I'm not aware of anyone actively working in this area. Disag

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

MaskRay added a comment. > This saves about ~0.275% of the optimised clang binary. Worth clarifying a bit which -O level and whether -g is used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127911/new/ https://reviews.llvm.org/D127911 __

[PATCH] D128022: [HIP] add -fhip-kernel-arg-name

MaskRay accepted this revision. MaskRay added inline comments. Herald added a subscriber: StephenFan. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6353 CmdArgs.push_back("-fgpu-allow-device-init"); +if (Args.hasFlag(options::OPT_fhip_kernel_arg_name, +

[PATCH] D128461: [X86] Simplify __cpuid_count

MaskRay created this revision. Herald added subscribers: jsji, StephenFan, pengfei. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use the x86-32 implementation for x86-64, i.e. replace =r plus xchgq wi

[PATCH] D128461: [X86] Simplify __cpuid_count

MaskRay added a comment. Ah, OK. I am investigating a miscompile that `: "=a"(__eax), "=r" (__ebx), "=c"(__ecx), "=d"(__edx) \` may assign `"=r"` to use RDX. The output then looks like xchgq %rbx, %rdx cpuid xchgq %rbx, %rdx and rbx is clobbered, since cpuid writes rax/rbx/rcx/rdx. Rep

[PATCH] D128482: [clang codegen] Add dso_local/hidden/etc. markings to VTT declarations

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/D128482/new/ https://reviews.llvm.org/D128482 _

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

MaskRay added a comment. You may add `-ftime-trace=` instead of introducing a new spelling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 ___ cfe-commits mail

[PATCH] D128512: [Driver][test] Add libclang_rt.profile{{.*}}.a tests for OpenBSD.

MaskRay added inline comments. Comment at: clang/test/Driver/coverage-ld.c:43 +// RUN: | FileCheck --check-prefix=CHECK-OPENBSD-X86-64 %s +// +// CHECK-OPENBSD-X86-64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}" Don't add `//\n` by cargo cult. It's not a recommende

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

MaskRay added a comment. Please include `Differential Revision: ` line for reland commits as well so that people know that this patch has a reland. https://github.com/llvm/llvm-project/issues/56204 is related to 655ba9c8a1d22075443711cc749f0b032e07adee

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

MaskRay added a comment. In addition, don't use `Reland "Reland "Reland "Reland ...` One `Reland` is sufficient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082 __

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

MaskRay added a comment. The CPython usage is a real UB: https://github.com/python/cpython/issues/94250 In D126864#3609497 , @efriedma wrote: > I suspect the refactoring in the latest version of the patch accidentally > made UBSan a bit more strict by d

[PATCH] D128554: [Driver][Minix] -r: imply -nostdlib like GCC

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/D128554/new/ https://reviews.llvm.org/D128554 __

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

MaskRay added a comment. In D126864#3609965 , @xbolva00 wrote: >>> -fsanitize=array-bounds workaround for size-1 array as the last member of a >>> structure > > Could you provide option for that (to enable stricker bound checks introduced > with this pa

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

MaskRay added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:219 + + static std::string concat(const std::string &Path, const Twine &A, +const Twine &B = "", const Twine &C = "", This can use `llvm::sys::path::app

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

MaskRay added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:219 + + static std::string concat(const std::string &Path, const Twine &A, +const Twine &B = "", const Twine &C = "", egorzhdan wrote: > MaskRay wrote:

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:371 +- Support ``-mharden-sls=all`` for X86. + nickdesaulniers wrote: > pengfei wrote: > > nickdesaulniers wrote: > > > This should be updated if additional options are supported. > > > > >

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added inline comments. Comment at: clang/test/Driver/x86-target-features.c:317 +// SLS-IND: "-target-feature" "+harden-sls-ind" +// SLS-ALL: "-target-feature" "+harden-sls-ind" "-target-feature" "+harden-sls-ret" +// SLS-NONE-NOT: harden-sls The pattern

[PATCH] D126289: [Clang][Driver] Fix include paths for `--sysroot /` on Linux

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChain.cpp:917 +/*static*/ std::string ToolChain::concat(const std::string &Path, + const Twin

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:368 -Select straight-line speculation hardening scope +Select straight-line speculation hardening scope (AArch64/X86 only). must be 'all', 'none', 'retbr'(AArch64), 'blr'(AArch64), 'return'(X8

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:257 +} else if (Scope != "none") { + D.Diag(diag::err_invalid_sls_hardening) << Scope << A->getA

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added a comment. > [X86] Add support for `-mharden-sls=all` The subject should list all supported values, or just say `Support -mharden-sls=` (don't list the values). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/ https://revie

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added inline comments. Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:355 +void X86AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) { + AsmPrinter::emitBasicBlockEnd(MBB); + if (Subtarget->hardenSlsRet() || Subtarget->hardenSlsInd()) { Thi

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

MaskRay added a comment. gcc has renamed indirect-branch to indirect-jmp. The rationale is that something like `call *(%rbx)` does not need `int3`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/ https://reviews.llvm.org/D126137 ___

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

MaskRay added a comment. Herald added a project: All. The convention is to use `err_drv_unsupported_option_argument` instead of adding a new diagnostic kind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81404/new/ https://reviews.llvm.org/D81404

[PATCH] D126511: [ARM][AArch64] Change -mharden-sls= to use err_drv_unsupported_option_argument

MaskRay created this revision. MaskRay added reviewers: kristof.beyls, ostannard, nickdesaulniers, pengfei. 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. Update t

[PATCH] D126192: [Driver] Support linking with lld for target AVR

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:435 - std::string Linker = getToolChain().GetProgramPath(getShortName()); + // Compute the linker program path, and use GNU "avr-ld" as default. + const Arg* A = Args.getLastArg(options::OPT_fuse_

[PATCH] D126192: [Driver] Support linking with lld for target AVR

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:530 + if (AVRLibcRoot && FamilyName) { +std::string Prefix(*AVRLibcRoot + "/lib/ldscripts/"); +if (llvm::sys::fs::is_directory(Prefix)) { I am concerned with the m

[PATCH] D124751: [HLSL] Support -E option for HLSL.

MaskRay added inline comments. Comment at: clang/test/SemaHLSL/entry.hlsl:2 +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo -DWITH_NUM_THREADS -ast-dump -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -Efoo -o -

[PATCH] D124753: [HLSL] Set main as default entry.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:194 +const StringRef DefaultEntry = "main"; +DAL->AddSeparateArg(nullptr, Opts.getOption(options::OPT_hlsl_entrypoint), +DefaultEntry); Instead of lett

[PATCH] D126511: [ARM][AArch64] Change -mharden-sls= to use err_drv_unsupported_option_argument

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG068b8af7961c: [ARM][AArch64] Change -mharden-sls= to use err_drv_unsupported_option_argument (authored by MaskRay). Repository: rG LLVM Github Mon

[PATCH] D126137: [X86] Add support for `-mharden-sls=[none|all|return|indirect-jmp]`

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:371 +- Support ``-mharden-sls=all`` for X86. + MaskRay wrote: > nickdesaulniers wrote: > > pengfei wrote: > > > nickdesaulniers wrote: > > > > This should be updated if additional options ar

[PATCH] D126669: [Driver][Modules] Remove dependence on linking support from clang/test/Driver/modules.cpp

MaskRay added a comment. This change is fine (if I were adding the tests in the first place, I'd add -c or -S), but I'd like to understand why you have such a toolchain not supporting linking. Such special requirements make me feel odd. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D126672: [Driver] Add multiarch path for RISC-V

MaskRay added a comment. This needs a test. Can Debian's riscv GCC be fixed to use a normalized triple for library and include paths? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126672/new/ https://reviews.llvm.org/D126672

[PATCH] D120484: More explicit message when failing to find a mandatory cfi ressource file

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:216 +def err_drv_missing_sanitizer_ignorelist : Error< + "missing

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay added a comment. > Adds -opaque-pointers/-no-opaque-pointers options to the gold plugin; > disabled by default. `-plugin-opt=[no-]opaque-pointers` > --opaque-pointers/--no-opaque-pointers options with > -plugin-opt=-opaque-pointers/-plugin-opt=-no-opaque-pointers aliases to lld; > dis

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay added inline comments. Comment at: clang/test/Driver/lto-no-opaque-pointers.c:2 +// UNSUPPORTED: enable-opaque-pointers +// RUN: %clang -target x86_64-unknown-linux -### %s -flto 2> %t +// RUN: FileCheck %s < %t -target is a legacy option. Use `--target=`

[PATCH] D126192: [Driver] Support linking with lld for target AVR

MaskRay added a comment. I am still uncomfortable with such a change. Trying to be smart sometimes may get in the way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 ___ cfe-commits mailing list cf

[PATCH] D124753: [HLSL] Set main as default entry.

MaskRay added inline comments. Comment at: clang/test/CodeGenHLSL/entry_default.hlsl:6 +// Make sure not mangle entry. +// CHECK:define void @main() [[MAIN_ATTR:#[0-9]]] +// CHECK:define void @_Z3foov() [[FOO_ATTR:#[0-9]]] space between `:` and the value. =

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

MaskRay added a comment. FYI `diagtool tree` dumps the diagnostics. You can even provide -W options and observe the differences. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126796/new/ https://reviews.llvm.org/D126796 __

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

MaskRay added a comment. In D126796#3551261 , @cjdb wrote: > In D126796#3550848 , @MaskRay wrote: > >> FYI `diagtool tree` dumps the diagnostics. You can even provide -W options >> and observe the differences. >

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > LTO: Decide upfront whether to use opaque/non-opaque pointer types This may need to mention "add options" to decide <...> Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay added inline comments. Comment at: llvm/tools/gold/gold-plugin.cpp:966 + Config.OpaquePointers = options.opaque_pointers; + `Conf`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125847/new/ https://re

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay added a comment. In D125847#3551841 , @MatzeB wrote: > address review feedback. I assume this is accepted and good to push when the > buildkit builds look reasonable. (I changed myself to a blocking review to check the component I mostly care a

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

MaskRay added inline comments. Comment at: llvm/tools/gold/gold-plugin.cpp:966 + Config.OpaquePointers = options.opaque_pointers; + MatzeB wrote: > MaskRay wrote: > > `Conf`? > > > > > Ouch, good catch. Guess we have no form of direct testing of the gold plu

[PATCH] D126812: [Binary] Promote OffloadBinary to inherit from Binary

MaskRay added inline comments. Comment at: llvm/lib/Object/OffloadBinary.cpp:18 - -namespace llvm { tra wrote: > tra wrote: > > Nit: `using namespace` seems a bit too heavy-handed when you only need one > > enum > > `using object_error = llvm::object::object_e

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

MaskRay added a comment. It's better to remove LLVM_BINUTILS_INCDIR for test directories in another patch. This change may expose some failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125624/new/ https://reviews.llvm.org/D125624 __

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

MaskRay added a comment. Your commit message seems to use the original summary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125847/new/ https://reviews.llvm.org/D125847 ___ cfe-commits mailing list cfe

[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

MaskRay added a comment. In D125862#3552465 , @tbaeder wrote: > Ping. @MaskRay Can you take a quick look and check if this is what you had in > mind? LG Comment at: clang/unittests/Driver/ToolChainTest.cpp:615 +TEST(ToolChainTest, To

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

MaskRay added a comment. If you still decide to add an option to clang, I think "diagnostics" is better than "warnings". There are many diagnostics which are errors by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126796/new/ https://revi

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1228 GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass); -else if (D->hasAttr() && !GV->isDeclarationForLinker()) +else if ((D->hasAttr() || + shouldMapVisi

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. This needs a test. There is existing testing gap as this change does not break any clang/test/Driver test. @@ -449,13 +449,15 @@ bool MSVCToolChain::

[PATCH] D119296: KCFI sanitizer

MaskRay added a comment. This lld/ELF change looks like a hack. I'll need to study what this patch does, but in the meanwhile I changed myself to a blocking reviewer as I think the lld/ELF change needs more investigation. Comment at: lld/ELF/Symbols.cpp:553 + // incompatible

[PATCH] D119296: KCFI sanitizer

MaskRay added inline comments. Comment at: lld/ELF/Symbols.cpp:553 + // incompatible declarations for the same function. + if (isWeak() && getName().startswith("__kcfi_typeid_") && + cast(this)->value != other.value) samitolvanen wrote: > MaskRay wrote: >

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

MaskRay accepted this revision. MaskRay added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126862/new/ https://reviews.llvm.org/D126862 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D126862: [clang] [MSVC] Enable unwind tables for ARM

MaskRay added a comment. (I still think it helps to drop some RUN lines from pic.c but that change can be separate.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126862/new/ https://reviews.llvm.org/D126862 __

[PATCH] D126672: [Driver] Add multiarch path for RISC-V

MaskRay added a comment. In D126672#3546782 , @Hahnfeld wrote: > In D126672#3546773 , @MaskRay wrote: > >> This needs a test. > > There are no tests for any of the other architectures. > >> Can Debian's riscv GCC

[PATCH] D126396: Clean "./" from __FILE__ expansion.

MaskRay added a comment. Is it feasible to add a test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126396/new/ https://reviews.llvm.org/D126396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

MaskRay added a comment. Add openmp to `LLVM_ENABLE_PROJECTS` to trigger the issue: cmake -GNinja -Sllvm -B/tmp/out/play -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;openmp' -DCMAKE_CXX_COMPILER=~/Stable/bin/clang++ -DCMAKE_C_COMPILER=~/Stable/bin/clang -DCMAKE_INSTALL_PREFIX=/tmp

[PATCH] D127009: Modify test case so it verifies the fix from D126396.

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! The convention is to add `[test] ` to test-only patches. You can change the Phabricator title, then run `arc amend` to amend the local commit. I believe on Debian testing `arc` is so

[PATCH] D125165: [Clang] Introduce clang-offload-packager tool to bundle device files

MaskRay added a comment. In D125165#3557693 , @jhuber6 wrote: > In D125165#3557441 , @MaskRay wrote: > >> Add openmp to `LLVM_ENABLE_PROJECTS` to trigger the issue: >> >> cmake -GNinja -Sllvm -B/tmp/out/play -DC

[PATCH] D127009: [test] Modify test case so it verifies the fix from D126396

This revision was automatically updated to reflect the committed changes. Closed by commit rG490990bb1f65: [test] Modify test to verify D126396 (Clean "./" from __FILE__ expansion) (authored by ppluzhnikov, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D127009?vs=434

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

MaskRay added a comment. I see that `clang/test/Driver/arm-float-abi-lto.c` now has a `// UNSUPPORTED: system-aix` workaround but I think someone working on AIX should investigate the issue. I think the reported issue isn't actionable to those who don't use AIX. Repository: rG LLVM Github Mo

[PATCH] D109977: LLVM Driver Multicall tool

MaskRay added a comment. > llvm-driver can be disabled from builds by setting > LLVM_TOOL_LLVM_DRIVER_BUILD=Off. I think this should be opt-in. The new `llvm` executable takes a lot of space and not needed by many developers/build bots. It's useful to some groups (distributions) but they can sp

[PATCH] D109977: LLVM Driver Multicall tool

MaskRay added a comment. % /tmp/out/custom1/bin/llvm --help OVERVIEW: llvm compiler driver USAGE: llvm [subcommand] [options] SUBCOMMANDS: ar- ar ## should be llvm-ar bitcode-strip - bitcode-strip ### should be llvm-bitcode-strip clang

[PATCH] D126865: [clang] [Headers] Check __SEH__, when checking if ARM EHABI is implied

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/D126865/new/ https://reviews.llvm.org/D126865 _

[PATCH] D109977: LLVM Driver Multicall tool

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM from my perspective, but this needs someone else's review. Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:50 + bool ConsumeFirstArg = false; + if (LaunchedTool =

[PATCH] D32199: [TySan] A Type Sanitizer (Clang)

MaskRay added a comment. Having the feature will be useful. `curl -L 'https://reviews.llvm.org/D32199?download=1' | patch -p1` doesn't apply cleanly. This needs a rebase. Comment at: clang/test/Driver/sanitizer-ld.c:250 +// RUN: %clangxx -no-canonical-prefixes %s -### -o %t

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:485 "amdhip64.lib"}); + CmdArgs.push_back(Args.MakeArgString("clang_rt.builtins-" + + getTriple().getArchName() + ".lib")); N

[PATCH] D127145: [Driver] add -lresolv for all but Android.

MaskRay added a comment. The description needs to mention the motivation. Is this meant for `isOSLinux() && isGNUEnvironment()` targets? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127145/new/ https://reviews.llvm.org/D127145 __

[PATCH] D127145: [Driver] add -lresolv for all but Android.

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:843 + if (!TC.getTriple().isAndroid()) { +CmdArgs.push_back("-lresolv

[PATCH] D127150: [doc] Add release notes about SEH unwind information on ARM

MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:357 + +- Switched MinGW mode on ARM to use SEH instead of Dwarf for unwind information. + The official spelling is `DWARF`. The official way to name the version is `DWARF version 5`. If abbr

[PATCH] D126192: [Driver] Improve linking options for target AVR

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:507 +// Add user specified linker script. +const Arg *LDS = Args.getLastArg(options::OPT_T); +if (LDS) { Just do Args.AddAllArgs. Please check how Gnu.cpp passes `-T` CH

[PATCH] D119296: KCFI sanitizer

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Please don't land the patch this state. There are several questions which should be sorted out first. a) Naming About the 'k' prefix: this is generic and does not need to be coupl

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

MaskRay added a comment. Do you have more authoritative answer when /root/usr is used and when it isn't? I'd wish that we have comments for them. This change also needs a unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127310/new/ https:/

[PATCH] D127093: [clang][pseudo] Add missing Support lib to cxx

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Obsoleted by D127269 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127093/new/ https://reviews.llvm

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. > These functions are not available in libgcc but in libclang_rt.builtins. > Therefore --hip-link needs to link with libclang_rt.builtins by default. I think this is problematic. The current link sequence is `... "-lamdhip64" "/tmp/Debug/lib/clang/15.0.0/lib/linux/libc

[PATCH] D123493: Support the min of module flags when linking, use for AArch64 BTI/PAC-RET

MaskRay added a comment. This behavior requires that all participating modules have the module flag. In the absence of the module flag, what should be behavior be? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123493/new/ https://reviews.llvm.org/

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added a comment. In D127142#3570290 , @yaxunl wrote: > In D127142#3568905 , @MaskRay wrote: > >>> These functions are not available in libgcc but in libclang_rt.builtins. >>> Therefore --hip-link needs to

[PATCH] D127142: [HIP] Link with clang_rt.builtins

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:684-690 CmdArgs.append( {Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()), "-rpath", Args.MakeArgString(RocmInstallation.getLibPath())}); CmdArgs.push_back("

[PATCH] D158476: [driver] Search for compatible Android runtime directories

MaskRay accepted this revision. MaskRay added a comment. nits Comment at: clang/docs/ReleaseNotes.rst:295 + target-specific runtime and standard libraries in directories named after the + target (e.g. if you're building with ``-target aarch64-none-linux-android21``, + Clang

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

MaskRay added a comment. In D147844#4646633 , @MaskRay wrote: > FWIW: adding parentheses for expressions, such as `overflow = (4608 * 1024 * > 1024) ? 4608 * 1024 * 1024 : 0;`, and `results.reason = (actions & > _UA_SEARCH_PHASE) ? ... : ...`, looks un

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

MaskRay added a comment. > So this is truly a test of the ld.otherlinker feature pattern, not some > special case driver feature. I guess we should leave the test alone. > Closing, we left the test alone, it still uses -fuse-ld=lld-link2. Perhaps in > the future we should reconsider this, but th

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

MaskRay accepted this revision. MaskRay added a comment. DataLayout needs a unittest in llvm/unittests/IR/DataLayoutTest.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152206/new/ https://reviews.llvm.org/D152206 ___

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/unittests/IR/DataLayoutTest.cpp:108 +TEST(DataLayoutTest, TargetTripleManglingComponent) { + Triple TT = Triple("x86_64-unknown-uefi"); UEFI may be a better name in case we test mo

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1261 HelpText<"HIP runtime installation path, used for finding HIP version and adding HIP include path.">; +def hipstdpar : Flag<["-", "--"], "hipstdpar">, + Visibility<[ClangOption, CC1Option]>

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6569 + + if (Args.hasArg(options::OPT_hipstdpar_interpose_alloc)) +CmdArgs.push_back("-hipstdpar-interpose-alloc"); This can use AddLastArg. CHANGES SINCE LAST ACTION

[PATCH] D155769: [HIP][Clang][docs][RFC] Add documentation for C++ Parallel Algorithm Offload

MaskRay added inline comments. Comment at: clang/docs/HIPSupport.rst:266 + +- ``-hipstdpar`` / ``--hipstdpar`` enables algorithm offload, which depending on + phase, has the following effects: Newer long options that don't use the common prefix like `-f` are pr

[PATCH] D158641: [AArch64] Fix FMV ifunc resolver usage on old Android APIs. Rename internal compiler-rt FMV functions.

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. The Clang Driver/CodeGen and compiler-rt changes look good to me. Of course please wait for an Android reviewer :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D145214: [TSAN] add support for riscv64

MaskRay added inline comments. Comment at: compiler-rt/lib/tsan/rtl/tsan_platform.h:971 }; -const uptr indicator = 0x0e00ull; +const uptr indicator = 0x0f00ull; const uptr ind_lsb = 1ull << LeastSignificantSetBitIndex(indicator);

[PATCH] D145214: [TSAN] add support for riscv64

MaskRay accepted this revision. MaskRay added a comment. Other than -Wframe-larger-than=656 and `const uptr indicator = 0x0f00ull;` changes, others looks good to me. I agree that this should be reviewed by @dvyukov :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1281 +"rocThrust path, required by the HIP Standard Parallel Algorithm " +"Acceleration library, used to implicitly include the rocThrust library.">; +def hips

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:340 + HasRocThrustLibrary = !HIPRocThrustPathArg.empty() && +D.getVFS().exists(HIPRocThrustPathArg + "/thrust"); + HIPRocPrimPathArg = AlexVlx wrote: > Mas

[PATCH] D159541: [UEFI] X86_64 UEFI Clang Driver

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:68 + // "Terminal Service Aware" flag is not needed for UEFI applications. + CmdArgs.push_back("-tsaware:no

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

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/ https://revie

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

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: clang/include/c

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

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 setDataLayou

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

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 cast

<    19   20   21   22   23   24   25   26   27   28   >