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

jrtc27 added inline comments. Comment at: clang/test/Driver/riscv64-toolchain.c:5 +// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \ +// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 | FileCheck -check-prefix=CC1 %s // CC1: clang{{.*}} "-cc1" "-triple" "r

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

jrtc27 added a comment. In D91442#2399750 , @abidh wrote: > In D91442#2399402 , @lenary wrote: > >> In D91442#2399341 , @abidh wrote: >> >>> In D91442#2399200

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

jrtc27 added a comment. In D91442#2399753 , @jrtc27 wrote: > In D91442#2399750 , @abidh wrote: > >> In D91442#2399402 , @lenary wrote: >> >>> In D91442#2399341

[PATCH] D91784: [RISCV] Set __GCC_HAVE_SYNC_COMPARE_AND_SWAP_x defines

jrtc27 accepted this revision. jrtc27 added reviewers: asb, lenary, luismarques. jrtc27 added a comment. This revision is now accepted and ready to land. This seems correct. GCC doesn't define the 1 and 2 variants but it implements those with libcalls rather than inline like LLVM so that's to be

[PATCH] D88759: [RISCV] Add SiFive cores to the CPU option

jrtc27 added a comment. In D88759#2313236 , @thakis wrote: > This seems to break tests: http://45.33.8.238/linux/29545/step_7.txt > > Can you take a look and revert for now if it takes a while to fix? I see it should already have been fixed in a48d480e1f

[PATCH] D91428: Add support for multiple program address spaces

jrtc27 added a comment. Currently `P0-P1` is valid and results in the program address space being 1, but this patch changes the semantics of that. How sure are you that nothing will break? I do not like the idea of reusing existing valid syntax to mean something else; if you want to introduce s

[PATCH] D92269: [TableGen] Eliminate the 'code' type

