[PATCH] D80660: clang: Add support for relative linker paths with -fuse-ld

MaskRay added inline comments. Comment at: clang/test/Driver/fuse-ld.c:7 +// RUN: cd "%S" && %clang %s -### \ +// RUN: -fuse-ld=Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \ This changes the working directory. It may be better moving cd tests to the bottom. R

[PATCH] D80289: [Driver][X86] Support branch align options with LTO

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/x86-malign-branch.c:6 // BOUNDARY: "-mllvm" "-x86-align-branch-boundary=16" +// RUN: %clang -target x86_64-unknown-linux -malign-branch-b

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

MaskRay added a comment. @theraven Are you ok with this change? You seem to have a use case not using ld.bfd, ld.gold or ld.lld . This change will require you to adapt. In D80225#2049466 , @jyknight wrote: > It's worrying to me that there number of place

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1213 SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc(); -return Error(ErrorLoc, - "operand must be e[8|16|32|64|128|256|512|1024],m[1|2|4|8]"

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:291 ///< including them in the name). +CODEGENOPT(DebugUnusedTypes, 1, 0) /// < Whether to emit typedefs that may be +

[PATCH] D80883: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

MaskRay created this revision. MaskRay added reviewers: dblaikie, echristo, efriedma, joerg, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D80883 Files: clang/include/clang/Driver/Options.td Index

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

MaskRay added a comment. `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

MaskRay added a comment. `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential

[PATCH] D80830: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

MaskRay added a comment. `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential

[PATCH] D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

MaskRay added a comment. `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:372 + llvm::opt::ArgStringList &CmdArgs) const { + // Add linker option -u__llvm_profile_runtime to cause runtime + // initialization module to be linked in. ---

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added inline comments. Comment at: lld/ELF/LTO.cpp:79 // Check if basic block sections must be used. // Allowed values for --lto-basicblock-sections are "all", "labels", // "", or none. This is the equivalent `--lto-basic-block-sections` I thin

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. LLD side changes look good. Are you waiting on an explicit approval from @rmisth ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D80828: [Clang][A32/T32][Linux] -O2 implies -fomit-frame-pointer

MaskRay added inline comments. Comment at: clang/test/Driver/frame-pointer-elim.c:125 +// For Android, keep the framepointers always. +// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O2 -S %s 2>&1 | \ +// RUN: FileCheck --check-prefix=KEEP-ALL %s `

[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

MaskRay accepted this revision. MaskRay added a comment. (You can change `[Clang]` to `[Driver]` as `[Clang]` may carry less information. `[Driver]` emphasizes this is related to clangDriver. Nothing in sema/codegen/analyzer/etc is affected.) Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:964 +<< Opts.BBSections; + } + grimar wrote: > Doesn't seem you need "{}" around this li

[PATCH] D80757: [PowerPC] Add clang option -m[no-]pcrel

MaskRay added a comment. `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential

[PATCH] D80913: [update_cc_test_checks.py] Correctly skip function definitions

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

[PATCH] D80914: [update_cc_test_checks.py] Handle C++ methods

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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

MaskRay added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:676 + Value v = vals[vd]; + if (isUninitialized(v)) +handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v)); This should use isAlwaysUninit. I fixed it in

[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

MaskRay added a subscriber: olista01. MaskRay added a comment. In D80828#2069287 , @nickdesaulniers wrote: > May I please have a non-Googler to review+(accept|reject) the revision? I guess @olista01 is an inactive account. Changed to @ostannard Reposi

[PATCH] D80883: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

This revision was automatically updated to reflect the committed changes. Closed by commit rG7694b571d9fd: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D77168: Add a flag to debug automatic variable initialization

MaskRay added a comment. Do we have a mechanism bisecting pragmas? If yes, we can let that tool add `#pragma clang attribute push([[clang::uninitialized]], apply_to = variable)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://revi

[PATCH] D77168: Add a flag to debug automatic variable initialization

MaskRay added a comment. In D77168#2070049 , @jcai19 wrote: > In D77168#2069783 , @MaskRay wrote: > > > Do we have a mechanism bisecting pragmas? If yes, we can let that tool add > > `#pragma clang attribute push([

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay updated this revision to Diff 278555. MaskRay edited the summary of this revision. MaskRay removed a subscriber: gkm. MaskRay added a comment. Herald added subscribers: dang, emaste. Respect -B and COMPILER_PATH Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay added a comment. I tweaked the following item in the last revision > a relative path without a path component separator (/), the value is searched > using the -B, COMPILER_PATH, then PATH. My communication with GCC folks is that they will be happy to adopt something like --ld-path. GCC

[PATCH] D83149: [gcov] Add __gcov_dump/__gcov_reset and delete __gcov_flush

MaskRay added a comment. Ping:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83149/new/ https://reviews.llvm.org/D83149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

This revision was automatically updated to reflect the committed changes. Closed by commit rG53880b8cb9c6: [CMake] Make `intrinsics_gen` dependency unconditional. (authored by michele.scandale, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D83149: [gcov] Add __gcov_dump/__gcov_reset and delete __gcov_flush

This revision was automatically updated to reflect the committed changes. Closed by commit rG5809a32e7c2d: [gcov] Add __gcov_dump/__gcov_reset and delete __gcov_flush (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83149/new/ ht

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

MaskRay added a comment. @dim Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Su

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

This revision was automatically updated to reflect the committed changes. Closed by commit rG1bc5c84710a8: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D83015?vs=278555&id=279282#toc Repo

[PATCH] D83592: [Coverage] Add comment to skipped regions

MaskRay added a comment. New tests should use `[[#@LINE]]` instead of `[[@LINE]]` https://llvm.org/docs/CommandGuide/FileCheck.html > To support legacy uses of @LINE as a special string variable, FileCheck also > accepts the following uses of @LINE with string substitution block syntax: > [[@L

[PATCH] D83519: [NewPM] Support optnone under new pass manager

MaskRay added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:77 + StandardInstrumentations(bool DebugLogging) + : PrintIR(), TimePasses(), OptNone(DebugLogging) {} (No need to change) PrintIR & TimePasses don't need to ap

[PATCH] D83592: [Coverage] Add comment to skipped regions

MaskRay added inline comments. Comment at: compiler-rt/test/profile/coverage_comments.cpp:9 +int y = 0; /* comment */ // CHECK: [[# @LINE]]| 1| int y = 0; /* comment */ +int z = 0; // comment // CHECK: [[# @LINE]]| 1| int z = 0; // comment +

[PATCH] D83592: [Coverage] Add comment to skipped regions

MaskRay added a comment. (This was committed within 10 minutes after it was approved. As a large change with updates to some sensitive places like clang/Lex/Preprocessor.h, this probably should have been waited a bit longer.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83592/new/ ht

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

MaskRay added a comment. llvm/utils/git/pre-push.py is recommended for your future commits. I guess `arc land` is a bad choice because llvm/llvm-project uses the github community version which can't set a pre-receive hook. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D84356: [AIX] remote -u from the clang when invoke aix as assembler

MaskRay added a comment. remote -> remove Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84356/new/ https://reviews.llvm.org/D84356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D84029: [clang][Driver] Default to /usr/bin/ld on Solaris

MaskRay added a comment. Can you add a test to `test/Driver/solaris-ld.c`? > However, if someone forgets this, it depends on the user's PATH whether or > not clang finds the correct linker, which doesn't make for a good user > experience. Not very sure about this. The last resort of GetProgram

[PATCH] D84414: [RISCV] Support Shadow Call Stack

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:33-37 + std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo(); + if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo &CSR) { +return CSR.getReg() == RISCV::X1; + }))

[PATCH] D84511: Fix update_cc_test_checks.py --llvm-bin after D78478

MaskRay added inline comments. Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:24 +# works as expected +config.substitutions.append( +('%update_cc_test_checks_llvm_bin', "%s %s %s" % ( The substitution is here just to make a test. Can it be t

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

MaskRay added a comment. Ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80391/new/ https://reviews.llvm.org/D80391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:623 // Compute Hash value for the CFG: the lower 32 bits are CRC32 of the index -// value of each BB in the CFG. The higher 32 bits record the number of edges. +// value of each

[PATCH] D84511: Fix update_cc_test_checks.py --llvm-bin after D78478

MaskRay accepted this revision. MaskRay added a comment. LGTM. Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:24 +# works as expected +config.substitutions.append( +('%update_cc_test_checks_llvm_bin', "%s %s %s" % ( vitalybuka wrote: > ari

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added a comment. Fixed by 6a75496836ea14bcfd2f4b59d35a1cad4ac58cee :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/ https://reviews.llvm.org/D83648 _

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:266 +PGOOldCFGHashing("pgo-instr-old-cfg-hashing", cl::init(false), cl::Hidden, + cl::desc("Use the old CFG function hashing.")); + Nit:

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

MaskRay added a comment. In D84743#2181031 , @jdoerfert wrote: > In D84743#2179441 , @tra wrote: > >> I'm not sure it's particularly useful, to be honest. CUDA code still needs >> to be compatible with NVCC so it c

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

MaskRay accepted this revision. MaskRay added a comment. LGTM as well with @davidxl's comments addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84782/new/ https://reviews.llvm.org/D84782 ___ cfe-

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661 +} Data; +Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts(); +JCH.update(Data.C); `#include "llvm/Support/Endian.h"` and use `support::endian::

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661 +} Data; +Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts(); +JCH.update(Data.C); MaskRay wrote: > `#include "llvm/Support/Endian.h"` > > and u

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

