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

jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:1-3 +; RUN: llc -verify-machineinstrs -o - %s -mtriple=riscv32-linux-gnu -mattr=+reserve-x18 | FileCheck

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

jrtc27 added a comment. Is there a reason for choosing X18? On AArch64 that's either a temporary or saved register depending on ABI, but determined to be the "platform register". Picking X18 on RISC-V because that's the same index as AArch64 seems a little arbitrary, but maybe it happens to mak

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

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. 1. Please use local variables with meaningful names for `RISCV::Xn` rather than inlining them everywhere and making it harder at a glance to work out what's going on without knowing

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

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:2 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \ +; RUN: | FileCheck %s

[PATCH] D78777: [AST] Use PrintingPolicy for format string diagnosis

jrtc27 created this revision. jrtc27 added reviewers: rsmith, Anastasia. Herald added subscribers: cfe-commits, arichardson. Herald added a project: clang. This is a small improvement for OpenCL diagnostics, but is also useful for our CHERI fork, as our __capability qualifier is suppressed from di

[PATCH] D78777: [AST] Use PrintingPolicy for format string diagnosis

This revision was automatically updated to reflect the committed changes. Closed by commit rG5fee6936b8b2: [AST] Use PrintingPolicy for format string diagnosis (authored by jrtc27). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78777/new/ https://re

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

jrtc27 added a comment. There is a possibly-compelling argument against using x18: RV32E only gives x0-x15, so would not be able to support the current implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84414/new/ https://reviews.llvm.o

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

jrtc27 added a comment. If changes like this are required for all these tests this is going to be a complete pain for downstream forks like ours. At the very least, make whatever script you used to update these public, as I don't want to have to recreate it from scratch when merging this in. I