jrtc27 added a comment. Losing types and permitting nonsensical operators like `!subst` on code seems a bit sad. Why not just define a `!codeconcat`, like how we have a separate `!listconcat`? I agree that the TypeOf_xxx stuff is really ugly and so having _greater_ expressivity like `!codeconca

[PATCH] D92269: [TableGen] Eliminate the 'code' type

jrtc27 added a comment. In D92269#2421399 , @Paul-C-Anagnostopoulos wrote: > Yes, we could create !codeconcat instead, along with !codeinterleave, > !codeeq, etc. We could also just extend the existing bang operators to work > on the code type. I though

[PATCH] D71124: [RISCV] support clang driver to select cpu

jrtc27 added a comment. In D71124#2423633 , @oceanfish81 wrote: > @khchen , > how should I obtain available (v)CPU features? > Any sample code, that should work on RISC-V? Probably `getauxval(AT_HWCAP)`; both Linux and FreeBSD put the ISA string in the l

[PATCH] D91270: [Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

jrtc27 added a comment. Seems good other than additional comments regarding code clarity. Comment at: clang/lib/CodeGen/TargetInfo.cpp:10577 NeededArgGPRs++; - return IsCandidate; + return true; } This NFC hunk definitely makes this clearer :) ===

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

jrtc27 added a comment. In D52050#2428743 , @hvdijk wrote: > In D52050#2428735 , @glaubitz wrote: > >> Hmm, I was pretty sure that autoconf can deal with x32 inside an x32 chroot. > > Most autoconf-using software wo

[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/RISCVVTypes.def:67 +RVV_VECTOR_TYPE_INT("__rvv_int8m2_t", RvvInt8m2, RvvInt8m2Ty, 16, 8, 1, true) +RVV_VECTOR_TYPE_INT("__rvv_int8m4_t", RvvInt8m4, RvvInt8m4Ty, 32, 8, 1, true) +RVV_VECTOR_TYPE_INT("__r

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

jrtc27 added inline comments. Comment at: clang/test/Driver/lto.c:81-90 +// Need to pass -target-abi in RISC-V target. +// RUN: %clang -target riscv64-unknown-elf %s -fuse-ld=gold -flto \ +// RUN: -mabi=lp64f -### 2>&1 | FileCheck %s --check-prefix=RISCV +// RUN: %clang -target

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

jrtc27 added a comment. In D71387#2762115 , @khchen wrote: > In D71387#1820995 , @efriedma wrote: > >> Okay. Please let me know if you want me to review anything. > > Hi all, > We had encoded the target-abi into mo

[PATCH] D102839: [RISCV][Clang] Add -mno-idiv option to disable hardware int division

jrtc27 added a comment. I continue to believe this kind of thing is a bad idea (missing fsqrt for F is another one). M means mul and div, if your hardware lacks div then it's not compliant with the spec. M-mode is free to lie to S-mode and above as to what's supported by the hardware and emulat

[PATCH] D102839: [RISCV][Clang] Add -mno-div option to disable hardware int division

jrtc27 added a comment. In D102839#2782557 , @kito-cheng wrote: > We have Zmmul extension in the ISA spec now, that's equivalent to `-mno-div` > , so I suggest we should go forward to implement that extension rather than > `-mno-div`. > > https://githu

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

jrtc27 added a comment. Is it just me or does having a backend-specific type in target-independent code feel wrong? It feels like there should be a space for target-specific TargetStackIDs... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101140/ne

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

jrtc27 added a comment. In D101140#2786844 , @wingo wrote: > In D101140#2786777 , @jrtc27 wrote: > >> Is it just me or does having a backend-specific type in target-independent >> code feel wrong? It feels like t

[PATCH] D103313: [RISCV][Clang] Implement support for zmmul-experimental

jrtc27 added a comment. In D103313#2787316 , @craig.topper wrote: > Do we need to add this to RISCVTargetStreamer::emitTargetAttributes? Yes, that, but also RISCVAsmParser::parseDirectiveAttribute and ELFObjectFileBase::getRISCVFeatures, though it seem

[PATCH] D103313: [RISCV][Clang] Implement support for zmmul-experimental

jrtc27 added a comment. In D103313#2787326 , @jrtc27 wrote: > In D103313#2787316 , @craig.topper > wrote: > >> Do we need to add this to RISCVTargetStreamer::emitTargetAttributes? > > Yes, that, but also RISCVAsm

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5216-5220 +if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args)) + TC = + std::make_unique(*this, Target, Args); +else + TC = std::make_unique(*this, Targ

[PATCH] D103603: [Sema][RISCV] Allow ?: to select Typedef BuiltinType in C

jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaExpr.cpp:8397 + if (LHSTy->isSizelessBuiltinType() && + (Context.getCanonicalType(LHSTy) == Context.getCanonicalType(RHSTy)

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5216-5220 +if (toolchains::RISCVToolChain::hasGCCToolchain(*this, Args)) + TC = + std::make_unique(*this, Target, Args); +else + TC = std::make_unique(*this, Targ

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

jrtc27 added a comment. There are a lot of "Resolve for v1.0" issues open against the spec still. Are we sure we want to brand this as 1.0? It will end up as such in the ELF attributes and thus be deemed compatible with future "real" 1.0 binaries. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

jrtc27 added a comment. In D94583#2513915 , @HsiangKai wrote: > In D94583#2513070 , @jrtc27 wrote: > >> There are a lot of "Resolve for v1.0" issues open against the spec still. >> Are we sure we want to brand this

[PATCH] D94583: [RISCV] Update V extension to v1.0-draft 08a0b464.

jrtc27 added a comment. (Their problem stems from having 1.0 drafts before they've resolved all the outstanding issues and frozen the instruction set; if they didn't jump the gun then things would be saner for people implementing it) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-rvv-intrinsics-generic/vadd.c:7-8 +// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \ +// RUN: -target-feature +experimental-zfh -Werror -Wall -o -

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/riscv_vector.td:161 + // This builtin is valid for the given exponental LMULs. + list ELMUL = [0, 1, 2, 3, -1, -2, -3]; + khchen wrote: > HsiangKai wrote: > > EMUL according to specification. >

[PATCH] D95680: [RISCV] Update the version number to v0.10 for vector.

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. \o/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95680/new/ https://reviews.llvm.org/D95680 ___ cf

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

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/M68k.cpp:77-79 + Builder.defineMacro("M68k"); + Builder.defineMacro("__M68k__"); + Builder.defineMacro("__M68K__"); myhsu wrote: > jrtc27 wrote: > > Where are these coming from? GCC only defines

[PATCH] D93446: [RISCV] Add vadd with mask and without mask builtin.

jrtc27 added inline comments. Comment at: clang/test/CodeGen/RISCV/vadd.c:22 +{ +return __builtin_rvv_vadd_vv_i8m1_vl(arg_0, arg_1, arg_2); +} Jim wrote: > Is it necessary with prefix "__builtin_"? > Refer to > https://github.com/riscv/rvv-intrinsic-doc/blob

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

jrtc27 added a comment. In D93298#2544775 , @asb wrote: > In D93298#2544459 , @StephenFan > wrote: > >> According to @jrtc27 's review that is >> "As for Zfinx itself, well, the idea is fine, but I really detest

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

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1541 + // NOTE: If you add any additional spellings, ARMInterrupt's, MipsInterrupt's + // and AnyX86Interrupt's spellings must match. + let Spellings = [GNU<"interrupt">]; Do you need

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:380 ve::getVETargetFeatures(D, Args, Features); +break; } Unrelated Comment at: clang/test/Driver/m68k-sub-archs.cpp:1 +// RUN: %clang -### -target m68k

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

jrtc27 added a comment. @rjmccall 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.ll

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:442 + +if (STI.getFeatureBits()[RISCV::FeatureExtZpsfoperand] && +!STI.getFeatureBits()[RISCV::Feature64Bit]) { Jim wrote: > jrtc27 wrote: > > The table

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

jrtc27 added a comment. The amount of duplication here really depresses me and is only going to get worse once codegen is added, but TableGen isn't able to have operands that use different register classes based on even HwMode, that I know of, and whilst you could make use of multi classes to g

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:539 +let RegAltNameIndices = [ABIRegAltName] in { + foreach Index = [0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, This needs to be coordinated with D95588; you both define GPR p

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:219 +def GPR32Pairs : RegisterTuples<[gpr32_pair_lo, gpr32_pair_hi], +[(add X0, X2, X4, X6, + X8, X10, X12, X14, -

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path

jrtc27 added a comment. NB: summary has x86_64-known rather than x86_64-unknown in a couple of places Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110663/new/ https://reviews.llvm.org/D110663 ___ cfe-co

[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

jrtc27 added a comment. In D110663#3029124 , @MaskRay wrote: > In D110663#3029088 , @phosek wrote: > >> The reason I removed this behavior in D101194 >> , aside from extra overhe

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

jrtc27 added a comment. Herald added a subscriber: dang. So I'm running into issues with this patch, specifically the line annotated below. I'm trying to compile our CHERI-LLVM fork as a native pure-capability CHERI[1] binary (which eventually will mean compiling the Morello-LLVM fork of our CH

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

jrtc27 added a comment. Issues from my previous review are still outstanding CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88393/new/ https://reviews.llvm.org/D88393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

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

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:380 ve::getVETargetFeatures(D, Args, Features); +break; } jrtc27 wrote: > Unrelated I committed this as 22215e492338, should disappear if you rebase (or you can just drop

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:69 +def sub_lo : SubRegIndex<32>; +def sub_hi : SubRegIndex<32, 32>; Jim wrote: > jrtc27 wrote: > > Jim wrote: > > > Jim wrote: > > > > luismarques wrote: > > > > > jrtc27 wrot

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

jrtc27 added inline comments. Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:49-50 + /// Parse RISCV ISA info from arch string. + static std::unique_ptr + parseArchString(llvm::Error &Error, StringRef Arch, + bool EnableExperimentalExtension, ---

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

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCV.td:186-188 + [FeatureExtZpsfoperand, +FeatureExtZpn, +FeatureExtZprvsfextra]>; Jim wrote: > jrtc27 wrote: > > These aren't

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

jrtc27 added a comment. Hm, AArch64 handles ExternalSymbolSDNode too, but I don't see how you could ever end up with one... Comment at: llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll:7 + +; Function Attrs: nofree nosync nounwind readnone +define dso_local i8* @constraint_S

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

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

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

jrtc27 added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:122 StringRef CodeModel = getTargetOpts().CodeModel; + unsigned FLEN = ISAInfo.getFLEN(); if (CodeModel == "default") Capitalise these as FLen and XLen everywhere ===

[PATCH] D105396: [RISCV] Remove Zvamo extension for v1.0-rc change

jrtc27 added a comment. This is going to be a lot of churn. Why not just keep Zvamo0p10 as an experimental extension until a re-encoded version appears? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105396/new/ https://reviews.llvm.org/D105396 __

[PATCH] D105396: [RISCV] Remove Zvamo implication for v1.0-rc change

jrtc27 added inline comments. Comment at: clang/test/Preprocessor/riscv-target-features.c:239-246 +// RUN: -o - | FileCheck --check-prefix=CHECK-VAMO-EXT %s +// RUN: %clang -target riscv64-unknown-linux-gnu -menable-experimental-extensions \ +// RUN: -march=rv32izvamo0p10 -x c -

[PATCH] D102582: [RISCV] Report an error when ABI mismatch with target-abi module flag.

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/module-target-abi-tests.ll:5 +; RUN: cat %s > %t.emptyabi +; RUN: echo '!0 = !{i32 1, !"target-abi", !""}' >> %t.emptyabi +; RUN: llc -mtriple=riscv32 < %t.emptyabi -o /dev/null khchen wrote: > khc

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

jrtc27 created this revision. jrtc27 added reviewers: chandlerc, rjmccall, rsmith. Herald added subscribers: ormris, dexonsmith, dang, s.egerton, simoncook, hiraditya, kristof.beyls. jrtc27 requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commit

[PATCH] D102582: [RISCV] Report an error when ABI mismatch with target-abi module flag.

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/module-target-abi-tests.ll:5 +; RUN: cat %s > %t.emptyabi +; RUN: echo '!0 = !{i32 1, !"target-abi", !""}' >> %t.emptyabi +; RUN: llc -mtriple=riscv32 < %t.emptyabi -o /dev/null khchen wrote: > jrt

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

jrtc27 added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:633 + // pass more options in specific target + switch (ToolChain.getArch()) { Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:634-638 + switch (T

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

jrtc27 added a comment. Extract the code out into a shared function, don't duplicate it otherwise we'll get confusing inconsistent defaults. Especially when there are proposals to change the defaults as currently there are some weird interactions where specifying seemingly-redundant flags chang

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

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

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 added a comment. In D98113#2872080 , @jroelofs wrote: >> compiler-rt depends on a libc, typically newlib, which then depends on your >> compiler > > The builtins should only depend on compiler-provided headers, and not on the > rest of libc. Agree

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

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll:27 + +; Function Attrs: nofree nosync nounwind readnone +define dso_local i8* @constraint_S_label() { Not needed Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D109372: [RISCV][RFC] Add Clang support for RISC-V overlay system

jrtc27 added a comment. In D109372#3099947 , @edward-jones wrote: > I reverted some of the previous changes I made so that this patch matches the > spec as currently written - this means it's two attributes again, and the > diagnostic messages have bee

[PATCH] D112890: headers: optionalise some generated resource headers

jrtc27 added a comment. This change seems pretty counter to Clang's ability to spit out IR for anything regardless of what backends you have. The right fix should be to shrink the headers, not mask the problem by changing the principles of how Clang works. Repository: rG LLVM Github Monorepo

[PATCH] D112890: headers: optionalise some generated resource headers

jrtc27 added a comment. And I don't see any AArch64 or ARM maintainers ACKing this. You haven't even tagged any as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 _

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

jrtc27 added inline comments. Comment at: clang/test/Driver/m68k-sub-archs.cpp:1 +// RUN: %clang -### -target m68k-unknown-linux -mcpu=68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s +// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68000 %s 2>&1 | FileCheck --check

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

jrtc27 added inline comments. Comment at: clang/test/Driver/m68k-sub-archs.cpp:1 +// RUN: %clang -### -target m68k-unknown-linux -mcpu=68000 %s 2>&1 | FileCheck --check-prefix=CHECK-MX00 %s +// RUN: %clang -### -target m68k-unknown-linux -mcpu=m68000 %s 2>&1 | FileCheck --check

[PATCH] D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg.

jrtc27 added inline comments. Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:1037 + assert(AddrAlign >= ResultTy->getPrimitiveSizeInBits() / 8 && + "Expected at least natural alignment at this point."); Doesn't this introduce another copy of the sam

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/riscv_vector.td:56 +// +// e: type of "t" as is (identity) +// v: computes a vector type whose element type is "t" for the current LMUL khchen wrote: > jrtc27 wrote: > > Do we really need to

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:116 + D = 1 << 2, + ZFH = 1 << 3 +}; Zfh Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:241 + case 8: +ExpResult = Log2LMUL + 3; +break; ---

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:567 + // Compute type transformers + for (char I : Transformer.take_front(Transformer.size() - 1)) { +switch (I) { craig.topper wrote: > jrtc27 wrote: > > craig.topper wrote: >

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/include/clang/Basic/riscv_vector.td:66 +// element type which is bool +// 0: void type, ignores "t" +// z: size_t, ignores "t" craig.topper wrote: > jrtc27 wrote: > > khchen wrote: > > > jrtc27 wrote: > > >

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

jrtc27 added a comment. In D64146#2567710 , @nand wrote: > CodePtr points into the bytecode emitted by the byte code compiler. In some > instances, pointers to auxiliary data structures are embedded into the byte > code, such as functions or AST nodes wh

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

jrtc27 added a comment. In D64146#2591732 , @jrtc27 wrote: > In D64146#2567710 , @nand wrote: > >> CodePtr points into the bytecode emitted by the byte code compiler. In some >> instances, pointers to auxiliary dat

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

jrtc27 created this revision. jrtc27 added reviewers: nand, jfb, Bigcheese, rsmith, dexonsmith. Herald added subscribers: kristof.beyls, arichardson. jrtc27 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The Clang interpreter's bytecode us

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

jrtc27 updated this revision to Diff 326872. jrtc27 added a comment. Reworked code slightly to make it look nicer after clang-format uglified it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97606/new/ https://reviews.llvm.org/D97606 Files: clan

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

jrtc27 added a comment. In D64146#2591830 , @jrtc27 wrote: > In D64146#2591732 , @jrtc27 wrote: > >> In D64146#2567710 , @nand wrote: >> >>> CodePtr points into the bytecode

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

jrtc27 added a comment. Is there any way I can usefully test this? As far as I can tell there's only a single constexpr test in the tree that uses the new interpreter, and it's pretty trivial? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97606/ne

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:50 +Boolean, +SignInteger, +UnsignedInteger, Signed Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:58-62 + bool IsPointer = false; + // IsConstan

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

jrtc27 added inline comments. Comment at: clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc:1 +#!/usr/bin/env python + Python 3 Comment at: clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 created this revision. jrtc27 added reviewers: jroelofs, abidh, manojgupta, asb, luismarques. Herald added subscribers: frasercrmck, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, niosHD, sabuasal, simoncook, johnrus

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 updated this revision to Diff 328771. jrtc27 added a comment. Added missing file for test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98113/new/ https://reviews.llvm.org/D98113 Files: clang/lib/Driver/ToolChain.cpp clang/lib/Driver/To

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 updated this revision to Diff 328772. jrtc27 added a comment. Dropped unintended change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98113/new/ https://reviews.llvm.org/D98113 Files: clang/lib/Driver/ToolChain.cpp clang/test/Driver/In

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 updated this revision to Diff 328773. jrtc27 added a comment. ... and another accidental change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98113/new/ https://reviews.llvm.org/D98113 Files: clang/lib/Driver/ToolChain.cpp clang/test/D

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 added a comment. One thing that is currently rather ugly about BareMetal is that it ignores the AddArch argument. Once you have a sysroot, the architecture suffix is rather unnecessary, and given compiler-rt uses LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to decide both whether to add a suffix a

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

jrtc27 added a comment. In D98113#2610775 , @abidh wrote: > In D98113#2609027 , @jrtc27 wrote: > >> One thing that is currently rather ugly about BareMetal is that it ignores >> the AddArch argument. Once you have

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

jrtc27 added inline comments. Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:58-62 + bool IsPointer = false; + // IsConstant indices are "int", but have the constant expression. + bool IsImmediate = false; + // const qualifier. + bool IsConstant = false;

[PATCH] D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

jrtc27 accepted this revision. jrtc27 added a comment. This revision is now accepted and ready to land. Any TableGen backend error that's a real message and not an assertion failure with a backtrace is a win in my books :) Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:627

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux

jrtc27 requested changes to this revision. jrtc27 added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/Sparc.cpp:160-164 + // Linux doesn't need these variants, but the BSDs do. + if (getTriple().getOS() != llvm::Trip

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53 +void RISCVInstrInfo::getNoop(MCInst &NopInst) const { + if (STI.getFeatureBits()[RISCV::FeatureStdExtC]) I will forever wonder why TII didn't make it `MCInst getNoop()`...

[PATCH] D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'.

jrtc27 added a comment. This seems like the obvious choice for the constraint, but it would be good to ensure there's consensus with GCC people, especially since their assembly constraints are intimately tied to their instruction patterns (or, really, the assembly constraints just expose those

[PATCH] D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'.

jrtc27 added a comment. In D98616#2626093 , @kito-cheng wrote: > GCC use `vr` for vector register and `vm` for vector mask register. How does that even work? Aren't multi character strings a set of options? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D98616: [RISCV] Add inline asm constraint 'v' in Clang for RISC-V 'V'.

jrtc27 added a comment. In D98616#2626094 , @jrtc27 wrote: > In D98616#2626093 , @kito-cheng > wrote: > >> GCC use `vr` for vector register and `vm` for vector mask register. > > How does that even work? Aren't mul

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3 +;; Test the function attribute "patchable-function-entry". +; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32 +; RUN: llc -mtriple=riscv64 < %s | FileCheck %

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/test/CodeGen/RISCV/patchable-function-entry.ll:1-3 +;; Test the function attribute "patchable-function-entry". +; RUN: llc -mtriple=riscv32 < %s | FileCheck %s --check-prefixes=CHECK,32 +; RUN: llc -mtriple=riscv64 < %s | FileCheck %

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:53 +void RISCVInstrInfo::getNoop(MCInst &NopInst) const { + if (STI.getFeatureBits()[RISCV::FeatureStdExtC]) MaskRay wrote: > jrtc27 wrote: > > I will forever wonder why TII did

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

jrtc27 added a comment. This results in the DWARF type info containing DW_TAG_namespace for C, which breaks DTrace's CTF (as the name implies, it is for C, not C++). This does not seem correct to me. If your mangling for va_list is broken then you should special case that in the CFI mangler as

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

jrtc27 added a comment. Or you can still do it in the frontend, just make sure that the DWARF emitted by the frontend doesn't have the DINamespace wrapper when not compiling C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/ https://re

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

jrtc27 added a comment. I also note that 32-bit Arm uses the same namespacing, and has CFI-icall support (at least, it's enabled, I don't know if it's actually expected to work) but did not have this change made to it (though probably best not to do so until the DWARF issue is resolved) Repos

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

jrtc27 added a comment. In D108479#3110003 , @rjmccall wrote: > `std::addressof(&someFunction)` certainly ought to return a signed pointer > under ptrauth, so if your goal here is to get a completely unadorned symbol > address, I think you do need a dif

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

jrtc27 added a comment. If it's bypassing the descriptors then __builtin_symbol_address is the wrong name (and a bit ambiguous). As far as dlsym is concerned, the symbol is the descriptor, but when you get down to the ELF representation itself that's not always true. For PPC64 ELFv1, the ELF sy

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

jrtc27 added a comment. This also leaks out to users via __builtin_dump_struct (https://godbolt.org/z/vx3rjdPdq), and of course the DWARF having the namespace in it will result in users debugging plain C seeing C++ namespaces. ASTDiagnostic's Desugar happens to special-case va_list so it doesn'

<    1   2   3   4   >