MaskRay added a comment. Added usual lld and LTO side reviewers. I am just a bit worried that option name changes like this patch and D77231 could accidentally slip through if I did not react in time... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D73307: Unique Names for Functions with Internal Linkage

MaskRay added inline comments. Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained by appending the hash of the + full module name to the original symbol. This option is particularly useful + in attributing profile information to the c

[PATCH] D77728: [Driver][X86] Add an alias for -mpad-max-prefix-size

MaskRay added a comment. > Make -malign-branch-prefix-size an alias for -mpad-max-prefix-size (For GCC > compatibility) I cannot find a discussion on -mpad-max-prefix-size. Is it a planned GNU as option or GCC driver option? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D77628: [Driver][X86] Add -mpad-max-prefix-size

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. > It has similar (but slightly different) effect as GAS's option > -malign-branch-prefix-size, e.g. -mpad-max-prefix-size can also elminate NOPs > emitted by align directive, so we

[PATCH] D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096

MaskRay created this revision. MaskRay added reviewers: rprichard, srhines, thieta. Herald added subscribers: cfe-commits, danielkiss, kristof.beyls. Herald added a project: clang. Similar to D55029 . The requirement arises when discussing increasing default max-p

[PATCH] D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096

MaskRay updated this revision to Diff 256085. MaskRay added a comment. Clarify a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77746/new/ https://reviews.llvm.org/D77746 Files: clang/lib/Driver/ToolChains/Linux.cpp clang/test/Driver/an

