[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

MaskRay added a comment. How do the new tests provide additional coverage? Comment at: clang/test/Driver/riscv-arch.c:45 + +// CHECK-ILP32: "-target-feature" "+m" +// CHECK-ILP32-SAME: "-target-feature" "+f" I suggest the style used in linux-cross.cpp Jus

[PATCH] D103878: [clang][RISCV][test] Add more tests of the -mabi and -march options

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Driver/riscv-arch.c:122 + +// CHECK-LP64F: "-target-feature" "+m" +// CHECK-LP64F-SAME: {{^}} "-target-feature" "+a" ``` // CH

[PATCH] D104155: Add documentation for -fsanitize-address-use-after-return.

MaskRay added inline comments. Comment at: clang/docs/AddressSanitizer.rst:17 * Use-after-free -* Use-after-return (runtime flag `ASAN_OPTIONS=detect_stack_use_after_return=1`) +* Use-after-return (clang flag `-fsanitize-address-use-after-return=(always|runtime|never)` default

[PATCH] D104192: [clang][RISCV] Change implicit ARCH for explicitly specified ABI

MaskRay added a comment. The thing looks weird. My mental model is that `-march` can infer `-mabi`, not the other way around... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104192/new/ https://reviews.llvm.org/D104192 ___

[PATCH] D104076: [clang-cl][sanitizer] Add -fsanitize-address-use-after-return to clang.

MaskRay added a comment. Note, clang-cl is an executable (`ninja clang-cl`), so using `clang-cl` in the subject can cause confusion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104076/new/ https://reviews.llvm.org/D104076 __

[PATCH] D104155: Add documentation for -fsanitize-address-use-after-return.

MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:3 --- NOTE: This file is automatically generated by running clang-tblgen -gen-opt-docs. Do not edit this file by hand!! ---

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

MaskRay added a comment. `__attribute__((no_instrument_function))` seems specific to `-finstrument-functions`. Somehow `gcc -pg` uses it as well. The name may not be generic, so it may be odd to exclude various instrumentations under this generic attribute. clang -fprofile-instr-generate (fro

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

MaskRay added a comment. For the function attributing disabling instrumentation of -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs, how about `no_profile`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104253/new/

[PATCH] D101832: [clang] accept -fsanitize-ignorelist= in addition to -fsanitize-blacklist=

MaskRay added a comment. Herald added a subscriber: ormris. In D101832#2736636 , @MaskRay wrote: > I think @vitalybuka has concrete suggestion on the naming. If ignorelist is used, I can delete `-fsanitize-coverage-blocklist=` Repository: rG LLVM Git

[PATCH] D104155: Add documentation for -fsanitize-address-use-after-return.

MaskRay added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:3 --- NOTE: This file is automatically generated by running clang-tblgen -gen-opt-docs. Do not edit this file by hand!! ---

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay added a comment. Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` don't specify that, either. As I commented on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223 , the Linux kernel specifies `noinline` so the exact inlining behavior doesn't matter. #define n

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay added a comment. In D104475#2825666 , @melver wrote: > In D104475#2825297 , @MaskRay wrote: > >> Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` >> don't specify that, either. >

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay added a comment. In D104475#2825772 , @nickdesaulniers wrote: > In D104475#2825711 , @MaskRay wrote: > >> So if we don't want to offer guarantee for IR/C/C++ attributes, we can >> document that users may

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay added a comment. In D104475#2825841 , @nickdesaulniers wrote: > In D104475#2825795 , @MaskRay wrote: > >> In D104475#2825772 , >> @nickdesaulniers wrote: >> >>>

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay accepted this revision. MaskRay added a subscriber: marxin. MaskRay added a comment. LG. @marxin FYI the GNU-style function attribute `no_profile` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104475/new/ https://reviews.llvm.org/D104475

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

MaskRay added inline comments. Comment at: clang/test/CodeGen/fprofile.c:1 +// RUN: %clang_cc1 -fprofile-instrument=llvm -disable-llvm-passes \ +// RUN: -emit-llvm -o - %s | FileCheck %s no_profile may be a better test name. Repository: rG LLVM Github Monor

[PATCH] D104658: [Clang][Codegen] rename no_profile fn attr no_profile_instrument_function

MaskRay accepted this revision. MaskRay added a comment. (This was why I suggested we waited a bit on GCC's response...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104658/new/ https://reviews.llvm.org/D104658 ___

[PATCH] D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular)

MaskRay updated this revision to Diff 353497. MaskRay added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. stack size -> stack frame size The former is ambiguous (it could mean the full stack size). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular)

MaskRay added inline comments. Comment at: clang/test/Misc/backend-resource-limit-diagnostics.cl:4 -// CHECK: error: local memory limit exceeded (48) in use_huge_lds +// CHECK: error: local memory (48) exceeds limit in function 'use_huge_lds' kernel void use_huge_lds()

[PATCH] D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular)

MaskRay updated this revision to Diff 353510. MaskRay added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104667/new/ https://reviews.llvm.org/D104667 Files: clang/test/Misc/backend-resource-limit-diagnostics.cl clang/test/Mi

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

MaskRay updated this revision to Diff 353519. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Mention the one-time exception for .profraw compatibility Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D104667: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf53d791520d8: Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in… (authored by MaskRay). Repository: rG LLVM Github Mon

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

MaskRay added a comment. In D104556#2836900 , @rnk wrote: > In D104556#2831787 , @MaskRay wrote: > >> Hmm, IMGREL (`IMAGE_REL_AMD64_ADDR32NB`) looks useful and is an alternative >> solution to PC-relative relocat

[PATCH] D104831: [clang] Add x86_64-redhat-linux-gnu as a platform triplet

MaskRay added a comment. > Adding the platform tripplet x86_64-redhat-linux-gnu the while list of > supported x86_64 triplets so that it can be used with the --gcc-toolchain > option to bypass this process and force-pick a given gcc install. Did you set `--target=x86_64-redhat-linux-gnu`? With

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

MaskRay added a comment. In D104556#2837136 , @mstorsjo wrote: > In D104556#2837104 , @rnk wrote: > >> Swift wanted the same thing, so I think the answer is yes, we should ask. > > What would the benefit of that b

[PATCH] D104819: [ADT] Rename StringRef case insensitive methods for clarity

MaskRay added inline comments. Comment at: llvm/include/llvm/ADT/StringRef.h:192 -/// equals_lower - Check for string equality, ignoring case. +/// equals_insensitive - Check for string equality, ignoring case. LLVM_NODISCARD https://llvm.org/docs/

[PATCH] D104831: [clang] Add x86_64-redhat-linux-gnu as a platform triplet

MaskRay added a comment. In D104831#2838835 , @hoy wrote: > In D104831#2837712 , @MaskRay wrote: > >>> Adding the platform tripplet x86_64-redhat-linux-gnu the while list of >>> supported x86_64 triplets so that

[PATCH] D104804: [AMDGPU] Add gfx1035 target

MaskRay added inline comments. Comment at: llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml:189 +# RUN: sed -e 's//64/' -e 's//AMDGCN_GFX1035/' %s | yaml2obj -o %t.o.AMDGCN_GFX1035 +# RUN: llvm-readobj -S --file-headers %t.o.AMDGCN_GFX1035 | FileCheck --check-prefixes=ELF-A

[PATCH] D105909: [clang][CallGraphSection] Add type id metadata to indirect call and targets

MaskRay added a comment. (Two comments I forgot to push yesterday.) Comment at: clang/test/CodeGen/call-graph-section-1.cpp:39 + void f6(const Cls1 *a) {} + + // FT-DAG: define {{.*}} void @_ZN4Cls22f7ER4Cls1({{.*}} !type [[F_TCLS2F7:![0-9]+]] needs a test f

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa1532ed27582: [InstrProfiling] Make CountersPtr in __profd_ relative (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D104

[PATCH] D107145: clangd: Add new semantic token modifier "virtual"

This revision was automatically updated to reflect the committed changes. Closed by commit rG159a26964840: [clangd] Add new semantic token modifier "virtual" (authored by ckandeler, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D58531: [clang] Specify type of pthread_create builtin

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

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

MaskRay added a comment. Ping. - -falign-loops= is currently silently ignored. - -fliang-loops= has user interest from at least x86 and RISC-V. - This patch makes it work for the non-LTO case. I think this is good enough. The LTO case require a function attribute (this doesn't apply to synthesi

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/test/CodeGen/macro-prefix-map.c:1 +// RUN: %clang_cc1 -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | FileCheck %s + T

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

MaskRay added a comment. I can push it on your behalf, I want to wait a bit for more comments. (You need to provide name/email so that you can get the proper attribution `git commit --amend --author='...'`) Looks like a good candidate for release/13.x for build reproducibility. absl has a macro

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

MaskRay added a comment. clang/include/clang/Driver/Options.td HelpText for -fmacro-prefix-map= needs an update. https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html has more documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107393/new/ https://reviews.llvm.org/D107

[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

MaskRay updated this revision to Diff 364224. MaskRay retitled this revision from "[clang] Add -falign-loops=N where N is a power of 2" to "[clang] Implement -falign-loops=N (N is a power of 2) for non-LTO". MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a subs

[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

MaskRay added a comment. In D106701#2924638 , @luismarques wrote: > Still LGTM. > > BTW, I liked that in the old version the help string included "In GCC =0 is > the same as =1". I find that in GCC =0 is not necessarily =1, but not particular clear abo

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

This revision was automatically updated to reflect the committed changes. Closed by commit rG7df405e079c5: Apply -fmacro-prefix-map to __builtin_FILE() (authored by Svenny, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107393/n

[PATCH] D107393: Apply -fmacro-prefix-map to __builtin_FILE()

MaskRay added a comment. Created release/13.x merge request: https://bugs.llvm.org/show_bug.cgi?id=51350 The release manager makes the call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107393/new/ https://reviews.llvm.org/D107393 ___

[PATCH] D107559: [clang] Fix libclang linking on Solaris

MaskRay added inline comments. Comment at: clang/tools/libclang/libclang.map:1 -/* If you add a symbol to this file, make sure to add it with the correct - * version. For example, if the LLVM main branch is LLVM 14.0.0, add new - * symbols with the version LLVM_14. - * On platfo

[PATCH] D107559: [clang] Fix libclang linking on Solaris

MaskRay added a comment. ld.lld, GNU ld, and gold support `/* */`. I am not sure about Solaris ld -z gnu-version-script-compat. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107559/new/ https://reviews.llvm.org/D107559 ___

[PATCH] D107559: [clang] Fix libclang linking on Solaris

MaskRay added inline comments. Comment at: clang/tools/libclang/libclang.map:1 -/* If you add a symbol to this file, make sure to add it with the correct - * version. For example, if the LLVM main branch is LLVM 14.0.0, add new - * symbols with the version LLVM_14. - * On platfo

[PATCH] D107559: [clang] Fix libclang linking on Solaris

MaskRay added inline comments. Comment at: clang/tools/libclang/libclang.map:1 -/* If you add a symbol to this file, make sure to add it with the correct - * version. For example, if the LLVM main branch is LLVM 14.0.0, add new - * symbols with the version LLVM_14. - * On platfo

[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc38efb4899ea: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO (authored by MaskRay). Changed prior to commit: https://reviews.ll

[PATCH] D107610: [AVR][clang] Pass '-fno-use-init-array' to cc1

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

MaskRay added a comment. I know nearly nothing about AVR, but from `avr-gcc a.c '-###'` I doubt this is the GCC behavior. GCC's library path is relative to the GCC installation. `/usr/lib/gcc/avr/5.4.0/../../../avr/lib` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

MaskRay added a comment. Thanks for the test. linux-cross.cpp has some `-SAME: {{^}} ` and `"[[RESOURCE]]/include"` patterns which you may use Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106737/new/ https://reviews.llvm.org/D106737 ___

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

MaskRay added a comment. @benshi001 This was prematurely committed. It has no test and has apparent unaddressed issue. (I revamped many parts in the Linux/Gnu search paths so quite confident about my observation.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D106701: [clang] Implement -falign-loops=N (N is a power of 2) for non-LTO

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4749 + << A->getAsString(Args) << A->getValue(); +else if (Value & Value - 1) + TC.getDriver().Diag(diag::err_drv_alignment_not_power_of_two) craig.topper wrote: >

[PATCH] D107559: [clang] Fix libclang linking on Solaris

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `#` LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107559/new/ https://reviews.llvm.org/D107559 ___

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:460 + if (llvm::sys::fs::is_directory(Path)) +return Optional(Path); + Path = GCCParent + "/../avr"; `return Path;` Comment at: clang/test/Driver/avr-toolchain

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

MaskRay added inline comments. Comment at: clang/test/Driver/avr-toolchain.c:1 // A basic clang -cc1 command-line. MaskRay wrote: > You can UNSUPPORT windows if you don't want to wrestle with \ and / path > separators. UNSUPPORTED: system-windows CHANGES SI

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

MaskRay added inline comments. Comment at: clang/test/Driver/avr-toolchain.c:17 +// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s +// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-inte

[PATCH] D107791: [InlineAdvisor] Add single quotes around caller/callee names

MaskRay created this revision. MaskRay added reviewers: anemet, hoy, tejohnson. Herald added subscribers: ormris, wenlei, steven_wu, hiraditya. MaskRay requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Clang diagnostics refe

[PATCH] D107791: [InlineAdvisor] Add single quotes around caller/callee names

MaskRay added a comment. (I haven't converted directories in tools/gold Transforms/ LTO/, but they should be pretty mechanical.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107791/new/ https://reviews.llvm.org/D107791 __

[PATCH] D107791: [InlineAdvisor] Add single quotes around caller/callee names

MaskRay updated this revision to Diff 365332. MaskRay added a comment. Update inline-replay code and tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107791/new/ https://reviews.llvm.org/D107791 Files: clang/test/CodeGen/thinlto-diagnostic-ha

[PATCH] D107797: [Driver][test] Improve avr-toolchain.c

MaskRay created this revision. MaskRay added reviewers: benshi001, mhjacobson. Herald added subscribers: Jim, dylanmckay. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.or

[PATCH] D107797: [Driver][test] Improve avr-toolchain.c

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb978df4af4c8: [Driver][test] Improve avr-toolchain.c (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

MaskRay added a comment. `--sysroot %S/Inputs/basic_avr_tree/usr/lib` seems incorrect. sysroot is usually a directory which contains `usr/lib` and `lib/`. `somewhere/usr/lib` is not usually used as a sysroot. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

MaskRay added a comment. Seems a new RUN line is needed. If having too many mock trees are inconvenient, consider `clang/unittests/Driver/ToolChainTest.cpp` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107682/new/ https://reviews.llvm.org/D107682

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

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/hexagon-toolchain-linux.c:106 +// CHECK008: InstalledDir: [[INSTALLED_DIR:.+]] +// CHECK008: "-resource-dir" "[[RESOURCE:[^"]+]]" +//

[PATCH] D107791: [InlineAdvisor] Add single quotes around caller/callee names

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG76093b17394a: [InlineAdvisor] Add single quotes around caller/callee names (authored by MaskRay). Herald added a subscriber: emaste. Changed prior t

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

MaskRay added a comment. In D107684#2936034 , @mhjacobson wrote: > In D107684#2936033 , @MaskRay wrote: > >> `--sysroot %S/Inputs/basic_avr_tree/usr/lib` seems incorrect. >> >> sysroot is usually a directory whic

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay created this revision. MaskRay added reviewers: dblaikie, mehdi_amini. MaskRay requested review of this revision. Herald added subscribers: cfe-commits, llvm-commits, aheejin. Herald added projects: LLVM, clang-tools-extra. The variable related top-level readability-identifier-naming optio

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. In D108265#2951222 , @dblaikie wrote: > I think it should probably stay as-is, given this is the documented LLVM > project naming convention: > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumer

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

MaskRay added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1752 + else { +UniqueID = NextUniqueID++; +COMDATSymName = MBB.getParent()->getName(); I think `UniqueID = NextUniqueID++;` is not needed. Com

[PATCH] D108286: [clang][Driver][Linux] fix regression issue when define LIBCXX_LIBDIR_SUFFIX=64

MaskRay added a comment. Please provide more information: the platform name and the output of `g++ empty.cc '-###' |& sed -E 's/ "?-[LiIr]/\n&/g'` Note that there is a FIXME. It's unclear why your platform needs the specific rule. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D108286: [clang][Driver][Linux] fix regression issue when define LIBCXX_LIBDIR_SUFFIX=64

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:307 + if (StringRef(D.Dir).startswith(SysRoot)) { addPathIfExists(D, D.Dir + "/../lib", Paths); +addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths); Is `D.Dir + "/../lib"

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. The number of top-level projects using `VariableName` is smaller than the number of projects not using the style. The top-level variable style just provoked projects to either override the options (flang/, lld/, mlir/) or disable the check. `VariableName` is not even a s

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. In D108265#2953305 , @dblaikie wrote: > In D108265#2952555 , @MaskRay wrote: > >> The number of top-level projects using `VariableName` is smaller than the >> number of projects not using

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. Raised https://lists.llvm.org/pipermail/llvm-dev/2021-August/152210.html ("Top-level .clang-tidy options and VariableName suggestion on CodingStandards") Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://re

[PATCH] D80392: [mips][mc][clang] Use pc-relative relocations in .eh_frame

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. `MCTargetOptionsCommandFlags.cpp` may be a better place for the internal cl::opt option. Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:30 +cl::opt +MipsPC64Re

[PATCH] D80392: [mips][mc][clang] Use pc-relative relocations in .eh_frame

MaskRay added a comment. If you want to do this proper, you may look at how -fbinutils-version= was implemented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80392/new/ https://reviews.llvm.org/D80392

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

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

[PATCH] D105148: [CMake] Don't use -Bsymbolic-functions for MinGW targets

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. > but I don't know if the solaris linker supports -Bsymbolic-functions or not. According to a Solaris "man pages section 1: User Commands", -Bsymbolic-functions is supported. Repository:

[PATCH] D105091: [RISCV] Pass -u to linker correctly.

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. > -u is a linker option used to pretend a symbol is undefined, this option are > common used for force pull-in weak symbol. for forcing archive member extraction. weak symbols are

[PATCH] D105254: [RISCV] Support machine constraint "S"

MaskRay created this revision. Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, jo

[PATCH] D105254: [RISCV] Support machine constraint "S"

MaskRay updated this revision to Diff 355749. MaskRay marked 6 inline comments as done. MaskRay added a comment. scrub labels/comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105254/new/ https://reviews.llvm.org/D105254 Files: clang/lib/Ba

[PATCH] D105254: [RISCV] Support machine constraint "S"

MaskRay added inline comments. Comment at: llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll:24 +; RV64-NEXT:ret +entry: + %0 = tail call i8* asm "lui $0, %hi($1)\0Aaddi $0,$0,%lo($1)", "=r,S"(i32* nonnull @var) jrtc27 wrote: > Label isn't needed Omitting

[PATCH] D105254: [RISCV] Support machine constraint "S"

MaskRay added a comment. In D105254#2851889 , @jrtc27 wrote: > Hm, AArch64 handles ExternalSymbolSDNode too, but I don't see how you could > ever end up with one... It can't and the code path is untested. I already deleted it from aarch64. Repository:

[PATCH] D105091: [RISCV] Pass -u to linker correctly.

MaskRay added inline comments. Comment at: clang/test/Driver/riscv-args.c:5 // RUN: %clang -### -target riscv32 \ // RUN: --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-LD %s MaskRay wrote: > Just place

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

MaskRay added a comment. In GCC -Wstack-usage= is more powerful. It can check `warning: stack usage might be unbounded`. If this is currently just an alias, does it address some application pain? If we don't actually implement the -Wstack-usage= functionality, I'd hope that applications know t

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

MaskRay updated this revision to Diff 356271. MaskRay edited the summary of this revision. MaskRay added a comment. Drop version 5 compatibility because the Linux kernel patch is suspended. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104556/new/

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

MaskRay updated this revision to Diff 356310. MaskRay marked an inline comment as done. MaskRay added a comment. Move ConstantExpr::getSub from .inc to InstrProfiling.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104556/new/ https://reviews.llv

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

MaskRay added a comment. @davidxl @vsk 😃 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104556/new/ https://reviews.llvm.org/D104556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

MaskRay added subscribers: rampitec, arsenm. MaskRay added a comment. This is great. unsigned long foo(unsigned long addr, unsigned long a0, unsigned long a1, unsigned long a2, unsigned long a3, unsigned long a4, unsigned long a5) {

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

MaskRay added inline comments. Comment at: clang/tools/libclang/CMakeLists.txt:82 + +if (LLVM_EXPORTED_SYMBOL_FILE) + add_custom_command(OUTPUT ${LLVM_EXPORTED_SYMBOL_FILE} What does this do? A hard-coded list cannot catch up with the real dynamic symbol list.

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

MaskRay added a comment. In D102782#2862065 , @rsanthir.quic wrote: > @MaskRay Yes this would unblock applications. Regarding your concern, the > information from this implementation as well as GCC's should be used > conservatively as both are approxim

[PATCH] D105091: [RISCV] Pass -u to linker correctly.

MaskRay added inline comments. Comment at: clang/test/Driver/riscv-args.c:5 // RUN: %clang -### -target riscv32 \ // RUN: --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-LD %s kito-cheng wrote: > MaskRay

[PATCH] D105254: [RISCV] Support machine constraint "S"

MaskRay added a comment. In D105254#2871278 , @luismarques wrote: > In D105254#2852489 , @luismarques > wrote: > >> Makes sense. Let's wait for the GCC Bugzilla feedback. > > With 'S' now documented on the GNU s

[PATCH] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm

MaskRay created this revision. MaskRay added reviewers: debug-info, dblaikie, osandov. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. While GNU as only allows the directory form of the .file directive for DWARF v5, the integrated p

[PATCH] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm

MaskRay added a comment. In D105835#2872362 , @dblaikie wrote: > Not sure I quite follow - is this an alternative to/replacement for D105662 > ? Is the intent to revert D105662 > after

[PATCH] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm

MaskRay added a comment. In D105835#2872393 , @osandov wrote: > I tested my reproducer and it also fixes it, thanks. Should it be an error to > specify `-fno-dwarf-directory-asm` together with `-gdwarf-5`, since that > produces incorrect results? I tho

[PATCH] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG51fc742ce716: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm (authored by MaskRay). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D105254: [RISCV] Support machine constraint "S"

This revision was automatically updated to reflect the committed changes. Closed by commit rG3d89fb4d13bc: [RISCV] Support machine constraint "S" (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D105254?vs=355749&id=358308#toc Repository: rG LLVM Github Monorepo CHAN

[PATCH] D105091: [RISCV] Pass -u to linker correctly.

MaskRay added a comment. The update was not what expected. I said that there should be just one RUN line and you can place -T, -u, --defsym and all linker options on that line. Each new RUN line consumes some resources on testing machines. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. How is this different from `clang::driver::Driver::SysRoot`? From `SysrootName + "/sys-root"` looks like the `sys-root` directory is the sysroot? Perhaps there is a better way describing `Sy

[PATCH] D112387: [clang] [MinGW] Rename the 'Arch' member to 'SysrootName'. NFC.

MaskRay added a comment. Making the word `Subdir` part of the variable name LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112387/new/ https://reviews.llvm.org/D112387 ___ cfe-commits mailing list cfe

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

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

<    13   14   15   16   17   18   19   20   21   22   >