[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-04-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Glad you took another look. I don't want to yield, let's find another reviewer :) Comment at: clangd/FuzzyMatch.cpp:230 void FuzzyMatcher::buildGraph() { + Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss}; for (int W = 0; W < WordN; ++W) { ---

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In https://reviews.llvm.org/D42893#1053754, @jbcoe wrote: > Can you add some simple tests? Where should I add tests? I cannot find tests for other clang_File_* functions. Repository: rC Clang https://reviews.llvm.org/D42893 __

[PATCH] D45285: [clangd-vscode] Update VScode dependencies

2018-04-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Do we really want to keep editor plugins in the repository? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45285 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. There is no unittest but I have verified with a C++ language server on Arch Linux. Without using this function, cquery/ccls will not be able to index system header files because of excessive .. in the path returned by clang_getFileName. Repository: rC Clang https:/

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 141492. MaskRay added a comment. Rebase Repository: rC Clang https://reviews.llvm.org/D42893 Files: include/clang-c/Index.h tools/libclang/CIndex.cpp tools/libclang/libclang.exports Index: tools/libclang/libclang.exports =

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 141496. MaskRay added a comment. Add unittests/libclang/LibclangTest.cpp test Repository: rC Clang https://reviews.llvm.org/D42893 Files: include/clang-c/Index.h tools/libclang/CIndex.cpp tools/libclang/libclang.exports unittests/libclang/Libclan

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Done. ninja -C ~/Dev/llvm/debug unittests/libclang/libclangTests ~/Dev/llvm/debug/tools/clang/unittests/libclang/libclangTests Repository: rC Clang https://reviews.llvm.org/D42893 ___ cfe-commits mailing list cfe-com

[PATCH] D42893: [libclang] Add clang_File_tryGetRealPathName

2018-04-07 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329515: [libclang] Add clang_File_tryGetRealPathName (authored by MaskRay, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42893?vs=141496&id=

[PATCH] D145999: [RISCV] Reserve X18 by default for Android

2023-03-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/test/Driver/riscv-fixed-x-register.c:343 + +// Check that x18 is reserved on Android by default +// RUN: %clang --target=riscv64-linux-android -### %s 2> %t asb wrote: > samitolvanen

[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

2023-03-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:108 case llvm::Triple::aarch64: [[fallthrough]]; + case llvm::Triple::riscv64: Remove `[[fallthrough]]` and just list the 3 `case` consecutively. Comment

[PATCH] D143587: [Docs] Multilib design

2023-03-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/docs/Multilib.rst:241 + +multilib.yaml and -print-multi-selection-flags-experimental are new interfaces +to Clang. In order for them to be usable over time and across LLVM versions A

[PATCH] D146054: [RISCV] Add -print-supported-marchs and -march=help support

2023-03-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/print-supported-marchs.c:17 +// CHECK-RISCV:i 2.0 +// CHECK-RISCV:e 1.9 +//

[PATCH] D146603: [docs] Document -fomit-frame-pointer

2023-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: aabhinavg, vitalybuka. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146603 Fil

[PATCH] D145840: [Docs] Added -fomit-frame-pointer and -fno-omit-frame-pointer flag documentation

2023-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry for my belated response as I was on a trip for quite a few days. We don't need to document the negative form. "Produce better stack traces," is not necessarily correct. See https://maskray.me/blog/2020-11-08-stack-unwinding I created D146603

[PATCH] D146603: [docs] Document -fomit-frame-pointer

2023-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 507220. MaskRay added a comment. . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146603/new/ https://reviews.llvm.org/D146603 Files: clang/include/clang/Driver/Options.td Index: clang/include/clang/Driver/

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

2023-03-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. "Close" seems more appropriate to me as this isn't a bug (previously not an intended use case). Comment at: clang/test/Driver/avr-ld.c:59 +// LINKS: {{".*ld.*"}} {{.*}} "-

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

2023-03-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D145646#4213547 , @gulfem wrote: > We started seeing a test failure in `avr-ld.c`. > > /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c:48:11: error: > // LINKP: {{".*ld.*"}} {{.*}} "--defsym=__DATA_REGION_ORIGIN

[PATCH] D146155: [clang][NFC] Fix location of 2>&1 in a few -print tests

2023-03-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you! (A somewhat common practice is to prefer `[test]` over `[NFC]` for pure-test changes (no code change) :) I see that " in a few -print tests" in the subject may render `[test]` unneeded.. perhaps what tag is used isn't really important.) Repository: rG LLV

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

2023-03-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:58 const auto *RVFI = MF.getInfo(); if (RVFI->useSaveRestoreLibCalls(MF)) { paulkirth wrote: > craig.topper wrote: > > paulkirth wrote: > > > craig.topper wrote: > > >

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

2023-03-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125 + // `-mroptr` implies the `-bforceimprw` linker option. + // The `-mroptr` option places constants in RO sections as much as possible. Delete `// `-mroptr` implies the `-bforc

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

2023-03-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ppc-roptr.c:1 +// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | \ +// RUN: FileCheck %s --check-prefixes=ROPTR,LINK Prefer `--target=` to `-target ` for new tests. =

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

2023-03-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This change is correct for Linux. `llvm/CMakeLists.txt` says: if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390") set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON) Some rpath using OSes (notably macOS) use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default=OFF. Is the rpath

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

2023-03-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D146686#4216612 , @yaxunl wrote: > In D146686#4215577 , @MaskRay wrote: > >> This change is correct for Linux. `llvm/CMakeLists.txt` says: >> >> if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linu

[PATCH] D146603: [docs] Document -fomit-frame-pointer

2023-03-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 507792. MaskRay added a comment. Adjust some terms. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146603/new/ https://reviews.llvm.org/D146603 Files: clang/include/clang/Driver/Options.td Index: clang/incl

[PATCH] D146603: [docs] Document -fomit-frame-pointer

2023-03-23 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. MaskRay marked 2 inline comments as done. Closed by commit rG5f883cdbfbe2: [docs] Document -fomit-frame-pointer (authored by MaskRay). Changed prior to commit: https

[PATCH] D146427: [clang-tools-extra] Fix linking ClangdTests when using libclang-cpp

2023-03-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. (back from a long vacation with little time spending on reviews.llvm.org ) I have verified that `tools/clang/tools/extra/clangd/unittests/ClangdTests` does not link in `-DLLVM_LINK_LLVM_DYLI

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

2023-03-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The new test can be placed in `arch-specific-libdir-rpath.c`. One or two additional RUN lines seem sufficient, no need to duplicate too many. Also, use `--target=` for new tests and avoid `^//$`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146686/new/ https:

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

2023-03-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/OHOS.cpp:140 getFilePaths().clear(); - std::string CandidateLibPath = getArchSpecificLibPath(); - if (getVFS().exists(CandidateLibPath)) -getFilePaths().push_back(CandidateLibPath); + for (const auto

[PATCH] D147030: [Clang][Driver] Default Generic_GCC::IsIntegratedAssemblerDefault to true

2023-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2924 + default: +if (getTriple().getVendor() == llvm::Triple::Myriad) + return false; `return getTriple().getVendor() != llvm::Triple::Myriad;` Repository: rG LLVM Github

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

2023-03-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Please update the commit message and comment about what binutils versions reject the construct. Comment at: clang/test/Driver/gcc_forward.c:49 +// DEBUG-NOT: "-gdwarf-4" \ No newline at end of file No newline at end of file Please fi

[PATCH] D140925: [CMake] Use Clang to infer the target triple

2023-02-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D140925#4151524 , @smeenai wrote: > Ping @ldionne, would you be able to take a look at this soon (or are you okay > waiving the libc++ blocking requirement for it)? This is really useful for > Android armv7, where the triple

[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit Power10 instructions from stubs

2023-02-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added a project: All. The used code sequence has 2 bugs so the output will crash. I fixed them in 7198c87f42f6c15d76b127c1c63530e9b4d5dd39 and added my notes to https://maskray.me/blog/2023-02-

[PATCH] D144853: [Clang][RISCV] Add CMake options to configure default CPU

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I object to this change which further complicates the build system and makes the difference difficult to observe by users. If your `clang` executable is at `bin/clang`, just create `bin/riscv64-unknown-linux-gnu.cfg` with `-mcpu=xxx`. You may configure `LLVM_DEFAULT_TARG

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Sorry I just saw this but I am not sure this is a good idea. Why can't the user use `--print-supported-cpus` instead? The additional alias doesn't seem useful. If you can make GCC add this as well, it will be different. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. FWIW I think this should be reverted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144914/new/ https://reviews.llvm.org/D144914 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D144914: [Clang][Driver] Add -mcpu=help to clang

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Since `llc` has `-mcpu=help`, it seems fine to have `clang -mcpu=help`. I made a bad suggestion there about `-mcpu=?`. Perhaps I should revert de94ac9357750fdba45e09eefa8f67a650ae6a64 . This `-mcpu=help` patch is good. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D136940: [clang][Driver] allow tilde in user config dir

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/config-file3.c:227 +// +// RUN: HOME=%S/Inputs/config %clang --config-user-dir=~ -v 2>&1 | FileCheck %s -check-prefix CHECK-TILDE +// CHECK-TILDE: User configuration file directory: {{.*}}/Inputs/config ---

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:80 -return llvm::StringSwitch(CPUName) -.Case("common", "generic") -.Case("440fp", "440") -.Case("630", "pwr3") -.Case("G3", "

[PATCH] D144533: [clang][driver] Do not emit default '-Tdata' for AVR devices

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. This change is an improvement but why is `__DATA_REGION_ORIGIN__` defined? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144533/new/ https://reviews.llvm.org/D144533 ___ cfe-comm

[PATCH] D144878: __builtin_FILE_NAME()

2023-02-28 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. It'd be nice to have compiler feature parity. I created a GCC feature request for `__builtin_FILE_NAME`: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108978 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144878/new/ https:/

[PATCH] D144934: [clang] drop buggy use of `-serialize-diagnostics` flag

2023-03-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. For pure test updates, best to include `[test] `in the subject :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144934/new/ https://reviews.llvm.org/D144934 ___ cfe-commits maili

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

2023-03-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/ibm-profiling.c:1 +// RUN: %clang -### 2>&1 \ +// RUN:--target=s390x-none-zos \ This is too verbose. Driver tests prefer packing many options on one line. The decreased number of lines actually

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

2023-03-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6342 +A->claim(); +if (TC.getTriple().isOSAIX()) { + if (!Args.hasArgNoClaim(options::OPT_pg)) Use `&&` Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D145141: [Driver] Reject -march= for ppc

2023-03-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: PowerPC, nemanjai. Herald added subscribers: shchenz, fedor.sergeev, kbarton, jyknight. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang -marc

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

2023-03-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D144967#4164858 , @nathanchance wrote: > Could this be merged into `main` and backported to `release/16.x`? If this > makes 16.0.0 final, I think the kernel can avoid working around this issue > altogether, as `-mtune` was o

[PATCH] D145164: [clang][RISCV] Enable -fasynchronous-unwind-tables by default on Linux

2023-03-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LG since GCC made a similar change and this matches several major architectures on Linux/other ELF based operating systems. (At heart I think `-fasynchronous-unwind-tables` is not a good def

[PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

2023-03-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D138539#4164313 , @steakhal wrote: > Oh, I forgot to mention why this change breaks the equality operator of > `llvm::StringSet`. It's because the `StringMap::operator==` will try to > compare the `value` as well, which is of

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

2023-03-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > the programmer is indicating the function has some special purpose, and is > not expected to be garbage collected. There are use cases that GC semantics are desired. They may place the address function in a variable. I don't think changing the behavior by default is f

[PATCH] D145141: [Driver] Reject -march= for ppc

2023-03-06 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7370b9c8ea00: [Driver] Reject -march= for ppc (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D145141?vs=501759&id=502686#toc Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D145393: [HIP] Make `--offload-add-rpath` alias of `-frtlib-add-rpath`

2023-03-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Seems fine. Should we eventually remove `--offload-add-rpath` and `-fopenmp-implicit-rpath`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145393/new/ https://reviews.llvm.org/D145393 ___ cfe-commits mailing list cfe

[PATCH] D145391: [HIP] Supports env var HIP_PATH

2023-03-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/rocm-detect.hip:42 + +// RUN: rm -rf %T/myhip +// RUN: rm -rf %T/myhip_nouse `%T` is not recommended. https://llvm.org/docs/CommandGuide/lit.html "parent directory of %t (not unique, deprecated, do not

[PATCH] D142174: [OpenMP] Don't set rpath for system paths

2023-03-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D142174#4171731 , @JonChesterfield wrote: > I'm happy with this but agree that "what might be a system path?" is a tricky > heuristic. What we want is to exclude the places that the application will > search anyway, but that

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

2023-03-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The inconsistency stem from the fact that with the default `.text`, the compiler may generate `.text.$suffix` if `-ffunction-sections` is specified or if a COMDAT is needed. While with an explicit section, there is no such suffix appending mechanism used together with `

[PATCH] D118493: Set rpath on openmp executables

2023-03-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D118493#4178250 , @JonChesterfield wrote: > Reporting after another round of discussion. I don't have much support from > the llvm openmp working group that we should maintain the current divergence > from libc++ and the lik

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

2023-03-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. In D144179#4166909 , @mgabka wrote: > In D144179#4146599 , @MaskRay wrote: > >> This looks like i

[PATCH] D145567: [Driver] Rename multilib flags to tags

2023-03-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Since the touched code is still in review, you need to merge the changes into the prior patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145567/new/ https://reviews.llvm.org/D145567 _

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-03-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay abandoned this revision. MaskRay added a comment. Abandoned by a revert of D118493 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143306/new/ https://reviews.llvm.org/D143306 __

[PATCH] D118493: Set rpath on openmp executables

2023-03-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D118493#4182583 , @JonChesterfield wrote: > Duplicating a comment from the commit thread so it's easier for me to find > later. > > You've applied this to the release branches going back as far as 14. It's a > user facing br

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

2023-03-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > Due to this we started seeing assembler errors with certain .c and .cpp files > - > "Error: file number 1 already allocated" What are the certain `.c` and `.cpp` files? The behavior is correct for the following two commands. clang --target=arm-linux-gnueabihf -fno-

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

2023-03-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D143306#4183759 , @tstellar wrote: > Should we apply this patch to the release/16.x branch to create a more smooth > transition since the option has been removed in main? Yes, it will be a good idea to apply this to `release/

[PATCH] D145146: [Driver][NFC] Remove some redundant code in Driver.cpp.

2023-03-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `computeTargetTriple` has several users. You probably want to check all these code paths do not need `if (const Arg *A = Args.getLastArg(options::OPT_target))`. For the uses in `BuildCompila

[PATCH] D145848: [Driver] Correct -f(no-)xray-function-index behavior

2023-03-11 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/test/CodeGen/xray-function-index.cpp:1 +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple x86_64-unkn

[PATCH] D145848: [Driver] Correct -f(no-)xray-function-index behavior

2023-03-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/test/CodeGen/xray-function-index.cpp:1 +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -triple x86_64-unknown-li

[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images

2023-03-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. While `--no-undefined` is an alias for `-z defs`, be consistent in the subject and the body. > Always use -zdefs `--no-undefined` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145849: [Driver][xray] Allow XRay on Apple Silicon

2023-03-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/XRay/xray-instrument-macos.c:1 +// RUN: %clang -o /dev/null -v -fxray-instrument -target aarch64-apple-darwin20 -c %s // RUN: %clang -o

[PATCH] D145725: [Driver] Make -X default for baremetal riscv

2023-03-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145725/new/ https://reviews.llvm.org/D145725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

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

2023-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. There is a `-fno-fat-lto-objects` issue, but otherwise looks good after some nits are addressed. Thanks! Comment at: clang/docs/ReleaseNotes.rst:250 +- `-ffat-lto-objects` can now be used to emit object files with both

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

2023-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:250 +- `-ffat-lto-objects` can now be used to emit object files with both object + code and bitcode. Previously this flag was ignored for GCC compatibility. MaskRay wrote: > `bitcode` => `

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

2023-06-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7298 + if (IsUsingLTO && Args.getLastArg(options::OPT_ffat_lto_objects)) { +assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin); +CmdArgs.push_back(Args.MakeArgString( We a

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Looking Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152785/new/ https://reviews.llvm.org/D152785 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/test/MC/ARM/thumb-fixups.s:1 +@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding > %t +@ RUN: FileCheck < %t %s We normally just pipe the output to FileCheck, e.g. `| Fi

[PATCH] D145849: [Driver][xray] Allow XRay on Apple Silicon

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D145849#4430870 , @ilammy wrote: > In D145849#4412286 , @MaskRay wrote: > >> However, `Triple.isMacOSX()` is allowed before the feature actually works >> and `compiler-rt/test/xray/lit

[PATCH] D145849: [Driver][xray] Allow XRay on Apple Silicon

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D145849#4436623 , @MaskRay wrote: > In D145849#4430870 , @ilammy wrote: > >> In D145849#4412286 , @MaskRay >> wrote: >> >>> However, `Triple.i

[PATCH] D145849: [Driver][xray] Allow XRay on Apple Silicon

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd5bebb3fb402: [Driver] Allow XRay on Apple Silicon (authored by ilammy, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D145849?vs=530285&id=533116#toc Repository: rG LLVM Gi

[PATCH] D152661: [XRay] Make xray_fn_idx entries PC-relative

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 533131. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. fix clang/test/CodeGen/xray-function-index.c Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D152661: [XRay] Make xray_fn_idx entries PC-relative

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Tested on aarch64-linux-gnu and powerpc64le-linux-gnu. I'll use `createLinkerPrivateSymbol` instead of `createLinkerPrivateTempSymbol` since the symbol is not considered `IsTemporary` by MC... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D152661: [XRay] Make xray_fn_idx entries PC-relative

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe0a6561ec9ec: [XRay] Make xray_fn_idx entries PC-relative (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D152661?vs=5331

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3898 if (!Args.hasArg(options::OPT_dumpdir)) { + Arg *FinalOutput = Args.getLastArg(options::OPT_o, options::OPT__SLASH_o); Arg *Arg = Args.MakeSeparateArg( Add a test to sho

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/test/Driver/split-debug.c:55 // RUN: %clang -### -c -target x86_64 -gsplit-dwarf=single -g -o %tfoo.o %s 2>&1 | FileCheck %s --check-prefix=SINGLE_WITH_FILENAME +// RUN: %clang_cl -### -c --target x86_64-unknown-windows-msvc -gs

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:1167 // GCC style -dumpdir. We intentionally don't implement the less useful -dumpbase{,-ext}. -def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option]>, +def dumpdir : Separate<["-"], "dumpdir

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:1167 // GCC style -dumpdir. We intentionally don't implement the less useful -dumpbase{,-ext}. -def dumpdi

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. CodeView is the default debug info format for COFF. Perhaps `-gsplit-dwarf` should be rejected when the default `-gcodeview` is used, with a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152785/new/ https://reviews.l

[PATCH] D152671: [doc][LoongArch] Add missed release note about `ual` feature addition

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: llvm/docs/ReleaseNotes.rst:164 +* An target feature ``ual`` is introduced to allow unaligned memory accesses and + this feature is enabled by default on g

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1896 +// Each section in COFF can directly contain relocations. +if (isa(&Obj) && Section.relocations().empty()) + continue; HaohaiWen wrote: > skan wrot

[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Tests seem mostly fine, but since this is the primary patch for a major feature, consider adding more descriptions how this works? For example, you can add something like the following to the summary/commit message, which should make a curious reader know how to play wi

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

2023-06-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Seems that the main change is in `llvm/include/llvm/Support/Error.h`. I think there is a small but not negligible chance that this patch may cause a breakage for some build configurations. I

[PATCH] D117929: [XRay] Add support for RISCV

2023-06-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Herald added subscribers: wangpc, luke, Enna1, shiva0217, arichardson. Herald added a project: All. I am still interested in a RISC-V XRay port :) Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:280 + // Emit NOP instructions + for (int8_t I = 0

[PATCH] D117929: [XRay] Add support for RISCV

2023-06-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: compiler-rt/lib/xray/xray_trampoline_riscv32.S:16 + .text + .file "xray_trampoline_riscv32.S" + .globl __xray_FunctionEntry Omit `.file` Consider using `#include "../sanitizer_common/sanitizer_asm.h"`

[PATCH] D125272: [clang] Add -fcheck-new support

2023-06-23 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG52c8f0bb20eb: [clang] Add -fcheck-new support (authored by heatd, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D125272

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

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

[PATCH] D153699: [clang] Fix pretty-printing for variables declared in a for-loop condition

2023-06-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! `Node->getConditionVariableDeclStmt()` is how `StmtPrinter::VisitWhileStmt` prints the declaration in the condition. https://reviews.llvm.org/D102502 added `PCH/for-loop-init-ternar

[PATCH] D153699: [clang] Fix pretty-printing for variables declared in a for-loop condition

2023-06-25 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdf8d6d95ca64: [clang] Fix pretty-printing for variables declared in a for-loop condition (authored by vaithak, committed by MaskRay). Repository:

[PATCH] D153707: [Clang][LoongArch] Consume and check -mabi and -mfpu even if -m*-float is present

2023-06-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:732 +def warn_drv_loongarch_conflicting_mabi : Warning< + "the -mabi setting '%0' conflicts wi

[PATCH] D153707: [Clang][LoongArch] Consume and check -mabi and -mfpu even if -m*-float is present

2023-06-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:732 +def warn_drv_loongarch_conflicting_mabi : Warning< + "ignoring -mabi value '%0' as it conflicts with that implied by -m*-float (%1)">, + InGroup; You can make `-

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

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

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

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

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

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

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

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfe5bab537270: [Driver][ARM] Warn about -mabi= for assembler input (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D153691

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

2023-06-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:20 + +void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst, +Environment &Env) { See https://llvm.org/docs/CodingStandards.html#use-

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

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

<    26   27   28   29   30   31   32   33   34   35   >