[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] 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] 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] 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] 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] 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 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] 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 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 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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/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/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] 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] 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] 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] D153008: [RISCV] Allow slash-star comments in instruction operands

jrtc27 added a comment. In D153008#4425238 , @abel-bernabeu wrote: > In D153008#4424821 , @jrtc27 wrote: > >> Clang tests should not compile to asm. You want an IR test. > > Jessica, are there any exceptions for

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

jrtc27 requested changes to this revision. jrtc27 added a comment. This revision now requires changes to proceed. Clang tests should not compile to asm. You want an IR test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.

[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] 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] 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] 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] 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] 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] 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] 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] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

jrtc27 added inline comments. Comment at: clang/test/Driver/riscv-default-build-attributes.s:3 +// RUN: %clang --target=riscv64 -### %s 2>&1 \ +// RUN:| FileCheck %s -check-prefix CHECK-ENABLED + craig.topper wrote: > jrtc27 wrote: > > jrtc27 wrote: > > > `=`

[PATCH] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

jrtc27 added a comment. Some more... hopefully spotted everything this time, sorry Comment at: clang/test/Driver/riscv-default-build-attributes.s:1 + Enabled by default for assembly +// RUN: %clang --target=riscv64 -### %s 2>&1 \ Comment

[PATCH] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

jrtc27 added a comment. Surprised that we didn't already do this; seems like a bit of an oversight. Comment at: clang/test/Driver/riscv-default-build-attributes.s:1 +// Enabled by default for assembly +// RUN: %clang --target=riscv64 -### %s 2>&1 \ Do we use //

[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? =

[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] 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] 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] 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] 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] 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] 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/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 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] 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] 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. 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] 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] 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] 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-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: 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: 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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. 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] 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] 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] 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] 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] 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] 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 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] 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] D139302: [RISCV] Add Syntacore SCR1 CPU model

jrtc27 added inline comments. Comment at: llvm/include/llvm/Support/RISCVTargetParser.def:22 PROC(SIFIVE_U74, {"sifive-u74"}, FK_64BIT, {"rv64gc"}) +PROC(SCR1_BASE, {"scr1-base"}, FK_NONE, {"rv32ic"}) +PROC(SCR1_MAX, {"scr1-max"}, FK_NONE, {"rv32imc"}) Alphabeti

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

jrtc27 added inline comments. Comment at: clang/test/CodeGen/annotations-field.c:5 -// CHECK: private unnamed_addr constant [8 x i8] c"v_ann_{{.}}\00", section "llvm.metadata" -// CHECK: private unnamed_addr constant [8 x i8] c"v_ann_{{.}}\00", section "llvm.metadata" +// CHE

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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

  1   2   3   4   >