[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

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

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

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

[PATCH] D114408: Fold a lot of ffixed_x if judgments

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. This has clearly not been tested whatsoever, it’s totally broken, the preprocessor does not work like that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D114533: LLVM IR should allow bitcast between address spaces with the same size.

jrtc27 added a comment. This seems like it should not apply to non-integral address spaces? Comment at: llvm/docs/LangRef.rst:10999 +with this instruction. Conversion of pointers to different address spaces +are legal, but may result in undefined behaviour where such a reinter

[PATCH] D114533: LLVM IR should allow bitcast between address spaces with the same size.

jrtc27 added a comment. In D114533#3154225 , @arsenm wrote: > In D114533#3153924 , @jrtc27 wrote: > >> This seems like it should not apply to non-integral address spaces? > > No, it shouldn’t care about the indivi

[PATCH] D64008: [RISCV] Avoid save-restore target feature warning

jrtc27 added a comment. Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, vkmr, frasercrmck, evandro, luismarques, sameer.abuasal, s.egerton, MaskRay. For historical documentation purposes: I don't think this ever actually fully worked. Whilst it stops passing the fea

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

jrtc27 added a comment. Any luck with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/ https://reviews.llvm.org/D104830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D136571: [RISCV] add svinval extension

jrtc27 added a comment. Hm, we only have two uses of Requires currently, both of which aren't really for any good reason as far as I can see. It'd be better to keep things uniform with Predicates IMO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D116735: [RISCV] Adjust RV64I data layout by using n32:64 in layout string

jrtc27 added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:122 been removed. +* n32 was added to the RV64I datalayout string. arichardson wrote: > Without additional context I don't think this makes much sense to most > readers. Before looking at

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986 +bool Ret = detectFPCCEligibleStructHelper( +B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off); +if (!Ret) With multiple inheritance this off

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986 +bool Ret = detectFPCCEligibleStructHelper( +B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off); +if (!Ret) jrtc27 wrote: > With multiple inhe

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986 +bool Ret = detectFPCCEligibleStructHelper( +B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off); +if (!Ret) craig.topper wrote: > jrtc27 wrote

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

jrtc27 added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-abi.cpp:1 // RUN: %clang_cc1 -triple riscv32 -emit-llvm -x c++ %s -o - \ // RUN: | FileCheck -check-prefixes=ILP32,ILP32-ILP32F,ILP32-ILP32F-ILP32D %s Not sure why you need -x c++ when i

[PATCH] D132247: [clang] Fix emitVoidPtrVAArg for non-zero default alloca address space

jrtc27 created this revision. jrtc27 added a reviewer: arsenm. Herald added subscribers: kosarev, tpr. Herald added a project: All. jrtc27 requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Indirect arguments are passed on the stack and s

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: clang/CMakeLists.txt:467 +option(CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION + "Force the SOVERSION of libclang to be equal to CLANG_MAJOR" OFF) +

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

jrtc27 added a comment. In D132486#3743461 , @h-vetinari wrote: > Thanks for the review. Given that you have concerns, could you voice them in > a larger forum > (https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground-to-k

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

jrtc27 added a comment. In D129824#3794221 , @MaskRay wrote: > Both D54214 and this look like a surprising > behavior to me. Do we still have time to go back the state before D54214 > a

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCV.td:366 + "'Zcmt' (table jump instuctions for code-size reduction)", + [FeatureExtZca]>; // TODO: add Zicsr as another dependence +def HasStdExtZcmt : Predicate<"Su

[PATCH] D135287: Disallow dereferencing of void* in C++.

jrtc27 added a comment. What about `__typeof__(*p)`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135287/new/ https://reviews.llvm.org/D135287 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

jrtc27 added a comment. It also will fail if the configured default target triple is one where the default program address space is non-zero, and maybe other things (do any targets do pre-IR name mangling for C?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

jrtc27 added a comment. Hm, this means that llvm/lib/Target/$TARGET/$TARGET.td needs to remain working (or at least mostly working, things like ISel patterns can fail to type check) in order for Clang to build, since we're now introducing a dependency on llvm/lib/Target/$TARGET in Clang where t

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCV.td:581 + list f = []> + : ProcessorModel; + Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137517/new/ https:

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

jrtc27 added a comment. In D137350#3906126 , @craig.topper wrote: > Do these need their own DecoderNameSpace? I was going to come here to request that CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137350/new/ https://reviews.llvm.org/D137350

[PATCH] D137517: [TargetSupport] Generate the defs for RISCV CPUs using llvm-tblgen.

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCV.td:509-517 +class RISCVProcessorModelPROC { + string Enum = enum; + string EnumFeatures = enum_features; + string DefaultMarch = default_march; +} + +class RISCVProcessorModelTUNE_PROC { Giv

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

jrtc27 added a comment. Arm and AArch64 are special because they're the architectures where `va_list` is `std::__va_list`, not a bare `__va_list_tag` or similar. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D1

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

jrtc27 added a comment. This should be readily reproducible with clang -target aarch64 on any machine, though? The current lit tests are just x86 ones, which isn't great coverage of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

jrtc27 added a comment. In D136886#4083762 , @vabridgers wrote: > It appears to me this change https://reviews.llvm.org/D116774 is responsible > for the unexpected behavior. Question for @jrtc27 : do you think if we could > make this change consistent

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:16 +#include "GISel/RISCVLegalizerInfo.h" +#include "GISel/RISCVRegisterBankInfo.h" #include "RISCV.h" Unrelated change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

jrtc27 added a comment. In D137517#4042875 , @fpetrogalli wrote: > In D137517#4042758 , @fpetrogalli > wrote: > >> After submitting this, I had to revert it >>

[PATCH] D137517: [TargetParser] Generate the defs for RISCV CPUs using llvm-tblgen.

jrtc27 added a comment. In D137517#4045315 , @fpetrogalli wrote: > In D137517#4045299 , @jrtc27 wrote: > >> In D137517#4042875 , @fpetrogalli >> wrote: >> >>> In D137517

[PATCH] D141666: [RISCV] Proper support of extensions Zicsr and Zifencei

jrtc27 added a comment. Because they used to be part of I and then a later version of the RISC-V spec split them out in an incompatible way. GCC/binutils bit the bullet and split them out last(?) year but LLVM has yet to make that breaking change. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D141798: Drop the ZeroBehavior parameter from countLeadingZeros and the like (NFC)

jrtc27 added inline comments. Comment at: llvm/include/llvm/Support/MathExtras.h:225 /// /// \param ZB the behavior on an input of 0. Only ZB_Max and ZB_Undefined are /// valid arguments. A bunch of functions still have this documentation, but these are now

[PATCH] D139704: [clang][RISCV] Added target attributes to runtime functions

jrtc27 added a comment. How does this affect LTO? LTO for RISC-V is broken for various reasons, but LTO isn't Clang's responsibility beyond passing on driver arguments. Is this not just one of many symptoms of RISC-V not having the right module-level subtarget under LTO, which is a real problem

[PATCH] D142085: [Clang][RISCV] Add `__riscv_` prefix for all RVV intrinsics

jrtc27 added a comment. (For some reason Phab won't let me comment inline from the Changeset View page, just does nothing when I click reply or click a line...) Or just leave the test names alone? The __riscv_ is for namespacing the intrinsics, you don't need to namespace the tests when they're

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

jrtc27 added inline comments. Comment at: compiler-rt/lib/tsan/rtl/CMakeLists.txt:5 append_list_if(COMPILER_RT_HAS_MSSE4_2_FLAG -msse4.2 TSAN_RTL_CFLAGS) -append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=530 +append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

jrtc27 added a comment. Posting on GitHub loses all context; please don't do that. Whose review are you looking for? You've addressed my feedback and you had approval from Aaron on the prior version, with the latest version just being a few minor tweaks to address the couple of comments I had l

[PATCH] D157331: [clang] Implement C23

jrtc27 added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:7-15 +// CHECK:%result64 = alloca i64, align 8 +// CHECK:%flag_add = alloca i8, align 1 +// CHECK:store i64 0, ptr %result64, align 8 +// CHECK:%0 = call { i64, i1 } @llvm.sadd.with.overflow.i64(

[PATCH] D157331: [clang] Implement C23

jrtc27 added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:14 +// CHECK:store i64 %2, ptr %[[RES64]], align 8 +// CHECK:%frombool = zext i1 %1 to i8 +// CHECK:store i8 %frombool, ptr %[[FLAG_ADD:.*]], align 1 You missed this one. But please

[PATCH] D157331: [clang] Implement C23

jrtc27 added a comment. One more thought: we need to specify a triple for the tests as otherwise it'll use whatever default you have configured in LLVM, which will affect the exact code generated (especially wrt argument and return lowering), no? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D157331: [clang] Implement C23

jrtc27 added a comment. In D157331#4653224 , @aaron.ballman wrote: > In D157331#4653222 , @jrtc27 wrote: > >> One more thought: we need to specify a triple for the tests as otherwise >> it'll use whatever defaul

[PATCH] D157331: [clang] Implement C23

jrtc27 added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:5-6 +#include +#include +// CHECK-LABEL: define dso_local void @test_add_overflow_to64( +// CHECK-SAME: ) #[[ATTR0:[0-9]+]] { UTC won't insert such a blank line if it doesn't exist in the inpu

[PATCH] D157663: [Driver] Default riscv*-linux* to -fdebug-default-version=4

jrtc27 added a comment. RISCVToolchain also needs an override Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157663/new/ https://reviews.llvm.org/D157663 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D157663: [Driver] Default riscv*-linux* to -fdebug-default-version=4

jrtc27 added a comment. Also Haiku I guess, as another OS that has a somewhat working RISC-V port but falls back on ToolChain's default implementation. So maybe this just belongs in the default implementation, given all the ones that need modifying are the ones that end up there. Repository:

[PATCH] D152793: [RISCV] Add MC layer support for Zicfiss.

jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:1 +//=== RISCVInstrInfoZicfiss.td - RISC-V CFG -*- tablegen -*===// +//

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

jrtc27 added inline comments. Comment at: clang/test/CodeGen/mrtd.c:2 +// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck --check-prefixes=CHECK,X86 %s +// RUN: %clang_cc1 -mrtd -triple m68k-unknown-unknown -std=c89 -emit-llvm -o -

[PATCH] D158624: [RISCV] Implement multiVersionSortPriority

jrtc27 added a comment. I don't think you can just use any arbitrary function? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158624/new/ https://reviews.llvm.org/D158624 ___ cfe-commits mailing list cfe-

[PATCH] D157331: [clang] Implement C23

jrtc27 added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:5-6 +#include +#include +// CHECK-LABEL: define dso_local void @test_add_overflow_to64( +// CHECK-SAME: ) #[[ATTR0:[0-9]+]] { jrtc27 wrote: > UTC won't insert such a blank line if it doesn't e

[PATCH] D157331: [clang] Implement C23

jrtc27 added a comment. Thanks, my concerns in the tests have been addressed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 ___ cfe-commits mailing list cfe-co

[PATCH] D156565: Diagnose use of VLAs in C++ by default

jrtc27 added a comment. Given GCC defines GNU C++ and regards this as a feature (unless you use things like -pedantic to ask for ISO C++), does it make sense to enable this for GNU C++? Every deviation from GCC is a point of friction for adopting Clang over GCC. Repository: rG LLVM Github M

[PATCH] D155668: [RISCV] Upgrade Zvfh version to 1.0 and move out of experimental state.

jrtc27 added a comment. Uh, why are there clang/test/Sema/aarch64* tests full of RISC-V extension names? That's not right at all. One of them is coming from https://reviews.llvm.org/D135011, I haven't traced the rest back, but that's clearly wrong. The test looks to be a copy of the RISC-V one

[PATCH] D157331: [clang] Implement C23

jrtc27 added inline comments. Comment at: clang/lib/Headers/stdckdint.h:12 +#define __STDCKDINT_H + +/* C23 7.20.1 Defines several macros for performing checked integer arithmetic*/ pirama wrote: > aaron.ballman wrote: > > Should a hosted build attempt to do an

[PATCH] D154285: [clang] Deprecate CGBuilderTy::CreateElementBitCast

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3954 Function *F = CGM.getIntrinsic(Intrinsic::eh_sjlj_setjmp); -Buf = Builder.CreateElementBitCast(Buf, Int8Ty); return RValue::get(Builder.CreateCall(F, Buf.getPointer()));

[PATCH] D154285: [clang] Deprecate CGBuilderTy::CreateElementBitCast

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3896 llvm::Type *OrigBaseElemTy = Addr.getElementType(); -Addr = Builder.CreateElementBitCast(Addr, Int8Ty); JOE1994 wrote: > barannikov88 wrote: > > jrtc27 wrote: > > > This one is

[PATCH] D154285: [clang] Deprecate CGBuilderTy::CreateElementBitCast

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:158 /// This method is to be deprecated. Use `Address::withElementType` instead. + [[deprecated("Use `Address::withElementType` instead.")]] JOE1994 wrote: > nikic wrote: > > JOE1994 wr

[PATCH] D154285: [clang] Deprecate CGBuilderTy::CreateElementBitCast

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:158 /// This method is to be deprecated. Use `Address::withElementType` instead. + [[deprecated("Use `Address::withElementType` instead.")]] jrtc27 wrote: > JOE1994 wrote: > > nikic wro

[PATCH] D154285: [clang] Remove CGBuilderTy::CreateElementBitCast

jrtc27 accepted this revision. jrtc27 added a comment. Thanks for the updated diff (would have also been happy with having a new commit *before* this one that does the non-trivial changes, but this works too and can be followed up with the cleanups you were making if you want to) Repository:

[PATCH] D154397: [Driver] Default to -ftls-model=initial-exec on SerenityOS

jrtc27 added a comment. This is pretty dodgy, I don't think it belongs upstream Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154397/new/ https://reviews.llvm.org/D154397 ___ cfe-commits mailing list cfe

[PATCH] D151730: [RISCV] Support target attribute for function

jrtc27 added a comment. Isn't multiversioning a separate thing that builds on top of per-function target attributes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 __

[PATCH] D151730: [RISCV] Support target attribute for function

jrtc27 added a comment. - Asm tests are a no in Clang - You seem to have some crazy long lists that I doubt need to be that long for test coverage (nor do you need so many variations, surely) - CHECL is not CHECK - You have some very strange spacing in CHECK lines Repository: rG LLVM Github M

[PATCH] D155326: [Clang][lld][RISCV] Passing 'mattr' to lld/llvmgold

jrtc27 added a comment. Why do you believe this is better than encoding it in the module's IR like the ABI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155326/new/ https://reviews.llvm.org/D155326 ___

[PATCH] D138930: [RISCV] Add macro to imply compiler availability on RISC-V Vector intrinsics version

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:196 +// Currently we support the v0.10 RISC-V V intrinsics +unsigned Version = (0 * 100) + (10 * 1000); +Builder.defineMacro("__riscv_v_intrinsic", Twine(Version)); Do we

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

jrtc27 added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2228-2230 + Width = Target->getPointerWidth( + LangAS::Default); // C++ 3.9.1p11: sizeof(nullptr_t) + Align = Target->getPointerAlign(LangAS::Default); // == sizeof(void*)

[PATCH] D139025: [NFC][RISCV] Extract utility to calculate value through MajorVersion and MinorVersion

jrtc27 added a comment. Should this live in RISCVISAInfo.h rather than be a Clang-specific thing given it's just for squeezing version info into the preprocessor? I can't think of a use for it outside that at the moment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

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

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

jrtc27 added a comment. None of the other fields are initialised, so blindly initialising it alone to 0 here seems highly suspect. What's the actual case in which it's used whilst uninitialised? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146847

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

jrtc27 added a comment. The field should just be deleted. D126742 added it without any uses, and I don't know what the original intent was. Maybe it was used before the bitfields were added and it stuck around rather than being GC'ed. I don't know why Coverity

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

jrtc27 added a comment. In D146847#4220721 , @schittir wrote: > In D146847#4220697 , @jrtc27 wrote: > >> None of the other fields are initialised, so blindly initialising it alone >> to 0 here seems highly suspec

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

jrtc27 added inline comments. Comment at: compiler-rt/CMakeLists.txt:529 # natively, but supports --as-needed/--no-as-needed for GNU ld compatibility. -if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc") +if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc|riscv") lis

[PATCH] D145346: [NFC] Format printstmt

jrtc27 added a comment. This isn't NFC, the output changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145346/new/ https://reviews.llvm.org/D145346 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:112-114 + case llvm::Triple::riscv64: +getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +break; mnadeem wrote: > identical code, could just do a fallthrough Why

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

jrtc27 added inline comments. Comment at: flang/test/Driver/target-features.f90:1 +! RUN: %flang --target=riscv64-linux-gnu --target=riscv64 -c %s -### 2>&1 \ +! RUN: | FileCheck %s -check-prefix=CHECK-RV64 awarzynski wrote: > What happens if the RISC-V backend i

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

jrtc27 added inline comments. Comment at: flang/test/Driver/target-features.f90:1 +! RUN: %flang --target=riscv64-linux-gnu --target=riscv64 -c %s -### 2>&1 \ +! RUN: | FileCheck %s -check-prefix=CHECK-RV64 awarzynski wrote: > jrtc27 wrote: > > awarzynski wrote:

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/Flang.cpp:112-114 + case llvm::Triple::riscv64: +getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +break; kiranchandramohan wrote: > jrtc27 wrote: > > mnadeem wrote: > > >

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

jrtc27 added inline comments. Comment at: flang/test/Driver/target-cpu-features-invalid.f90:13 +! CHECK-INVALID-CPU: 'supercpu' is not a recognized processor for this target (ignoring processor) +! CHECK-INVALID-FEATURE: '+superspeed' is not a recognized feature for this target

[PATCH] D153170: [RISCV] Sort the extensions in SupportedExtensions and SupportedExperimentalExtensions.

jrtc27 added a comment. Can we have an expensive check that the table is sorted? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153170/new/ https://reviews.llvm.org/D153170 ___ cfe-commits mailing list cf

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

jrtc27 added a comment. In D147875#4448545 , @hans wrote: > I noticed that at least for some cases of `-Wformat`, the line numbers on the > left seem to be off: https://github.com/llvm/llvm-project/issues/63524 That's because DisplayLineNo refers to the

[PATCH] D154050: [Clang][RISCV] Fix RISC-V vector / SiFive intrinsic inclusion in SemaLookup

jrtc27 added inline comments. Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:201 -void RISCVIntrinsicManagerImpl::InitIntrinsicList() { +void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics( +ArrayRef Recs, IntrinsicKind K) { As far as I can tell th

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

jrtc27 added a comment. From an ABI/codegen perspective the small data limit doesn’t matter here, all it does is govern whether things go in .sdata or not. The linker is what chooses whether to use GP, which happens regardless of whether something is in .sdata, just things in .sdata are more li

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

jrtc27 added inline comments. Comment at: clang/docs/ShadowCallStack.rst:157 +linker. This can be done with the ``--no-relax-gp`` flag in GNU ld. It may also +be useful to compile with ``-msmall-data-limit=0``. This doesn't really achieve anything other than no

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:573 + Kinds & SanitizerKind::ShadowCallStack) + << "-msmall-data-limit=0"; +} paulkirth wrote: > jrtc27 wrote: > > Why is this an error? It m

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:573 + Kinds & SanitizerKind::ShadowCallStack) + << "-msmall-data-limit=0"; +} jrtc27 wrote: > paulkirth wrote: > > jrtc27 wrote: > > > Why is

[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

jrtc27 added inline comments. Comment at: clang/test/Preprocessor/riscv-target-features.c:51 // CHECK-NOT: __riscv_zcf {{.*$}} +// CHECK-NOT: __riscv_zcmp // CHECK-NOT: __riscv_h {{.*$}} Does this really belong in an MC patch? Comment at: llv

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:11390 +ResType = llvm::ScalableVectorType::get( +llvm::Type::getIntNTy(getVMContext(), XLen), 64 / XLen); +break; erichkeane wrote: > craig.topper wrote: > > erichkeane wro

[PATCH] D148034: [clang][driver] Disable GP relaxation with RISC-V ShadowCallStack

jrtc27 added a comment. In D148034#4262934 , @craig.topper wrote: > I'm not sure I want to suggest this, but could we disable the emission of the > relocations that can be GP relaxed? That would also lose the ability to do x0-relative and LUI -> C.LUI

[PATCH] D147989: [clang] Fix Attribute Placement

jrtc27 added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:5041 case DeclSpec::TST_enum: return 4; default: Why not just always pass the full DeclSpec and handle the class case here, maybe with a bool to allow other users to not need to tre

[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. I disagree. Being experimental doesn't mean you should do the wrong thing. Reusing std call in the frontend is ugly, pollutes non-m68k code paths (doing your own thing _avoids_ that

[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

jrtc27 added a comment. In D149867#4387189 , @glaubitz wrote: > In D149867#4386271 , @jrtc27 wrote: > >> I disagree. Being experimental doesn't mean you should do the wrong thing. >> Reusing stdcall in the fronte

[PATCH] D137309: [clang] Added Swift support for RISCV

jrtc27 added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10557 private: + DefaultABIInfo defaultInfo; // Size of the integer ('x') registers in bits. Huh? This is unused. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10601

[PATCH] D143953: [RISCV] Accept zicsr and zifencei command line options

jrtc27 added a comment. I'm fine with this approach but I think they should be filtered out of the ELF attributes, maybe also preprocessor macros? That is, treating them as a redundant no-op in the driver rather than like a true extension. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D143953: [RISCV] Accept zicsr and zifencei command line options

jrtc27 added a comment. In D143953#4124636 , @reames wrote: > @jrtc27 Not sure if this changes your take, but I realized the variant being > introduced is maybe much less interesting than I'd first thought. We > generally make no effort to make sure an

[PATCH] D143953: [RISCV] Accept zicsr and zifencei command line options

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. Personally happy with the concept then, seems consistent and overall helpful, just some nits Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:83-86 +

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

jrtc27 added inline comments. Comment at: clang/test/AST/ast-dump-overloaded-operators.cpp:27 // CHECK-NEXT: | `-ParmVarDecl {{.*}} col:19{{( imported)?}} 'E' -// CHECK-NEXT: `-FunctionDecl {{.*}} line:14:6{{( imported)?}} test 'void ()' +// CHECK-NEXT: -FunctionDecl {{.*}}

[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

jrtc27 added inline comments. Comment at: llvm/test/MC/RISCV/rv32zcmt-invalid.s:9 + +# CHECK-ERROR: error: immediate must be an integer in the range [0, 255] +cm.jalt 256 This is wrong; the immediate must be in the range [32, 255]. This needs to be enforced in t

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

jrtc27 added inline comments. Comment at: clang/lib/Frontend/TextDiagnostic.cpp:1121-1138 +static unsigned getNumDisplayWidth(unsigned N) { + if (N < 10) +return 1; + if (N < 100) +return 2; + if (N < 1'000) +return 3; kwk wrote: > This function sc

[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC

jrtc27 added a comment. So GCC gives me: warning: ‘stdcall’ attribute directive ignored [-Wattributes] when trying to use `__attribute__((stdcall))` on m68k, which matches the fact it's only mentioned in the manage for the x86 option. Interestingly it seems to ICE during expand when I use -m

[PATCH] D132247: [clang] Fix emitVoidPtrVAArg for non-zero default alloca address space

This revision was automatically updated to reflect the committed changes. Closed by commit rG74f207883bc5: [clang] Fix emitVoidPtrVAArg for non-zero default alloca address space (authored by jrtc27). Changed prior to commit: https://reviews.llvm.org/D132247?vs=454033&id=522295#toc Repository:

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

jrtc27 added a comment. We heavily rely on this extension in CheriBSD via `__typeof__((*(p))) * __capability` as we want to be able to take any pointer, including to an array or function that needs to undergo decay to be an actual pointer, and turn it into a `__capability`-qualified one. Presum

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

jrtc27 added a comment. In D150875#4353484 , @erichkeane wrote: > In D150875#4353467 , @jrtc27 wrote: > >> We heavily rely on this extension in CheriBSD via `__typeof__((*(p))) * >> __capability` as we want to b

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

jrtc27 added a comment. I can't help but feel the core of this should be target-independent, with TLI or similar hooks to actually do the target-specific bits? Comment at: llvm/test/CodeGen/RISCV/kcfi.ll:1 +; RUN: llc -mtriple=riscv64 -verify-machineinstrs -riscv-no-aliases <

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:277 +void RISCVAsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) { + assert(STI->is64Bit() && "KCFI_CHECK requires RV64"); + What about this code relies on it being 64-bit? =

<    1   2   3   4   >