[PATCH] D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096

This revision was automatically updated to reflect the committed changes. Closed by commit rG969b91af732d: [Driver] Default arm-linux-androideabi to -z max-page-size=4096 (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77746/new/

[PATCH] D77728: [Driver][X86] Add an alias for -mpad-max-prefix-size

MaskRay added a comment. In D77728#1970912 , @skan wrote: > In D77728#1969788 , @MaskRay wrote: > > > > Make -malign-branch-prefix-size an alias for -mpad-max-prefix-size (For > > > GCC compatibility) > > > > I can

[PATCH] D77628: [Driver][X86] Add -mpad-max-prefix-size

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Driver/Options.td:2190 HelpText<"Specify the boundary's size to align branches">; -def malign_branch_prefix_size_EQ : Joined<["-"],

[PATCH] D68049: Propeller: Clang options for basic block sections

MaskRay added a comment. In D68049#1970825 , @tmsriram wrote: > Ping. @rsmith ^^^ More specific question, do you think `clang/test/CodeGen/basicblock-sections.c` should be converted to a `-S -emit-llvm` test? CHANGES SINCE LAST ACTION https://revie

[PATCH] D73845: [Gnu toolchain] Move GCC multilib/multiarch paths support from Linux to Gnu

MaskRay added a comment. Moving functions from Linux.cpp to Gnu.cpp is definitely appropriate. Have you made any adjustment? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73845/new/ https://reviews.llvm.org/D73845 ___ cfe-commits mailing l

[PATCH] D73845: [Gnu toolchain] Move GCC multilib/multiarch paths support from Linux to Gnu

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! You may need to wait for @phosek to confirm. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73845/new/ https://reviews.llvm.org/D73845 ___

[PATCH] D78030: [TimeProfiler] Emit clock synchronization point

MaskRay added inline comments. Comment at: lld/test/ELF/time-trace.s:21 -# CHECK: "traceEvents": [ +# CHECK: "beginningOfTime": {{[0-9]{16},}} +# CHECK-NEXT: "traceEvents": [ Add 5 spaces after `:` so that the key is aligned Repository: rG LLVM Github Monor

[PATCH] D78117: [AVR] Define __ELF__

MaskRay added a comment. The `test/clang/Preprocessor/` directory may make more sense for such tests. Is it time to create a target-specific sub directory `test/clang/Preprocessor/AVR/`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78117/new/ ht

[PATCH] D78030: [TimeProfiler] Emit clock synchronization point

MaskRay added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:266 + const auto BeginningOfTimeUs = SystemTime - ProcessLocalTime; + J.attribute("beginningOfTime", + BeginningOfTimeUs.time_since_epoch().count()); What is t

[PATCH] D78027: [TimeProfiler] Emit real process ID and thread names

MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: llvm/lib/Support/TimeProfiler.cpp:77 +static std::string getThreadName() { + SmallString<64> Name; You can define ThreadName a

[PATCH] D78027: [TimeProfiler] Emit real process ID and thread names

MaskRay added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:77 +static std::string getThreadName() { + SmallString<64> Name; broadwaylamb wrote: > MaskRay wrote: > > You can define ThreadName as `SmallString<0>` to avoid this function. > Then

[PATCH] D78027: [TimeProfiler] Emit real process ID and thread names

MaskRay added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:249 + +for (const auto &TTP : ThreadTimeTraceProfilerInstances) { + writeMetadataEvent("thread_name", TTP->Tid, TTP->ThreadName); MaskRay wrote: > broadwaylamb wrote: > > MaskRa

[PATCH] D78030: [TimeProfiler] Emit clock synchronization point

MaskRay added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:266 + const auto BeginningOfTimeUs = SystemTime - ProcessLocalTime; + J.attribute("beginningOfTime", + BeginningOfTimeUs.time_since_epoch().count()); broadwayl

[PATCH] D78163: [AVR][NFC] Move preprocessor tests to Preprocessor directory

MaskRay added a comment. Not sure we need `target-cpu-defines/` You can add a `lit.local.cfg` to avoid `REQUIRES: avr-registered-target` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78163/new/ https://reviews.llvm.org/D78163 __

[PATCH] D78125: [AVR] Use the correct address space for non-prototyped function calls

MaskRay added a comment. > This fix is necessary to let Clang compile compiler-rt for AVR. This suggests that some compiler-rt declarations do not follow the best practice... What are they? Comment at: clang/test/CodeGen/address-space-avr.c:3 + +// CHECK: define void @bar() a

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

MaskRay closed this revision. MaskRay added a comment. Closed by 6551ac7489fe070a2edcfac88f68c93321cba9a9 The commit does not have `Differential Revision: ...` so the differential is not closed automatically. Repository:

[PATCH] D78273: [clang-tools-extra] reimplement PreprocessorTracker in terms of StringSet.

MaskRay added inline comments. Comment at: clang-tools-extra/modularize/PreprocessorTracker.cpp:912 // Lookup/add string. - StringHandle addString(llvm::StringRef Str) { return Strings.intern(Str); } + StringHandle addString(llvm::StringRef Str) { +return Strings.insert(

[PATCH] D77168: Add a flag to debug automatic variable initialization

MaskRay added a comment. In D77168#2070334 , @llozano wrote: > In D77168#2070083 , @MaskRay wrote: > > > In D77168#2070049 , @jcai19 wrote: > > > > > In D77168#2069783

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

MaskRay added a comment. Drive-by comment: the clang side change isn't tightly coupled with the LLVM side changes. It should be a separate patch. Comment at: llvm/test/MC/RISCV/rvv/add.s:1 +# RUN: llvm-mc -triple=riscv64 -show-encoding --mattr=+experimental-v < %s \ +# RUN:

[PATCH] D81188: [RISCV] Support experimental v extensions.

MaskRay added inline comments. Comment at: clang/test/Driver/riscv-arch.c:364 + +// RUN: %clang -target riscv32-unknown-elf -march=rv32iv -### %s \ +// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-EXPERIMENTAL-V-NOFLAG %s For the RUN lines, I'd prefer

[PATCH] D81188: [RISCV] Support experimental v extensions.

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

[PATCH] D80966: [codeview] Put !heapallocsite on calls to operator new

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > but the operator declared at global scope returns void. void -> void pointer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80966/new/ https

[PATCH] D79842: [clang][Driver] Correct tool search path priority

MaskRay added inline comments. Comment at: clang/test/Driver/program-path-priority.c:15 +// "program path" for these tests +// RUN: rm -rf %t +// RUN: mkdir -p %t `rm -rf %t && mkdir -p %t` You can do this on one line Comment at: clang/test/Dri

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

MaskRay added inline comments. Comment at: compiler-rt/lib/builtins/fp_lib.h:49 -static __inline int rep_clz(rep_t a) { return __builtin_clz(a); } +static __inline int rep_clz(rep_t a) { return clzsi(a); } Why is this needed? Repository: rG LLVM Github Mo

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

MaskRay added inline comments. Comment at: compiler-rt/lib/builtins/fp_lib.h:49 -static __inline int rep_clz(rep_t a) { return __builtin_clz(a); } +static __inline int rep_clz(rep_t a) { return clzsi(a); } atrosinenko wrote: > MaskRay wrote: > > Why is this n

[PATCH] D80730: [OPENMP50]Codegen for use_device_addr clauses.

MaskRay added a comment. Hi, you can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` with the following script arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary

[PATCH] D79842: [clang][Driver] Correct tool search path priority

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Please give other reviews a day or two to respond in case they have comments. Comment at: clang/test/Driver/program-path-priority.c:22 +/// No gccs at all, no

[PATCH] D81995: [xray] Option to omit the function index

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:114 +///< Set with -fno-xray-function-index to omit the index section. +CODEGENOPT(XRayOmitFunctionIndex , 1, 0) + Nit: a variable name with a positive meaning may be easier t

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

MaskRay added a comment. Hi, you can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{

[PATCH] D81995: [xray] Option to omit the function index

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:114 +///< Set with -fno-xray-function-index to omit the index section. +CODEGENOPT(XRayOmitFunctionIndex , 1, 0) + ianlevesque wrote: > MaskRay wrote: > > Nit: a variable name

[PATCH] D81995: [xray] Option to omit the function index

MaskRay added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:114 +///< Set with -fno-xray-function-index to omit the index section. +CODEGENOPT(XRayOmitFunctionIndex , 1, 0) + ianlevesque wrote: > MaskRay wrote: > > ianlevesque wrote: >

[PATCH] D82992: clang CoverageMapping tests bot cleanup

MaskRay added a comment. Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk '/Reviewers:|Subscrib

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay created this revision. MaskRay added reviewers: echristo, jyknight, martell, theraven, whitequark. Herald added a project: clang. Herald added a subscriber: cfe-commits. Supersedes D80225 . Add -fld-path= to avoid strange target specific prefixes and make

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay added a comment. I got pinged for my GCC -fuse-ld=path patch today. On GCC side, Martin Liška is fine with clang's choice if we can reach a reasonable consensus: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549236.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D82906: [flang][openmp] Use common Directive and Clause enum from llvm/Frontend

MaskRay added a comment. In D82906#2127047 , @clementval wrote: > Fix for warning I find clang better at -Wswitch related warnings than GCC. Might be nice building llvm-project with clang and testing with `check-clang`. If you don't mind `curl | sh`, `c

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

MaskRay added subscribers: xbolva00, greened, spatel, lebedev.ri. MaskRay added a comment. The idea look good to me, but I want some opinions on the name `UTC_ARGS` (I can't help associating it with Coordinated Universal Time). Adding some folks who may have opinions: @greened @lebedev.ri @RKSi

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

MaskRay abandoned this revision. MaskRay added a comment. Abandon in favor of D83015 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80225/new/ https://reviews.llvm.org/D80225 _

[PATCH] D83149: [gcov] Add __gcov_dump/__gcov_reset and delete __gcov_flush

MaskRay created this revision. MaskRay added reviewers: calixte, marco-c, serge-sans-paille, vsk. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, hiraditya. Herald added projects: clang, Sanitizers, LLVM. GCC r187297 (2012-05) introduced __gcov_dump and __gcov_reset. __gcov_flu

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay updated this revision to Diff 275507. MaskRay added a comment. Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83015/new/ https://reviews.llvm.org/D83015 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/inclu

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay updated this revision to Diff 275517. MaskRay added a comment. Add a -working-directory test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83015/new/ https://reviews.llvm.org/D83015 Files: clang/include/clang/Basic/DiagnosticDriverKinds.

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

MaskRay added subscribers: zhizhouy, void. MaskRay added inline comments. Comment at: llvm/tools/opt/opt.cpp:281 +static cl::opt EnableCallGraphProfile( +"enable-call-graph-profile", cl::init(true), cl::Hidden, If there is no strong need for tuning this, pl

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

MaskRay added inline comments. Comment at: llvm/tools/opt/opt.cpp:281 +static cl::opt EnableCallGraphProfile( +"enable-call-graph-profile", cl::init(true), cl::Hidden, zhizhouy wrote: > MaskRay wrote: > > If there is no strong need for tuning this, please d

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

MaskRay added inline comments. Comment at: llvm/tools/opt/opt.cpp:281 +static cl::opt EnableCallGraphProfile( +"enable-call-graph-profile", cl::init(true), cl::Hidden, zhizhouy wrote: > MaskRay wrote: > > zhizhouy wrote: > > > MaskRay wrote: > > > > If ther

[PATCH] D80952: [FPEnv][Clang][Driver] Disable constrained floating point on targets lacking support.

MaskRay added a comment. Note also that @arsenm is still a blocking reviewer. It is generally expected that all feedback is acknowledged. @kpn you should probably wait for @arsenm to explicitly clear the blocker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:556-557 + // -fld-path= takes precedence over -fuse-ld= and specifies the executable + // name. PATH is consulted if the value does not contain /. -B is + // int

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