[PATCH] D131141: [RISCV] Add MC support of RISCV Zcb Extension

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:169 + +let isCompressOnly = true in { + This feels wrong to me, but decompression is a bit dodgy if you can have all of Zcb without some of the extensions that its instructions d

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

jrtc27 added a comment. In D132843#3770936 , @efriedma wrote: >> So we need to keep ABI in somewhere and read that at LTO phase, the most >> ideal place is the module flags. We already did that[6], but that comes with >> a problem is it's too late to up

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:709 + +let Predicates = [HasStdExtZawrs] in { +def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">, This doesn't really belong here, but a separate RISCVInstrInfo

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

jrtc27 added a comment. GCC only ever defines __riscv_32e Comment at: clang/lib/Basic/Targets/RISCV.cpp:210 +if (Is64Bit) + Builder.defineMacro("__riscv_64e"); +else Ugh, these don't align with the normal pattern. __riscv_e already exists in the s

[PATCH] D70401: [RISCV] CodeGen of RVE and ilp32e/lp64e ABIs

jrtc27 added a comment. In D70401#4657098 , @jrtc27 wrote: > GCC only ever defines __riscv_32e Hm, seems the comments about __riscv_32e were from months ago, ignore them if they aren't correct or have become outdated... Repository: rG LLVM Github Mon

[PATCH] D128612: RISC-V big-endian support implementation

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.h:113 +if (Triple.isLittleEndian()) + resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128"); +else And please avoid repeating the whole data layout, just make the e/E a variable ===

[PATCH] D128726: [RISCV][NFC] Move static global variables into static variable in function.

jrtc27 added a comment. The commit message is poor, because the problem is not the use of static globals, it's the use of globals that have constructors. Moving them to be static function-scoped variables doesn't change the fact that they still have static duration; you change the scope of them

[PATCH] D128726: [RISCV][NFC] Move static global variables into static variable in function.

jrtc27 added a comment. Seems you're right, the C++11 standard does clearly say (9.7p4): > Dynamic initialization of a block-scope variable with static storage duration > (6.6.4.1) or thread storage duration (6.6.4.2) is performed the first time > control passes through its declaration; such a

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain`

jrtc27 added a comment. Commit subject/body should say C++ in it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129065/new/ https://reviews.llvm.org/D129065 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

jrtc27 added a comment. As a developer who often deals with crashes locally this is more annoying; currently I can just point tools at the shell script and C file in /tmp and let them go to work reducing, but now I have to also extract the files CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D122556: [RISCV] Add definitions for Xiangshan processors.

jrtc27 added inline comments. Comment at: llvm/include/llvm/Support/RISCVTargetParser.def:34 PROC(SIFIVE_U74, {"sifive-u74"}, FK_64BIT, {"rv64gc"}) +PROC(XIANGSHAN_YANQIHU,{"xiangshan-yanqihu"},FK_64BIT,{"rv64gc"}) +PROC(XIANGSHAN_NANHU,{"xiangshan-nanhu"},FK_64BIT,{"rv64imafdc_

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added inline comments. Comment at: clang/test/Driver/riscv-arch.c:410 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s +// RUN: %clang --target=riscv32-unknown-elf -mcpu=sifive-s21 -### %s \ +// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TAR

[PATCH] D43630: [Driver] Fix search paths on x32

jrtc27 abandoned this revision. jrtc27 added a comment. Herald added a subscriber: pengfei. Herald added a project: All. Superseded by D52050 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43630/new/ https://reviews.llvm.org/D4363

[PATCH] D105959: Use ManagedStatic and lazy initialization of cl::opt in libSupport to make it free of global initializer

jrtc27 added a comment. It's just a standard linker set; the static linker combines all the input sections containing the array elements into a single output section. It's similar to init_array etc, just without the special meaning (i.e. you have to write the code to iterate over the array and

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

jrtc27 created this revision. jrtc27 added reviewers: arichardson, jdoerfert. jrtc27 requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. ClassTemplateSpecializationDecl not within a ClassTemplateDecl represents an explicit ins

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

jrtc27 updated this revision to Diff 359643. jrtc27 added a comment. Drop the --llvm-bin test; only basic-cplusplus.test does that (which happened to be the one I used as a reference), and that only needs to be done for one file as it has no relation to the input. Repository: rG LLVM Github

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

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

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

jrtc27 added a comment. This is not meant to be an -O0.5, this is meant to be an -Oepsilon. I don't want optimised code, I just want code that I can actually disassemble and understand without having to trawl through a mess of stack spills and loads. This is for debugging really basic bugs (eit

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

jrtc27 added a comment. In D105516#2889411 , @efriedma wrote: > The part I'm most uncomfortable with is sticking "mem2reg" in a public, > documented driver option. I don't want to promise that the mem2reg pass will > exist forever. We should be focuse

[PATCH] D106347: [PoC][RISCV] Encode arch information in a new module flag meatadata 'riscv-isa-bits'.

jrtc27 added a comment. Why can't we just save target-features itself as a module flag instead of inventing yet another equivalent encoding? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106347/new/ https://reviews.llvm.org/D106347 __

[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/riscv_vector.td:555 +defvar Xlen32EEWList = [["8", "(Log2EEW:3)"], +["16", "(Log2EEW:4)"], Ignoring whether the change is actually correct, this should be capitalised as

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

jrtc27 added inline comments. Comment at: clang/test/Driver/riscv-abi.c:68 -// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d -mabi=lp64d 2>&1 \ +// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64ifd -mabi=lp64d 2>&1 \ // RUN: | Fi

[PATCH] D106347: [PoC][RISCV] Encode arch information in a new module flag meatadata 'riscv-isa-features'.

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:835 +llvm::RISCVISAInfo::filterISAStrings(Features); +std::vector Ops; +if (Features.empty()) { Why is this building a list? Just use a string so it's in the same format as

[PATCH] D106347: [PoC][RISCV] Encode arch information in a new module flag meatadata 'riscv-isa-features'.

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/riscv-isa-features.ll:1 +; RUN: llc -o - < %s | FileCheck %s +; -mattr option would overwrite target-feature and module flag riscv-isa-features Use update_llc_test_checks for these.

[PATCH] D106347: [PoC][RISCV] Encode arch information in a new module flag meatadata 'riscv-isa-features'.

jrtc27 added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-metadata-isa-features-empty-target-feature.cpp:9 +// We need to record extension target-feature in riscv-isa-features module flag metadata because there is some empty target-features attribute +// CHECK: !

[PATCH] D95588: [RISCV] Implement the MC layer support of P extension

jrtc27 added a comment. I can't help but feel the assembly syntax for the register pair instructions shouldn't include both registers (perhaps in curly braces). The implicit use of the other register when reading the source is rather ugly, and particularly hard to remember when the RV64 version

[PATCH] D95588: [RISCV] Implement the MC layer support of P extension

jrtc27 added a comment. Oh, technically none of the clang changes belong in this patch. Those are for the Clang driver and preprocessor, not the MC layer which is purely llvm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95588/new/ https://review

[PATCH] D105555: [RISCV][Clang] Compute the default target-abi if it's empty.

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:235 +bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64; +ABI = llvm::RISCV::computeDefaultABIFromArch(ISAInfo, Is64Bit).str(); + } The ISAInfo includes XLen ===

[PATCH] D105555: [RISCV][Clang] Compute the default target-abi if it's empty.

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:233 + // Use the explicitly target-feature to compute default ABI. + if (getABI().empty()) { +bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64; I don't think using the

[PATCH] D105555: [RISCV][Clang] Compute the default target-abi if it's empty.

jrtc27 added inline comments. Comment at: llvm/lib/Support/TargetParser.cpp:341-344 + } + if (ISAInfo.hasExtension("d")) +return "lp64d"; + return "lp64"; makes me feel more comfortable inside, and also is a bit more like the old code where we did explici

[PATCH] D105555: [RISCV][Clang] Compute the default target-abi if it's empty.

jrtc27 added a comment. The commit message needs rewriting to reflect the final patch. Also, "Explicitly target-abi encoded in IR is clear than empty target-abi." is wrong, it's not about clarity, it's about robustness and correctness. Comment at: clang/lib/Basic/Targets/RISC

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

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | Fi

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

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | Fi

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

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4 +; RUN: llc < %s -mtriple=riscv64 | FileCheck %s +; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s -check-prefix=ALIGN_16 +; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | Fi

[PATCH] D105555: [RISCV][Clang] Compute the default target-abi if it's empty.

jrtc27 added a comment. Commit message needs rewriting still, but the patch looks good to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10/new/ https://reviews.llvm.org/D10 ___ cfe-commits mai

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

jrtc27 added inline comments. Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:31 +public: + RISCVISAInfo() : XLen(0), FLen(0) {} + Does Exts need initialising to be empty here? I can never remember Comment at: llvm/include/llvm/Support/RI

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

jrtc27 updated this revision to Diff 362214. jrtc27 added a comment. Now only a CC1 option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105516/new/ https://reviews.llvm.org/D105516 Files: clang/include/clang/Basic/CodeGenOptions.def clang/inc

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

jrtc27 added a comment. In D105516#2892512 , @rjmccall wrote: > I agree with Eli: we should decide what the goals are here and then use those > goals to decide if we can identify a desirable permanent feature and, if so, > what the appropriate name for

[PATCH] D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/PowerPC/spe.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \ -; RUN: -mattr=+spe | FileCheck %s +;

[PATCH] D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/PowerPC/spe.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \ -; RUN: -mattr=+spe | FileCheck %s +;

[PATCH] D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/PowerPC/spe.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \ -; RUN: -mattr=+spe | FileCheck %s +;

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

jrtc27 added a comment. I'm seeing the test fail locally with: 000d1799 t clang_CompileCommand_getNumMappedSources being found by grep. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105527/new/ https://reviews.llvm.org/D105527

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e79a94836d7: [Utils] Support class template specializations in update_cc_test_checks (authored by jrtc27). Repository: rG LLVM Github Monorepo C

[PATCH] D97606: [Clang interpreter] Avoid storing pointers at unaligned locations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG40080e7e7f42: [Clang interpreter] Avoid storing pointers at unaligned locations (authored by jrtc27). Repository: rG LLVM Github Monorepo CHANGES

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

jrtc27 added a comment. In D105527#2910616 , @tstellar wrote: > In D105527#2910319 , @jrtc27 wrote: > >> I'm seeing the test fail locally (on >> ca0fe3447fb85762838468537d93d4ef82c5a1af >>

[PATCH] D106974: libcang: Add missing function to libclang.map

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. Thanks; I can confirm the test passes if and only if I apply this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106974/new/ https://review

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

jrtc27 added a comment. Ping? I'd really like to get this fixed in 14. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116774/new/ https://reviews.llvm.org/D116774 ___ cfe-commits mailing list cfe-commits@

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

jrtc27 added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:641 +getASTContext().getTargetInfo().getTriple().getArch(); +if (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be || +Arch == llvm::Triple::arm || Arch == llvm::Triple::

[PATCH] D118333: [RISCV] Use computeTargetABI from llc as well as clang

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/double-mem.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: llc -mtriple=riscv32 -mattr=+d -verify-machineinstrs < %s \ +; RUN: llc -mtriple=riscv32 -mattr=+d -target-abi=

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. Thanks, looks good to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116774/new/ https://reviews.llvm.org/D116774 ___

[PATCH] D116773: AST: Make getEffectiveDeclContext() a member function of ItaniumMangleContextImpl. NFCI.

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. This makes sense to me but I don't know if you want someone with more authority in these parts to review it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

jrtc27 added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8555-8556 +// namespace std { struct __va_list { +NamespaceDecl *NS; +NS = NamespaceDecl::Create(const_cast(*Context), + Context->getTranslationUnitDecl(), ---

[PATCH] D129802: [DRAFT] Implementing new atomic orderings in LLVM and generate barriers for legacy __sync builtins. Support corresponding memory model in outline atomics as well.

jrtc27 added a comment. I can't obviously see a description of what the additional barriers implied by the sync variants is (which should be in an update to LangRef at the very least, if not also in the summary itself). Inferring it from the AArch64 assembly is also difficult, and the RISC-V lo

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added a comment. Does GCC allow this or not? Because this strikes me as a bad idea at first sight… Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/new/ https://reviews.llvm.org/D129824 ___ c

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

jrtc27 added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:3 +// RUN: %clang_cc1 -fcheck-new -triple x86_64-linux-gnu -S -disable-O0-optnone \ +// RUN: -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s + MaskRay wrote: > Please remove `opt`

[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

jrtc27 added a comment. If you still want to pursue this, discussion belongs at https://github.com/riscv-non-isa/riscv-toolchain-conventions, not here, since it's an interface shared by Clang and GCC and the two should be consistent CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129824/

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

jrtc27 added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:2 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang -fcheck-new -emit-llvm -S %s -o - -O2 | FileCheck %s + Do you really want -O2 or do you

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

jrtc27 added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:2 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang -fcheck-new -emit-llvm -S %s -o - -O2 | FileCheck %s + heatd wrote: > jrtc27 wrote: > >

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

jrtc27 added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:1 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang -fcheck-new --target=x86_64-linux-gnu -emit-llvm -S %s -o - | opt -S -mem2reg | FileCheck %s --

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

jrtc27 added inline comments. Comment at: clang/test/CodeGenCXX/fcheck-new.cpp:2 +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang -fcheck-new --target=x86_64-linux-gnu -emit-llvm -S %s -o - | opt -S -mem2reg | FileCheck %s +

[PATCH] D125947: [RISCV] Add default ABI for archs with only F extension

jrtc27 added a comment. It's currently this way in order to be compatible with GCC. Changing this requires consensus from both toolchains to ensure compatibility is preserved. See https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/13 for some discussion on this. Repository:

[PATCH] D125947: [RISCV] Add default ABI for archs with only F extension

jrtc27 added a comment. Also, the tests where you have codegen changes rather than preserving a soft-float ABI should probably be put up for review separately by adding an explicit hard single-float ABI, as those seem worthwhile for reducing noise Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D157331: [clang] Implement C23

jrtc27 added a comment. In D157331#4654353 , @aaron.ballman wrote: > In D157331#4654265 , @mstorsjo > wrote: > >> This change broke building a recent version of gettext. Gettext uses gnulib >> for polyfilling v

[PATCH] D159546: [OpenMP 5.2] Initial parsing and semantic analysis support for 'step' modifier on 'linear' clause

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1352 +def err_omp_multiple_step_or_linear_modifier : Error< + "multiple %select{'step size'|'linear modifier'}0 found in linear clause">; def warn_pragma_expected_colon_r_paren : Warnin

[PATCH] D159546: [OpenMP 5.2] Initial parsing and semantic analysis support for 'step' modifier on 'linear' clause

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1352 +def err_omp_multiple_step_or_linear_modifier : Error< + "multiple %select{'step size'|'linear modifier'}0 found in linear clause">; def warn_pragma_expected_colon_r_paren : Warnin

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

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. This is currently incompatible with the save/restore libcalls, and I don't think there's any way to avoid that (the restore libcall both loads ra and jumps to it). We should ensure c

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:86 + + // Get shadow call stack pointer register. + Register SCSPReg = RISCVABI::getSCSPReg(); Pointless comment; remove Comment at: llvm/lib/Target/RISCV/

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:38 + // Do not save RA to the SCS if it's not saved to the regular stack, + // i.e. RA is not at risk of being to overwritten. + std::vector &CSI = MF.getFrameInfo().getCalleeSavedInfo(); --

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added subscribers: trasz, dim, jrtc27. jrtc27 added a comment. Herald added a subscriber: dang. This has significantly regressed FreeBSD's performance with the new version of Clang. It seems Clang does not inline functions at -O1, unlike GCC, and since FreeBSD currently compiles its kerne

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

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. Yes I think everything's been addressed now (though if I keep looking over it I might start nit-picking even more :)). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279812 , @Bdragon28 wrote: > In D79916#2279045 , @jrtc27 wrote: > >> This has significantly regressed FreeBSD's performance with the new version >> of Clang. It seems Clang does n

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279863 , @Bdragon28 wrote: > In D79916#2279816 , @jrtc27 wrote: > >> In D79916#2279812 , @Bdragon28 >> wrote: >> >>> In D79916#2279045

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. (and FreeBSD has an `__always_inline` in sys/sys/cdef.s like `__inline`) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79916/new/ https://reviews.llvm.org/D79916 ___ cfe-commits m

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279871 , @Bdragon28 wrote: > In D79916#2279866 , @jrtc27 wrote: > >> In D79916#2279863 , @Bdragon28 >> wrote: >> >>> In D79916#2279816

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279875 , @jrtc27 wrote: > In D79916#2279871 , @Bdragon28 wrote: > >> In D79916#2279866 , @jrtc27 wrote: >> >>> In D79916#2279863

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279901 , @Bdragon28 wrote: > In D79916#2279884 , @jrtc27 wrote: > >> But also you really should not get warnings for unused static functions in >> included headers, only ones defi

[PATCH] D79916: Map -O to -O1 instead of -O2

jrtc27 added a comment. In D79916#2279987 , @MaskRay wrote: > Several previous comments are FreeBSD specific. To we clang developers, the > concrete request is > >> Given that GCC will inline at -O, at least these days, ... > > right? I think this makes s

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

jrtc27 added a comment. If someone cares about pthread_create they might wish to follow up on my https://reviews.llvm.org/D58531, which I filed early last year to permit pthread_create to have a proper type in the syntax. It will likely need rebasing, but probably isn't that much work. Reposi

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

jrtc27 added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889 + if (Left.is(TT_PointerOrReference)) { +if (Style.SpaceBeforePointerQualifiers && +Right.canBePointerOrReferenceQualifier()) + return true; return Right.Tok.isLiteral()

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

jrtc27 added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889 + if (Left.is(TT_PointerOrReference)) { +if (Style.SpaceBeforePointerQualifiers && +Right.canBePointerOrReferenceQualifier()) + return true; return Right.Tok.isLiteral()

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

jrtc27 added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889 + if (Left.is(TT_PointerOrReference)) { +if (Style.SpaceBeforePointerQualifiers && +Right.canBePointerOrReferenceQualifier()) + return true; return Right.Tok.isLiteral()

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. GCC on Linux defines `__sparc_v9__` even with `-m32`. I don't know what Solaris does but please don't break other operating systems just because Solaris has broken headers that confl

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

jrtc27 added a comment. And notably _doesn't_ define the V8 macros, which this patch then reintroduces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86621/new/ https://reviews.llvm.org/D86621 ___ cfe-co

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

jrtc27 added a comment. Actually, `__sparcv8` is only for V8; if you have 32-bit V9 on Solaris it defines `__sparcv8plus` _instead_: jrtc27@gcc-solaris11:~$ /opt/solarisstudio12.4/bin/cc -E - -xarch=v9 -m32 -xdumpmacros &1 | grep sparc #define __sparcv8plus 1 #define __sparc 1 #define s

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. The documentation currently shows `__capability` being included, but from looking at this patch does the configuration file not append (which I think makes sense, at least for `__cap

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

jrtc27 added a comment. In D52050#2441133 , @glaubitz wrote: > In D52050#2441094 , @hvdijk wrote: > >> I've been able to check what Ubuntu 20.10 offers in terms of x32 support. >> Its kernel supports x32 binaries,

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

jrtc27 added a comment. i.e. can we not just support both approaches and prefer x86_64-linux-gnux32 if it exists? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52050/new/ https://reviews.llvm.org/D52050 ___

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

jrtc27 added a comment. Firstly, please generate your diffs with full context (-U with a sufficiently-large number). Secondly, can we avoid having to do a bunch of duplication with some clever use of multiclasses for F/D/Zfh and pseudos? Though maybe it's small enough that the duplication is ea

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/M68k.cpp:38-50 + // M68k pointers are always 32 bit wide even for 16 bit cpus + Layout += "-p:32:32"; + + // M68k integer data types + Layout += "-i8:8:8-i16:16:16-i32:32:32"; + + // FIXME no floats at the mom

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2089 + static const char *const M68kTriples[] = { + "m68k-linux-gnu", "m68k-unknown-linux-gnu", "m68k-suse-linux"}; + rengolin wrote: > The front-end supports FreeBSD, too. Aren't

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

jrtc27 added a comment. In D52050#2466874 , @glaubitz wrote: > In D52050#2441164 , @glaubitz wrote: > >> In D52050#2441141 , @jrtc27 wrote: >> >>> What gets done currently fo

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

jrtc27 added a comment. We use pure LLVM toolchains so improving support for that out of the box is good in my books. However, I do worry this is going to cause friction for a lot of people using LLVM for RISC-V; my understanding is that most use LLVM with a GNU sysroot and binutils, and so thi

  1   2   3   4   >